fix: add content type in provider response header filter and harness fixes - #4024
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds ExtractPassthroughProviderResponseHeaders (which preserves Content-Type), marks content-type as excluded by default in the generic filter, updates all providers (Anthropic, Azure, Gemini, OpenAI, Vertex) to use the passthrough extractor in both Passthrough and PassthroughStream, and updates related tests and provider-harness data. ChangesPassthrough Response Header Filtering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/e2e/api/collections/provider-harness.json`:
- Around line 479-480: The "10 latest" matrices contain duplicate model entries
(e.g., "gemini/gemini-2.5-flash" and "gemini/gemini-2.5-flash-lite" are
repeated); update the matrix so each model ID appears only once by removing the
repeated entries and replacing them with the missing unique models to restore
true "10 latest" coverage (search for the arrays that include
"gemini/gemini-2.5-flash", "gemini/gemini-2.5-flash-lite" and the later
duplicate "gemini" entries and deduplicate them so the list contains ten
distinct model IDs).
🪄 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: 03329a23-cc74-4f69-8717-be991706cb84
📒 Files selected for processing (8)
core/providers/anthropic/anthropic.gocore/providers/azure/azure.gocore/providers/gemini/gemini.gocore/providers/openai/openai.gocore/providers/utils/dialer_test.gocore/providers/utils/utils.gocore/providers/vertex/vertex.gotests/e2e/api/collections/provider-harness.json
Confidence Score: 5/5Safe to merge — the core header-filtering change is well-scoped, all five implementing providers are updated consistently, and the new function is covered by a targeted regression test. The change is narrowly targeted: a new header extractor function, a one-line blocklist addition, and mechanical call-site updates. The new test correctly validates content-type passthrough, transport-header stripping, and secret-header stripping. The dialer test fix aligns with the actual implementation. No correctness, concurrency, or security issues were found. tests/e2e/api/collections/provider-harness.json contains duplicate harness entries introduced by this PR (gemini-2.5-flash/lite appear twice in the Gemini section, vertex/gemini-2.5-flash appears twice in the Vertex section) — these are redundant rather than broken, but the gaps in 2.0 model coverage should be addressed. Important Files Changed
Reviews (2): Last reviewed commit: "fix: add content type in provider respon..." | Re-trigger Greptile |
f506e85 to
c647276
Compare
Merge activity
|
…fixes (#4024) ## Summary Passthrough endpoints (`Passthrough` and `PassthroughStream`) were stripping `Content-Type` from provider response headers before forwarding them to callers. Since passthrough is meant to transparently relay provider responses, `Content-Type` must be preserved so clients can correctly interpret the response body. ## Changes - Introduced `ExtractPassthroughProviderResponseHeaders`, a variant of `ExtractProviderResponseHeaders` that retains `Content-Type` while still filtering out transport-level headers (e.g., `content-encoding`, `transfer-encoding`, `connection`). - Added `content-type` to the `providerResponseFilterHeaders` blocklist used by the standard `ExtractProviderResponseHeaders`, so non-passthrough paths continue to strip it. - Switched all `Passthrough` and `PassthroughStream` implementations across Anthropic, Azure, Gemini, OpenAI, and Vertex providers to use the new `ExtractPassthroughProviderResponseHeaders`. - Updated the `ErrConnectionClosed` retry test expectation to `wantRetry: true` to correctly reflect intended retry behavior. - Replaced deprecated `gemini-2.0-flash` and `gemini-2.0-flash-lite` test harness entries with `gemini-2.5-flash` and `gemini-2.5-flash-lite`, and fixed a copy-paste error in the Azure embedding test path. ## 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/... go test ./... ``` Send a passthrough request to any supported provider and verify that the `Content-Type` header (e.g., `application/json`) is present in the response forwarded by Bifrost. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only affects which response headers are forwarded to the caller on passthrough paths. ## 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** * Passthrough responses now consistently preserve Content-Type and forward appropriate response headers across all AI providers, improving downstream compatibility. * **Tests** * Updated test collections with latest Gemini/Vertex model identifiers and corrected Azure embeddings model path. * Added regression tests for passthrough header forwarding and adjusted connection-retry expectations. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/4024?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 -->
## Summary This PR bumps the Go toolchain version from `1.26.3` to `1.26.4` across all modules and CI workflows, and cuts a new release (`core` v1.5.17, `framework` v1.3.17, `transports` v1.5.9, `plugins/compat` v0.1.16, `plugins/governance` v1.5.17, and associated plugin versions) incorporating a large batch of features and fixes accumulated since the previous release. ## Changes - **Go 1.26.4** — Updated `go-version` in all GitHub Actions workflows (`e2e-tests`, `helm-release`, `pr-tests`, `release-cli`, `release-pipeline`, `snyk`) and all `go.mod` files (core, framework, transports, cli, all plugins, examples, and test modules). - **Core (v1.5.17)** — OpenAI compaction support, multi-customer logs and usage tracking, multiple team/business unit support, `request_headers` wildcard pattern capture for OTel and Maxim plugins, xAI `x_search` tool, fetch URL validation with SSRF hardening, `file://` pricing URL scheme, virtual key provider fan-out filtering, and a broad set of fixes including Anthropic prompt cache key, empty thinking block stripping, OpenAI stream usage event cleanup, Gemini numeric schema constraints, stale connection retries, Azure Claude diagnostic strip, and passthrough budget handling. - **Framework (v1.3.17)** — Scope-aware budgets and limits wired from model configs, provider-level governance, multiple customer budget support with `calendar_aligned` windows, paginated virtual key fetch, `config.json` source-of-truth flow, FTS index cap reduction, sync worker drift fix, cascade deletes for model configs, and high-scale virtual key flow improvements. - **Transports (v1.5.9)** — Full changelog covering all of the above plus UI improvements (log navigation, customer detail sheet, `BudgetDisplay` component, inline loading shell, materialized view alias), SCIM provisioning fields, Helm/config schema additions (`roles`, `per_user_oauth`), client IP resolution from forwarded headers, and dependency upgrades (`recharts` to 3.8.1, `golang.org/x` CVE remediation). - **Plugins** — `governance` v1.5.17 adds team budget/rate-limit exporters, ghost node reconciliation fix, and VK double usage counting fix; `logging` v1.5.17 adds wildcard header capture and file attachment rendering; `otel` v1.2.17 adds `disable_content_logging` and multiple collectors support; `maxim` v1.6.17 adds `request_headers` wildcard capture; `compat` v0.1.16 fixes `max_tokens` preservation during param filtering. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify Go version go version # should report go1.26.4 # Run core tests cd core && go test ./... # Run framework tests cd framework && go test ./... # Run transports tests cd transports && go test ./... # Run plugin tests cd plugins/governance && go test ./... cd plugins/logging && go test ./... cd plugins/otel && go test ./... # UI cd ui pnpm i pnpm build pnpm test ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues #4053, #4066, #4041, #4012, #3976, #3947, #3991, #4045, #3957, #3938, #3937, #3939, #3981, #3998, #3997, #4092, #4091, #4079, #4080, #4086, #3929, #3994, #4028, #3970, #3919, #3861, #3664, #3999, #4088, #4070, #4051, #4043, #4057, #4023, #3941, #3955, #4024, #3956, #3967, #3925, #3992, #3900 ## Security considerations - Fetch URL validation hardened against SSRF by tightening IP checks for private networks and link-local addresses (#4092, #3947, #3991). - Transitive `golang.org/x` dependencies (crypto, net, sys, text) bumped to address Docker Scout CVEs (#3900). ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OpenAI compaction, multi-customer/team logstore support, request-header wildcard capture, enhanced governance (provider-level & scope-aware limits), disable-content-logging option, support for multiple OpenTelemetry collectors, SSRF hardening and URL validation. * **Chores** * Bumped Go toolchain across modules and updated component/plugin version releases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…fixes (#4024) ## Summary Passthrough endpoints (`Passthrough` and `PassthroughStream`) were stripping `Content-Type` from provider response headers before forwarding them to callers. Since passthrough is meant to transparently relay provider responses, `Content-Type` must be preserved so clients can correctly interpret the response body. ## Changes - Introduced `ExtractPassthroughProviderResponseHeaders`, a variant of `ExtractProviderResponseHeaders` that retains `Content-Type` while still filtering out transport-level headers (e.g., `content-encoding`, `transfer-encoding`, `connection`). - Added `content-type` to the `providerResponseFilterHeaders` blocklist used by the standard `ExtractProviderResponseHeaders`, so non-passthrough paths continue to strip it. - Switched all `Passthrough` and `PassthroughStream` implementations across Anthropic, Azure, Gemini, OpenAI, and Vertex providers to use the new `ExtractPassthroughProviderResponseHeaders`. - Updated the `ErrConnectionClosed` retry test expectation to `wantRetry: true` to correctly reflect intended retry behavior. - Replaced deprecated `gemini-2.0-flash` and `gemini-2.0-flash-lite` test harness entries with `gemini-2.5-flash` and `gemini-2.5-flash-lite`, and fixed a copy-paste error in the Azure embedding test path. ## 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/... go test ./... ``` Send a passthrough request to any supported provider and verify that the `Content-Type` header (e.g., `application/json`) is present in the response forwarded by Bifrost. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only affects which response headers are forwarded to the caller on passthrough paths. ## 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** * Passthrough responses now consistently preserve Content-Type and forward appropriate response headers across all AI providers, improving downstream compatibility. * **Tests** * Updated test collections with latest Gemini/Vertex model identifiers and corrected Azure embeddings model path. * Added regression tests for passthrough header forwarding and adjusted connection-retry expectations. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/4024?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 -->
## ✨ Features - **OpenAI Compaction** — Added OpenAI conversation compaction support across core, framework, logging, and the API surface (#4053) - **Multi-Customer & Org Hierarchy** — Logs and usage tracking now support multiple customers, teams, and business units, including business unit CRUD, team assignment, and governance endpoints in the OpenAPI spec (#4066, #4041, #4082) - **Provider-Level Governance** — Budgets & limits are now scope-aware and can be applied at the virtual-key top level and per provider, wired from the model configs table, with UI filters for scope and providers (#3938, #3937, #3939, #3981, #3962) - **Customer Budgets** — Customers support multiple budgets and `calendar_aligned` budget windows (#3998, #3997) - **Virtual Key Attribution & Controls** — Added a `created_by` user attribution column and a `blacklisted_models` column for virtual key provider configs (#3672, #3653) - **Request Header Capture** — OTel and Maxim observability plugins capture `request_headers` by pattern, with wildcard support (e.g. `x-custom-*`); logging gained the same wildcard header capture (#4012, #3958) - **OTel Content Controls & Collectors** — New `disable_content_logging` option drops message/tool content from exported spans, plus support for multiple OTel collectors (#4064, #3894) - **xAI x_search** — Added xAI `x_search` tool support (#3976) - **URL Validation** — Added fetch URL validation with private-network configuration and link-local blocking (#3947, #3991) - **File Scheme Pricing URLs** — Pricing source URLs now accept the `file://` scheme for air-gapped and self-hosted deployments (#4045) - **Paginated Virtual Keys** — Virtual key fetching is paginated to handle deployments with very large numbers of keys (#3957) - **Client IP Resolution** — Resolve client IP from `X-Forwarded-For`/`X-Real-IP` headers - **SCIM Provisioning** — Added `attributeType`/`attributeValue` SCIM provisioning fields - **Helm/Config Schema** — Added `roles` RBAC governance config and `per_user_oauth` MCP auth to the Helm chart and config schema (#4004, #4009) - **Log Navigation UI** — Added a "View logs" menu item to customer, team, and virtual key tables, clickable links in log detail views, a customer detail sheet, and a reusable `BudgetDisplay` component (#4073, #4054, #4026, #4055) - **Faster First Paint** — Added an inline loading shell to `#root` before React mounts (#4063) - **Materialized View Alias** — Added an `alias` column to the materialized view with filter support (#4078) ## 🐞 Fixed - **Fetch URL IP Checks** — Hardened fetch URL IP checks against SSRF (#4092) - **Mantle Model Matching** — Broadened Mantle model matching to all `gpt` variants (#4091) - **Empty Thinking Blocks** — Strip thinking blocks when the signature is empty (#4079) - **OpenAI Stream Usage** — Removed usage from the `responses.created` event in the OpenAI stream (#4080) - **Prompt Cache Key** — Set the prompt cache key from the Anthropic integration (#4086) - **Upstream Failure Status** — Map upstream connection failures to 502 instead of 400 (#3929) (thanks [@chris-colinsky](https://github.com/chris-colinsky)!) - **Gemini Schema Constraints** — Accept numeric schema integer constraints for Gemini (#3994) (thanks [@yanhao98](https://github.com/yanhao98)!) - **Files Provider Param** — Accept the `?provider=` query param on `GET /v1/files` (#3971) (thanks [@alexef](https://github.com/alexef)!) - **Optional Batch Model** — Made the `model` field optional on `POST /v1/batches` (#3973) (thanks [@alexef](https://github.com/alexef)!) - **Helm Azure Config** — Added missing `azure_key_config` fields to the Helm schema (#3996) (thanks [@axelray-dev](https://github.com/axelray-dev)!) - **Text Completion Chunk Model** — Added the missing `Model` field to `TextCompletionChunkResponse` (#3970) (thanks [@kuishou68](https://github.com/kuishou68)!) - **MCP Inline stdio Env** — MCP stdio server configs accept inline environment variable assignments (#3861) (thanks [@Shushmitaaaa](https://github.com/Shushmitaaaa)!) - **Orphaned Tool Results** — Orphaned tool results in the OpenAI to Anthropic conversion flow are no longer rejected by the Anthropic API (#3919) - **Node Usage Reconciliation** — Added a monotonic `inc_number` log cursor so node usage reconciliation does not skip late async log writes (#3664) - **Bedrock Output Assessments** — Corrected the type of `outputAssessments` in Bedrock responses (#4028) - **Model Pool Pricing Reloads** — Preserve non-pricing model pool entries across pricing reloads (#3999) - **Ghost Node Reconciliation** — Replicate the VK hierarchy flow for ghost node reconciliation (#4088) - **VK Double Usage Counting** — Fixed double usage counting when creating a virtual key (#4070) - **Model Config Lifecycle** — Cascade deletes for model configs and removal of stale in-memory model configs (#4051, #4043) - **FTS Index Cap** — Reduced the FTS index `left()` cap from 800k to 250k chars to stay within the tsvector limit (#4057) - **Sync Worker Drift** — Reduced the sync worker ticker period to 5m to prevent threshold drift (#4023) - **Passthrough** — Fixed passthrough budgets, gated passthrough models per VK, model extraction for Azure passthrough, and restricted fallbacks/provider selection to the VK boundary (#3941, #3988, #3983, #3924) - **Provider Response Headers** — Strip provider response headers and add a content-type filter (#3955, #4024) - **Stream Handling** — Drain non-SSE stream readers and retry stale connections (#3956, #3967) - **Azure Claude** — Strip Azure diagnostic property for Claude models (#3925) - **Compat max_tokens** — Preserve chat `max_tokens` during param filtering (#3992) - **Raw Request Flag** — Removed the raw request flag from providers that don't support it (#4058) - **UI Fixes** — Standardized page container layout, virtual key model configs UI, and dashboard chart tooltips (#4046, #4052, #4044) ## 🔧 Maintenance - **Dependency Upgrades** — Bumped transitive `golang.org/x` dependencies (crypto, net, sys, text) for Docker Scout CVE remediation and `recharts` to 3.8.1; cascaded version bumps across all modules (#3900, #4003)

