Skip to content

Commit b6d9432

Browse files
os-samclaude
andauthored
feat(messaging): scope the plugin-facing inbox writes to the authenticated caller (#11450)
* feat(messaging): plugin-facing inbox writes scoped to the authenticated caller Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 * test(messaging): pin authenticated-caller scoping for the plugin-facing inbox writes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a58eac3 commit b6d9432

5 files changed

Lines changed: 368 additions & 0 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/service-messaging": minor
3+
---
4+
5+
**Feature:** `MessagingService` gains a plugin-facing inbox write door scoped to the **authenticated caller**`markReadAsCaller(caller, ids)` and `markAllReadAsCaller(caller)` (#10753).
6+
7+
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.
8+
9+
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.
10+
11+
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.
12+
13+
`userId` is read, and nothing that merely resembles one:
14+
15+
- `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.
16+
- `actor` is a service-principal label (`svc:<name>`), not a `sys_user` id.
17+
- `isSystem: true` with no user is refused rather than elevated: the system has no inbox to be the recipient of.
18+
19+
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.
20+
21+
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.
22+
23+
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.
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Authenticated-caller scoping for the plugin-facing inbox write surface
5+
* (ADR-0030 Layer 5).
6+
*
7+
* ## The shape this closes
8+
*
9+
* `MessagingService.markRead(userId, ids)` is the REST door's contract method:
10+
* `INotificationService.markRead?(userId, ids)`, called by
11+
* `packages/runtime/src/domains/notifications.ts`, which binds `userId` to
12+
* `context.executionContext.userId` — the session user the HTTP door already
13+
* authenticated. On that path the parameter is trustworthy because the door
14+
* filled it.
15+
*
16+
* The service is also registered as a kernel service (`registerService('messaging', service)`
17+
* in `messaging-service-plugin.ts`), and the kernel hands every plugin ONE
18+
* shared `PluginContext` whose `getService` carries no caller identity. So for
19+
* an in-process caller that same `userId` is a FREE PARAMETER: any plugin can
20+
* mark ANY user's inbox messages read, and the receipt write lands
21+
* context-lessly on an `engine-owned` object (ADR-0103), so no engine-level
22+
* permission check sees it either. Unconstrained and undeclared, in both
23+
* directions.
24+
*
25+
* The verbs here are the plugin-facing door, and the whole design is that they
26+
* take NO target user. The recipient is DERIVED from the caller's
27+
* {@link ExecutionContext}, so "mark someone else's inbox read" has no
28+
* spelling on this surface — it is unrepresentable rather than merely
29+
* discouraged. A plugin closing out a notification it pushed (the
30+
* emit-in-a-hook → close-out-in-a-later-hook pattern) is acting inside the
31+
* recipient's own request, and this is exactly the identity that request
32+
* carries.
33+
*
34+
* ## Why `userId` ONLY, and nothing that looks like it
35+
*
36+
* `ExecutionContext` carries three principal-shaped fields and only one of
37+
* them is an authorization subject:
38+
*
39+
* - `userId` — "the subject the engine authorizes AS". The only one read here.
40+
* - `attributedUserId` — ATTRIBUTION ONLY. Its own contract states the
41+
* invariant: "nothing in the authorization path reads this", and a context
42+
* carrying only it "authorizes exactly like a context carrying nothing —
43+
* ANONYMOUS, per ADR-0118 D2". Promoting it here would open the second
44+
* adjudication track ADR-0095 D3 closed, and it would do so on a write that
45+
* clears someone's unread badge.
46+
* - `actor` — a service-principal LABEL (`svc:<name>`), not a `sys_user` id.
47+
* There is no inbox to clear for `svc:flow`.
48+
*
49+
* A tolerant `caller.userId ?? caller.attributedUserId ?? caller.actor` chain
50+
* is precisely the consumer-side widening contract-first exists to refuse: it
51+
* would read as working, and it would silently authorize the wrong principal.
52+
* There is no fallback and there must not be one.
53+
*
54+
* `isSystem: true` without a `userId` is refused for the same reason rather
55+
* than elevated: the system has no inbox, so there is no receipt it could be
56+
* the recipient of. A system caller that genuinely means "sweep this user"
57+
* still has `markRead(userId, ids)` — the door where naming a target user is
58+
* the declared contract.
59+
*
60+
* ## What this is, and what it is honestly NOT
61+
*
62+
* It is a DISCIPLINE boundary, not a security boundary, and the difference is
63+
* worth stating where the code is rather than discovering later. An in-process
64+
* plugin already holds the data engine and can write `sys_notification_receipt`
65+
* rows directly; nothing at this layer can stop trusted code that means to.
66+
* What this does is make the CORRECT pattern the only one the plugin-facing
67+
* surface expresses, and make the incorrect one fail loudly at the call site
68+
* instead of silently succeeding — which is the failure mode measured on the
69+
* existing path, where an absent caller returns `{ success: true, readCount: 0 }`.
70+
*/
71+
72+
import type { ExecutionContext } from '@objectstack/spec/kernel';
73+
74+
/**
75+
* The authenticated caller a plugin-facing inbox write acts as — the
76+
* `ExecutionContext` the caller was handed, passed through whole.
77+
*
78+
* Passed WHOLE, deliberately: the measured defect family behind
79+
* `assembleExecutionContext` (#6071, #6206, #6551) is "a field exists on
80+
* `ExecutionContext`, one copy carries it, another silently does not". A
81+
* hand-picked `{ userId }` slice here would be one more such copy.
82+
*/
83+
export type InboxCaller = ExecutionContext;
84+
85+
/**
86+
* The plugin-facing inbox write refusal. Carries the ADR-0112 envelope pair a
87+
* boundary reads — `status` + a registered `code` — so a caller that surfaces
88+
* it over HTTP answers `401 UNAUTHENTICATED` rather than the `500
89+
* INTERNAL_ERROR` a bare `Error` demotes to (`resolveThrownHttpError`,
90+
* `@objectstack/types`).
91+
*
92+
* `UNAUTHENTICATED` rather than `PERMISSION_DENIED`, and the distinction is
93+
* the point of the whole axis: there is no second identity for the caller to
94+
* disagree with, so there is no forbidden-target case to answer 403 for. The
95+
* only thing that can go wrong is having no authenticated principal at all.
96+
*/
97+
export class InboxCallerError extends Error {
98+
/** Registered `StandardErrorCode` — 401's standard member. */
99+
readonly code = 'UNAUTHENTICATED';
100+
/** HTTP answer this refusal declares (ADR-0112). */
101+
readonly status = 401;
102+
103+
constructor(message: string) {
104+
super(message);
105+
this.name = 'InboxCallerError';
106+
}
107+
}
108+
109+
/**
110+
* The recipient a plugin-facing inbox write acts on: the caller's
111+
* authenticated `userId`, or a refusal.
112+
*
113+
* Never returns a guess. Never falls back to `attributedUserId` / `actor` /
114+
* `isSystem` — see this module's header for why each of those is a wrong
115+
* answer rather than a missing feature.
116+
*
117+
* @param caller The caller's execution context (`undefined` is a refusal).
118+
* @param verb The plugin-facing method name, so the refusal names the call.
119+
* @throws {InboxCallerError} when no authenticated user can be resolved.
120+
*/
121+
export function resolveInboxRecipient(caller: InboxCaller | undefined, verb: string): string {
122+
const userId = typeof caller?.userId === 'string' ? caller.userId.trim() : '';
123+
if (userId) return userId;
124+
125+
// Name what WAS present, so the caller can tell "I passed nothing" from "I
126+
// passed a context whose principal is not an authorization subject" — the
127+
// second is the mistake that otherwise reads as a platform bug.
128+
const carried: string[] = [];
129+
if (caller?.attributedUserId) carried.push('attributedUserId');
130+
if (caller?.actor) carried.push('actor');
131+
if (caller?.isSystem) carried.push('isSystem');
132+
const detail = carried.length
133+
? ` The context carries ${carried.join(' + ')}, which is attribution/privilege, never an authorization subject`
134+
: '';
135+
136+
throw new InboxCallerError(
137+
`messaging: ${verb} requires an authenticated caller — no 'userId' on the execution context.${detail}. `
138+
+ `This surface acts only on the CALLER'S OWN inbox and takes no target user; `
139+
+ `a system/background sweep that must name one uses markRead(userId, ids).`,
140+
);
141+
}

