Skip to content

fix: makes mcp header temp token flow follow the UI toggle - #3836

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
05-28-fix_makes_mcp_header_temp_token_flow_follow_the_ui_toggle
May 28, 2026
Merged

fix: makes mcp header temp token flow follow the UI toggle#3836
Pratham-Mishra04 merged 1 commit into
devfrom
05-28-fix_makes_mcp_header_temp_token_flow_follow_the_ui_toggle

Conversation

@roroghost17

@roroghost17 roroghost17 commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

The mcp_headers provider's tempTokens field was previously guarded by a sync.RWMutex. This PR replaces that mutex with an atomic.Pointer[temptoken.Service] for lock-free reads on the hot request path. Additionally, temp-token embedding in MCP per-user-headers auth links is now gated behind the MCPEnableTempTokenAuth client-config toggle, mirroring the existing OAuth surface behavior so the UI switch controls both per-user auth kinds uniformly.

Changes

  • Replaced sync.RWMutex + *temptoken.Service field with atomic.Pointer[temptoken.Service] in mcp_headers.Provider, enabling lock-free reads during request handling.
  • Added tempTokenService() helper for clean atomic load access.
  • Added mcpTempTokenAuthEnabled() which reads the MCPEnableTempTokenAuth client-config flag, mirroring oauth2.OAuth2Provider.mcpTempTokenAuthEnabled, so the same toggle governs both OAuth and MCP headers auth link generation.
  • InitiateUserSubmissionFlow now checks both that the service is set and that MCPEnableTempTokenAuth is enabled before minting a temp token fragment.
  • Updated flake.lock to latest nixpkgs revision.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./framework/mcp_headers/...
go test ./framework/temptoken/...
go test ./...

Verify that:

  • With MCPEnableTempTokenAuth disabled in client config, InitiateUserSubmissionFlow returns a URL without a temp-token fragment.
  • With MCPEnableTempTokenAuth enabled and a valid temptoken.Service installed, the returned URL includes the auth fragment.
  • No regressions in OAuth per-user auth flow behavior.

Breaking changes

  • Yes
  • No

Security considerations

Temp-token fragments in MCP per-user-headers auth URLs are now explicitly gated behind the MCPEnableTempTokenAuth client-config toggle. This ensures operators have a single, consistent control surface for enabling short-lived token embedding across both OAuth and MCP headers auth flows, reducing the risk of unintended token issuance.

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

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (4)
  • flake.lock is excluded by !**/*.lock and included by none
  • framework/mcp_headers/main.go is excluded by none and included by none
  • framework/mcp_headers/main_test.go is excluded by none and included by none
  • framework/temptoken/scope.go is excluded by none and included by none

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 735169b7-aee0-4498-bc6b-93809137263f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-28-fix_makes_mcp_header_temp_token_flow_follow_the_ui_toggle

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CLAassistant

CLAassistant commented May 28, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@roroghost17
roroghost17 marked this pull request as ready for review May 28, 2026 09:49
@greptile-apps

greptile-apps Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the refactor is mechanical and well-tested, with no behavioral changes beyond the intended toggle gate.

The atomic.Pointer swap is straightforward and matches the existing oauth2 pattern exactly. The new toggle gate is fail-closed, mirrors the OAuth surface faithfully, and is now covered by dedicated tests. No regressions were identified across the changed paths.

No files require special attention.

Important Files Changed

Filename Overview
framework/mcp_headers/main.go Replaces sync.RWMutex+pointer with atomic.Pointer[temptoken.Service] and adds mcpTempTokenAuthEnabled gate, correctly mirroring oauth2.OAuth2Provider
framework/mcp_headers/main_test.go New test file covering mcpTempTokenAuthEnabled, toggle-gated URL fragment, nil-service, and config-read-error (fail-closed) cases
framework/temptoken/scope.go Removes inline file-path reference from a comment; no functional change
flake.lock Routine nixpkgs revision bump; no code impact

Reviews (2): Last reviewed commit: "fix: makes mcp header temp token flow fo..." | Re-trigger Greptile

Comment thread framework/mcp_headers/main.go
@roroghost17
roroghost17 force-pushed the 05-28-fix_makes_mcp_header_temp_token_flow_follow_the_ui_toggle branch from 90f8036 to 20e0caa Compare May 28, 2026 09:57

Pratham-Mishra04 commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Merge activity

  • May 28, 10:01 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 28, 10:02 AM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 merged commit 62298db into dev May 28, 2026
14 of 15 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 05-28-fix_makes_mcp_header_temp_token_flow_follow_the_ui_toggle branch May 28, 2026 10:02
akshaydeo pushed a commit that referenced this pull request May 29, 2026
## Summary

The `mcp_headers` provider's `tempTokens` field was previously guarded by a `sync.RWMutex`. This PR replaces that mutex with an `atomic.Pointer[temptoken.Service]` for lock-free reads on the hot request path. Additionally, temp-token embedding in MCP per-user-headers auth links is now gated behind the `MCPEnableTempTokenAuth` client-config toggle, mirroring the existing OAuth surface behavior so the UI switch controls both per-user auth kinds uniformly.

## Changes

- Replaced `sync.RWMutex` + `*temptoken.Service` field with `atomic.Pointer[temptoken.Service]` in `mcp_headers.Provider`, enabling lock-free reads during request handling.
- Added `tempTokenService()` helper for clean atomic load access.
- Added `mcpTempTokenAuthEnabled()` which reads the `MCPEnableTempTokenAuth` client-config flag, mirroring `oauth2.OAuth2Provider.mcpTempTokenAuthEnabled`, so the same toggle governs both OAuth and MCP headers auth link generation.
- `InitiateUserSubmissionFlow` now checks both that the service is set and that `MCPEnableTempTokenAuth` is enabled before minting a temp token fragment.
- Updated `flake.lock` to latest nixpkgs revision.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./framework/mcp_headers/...
go test ./framework/temptoken/...
go test ./...
```

Verify that:
- With `MCPEnableTempTokenAuth` disabled in client config, `InitiateUserSubmissionFlow` returns a URL without a temp-token fragment.
- With `MCPEnableTempTokenAuth` enabled and a valid `temptoken.Service` installed, the returned URL includes the auth fragment.
- No regressions in OAuth per-user auth flow behavior.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

Temp-token fragments in MCP per-user-headers auth URLs are now explicitly gated behind the `MCPEnableTempTokenAuth` client-config toggle. This ensures operators have a single, consistent control surface for enabling short-lived token embedding across both OAuth and MCP headers auth flows, reducing the risk of unintended token issuance.

## 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
@akshaydeo akshaydeo mentioned this pull request May 29, 2026
18 tasks
akshaydeo added a commit that referenced this pull request May 29, 2026
## 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
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.

3 participants