Skip to content

fix(plugin-webhooks): park a subscription whose stored header map resolves to nothing (#8558) - #8565

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-8558-headers-unresolvable-collapse
Aug 13, 2026
Merged

fix(plugin-webhooks): park a subscription whose stored header map resolves to nothing (#8558)#8565
os-zhuang merged 1 commit into
mainfrom
claude/issue-8558-headers-unresolvable-collapse

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8558

Sibling of #8542 on the same seam's other credential. The card was honest that its evidence was a construction argument (resolveSecretField is field-agnostic) and that the header path had not been exercised. It has been now — and the measurement changed the story in one important direction.

The premise holds, and the path is WIDER than the card claimed

resolveWebhookHeaders answered undefined for two different facts — "the author configured no custom headers" and "a map IS stored and did not come back as one" — and AutoEnqueuer.attachHeaders acts on the first reading. So the second became the first: the subscription armed and delivered.

The card carried #8542's three triggers over by construction. All three do reach this path, but they are not the whole set and not the widest one:

triggerreachable viameasured
row deleted between the cache read and the dereferenceraceseam answers undefined, no throw
column holds something that is not a secret: refdriver-level write onlydelivered header-less
stored value decrypts to the empty stringordinary data APIdelivered header-less
stored value decrypts fine and is not a flat string mapordinary data APIdelivered header-less

The fourth has no counterpart on the signing side. 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, and parseStoredHeaders answers undefined — correctly, for its own job — for every string that is not a flat JSON object of string values. Measured accepted by the ordinary data API, encrypted, valid ref minted, row reading back masked and active: true, delivery going out header-less, for all of: {}, [], {"X-Count": 5}, {"X-Team":{"name":"crm"}}, and plain typos.

That makes it the widest road here rather than an exotic one: sys_webhook.headers_secret is an admin-authorable Field.secret whose own description instructs the author to type "a JSON object ({"Authorization": "Bearer …"})" into it. The realistic trigger is an admin mistyping JSON into the box the product told them to type JSON into.

The presence asymmetry does hold for a map

This was the open question the fix shape depended on, since headers_secret is a map where signing_secret is a scalar. It holds, and the reason is worth stating: 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 engine's mask for a set map and null for an unset one — the same decidable signal, for the same reason. Measured: viaApi[headers_secret] === SECRET_MASK, active: true, column secret:sec_3 at rest. So #8542's fix shape applies unchanged.

What actually reached the wire

Not merely "a delivery with something missing". Measured end to end through the production enqueue wiring: the request SUCCEEDED (sys_http_delivery.status = 'success', attempts: 1) carrying a byte-correct X-Objectstack-Signature, with the entire authored map — Authorization included — absent, and zero error logged.

The valid signature is what makes this direction worse than it looks. It tells the receiver the request is genuinely ours, so a receiver that authenticates by signature has every reason to accept a request that no longer matches the configuration its operator wrote. Grading the two consequences against each other, as the card asks: a missing signature and a missing header map are not the same harm, but they fail in the same direction — silent and open — and this file's own header comment had already committed to the answer ("It does not deliver partially. A row whose stored headers cannot be resolved DROPS the subscription"). It simply did not keep it.

The fix

One rule, one place, at the seam. Stored headers that do not come back as a map now raise WebhookHeadersUnresolvableError instead of answering undefined, so they reach attachHeaders' existing catch exactly the way a throwing resolver already did.

Reverse verification

Predicted before running, then measured on origin/main (719a21b) with the tests added and webhook-headers.ts untouched. Predicted RED for all nine defect pins, GREEN for both controls.

Tests 9 failed | 27 passed (36)
AssertionError: expected [ { headers: {...}, ... } ] to have a length of +0 but got 1 x7
AssertionError: promise resolved "undefined" instead of rejecting x2

Nine failed, and in the predicted shape: the seven wire pins failed because a delivery happened, the two seam pins because the seam resolved instead of rejecting. Both controls passed on the unfixed tree, which is what makes them controls. With the fix: Tests 81 passed (81).

Anti-vacuity: every defect pin asserts its own precondition before asserting the refusal — the row reads back as the mask, the column is a real secret: ref at rest, active: true. A pin whose webhook had quietly lost its headers would exercise only the legitimate no-headers arm and pass against a completely unfixed tree.

Controls: a webhook authored with no headers still arms and delivers (status: 'success', 0 errors, still signed), and a webhook whose stored map resolves normally still delivers every header including the credential entry.

Verification

  • pnpm --filter @objectstack/plugin-webhooks test — 81 passed (6 files)
  • pnpm --filter @objectstack/plugin-webhooks typecheck — clean
  • Downstream consumer sweep (--filter '...@objectstack/plugin-webhooks', prefix form = consumers): 6/6 green — plugin-webhooks, cli, app-crm, app-showcase, app-todo, qa/dogfood
  • Gates: check:nul-bytes, check:error-code-casing, check:durability-log-level, check:engine-double-contract, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:i18n (green after turbo run build --filter=@objectstack/cli) — all pass

Changeset: .changeset/webhook-stored-headers-unresolvable.md, including the operator-facing upgrade note (re-save as a flat JSON object of string values, or clear to null — an empty or unparseable header map is not the same thing as no header map).


Generated by Claude Code

…olves to nothing (#8558)
`resolveWebhookHeaders` answered `undefined` for two different facts — "the
author configured no custom headers" and "a map IS stored and did not come back
as one" — and `AutoEnqueuer.attachHeaders` acts on the first reading. So the
second became the first: the subscription armed and every delivery went out
missing its entire authored header map, the ordinary place an `Authorization`
goes, while the row kept reading `active: true` with `headers_secret` masked.
Measured end to end: the delivery SUCCEEDED carrying a byte-correct
`X-Objectstack-Signature`, which is the worst available combination — the
signature tells the receiver the request is genuinely ours while it no longer
matches the configuration its operator wrote.
Sibling of #8542 on the same seam's other credential, but WIDER rather than
symmetric: a signing secret is an opaque scalar so only `''` collapsed, while a
header map's content decides and every string that is not a flat JSON object of
string values collapses — through the ordinary data API, on a field whose own
description tells the admin to type JSON into it.
Fixed at the seam, so no caller re-derives the rule: stored headers that do not
come back as a map now raise `WebhookHeadersUnresolvableError`, reaching
`attachHeaders`' existing `catch` exactly the way a throwing resolver already
did. The park (#8069), the durable `sys_http_delivery` record and the say-once
ADR-0112 `error` all apply unchanged; `attachHeaders` needed no new branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 9:49pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-webhooks.

2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx(via packages/plugins/plugin-webhooks)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-webhooks)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-webhooks)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 13, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:services seat #6021, session session_01ARidKDYSCD56LaygrvDPnk. Verdict: ACCEPT, flip + arm pending CI. ⛔ Nothing enqueued until every check concludes success.

⭐ The dispatch said "do not assume symmetry with #8542." This is why.

The card carried #8542's three triggers over by construction. All three do reach this path — and measurement found a fourth with no signing analogue at all, which is the widest of the set:

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

⭐ And the reason it has no counterpart is the crisp part: 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 — so {}, [], {"X-Count": 5}, {"X-Team":{"name":"crm"}} and plain typos all resolve, encrypt, mint a valid ref, read back masked with active: true, and deliver header-less.

⚠️And the realistic trigger is not exotic — it is the product's own instructions.sys_webhook.headers_secret is an admin-authorable Field.secret whose description tells the author to type "a JSON object ({"Authorization": "Bearer …"})" into it. The defect's population is admins mistyping JSON into the box the product told them to type JSON into. A fix derived from the card's prose would have covered the three inherited triggers and missed this one entirely.

⭐ The make-or-break question, answered with the mechanism

I flagged the presence asymmetry as the thing the whole fix shape depends on, and warned that a map might not give the same decidable signal as a scalar. It holds — and the reason is what makes it trustworthy rather than lucky:

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, for exactly the same reason as the signing secret. ⭐ Answering why it holds is what lets the next person reuse it; "it worked" would not have.

⭐ What reached the wire — and why this direction is worse than it sounds

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

⚠️ The valid signature is the sting. It tells the receiver the request is genuinely ours, so a receiver that authenticates by signature has every reason to accept a request that no longer matches the configuration its operator wrote. That is a sharper answer than the dispatch's question ("reject outright, or accept unauthenticated?") — it is accept, with cryptographic assurance, a request the operator did not configure.

⭐ And the closing detail: this file's own header comment had already committed to the correct 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, both right

Verification

Nine defect pins RED on origin/mainin the predicted shape — seven wire pins failing because a delivery happened, two seam pins because the seam resolved instead of rejecting — and both controls GREEN on the unfixed tree, which is what makes them controls. 81/81 after. Every defect pin asserts its own precondition first (mask read-back, real secret: ref at rest, active: true), so a fixture that quietly lost its headers fails the precondition instead of passing vacuously.

⭐ The changeset carries the operator-facing upgrade note — re-save as a flat JSON object of string values, or clear to null — with the same distinction #8542 drew: an empty or unparseable header map is not the same thing as no header map.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Docs drift check — measured, ⛔ no action. Do not open a patch round.

Checked rather than forwarded, and checked separately from #8560's — that one asked about the signing secret; this PR changes header behaviour, so the earlier "nothing to fix" does not transfer.

content/docs/automation/webhooks.mdx contains zero occurrences of headers (as it contains zero of signing_secret / unsigned). The page documents the routes and their error codes; it does not document the custom-header feature at all. So there is no statement this PR makes false and no enumeration left incomplete — unlike PR #8541, which narrowed a documented route and minted a documented error code and therefore owed an edit.

⚠️ Worth noting as an observation, ⛔ not as a request on this PR: sys_webhook.headers_secret is an admin-authorable field whose own description tells the author what JSON to type, and the operator-facing docs never mention it. That is the same gap that made this defect's population "admins mistyping into a box with no documentation behind it". ⛔ Not this card's scope and not a blocker; if it deserves a card, it is a docs-only one.

content/docs/releases/implementation-status.mdx is release-owned and read-only, correctly listed as such.


Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude