Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/messaging-inbox-authenticated-caller-scope.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
---
"@objectstack/service-messaging": minor
---

**Feature:** `MessagingService` gains a plugin-facing inbox write door scoped to the **authenticated caller** — `markReadAsCaller(caller, ids)` and `markAllReadAsCaller(caller)` (#10753).

A plugin that pushes an "…awaiting your approval" message through `emit()` had no legitimate way to close it out again once the work was done, so the Console bell's unread badge stayed lit through a full page reload until the user hit "mark all read". The reporting project carries 30+ business hooks in that shape.

What it was reaching for instead is the shape this closes. `markRead(userId, ids)` is the REST door's contract method (`INotificationService.markRead?`), and on that path its `userId` is trustworthy because `runtime/src/domains/notifications.ts` binds it to an already-authenticated session and answers 401 when there is none. But the service is also a kernel service, and the kernel hands every plugin ONE shared `PluginContext` whose `getService` carries no caller identity — so for an in-process caller that same parameter is a free string. **Any plugin could mark any user's inbox messages read**, and the receipt lands context-lessly on an `engine-owned` object (ADR-0103), so no engine permission check saw it either. This release is therefore both an API widening and the first tightening of in-process power on that path.

The new pair takes **no target user at all**. The recipient is derived from the caller's `ExecutionContext.userId`, so "mark someone else's inbox read" has no spelling on this surface — it is unrepresentable rather than discouraged. That fits the case it was asked for exactly: the approver who clears a request *is* the recipient whose badge is stuck.

`userId` is read, and nothing that merely resembles one:

- `attributedUserId` is **attribution only** — its own contract states that nothing in the authorization path reads it, and a context carrying only it authorizes as anonymous (ADR-0118 D2). A `userId ?? attributedUserId` fallback would read as working and clear the wrong person's badge.
- `actor` is a service-principal label (`svc:<name>`), not a `sys_user` id.
- `isSystem: true` with no user is refused rather than elevated: the system has no inbox to be the recipient of.

Each refusal throws `InboxCallerError` carrying the ADR-0112 envelope pair a boundary reads — `status: 401` and the registered `code: 'UNAUTHENTICATED'` — and the refusal is evaluated **before** the empty-`ids` and no-data-engine short-circuits, which return `{ success: true, readCount: 0 }`. Reaching one of those with no authenticated caller would report success for a write that was never authorized, which is the silent-success shape this door exists to replace.

Honest about what it is: a **discipline** boundary, not a security boundary. An in-process plugin already holds the data engine and can write `sys_notification_receipt` directly; nothing at this layer stops trusted code that means to. What changes is that the correct pattern is the only one the plugin-facing surface expresses, and the incorrect one now fails loudly at the call site.

Nothing existing changes behaviour: `markRead` / `markAllRead` / `listInbox` keep their signatures (they are the published `INotificationService` contract the REST door needs), and no schema, column or object declaration moves.
141 changes: 141 additions & 0 deletions packages/services/service-messaging/src/inbox-caller.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Authenticated-caller scoping for the plugin-facing inbox write surface
* (ADR-0030 Layer 5).
*
* ## The shape this closes
*
* `MessagingService.markRead(userId, ids)` is the REST door's contract method:
* `INotificationService.markRead?(userId, ids)`, called by
* `packages/runtime/src/domains/notifications.ts`, which binds `userId` to
* `context.executionContext.userId` — the session user the HTTP door already
* authenticated. On that path the parameter is trustworthy because the door
* filled it.
*
* The service is also registered as a kernel service (`registerService('messaging', service)`
* in `messaging-service-plugin.ts`), and the kernel hands every plugin ONE
* shared `PluginContext` whose `getService` carries no caller identity. So for
* an in-process caller that same `userId` is a FREE PARAMETER: any plugin can
* mark ANY user's inbox messages read, and the receipt write lands
* context-lessly on an `engine-owned` object (ADR-0103), so no engine-level
* permission check sees it either. Unconstrained and undeclared, in both
* directions.
*
* The verbs here are the plugin-facing door, and the whole design is that they
* take NO target user. The recipient is DERIVED from the caller's
* {@link ExecutionContext}, so "mark someone else's inbox read" has no
* spelling on this surface — it is unrepresentable rather than merely
* discouraged. A plugin closing out a notification it pushed (the
* emit-in-a-hook → close-out-in-a-later-hook pattern) is acting inside the
* recipient's own request, and this is exactly the identity that request
* carries.
*
* ## Why `userId` ONLY, and nothing that looks like it
*
* `ExecutionContext` carries three principal-shaped fields and only one of
* them is an authorization subject:
*
* - `userId` — "the subject the engine authorizes AS". The only one read here.
* - `attributedUserId` — ATTRIBUTION ONLY. Its own contract states the
* invariant: "nothing in the authorization path reads this", and a context
* carrying only it "authorizes exactly like a context carrying nothing —
* ANONYMOUS, per ADR-0118 D2". Promoting it here would open the second
* adjudication track ADR-0095 D3 closed, and it would do so on a write that
* clears someone's unread badge.
* - `actor` — a service-principal LABEL (`svc:<name>`), not a `sys_user` id.
* There is no inbox to clear for `svc:flow`.
*
* A tolerant `caller.userId ?? caller.attributedUserId ?? caller.actor` chain
* is precisely the consumer-side widening contract-first exists to refuse: it
* would read as working, and it would silently authorize the wrong principal.
* There is no fallback and there must not be one.
*
* `isSystem: true` without a `userId` is refused for the same reason rather
* than elevated: the system has no inbox, so there is no receipt it could be
* the recipient of. A system caller that genuinely means "sweep this user"
* still has `markRead(userId, ids)` — the door where naming a target user is
* the declared contract.
*
* ## What this is, and what it is honestly NOT
*
* It is a DISCIPLINE boundary, not a security boundary, and the difference is
* worth stating where the code is rather than discovering later. An in-process
* plugin already holds the data engine and can write `sys_notification_receipt`
* rows directly; nothing at this layer can stop trusted code that means to.
* What this does is make the CORRECT pattern the only one the plugin-facing
* surface expresses, and make the incorrect one fail loudly at the call site
* instead of silently succeeding — which is the failure mode measured on the
* existing path, where an absent caller returns `{ success: true, readCount: 0 }`.
*/

import type { ExecutionContext } from '@objectstack/spec/kernel';

/**
* The authenticated caller a plugin-facing inbox write acts as — the
* `ExecutionContext` the caller was handed, passed through whole.
*
* Passed WHOLE, deliberately: the measured defect family behind
* `assembleExecutionContext` (#6071, #6206, #6551) is "a field exists on
* `ExecutionContext`, one copy carries it, another silently does not". A
* hand-picked `{ userId }` slice here would be one more such copy.
*/
export type InboxCaller = ExecutionContext;

/**
* The plugin-facing inbox write refusal. Carries the ADR-0112 envelope pair a
* boundary reads — `status` + a registered `code` — so a caller that surfaces
* it over HTTP answers `401 UNAUTHENTICATED` rather than the `500
* INTERNAL_ERROR` a bare `Error` demotes to (`resolveThrownHttpError`,
* `@objectstack/types`).
*
* `UNAUTHENTICATED` rather than `PERMISSION_DENIED`, and the distinction is
* the point of the whole axis: there is no second identity for the caller to
* disagree with, so there is no forbidden-target case to answer 403 for. The
* only thing that can go wrong is having no authenticated principal at all.
*/
export class InboxCallerError extends Error {
/** Registered `StandardErrorCode` — 401's standard member. */
readonly code = 'UNAUTHENTICATED';
/** HTTP answer this refusal declares (ADR-0112). */
readonly status = 401;

constructor(message: string) {
super(message);
this.name = 'InboxCallerError';
}
}

/**
* The recipient a plugin-facing inbox write acts on: the caller's
* authenticated `userId`, or a refusal.
*
* Never returns a guess. Never falls back to `attributedUserId` / `actor` /
* `isSystem` — see this module's header for why each of those is a wrong
* answer rather than a missing feature.
*
* @param caller The caller's execution context (`undefined` is a refusal).
* @param verb The plugin-facing method name, so the refusal names the call.
* @throws {InboxCallerError} when no authenticated user can be resolved.
*/
export function resolveInboxRecipient(caller: InboxCaller | undefined, verb: string): string {
const userId = typeof caller?.userId === 'string' ? caller.userId.trim() : '';
if (userId) return userId;

// Name what WAS present, so the caller can tell "I passed nothing" from "I
// passed a context whose principal is not an authorization subject" — the
// second is the mistake that otherwise reads as a platform bug.
const carried: string[] = [];
if (caller?.attributedUserId) carried.push('attributedUserId');
if (caller?.actor) carried.push('actor');
if (caller?.isSystem) carried.push('isSystem');
const detail = carried.length
? ` The context carries ${carried.join(' + ')}, which is attribution/privilege, never an authorization subject`
: '';

throw new InboxCallerError(
`messaging: ${verb} requires an authenticated caller — no 'userId' on the execution context.${detail}. `
+ `This surface acts only on the CALLER'S OWN inbox and takes no target user; `
+ `a system/background sweep that must name one uses markRead(userId, ids).`,
);
}
4 changes: 4 additions & 0 deletions packages/services/service-messaging/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -52,6 +52,10 @@ export type {
QuietHours,
} from './preference-resolver.js';

// Plugin-facing inbox writes scoped to the authenticated caller (ADR-0030 L5)
export { InboxCallerError, resolveInboxRecipient } from './inbox-caller.js';
export type { InboxCaller } from './inbox-caller.js';

// Channel seam
export { createInboxChannel, INBOX_OBJECT, RECEIPT_OBJECT } from './inbox-channel.js';
export type { InboxChannelOptions } from './inbox-channel.js';
Expand Down
125 changes: 125 additions & 0 deletions packages/services/service-messaging/src/messaging-service.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@

import { describe, it, expect, beforeEach } from 'vitest';
import { MessagingService } from './messaging-service.js';
import { InboxCallerError } from './inbox-caller.js';
import { MemoryNotificationOutbox } from './memory-outbox.js';
import type { Delivery, MessagingChannel, SendResult } from './channel.js';

Expand DownExpand Up@@ -1070,3 +1071,127 @@ describe('[#6436] markAllRead — sweeps the whole inbox, not one 200-row window
expect(await svc.markAllRead('')).toEqual({ success: true, readCount: 0 });
});
});

