feat(security): sign webhooks with the request-signing key; deprecate webhook-signing#5552
Conversation
Webhook verifier checklist step 8 now accepts adcp_use of 'webhook-signing' OR 'request-signing' — a signer may reuse the request-signing key it already publishes instead of minting a dedicated webhook key. Other purposes (response-signing, governance-signing) remain rejected with webhook_mode_mismatch; absent adcp_use with webhook_signature_key_purpose_invalid. Safe and one-directional: domain separation is carried by the RFC 9421 tag (adcp/webhook-signing/v1, step 3) and mandatory content-digest coverage, not by the key-purpose discriminator. The reverse (webhook key verifying a request) stays forbidden. A dedicated webhook key remains RECOMMENDED. Conformance vectors: former negative 008-wrong-adcp-use -> positive 008-request-signing-key-reuse; new negative 008 covers a response-signing key. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![aao-release-bot[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
The cryptographic argument is sound — the signed tag at step 3 is the real domain-separation control, and the adcp_use check was never load-bearing for confusion-resistance on the webhook path. The relaxation is safe. But the spec now contradicts itself in three places and ships a conformance vector that enforces one side of the contradiction, and the PR is explicitly an RFC with open design questions. Holding for the author/maintainer call rather than blocking or rubber-stamping.
security-reviewer: no High/Medium. A captured request signature carries tag=adcp/request-signing/v1 in its signed @signature-params base and is rejected at webhook step 3 (security.mdx:1469) before the relaxed step 8 is ever reached — and would fail crypto verify at step 10 regardless. The only party who can mint a webhook-tagged signature verifiable under the request-signing key is the holder of that private key, i.e. the legitimate signer reusing its own key. Asymmetry intact: request step 8 (security.mdx:1237) still requires adcp_use == "request-signing" exactly. No replay path, no cross-protocol confusion.
ad-tech-protocol-expert: sound-with-caveats. Domain-separation reasoning verified against the vectors (tag is inside the signed @signature-params). The one-directional asymmetry is coherent.
Things I checked
- Domain separation holds. Webhook step 3 tag check (
security.mdx:1469) fires before step 8; request signature replay onto the webhook path fails at step 3 and again at step 10. Theadcp_usediscriminator adds zero confusion-resistance on the webhook path — it is blast-radius hygiene, as the PR states. - Asymmetry is real. Request verifier step 8 (
security.mdx:1237) still demandsadcp_use == "request-signing"exactly. A webhook-signing key cannot verify a request; response/governance keys are rejected on both paths. - Positive vector 008 reuses the original signature bytes (
OT_F-yQs..., keyidtest-wrong-purpose-2026). Legitimate — Ed25519 is deterministic and only the verifier's accept/reject decision changed. Was always cryptographically valid; only step 8 rejected it. - Changeset present,
minor. Defensible for a verifier-acceptance widening — but see the error-code point below, which is the part that is observably breaking.
Blocking the merge in the author's own framing (this is the RFC's open question)
This PR's own "Notes for reviewers" offers two paths on the error code. The path it took ships a self-contradictory normative document. Three sites were not updated:
- Taxonomy table contradicts step 8.
security.mdx:1556still readsJWK adcp_use ≠ webhook-signing → webhook_signature_key_purpose_invalid. After this PR step 8 emitswebhook_mode_mismatchfor a wrong purpose and acceptsrequest-signingoutright. The row is now false. webhook_mode_mismatchis overloaded.security.mdx:1565defines it as the HMAC-vs-9421 auth-mode selector mismatch (reinforced at:1462). Reusing it for a key-purpose failure collapses two distinct failure classes onto one stable code — which defeats the taxonomy's stated goal that "SDK error handling distinguishes the two profiles" (:1545). Reusing a stable error code for a new failure class is the observably breaking part of this change for any receiver that branches on it.- Blast-radius sentence contradicts the new design.
security.mdx:1438(untouched by the diff) still stateskidcollisions are forbidden "specifically so a request-signing-key compromise cannot be repurposed as a webhook-signing capability." That repurposing is now the intended behavior. The sentence's rationale is now false.
And the conformance README was not touched: static/compliance/source/test-vectors/webhook-signing/README.md L34-36, L45, L105 still describe 008 as the negative request-signing → webhook_signature_key_purpose_invalid vector and claim test-wrong-purpose-2026 "causes step 8 to reject." It now causes step 8 to accept. An implementer building a runner from that README, or a verifier built from the L1556 table, fails the new negative/008 vector — the spec disagrees with itself and the bundled vector picks a side.
What flips this to approve
- Pick one error-code story and make the whole document tell it. Either revert step 8 to
webhook_signature_key_purpose_invalidfor all purpose failures (your offered fallback — lowest surface, no overload), OR keep the adcp#2467 split and updatesecurity.mdx:1556and:1565to match, plusnegative/008'serror_code. - Fix the
security.mdx:1438blast-radius sentence so the kid-collision rationale reflects the new model (collision forbidden so eachkidcarries one declared purpose; dedicated webhook key is the isolation mechanism, RECOMMENDED). - Update
webhook-signing/README.md(L34-36, L45, L105) for the positive/negative 008 split and the newresponse-signingkey.
Follow-ups (non-blocking — file as issues)
- The PR scopes itself to normative
security.mdx; migration guides, whats-new, and brand-rights/governance/collection-lists task specs still say webhook signing uses awebhook-signingkey. Sweep once direction is agreed, as the PR notes. - Confirm with the WG whether the implicit reuse form is preferred over an explicit multi-purpose
adcp_use(the PR raises this; it is a real fork worth a deliberate decision, not a default).
Crypto's right. The spec just needs to agree with itself before it goes on the wire. Resolve the three doc sites and the README and I'll approve.
Per reviewer feedback: resolve the self-contradiction. Step 8 uses
webhook_signature_key_purpose_invalid for ALL key-purpose failures (absent
adcp_use, missing verify key_op, or adcp_use outside {webhook-signing,
request-signing}); webhook_mode_mismatch is left reserved for the HMAC-vs-9421
auth-mode selector. Fixes the taxonomy table row, the blast-radius kid-collision
sentence (reuse is now intended; dedicated key is the isolation mechanism), the
negative 008 vector error_code, and the conformance README (008 split + new
response-signing key).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks — this is a great review, and you're right on all four. I picked option A (your offered fallback): one error-code story, Pushed
The paired SDK PR (adcontextprotocol/adcp-client#2239, commit Follow-ups (filing as separate issues, per your note): (a) sweep the non-normative docs — migration guides, whats-new, brand-rights/governance/collection-lists — that still say webhook uses a dedicated |
bokelley
left a comment
bokelley
left a comment
There was a problem hiding this comment.
All three blocking items from the prior review are resolved in c737525:
- Taxonomy table — now reads
adcp_use ∉ {webhook-signing, request-signing}, absent, or key_ops lacks verify → webhook_signature_key_purpose_invalid. Consistent with step 8. ✓ - Error-code overload — step 8 emits
webhook_signature_key_purpose_invalidfor all purpose failures. Explicit inline note thatwebhook_mode_mismatchis reserved for the HMAC-vs-9421 selector and is NOT used here. ✓ - Blast-radius sentence — rewritten:
kidcollisions are forbidden so eachkidcarries one declared purpose; reuse is an intended option so a request-signing-key compromise does extend to webhooks; dedicatedwebhook-signingkey is the isolation mechanism and remains RECOMMENDED. ✓ - Conformance README — L34-36, L45, L105 all updated;
test-response-purpose-2026added to the exploitable-kids list; positive/008 and negative/008 entries correctly described. ✓
Crypto reasoning still holds. Option A (single webhook_signature_key_purpose_invalid story) is the right call — no overload, lowest surface, taxonomy intact.
Two non-blocking follow-ups noted; no change needed in this PR:
- Non-normative doc sweep (migration guides, whats-new, brand-rights/governance/collection-lists) — tracked per your note.
- WG decision on implicit vs explicit multi-purpose
adcp_use— PR carries it as an open question; correct place for it.
Approved on the technical merits. Maintainer sign-off still needed for the normative design direction reversal per the RFC framing.
Generated by Claude Code
There was a problem hiding this comment.
The design is sound — the one-directional relaxation (request-signing key OK for webhooks, webhook key never OK for requests) holds up — but the PR ships a normative error-code contract that contradicts itself across four surfaces, and one of those surfaces is the conformance vector it also ships. Not approving until the contradiction is resolved; this is also flagged in the PR body as an RFC reversal of a normative design choice, so it's the maintainers' call regardless.
The architectural principle is right: domain separation on the webhook path is carried by the RFC 9421 tag (in the signed base, checked at step 3) and mandatory content-digest, not by the adcp_use discriminator. security-reviewer confirmed the crypto: a captured request signature carries tag=adcp/request-signing/v1 and dies at step 3 before step 8 ever runs — the tag is bound into the base, so it can't be rewritten to pass. No replay or signature-confusion path. Accepting a request-signing key does not let a request signature through.
Blocking the approve — error-code contradiction (fix flips this to approve)
The new "Key publication" paragraph at security.mdx:1436 says the verifier:
MUST reject any other purpose with `webhook_mode_mismatch` (and absent `adcp_use` with `webhook_signature_key_purpose_invalid`)
That directly contradicts the other four surfaces this PR touches, all of which use `webhook_signature_key_purpose_invalid` for a declared-but-wrong purpose like `response-signing`:
- Step 8 (
security.mdx:1474): rejects response/governance/absent/missing-verify all with `webhook_signature_key_purpose_invalid`, and explicitly states `webhook_mode_mismatch` is "NOT used for key-purpose failures." - Error-taxonomy table (
security.mdx:1556):adcp_use ∉ {webhook-signing, request-signing}→ `webhook_signature_key_purpose_invalid`. - Changeset: "rejected with `webhook_signature_key_purpose_invalid`. `webhook_mode_mismatch` is unchanged and remains reserved for the HMAC-vs-9421 auth-mode selector."
- Conformance vector
negative/008-wrong-adcp-use.json:response-signingkey →error_code: webhook_signature_key_purpose_invalid,failed_step: 8.
So four surfaces say `key_purpose_invalid`; only line 1436 says `webhook_mode_mismatch`. This is the more dangerous half of the split: line 1436 is also the only surface that matches the SDK behavior the PR body's "Error-code nuance" note advertises ("the SDK already emits `webhook_mode_mismatch` for declared-but-wrong purposes"). An SDK that does that would emit `webhook_mode_mismatch` for a `response-signing` key and fail the 008-wrong-adcp-use vector this same PR ships, which asserts `webhook_signature_key_purpose_invalid`. The PR can't have it both ways. `webhook_mode_mismatch` is also semantically wrong here independent of consistency — it's the HMAC-vs-9421 auth-mode selector code (security.mdx:1565), not a key-purpose code.
Fix: change line 1436 to `webhook_signature_key_purpose_invalid` and drop the `webhook_mode_mismatch` reference there, matching step 8 + the table + the vector. Confirm the paired adcp-client#2239 SDK emits `key_purpose_invalid` for `response-signing`/`governance-signing` (not `webhook_mode_mismatch`), or it fails 008. That fix plus that confirmation flips this to approve.
Things I checked
- Crypto domain-separation argument is sound (
security-reviewer): tag is in the@signature-paramssigned base, checked at step 3 ahead of step 8 — no cross-protocol replay, no key-confusion path the tag doesn't already cover. Component-set overlap is irrelevant once the tag differs byte-for-byte. - Positive vector reuses the original signature correctly.
positive/008-request-signing-key-reuse.jsonkeeps keyidtest-wrong-purpose-2026and the original Ed25519 signatureOT_F-yQ…over an unchanged base — Ed25519 is deterministic, so reuse is valid. Right move. - One-directional asymmetry preserved. Request step 8 still requires
adcp_use == \"request-signing\"exactly; a webhook key is still rejected for requests. The relaxation does not leak backward. - Changeset type is correct.
minor— relaxes a constraint, removes no previously-valid behavior; existing dedicated-key signers stay valid. Blast-radius tradeoff is disclosed, not hidden (security.mdx:1438). - Negative-coverage preserved. New
test-response-purpose-2026keeps a genuinely-wrong purpose under test at step 8.
Follow-ups (non-blocking — file as issues)
- Non-normative docs still assert a dedicated
webhook-signingkey is required (migration guides, whats-new, brand-rights/governance/collection-lists task specs). The PR body already scopes these out; track the sweep.
Minor nits (non-blocking)
- Stale README Scope line.
static/compliance/source/test-vectors/webhook-signing/README.md:27still reads "adcp_useMUST be\"webhook-signing\"on the verifying JWK" — directly contradicts the relaxed rule. The PR edited the file-layout block right below it but left this normative sentence untouched. Update it to the{webhook-signing, request-signing}set. kidname now misleading.test-wrong-purpose-2026is used by a positive vector (accepted reuse) but still reads "wrong-purpose." The$commentexplains it, but the name will tempt a future maintainer to re-tighten step 8. A rename or inline note would age better.
The design is right. Resolve the line-1436 error code so the spec stops contradicting its own conformance suite, confirm the SDK agrees, and this is an approve.
|
Update: Let's consider completely removed the |
|
The direction is coherent (rely entirely on the RFC 9421 tag + KID isolation; skip the Two concerns worth putting in a follow-up issue for WG input before going further:
Recommendation: land this PR as the minimal non-breaking relaxation (accepts both; dedicated webhook key RECOMMENDED), then file a Triaged by Claude Code. Session: https://claude.ai/code/session_01LcC8kwr6UUN4Vz9nin1VBB Generated by Claude Code |
|
Will mark the |
|
Deprecation direction is coherent — marking One unresolved finding from the last automated review to fix before adding more: On the Triaged by Claude Code. Session: https://claude.ai/code/session_01HrKRMc5Lj75nXDXvfHPgUh Generated by Claude Code |
…ebhook-signing Reframe to the unified model: webhooks are signed with the agent's request-signing key (differentiated from requests by the RFC 9421 tag), with no separate webhook key purpose. Step 8 requires adcp_use == request-signing; the deprecated webhook-signing value is still accepted for backward compatibility (removed in 4.0). Key isolation, when wanted, is a second request-signing key under a distinct kid — not a distinct adcp_use. Rewrites the webhook key-publication section, the publication-shape note, the blast-radius sentence, the adcp_use enum row, and the conformance README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Design update — went with the unified model (deprecate After discussion on our side, this PR no longer frames webhook signing as "reuse your request key OR publish a dedicated webhook-signing key." Instead:
This resolves the "implicit reuse vs explicit multi-purpose Error-code resolution from your review is unchanged: all webhook key-purpose failures use Updated in Follow-up (non-blocking, will file): the non-normative docs (migration guides, whats-new, brand-rights/governance/collection-lists) still describe a |
BaiyuScope3
changed the title
|
Support the behavior, with sequencing caveats. The core model is sound: webhook delivery can accept either a dedicated I’d merge this direction once the spec is internally consistent:
So: +1 on the protocol behavior; hold merge until the doc/vector consistency and error-code cleanup are done. |
There was a problem hiding this comment.
Clean execution of a normative reversal. The problem is that a normative reversal is a WG call, not a reviewer's signature — and you said as much in the PR body.
Things I checked
- Domain-separation argument holds. The
tagis in the signed base and rejected byte-for-byte at step 3 (security.mdx:1469); existing negative vector001-wrong-tagalready proves arequest-signingtag dies at step 3. So the step-8adcp_usecheck adds zero confusion resistance on the webhook path — it was blast-radius hygiene, not domain separation.security-reviewer: Low, no reachable confusion / replay / cross-protocol attack is reopened by the relaxation. - One-directional asymmetry preserved. Request verification still requires
adcp_use == "request-signing"exactly;response-signingandgovernance-signingkeys are still rejected for webhooks. New negative008-wrong-adcp-usecovers theresponse-signingcase, so cross-purpose rejection coverage isn't lost. - Vectors are right, including the step-ordering subtlety. Positive
008-request-signing-key-reusereuses the original valid Ed25519 signature over the same webhook-tagged base — a holder of the request-signing private key can legitimately sign a webhook-tagged base, so it verifies cleanly. Negative008supplies a fresh valid signature so the rejection is provably attributable to step 8 (purpose), which runs before step 10 (crypto). Bothsuccessoutcomes are correct. - In-file normative coherence. The PR reconciles the load-bearing statements that contradict the new rule: step 8 (
security.mdx:1474), both error-taxonomy tables, the request-signing JWK table row (:1073), the "One JWK per `adcp_use`" paragraph (:955), and the Key-publication block (:1426). Checked each — they are not left stale.
The open question (why this is a comment, not an approve)
- The changeset is
minor. Flipping conformance vector 008 negative→positive means a verifier that correctly implemented the old normative MUST now fails the suite.ad-tech-protocol-expertreads that asmajor. I read it as a defensibleminor— nothing is removed on the wire,webhook-signingkeys still verify, no existing production traffic breaks; it's an additive relaxation plus a deprecation. Reasonable people differ, and that's a maintainer ruling. - More to the point: this reverses a deliberate, currently-normative single-purpose-key rule, and you flagged it yourself as a proposal for the maintainers. I'm not going to auto-approve a normative reversal on a reviewer's signature.
What flips this to approve: a WG/maintainer sign-off on the reversal plus a decision on minor vs major. The execution is sound; the call isn't mine.
Follow-ups (non-blocking)
security.mdx"five signing systems" list (~:1585) still enumerateswebhook signing (RFC 9421) — adcp_use: "webhook-signing"as a live purpose with no deprecation marker. Residual; fold it into this PR rather than the later sweep, since it sits in the same normative file you just edited.- PR body "What changed" bullet says wrong purposes reject with `webhook_mode_mismatch`. The diff, changeset, and vectors correctly use `webhook_signature_key_purpose_invalid` and reserve `webhook_mode_mismatch` for the HMAC-vs-9421 selector. The spec is right; the description is stale. Fix it before it misleads the paired adcp-client#2239 review.
- Blast-radius posture flips from "a request-key compromise cannot forge webhooks" to "it can, unless you split by `kid`." Documented and acceptable, but make the RECOMMENDED second-`kid` guidance loud in the migration note — it's the one thing operators will get wrong.
Minor nits (non-blocking)
- Key name lags its job. The reused key keeps
kid="test-wrong-purpose-2026"while now serving the positive reuse vector, and its siblingkeys.json$commentstill frames it as cross-purpose material. A key named "wrong-purpose" that is now the right purpose is a small standing irony. Rename in a follow-up.
Holding for a maintainer call on the reversal and the changeset type.
|
Checking @bokelley's five conditions against the
Triaged by Claude Code. Session: https://claude.ai/code/session_01LbBeWCWKeELirPfwjDQ6WC Generated by Claude Code |
… not 4.0 Address review on adcontextprotocol#5552: update the remaining stale webhook-signing sites missed in the first pass — the request-signing profile prose (lines 870/878), the JWKS publication example (second entry is now a request-signing key under a distinct kid, matching the prose), the five-signing-systems list, and the conformance README scope line. Replace hard-coded 'AdCP 4.0' removal language with a reference to the deprecation tracking issue (adcontextprotocol#5555), per reviewer note that the removal window is a WG/RFC decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @bokelley — all of item 4 and the 4.0 timing are addressed in Item 4 — stale
4.0 timing ✅ — removed the hard-coded "AdCP 4.0" from the deprecation notice ( The paired SDK PR (#2239, Deferred (non-blocking nit you and the bots flagged): renaming |
There was a problem hiding this comment.
The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.
This is an automated message from the Argus AI review workflow.
There was a problem hiding this comment.
The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.
This is an automated message from the Argus AI review workflow.
There was a problem hiding this comment.
Reverses the single-purpose-key rule on the webhook path only, and does it correctly: domain separation was always carried by the RFC 9421 tag, not the adcp_use discriminator, so the step-8 purpose check was redundant on a surface that already gates on the tag at step 3.
The safety argument holds in both directions. A captured request signature carries tag=adcp/request-signing/v1 and dies at webhook step 3 (webhook_signature_tag_invalid, security.mdx:1469) before the relaxed step 8 is ever reached. A captured webhook signature carries tag=adcp/webhook-signing/v1 and dies at request step 3 — and request step 8 still demands adcp_use == "request-signing" exactly (security.mdx:1237, untouched by this PR). The relaxation is one-directional, the tag is in @signature-params so it's signed, and the reverse stays strict. security-reviewer: APPROVE, no replay or bypass path, Low findings only.
minor is the right changeset type. The verifier accepts a strict superset ({webhook-signing} → {webhook-signing, request-signing}) and rejects nothing it used to accept; webhook-signing is deprecated but still honored, with removal deferred to a future major (#5555). The only major-triggering act — removing the value — is explicitly not done here.
Things I checked
- Error-code flip-flop settled correctly. Five commits bounced between
webhook_mode_mismatchandwebhook_signature_key_purpose_invalid. Final state is consistent: every key-purpose failure (wrong purpose, absentadcp_use, missingverifykey_op) returnswebhook_signature_key_purpose_invalidat step 8 (security.mdx:157, taxonomysecurity.mdx:166), andwebhook_mode_mismatchstays reserved for the HMAC-vs-9421 selector. Negative vector008-wrong-adcp-use.jsonand the taxonomy row agree. - Conformance vectors are cryptographically coherent. Positive
008-request-signing-key-reusereuses the exactSignaturebytes (OT_F-yQs…) andkeyid(test-wrong-purpose-2026) from the former negative 008 over a byte-identical base — genuinely valid, only the verdict flipped from reject-at-8 to accept. New negative 008 swaps intest-response-purpose-2026(realresponse-signingkey added tokeys.json) and still rejects at step 8. Bothjwks_refkids resolve. response-signing/governance-signingstay rejected for webhooks. Named explicitly in step 8 and exercised by the new negative vector.- Schema change is description-only.
get-adcp-capabilities-response.jsontouches only thekey_originsandwebhook_signingdescriptionstrings — no field/type/enum/required change. Non-breaking, and thewebhook_signingdescription now matches the security.mdx reframing of webhook signing as a delivery surface. Changeset present,minor, correct.
Follow-ups (non-blocking — file as issues)
request-signing.mdx:635directly contradicts the new normative rule. It still reads: "Never reuse the same key for both purposes — receivers enforce purpose at the JWKadcp_uselevel, not the RFC 9421 tag." That is the exact statement this PR reverses, stated as an imperative, in an unmodified L1 doc sitting next to the one you changed. Two L1 normative docs now disagree on the load-bearing question. The author scoped the sweep tosecurity.mdx+ vectors and disclosed it — fair — but this one is sharper than the rest of the un-swept set (webhooks.mdx,security-model.mdx,quickstart.mdx, etc.) because those stay wire-correct whilewebhook-signingis still accepted, whereas this one is an active contradiction. Worth pulling into this PR or the immediate follow-up rather than the general sweep.- #5555 should carry a hard deprecation date plus a conformance vector asserting
webhook-signingis rejected after the window (security-reviewerL1). The two-value allow-set on the webhook path is the kind of temporary breadth that ossifies if nothing forces it closed. - Make the step-3-before-step-8 ordering an explicit invariant in the webhook checklist (
security-reviewerL2). The whole relaxation rests on the tag check short-circuiting before the purpose check. The checklist already carries a "future edits MUST preserve ordering" note for the step-13 cache insert; an analogous one-liner here protects against a well-meaning future reorder.
Minor nits (non-blocking)
- PR description disagrees with what shipped. The body says wrong-purpose keys reject with
webhook_mode_mismatchand that the PR "aligns the spec to that split." The committed artifacts do the opposite — no split, all purpose failures usewebhook_signature_key_purpose_invalid. The files are self-consistent and correct; only the prose is stale. Worth correcting so the history isn't misleading. - Changeset opening sentence imprecise. "now requires
adcp_use == \"request-signing\"" reads as ifwebhook-signingis already rejected; step 8 accepts both. The changeset's own second paragraph says "both accepted." "now accepts" fixes it. test-wrong-purpose-2026is now the accepted positive case but keeps a kid named "wrong-purpose." The$commentwas updated to explain the role flip; renaming the kid would churn the fixtures, so the comment is the right call. Noted only because the name now means the opposite of what it says.
Approving on the strength of the tag-based domain-separation argument plus clean conformance-vector coverage. Fix the request-signing.mdx:635 contradiction soon — two L1 docs contradicting each other on key reuse is the one thing here that will actually confuse an implementer.
There was a problem hiding this comment.
Technically sound and safe to ship — but it reverses a deliberate normative choice and leaves a normative sibling doc asserting the opposite, so I'm holding for confirmation rather than approving. The architectural call is right: domain separation on webhooks is carried by the RFC 9421 tag (step 3) and mandatory content-digest (step 6), not by the step-8 adcp_use discriminator — so relaxing step 8 removes no confusion resistance.
Things I checked
- Safety argument holds.
security-reviewer: SOUND, no High/Medium. A captured request signature carriestag=adcp/request-signing/v1and dies at webhook step 3 before step 8 is ever reached (negative/001-wrong-tag.jsonstill exercises this). Accepting a request-signing key on the webhook path opens no new replay, cross-protocol-confusion, or escalation path. - One-directional asymmetry is intact. The request-side checklist (
security.mdxrequest step 8) is untouched — stilladcp_use == "request-signing"exactly. The relaxation is webhook-only. No hunk makes it bidirectional. Awebhook-signingkey still cannot verify a request. minoris the right semver.ad-tech-protocol-expert: this is an additive relaxation that retainswebhook-signingas an accepted value — nothing removed, no type/enum/required flip.majorwould be over-rotation. Existingwebhook-signingsigners verify cleanly against both old and new verifiers.- Schema change is description-only.
get-adcp-capabilities-response.jsontouches only twodescriptionstrings onkey_origins/key_origins.webhook_signing. Wire shape identical,webhook_signingfield retained. Not spec drift. - Error-code taxonomy is internally consistent in the shipped artifacts. Step 8, the taxonomy table, the changeset, and the vector
\$comments all agree:webhook_signature_key_purpose_invalidfor wrong/absent purpose + missingverify;webhook_mode_mismatchreserved for the HMAC-vs-9421 selector. - positive/008 is sound by construction. It reuses the original valid Ed25519 signature over a byte-identical base for
test-wrong-purpose-2026; Ed25519 is deterministic, so a sig valid in the old negative role (which died at step 8, never reaching crypto verify) stays valid now that it must pass it.code-reviewerconfirmed the 64-byte sig length and recomputed the content-digest; it could not execute a rawcrypto.verifyin the sandbox. - CI refspec change is benign.
build-check.yml:25adds+to force-update the 3.0.x drift-comparison ref. Unrelated to the webhook change, harmless.
What flips this to approve
- Resolve the normative contradiction in
request-signing.mdx:635. That line — same L1 layer, normative hands-on guide — still says: "Never reuse the same key for both purposes — receivers enforce purpose at the JWKadcp_uselevel, not the RFC 9421 tag." That is the exact inverse of this PR's central claim. An implementer following it ships a verifier that rejects your new positive/008. Your PR body scoped the sweep to "non-normative docs," but this one is normative and directly load-bearing. Sweep it here (or add a one-line deprecation banner pointing at#webhook-callbacks).security-model.mdx:145,164is the next-most-load-bearing offender — it's linked directly from the paragraph you edited. - Confirm the directional reversal is sanctioned. You flagged this yourself as an RFC reversing a deliberate single-purpose-key rule and "the maintainers' call." I'm not going to auto-approve a normative reversal on an automated pass — that's a WG decision. If the WG has signed off, say so and I'll move.
Follow-ups (non-blocking — file as issues)
- PR description contradicts the diff on error codes. The "What changed" bullet and "Notes for reviewers" say step 8 rejects wrong purposes with
webhook_mode_mismatch. Every committed artifact correctly useswebhook_signature_key_purpose_invalid. The diff is right; the prose is stale. Fix it before the pairedadcp-client#2239implements the wrong mapping off your description. - Reverse-direction coverage is spec-only. No request-signing conformance vector exercises "a
webhook-signingkey is rejected for a request" — the now-load-bearing forbidden direction is asserted in prose but not graded. Addrequest-signing/negative/*-webhook-purpose-key.json. - No governance-signing webhook negative vector. The new negative/008 covers
response-signing; the step-8 prose also namesgovernance-signingas rejected.response-signingis the representative cross-purpose case, but a governance-signing vector would match the enumerated prose. - Run one
crypto.verifyover positive/008 in CI. Closes the one check the review sandbox couldn't execute. Sound by construction, but measure it once.
Minor nits (non-blocking)
kidname is now misleading. positive/008 accepts a key whosekidis literallytest-wrong-purpose-2026. Thekeys.json\$commentwas updated to explain it, so it's documented — but akidnamed "wrong-purpose" on an accepted positive vector is a notable choice. Rename on the next vector-regen pass.
--comment, not --approve: the execution is clean and the threat model is unchanged, but I won't rubber-stamp a normative reversal that still has a sibling spec saying the opposite. Resolve request-signing.mdx:635 and confirm the WG call, and I'll approve.
There was a problem hiding this comment.
The relaxation is sound and I can't fault the security model — but this is a normative reversal you've flagged as the maintainers' call, and it ships with a conformance suite that now contradicts itself. Holding at comment, not approve, until both clear.
The crux is right: domain separation between requests and webhooks is carried by the RFC 9421 tag, which is inside @signature-params, covered by the signature, and checked at step 3 — before key resolution. adcp_use == "webhook-signing" at step 8 was redundant domain separation, not the load-bearing control. security-reviewer: no-finding. I walked both replay directions with it — a captured request signature carries tag=adcp/request-signing/v1 and dies at webhook step 3; a captured webhook signature carries tag=adcp/webhook-signing/v1 and dies at request step 3. No single signature verifies on both paths. The one-directional asymmetry (webhook MAY use a request key; request MUST still require request-signing exactly) is enforced at security.mdx:1237 and unchanged.
Things I checked
- Schema is description-only.
get-adcp-capabilities-response.jsontouches twodescriptionstrings (key_origins,webhook_signing);type/format/additionalProperties:false/x-adcp-validationanchoring all retained. Wire shape unchanged.ad-tech-protocol-expert:minoris correct — a verifier-acceptance relaxation plus a deprecation-with-no-removal is additive; no previously-accepted message becomes rejected. Removal in #5555 is the futuremajor. - Error-code routing is internally consistent. Step 8 (
security.mdx:230), the taxonomy table (security.mdx:239), the changeset, andnegative/008all route every purpose failure — wrong purpose, absentadcp_use, missingverifykey_op — towebhook_signature_key_purpose_invalid, and reservewebhook_mode_mismatchfor the HMAC-vs-9421 selector.error-code.jsoncorrectly untouched (these transport codes don't live in that enum). - Vector 008 wiring.
positive/008-request-signing-key-reusereuses byte-identical kid + base + signature from the prior valid negative vector and flipsexpected_outcometo success — validity inherited, self-consistent.negative/008retargets to the newtest-response-purpose-2026(response-signing), still rejected at step 8. Governance- and response-signing keys remain invalid on the webhook path — confirmed by the new negative vector. build-check.yml+3.0.x. Correct and safe — forces the ephemeral CI tracking ref to update on a non-fast-forward after a 3.0.x rebase; mutates only the checkout, never the remote.
What would flip this to approve
- WG/maintainer sign-off on the direction. You've framed this as a proposal reversing a currently-normative rule. No
do-not-auto-approvelabel is set, so I'm not auto-holding — but I'm not going to auto-approve a normative security-model reversal on my own signature either. That's a human call. - Resolve the conformance-suite contradiction.
static/compliance/source/universal/webhook-emission.yaml:630(assert_webhook_signing_key_present) still asserts a webhook-emitting agent's JWKS MUST contain anadcp_use: "webhook-signing"key and explicitly statesrequest-signingkeys "do not satisfy this check," failing withwebhook_signing_keys_wrong_purpose. A signer that adopts this PR's recommended pattern — sign webhooks with arequest-signingkey, publish nowebhook-signingkey — fails the suite's own on-ramp gate. The verifier vectors say acceptrequest-signing; the emission grader says signers must publishwebhook-signing. Update the grader to acceptrequest-signingon emission, or explicitly track it as a blocking follow-up that lands before #5555.ad-tech-protocol-expert: sound-with-caveats, this is the caveat.
Follow-ups (non-blocking — file as issues)
- PR body contradicts the implementation on the error code. The description's "What changed" says step 8 rejects wrong purposes with
webhook_mode_mismatch. The shipped spec, changeset, taxonomy table, andnegative/008all usewebhook_signature_key_purpose_invalidand explicitly reservewebhook_mode_mismatchfor the auth-mode selector. The artifacts are right; only the prose is stale — fix it so the paired adcp-client#2239 doesn't follow the wrong routing. negative/008signature is not CI-validated. It carries a fresh signature over a regenerated base fortest-response-purpose-2026and assertsfailed_step: 8. Nothing inbuild-compliance.cjsortest:hmac-vectorscryptographically verifies the 9421 webhook vectors — if that signature is invalid, a runner rejects at step 10 (crypto), not step 8 (purpose), and the assertion silently diverges. Confirm it was generated against that key's publishedx.- Non-normative doc sweep (acknowledged in the PR):
docs/reference/test-vectors/index.mdx:50omits the newly-publishedtest-response-purpose-2026from the public-keys warning — a security-warning completeness gap, since its private material is now on the CDN. Plus theL3/webhooks.mdx,seller-integration.mdx, andget_adcp_capabilities.mdxreferences the PR body lists.
Minor nits (non-blocking)
keys.jsonkidtest-wrong-purpose-2026is now a misnomer. It is an accepted request-signing key after this change, not a wrong-purpose one. All in-repo references are internally consistent with its new role, so nothing dangles — renaming would churn the signature base, so leaving it is defensible. Notable that the negative-path key kept its name while switching sides.
Sound model, clean schema. Comment, not approve — get WG agreement on the reversal and fix the emission grader (webhook-emission.yaml:630) so the suite doesn't reject the pattern it now recommends, and this flips to approve.
There was a problem hiding this comment.
The implementation is clean and ships as-is; the open question is direction, not code. You labeled this "Proposal / RFC for discussion … it's the maintainers' call" and the committed artifacts read as a decided change with a removal issue (#5555). Those can't both be true. Resolve that and I approve.
The technical relaxation is sound. Domain separation moves off the adcp_use discriminator and onto the RFC 9421 tag, which is the right control: the tag is inside @signature-params, so it's part of the signed base and can't be swapped without the private key. Verified bidirectionally:
- Request sig replayed on the webhook route → dies at webhook step 3 (
security.mdx:1469,webhook_signature_tag_invalid). Unchanged by this PR. - Webhook sig (now signed with a
request-signingkey,tag=adcp/webhook-signing/v1) replayed on the request route → request step 8adcp_use == "request-signing"now passes, but request step 3 (security.mdx:1232) requiresadcp/request-signing/v1and rejects first. Step 3 runs before step 8, so the tag is the load-bearing gate and it holds.
The relaxation is correctly one-directional: request verification still requires adcp_use == "request-signing" exactly (security.mdx:218), and response-signing/governance-signing are never valid on the webhook path — new negative 008-wrong-adcp-use exercises exactly that.
security-reviewer: no High, no Medium. ad-tech-protocol-expert: sound-with-caveats, minor is correct (additive acceptance + deprecation, nothing previously-valid starts rejecting; repo is on the 3.1.0-rc.0 line). code-reviewer: all six machine artifacts internally consistent.
Things I checked
- Bidirectional replay defense. Request step 3 and webhook step 3 tag checks are both untouched (
security.mdx:1232,:1469); the tag is in the signed base. This is the whole safety argument and it's correct. - Conformance vectors. Positive
008-request-signing-key-reusereuses byte-identicalSignature/expected_signature_basefrom the former negative 008 undertest-wrong-purpose-2026(an Ed25519 request-signing key) — same base + same key + deterministic Ed25519 → still verifies,success:trueis right. New negative 008 fails at step 8 before crypto, so its fresh signature is non-load-bearing. - Schema is description-only.
get-adcp-capabilities-response.jsonedits twodescriptionstrings;webhook_signingcapability, the["adcp/webhook-signing/v1"]profile_version enum,key_origins.webhook_signing, and thex-adcp-validationrule are all retained. No wire-shape change, no fail-open. Not drift — the new prose matches the docs. - Changeset present, type correct.
minormatches a relaxation + deprecation that keepswebhook-signingaccepted. - Error taxonomy (committed) is internally consistent. Step 8 routes all key-purpose failures to
webhook_signature_key_purpose_invalid;webhook_mode_mismatchstays reserved for the HMAC-vs-9421 selector (security.mdx:230,:239).
What flips me to approve
- Resolve the RFC framing. Drop the "Proposal / RFC for discussion" banner if this is a decided WG direction, or mark the PR draft if it isn't. Reversing the single-purpose-key rule across the ecosystem is a direction call; I won't auto-approve something the author flagged as still open.
- Reconcile the PR description with the diff. The "What changed" / "Error-code nuance" section says step 8 should reject wrong purposes with
webhook_mode_mismatchand reservekey_purpose_invalidfor absent purpose. The committedsecurity.mdxand changeset do the opposite — all purpose failures getkey_purpose_invalid. The committed taxonomy is the better one (webhook_mode_mismatchis an auth-mode concept, not a key-purpose one). Just update the description; the code is right. The PR body is reviewing an earlier draft than the one in the diff.
Follow-ups (non-blocking — file as issues)
- Doc sweep debt. Non-normative surfaces (migration guides, what's-new, brand-rights/governance/collection-list task specs) still say "mint a webhook-signing key." You scoped that out deliberately — track it so adopters don't hit stale guidance during the deprecation window.
- Protect the asymmetry in shared verifier code (
security-reviewerL1). Safety depends onrequest-signingbeing accepted on the webhook path whilewebhook-signingis NOT accepted on the request path. The spec recommends sharing verifier code — add a request-suite negative vector asserting awebhook-signing-tagged signature is rejected at request step 3, so a future "symmetry" refactor can't silently reopen reverse replay. - #5555 narrowing. Confirm removal of
webhook-signingacceptance is scheduled so the webhook-path accepted set narrows back to one value once signers migrate. Note in #5555 that thetag/profile name (adcp/webhook-signing/v1) deliberately outlives the key-purpose name — a cleanup shouldn't try to rename the tag.
Minor nits (non-blocking)
- Drive-by CI fix.
build-check.yml:25adds the+force prefix toorigin +3.0.x:refs/remotes/origin/3.0.x— correct (forces a non-FF update of the ephemeral local tracking ref, can't touch the remote), but unrelated to webhook signing. Fine to keep; worth a line in the description so it isn't a surprise ingit blame.
Clean work. Flip the two items above and this is an approve.
There was a problem hiding this comment.
Widens the webhook verifier to accept a request-signing key and deprecates the dedicated webhook-signing purpose. Right call: domain separation on webhooks is carried by the RFC 9421 tag in the signed base, not by the adcp_use discriminator — so dropping the discriminator's strictness on the webhook path costs nothing the tag wasn't already buying.
Things I checked
- The load-bearing change is step 8 of the webhook verifier checklist (
security.mdx). Final state acceptsadcp_use ∈ {request-signing, webhook-signing}, rejects everything else (absentadcp_use, missingverifykey_op,response-signing,governance-signing) withwebhook_signature_key_purpose_invalid.webhook_mode_mismatchstays reserved for the HMAC-vs-9421 selector. Checklist, taxonomy table row,negative/008expected_outcome, and thekeys.json$commentfortest-response-purpose-2026all agree. - The relaxation is one-directional and the asymmetry is enforced by two independent gates. Webhook path accepts
request-signing; request path (untouched) still demandsadcp_use == "request-signing"exactly. A captured request signature carriestag=adcp/request-signing/v1and dies at webhook step 3 before step 8 is reached; a webhook signature carriestag=adcp/webhook-signing/v1and dies at request step 3. No replay or downgrade path opens.security-reviewer: no High — thetagis the load-bearing separator both directions,content-digeststays mandatory with no opt-out, request verification is unchanged and strict. - Conformance vectors are consistent by construction.
positive/008-request-signing-key-reusereuses kidtest-wrong-purpose-2026and the verbatim Ed25519 signature from the former negative 008 — valid because that vector always failed at step 8 (purpose), never step 10 (crypto), over an identical signed base. Newnegative/008swaps intest-response-purpose-2026(response-signing).jwks_ref,Signature-Inputkeyid, andexpected_signature_basekeyid line up in both. - No residual normative contradiction in
security.mdx. The old "MUST reject any JWK whoseadcp_useis not exactlywebhook-signing" passages (key-publication prose, step 8, taxonomy row) are all removed-and-replaced in-hunk, not left dangling. Verified against the diff directly and viacode-reviewer's base-line→replacing-hunk mapping. - Schema change is description-only (
get-adcp-capabilities-response.json:key_originsandkey_origins.webhook_signing). No field/type/enum/required change;adcp_usestays a string;webhook_signingcorrectly reframed as the delivery surface, not a required live key purpose.ad-tech-protocol-expert: sound-with-caveats, no wire drift,minordefensible. - Changeset present (
.changeset/webhook-allow-request-signing-key-reuse.md,minor). Additive widening + deprecation-without-removal —minoris the right shape. Thetag/profile name staysadcp/webhook-signing/v1(correct — that's the wire identifier; only the key purpose moved). build-check.yml+3.0.x:refs/remotes/origin/3.0.xforce-updates a local remote-tracking ref in a throwaway CI checkout. Pushes nothing, makes the drift comparison resilient to a non-fast-forward3.0.x. Benign.
Follow-ups (non-blocking — file as issues)
- State the semver rationale in the changeset. This relaxes a verifier MUST — an implementation built to the old "exactly
webhook-signing" text now fails the newpositive/008grading.minoris correct under AdCP's wire-contract semver policy (no signer/verifier on the network desyncs), but a reviewer applying the strict "MUST relaxed = major" reading will challenge it. Name the verifier-relaxation-vs-wire-contract distinction in the changeset. The eventual #5555 removal ofwebhook-signingacceptance will bemajor— say so to set the deprecation arc cleanly. - Sweep the remaining non-normative "mint a webhook-signing key" prose. The PR body already flags this. The authoritative passages are reconciled; adjacent examples/asides during the compat window are not. Sweep before #5555 lands so the deprecation removal isn't fighting stale guidance.
- Confirm the regenerated
negative/008signature in CI.code-reviewercould not re-run Ed25519 verification on the newtest-response-purpose-2026signature bytes (sandbox). Structurally consistent; let the.context/generate-webhook-vectors.mjsgrading path confirm the bytes before merge. - (Security, Low — already disclosed.) Default posture drops from isolation-by-default to isolation-opt-in: an operator publishing a single
request-signingkey now shares key material across requests and webhooks. The PR documents the distinct-kidmitigation honestly. Consider a SHOULD nudging a webhook-dedicatedkidfor agents whose request-signing key also authenticates mutating tool calls — parallel to the existing publisher-pin MUST for mutating-scope agents.
Minor nits (non-blocking)
- Imprecise anchor.
security.mdxstep 8 links[Downgrade and injection resistance](#webhook-callbacks)— the label names a subsection with no heading of its own, so the anchor resolves to the parent#### Webhook callbacks. Link works; label is loose.
The error-code taxonomy changed its mind once between commits 1 and 2 (webhook_mode_mismatch → webhook_signature_key_purpose_invalid for the response-signing case); the final state is internally consistent across all five sites. Worth noting only so the next reader doesn't re-derive the contradiction from the commit history.
Approving on the strength of the tag-carries-separation argument plus consistent conformance vectors. Follow-ups noted.
There was a problem hiding this comment.
Sound relaxation. Domain separation on webhooks was always carried by the RFC 9421 tag and mandatory content-digest, never by the adcp_use discriminator — so widening step 8 to accept the request-signing key costs no confusion resistance.
Things I checked
- The relaxation is one-directional. Webhook step 8 now accepts
adcp_use ∈ {request-signing, webhook-signing}(security.mdxverifier checklist step 8), but the request checklist is untouched: request step 3 still requirestag=adcp/request-signing/v1and request step 8 still requiresadcp_use == "request-signing"exactly. A webhook-signing key verifying a request is forbidden by two independent gates, neither relaxed.security-reviewer: no-issue. - Replay is blocked before step 8 is reached. The
tagis inside@signature-params(signed base), and webhook step 3 rejectsadcp/request-signing/v1withwebhook_signature_tag_invalid— five steps before the key-purpose check. A captured request signature dies at step 3 regardless of which key the webhook path now accepts. Regression-covered bynegative/001-wrong-tag. - Schema source changes are description-only. All four files (
get-adcp-capabilities-response.jsonkey_origins,sync-accounts-request.json,push-notification-config.json,webhook-challenge.json) touch onlydescriptionstrings — no field rename, type, enum, required↔optional,additionalProperties, oroneOfchange.ad-tech-protocol-expert: no drift. Property namewebhook_signingand itsx-adcp-validationanchor are unchanged; it now names the delivery surface, not a required live purpose. minoris correct. Acceptance-set widening + deprecation without removal. No conformant signer or verifier breaks. The breaking removal ofwebhook-signingis correctly deferred to #5555 / a major bump.- Error-code consistency holds across all four surfaces — spec step 8, the taxonomy table row, the negative-vector
error_code, and the test assertion all usewebhook_signature_key_purpose_invalidfor wrong/absent/missing-verifypurposes, and reservewebhook_mode_mismatchstrictly for the HMAC-vs-9421 selector. - Vectors are cryptographically self-consistent. Positive
008-request-signing-key-reusereuses the identical key, signature, and base as the former negative, flipping onlyexpected_outcome— so the crypto is unchanged and only the policy outcome moved. New negative008-wrong-adcp-useswaps in a freshresponse-signingkey, still failing step 8.tests/webhook-signing-vectors.test.cjsproves negative/008 verifies cryptographically before the step-8 policy rejection — exactly the right way to encode this. build-check.ymlforce-fetch (+3.0.x:refs/remotes/origin/3.0.x) is the correct fix for surviving a non-fast-forward update of the comparison branch under--depth=1; the only downside (silent overwrite of a diverged local ref) is irrelevant in a fresh CI checkout.
Follow-ups (non-blocking — file as issues)
- The test that proves this change is the one test not wired into CI.
tests/webhook-signing-vectors.test.cjsis only reachable vianpm run test:webhook-signing-vectors(package.json:47);build-check.yml:123runs only the two HMAC scripts, andvitest.config.tsexcludes**/*.test.cjs, sotest:unitskips it. The cryptographic proof the PR rests on is enforced on local pre-commit, not on PRs. Append&& npm run test:webhook-signing-vectorsto the conformance step atbuild-check.yml:123. (code-reviewer: Minor.) - Non-normative docs sweep: several task docs still say to mint a
webhook-signingkey and still link/docs/building/implementation/security#...anchors against the canonicaldocs/building/by-layer/L1/security.mdx. Wire-compatible during the deprecation window; sweep after this lands (already noted in the PR description).
Minor nits (non-blocking)
- Changeset opener reads as a tightening.
.changeset/webhook-allow-request-signing-key-reuse.md:11leads with "step 8 now requiresadcp_use == \"request-signing\"" before the parenthetical restoreswebhook-signing. Not contradictory, but "acceptsrequest-signing(canonical) orwebhook-signing(deprecated)" removes the one spot a reader could misinfer a breaking change. - sf-binary parser is base64url-only.
tests/webhook-signing-vectors.test.cjs:24regex^sig1=:([A-Za-z0-9_-]+):$matches only the base64url alphabet. All current vectors happen to emit base64url, so it works; a future vector with standard base64 (+///=) would throw rather than parse. Fails loud, not a false-pass — widen the charset when convenient.
Approving on the strength of the verified step-3 tag gate plus the cryptographic vector test. Follow-ups noted.
There was a problem hiding this comment.
Clean key-reuse relaxation: webhooks now verify under request-signing keys, with webhook-signing deprecated-but-accepted. The right call, because the load-bearing domain-separation control was never the adcp_use discriminator — it's the RFC 9421 tag in the signed base, checked at step 3 before step 8 is ever reached.
Things I checked
- Replay direction is blocked. A captured request signature carries
tag=adcp/request-signing/v1in its@signature-params; webhook step 3 (security.mdx:1469) rejects on tag byte-mismatch before the relaxed step-8 purpose check runs. Step 3 gates step 8 — the ordering is load-bearing and correct.security-reviewer: no new key-confusion vector. - Relaxation is one-directional. Request verifier step 8 still requires
adcp_use == "request-signing"exactly and is untouched by this PR. A webhook frame can't replay as a request (request step 3 rejectsadcp/webhook-signing/v1). The widening leaks nowhere. response-signing/governance-signingstill reject. Step 8 prose (diff L300) and the newnegative/008-wrong-adcp-use.json(now aresponse-signingkey) both enforce it. Accepted set is exactly{request-signing, webhook-signing}.- Vector flip is sound by construction.
positive/008-request-signing-key-reuse.jsonreuses the byte-identical signature (sig1=:OT_F-yQs…:) and base from the formernegative/008undertest-wrong-purpose-2026— that signature was always cryptographically valid; only the verdict flips from reject to accept. Newnegative/008is a freshly generated valid signature undertest-response-purpose-2026. The new test (tests/webhook-signing-vectors.test.cjs) asserts the negative signature verifies before the step-8 policy rejection — so it fails for the right reason. - No schema drift. The four
static/schemas/source/**edits are description-only — no field/type/enum/required-optional change,adcp_usestays a string. The one live enum in blast radius,webhook_signing.versions: ["adcp/webhook-signing/v1"]atget-adcp-capabilities-response.json:1192, is the wire-profile tag and is correctly left alone.ad-tech-protocol-expert: sound. minoris correct. Widens the verifier accept-set and deprecates without removing — every previously-conformant signer/verifier stays conformant. The removal ofwebhook-signingis deferred to #5555 and flagged major. Changeset present and accurately worded.- CI wiring.
test:webhook-signing-vectorsadded to the dedicated build-check step and chained into thetestaggregate. The3.0.x:→+3.0.x:refspec force-update is standard syntax, safe under--depth=1in an ephemeral checkout.
Follow-ups (non-blocking — file as issues)
- The doc-path sweep missed the strings it was standing on. This PR migrates
docs/building/implementation/security→docs/building/by-layer/L1/securityacross the corpus, butget-adcp-capabilities-response.json:~1015still points itskey_originsdescription at the olddocs/building/implementation/security.mdx §Origin separation. Cosmetic prose inside a description string — fold it into the follow-up sweep the PR body already acknowledges. - Blast-radius migration wording. Default key reuse means a
request-signing-key compromise now extends to webhooks. The docs disclose this and name the mitigation (a secondrequest-signingkey under a distinctkid). For operators who today run separatewebhook-signingmaterial, strengthen the migration note to "keep separate key material to preserve your existing isolation" so a collapse-to-one-key migration isn't a silent posture downgrade.security-reviewerL1.
Minor nits (non-blocking)
- Run the suite once on PR head. Static + cryptographic analysis says green (8 positive + negative/008), but capture the actual
test:webhook-signing-vectorstally before merge —code-reviewerand I both verified by inspection, not execution.
LGTM. Follow-ups noted below.
…re paths Align with the merged spec (adcontextprotocol/adcp#5552): webhooks are signed with the request-signing key, so the sell-side emission grader (assertJwksPurpose) and the tenant-registry webhook auto-wire (assertWebhookSigningUse) must accept adcp_use 'request-signing' as well as the deprecated 'webhook-signing'. Previously both hard-required 'webhook-signing' and would reject a conformant request-signing agent. Updates the auto-wire tests and the signWebhook doc comment to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…re paths Align with the merged spec (adcontextprotocol/adcp#5552): webhooks are signed with the request-signing key, so the sell-side emission grader (assertJwksPurpose) and the tenant-registry webhook auto-wire (assertWebhookSigningUse) must accept adcp_use 'request-signing' as well as the deprecated 'webhook-signing'. Previously both hard-required 'webhook-signing' and would reject a conformant request-signing agent. Updates the auto-wire tests and the signWebhook doc comment to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(signing): accept reused request-signing key for webhook delivery
Webhook verifier step 8 now accepts a key whose adcp_use is either
'webhook-signing' or 'request-signing', letting a signer reuse the
request-signing key it already publishes instead of minting a dedicated
webhook key. Any other purpose (response-signing, governance-signing,
unknown) is still rejected with webhook_mode_mismatch.
Safe because domain separation is carried by the RFC 9421 tag
(adcp/webhook-signing/v1, part of the signed base) plus mandatory
content-digest coverage, not by the key-purpose discriminator: a captured
request signature can never be replayed against the webhook verifier
because step 3 rejects its tag.
signWebhook / signWebhookAsync accept the same set. Conformance vector
008 flipped from negative (request-signing rejected) to positive
(request-signing reuse accepted); a new negative 008 covers a
response-signing key. A dedicated webhook key remains RECOMMENDED but is
no longer REQUIRED.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(signing): use key_purpose_invalid for all webhook key-purpose failures
Per reviewer feedback on the paired spec PR: stop overloading
webhook_mode_mismatch (reserved for the HMAC-vs-9421 auth-mode selector) for
key-purpose failures. Step 8 now emits webhook_signature_key_purpose_invalid
for absent adcp_use, a missing verify key_op, OR an adcp_use outside
{webhook-signing, request-signing}. Updates the negative 008 vector, keys.json
and fixture README, and the test assertion to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(signing): webhooks use the request-signing key; deprecate webhook-signing
Reframe to the unified model: webhooks are signed with a request-signing key
(no separate webhook key purpose). Verifier step 8 and the signer gates still
accept request-signing and the deprecated webhook-signing (backward compat,
removed in 4.0); the error message and all doc comments now lead with
request-signing. Marks the AdcpUse 'webhook-signing' member deprecated and
updates the webhook-emitter key-purpose guidance (reuse the request-signing
provider; isolate via a second request-signing kid, not a distinct adcp_use).
No behavior change — code already accepted both purposes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(signing): reference webhook-signing removal issue instead of 4.0
Mirror the spec review fix: replace hard-coded '4.0' removal language in the
AdcpUse deprecation notes, verifier/signer/emitter comments, changeset, and
vendored conformance README with a reference to the tracking issue
(adcontextprotocol/adcp#5555), since the removal window is a WG decision.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(signing): accept request-signing keys on webhook emission/auto-wire paths
Align with the merged spec (adcontextprotocol/adcp#5552): webhooks are signed
with the request-signing key, so the sell-side emission grader
(assertJwksPurpose) and the tenant-registry webhook auto-wire
(assertWebhookSigningUse) must accept adcp_use 'request-signing' as well as the
deprecated 'webhook-signing'. Previously both hard-required 'webhook-signing'
and would reject a conformant request-signing agent. Updates the auto-wire
tests and the signWebhook doc comment to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Why
This PR implements the chosen direction for webhook key reuse: deprecate dedicated
adcp_use: "webhook-signing"keys while continuing to accept them for backward compatibility. Paired implementation PR: adcontextprotocol/adcp-client#2239.Today a signer that already publishes an
adcp_use: "request-signing"key must also mint and publish a second, dedicatedadcp_use: "webhook-signing"key to emit webhooks. The webhook verifier rejects any other purpose at step 8. That means two keys, two rotation procedures, and two JWKS entries for an operator who would rather reuse one key.This PR lets the signer choose: reuse the request-signing key for webhooks, or keep a dedicated webhook-signing key. The verifier accepts both during the deprecation window.
Why it's safe
Cross-protocol confusion on webhooks is already prevented by two controls that are independent of key purpose:
tag:adcp/webhook-signing/v1is part of the RFC 9421 signed base and checked at step 3. A captured request signature carriestag=adcp/request-signing/v1and is rejected there. The spec already says this: "The distinct tag prevents a request signature from being replayed as a webhook signature and vice versa."content-digestcoverage at step 6.The step-8
adcp_usediscriminator adds blast-radius isolation, not the load-bearing domain-separation control. Dedicated webhook keys remain RECOMMENDED for operators that want that isolation.The relaxation is one-directional: a webhook MAY be verified with a request-signing key, but request verification still requires
adcp_use == "request-signing"exactly.response-signingandgovernance-signingkeys are never valid for webhooks.What changed
security.mdxwebhook verifier checklist step 8 acceptsadcp_usein{webhook-signing, request-signing}.webhook_signature_key_purpose_invalid.webhook_mode_mismatchremains reserved for the HMAC-vs-9421 auth-mode selector and is not used for key-purpose failures.security.mdxkey publication and cross-purpose reuse guidance now permits one-directional reuse for webhooks and downgrades the dedicated webhook key from REQUIRED to RECOMMENDED.security.mdxone-JWK-per-adcp_usepublication-shape section documents the reuse exception.adcp_useremains a string, never an array.request-signingkeys; deprecatedwebhook-signingkeys are still accepted for compatibility.identity.key_origins.webhook_signingnames the webhook delivery surface, not a required live key purpose.008-wrong-adcp-usebecame positive008-request-signing-key-reuse; new negative008-wrong-adcp-usecovers aresponse-signingkey and still fails step 8.request-signingor deprecatedwebhook-signing.build-check.ymlforce-refreshes the ephemeral localorigin/3.0.xref with+3.0.x:refs/remotes/origin/3.0.xso CI does not fail after a non-fast-forward update of the comparison branch.minor: this widens verifier acceptance and deprecates the old purpose without removing support.Notes for reviewers
adcp_usefrom a string into an array or introducing a combined value, which is intentionally out of scope.webhook-signingkey. They remain wire-compatible during the deprecation window, but should be swept after this lands.dist/compliance/**is left to the release pipeline; only source compliance artifacts are edited here.Generated with Claude Code