feat(cua-driver): add experimental MCP OAuth front door - #2063
Open
outdog-hwh wants to merge 2 commits into
Open
feat(cua-driver): add experimental MCP OAuth front door#2063outdog-hwh wants to merge 2 commits into
outdog-hwh wants to merge 2 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
@outdog-hwh is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds an experimental ChangesExperimental OAuth connector bridge
Sequence Diagram(s)sequenceDiagram
participant OAuthClient
participant mcp_oauth as cua-driver mcp-oauth
participant JSONStore as JSON-backed store
participant LoopbackMCPUpstream as Loopback MCP upstream
OAuthClient->>mcp_oauth: GET /authorize
mcp_oauth->>JSONStore: store pending consent / code
OAuthClient->>mcp_oauth: POST /token
mcp_oauth->>JSONStore: redeem authorization code
OAuthClient->>mcp_oauth: POST /mcp with Bearer token
mcp_oauth->>LoopbackMCPUpstream: forward MCP request
LoopbackMCPUpstream-->>mcp_oauth: upstream response
mcp_oauth-->>OAuthClient: proxied response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/cua-driver/rust/crates/cua-driver/src/mcp_oauth.rs`:
- Around line 631-640: The denial branch in handle_authorize_post currently
redirects using form-provided redirect_uri and state before validating the
consent transaction, which leaves the /authorize flow open to redirect abuse.
Update the nonce/pending lookup to happen before any denial redirect, require a
valid pending consent transaction, and use pending.redirect_uri plus
pending.state for the redirect instead of values from the submitted form; also
ensure the registered redirect URI is validated before constructing the Location
header.
- Around line 1092-1097: The read_upstream_body helper currently checks
MAX_HTTP_BODY_BYTES only after read_to_end finishes, so it can allocate too much
before rejecting. Update read_upstream_body to read through a limited reader
capped at MAX_HTTP_BODY_BYTES + 1, and fail as soon as the cap is exceeded; keep
the change localized to this function and preserve the existing timeout/error
handling.
- Around line 958-978: The buffered `tools_list` handling in `mcp_oauth.rs` is
treating chunked upstream bodies as plain bytes, so
`rewrite_tools_list_response`, `proxy_http_response`, and the
`is_tools_list_request` branch can forward chunk-framed data with a mismatched
`Content-Length`. Update the `read_upstream_body`/`proxy_http_response` flow to
either decode chunked `tools/list` responses before rewriting or bypass
buffering and stream them while preserving the original framing, and apply the
same fix in the later `tools/list` path referenced by the comment.
- Around line 352-360: The TTL checks in the OAuth token/consent cleanup path
can overflow when adding expiration offsets, so update the arithmetic in
prune_tokens and prune_consents to use saturating addition instead of direct +
with now_secs(), code_ttl_seconds, or token TTL values. Also apply the same
saturating TTL handling in the related token/consent creation logic in
mcp_oauth.rs so all expiration calculations remain safe for near-u64::MAX
values.
- Around line 1585-1587: The OAuth persistence helpers currently only create the
state directory, but the directory and temp files still inherit default
permissions, so private data can be exposed. Update ensure_private_dir and the
temp-file write/rename flow in mcp_oauth.rs so the OAuth state directory is
created with restrictive access and the temporary files used for client secrets,
authorization codes, and access tokens are explicitly locked down before being
renamed. Make the fix in the same helpers that handle the fs::create_dir_all and
fs::write paths, and ensure the final on-disk files remain private after the
rename.
- Around line 294-431: The store mutation paths in mcp_oauth.rs are performing
load/modify/write without any shared process-local synchronization, which allows
concurrent calls to race and lose updates or redeem the same authorization code
more than once. Add a mutex-protected critical section around every
read-modify-write helper, especially `redeem_code_for_token`, and apply the same
locking to `upsert_client`, `upsert_code`, `upsert_token`, `upsert_consent`, and
`consume_consent` so the loaded map, mutation, and `write_json_map` happen
atomically.
- Around line 593-605: The registration and token responses in mcp_oauth should
not be cacheable because they include secrets and bearer tokens. Update the
response-building paths that call json_response for client registration and the
/token flow so they also set Cache-Control: no-store and Pragma: no-cache on the
HTTP response. Use the existing registration/token handler logic in mcp_oauth.rs
to apply these headers consistently wherever client_secret or access_token is
returned.
- Around line 1686-1688: The current is_loopback_host helper is too permissive
because host.starts_with("127.") matches non-literal hostnames that can resolve
off-loopback. Update is_loopback_host in mcp_oauth.rs to allow only exact
"localhost" (case-insensitive) and parsed IP literals whose
IpAddr::is_loopback() returns true, so the loopback redirect check cannot be
bypassed by crafted DNS names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb0acae3-b892-450d-8e0f-427a38c4bb81
⛔ Files ignored due to path filters (1)
libs/cua-driver/rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
libs/cua-driver/README.mdlibs/cua-driver/rust/crates/cua-driver/Cargo.tomllibs/cua-driver/rust/crates/cua-driver/src/cli.rslibs/cua-driver/rust/crates/cua-driver/src/main.rslibs/cua-driver/rust/crates/cua-driver/src/mcp_oauth.rs
Contributor
Author
|
Follow-up for the CodeRabbit review comments, addressed in |
f-trycua
requested review from
ddupont808,
f-trycua and
r33drichards
as code owners
July 16, 2026 11:21
This was referenced Aug 16, 2026
Collaborator
|
We moved the architecture decision into RFC #3197 with draft document PR #3198. The proposed boundary keeps Cua Driver transport-neutral and places OAuth, DCR, TLS, tunnels, and provider-specific adapters in a separate optional gateway. This PR remains linked as the design and implementation exploration, and the RFC preserves @outdog-hwh’s credit. We should decide the RFC before continuing the in-core implementation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Adds an experimental
cua-driver mcp-oauthsubcommand that serves as an OAuth/DCR front door for an existing local MCP HTTP endpoint.The new entry point:
<public-url>/mcp/mcprequests to a loopback-only upstream such ashttp://127.0.0.1:7677/mcptools/listresponsestools/listdescriptor view while leavingtools/calland other MCP JSON-RPC methods untouchedThis does not change the existing
cua-driver mcp,serve, ormcp_httpbehavior.Why
Some OAuth-only MCP clients require OAuth discovery and Dynamic Client Registration before they will call an MCP endpoint. This keeps that compatibility layer opt-in and isolated from the existing local stdio/HTTP MCP transports.
Safety notes
--public-urlmust be HTTPS.--mcp-upstreamis restricted to loopback HTTP hosts.https://your-tunnel.example/mcp.Testing
cargo check --offline -p cua-drivercargo test --offline -p cua-driver mcp_oauth -- --nocapturecargo test --offline -p cua-driver parse_positive_u64_accepts_positive_and_rejects_zero_or_invalid -- --nocapturecargo test --offline -p cua-driver manifest_has_documented_top_level_shape -- --nocapturerustfmt --edition 2021 --check crates/cua-driver/src/mcp_oauth.rsgit diff --checkManual acceptance:
127.0.0.1:7677.mcp-oauththrough a Tailscale HTTPS URL./mcpURL.Summary by CodeRabbit
New Features
cua-driver, enabling an opt-in HTTPS front door for MCP HTTP access.Documentation