feat(mcp): add sandbox-safe OAuth and failure-aware routing / 添加沙箱安全的 MCP OAuth 与失败路由 - #7932
Merged
SivanCola merged 12 commits intoAug 8, 2026
Merged
Conversation
SivanCola
force-pushed
the
feature/mcp-oauth-sandbox-auth
branch
from
August 8, 2026 05:16
1be2309 to
b1783f0
Compare
Problem: - Remote MCP OAuth depended on externally injected credentials that sandboxed desktop and CLI sessions could not access. - Failed cached MCP tools could still be preferred by capability routing. Root cause: - Reasonix had no first-party user-initiated OAuth flow or private credential state. - Cached tool entries did not inherit failed or disabled server status. Fix: - Add standards-based OAuth discovery, DCR, PKCE, refresh rotation, proxy-aware HTTP, and resource-bound private state. - Serialize token rotation across processes and prevent stale transports from restoring cleared state. - Add CLI, TUI, and Desktop sign-in and reconnect flows. - Propagate failed and disabled lifecycle state to cached MCP tools. Verification: - go test -race ./internal/plugin ./internal/capability ./internal/boot ./internal/cli - go test ./... - (cd desktop && go test ./...) - (cd desktop/frontend && pnpm exec tsx src/__tests__/capabilities-panel-actions.test.ts) - (cd desktop/frontend && pnpm typecheck) - (cd desktop/frontend && pnpm lint:hooks) - go vet ./... - git diff --check Co-authored-by: andrei-kiparuk <88461327+andrei-kiparuk@users.noreply.github.com>
SivanCola
force-pushed
the
feature/mcp-oauth-sandbox-auth
branch
from
August 8, 2026 05:21
b1783f0 to
1c30748
Compare
Problem: - Failed or disabled cached MCP tools could remain ready after catalog deduplication. - stdio, legacy SSE, and statically authenticated servers could enter the HTTP OAuth flow. - Removing an MCP declaration retained Reasonix OAuth client and token state. Root cause: - Registry-backed MCP tool entries did not inherit host failure state. - Authentication diagnosis and UI actions trusted auth failures before checking transport and configured credentials. - Remove paths did not reconcile private OAuth state with the effective fallback declaration. Fix: - Propagate failed and disabled status through the real catalog assembly path. - Gate native OAuth consistently in diagnostics, CLI, TUI, Desktop, frontend, and the plugin owner. - Reconcile OAuth state under the cross-process lock, preserving only same-resource fallbacks and covering open Desktop workspaces. Verification: - go test ./... - go test -race ./internal/plugin ./internal/capability ./internal/mcpdiag ./internal/cli ./internal/control - cd desktop && go test ./... - cd desktop && go test -race ./... - frontend focused test, typecheck, ESLint, and CSS checks - go vet ./... - go run ./tools/repolint - git diff --check
Problem: - The Desktop Linux and macOS CI jobs rejected two test fixtures with the fmtappendf modernize rule. Root cause: - The fixtures converted fmt.Sprintf output with []byte instead of appending formatted bytes directly. Fix: - Build the OAuth JSON fixtures with fmt.Appendf. Verification: - cd desktop && go test ./... -run TestRemoveMCPServerReconcilesOAuthState -count=1 - cd desktop && golangci-lint run --timeout=5m ./... - go run ./tools/repolint - git diff --check
Problem: - OAuth authorization held the MCP state lock across browser interaction and could race with removal. - Existing OAuth state could be loaded alongside static MCP credentials. Root cause: - The authorization flow used one lock scope for the entire interactive exchange and had no invalidation generation. - HTTP transport only excluded OAuth when an Authorization header was present. Fix: - Capture and compare a private OAuth generation around interactive authorization, and advance it during removal reconciliation. - Treat all explicit MCP authentication configuration as authoritative when constructing HTTP transports. - Add deterministic regression coverage for browser-lock release, removal invalidation, fallback reconciliation, and static API keys. Verification: - go test -race ./internal/plugin ./internal/mcpdiag - go test ./internal/cli ./internal/control ./internal/capability - go test ./... - cd desktop && go test ./... - git diff --check
Problem: - Explicit URL and header credentials could be combined with stale OAuth state. - Remote HTTP endpoints and failed semantic IDs could be offered as usable capabilities. - Token refresh held the cross-process state lock during network I/O. Root cause: - OAuth eligibility used incomplete string heuristics and refresh used one lock scope for read, request, and write. Fix: - Parse URL userinfo/query credentials and sensitive headers, restrict OAuth to HTTPS or loopback HTTP, and filter failed/disabled semantic candidates. - Add a process-local refresh gate plus two-phase locked compare-and-apply with generation checks. - Add regression coverage for credentials, transport eligibility, routing status, and clear-during-refresh. Verification: - go test -race ./internal/plugin ./internal/capability ./internal/mcpdiag ./internal/control ./internal/cli - go vet ./... - go run ./tools/repolint - pnpm exec tsx src/__tests__/mcp-oauth-eligibility.test.ts - pnpm typecheck Co-authored-by: Andrei Kiparuk <88461327+andrei-kiparuk@users.noreply.github.com>
Problem: Full-suite macOS sandbox probes could time out while desktop filesystem tests could fail during temporary-directory teardown with EBADF. Root cause: The sandbox tests depended on process-global PATH and a two-second shell probe, while atomic file paths used repeated close paths and affected desktop tests used fragile t.TempDir cleanup. Fix: Probe sandbox-exec by explicit path with an absolute probe command, avoid caching transient timeouts, close temporary files exactly once, and use robustTempDir for the affected desktop filesystem tests. Verification: Focused tests and race tests pass; root go test ./..., desktop go test ./..., go vet ./... in both modules, and repolint pass.
Problem: The PR merge commit exceeded the main-v2 controller file-size budget by one line, blocking the repository standards job. Root cause: The latest main-v2 reduced the carried-forward budget while this branch retained a two-line dispatcher comment. Fix: Keep the same dispatcher documentation in one line without changing behavior. Verification: `go run ./tools/repolint` and `go test ./internal/control` pass.
Resolve the latest main-v2 controller/runtime changes in favor of the current base implementation while retaining the MCP OAuth and sandbox fixes.
Problem: Controller.RemoveMCPServer left the OAuth removal reconciler unused after the main-v2 merge, so the controller path could retain stale Reasonix OAuth state and failed lint. Root cause: The merge kept the pre-OAuth fallback lookup and nil cleanup return instead of consuming the reconciled post-removal configuration. Fix: Reconcile OAuth state immediately after persistence removal, restore the resolved fallback from that state, and return cleanup errors to the caller while preserving the lint budget. Verification: go test ./internal/control ./internal/cli; golangci-lint run --timeout=5m; go run ./tools/repolint; git diff --check.
Problem: The first lint fix removed a function separator to stay within the controller file budget, which made the file fail the CI gofmt check. Root cause: The file-size budget was reduced by a comment change without restoring gofmt-required spacing. Fix: Restore gofmt output and shorten the adjacent MCP removal documentation by one line, preserving the exact removal and OAuth reconciliation behavior. Verification: gofmt -l internal/control/controller.go; go test ./internal/control ./internal/cli; golangci-lint run --timeout=5m; go run ./tools/repolint; git diff --check.
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
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.
Summary
Add first-party, user-initiated OAuth authorization for remote Streamable HTTP MCP servers, with private Reasonix-owned state that works from sandboxed Desktop and CLI sessions. Also prevent failed or disabled cached MCP tools from being preferred by capability routing.
Problem
Users could configure remote MCP servers such as Figma, but Desktop and CLI sessions could not reuse credentials stored by another client or keychain path. The resulting authentication failure was presented as a sandbox limitation, while cached tools from the failed server could still be selected by capability routing.
Root cause
Changes
User flow
Security
Compatibility
Cache and documentation impact
Cache-impact: none - OAuth clients, token state, and failure lifecycle are runtime-only; MCP tool names, schemas, descriptions, registration order, and provider-visible prompt prefixes are unchanged.
Cache-guard: go test ./internal/plugin ./internal/capability verifies schema-cache isolation, runtime identity, cached failure propagation, and failed-route exclusion.
System-prompt-review: self-reviewed - internal/boot changes only inject a host-local proxy-aware OAuth client and do not change system-prompt assembly or provider-visible schemas.
Documentation-impact: updated - English and Chinese GUIDE and SPEC now document the OAuth sign-in flow, proxy behavior, private state, static-header precedence, and clear-authentication boundary.
Failed or disabled cached tools remain in the catalog for diagnostics but are no longer routing candidates.
Verification
Consolidation of #6759
This PR is an integration and superseding implementation based on current main-v2.
Kept or adapted from #6759 by @andrei-kiparuk:
Reviewed but not adopted:
The narrower design keeps user interaction explicit, binds credentials to the full MCP resource and workspace/server private state, preserves existing static headers, fails closed for damaged state, and avoids expanding the cryptographic and configuration surface.
Refs #6759
Contributors