Skip to content

spec(request-signing): protocol_methods_* namespace; widen test-agent strict route#4326

Merged
bokelley merged 3 commits into
mainfrom
bokelley/test-agent-signing-opt-in
May 10, 2026
Merged

spec(request-signing): protocol_methods_* namespace; widen test-agent strict route#4326
bokelley merged 3 commits into
mainfrom
bokelley/test-agent-signing-opt-in

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

  • request_signing: declare verifier coverage of A2A protocol methods (tasks/cancel, tasks/get) #4318 — adds request_signing.protocol_methods_supported_for / _warn_for / _required_for so sellers can declare verifier coverage of JSON-RPC methods (tasks/cancel, tasks/get) separately from AdCP tool names. Schema enforces the namespace split via pattern: "/"; subset_of / disjoint_with mirror the AdCP-namespace rules; identity.brand_json_url is required_when any new field is non-empty. Normative text added to docs/building/by-layer/L1/security.mdx.
  • test-agent: per-session opt-in to enforce request_signing.required_for (X-Test-Require-Signing header) #4314 — widens the test-agent /<tenant>/mcp-strict route to enforce the new bucket. STRICT_REQUIRED_FOR adds update_media_buy + sync_creatives; new STRICT_PROTOCOL_METHODS_REQUIRED_FOR = ['tasks/cancel'] feeds a namespace-aware mcpOperationResolver (returns params.name for tools/call, the method field for tasks/*). Wire shape from get_adcp_capabilities splits the bundle into the two namespaces; SDK-internal VerifierCapability stays flat (match is by-string-equality).

The earlier #4314 X-Test-Require-Signing header proposal is not adopted — declaration-enforcement coherence (security.mdx:927) and the SDK's singleton/eager-build authenticator architecture were both flagged in triage. Strict-route enforcement is the spec-coherent path that satisfies the original tasks/cancel-on-abort regression-test ask in adcp-client#1617 Phase 2.

minor changeset per the playbook (signing profile changes are never patch-eligible).

Test plan

  • npm run test:schemas (525 schemas validated)
  • npm run test:composed — new request_signing.protocol_methods_* accept/reject cases pass
  • npm run test:json-schema (256 docs JSON blocks validated)
  • npx vitest run server/tests/unit/training-agent.test.ts (365/365)
    • new: default route emits empty required_for and no protocol_methods_* buckets
    • new: strict route emits AdCP names in required_for and tasks/cancel in protocol_methods_required_for
    • new: mcpOperationResolver handles tools/call, tasks/cancel, tasks/get, malformed bodies, missing rawBody
  • npx tsc --noEmit -p server/tsconfig.json (clean)
  • npm run test:storyboard-doc-parity (passes)
  • (follow-up) signed-requests storyboard adds a tasks/cancel step asserting verifier coverage matches declared protocol_methods_required_for

🤖 Generated with Claude Code

…gent strict route

Closes #4318: declares verifier coverage of JSON-RPC protocol methods
(`tasks/cancel`, `tasks/get`) in a separate namespace from AdCP tool names.
Schema enforces split via `pattern: "/"`; subset_of/disjoint_with mirror
the AdCP-namespace rules. `identity.brand_json_url` is required_when any
new field is non-empty. Normative text in security.mdx specifies that
matches bind to the JSON-RPC `method` field and the same RFC 9421 covered
components apply to JSON-RPC method calls.

Closes #4314: test-agent `/<tenant>/mcp-strict` route enforces the new
bucket. STRICT_REQUIRED_FOR widens to update_media_buy and sync_creatives
so a buyer that signs the initial create but forgets follow-on mutations
gets a 401 instead of a silent green light. STRICT_PROTOCOL_METHODS_REQUIRED_FOR
adds tasks/cancel; new namespace-aware mcpOperationResolver returns the
JSON-RPC method name for protocol methods (tasks/cancel, tasks/get) and the
tool name for tools/call. Wire response from get_adcp_capabilities splits
the bundle into the two namespaces; SDK-internal VerifierCapability stays
flat (match is by-string-equality).

The earlier #4314 X-Test-Require-Signing header proposal is not adopted
(declaration-enforcement coherence, SDK singleton/eager-build constraints
flagged in triage). Strict-route enforcement is the spec-coherent path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…space

Follow-up to expert review on PR #4326:

- Tighten schema `pattern` from `"/"` to `"^[a-z][a-z0-9_]*/[a-z][a-z0-9_]*$"`
  so `protocol_methods_*` items reject malformed strings like `/`, `tasks/`,
  or `tasks/cancel/extra`. Matches A2A 0.3.0 §7 single-slash family/verb
  conventions.
- Add normative MUST in security.mdx: verifiers MUST NOT cross-namespace
  match (a `tools/call` body whose `params.name` matches a
  `protocol_methods_required_for` entry MUST NOT satisfy the match, and
  vice-versa). The two buckets are matched against disjoint envelope fields.
- Add normative SHOULD in security.mdx: sellers populating
  `protocol_methods_*` on a transport that also serves `tools/call` SHOULD
  set `covers_content_digest: 'required'`. Without it, an on-path attacker
  who captures a signed `tools/call` can swap the body to `tasks/cancel`
  within the signature window. Sellers that cannot adopt 'required' MUST
  partition by `@target-uri`.
- Harden `mcpOperationResolver` to enforce the cross-namespace MUST: a
  `tools/call` body with `params.name` containing `/` resolves to undefined
  (refuses the smuggled namespace), and a non-`tools/call` method without
  `/` resolves to undefined (defense-in-depth on the protocol-method side).
