fix: gate user-mode flows on caller user_id and skip temp token mint - #3841
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
|
Confidence Score: 5/5Safe to merge — the change is targeted security hardening with no functional regressions on existing non-user-mode paths. All five changed surfaces consistently apply the identity gate. The temp-token skip is symmetric across both framework modules and correctly conditioned on MCPAuthModeUser only, leaving VK and session-mode behavior untouched. The UI goto redirect is encoded and validated against a /workspace allowlist before use. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "feat: user bound to checks on mcp per us..." | Re-trigger Greptile |
ffdd416 to
7d4e3b4
Compare
Merge activity
|
…3841) ## Summary User-mode MCP auth flows were not properly scoped to the user who initiated them. Any authenticated SSO user holding a user-mode auth URL could view, start, or submit a flow bound to a different user, potentially planting their upstream credentials under that user's identity. This PR adds an identity gate to all user-mode flow endpoints and fixes the temp token minting logic that was causing legitimate users to receive 403s on their own flows. ## Changes - Added `canAccessUserFlow` helper that enforces caller `user_id` must match `flow.UserID` for user-mode flows; VK and session-mode flows remain intentionally shareable and pass through unchanged. - Applied the identity gate to `flowDetail`, `flowSubmit`, and `flowStart` in both the OAuth sessions handler and the per-user headers handler, returning a 403 with a descriptive message when the check fails. - Skipped temp token minting for user-mode flows in both `InitiateUserSubmissionFlow` and `InitiateUserOAuthFlow`. Minting a temp token routes requests through the temp-token middleware branch, which bypasses cookie resolution and leaves the caller `user_id` empty, causing the new identity gate to 403 even legitimate users. - Updated the UI's 401 redirect to preserve the current path and query string as a `goto` parameter so users are returned to their original destination after logging in. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Create a user-mode MCP auth flow as User A and copy the auth URL. 2. Log in as User B and navigate to that URL — expect a 403 with `"This authentication link is bound to a different user."`. 3. Log in as User A and navigate to the same URL — expect normal flow behavior. 4. Verify VK-mode and session-mode flow URLs remain accessible across users. 5. Trigger a 401 response in the UI while on a non-login page and confirm the redirect lands on `/login?goto=<original-path>` and returns you there after login. ```sh go test ./framework/... ./transports/... cd ui pnpm i pnpm build ``` ## Breaking changes - [x] No ## Security considerations Without this gate, any authenticated user holding a user-mode auth URL could complete a flow bound to a different user, injecting their upstream OAuth tokens or header credentials under that user's identity. The `canAccessUserFlow` check mirrors the post-claim gate already present in `CompleteUserOAuthFlow` and closes the same class of vulnerability on the flow detail, start, and submit endpoints. Temp token minting is intentionally skipped for user-mode flows to prevent the middleware bypass that would nullify the gate. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary This PR releases **core v1.5.14**, **framework v1.3.14**, **transports v1.5.6**, and bumps all dependent plugins to their respective `.14` patch versions. It delivers a broad set of new capabilities across MCP authentication, key rotation, OTel metrics, Bedrock/Anthropic compatibility, and UI improvements, alongside a number of targeted bug fixes and refactors. ## Changes - **Direct API Key Header** — Providers can now receive an API key passed directly via a request header (#3817) - **MCP Per-User Auth** — Introduced `MCPCredentialStore` abstraction, per-user MCP credential reconciliation, and a new per-user header auth type with lazy-auth submission flow (#3656, #3702, #3703, #3704, #3705) - **MCP TLS Configuration** — Added configurable TLS (`insecureSkipVerify`, `caCertPem`) for HTTP/SSE MCP client connections (#3779, #3783) - **MCP Sessions Management** — Filter, search, and pagination on the MCP sessions list API and table, plus a `can_reauth` identity gate (#3823, #3824, #3825) - **Key Rotation** — Keys now rotate on 401/402/403 responses; returns `502 upstream_credentials_exhausted` when all keys are permanently exhausted. Added `triggered_rotation` to `KeyAttemptRecord` and tightened `bifrost_key_rotation_events_total` semantics (#3430, #3491) - **OTel Metrics** — Added OTel spec-compatible metrics (backward compatible) with provider cache and semantic cache attributes in metrics export (#3865, #3816) - **Opus 4.8 Support** — System message handling and general compatibility for Opus 4.8 (#3868, #3878) - **Dimension Rankings** — New `GetDimensionRankings` API and dashboard tabs for team, customer, BU, and user rankings (#3766) - **Model Pricing Attributes** — `additional_attributes` field on model pricing rows with management API and UI editor (#3829) - **Prompt Cache Retention** — Added prompt cache retention parameter on responses requests (#3810) - **Tool Call Execution UI** — Inline tool-call execution, stop streaming, bulk execute/submit, and a redesigned tool-call UI (#3837, #3843) - **Sheet Navigation** — Prev/next keyboard navigation and URL state across virtual key, MCP client, and routing rule sheets (#3739, #3740, #3744, #3745) - **Bedrock Tool Name Truncation** — Truncate Bedrock function/tool names to the provider length limit - **Bedrock Guardrails** — Set guardrail config in Bedrock requests built from responses (#3862) - **Anthropic Tool Use** — Default `tool_use` input to `{}` when arguments are absent (#3880) - **Responses Streaming** — Fixed responses stream events (#3838) - **Compat Flow** — Fixed missing parameter parsing on the compat flow (#3881) - **Passthrough API Version** — Set a default API version in passthrough requests as a fallback (#3853) - **Virtual Key Updates** — Avoid overriding optional fields during virtual key update (#3855) - **User-Mode Flows** — Gate user-mode flows on caller `user_id`, skip temp token mint, and unify flow/credential kind filtering for pending flows (#3841, #3859) - **Partial Tool Calls** — Handle partial tool call execution failures and return successful results (#3849) - **URL Query Escaping** — Support escaped characters in URL query parameters (#3826) - **MCP Auth Errors** — Inline banner and retry support for MCP auth-required errors (#3856) - **Renamed Resolvers** — `staticHeadersResolver`/`serverOAuthResolver` renamed to `sharedHeadersResolver`/`sharedOAuthResolver` (#3840) - **Starlark Nested Tool Calls** — Exposed `RunWithPluginPipeline` on `ClientManager` and routed Starlark nested tool calls through the canonical plugin gate (#3794) - **Deferred-Fill OAuth Removed** — Removed deferred-fill user-mode OAuth flow support (#3839) - **Go 1.26.3** — Upgraded toolchain to Go 1.26.3 (#3782) ## Type of change - [x] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version # should report go1.26.3 go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` - Validate MCP per-user auth by configuring a per-user header auth type and confirming credentials are stored and reconciled on virtual key and MCP client changes. - Validate key rotation by triggering a 401/402/403 from an upstream provider and confirming rotation occurs; exhaust all keys and confirm a `502 upstream_credentials_exhausted` is returned. - Validate OTel metrics output includes `provider_cache` and `semantic_cache` attributes. - Validate Bedrock requests with tool names exceeding the provider limit are truncated correctly. - Validate Opus 4.8 system message handling by sending a request with a system message to an Opus 4.8 endpoint. ## Breaking changes - [x] Yes - [ ] No The deferred-fill user-mode OAuth flow has been removed (#3839). Any integrations relying on that flow must migrate to the new per-user credential store approach. The `staticHeadersResolver` and `serverOAuthResolver` identifiers have been renamed to `sharedHeadersResolver` and `sharedOAuthResolver` respectively (#3840); any direct references must be updated. ## Related issues #3817, #3656, #3702, #3703, #3704, #3705, #3779, #3783, #3823, #3824, #3825, #3430, #3491, #3865, #3816, #3868, #3878, #3766, #3829, #3810, #3837, #3843, #3739, #3740, #3744, #3745, #3862, #3880, #3838, #3881, #3853, #3855, #3841, #3859, #3849, #3826, #3856, #3840, #3794, #3839, #3782, #3724, #3814, #3836, #3869, #3886 ## Security considerations - MCP per-user credentials are stored via the new `MCPCredentialStore` abstraction; ensure the backing store is appropriately access-controlled and that credential values are encrypted at rest. - The direct API key header feature passes provider secrets via HTTP headers; ensure TLS is enforced on all ingress paths and that headers are not logged in plaintext. - User-mode flows are now gated on `caller user_id` and temp token minting is skipped where appropriate, reducing the surface for privilege escalation. - TLS configuration for MCP HTTP/SSE connections supports `insecureSkipVerify`; this should only be enabled in controlled environments. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
## ✨ Features - **Direct API Key Header** - Pass a provider API key directly via request header (#3817) - **MCP Per-User Authentication** - New per-user header auth type with credential storage and lazy-auth submission flow (#3703, #3704, #3705) - **MCP TLS Configuration** - Configurable TLS (insecureSkipVerify, caCertPem) for HTTP/SSE MCP client connections (#3779, #3783) - **MCP Sessions Management** - Filter, search, and pagination on the MCP sessions list API and table, plus a can_reauth identity gate (#3823, #3824, #3825) - **Tool Call Execution UI** - Inline tool-call execution, stop streaming, bulk execute/submit, and a redesigned tool-call UI (#3837, #3843) - **Dimension Rankings Dashboard** - New dashboard tabs for team, customer, BU, and user rankings, backed by a GetDimensionRankings API (#3766) - **Model Pricing Attributes** - additional_attributes on model pricing rows with management API and UI editor (#3829) - **Prompt Cache Retention** - Prompt cache retention parameter on responses requests (#3810) - **Opus 4.8 Support** - System message handling and compatibility for Opus 4.8 (#3878, #3868) - **Key Rotation** - Rotate keys on 401/402/403 and return 502 upstream_credentials_exhausted when all keys are permanently dead (#3491) - **OTel Metrics** - OTel spec compatible metrics plus provider and semantic cache attributes in metrics export (#3865, #3816) - **Sheet Navigation** - Prev/next keyboard navigation and URL state across virtual key, MCP client, and routing rule sheets (#3739, #3740, #3744, #3745) - **Go 1.26.3** - Upgraded toolchain to Go 1.26.3 (#3782) ## 🐞 Fixed - **Bedrock Tool Names** - Truncate Bedrock function/tool names to the provider length limit - **Bedrock Guardrails** - Set guardrail config in Bedrock request built from responses (#3862) - **Anthropic Tool Use** - Default Anthropic tool_use input to {} when arguments are absent (#3880) - **Responses Streaming** - Fixed responses stream events (#3838) - **Compat Flow** - Fixed missing parameter parsing on the compat flow (#3881) - **Passthrough API Version** - Set a default API version in passthrough requests as a fallback (#3853) - **Virtual Key Updates** - Avoid overriding optional fields during virtual key update (#3855) - **User-Mode Flows** - Gate user-mode flows on caller user_id, skip temp token mint, and unify flow/credential kind filtering for pending flows (#3841, #3859) - **Partial Tool Calls** - Handle partial tool call execution failures and return successful results (#3849) - **URL Query Escaping** - Support escaped characters in URL query parameters (#3826) - **MCP Auth Errors** - Inline banner and retry support for MCP auth-required errors (#3856) - **JSON Editor Height** - Cap JSON editor max height at 400px in message views (#3842)

Summary
User-mode MCP auth flows were not properly scoped to the user who initiated them. Any authenticated SSO user holding a user-mode auth URL could view, start, or submit a flow bound to a different user, potentially planting their upstream credentials under that user's identity. This PR adds an identity gate to all user-mode flow endpoints and fixes the temp token minting logic that was causing legitimate users to receive 403s on their own flows.
Changes
canAccessUserFlowhelper that enforces calleruser_idmust matchflow.UserIDfor user-mode flows; VK and session-mode flows remain intentionally shareable and pass through unchanged.flowDetail,flowSubmit, andflowStartin both the OAuth sessions handler and the per-user headers handler, returning a 403 with a descriptive message when the check fails.InitiateUserSubmissionFlowandInitiateUserOAuthFlow. Minting a temp token routes requests through the temp-token middleware branch, which bypasses cookie resolution and leaves the calleruser_idempty, causing the new identity gate to 403 even legitimate users.gotoparameter so users are returned to their original destination after logging in.Type of change
Affected areas
How to test
"This authentication link is bound to a different user."./login?goto=<original-path>and returns you there after login.Breaking changes
Security considerations
Without this gate, any authenticated user holding a user-mode auth URL could complete a flow bound to a different user, injecting their upstream OAuth tokens or header credentials under that user's identity. The
canAccessUserFlowcheck mirrors the post-claim gate already present inCompleteUserOAuthFlowand closes the same class of vulnerability on the flow detail, start, and submit endpoints. Temp token minting is intentionally skipped for user-mode flows to prevent the middleware bypass that would nullify the gate.Checklist
docs/contributing/README.mdand followed the guidelines