From 67ab342bd62689564033670410b54da85761ed00 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 19:12:11 +0000 Subject: [PATCH 1/2] feat(messaging): plugin-facing inbox writes scoped to the authenticated caller Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .../service-messaging/src/inbox-caller.ts | 141 ++++++++++++++++++ .../services/service-messaging/src/index.ts | 4 + .../src/messaging-service.ts | 75 ++++++++++ 3 files changed, 220 insertions(+) create mode 100644 packages/services/service-messaging/src/inbox-caller.ts diff --git a/packages/services/service-messaging/src/inbox-caller.ts b/packages/services/service-messaging/src/inbox-caller.ts new file mode 100644 index 0000000000..5988335501 --- /dev/null +++ b/packages/services/service-messaging/src/inbox-caller.ts @@ -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:`), 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).`, + ); +} diff --git a/packages/services/service-messaging/src/index.ts b/packages/services/service-messaging/src/index.ts index c036cbbe57..85cea966ab 100644 --- a/packages/services/service-messaging/src/index.ts +++ b/packages/services/service-messaging/src/index.ts @@ -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'; diff --git a/packages/services/service-messaging/src/messaging-service.ts b/packages/services/service-messaging/src/messaging-service.ts index f542a73b8c..2d41b23898 100644 --- a/packages/services/service-messaging/src/messaging-service.ts +++ b/packages/services/service-messaging/src/messaging-service.ts @@ -19,6 +19,7 @@ import type { RedeliverOptions, } from './http-outbox.js'; import { INBOX_OBJECT, RECEIPT_OBJECT } from './inbox-channel.js'; +import { type InboxCaller, resolveInboxRecipient } from './inbox-caller.js'; /** The L2 event object every `emit()` writes one row to (ADR-0030). */ export const NOTIFICATION_EVENT_OBJECT = 'sys_notification'; @@ -493,6 +494,20 @@ export class MessagingService { * `(notification_id, user_id, channel:'inbox')`); inserts one only when * absent. `ids` are notification (event) ids. Returns the REST contract * shape (`MarkNotificationsReadResponseSchema`): `{ success, readCount }`. + * + * **This is the REST door's method** — `INotificationService.markRead?`, + * called by `runtime/src/domains/notifications.ts`, which binds `userId` + * to `context.executionContext.userId` after answering 401 for a request + * that has none. The target user is a declared parameter here *because* + * that door has already authenticated it. + * + * An IN-PROCESS caller has no such door in front of it, and for that caller + * the parameter is simply a free string — the "any plugin can mark any + * user's messages read" shape. Plugins use {@link markReadAsCaller}, which + * derives the recipient from the caller's execution context and has no + * target-user parameter to get wrong. This signature stays as it is: it is + * the published `INotificationService` contract, and the REST door needs + * exactly it. */ async markRead(userId: string, ids: readonly string[]): Promise<{ success: boolean; readCount: number }> { const data = this.ctx.getData?.(); @@ -560,6 +575,66 @@ export class MessagingService { return this.markRead(userId, await this.unreadNotificationIds(data, userId)); } + /* ------------------------------------------------------------------ */ + /* Plugin-facing inbox writes — scoped to the AUTHENTICATED caller */ + /* */ + /* The pair above is the REST door's contract surface */ + /* (`INotificationService.markRead?/markAllRead?`), whose `userId` is */ + /* filled by the runtime from an already-authenticated session. The */ + /* pair below is the door for IN-PROCESS callers, and it takes no */ + /* target user at all: the recipient is derived from the caller's */ + /* execution context, so acting on someone else's inbox has no */ + /* spelling here. See `inbox-caller.ts` for the full rationale, */ + /* including why `attributedUserId` / `actor` / `isSystem` are refused */ + /* rather than accepted as fallbacks. */ + /* ------------------------------------------------------------------ */ + + /** + * Mark notifications read **in the calling user's own inbox** — the + * plugin-facing counterpart to {@link markRead}. + * + * This is what a plugin closing out a notification it pushed should call: + * `emit()` returns the `notificationId`, the business record keeps it, and + * the hook that completes the work hands that id back here together with + * the context it is running under. The approval case the surface was asked + * for is exactly this shape — the approver who clears the request IS the + * recipient whose badge is stuck, so the authenticated caller and the + * receipt's owner are the same principal. + * + * Refuses (`InboxCallerError`, 401 `UNAUTHENTICATED`) when the context + * names no authenticated user. **The refusal is evaluated FIRST**, before + * the empty-`ids` and no-data-engine short-circuits {@link markRead} + * applies: those return a `{ success: true, readCount: 0 }` envelope, and + * letting an unauthenticated call reach one would report success for a + * write that was never authorized to happen — the silent-success shape + * this door exists to replace. + * + * @param caller The caller's execution context. The recipient is read from + * its `userId` and from nothing else. + * @param ids Notification (event) ids, as returned by `emit()`. + */ + async markReadAsCaller( + caller: InboxCaller | undefined, + ids: readonly string[], + ): Promise<{ success: boolean; readCount: number }> { + return this.markRead(resolveInboxRecipient(caller, 'markReadAsCaller'), ids); + } + + /** + * Mark the calling user's WHOLE inbox read — the plugin-facing counterpart + * to {@link markAllRead}, on the same authenticated-caller axis as + * {@link markReadAsCaller} (and with the same refusal, evaluated first). + * + * Deliberately still the caller's own inbox and not a sweep primitive: a + * background job that must clear someone else's inbox is naming a target + * user, which is {@link markAllRead}'s declared contract, not this one's. + */ + async markAllReadAsCaller( + caller: InboxCaller | undefined, + ): Promise<{ success: boolean; readCount: number }> { + return this.markAllRead(resolveInboxRecipient(caller, 'markAllReadAsCaller')); + } + /** * Every notification id in the user's inbox that has no `read`-class * receipt yet — the set `markAllRead` must flip, deduplicated so a From 1ce71df0e5d7783bb4d7b34ff9eb4e904c22c9f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 19:33:19 +0000 Subject: [PATCH 2/2] test(messaging): pin authenticated-caller scoping for the plugin-facing inbox writes Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- ...saging-inbox-authenticated-caller-scope.md | 23 ++++ .../src/messaging-service.test.ts | 125 ++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 .changeset/messaging-inbox-authenticated-caller-scope.md diff --git a/.changeset/messaging-inbox-authenticated-caller-scope.md b/.changeset/messaging-inbox-authenticated-caller-scope.md new file mode 100644 index 0000000000..35f40ca4f3 --- /dev/null +++ b/.changeset/messaging-inbox-authenticated-caller-scope.md @@ -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:`), 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. diff --git a/packages/services/service-messaging/src/messaging-service.test.ts b/packages/services/service-messaging/src/messaging-service.test.ts index fc81813456..10622c5efd 100644 --- a/packages/services/service-messaging/src/messaging-service.test.ts +++ b/packages/services/service-messaging/src/messaging-service.test.ts @@ -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'; @@ -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): Promise { + 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({})); + }); + }); +});