packages/services/service-messaging/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ export type {
5252
QuietHours,
5353
} from './preference-resolver.js';
5454

55+
// Plugin-facing inbox writes scoped to the authenticated caller (ADR-0030 L5)
56+
export { InboxCallerError, resolveInboxRecipient } from './inbox-caller.js';
57+
export type { InboxCaller } from './inbox-caller.js';
58+
5559
// Channel seam
5660
export { createInboxChannel, INBOX_OBJECT, RECEIPT_OBJECT } from './inbox-channel.js';
5761
export type { InboxChannelOptions } from './inbox-channel.js';

packages/services/service-messaging/src/messaging-service.test.ts

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

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

@@ -1070,3 +1071,127 @@ describe('[#6436] markAllRead — sweeps the whole inbox, not one 200-row window
10701071
expect(await svc.markAllRead('')).toEqual({ success: true, readCount: 0 });
10711072
});
10721073
});
1074+
1075+
/**
1076+
* [#10753] The plugin-facing inbox write door.
1077+
*
1078+
* The measured BEFORE, and it is this card's real severity: the messaging
1079+
* service is registered as a kernel service and the kernel hands every plugin
1080+
* ONE shared `PluginContext` whose `getService` carries no caller identity, so
1081+
* `markRead(userId, ids)`'s first parameter is a free string for an in-process
1082+
* caller — any plugin could mark ANY user's inbox messages read, unconstrained
1083+
* and undeclared, with the receipt landing context-lessly on an `engine-owned`
1084+
* object so no engine permission check saw it either.
1085+
*
1086+
* `markReadAsCaller` / `markAllReadAsCaller` take no target user at all. These
1087+
* pin that the recipient comes from the caller's authenticated `userId` and
1088+
* from NOTHING that merely resembles one.
1089+
*/
1090+
describe('MessagingService — plugin-facing inbox writes scoped to the authenticated caller (#10753)', () => {
1091+
const logger = silentLogger();
1092+
1093+
/** u1 and u2 each hold one unread message, so cross-user reach is visible. */
1094+
function twoUserInbox() {
1095+
return inboxEngine({
1096+
inbox: [
1097+
{ id: 'm1', user_id: 'u1', notification_id: 'n1', title: 'Approve me', created_at: '1' },
1098+
{ id: 'm2', user_id: 'u2', notification_id: 'n2', title: 'Approve me too', created_at: '2' },
1099+
],
1100+
receipts: [
1101+
{ id: 'r1', notification_id: 'n1', user_id: 'u1', channel: 'inbox', state: 'delivered' },
1102+
{ id: 'r2', notification_id: 'n2', user_id: 'u2', channel: 'inbox', state: 'delivered' },
1103+
],
1104+
});
1105+
}
1106+
1107+
it("marks the caller's OWN message read", async () => {
1108+
const engine = twoUserInbox();
1109+
const svc = new MessagingService({ logger, getData: () => engine });
1110+
1111+
expect(await svc.markReadAsCaller({ userId: 'u1' }, ['n1'])).toEqual({ success: true, readCount: 1 });
1112+
expect((await svc.listInbox('u1')).unreadCount).toBe(0);
1113+
});
1114+
1115+
it("cannot reach another user's read-state even when handed their notification id", async () => {
1116+
// The id is not secret — `sys_notification` publishes get/list and every
1117+
// recipient's own `listInbox` hands them out — so "holding the id" was
1118+
// never a capability. What makes u2 unreachable is that the receipt is
1119+
// keyed `(notification_id, user_id, channel)` and the user half comes
1120+
// from the CALLER, which this surface does not let you name.
1121+
const engine = twoUserInbox();
1122+
const svc = new MessagingService({ logger, getData: () => engine });
1123+
1124+
await svc.markReadAsCaller({ userId: 'u1' }, ['n2']);
1125+
1126+
expect((await svc.listInbox('u2')).unreadCount).toBe(1);
1127+
expect(engine.store.sys_notification_receipt.find((r: any) => r.user_id === 'u2').state).toBe('delivered');
1128+
});
1129+
1130+
it("sweeps only the caller's own inbox on markAllReadAsCaller", async () => {
1131+
const engine = twoUserInbox();
1132+
const svc = new MessagingService({ logger, getData: () => engine });
1133+
1134+
expect(await svc.markAllReadAsCaller({ userId: 'u1' })).toEqual({ success: true, readCount: 1 });
1135+
expect((await svc.listInbox('u1')).unreadCount).toBe(0);
1136+
expect((await svc.listInbox('u2')).unreadCount).toBe(1);
1137+
});
1138+
1139+
describe('refusals — the ADR-0112 envelope, not a silent success', () => {
1140+
const svc = () => new MessagingService({ logger, getData: () => twoUserInbox() });
1141+
1142+
/** Assert the declared envelope pair a boundary reads: `status` + `code`. */
1143+
async function expectRefusal(run: () => Promise<unknown>): Promise<InboxCallerError> {
1144+
const err = await run().then(
1145+
() => { throw new Error('expected InboxCallerError, but the call resolved'); },
1146+
(e: unknown) => e as InboxCallerError,
1147+
);
1148+
expect(err).toBeInstanceOf(InboxCallerError);
1149+
expect(err.code).toBe('UNAUTHENTICATED');
1150+
expect(err.status).toBe(401);
1151+
return err;
1152+
}
1153+
1154+
it('refuses an absent context', async () => {
1155+
await expectRefusal(() => svc().markReadAsCaller(undefined, ['n1']));
1156+
await expectRefusal(() => svc().markAllReadAsCaller(undefined));
1157+
});
1158+
1159+
it('refuses a context with no userId, and a blank one', async () => {
1160+
await expectRefusal(() => svc().markReadAsCaller({}, ['n1']));
1161+
await expectRefusal(() => svc().markReadAsCaller({ userId: ' ' }, ['n1']));
1162+
});
1163+
1164+
it('refuses a context carrying only attributedUserId — attribution never becomes authorization', async () => {
1165+
// `attributedUserId` is the real human behind a write whose
1166+
// authorization subject is the SYSTEM (#4586). Its own contract
1167+
// states the invariant — "nothing in the authorization path reads
1168+
// this", and a context carrying only it authorizes ANONYMOUS
1169+
// (ADR-0118 D2). A `userId ?? attributedUserId` fallback here would
1170+
// read as working and clear the wrong person's badge.
1171+
const engine = twoUserInbox();
1172+
const service = new MessagingService({ logger, getData: () => engine });
1173+
1174+
const err = await expectRefusal(() => service.markReadAsCaller({ attributedUserId: 'u1' }, ['n1']));
1175+
expect(err.message).toContain('attributedUserId');
1176+
1177+
// The refusal is the point, but so is this: u1's message is still unread.
1178+
expect((await service.listInbox('u1')).unreadCount).toBe(1);
1179+
});
1180+
1181+
it('refuses a service-principal label and a system context — neither owns an inbox', async () => {
1182+
await expectRefusal(() => svc().markReadAsCaller({ actor: 'svc:flow:nightly' }, ['n1']));
1183+
await expectRefusal(() => svc().markAllReadAsCaller({ isSystem: true }));
1184+
});
1185+
1186+
it('refuses BEFORE the empty-ids and no-data-engine short-circuits', async () => {
1187+
// Both of those return `{ success: true, readCount: 0 }`. Reaching
1188+
// one with no authenticated caller would report success for a write
1189+
// that was never authorized — the silent-success shape this door
1190+
// exists to replace, and the reason the order is pinned rather than
1191+
// left to reading.
1192+
await expectRefusal(() => svc().markReadAsCaller(undefined, []));
1193+
await expectRefusal(() => new MessagingService({ logger }).markReadAsCaller(undefined, ['n1']));
1194+
await expectRefusal(() => new MessagingService({ logger }).markAllReadAsCaller({}));
1195+
});
1196+
});
1197+
});

0 commit comments

Comments
 (0)