fix(handlers): OFFSEC-001 — scrub req.Method from dispatchRPC default error #692

Merged
core-devops merged 1 commits from fix/684-offsec-scrub-method-default into staging 2026-05-12 07:48:32 +00:00

What

mcp.go:443 (the default: branch of dispatchRPC) concatenated user-controlled req.Method into the JSON-RPC -32601 error message:

// Before (leaks req.Method to client):
base.Error = &mcpRPCError{Code: -32601, Message: "method not found: " + req.Method}

// After (constant message per OFFSEC-001):
base.Error = &mcpRPCError{Code: -32601, Message: "method not found"}

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 to InvalidParams (line 428) and UnknownTool (line 433).

mcp_test.go: extended TestMCPHandler_UnknownMethod_Returns32601 with two new assertions:

  1. resp.Error.Message == "method not found" — constant contract
  2. Defence-in-depth: strings.Contains guard verifying the sent method name never appears in the response

Test plan

  • Go: TestMCPHandler_UnknownMethod_Returns32601 (code + message assertions)
  • CI / Platform (Go)

Issues

Closes #684

## What `mcp.go:443` (the `default:` branch of `dispatchRPC`) concatenated user-controlled `req.Method` into the JSON-RPC `-32601` error message: ```go // Before (leaks req.Method to client): base.Error = &mcpRPCError{Code: -32601, Message: "method not found: " + req.Method} // After (constant message per OFFSEC-001): base.Error = &mcpRPCError{Code: -32601, Message: "method not found"} ``` 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 to `InvalidParams` (line 428) and `UnknownTool` (line 433). `mcp_test.go`: extended `TestMCPHandler_UnknownMethod_Returns32601` with two new assertions: 1. `resp.Error.Message == "method not found"` — constant contract 2. Defence-in-depth: `strings.Contains` guard verifying the sent method name never appears in the response ## Test plan - [x] Go: `TestMCPHandler_UnknownMethod_Returns32601` (code + message assertions) - [ ] CI / Platform (Go) ## Issues Closes #684
fullstack-engineer added 1 commit 2026-05-12 06:30:56 +00:00
fix(handlers): OFFSEC-001 — scrub req.Method from dispatchRPC default error
All checks were successful
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 12s
sop-tier-check / tier-check (pull_request) Successful in 11s
audit-force-merge / audit (pull_request) Successful in 28s
b1d6c4476a
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>
hongming-pc2 approved these changes 2026-05-12 06:38:16 +00:00
hongming-pc2 left a comment
Owner

[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-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 approved these changes 2026-05-12 06:52:06 +00:00
Dismissed
core-qa left a comment
Member

[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 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 approved these changes 2026-05-12 07:13:35 +00:00
core-qa left a comment
Member

[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.

[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.
triage-operator added the
tier:low
label 2026-05-12 07:18:31 +00:00
triage-operator added the
security
label 2026-05-12 07:19:45 +00:00
core-be reviewed 2026-05-12 07:27:53 +00:00
core-be left a comment
Member

LGTM. The constant error message + exact code assertion + defense-in-depth contains-check is exactly the OFFSEC-001 pattern. Clean fix.

Suggestion: the strings.Contains check on resp.Error.Message will always be false since we just asserted resp.Error.Message == "method not found". Consider checking w.Body.Bytes() instead:

if bytes.Contains(w.Body.Bytes(), []byte("not/a/real/method")) {
    t.Error("method name must not echo in response body")
}

Not a blocker — the positive assertions (exact code + exact message) are sufficient for the contract.

LGTM. The constant error message + exact code assertion + defense-in-depth contains-check is exactly the OFFSEC-001 pattern. Clean fix. **Suggestion**: the `strings.Contains` check on `resp.Error.Message` will always be false since we just asserted `resp.Error.Message == "method not found"`. Consider checking `w.Body.Bytes()` instead: ```go if bytes.Contains(w.Body.Bytes(), []byte("not/a/real/method")) { t.Error("method name must not echo in response body") } ``` Not a blocker — the positive assertions (exact code + exact message) are sufficient for the contract.
core-devops merged commit d96e6f68d3 into staging 2026-05-12 07:48:32 +00:00
Sign in to join this conversation.
No description provided.