chore: grader MCP follow-ups — op allowlist + rate-abuse subtest#620
Merged
Conversation
Two follow-ups deferred from #617. 1. Operation-name allowlist in extractOperationFromVectorUrl. Throws if the vector URL's last path segment isn't an AdCP operation identifier (lowercase_snake_case). No exploit today — fixtures are spec-published — but blocks arbitrary bytes from reaching params.name if a compliance cache gets corrupted or swapped. 2. MCP rate-abuse subtest. Dedicated MCP agent with ADCP_REPLAY_CAP=10 + grader configured with matching cap + onlyVectors=['020-rate-abuse'] + allowLiveSideEffects=true. Validates the 11-request flood trips request_signature_rate_abuse under MCP transport. Adds ADCP_REPLAY_CAP env override to seller-agent-signed-mcp.ts so tests can tune the cap without touching agent code. All 5 MCP tests pass; full suite 3947/3949 (2 pre-existing skips). Refs #617 review thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three low-severity items from the expert-review round:
1. NaN guard on ADCP_REPLAY_CAP. A typo (ADCP_REPLAY_CAP=abc) turns
Number.parseInt into NaN; InMemoryReplayStore's `size >= NaN` check
is always false, silently disabling the rate-abuse guard. Now falls
back to the default on any non-positive int.
2. PORT clobber fix in startMcpAgent. Spread overrides BEFORE PORT so
a caller passing { PORT: '9999' } in overrides can't override the
explicit port argument. Prophylactic — no caller does this today.
3. Aspirational comment reference in builder.ts operation allowlist.
The reviewer noted `static/schemas/source/enums/operation.json`
isn't in the local sync (it lives in the adcp spec repo). Reworded
to reference the actual observable source — task values in
compliance/cache/{version}/protocols/**/*.yaml.
Bonus: documented the port-stride requirement (≥2) for parallel runs
of request-signing-grader-mcp.test.js, since the rate-abuse subtest
binds PORT+1.
All 5 MCP tests still pass. Refs #620 review thread.
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
Closes the two deferred items from the #617 review thread (#617 (comment)).
Operation-name allowlist on
extractOperationFromVectorUrl. Constrains the extracted operation to/^[a-z][a-z0-9_]*$/(AdCP operation identifier shape perstatic/schemas/source/enums/operation.json). Throws with a clear diagnostic if a vector URL's last path segment doesn't match — defense in depth against a corrupted compliance cache. No exploit today; fixtures are spec-published.MCP rate-abuse subtest. Spins up a dedicated MCP agent with
ADCP_REPLAY_CAP=10+ grades withonlyVectors: ['020-rate-abuse']/rateAbuseCap: 10/allowLiveSideEffects: true/transport: 'mcp'. Validates the 11-request flood tripsrequest_signature_rate_abuseunder MCP transport. Previously the rate-abuse flow was only tested against the raw-HTTP reference verifier.Adds
ADCP_REPLAY_CAPenv override totest-agents/seller-agent-signed-mcp.tsso tests can tune the cap without forking the agent.Test plan
npx tsc --noEmitcleannpm run build:libcleannpm run format:checkcleannpm test— 3947/3949 pass, 0 fail, 2 skipped (pre-existing)🤖 Generated with Claude Code