feat(bedrock): add profile field for AWS named profile / SSO auth - #4030
feat(bedrock): add profile field for AWS named profile / SSO auth#4030AndreKurait wants to merge 54 commits into
profile field for AWS named profile / SSO auth#4030Conversation
…on rankings chart (maximhq#3950) ## Summary Y-axis labels in the dimension rankings bar chart were taking up too much horizontal space and could overflow without truncation. This PR truncates long dimension names in the chart's Y-axis labels and adds a tooltip `<title>` element so users can still see the full name on hover. ## Changes - Y-axis labels longer than 14 characters are now truncated with an ellipsis (`…`), with the full value exposed via an SVG `<title>` for accessibility and hover visibility - Y-axis width reduced from `110` to `92` to match the shorter label space - Left margin reduced from `4` to `0` to reclaim horizontal space ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test Navigate to the workspace dashboard and open the Dimension Rankings tab. Find a dimension with a long name (more than 14 characters) and verify: 1. The label is truncated with an ellipsis in the chart 2. Hovering over the label shows the full name via the browser's native tooltip ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## Screenshots/Recordings Add before/after screenshots showing the truncated Y-axis labels vs. the previous full-length labels. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Style** - Improved dimension ranking chart layout with optimized spacing - Long category labels now display with ellipsis truncation for better readability, with full text visible on hover <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…le (maximhq#3952) ## Summary Fixes layout issues in the `HeadersTable` component where columns were not properly constrained, causing inconsistent sizing of the Name, Value, and Actions columns. ## Changes - Applied `table-fixed` layout to the table to enforce column width constraints - Set the Name column to a fixed width of 40% to ensure consistent proportions between Name and Value columns - Reduced the Actions column width from `w-12` to `w-10` and removed excess padding (`p-0`) to tighten the delete button column - Removed padding from the Actions cell to better align the delete button within its column ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test Navigate to any view that renders the `HeadersTable` component (e.g., a request headers configuration panel) and verify: 1. The Name and Value columns maintain consistent proportions as rows are added or removed. 2. The Actions (delete) column remains compact and does not expand unexpectedly. 3. Long header names or values do not cause the table layout to shift. ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## Screenshots/Recordings Add before/after screenshots showing the corrected column widths in the headers table. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Style** * Implemented fixed table layout to ensure consistent column widths and improved visual stability. * Refined header and action column sizing for better alignment and visual consistency. * Optimized spacing in the actions column for improved usability of row control buttons. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
`StaleConnectionRetryIfErr` was not retrying on several real-world connection-closure errors because it relied on direct equality checks (`err == io.EOF`) and a fixed set of string patterns. Wrapped errors (e.g. `fmt.Errorf("read response: %w", io.EOF)`) were silently falling through without a retry, and error strings like `"use of closed network connection"` and `"server closed connection"` were not covered.
## Changes
- Replaced `err == io.EOF` with `errors.Is(err, io.EOF)` so wrapped EOF errors are correctly detected.
- Added `errors.Is(err, io.ErrUnexpectedEOF)` to handle unexpected EOF variants.
- Added string match patterns for `"use of closed network connection"` and `"server closed connection"` to cover additional OS- and fasthttp-level connection closure signals.
- Added an early-exit guard for `fasthttp.ErrConnectionClosed` to avoid retrying when fasthttp has already handled the error post-loop.
- Switched `err.Error()` to `strings.ToLower(err.Error())` for case-insensitive string matching consistency.
- Added corresponding test cases for wrapped `io.EOF`, `io.ErrUnexpectedEOF`, wrapped `io.ErrUnexpectedEOF`, `"use of closed network connection"`, and `"server closed connection"`.
## 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
```sh
go test ./core/network/... -v -run TestStaleConnectionRetryIfErr
```
All existing and new test cases should pass, including the wrapped EOF and new connection-closure string variants.
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
## Security considerations
None. This change only affects retry logic for stale HTTP connections and does not touch authentication, secrets, or PII handling.
## 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
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved HTTP client resilience by expanding retry behavior to handle a broader set of EOF and connection-closed scenarios.
* **Tests**
* Expanded test coverage to validate additional EOF, wrapped-EOF, and server-closure cases.
* **Chores**
* Minor formatting adjustment to an example configuration file.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
Introduces a `source_of_truth` field to `config.json` that allows operators to make config.json sections authoritative during startup reconciliation. When set to `"config.json"`, any section explicitly present in the file becomes the single source of truth — database-only rows for that section are pruned. When omitted or set to `"split"` (the default), existing merge behavior is preserved.
## Changes
- Added `source_of_truth` field to `ConfigData` with two modes: `"split"` (default, existing behavior) and `"config.json"` (file-authoritative).
- Added `presentSections` and `presentGovernanceSections` tracking maps populated during `UnmarshalJSON` so that explicitly-present-but-empty sections (e.g., `"providers": {}`) can be distinguished from absent sections.
- Added `sectionPresent` and `governanceSectionPresent` helpers on `ConfigData` to query section presence.
- Introduced `syncAuthoritativeProvidersInStore` which, under `config.json` mode, deletes DB-only providers and DB-only keys within kept providers inside a single transaction.
- Introduced `processAuthoritativeProvider` as the authoritative counterpart to `processProvider`, preserving runtime-only fields (status, description) from the DB while making the file's key list canonical.
- Introduced `syncMCPConfigFromFile` which replaces stored MCP clients with exactly those declared in config.json, deleting DB-only clients and upserting file clients.
- Introduced `syncPluginsFromFile` which replaces stored plugins with exactly those declared in config.json, deleting DB-only plugins and upserting file plugins within a transaction.
- Introduced `pruneGovernanceConfigToFile` which, after the normal governance merge, removes DB-only rows for each governance collection that was explicitly present in the file (virtual keys, routing rules, pricing overrides, model configs, teams, customers, providers, budgets, rate limits).
- Added `source_of_truth` to `config.schema.json` as an enum of `["split", "config.json"]` with schema validation.
- Added a schema candidate path for tests running from `transports/bifrost-http/lib/`.
- Fixed `MockConfigStore.DeleteMCPClientConfig` and `DeletePlugin` to actually remove entries so sync tests can assert on store state.
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
```sh
go test ./transports/bifrost-http/lib/... -run TestConfigDataSourceOfTruth
go test ./transports/bifrost-http/lib/... -run TestSourceOfTruthConfigJSON
go test ./transports/bifrost-http/lib/... -run TestSQLite_SourceOfTruthConfigJSON
go test ./transports/bifrost-http/lib/... -run TestConfigSchemaSourceOfTruthValidation
go test ./transports/bifrost-http/lib/...
```
**New `config.json` field:**
| Field | Type | Values | Default |
|---|---|---|---|
| `source_of_truth` | `string` | `"split"`, `"config.json"` | `"split"` |
To enable authoritative mode, add to `config.json`:
```json
{
"source_of_truth": "config.json",
"providers": { ... },
"governance": { "budgets": [...] }
}
```
Only sections explicitly present in the file will be pruned in the database. Sections omitted from the file leave database rows untouched regardless of mode.
## Breaking changes
- [ ] Yes
- [x] No
Default behavior (`"split"`) is unchanged. Operators must explicitly opt in to `"config.json"` mode.
## Related issues
## Security considerations
Provider API keys that exist only in the database will be permanently deleted when `source_of_truth: "config.json"` is set and the `providers` section is present in the file. Operators should ensure all required keys are declared in config.json before enabling this mode.
## 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
## Summary This PR closes a DNS rebinding vulnerability that existed between the time `ValidateExternalURL` validated a URL and the time the HTTP client actually opened a TCP connection. An attacker could exploit this window by having a hostname resolve to a public IP during validation and then to a private/internal IP at dial time, bypassing SSRF protections. ## Changes - Extracted `IsLocalhost` and `IsPrivateIP` into a new `core/network` package so they can be shared across validation and dialing layers. - Updated `ConfigureDialer` to perform its own DNS resolution at dial time, reject any resolved private or loopback IPs, and dial the resolved IP literal directly — eliminating the TOCTOU window between URL validation and connection. - Added `ValidateExternalURL` calls in the HTTP transport's `addProvider` and `updateProvider` handlers to reject private or loopback `BaseURL` values at the API boundary. - Added `core/utils_test.go` with comprehensive tests covering `ValidateExternalURL`, `IsLocalhost`, and `IsPrivateIP`, including RFC 1918 ranges, link-local addresses, the AWS metadata endpoint (`169.254.169.254`), IPv6 private ranges, and query-parameter injection vectors. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations ## How to test ```sh go version go test ./... ``` Key scenarios validated by the test suite: - `http://169.254.169.254/latest/meta-data/` → rejected as private IP - `http://10.0.0.1/path?x=` → rejected as private IP - `http://localhost:8080` → rejected as loopback - `https://api.openai.com` → allowed - IPv6 loopback (`::1`), link-local (`fe80::1`), and unique-local (`fc00::/7`) → all rejected ## Breaking changes - [ ] Yes - [x] No ## Security considerations This change directly addresses an SSRF / DNS rebinding attack vector. Previously, a malicious actor could register a hostname that resolved to a public IP during `ValidateExternalURL` and then switch the DNS record to an internal address (e.g., `169.254.169.254`, `10.x.x.x`) before the dialer connected. The dialer now resolves DNS independently, validates every returned IP against private ranges, and dials the IP literal, closing this window entirely. The `BaseURL` field on provider add/update endpoints is also now validated at the API layer. ## Checklist - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security Improvements** * Strengthened SSRF protections: centralized hostname/IP checks now block localhost, link-local, unspecified, and private IPs when validating external URLs and provider Base URLs. * Provider create/update endpoints validate Base URLs and return clear Bad Request responses for unsafe targets. * Network dialing now resolves hosts and rejects disallowed/private addresses early to prevent unsafe connections. * **Tests** * Added extensive unit tests for URL validation, network address classification, and dialer SSRF behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Some OpenAI-compatible backends return valid SSE frames without a `Content-Type: text/event-stream` header. The previous `DrainNonSSEStreamResponse` helper would unconditionally drain and discard the response body in this case, causing the downstream SSE parser to receive an empty stream. This PR introduces a reader-preserving variant that peeks at the first bytes of the stream to detect SSE field prefixes (`data:`, `event:`, `id:`, `retry:`, `:`, or leading newlines) before deciding whether to drain or pass the reader through intact. ## Changes - Introduced `DrainNonSSEStreamReader(resp, reader)` which accepts an `io.Reader` (e.g. a decompressed stream) and returns a potentially buffered reader alongside a `drained` boolean, preserving the stream when it looks like SSE even if the content type header is absent. - `DrainNonSSEStreamResponse` is retained as a thin wrapper delegating to `DrainNonSSEStreamReader` for backward compatibility. - All streaming handlers in the OpenAI provider (`text completion`, `chat completion`, `responses`, `speech`, `transcription`, `image generation`, `image edit`) now use `DrainNonSSEStreamReader` and reassign the reader from its return value so the buffered peek bytes are not lost. - Added `looksLikeSSEPrefix` to perform a case-sensitive, 16-byte peek-based heuristic for SSE field prefixes. - Added tests covering: SSE without content type remains readable, gzip-compressed SSE without content type remains readable, JSON without content type is drained, and uppercase SSE-like prefixes are treated as non-SSE. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/utils/... -run TestDrainNonSSEStreamReader go test ./... ``` Expected: all new `TestDrainNonSSEStreamReader_*` tests pass, and existing streaming tests remain green. To validate end-to-end, route a streaming request through a backend that returns SSE without `Content-Type: text/event-stream` and confirm the response is streamed correctly rather than returning an error. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. The peek reads at most 16 bytes from the stream and does not log or expose any content. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved streaming across text, chat, responses, speech, transcription, image generation and edit flows to detect SSE-like streams, avoid prematurely draining them, and prevent stream hangs or unexpected termination. * **Tests** * Added and updated unit tests for SSE detection, compressed-stream handling, fragmented/tiny-prefix delivery, and correct draining behavior for non-SSE payloads. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Fixes a security vulnerability where provider API key headers injected by Bifrost upstream were being echoed back to clients in response headers. This was identified as a regression in the `/genai_passthrough` endpoint where `x-goog-api-key` values were leaking to clients (e.g., via Google's file-download 302 redirects). fixes maximhq#3954 ## Changes - Added `x-goog-api-key`, `x-api-key`, and `api-key` to the `providerResponseFilterHeaders` blocklist so they are stripped from upstream responses before being forwarded to clients. - Added a regression test `TestExtractProviderResponseHeaders_StripsProviderSecrets` that verifies all four sensitive headers (`x-goog-api-key`, `x-api-key`, `api-key`, `authorization`) are stripped while benign headers like `x-request-id` are preserved. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/utils/... -run TestExtractProviderResponseHeaders_StripsProviderSecrets -v ``` Expected output: the test passes, confirming that `x-goog-api-key`, `x-api-key`, `api-key`, and `authorization` are absent from the extracted response headers map, and that `x-request-id` is preserved. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues Regression fix for `x-goog-api-key` leak via `/genai_passthrough`. ## Security considerations This patch closes a credential leak where provider API keys (`x-goog-api-key`, `x-api-key`, `api-key`) injected into upstream requests could be reflected back to end clients in HTTP response headers. Any client receiving these responses prior to this fix may have been exposed to the upstream provider credentials. No new secrets or auth mechanisms are introduced. ## Checklist - [x] 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) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * API key and authentication headers (case-insensitive) are now filtered from provider responses to prevent exposure of sensitive credentials to clients. * Legitimate non-sensitive response headers are preserved to maintain request tracing and debugging. * **Tests** * Added a test that verifies sensitive provider headers are stripped while ensuring benign headers remain intact. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Passthrough requests previously had no usage extraction, meaning cost calculation and token logging were silently skipped for all provider passthrough endpoints. This PR adds per-provider usage extraction for both streaming and non-streaming passthrough responses across OpenAI, Azure, Anthropic, and Gemini, and wires the extracted usage into the pricing, logging, and governance plugins. ## Changes - **New `BifrostPassthroughUsage` schema** added to `schemas/passthrough.go` carrying LLM tokens, image counts, audio chars/seconds, video seconds, and container identifiers — covering every billable endpoint type. - **`PassthroughPath` field** added to `BifrostResponseExtraFields` and `BifrostPassthroughResponse` so the path is available downstream without re-parsing the original request. - **Provider-level usage extractors** introduced as new files: - `core/providers/openai/passthrough_usage.go` — handles chat/completions, responses API, embeddings, speech (TTS), transcription/translation, image generation/edit/variation, video generation, and container creation. - `core/providers/anthropic/passthrough_usage.go` — handles `/messages` (SSE and non-streaming) and legacy `/complete`, including cache token details. - `core/providers/gemini/passthrough_usage.go` — handles `:generateContent`/`:streamGenerateContent` (text, audio, image output modalities), embeddings, Imagen (`:predict`), Veo (`:predictLongRunning`), and the Interactions API. - **Streaming accumulation** updated across all four providers to accumulate the full response body (`accBody`) and call the usage extractor on the final EOF chunk, attaching `PassthroughUsage` to the terminal response. - **`core/providers/utils/passthrough.go`** added with shared SSE parsing helpers (`ScanSSEDataLines`, `LastSSEDataLine`, `LastSSEOrBody`) used by all extractors. - **Pricing integration** (`framework/modelcatalog/pricing.go`): `extractCostInput` now checks `PassthroughResponse.PassthroughUsage` first; `inferPassthroughRequestType` maps usage fields and path to the correct `RequestType`; `passthroughUsageToCostInput` converts the usage struct into the existing `costInput` shape so all existing compute functions apply without modification. - **Logging plugin** (`plugins/logging/main.go`, `operations.go`): passthrough token usage is now applied to log entries via `applyNonStreamingOutputToEntry`, and streaming passthrough cost is computed in `PostLLMHook` when `PassthroughUsage` is present. The `Model` field is now forwarded in `PassthroughLogParams`. - **Governance plugin** (`plugins/governance/main.go`): token usage is read from `PassthroughUsage.LLMUsage` for passthrough responses; `HasUsageData` now also triggers when `cost > 0` so non-token-based billing (images, audio, video) is tracked correctly. - **`content-type` removed** from the provider response header filter list so it is forwarded to callers. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./framework/... ./plugins/... ``` To validate end-to-end: 1. Send a passthrough request to `/v1/chat/completions`, `/v1/images/generations`, `/v1/audio/speech`, and a streaming `/v1/responses` endpoint via each supported provider. 2. Confirm that the log entry for each request contains a non-zero `cost` and populated `token_usage_parsed` (or the appropriate usage field for non-token endpoints). 3. For streaming passthrough, confirm that the final accumulated response includes `PassthroughUsage` and that cost appears in the governance usage tracker. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No new auth surfaces. The `content-type` header is now forwarded from providers to callers, which was previously suppressed — callers should be aware the response content type now reflects the provider's actual content type. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced passthrough tracking: upstream request path is surfaced and detailed usage metrics (tokens, images, audio, video, container identifiers) are captured and returned for passthrough requests. * Streaming passthroughs now reliably forward raw chunks, observe incremental usage, and emit final usage on completion. * **Improvements** * Pricing and logging now use passthrough usage to improve cost calculation and reporting. * **Tests** * Added comprehensive tests for passthrough usage extraction and streaming across providers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Enables CodeRabbit auto-reviews on all branches, not just the default branch. ## Changes - Added `base_branches: [".*"]` to the CodeRabbit auto-review configuration so that pull requests targeting any branch are automatically reviewed, rather than only those targeting the default branch. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Open a pull request targeting a non-default branch and verify that CodeRabbit automatically triggers a review. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None. ## Checklist - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated automated review configuration to expand branch coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…o team and business unit attribute mappings (maximhq#3974) ## Summary Adds optional `attributeType` and `attributeValue` fields to `attributeTeamMappings` and `attributeBusinessUnitMappings` to enable SCIM provisioning on a per-mapping basis. When these fields are present, a mapping can be matched against either a SCIM User attribute (`attributeType: "user"`) or a SCIM Group displayName (`attributeType: "group"`). ## Changes - Added `attributeType` (enum: `"user"` | `"group"`) and `attributeValue` (string) as optional properties to `attributeTeamMappings` and `attributeBusinessUnitMappings` in both `helm-charts/bifrost/values.schema.json` and `transports/config.schema.json`. - Added inline `description` fields to existing `attribute`, `value`, `team`, and `business_unit` properties for improved schema documentation. - Added commented-out examples in `values.yaml` demonstrating SCIM provisioning via user attribute matching and group displayName matching. - For `attributeType: "group"`, `attributeValue` is always expected to be `"displayName"` and is auto-set accordingly. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Configure `attributeTeamMappings` or `attributeBusinessUnitMappings` with the new fields and verify schema validation accepts valid inputs and rejects invalid ones (e.g., an `attributeType` value outside `["user", "group"]` or extra properties beyond those declared). ```sh # Validate schema changes go test ./... ``` Example mapping to validate: ```yaml attributeTeamMappings: - attribute: "department" value: "engineering" team: "eng-team" attributeType: "user" attributeValue: "engineering" - attribute: "groups" value: "Engineering" team: "eng-team" attributeType: "group" attributeValue: "displayName" ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No new auth flows or secrets handling introduced. The new fields extend existing JWT claim-to-team/business-unit mapping logic with SCIM provisioning metadata only. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced team and business-unit attribute mappings with two new optional fields to support SCIM provisioning metadata, enabling more flexible attribute-based provisioning. * **Documentation** * Updated commented configuration examples to illustrate the new SCIM attribute/type/value mapping patterns. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary When forwarding raw request bodies to Azure-hosted Anthropic models, the `diagnostics` field (used by Claude Code) is not supported by Azure's API and causes request failures. This PR strips the `diagnostics` field from raw request bodies when the target provider is Azure. ## Changes - `StripUnsupportedFieldsFromRawBody` in `utils.go` now removes the `diagnostics` field from the raw JSON body when the provider is Azure. - A new test case `azure_strips_claude_code_diagnostics` verifies that the `diagnostics` field is removed and that the model is correctly rewritten to the Azure deployment name when using raw request bodies. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/anthropic/... ``` The new test `azure_strips_claude_code_diagnostics` confirms that a raw request body containing a `diagnostics` field is sanitized before being sent to Azure, and that the Azure deployment name is correctly substituted as the model value. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only removes a non-sensitive, provider-incompatible field from outbound requests. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Azure Anthropic requests now strip an unsupported diagnostics field so deployments accept requests while preserving model selection. * **New Features** * Diagnostics data is preserved for the Anthropic provider when the provider supports it. * **Tests** * Added tests confirming diagnostics are kept for Anthropic and removed for other providers, and that Azure payloads have unsupported fields removed. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary When a virtual key has provider constraints (but no weights), the governance plugin now propagates the list of allowed providers into the Bifrost context. The router then intersects that list with the model catalog's provider list, ensuring that only providers permitted by the virtual key are considered for routing and fallbacks. ## Changes - The governance plugin sets `BifrostContextKeyAvailableProviders` on the context after filtering provider configs, including setting an empty slice when no provider configs exist or when no providers pass the model filter. - The router's `createHandler` intersects the catalog-derived provider list with any pre-existing `BifrostContextKeyAvailableProviders` value set by upstream plugins (e.g., governance). If the intersection is empty, an empty provider list is stored rather than falling back to the full catalog set. - `extractAndParseFallbacks` now accepts a `BifrostContext` and filters parsed fallbacks to only those whose provider appears in `BifrostContextKeyAvailableProviders`. If all fallbacks are filtered out, the fallback list on the request is explicitly cleared to `nil`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins ## How to test ```sh go test ./plugins/governance/... go test ./transports/bifrost-http/integrations/... ``` - A virtual key with `openai/gpt-4o` and `anthropic/claude-3-5-sonnet` provider configs (no weights) and a request for `gpt-4o` should result in `BifrostContextKeyAvailableProviders` containing only `openai`. - A virtual key with only `openai/gpt-4o` and a request for `claude-3-5-sonnet` should result in an empty `BifrostContextKeyAvailableProviders`. - A request with fallbacks that include providers not in the allowed list should have those fallbacks stripped before the request is dispatched. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#2516 ## Security considerations Provider constraints enforced by virtual keys are now respected end-to-end through routing and fallback resolution, preventing requests from being routed to providers that the virtual key does not permit. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Tests** * Added comprehensive test coverage for governance HTTP transport pre-hook with provider-constrained virtual keys. * Added router tests verifying proper provider constraint enforcement during request handling. * **Bug Fixes** * Router now correctly respects provider availability constraints when selecting providers for requests. * Fallback extraction now filters to only providers permitted by governance constraints. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…3930) ## Summary When a routing layer selects a specific provider, `CheckAndSetDefaultProvider` was ignoring that selection and falling back to the route's default provider. This PR ensures that a routing-resolved provider takes precedence over the default, as long as it is still within the allowed set of available providers. ## Changes - Added a new context key `BifrostContextKeyResolvedProvider` to carry the provider selected by the routing layer. - Updated `CheckAndSetDefaultProvider` to check for a resolved provider in context and return it immediately if it is present in the available providers list, before falling back to the default provider check. - Added two tests: one verifying the resolved provider is used when allowed, and one verifying it is ignored when not in the available providers list. ## 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 go test ./core/providers/utils/... -run TestCheckAndSetDefaultProvider ``` Expected: both `TestCheckAndSetDefaultProviderUsesResolvedProvider` and `TestCheckAndSetDefaultProviderIgnoresDisallowedResolvedProvider` pass. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations The resolved provider context key is explicitly marked `DO NOT SET THIS MANUALLY` and is only populated by the routing layer. It cannot be used to bypass available-provider constraints, as the check enforces membership in the allowed list before honoring the resolved provider. ## Checklist - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Improvements** * Enhanced provider selection logic to better utilize routing-determined providers when available. * Improved fallback behavior for provider resolution in routing scenarios. * **Tests** * Added test coverage for provider selection scenarios with and without routing-resolved providers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Adds support for `file` content blocks in chat message logs. Users can now view file metadata and download attached files directly from the log detail view. ## Changes - Added a `file` content type and corresponding `file` field to the `ContentBlock` type in `logs.ts`, enabling the frontend to parse file blocks from message content - Introduced `LogChatFileBlockView`, a new exported component that renders file block metadata (filename, type, size, file ID) and provides a download button when inline file data is available - Integrated `LogChatFileBlockView` into `ContentBlockView` to handle `file`-typed content blocks in the general message view - Rendered attached file blocks in `logDetailView.tsx` alongside image attachments when a message contains `file`-typed content - Exposed `EnqueueLogEntry` as a public method on `LoggerPlugin` to allow external callers to push complete log entries through the plugin's async write queue ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Send a request that includes a `file` content block in a chat message (e.g., using the Files API with an inline `file_data` payload or a `file_id` reference). 2. Open the log detail view for that request. 3. Verify the file block is rendered with its filename, type, size, and file ID. 4. If `file_data` is present, click **Download** and confirm the file downloads correctly. 5. If `file_url` is present, confirm the **Open file** link is rendered and navigates correctly. ```sh # UI cd ui pnpm i || npm i pnpm build || npm run build ``` ## Screenshots/Recordings _Add before/after screenshots showing the file block rendered in the log detail view._ ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Inline `file_data` is base64-encoded and decoded entirely in the browser before being offered as a download. No file data is sent to any external endpoint. Care should be taken to ensure that `file_data` in logs does not inadvertently expose sensitive content to unauthorized users viewing logs. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * File content blocks are now rendered and viewable within message histories * Files attached to messages can be downloaded directly from log entries * File details including name, type, and size are displayed alongside message content <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary When replaying reasoning items through the OpenAI Responses API (e.g. Codex/GPT-5.5), the `content` field on a reasoning message can arrive as a string (notably an empty `""` after round-tripping). OpenAI types `reasoning.content` as an array of `reasoning_text` blocks and rejects a string value with `"expected an array ... got a string"`. This fix normalizes string content on outbound reasoning messages: empty strings are dropped entirely, and non-empty strings are promoted to a `reasoning_text` block. ## Changes - In `ToOpenAIResponsesRequest`, when a reasoning message has `Content.ContentStr` set, the string is either dropped (if empty) or converted to a `ResponsesMessageContentBlock` with type `reasoning_text` (if non-empty). The reassignment operates on the local value copy to avoid mutating the caller's input. - Tests cover both the empty-string drop case (including a marshal check to ensure `"content":""` never appears in the serialized output) and the non-empty string promotion case. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/openai/... -run TestToOpenAIResponsesRequest_ReasoningStringContent -v ``` Expected: both subtests (`empty string content is dropped` and `non-empty string content becomes a reasoning_text block`) pass, and the marshalled output does not contain `"content":""` on any reasoning item. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No auth, secrets, or PII implications. The fix only affects how reasoning message content is serialized before being sent to the OpenAI API. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Fixed handling of reasoning message content when formatted as strings in OpenAI responses * Fixed JSON serialization of empty content fields to ensure API compliance * **Chores** * Updated Go module dependencies across core, framework, and plugin packages for performance and security improvements * **Tests** * Added test coverage for reasoning content normalization and empty content marshaling <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary OpenAI's Responses API rejects requests containing a `summary` field on compaction input items with an "Unknown parameter" error. Because Bifrost has no dedicated compaction item model, `encrypted_content` is carried via the embedded `*ResponsesReasoning` struct, which re-injects `"summary": null` during marshaling due to the absence of `omitempty`. This PR strips the `summary` field from compaction items post-serialization while leaving it intact on reasoning items, where it is required by OpenAI. ## Changes - Added `ResponsesMessageTypeCompaction` constant to the `ResponsesMessageType` enum. - Introduced `stripCompactionItemSummary`, which uses `sjson.DeleteBytes` to remove the `summary` key from any serialized item whose type is `compaction`. - Wired `stripCompactionItemSummary` into both marshaling paths inside `OpenAIResponsesRequestInput.MarshalJSON` (the fast path and the `CacheControl` copy path). - Added `github.com/tidwall/sjson` as a dependency for targeted JSON key deletion without full re-deserialization. - Added `TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped` to verify that compaction items have `summary` removed and `encrypted_content` retained, while sibling reasoning items keep their `summary` array. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/openai/... -run TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped -v go test ./... ``` The new test asserts: - Index 0 (compaction item): no `summary` key present, `encrypted_content` key present. - Index 1 (reasoning item): `summary` key present with value `[]`. ## Breaking changes - [x] No ## Security considerations No auth, secrets, or PII implications. The change only affects JSON serialization of compaction items before they are sent to the OpenAI API. ## 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
* azure dignotstic property strip for claude models * restrict fallbacks and provider selection to vk boundry * capture resolved provider from the loadbalancer for logging * logs adds support for rendering file attachments * Update logChatMessageView.tsx * openai integration content string handling * handles compaction message type * coderabbit yml changes --------- Signed-off-by: Akshay Deo <akshay@akshaydeo.com>
## Summary Adds support for xAI's native `x_search` tool in the Responses API. Previously, the tool-type filter in `filterUnsupportedTools` would strip `x_search` from requests routed to xAI because it wasn't part of the OpenAI spec. This change propagates the originating provider through the request pipeline so provider-specific tools can be selectively allowed. ## Changes - Added `ResponsesToolTypeXSearch` (`"x_search"`) as a recognized `ResponsesToolType` constant - Defined `ResponsesToolXSearch` struct with all optional xAI-specific fields: `allowed_x_handles`, `excluded_x_handles`, `from_date`, `to_date`, `enable_image_understanding`, `enable_video_understanding` - Wired `ResponsesToolXSearch` into `ResponsesTool` marshal/unmarshal logic - Added a `Provider` field (tagged `json:"-"`) to `OpenAIResponsesRequest` so the originating provider is available during filtering without being serialized to the wire - Populated `Provider` from `BifrostResponsesRequest` in `ToOpenAIResponsesRequest` - Updated `filterUnsupportedTools` to allow `x_search` when the provider is `xAI`, keeping it stripped for all other providers - Added six integration tests covering non-streaming and streaming `x_search` usage with various parameter combinations (no params, `allowed_x_handles`, date ranges, all params combined) ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh # Core go test ./... # Integration tests (requires xAI API key) cd tests/integrations/python pytest tests/test_openai.py -k "xai_x_search" -v ``` The integration tests route through Bifrost's `/openai` endpoint using a model prefixed with `xai/`. Each test asserts that `custom_tool_call` output items (named `x_semantic_search` or `x_keyword_search`) appear in the response, confirming the tool was not filtered and xAI executed the search. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The `Provider` field is tagged `json:"-"` and is never serialized to the outbound request. No credentials or PII are introduced. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for X search tool (x_search) in Responses API and enabled xAI model integration with configurable params: allowed handles, date ranges, and image/video understanding. * **Bug Fixes** * Prevents validation errors when no tools remain by clearing tool choice for empty-tool requests. * **Tests** * Added comprehensive integration and end-to-end tests (non-streaming and streaming) covering x_search scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
Azure OpenAI deployment-based routes encode the model identifier in the URL path as `deployments/{deployment}` rather than in the request body. Without recognising this path segment, model extraction would fail for Azure passthrough requests, causing the deployment name to be lost.
## Changes
- Added `"deployments"` as a recognised path segment in `extractModelFromPath`, alongside `"models"` and `"tunedModels"`, so that Azure OpenAI routes like `/openai/deployments/my-gpt4o/chat/completions` correctly resolve `my-gpt4o` as the model identifier.
- Added `TestExtractModelFromPath` covering GenAI (`models`/`tunedModels` with `:action` suffixes), Vertex fully-qualified publisher paths, Azure `deployments/{deployment}` paths, and edge cases with no model segment.
- Added `TestExtractPassthroughModel` verifying that the path-extracted value takes precedence over the body model, with the body model used as a fallback when the path contains no model segment — the expected behaviour for Azure deployment routes where the body typically omits `"model"`.
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs
## How to test
```sh
go test ./transports/bifrost-http/integrations/...
```
The two new test functions `TestExtractModelFromPath` and `TestExtractPassthroughModel` directly exercise the changed logic. Confirm all cases pass, particularly:
- `azure deployment chat`: expects `my-gpt4o` extracted from `/openai/deployments/my-gpt4o/chat/completions`
- `azure deployment path overrides empty body`: expects `my-gpt4o` when body model is empty
- `deployments with no trailing segment`: expects `""` when no deployment name follows the segment
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
## Security considerations
None. This change only affects URL path parsing for model name extraction and introduces no new auth, secret handling, or external surface area.
## 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
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved Azure OpenAI integration support.
* **Tests**
* Added test coverage for model extraction and routing logic.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Model configs previously applied globally to all traffic. This PR introduces a `scope` / `scope_id` system that allows model-level rate limits and budgets to be pinned to a specific virtual key, so per-VK model limits can be enforced independently of (and in addition to) the global model limits. ## Changes - Added `scope` (default `"global"`) and `scope_id` (nullable) columns to `governance_model_configs` via a new idempotent migration. Existing rows are backfilled to `"global"` and the unique index is swapped from `(model_name, provider)` to `(scope, scope_id, model_name, provider)`. - Introduced `ModelConfigScopeGlobal` and `ModelConfigScopeVirtualKey` constants and a `BeforeSave` hook that validates and normalises scope/scope_id on every write. - Extended `GetModelConfig` to accept `scope` and `scopeID` as lookup parameters so the identity check on create is scope-aware. - Added `CheckVirtualKeyScopedModelBudget` / `CheckVirtualKeyScopedModelRateLimit` and their corresponding `UpdateVirtualKeyScopedModel*UsageInMemory` methods to `GovernanceStore`. These are wired into `EvaluateVirtualKeyRequest` (resolver) and `UsageTracker.UpdateUsage` so scoped limits are both enforced pre-request and incremented post-response. - In-memory store keys are namespaced by scope via `modelConfigStoreKey`, preventing global and scoped configs from colliding. `DeleteVirtualKeyInMemory` now evicts scoped model configs (and their owned budgets/rate-limits) when a VK is removed; `DeleteVirtualKey` does the same on the DB side. - `GenerateModelConfigHash` now includes `scope` and `scope_id` so config.json ↔ DB drift detection works correctly for scoped configs. - The `getModelConfigs` HTTP handler gains a `?from_memory=true` shortcut and enriches all responses with a transient `scope_name` field (resolved VK name) for UI display. - `CreateModelConfigRequest` accepts `scope` / `scope_id`; the handler validates the scope value, enforces that `virtual_key` scope references an existing VK, and returns scope-aware conflict messages. - UI: the model-limit sheet gains a Scope selector and a virtual-key combobox (shown only for the `virtual_key` scope). The model-limits table gains a Scope column. `getScopeLabel` is extracted to a shared `ui/lib/utils/labels.ts` and the routing-rules views are updated to import from there. - `.zed/` added to `.gitignore`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./plugins/governance/... # UI cd ui pnpm i pnpm build ``` 1. Start the server against a fresh or existing DB — the migration runs automatically and backfills existing model configs to `scope = "global"`. 2. Create a virtual key, then create a model config with `scope = "virtual_key"` and `scope_id = <vk_id>` via the UI or `POST /api/governance/model-configs`. 3. Send requests using that virtual key for the configured model and verify the scoped rate limit / budget is enforced independently of the global model config. 4. Delete the virtual key and confirm the scoped model config and its owned budget/rate-limit rows are removed. ## Breaking changes - [ ] Yes - [x] No The migration is additive and fully backward-compatible. Existing model configs default to `"global"` scope and behave identically to before. ## Related issues ## Security considerations `scope_id` for the `virtual_key` scope is validated against the VK table on creation, preventing configs from being attached to non-existent keys. `scope_id` carries no FK constraint in the DB, so the explicit cascade cleanup on VK deletion is required to avoid orphaned rows — this is implemented both in the DB layer and the in-memory store. ## Checklist - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Model limits and rate limits can be scoped to individual virtual keys; UI supports selecting scope and picking a virtual key. API accepts scope/scope_id on create. * **UI** * Model limits table shows a Scope column and friendly scope labels; forms include Scope + Virtual Key selector. * **Behavior / Bug Fixes** * Deleting a virtual key removes its scoped model configs and related owned records. Conflicts now consider scope/scope_id. * **Tests & Migrations** * Added tests and a migration to introduce and backfill model-config scoping. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
Provider-level governance (budget and rate limit) has been migrated from `config_providers.budget_id / rate_limit_id` into `governance_model_configs` as wildcard rows with `scope='global'`, `model_name='*'`, and `provider=<name>`. This makes `governance_model_configs` the single source of truth for all governance enforcement, eliminating the separate provider-governance enforcement path.
## Changes
- **Database migration** (`migrationMigrateProviderGovernanceToModelConfigs`): Folds existing provider-level budget/rate-limit FK references into new `(global, *, <provider>)` model config rows, reusing the same budget/rate-limit rows. Provider FKs are then nulled out. The migration is idempotent and includes a rollback path.
- **`ModelConfigAllModels = "*"` sentinel**: Introduced as a named constant to represent "all models" in a model config row. The `"*"` sentinel is excluded from catalog-based model name normalization.
- **`collectModelConfigsFor`**: New helper that resolves all applicable model configs for a request across four tiers — exact model+provider, exact model (all providers), all models on this provider (`*:provider`), and all models on all providers (`*:nil`) — deduped by config ID. All budget/rate-limit check and usage-tracking paths now use this helper instead of duplicating two-tier lookup logic.
- **`modelConfigEntityKey`**: New helper that builds a stable entity key for a model config, replacing ad-hoc `fmt.Sprintf` strings scattered across check and update functions.
- **`GetProviderGovernanceModelConfigs`**: New store method that queries the wildcard model config rows backing provider governance, with budget and rate-limit preloads.
- **`DeleteProvider` cleanup**: When a provider is deleted, its associated wildcard model configs (and their owned budget/rate-limit rows) are now cleaned up as part of the transaction.
- **Provider governance HTTP handlers**: `getProviderGovernance`, `updateProviderGovernance`, and `deleteProviderGovernance` are rewritten to operate on wildcard model config rows instead of provider FK columns. The GET endpoint gains a `from_memory` query parameter to serve data from the in-memory governance store.
- **UI**: The model limits table renders `"*"` as `"All Models"`. The model limit sheet exposes an `allowAllOption` on the model selector. RTK Query cache tags are cross-invalidated so that changes to model configs refresh the provider governance view and vice versa.
## Type of change
- [ ] Bug fix
- [x] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (React)
- [ ] Docs
## How to test
```sh
# Core/Transports
go test ./framework/configstore/... ./plugins/governance/...
# UI
cd ui
pnpm i
pnpm build
```
1. Start with a deployment that has providers with existing `budget_id` or `rate_limit_id` values. After the migration runs, verify those providers have `budget_id = NULL` and `rate_limit_id = NULL`, and that corresponding `(global, *, <provider>)` rows exist in `governance_model_configs` pointing to the same budget/rate-limit IDs.
2. Re-run the migration and confirm no duplicate wildcard rows are created (idempotency).
3. Make a request through a provider that had governance configured and confirm the budget/rate-limit is still enforced.
4. Use `GET /api/governance/providers` and `GET /api/governance/providers?from_memory=true` and confirm both return the expected provider governance data.
5. Use `PUT` and `DELETE` on `/api/governance/providers/{name}` and confirm the wildcard model config rows are created, updated, or removed accordingly, and that the Model Limits UI reflects the change without a manual refresh.
6. Delete a provider and confirm its wildcard model config and owned budget/rate-limit rows are removed.
## Breaking changes
- [x] Yes
- [ ] No
Provider governance is no longer stored in `config_providers.budget_id / rate_limit_id`. Any code or tooling that reads governance directly from the providers table will no longer find it there. All governance enforcement and management must go through `governance_model_configs`. The HTTP API surface is unchanged; the migration handles existing data automatically.
## Security considerations
Budget and rate-limit rows are reused (not duplicated) during migration. The rollback path restores FK references to the provider table and removes the wildcard model config rows, leaving no orphaned governance rows in either direction.
## Checklist
- [x] 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
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **New Features**
* Enabled governance policy configuration for "All Models" on specific providers, allowing budget and rate-limit controls at the provider level.
* **UI Improvements**
* Model limits table now displays "All Models" label for improved readability.
* Added "All Models" selection option in model name picker for creating provider-scoped policies.
* **Tests**
* Added comprehensive test coverage for provider-scoped all-models governance functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
…s table (maximhq#3939) ## Summary This PR migrates virtual key (VK) governance (budgets and rate limits) from being owned directly by VK and provider-config rows into VK-scoped all-models wildcard model configs. It also upgrades model configs from a single `budget_id` FK to a `has-many` `Budgets` relationship via `TableBudget.ModelConfigID`, enabling multiple budgets with distinct reset windows on a single model config. ## Changes - **New `governance_budgets.model_config_id` column**: Adds a `ModelConfigID` FK on `TableBudget`, making model configs the owner of budgets rather than the reverse. Three new migrations handle the column addition, backfill from the legacy `budget_id`, VK governance cutover, and a new `calendar_aligned` column on model configs. - **VK governance folded into wildcard model configs**: VK top-level budgets/rate-limits move to a `(scope=virtual_key, model_name='*', provider=NULL)` model config; per-provider-config budgets/rate-limits move to `(scope=virtual_key, model_name='*', provider=<provider>)` configs. `syncVKGovernanceToModelConfigs` and `upsertVKWildcard` handle create/update; `hydrateVKGovernance` / `hydrateVKListGovernance` reverse-map them back onto VK responses for display. - **Multi-budget enforcement**: `CheckModelBudget`, `CheckVirtualKeyScopedModelBudget`, `UpdateProviderAndModelBudgetUsageInMemory`, and `UpdateVirtualKeyScopedModelBudgetUsageInMemory` now iterate `mc.Budgets` instead of reading a single `BudgetID`. All budget checks block if any one budget is exceeded. - **`CollectApplicableGovernanceIDs` rewrite**: Uses `collectModelConfigsFor` across all four tiers (exact model+provider, model-only, all-models+provider, all-models wildcard) and the full VK scope chain, replacing the previous partial lookup. - **`DeleteModelConfig` / `DeleteVirtualKey` / `DeleteProvider` cleanup**: Now preload and delete all owned budgets (via `ModelConfigID`) in addition to the legacy single `BudgetID`. - **`UpdateModelConfig` association safety**: Uses `Omit(clause.Associations)` on save to prevent cascading saves from clobbering live budget usage counters. - **`calendar_aligned` on model configs**: Propagated from the owning VK for VK-scoped configs; stamped onto owned budgets via `AfterFind` and `rebuildInMemoryStructures` so the reset path reads the correct window. - **API shape changes**: `CreateModelConfigRequest.budget` → `budgets []CreateBudgetRequest`; `UpdateModelConfigRequest.budget` → `budgets []CreateBudgetRequest` (full desired set, reconciled server-side). `reconcileModelConfigBudgets` handles upsert/delete of the set. - **UI**: Model limit sheet replaced the single budget field with a `MultiBudgetLines` component. The model limits table now shows all budgets per config, a "Scope Target" column with a deep-link to the VK page, and calendar-alignment labels. VK table gains deep-link support via a `?vk=` query param consumed from the model limits table. - **Cache invalidation**: VK create/update/delete mutations now also invalidate `ModelConfigs`; model config mutations invalidate `VirtualKeys`; provider governance mutations invalidate `VirtualKeys`. - **Tests**: New unit tests cover multi-budget enforcement (one exceeded blocks, all within passes, all budgets bumped on usage update), no-double-count guard for VK governance budgets, and multi-budget VK-scoped model config blocking. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./plugins/governance/... ./transports/bifrost-http/... # UI cd ui pnpm i pnpm build ``` 1. Create a VK with top-level budgets and per-provider budgets via the API or UI. 2. Verify that `GET /api/governance/virtual-keys/:id` returns the budgets and rate limits hydrated from the VK-scoped wildcard model configs. 3. Verify that `GET /api/governance/model-configs` shows the VK-scoped wildcard rows with their owned budgets. 4. Make requests through the VK and confirm usage is charged to the wildcard model config budgets exactly once (not double-counted via both the VK hierarchy and scoped-model paths). 5. Delete the VK and confirm no orphaned budget or rate-limit rows remain. 6. In the UI, open Model Limits, confirm multi-budget lines render and the Scope Target column links to the correct VK. ## Breaking changes - [x] Yes - [ ] No The `CreateModelConfigRequest` and `UpdateModelConfigRequest` API shapes change: the single `budget` field is replaced by a `budgets` array. Callers using the single-budget field must migrate to the array form. Existing database rows are backfilled automatically by the migrations; the legacy `budget_id` column and `Budget` association are retained as inert for backward compatibility. ## Related issues ## Security considerations No new auth surfaces or secrets handling. Budget ownership is enforced via a `BeforeSave` hook that rejects a budget row with more than one owner FK set, preventing accidental cross-owner budget sharing. ## Checklist - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Model configs now support multiple budgets for more granular control over API usage * Virtual-key governance is now organized within model configurations * Added deep-link support for virtual keys via URL parameter * Model limits table now displays "Scope Target" with clickable navigation * **Tests** * Added multi-budget governance test coverage <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary This PR generalizes the virtual-key-scoped model governance system into a scope-agnostic framework, enabling any registered scope (e.g. `user`) to carry per-scope model rate limits and budgets — not just virtual keys. It also introduces a runtime scope registry on both the backend and frontend so downstream (enterprise) builds can extend the system without modifying OSS code. ## Changes - **`CheckVirtualKeyScopedModelBudget` / `CheckVirtualKeyScopedModelRateLimit`** and their `UpdateVirtualKeyScoped*` counterparts are replaced by `CheckScopedModelBudget`, `CheckScopedModelRateLimit`, `UpdateScopedModelBudgetUsageInMemory`, and `UpdateScopedModelRateLimitUsageInMemory`. These accept a `(scope, scopeID)` pair instead of a `*TableVirtualKey`, making them scope-agnostic. An empty scope or scopeID is a no-op. - **`ModelConfigScopeUser`** constant added to `tables/modelconfig.go`, along with a `RegisterModelConfigScope` function and a `sync.RWMutex`-guarded registry so downstream builds can add scopes at startup without forking the OSS validation logic. - **`EvaluateUserRequest`** in `resolver.go` and `UpdateUsage` in `tracker.go` now invoke the scoped model check/update paths for the `user` scope, mirroring the existing VK-scoped block. - **`DeleteProvider`** in `rdb.go` is refactored to batch-delete budgets and rate limits with `IN` clauses instead of one-by-one, and the model config row is deleted before its owned resources to avoid constraint issues. - **`DeleteVirtualKey`** removes the loop that deleted budgets via `ModelConfigID`; only the `BudgetID` foreign key path is retained. - Internal naming throughout `migrations.go`, `governance.go`, and `store.go` drops the "wildcard" terminology (`ensureVKWildcardModelConfig` → `ensureVKModelConfig`, `vkWildcardDesired` → `vkModelConfigDesired`, `upsertVKWildcard` → `reconcileVKModelConfig`, etc.) to reflect that these configs are not exclusively wildcard rows. - **`RegisterScopeNameResolver`** added to `handlers/governance.go` with a package-level `sync.RWMutex`-guarded map. `resolveModelConfigScopeName` now dispatches to the registered resolver for any scope rather than hard-coding the VK lookup. The VK resolver is wired automatically in `NewGovernanceHandler`. - **UI scope registry** (`ui/lib/registries/modelLimitScopes.tsx`) replaces the static `MODEL_LIMIT_SCOPES` constant. Each entry can declare a `PickerComponent` and a `buildDeepLink` function. The OSS build registers `global` and `virtual_key` at module load; enterprise builds extend the registry via the `@enterprise` alias side-effect import. - The Model Limit sheet's VK picker is replaced by a registry-driven `PickerComponent` render, and the deep-link navigation in the table is driven by `buildDeepLink`, so adding a new scope (e.g. `user`) requires no changes to OSS sheet or table code. - `invalidatesTags` for model config mutations now includes `"Users"` and `"UserGovernance"` (no-op in OSS; picked up by enterprise tag wiring). ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./plugins/governance/... ./transports/bifrost-http/... # UI cd ui pnpm i pnpm build ``` Existing governance tests in `modelprovidergovernance_test.go` have been updated to call the new `CheckScopedModel*` / `UpdateScopedModel*` signatures and continue to cover the VK-scoped budget and rate-limit paths. ## Breaking changes - [x] Yes - [ ] No The `GovernanceStore` interface methods `CheckVirtualKeyScopedModelBudget`, `CheckVirtualKeyScopedModelRateLimit`, `UpdateVirtualKeyScopedModelBudgetUsageInMemory`, and `UpdateVirtualKeyScopedModelRateLimitUsageInMemory` are removed and replaced by their scope-agnostic equivalents. Any downstream implementation of `GovernanceStore` must be updated to implement `CheckScopedModelBudget`, `CheckScopedModelRateLimit`, `UpdateScopedModelBudgetUsageInMemory`, and `UpdateScopedModelRateLimitUsageInMemory`. ## Security considerations The new `RegisterModelConfigScope` and `RegisterScopeNameResolver` functions are intended to be called once at process startup before serving requests. No user-supplied input reaches the registry directly. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added user-scoped model budget and rate-limit enforcement. * Added calendar-aligned reset support for model limits. * Implemented dynamic model-limit scope registry with scope-specific pickers and deep-linking. * **Bug Fixes** * Improved bulk cleanup for provider- and virtual-key-scoped model configs. * Ensured consistent calendar-alignment when creating scoped model configs. * **Refactor** * Migrated virtual-key governance to model-config backed storage. * Generalized scoped model checks/usage to a (scope, scope_id) model for reuse. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…aximhq#3962) ## Summary Adds `scope` and `provider` filter support to the Model Limits (Model Configs) list, allowing users to narrow results by scope (e.g. `global`, `virtual_key`) and provider (e.g. `openai`) independently of the existing search filter. ## Changes - Added `Scope` and `Provider` fields to `ModelConfigsQueryParams` and wired them into the RDB query as exact-match `WHERE` clauses. - Extended the HTTP handler to read `scope` and `provider` query parameters and apply them on both the in-memory (non-paginated) path and the paginated database path. - Added `scope` and `provider` to `GetModelConfigsParams` and passed them through the RTK Query API call, including query string serialization. - Updated the optimistic cache update on model config creation to skip inserting a newly created config into cached query results when its `scope` or `provider` does not match the active filter arguments. - Added Scope and Provider `<Select>` dropdowns to the model limits toolbar. The scope options are sourced from the existing `getModelLimitScopes` registry; provider options are sourced from the providers API with icons and labels. - Added a "Clear filters" button that appears when any filter (search, scope, or provider) is active and resets all three at once. - Pagination offset resets to 0 when scope or provider filters change, consistent with existing search behavior. ## 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) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./transports/bifrost-http/... # UI cd ui pnpm i pnpm build ``` 1. Navigate to the Model Limits page. 2. Use the **Scope** dropdown to select a scope (e.g. `global`) — only model configs with that scope should appear. 3. Use the **Provider** dropdown to select a provider (e.g. `openai`) — only model configs for that provider should appear. 4. Combine scope and provider filters together and verify results are correctly intersected. 5. Verify the **Clear filters** button resets all three filters and restores the full list. 6. Verify pagination resets to page 1 when either filter changes. 7. Create a new model config while a scope/provider filter is active and confirm it only appears in the cached list if it matches the active filters. ## Screenshots/Recordings _Add before/after screenshots of the model limits toolbar showing the new Scope and Provider dropdowns._ ## Breaking changes - [ ] Yes - [x] No ## Related issues Link related issues and discussions. ## Security considerations The new `scope` and `provider` query parameters are passed as parameterized query arguments (`WHERE scope = ?`, `WHERE provider = ?`), preventing SQL injection. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added scope and provider filtering capabilities to model configuration listings * UI now includes dropdown controls for filtering by scope and provider * Filters work alongside existing search functionality for comprehensive model discovery <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…maximhq#3981) ## Summary Provider governance previously supported only a single budget per provider. This PR upgrades the system to support multiple budgets per provider, adds a `calendar_aligned` flag to control whether budgets reset on a fixed calendar cycle or roll from creation date, and maintains full backward compatibility with the legacy single-`budget` field. ## Changes - Added a `budgets` field (`*[]CreateBudgetRequest`) to `UpdateProviderGovernanceRequest` alongside the existing `budget` field (now deprecated). Sending both fields in the same request returns a 400 error. - Introduced `coerceLegacyBudget` to convert a single `UpdateBudgetRequest` into a `*[]CreateBudgetRequest`, enabling the legacy `budget` field to be handled uniformly through `reconcileModelConfigBudgets`. - `ProviderGovernanceResponse` now includes a `budgets []TableBudget` slice (all budgets) in addition to the deprecated `budget *TableBudget` (first entry, kept for backward compatibility), and exposes `calendar_aligned`. - `modelConfigToProviderGovernance` now populates `Budgets` as a defensive copy and surfaces `CalendarAligned`. - The `updateProviderGovernance` transaction was refactored: per-budget deletion now iterates `mc.Budgets` directly, and budget lifecycle management is delegated entirely to `reconcileModelConfigBudgets` when `effectiveBudgets` is non-nil. - The UI governance form replaced the single budget input with a `MultiBudgetLines` component supporting multiple budget entries, and added a `calendar_aligned` toggle that appears only when at least one budget is configured. - The provider governance table now renders one `MetricCard` per budget window instead of a single card. - `ProviderGovernance` and `UpdateProviderGovernanceRequest` TypeScript types updated to reflect `budgets`, `calendar_aligned`, and `CreateBudgetRequest` usage; the deprecated `budget` field is retained with a comment. - Unit tests added for `coerceLegacyBudget`, `modelConfigToProviderGovernance` new fields, and the mutual-exclusion validation between `budget` and `budgets`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./transports/bifrost-http/handlers/... # UI cd ui pnpm i pnpm test pnpm build ``` **Manual validation:** 1. `POST /api/governance/providers/{provider}` with `{"budgets": [{"max_limit": 100, "reset_duration": "1d"}, {"max_limit": 500, "reset_duration": "1w"}]}` — verify both budgets are persisted and returned. 2. `POST` with both `budget` and `budgets` set — verify a 400 is returned with a message mentioning `budget`. 3. `POST` with the legacy `{"budget": {"max_limit": 100, "reset_duration": "1d"}}` — verify backward compatibility: budget is created and returned in both `budget` and `budgets` fields. 4. `POST` with `{"calendar_aligned": true}` — verify the flag is persisted and reflected in the GET response. 5. In the UI, add multiple budget lines for a provider, toggle calendar alignment, save, and confirm the current usage section renders one card per budget. ## Breaking changes - [ ] Yes - [x] No The `budget` field is deprecated but still accepted and returned. Clients using only `budget` continue to work without modification. ## Related issues ## Security considerations No new authentication, secrets, or PII handling introduced. The mutual-exclusion check on `budget`/`budgets` prevents ambiguous requests from reaching the database layer. ## 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 applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Provider governance now supports multiple budgets (add/remove-all semantics) and a calendar-aligned toggle; UI and metrics display per-budget usage and exhaustion. Model configs can reference multiple budget IDs for config-driven governance. - **Tests** - Added unit tests for legacy-budget coercion, multi-budget response shaping, and mutual-exclusion validation for update requests. - **Documentation** - Config schema updated: deprecated single-budget reference and introduced budget_ids array. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Adds a new **Model Limits** documentation page and updates related API schemas and access profile docs to expose the unified model-level governance interface in Bifrost. This gives users a single place to understand and configure spending caps and rate limits scoped globally, per virtual key, or per user — keyed on a specific model or all models. ## Changes - Added `docs/features/governance/model-limits.mdx` — a full reference page covering the scope system, Web UI walkthrough, REST API usage, `config.json` configuration, worked examples, and how limits interact at request time. - Registered the new page in `docs/docs.json` under the governance navigation group. - Extended `docs/enterprise/access-profiles.mdx` with a section explaining how to attach per-model limits to individual users without modifying the shared access profile template, including a step-by-step guide and a `curl` example. - Updated the `listModelConfigs` OpenAPI operation to reflect pagination query parameters (`limit`, `offset`, `search`, `scope`, `provider`, `from_memory`) and renamed the summary to "List model limits". - Updated `ModelConfig`, `ListModelConfigsResponse`, `CreateModelConfigRequest`, `UpdateModelConfigRequest`, `ProviderGovernanceResponse`, and `UpdateProviderGovernanceRequest` schemas to add `scope`, `scope_id`, `scope_name`, `calendar_aligned`, and `budgets` (array) fields. The singular `budget` field is retained but marked deprecated for backward compatibility. `count` is renamed to `total_count`. ## 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 1. Navigate to the Bifrost docs site and confirm **Model Limits** appears under **Budget & Limits** in the governance sidebar. 2. Verify all internal links resolve — particularly the cross-references between `access-profiles.mdx` and `model-limits.mdx`. 3. Confirm the OpenAPI spec renders the new query parameters for `GET /api/governance/model-configs` and that the updated schema fields (`scope`, `budgets`, `total_count`, etc.) appear correctly. ## Breaking changes - [ ] Yes - [x] No The `budget` (singular) field and `count` field are preserved in the schema for backward compatibility. Consumers relying on `count` should migrate to `total_count`. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added comprehensive Model Limits documentation covering configuration, API usage, and scope management. * Added guidance on overriding Access Profiles with per-user model limits for Enterprise users. * Updated API documentation for model limits endpoints with pagination and filtering parameters. * Updated API schemas to reflect multi-budget support, scoping options, and calendar-aligned reset capabilities. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Adds `scope` and `scope_id` fields to `governance.modelConfigs` (Helm) / `governance.model_configs` (config.json), enabling model-level budgets and rate limits to be applied either globally across all traffic or scoped to a specific virtual key. Documentation is updated to reflect the new fields, rename the section to "Model Limits" for consistency with the UI, and provide richer examples covering global, provider-level, and VK-scoped configurations. ## Changes - Added `scope` (`"global"` | `"virtual_key"`, default `"global"`) and `scope_id` (required when `scope` is `"virtual_key"`) to `modelConfigs[]` items in `values.yaml` and `values.schema.json`. - Renamed the Helm docs section from "Model Configs" to "Model Limits" to match the **Budget & Limits → Model Limits** UI label. - Expanded the Helm and config.json documentation with a field reference table and multiple annotated examples (global model cap, provider-level budget, VK-scoped top-level budget, VK-scoped per-provider budget). - Added a `2.1.21` changelog entry to the Helm chart README describing the new fields. ## 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 Verify the Helm schema accepts the new fields without validation errors: ```sh helm lint helm-charts/bifrost helm template bifrost helm-charts/bifrost --values helm-charts/bifrost/values.yaml ``` Confirm a `modelConfigs` entry with `scope: "virtual_key"` and a valid `scope_id` renders correctly in the generated manifests, and that an entry with `scope: "global"` (or no `scope`) also renders without errors. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No new auth, secrets, or PII surface introduced. The `scope_id` field references an existing virtual key ID and does not expose any new sensitive data. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated governance documentation with a new Model Limits section detailing budget and rate-limit configuration per model. * Added scoping capabilities allowing limits to apply globally or to specific virtual keys. * Updated configuration examples and schema references with new fields. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…3991) ## Summary A new `allow_private_network` configuration option has been added to `NetworkConfig`, giving operators explicit control over whether provider connections may target RFC 1918 private IP ranges (e.g. `10.x`, `172.16.x`, `192.168.x`). This is required for deployments where providers run on a Kubernetes pod network, LAN, or private VPC. Loopback addresses (`localhost`, `127.0.0.1`, `::1`) are now unconditionally permitted. Link-local addresses (`169.254.x.x`, `fe80::`) and unspecified addresses (`0.0.0.0`, `::`) are always blocked regardless of this setting, protecting cloud instance metadata endpoints. ## Changes - Added `AllowPrivateNetwork bool` to `NetworkConfig` in `core/schemas/provider.go`, with full JSON marshal/unmarshal support. - Added `IsLinkLocal` to `core/network/utils.go` with a dedicated `169.254.0.0/16` subnet check for IPv4 and `IsLinkLocalUnicast()` for IPv6, separating link-local blocking from the general private-IP check. - Updated `ConfigureDialer` in `core/providers/utils/utils.go` to accept `allowPrivateNetwork bool`. Unspecified and link-local IPs are always rejected; loopback is always allowed; RFC 1918 ranges are only rejected when `allowPrivateNetwork` is `false`. - Removed the `IsLocalhost` pre-check from `ValidateExternalURL` in `core/utils.go`. Loopback addresses now pass validation. Unspecified and link-local addresses are checked explicitly with distinct error messages. - Updated `ValidateExternalURL` signature to accept `allowPrivateNetwork bool` and propagated the flag through all call sites in the HTTP transport handlers. - Passed `config.NetworkConfig.AllowPrivateNetwork` to `ConfigureDialer` across all provider constructors (Anthropic, Azure, Bedrock, Cerebras, Cohere, ElevenLabs, Fireworks, Gemini, Groq, HuggingFace, Mistral, Nebius, Ollama, OpenAI, OpenRouter, Parasail, Perplexity, Replicate, Runway, SGL, Vertex, vLLM, xAI). - Added `allow_private_network` to `transports/config.schema.json` in both provider definition locations. - Added an "Allow Private Network" toggle to the provider network configuration form in the UI (`networkFormFragment.tsx`), with updated type definitions and Zod schemas. - Removed workarounds in dialer tests that pre-set `client.Dial` to bypass SSRF protection for `httptest` servers on `127.0.0.1`. Tests now exercise the direct connection path. Added a `without_existing_dial` sub-test to `TestConfigureDialer_TCPKeepAliveEnabled`. Updated SSRF test cases to reflect new error message strings (`link-local IP`, `unspecified IP`) and removed loopback cases that are no longer blocked. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./core/providers/utils/... ``` Configure a provider with `"allow_private_network": true` and a `base_url` pointing to a private IP (e.g. `http://10.0.0.5:8000`). Verify the URL passes `ValidateExternalURL` and connections succeed. Verify that `http://169.254.169.254` is still rejected with a `link-local IP addresses are not allowed` error even when `allow_private_network` is `true`. Verify that `http://localhost:11434` passes validation regardless of the setting. ## Breaking changes - [x] Yes - [ ] No `ValidateExternalURL` now takes a second `allowPrivateNetwork bool` argument. Any direct callers outside this repository must be updated. Additionally, loopback addresses (`localhost`, `127.0.0.1`, `::1`) are no longer rejected by `ValidateExternalURL` or `ConfigureDialer`. Deployments that relied on loopback blocking as a security boundary should note this change. ## Security considerations Link-local addresses (`169.254.x.x`, `fe80::`) are split out into a dedicated `IsLinkLocal` check and are **always** blocked, even when `allow_private_network` is `true`. This ensures cloud instance metadata endpoints (AWS `169.254.169.254`, Azure IMDS, GCP metadata) cannot be reached under any operator configuration. Unspecified addresses (`0.0.0.0`, `::`) are also always blocked. RFC 1918 private ranges are only reachable when an operator explicitly sets `allow_private_network: true`, making the security trade-off an intentional, visible configuration decision rather than a silent bypass. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `allow_private_network` configuration option to network settings, enabling providers to connect to RFC1918 private IP ranges when explicitly enabled. * Loopback addresses (localhost, 127.0.0.1, ::1) are now always permitted, regardless of settings. * **Bug Fixes** * Improved SSRF protection: link-local and unspecified IPs remain unconditionally blocked for security, while private IP blocking is now configurable. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Akshay Deo <akshay@akshaydeo.com>
…d log error response body on 4xx/5xx (maximhq#3985) ## Summary Improves HTTP request logging by resolving the true client IP from reverse-proxy headers and including error response bodies in logs for failed requests. ## Issues Closes maximhq#3904 ## Changes - Added a `clientIP` helper that extracts the originating client IP by checking `X-Forwarded-For` (taking the leftmost entry in comma-separated lists), then `X-Real-IP`, and finally falling back to the direct peer address. This ensures accurate client identification when Bifrost sits behind a reverse proxy. - Replaced the direct `ctx.RemoteAddr().String()` call in the `http.remote_addr` log field with `clientIP(ctx)` so logs reflect the real client rather than the proxy. - Added logging of the response body as `http.error` for any request that results in a 4xx or 5xx status code, making it easier to diagnose failures from logs alone. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` - Deploy Bifrost behind a reverse proxy that sets `X-Forwarded-For` or `X-Real-IP` headers and verify that `http.remote_addr` in logs reflects the originating client IP rather than the proxy address. - Trigger a request that returns a 4xx or 5xx response and confirm the `http.error` field appears in the log output with the response body. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The `clientIP` helper trusts `X-Forwarded-For` and `X-Real-IP` headers as provided by upstream proxies. If Bifrost is exposed directly to the internet without a trusted reverse proxy, these headers could be spoofed by clients, resulting in inaccurate IP logging. Ensure Bifrost is always deployed behind a trusted proxy when relying on these values. ## 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 applicablecs
📝 WalkthroughWalkthroughAdds an optional per-key AWS ChangesBedrock AWS Profile & SSO Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/lib/schemas/providerForm.ts (1)
111-142:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisallow
profilewhen static Bedrock keys are provided.
profileis accepted even when bothaccess_keyandsecret_keyare set, which permits ambiguous auth config that conflicts with the Bedrock profile semantics in this PR.Suggested fix
const BedrockKeyConfigSchema = z .object({ access_key: z.string(), secret_key: z.string(), session_token: z.string().optional(), region: z.string().min(1, "Region is required for Bedrock keys"), - profile: z.string().optional(), + profile: z.string().trim().optional(), role_arn: z.string().optional(), external_id: z.string().optional(), session_name: z.string().optional(), arn: z.string().optional(), batch_s3_config: BatchS3ConfigSchema.optional(), }) .refine( (data) => { const accessKey = data.access_key?.trim() || ""; const secretKey = data.secret_key?.trim() || ""; const bothEmpty = accessKey === "" && secretKey === ""; const bothProvided = accessKey !== "" && secretKey !== ""; + const profile = data.profile?.trim() || ""; // Either both empty (IAM role auth) or both provided (explicit credentials) if (!bothEmpty && !bothProvided) { return false; } + + // Profile-based auth is only valid when static keys are not provided. + if (bothProvided && profile !== "") { + return false; + } // Check for session token when using IAM role path (both keys empty) const sessionToken = data.session_token?.trim() || ""; if (bothEmpty && sessionToken !== "") { return false; } return true; }, { - message: "For Bedrock: either provide both Access Key and Secret Key, or leave both empty for IAM role authentication", - path: ["access_key"], + message: + "For Bedrock: either provide both Access Key and Secret Key, or leave both empty for IAM role/profile authentication; profile cannot be combined with static keys", + path: ["profile"], }, );🤖 Prompt for 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. In `@ui/lib/schemas/providerForm.ts` around lines 111 - 142, The refine predicate on the schema (the function that validates access_key, secret_key, and session_token) must also reject a non-empty profile when static credentials are provided: if access_key and secret_key are both provided (access_key !== "" && secret_key !== "") then ensure data.profile is empty/undefined and return false if it isn’t; update the validation error to indicate “profile must be omitted when using explicit Access Key and Secret Key” (adjust the message/path from access_key to profile or add a second error) so the check in the refine with symbols access_key, secret_key, session_token, and profile enforces that profile cannot be set alongside static Bedrock keys.core/providers/bedrock/bedrock.go (1)
2901-2915:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThread AssumeRole settings into
uploadToS3.This upload path only forwards access key / secret / session token / profile, so it cannot honor
BedrockKeyConfig.RoleARN,ExternalID, orRoleSessionName. Every other S3/Bedrock request in this file signs viasignAWSRequest(...), which does assume the configured role. A key that relies onprofile + role_arnwill therefore upload the JSONL file with the source credentials and then create the batch job with assumed-role credentials, which commonly fails with S3 authorization errors on the input object. Please extenduploadToS3to take the role fields as well, or route this upload through the same role-aware signing helper used by the rest of the provider.🤖 Prompt for 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. In `@core/providers/bedrock/bedrock.go` around lines 2901 - 2915, The uploadToS3 call currently only forwards AccessKey/SecretKey/SessionToken/Profile and therefore ignores BedrockKeyConfig.RoleARN, ExternalID, and RoleSessionName, causing uploads to use source creds instead of the assumed role; update the upload path to either accept the role-related fields (RoleARN, ExternalID, RoleSessionName) on uploadToS3 and have uploadToS3 perform STS AssumeRole to produce temporary creds before uploading, or reuse the existing signAWSRequest helper to sign the S3 PUT with assumed-role credentials the same way other functions do (ensure you reference uploadToS3, BedrockKeyConfig.RoleARN/ExternalID/RoleSessionName, and signAWSRequest so the code routes S3 uploads through role-aware signing).
🤖 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 `@core/providers/bedrock/signer.go`:
- Around line 274-295: signAWSRequestFastHTTP currently loads credentials via
config.LoadDefaultConfig but ignores RoleARN/ExternalID/RoleSessionName so it
skips AssumeRole behavior that signAWSRequest (in
core/providers/bedrock/bedrock.go) supports; extract the credential resolution
logic into a shared role-aware helper (e.g. resolveAWSCredentialsWithRole or
getAWSCredentials) that accepts ctx, region, profile, optional RoleARN,
ExternalID, RoleSessionName and returns the resolved
accessKey/secretKey/sessionToken or an error, implement AssumeRole using STS
when RoleARN is provided, and update both signAWSRequestFastHTTP and
signAWSRequest to call that helper instead of directly calling
config.LoadDefaultConfig so both fasthttp and net/http signers honor role-based
profiles consistently.
---
Outside diff comments:
In `@core/providers/bedrock/bedrock.go`:
- Around line 2901-2915: The uploadToS3 call currently only forwards
AccessKey/SecretKey/SessionToken/Profile and therefore ignores
BedrockKeyConfig.RoleARN, ExternalID, and RoleSessionName, causing uploads to
use source creds instead of the assumed role; update the upload path to either
accept the role-related fields (RoleARN, ExternalID, RoleSessionName) on
uploadToS3 and have uploadToS3 perform STS AssumeRole to produce temporary creds
before uploading, or reuse the existing signAWSRequest helper to sign the S3 PUT
with assumed-role credentials the same way other functions do (ensure you
reference uploadToS3, BedrockKeyConfig.RoleARN/ExternalID/RoleSessionName, and
signAWSRequest so the code routes S3 uploads through role-aware signing).
In `@ui/lib/schemas/providerForm.ts`:
- Around line 111-142: The refine predicate on the schema (the function that
validates access_key, secret_key, and session_token) must also reject a
non-empty profile when static credentials are provided: if access_key and
secret_key are both provided (access_key !== "" && secret_key !== "") then
ensure data.profile is empty/undefined and return false if it isn’t; update the
validation error to indicate “profile must be omitted when using explicit Access
Key and Secret Key” (adjust the message/path from access_key to profile or add a
second error) so the check in the refine with symbols access_key, secret_key,
session_token, and profile enforces that profile cannot be set alongside static
Bedrock keys.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 8cf94831-6095-4dc6-bba2-9e941d350a95
📒 Files selected for processing (17)
core/providers/bedrock/bedrock.gocore/providers/bedrock/s3.gocore/providers/bedrock/signer.gocore/schemas/account.godocs/openapi/schemas/management/providers.yamldocs/providers/supported-providers/bedrock.mdxframework/configstore/clientconfig.goframework/configstore/encryption_test.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.gotransports/bifrost-http/lib/config.gotransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/lib/schemas/providerForm.tsui/lib/types/schemas.ts
Confidence Score: 5/5Safe to merge; all plumbing is additive and follows established patterns, the new field is optional, and no existing paths are altered. Every changed layer — credential resolution, signing, S3 upload, GORM persistence, encryption, redaction, UI, config schema — follows the exact same pattern used when No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "feat(bedrock): add `profile` field for A..." | Re-trigger Greptile |
…t at minimum sync interval (maximhq#4023) ## Summary The sync worker ticker period was set to 1 hour, which created a subtle scheduling bug: when `pricingSyncInterval` is set near the minimum supported value, the few seconds a sync takes to complete causes the next ticker wake-up to land just under the elapsed-time threshold, effectively doubling the actual sync cadence. Reducing the ticker period to 5 minutes ensures the check granularity stays well below the minimum supported `pricingSyncInterval`, preventing ticker drift from defeating the threshold check. ## Changes - Reduced `syncWorkerTickerPeriod` from 1 hour to 5 minutes so that ticker drift (caused by sync execution time) does not push the next wake-up just under the `pricingSyncInterval` threshold and inadvertently double the effective sync interval. - Updated the scheduling model comment to accurately describe the relationship between the ticker period and `pricingSyncInterval`, removing the outdated note that implied the 1-hour ticker was a hard lower bound on sync frequency. ## 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 go test ./framework/modelcatalog/... ``` Set `pricingSyncInterval` to a value near `MinimumPricingSyncIntervalSec` and verify that syncs occur at the expected cadence without doubling. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Increased frequency of pricing synchronization checks from hourly intervals to 5-minute intervals, improving update timeliness and overall system reliability across different configurations. * **Documentation** * Updated internal documentation clarifying the pricing synchronization scheduling mechanism and how timing parameters influence sync behavior. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/4023?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Adds a per-key `profile` field on `bedrock_key_config` that selects a named profile from `~/.aws/config` / `~/.aws/credentials` (including SSO profiles via the SDK's built-in ssocreds provider). Used when `access_key`/`secret_key` are empty; threads through `aws-sdk-go-v2`'s default credential chain via `config.WithSharedConfigProfile`. Composes with `role_arn`: the profile's resolved credentials become the source identity for STS AssumeRole. This unblocks per-key multi-AWS-account load balancing for users on corporate SSO without requiring static keys or process-wide `AWS_PROFILE` env vars. Mirrors the plumbing pattern from maximhq#1882 (which added role_arn / external_id / session_name): adds the schema field, threads it through all three signing paths (signAWSRequest, signAWSRequestFastHTTP, uploadToS3), adds the GORM column with migration, JSON schema + OpenAPI, UI form + types/zod, redaction, docs, and a round-trip persistence test. Also includes the profile name in the AssumeRole creds cache key so two profiles assuming the same role do not collide. Signed-off-by: Andre Kurait <akurait@amazon.com>
8397278 to
ce97aab
Compare
|
Thanks @coderabbitai and @greptile-apps for the careful review. Pushed What changed in this revision1. Shared role-aware credential resolver (CodeRabbit's main concern about Extracted
The two paths can no longer drift on AssumeRole / profile semantics — they share one resolver. 2. UI: profile gated to IAM Role tab + Zod refines
3. Docs: redundant SSO cache mount removed
Why I did not extract a "real" caller for
|
|
Tip For best results, initiate chat on the files or code changes.
The The rationale for keeping Looks good to me — changes are well-reasoned, verified, and documented. (∩`-´)⊃━☆゚.*・。゚ 🧠 Learnings used |
|
Both fixes look correct and complete. On the UI concern (my comment): The profile field is now properly gated — it only renders under On CodeRabbit's The rationale for keeping LGTM on this revision. Tip: You can customize Greptile's behavior for this repo with |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/providers/bedrock/bedrock.go (1)
2930-2943:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the effective batch role for the inline S3 upload.
Line 2938 passes
key.BedrockKeyConfigdirectly intouploadToS3, butBatchCreatemay override the role withrequest.ExtraParams["role_arn"]and then submits the job with that effective role on Line 2960. When those differ, the JSONL upload is signed with one identity and the batch job runs as another, which can break cross-account buckets withAccessDeniedor leave the input object under the wrong principal.Suggested fix
- if bifrostErr := uploadToS3( - ctx, - key.BedrockKeyConfig, + uploadCfg := key.BedrockKeyConfig + if roleArn != "" && (uploadCfg == nil || uploadCfg.RoleARN == nil || uploadCfg.RoleARN.GetValue() != roleArn) { + copied := schemas.BedrockKeyConfig{} + if uploadCfg != nil { + copied = *uploadCfg + } + copied.RoleARN = schemas.Ptr(schemas.EnvVar{Val: roleArn}) + uploadCfg = &copied + } + if bifrostErr := uploadToS3( + ctx, + uploadCfg, region, bucket, s3Key, jsonlData, ); bifrostErr != nil {🤖 Prompt for 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. In `@core/providers/bedrock/bedrock.go` around lines 2930 - 2943, The upload is currently signed with key.BedrockKeyConfig but BatchCreate can override the execution role via request.ExtraParams["role_arn"], so change the code to resolve an "effective" BedrockKeyConfig (apply the role_arn override from request.ExtraParams when present) and pass that effective config into uploadToS3 instead of key.BedrockKeyConfig; ensure the same effective config/credentials used to sign the JSONL upload match the credentials used when submitting the batch job (BatchCreate/SubmitJob) so uploads and job execution use the same principal.
🤖 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 `@docs/providers/supported-providers/bedrock.mdx`:
- Around line 1733-1738: The Docker example currently includes the `-e
AWS_PROFILE` env var which may confuse readers about per-key profiles; update
the example to either remove `-e AWS_PROFILE` or add a one-line clarifying
comment after the docker command explaining that `-e AWS_PROFILE` is for
process-wide profile selection (used when relying on the default credential
chain) while per-key selection is configured via `bedrock_key_config.profile` in
the AWS Profile / SSO section—reference `-e AWS_PROFILE` and
`bedrock_key_config.profile` in the comment so readers know which to use.
In `@transports/config.schema.json`:
- Around line 2437-2440: The schema currently allows mixing "profile" with
static Bedrock credentials in the "bedrock_key_config" object; change the JSON
schema for bedrock_key_config to forbid combinations by adding a
mutually-exclusive constraint (e.g., use "oneOf" or "anyOf" with explicit
alternatives) so either the profile variant (required "profile" and disallowed
"access_key"/"secret_key") or the static variant (required "access_key" and
"secret_key" and no "profile") is valid; update the bedrock_key_config
definition to include these alternatives and proper "required"/"not" clauses so
the validator rejects configs that include both profile and static keys.
---
Outside diff comments:
In `@core/providers/bedrock/bedrock.go`:
- Around line 2930-2943: The upload is currently signed with
key.BedrockKeyConfig but BatchCreate can override the execution role via
request.ExtraParams["role_arn"], so change the code to resolve an "effective"
BedrockKeyConfig (apply the role_arn override from request.ExtraParams when
present) and pass that effective config into uploadToS3 instead of
key.BedrockKeyConfig; ensure the same effective config/credentials used to sign
the JSONL upload match the credentials used when submitting the batch job
(BatchCreate/SubmitJob) so uploads and job execution use the same principal.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: cf199efb-a7e8-47f3-8603-a40f6fdcb2f7
📒 Files selected for processing (17)
core/providers/bedrock/bedrock.gocore/providers/bedrock/s3.gocore/providers/bedrock/signer.gocore/schemas/account.godocs/openapi/schemas/management/providers.yamldocs/providers/supported-providers/bedrock.mdxframework/configstore/clientconfig.goframework/configstore/encryption_test.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.gotransports/bifrost-http/lib/config.gotransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/lib/schemas/providerForm.tsui/lib/types/schemas.ts
| docker run -p 8080:8080 \ | ||
| -e AWS_PROFILE \ | ||
| -v ~/.aws:/root/.aws:ro \ | ||
| -v "$PWD/config.json:/app/config.json" \ | ||
| maximhq/bifrost | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider clarifying the Docker example context.
The Docker example shows -e AWS_PROFILE within the AWS Profile / SSO subsection, which documents the per-key profile field. Earlier text (line 678) clearly distinguishes process-wide AWS_PROFILE from per-key bedrock_key_config.profile. While technically correct (both can coexist), showing -e AWS_PROFILE here might confuse readers about whether they need to set both the environment variable AND the profile field when using per-key profiles.
Consider either:
- Adding a brief comment explaining when to use
-e AWS_PROFILEvs theprofilefield, or - Omitting
-e AWS_PROFILEfrom this example since the focus is on per-key profile selection
📝 Example with clarifying comment
docker run -p 8080:8080 \
-v ~/.aws:/root/.aws:ro \
-v "$PWD/config.json:/app/config.json" \
maximhq/bifrost
# Note: Add `-e AWS_PROFILE` only if you need process-wide profile selection
# for keys using the default credential chain (authentication method 2).
# Per-key profile selection uses bedrock_key_config.profile instead.🤖 Prompt for 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.
In `@docs/providers/supported-providers/bedrock.mdx` around lines 1733 - 1738, The
Docker example currently includes the `-e AWS_PROFILE` env var which may confuse
readers about per-key profiles; update the example to either remove `-e
AWS_PROFILE` or add a one-line clarifying comment after the docker command
explaining that `-e AWS_PROFILE` is for process-wide profile selection (used
when relying on the default credential chain) while per-key selection is
configured via `bedrock_key_config.profile` in the AWS Profile / SSO
section—reference `-e AWS_PROFILE` and `bedrock_key_config.profile` in the
comment so readers know which to use.
| "profile": { | ||
| "type": "string", | ||
| "description": "Named profile from ~/.aws/config or ~/.aws/credentials (including SSO profiles). Used when access_key/secret_key are empty. Composes with role_arn (profile credentials become the source identity for AssumeRole). Can use env. prefix" | ||
| }, |
There was a problem hiding this comment.
Reject mixed profile and static Bedrock credentials in the schema.
Line 2439 documents profile as valid only when access_key/secret_key are empty, but bedrock_key_config currently accepts all three together. That lets config.json bypass the new UI validation and silently fall back to static creds, so the configured profile is ignored.
Proposed schema guard
"bedrock_key_config": {
"type": "object",
"properties": {
"access_key": {
"type": "string",
"description": "AWS access key (can use env. prefix)"
},
"secret_key": {
"type": "string",
"description": "AWS secret key (can use env. prefix)"
},
"session_token": {
"type": "string",
"description": "AWS session token (can use env. prefix)"
},
"region": {
"type": "string",
"description": "AWS region"
},
"arn": {
"type": "string",
"description": "AWS ARN"
},
"role_arn": {
"type": "string",
"description": "AWS IAM role ARN for AssumeRole (can use env. prefix)"
},
"external_id": {
"type": "string",
"description": "External ID for AssumeRole (can use env. prefix)"
},
"session_name": {
"type": "string",
"description": "Role session name for AssumeRole (can use env. prefix)"
},
"profile": {
"type": "string",
"description": "Named profile from ~/.aws/config or ~/.aws/credentials (including SSO profiles). Used when access_key/secret_key are empty. Composes with role_arn (profile credentials become the source identity for AssumeRole). Can use env. prefix"
},
"deployments": {
"type": "object",
"additionalProperties": {
"type": "string"
},
"description": "Model to deployment mappings"
},
"batch_s3_config": {
"type": "object",
"description": "S3 bucket configuration for Bedrock batch operations",
"properties": {
"buckets": {
"type": "array",
"description": "List of S3 bucket configurations",
"items": {
"type": "object",
"properties": {
"bucket_name": {
"type": "string",
"description": "S3 bucket name"
},
"prefix": {
"type": "string",
"description": "S3 key prefix for batch files"
},
"is_default": {
"type": "boolean",
"description": "Whether this is the default bucket for batch operations"
}
},
"required": ["bucket_name"],
"additionalProperties": false
}
}
},
"additionalProperties": false
}
},
+ "allOf": [
+ {
+ "not": {
+ "anyOf": [
+ { "required": ["profile", "access_key"] },
+ { "required": ["profile", "secret_key"] }
+ ]
+ }
+ }
+ ],
"required": ["region"],
"additionalProperties": false
}As per coding guidelines, transports/config.schema.json is the source of truth for config fields.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "profile": { | |
| "type": "string", | |
| "description": "Named profile from ~/.aws/config or ~/.aws/credentials (including SSO profiles). Used when access_key/secret_key are empty. Composes with role_arn (profile credentials become the source identity for AssumeRole). Can use env. prefix" | |
| }, | |
| "bedrock_key_config": { | |
| "type": "object", | |
| "properties": { | |
| "access_key": { | |
| "type": "string", | |
| "description": "AWS access key (can use env. prefix)" | |
| }, | |
| "secret_key": { | |
| "type": "string", | |
| "description": "AWS secret key (can use env. prefix)" | |
| }, | |
| "session_token": { | |
| "type": "string", | |
| "description": "AWS session token (can use env. prefix)" | |
| }, | |
| "region": { | |
| "type": "string", | |
| "description": "AWS region" | |
| }, | |
| "arn": { | |
| "type": "string", | |
| "description": "AWS ARN" | |
| }, | |
| "role_arn": { | |
| "type": "string", | |
| "description": "AWS IAM role ARN for AssumeRole (can use env. prefix)" | |
| }, | |
| "external_id": { | |
| "type": "string", | |
| "description": "External ID for AssumeRole (can use env. prefix)" | |
| }, | |
| "session_name": { | |
| "type": "string", | |
| "description": "Role session name for AssumeRole (can use env. prefix)" | |
| }, | |
| "profile": { | |
| "type": "string", | |
| "description": "Named profile from ~/.aws/config or ~/.aws/credentials (including SSO profiles). Used when access_key/secret_key are empty. Composes with role_arn (profile credentials become the source identity for AssumeRole). Can use env. prefix" | |
| }, | |
| "deployments": { | |
| "type": "object", | |
| "additionalProperties": { | |
| "type": "string" | |
| }, | |
| "description": "Model to deployment mappings" | |
| }, | |
| "batch_s3_config": { | |
| "type": "object", | |
| "description": "S3 bucket configuration for Bedrock batch operations", | |
| "properties": { | |
| "buckets": { | |
| "type": "array", | |
| "description": "List of S3 bucket configurations", | |
| "items": { | |
| "type": "object", | |
| "properties": { | |
| "bucket_name": { | |
| "type": "string", | |
| "description": "S3 bucket name" | |
| }, | |
| "prefix": { | |
| "type": "string", | |
| "description": "S3 key prefix for batch files" | |
| }, | |
| "is_default": { | |
| "type": "boolean", | |
| "description": "Whether this is the default bucket for batch operations" | |
| } | |
| }, | |
| "required": ["bucket_name"], | |
| "additionalProperties": false | |
| } | |
| } | |
| }, | |
| "additionalProperties": false | |
| } | |
| }, | |
| "allOf": [ | |
| { | |
| "not": { | |
| "anyOf": [ | |
| { "required": ["profile", "access_key"] }, | |
| { "required": ["profile", "secret_key"] } | |
| ] | |
| } | |
| } | |
| ], | |
| "required": ["region"], | |
| "additionalProperties": false | |
| } |
🤖 Prompt for 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.
In `@transports/config.schema.json` around lines 2437 - 2440, The schema currently
allows mixing "profile" with static Bedrock credentials in the
"bedrock_key_config" object; change the JSON schema for bedrock_key_config to
forbid combinations by adding a mutually-exclusive constraint (e.g., use "oneOf"
or "anyOf" with explicit alternatives) so either the profile variant (required
"profile" and disallowed "access_key"/"secret_key") or the static variant
(required "access_key" and "secret_key" and no "profile") is valid; update the
bedrock_key_config definition to include these alternatives and proper
"required"/"not" clauses so the validator rejects configs that include both
profile and static keys.
e389df7 to
a65fce4
Compare
fa15f50 to
ca190fc
Compare
ac30a53 to
7c66b20
Compare
44564de to
493bff0
Compare
|
Hi @AndreKurait — thanks for the contribution! This PR is currently blocked because our CLA bot shows the Contributor License Agreement as not yet signed. Could you sign it here so we can move this forward: https://cla-assistant.io/maximhq/bifrost?pullRequest=4030 Let us know if you run into any issues signing. |
244a01d to
ce1b2a6
Compare
Summary
Adds a per-key
profilefield onbedrock_key_configthat selects a named profile from~/.aws/config/~/.aws/credentials(including AWS SSO profiles via the SDK's built-inssocredsprovider). Used whenaccess_key/secret_keyare empty; threads throughaws-sdk-go-v2's default credential chain viaconfig.WithSharedConfigProfile. Composes withrole_arn— the profile's resolved credentials become the source identity for STS AssumeRole.This unblocks per-key multi-AWS-account load balancing for users on corporate SSO without requiring static keys or process-wide
AWS_PROFILEenv vars.Closes #4029
Changes
Profile *EnvVarfield onBedrockKeyConfig(core/schemas/account.go).profileparameter andWithSharedConfigProfilebranch to:signAWSRequestincore/providers/bedrock/bedrock.go(net/http path; 14 call sites updated)signAWSRequestFastHTTPincore/providers/bedrock/signer.go(fasthttp path)uploadToS3incore/providers/bedrock/s3.go(batch path; caller in bedrock.go updated)assumeRoleCredsCachekey (profile:<name>source identity) so two profiles assuming the same role do not collide.BedrockProfileGORM column onTableKey, copy inBeforeSave/AfterFind, encryption pair, included inBedrockKeyConfigreconstruction (framework/configstore/tables/key.go); newmigrationAddBedrockProfileColumn(framework/configstore/migrations.go); copy inrdb.go(4 sites); cleared invirtualkey.goredaction.profileproperty intransports/config.schema.jsonanddocs/openapi/schemas/management/providers.yaml(also adds the previously-missingrole_arn/external_id/session_nameentries for the OpenAPI schema, mirroring [Bug]: Published JSON schema missing role_arn, session_name, external_id in bedrock_key_config #2484's class of fix).clientconfig.goandtransports/bifrost-http/lib/config.go.docs/providers/supported-providers/bedrock.mdx(renumbers AssumeRole to section 4) with config examples, multi-account load-balance example, SSO token expiry note, Docker volume-mount guidance, and region-precedence note. New row in thebedrock_key_configfield table.TestBeforeSave_DoesNotMutateSharedProviderConfigsinframework/configstore/encryption_test.goto exercise the round-trip (no-mutation + DB persistence) of the newProfilefield.Mirrors the plumbing pattern from #1882 (which added
role_arn/external_id/session_name).Type of change
Affected areas
How to test
End-to-end smoke test (multi-account):
~/.aws/config(e.g. corporate SSO):aws sso login --sso-session my-ssoconfig.jsoncontaining two Bedrock keys (one per profile, same model inmodels[]).curl http://localhost:8080/anthropic/v1/messages -d '{"model":"bedrock/anthropic.claude-3-5-sonnet-20241022-v2:0", ...}'— observe weighted-random selection between accounts.-H "x-bf-api-key: <key-name>"for deterministic routing.I do not have a corporate SSO setup to validate the SSO leg end-to-end; static-key + AssumeRole paths are unchanged and covered by existing tests.
Breaking changes
The new
profilefield is optional; existingbedrock_key_configblobs deserialize unchanged. The newbedrock_profileGORM column is added bymigrationAddBedrockProfileColumn.signAWSRequest/signAWSRequestFastHTTP/uploadToS3are package-private, so the new positional arg is contained.Related issues
Closes #4029
Related: #1882 (added STS AssumeRole fields), #268 (added default credential chain fallback)
Security considerations
Profileis treated as a secret-like field for consistency with other Bedrock credential fields: encrypted at rest inTableKeyviaencryptEnvVarPtr, redacted inclientconfig.goandlib/config.go. Profile names themselves aren't secret, but treating them uniformly avoids accidental leaks (e.g. if a profile name encodes account/team identifiers).WithSharedConfigProfilereads from the user's filesystem (~/.aws/config,~/.aws/credentials,~/.aws/sso/cache). When running under Docker, operators must explicitly mount these — documented in the newbedrock.mdxsection.profile:<name>so cross-profile/role collisions are impossible.~/.aws/sso/cache) and lifetime are managed entirely byaws-sdk-go-v2'sssocredsprovider; this PR does not introduce custom token handling.Checklist
docs/contributing/README.mdand followed the guidelinesgo buildclean acrosscore,framework, andtransports(with localreplacedirectives that I reverted before commit). UI not built locally; existing CI covers it.Summary by CodeRabbit
New Features
~/.aws/configinstead of explicit access keys.Documentation
UI Updates