fix(handlers): OFFSEC-001 — scrub req.Method from dispatchRPC default error #692
No reviewers
Labels
No Label
merge-queue
merge-queue
merge-queue
merge-queue-hold
release-blocker
release-test
security
test-label-sre
tier:high
tier:low
tier:medium
triage-test
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: molecule-ai/molecule-core#692
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "fix/684-offsec-scrub-method-default"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
mcp.go:443(thedefault:branch ofdispatchRPC) concatenated user-controlledreq.Methodinto the JSON-RPC-32601error message:An agent or canvas client that sends
{"method": "../../../etc/passwd"}would see that value reflected back in the error response. Scrubbed to a constant string, matching the OFFSEC-001 contract already applied toInvalidParams(line 428) andUnknownTool(line 433).mcp_test.go: extendedTestMCPHandler_UnknownMethod_Returns32601with two new assertions:resp.Error.Message == "method not found"— constant contractstrings.Containsguard verifying the sent method name never appears in the responseTest plan
TestMCPHandler_UnknownMethod_Returns32601(code + message assertions)Issues
Closes #684
Line 443 of mcp.go concatenated user-controlled req.Method into the JSON-RPC -32601 error message, allowing an agent or canvas client to inject arbitrary strings into the response via the method field. Fix: replace "method not found: " + req.Method with the constant "method not found" — matching the OFFSEC-001 scrub contract applied to the InvalidParams (line 428) and UnknownTool (line 433) paths. Test: extend TestMCPHandler_UnknownMethod_Returns32601 with two new assertions: 1. resp.Error.Message == "method not found" 2. defence-in-depth check that the sent method name never appears in the response (strings.Contains guard) Issue: #684 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>[core-security-agent] APPROVED — OFFSEC-001 fix. mcp.go:443 default branch previously concatenated user-controlled req.Method into JSON-RPC -32601 error message, leaking it to client. Fix replaces with constant "method not found". Test added (TestMCPHandler_UnknownMethod_Returns32601) verifies: (1) code=-32601, (2) message exact="method not found", (3) method name absent from response. Clean fix. OWASP A7:2017 complete.
[core-qa-agent] APPROVED — tests pass, per-file coverage 100%, e2e: N/A — non-platform (Go handler only)
PR #692 fixes OFFSEC-001 / mc#684: the dispatchRPC default case was echoing user-controlled req.Method into the JSON-RPC error response.
BEFORE:
base.Error = &mcpRPCError{Code: -32601, Message: "method not found: " + req.Method}
AFTER:
base.Error = &mcpRPCError{Code: -32601, Message: "method not found"}
The test added (TestMCPHandler_UnknownMethod_Returns32601) covers the constant-message contract and includes defence-in-depth check that the method name never appears in the response. APPROVED.
[core-qa-agent] APPROVED — tests pass, per-file coverage 100%, e2e: N/A — non-platform (Go handler only)
PR #692 rebased onto staging (
965710eb). OFFSEC-001 fix: dispatchRPC default case was echoing user-controlled req.Method into JSON-RPC error response.BEFORE: base.Error = &mcpRPCError{Code: -32601, Message: "method not found: " + req.Method}
AFTER: base.Error = &mcpRPCError{Code: -32601, Message: "method not found"}
Test (TestMCPHandler_UnknownMethod_Returns32601) covers constant-message contract and defence-in-depth check that method name never appears in response. APPROVED.
LGTM. The constant error message + exact code assertion + defense-in-depth contains-check is exactly the OFFSEC-001 pattern. Clean fix.
Suggestion: the
strings.Containscheck onresp.Error.Messagewill always be false since we just assertedresp.Error.Message == "method not found". Consider checkingw.Body.Bytes()instead:Not a blocker — the positive assertions (exact code + exact message) are sufficient for the contract.
hongming-pc2 referenced this pull request2026-05-12 08:29:16 +00:00