- Add tests for cross-namespace refusal + `tools/call` with missing params.
- Comment at the wire-shape splitter cross-references the schema constraint.

Filed #4327 for the storyboard-vector gap (tasks/cancel negative vector).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley
Copy link
Copy Markdown
Contributor Author

Issue #4327 proposes adding 021-unsigned-protocol-method-required-for.json — a negative test vector grading the protocol_methods_required_for conformance gap (unsigned tasks/cancel when the field is non-empty). Same surface as this PR; your unchecked test-plan item matches it exactly. Consider folding before merge or confirm as a follow-up.


Generated by Claude Code

@bokelley
Copy link
Copy Markdown
Contributor Author

Expert review summary

Three reviews ran — ad-tech-protocol-expert, code-reviewer, security-reviewer. All three returned approve with overlapping findings; the actionable ones are addressed in 0431d7a4 (force-push not used — additive commit on top of ab991a84).

Findings addressed in 0431d7a

Finding Source Fix
Cross-namespace replay risk on shared @target-uri with covers_content_digest: 'either' (medium) security-reviewer + ad-tech-protocol-expert Normative SHOULD added to security.mdx — sellers populating protocol_methods_* on a transport that also serves tools/call SHOULD set covers_content_digest: 'required'; otherwise MUST partition by @target-uri.
pattern: "/" too loose, accepts "/", "//", "a/b/c" (nit, all three) all three Tightened to ^[a-z][a-z0-9_]*/[a-z][a-z0-9_]*$ matching A2A 0.3.0 §7 single-slash family/verb.
Converse normative missing — verifiers MUST NOT cross-namespace match (nit) ad-tech-protocol-expert Normative MUST added to security.mdx; resolver hardened to refuse tools/call with slashed params.name and non-tools/call methods without slash. Tests cover both refusal cases.
tools/call with missing params not test-covered (nit) code-reviewer Test added.
isProtocolMethod heuristic at task-handlers.ts:2960 (nit) code-reviewer + security-reviewer Comment cross-references the schema constraint as the structural inverse.

Findings deferred

Finding Disposition
Storyboard runner doesn't grade the new bucket (no tasks/cancel negative vector) Filed as #4327. Conformance suite update is a separate PR by design — this PR ships the spec + test-agent enforcement; the runner-side vector is tracked under the #2331 umbrella.
Strict-route default covers_content_digest: 'either' instead of 'required' (medium config default) Out of scope. The strict route's 'either' mode is required to fire neg/006 (missing-content-digest with covers_content_digest: 'either' accepts both forms). The repo already has /mcp-strict-required and /mcp-strict-forbidden siblings for vectors that need stricter modes. The new SHOULD in security.mdx now nudges real sellers toward 'required' when populating protocol_methods_*.
Future-proofing: includes('/') heuristic could break if AdCP ever adopts a tool name with / Schema constraint forbids it; comment added. Defense-in-depth deferred.

Verdict

  • ad-tech-protocol-expert: approve with caveats — all caveats addressed.
  • code-reviewer: ready to push — no blockers, all nits addressed or deferred.
  • security-reviewer: approve with notes — replay-text + cross-namespace MUST landed.

Tests passing (368/368 vitest, 35/35 composed schema, 525 schemas valid, tsc clean). Will admin-merge once CI is green.

@bokelley
Copy link
Copy Markdown
Contributor Author

Confirmed follow-up. The test plan already marks this (follow-up) — the scope boundary was deliberate. The PR is non-draft and awaiting review; folding scope now re-opens the review cycle. Issue #4327 is the right tracking surface: filed, labeled compliance-suite, and linked back here in its Tracking section. No fold needed before merge.


Generated by Claude Code

@bokelley
Copy link
Copy Markdown
Contributor Author

Fold recommendation withdrawn — #4327 is the right tracking surface for the tasks/cancel storyboard step. PR scope stands as documented.


Generated by Claude Code

@bokelley bokelley merged commit f7f6600 into main May 10, 2026
24 checks passed
@bokelley bokelley deleted the bokelley/test-agent-signing-opt-in branch May 10, 2026 12:05
bokelley added a commit that referenced this pull request May 10, 2026
… protocol_methods_required_for

Closes #4327. Adds negative vector that grades the `protocol_methods_required_for`
namespace introduced in #4326: a verifier declaring `protocol_methods_required_for:
["tasks/cancel"]` but not enforcing it would silently accept an unsigned
`tasks/cancel` POST. The vector POSTs an unsigned JSON-RPC body and asserts
`request_signature_required` byte-for-byte.

Capability-gated: runner skips when the agent doesn't declare the bucket;
FAILs (not SKIPs) when it declares but doesn't enforce. Same shape as
existing capability-gated negatives.

No schema changes; no normative spec changes. Patch-eligible per the
playbook (additive conformance scenarios).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 10, 2026
… protocol_methods_required_for (#4335)

Closes #4327. Adds negative vector that grades the `protocol_methods_required_for`
namespace introduced in #4326: a verifier declaring `protocol_methods_required_for:
["tasks/cancel"]` but not enforcing it would silently accept an unsigned
`tasks/cancel` POST. The vector POSTs an unsigned JSON-RPC body and asserts
`request_signature_required` byte-for-byte.

Capability-gated: runner skips when the agent doesn't declare the bucket;
FAILs (not SKIPs) when it declares but doesn't enforce. Same shape as
existing capability-gated negatives.

No schema changes; no normative spec changes. Patch-eligible per the
playbook (additive conformance scenarios).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant