test(webhooks): duplicate-key HMAC vector for verifier MAY-reject clause by bokelley · Pull Request #2522 · adcontextprotocol/adcp

bokelley · 2026-04-20T06:00:40Z

Summary

Closes #2483. Addresses the webhook portion of #2523 (legacy HMAC + 9421 webhook profile); request signing, TMP signed bodies, adagents.json manifests, and governance-signing remain open in #2523 and are now explicitly flagged in the request-signing checklist as a known gap with a pinned interim contract so implementations do not diverge before #2523 lands.

Normative changes (`security.mdx`)

Duplicate-object-keys on signed webhook bodies: MAY → MUST-reject on both sides.

Legacy HMAC scheme: verifier MUST-reject; signer MUST reject duplicate-key input before serialization (unverifiable on wire; enforced via out-of-band audit / interop testing, routine COSE/JOSE pattern).
9421 webhook verifier checklist: new step 14 (body well-formedness), ordered AFTER the replay-cache insert at step 13 so the nonce is burned on first sighting of any cryptographically-valid frame — captured (keyid, nonce, valid-signature, malformed-body) tuples cannot be replayed to burn crypto-verify CPU. Split into:
- 14a. Strict-parse requirement — library-accurate per-language escape hatches. tidwall/gjson explicitly excluded (query library, not validator). goccy/go-json requires explicit DisallowDuplicateKey() (not default). secure-json-parse defaults target prototype-pollution keys, not data-key duplicates.
- 14b. Logging discipline — SHOULD NOT log full body bytes on webhook_body_malformed; log keyid, nonce, byte length, duplicate key names only. Prevents SIEM poisoning / credential-exfiltration follow-on via attacker-chosen log bytes.
Load-bearing cap invariant — moved to its own paragraph after step 14b, mirroring the request-signing structure: external traffic without the signer's private key cannot grow this cache; per-keyid cap (9a) and new-keyid admission-pressure alarm catch the compromised-key cases.
Preamble reframed: "two parameter substitutions and one additional check unique to the webhook profile." Implementations SHOULD gate step 14 behind a profile flag, not fork the verifier code.
New error code webhook_body_malformed in the webhook error taxonomy, distinct from webhook_signature_digest_mismatch (signature IS valid; body is malformed).

New-keyid admission pressure in webhook replay dedup sizing. Verifiers MUST track rate of entries admitted from previously-unseen keyids per unit time; SHOULD alert above operator-defined threshold (example: 3× 24-hour moving average, floored at 5 distinct new keyids / 5 minutes). Distinguishes distributed-compromise attack shape (N compromised keys collectively saturating aggregate cache) from legitimate traffic; alarms BEFORE the aggregate cap triggers and rejects everyone.

Request-signing known-gap paragraph — pinned, not just recommended. Implementers adopting the rule ahead of #2523 MUST mirror the webhook profile exactly: step 14 placement after step 13's insert, error code request_body_malformed (mirrors webhook_body_malformed under the request_* prefix, distinct from request_signature_digest_mismatch), same strict-parse and logging-discipline rules. Vendor-custom codes / alternate placements / full-body logging MUST NOT ship in the interim. Closes the compat-break window between interim implementations and #2523's formal spec edit.

Test vector + harness

Vector duplicate-keys-conflicting-values with conflicting values (status=approved / status=rejected). HMAC mathematically valid over the raw bytes.
Single-outcome shape: expected_verifier_action: "reject-malformed" + rfc9421_error_code: "webhook_body_malformed". Description reduced to one sentence; all normative detail lives in verifier_action_values and non_conformant_outcomes structured fields.
Test harness: 7 structural assertions including id + exact-values checks so a silent rename between vector and spec text fails CI.
34 vector tests, 587 unit tests, typecheck, Mintlify validation — all pass.

Scope and follow-ups

This PR addresses the webhook surface of #2523 completely. Three follow-up issues filed for residuals security reviewer surfaced:

security(webhooks): anchor admission-pressure baseline against a longer reference window #2545 — anchor admission-pressure baseline against a longer reference window (prevents slow-burn attackers dragging baseline up during ramp-up)
test-vectors(webhooks): signer-side conformance fixture for duplicate-key input rejection #2546 — signer-side conformance fixture paralleling the verifier fixture
security(webhooks): sanitize key-name logging on webhook_body_malformed to prevent attacker-controlled log input #2547 — sanitize key-name logging on webhook_body_malformed (truncate to 64 bytes, strip non-printables, cap count at 8)

Request signing, TMP signed bodies, adagents.json manifests, governance-signing remain open in #2523. The request-signing section now carries a pinned-shape known-gap paragraph so any interim implementations will drop in cleanly once #2523 lands.

Test plan

node --test tests/webhook-hmac-vectors.test.cjs — 34 tests pass
npm run test:unit && npm run typecheck — 587 tests, typecheck clean
Mintlify docs validation — pass (git push pre-push hook verified anchor links resolve)
Reviewer confirms step 14 decomposition (14a/14b following the 9a sub-step convention) reads cleanly
Reviewer confirms request-signing gap paragraph is the right interim contract pending security: tighten duplicate-object-keys from MAY to MUST-reject for state-change payloads #2523
Reviewer confirms new-keyid admission-pressure example threshold (3× moving avg, floored at 5) is tight enough (follow-up in security(webhooks): anchor admission-pressure baseline against a longer reference window #2545 will anchor against a longer window)

🤖 Generated with Claude Code

…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>

…ve 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>

github-actions · 2026-04-20T06:14:22Z

Schema Link Check Results

Commit: e13d693 - spec(webhooks): round-3/4 expert feedback on duplicate-key handling

⚠️ Warnings (schema not yet released)

These schemas exist in source but haven't been released yet. The links will be broken until the next version is published:

https://adcontextprotocol.org/schemas/v3/enums/specialism.json
- Schema exists in latest (source) but not yet released in v3
- Action: This link will work after next 3.x release is published

To fix: Either:

Wait for the next release and merge this PR after the release is published
Use latest instead of a version alias if you need the link to work immediately (note: latest is the development version and may change)
Coordinate with maintainers to cut a new release before merging

…T-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>

…uest-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>

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>

… fixtures, log sanitization Lands the three residuals security review flagged on PR #2522 before close. Admission-pressure baseline anchor (#2545): triple-threshold shape in webhook replay dedup sizing. 3x 24hr moving avg OR 2x 30-day P95 OR fixed 50-new-keyids-per-5-min ceiling, whichever triggers first. Prevents patient attackers from dragging baseline up during multi-week ramp-up — P95 over 30 days is dominated by baseline-traffic tail, not attack ramp. Signer-side conformance fixtures (#2546): new signer_input_rejection_vectors array with two vectors — top-level and deep-nested duplicate-key rejection. New signer_action_values enum defines reject-input-before-sign token. Test harness adds five structural assertions including byte-level check that fixture actually contains duplicate keys. security.mdx clause now references both fixtures and mandates interop harnesses exercise both. Key-name logging sanitization (#2547): step 14b mandates truncate-to-64-bytes, replace-non-printables-with-placeholder, cap-count-at-8 before logging duplicate key names. Closes attacker-controlled-byte log-injection channel without losing diagnostic value. Closes #2545, #2546, #2547. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… fixtures, log sanitization (#2548) * spec(webhooks): duplicate-key follow-ups — admission baseline, signer fixtures, log sanitization Lands the three residuals security review flagged on PR #2522 before close. Admission-pressure baseline anchor (#2545): triple-threshold shape in webhook replay dedup sizing. 3x 24hr moving avg OR 2x 30-day P95 OR fixed 50-new-keyids-per-5-min ceiling, whichever triggers first. Prevents patient attackers from dragging baseline up during multi-week ramp-up — P95 over 30 days is dominated by baseline-traffic tail, not attack ramp. Signer-side conformance fixtures (#2546): new signer_input_rejection_vectors array with two vectors — top-level and deep-nested duplicate-key rejection. New signer_action_values enum defines reject-input-before-sign token. Test harness adds five structural assertions including byte-level check that fixture actually contains duplicate keys. security.mdx clause now references both fixtures and mandates interop harnesses exercise both. Key-name logging sanitization (#2547): step 14b mandates truncate-to-64-bytes, replace-non-printables-with-placeholder, cap-count-at-8 before logging duplicate key names. Closes attacker-controlled-byte log-injection channel without losing diagnostic value. Closes #2545, #2546, #2547. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): round-2 expert feedback on duplicate-key follow-ups Admission-pressure rule tightened to quadruple-threshold: 3x 24hr MA OR 2x 30-day P95 OR 1.5x 90-day P99 OR max(20, 10% of 30-day unique-keyid count) per 5-min, whichever triggers first. Alarm payload names triggering clause. 90-day P99 resists multi-month ramps; proportional ceiling auto-scales with operator size (1,000 for 10k-keyid verifier, 20 for 20-keyid verifier). Spec makes explicit these are operator-tunable defaults — published normative values would be an attacker oracle. Step 14b logging discipline tightened: truncate at first non-printable and log <sanitized:N> (was fixed <non-printable> which leaked position), truncate to 32 bytes at last complete UTF-8 codepoint boundary (was 64), cap count at 4 (was 8). Signer-side clause normatively requires same sanitization on signer-surfaced key names. Test vectors restructured from flat signer_input_rejection_vectors + signer_action_values into top-level signer_side object with action_values, rejection_vectors, positive_vectors. Rejection vectors expanded from 2 to 4 shape-classes: top-level, plain-nested, array-contained (real-world AdCP payload shape — packages[], creative_assets[]), three-deep. New positive_vectors array with signer-upstream-clean-input so reject-everything signers cannot trivially pass conformance. Test harness: 9 new structural assertions + scope-aware duplicate-key detector (walks JSON tracking object vs array nesting, correctly distinguishes duplicate keys at same object scope from same key name in distinct array-contained objects). Previously-flagged regex false-positive comment added at tests/webhook-hmac-vectors.test.cjs for the legacy heuristic check used as a secondary probe. Two further follow-ups filed separately: (a) CI reference-signer harness to actually enforce the 'interop harnesses MUST exercise both' spec language; (b) threshold publication debate — concrete numbers in spec vs operator guide. Closes #2545, #2546, #2547. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): round-5 follow-ups — Unicode sanitization fix + operator tuning guide + conformance harness Round-5 expert review landed three types of fixes in one PR: SECURITY FIX: step 14b sanitization was ASCII-only (< 0x20 || === 0x7F); bidi overrides (U+202E), line separators (U+2028/U+2029), zero-width chars (U+200B-200D), C1 controls (U+0080-009F), and BOM (U+FEFF) all passed through, reopening the log-injection channel the rule exists to close. Fixed: security.mdx step 14b now normatively enumerates the minimum non-printable set with rationale; implementations MAY extend but MUST NOT narrow; ASCII-only is called out as non-conformant. Reference signer implements via isNonPrintableCodepoint with the full range; conformance tests cover every class plus UTF-8 codepoint-boundary truncation for multi-byte sequences. STRUCTURE: reference signer extracted to tests/helpers/reference-webhook-signer.cjs with explicit CONTRACT BOUNDARY comment — fixtures ARE the contract, reference signer is one implementation. Test file split: webhook-hmac-vectors.test.cjs keeps structural/signature checks, webhook-hmac-signer-conformance.test.cjs runs signer harness. hasDuplicateKeyInAnyObjectScope delegates to findDuplicateKeyNames (single-sourced parser). 75 tests (52 structural + 23 signer-conformance). NON-NORMATIVE TUNING GUIDE: new docs/building/implementation/webhook-verifier-tuning.mdx replacing inline concrete threshold numbers in security.mdx. Contains starting-values table, first-30-days oracle warning (operators MUST tune within 30 days; implementations SHOULD randomize defaults ±30% on first deployment), baselining methodology, 10 attack scenarios (added: key-rotation storm, thin-history window, intermittent low-volume rule-shape limitation, onboarding-window-timed attack), tuning-adjustments table, 'DO NOT publish' three-audience split (public prohibited / NDA-attested permitted / internal runbooks required) replacing the prior too-absolute rule. security.mdx normative text drops concrete numbers and 'order' hints — only category names survive. Scenario 4 wording fixed (the previous version said 'What trips: clause (d)' then concluded 15 did NOT trip (d) — contradictory). Positive-vector tests assert result.error is absent — a signer emitting both signed_frame AND error is ambiguous response shape. Mintlify Note/Warning callouts at the tuning guide top replace bold-inline non-normative framing. Closes #2545, #2546, #2547, #2549, #2551. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(webhooks): round-5 polish — normative error code, 32b jitter, tuning-overdue hook, scenario adds Round-5 final expert review flagged 2 should-fix and 5 consider items. All landed here to avoid a return trip. Security should-fix: widen jitter from ±30% (1.86x spread) to log-uniform [0.5x, 2x] (4x spread) — narrower fleet distribution lets a disciplined attacker tune to 0.7x-published and stay under every jittered deployment. Scenario 9 corrected: previously said per-keyid + aggregate caps cover slow-drip; they don't — 1,440 new keyids/month is 0.014% of a 10M cap. Tuning guide now states operators with slow-drip in scope MUST layer application-level detection, not MAY. Protocol/security consider: normative error identifier 'duplicate_key_input' — cross-SDK stable for multi-SDK error dispatch; internal error-object shape stays implementation-defined. 'threshold_tuning_overdue' logging SHOULD on implementations gives the 30-day operator-tuning obligation a testable hook rather than operator-diligence-only. Protocol consider: added Scenario 11 (baseline reset at mature verifier — failover, cache rebuild, config change). Mirrors Scenario 8's thin-history posture at a deployment that was supposed to be mature; mitigation is baseline-state persistence across failover (Redis / shared dedup service — same infrastructure choice the spec already requires for cross-endpoint replay-cache scoping). Code consider: step 14b split into three bulleted sub-rules for scannability. Tuning guide Related section tightened with direct anchor to #webhook-replay-dedup-sizing plus scroll-target prose. Conformance harness asserts the normative error code. 75 tests still pass. Closes #2545, #2546, #2547, #2549, #2551. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

bokelley mentioned this pull request Apr 20, 2026

security: tighten duplicate-object-keys from MAY to MUST-reject for state-change payloads #2523

Closed

bokelley and others added 3 commits April 20, 2026 02:31

bokelley merged commit 940b9cd into main Apr 20, 2026
16 checks passed

This was referenced Apr 20, 2026

spec(webhooks): duplicate-key follow-ups — admission baseline, signer fixtures, log sanitization #2548

Merged

test: signer-conformance CI harness for webhook duplicate-key fixtures #2549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(webhooks): duplicate-key HMAC vector for verifier MAY-reject clause#2522

test(webhooks): duplicate-key HMAC vector for verifier MAY-reject clause#2522
bokelley merged 5 commits intomainfrom
bokelley/issue-2483

bokelley commented Apr 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bokelley commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Normative changes (security.mdx)

Test vector + harness

Scope and follow-ups

Test plan

Uh oh!

github-actions Bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Schema Link Check Results

⚠️ Warnings (schema not yet released)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bokelley commented Apr 20, 2026 •

edited

Loading

Normative changes (`security.mdx`)

github-actions Bot commented Apr 20, 2026 •

edited

Loading