Skip to content

Commit a7a48b7

Browse files
fix(service-storage, service-messaging): the storage store's by-id methods and the HTTP outbox's redeliver take the explicit system opt-in (#22045)
Part of #21908 Clause-②: no Phase two, stage 2a of the principal-less hand-off closure (ADR-0096 E1 / D5), per the maintainer's ruling on the card (letter A). Seven more producers of a principal-less, non-system data context now take the explicit system opt-in (`isSystem: true`) inside their owning service. The deny itself lands last and is not in this PR, so #21908 stays open. ⛔ No `plugin-security`, `packages/spec`, `metadata-protocol` or `docs/adr` file is in the diff, there is no new elevation API, and no door's authorization changed. ## What moved Positions are at the base `9a0401fdd3`. "Gate fired" means one of the six gates the security middleware still runs before its hand-off (package-managed, system-row, curated-capability, audience-anchor, ADR-0103 engine-owned, ADR-0090 D12 delegated-admin) fires on the producer's calls. Counts are principal-less calls at the hand-off, before and after the change; every "after" call arrived as an `isSystem` call. | Position | Function | Doors | Door authorizes (where) | Gate fired | Moved | Evidence (harness · dogfood subset) | | :-- | :-- | :-- | :-- | :-- | :-- | :-- | | `service-storage` `metadata-store.ts:373` | `StorageMetadataStore.getFile` | upload commit (`storage-routes.ts:453`), both download doors (`:799`, `:846`), and the chunked completion through `updateFile` | yes. Commit and chunked completion: session authentication (`requireUploadSession`, `:458`, `:684`) before the read. Downloads: `authorizeDownload` (`:808`, `:855`) after the read and before anything is disclosed, because its verdict reads the row | no | yes | 5 → 0 · 111 → 0 | | `metadata-store.ts:399` | `StorageMetadataStore.updateFile` | upload commit (`:472`), chunked completion (`:720`) | yes. Session authentication before the write; the session's organization scopes the write | no | yes | 2 → 0 · 46 → 0 | | `metadata-store.ts:434` | `StorageMetadataStore.deleteFile` | none. No production caller in this repository; the class is exported | not applicable, no door | no | yes | 1 → 0 (direct call) · not reached | | `metadata-store.ts:502` | `StorageMetadataStore.getSession` | chunk upload (`:593`), chunked completion (`:681`), progress (`:751`) | yes. Session authentication before the read; at the chunk door the resume-token check (`:617`) follows the read, because it reads the row's token, and precedes every write | no | yes | 8 → 0 · 2 → 0 | | `metadata-store.ts:529` | `StorageMetadataStore.updateSession` | chunk upload (`:658`, expiry via `:623`), chunked completion (`:705`, `:725`, expiry via `:694`, failure stamp via `:732`), progress (expiry via `:770`) | yes. Session authentication before every write; the chunk door also checks the resume token first | no | yes | 4 → 0 · 1 → 0 | | `metadata-store.ts:562` | `StorageMetadataStore.deleteSession` | none. No production caller in this repository | not applicable, no door | no | yes | 1 → 0 (direct call) · not reached | | `service-messaging` `sql-http-outbox.ts:468` | `SqlHttpOutbox.redeliver` (two reads, one reset write) | `POST /api/v1/webhooks/redeliver` (`plugin-webhooks` `webhook-outbox-plugin.ts:397`) through `MessagingService.redeliverHttp` | yes. An authenticated session or `401` before the call; the session's active organization is the tenant; the producer's veto runs before the write | no | yes | 3 → 0 · not reached | Read scope, asked for by the claim: `getFile` and `getSession` read by id with no organization scope before the move (no context at all, so no `tenantId` reached the driver, and the middleware handed the call through before any row or tenant filter) and after it (the middleware short-circuits before the same filters, and still no `tenantId` reaches the driver). ## What the opt-in would also have changed, and what holds it The opt-in skips more than the six gates. Measured on the real engine, per call: - **The update-side `readonly` strip.** `updateFile` and `updateSession` used to send the whole row read back, merged with the patch. Under the strip, the engine took `organization_id`, `created_at`, `updated_at`, `created_by` and `updated_by` out of that row on every call. Under the opt-in the strip does not run, so the full row, `organization_id` included and read without tenant scope, would have been written back. **Fix inside the store:** the two updates now send the caller's patch alone (`changedColumns`). Measured on SQLite through the real engine: on the base the strip took those five columns; on the change it takes none, and the stored rows are equal on every non-timestamp column, with `updated_at` still stamped by the platform. - **The tenant-audit fill-in.** For an `isSystem` write, `ObjectQL.buildDriverOptions` fills in `bypassTenantAudit: true` when the object is outside the platform tenancy inventory. `sys_file` and `sys_upload_session` are in it as tenant-scoped, so nothing changes for them (pinned). `sys_http_delivery` is not, so the opt-in would have silenced the audit for a redelivery from a caller with no organization, which is the one line `RedeliverOptions` keeps. **Fix inside the outbox:** the reset write states `bypassTenantAudit: false`. The engine never overwrites an explicit value, and the driver still audits. - **Unchanged, measured or read:** row and field security, the masker and the Layer 0 tenant wall are skipped by the hand-off and by the opt-in alike. Read auditing records neither (no user id). No lookup field exists on the three objects, so the referential-integrity skip does not apply. Redelivery's bulk data event publishes without an organization in both cases. ## Pins - **Store, `tenant-audit-update-delete-half-repairs.test.ts`.** - Every by-id read carries `{ context: { isSystem: true } }` and no tenant. - Every by-id write carries the opt-in beside the door's tenant, or the opt-in alone with no tenant invented. The route-level pins are updated to match. - ⛔ An update payload never carries the provisioned columns of the row read back. - The opt-in leaves these writes' tenant audit armed. - ⛔ Negative, through the real `ObjectQL` over a real `SqlDriver` on SQLite in the isolated posture: under the opt-in, a door tenant's `updateFile`, `updateSession`, `deleteFile` and `deleteSession` on a row stamped for another organization are refused (`StorageMetadataStoreError` wrapping `RECORD_NOT_FOUND`) and leave the row untouched. The still-works half shows the caller's own row and an organization-less row are reached. - **Outbox, `system-context.pin.test.ts`.** Both reads and the reset write carry the opt-in. The caller's `tenantId` stays on every bag. The reset states `bypassTenantAudit: false`, and a caller with no tenant gets none invented. - **Outbox, `delivery-update-tenant-audit.integration.test.ts`.** - ⛔ Negative, on the real driver: a foreign row stays `RESOURCE_NOT_FOUND` with zero writes under the opt-in, and the caller's own row resets. - The two existing audit assertions now read `false`, the value the driver receives, where they read "absent". - **The stage-1 pins stay green.** These are #22025's insert pins and #22037's inbox pins, inside the full package suites. **Ablations**, run at `0f6df0f306`. Each mutation went through `scripts/ablation-replace.mjs`: the anchor hit, the blob changed, and the restore was proved by blob equal to `HEAD` and an empty `git diff HEAD`. The pins import from `src`, so no `dist` leg applies. | # | Mutation | Result | | :-- | :-- | :-- | | A1 | by-id writes keep the opt-in but drop the door tenant | 13 of 28 red; the negative pins read "promise resolved … instead of rejecting" (the foreign row was reached) | | A3 | `updateFile` sends the merged row again | 1 of 28 red; the payload carried `organization_id` and `created_by` | | A4 | `redeliver`'s deciding read drops `tenantId` | 5 of 30 red; the negative pin got `DELIVERY_NOT_ELIGIBLE` where it expects `RESOURCE_NOT_FOUND` | | A5 | the reset write's `bypassTenantAudit: false` removed | 4 of 21 red; the driver received `true` ("expected true to be false"), and the tenant-less redelivery went silent | ## Measurement The measurement used a local instrument in `plugin-security`'s middleware, never committed. Its instrument file was reverted, and blob `5b4ab28045af` equals `HEAD`. `plugin-security` was rebuilt, and `ablation-dist-preflight --absent` passes; its positive control found the marker in 2 built files while it was live. For each call from the seven producers, the instrument recorded the producer frame, whether the call was principal-less, and a dry run of the six gates with the system flag cleared. Two runs used it: - A scratch harness, deleted and not in the diff. It drove every door on a booted stack: upload commit, both downloads, the chunk upload, progress, chunked completion, progress on an expired session, and `POST /api/v1/webhooks/redeliver`. It called `deleteFile` and `deleteSession` directly. - The 11-file dogfood subset named below. Every door answered the same on the base and on the change (200, or 302 for the redirect). There were 0 gate firings before and after. ## Acceptance notes - **Premise correction.** `updateFile` and `updateSession` already accepted the optional organization context (`metadata-store.ts:399`, `:529`), as `deleteFile` and `deleteSession` did. Only `getFile` and `getSession` take none, and they still take none: the reads carry the opt-in without a tenant. No public signature changed. - **Door ordering, reported for the seat.** At both download doors the read precedes `authorizeDownload`. At the chunk door the read precedes the resume-token check. Each check reads the row it needs, and each precedes any disclosure and any write. The read's reach is the same before and after (unscoped by principal and by organization). - **Doors without a door-level binding.** The chunked completion and progress doors authorize by session authentication and check no resume token. The commit door checks no ownership of the file id. The move leaves all three unchanged, and owner checks are the question the ruling sent to its own card (option C). Reported to the seat at class level. - **`deleteFile` and `deleteSession` have no caller in this repository.** They were moved as the ruling wires all seven. Their door check is vacuous. - **Files beyond the two named files, all in the same packages.** - `outbox-dispatcher-scope.ts` declares `REDELIVER_SYSTEM_CONTEXT` (module-internal) with its own warrant, and amends the two docblocks that kept the outbox opt-ins off `redeliver`. - `messaging-service.ts` gets one sentence made true. - `scripts/engine-double-contract.pinned.json` counts the storage test file's second double, which `--write` grew. - `plugin-audit`'s `audit-writers.test.ts` fixture prose still describes `updateSession` writing the merged full record. That is test residue in another package and is left alone. ## Verification (head `2665532dc9`) Each command ran in the foreground of this worktree, and its exit code was captured before any pipe. - **Gates.** Running `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `2665532dc9`, with no paths, derived 73 commands. The dispatch-time 51 are a subset. All 73 ran and all exited 0. The reconciliation (`--ran`) printed: "73 derived famil(ies) accounted for — 73 run, 0 NOT-MEASURED (a DERIVED zero — all 73 recorded an exit code and none of them is 3)". - On an earlier pass at `6f366adcb1`, `check:engine-double-contract` exited 1. It had counted the new recording wrapper as a fake engine. The wrapper now routes each verb through the engine's dispatch predicates, and `--write` grew the ledger. - `check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET: 8 packages outside the dogfood closure had no `dist/`). They were built, and it then exited 0. - **Suites at `2665532dc9`.** `@objectstack/service-storage`: 42 files, 659 tests passed. `@objectstack/service-messaging`: 48 files, 534 passed. `typecheck` passed for both, the storage test layer's `check:test-typecheck` included. `--listFiles` confirms both programs compile the edited test files. `@objectstack/plugin-webhooks`, which mounts the redeliver door, passed at `0f6df0f306`: 15 files, 165 tests. - **Dogfood.** The 11 files below passed, 100 passed and 1 skipped, on both the base and the change. They ran against the change's built `dist/`; the source is unchanged since `230ed9ea15`. The files: `attachments-permission-matrix`, `attachments-public-read-acl`, `attachments-unscoped-delete-gate`, `field-file-collection`, `file-field-constraint-refusal`, `sys-file-metadata-write-refusal`, `predicate-write-unreadable-not-matched`, `write-door-unreadable-is-not-found`, `storage-growth`, `temporal-storage-e2e`, `webhook-materialization`. None of these files reaches the redeliver door; the scratch harness covered it. - **Lint, a proven narrowing; CI runs the full `pnpm lint`.** The population was read from `eslint.config.mjs`: the `**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` block plus the `packages/**` blocks. `eslint --no-inline-config --format json` over the 7 changed `.ts` files reported 7 files, 0 errors and 0 warnings at `c4f10218af`. The config enables no type-aware linting (no `parserOptions.project` and no typed rules), so no untouched file's verdict can move. ## Changeset `.changeset/21908-by-id-producers-opt-in.md` grades `patch` for `@objectstack/service-storage` and `@objectstack/service-messaging`. It names the opt-in, the patch-only update payload and the held tenant audit. No exported type or entry symbol changed: the new constants and the store helper are module-internal. --- _Generated by [Claude Code](https://claude.ai/code/session_01WMQprn46CND82KmY8sZWBu)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c565813 commit a7a48b7

9 files changed

Lines changed: 548 additions & 42 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/service-storage": patch
3+
"@objectstack/service-messaging": patch
4+
---
5+
6+
The storage store's by-id methods and the HTTP outbox's `redeliver` now pass the explicit system opt-in (`{ isSystem: true }`) on their data-engine calls. Until now they reached the engine with no principal and no opt-in, and the security middleware let that through only because of its principal-less hand-off.
7+
8+
Clause-②: no
9+
10+
- **service-storage.** `StorageMetadataStore.getFile`, `updateFile`, `deleteFile`, `getSession`, `updateSession` and `deleteSession` take the opt-in inside the store. Access stays by id, and the reads stay unscoped by organization, as before. On update and delete the acting organization still reaches the driver beside the opt-in, so a row stamped for another organization is still out of reach of these doors. The doors keep the authorization they already ran.
11+
- **service-storage, the update payload.** `updateFile` and `updateSession` now send the caller's patch alone, where they used to send the whole row read back merged with it. The engine's read-only strip, which does not run for a system write, used to take `organization_id` and the four audit columns out of that row; now the store never sends them. The stored row is the same as before, and a column another writer changed between the read and the write is no longer reverted by it.
12+
- **service-messaging.** `SqlHttpOutbox.redeliver` takes the opt-in on both of its reads and on its reset write. The caller's `tenantId` stays on every call as the driver-level scope, so a delivery in another organization is still not found. The reset write states `bypassTenantAudit: false`, so a redelivery from a caller with no organization is still reported by the driver's tenant audit.
13+
- None of the gates the security middleware runs before its hand-off applies to these calls. ⛔ No new export on either package entry, and no new elevation API.

‎packages/services/service-messaging/src/delivery-update-tenant-audit.integration.test.ts‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
*/
7171

7272
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
73-
import { ObjectQL } from '@objectstack/objectql';
73+
import { ObjectQL, assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/objectql';
7474
import { SqlDriver } from '@objectstack/driver-sql';
7575
import { SqlHttpOutbox } from './sql-http-outbox.js';
7676
import { SqlNotificationOutbox, DELIVERY_OBJECT } from './sql-outbox.js';
@@ -325,8 +325,11 @@ describe('redeliver — the request-reachable site is SCOPED, never bypassed', (
325325
expect(writes).toHaveLength(1);
326326
expect(writes[0].where).toMatchObject({ id: 'h_a', status: { $in: ['success', 'failed', 'dead'] } });
327327
// ⛔ The forbidden implementation, named: a bypass here would silence
328-
// the audit for an authenticated user's unscoped write.
329-
expect(writes[0].options?.bypassTenantAudit).toBeUndefined();
328+
// the audit for an authenticated user's unscoped write. [#21908] The
329+
// write now carries the explicit system opt-in, for which the engine
330+
// fills in a bypass on this object unless one is stated — so the
331+
// outbox states `false`, and that is what must reach the driver.
332+
expect(writes[0].options?.bypassTenantAudit).toBe(false);
330333
// …and the remedy that replaces it, present.
331334
expect(writes[0].options?.tenantId).toBe('org_a');
332335
// The line is absent BECAUSE the write is scoped — the two assertions
@@ -373,6 +376,47 @@ describe('redeliver — the request-reachable site is SCOPED, never bypassed', (
373376
expect(rows.map((r) => `${r.id}:${r.status}`).sort()).toEqual(['h_a:pending', 'h_b:dead']);
374377
});
375378

379+
it('[#21908] ⛔ under the explicit system opt-in the threaded tenant still walls a foreign row', async () => {
380+
// The opt-in replaces the security middleware's principal-less
381+
// hand-off on these calls; it must not replace the tenant scope. Read
382+
// through a wrapper that records the context each engine call carried,
383+
// so this is a reading of the opt-in path and of no other.
384+
await seedDeadRow('h_a', 'org_a');
385+
await seedDeadRow('h_b', 'org_b');
386+
const contexts: Array<{ verb: string; context: unknown; tenantId: unknown }> = [];
387+
const recorded: any = {
388+
findOne: (o: string, q: any, opts: any) => {
389+
assertEngineFindOnePredicate(o, q);
390+
contexts.push({ verb: 'findOne', context: opts?.context, tenantId: q?.tenantId });
391+
return engine.findOne(o, q, opts);
392+
},
393+
update: (o: string, d: any, opts: any) => {
394+
assertEngineUpdateDispatch(d, opts);
395+
contexts.push({ verb: 'update', context: opts?.context, tenantId: opts?.tenantId });
396+
return engine.update(o, d, opts);
397+
},
398+
};
399+
const outbox = new SqlHttpOutbox(recorded, { partitionCount: 1 });
400+
401+
await expect(outbox.redeliver('h_b', { tenantId: 'org_a' })).rejects.toMatchObject({
402+
name: 'HttpRedeliverError',
403+
code: 'RESOURCE_NOT_FOUND',
404+
});
405+
expect(contexts).toEqual([{ verb: 'findOne', context: { isSystem: true }, tenantId: 'org_a' }]);
406+
expect(driverUpdateManys.filter((u) => u.object === SYS_HTTP_DELIVERY)).toHaveLength(0);
407+
408+
// The still-works half, on the same path: the caller's own row resets.
409+
const replayed = await outbox.redeliver('h_a', { tenantId: 'org_a' });
410+
expect(`${replayed.id}:${replayed.status}:${replayed.attempts}`).toBe('h_a:pending:0');
411+
expect(contexts.slice(1)).toEqual([
412+
{ verb: 'findOne', context: { isSystem: true }, tenantId: 'org_a' },
413+
{ verb: 'update', context: { isSystem: true }, tenantId: 'org_a' },
414+
{ verb: 'findOne', context: { isSystem: true }, tenantId: 'org_a' },
415+
]);
416+
const rows = (await engine.find(SYS_HTTP_DELIVERY, { where: {} })) as any[];
417+
expect(rows.map((r) => `${r.id}:${r.status}:${r.attempts}`).sort()).toEqual(['h_a:pending:0', 'h_b:dead:3']);
418+
});
419+
376420
it('a tenant-less caller is NOT silenced — the audit still reports the gap', async () => {
377421
// The honest half of `tenantId: string | undefined`. Passing
378422
// `undefined` leaves the write unscoped, and the finding is REPORTED
@@ -389,7 +433,8 @@ describe('redeliver — the request-reachable site is SCOPED, never bypassed', (
389433
// have consumed this op's one warning.
390434
const writes = driverUpdateManys.filter((u) => u.object === SYS_HTTP_DELIVERY);
391435
expect(writes).toHaveLength(1);
392-
expect(writes[0].options?.bypassTenantAudit).toBeUndefined();
436+
// [#21908] Stated `false` under the opt-in — see the first test.
437+
expect(writes[0].options?.bypassTenantAudit).toBe(false);
393438
expect(auditedUpdateMany(SYS_HTTP_DELIVERY)).toBe(true);
394439
});
395440
});

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,10 @@ export class MessagingService {
358358
* authenticated user, and `sys_http_delivery` is tenant-scoped. The
359359
* outbox applies it to the rows it reads and the row it writes, so a
360360
* caller can only replay deliveries in its own organization; a row
361-
* elsewhere is `RESOURCE_NOT_FOUND`. ⛔ There is no `bypassTenantAudit`
361+
* elsewhere is `RESOURCE_NOT_FOUND`. ⛔ There is no tenant-audit bypass
362362
* anywhere on this path and there must not be — see `RedeliverOptions`.
363+
* [#21908] The outbox's write states `bypassTenantAudit: false` outright,
364+
* so the explicit system opt-in it now carries cannot fill one in either.
363365
*/
364366
async redeliverHttp(id: string, options: RedeliverOptions): Promise<HttpDelivery> {
365367
if (!this.httpOutbox) {

‎packages/services/service-messaging/src/outbox-dispatcher-scope.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ export function dispatcherSweepOptions(
9595
* queue.
9696
*
9797
* ⛔ Never on `redeliver`: it is request-reachable and threads the caller's
98-
* tenant, the line this file draws for `bypassTenantAudit` too.
98+
* tenant, the line this file draws for `bypassTenantAudit` too. [#21908] It
99+
* takes its own opt-in, {@link REDELIVER_SYSTEM_CONTEXT}, on a different
100+
* warrant — the door's — and keeps that tenant beside it.
99101
*/
100102
export const DISPATCHER_SYSTEM_CONTEXT = { isSystem: true } as const;
101103

@@ -117,10 +119,44 @@ export const DISPATCHER_SYSTEM_CONTEXT = { isSystem: true } as const;
117119
*
118120
* ⛔ Never on `redeliver`: it is the one request-reachable call on these
119121
* objects, and it threads the caller's tenant (see {@link DISPATCHER_SYSTEM_CONTEXT}).
122+
* [#21908] Its opt-in is {@link REDELIVER_SYSTEM_CONTEXT}, whose warrant is not
123+
* this one: a caller exists there, and the door authorizes it.
120124
*/
121125
export const OUTBOX_SYSTEM_CONTEXT = { isSystem: true } as const;
122126

123127

128+
/**
129+
* [#21908] The execution context `SqlHttpOutbox.redeliver` runs under — its
130+
* two reads and its reset write: the explicit system opt-in.
131+
*
132+
* The one request-reachable call on `sys_http_delivery` used to be the one
133+
* outbox call kept OFF the opt-ins above, because it threads the caller's
134+
* tenant and their warrants rest on there being no caller. Both still hold.
135+
* What changed is that the security middleware's principal-less hand-off,
136+
* which this call passed through with no principal and no opt-in, closes
137+
* (ADR-0096 D5), and the caller's own principal is no replacement: no member
138+
* grant exists on `sys_http_delivery`, so every member's redelivery would be
139+
* refused. The maintainer's ruling on this card puts the explicit opt-in on it
140+
* inside this service, on a warrant of its own:
141+
*
142+
* 1. **The door authorizes.** `POST /api/v1/webhooks/redeliver` admits an
143+
* authenticated session only (else `401`), and the producer's veto
144+
* (`RedeliverGuard`) runs before the write.
145+
* 2. **The tenant stays the scope.** The caller's `tenantId` rides every
146+
* call's options bag beside this context, as the driver-level scope it
147+
* always was, so a row in another organization is still not found.
148+
* 3. **The audit stays armed.** The reset write states
149+
* `bypassTenantAudit: false` itself, because the engine fills in `true` for
150+
* an `isSystem` write on an object outside the tenant-audit inventory —
151+
* which would silence the line `RedeliverOptions` keeps for a caller with
152+
* no tenant.
153+
*
154+
* ⛔ Never borrowed by the dispatcher or producer sites, and never the other
155+
* way round: the three contexts are equal in value and differ in warrant.
156+
*/
157+
export const REDELIVER_SYSTEM_CONTEXT = { isSystem: true } as const;
158+
159+
124160
/**
125161
* The write options for a dispatcher **`ack`** — the single-record
126162
* (`multi: false`) write that records one delivery attempt's outcome on

‎packages/services/service-messaging/src/sql-http-outbox.ts‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import { randomUUID } from 'node:crypto';
44
import type { IDataEngine } from '@objectstack/spec/contracts';
5+
import type { EngineUpdateOptions } from '@objectstack/spec/data';
56
import { hashPartition } from './backoff.js';
67
import { toEpochMs } from './audit-timestamp.js';
78
import {
89
DISPATCHER_SYSTEM_CONTEXT,
910
OUTBOX_SYSTEM_CONTEXT,
11+
REDELIVER_SYSTEM_CONTEXT,
1012
dispatcherAckCasOptions,
1113
dispatcherAckOptions,
1214
dispatcherSweepOptions,
@@ -464,6 +466,26 @@ export class SqlHttpOutbox implements IHttpOutbox {
464466
* so the endpoint neither replays it nor confirms it exists. It is also
465467
* what keeps the guard honest — the producer veto never sees a row the
466468
* caller may not read.
469+
*
470+
* [#21908] Every engine call here also carries the explicit system opt-in
471+
* ({@link REDELIVER_SYSTEM_CONTEXT}). Until now they reached the data engine
472+
* with no principal and no opt-in and passed the security middleware only
473+
* through its principal-less hand-off (ADR-0096 E1), which D5 closes; the
474+
* caller's own principal is no answer, because no member grant exists on
475+
* `sys_http_delivery`. The door keeps its authorization (an authenticated
476+
* session, else `401`) and the producer's veto still runs before the write.
477+
* Two things the opt-in would otherwise change are held where they were:
478+
*
479+
* - the REACH. `tenantId` stays on the bag of every call below, the
480+
* driver-level scope it always was. The opt-in replaces the hand-off,
481+
* not this scope, so a row in another organization is still not found.
482+
* - the AUDIT. For an `isSystem` write the engine fills in
483+
* `bypassTenantAudit: true` on an object outside the tenant-audit
484+
* inventory, which `sys_http_delivery` is. That would silence exactly
485+
* the line {@link RedeliverOptions} keeps for a caller with no tenant, so
486+
* the write states `bypassTenantAudit: false` itself: an explicit value
487+
* is never overwritten by that fill-in, and the driver still audits an
488+
* unscoped redeliver.
467489
*/
468490
async redeliver(id: string, options: RedeliverOptions): Promise<HttpDelivery> {
469491
// One options bag for every engine call below, so the read that decides
@@ -473,7 +495,7 @@ export class SqlHttpOutbox implements IHttpOutbox {
473495
const current = (await this.engine.findOne(this.objectName, {
474496
where: { id },
475497
...scope,
476-
})) as DeliveryRow | null;
498+
}, { context: REDELIVER_SYSTEM_CONTEXT })) as DeliveryRow | null;
477499
if (!current) {
478500
throw new HttpRedeliverError(`Delivery row '${id}' not found`, 'RESOURCE_NOT_FOUND');
479501
}
@@ -493,6 +515,16 @@ export class SqlHttpOutbox implements IHttpOutbox {
493515
// included, so the reset lands only if the row is still terminal.
494516
// A miss writes 0 rows and the read-back below reports the refusal
495517
// (`DELIVERY_NOT_ELIGIBLE`) instead of a false success.
518+
// [#21908] Typed with the two driver pass-through keys it carries: the
519+
// caller's tenant as the statement's scope, and the tenant audit held
520+
// armed explicitly — see the method docs.
521+
const resetOptions: EngineUpdateOptions & { tenantId: string | undefined; bypassTenantAudit: false } = {
522+
where: { id, status: { $in: ['success', 'failed', 'dead'] } },
523+
multi: true,
524+
...scope,
525+
bypassTenantAudit: false,
526+
context: REDELIVER_SYSTEM_CONTEXT,
527+
};
496528
await this.engine.update(
497529
this.objectName,
498530
{
@@ -506,12 +538,12 @@ export class SqlHttpOutbox implements IHttpOutbox {
506538
response_body: null,
507539
error: null,
508540
},
509-
{ where: { id, status: { $in: ['success', 'failed', 'dead'] } }, multi: true, ...scope },
541+
resetOptions,
510542
);
511543
const after = (await this.engine.findOne(this.objectName, {
512544
where: { id },
513545
...scope,
514-
})) as DeliveryRow | null;
546+
}, { context: REDELIVER_SYSTEM_CONTEXT })) as DeliveryRow | null;
515547
if (!after || after.status !== 'pending') {
516548
throw new HttpRedeliverError(`Delivery row '${id}' state changed during redeliver`, 'DELIVERY_NOT_ELIGIBLE');
517549
}

‎packages/services/service-messaging/src/system-context.pin.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,3 +420,56 @@ describe('[#21908] rows 24 onward — the remaining fan-out and outbox calls car
420420
expectAllSystem(calls);
421421
});
422422
});
423+
424+
describe('[#21908] SqlHttpOutbox.redeliver carries the explicit system opt-in beside its threaded tenant', () => {
425+
/** A terminal, genuinely-attempted row on the first read; `pending` on the read-back. */
426+
function redeliverEngine() {
427+
let reads = 0;
428+
return recordingEngine((verb) => {
429+
if (verb !== 'findOne') return [];
430+
reads += 1;
431+
return {
432+
id: 'h1', source: 'webhook', ref_id: 'wh1', dedup_key: 'k1', url: 'https://example.test/hook',
433+
payload_json: '{}', signature: 'sig', organization_id: 'org_a', partition_key: 0,
434+
status: reads === 1 ? 'dead' : 'pending', attempts: reads === 1 ? 2 : 0,
435+
created_at: NOW, updated_at: NOW,
436+
};
437+
});
438+
}
439+
440+
it('both reads and the reset write: isSystem, the caller’s tenantId on the bag, and the audit stated armed', async () => {
441+
const { engine, calls } = redeliverEngine();
442+
const outbox = new SqlHttpOutbox(engine, { partitionCount: 1 });
443+
444+
const row = await outbox.redeliver('h1', { tenantId: 'org_a' });
445+
expect(row.status).toBe('pending');
446+
447+
expect(calls.map((c) => c.verb)).toEqual(['findOne', 'update', 'findOne']);
448+
expect(calls.every((c) => c.object === 'sys_http_delivery')).toBe(true);
449+
expectAllSystem(calls);
450+
// The scope stays the driver-level tenant on every call, never moved
451+
// into the context and never dropped.
452+
expect(calls[0].query).toMatchObject({ where: { id: 'h1' }, tenantId: 'org_a' });
453+
expect(calls[2].query).toMatchObject({ where: { id: 'h1' }, tenantId: 'org_a' });
454+
expect(calls[1].options).toMatchObject({
455+
where: { id: 'h1', status: { $in: ['success', 'failed', 'dead'] } },
456+
multi: true,
457+
tenantId: 'org_a',
458+
bypassTenantAudit: false,
459+
});
460+
});
461+
462+
it('⛔ a caller with no tenant gets no tenant invented, and still no audit bypass', async () => {
463+
const { engine, calls } = redeliverEngine();
464+
const outbox = new SqlHttpOutbox(engine, { partitionCount: 1 });
465+
466+
await outbox.redeliver('h1', { tenantId: undefined });
467+
468+
expectAllSystem(calls);
469+
for (const c of calls) {
470+
const bag = c.verb === 'update' ? c.options : c.query;
471+
expect(bag).toHaveProperty('tenantId', undefined);
472+
}
473+
expect(calls[1].options.bypassTenantAudit).toBe(false);
474+
});
475+
});

0 commit comments

Comments
 (0)