Summary
Passthrough endpoints (
PassthroughandPassthroughStream) were strippingContent-Typefrom provider response headers before forwarding them to callers. Since passthrough is meant to transparently relay provider responses,Content-Typemust be preserved so clients can correctly interpret the response body.Changes
ExtractPassthroughProviderResponseHeaders, a variant ofExtractProviderResponseHeadersthat retainsContent-Typewhile still filtering out transport-level headers (e.g.,content-encoding,transfer-encoding,connection).content-typeto theproviderResponseFilterHeadersblocklist used by the standardExtractProviderResponseHeaders, so non-passthrough paths continue to strip it.PassthroughandPassthroughStreamimplementations across Anthropic, Azure, Gemini, OpenAI, and Vertex providers to use the newExtractPassthroughProviderResponseHeaders.ErrConnectionClosedretry test expectation towantRetry: trueto correctly reflect intended retry behavior.gemini-2.0-flashandgemini-2.0-flash-litetest harness entries withgemini-2.5-flashandgemini-2.5-flash-lite, and fixed a copy-paste error in the Azure embedding test path.Type of change
Affected areas
How to test
Send a passthrough request to any supported provider and verify that the
Content-Typeheader (e.g.,application/json) is present in the response forwarded by Bifrost.Breaking changes
Related issues
Security considerations
No security implications. This change only affects which response headers are forwarded to the caller on passthrough paths.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
Bug Fixes
Tests