Refs #576: Reject control chars in MCP integer args

[xingxi0614-cpu]

Refs #576

Summary

Reject raw control characters in MCP integer-like string arguments before trimming/parsing.

This keeps MCP tool calls from accepting tab/newline/DEL-padded numeric strings as if they were clean integers:

• get_bounty with id="\t1"
• list_bounty_attempts with bounty_id="1\n"
• submit_work_proof with issue_number="\x7f390"

Distinctness

This is limited to MCP integer argument parsing in app/mcp_tools.py.

It does not overlap PR #612, which covers MCP optional clean-string filters, or PR #614, which covers HTTP JSON integer parsing. It also stays separate from the wallet/account/bounty filter, source URL, MRWK amount, webhook, treasury, and public search control-character fixes.

Validation

• uv run --extra dev python -m pytest tests/test_mcp_tools.py -q -> 8 passed
• uv run --extra dev python -m pytest tests/test_mcp_tools.py tests/test_api_mcp.py -q -> 90 passed, 1 warning
• uv run --extra dev python -m pytest -q -> 497 passed, 1 warning
• uv run --extra dev ruff check app/mcp_tools.py tests/test_mcp_tools.py -> passed
• uv run --extra dev ruff format --check app/mcp_tools.py tests/test_mcp_tools.py -> 2 files already formatted
• uv run --extra dev mypy app/mcp_tools.py -> success
• uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
• git diff --check -> clean
• git merge-tree --write-tree origin/main HEAD -> clean

Safety

No private data, wallet material, tokens, signatures, admin access, production mutation, price/liquidity/exchange/bridge/off-ramp claims, private security details, or fabricated payout claims are included.

Summary by CodeRabbit

• Refactor

  • Code formatting improvements to import statements.

• Tests

  • Added validation tests ensuring control characters are properly rejected in tool arguments.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9ddc9920-eb08-4245-8b1f-38be35e40a92

📥 Commits

Reviewing files that changed from the base of the PR and between 0708f8a and 11b5d84.

📒 Files selected for processing (2)

• app/mcp_tools.py
• tests/test_mcp_tools.py

---

📝 Walkthrough

Walkthrough

Adds control-character validation to integer-like string arguments in MCP tool calls. Imports CONTROL_CHAR_RE from the ledger service, implements the validation check in argument parsing, and tests the rejection of control characters for three integer-argument MCP tools.

Changes

Control character validation for MCP integer arguments

Layer / File(s)	Summary
Control character validation for MCP integer arguments app/mcp_tools.py, tests/test_mcp_tools.py	Import refactor exposes CONTROL_CHAR_RE, argument parsing adds a pre-trim validation check that raises ValueError for control characters in integer arguments, and parameterized tests verify rejection for get_bounty, list_bounty_attempts, and submit_work_proof.

Possibly related PRs

• ramimbo/mergework#323: Extends MCP integer-argument validation in the same call_mcp_tool/_call_mcp_tool parsing path to enforce SQLite integer bounds and prevent 500 errors.

• ramimbo/mergework#617: Expands CONTROL_CHAR_RE and related control-character validation in parse_mrwk_amount and treasury parsing, using the same shared validation pattern.

• ramimbo/mergework#398: Introduces the initial call_mcp_tool argument-validation test infrastructure that this PR extends with control-character rejection coverage.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately names the surface changed: MCP integer argument handling with control character validation, matching the PR's focused scope.
Description check	✅ Passed	The description includes all required sections with concrete evidence: summary of the change, distinctness from related PRs, validation results covering tests/linting/type-checking, and safety attestation.
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.
Mergework Public Artifact Hygiene	✅ Passed	PR only adds input validation for MCP integer arguments; no README, docs, or public description contains investment, price, cash-out, or fabricated payout claims. MRWK framing is correct.
Bounty Pr Focus	✅ Passed	PR diff matches stated scope: control character rejection in MCP integer args (int_arg checks CONTROL_CHAR_RE before trim) with test coverage for three specified tools. No scope creep detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[barnacleagent-svg]

PR Review — #626

PR: #626
Author: @xingxi0614-cpu
Head SHA: 11b5d84

Files Changed (2 files, +45/-1)

• app/mcp_tools.py: Added CONTROL_CHAR_RE check before strip/parse in int_arg()
• tests/test_mcp_tools.py: Parametrized test covering ,
  , � control chars

CI Status

Quality/readiness/docs/image checks: completed/success

Code Review

• Correct: checks raw value before strip so control chars at start/end are caught
• Import added for CONTROL_CHAR_RE
• Three test cases cover tab, newline, DEL character
• No side effects, no stale references

Verdict

APPROVE — Ready for merge.

[peterxing]

Reviewed for Bounty #578.

Evidence checked:

• Inspected the fetched patch for app/mcp_tools.py: int_arg() now checks the raw string with CONTROL_CHAR_RE before .strip() or numeric parsing, so tab/newline/DEL-padded integer arguments are rejected instead of normalized into valid IDs.
• Inspected the fetched patch for tests/test_mcp_tools.py: the new parameterized test covers the three exposed integer-argument tool paths (get_bounty.id, list_bounty_attempts.bounty_id, and submit_work_proof.issue_number) with leading/trailing control characters.
• Compared the scope against nearby control-character PRs noted in the thread: this slice is MCP integer-argument parsing only, distinct from MCP clean-string filters (Refs #576: Reject control chars in MCP clean filters #612), JSON integer parsing (Refs #576: Reject control chars in JSON integer fields #614), and amount/treasury parsing (Refs #576: Reject control chars in MRWK amounts #617).
• CodeRabbit reports the focused pre-merge checks passed and selected only app/mcp_tools.py plus tests/test_mcp_tools.py for this head.

No blocker from this review. The patch is narrow and the regression test covers the intended pre-trim rejection behavior.

[wangedmund77-cmyk]

Review note after inspecting app/mcp_tools.py and tests/test_mcp_tools.py: no blocker found.

The important detail is that int_arg() now checks CONTROL_CHAR_RE before calling strip(), so inputs like "\t1", "1\n", or "\x7f390" cannot be silently normalized into valid integer strings. The parametrized test covers three MCP tools that consume integer arguments (get_bounty, list_bounty_attempts, and submit_work_proof), which makes this a shared parser regression test rather than a single endpoint-only check.