fix(mcp): reject tools/call requests with non-object arguments by VectorPeak · Pull Request #99180 · openclaw/openclaw

VectorPeak · 2026-07-02T17:04:08Z

What Problem This Solves

The raw Gateway MCP loopback handler accepted any JSON value in tools/call.params.arguments and cast it to an object record. Malformed null, array, or primitive arguments could therefore reach before-tool hooks and tool implementations even though MCP defines this field as an optional object map.

Why This Change Was Made

The loopback handler now rejects a present non-object arguments value with JSON-RPC -32602 before hooks, capture preparation, or tool execution. Omitted arguments still become {}, and valid object arguments pass through unchanged.

This is deliberately limited to the raw loopback HTTP boundary. The sibling stdio MCP server already registers CallToolRequestSchema from the SDK and receives the same validation there. Applying full per-tool schema validation or parsing every loopback request through the SDK would broaden behavior beyond this malformed envelope.

User Impact

Valid MCP clients are unchanged. Clients sending malformed primitive, null, or array tool arguments now receive Invalid params: tools/call arguments must be an object instead of passing the wrong runtime shape into OpenClaw tools.

Evidence

Direct dependency inspection: the pinned @modelcontextprotocol/sdk 1.29.0 defines CallToolRequestParamsSchema.arguments as z.record(z.string(), z.unknown()).optional() in dist/esm/types.js; src/mcp/tools-stdio-server.ts uses that request schema.
Sanitized AWS Crabbox baseline run run_2be60da5f513 combined the regression test with current-main src/gateway/mcp-http.handlers.ts and reproduced the malformed-arguments failure.
Sanitized AWS Crabbox run run_6273eafe9b17, provider aws, lease cbx_e322edfdf5fc: pnpm test src/gateway/mcp-http.test.ts passed 272/272 tests across four Gateway projects before the test-only mock typing correction.
Exact final-head GitHub Actions workflow 28858393759 passed on 705e63af0e793e46db0467031e1f405db93568a9, including check-test-types and every selected test shard. A repeat sanitized AWS attempt (run_1fa1a1d425db) could not install dependencies because AWS fell the requested beast lease back to a t3.small and the install was OOM-killed.
The focused HTTP tests prove null, array, and string inputs return -32602 with zero hook/tool calls; omitted arguments reach both as {}; the existing object execution test now verifies a non-empty argument object reaches the hook and executor unchanged.
The branch was rebuilt from current main as one commit, preserving contributor credit with Co-authored-by: VectorPeak.
git diff --check passed.
Final structured autoreview on exact head 705e63af0e793e46db0467031e1f405db93568a9 was clean with 0.92 confidence.

clawsweeper · 2026-07-02T17:14:43Z

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 6:23 AM ET / 10:23 UTC.

Summary
The branch adds a Gateway MCP HTTP guard that rejects present non-object tools/call.params.arguments with JSON-RPC -32602 and adds loopback tests for invalid, omitted, and object argument paths.

PR surface: Source +5, Tests +67. Total +72 across 2 files.

Reproducibility: yes. at source level: current main casts params.arguments and passes it to the hook/tool path, while the PR exercises the raw loopback HTTP path for malformed, omitted, and object argument cases. I did not run tests because this review was required to keep the checkout read-only.

Review metrics: 1 noteworthy metric.

MCP request contract tightened: 1 tightened request parameter: tools/call.params.arguments. Maintainers should notice this because malformed loopback clients may need to change from primitive, null, or array arguments to omitted or object arguments.

Stored data model
Persistent data-model change detected: serialized state: src/gateway/mcp-http.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] Malformed MCP loopback clients that currently send primitive, null, or array arguments will now fail early with JSON-RPC -32602 instead of reaching before-tool hooks or tool execution.

Maintainer options:

Accept strict malformed-payload rejection (recommended)
Approve the intentional behavior change because it aligns raw HTTP loopback with the MCP SDK object-argument contract and has focused compatibility coverage.
Request compatibility-preserving rework
Ask the contributor to preserve current malformed-client behavior or provide a maintainer-approved transition path before merge.
Pause for broader MCP validation design
Leave the PR open if maintainers want one coherent validation policy for raw HTTP MCP params instead of a targeted arguments fix.

