From b3dad3f7aa7ddd40ad26839cf5f92c0454b29e2f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 13:29:29 +0000 Subject: [PATCH 1/2] fix(platform-objects,plugin-email): stop serving sys_email.headers_json on the generic read path internal: true on the column plus a privileged readback seam on the delivery paths, adopting the remedy #8118 ruled and PR #8348 landed. Refs #8149 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH --- .../src/audit/sys-email.object.ts | 40 ++- packages/plugins/plugin-email/package.json | 1 + ...email-headers-internal.integration.test.ts | 256 ++++++++++++++++++ .../plugins/plugin-email/src/email-plugin.ts | 11 + .../plugins/plugin-email/src/email-service.ts | 37 ++- packages/plugins/plugin-email/src/index.ts | 9 + .../src/internal-header-readback.ts | 152 +++++++++++ pnpm-lock.yaml | 3 + 8 files changed, 506 insertions(+), 3 deletions(-) create mode 100644 packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts create mode 100644 packages/plugins/plugin-email/src/internal-header-readback.ts diff --git a/packages/platform-objects/src/audit/sys-email.object.ts b/packages/platform-objects/src/audit/sys-email.object.ts index 33114c60be..073cee51bf 100644 --- a/packages/platform-objects/src/audit/sys-email.object.ts +++ b/packages/platform-objects/src/audit/sys-email.object.ts @@ -115,13 +115,51 @@ export const SysEmail = ObjectSchema.create({ // part of the message the row cannot carry is therefore a part a durable // delivery silently drops — which is why messages with headers or // attachments used to be pushed back onto inline delivery instead. + // [#8149] `internal: true` — custom headers are the ordinary place a + // credential goes (an SMTP relay's `Authorization`, a provider token, a + // routing secret), the same shape #8118 ruled on for + // `sys_http_delivery.headers_json`, 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`, and the write-response bodies — with no system + // carve-out (#7728's explicit design; `SYSTEM_CTX` does not reopen it). + // + // The redaction sits at the ROW layer, so it covers every producer of the + // column by construction, not just the one that exists today + // (`IEmailService.send` → `encodeHeadersForRow`). Unlike + // `sys_http_delivery`, `sys_email` has no second authoring population to + // cover — `apiMethods` admits no `create`, so the only writer is the mail + // service's own persistence seam — but the placement means an in-process + // writer added later inherits the protection instead of having to + // re-declare it. + // + // Delivery is unaffected: the durable delivery paths (queue worker, boot + // outbox sweep, the after-insert drain hook) all re-read the row and hand + // it to `EmailService.deliverPersistedRow`, which recovers this column + // through ObjectQL's privileged accessor (`resolveInternalField`, the + // remedy #7728 named and #8118 landed) — see + // `plugin-email/src/internal-header-readback.ts`. Fail-closed: a message + // whose authored headers cannot be recovered is NOT sent without them (a + // header that silently goes missing is not self-announcing — the receiver + // that does not require it accepts the mail while the delivery deviates + // from the authored configuration). + // + // Read-side only, deliberately, exactly as #8118 ruled for the sibling + // column: storage is untouched and the row still carries the map in + // cleartext. `Field.secret()` was measured and REJECTED there (an orphan + // `sys_secret` row per delivery with no cascade or retention, a + // boot-window fail-open, a per-row decrypt on every tick); this card + // adopts that decision rather than re-deciding it. headers_json: Field.textarea({ label: 'Headers (JSON)', required: false, + internal: true, description: 'Custom headers supplied to IEmailService.send, as a JSON object of name → value. ' + 'Written in both delivery modes (it is audit evidence as much as delivery input). ' - + 'Absent on rows written before this column existed, which read back as "no custom headers".', + + 'Absent on rows written before this column existed, which read back as "no custom headers". ' + + 'Never returned on the generic data path (#8149) — headers are the ordinary place a ' + + 'credential goes; the delivery paths recover it through the engine\'s privileged accessor.', group: 'Content', }), diff --git a/packages/plugins/plugin-email/package.json b/packages/plugins/plugin-email/package.json index d71f26ff14..594dd02e20 100644 --- a/packages/plugins/plugin-email/package.json +++ b/packages/plugins/plugin-email/package.json @@ -25,6 +25,7 @@ "nodemailer": "^9.0.3" }, "devDependencies": { + "@objectstack/driver-memory": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/service-queue": "workspace:*", "@objectstack/service-settings": "workspace:*", diff --git a/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts b/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts new file mode 100644 index 0000000000..042774ec7b --- /dev/null +++ b/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts @@ -0,0 +1,256 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8149 — `sys_email.headers_json` must not be readable over the generic data + * API, and every authored header must still reach the transport VERBATIM. + * + * The gap: the #5172/#5177 attachments/headers work gave `sys_email` a + * `headers_json` column so a QUEUED message could carry its custom headers + * durably — and custom headers are the ordinary place a credential goes (an + * SMTP relay's `Authorization`, a provider token). `sys_email` is readable + * over the ordinary data API (`enable.apiMethods: ['get', 'list']`), so the + * column served those credentials to every caller the data API admits. Same + * shape as `sys_http_delivery.headers_json`; this card adopts the remedy + * #8118 ruled and PR #8348 landed rather than deciding it a second time. + * + * The fix under test: `headers_json` is declared `internal: true`, so the + * ENGINE omits it from every generic read with no system carve-out (#7728), + * and the delivery paths — the readers that must see the map — recover it + * through ObjectQL's purpose-built privileged accessor + * (`resolveInternalField`, the remedy #7728 named). + * + * These run the REAL engine — `ObjectQL` + `@objectstack/driver-memory`, the + * real `SysEmail` schema, the real `EmailService` with the same persistence + * seam `EmailServicePlugin` wires — because both halves of this card live in + * one hand-off: what the data API serves, and what the transport was actually + * handed after the row was re-read. A fake engine could not show the strip at + * all, and the strip is the fix. + * + * Every redaction pin is PAIRED with its wire pin. That pairing is the point: + * a "fix" that merely dropped the headers would satisfy every read-path + * assertion here and silently break every authenticated relay in production — + * a missing header is not self-announcing, so the mail is accepted while the + * delivery deviates from the authored configuration. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { InMemoryDriver } from '@objectstack/driver-memory'; +import { SysEmail } from '@objectstack/platform-objects/audit'; +import { EmailService, type EmailPersistence } from './email-service.js'; +import { readInternalHeadersJson, isHeadersColumnRedacted } from './internal-header-readback.js'; +import type { IEmailTransport, TransportSendResult } from '@objectstack/spec/contracts'; + +/** A credential a real deployment would put in `headers`. Distinctive on purpose. */ +const RELAY_TOKEN = 'Bearer prod_relay_tok_8149_do_not_serve'; + +const SYSTEM_CTX = { isSystem: true, userId: 'system' } as any; + +/** Records exactly what the transport was handed. */ +function recordingTransport(): { + transport: IEmailTransport; + sent: Array< { subject: string; headers?: Record< string, string > } >; +} { + const sent: Array< { subject: string; headers?: Record< string, string > } > = []; + const transport: IEmailTransport = { + async send(message): Promise< TransportSendResult > { + sent.push({ subject: message.subject, headers: message.headers }); + return { messageId: `< sent-${sent.length}@test >` }; + }, + }; + return { transport, sent }; +} + +describe('sys_email.headers_json — authored headers vs the data API (#8149)', () => { + let engine: ObjectQL | undefined; + + beforeEach(async () => { + engine = new ObjectQL(); + engine.registerDriver(new InMemoryDriver({}) as any, true); + await engine.init(); + engine.registry.registerObject(SysEmail as any, '@objectstack/platform-objects'); + }); + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = undefined; + }); + + /** The persistence seam EmailServicePlugin wires, verbatim in shape. */ + function persistenceFor(eng: ObjectQL): EmailPersistence { + return { + async insert(row) { + const created: any = await (eng as any).insert('sys_email', row, { context: SYSTEM_CTX }); + return created?.id ? { id: String(created.id) } : { id: String(row.id) }; + }, + async update(id, patch) { + await (eng as any).update('sys_email', { id, ...patch }, { context: SYSTEM_CTX }); + }, + async readHeadersJson(rowIds) { + return readInternalHeadersJson(eng as any, rowIds); + }, + }; + } + + async function insertQueuedRow(eng: ObjectQL, headers?: Record) { + const created: any = await (eng as any).insert('sys_email', { + id: `em_${Math.random().toString(36).slice(2, 10)}`, + from_address: 'noreply@example.com', + to_addresses: 'user@example.com', + subject: 'Quarterly report', + body_text: 'hello', + status: 'queued', + ...(headers ? { headers_json: JSON.stringify(headers) } : {}), + created_at: new Date().toISOString(), + }, { context: SYSTEM_CTX }); + return String(created.id); + } + + it('the column is declared internal — the seam probe agrees with the engine', () => { + // The probe the delivery path uses to decide "did the engine redact?" is + // the SCHEMA FLAG, never the absence of the key from a row. Pinned here + // because the distinction is what keeps this seam safe on an OPTIONAL + // column: `headers_json` is `required: false`, so a row legitimately + // lacking it is the ordinary case, not evidence of a strip. + expect(isHeadersColumnRedacted(engine as any)).toBe(true); + expect((SysEmail as any).fields.headers_json.internal).toBe(true); + expect((SysEmail as any).fields.headers_json.required).toBe(false); + }); + + it('a queued message: redacted on the data API, verbatim to the transport', async () => { + const eng = engine!; + const rowId = await insertQueuedRow(eng, { Authorization: RELAY_TOKEN, 'X-Campaign': 'q3' }); + + // ── The read path an ordinary GET /api/v1/data/sys_email takes ── + const viaApi: any[] = await (eng as any).find('sys_email', {}); + expect(viaApi).toHaveLength(1); + // The KEY is absent — omitted, not masked, not nulled (#7728 (b)). + expect(Object.keys(viaApi[0])).not.toContain('headers_json'); + expect(JSON.stringify(viaApi)).not.toContain(RELAY_TOKEN); + // Falsifiability: this is the real row, not an emptied one. + expect(viaApi[0].subject).toBe('Quarterly report'); + + // An EXPLICIT projection naming the column does not bypass the omit — + // `?select=headers_json` is served without it, not refused (#7823). + const named: any[] = await (eng as any).find('sys_email', { + fields: ['id', 'subject', 'headers_json'], + }); + expect(Object.keys(named[0])).not.toContain('headers_json'); + expect(named[0].subject).toBe('Quarterly report'); + + // …and SYSTEM context does not reopen it: #7728 has no system carve-out, + // which is exactly why the delivery paths need the accessor rather than + // an elevated read. + const viaSystem: any[] = await (eng as any).find('sys_email', { context: SYSTEM_CTX }); + expect(Object.keys(viaSystem[0])).not.toContain('headers_json'); + + // ── The wire: every authored header reaches the transport verbatim ── + const { transport, sent } = recordingTransport(); + const svc = new EmailService({ transport, persistence: persistenceFor(eng) }); + // Deliver the way the durable paths do: re-read the row (redacted!), then + // hand THAT row to deliverPersistedRow. + const row = (await (eng as any).find('sys_email', { + where: { id: rowId }, limit: 1, context: SYSTEM_CTX, + }))[0]; + expect(Object.keys(row)).not.toContain('headers_json'); // the input really is stripped + const result = await svc.deliverPersistedRow(row); + + expect(result.status).toBe('sent'); + expect(sent).toHaveLength(1); + expect(sent[0].headers).toEqual({ Authorization: RELAY_TOKEN, 'X-Campaign': 'q3' }); + }); + + it('a message authored WITHOUT headers still delivers — the optional-column trap', async () => { + // The regression PR #8675 measured on a sibling card: `headers_json` is + // `required: false`, and the overwhelming majority of real rows have no + // custom headers at all. A seam that inferred "key missing ⇒ the strip + // ran" would treat every ordinary email as a redacted row. This pins that + // the ordinary case is untouched — and delivers. + const eng = engine!; + const rowId = await insertQueuedRow(eng); + + const { transport, sent } = recordingTransport(); + const svc = new EmailService({ transport, persistence: persistenceFor(eng) }); + const row = (await (eng as any).find('sys_email', { + where: { id: rowId }, limit: 1, context: SYSTEM_CTX, + }))[0]; + const result = await svc.deliverPersistedRow(row); + + expect(result.status).toBe('sent'); + expect(sent).toHaveLength(1); + // No headers authored ⇒ none synthesised. `undefined`/`null` must never + // become a header map. + expect(sent[0].headers).toBeUndefined(); + }); + + it('FAIL-CLOSED: a redacting engine that cannot dereference refuses to send', async () => { + const eng = engine!; + const rowId = await insertQueuedRow(eng, { Authorization: RELAY_TOKEN }); + + // An engine that REDACTS (the 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 + // version-skewed engine would take, and the one combination in which + // "keep going" means sending a message that silently deviates from its + // authored configuration. + const noAccessor = { + getSchema: (o: string) => (eng as any).getSchema(o), + find: (o: string, q: unknown) => (eng as any).find(o, q), + insert: (o: string, d: unknown, opt: unknown) => (eng as any).insert(o, d, opt), + update: (o: string, d: unknown, opt: unknown) => (eng as any).update(o, d, opt), + }; + + const { transport, sent } = recordingTransport(); + const svc = new EmailService({ + transport, + persistence: { + ...persistenceFor(eng), + async readHeadersJson(rowIds) { return readInternalHeadersJson(noAccessor as any, rowIds); }, + }, + }); + const row = (await (eng as any).find('sys_email', { + where: { id: rowId }, limit: 1, context: SYSTEM_CTX, + }))[0]; + + await expect(svc.deliverPersistedRow(row)).rejects.toThrow(/resolveInternalField/); + // Nothing went out missing its headers. + expect(sent).toHaveLength(0); + + // …and no work was lost: the row is still `queued`, NOT `failed`, so the + // queue's retry or the next boot's outbox sweep delivers it intact. This + // is the analogue of #8118's claim-TTL revert, and the reason the + // recovery sits outside the catch that marks rows failed. + const after: any[] = await (eng as any).find('sys_email', { + where: { id: rowId }, limit: 1, context: SYSTEM_CTX, + }); + expect(after[0].status).toBe('queued'); + expect(after[0].error ?? null).toBeNull(); + }); + + it('the privileged accessor refuses a field that is not flagged', async () => { + // The guard that makes the accessor safe rather than a generic + // read-protection bypass (#8118 step 3). Pinned from the consumer side + // too: this card consumes that guard, so it must keep holding here. + const eng = engine!; + await insertQueuedRow(eng, { Authorization: RELAY_TOKEN }); + const err = await (eng as any) + .resolveInternalField('sys_email', ['whatever'], 'subject') + .then(() => null, (e: any) => e); + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('INVALID_FIELD'); + expect(err.status).toBe(400); + }); + + it('at rest the bytes are unchanged — the ruled posture, pinned deliberately', async () => { + // #8149 adopts #8118's decision, which narrows the READ surface only: + // the row still holds the map in cleartext. `Field.secret()` was measured + // and rejected there. Stated so the next reader is not misled, and so a + // future card that changes the at-rest story flips this pin deliberately + // rather than by accident. + const eng = engine!; + const rowId = await insertQueuedRow(eng, { Authorization: RELAY_TOKEN }); + const stored = await (eng as any).resolveInternalField('sys_email', [rowId], 'headers_json'); + expect(String(stored.get(rowId))).toContain(RELAY_TOKEN); + }); +}); diff --git a/packages/plugins/plugin-email/src/email-plugin.ts b/packages/plugins/plugin-email/src/email-plugin.ts index 1ee123a695..717a4477d4 100644 --- a/packages/plugins/plugin-email/src/email-plugin.ts +++ b/packages/plugins/plugin-email/src/email-plugin.ts @@ -44,6 +44,7 @@ import { unbindEmailTemplateProvenanceStamp, } from './email-template-provenance.js'; import { sweepStrandedOutbox, type OutboxSweepResult } from './outbox-sweep.js'; +import { readInternalHeadersJson } from './internal-header-readback.js'; import { EMAIL_ATTACHMENT_RECLAIM_QUEUE, EMAIL_ATTACHMENT_RECLAIM_GRACE_MS, @@ -581,6 +582,16 @@ export class EmailServicePlugin implements Plugin { context: SYSTEM_CTX, }); }, + // [#8149] The privileged readback for the `internal: true` + // `headers_json` column. Taken off the RAW engine on purpose: the + // dereference is a separately-named privileged verb (#8118) + // precisely so it cannot be reached from a query string, and the + // delivery paths above re-read rows through `find`, which omits the + // column for every caller including `SYSTEM_CTX`. Inert — and never + // calls the accessor — when the column is not flagged. + async readHeadersJson(rowIds) { + return readInternalHeadersJson(engine as any, rowIds); + }, }; // Locale resolution lives in its own module (#7731) — the inline version diff --git a/packages/plugins/plugin-email/src/email-service.ts b/packages/plugins/plugin-email/src/email-service.ts index 7255aa1c10..5aeec2383a 100644 --- a/packages/plugins/plugin-email/src/email-service.ts +++ b/packages/plugins/plugin-email/src/email-service.ts @@ -24,6 +24,7 @@ import { storageKeysInColumn, type EncodedAttachments, } from './sys-email-payload.js'; +import { withRecoveredHeaders } from './internal-header-readback.js'; import { EMAIL_ATTACHMENT_RECLAIM_QUEUE, EMAIL_ATTACHMENT_RECLAIM_GRACE_MS, @@ -150,6 +151,19 @@ export interface EmailPersistence { */ insert(row: Record): Promise<{ id: string } | string>; update?(id: string, patch: Record): Promise; + /** + * [#8149] Recover the `internal: true` `headers_json` column for rows the + * engine's generic read path handed back without it. + * + * Optional because it is only meaningful over a redacting engine: the + * implementation returns `undefined` ("nothing was withheld — use the row's + * own value") whenever the column is not flagged, and THROWS when it is + * flagged but the engine cannot dereference it, so a message is never sent + * missing headers it was authored with. See + * {@link ../internal-header-readback} for why the probe is the schema flag + * and never the absence of the key. + */ + readHeadersJson?(rowIds: readonly string[]): Promise | undefined>; } /** @@ -1079,9 +1093,28 @@ export class EmailService implements IEmailService { ): Promise { const rowId = String(row?.id ?? ''); if (!rowId) throw new Error('deliverPersistedRow: row.id is required'); + + // [#8149] `headers_json` is `internal: true`, so EVERY path that reaches + // here by re-reading the row (the drain hook, the queue subscriber, the + // boot outbox sweep) was handed the row WITHOUT its custom headers — + // `SYSTEM_CTX` does not reopen the omission (#7728 has no system + // carve-out). Recover them through the engine's privileged accessor + // before the row becomes a message, so the durable paths send exactly + // what was authored. + // + // Deliberately OUTSIDE the try/catch below: a recovery failure is a + // composition error (a redacting engine with no accessor), not a bad + // message, so it must NOT land the row at `failed` — it propagates, the + // row stays `queued`, and a healthy process (the queue's retry, the next + // boot's sweep) delivers it intact. Marking it `failed` here would burn + // the one durable record of a message that is still perfectly + // deliverable. Inert when nothing redacts: no accessor call at all. + const recoveredHeaders = await this.options.persistence?.readHeadersJson?.([rowId]); + const source = withRecoveredHeaders(row, recoveredHeaders); + // Keys read from the ROW, so a row delivered by the queue worker (which // never saw the send) still schedules its own content's reclamation. - const reclaimKeys = storageKeysInColumn(row.attachments_json); + const reclaimKeys = storageKeysInColumn(source.attachments_json); let normalized: NormalizedEmailMessage; try { // Async because a row's attachments may live in the file-storage @@ -1089,7 +1122,7 @@ export class EmailService implements IEmailService { // capability mounted at all — throws and lands the row at `failed` with // the reason, which is the whole point: an unfetchable attachment must // never become a message delivered without it. - normalized = await rowToNormalizedAsync(row, { fetchContent: this.attachmentFetcher() }); + normalized = await rowToNormalizedAsync(source, { fetchContent: this.attachmentFetcher() }); } catch (err: any) { const errMessage = String(err?.message ?? err ?? 'invalid row').slice(0, 1000); await this.updateRow(rowId, { diff --git a/packages/plugins/plugin-email/src/index.ts b/packages/plugins/plugin-email/src/index.ts index a4a8dc6ddf..cf26693801 100644 --- a/packages/plugins/plugin-email/src/index.ts +++ b/packages/plugins/plugin-email/src/index.ts @@ -120,6 +120,15 @@ export { unbindEmailTemplateProvenanceStamp, EMAIL_TEMPLATE_PROVENANCE_PACKAGE, } from './email-template-provenance.js'; +// [#8149] The `internal: true` header readback seam. +export { + readInternalHeadersJson, + isHeadersColumnRedacted, + withRecoveredHeaders, + SYS_EMAIL_OBJECT, + HEADERS_COLUMN, + type InternalFieldResolvingEngine, +} from './internal-header-readback.js'; export { AUTH_PASSWORD_RESET_TEMPLATE, AUTH_VERIFY_EMAIL_TEMPLATE, diff --git a/packages/plugins/plugin-email/src/internal-header-readback.ts b/packages/plugins/plugin-email/src/internal-header-readback.ts new file mode 100644 index 0000000000..e18438cfa4 --- /dev/null +++ b/packages/plugins/plugin-email/src/internal-header-readback.ts @@ -0,0 +1,152 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8149] The `sys_email.headers_json` readback seam — the durable delivery + * paths get the `internal`-stripped header column back, through the engine's + * privileged accessor, at the one layer this package owns. + * + * ## Why a seam is needed at all + * + * `sys_email.headers_json` is declared `internal: true`, so the engine omits + * it from every generic read — with NO system carve-out (#7728's explicit + * design). That is the fix #8149 exists for. But this plugin's durable + * delivery does not send from the in-memory message: it sends FROM THE ROW. + * Three paths re-read a persisted row and hand it to + * `EmailService.deliverPersistedRow`: + * + * - the after-insert outbox drain hook (`email-plugin.ts`), which re-reads + * the committed row under `SYSTEM_CTX`; + * - the `email.send.async` queue subscriber, which re-reads by `rowId`; + * - the boot outbox sweep (`outbox-sweep.ts`), which re-reads stranded rows. + * + * All three read through `engine.find`, which is exactly what the strip + * empties. Without this seam the flag would not merely hide the headers from + * the data API — it would drop them from the mail actually sent, on every + * durable path, while every row still reported `sent`. That is the failure + * mode the flag must not create, so the readback ships WITH the flag. + * + * ## The probe: the schema flag, never key-absence + * + * This seam decides "did the engine redact?" by asking the OBJECT SCHEMA + * whether the column is flagged — never by noticing the key is missing from a + * result row. The distinction is load-bearing and was measured on a sibling + * card: `sys_email.headers_json` is `required: false`, and the overwhelming + * majority of real rows have no custom headers at all. Under a key-absence + * inference every ordinary header-less email would look like a redacted row + * and force a privileged read, and an engine without the accessor would fail + * every ordinary send. (PR #8675 hit exactly this on `sys_account`'s optional + * token columns: inheriting "key missing ⇒ the strip ran" from a + * `required: true` column broke ordinary sign-in, 16 red tests.) The schema + * flag is cardinality-independent: it is true when the engine redacts and + * false when it does not, whatever any individual row happens to carry — + * which is why #8118's `SqlHttpOutbox.claim()` probes the same way, and why + * this seam needs no `absenceProvesStrip` discriminator. + * + * ## Fail-closed, loudly + * + * The one combination that must not pass silently is "the column is flagged, + * so the row came back without it, and the engine cannot dereference it": + * headers are stored but unrecoverable. Delivering then would put a message + * on the wire missing headers it was authored with — and a missing header is + * not self-announcing: an SMTP relay or API endpoint that does not require it + * ACCEPTS the message, so the send succeeds while silently deviating from the + * authored configuration, and nothing records that it went out incomplete. + * So that combination THROWS. The row stays `queued` (the delivery paths do + * not mark it `failed` for this class of error), so a healthy process — the + * queue's own retry, or the next boot's outbox sweep — delivers it intact, + * the same posture as #8118's claim-TTL revert. + * + * Engines that never redacted are left completely alone: no schema, or an + * unflagged column, means nothing was withheld and the row's own value is + * authoritative. The seam is therefore inert against the in-memory fakes and + * minimal test engines this package is exercised with, and triggers no + * privileged read there at all. + */ + +/** The object this seam reads, and the one column it recovers. */ +export const SYS_EMAIL_OBJECT = 'sys_email'; +export const HEADERS_COLUMN = 'headers_json'; + +/** + * The engine surface this seam needs. Structural, not nominal: this package + * depends on the data-engine contract, and the privileged verb is separately + * named (#8118) precisely so it cannot be reached from a query string. An + * engine implementing neither method is an engine whose `find` does not + * redact either. + */ +export interface InternalFieldResolvingEngine { + resolveInternalField?( + object: string, + recordIds: readonly string[], + field: string, + ): Promise>; + getSchema?(objectName: string): unknown; +} + +/** + * Is `sys_email.headers_json` declared `internal: true` on this engine — i.e. + * does the generic read path hand rows back without it? + * + * Strict `=== true`, matching the engine's own collector: a truthy-but-not- + * `true` value does not enrol a field in the redaction, so it must not enrol + * one in the recovery either. + */ +export function isHeadersColumnRedacted(engine: InternalFieldResolvingEngine): boolean { + if (typeof engine?.getSchema !== 'function') return false; + let schema: { fields?: Record } | undefined; + try { + schema = engine.getSchema(SYS_EMAIL_OBJECT) as typeof schema; + } catch { + // An engine that cannot describe the object cannot be redacting it on a + // path this package controls; treat the row's own value as authoritative. + return false; + } + return schema?.fields?.[HEADERS_COLUMN]?.internal === true; +} + +/** + * Recover `headers_json` for a batch of `sys_email` row ids. + * + * Returns `undefined` — "nothing was redacted, use each row's own value" — + * when the column is not flagged on this engine, or when the batch is empty. + * + * @throws when the column IS flagged but the engine exposes no + * `resolveInternalField`: the headers are stored and unrecoverable, and the + * message must not go out without them (see the module header). + */ +export async function readInternalHeadersJson( + engine: InternalFieldResolvingEngine | undefined, + rowIds: readonly string[], +): Promise | undefined> { + if (!engine || rowIds.length === 0) return undefined; + if (!isHeadersColumnRedacted(engine)) return undefined; + if (typeof engine.resolveInternalField !== 'function') { + throw new Error( + `EmailService: ${SYS_EMAIL_OBJECT}.${HEADERS_COLUMN} is declared \`internal: true\`, but this ` + + 'data engine does not implement resolveInternalField() — the custom headers this message was ' + + 'authored with are stored but cannot be recovered, and a message must not be sent missing them ' + + '(#8149). The row stays `queued`: the queue retry or the next boot outbox sweep delivers it ' + + 'intact once an engine that implements the privileged accessor is mounted.', + ); + } + return engine.resolveInternalField(SYS_EMAIL_OBJECT, rowIds, HEADERS_COLUMN); +} + +/** + * Re-attach the recovered `headers_json` to one row, returning the row a + * delivery path should normalize. + * + * A row whose id is absent from the map keeps whatever it already carried — + * "no such row" belongs to the caller (the delivery paths already re-check + * the row's existence and status), and an unset column resolves to `null`, + * which decodes to "no custom headers" exactly as a stored empty column does. + */ +export function withRecoveredHeaders( + row: Record, + recovered: Map | undefined, +): Record { + if (!recovered) return row; + const id = row?.id != null ? String(row.id) : ''; + if (!id || !recovered.has(id)) return row; + return { ...row, [HEADERS_COLUMN]: recovered.get(id) ?? null }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b67c7965b5..21b5c56339 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1604,6 +1604,9 @@ importers: specifier: ^9.0.3 version: 9.0.3 devDependencies: + '@objectstack/driver-memory': + specifier: workspace:* + version: link:../../drivers/driver-memory '@objectstack/objectql': specifier: workspace:* version: link:../../objectql From 16a35089ca7d364a5ac34cf6eb43eb4838b4992c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:05:13 +0000 Subject: [PATCH 2/2] test(plugin-email): pin the redaction/wire pair on a stub driver; add changeset + i18n Drops the driver-memory devDependency in favour of the stub-driver pattern objectql/src/internal-fields.test.ts already pins the flag against, so the package adds no new workspace dep, no vitest alias config and no entry to the shrink-only unaliased-import ledger. The at-rest pin now scans raw storage instead of reading back through the privileged accessor. Refs #8149 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH --- .changeset/sys-email-headers-internal.md | 14 ++ .../apps/translations/en.objects.generated.ts | 2 +- packages/plugins/plugin-email/package.json | 1 - ...email-headers-internal.integration.test.ts | 130 +++++++++++++++--- pnpm-lock.yaml | 3 - 5 files changed, 127 insertions(+), 23 deletions(-) create mode 100644 .changeset/sys-email-headers-internal.md diff --git a/.changeset/sys-email-headers-internal.md b/.changeset/sys-email-headers-internal.md new file mode 100644 index 0000000000..eafab8ec83 --- /dev/null +++ b/.changeset/sys-email-headers-internal.md @@ -0,0 +1,14 @@ +--- +'@objectstack/platform-objects': minor +'@objectstack/plugin-email': patch +--- + +Stop serving custom email headers through the generic data-API read of `sys_email` (#8149). + +**What this closes.** `sys_email.headers_json` — the custom headers handed to `IEmailService.send`, the ordinary place a relay credential or provider token 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); `SYSTEM_CTX` does not reopen it either. This is the same shape #8118 ruled on for `sys_http_delivery.headers_json`: this change adopts that remedy rather than deciding it a second time. + +**Delivery is unaffected, and fail-closed.** `sys_email` is not delivered from the in-memory message but FROM THE ROW: the after-insert outbox drain hook, the `email.send.async` queue subscriber and the boot outbox sweep all re-read the row and hand it to `EmailService.deliverPersistedRow`. All three read through `engine.find`, which is exactly what the flag empties — so the recovery ships with the flag. `deliverPersistedRow` now recovers the column through ObjectQL's privileged accessor (`resolveInternalField`, consumed unchanged) and sends every authored header verbatim. A message whose headers cannot be recovered is NOT sent without them: a missing header is not self-announcing — a relay that does not require it accepts the mail while the delivery silently deviates from the authored configuration. That case throws and leaves the row `queued`, not `failed`, so the queue retry or the next boot's sweep delivers it intact. + +**New optional seam.** `EmailPersistence.readHeadersJson(rowIds)` — the readback the plugin wires off the raw engine. It probes the OBJECT SCHEMA flag, never the absence of the key from a result row: `headers_json` is `required: false` and most real rows carry no custom headers at all, so a key-absence inference would treat every ordinary email as redacted (the regression measured on `sys_account`'s optional token columns in #7987/PR #8675). Engines that do not redact are left untouched and trigger no privileged read. + +**What this deliberately does NOT close.** The row still holds the header map in cleartext at rest. Encrypting it (`Field.secret()`) was measured and rejected on #8118 — an orphan `sys_secret` row per message with no cascade or retention, a boot-window fail-open, and a per-row decrypt on every delivery — and this change adopts that ruling unchanged. diff --git a/packages/platform-objects/src/apps/translations/en.objects.generated.ts b/packages/platform-objects/src/apps/translations/en.objects.generated.ts index 859bab8156..30d2bbee7f 100644 --- a/packages/platform-objects/src/apps/translations/en.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/en.objects.generated.ts @@ -2050,7 +2050,7 @@ export const enObjects: NonNullable = { }, headers_json: { label: "Headers (JSON)", - help: "Custom headers supplied to IEmailService.send, as a JSON object of name → value. Written in both delivery modes (it is audit evidence as much as delivery input). Absent on rows written before this column existed, which read back as \"no custom headers\"." + help: "Custom headers supplied to IEmailService.send, as a JSON object of name → value. Written in both delivery modes (it is audit evidence as much as delivery input). Absent on rows written before this column existed, which read back as \"no custom headers\". Never returned on the generic data path (#8149) — headers are the ordinary place a credential goes; the delivery paths recover it through the engine's privileged accessor." }, attachments_json: { label: "Attachments (JSON)", diff --git a/packages/plugins/plugin-email/package.json b/packages/plugins/plugin-email/package.json index 594dd02e20..d71f26ff14 100644 --- a/packages/plugins/plugin-email/package.json +++ b/packages/plugins/plugin-email/package.json @@ -25,7 +25,6 @@ "nodemailer": "^9.0.3" }, "devDependencies": { - "@objectstack/driver-memory": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/service-queue": "workspace:*", "@objectstack/service-settings": "workspace:*", diff --git a/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts b/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts index 042774ec7b..88d690d633 100644 --- a/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts +++ b/packages/plugins/plugin-email/src/email-headers-internal.integration.test.ts @@ -35,7 +35,6 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; -import { InMemoryDriver } from '@objectstack/driver-memory'; import { SysEmail } from '@objectstack/platform-objects/audit'; import { EmailService, type EmailPersistence } from './email-service.js'; import { readInternalHeadersJson, isHeadersColumnRedacted } from './internal-header-readback.js'; @@ -46,6 +45,91 @@ const RELAY_TOKEN = 'Bearer prod_relay_tok_8149_do_not_serve'; const SYSTEM_CTX = { isSystem: true, userId: 'system' } as any; +/** + * Minimal stub driver — the same shape `objectql/src/internal-fields.test.ts` + * pins the flag itself against, including the `$in` batch form + * `resolveInternalField` reads by. A stub rather than a real storage driver on + * purpose: what is under test is ENGINE behaviour (the strip, and the + * privileged dereference), so the driver only has to store and hand back + * copies. Rows leave as COPIES, as a real driver's do — handing out the live + * stored object would let the engine's own strip mutate storage, which reads + * exactly like an engine bug. + */ +function makeStubDriver() { + const stores = new Map< string, Map< string, Record< string, unknown > > >(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + let nextId = 0; + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + 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; + } + return true; + }; + const copy = < T, >(r: T): T => (r == null ? r : ({ ...r } as T)); + const project = (row: any, fields?: string[]) => { + if (!row || !Array.isArray(fields) || fields.length === 0) return copy(row); + const out: Record< string, unknown > = {}; + for (const f of fields) if (f in row) out[f] = row[f]; + return out; + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + return Array.from(storeFor(object).values()) + .filter((r) => matches(r, ast?.where)) + .map((r) => project(r, ast?.fields)); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return project(r, ast?.fields); + return null; + }, + async create(object: string, data: Record< string, unknown >) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return copy(row); + }, + async update(object: string, id: string, data: Record< string, unknown >) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) throw new Error(`not found: ${object}/${id}`); + const updated = { ...cur, ...data, id }; + s.set(id, updated); + return copy(updated); + }, + async upsert(object: string, data: Record< string, unknown >) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { return (await this.find(object, ast)).length; }, + async bulkCreate(object: string, rows: Record< string, unknown >[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, stores }; +} + /** Records exactly what the transport was handed. */ function recordingTransport(): { transport: IEmailTransport; @@ -63,10 +147,13 @@ function recordingTransport(): { describe('sys_email.headers_json — authored headers vs the data API (#8149)', () => { let engine: ObjectQL | undefined; + let stores: Map< string, Map< string, Record< string, unknown > > >; beforeEach(async () => { + const stub = makeStubDriver(); + stores = stub.stores; engine = new ObjectQL(); - engine.registerDriver(new InMemoryDriver({}) as any, true); + engine.registerDriver(stub.driver, true); await engine.init(); engine.registry.registerObject(SysEmail as any, '@objectstack/platform-objects'); }); @@ -187,19 +274,19 @@ describe('sys_email.headers_json — authored headers vs the data API (#8149)', const eng = engine!; const rowId = await insertQueuedRow(eng, { Authorization: RELAY_TOKEN }); - // An engine that REDACTS (the 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 - // version-skewed engine would take, and the one combination in which - // "keep going" means sending a message that silently deviates from its - // authored configuration. - const noAccessor = { - getSchema: (o: string) => (eng as any).getSchema(o), - find: (o: string, q: unknown) => (eng as any).find(o, q), - insert: (o: string, d: unknown, opt: unknown) => (eng as any).insert(o, d, opt), - update: (o: string, d: unknown, opt: unknown) => (eng as any).update(o, d, opt), - }; + // An engine that REDACTS (the REAL ObjectQL `getSchema` underneath, so + // `headers_json` is genuinely flagged and the rows genuinely came back + // without it) but exposes no `resolveInternalField`. Not a shape ObjectQL + // can produce — the flag and the accessor ship together — but exactly the + // shape a foreign or version-skewed engine would take, and the one + // combination in which "keep going" means sending a message that silently + // deviates from its authored configuration. + // + // Deliberately NOT an engine double: the seam's whole surface is + // `getSchema` + `resolveInternalField`, so this stub declares exactly the + // one member it needs. Adding CRUD verbs it never calls would make it a + // second fake engine to keep in contract-sync for no test value. + const noAccessor = { getSchema: (o: string) => (eng as any).getSchema(o) }; const { transport, sent } = recordingTransport(); const svc = new EmailService({ @@ -249,8 +336,15 @@ describe('sys_email.headers_json — authored headers vs the data API (#8149)', // future card that changes the at-rest story flips this pin deliberately // rather than by accident. const eng = engine!; - const rowId = await insertQueuedRow(eng, { Authorization: RELAY_TOKEN }); - const stored = await (eng as any).resolveInternalField('sys_email', [rowId], 'headers_json'); - expect(String(stored.get(rowId))).toContain(RELAY_TOKEN); + await insertQueuedRow(eng, { Authorization: RELAY_TOKEN }); + + // Scanned straight out of STORAGE, not through the accessor — an at-rest + // claim read back through the privileged reader would only be restating + // that the reader works. + const rows = [...(stores.get('sys_email')?.values() ?? [])]; + expect(rows.length).toBeGreaterThan(0); // a scan of nothing proves nothing + const dump = rows.map((r) => Object.values(r).map((v) => String(v ?? '')).join(' ')).join(' '); + expect(dump).toContain(RELAY_TOKEN); + expect(dump).toContain('Quarterly report'); // guard the guard }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 21b5c56339..b67c7965b5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1604,9 +1604,6 @@ importers: specifier: ^9.0.3 version: 9.0.3 devDependencies: - '@objectstack/driver-memory': - specifier: workspace:* - version: link:../../drivers/driver-memory '@objectstack/objectql': specifier: workspace:* version: link:../../objectql