fix(mcp): reject non-ASCII OAuth callback state instead of crashing with 500

[Drexuxux]

What

The dashboard MCP OAuth callback compared the returned state against the expected value via secrets.compare_digest(expected_state, state) on raw str operands. state is an attacker-controllable callback query parameter, and secrets.compare_digest raises TypeError: comparing strings with non-ASCII characters is not supported when either str operand holds a non-ASCII char. A crafted callback like ?state=café&code=x then surfaced as an unhandled 500 on the public callback endpoint instead of the clean "OAuth flow expired" / "state mismatch" rejection.

Same non-ASCII compare_digest class as the api-server bearer (#65305) and webhook-signature (#65307) hardenings — the OAuth dashboard callback state was the remaining unencoded comparison.

Fix

Compare the UTF-8 bytes of both operands at the two callback sites:

• tools/mcp_dashboard_oauth.py — DashboardOAuthFlow.deliver_callback
• hermes_cli/web_server.py — the mcp_oauth_callback route

A non-ASCII state now misses like any other wrong state.

Tests

tests/tools/test_mcp_dashboard_oauth.py — new test_dashboard_flow_rejects_non_ascii_state_without_crashing: a callback with a non-ASCII state is rejected with the clean state mismatch ValueError; before the fix it raised TypeError.

pytest tests/tools/test_mcp_dashboard_oauth.py tests/hermes_cli/test_mcp_dashboard_oauth.py -q
# 23 passed

[tonydwb]

Code Review Summary

Verdict: Approved with Comment

PR #66614 — fix(mcp): reject non-ASCII OAuth callback state instead of crashing with 500

• Validates non-ASCII OAuth callback state and returns proper error instead of 500.
• 51 additions, 2 deletions — security-relevant fix.
• Properly handles error case that could be exploited for DoS.

Suggestions

• Confirm the error response uses an appropriate HTTP status code (4xx, not 500).

---

Reviewed by Hermes Agent

[teknium1]

Thanks for addressing both raw-string comparisons; the premise is confirmed on current main at tools/mcp_dashboard_oauth.py:75 and hermes_cli/web_server.py:11558.

Problems

• The new test exercises only DashboardOAuthFlow.deliver_callback. The public callback's candidate lookup at hermes_cli/web_server.py:11552-1161 performs the first vulnerable comparison, before deliver_callback runs. Existing route coverage at tests/hermes_cli/test_mcp_dashboard_oauth.py:118-143 covers only an ASCII mismatch.

Suggested changes

• Add a TestClient regression for a non-ASCII state on /api/mcp/oauth/callback/reports; assert the existing 404 flow-expired response and that the pending flow remains unconsumed. This verifies the public 500-to-4xx behavior described by the PR.

Automated hermes-sweeper review.