fix(service-messaging,plugin-webhooks): classify the delivery outboxes' update-op tenant-audit surface — ack is a dispatcher sweep, redeliver threads the caller's tenant

.changeset/delivery-update-op-tenant-classification.md

@@ -0,0 +1,66 @@
+---
+"@objectstack/service-messaging": minor
+"@objectstack/plugin-webhooks": minor
+---
+
+fix(service-messaging,plugin-webhooks): the `update`-op tenant-audit surface on the delivery outboxes is classified — `ack` is a dispatcher sweep, `redeliver` threads the caller's tenant (#10740)
+
+**BREAKING** signature change on `IHttpOutbox.redeliver` and
+`MessagingService.redeliverHttp`, shipped as `minor` under the repo's
+launch-window convention for breaking changes.
+
+`sys_http_delivery` and `sys_notification_delivery` carry three single-record
+(`multi: false`) writes that the SQL driver audits under the **`update`** op —
+a different op, and a different throttle key, from the `updateMany` half
+classified previously. Their correct classifications are **opposite**, and
+treating them as one sweep is the dangerous reading:
+
+| site | reachable from | classification |
+| --- | --- | --- |
+| `SqlNotificationOutbox.ack` | dispatcher tick only | global sweep |
+| `SqlHttpOutbox.ack` | dispatcher tick only | global sweep |
+| `SqlHttpOutbox.redeliver` | `POST /api/v1/webhooks/redeliver` | request-contextual |
+
+**The two `ack` sites** are declared global sweeps through a new
+`dispatcherAckOptions()` helper, sibling to `dispatcherSweepOptions()` and
+deliberately not the same function — that one returns `& { multi: true }`, so a
+`multi: false` site cannot borrow it by accident. The warrant was re-derived
+against the current tree rather than inherited: `ack` has exactly two callers,
+both inside `runPartition()` on a `setInterval` tick holding a per-partition
+cluster lock, so no request context exists to thread; and the row being acked
+was claimed by a sweep that crosses organizations by construction
+(`hash(refId | notificationId | digestKey) mod N` is a load-spreading key, and
+one outbox per environment drains the whole queue). Passing the claimed row's
+own `organization_id` is documented at the helper as the tempting wrong answer:
+a predicate read off the row you are about to write matches exactly that row,
+adds no isolation, and silences the audit anyway — the appearance of scoping
+without the substance.
+
+**`redeliver` is not that**, and it is the reason this shipped separately. The
+route in front of it is served to any authenticated user, so on a walled
+deployment (`OS_TENANCY_POSTURE=isolated|group`) an unscoped replay is an
+authenticated user writing another organization's delivery row — the case the
+tenant audit exists to catch. It now carries the caller's tenant, applied to
+the rows it reads as well as the row it writes, and it must never be given
+`bypassTenantAudit`: a scoped write and a bypassed write produce the same
+silence in the log, so the flag would convert a detectable hole into an
+undetectable one. The webhook route resolves the session's
+`activeOrganizationId` and threads it.
+
+Behaviour change at the endpoint: a delivery row outside the caller's
+organization is now **not found** (`RESOURCE_NOT_FOUND`, HTTP 404) rather than
+replayed. It is deliberately invisible rather than forbidden, so the endpoint
+is not an existence oracle for other tenants' delivery ids. An in-tenant
+redelivery is unchanged.
+
+Migrating a caller: `redeliver(id, guard?)` becomes
+`redeliver(id, { tenantId, guard? })`, and `redeliverHttp(id)` becomes
+`redeliverHttp(id, { tenantId })`. `tenantId` is a **required** property typed
+`string | undefined`, so omitting it does not compile — a caller with no tenant
+has to write `tenantId: undefined` and mean it. That is the point of the shape:
+an optional property would let the dangerous case, a request path that simply
+forgot, type-check in silence. Passing `undefined` leaves the write unscoped
+and the audit line still fires, which is the intended reporting behaviour on a
+deployment that cannot resolve an organization for the caller.
+
+<!-- adr-0087: not-required (runtime-interface-only packages/services/service-messaging/src/http-outbox.ts#IHttpOutbox, packages/services/service-messaging/src/http-outbox.ts#RedeliverOptions) The surface that changed shape is `IHttpOutbox.redeliver`, plus the new `RedeliverOptions` argument type beside it. Both are TypeScript declarations in a service package with no `packages/spec` schema behind them: no metadata author writes a `redeliver` key, there is no authorable spelling and no `retiredKey()` tombstone, so `os migrate meta` has no stack source to rewrite. The change is a required second argument on an in-process method — a compile error at every call site, which is the notification channel, not a silent runtime gap. `MessagingService.redeliverHttp` moves with it and is deliberately NOT in the list above: this gate refuses that symbol as unresolvable, because `packages/spec/src/api/protocol.zod.ts` mentions the class name in a prose comment about `MessagingService.listInbox` while neither declaring nor importing it. The claim would be true and the gate cannot check it, so it is stated here for a reviewer instead of asserted where it would read as verified. It is a thin delegate to `IHttpOutbox.redeliver` in the same package and carries no schema of its own either. -->

packages/plugins/plugin-webhooks/src/webhook-drop-durable-record.test.ts

@@ -200,7 +200,11 @@ describe('dropped webhook subscription leaves a durable, unsendable record (#806
  await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick();
  expect(calls).toHaveLength(0);
 
- await expect(messaging.redeliverHttp(row.id)).rejects.toMatchObject({
+ // [#10740] `redeliverHttp` now requires the requesting caller's tenant.
+ // This fixture has no organization and no tenancy posture, so
+ // `undefined` is the honest value — required rather than optional
+ // precisely so that answer is written down instead of defaulted into.
+ await expect(messaging.redeliverHttp(row.id, { tenantId: undefined })).rejects.toMatchObject({
  code: 'DELIVERY_NEVER_SENT',
  });
  // The refusal did not mutate the row on its way out.

packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts

@@ -28,7 +28,17 @@ import {
 interface MessagingHttpSurface {
  isHttpDeliveryReady(): boolean;
  enqueueHttp(input: EnqueueHttpInput): Promise<string>;
- redeliverHttp(id: string): Promise<{ id: string; status: string }>;
+ /**
+ * [#10740] Takes the REQUESTING caller's organization. Declared with the
+ * required-but-nullable `tenantId` the service declares, so this plugin
+ * cannot call the endpoint's backing method without deciding what tenant
+ * the request carries — the omission this structural view would otherwise
+ * type-check happily.
+ */
+ redeliverHttp(
+ id: string,
+ options: { tenantId: string | undefined },
+ ): Promise<{ id: string; status: string }>;
  /**
  * [#8069] Where this plugin's veto over redelivering `source: 'webhook'`
  * rows is installed. Declared REQUIRED on this structural view even though
@@ -347,8 +357,21 @@ export class WebhookOutboxPlugin implements Plugin {
 
  /**
  * Mount POST /api/v1/webhooks/redeliver on the host Hono app, if one is
- * available. Delegates to `messaging.redeliverHttp(deliveryId)`. Auth is the
- * better-auth session cookie — every authenticated user counts.
+ * available. Delegates to `messaging.redeliverHttp(deliveryId, …)`. Auth is
+ * the better-auth session cookie — every authenticated user counts.
+ *
+ * [#10740] Which is precisely why the caller's ACTIVE ORGANIZATION is
+ * resolved here and threaded into the call. `sys_http_delivery` is
+ * tenant-scoped, and this is the one door on it a request can reach: an
+ * unscoped replay from here is an authenticated user reaching another
+ * organization's delivery row on a walled deployment. With the tenant
+ * threaded, a row outside the caller's organization is simply not found.
+ *
+ * ⚠️ A session with no active organization threads `undefined`, and the
+ * driver's tenant-audit line then fires for that write. That is deliberate:
+ * the deployment could not tell us who is asking, and reporting the gap is
+ * the correct outcome. ⛔ It is never repaired with `bypassTenantAudit`,
+ * which would silence the report without closing anything.
  */
  private registerAdminRoutes(ctx: PluginContext): void {
  const http = this.tryGetService<any>(ctx, ['http-server']);
@@ -361,8 +384,9 @@ export class WebhookOutboxPlugin implements Plugin {
  if (!rawApp || !messaging) return;
 
  rawApp.post('/api/v1/webhooks/redeliver', async (c: any) => {
- const userId = await this.resolveSessionUserId(ctx, c);
- if (!userId) {
+ const session = await this.resolveSession(ctx, c);
+ const userId = session?.user?.id;
+ if (typeof userId !== 'string' || userId.length === 0) {
  return c.json(
  { success: false, error: { code: 'UNAUTHENTICATED', message: 'Sign in to redeliver webhook deliveries.' } },
  401,
@@ -382,8 +406,20 @@ export class WebhookOutboxPlugin implements Plugin {
  );
  }
  try {
- const row = await messaging.redeliverHttp(deliveryId);
- ctx.logger.info?.('[webhook-outbox] redelivered', { deliveryId, requestedBy: userId });
+ // [#10740] `session.session.activeOrganizationId` is the
+ // canonical spelling of the caller's active organization
+ // (better-auth's organization plugin; see
+ // `plugin-auth/auth-schema-config.ts`). Read from the one
+ // place it lives — a `??` chain over alternative spellings
+ // would make a MISSING organization indistinguishable from a
+ // differently-shaped one, and the missing case is the one that
+ // must stay visible.
+ const activeOrg = session?.session?.activeOrganizationId;
+ const tenantId = typeof activeOrg === 'string' && activeOrg.length > 0
+ ? activeOrg
+ : undefined;
+ const row = await messaging.redeliverHttp(deliveryId, { tenantId });
+ ctx.logger.info?.('[webhook-outbox] redelivered', { deliveryId, requestedBy: userId, tenantId });
  return c.json({ success: true, data: { id: row.id, status: row.status } });
  } catch (err: any) {
  const code = err?.code;
@@ -412,7 +448,18 @@ export class WebhookOutboxPlugin implements Plugin {
  ctx.logger.info?.('[webhook-outbox] redeliver endpoint mounted at POST /api/v1/webhooks/redeliver');
  }
 
- private async resolveSessionUserId(ctx: PluginContext, c: any): Promise<string | undefined> {
+ /**
+ * [#10740] The better-auth session envelope (`{ user, session }`) for this
+ * request, or `undefined`.
+ *
+ * Widened from the previous `resolveSessionUserId` because the route now
+ * needs two facts from ONE lookup: who is asking (`user.id`, the
+ * authentication gate) and which organization they are asking as
+ * (`session.activeOrganizationId`, the tenant threaded into the write).
+ * Resolving them separately would mean two `getSession` calls that can
+ * disagree.
+ */
+ private async resolveSession(ctx: PluginContext, c: any): Promise<any | undefined> {
  try {
  const authService: any = this.tryGetService<any>(ctx, ['auth']);
  if (!authService) return undefined;
@@ -421,9 +468,7 @@ export class WebhookOutboxPlugin implements Plugin {
  api = await authService.getApi();
  }
  if (!api?.getSession) return undefined;
- const session = await api.getSession({ headers: c.req.raw.headers });
- const uid = session?.user?.id;
- return typeof uid === 'string' && uid.length > 0 ? uid : undefined;
+ return await api.getSession({ headers: c.req.raw.headers });
  } catch {
  return undefined;
  }

packages/plugins/plugin-webhooks/src/webhook-redeliver-tenant-scope.test.ts

@@ -0,0 +1,167 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #10740 — `POST /api/v1/webhooks/redeliver` carries the CALLER'S tenant.
+ *
+ * This route is the reason `SqlHttpOutbox.redeliver` is classified
+ * request-contextual rather than as a dispatcher sweep: its auth gate is "any
+ * authenticated user", and `sys_http_delivery` is a tenant-scoped object. On a
+ * walled deployment (`OS_TENANCY_POSTURE=isolated`) an unscoped replay from
+ * here is an authenticated user writing another organization's delivery row —
+ * exactly what the driver's tenant audit exists to catch, and the one site
+ * where silencing that audit would convert a detectable hole into an
+ * undetectable one.
+ *
+ * ## What this file pins that the service-level test cannot
+ * The tenant has to come from the REQUEST. `service-messaging`'s
+ * `delivery-update-tenant-audit.integration.test.ts` proves the outbox applies
+ * whatever tenant it is handed, right down to the options that reach the
+ * driver; nothing there can prove the route hands it the right one, or hands
+ * it anything at all. Here the session is the only source of the value, so a
+ * route that dropped it would go red.
+ *
+ * It also pins the HTTP half of the ADR-0112 envelope: the service layer
+ * carries the `code`, and the `status` exists only at this boundary. Both are
+ * asserted for the cross-tenant refusal — a `code` assertion alone would not
+ * notice the refusal surfacing as a 500.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { WebhookOutboxPlugin } from './webhook-outbox-plugin.js';
+
+/** Captures the handler `registerAdminRoutes` mounts, and lets us call it. */
+function mountRoute(opts: {
+ session: any;
+ redeliverHttp: (id: string, options: { tenantId: string | undefined }) => Promise<any>;
+}): { post: (body: any) => Promise<{ status: number; json: any }> } {
+ let handler: ((c: any) => Promise<any>) | undefined;
+ const rawApp = {
+ post(path: string, h: (c: any) => Promise<any>) {
+ if (path === '/api/v1/webhooks/redeliver') handler = h;
+ },
+ };
+ const services: Record<string, any> = {
+ 'http-server': { getRawApp: () => rawApp },
+ messaging: {
+ // `getMessaging` gates on this being a function.
+ enqueueHttp: async () => 'unused',
+ isHttpDeliveryReady: () => true,
+ registerRedeliverGuard: () => {},
+ redeliverHttp: opts.redeliverHttp,
+ },
+ auth: { api: { getSession: async () => opts.session } },
+ };
+ const ctx: any = {
+ getService: (n: string) => services[n],
+ logger: { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} },
+ };
+ (new WebhookOutboxPlugin() as any).registerAdminRoutes(ctx);
+ if (!handler) throw new Error('route was not mounted');
+
+ return {
+ async post(body: any) {
+ let status = 200;
+ let json: any;
+ const c = {
+ req: { raw: { headers: new Headers() }, json: async () => body },
+ json(payload: any, s?: number) {
+ json = payload;
+ if (s !== undefined) status = s;
+ return { status, json };
+ },
+ };
+ await handler!(c);
+ return { status, json };
+ },
+ };
+}
+
+/** A better-auth session envelope: `{ user, session }`. */
+const sessionFor = (userId: string, activeOrganizationId?: string) => ({
+ user: { id: userId },
+ session: { userId, ...(activeOrganizationId ? { activeOrganizationId } : {}) },
+});
+
+describe('POST /api/v1/webhooks/redeliver — the caller\'s tenant reaches the outbox (#10740)', () => {
+ it('threads the session\'s active organization into redeliverHttp', async () => {
+ const seen: Array<{ id: string; tenantId: string | undefined }> = [];
+ const route = mountRoute({
+ session: sessionFor('user_1', 'org_a'),
+ async redeliverHttp(id, options) {
+ seen.push({ id, tenantId: options.tenantId });
+ return { id, status: 'pending' };
+ },
+ });
+
+ const res = await route.post({ deliveryId: 'del_1' });
+
+ expect(res.status).toBe(200);
+ expect(res.json).toEqual({ success: true, data: { id: 'del_1', status: 'pending' } });
+ // The whole point: not `undefined`, and not some other org.
+ expect(seen).toEqual([{ id: 'del_1', tenantId: 'org_a' }]);
+ });
+
+ it('refuses a cross-tenant delivery id with RESOURCE_NOT_FOUND and 404', async () => {
+ // The outbox scopes its reads by the tenant it is handed, so a row in
+ // another organization is INVISIBLE rather than forbidden — which is
+ // also what stops this endpoint being an existence oracle for other
+ // tenants' delivery ids.
+ const route = mountRoute({
+ session: sessionFor('user_1', 'org_a'),
+ async redeliverHttp() {
+ const err: any = new Error("Delivery row 'del_other' not found");
+ err.name = 'HttpRedeliverError';
+ err.code = 'RESOURCE_NOT_FOUND';
+ throw err;
+ },
+ });
+
+ const res = await route.post({ deliveryId: 'del_other' });
+
+ // ADR-0112: `code` AND `status`. Either alone passes on a refusal that
+ // surfaced as a 500, or on a 404 carrying the wrong code.
+ expect(res.json?.error?.code).toBe('RESOURCE_NOT_FOUND');
+ expect(res.status).toBe(404);
+ });
+
+ it('threads `undefined` for a session with no active organization — reported, never silenced', async () => {
+ // The honest half. The deployment could not tell the route which
+ // organization is asking, so the write goes out unscoped and the
+ // driver's tenant-audit line fires for it. ⛔ The repair for that is
+ // never `bypassTenantAudit`, which would hide the report and close
+ // nothing — so what this pins is that the route invents no tenant.
+ const seen: Array<string | undefined> = [];
+ const route = mountRoute({
+ session: sessionFor('user_1'),
+ async redeliverHttp(id, options) {
+ seen.push(options.tenantId);
+ return { id, status: 'pending' };
+ },
+ });
+
+ const res = await route.post({ deliveryId: 'del_1' });
+
+ expect(res.status).toBe(200);
+ expect(seen).toEqual([undefined]);
+ });
+
+ it('never reaches the outbox at all for an unauthenticated caller', async () => {
+ // The pre-existing gate, re-pinned because the session lookup was
+ // widened from "user id" to the whole envelope: a widening that lost
+ // the auth check would be invisible to every assertion above.
+ let called = 0;
+ const route = mountRoute({
+ session: null,
+ async redeliverHttp(id) {
+ called += 1;
+ return { id, status: 'pending' };
+ },
+ });
+
+ const res = await route.post({ deliveryId: 'del_1' });
+
+ expect(res.status).toBe(401);
+ expect(res.json?.error?.code).toBe('UNAUTHENTICATED');
+ expect(called).toBe(0);
+ });
+});