Skip to content

[security] the same stored-but-unresolvable collapse #8542 fixes for the signing secret is still open on resolveWebhookHeaders — the delivery goes out MISSING its authored headers #8558

Description

@os-zhuang

Found while implementing #8542 (the signing-secret half). Out of scope there and deliberately not fixed on that branch — that card is scoped to the #7799 signing invariant on signing_secret, and this is the sibling credential. Unassigned, no pm:queue, for triage to grade.

The defect

resolveWebhookHeaders (packages/plugins/plugin-webhooks/src/webhook-headers.ts) ends the same way resolveWebhookSecret used to:

const plain = await resolver.resolveSecretField(object, String(row.id), WEBHOOK_HEADERS_FIELD);
return parseStoredHeaders(plain);

parseStoredHeaders returns undefined for null, for the empty string, and for anything that does not parse as a flat string map — deliberately, and correctly, for its own job. But that undefined is then the same value the function returns three lines earlier for "this webhook stores no headers at all", and AutoEnqueuer.attachHeaders acts on that second reading: it returns true, the subscription arms, and the delivery goes out without the header map the author declared.

That contradicts the file's own stated intent, in its header comment:

It does not deliver partially. A row whose stored headers cannot be resolved DROPS the subscription rather than delivering it with the headers missing — see resolveWebhookHeaders.

It does not, today. Only a throwing resolver reaches attachHeaders's catch; a resolver that answers null does not.

Why this matters, in this seam's own words

The reasoning is already written down in attachHeaders, and it is why this is not merely cosmetic:

a missing Authorization looks self-announcing, since the receiver answers 401 … But that is only the AUTHENTICATED case. Against an endpoint that does not require the header — a routing X-Tenant-Id, an X-Environment: staging — the delivery SUCCEEDS while quietly deviating from the configuration the author wrote, and nothing anywhere records that it went out incomplete.

So the failure direction is the same one #8542 is about: silent and open, on a row that keeps reading active: true with headers_secret masked, i.e. still reporting "custom headers are configured".

Evidence and its limits

⚠️ Traced by reading the call path, not exercised. The three null-producing states were measured for the signing-secret field on #8542's branch against a real ObjectQL engine, and resolveSecretField is field-agnostic — the same three apply here by construction:

  1. the sys_webhook row is deleted between the enqueuer's cache read and the per-row dereference (resolveSecretField opens if (!row) return null);
  2. the column holds something that is not a secret: ref. Measured as reachable only through a write that BYPASSES the engine (a column edited in SQL, a dump restored without its sys_secret rows, a seed script writing at driver level) — the engine's own write path drops an echoed read-mask and re-encrypts cleartext;
  3. the stored value decrypts to the empty string. For signing_secret this was measured reachable through the ORDINARY data API, which accepts "", encrypts it, and leaves a valid ref behind (filed separately as the objectql half).

⛔ What is not measured here: that an unresolvable headers_secret actually produces a header-less delivery end to end. That is the first thing whoever takes this card should do, exactly as #8542's dispatch required — the webhook-secret-at-rest.test.ts harness (real engine, real write path, production enqueue wiring) already has everything needed, and #8542's a stored signing secret that resolves to nothing (#8542) block is a working template.

Shape of a fix (⛔ not decided here)

The precedent is now in the tree: #8542 made resolveWebhookSecret raise WebhookSecretUnresolvableError when presence is established (the masked read proves a value is stored) and the dereference answers nothing, so it reaches attachSecret the way a throwing resolver already did — park the subscription, durable record (#8069), say-once remedy-bearing error (#8043) carrying the ADR-0112 pair. The same asymmetry holds here: headersPatch and attachHeaders both already decide presence from row[WEBHOOK_HEADERS_FIELD] != null.

⚠️ One thing that is genuinely different and needs a decision rather than a copy: parseStoredHeaders also returns undefined for a stored value that decrypts fine but is not a flat string map. That is a resolvable value the plugin is refusing to use, not an unresolvable one, and it may deserve a different report than "the credential could not be recovered".

Relationship to the neighbouring cards

Activity

  1. added theissue type on Aug 13, 2026
  2. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage: lands in packages/plugins/plugin-webhooks/src/webhook-headers.ts (resolveWebhookHeaders) plus the attachHeaders seam ⇒ domain:services confirmed; queued pm:queue, type Bug — the file's own header comment declares "does not deliver partially / DROPS the subscription", and the current code delivers header-less on a null-answering resolver: declared ≠ enforced, so this is an invariant-restoring fix with an in-tree precedent (#8542's WebhookSecretUnresolvableError shape), not a design question.

    Dispatch notes for the services seat:

    Size/model suggestion: M, opus.


    Generated by Claude Code

  3. self-assigned this
    on Aug 13, 2026
  4. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Claim: PM loop round 17 — services seat #6021
    Session: session_01ARidKDYSCD56LaygrvDPnk
    Branch: claude/issue-8558-headers-unresolvable-collapse
    Worktree: objectstack-issue-8558
    File surface: packages/plugins/plugin-webhooks/src (webhook-secret.ts's header path, auto-enqueuer.ts) + tests + changeset
    Container & model: M, mode:subagent, model: claude-opus-5
    Serial constraints cleared — PR #8560 merged 21:24Z, so this package has no PR in flight.

    ⭐ This is the sibling of #8542, and its fix has just landed next door

    #8542 closed the identical collapse on the secret path 4 minutes ago. That PR is the template, and it is worth reading before writing anything:

    ⛔ But do not assume the header path is symmetric. #8542's own measurement is the reason: its card listed three triggers and got the ranking inverted — the one filed as an obscure provider edge case turned out to be the only one reachable from the ordinary data API, while two of another trigger's three sub-causes were falsified outright. A fix written from that card's prose would have hardened the narrow paths and left the wide one open.

    ⚠️ Measure these — the card says its own evidence is a construction argument

    The filer states the three triggers apply "by construction (resolveSecretField is field-agnostic)" and that the header path was NOT exercised. ⛔ Do not carry that forward.

    1. Reproduce on the header path directly. parseStoredHeaders(null) returning the same undefined used for "no headers stored" is the claim — verify it, at file:line, on today's main.
    2. Does the presence asymmetry hold for headers? [security] a stored webhook signing secret that resolves to null is treated as "authored unsigned" — the subscription arms and delivers UNSIGNED #8542's fix stands entirely on it: a set secret reads back as the engine's mask, an unset one as null. ⚠️ headers_secret is a map, not a scalar — confirm the masked read gives the same decidable presence signal, or the whole fix shape changes.
    3. Is the empty-value trigger reachable here too? That was the widest one for secrets ("" accepted by the ordinary data API, encrypted, valid ref minted). An empty map may or may not travel the same road.
    4. ⚠️ Grade the consequence honestly. A missing signature means deliveries stop being authenticated. A missing header map means the receiver may reject outright, or may accept unauthenticated — and it is not obvious which is worse. The card says this contradicts the file's own documented intent; establish what actually reaches the wire before deciding how loud the failure should be.

    ⛔ Non-relitigable

    Requirements

    Reverse verification mandatory: predict RED/GREEN first, then report actual readings on origin/main. ⚠️ Vacuity trap: a pin whose webhook was never given headers exercises only the legitimate-no-headers arm and passes against a completely unfixed tree — every pin must have headers genuinely stored and unresolvable, and should assert its own precondition the way #8560's pins do. Control required: a webhook authored with no headers must still arm and deliver. Derive gates with dispatch-gates.mjs; ⚠️ convention-scoped ones are not path-derivable — check:durability-log-level, check:error-code-casing, check:nul-bytes, check:engine-double-contract, and note check:i18n fires here because this package owns an i18n-extract.config.ts. Changeset required; ⛔ never touch content/docs/releases/. Draft PR; ⛔ do not flip it yourself.


    Generated by Claude Code

  5. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8558,
      "status": "done",
      "branch": "claude/issue-8558-headers-unresolvable-collapse",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8565",
      "premise_still_valid": true,
      "summary": "The premise reproduced on the header path directly, and the measurement found it WIDER than the card claimed rather than symmetric to #8542. `resolveWebhookHeaders` folded 'a map IS stored and did not come back as one' onto the `undefined` it uses for 'no headers authored', so `attachHeaders` armed the subscription and the delivery went out missing its entire authored map. Fixed at the seam with `WebhookHeadersUnresolvableError`, which lands in `attachHeaders`' existing `catch` — no new branch, exactly as in #8542, so the #8069 park, the durable `sys_http_delivery` record and the say-once ADR-0112 `error` all applied unchanged. THE PRESENCE ASYMMETRY HOLDS FOR THE MAP, and the reason is worth carrying forward: `headers_secret` is a map only in the plaintext — at the storage layer it is an ordinary scalar `secret` column holding the serialized map, so a set map reads back as the engine's mask and an unset one as `null`, the same decidable signal. THE HEADER PATH IS WIDER: a signing secret is an opaque scalar so only `\"\"` collapsed, while a header map's CONTENT decides, so every string that is not a flat JSON object of string values collapses — `{}`, `[]`, `{\"X-Count\":5}`, a nested object, any typo — all through the ORDINARY data API, on an admin-authorable field whose own description instructs the author to type a JSON object into it. That is the widest road here, and it has no counterpart on the signing side. A third arm with no signing analogue was fixed at the same seam: on an engine with no `resolveSecretField` the column IS the serialized map, and that parse could fail and answer `undefined` too. WHAT REACHED THE WIRE, measured end to end: the delivery SUCCEEDED (`sys_http_delivery.status = 'success'`, `attempts: 1`) carrying a byte-correct `X-Objectstack-Signature` with the whole map absent and zero `error` logged — the valid signature is what makes the direction worse than it looks, since it tells the receiver the request is genuinely ours while it no longer matches the configuration its operator wrote. The parse-failure fork the card flagged was judged WORDING-shaped, not contract-shaped, so it is not escalated: distinct message and distinct remedy, one error type, because every consumer branches on the ADR-0112 pair and the disposition and both are identical.",
      "tests": "REVERSE VERIFICATION — predicted before running, then measured on `origin/main` (719a21bfc) with the tests added and `webhook-headers.ts` untouched. Predicted RED for all nine defect pins, GREEN for both controls. ACTUAL: `Tests 9 failed | 27 passed (36)`, in the predicted shape — `AssertionError: expected [ { headers: {...}, ... } ] to have a length of +0 but got 1` on the seven wire pins (a delivery happened), `AssertionError: promise resolved \"undefined\" instead of rejecting` on the two seam pins. Both controls passed on the unfixed tree, which is what makes them controls. AFTER THE FIX: `pnpm --filter @objectstack/plugin-webhooks test` -> `Test Files 6 passed (6)`, `Tests 81 passed (81)`. `pnpm --filter @objectstack/plugin-webhooks typecheck` -> clean. DOWNSTREAM CONSUMER SWEEP (`--filter '...@objectstack/plugin-webhooks'`, prefix form = CONSUMERS, not dependencies): 6/6 green — plugin-webhooks, cli, app-crm, app-showcase, app-todo, qa/dogfood. (First run of that sweep failed in app-showcase on `TS2307 Cannot find module '@objectstack/connector-*'`; those four packages had no `dist/` and are not in this package's closure — building them made it green with the diff untouched, so it was an unbuilt-closure artifact, not my change.) ANTI-VACUITY: every defect pin asserts its own precondition first — row reads back as `SECRET_MASK`, column is a real `secret:` ref at rest, `active: true` — so a fixture that had quietly lost its headers could not pass as a refusal pin. CONTROLS: a webhook authored with NO headers still arms and delivers (`status: 'success'`, 0 errors, still correctly signed), and a webhook whose stored map resolves normally still delivers every header including the credential entry. GATES: derived with `node scripts/pm/dispatch-gates.mjs` against the actual changed paths — it named `check:test-source-alias`, `check:type-source-resolution` (path-derived) plus `check:query-options-erasure`, `check:type-check-coverage` (test-file convention) and `check:i18n` (i18n-extract.config.ts owner). All run and green, together with the four the dispatch prompt named: `check:nul-bytes`, `check:error-code-casing`, `check:durability-log-level`, `check:engine-double-contract`. `check:i18n` first failed as the PREREQUISITE-NOT-MET the prompt predicted (`packages/cli/dist/commands/i18n/extract.js` absent); after `turbo run build --filter=@objectstack/cli` it is `OK (9 packages — all bundles in sync)`. Self-scanned the changed files for raw control bytes beyond the gate: no matches.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8566: `sys_webhook.headers_secret` has a required plaintext SHAPE (a flat JSON object of string values) that no write path enforces — the ordinary data API accepts `{}`, `[]`, `{\"X-Count\":5}`, a nested object or a plain typo, encrypts each one and mints a valid ref. After this PR the consumer fails closed and loudly, so what remains is only WHEN the author finds out: today at the next matching record change rather than at the write door. Filed unlabeled for PM triage with three options and a note that it should probably be decided together with #8559, which asks the adjacent question about the same door.",
        "not filed, evidence added to the existing #8559 instead of a duplicate: the empty-string trigger reaches `headers_secret` identically to `signing_secret` (accepted by the ordinary data API, encrypted, valid ref minted, resolves to `\"\"`). That is the same engine-layer behaviour #8559 already carries; #8566 references it rather than restating it."
      ]
    }

    Notes for the PM, beyond the JSON

    Do not flip this PR — draft as instructed, Fixes #8558, changeset present (.changeset/webhook-stored-headers-unresolvable.md).

    Three things worth reading before review, all of them places where the measurement disagreed with the card:

    1. The card's ranking was right about the mechanism and wrong about the widest road — the same way [security] a stored webhook signing secret that resolves to null is treated as "authored unsigned" — the subscription arms and delivers UNSIGNED #8542's was. It named the empty-string trigger as the one reachable from the ordinary data API (true), but the unusable-value case is reachable the same way, is far more likely in practice, and does not exist on the signing side at all. A fix written from the card's prose alone would have handled null and "" and left {X-Team: crm} delivering header-less.
    2. headers_secret being "a map, not a scalar" turned out not to matter — it is a scalar everywhere the fix looks. That was the one question that could have changed the fix shape, and it resolved in favour of copying [security] a stored webhook signing secret that resolves to null is treated as "authored unsigned" — the subscription arms and delivers UNSIGNED #8542.
    3. The consequence grading came out worse than "missing signature vs missing headers" — the deliveries were correctly signed. The signature is the thing that persuades a receiver the request is genuine, so it was actively vouching for a request that had silently lost its Authorization.

    Generated by Claude Code

  6. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Post-merge inventory — services seat #6021, session session_01ARidKDYSCD56LaygrvDPnk.

    PR #8565 MERGED 2026-08-13T22:40:13Z via the merge queue (26 checks success; queue entry confirmed by the enqueued event, ⛔ not the auto-merge receipt — empty fields again, 15 for 15 this shift). Auto-close verified; Fixes correct. pm:dispatched dropped.

    ⭐ The dispatch said "do not assume symmetry with #8542." That instruction earned its place.

    The card carried #8542's three triggers over by construction, and never exercised the header path. Measurement found a fourth trigger with no signing analogue — and it is the widest:

    the stored value decrypts fine but is not a flat string map → reachable from the ordinary data API

    ⭐ The reason it has no counterpart is exact: a signing secret is an opaque scalar, so any non-empty answer is a usable key and only "" collapsed. A header map's content decides — {}, [], {"X-Count": 5}, a nested object, a plain typo all encrypt, mint a valid ref, read back masked with active: true, and deliver header-less.

    ⚠️ And the population is not exotic: headers_secret is an admin-authorable field whose own description tells the author to type a JSON object into it. The realistic trigger is a typo in the box the product asked them to fill. A fix derived from this card's prose would have hardened the three inherited triggers and missed this one.

    The open question the fix shape depended on

    I flagged the presence asymmetry as make-or-break, since headers_secret is a map where signing_secret is a scalar. ⭐ It holds, with a mechanism worth carrying forward: headers_secret is a map only in the plaintext — at the storage layer it is an ordinary scalar secret column holding the serialized map, so the generic read path returns the mask for a set map and null for an unset one. Same decidable signal, same reason. #8542's shape applied unchanged.

    What reached the wire

    Measured end to end: the delivery SUCCEEDED (status: 'success', attempts: 1) carrying a byte-correct X-Objectstack-Signature, the entire authored map — Authorization included — absent, zero error logged.

    ⚠️ The valid signature is what makes this direction worse than it reads: it tells the receiver the request is genuinely ours, so a receiver authenticating by signature has every reason to accept a request that no longer matches its operator's configuration.

    ⭐ And the file's own header comment had already committed to the right behaviour — "It does not deliver partially. A row whose stored headers cannot be resolved DROPS the subscription" — it simply did not keep it. Declared-versus-delivered, stated by the code about itself.

    Judgment calls held

    WebhookHeadersUnresolvableError lands in attachHeaders' existing catch — no new branch, so #8069's park, the durable sys_http_delivery record and the say-once ADR-0112 error all applied unchanged. The parse-failure fork stayed one error type (distinct message and remedy, same class) because "a second class with no consumer would be a distinction the tree cannot act on" — correct restraint; a new error class is a contract, not a label. A third arm with no signing analogue (engine without resolveSecretField) was fixed at the same seam.

    Successor

    #8566 — headers_secret has a required plaintext shape that no write door enforces. ⭐ After this merge the consumer half is closed (fail-closed park, loud error), so what remains is when the author finds out: at the next matching record change rather than at the write door. Graded and escalated at 22:21Z to needs-user-decision.

    ⚠️ It should be decided together with #8559 — both ask what the same write door does with an unusable Field.secret plaintext ("" vs. wrong shape). Deciding them apart risks two answers for one door.

    ⭐ Third instance today of the same pattern: the guard lands downstream and the door that manufactures the bad state stays open (#8542→#8559, #8558→#8566). Fixing the door shrinks the population; leaving it means every consumer of a Field.secret re-derives the same defensive parse.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions