Skip to content

fix(desktop): harden remote MCP OAuth after landing - #582

Merged
vastsa merged 1 commit into
mainfrom
fix/mcp-oauth-review
Sep 18, 2026
Merged

vastsa merged 1 commit into
mainfrom
fix/mcp-oauth-review

Conversation

@vastsa

@vastsa vastsa commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Follow-up to #556. Lands the review hardening without changing the OAuth feature shape.

Fixes

  • TLS: authorization-server endpoints (authorization_endpoint, token_endpoint, registration_endpoint, authorization_servers, resource_metadata) must be HTTPS. Loopback HTTP remains allowed; the MCP resource URL may still be http:// (ADR 0142).
  • DCR ports: register RFC 8252 portless http://127.0.0.1/callback plus the current exact URI; reuse a stored client only when that portless URI is on file or the exact redirect matches. Rebind the previous loopback port when possible.
  • Callback CSRF / replay: validate state before error/code. Mismatched state does not abort the login. A matching callback is consumed once (replay → 409).
  • Cross-project handshake: mcp/oauth/start passes the listed McpServerRecord through to onAuthorized.
  • UI leak: Settings unsubscribes the OAuth IPC listener on unmount.
  • Refresh: invalid_grant deletes the stored secret; 5xx keeps the existing access token. Token-endpoint error bodies stay in main-process logs.

Tests

apps/desktop/test/mcp-oauth.test.mjs — 18/18 passing (TLS, CSRF/replay, DCR reuse vs re-register, refresh 400/5xx, onAuthorized record).

Refs #556.

Force HTTPS on authorization-server endpoints (loopback excepted),
reuse DCR clients only when RFC 8252 portless or exact redirect
matches, and consume loopback callbacks only after a matching state.

Pass the listed server record through to onAuthorized so a
project-level MCP can handshake after login, and unsubscribe the
settings OAuth listener on unmount.

Refs #556.
Copilot AI lite review requested due to automatic review settings September 18, 2026 09:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vastsa
vastsa merged commit 63b64b2 into main Sep 18, 2026
2 of 4 checks passed
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.

2 participants