test(training-agent): omit bearer in create_media_buy 401 assertion (#3080)#3096
Merged
Merged
Conversation
The /mcp-strict 401 test sent a valid bearer token by default. The SDK's requireAuthenticatedOrSigned short-circuits on a successful bearer result before the required_for gate runs (bearer bypass, per SDK #2586), so the test got 200 instead of 401. Fix: pass { auth: false } to simulate the grader scenario — compliance graders carry no bearer token and authenticate via RFC 9421 signatures only. With no bearer and no signature on a required_for op, the gate fires and returns 401 request_signature_required. Closes #3080 https://claude.ai/code/session_01S7aDiMrwhfYPr7FzYQdn9C
…scheme
The test was asserting /^Bearer / on the WWW-Authenticate header for the
request_signature_required path. Per index.ts:207 and signed-requests.yaml:52,
respondUnauthorized({ signatureError }) emits:
WWW-Authenticate: Signature error="<code>"
not a Bearer challenge. Bearer challenges come from the no-credentials /
invalid-token paths. Pinning to the Signature scheme catches regressions
where the SDK emits the wrong challenge type — the compliance grader reads
the error code off this header.
https://claude.ai/code/session_01S7aDiMrwhfYPr7FzYQdn9C
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.
Closes #3080
Summary
Two-part fix for the
/mcp-strict401 test that was failing even after PR #3082 merged.Root cause:
callToolsendsAuthorization: Bearer test-token-for-strictby default. The SDK'srequireAuthenticatedOrSignedshort-circuits on a successful bearer result before therequired_forgate runs (auth-signature.js:146-147:if (fallbackResult !== null) return fallbackResult). This is intentional SDK behavior per #2586 — a valid credential is sufficient even when signing is required for unauthenticated callers. PR #3082'sresolveOperation rawBodyfix only helps when bearer returns null; with a valid bearer present,resolveOperationis never reached in therequired_forcode path.Fix 1 — omit bearer: Pass
{ auth: false }on the failing test call to suppress the bearer token. This models the actual grader scenario: compliance graders don't carry bearer tokens — they authenticate via RFC 9421 signature credentials only. With no bearer and no signature on arequired_forop, the gate fires and returns401 request_signature_required.Fix 2 — correct
www-authenticateassertion: The test was asserting/^Bearer /on theWWW-Authenticateheader. Perindex.ts:207andstatic/compliance/source/universal/signed-requests.yaml:52/188,respondUnauthorized({ signatureError })emitsWWW-Authenticate: Signature error="<code>", not a Bearer challenge. Fixed to/Signature error="request_signature_required"/, pinning exactly what the compliance grader reads.Non-breaking justification: Server integration tests only. No schema, task definition, wire protocol, or public API surface changed.
Aside — CI gap: Vitest integration tests don't run in CI for this repo, which is why #3082, #3079, and the original regression all reported clean CI with a failing test. Wiring vitest into the CI pipeline needs a separate human-authored PR touching
.github/workflows/.Pre-PR review (two iterations)
Iteration 1:
{ auth: false }is type-safe percallTool's signature; no test overlap with existingPOST /mcp-strict without authassertion (different tool, different rejection reason).www-authenticateassertion/^Bearer /is wrong for thesignatureErrorcode path; grader readsSignature error="..."scheme, not Bearer.Iteration 2 (after fixing blocker):
respondUnauthorized({ signatureError }); regex correctly non-anchored to tolerate additional challenge parameters.Session: https://claude.ai/code/session_01S7aDiMrwhfYPr7FzYQdn9C