fix(service-storage, service-messaging): the storage store's by-id methods and the HTTP outbox's redeliver take the explicit system opt-in

.changeset/21908-by-id-producers-opt-in.md

@@ -0,0 +1,13 @@
+---
+"@objectstack/service-storage": patch
+"@objectstack/service-messaging": patch
+---
+
+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.
+
+Clause-②: no
+
+- **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.
+- **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.
+- **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.
+- 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

@@ -70,7 +70,7 @@
  */
 
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
-import { ObjectQL } from '@objectstack/objectql';
+import { ObjectQL, assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/objectql';
 import { SqlDriver } from '@objectstack/driver-sql';
 import { SqlHttpOutbox } from './sql-http-outbox.js';
 import { SqlNotificationOutbox, DELIVERY_OBJECT } from './sql-outbox.js';
@@ -325,8 +325,11 @@ describe('redeliver — the request-reachable site is SCOPED, never bypassed', (
         expect(writes).toHaveLength(1);
         expect(writes[0].where).toMatchObject({ id: 'h_a', status: { $in: ['success', 'failed', 'dead'] } });
         // ⛔ The forbidden implementation, named: a bypass here would silence
-        // the audit for an authenticated user's unscoped write.
-        expect(writes[0].options?.bypassTenantAudit).toBeUndefined();
+        // the audit for an authenticated user's unscoped write. [#21908] The
+        // write now carries the explicit system opt-in, for which the engine
+        // fills in a bypass on this object unless one is stated — so the
+        // outbox states `false`, and that is what must reach the driver.
+        expect(writes[0].options?.bypassTenantAudit).toBe(false);
         // …and the remedy that replaces it, present.
         expect(writes[0].options?.tenantId).toBe('org_a');
         // 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', (
         expect(rows.map((r) => `${r.id}:${r.status}`).sort()).toEqual(['h_a:pending', 'h_b:dead']);
     });
 
+    it('[#21908] ⛔ under the explicit system opt-in the threaded tenant still walls a foreign row', async () => {
+        // The opt-in replaces the security middleware's principal-less
+        // hand-off on these calls; it must not replace the tenant scope. Read
+        // through a wrapper that records the context each engine call carried,
+        // so this is a reading of the opt-in path and of no other.
+        await seedDeadRow('h_a', 'org_a');
+        await seedDeadRow('h_b', 'org_b');
+        const contexts: Array<{ verb: string; context: unknown; tenantId: unknown }> = [];
+        const recorded: any = {
+            findOne: (o: string, q: any, opts: any) => {
+                assertEngineFindOnePredicate(o, q);
+                contexts.push({ verb: 'findOne', context: opts?.context, tenantId: q?.tenantId });
+                return engine.findOne(o, q, opts);
+            },
+            update: (o: string, d: any, opts: any) => {
+                assertEngineUpdateDispatch(d, opts);
+                contexts.push({ verb: 'update', context: opts?.context, tenantId: opts?.tenantId });
+                return engine.update(o, d, opts);
+            },
+        };
+        const outbox = new SqlHttpOutbox(recorded, { partitionCount: 1 });
+
+        await expect(outbox.redeliver('h_b', { tenantId: 'org_a' })).rejects.toMatchObject({
+            name: 'HttpRedeliverError',
+            code: 'RESOURCE_NOT_FOUND',
+        });
+        expect(contexts).toEqual([{ verb: 'findOne', context: { isSystem: true }, tenantId: 'org_a' }]);
+        expect(driverUpdateManys.filter((u) => u.object === SYS_HTTP_DELIVERY)).toHaveLength(0);
+
+        // The still-works half, on the same path: the caller's own row resets.
+        const replayed = await outbox.redeliver('h_a', { tenantId: 'org_a' });
+        expect(`${replayed.id}:${replayed.status}:${replayed.attempts}`).toBe('h_a:pending:0');
+        expect(contexts.slice(1)).toEqual([
+            { verb: 'findOne', context: { isSystem: true }, tenantId: 'org_a' },
+            { verb: 'update', context: { isSystem: true }, tenantId: 'org_a' },
+            { verb: 'findOne', context: { isSystem: true }, tenantId: 'org_a' },
+        ]);
+        const rows = (await engine.find(SYS_HTTP_DELIVERY, { where: {} })) as any[];
+        expect(rows.map((r) => `${r.id}:${r.status}:${r.attempts}`).sort()).toEqual(['h_a:pending:0', 'h_b:dead:3']);
+    });
+
     it('a tenant-less caller is NOT silenced — the audit still reports the gap', async () => {
         // The honest half of `tenantId: string | undefined`. Passing
         // `undefined` leaves the write unscoped, and the finding is REPORTED
@@ -389,7 +433,8 @@ describe('redeliver — the request-reachable site is SCOPED, never bypassed', (
         // have consumed this op's one warning.
         const writes = driverUpdateManys.filter((u) => u.object === SYS_HTTP_DELIVERY);
         expect(writes).toHaveLength(1);
-        expect(writes[0].options?.bypassTenantAudit).toBeUndefined();
+        // [#21908] Stated `false` under the opt-in — see the first test.
+        expect(writes[0].options?.bypassTenantAudit).toBe(false);
         expect(auditedUpdateMany(SYS_HTTP_DELIVERY)).toBe(true);
     });
 });

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

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

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

@@ -95,7 +95,9 @@ export function dispatcherSweepOptions(
  * queue.
  *
  * ⛔ Never on `redeliver`: it is request-reachable and threads the caller's
- * tenant, the line this file draws for `bypassTenantAudit` too.
+ * tenant, the line this file draws for `bypassTenantAudit` too. [#21908] It
+ * takes its own opt-in, {@link REDELIVER_SYSTEM_CONTEXT}, on a different
+ * warrant — the door's — and keeps that tenant beside it.
  */
 export const DISPATCHER_SYSTEM_CONTEXT = { isSystem: true } as const;
 
@@ -117,10 +119,44 @@ export const DISPATCHER_SYSTEM_CONTEXT = { isSystem: true } as const;
  *
  * ⛔ Never on `redeliver`: it is the one request-reachable call on these
  * objects, and it threads the caller's tenant (see {@link DISPATCHER_SYSTEM_CONTEXT}).
+ * [#21908] Its opt-in is {@link REDELIVER_SYSTEM_CONTEXT}, whose warrant is not
+ * this one: a caller exists there, and the door authorizes it.
  */
 export const OUTBOX_SYSTEM_CONTEXT = { isSystem: true } as const;
 
 
+/**
+ * [#21908] The execution context `SqlHttpOutbox.redeliver` runs under — its
+ * two reads and its reset write: the explicit system opt-in.
+ *
+ * The one request-reachable call on `sys_http_delivery` used to be the one
+ * outbox call kept OFF the opt-ins above, because it threads the caller's
+ * tenant and their warrants rest on there being no caller. Both still hold.
+ * What changed is that the security middleware's principal-less hand-off,
+ * which this call passed through with no principal and no opt-in, closes
+ * (ADR-0096 D5), and the caller's own principal is no replacement: no member
+ * grant exists on `sys_http_delivery`, so every member's redelivery would be
+ * refused. The maintainer's ruling on this card puts the explicit opt-in on it
+ * inside this service, on a warrant of its own:
+ *
+ *  1. **The door authorizes.** `POST /api/v1/webhooks/redeliver` admits an
+ *     authenticated session only (else `401`), and the producer's veto
+ *     (`RedeliverGuard`) runs before the write.
+ *  2. **The tenant stays the scope.** The caller's `tenantId` rides every
+ *     call's options bag beside this context, as the driver-level scope it
+ *     always was, so a row in another organization is still not found.
+ *  3. **The audit stays armed.** The reset write states
+ *     `bypassTenantAudit: false` itself, because the engine fills in `true` for
+ *     an `isSystem` write on an object outside the tenant-audit inventory —
+ *     which would silence the line `RedeliverOptions` keeps for a caller with
+ *     no tenant.
+ *
+ * ⛔ Never borrowed by the dispatcher or producer sites, and never the other
+ * way round: the three contexts are equal in value and differ in warrant.
+ */
+export const REDELIVER_SYSTEM_CONTEXT = { isSystem: true } as const;
+
+
 /**
  * The write options for a dispatcher **`ack`** — the single-record
  * (`multi: false`) write that records one delivery attempt's outcome on

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

@@ -2,11 +2,13 @@
 
 import { randomUUID } from 'node:crypto';
 import type { IDataEngine } from '@objectstack/spec/contracts';
+import type { EngineUpdateOptions } from '@objectstack/spec/data';
 import { hashPartition } from './backoff.js';
 import { toEpochMs } from './audit-timestamp.js';
 import {
     DISPATCHER_SYSTEM_CONTEXT,
     OUTBOX_SYSTEM_CONTEXT,
+    REDELIVER_SYSTEM_CONTEXT,
     dispatcherAckCasOptions,
     dispatcherAckOptions,
     dispatcherSweepOptions,
@@ -464,6 +466,26 @@ export class SqlHttpOutbox implements IHttpOutbox {
      * so the endpoint neither replays it nor confirms it exists. It is also
      * what keeps the guard honest — the producer veto never sees a row the
      * caller may not read.
+     *
+     * [#21908] Every engine call here also carries the explicit system opt-in
+     * ({@link REDELIVER_SYSTEM_CONTEXT}). Until now they reached the data engine
+     * with no principal and no opt-in and passed the security middleware only
+     * through its principal-less hand-off (ADR-0096 E1), which D5 closes; the
+     * caller's own principal is no answer, because no member grant exists on
+     * `sys_http_delivery`. The door keeps its authorization (an authenticated
+     * session, else `401`) and the producer's veto still runs before the write.
+     * Two things the opt-in would otherwise change are held where they were:
+     *
+     *  - the REACH. `tenantId` stays on the bag of every call below, the
+     *    driver-level scope it always was. The opt-in replaces the hand-off,
+     *    not this scope, so a row in another organization is still not found.
+     *  - the AUDIT. For an `isSystem` write the engine fills in
+     *    `bypassTenantAudit: true` on an object outside the tenant-audit
+     *    inventory, which `sys_http_delivery` is. That would silence exactly
+     *    the line {@link RedeliverOptions} keeps for a caller with no tenant, so
+     *    the write states `bypassTenantAudit: false` itself: an explicit value
+     *    is never overwritten by that fill-in, and the driver still audits an
+     *    unscoped redeliver.
      */
     async redeliver(id: string, options: RedeliverOptions): Promise<HttpDelivery> {
         // One options bag for every engine call below, so the read that decides
@@ -473,7 +495,7 @@ export class SqlHttpOutbox implements IHttpOutbox {
         const current = (await this.engine.findOne(this.objectName, {
             where: { id },
             ...scope,
-        })) as DeliveryRow | null;
+        }, { context: REDELIVER_SYSTEM_CONTEXT })) as DeliveryRow | null;
         if (!current) {
             throw new HttpRedeliverError(`Delivery row '${id}' not found`, 'RESOURCE_NOT_FOUND');
         }
@@ -493,6 +515,16 @@ export class SqlHttpOutbox implements IHttpOutbox {
         // included, so the reset lands only if the row is still terminal.
         // A miss writes 0 rows and the read-back below reports the refusal
         // (`DELIVERY_NOT_ELIGIBLE`) instead of a false success.
+        // [#21908] Typed with the two driver pass-through keys it carries: the
+        // caller's tenant as the statement's scope, and the tenant audit held
+        // armed explicitly — see the method docs.
+        const resetOptions: EngineUpdateOptions & { tenantId: string | undefined; bypassTenantAudit: false } = {
+            where: { id, status: { $in: ['success', 'failed', 'dead'] } },
+            multi: true,
+            ...scope,
+            bypassTenantAudit: false,
+            context: REDELIVER_SYSTEM_CONTEXT,
+        };
         await this.engine.update(
             this.objectName,
             {
@@ -506,12 +538,12 @@ export class SqlHttpOutbox implements IHttpOutbox {
                 response_body: null,
                 error: null,
             },
-            { where: { id, status: { $in: ['success', 'failed', 'dead'] } }, multi: true, ...scope },
+            resetOptions,
         );
         const after = (await this.engine.findOne(this.objectName, {
             where: { id },
             ...scope,
-        })) as DeliveryRow | null;
+        }, { context: REDELIVER_SYSTEM_CONTEXT })) as DeliveryRow | null;
         if (!after || after.status !== 'pending') {
             throw new HttpRedeliverError(`Delivery row '${id}' state changed during redeliver`, 'DELIVERY_NOT_ELIGIBLE');
         }

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

@@ -420,3 +420,56 @@ describe('[#21908] rows 24 onward — the remaining fan-out and outbox calls car
         expectAllSystem(calls);
     });
 });
+
+describe('[#21908] SqlHttpOutbox.redeliver carries the explicit system opt-in beside its threaded tenant', () => {
+    /** A terminal, genuinely-attempted row on the first read; `pending` on the read-back. */
+    function redeliverEngine() {
+        let reads = 0;
+        return recordingEngine((verb) => {
+            if (verb !== 'findOne') return [];
+            reads += 1;
+            return {
+                id: 'h1', source: 'webhook', ref_id: 'wh1', dedup_key: 'k1', url: 'https://example.test/hook',
+                payload_json: '{}', signature: 'sig', organization_id: 'org_a', partition_key: 0,
+                status: reads === 1 ? 'dead' : 'pending', attempts: reads === 1 ? 2 : 0,
+                created_at: NOW, updated_at: NOW,
+            };
+        });
+    }
+
+    it('both reads and the reset write: isSystem, the caller’s tenantId on the bag, and the audit stated armed', async () => {
+        const { engine, calls } = redeliverEngine();
+        const outbox = new SqlHttpOutbox(engine, { partitionCount: 1 });
+
+        const row = await outbox.redeliver('h1', { tenantId: 'org_a' });
+        expect(row.status).toBe('pending');
+
+        expect(calls.map((c) => c.verb)).toEqual(['findOne', 'update', 'findOne']);
+        expect(calls.every((c) => c.object === 'sys_http_delivery')).toBe(true);
+        expectAllSystem(calls);
+        // The scope stays the driver-level tenant on every call, never moved
+        // into the context and never dropped.
+        expect(calls[0].query).toMatchObject({ where: { id: 'h1' }, tenantId: 'org_a' });
+        expect(calls[2].query).toMatchObject({ where: { id: 'h1' }, tenantId: 'org_a' });
+        expect(calls[1].options).toMatchObject({
+            where: { id: 'h1', status: { $in: ['success', 'failed', 'dead'] } },
+            multi: true,
+            tenantId: 'org_a',
+            bypassTenantAudit: false,
+        });
+    });
+
+    it('⛔ a caller with no tenant gets no tenant invented, and still no audit bypass', async () => {
+        const { engine, calls } = redeliverEngine();
+        const outbox = new SqlHttpOutbox(engine, { partitionCount: 1 });
+
+        await outbox.redeliver('h1', { tenantId: undefined });
+
+        expectAllSystem(calls);
+        for (const c of calls) {
+            const bag = c.verb === 'update' ? c.options : c.query;
+            expect(bag).toHaveProperty('tenantId', undefined);
+        }
+        expect(calls[1].options.bypassTenantAudit).toBe(false);
+    });
+});