Skip to content

fix(plugin-webhooks): refuse the retired definition_json credential location at delivery and at the write door - #19944

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-10164-legacy-webhook-cleartext-refusal
Sep 24, 2026
Merged

huangyiirene merged 4 commits into
mainfrom
claude/issue-10164-legacy-webhook-cleartext-refusal

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Closes #10164

Clause-②: no (narrowing)

Step 3 of the retirement ruled on #9930, now in scope under the 2026-09-23 maintainer ruling recorded on #10164: 「10164 不考虑旧的」 and 「10164 现在就做,不等v18」. The legacy cleartext credential location, the secret / headers keys inside sys_webhook.definition_json, goes from warn-and-accept to a loud, dated refusal. The refusal applies at both doors in one change.

What changed

Delivery door (AutoEnqueuer.attachSecret / attachHeaders, auto-enqueuer.ts). A row whose signing secret or header map exists ONLY as the legacy key, with nothing in signing_secret / headers_secret, is no longer delivered from the cleartext. The new refuseLegacyCleartext parks the subscription in the same fail-closed shape an unrecoverable encrypted credential already uses:

  • nothing is sent;
  • every matching event becomes a dead sys_http_delivery row with 0 attempts, no signature, no headers, and an error reading [VALIDATION_ERROR/400] ...;
  • the drop is reported ONCE at error, on the existing say-once ledger. Its meta carries code, status, field: 'definition_json' and the refused keys.

readLegacySecret / readLegacyHeaders stay, because the boot sweep is their other caller and the sweep is the remedy.

Write door (new webhook-legacy-cleartext.ts, bindWebhookLegacyCleartextGate). A beforeInsert / beforeUpdate hook on sys_webhook refuses any definition_json that carries a secret or headers key, whatever its value. It follows the #8566 shape: the same hook seam as the headers_secret shape gate, bound in bootDeclaredWebhooks before the seeder and the sweep, unbound in destroy(). It is not exempt for isSystem. The error, WebhookLegacyCleartextError, carries code: 'VALIDATION_ERROR', status: 400, object, field and keys. It judges key presence: "headers": {} still teaches the wrong location. It never echoes a value. A clean blob, an omitted definition_json and a non-JSON blob all pass. The last is not this gate's verdict.

Refusal text: it is dated 2026-09-23 (the ruling's date; the message says why), names the sweep migrateLegacyWebhookSecrets, and gives the remedy. Both remedies need a registered CryptoProvider. With one registered, either restart so the sweep converts the row, or write the values into signing_secret / headers_secret and remove the keys. The text carries no tracker number. Per 「不考虑旧的」, there is no transition path for a deployment without a CryptoProvider.

Changeset: @objectstack/plugin-webhooks: minor with a BREAKING banner (launch-window convention; check-changeset-no-major refuses major). It states plainly that webhooks still on the legacy shape STOP DELIVERING, and gives the remedy. ADR-0087 disposition: not-required (no-migration-prescription). Nothing authored moves: packages/spec is untouched, and WebhookSchema still declares secret / headers, which the materializer still routes to the encrypted columns. The refused shape is a stored data row, and its converter, the boot sweep, already ships.

⛔ Zero packages/spec. ⛔ No content/docs/releases/**.

Tests (all in packages/plugins/plugin-webhooks)

  • webhook-secret-at-rest.test.ts. The two "un-swept row keeps signing / keeps delivering" tests are rewritten as refusal witnesses on a real ObjectQL engine. Each asserts no call reaches the receiver, exactly one error whose meta matches { code: 'VALIDATION_ERROR', status: 400, field: 'definition_json', keys: [...] }, and a parked dead row whose error starts [VALIDATION_ERROR/400]. Each also checks that the prose carries the date, the sweep name, the remedy, no credential and no tracker number. A control leg shows the same row, once swept, delivers its headers.
  • Write-door suite on a real engine:
    • update with headers, with secret, and with both is refused with the envelope (code, status, object, field, keys), and nothing lands;
    • an insert is refused whole;
    • an emptied key is refused;
    • a clean blob, an omitted blob and a non-JSON blob pass;
    • an unbound counterfactual lets the write land;
    • the boot sweep still converts a pre-existing legacy row with the gate bound;
    • the declared-webhook seeder still materializes secret + headers through the gate.
  • webhook-signing-secret.test.ts. Fixture re-spelled onto the encrypted columns, with a resolveSecretField stub. It pinned the signature path by riding the legacy blob, which is now refused.

Reverse verification (from committed state, scripts/ablation-replace.mjs WRAP mode, restore proven by blob hash == HEAD and an empty git diff HEAD)

leg mutation witness file result
A attachSecret honours the legacy secret again 1 failed / 45 passed: "an un-swept row is REFUSED, not signed from the blob"
B attachHeaders honours the legacy headers again 1 failed / 45 passed: "an un-swept row is REFUSED, not delivered with the blob's headers"
C the write-door verdict passes everything 5 failed / 41 passed: all five write-door refusal cases

All three were restored: auto-enqueuer.ts blob 1cfd125c6c79 == HEAD, webhook-legacy-cleartext.ts blob 1c50d2b4321b == HEAD.

Local verification (HEAD 36d17abc8e)

  • pnpm --filter @objectstack/plugin-webhooks exec vitest run: 13 files, 160 tests passed.

  • pnpm --filter @objectstack/plugin-webhooks run typecheck: exit 0, measured at 7d448ba52a. The only later commit touches scripts/doc-authoring-prose-id.baseline.json.

  • eslint over the 7 changed .ts files: 7 files, 0 errors, 0 warnings. Population read from eslint.config.mjs; --format json counts. Invariance: type-aware linting is not enabled (no parserOptions.project), so this diff cannot move any untouched file's verdict.

  • dispatch-gates --ran: 101 derived families accounted for, 98 run green and 3 NOT MEASURED:

    • check:skill-examples needs built client SDK output;
    • check:dual-build-cjs-loads and check:type-check-debt need the full workspace build.

    All three exited 3 on their own prerequisite, and CI builds the full closure. check:doc-authoring first went red on a burn-down, meaning the removed warn strings cited two tracker ids. The baseline was regenerated with the gate's own shrink-only command (#7799/#7986 3 to 2 in auto-enqueuer.ts). check:i18n went green after its prerequisite closure was built. check-adr-0087-registration, check-changeset-no-major and check-empty-changeset are green.

Acceptance notes

  • Docs: content/docs/automation/webhooks.mdx §3.1 said an un-swept row "is still delivered from the blob, with a warn". That sentence is corrected to the refusal, in the one page describing this exact behaviour. No other hand-written page describes legacy acceptance.
  • Not exported from the package entry. bindWebhookLegacyCleartextGate is wired by WebhookOutboxPlugin only. The headers_secret gate is exported for hosts that boot the pieces themselves; this one is deliberately not, to keep the public surface unchanged under no (narrowing). A consumer branches on code / status.
  • Residue left as-is (observation, not filed): a row holding BOTH an encrypted credential and a legacy key is not refused at delivery. The encrypted value wins and the legacy key is ignored, as before. Such a row can no longer be written through the engine. Any existing one is still readable over the data API until the boot sweep, which treats it as found and writes the blob's value into the encrypted column. Whether that overwrite can clobber a rotated key on a real deployment is unmeasured. Carrier: none.
  • Commit trailers are the model-free pair AGENTS.md prescribes. The harness's own attribution reminder asked for a model-named trailer, which the pre-push hook refuses; AGENTS.md wins.

Generated by Claude Code

…ocation

A sys_webhook row whose signing secret or header map exists only as a
secret/headers key inside definition_json is parked with a dated
VALIDATION_ERROR/400 refusal instead of being delivered from the
cleartext, and a write that puts either key into definition_json is
refused at the door.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 24, 2026
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-webhooks, touching 31 documentable anchor(s).

30 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 43460b95aa196410b1acfcaa0bb9af8905066ddc.

⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1/data (route, 36 pages)
  • 8 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 4 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 43460b95aa196410b1acfcaa0bb9af8905066ddc → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7d3e090bf72b417aaf00dbd98068091ea3868cf1 — the merge of head 4c78b034d56d2386d771eec10e87d2cacce9daab into base 43460b95aa196410b1acfcaa0bb9af8905066ddc, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7d3e090bf72b417aaf00dbd98068091ea3868cf1 && git checkout 7d3e090bf72b417aaf00dbd98068091ea3868cf1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 43460b95aa196410b1acfcaa0bb9af8905066ddc 4c78b034d56d2386d771eec10e87d2cacce9daab && git checkout -B drift-repro 43460b95aa196410b1acfcaa0bb9af8905066ddc && git merge --no-ff 4c78b034d56d2386d771eec10e87d2cacce9daab

node scripts/docs-audit/affected-docs.mjs --json 43460b95aa196410b1acfcaa0bb9af8905066ddc

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 43460b95aa196410b1acfcaa0bb9af8905066ddc → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…es under a CryptoProvider

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4c78b034d56d2386d771eec10e87d2cacce9daab

An isolated reviewer ran at the served tier. It saw only card #10164, ruling 5792274752, claim 5805810063, and this PR. It first reviewed 36d17abc8e and returned PASS with two prose defects. The dev fixed both in one prose-only commit (4c78b034d5), and the reviewer then re-reviewed that delta. This record covers the current head. The domain:services seat adopts the verdict.

① Derived judgments

Every changeset and docs claim is TRUE against source:

  • Delivery door parks. auto-enqueuer.ts:584-654 routes the refusal through refuseLegacyCleartext (:676-704). The subscription writes a dead sys_http_delivery row with the error [VALIDATION_ERROR/400] and logs it once.
  • Write door covers every write path. It refuses secret/headers in definition_json on insert and update, including raw PATCH and isSystem writes (webhook-legacy-cleartext.ts:214-234; engine :3161-3168, :12496; REST :9057-9110).
  • Legitimate writers still pass. The seeder writes only after the split (bootstrap-declared-webhooks.ts:336-349). The sweep strips both keys in the same update (migrate-webhook-secrets.ts:114-124, :158-163). Both are pinned by tests with the gate bound. There is no other in-repo writer of definition_json.
  • Unchanged behaviour holds. Rows with encrypted columns deliver as before.
  • Error code. VALIDATION_ERROR is a published StandardErrorCode (packages/spec/src/api/errors.zod.ts:173).
  • No new export. Nothing new is exported from the package entry.
  • ADR-0087 disposition is correct. not-required (no-migration-prescription): the refused shape is a data row outside ADR-0087's metadata scope. The gate is green at this head.
  • D1 is closed. The write-door claim is now conditioned on WebhookOutboxPlugin being mounted (webhook-outbox-plugin.ts:254-259). A host that composes the pieces itself gets the delivery-path refusal only, and the changeset says so.
  • D2 is closed. Both remedies now require a registered CryptoProvider (engine.ts:7405-7410 refuses any secret-typed write without one), in the changeset and in webhooks.mdx. The PR body was aligned the same way.

② Semver level

minor + a BREAKING banner + Clause-②: no (narrowing) is consistent:

  • clause2-line.mjs reads the declaration as declared/no/narrowing.
  • AGENTS.md §Post-Task 3 says 「(narrowing) is BREAKING」.
  • During the launch window a breaking change ships as minor (check-changeset-no-major header; pr-automation.yml:753-760). That gate is green at this head.

③ Boundary flags

Zero packages/spec, zero content/docs/releases/**, zero CHANGELOG edits. The scope matches the ruling: both read paths plus the write door, and no transition path for a deployment without a CryptoProvider.

Implemented-by: claude/issue-10164-legacy-webhook-cleartext-refusal
Reviewed-by: session_01AhQASwqJr2Z7XfGWUdvnbF

VERDICT: PASS

Non-blocking follow-ups:

  • The runtime remedy() text (webhook-legacy-cleartext.ts:95-103) still mentions the CryptoProvider requirement only for the sweep. It is message text only; the contract carriers are code/status/object/field/keys. An operator who follows the manual path without a provider gets a loud engine refusal naming engine.setCryptoProvider. The fix is a small code+test change.
  • The two doors judge "present" differently. The write door refuses on key presence. The delivery door refuses only a non-empty secret or a valid header map. No claim in the PR is wrong because of this.

Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 24, 2026 02:47
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit 9a0c0b5 Sep 24, 2026
44 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-10164-legacy-webhook-cleartext-refusal branch September 24, 2026 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flip readLegacyHeaders from warn-and-accept to a loud dated rejection — step 3 of the #9930 retirement

2 participants