Reject non-json MCP content types#857
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds Content-Type header validation to MCP request handling. A helper function normalizes and checks whether the header indicates JSON, while the request handler uses it to reject non-JSON requests early with HTTP 415 and a JSON-RPC error. Tests verify both accepted JSON variants and multiple rejected non-JSON media types. ChangesMCP Content-Type Validation
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
NiXouuuu
left a comment
There was a problem hiding this comment.
Reviewed current head ce11ee6cd671918cc06418fed6acce5b21925797 as a non-author for Bounty #838 review evidence.
Scope inspected:
app/mcp.py: verified the new media-type gate rejects missing or non-application/jsonrequest content types before JSON parsing, while still acceptingapplication/json; charset=utf-8.tests/test_api_mcp.py: verified regressions covertext/plain,text/html,application/xml,multipart/form-data, missingContent-Type, charset-qualified JSON, malformed JSON, non-object requests, and invalid params.
Live production repro before this fix, using a valid tools/list JSON-RPC body against https://mcp.mrwk.online/mcp:
Content-Type: application/json-> HTTP 200, tools list returned.Content-Type: application/json; charset=utf-8-> HTTP 200, tools list returned.Content-Type: text/plain-> HTTP 200, tools list returned.Content-Type: text/html-> HTTP 200, tools list returned.Content-Type: application/xml-> HTTP 200, tools list returned.Content-Type: multipart/form-data-> HTTP 200, tools list returned.- no explicit
Content-Typewith a raw byte body -> HTTP 200, tools list returned.
Validation run locally on this exact head:
uv run --extra dev pytest tests/test_api_mcp.py::test_mcp_rejects_malformed_requests_without_500 tests/test_api_mcp.py::test_mcp_rejects_non_json_content_types tests/test_api_mcp.py::test_mcp_tools_list_and_call -q-> 7 passed, 1 warning.uv run --extra dev pytest tests/test_api_mcp.py tests/test_mcp_tools.py -q-> 117 passed, 1 warning.uv run --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> passed.uv run --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> 2 files already formatted.uv run --extra dev mypy app/mcp.py-> success.uv run --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree53079ae854d4657d7a3947244732352d41514e91.
Current GitHub state before review: mergeable=MERGEABLE, mergeStateStatus=UNSTABLE only because CodeRabbit is still pending, hosted Quality, readiness, docs, and image checks is SUCCESS, and no current-head human reviews are present.
No blockers found in the reviewed scope. The change is limited to the public MCP request media-type gate and does not touch MCP tool semantics, wallet behavior, ledger mutation, bounty creation, payout execution, treasury mutation, private data, secrets, bridge/exchange/cash-out behavior, or MRWK price behavior.
MolhamHamwi
left a comment
There was a problem hiding this comment.
Reviewed current head ce11ee6cd671918cc06418fed6acce5b21925797.
Checked paths:
app/mcp.pytests/test_api_mcp.py
Evidence:
- The new
_is_json_content_type()gate normalizes the media type before parameters, soapplication/json; charset=utf-8remains accepted while missing/non-JSON content types are rejected beforerequest.json()is called. - The 415 response uses a JSON-RPC error with
id: null, which is appropriate because the server intentionally does not parse the body after rejecting the media type. - Existing malformed JSON behavior remains covered separately: with JSON content type, invalid JSON still returns the prior parse error path rather than being conflated with unsupported media type.
Validation run:
.venv/bin/python -m pytest tests/test_api_mcp.py -q→ 110 passed, 1 existing Starlette/httpx warning..venv/bin/python -m ruff check app/mcp.py tests/test_api_mcp.py→ passed.
No blocker found; this is a focused fix for the reported MCP content-type handling issue.
alan747271363-art
left a comment
There was a problem hiding this comment.
Reviewed current head ce11ee6cd671918cc06418fed6acce5b21925797 against current origin/main d7e9b530fffec7bd774da7708597648096a37393.
Scope inspected:
app/mcp.pytests/test_api_mcp.py- supporting MCP tool coverage in
tests/test_mcp_tools.py
The branch-local media-type gate remains focused: _is_json_content_type() accepts application/json and application/json; charset=utf-8, rejects missing/non-JSON Content-Type before JSON parsing, and keeps malformed JSON on the existing JSON-RPC parse-error path. It does not alter MCP tool semantics, wallet behavior, ledger mutation, bounty creation, payout execution, treasury mutation, private data, or secret handling.
Validation on this exact head:
.\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py -q-> 112 passed, 1 existing Starlette/httpx warning..\.venv\Scripts\python.exe -m ruff check app\mcp.py tests\test_api_mcp.py-> passed..\.venv\Scripts\python.exe -m ruff format --check app\mcp.py tests\test_api_mcp.py-> 2 files already formatted..\.venv\Scripts\python.exe -m mypy app\mcp.py-> success..\.venv\Scripts\python.exe scripts\docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.
Current blocker: GitHub now reports mergeStateStatus=DIRTY / conflicting. git merge-tree --write-tree origin/main HEAD exits non-zero with a current-main conflict in app/bounty_api.py while auto-merging tests/test_bounty_api_routes.py. That conflict is outside the visible MCP content-type diff, so this appears to be a stale branch-base/rebase issue, but it blocks merging and should be resolved before merge.
No private data, credentials, wallet material, production mutation, payout execution, treasury execution, ledger mutation, exchange, bridge, cash-out, price behavior, or fabricated payout claims were used.
Summary:
/mcprequests whoseContent-Typeis missing or notapplication/jsonbefore parsing the JSON-RPC body;application/json; charset=utf-8accepted and preserves the existing JSON-RPC parse-error response for malformed JSON;text/plain,text/html,application/xml,multipart/form-data, and missingContent-Type.Bounty #799
Source report: #798 (comment)
Production evidence before fix:
POST https://mcp.mrwk.online/mcpwith a validtools/listJSON-RPC body andContent-Type: application/json-> HTTP 200, returns the tools list.Content-Type: text/plain-> HTTP 200, returns the tools list.Content-Type: text/html-> HTTP 200, returns the tools list.Content-Type: application/xml-> HTTP 200, returns the tools list.Content-Type-> HTTP 200, returns the tools list.Duplicate/current check:
gh pr list --repo ramimbo/mergework --state open --search "content-type mcp in:title,body"returned no open PRs for this scope.Scope:
/mcprequest media-type gate.Validation:
uv run --extra dev pytest tests/test_api_mcp.py::test_mcp_rejects_malformed_requests_without_500 tests/test_api_mcp.py::test_mcp_rejects_non_json_content_types tests/test_api_mcp.py::test_mcp_tools_list_and_call -q-> 7 passed, 1 warning.uv run --extra dev pytest tests/test_api_mcp.py tests/test_mcp_tools.py -q-> 117 passed, 1 warning.uv run --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> passed.uv run --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> 2 files already formatted.uv run --extra dev mypy app/mcp.py-> success.uv run --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean treeabda6f4c7b8cc6d19c1ef4bc0fd2711b121cfce1.Summary by CodeRabbit
Bug Fixes
Tests