Skip to content

[mcp] fix: reject encoded rpc aliases - #228

Merged
Wangmerlyn merged 1 commit into
mainfrom
codex/rpc-encoded-aliases
Jul 1, 2026
Merged

[mcp] fix: reject encoded rpc aliases#228
Wangmerlyn merged 1 commit into
mainfrom
codex/rpc-encoded-aliases

Conversation

@Wangmerlyn

@Wangmerlyn Wangmerlyn commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • reject percent-encoded exact /rpc aliases such as /rp%63 and /%72pc as structured JSON 404 Unknown endpoint
  • preserve exact raw /rpc behavior as the canonical POST-only JSON-RPC endpoint with structured 405 for unsupported methods
  • document the route contract in AGENTS, MCP guide/reference, and the branch plan

Verification

  • RED: PYTHONPATH=$PWD/src pytest tests/mcp/test_http_api.py -q -k 'http_rpc_noncanonical_get_returns_json_404_without_static_fallback or http_rpc_encoded_exact_alias' failed with 4 expected failures before the fix
  • GREEN: PYTHONPATH=$PWD/src pytest tests/mcp/test_http_api.py -q -k 'http_rpc_noncanonical_get_returns_json_404_without_static_fallback or http_rpc_encoded_exact_alias or http_rpc_get_rejects_with_json_405 or http_jsonrpc_parse_error_returns_jsonrpc_envelope' passed with 13 passed
  • PYTHONPATH=$PWD/src pytest tests/mcp/test_http_api.py -q passed with 121 passed
  • PYTHONPATH=$PWD/src pytest tests -q passed with 862 passed, 11 skipped
  • PYTHONPATH=$PWD/src mkdocs build --strict passed with the existing Material for MkDocs warning
  • pre-commit run --all-files --show-diff-on-failure passed
  • git diff --check passed

Local Review

  • local subagent review: no must-fix issues; reviewer reran the focused regression slice with 13 passed

Summary by CodeRabbit

  • Bug Fixes

    • Requests to encoded or noncanonical /rpc URLs now return a structured 404 Unknown endpoint response.
    • These paths no longer fall through to other handlers or static content, and unsupported methods are handled consistently.
  • Documentation

    • Updated API and CLI guides to clarify which /rpc URL forms are treated as invalid.
    • Added a planning note describing the expected behavior for encoded endpoint variants.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6b9e57a0-5054-458b-a6b1-c7aa58f47773

📥 Commits

Reviewing files that changed from the base of the PR and between 1e3d43c and 5830194.

📒 Files selected for processing (6)
  • AGENTS.md
  • docs/guides/mcp.md
  • docs/plans/rpc-encoded-aliases.md
  • docs/reference/cli.md
  • src/keep_gpu/mcp/server.py
  • tests/mcp/test_http_api.py

📝 Walkthrough

Walkthrough

The _is_noncanonical_rpc_route route predicate in server.py was extended to flag paths that percent-decode to /rpc but differ from the raw path, causing structured 404 "Unknown endpoint" responses. Tests were added/updated, and documentation across four files reflects this behavior.

Changes

Encoded /rpc Alias Rejection

Layer / File(s) Summary
Route predicate update
src/keep_gpu/mcp/server.py
_is_noncanonical_rpc_route gains a branch flagging paths whose unquoted form equals /rpc but whose raw path differs, alongside existing checks for query/params/fragment.
Test coverage for encoded aliases
tests/mcp/test_http_api.py
Parametrized GET test extended with /rp%63 and /%72pc; new tests verify POST (invalid body rejected before JSON-RPC parsing) and OPTIONS (no allow header, no HTML) return JSON 404 for encoded aliases.
Documentation and plan updates
AGENTS.md, docs/guides/mcp.md, docs/reference/cli.md, docs/plans/rpc-encoded-aliases.md
Documentation clarified to state encoded exact /rpc aliases return structured 404 errors; new plan document details background, goals, tasks, and verification steps.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • Wangmerlyn/KeepGPU#122: Both PRs touch the /rpc route handling in the same server file, one adjusting routing predicates and the other the parse-error response path.
  • Wangmerlyn/KeepGPU#172: Both PRs modify the same noncanonical /rpc routing logic and extend the same test file for "unknown endpoint" behavior.

Poem

  • A rabbit hopped down the URL trail,
  • Found %63 hiding a sly little tale,
  • "You're just /rpc in disguise!" it cried,
  • Slammed the door with a 404 reply,
  • Docs updated, tests all pass — hooray!
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: rejecting encoded RPC aliases.
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
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/rpc-encoded-aliases

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

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

@Wangmerlyn
Wangmerlyn merged commit 2fe2748 into main Jul 1, 2026
6 checks passed
@Wangmerlyn
Wangmerlyn deleted the codex/rpc-encoded-aliases branch July 1, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant