fix(csrf): bypass on training-agent hostname so per-tenant routes reach verifier (closes #2368)#4503
Merged
Conversation
…ch verifier Per-tenant strict routes (`/<tenant>/mcp-strict`, mounted at root via host-based dispatch on `test-agent.adcontextprotocol.org`) were 403'd by global CSRF before reaching the request-signing verifier. The path-based exemption lists only matched the legacy single-URL shape (`/mcp-strict`) and the AAO mount prefix (`/api/training-agent/`). The training-agent host serves exclusively server-to-server bearer/signed traffic, so CSRF has nothing to protect there. Hostname-based bypass mirrors the existing `EXEMPT_EXACT` entries that were added for the same host's legacy URL shape. Closes adcp#2368. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… docstring describes The script's docstring describes a dual-branch release model where, on main during a forward-merge window, `adcp_version` (legacy alias tracking the latest released 3.0.x) may legitimately be ahead of `published_version` (canonical, tracks this branch's package.json). Rule 2 enforces that direction correctly. But Rule 1 required strict equality between the two registry fields — false-positiving the exact scenario the docstring permits. Replace strict equality with the same at-or-ahead-of semantic Rule 2 uses: `adcp_version` must be `>= published_version`. Catches the real bug (canonical bumps without keeping the alias in sync, leaving alias readers stale) without blocking the forward-merge window. Concrete trigger that exposed this: package.json `3.0.3`, `published_version: 3.0.3`, `adcp_version: 3.0.12` (from a prior forward-merge). Rule 2 was fine; Rule 1 fired and blocked unrelated pre-push pushes on main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Security review caught that the prior hostname-based bypass used `req.hostname`, which Express derives from `X-Forwarded-Host` under `trust proxy = 1`. Fly's edge forwards that header as-received, so an attacker could spoof `X-Forwarded-Host: test-agent.adcontextprotocol.org` on a request to `app.adcontextprotocol.org/api/me/<cookie-authed-route>` and bypass CSRF — the cookie-authed handler would still match by path and run. Switch to a path-shape regex that matches the per-tenant MCP route shape (`/<tenant>/mcp[-strict[-required|-forbidden]]`) without trusting any client-supplied header. Path shape isn't client-spoofable; adds nine bypass tests and seven near-miss tests to guard the regex. 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
1 participant
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
test-agent.adcontextprotocol.org. Per-tenant strict routes (/<tenant>/mcp-strict, mounted at root via host-based dispatch) were 403'd by global CSRF before reaching the RFC 9421 verifier — the path-based exemption lists only matched the legacy single-URL shape (/mcp-strict) and the AAO mount prefix (/api/training-agent/), not the per-tenant shape introduced in feat(training-agent): close #2347 (SDK dispatcher) + #2368 (/mcp-strict grader route) #2592. The training-agent host serves exclusively server-to-server bearer/signed traffic, so CSRF has no work to do there.scripts/verify-version-sync.cjsRule 1, which was inconsistent with the dual-branch release model its own docstring describes. The hook now allowsadcp_version >= published_version(catching the real bug — stale legacy alias when canonical bumps) instead of requiring strict equality (which false-positived the forward-merge window). This was blocking unrelated pre-push pushes on main.Why this issue persisted
The verifier wiring this issue originally asked for has actually been in the tree since PR #2592 —
request-signing.tsships JWKS loading,test-revoked-2026pre-revocation,PostgresReplayStore, and four route variants (/mcp,/mcp-strict,/mcp-strict-required,/mcp-strict-forbidden). The earlier triage on this issue concluded/mcp-strictreturned 404 because the route wasn't mounted; the deeper diagnosis is that the route shape changed to per-tenant (/<tenant>/mcp-strict) and the curl probe was hitting the obsolete root path. Probing/sales/mcp-strictdirectly surfaces the actual blocker: CSRF 403 before the verifier ever runs.How to verify against the live agent (post-merge / post-deploy)
Expected: positive vectors return 200, vector 001 returns
request_signature_required, vector 017 returnsrequest_signature_revoked(via the pre-loadedtest-revoked-2026keyid).Test plan
npx vitest run server/tests/unit/csrf-middleware.test.ts— 47/47 pass (44 existing + 3 new covering training-agent hostname bypass, per-tenant URL matrix, and the negative case that non-exempt hostnames still validate).npm run verify-version-sync— green with the new at-or-ahead semantic.test:unit+ typecheck) green at each commit.https://test-agent.adcontextprotocol.org/sales/mcp-strictwithgrade_agent_signingand confirm vectors flow through to the verifier.Closes #2368.
🤖 Generated with Claude Code