fix(mcp_impl): reject balance amounts beyond the safe JSON integer range

[ascandone]

Supersedes #142 and #143 (closing those, see comments there for why they no longer apply as-is).

Background

#142/#143 pinned/fixed a float64-truncation bug in parseBalancesJson, a manual JSON parser for the evaluate MCP tool's balances that no longer exists — it was removed when the tool switched to request.BindArguments into a typed interpreter.Balances (with Amount *big.Int).

That switch does fix the fractional-amount case: math/big.Int's own UnmarshalJSON rejects "100.9" outright.

It does not fix the large-magnitude case, though — it just moved it. I confirmed by reproducing the actual request path: the MCP server decodes the incoming JSON-RPC message via json.Unmarshal(message, &request) into Params.Arguments any, and Go's default for JSON numbers into any is always float64, regardless of magnitude. That happens before BindArguments ever runs. So an amount like 9007199254740993 (one past float64's exact range, 2^53-1) is silently rounded to 9007199254740992 at the transport layer, and by the time BindArguments sees it, it looks like a perfectly clean, valid integer — big.Int has no way to tell it was ever anything else.

Fix

Added checkBalanceAmountsInSafeRange, which inspects the raw (pre-BindArguments) float64 amounts via request.GetArguments() and rejects anything outside ±(2^53-1), before the value is converted to *big.Int and the corruption becomes unobservable. Mirrors the bounds-check approach from the old #143, just applied at the current call site.

Test plan

• go test ./internal/mcp_impl/... — all existing tests pass, plus new coverage:
  • rejects an amount one past the safe range, reproduced via an actual JSON-RPC message round-tripped through json.Unmarshal (not just a hand-built Go struct) to match the real corruption path
  • rejects a large negative amount
  • accepts amounts within the safe range
  • still rejects fractional amounts (regression check that this path is unaffected)

[NumaryBot]

🛑 Changes requested — automated review

The new guard does not fully prevent precision-corrupted amounts from being accepted after float64 decoding, leaving an edge case where invalid values round into the accepted range.

[NumaryBot]

NumaryBot posted 1 new inline finding.

Summary: #178 (comment)

[NumaryBot]

🟠 [major] Reject rounded fractional amounts near the safe boundary

For JSON numbers just over the safe bound that round back inside it, such as 9007199254740991.1 decoding to the float64 value 9007199254740991, this comparison passes and BindArguments then accepts the rounded integer as a big.Int. Because the original token is already lost here, this still allows silently corrupted out-of-range/fractional balances; the check needs access to the raw JSON/json.Number value or another conservative rejection strategy before binding.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d85fd2ac-80d5-40e6-968e-cf3adeb08263

📥 Commits

Reviewing files that changed from the base of the PR and between cb94e4e and 5778fd1.

📒 Files selected for processing (2)

• internal/mcp_impl/handlers.go
• internal/mcp_impl/handlers_test.go

---

Walkthrough

The MCP evaluation handler now validates raw balance amounts before binding. It rejects values outside the JSON safe integer range and preserves downstream rejection of fractional amounts. Tests cover positive, negative, valid, and fractional inputs.

Changes

Safe balance validation

Layer / File(s)	Summary
Pre-binding balance validation internal/mcp_impl/handlers.go	The evaluation handler scans raw balance arguments and rejects amounts outside ±(2^53−1) before argument binding and execution.
Transport-level validation coverage internal/mcp_impl/handlers_test.go	Tests verify rejection of unsafe positive, unsafe negative, and fractional amounts, plus acceptance of amounts within the safe range.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

A rabbit checks each number bright,
Before it bounds the values right.
Too large, too small, or split in two—
The handler knows what it should do.
Safe sums pass with ears held high.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting balance amounts beyond the safe JSON integer range.
Description check	✅ Passed	The description explains the precision issue, the implemented validation, and the related test coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/mcp-reject-unsafe-integer-amounts

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}