spec(webhook-signing): pin on-wire JSON form for HMAC signature (closes #2464)#2478
Merged
spec(webhook-signing): pin on-wire JSON form for HMAC signature (closes #2464)#2478
Conversation
#2464) Legacy HMAC-SHA256 webhook scheme signs {ts}.{raw_body} but did not pin the JSON serialization form, so Python json.dumps (spaced) signers sent compact bytes on the wire and every compliant verifier returned 401. Pin compact separators ("," / ":"), mandate byte-equality with the wire body, and surface the symmetric signer-side trap on the RFC 9421 content-digest path. - security.mdx: canonical on-wire form, non-canonicalized aspects (duplicate-key SHOULD NOT), verifier MUST use raw bytes / SHOULD NOT re-serialize. Parallel signer-side note on 9421 content-digest. - webhooks.mdx: signing-algorithm prose calls out the Python trap with json.dumps(..., separators=(",", ":")) as the fix. - test vectors: positive (whitespace-sensitive + nested + escaped unicode), rejection (spaced-form bug). - CI: rejection_vectors now iterated; compact/spaced matcher tightened. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Schema Link Check ResultsCommit:
|
EmmaLouise2018
approved these changes
Apr 20, 2026
…number-format notes Review nudges on #2478: - Flip "Python outlier" framing — compact is the default in JS, Go, Ruby, Jackson, httpx. Python json.dumps is the notable exception. - Middleware-ordering note on the verifier raw-capture rule — express.raw() / FastAPI Request.body() must run before any JSON-parse middleware on the same route, or the verifier ends up on a re-stringified body. - Concrete number-representation example on the non-canonicalized bullet (JSON.stringify(1.0) -> '1' vs Python json.dumps(1.0) -> '1.0' vs Go json.Marshal(1.0) -> '1') — makes the "compare bytes, not parsed values" rule land. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
to adcontextprotocol/adcp-client-python
that referenced
this pull request
Apr 20, 2026
Upstream AdCP spec PR adcontextprotocol/adcp#2478 tightened the webhook signature verifier rule from "MAY re-serialize" to "SHOULD NOT re-serialize", with verifiers that cannot capture raw bytes required to MUST fail closed. Rationale: re-serializing a parsed payload to reconstruct the signed bytes silently fails against signers whose output differs in separator choice, key order, unicode escape policy, or number formatting — masking signer bugs the verifier should surface. Our ADCPClient._verify_webhook_signature had exactly that fallback path at client.py:3307: when raw_body was None, it called json.dumps(payload) with Python-default spaced separators. Historically this "worked" only in the coincidental case where the signer also used Python-default separators; against any compliant signer (httpx json= or JS JSON.stringify, both compact) it produced false-rejects. After PR #205's signer fix it produced false-rejects against our own signer too. Fix --- - Remove the json.dumps(payload) fallback entirely. When raw_body is None, log an actionable error and return False. - Update docstring to cite adcp#2478 and point at common framework hooks for capturing raw bytes (FastAPI Request.body, Flask request.get_data, aiohttp Request.read, Express express.raw). - Existing tests that relied on the fallback now pass raw_body= explicitly. Same signature math, just spec-conformant call shape. - New test_verify_fails_closed_when_raw_body_missing pins the fail-closed behavior by signing a real HMAC, omitting raw_body, and asserting the verifier raises ADCPWebhookSignatureError. Paired with a positive sibling that proves the rejection is specifically about missing raw_body, not signature or payload shape. Audit of the RFC 9421 content-digest path in src/adcp/signing/ — the parallel trap called out in adcp#2478 is absent here: sign_request takes body: bytes and client.py:488 passes request.content (httpx's actual wire bytes), so digest input and wire body are byte-identical by construction. BREAKING CHANGE: ADCPClient webhook verification now requires raw_body to be passed through from the HTTP handler. Callers that relied on the implicit re-serialize-from-payload fallback will start seeing ADCPWebhookSignatureError until they plumb the raw body through from their framework's pre-parse hook. Fix path: # FastAPI @app.post("/webhook") async def hook(request: Request, x_adcp_signature: str = Header(...)): raw = await request.body() payload = await request.json() # order matters: read raw first result = await client.handle_webhook( payload, ..., signature=x_adcp_signature, raw_body=raw, ) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
to adcontextprotocol/adcp-client-python
that referenced
this pull request
Apr 20, 2026
Pull in the merged test vectors from adcontextprotocol/adcp#2478 which pinned compact separators as the canonical on-wire form, added vectors covering whitespace-sensitive keys / nested objects / escaped unicode, and added the rejection vector that reproduces the Python-default spaced-separators signer bug. Changes ------- - tests/fixtures/webhook-hmac-sha256.json — regenerated from https://github.com/adcontextprotocol/adcp/blob/main/static/test-vectors/webhook-hmac-sha256.json (14 positive vectors including 2 new, 10 rejection vectors including the signer-serialization-mismatch case). - HMAC_REJECTION_VECTORS loaded alongside the existing positive vectors. - New test_rejection_vectors_do_not_collapse_to_positive mirrors the upstream CI check: for every rejection vector whose claimed signature has a well-formed sha256=<hex> shape, verify a correctly-computed HMAC over the claimed raw_body does NOT match the claimed signature — otherwise the rejection vector silently collapses into a positive case and stops catching what it claims to. - New test_verifier_rejects_upstream_rejection_vectors is the behavioral mirror: for every rejection vector with enough context to run end-to-end, assert ADCPClient._verify_webhook_signature returns False. Together the two parametrizations pin both the math (claimed sig is genuinely invalid) and the behavior (our verifier catches it). Vectors with non-computable rejection shapes (empty/null signature, wrong length, double-prefix) skip the computational check — they're documented for verifier implementers but don't have a signature a correctly-configured HMAC would produce. Full pytest: 1674 passed / 18 skipped (15 of the new skips are structural-rejection vectors appropriately declining the computational parametrization). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…ors from prose Follow-up to #2478 addressing three review comments: - Promote MUST-compare-bytes to its own top-level bullet in security.mdx (was buried mid-paragraph in "Non-canonicalized aspects"). Frame "Canonical on-wire form" and "Verifier input" as failure-class narrowers on either side of the invariant. Inline the rationale (key order, unicode escapes, number formatting diverge across serializers) so readers don't have to scan forward. - Replace vague "httpx / most HTTP-client JSON bodies" with a concrete enumeration split by language-level serializer vs HTTP client. Verified locally that httpx uses compact separators and requests/aiohttp pass through stdlib json.dumps spaced defaults. Added non-exhaustiveness disclaimer so readers don't treat the list as an allowlist. - Add stable kebab-case `id` fields to every vector and rejection_vector. Switch CI compact-vs-spaced sanity check from description.startsWith() prose matching to `id === 'compact-js-style'` lookup. New structural test asserts id uniqueness + format. Descriptions can now be revised without silently breaking downstream tests. No signature values change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…ors from prose (#2493) Follow-up to #2478 addressing three review comments: - Promote MUST-compare-bytes to its own top-level bullet in security.mdx (was buried mid-paragraph in "Non-canonicalized aspects"). Frame "Canonical on-wire form" and "Verifier input" as failure-class narrowers on either side of the invariant. Inline the rationale (key order, unicode escapes, number formatting diverge across serializers) so readers don't have to scan forward. - Replace vague "httpx / most HTTP-client JSON bodies" with a concrete enumeration split by language-level serializer vs HTTP client. Verified locally that httpx uses compact separators and requests/aiohttp pass through stdlib json.dumps spaced defaults. Added non-exhaustiveness disclaimer so readers don't treat the list as an allowlist. - Add stable kebab-case `id` fields to every vector and rejection_vector. Switch CI compact-vs-spaced sanity check from description.startsWith() prose matching to `id === 'compact-js-style'` lookup. New structural test asserts id uniqueness + format. Descriptions can now be revised without silently breaking downstream tests. No signature values change. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
to adcontextprotocol/adcp-client-python
that referenced
this pull request
Apr 20, 2026
Upstream AdCP spec PR adcontextprotocol/adcp#2478 tightened the webhook signature verifier rule from "MAY re-serialize" to "SHOULD NOT re-serialize", with verifiers that cannot capture raw bytes required to MUST fail closed. Rationale: re-serializing a parsed payload to reconstruct the signed bytes silently fails against signers whose output differs in separator choice, key order, unicode escape policy, or number formatting — masking signer bugs the verifier should surface. Our ADCPClient._verify_webhook_signature had exactly that fallback path at client.py:3307: when raw_body was None, it called json.dumps(payload) with Python-default spaced separators. Historically this "worked" only in the coincidental case where the signer also used Python-default separators; against any compliant signer (httpx json= or JS JSON.stringify, both compact) it produced false-rejects. After PR #205's signer fix it produced false-rejects against our own signer too. Fix --- - Remove the json.dumps(payload) fallback entirely. When raw_body is None, log an actionable error and return False. - Update docstring to cite adcp#2478 and point at common framework hooks for capturing raw bytes (FastAPI Request.body, Flask request.get_data, aiohttp Request.read, Express express.raw). - Existing tests that relied on the fallback now pass raw_body= explicitly. Same signature math, just spec-conformant call shape. - New test_verify_fails_closed_when_raw_body_missing pins the fail-closed behavior by signing a real HMAC, omitting raw_body, and asserting the verifier raises ADCPWebhookSignatureError. Paired with a positive sibling that proves the rejection is specifically about missing raw_body, not signature or payload shape. Audit of the RFC 9421 content-digest path in src/adcp/signing/ — the parallel trap called out in adcp#2478 is absent here: sign_request takes body: bytes and client.py:488 passes request.content (httpx's actual wire bytes), so digest input and wire body are byte-identical by construction. BREAKING CHANGE: ADCPClient webhook verification now requires raw_body to be passed through from the HTTP handler. Callers that relied on the implicit re-serialize-from-payload fallback will start seeing ADCPWebhookSignatureError until they plumb the raw body through from their framework's pre-parse hook. Fix path: # FastAPI @app.post("/webhook") async def hook(request: Request, x_adcp_signature: str = Header(...)): raw = await request.body() payload = await request.json() # order matters: read raw first result = await client.handle_webhook( payload, ..., signature=x_adcp_signature, raw_body=raw, ) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
to adcontextprotocol/adcp-client-python
that referenced
this pull request
Apr 20, 2026
Pull in the merged test vectors from adcontextprotocol/adcp#2478 which pinned compact separators as the canonical on-wire form, added vectors covering whitespace-sensitive keys / nested objects / escaped unicode, and added the rejection vector that reproduces the Python-default spaced-separators signer bug. Changes ------- - tests/fixtures/webhook-hmac-sha256.json — regenerated from https://github.com/adcontextprotocol/adcp/blob/main/static/test-vectors/webhook-hmac-sha256.json (14 positive vectors including 2 new, 10 rejection vectors including the signer-serialization-mismatch case). - HMAC_REJECTION_VECTORS loaded alongside the existing positive vectors. - New test_rejection_vectors_do_not_collapse_to_positive mirrors the upstream CI check: for every rejection vector whose claimed signature has a well-formed sha256=<hex> shape, verify a correctly-computed HMAC over the claimed raw_body does NOT match the claimed signature — otherwise the rejection vector silently collapses into a positive case and stops catching what it claims to. - New test_verifier_rejects_upstream_rejection_vectors is the behavioral mirror: for every rejection vector with enough context to run end-to-end, assert ADCPClient._verify_webhook_signature returns False. Together the two parametrizations pin both the math (claimed sig is genuinely invalid) and the behavior (our verifier catches it). Vectors with non-computable rejection shapes (empty/null signature, wrong length, double-prefix) skip the computational check — they're documented for verifier implementers but don't have a signature a correctly-configured HMAC would produce. Full pytest: 1674 passed / 18 skipped (15 of the new skips are structural-rejection vectors appropriately declining the computational parametrization). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EmmaLouise2018
added a commit
that referenced
this pull request
Apr 20, 2026
…non-circular targeting rule, JCS test vectors - Add "Plan-hash computation" subsection in security.mdx pinning the normative scope: which bytes (sync_plans plans[i] body), which fields stripped (timestamps, governance/seller annotations, soft-deleted packages), RFC 8785 JCS canonicalization, and a byte-equality invariant modeled on the #2478/#2493 webhook HMAC treatment. - Require sellers to pin the plan revision at verification step 13 and execute against the pinned revision (or re-verify immediately before execution) to close the TOCTOU window where a concurrent sync_plans could land between hash-check and execution. - Replace the subjective "broadens the addressable audience" seller rule with a mechanical one: any change to bid_price, budget, daily_cap, flight dates, targeting, or targeting_overlay requires a fresh modification-phase token. The governance agent (not the seller) decides whether the change is within plan authority at check_governance time — removes the circularity where the seller had to answer "is this a broadening?" before the answering token existed. - Strengthen response-level plan_hash description: MUST NOT verify against it (not signature-protected and forgeable by an intermediary that mutates the response body before token verification runs). - Add a worked modification-phase example on update_media_buy showing the buyer's local-patch → JCS → check_governance flow. - Add PLAN_HASH_MISMATCH vs UPDATE_REQUIRES_GOVERNANCE vs PERMISSION_DENIED decision tree in security.mdx. - Add static/test-vectors/governance-plan-hash.json with four conformance vectors (intent multi-channel, modification-phase, minimal plan, unicode+number edge cases) pinning canonical JCS bytes and SHA-256 so independent implementations cannot diverge. - Note the checklist-renumbering anchor-link break (13-15 → 14-16) in the changeset.
bokelley
added a commit
to adcontextprotocol/adcp-client-python
that referenced
this pull request
Apr 20, 2026
Upstream AdCP spec PR adcontextprotocol/adcp#2478 tightened the webhook signature verifier rule from "MAY re-serialize" to "SHOULD NOT re-serialize", with verifiers that cannot capture raw bytes required to MUST fail closed. Rationale: re-serializing a parsed payload to reconstruct the signed bytes silently fails against signers whose output differs in separator choice, key order, unicode escape policy, or number formatting — masking signer bugs the verifier should surface. Our ADCPClient._verify_webhook_signature had exactly that fallback path at client.py:3307: when raw_body was None, it called json.dumps(payload) with Python-default spaced separators. Historically this "worked" only in the coincidental case where the signer also used Python-default separators; against any compliant signer (httpx json= or JS JSON.stringify, both compact) it produced false-rejects. After PR #205's signer fix it produced false-rejects against our own signer too. Fix --- - Remove the json.dumps(payload) fallback entirely. When raw_body is None, log an actionable error and return False. - Update docstring to cite adcp#2478 and point at common framework hooks for capturing raw bytes (FastAPI Request.body, Flask request.get_data, aiohttp Request.read, Express express.raw). - Existing tests that relied on the fallback now pass raw_body= explicitly. Same signature math, just spec-conformant call shape. - New test_verify_fails_closed_when_raw_body_missing pins the fail-closed behavior by signing a real HMAC, omitting raw_body, and asserting the verifier raises ADCPWebhookSignatureError. Paired with a positive sibling that proves the rejection is specifically about missing raw_body, not signature or payload shape. Audit of the RFC 9421 content-digest path in src/adcp/signing/ — the parallel trap called out in adcp#2478 is absent here: sign_request takes body: bytes and client.py:488 passes request.content (httpx's actual wire bytes), so digest input and wire body are byte-identical by construction. BREAKING CHANGE: ADCPClient webhook verification now requires raw_body to be passed through from the HTTP handler. Callers that relied on the implicit re-serialize-from-payload fallback will start seeing ADCPWebhookSignatureError until they plumb the raw body through from their framework's pre-parse hook. Fix path: # FastAPI @app.post("/webhook") async def hook(request: Request, x_adcp_signature: str = Header(...)): raw = await request.body() payload = await request.json() # order matters: read raw first result = await client.handle_webhook( payload, ..., signature=x_adcp_signature, raw_body=raw, ) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
to adcontextprotocol/adcp-client-python
that referenced
this pull request
Apr 20, 2026
Pull in the merged test vectors from adcontextprotocol/adcp#2478 which pinned compact separators as the canonical on-wire form, added vectors covering whitespace-sensitive keys / nested objects / escaped unicode, and added the rejection vector that reproduces the Python-default spaced-separators signer bug. Changes ------- - tests/fixtures/webhook-hmac-sha256.json — regenerated from https://github.com/adcontextprotocol/adcp/blob/main/static/test-vectors/webhook-hmac-sha256.json (14 positive vectors including 2 new, 10 rejection vectors including the signer-serialization-mismatch case). - HMAC_REJECTION_VECTORS loaded alongside the existing positive vectors. - New test_rejection_vectors_do_not_collapse_to_positive mirrors the upstream CI check: for every rejection vector whose claimed signature has a well-formed sha256=<hex> shape, verify a correctly-computed HMAC over the claimed raw_body does NOT match the claimed signature — otherwise the rejection vector silently collapses into a positive case and stops catching what it claims to. - New test_verifier_rejects_upstream_rejection_vectors is the behavioral mirror: for every rejection vector with enough context to run end-to-end, assert ADCPClient._verify_webhook_signature returns False. Together the two parametrizations pin both the math (claimed sig is genuinely invalid) and the behavior (our verifier catches it). Vectors with non-computable rejection shapes (empty/null signature, wrong length, double-prefix) skip the computational check — they're documented for verifier implementers but don't have a signature a correctly-configured HMAC would produce. Full pytest: 1674 passed / 18 skipped (15 of the new skips are structural-rejection vectors appropriately declining the computational parametrization). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
3 tasks
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…eject clause Closes #2483. Adds a positive signature-computation vector (`id=duplicate-keys`) with `raw_body={"status":"ok","status":"ok"}` and a correctly-computed HMAC. The vector description and new `verifier_outcomes: ["accept", "reject-malformed"]` field document that both verifier outcomes are spec-conformant under the #2478 clause — accept because the signature is mathematically valid over the raw bytes, or reject because parser behavior for duplicates is undefined per RFC 8259 §4. Makes the previously-untestable "verifiers MAY reject" clause probe-able so SDK conformance suites can catch drift (e.g., SDKs that crash on duplicate keys rather than either accepting or rejecting cleanly). No spec text change — security.mdx already permits both outcomes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 20, 2026
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…use (#2522) * spec(webhooks): add duplicate-key HMAC test vector for verifier MAY-reject clause Closes #2483. Adds a positive signature-computation vector (`id=duplicate-keys`) with `raw_body={"status":"ok","status":"ok"}` and a correctly-computed HMAC. The vector description and new `verifier_outcomes: ["accept", "reject-malformed"]` field document that both verifier outcomes are spec-conformant under the #2478 clause — accept because the signature is mathematically valid over the raw bytes, or reject because parser behavior for duplicates is undefined per RFC 8259 §4. Makes the previously-untestable "verifiers MAY reject" clause probe-able so SDK conformance suites can catch drift (e.g., SDKs that crash on duplicate keys rather than either accepting or rejecting cleanly). No spec text change — security.mdx already permits both outcomes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): swap to conflict-value duplicate-key vector + normative conformance guidance Addresses reviewer feedback on PR #2522: **Vector body** — swap `{"status":"ok","status":"ok"}` (identical values, hides the threat) for `{"event":"creative.status_changed",...,"status":"approved","status":"rejected"}` (conflicting values, actually probes the parser-differential attack class that motivates the clause). Vector id renamed to `duplicate-keys-conflicting-values`. **Conformance contract** — promote the duplicate-keys clause in security.mdx to its own bullet. Fix RFC 8259 §4 wording (RFC says "SHOULD be unique" / "unpredictable" behavior, not strictly "undefined"). Anchor on CVE-2017-12635 (CouchDB duplicate-key privilege escalation). Define both outcomes explicitly, call out that deterministic last-wins/first-wins downstream parsing counts as `accept`, and mark `reject-malformed` as SHOULD for state-change / spend-committing bodies. **Crash hierarchy** — invert the previous "crash is non-conformant" framing. Fail-closed crash is conformant-but-suboptimal (SHOULD return a structured malformed-body error instead). The actually-non-conformant outcome is silent accept with parser divergence between the signature verifier and the downstream consumer — the CVE-class failure. **Field shape** — rename `verifier_outcomes` → `acceptable_outcomes`, add `recommended_outcome: "reject-malformed"` so SDKs have a spec-blessed default. Add top-level `outcome_values` enum map and `non_conformant_outcomes` array so SDK conformance suites resolve tokens without scraping description prose. **Test harness** — structural assertions for the new fields so typos fail CI instead of shipping silently: `outcome_values` map required, every `acceptable_outcomes` must be a non-duplicate ≥2-element subset of the enum, every `recommended_outcome` must be in its vector's `acceptable_outcomes`. **Follow-up filed** — #2523 tracks tightening MAY → MUST for state-change payloads in a future release. Linked from the security.mdx clause so implementers who adopt `reject-malformed` now won't face a re-do later. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): tighten duplicate-object-key handling from MAY to MUST-reject Closes the parser-differential attack class (CVE-2017-12635 family) that the prior MAY clause permitted by spec. Both the legacy HMAC scheme and the RFC 9421 webhook profile now MUST-reject duplicate-key bodies after signature verification succeeds. **security.mdx (legacy HMAC):** - Duplicate-object-keys clause rewritten from MAY to MUST. Every body carried on the legacy HMAC scheme is a state-change notification (creative/media-buy/governance transitions), so the MUST applies unconditionally to this scheme. - Rejection is a structured malformed-body error class, distinct from signature-mismatch (the signature IS valid; the body is malformed). Verifiers that crash/fail-closed are conformant-but-suboptimal. **security.mdx (9421 webhook profile):** - New verifier-checklist step 11a (body well-formedness) between content-digest verification (11) and replay-cache check (12). Requires a parse mode that exposes duplicate keys. - New error code webhook_body_malformed in the webhook error taxonomy. - Verifier-checklist preamble: 15 checks, three substitutions from request-signing. **Test vector + harness:** - Shape shifted from dual-outcome to single-outcome: expected_verifier_action: reject-malformed + rfc9421_error_code: webhook_body_malformed. - verifier_action_values enum keeps accept as documentation-only with explicit non-conformant note. - non_conformant_outcomes expanded to three modes. - Test assertions updated for single-outcome shape. **Scope carve-out.** Webhook surfaces only. Request signing, TMP signed bodies, and adagents.json manifests remain open in #2523. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): fix DoS ordering, add strict-parse guidance, flag request-signing gap Addresses expert review of PR #2522's MUST-reject tightening: - **DoS fix**: body-well-formedness moved from 11a to step 14 (after replay-insert at 13) so nonce is burned on first sighting of any cryptographically-valid frame, preventing replay-CPU amplification on captured malformed-body frames. - **Per-language strict-parse enumeration** in both legacy HMAC clause and 9421 step 14: Python object_pairs_hook, Node secure-json-parse, Go json.Decoder / goccy / gjson (encoding/json has no strict mode), Jackson FAIL_ON_READING_DUP_TREE_KEY, Oj strict_mode. - **Signer-side clause**: signers SHOULD reject duplicate-key input from upstream callers before serialization, closing the pre-verification parse residual. - **Known-gap paragraph** at the request-signing verifier checklist: the parallel body-well-formedness check is deferred to #2523 (request bodies carry create_media_buy, update_media_buy_delivery — larger blast radius than webhooks), but explicitly flagged so SDK authors do not read the checklist as exhaustive. - **Prose hygiene**: preamble fixed (two substitutions plus one added step, not three substitutions); vector description reduced to one sentence (normative prose now in verifier_action_values and non_conformant_outcomes structured fields). - **CI coverage**: two new assertions — at least one vector must carry expected_verifier_action; duplicate-keys-conflicting-values fixture must exist with exact values security.mdx references. 34 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): round-3/4 expert feedback on duplicate-key handling Round-3 must-fixes (security): signer-side SHOULD to MUST in legacy HMAC bullet (rationale: unverifiable on wire, COSE/JOSE precedent). Strict-parse library corrections at step 14 — tidwall/gjson removed (query library), goccy/go-json requires explicit DisallowDuplicateKey(), secure-json-parse defaults target prototype-pollution keys not data-key duplicates. Logging discipline at step 14b: SHOULD NOT log full body bytes on webhook_body_malformed, log keyid+nonce+byte length+duplicate key names only (SIEM poisoning defense). Round-3 protocol/code fixes: preamble reframed (two parameter substitutions plus one additional check unique to the webhook profile; gate step 14 behind a profile flag, not fork the implementation). Per-language enum deduplicated — legacy HMAC bullet points to step 14 canonical list. Round-3 request-signing taxonomy pin: known-gap paragraph now mandates exact shape ahead of #2523 — step 14 placement after step 13 insert, error code request_body_malformed (mirrors webhook_body_malformed, distinct from request_signature_digest_mismatch), same strict-parse and logging rules. Vendor-custom codes / alternate placements MUST NOT ship in interim. Round-3 scope expansion (absorbed security follow-up): new-keyid admission pressure bullet in webhook replay dedup sizing. MUST track rate of entries admitted from previously-unseen keyids; SHOULD alert above operator-defined threshold. Distinguishes distributed-compromise attack shape (N compromised keys collectively saturating aggregate cache) from legitimate traffic. Round-4 code-reviewer should-fixes: step 14 split into 14a (strict-parse requirement) and 14b (logging discipline) following the 9a sub-step convention. Load-bearing cap invariant moved out of step 13 into its own paragraph after step 14b, mirroring the request-signing structure. Step 13 now contains only the insert + step-14 ordering rationale. Closes #2483. Addresses webhook portion of #2523. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pin the canonical on-wire JSON form for AdCP webhook signatures. The legacy HMAC-SHA256 scheme signs
{ts}.{raw_http_body_bytes}but did not pin the JSON serialization form the signer produces — Python signers using defaultjson.dumps(payload)(spaced separators) sent compact bytes on the wire (httpx default) and every compliant verifier returned 401. Closes #2464. Motivating bug: adcontextprotocol/adcp-client-python#205.Option (1) from the issue — compact separators (
","/":"), byte-equality with the wire body. Not JCS. Key ordering, unicode-escape policy, and number representation are NOT canonicalized; signers and verifiers compare bytes.Expanded scope after review to include:
content-digestpath — same trap class (signer digests a serialization that differs from the on-wire bytes), fails loud under 9421 (*_signature_digest_mismatch) but worth calling out symmetrically.rejection_vectors(previously uncovered); tightened compact-vs-spaced matcher tostartsWith(...).All three reviewers flagged no Must fix; two (security, protocol) independently recommended tightening the verifier-fallback carve-out. Full review feedback addressed.
Files
docs/building/implementation/security.mdx— 3 new normative bullets on legacy HMAC (canonical form, non-canonicalized aspects, verifier input). New signer-side bullet in the 9421 content-digest warning.docs/building/implementation/webhooks.mdx— signing-algorithm prose.static/test-vectors/webhook-hmac-sha256.json— +2 positive, +1 rejection vector.tests/webhook-hmac-vectors.test.cjs— rejection_vectors now exercised; 26/26 subtests pass..changeset/webhook-hmac-json-on-wire-form.md.Test plan
npm run test:hmac-vectors— 26/26 pass (15 positive + 10 rejection + 1 compact/spaced sanity)npm run test:unit— 587/587 pass via precommit hooknpm run typecheck— cleannpm run test:docs-nav— clean🤖 Generated with Claude Code