/**
* [#10753] The plugin-facing inbox write door.
*
* The measured BEFORE, and it is this card's real severity: the messaging
* service is registered as a kernel service and the kernel hands every plugin
* ONE shared `PluginContext` whose `getService` carries no caller identity, so
* `markRead(userId, ids)`'s first parameter is a free string for an in-process
* caller — any plugin could mark ANY user's inbox messages read, unconstrained
* and undeclared, with the receipt landing context-lessly on an `engine-owned`
* object so no engine permission check saw it either.
*
* `markReadAsCaller` / `markAllReadAsCaller` take no target user at all. These
* pin that the recipient comes from the caller's authenticated `userId` and
* from NOTHING that merely resembles one.
*/
describe('MessagingService — plugin-facing inbox writes scoped to the authenticated caller (#10753)', () => {
const logger = silentLogger();

/** u1 and u2 each hold one unread message, so cross-user reach is visible. */
function twoUserInbox() {
return inboxEngine({
inbox: [
{ id: 'm1', user_id: 'u1', notification_id: 'n1', title: 'Approve me', created_at: '1' },
{ id: 'm2', user_id: 'u2', notification_id: 'n2', title: 'Approve me too', created_at: '2' },
],
receipts: [
{ id: 'r1', notification_id: 'n1', user_id: 'u1', channel: 'inbox', state: 'delivered' },
{ id: 'r2', notification_id: 'n2', user_id: 'u2', channel: 'inbox', state: 'delivered' },
],
});
}

it("marks the caller's OWN message read", async () => {
const engine = twoUserInbox();
const svc = new MessagingService({ logger, getData: () => engine });

expect(await svc.markReadAsCaller({ userId: 'u1' }, ['n1'])).toEqual({ success: true, readCount: 1 });
expect((await svc.listInbox('u1')).unreadCount).toBe(0);
});

it("cannot reach another user's read-state even when handed their notification id", async () => {
// The id is not secret — `sys_notification` publishes get/list and every
// recipient's own `listInbox` hands them out — so "holding the id" was
// never a capability. What makes u2 unreachable is that the receipt is
// keyed `(notification_id, user_id, channel)` and the user half comes
// from the CALLER, which this surface does not let you name.
const engine = twoUserInbox();
const svc = new MessagingService({ logger, getData: () => engine });

await svc.markReadAsCaller({ userId: 'u1' }, ['n2']);

expect((await svc.listInbox('u2')).unreadCount).toBe(1);
expect(engine.store.sys_notification_receipt.find((r: any) => r.user_id === 'u2').state).toBe('delivered');
});

it("sweeps only the caller's own inbox on markAllReadAsCaller", async () => {
const engine = twoUserInbox();
const svc = new MessagingService({ logger, getData: () => engine });

expect(await svc.markAllReadAsCaller({ userId: 'u1' })).toEqual({ success: true, readCount: 1 });
expect((await svc.listInbox('u1')).unreadCount).toBe(0);
expect((await svc.listInbox('u2')).unreadCount).toBe(1);
});

describe('refusals — the ADR-0112 envelope, not a silent success', () => {
const svc = () => new MessagingService({ logger, getData: () => twoUserInbox() });

/** Assert the declared envelope pair a boundary reads: `status` + `code`. */
async function expectRefusal(run: () => Promise<unknown>): Promise<InboxCallerError> {
const err = await run().then(
() => { throw new Error('expected InboxCallerError, but the call resolved'); },
(e: unknown) => e as InboxCallerError,
);
expect(err).toBeInstanceOf(InboxCallerError);
expect(err.code).toBe('UNAUTHENTICATED');
expect(err.status).toBe(401);
return err;
}

it('refuses an absent context', async () => {
await expectRefusal(() => svc().markReadAsCaller(undefined, ['n1']));
await expectRefusal(() => svc().markAllReadAsCaller(undefined));
});

it('refuses a context with no userId, and a blank one', async () => {
await expectRefusal(() => svc().markReadAsCaller({}, ['n1']));
await expectRefusal(() => svc().markReadAsCaller({ userId: ' ' }, ['n1']));
});

it('refuses a context carrying only attributedUserId — attribution never becomes authorization', async () => {
// `attributedUserId` is the real human behind a write whose
// authorization subject is the SYSTEM (#4586). Its own contract
// states the invariant — "nothing in the authorization path reads
// this", and a context carrying only it authorizes ANONYMOUS
// (ADR-0118 D2). A `userId ?? attributedUserId` fallback here would
// read as working and clear the wrong person's badge.
const engine = twoUserInbox();
const service = new MessagingService({ logger, getData: () => engine });

const err = await expectRefusal(() => service.markReadAsCaller({ attributedUserId: 'u1' }, ['n1']));
expect(err.message).toContain('attributedUserId');

// The refusal is the point, but so is this: u1's message is still unread.
expect((await service.listInbox('u1')).unreadCount).toBe(1);
});

it('refuses a service-principal label and a system context — neither owns an inbox', async () => {
await expectRefusal(() => svc().markReadAsCaller({ actor: 'svc:flow:nightly' }, ['n1']));
await expectRefusal(() => svc().markAllReadAsCaller({ isSystem: true }));
});

it('refuses BEFORE the empty-ids and no-data-engine short-circuits', async () => {
// Both of those return `{ success: true, readCount: 0 }`. Reaching
// one with no authenticated caller would report success for a write
// that was never authorized — the silent-success shape this door
// exists to replace, and the reason the order is pinned rather than
// left to reading.
await expectRefusal(() => svc().markReadAsCaller(undefined, []));
await expectRefusal(() => new MessagingService({ logger }).markReadAsCaller(undefined, ['n1']));
await expectRefusal(() => new MessagingService({ logger }).markAllReadAsCaller({}));
});
});
});
Loading
Loading