diff --git a/.changeset/delivery-headers-privileged-read.md b/.changeset/delivery-headers-privileged-read.md new file mode 100644 index 0000000000..75bfb9efd1 --- /dev/null +++ b/.changeset/delivery-headers-privileged-read.md @@ -0,0 +1,15 @@ +--- +'@objectstack/objectql': minor +'@objectstack/service-messaging': patch +--- + +Stop serving webhook/flow callout credentials through the generic data-API read of `sys_http_delivery` (#8118). + +**What this closes.** `sys_http_delivery.headers_json` — the authored request-header map, the ordinary place an `Authorization: Bearer …` goes — was readable by every caller the data API admits (list, get, an explicit `?select=headers_json`). The column is now declared `internal: true`, so the engine omits it from every generic read with no system carve-out (#7728). The redaction sits at the row layer, so it covers the whole delivery population: `source: 'webhook'` rows (WebhookSchema-authored headers) and `source: 'flow'` rows (per-run interpolated headers that never pass through WebhookSchema at all). Deliveries are unaffected on the wire: the dispatcher's claim path recovers the map through the engine's privileged accessor and still sends every authored header verbatim — fail-closed, a delivery never goes out missing a header, and one that cannot recover its headers refuses loudly instead of going out incomplete. `IHttpOutbox.list()` and `redeliver()` now return the redacted view (`headers: undefined`) under a redacting engine; `claim()` results carry the map verbatim. + +**New public API.** `ObjectQL.resolveInternalField(object, recordIds, field)` — the purpose-built privileged accessor #7728 itself named as the remedy for a legitimate system reader of an `internal: true` field: a batch, driver-level read of one flagged field, refusing (ADR-0112 `INVALID_FIELD`, status 400) any field not so declared. The sibling of `resolveSecretField`, batch-shaped because its consumer claims a batch per dispatcher tick. + +**What this deliberately does NOT close.** + +- The delivery row still holds the header map in cleartext at rest until the 30d telemetry retention ages it out. Encrypting it (`Field.secret()`) was measured and rejected on #8118: one orphan `sys_secret` row per delivery with no cascade or retention, a boot-window fail-open on the fire-and-forget enqueue, and a per-row decrypt on every dispatcher tick. +- `sys_email.headers_json` (#7986 ①-f) has the same shape; it follows this card's decision but is not part of this change. diff --git a/content/docs/automation/webhooks.mdx b/content/docs/automation/webhooks.mdx index d0b9115112..41b3b2601f 100644 --- a/content/docs/automation/webhooks.mdx +++ b/content/docs/automation/webhooks.mdx @@ -141,7 +141,11 @@ at enqueue and replayed byte-for-byte by retries and redelivery, so the HMAC has exactly one correct value: the outbox computes it once at enqueue from the subscriber's `secret` and stores only the result — the same `sha256=` the receiver is handed on the wire. Reading a delivery row tells you what was sent, -not how to forge it. +not how to forge it. The same rule covers the authored headers: `headers_json` +is the ordinary place a credential goes (`Authorization: Bearer …`), so the +column is `internal` — omitted from every generic read, with no system +carve-out — and only the dispatcher recovers it, through the engine's +privileged accessor, when it claims the row for sending. | Field | Type | Notes | |-------------------|----------|-----------------------------------------------------------------------------| @@ -152,7 +156,7 @@ not how to forge it. | `label` | text | Diagnostic label / event type — surfaced on `X-Objectstack-Event`. | | `url` | text | Target URL, snapshotted at enqueue so config edits do not rewrite live rows. | | `method` | text | HTTP method. | -| `headers_json` | textarea | Custom headers, serialised. | +| `headers_json` | textarea | Custom headers, serialised. `internal` — never returned on the generic data path (list, get, an explicit `?select=`); recovered only by the dispatcher's privileged read at claim time. | | `signature` | text | The `X-Objectstack-Signature` value sent with this delivery, `sha256=` — computed at enqueue; the secret is not stored (see §6). | | `timeout_ms` | number | Per-attempt timeout. | | `payload_json` | textarea | The full payload that will be POSTed. | diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index e2ddec9d7b..f6ea2c9c96 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -5158,6 +5158,95 @@ export class ObjectQL implements IObjectQLEngine { return this.resolveSecret(row[field], opts); } + /** + * Privileged: recover the stored values of ONE `internal: true` field for a + * batch of rows, keyed by record id. + * + * [#8118] {@link omitInternalFields} deletes a flagged field from every row + * the engine hands back — with NO system carve-out, by explicit design + * (#7728): an escape hatch nobody needs is a hole in a non-exposure + * guarantee. The same ruling names the shape a legitimate system reader uses + * when one finally appears: it reads the column through a purpose-built + * privileged accessor, the way {@link resolveSecret} does for `secret`. This + * method is that accessor. Its first consumer is the outbound-HTTP + * dispatcher's claim path (`SqlHttpOutbox.claim()` in + * `@objectstack/service-messaging`): `sys_http_delivery.headers_json` — the + * authored header map, the ordinary place an `Authorization: Bearer …` goes — + * is flagged `internal` so the generic data API returns rows without it, + * while the dispatcher must hand exactly that map to the wire VERBATIM (a + * delivery that goes out missing a header is not self-announcing: against an + * endpoint that does not require it, the delivery succeeds while silently + * deviating from the authored configuration). + * + * Batch-shaped, deliberately, where {@link resolveSecretField} is + * single-record: its consumer claims up to a full batch per dispatcher tick, + * and #8118's triage rejected the `Field.secret()` route partly BECAUSE that + * shape costs a driver read plus a decrypt per row per tick. One driver read + * serves the whole claim batch; a single record is the batch of one. This is + * the sibling of {@link resolveSecretField}'s pattern — guard first, then a + * driver-level read — not a second divergent privileged-read path. + * + * The rows are read at DRIVER level on purpose — the only layer where the + * value still exists — so this bypasses read hooks, field-level security and + * sharing: the same trust {@link resolveSecret} and {@link resolveSecretField} + * place in their callers, and the same reason all three are explicit, + * separately-named privileged verbs rather than an option on `find`. No + * query string reaches this, so it cannot be turned on from outside the + * process. + * + * Refuses (ADR-0112 `code` + `status`) any field not declared + * `internal: true` on the object. Without that guard this would be a generic + * read-protection bypass — over `password` plaintext in particular, which is + * masked deliberately (ADR-0100) — rather than the internal channel's + * dereference. A `secret`-typed field is likewise refused unless it is also + * flagged, and even then this returns the stored `secret:` ref, never a + * plaintext: decryption stays with {@link resolveSecretField}. + * + * Returns the stored value per id — `null` when the column is unset. An id + * whose row does not exist is absent from the map; what a missing row means + * belongs to the caller (for the dispatcher: a row deleted mid-claim). No + * decrypt is involved: `internal` is a read-side omission flag, not an + * encrypted channel — the at-rest story is the object's own (for + * `sys_http_delivery`, 30d telemetry retention; encrypting the delivery row + * was measured and rejected on #8118). + */ + async resolveInternalField( + object: string, + recordIds: readonly string[], + field: string, + ): Promise> { + const schema = this._registry.getObject(object); + if (!collectInternalReadFields(schema).includes(field)) { + const err: Error & { code?: string; status?: number; object?: string; field?: string } = + new Error( + `Cannot resolve internal field "${object}.${field}": it is not declared \`internal: true\`. ` + + 'Only fields the engine omits from the generic read path are dereferenceable here — ' + + 'anything else either comes back on find/findOne already, or is protected by its own ' + + 'channel (`secret` refs via resolveSecretField; `password` is masked deliberately, ' + + 'ADR-0100, so dereferencing one here would be a mask bypass).', + ); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.object = object; + err.field = field; + throw err; + } + const out = new Map(); + if (recordIds.length === 0) return out; + const driver = this.getDriver(object); + const found = await driver.find(object, { + where: { id: { $in: [...recordIds] } }, + fields: ['id', field], + }); + for (const row of Array.isArray(found) ? found : [found]) { + if (!row || typeof row !== 'object') continue; + const id = (row as Record).id; + if (typeof id !== 'string' && typeof id !== 'number') continue; + out.set(String(id), (row as Record)[field] ?? null); + } + return out; + } + /** * Helper to get object definition */ diff --git a/packages/objectql/src/internal-fields.test.ts b/packages/objectql/src/internal-fields.test.ts index a9977f1433..b28ebdbd9c 100644 --- a/packages/objectql/src/internal-fields.test.ts +++ b/packages/objectql/src/internal-fields.test.ts @@ -43,6 +43,12 @@ function makeStubDriver() { if (!where || typeof where !== 'object') return true; for (const [k, v] of Object.entries(where)) { if (k.startsWith('$')) continue; + // `$in` — the batch shape `resolveInternalField` reads by (#8118). + if (v && typeof v === 'object' && '$in' in (v as any)) { + const members = (v as any).$in; + if (!Array.isArray(members) || !members.includes(row[k] ?? null)) return false; + continue; + } const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; if ((row[k] ?? null) !== (expected ?? null)) return false; } @@ -421,4 +427,83 @@ describe('#7728: the `internal` field flag omits a value from the generic data p expect(err!.message.match(/itest_api_key\.key/g)).toHaveLength(1); }); }); + + /** + * [#8118] `resolveInternalField` — the purpose-built privileged accessor + * #7728 itself named as the shape a legitimate system reader uses ("it reads + * the column through a purpose-built privileged accessor, the way + * `resolveSecret` does"). The omit above has NO system carve-out, so this is + * the ONLY supported door to a flagged value; its first consumer is the + * outbound-HTTP dispatcher's claim path, which must put + * `sys_http_delivery.headers_json` on the wire verbatim while the data API + * returns rows without it. + * + * Batch-shaped (ids in, `Map` out) because that consumer claims a batch per + * dispatcher tick — #8118's triage rejected `Field.secret()` partly for + * costing a per-row read on that tick, and the accessor must not re-acquire + * the rejected cost. + */ + describe('#8118: resolveInternalField — the privileged dereference', () => { + it('resolves the flagged field for a batch of ids, straight from storage', async () => { + const a = await seed(); + const b = await ctx.engine.insert('itest_api_key', { + name: 'k2', prefix: 'svc_', revoked: false, key: `${HASH}-2`, + }, { context: { isSystem: true } } as any); + + const resolved = await ctx.engine.resolveInternalField('itest_api_key', [a.id, b.id], 'key'); + expect(resolved.get(a.id)).toBe(HASH); + expect(resolved.get(b.id)).toBe(`${HASH}-2`); + expect(resolved.size).toBe(2); + + // …while the generic read path, asked in the same breath, still omits — + // the accessor is a second DOOR, not a hole in the first one. + const viaFind = (await ctx.engine.find('itest_api_key', { where: { id: a.id } }))[0] as any; + expect(Object.keys(viaFind)).not.toContain('key'); + }); + + it('an unset value resolves to null; a missing row is absent from the map', async () => { + const a = await ctx.engine.insert('itest_api_key', { + name: 'k-unset', prefix: 'osk_', revoked: false, key: null, + }, { context: { isSystem: true } } as any); + + const resolved = await ctx.engine.resolveInternalField( + 'itest_api_key', [a.id, 'r_does_not_exist'], 'key', + ); + // Unset ≠ missing: the caller can tell "row exists, nothing stored" + // (null) from "no such row" (absent) — the dispatcher treats the latter + // as a row deleted mid-claim. + expect(resolved.has(a.id)).toBe(true); + expect(resolved.get(a.id)).toBeNull(); + expect(resolved.has('r_does_not_exist')).toBe(false); + }); + + it('an empty batch resolves to an empty map without touching the driver', async () => { + const resolved = await ctx.engine.resolveInternalField('itest_api_key', [], 'key'); + expect(resolved.size).toBe(0); + }); + + it('refuses a field not declared `internal: true` — ADR-0112 code AND status', async () => { + const created = await seed(); + // `prefix` comes back on every find — dereferencing it here is not a + // privilege, and an accessor that allowed it would be a generic + // read-protection bypass one field-name away from `password`. + const err = await ctx.engine.resolveInternalField('itest_api_key', [created.id], 'prefix').then( + () => null, + (e: unknown) => e as Error & { code?: string; status?: number; field?: string }, + ); + expect(err).toBeInstanceOf(Error); + expect(err!.code).toBe('INVALID_FIELD'); + expect(err!.status).toBe(400); + expect(err!.field).toBe('prefix'); + expect(err!.message).toContain('itest_api_key.prefix'); + }); + + it('refuses on an object with no flagged fields at all (guard before fast path)', async () => { + // The guard outranks the empty-ids fast path on purpose: a caller that + // wired the wrong object name hears about it deterministically, not only + // on the first non-empty batch. + await expect(ctx.engine.resolveInternalField('itest_plain', [], 'key')) + .rejects.toMatchObject({ code: 'INVALID_FIELD', status: 400 }); + }); + }); }); diff --git a/packages/services/service-messaging/src/delivery-headers-at-rest.integration.test.ts b/packages/services/service-messaging/src/delivery-headers-at-rest.integration.test.ts new file mode 100644 index 0000000000..df90fbbd50 --- /dev/null +++ b/packages/services/service-messaging/src/delivery-headers-at-rest.integration.test.ts @@ -0,0 +1,310 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8118 — `sys_http_delivery.headers_json` must not be readable over the + * generic data API, and every authored header must still reach the wire + * VERBATIM. + * + * The defect: #8114 moved the authored `headers` map onto the encrypted + * channel of the CONFIGURATION row (`sys_webhook.headers_secret`), but the + * enqueuer decrypts that map and `SqlHttpOutbox.enqueue()` writes it verbatim + * into the DELIVERY row — a table the ordinary data API reads + * (`enable.apiMethods: ['get', 'list']`). "Webhook headers are no longer in a + * blob" read as true after #8114 and was not: the same exposure, one layer + * down. + * + * The fix under test (route 3, ruled on #8118): `headers_json` is declared + * `internal: true`, so the ENGINE omits it from every generic read with no + * system carve-out (#7728), and the dispatcher's claim path — the one reader + * that must see the map — recovers it through ObjectQL's purpose-built + * privileged accessor (`resolveInternalField`, the remedy #7728 itself names). + * + * These tests run the REAL storage path — `ObjectQL` + `@objectstack/driver-sql` + * on better-sqlite3 `:memory:`, the production `SqlHttpOutbox`, the production + * `HttpDispatcher` — the same harness as the #7722 signing-secret pins, because + * both halves of every claim live in that hand-off: what the data API serves, + * and what a real dispatcher put on the wire after reloading the row. + * + * Population coverage is deliberate and explicit: + * + * - **`source: 'webhook'`** — headers authored through `WebhookSchema`, + * decrypted by the enqueuer, handed to `enqueueHttp`. + * - **`source: 'flow'`** — a flow `http` node's headers are interpolated PER + * RUN from run-scoped variables and never pass through `WebhookSchema` at + * all; this is the half every author-declared shape forgets (#8118 rejected + * route 2 for exactly that). The redaction sits at the ROW layer, so it + * catches this population by construction — and the flow case below drives + * it end-to-end through the production enqueue → claim → send path, with + * the exact input shape `http-nodes.ts` produces. + * + * Every redaction pin is paired with its wire pin: a fix that merely DELETED + * the headers would pass the read-path assertions and break every + * authenticated callout in production. The fail-closed rule is pinned on its + * own: a delivery that cannot recover its headers must not go out without + * them — missing headers are not self-announcing (the receiver that does not + * require one answers 200 to an incomplete request). + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { createHmac } from 'node:crypto'; +import { ObjectQL, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { SqlHttpOutbox } from './sql-http-outbox.js'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { HttpDelivery, SYS_HTTP_DELIVERY } from './objects/http-delivery.object.js'; +import type { FetchImpl } from './http-sender.js'; + +/** A credential a real deployment would put in `headers`. Distinctive on purpose. */ +const BEARER = 'Bearer prod_tok_8118_do_not_serve'; +/** The flow half's credential — "interpolated per run", so a run-scoped value. */ +const FLOW_BEARER = 'Bearer run_scoped_tok_8118_run_42'; +/** Signing secret — proves the #7799/#7722 half is untouched by this change. */ +const SECRET = 'whsec_8118_signing_untouched'; + +function makeSqliteDriver() { + return new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); +} + +/** Records what actually went on the wire. */ +function makeFetch(): { + impl: FetchImpl; + calls: Array<{ url: string; headers: Record; body: string }>; +} { + const calls: Array<{ url: string; headers: Record; body: string }> = []; + const impl: FetchImpl = async (url, init) => { + calls.push({ url, headers: init.headers, body: init.body }); + return { ok: true, status: 204, async text() { return ''; } }; + }; + return { impl, calls }; +} + +describe('sys_http_delivery — authored headers vs the data API (#8118)', () => { + let engine: ObjectQL | undefined; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = undefined; + }); + + async function boot() { + const driver = makeSqliteDriver(); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(HttpDelivery as any, '@objectstack/service-messaging'); + // Real DDL through the real path — physical columns exactly as prod. + await engine.syncSchemas(); + return { driver, engine: engine! }; + } + + /** Every value in every column of every delivery row, as one string. */ + async function scanDeliveryTable(driver: SqlDriver): Promise { + const rows: Array> = await driver.getKnex()(SYS_HTTP_DELIVERY).select('*'); + expect(rows.length).toBeGreaterThan(0); // a scan of nothing proves nothing + return rows.map((r) => Object.values(r).map((v) => String(v ?? '')).join(' ')).join(' '); + } + + it('a webhook-source delivery: redacted on the data API, verbatim on the wire', async () => { + const { driver, engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + + await outbox.enqueue({ + source: 'webhook', + refId: 'wh_1', + dedupKey: 'crm_account:acc_1:created:1760000000000', + label: 'data.record.created', + url: 'https://receiver.example/hook', + headers: { Authorization: BEARER, 'X-Team': 'crm' }, + signingSecret: SECRET, + payload: { object: 'crm_account', recordId: 'acc_1', action: 'created' }, + }); + + // ── The read path an ordinary GET /api/v1/data/sys_http_delivery + // takes (`enable.apiMethods: ['get','list']` route through here) ── + const viaApi = await eng.find(SYS_HTTP_DELIVERY, {}); + expect(viaApi).toHaveLength(1); + // The KEY is absent — omitted, not masked, not nulled (#7728 (b)). + expect(Object.keys(viaApi[0] as any)).not.toContain('headers_json'); + expect(JSON.stringify(viaApi)).not.toContain(BEARER); + // Falsifiability: this is the real row, not an emptied one. + expect((viaApi[0] as any).url).toBe('https://receiver.example/hook'); + + // An EXPLICIT projection naming the column does not bypass the omit — + // `?select=headers_json` is served without it, not refused (#7823). + const named = await eng.find(SYS_HTTP_DELIVERY, { fields: ['id', 'url', 'headers_json'] }); + expect(Object.keys(named[0] as any)).not.toContain('headers_json'); + expect((named[0] as any).url).toBe('https://receiver.example/hook'); + + // The service-level admin surface narrows too: `list()` is not a + // dispatch path and returns the redacted view. + expect(JSON.stringify(await outbox.list())).not.toContain(BEARER); + + // ── The wire: every authored header arrives verbatim ── + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + expect(calls).toHaveLength(1); + expect(calls[0].headers.Authorization).toBe(BEARER); + expect(calls[0].headers['X-Team']).toBe('crm'); + // …and the signing half is byte-identical (#7799 / #7722 untouched): + // verified the way a receiver verifies, not by header presence. + const expected = createHmac('sha256', SECRET).update(calls[0].body).digest('hex'); + expect(calls[0].headers['X-Objectstack-Signature']).toBe(`sha256=${expected}`); + + // ── At rest the bytes are still there, and that is THIS CARD's ruled + // posture, stated so the next reader is not misled: #8118 narrows the + // READ surface only. `Field.secret()` (encrypt-at-rest) was measured + // and rejected — orphan `sys_secret` row per delivery, boot-window + // fail-open, per-tick decrypt; the row ages out via 30d retention. A + // future card that changes the at-rest story flips this pin + // deliberately, not by accident. + const dump = await scanDeliveryTable(driver); + expect(dump).toContain(BEARER); + expect(dump).toContain('https://receiver.example/hook'); // guard the guard + }); + + it("a flow-source delivery — the half every author-declared shape forgets — gets the same pair", async () => { + const { engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + + // Exactly the input shape `http-nodes.ts` produces for a durable flow + // `http` node: `source: 'flow'`, `refId` = node id, a per-run dedup + // key, and headers already interpolated from run-scoped variables — + // they never passed through `WebhookSchema`, so no author-declared + // sensitive-header list could ever have covered them. + await outbox.enqueue({ + source: 'flow', + refId: 'node_http_1', + dedupKey: 'run_42:node_http_1', + label: 'flow:node_http_1', + url: 'https://receiver.example/flow', + method: 'POST', + headers: { Authorization: FLOW_BEARER, 'X-Run-Id': 'run_42' }, + payload: { hello: 'world' }, + }); + + // Redacted on the generic read path — the row layer does not care + // which producer wrote the map, which is WHY route 3 covers the whole + // population. + const viaApi = await eng.find(SYS_HTTP_DELIVERY, {}); + expect(Object.keys(viaApi[0] as any)).not.toContain('headers_json'); + expect(JSON.stringify(viaApi)).not.toContain(FLOW_BEARER); + + // …and delivered verbatim, per-run values intact. + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + expect(calls).toHaveLength(1); + expect(calls[0].headers.Authorization).toBe(FLOW_BEARER); + expect(calls[0].headers['X-Run-Id']).toBe('run_42'); + }); + + it('a redelivered row still carries its headers — the privileged read serves every attempt', async () => { + const { engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + + const id = await outbox.enqueue({ + source: 'webhook', + refId: 'wh_1', + dedupKey: 'once', + url: 'https://receiver.example/hook', + headers: { Authorization: BEARER }, + signingSecret: SECRET, + payload: { a: 1 }, + }); + + const first = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: first.impl, partitionCount: 1 }).tick(); + expect(first.calls[0].headers.Authorization).toBe(BEARER); + + // `redeliver()` itself returns the REDACTED view (it is an admin verb, + // not a dispatch path)… + const redelivered = await outbox.redeliver(id); + expect(redelivered.status).toBe('pending'); + expect(redelivered.headers).toBeUndefined(); + + // …but the re-send goes through claim, which recovers the map: the + // wire request is byte-identical to the first attempt. + const second = makeFetch(); + await new HttpDispatcher({ nodeId: 'n2', outbox, fetchImpl: second.impl, partitionCount: 1 }).tick(); + expect(second.calls).toHaveLength(1); + expect(second.calls[0].headers.Authorization).toBe(BEARER); + expect(second.calls[0].body).toBe(first.calls[0].body); + expect(second.calls[0].headers['X-Objectstack-Signature']) + .toBe(first.calls[0].headers['X-Objectstack-Signature']); + }); + + it('a delivery authored WITHOUT headers still delivers — recovery of nothing is not a failure', async () => { + const { engine: eng } = await boot(); + const outbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + await outbox.enqueue({ + source: 'flow', + refId: 'n1', + dedupKey: 'plain', + url: 'https://receiver.example/plain', + payload: { a: 1 }, + }); + + const { impl, calls } = makeFetch(); + await new HttpDispatcher({ nodeId: 'n1', outbox, fetchImpl: impl, partitionCount: 1 }).tick(); + + expect(calls).toHaveLength(1); + // Standard transport headers only — no authored ones, and no artifact + // of the hydration (`undefined`/`null` never becomes a header value). + expect(calls[0].headers.Authorization).toBeUndefined(); + expect(calls[0].headers['Content-Type']).toBe('application/json'); + }); + + it('FAIL-CLOSED: a redacting engine that cannot dereference refuses the claim — nothing goes out bare', async () => { + const { engine: eng } = await boot(); + const realOutbox = new SqlHttpOutbox(eng as any, { partitionCount: 1 }); + await realOutbox.enqueue({ + source: 'webhook', + refId: 'wh_1', + dedupKey: 'guarded', + url: 'https://receiver.example/hook', + headers: { Authorization: BEARER }, + payload: { a: 1 }, + }); + + // An engine that REDACTS (real ObjectQL find/getSchema underneath, so + // `headers_json` is both flagged and omitted) but exposes no + // `resolveInternalField`. Not a shape ObjectQL can produce — the flag + // and the accessor ship together — but exactly the shape a foreign or + // partially-faked engine would take, and the one combination in which + // "keep going" means delivering a request that silently deviates from + // its authored configuration. + const noAccessor = { + find: (o: string, q: unknown) => (eng as any).find(o, q), + findOne: (o: string, q: unknown) => (eng as any).findOne(o, q), + insert: (o: string, d: unknown) => (eng as any).insert(o, d), + update: (o: string, d: unknown, q: unknown) => { + // Engine-double contract: this fake's update() must be exactly + // as strict as ObjectQL.update's dispatch — it delegates to the + // real engine, but the pin is asserted up front so the gate can + // see it (check:engine-double-contract). + assertEngineUpdateDispatch(d as any, q as any); + return (eng as any).update(o, d, q); + }, + getSchema: (o: string) => (eng as any).getSchema(o), + } as unknown as IDataEngine; + const blindOutbox = new SqlHttpOutbox(noAccessor, { partitionCount: 1 }); + + const { impl, calls } = makeFetch(); + const dispatcher = new HttpDispatcher({ nodeId: 'n1', outbox: blindOutbox, fetchImpl: impl, partitionCount: 1 }); + await expect(dispatcher.tick()).rejects.toThrow(/resolveInternalField/); + // The delivery did NOT go out missing its headers. + expect(calls).toHaveLength(0); + + // No work is lost either: the claim marked the row in_flight before + // refusing, and the visibility timeout hands it back to a HEALTHY + // claimer, headers intact. + const later = Date.now() + 60_000; + const recovered = await realOutbox.claim({ nodeId: 'n2', limit: 10, claimTtlMs: 1_000, now: later }); + expect(recovered).toHaveLength(1); + expect(recovered[0].headers).toEqual({ Authorization: BEARER }); + }); +}); diff --git a/packages/services/service-messaging/src/http-outbox.ts b/packages/services/service-messaging/src/http-outbox.ts index f51a2ebd3e..199d2b5978 100644 --- a/packages/services/service-messaging/src/http-outbox.ts +++ b/packages/services/service-messaging/src/http-outbox.ts @@ -55,7 +55,20 @@ export interface HttpDelivery { url: string; /** HTTP method — defaults to POST. */ method?: string; - /** Custom headers. */ + /** + * Custom headers — the ordinary place a credential (`Authorization: + * Bearer …`) goes, whichever producer authored them (a `WebhookSchema` + * `headers` map, or a flow `http` node's per-run interpolated values). + * + * [#8118] On engine-backed storage the row column (`headers_json`) is + * declared `internal: true`, so the generic data path never returns it. + * Consequence for THIS field: `claim()` results carry the map VERBATIM — + * the dispatch path is fail-closed, a delivery never goes out missing an + * authored header — while `list()` / `redeliver()` results are the + * redacted view (`headers: undefined`) under a redacting engine. The + * in-memory outbox stores no engine-readable row, so it has nothing to + * redact. + */ headers?: Record; /** * Pre-computed `X-Objectstack-Signature` value (`sha256=`), or absent @@ -190,13 +203,24 @@ export interface IHttpOutbox { * Atomically claim up to `limit` rows whose `nextRetryAt <= now` (or null) * and matching the partition predicate. Claimed rows MUST be marked * `in_flight` so concurrent claimers don't see them. + * + * [#8118] Claim results MUST carry {@link HttpDelivery.headers} verbatim — + * this is the dispatch path, and a delivery must never go out missing an + * authored header. An implementation whose storage redacts the column + * recovers it through a privileged read (see `SqlHttpOutbox`) or fails the + * claim loudly; it must not return the row with the map silently absent. */ claim(opts: HttpClaimOptions): Promise; /** Record the outcome of an attempt. */ ack(id: string, result: HttpAckResult): Promise; - /** Snapshot accessor for tests / admin tooling. */ + /** + * Snapshot accessor for tests / admin tooling. [#8118] Not a dispatch + * path: under a redacting engine the rows come back WITHOUT + * {@link HttpDelivery.headers} (the redacted view), and callers must not + * expect the map here. + */ list(filter?: { status?: HttpDeliveryStatus; source?: string }): Promise; /** diff --git a/packages/services/service-messaging/src/objects/http-delivery.object.ts b/packages/services/service-messaging/src/objects/http-delivery.object.ts index 0d2577b10f..7cd0bd0ae8 100644 --- a/packages/services/service-messaging/src/objects/http-delivery.object.ts +++ b/packages/services/service-messaging/src/objects/http-delivery.object.ts @@ -123,7 +123,35 @@ export const HttpDelivery = ObjectSchema.create({ }), method: Field.text({ label: 'Method', required: false, maxLength: 10 }), - headers_json: Field.textarea({ label: 'Headers JSON', required: false }), + // [#8118] `internal: true` — the authored header map is the ordinary + // place an `Authorization: Bearer …` goes (#7986), and this table is + // readable over the ordinary data API (`enable.apiMethods` below), so + // the engine OMITS the column from every generic read: list, get, an + // explicit `?select=headers_json`, the write-response bodies — with no + // system carve-out (#7728's explicit design). The redaction sits at + // the ROW layer on purpose: it covers `source: 'webhook'` rows + // (WebhookSchema-authored headers) and `source: 'flow'` rows (per-run + // interpolated headers that never pass through WebhookSchema) alike. + // The dispatcher — the one reader that must hand the map to the wire + // VERBATIM — reads it back through ObjectQL's purpose-built privileged + // accessor (`resolveInternalField`, the remedy #7728 itself names) on + // the claim path; see `SqlHttpOutbox.claim()`. + // + // Read-side only, deliberately. Storage is untouched: the row still + // carries the map in cleartext until the 30d telemetry retention above + // ages it out. `Field.secret()` was measured and REJECTED on #8118 — + // one orphan `sys_secret` row per delivery with no cascade or + // retention, a boot-window fail-open on the fire-and-forget enqueue, + // and a per-row decrypt on every dispatcher tick. + headers_json: Field.textarea({ + label: 'Headers JSON', + required: false, + internal: true, + description: + 'Authored request headers for this delivery — the ordinary place a credential goes, ' + + 'so never returned on the generic data path (#8118). The dispatcher recovers it ' + + "through the engine's privileged accessor on the claim path.", + }), // [#7722] The SIGNATURE, not the key that produced it. This column used // to be `signing_secret` — a verbatim copy of the subscriber's shared // secret on every attempt row — and this table is readable over the diff --git a/packages/services/service-messaging/src/sql-http-outbox.ts b/packages/services/service-messaging/src/sql-http-outbox.ts index 4e13992c39..909f88ee15 100644 --- a/packages/services/service-messaging/src/sql-http-outbox.ts +++ b/packages/services/service-messaging/src/sql-http-outbox.ts @@ -26,6 +26,22 @@ export interface SqlHttpOutboxOptions { objectName?: string; } +/** + * [#8118] Engines that expose the privileged internal-field dereference + * (ObjectQL — the sibling of the `resolveSecretField` probe in + * `plugin-webhooks/webhook-headers.ts`). Structural on purpose: this package + * depends only on the `IDataEngine` contract, and a minimal fake engine that + * implements neither method is an engine whose `find` does not redact either. + */ +type InternalFieldResolvingEngine = IDataEngine & { + resolveInternalField?( + object: string, + recordIds: readonly string[], + field: string, + ): Promise>; + getSchema?(objectName: string): unknown; +}; + interface DeliveryRow { id: string; source: string; @@ -177,7 +193,63 @@ export class SqlHttpOutbox implements IHttpOutbox { where: { id: { $in: ids }, claimed_by: opts.nodeId, claimed_at: now, status: 'in_flight' }, })) as DeliveryRow[]; - return claimed.map((r) => this.toDelivery(r)); + // 5. [#8118] Recover the redacted header column for the rows this + // claim now owns — the one read that must see the authored map. + const headerColumns = await this.readClaimedHeaderColumns(claimed.map((r) => r.id)); + + return claimed.map((r) => this.toDelivery(r, headerColumns)); + } + + /** + * [#8118] Recover `headers_json` for a batch of just-claimed rows. + * + * `sys_http_delivery.headers_json` is declared `internal: true`, so the + * generic read path — including the step-4 read-back above — hands rows + * back WITHOUT it, for every caller, with no system carve-out (#7728's + * explicit design). The dispatcher is the one reader that must see the + * authored map verbatim: a delivery that goes out MISSING a header is not + * self-announcing — against an endpoint that does not require it (a + * routing `X-Tenant-Id`, an `X-Environment: staging`) the delivery + * SUCCEEDS while silently deviating from the authored configuration, and + * nothing records that it went out incomplete. So the claim path reads the + * column back through the engine's purpose-built privileged accessor + * (`resolveInternalField` — the remedy #7728 itself names), one driver + * read per claim batch, never per row. + * + * Returns `undefined` — "use the row's own value" — when nothing redacts: + * - an object whose `headers_json` is not flagged `internal` has nothing + * withheld (a custom `objectName` override without the marking), and + * - an engine that exposes no schema at all is a minimal fake whose + * `find` does not omit the column either. + * + * The one combination this must NOT survive silently is "the column is + * flagged, rows were claimed, and the engine cannot dereference": headers + * are stored but unrecoverable, and delivering without them is the exact + * fail-closed violation above. That combination THROWS — the same + * discipline as `resolveWebhookHeaders` in plugin-webhooks (drop the + * delivery attempt loudly, never deliver incomplete) — and the claimed + * rows revert to `pending` via the claim TTL instead of going out bare. + */ + private async readClaimedHeaderColumns(ids: string[]): Promise | undefined> { + if (ids.length === 0) return undefined; + const engine = this.engine as InternalFieldResolvingEngine; + const schema = typeof engine.getSchema === 'function' + ? (engine.getSchema(this.objectName) as + | { fields?: Record } + | undefined) + : undefined; + // Strict `=== true`, matching the engine's own collector — a truthy- + // but-not-true value does not enrol a field in the redaction either. + if (schema?.fields?.headers_json?.internal !== true) return undefined; + if (typeof engine.resolveInternalField !== 'function') { + throw new Error( + `SqlHttpOutbox.claim: ${this.objectName}.headers_json is declared \`internal: true\`, ` + + 'but this data engine does not implement resolveInternalField() — stored headers ' + + 'cannot be recovered, and a delivery must not go out missing the headers it was ' + + 'authored with (#8118). The claimed rows revert to pending via the claim TTL.', + ); + } + return engine.resolveInternalField(this.objectName, ids, 'headers_json'); } async ack(id: string, result: HttpAckResult): Promise { @@ -264,7 +336,18 @@ export class SqlHttpOutbox implements IHttpOutbox { return this.toDelivery(after); } - private toDelivery(r: DeliveryRow): HttpDelivery { + private toDelivery(r: DeliveryRow, headerColumns?: Map): HttpDelivery { + // [#8118] On the claim path the read-back row no longer carries + // `headers_json` (`internal: true`) — the value arrives through the + // privileged per-batch read instead. Parse semantics are IDENTICAL for + // both sources on purpose: this is one column with two doors, not two + // formats. Rows materialised WITHOUT the map (`list()`, `redeliver()`) + // yield `headers: undefined` under a redacting engine — the redacted + // view, which is the surface narrowing #8118 rules; the dispatcher + // never sends from those. + const headersJson = headerColumns + ? (headerColumns.get(r.id) as string | null | undefined) + : r.headers_json; return { id: r.id, source: r.source, @@ -273,7 +356,7 @@ export class SqlHttpOutbox implements IHttpOutbox { label: r.label ?? undefined, url: r.url, method: r.method ?? undefined, - headers: r.headers_json ? JSON.parse(r.headers_json) : undefined, + headers: headersJson ? JSON.parse(headersJson) : undefined, signature: r.signature ?? undefined, timeoutMs: r.timeout_ms ?? undefined, payload: JSON.parse(r.payload_json), diff --git a/packages/services/service-messaging/src/translations/en.objects.generated.ts b/packages/services/service-messaging/src/translations/en.objects.generated.ts index 1c95aa807b..53f78ee43c 100644 --- a/packages/services/service-messaging/src/translations/en.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/en.objects.generated.ts @@ -327,7 +327,8 @@ export const enObjects: NonNullable = { label: "Method" }, headers_json: { - label: "Headers JSON" + label: "Headers JSON", + help: "Authored request headers for this delivery — the ordinary place a credential goes, so never returned on the generic data path (#8118). The dispatcher recovers it through the engine's privileged accessor on the claim path." }, signature: { label: "HMAC Signature", diff --git a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts index 5b13078504..e5e3b4faf1 100644 --- a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts @@ -327,7 +327,8 @@ export const esESObjects: NonNullable = { label: "Method" }, headers_json: { - label: "Headers JSON" + label: "Headers JSON", + help: "Authored request headers for this delivery — the ordinary place a credential goes, so never returned on the generic data path (#8118). The dispatcher recovers it through the engine's privileged accessor on the claim path." }, signature: { label: "HMAC Signature", diff --git a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts index f336aae431..62d9f42e80 100644 --- a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts @@ -327,7 +327,8 @@ export const jaJPObjects: NonNullable = { label: "Method" }, headers_json: { - label: "Headers JSON" + label: "Headers JSON", + help: "Authored request headers for this delivery — the ordinary place a credential goes, so never returned on the generic data path (#8118). The dispatcher recovers it through the engine's privileged accessor on the claim path." }, signature: { label: "HMAC Signature", diff --git a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts index b678bb3f54..82922a5251 100644 --- a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts @@ -327,7 +327,8 @@ export const zhCNObjects: NonNullable = { label: "请求方法" }, headers_json: { - label: "请求头 JSON" + label: "请求头 JSON", + help: "Authored request headers for this delivery — the ordinary place a credential goes, so never returned on the generic data path (#8118). The dispatcher recovers it through the engine's privileged accessor on the claim path." }, signature: { label: "HMAC 签名",