[fix]: Helm chart - wire up MCP OAuth config and token_exchange auth - #6060
Closed
CallumWayve wants to merge 119 commits into
Closed
[fix]: Helm chart - wire up MCP OAuth config and token_exchange auth#6060CallumWayve wants to merge 119 commits into
CallumWayve wants to merge 119 commits into
Conversation
…iew maintenance (maximhq#5693) * feat: support matview_refresh_interval "off" to disable logstore matview maintenance The materialized views back only the dashboard UI. Deployments that run Bifrost headless behind their own observability stack pay the REFRESH MATERIALIZED VIEW CONCURRENTLY cost for views nothing reads, and the 5s floor means the interval alone cannot turn maintenance off. With "off" (or a non-positive duration) the logs store skips view creation, the initial refresh, and the periodic refresher entirely. matViewsReady stays false, so dashboard queries fall back to the raw tables, and the runtime self-heal path cannot re-arm maintenance since it only triggers from matview-path queries. * fix: guard matview self-heal when maintenance is disabled Review follow-up: carry the resolved disabled state onto the store so triggerMatViewSelfHeal cannot recreate views the configuration says must not exist, and make the schema/docs explicit that a zero duration also disables (positive sub-5s values still clamp up).
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Bug Fixes** * Improved token parameter compatibility handling to preserve alternative formats when the primary option is unsupported. * **Chores** * Version updated to 2.0.0. * Enhanced load testing configuration for more reliable builds. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Briefly explain the purpose of this PR and the problem it solves. - What was changed and why - Any notable design decisions or trade-offs - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs Describe the steps to validate this change. Include commands and expected outcomes. ```sh go version go test ./... cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. If UI changes, add before/after screenshots or short clips. - [ ] Yes - [ ] No If yes, describe impact and migration instructions. Link related issues and discussions. Example: Closes maximhq#123 Note any security implications (auth, secrets, PII, sandboxing, etc.). - [ ] 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 Adds a `THIRD_PARTY_NOTICES.md` file to formally document third-party components used in Bifrost that carry license terms requiring explicit attribution — specifically MPL-2.0 licensed dependencies and embedded source code derived from external projects. ## Changes - Introduces `THIRD_PARTY_NOTICES.md` to attribute: - Embedded source code in `framework/migrator/migrator.go` derived from `go-gormigrate/gormigrate` (MIT) - Go binary dependencies carrying MPL-2.0 terms: `github.com/cyphar/filepath-securejoin` and `github.com/hashicorp/go-version` - npm build-time devDependencies carrying MPL-2.0 terms: `lightningcss` (never shipped to end users) and `dompurify` (Apache-2.0 option elected) - All MPL-2.0 components are used unmodified and combined as a "Larger Work" per MPL-2.0 Section 3.3; no Bifrost source files are themselves MPL-licensed. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test No functional changes — review the file contents to confirm accuracy of license attributions against the listed upstream repositories. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations This change has no security implications. It is a legal/compliance attribution document only. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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 Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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 Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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
…aximhq#5759) ## Summary Closes a race-condition security gap where an unauthenticated network caller could reach a freshly deployed, not-yet-configured Bifrost instance and create the first admin account before the real operator does. Previously, `PUT /api/config` was intentionally open when no admin account existed (zero-config UX), but this left a window of exposure on any publicly reachable host. The fix introduces a one-time **setup token** — generated in-memory at startup when no admin account is configured, printed to the server's startup logs, and required alongside the username/password when creating the first admin account. The token is never persisted, is regenerated on every restart until an admin account exists, and is permanently invalidated once the first admin account is created. ## Changes - **Bootstrap token generation (`middlewares.go`):** `InitAuthMiddleware` generates a UUID setup token via `atomic.Pointer[string]` when no admin account is configured, logs it prominently to stdout, and exposes `CheckBootstrapToken` (constant-time comparison) and `ClearBootstrapToken` methods. - **Token validation in the config handler (`config.go`):** `updateConfig` now calls `ValidateSetupToken` before allowing the first admin account to be created. Returns HTTP 403 if the token is missing or wrong. - **Token cleared on first admin account creation (`server.go`):** `UpdateAuthConfig` calls `ClearBootstrapToken` after successfully persisting the first admin account, permanently closing the gate. - **`setup_token`** **field added to** **`UpdateConfigRequest`:** The field is accepted in the request body but never persisted or returned by `GET /api/config`. - **UI (`securityView.tsx`):** When no `auth_config` exists server-side (`isFirstTimeSetup`), a **Setup token** input field is shown below the password field. The token is validated client-side before submission and cleared from state after a successful save. - **TypeScript types (`config.ts`):** `setup_token?: string` added to `BifrostConfig`. - **OpenAPI schema (`config.yaml`):** `setup_token` documented on `UpdateConfigRequest`. - **Docs:** A `<Warning>` block added to `security-best-practices.mdx` and a `<Note>` added to `setting-up-auth.mdx` explaining the setup token flow, where to find it, and that it only applies once. - **Tests (`middlewares_test.go`):** Two new test cases cover the no-token-generated (pass-through) case and the validate-then-clear lifecycle. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [x] Docs ## How to test **Manual flow:** 1. Start a fresh Bifrost instance with no existing admin account. 2. Check startup logs for the block beginning `No admin account is configured for this Bifrost instance yet.` and copy the setup token. 3. Open the dashboard → Security Settings. Confirm the **Setup token** field appears below the password field. 4. Attempt to save with auth enabled but without the setup token — expect a toast error. 5. Paste the correct token and save — expect success and the Setup token field to disappear on reload. 6. Confirm that `PUT /api/config` without the token returns HTTP 403 while no admin account exists. 7. Restart the server before completing setup and confirm a new token is printed. ```sh # Core/Transports go test ./transports/bifrost-http/handlers/... # UI cd ui pnpm i pnpm build ``` ## Breaking changes - [x] Yes - [ ] No Any automation or scripts that call `PUT /api/config` to create the first admin account on a fresh instance must now include `setup_token` in the request body. The token is available in the server's startup logs. Instances that already have an admin account configured are unaffected — the field is ignored once an admin account exists. ## Security considerations - The setup token is generated with `uuid.NewString()` (crypto-random UUID), stored only in process memory, and compared with `crypto/subtle.ConstantTimeCompare` to prevent timing attacks. - The token is never written to disk, never returned by any API endpoint, and is permanently invalidated after first use. - Operators must have access to the process's stdout/log stream (`docker logs`, `kubectl logs`, or terminal) to retrieve the token, which is the same access level required to operate the host — this is the intended trust boundary. ## 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) - [ ] I verified the CI pipeline passes locally if applicablecg
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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
Loading a custom plugin `path` causes native code (a `.so`) to be `dlopen()`'d directly into the gateway process. Previously, this was allowed even when dashboard authentication was disabled or unconfigured — meaning any caller who could reach the management API could inject arbitrary native code. This PR closes that gap by requiring a genuinely authenticated admin session for any create or update operation that sets a non-builtin plugin `path`, and separately hardens the plugin downloader against SSRF. - Added `BifrostContextKeyAuthBypassed` context key, set by the auth middleware exclusively when a request is let through because dashboard auth is disabled/unconfigured (distinct from `IsLocalAdminContextKey`, which is also set on real authenticated sessions). - `createPlugin` and `updatePlugin` handlers now check `BifrostContextKeyAuthBypassed` and return `403` before any DB write when a non-builtin `path` is supplied without genuine authentication. - Replaced the `fasthttp`-based plugin downloader with a `net/http` client backed by `network.SSRFSafeDialContext`, matching the SSRF hardening already applied to `core/providers/utils.FetchAndEncodeURL`. The new client: rejects non-`http`/`https` schemes before any network call, refuses connections to loopback, private, CGNAT, link-local, and unspecified addresses (including IPv4-in-IPv6 transition addresses) at dial time (not just DNS lookup time, so DNS rebinding doesn't bypass it), applies the same IP check to redirect targets, caps redirect depth at 5, and limits response body reads to 200 MB. - Tests for `DownloadPlugin` now use a `useNonSSRFGuardedClient` helper that swaps in a plain dialer for the duration of each test (since `httptest` servers bind to loopback, which the production dialer correctly blocks). A new `TestDownloadPlugin_BlocksSSRFToLoopback` test verifies the production guard is active by default, and `TestDownloadPlugin_RejectsNonHTTPScheme` verifies `file://` and similar schemes are rejected before any network call. - New handler tests cover all four cases: create with bypassed auth (expect 403, no DB write), create with real auth (expect 201, path stored), update with bypassed auth (expect 403, no DB write), and the existing config-merge behaviour. - OpenAPI docs and the plugin sequencing guide updated to document the 403 response and the authentication requirement for `path`. - Dependency bumps: `aws-sdk-go-v2/config` → v1.32.14, `aws-sdk-go-v2/service/s3` → v1.99.0, `aws-sdk-go-v2/internal/ini` → v1.8.6, `buger/jsonparser` → v1.2.0. - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI > This is primarily a security hardening change. - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [x] Docs ```sh go test ./... go test ./transports/bifrost-http/handlers/... -run TestCreatePlugin_RejectsCustomPathWhenAuthBypassed go test ./transports/bifrost-http/handlers/... -run TestCreatePlugin_AllowsCustomPathWhenNotBypassed go test ./transports/bifrost-http/handlers/... -run TestUpdatePlugin_RejectsCustomPathWhenAuthBypassed go test ./framework/plugins/... -run TestDownloadPlugin_BlocksSSRFToLoopback go test ./framework/plugins/... -run TestDownloadPlugin_RejectsNonHTTPScheme ``` To manually verify the 403 behaviour: start the gateway with no dashboard auth configured, then attempt `POST /api/plugins` with a `path` field pointing to a `.so`. The response should be `403` with a message instructing the operator to enable dashboard authentication first. - [x] Yes - [ ] No Operators running with dashboard authentication disabled who were previously able to create or update custom plugin paths via the API will now receive a `403`. To restore the capability, enable dashboard authentication and authenticate before calling those endpoints. - Closes an unauthenticated native code injection vector: without this change, any network-reachable caller could `dlopen()` an attacker-controlled `.so` into the gateway process when dashboard auth was off. - The SSRF fix on the plugin downloader prevents a crafted plugin URL from causing the gateway to fetch from internal/metadata endpoints (e.g. cloud IMDS). The guard runs at dial time, not DNS resolution time, so DNS rebinding attacks do not bypass it. - `BifrostContextKeyAuthBypassed` is intentionally separate from `IsLocalAdminContextKey` so that future handlers gating other high-risk operations can use the same signal without ambiguity. - [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
## Summary Resolves merge conflicts in `core/go.sum` that were left over from merging the path normalization auth bypass fix (maximhq#5763). ## Changes - Removed leftover `<<<<<<< HEAD`, `=======`, and `>>>>>>> e0057ff` conflict markers from `core/go.sum` - Retained the correct `go.mod` hash lines for `aws-sdk-go-v2/config`, `aws-sdk-go-v2/internal/ini`, and `aws-sdk-go-v2/service/s3` that were dropped during the conflict ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh cd core go mod verify go test ./... ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#5763 ## Security considerations No security implications. This is a cleanup of unresolved merge conflict markers in the dependency lockfile. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Adds a dedicated **MCP Guardrails** section to the guardrails documentation, explaining how guardrails apply at the tool-execution boundary for MCP targets, including phase behavior, UI field descriptions, and provider compatibility notes. ## Changes - Replaced the brief inline mention of MCP rule behavior in the Architecture section with a cross-reference link to the new dedicated section. - Added a new `## MCP Guardrails` section covering: - How `input`, `output`, and `both` phases apply at the tool-execution boundary. - A table describing the flow and block behavior for each phase. - Guidance on selecting MCP clients, tools, and tool arguments when creating rules. - A note clarifying that all supported guardrail providers work with MCP rules and that redaction support follows the same provider constraints as LLM rules. - Cross-references to existing sections for rule configuration and CEL expression examples. - Added a screenshot (`ui-mcp-guardrail-rule.png`) showing the MCP guardrail rule editor in the UI. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Navigate to the rendered guardrails documentation page and verify: - The Architecture section links to `#mcp-guardrails` instead of containing inline MCP text. - The new MCP Guardrails section renders correctly with the phase table, screenshot, and note. - The screenshot image loads without errors. ## Screenshots/Recordings The new section includes a screenshot of the MCP guardrail rule editor (`ui-mcp-guardrail-rule.png`). ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. Documentation-only change. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…#5962) Fix the budget override **Valid until** preview for calendar-aligned budgets. The UI now calculates expiry from the current UTC calendar-period boundary instead of directly from a preserved, mid-period `last_reset` timestamp. For example, a monthly calendar-aligned budget with `last_reset = Aug 3` should expire at the next monthly boundary on Sep 1, not Sep 3: ```text Aug 1 00:00 UTC Aug 3 Sep 1 00:00 UTC |--------------------------|--------------------------| calendar period start last_reset valid until Old UI: Aug 3 + 1 month -> Sep 3 08:59 UTC New UI: Aug 1 + 1 month -> Sep 1 00:00 UTC ``` The backend override cycle was already correct; this only fixes the date displayed by the UI. - Added a helper that snaps calendar-aligned reset timestamps to their current UTC period boundary: - Day: 00:00 UTC on the current day - Week: Monday at 00:00 UTC - Month: first day of the month at 00:00 UTC - Year: January 1 at 00:00 UTC - Applied that boundary before adding override cycles in `getBudgetOverrideValidUntil`. - Kept rolling and sub-day calculations unchanged. - Added regression coverage for daily, weekly, monthly, and yearly calendar-aligned overrides. The calculation now follows this flow: ```text preserved last_reset Aug 3 08:59 | v snap to calendar period Aug 1 00:00 | v add one monthly cycle Sep 1 00:00 ``` - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs Run the focused utility tests: ```sh cd ui npx vitest run lib/utils/governance.test.ts ``` Expected outcome: ```text Test Files 1 passed Tests 5 passed ``` Run the TypeScript typecheck: ```sh cd ui npm run typecheck ``` Expected outcome: the command completes without TypeScript errors. The regression test verifies these representative cases: ```text Monthly: Aug 3 08:59 + 1 cycle -> Sep 1 00:00 UTC Weekly: Wed Aug 5 + 1 cycle -> Mon Aug 10 00:00 UTC Daily: Aug 3 08:59 + 1 cycle -> Aug 4 00:00 UTC Yearly: Aug 3 2026 + 1 cycle -> Jan 1 2027 00:00 UTC ``` No new configuration or environment variables are introduced. Not included. This changes the date calculation behind the existing **Valid until** field without changing the UI layout. Before: ```text Valid until: Sep 3, 2026 ``` After: ```text Valid until: Sep 1, 2026 ``` - [ ] Yes - [x] No No linked issue. No security implications. This change only adjusts a client-side date preview and does not affect authentication, authorization, secrets, PII, or backend enforcement. - [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
…ootstrap cleanup failure - initiateMCPClientVerification now checks h.store.ConfigStore for nil before dereferencing, matching every other handler in this file. - pendingOAuthConfigToRequest now carries the config.json oauth_config's Resource (RFC 8707) into the OAuth initiation request instead of silently dropping it. - completeMCPClientOAuth now reports failure (mirroring the existing [PARTIAL SUCCESS] pattern used for the VK-assignment case) when clearing a config.json client's pending-bootstrap stash fails, instead of returning success while DB state can regress the client to pending_verification after a restart.
…r-safe bootstrap authorize flow, handle OAuth 409 conflict
…r-user-header fields on OAuth-completion updates
…_type The Authorize/Verify label ternary keyed off auth_type === 'oauth', but handleStartBootstrap's OAuth-based path is the same one-time admin verification step for every pending_verification client — oauth and per_user_oauth alike. Drop the ternary and always show "Authorize".
…en perf fixtures so the migration's join is exercised, not just its miss path
…aim atomic against concurrent reauth
…ntity oauth token lookup contract
…losed on missing tool annotations, dedupe background reconnect
Per-user auth types now show the same connection-state badge as every other client instead of a session-list link, keeping the edit sheet's header consistent across auth types.
… on the actions trigger Reauthorize appeared for every client whose state wasn't pending_verification/disabled, including non-OAuth and per-user OAuth clients — but reauthorizeMCPClient 400s on any auth_type other than 'oauth'. Gate the menu item on auth_type === 'oauth' as well, keeping the existing state condition (the endpoint has no state precondition of its own, and a later per_user_oauth extension of this same block relies on that state condition staying broad). isReauthorizing already disabled the menu item, but onSelect closes the menu immediately, so the trigger kept showing MoreHorizontal with no visible feedback while the request ran. The trigger now also spins and disables during reauthorization, matching the existing isReconnecting behavior.
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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 Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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 Extends environment variable substitution support to the `service_name`, `ml_app`, `env`, and `version` fields in the Datadog integration configuration, allowing these values to be sourced dynamically from environment variables at runtime. ## Changes - Added `env.VAR_NAME` support notation to the `service_name`, `ml_app`, `env`, and `version` fields in the configuration reference table - Updated the Environment Variable Substitution section to include `service_name`, `ml_app`, `env`, and `version` in the list of supported fields - Added example usage of `env.BIFROST_DD_SERVICE`, `env.BIFROST_DD_ENV`, and `env.BIFROST_DD_VERSION` in the JSON code snippet ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Review the updated Datadog observability documentation to confirm the configuration table and environment variable substitution section accurately reflect the supported fields and example usage. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only updates documentation to reflect existing or newly supported environment variable substitution behavior, which helps avoid hardcoding sensitive or environment-specific values in configuration files. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…hq#6017) ## Summary Adds end-to-end API tests for the virtual key budget override endpoints, covering both applying and removing a budget override on a virtual key. ## Changes - Added `vk_budget_id` and `vk_budget_max_limit` as collection variables, populated dynamically from the first budget returned when fetching a virtual key. - Added a **Set Virtual Key Budget Override** test (`PUT /api/governance/virtual-keys/{vk_id}/budgets/{vk_budget_id}/override`) that applies an override of `7.5` with mode `forever` and asserts the response contains the correct `override_mode`, `override_amount`, and that the `effective_max_limit` is raised above the base budget. - Added a **Remove Virtual Key Budget Override** test (`DELETE /api/governance/virtual-keys/{vk_id}/budgets/{vk_budget_id}/override`) that clears the override and asserts `override_mode` and `override_amount` are empty/zero, and that `effective_max_limit` falls back to the original base budget value. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Run the Postman collection against a running Bifrost instance: ```sh newman run tests/e2e/api/collections/bifrost-api-management.postman_collection.json \ --env-var base_url=http://localhost:8080 ``` The **Set Virtual Key Budget Override** and **Remove Virtual Key Budget Override** requests will execute after the virtual key fetch step, which populates `vk_budget_id` and `vk_budget_max_limit`. Both requests will be skipped automatically if no budget ID is available. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. Tests exercise existing governance endpoints using collection-scoped variables only. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…rrect describe block (maximhq#6048) ## Summary A test for calendar-aligned budget override validity was accidentally nested inside an unrelated `it` block in the `budgetSignature without ids` describe block, causing it to never actually run. This PR moves the test to the correct location within the `budget overrides` describe block. ## Changes - Removed the misplaced `it("anchors calendar-aligned validity to the current period boundary", ...)` block that was nested inside the `budgetSignature without ids` test, where it would never execute - Added the same test cases correctly at the top level of the `budget overrides` describe block, where they are properly registered and run ## Type of change - [x] Bug fix ## Affected areas - [x] UI (React) ## How to test ```sh cd ui pnpm i || npm i pnpm test || npm test ``` The previously dormant test cases for `getBudgetOverrideValidUntil` with monthly, weekly, daily, and yearly reset durations should now execute and pass. ## Breaking changes - [x] No ## Related issues ## Security considerations None. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…ximhq#6049) ## Summary Adds `needs_session_stickiness: true` to the MCP client creation request in the end-to-end API test collection, ensuring the test accurately reflects the expected payload for MCP clients that require session stickiness. ## Changes - Added `needs_session_stickiness: true` to the raw request body in the `bifrost-api-management` Postman collection for the MCP client creation endpoint. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Run the Postman collection against a running Bifrost instance and verify the MCP client creation request succeeds with the `needs_session_stickiness` field included. ```sh go test ./... ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only adds a session stickiness flag to a test request payload. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] 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 Updates the documentation URL for custom pricing overrides to reflect the correct path in the docs site. ## Changes - Updated `PRICING_OVERRIDES_DOCS_URL` from `https://docs.getbifrost.ai/features/governance/custom-pricing` to `https://docs.getbifrost.ai/providers/custom-pricing` to point to the accurate documentation location. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test Navigate to the custom pricing overrides empty state in the UI and click the documentation link. Verify it redirects to `https://docs.getbifrost.ai/providers/custom-pricing`. ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None. ## 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
Fix Realtime input guardrails so they inspect prompt content supplied directly
in a valid `response.create` event.
Previously, Realtime pre-hooks only received conversation items accumulated
before the turn. A request that placed its prompt in `response.instructions` or
`response.input` was forwarded to the provider correctly, but those fields were
absent from the normalized request evaluated by guardrails. This caused
equivalent non-Realtime requests to be blocked while valid Realtime requests
could pass.
## Changes
- Pass the complete turn-start event into the Realtime pre-hook pipeline for
both WebSocket and WebRTC transports.
- Extract `response.instructions`, `response.input`, and response-level
`response.tools` from `response.create`.
- Combine inline response input with previously accumulated conversation input
before running pre-hooks.
- Prefer response-level tools for that turn, falling back to session-level tools
when no response-level tools are supplied.
- Preserve the existing behavior for turn starts that do not carry a `response`
payload.
- Add regression tests for direct `response.create` content and mixed
conversation/inline input.
### Before
A valid request could send all prompt content in `response.create`:
```json
{
"type": "response.create",
"response": {
"output_modalities": ["text"],
"instructions": "<some stuff that should be flagged>"
}
}
```
The provider received the instructions, but the guardrail request was
effectively empty:
```text
response.create
├── response.instructions ───────────────► provider
└── accumulated conversation (empty) ───► input guardrails
Result: the model generated a response even when the same prompt was blocked through the Responses API.
```
### After
The turn-start event is now included while constructing the request inspected by
pre-hooks:
```text
response.create
├── accumulated conversation ─┐
├── response.input ────────────┼──► normalized Realtime request ──► input guardrails
├── response.instructions ─────┤
└── response.tools ────────────┘
│
├── allowed ─► provider
└── blocked ─► error returned; event is not forwarded
```
This also supports combining conversational and response-specific input:
```json
// Earlier event
{
"type": "conversation.item.create",
"item": {
"type": "message",
"role": "user",
"content": [{ "type": "input_text", "text": "conversation context" }]
}
}
// Turn-start event
{
"type": "response.create",
"response": {
"instructions": "answer in one sentence",
"input": [
{
"type": "message",
"role": "user",
"content": "inline request"
}
]
}
}
```
Both `conversation context` and `inline request`, together with the
instructions, are now visible to the input guardrail pipeline.
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
Run the focused regression tests:
```sh
go test ./transports/bifrost-http/handlers -run 'TestBuildRealtimeTurnPreRequest|TestPendingRealtimeInputUpdate'
```
Expected outcome: the tests pass and confirm that:
- `response.instructions`, inline `response.input`, and response-level tools are
present in the pre-hook request;
- accumulated conversation input and inline response input are combined.
Run the complete handler test package:
```sh
go test ./transports/bifrost-http/handlers
```
Expected outcome: all handler tests pass.
No new configuration or environment variables are required.
## Screenshots/Recordings
Not applicable; this change affects Realtime request processing.
## Breaking changes
- [ ] Yes
- [x] No
Existing conversation-item flows continue to work. Valid `response.create`
prompt fields now receive the same input guardrail coverage.
## Related issues
No linked issue.
## Security considerations
This closes a guardrail coverage gap. Prompt content sent through
`response.create` is now evaluated before the event is forwarded to the Realtime
provider. No secrets, authentication behavior, or stored data formats are
changed.
This change concerns input enforcement only; it does not alter the existing
post-turn behavior of Realtime output guardrails.
## 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 (not required for this internal
behavior fix)
- [x] I verified builds succeed (Go and UI) (affected Go handler package
verified; UI not affected)
- [x] I verified the CI pipeline passes locally if applicable (focused and
complete handler tests pass)
MCPClientConfig.PerUserHeaderKeys (per_user_header_keys) is required (non-empty) for authType per_user_headers MCP clients, but the clientConfigs -> config.json template in _helpers.tpl never mapped it, so it silently dropped out of Helm-rendered config regardless of what was set in values.yaml. values.schema.json also never declared the property. Fixes maximhq#6033. Affected packages: - helm-charts/bifrost/values.schema.json - helm-charts/bifrost/templates/_helpers.tpl - helm-charts/bifrost/values.yaml - helm-charts/bifrost/README.md - .github/workflows/scripts/validate-helm-config-fields.sh
…er_headers Address review feedback on the previous commit: - values.schema.json now requires perUserHeaderKeys (non-empty, no whitespace-only entries) via an allOf/if/then keyed on authType, scoped to per_user_headers only — other auth types are unaffected. - values.yaml's per_user_headers example used a bare connectionString with connectionType: http, which the schema requires httpConfig for; fixed to use httpConfig.url like the adjacent http example. Affected packages: - helm-charts/bifrost/values.schema.json - helm-charts/bifrost/values.yaml
transports/config.schema.json already supports an inline oauth_config bootstrap block and a token_exchange auth type for MCP clients, but the Helm chart only exposed oauthConfigId - a field config.schema.json no longer even accepts (it referenced an API-created OAuth config row, not a static-config field), so setting it would silently fail schema validation. Adds mcp.clientConfigs[].oauthConfig (clientId, clientSecret, authorizeUrl, tokenUrl, registrationUrl, scopes) and .tokenExchange (audience, clientId, clientSecret, scopes), rendered into oauth_config and token_exchange respectively, with the same conditional enforcement config.schema.json uses (oauthConfig forbidden unless authType is oauth/per_user_oauth, tokenExchange required iff authType is token_exchange). Also adds token_exchange to the authType enum. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughMCP Helm configuration now supports per-user header keys, inline OAuth settings, and token-exchange authentication. The schema validates conditional fields, templates render nested authentication objects, examples document the new values, and workflow checks verify rendered output. ChangesMCP Helm authentication
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant ValuesSchema
participant HelmHelpers
participant ValidationScript
HelmValues->>ValuesSchema: Validate MCP authentication fields
ValuesSchema->>HelmHelpers: Provide accepted configuration
HelmHelpers->>ValidationScript: Produce rendered MCP configuration
ValidationScript->>ValidationScript: Assert authentication fields
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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 `@helm-charts/bifrost/values.schema.json`:
- Around line 6165-6180: Update the mcpClientConfig schema rule around the
authType conditional in helm-charts/bifrost/values.schema.json lines 6165-6180
to reject the removed oauthConfigId property, either by adding a not rule or by
disallowing undeclared properties while preserving supported fields. In
helm-charts/bifrost/README.md line 17, keep the schema-rejection statement only
once the schema enforces this rejection.
In `@helm-charts/bifrost/values.yaml`:
- Around line 515-530: Update the commented “example-oauth-mcp” configuration in
values.yaml to replace connectionString with the required httpConfig.url
structure, while preserving the existing HTTP endpoint and OAuth settings so the
example passes the schema when enabled.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 026187ca-2d15-4b12-b17a-be15d2fcba32
📒 Files selected for processing (5)
.github/workflows/scripts/validate-helm-config-fields.shhelm-charts/bifrost/README.mdhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yaml
…sage Removing the oauthConfigId property definition wasn't enough on its own - without additionalProperties:false at this level, the field was still silently accepted by schema validation and then dropped by _helpers.tpl. Add an explicit not/required rule so it's rejected outright, matching config.schema.json's behavior. Also fixes the example-oauth-mcp comment block in values.yaml, which used a bare connectionString with connectionType: http - the schema requires httpConfig for that connection type, and _helpers.tpl only maps connectionString for sse. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 11, 2026
akshaydeo
dismissed
coderabbitai[bot]’s stale review
August 13, 2026 09:03
The merge-base changed after approval.
akshaydeo
force-pushed
the
dev
branch
2 times, most recently
from
August 13, 2026 09:47
244a01d to
ce1b2a6
Compare
This was referenced Aug 13, 2026
This was referenced Aug 13, 2026
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
13 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.
Summary
transports/config.schema.jsonalready supports an inlineoauth_configbootstrap block and atoken_exchangeauth type for MCP clients (mcp.client_configs[]), but the Helm chart only exposedoauthConfigId— a fieldconfig.schema.jsonno longer even accepts (it referenced an API-created OAuth config row, not a static-config field), so any chart deployment that set it would silently fail schema validation. This wires up both auth modes so they can be defined declaratively invalues.yaml.Changes
mcp.clientConfigs[].oauthConfig(clientId,clientSecret,authorizeUrl,tokenUrl,registrationUrl,scopes) forauthType: oauth/per_user_oauth, rendered intooauth_config. All fields are optional — missing URLs are discovered via RFC 8414, and a missingclientIdtriggers RFC 7591 dynamic client registration.authType: token_exchangeplusmcp.clientConfigs[].tokenExchange(audience,clientId,clientSecret,scopes) for delegated token-exchange auth, rendered intotoken_exchange.config.schema.json's conditional enforcement invalues.schema.json:oauthConfigforbidden unlessauthTypeisoauth/per_user_oauth;tokenExchangerequired iffauthTypeistoken_exchange, forbidden otherwise.oauthConfigIdfield/mapping since it's no longer valid againstconfig.schema.json. This is technically a breaking change for any chart user who had set it (see Breaking changes below).values.yamlexamples,.github/workflows/scripts/validate-helm-config-fields.shfixtures/assertions, and the Helm README changelog (### 2.1.35— unreleased; version bump/docs mdx/nav update happen separately at release time per this repo's convention).Type of change
Affected areas
(Helm chart under
helm-charts/bifrost/— not listed as its own category above.)How to test
Also validated the rendered
config.jsonoutput against the realtransports/config.schema.jsonvia Python'sjsonschemalibrary — passes cleanly.If adding new configs or environment variables, document them here.
New
values.yamlfields:bifrost.mcp.clientConfigs[].oauthConfig.{clientId,clientSecret,authorizeUrl,tokenUrl,registrationUrl,scopes}andbifrost.mcp.clientConfigs[].tokenExchange.{audience,clientId,clientSecret,scopes}. No new environment variables —clientId/clientSecretvalues can referenceenv.VAR_NAMEorvault.pathas with existing secret-bearing fields.Screenshots/Recordings
N/A — no UI changes.
Breaking changes
oauthConfigIdis removed from the Helm values schema, but it was already non-functional:config.schema.jsondroppedoauth_config_idfrommcp_client_configbefore this PR, so any chart deployment setting it would already fail schema validation upstream. No working deployment depended on it.Related issues
N/A
Security considerations
oauthConfig.clientSecretandtokenExchange.clientSecretare secret-bearing fields, consistent with existing MCP client fields (e.g.headers, TLS config) — documented to supportenv.VAR_NAME/vault.pathindirection rather than being hardcoded intovalues.yaml.Checklist
docs/contributing/README.mdand followed the guidelines🤖 Generated with Claude Code