Next step before merge

[P2] A Gateway/MCP owner needs to choose the malformed-client compatibility policy; there is no narrow automated repair to make before that decision.

Maintainer decision needed

Question: Should Gateway HTTP MCP loopback reject present non-object tools/call.params.arguments with JSON-RPC -32602, even though malformed clients that previously reached hooks or tool execution will now fail early?
Rationale: The MCP SDK and stdio sibling surface support the object-or-omitted contract, but current shipped/raw HTTP behavior tolerated malformed values, so the merge depends on owner acceptance of the compatibility tradeoff.
Likely owner: steipete — steipete is the live assignee and has the strongest recent Gateway/MCP history signal for this boundary.
Options:
- Accept stricter MCP contract (recommended): Merge the PR as-is, treating primitive, null, and array arguments as invalid while preserving omitted and object-shaped calls.
- Preserve malformed-client tolerance: Ask for a rework that keeps accepting or coercing malformed arguments, at the cost of keeping raw HTTP loopback looser than the SDK schema.
- Pause for broader validation policy: Hold this PR until maintainers define wider MCP HTTP request validation beyond tools/call.arguments.

Security
Cleared: Cleared: the diff adds input validation and tests only, with no dependency, workflow, secret, permission, package, or supply-chain changes.

Review details

Best possible solution:

Land the narrow handler validation and regression coverage once a Gateway/MCP owner explicitly accepts the stricter malformed-client boundary.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main casts params.arguments and passes it to the hook/tool path, while the PR exercises the raw loopback HTTP path for malformed, omitted, and object argument cases. I did not run tests because this review was required to keep the checkout read-only.

Is this the best way to solve the issue?

Yes, conditional on maintainer acceptance of the compatibility break: validating at the raw HTTP MCP handler is narrower than changing stdio handling or adding broad per-tool schema validation for this boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9fb24032215a.

Label changes

Label justifications:

P2: This is a focused Gateway/MCP bug fix with limited blast radius to malformed loopback tool-call payloads.
merge-risk: 🚨 compatibility: Merging intentionally changes existing malformed-client behavior from hook/tool execution to JSON-RPC invalid-params rejection.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Sufficient: the PR body and comments provide terminal/Crabbox validation for the raw loopback HTTP behavior, and the latest head keeps the same production handler while changing only tests.
proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body and comments provide terminal/Crabbox validation for the raw loopback HTTP behavior, and the latest head keeps the same production handler while changing only tests.

Evidence reviewed

PR surface:

Source +5, Tests +67. Total +72 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	6	1	+5
Tests	1	68	1	+67
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	74	2	+72

What I checked:

Current main behavior: Current main still casts methodParams?.arguments ?? {} to Record<string, unknown> before hooks and tool execution, so the central issue is not already fixed on main. (src/gateway/mcp-http.handlers.ts:102, 9fb24032215a)
PR handler behavior: At PR head, present non-record arguments values return JSON-RPC invalid params before toolArgs, hooks, or tool execution are reached. (src/gateway/mcp-http.handlers.ts:103, 705e63af0e79)
Regression coverage: The PR head tests rejection for null, array, and string arguments, keeps omitted arguments as {}, and verifies object arguments reach the hook and executor unchanged through the loopback tool-call helper. (src/gateway/mcp-http.test.ts:1155, 705e63af0e79)
HTTP entry point: The /mcp HTTP loopback parses raw JSON-RPC and calls handleMcpJsonRpc, making the handler the narrow boundary for rejecting malformed tools/call params before side effects. (src/gateway/mcp-http.ts:269, 9fb24032215a)
Sibling stdio surface: The stdio MCP server registers the SDK CallToolRequestSchema before invoking shared MCP handlers, so the raw HTTP loopback is the sibling surface missing this request-shape validation. (src/mcp/tools-stdio-server.ts:17, 9fb24032215a)
Dependency contract: The pinned MCP TypeScript SDK v1.29.0 defines CallToolRequestParamsSchema.arguments as an optional record, matching this PR's object-or-omitted contract. (modelcontextprotocol/typescript-sdk/src/types.ts:1489)

Likely related people:

steipete: Live PR state assigns this review to steipete, and history shows recent changes to MCP loopback handler, transport helper, and audit/tool-call correlation paths near this boundary. (role: recent Gateway/MCP area contributor and assignee; confidence: high; commits: f53346944db0, 25b069a6f3cd, 3d23103081f4; files: src/gateway/mcp-http.handlers.ts, src/gateway/mcp-http.ts, src/gateway/mcp-http.test.ts)
vincentkoc: Blame on the current tools/call block and GitHub history show recent MCP loopback schema hardening, JSON-RPC failure classification, and message-delivery evidence work touching this area. (role: recent Gateway/MCP runtime contributor; confidence: high; commits: 07a765522a, 705bdcec7050, 172c3f60640e; files: src/gateway/mcp-http.handlers.ts, src/gateway/mcp-http.ts, src/gateway/mcp-http.test.ts)
anagnorisis2peripeteia: Recent commits added and hardened the Streamable HTTP MCP loopback transport and attach launcher paths that consume this request boundary. (role: adjacent MCP HTTP transport contributor; confidence: medium; commits: 6df7db9f9e49, 224ea76d29d0, 7f69fe009a78; files: src/gateway/mcp-http.ts, src/gateway/mcp-http.test.ts)
mmaps: History ties MCP loopback owner policy and before-tool-call hook enforcement to this contributor, which matters because this PR rejects malformed arguments before that hook path. (role: adjacent hook-boundary contributor; confidence: low; commits: 8b76392e3e79; files: src/gateway/mcp-http.handlers.ts, src/gateway/mcp-http.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (7 earlier review cycles)

reviewed 2026-07-02T18:48:29.878Z sha 26f7f71 :: needs maintainer review before merge. :: none
reviewed 2026-07-07T05:42:46.211Z sha ac96494 :: needs maintainer review before merge. :: none
reviewed 2026-07-07T06:03:49.897Z sha ea83601 :: needs maintainer review before merge. :: none
reviewed 2026-07-07T06:10:54.369Z sha 8073af6 :: needs maintainer review before merge. :: none
reviewed 2026-07-07T06:18:34.435Z sha 8073af6 :: needs maintainer review before merge. :: none
reviewed 2026-07-07T06:52:17.951Z sha 0097d85 :: needs maintainer review before merge. :: none
reviewed 2026-07-07T09:30:20.951Z sha 2652005 :: needs maintainer review before merge. :: none

VectorPeak · 2026-07-02T17:52:59Z

Updated the PR body to remove the leading BOM and added the requested loopback behavior proof: malformed tools/call.params.arguments now returns JSON-RPC -32602, and the focused proof asserts no before-tool hook call and no tool execution.

The Real behavior proof check is now passing.

@clawsweeper re-review

clawsweeper · 2026-07-02T17:56:14Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

VectorPeak · 2026-07-02T17:57:16Z

@clawsweeper re-review

clawsweeper · 2026-07-02T18:00:10Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

VectorPeak · 2026-07-02T18:18:09Z

Strengthened the proof without expanding the production scope.

The handler change is unchanged and remains limited to the Gateway HTTP tools/call boundary. I added test-only coverage for the full present-but-non-object matrix (bad, 42, true, null, []) and a positive compatibility case showing omitted arguments still reaches the hook/tool path as {}.

Focused validation:

$env:CI='true'; pnpm exec vitest run --config test/vitest/vitest.gateway.config.ts src/gateway/mcp-http.test.ts
Test Files  3 passed (3)
Tests       183 passed (183)

@clawsweeper re-review

clawsweeper · 2026-07-02T18:18:12Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

VectorPeak · 2026-07-02T18:26:11Z

Strengthened the PR proof without changing code: added the SDK argument-contract rationale, sibling stdio-surface comparison, accepted/rejected argument matrix, and current proof/check status. The PR body is now UTF-8 without BOM.\n\n@clawsweeper re-review

VectorPeak · 2026-07-02T18:36:33Z

Added a small test-only proof update on top of the latest PR head.

What changed:

Added a positive regression test showing object-shaped tools/call.params.arguments reaches both runBeforeToolCallHook and the tool executor unchanged.
Production code is unchanged from the existing boundary fix.
Re-ran focused validation after rebasing onto the updated PR branch:

$env:CI='true'
pnpm exec vitest run --config test/vitest/vitest.gateway.config.ts src/gateway/mcp-http.test.ts
Test Files  3 passed (3)
Tests       186 passed (186)

This completes the proof triangle: present non-object values are rejected before side effects, omitted arguments remains {}, and valid object arguments still pass through unchanged.

@clawsweeper re-review

clawsweeper · 2026-07-02T18:36:35Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

VectorPeak · 2026-07-02T18:48:27Z

@clawsweeper re-review

VectorPeak · 2026-07-07T06:40:14Z

All checks are green on the latest pushed head (8073af6). The branch was synced with upstream/main; the current BEHIND state is from newer unrelated upstream churn in QA/GitHub Copilot files and does not touch this MCP gateway change.

@clawsweeper re-review

clawsweeper · 2026-07-07T06:40:18Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Co-authored-by: VectorPeak <[email protected]>

steipete · 2026-07-07T10:25:47Z

Land-ready maintainer verification:

Rebuilt the branch as one current-main commit while preserving @VectorPeak credit.
Directly checked pinned @modelcontextprotocol/sdk 1.29.0: tools/call.params.arguments is an optional object record; the sibling stdio server already enforces CallToolRequestSchema.
Current-main baseline reproduced the malformed-value path in sanitized AWS Crabbox run run_2be60da5f513.
Focused sanitized AWS run run_6273eafe9b17 passed 272/272 Gateway MCP HTTP tests before the final test-only mock typing correction.
Exact final head 705e63af0e793e46db0467031e1f405db93568a9 passed GitHub Actions workflow https://github.com/openclaw/openclaw/actions/runs/28858393759, including check-test-types and all selected test shards.
A repeat sanitized AWS run (run_1fa1a1d425db) was infrastructure-blocked: the requested beast lease fell back to t3.small, then dependency installation was OOM-killed. No product test ran or failed in that attempt.
git diff --check passed; final structured autoreview was clean at 0.92 confidence.
OPENCLAW_TESTBOX=1 scripts/pr prepare-run 99180 passed the exact-head hosted CI/Testbox gate.

No unresolved review threads or known proof gaps remain.

steipete · 2026-07-07T10:26:05Z

Merged via squash.

Prepared head SHA: 705e63af0e793e46db0467031e1f405db93568a9
Landed commit: cd5c3fc3b779d803faf4099771c7b2dbaca5f324

Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added gateway Gateway runtime size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 2, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 2, 2026

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 2, 2026

openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 2, 2026

github-actions Bot mentioned this pull request Jul 7, 2026

🦞 OpenClaw Ecosystem Digest 2026-07-07 THTHDGCS/agents-radar#14

Closed

steipete self-assigned this Jul 7, 2026

steipete force-pushed the fix/mcp-tools-call-arguments branch from 2652005 to 412ef8c Compare July 7, 2026 10:02

fix(mcp): reject non-object tool call arguments

705e63a

Co-authored-by: VectorPeak <[email protected]>

steipete force-pushed the fix/mcp-tools-call-arguments branch from 412ef8c to 705e63a Compare July 7, 2026 10:09

steipete merged commit cd5c3fc into openclaw:main Jul 7, 2026
107 checks passed

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 10:48 UTC curtismercier/openclaw-mods#1229

Open

This was referenced Jul 7, 2026

fix(mattermost): truncate inbound preview on code-point boundary #101630

Merged

fix(gateway): suppress MCP notification responses #101730

Merged

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 8, 2026

fix(mcp): reject non-object tool call arguments (openclaw#99180)

25052fa

Co-authored-by: Peter Steinberger <[email protected]>

giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026

fix(mcp): reject non-object tool call arguments (openclaw#99180)

0dcdab3

Co-authored-by: Peter Steinberger <[email protected]>

Uh oh!

Uh oh!

Conversation

VectorPeak commented Jul 2, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VectorPeak commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VectorPeak commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026

Uh oh!

VectorPeak commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VectorPeak commented Jul 2, 2026

Uh oh!

VectorPeak commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VectorPeak commented Jul 2, 2026

Uh oh!

VectorPeak commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

VectorPeak commented Jul 2, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

VectorPeak commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading