feat(headers): per-provider custom upstream and passthrough header overrides#513
Draft
weselben wants to merge 17 commits into
Draft
feat(headers): per-provider custom upstream and passthrough header overrides#513weselben wants to merge 17 commits into
weselben wants to merge 17 commits into
Conversation
Introduce a HeaderOverrides block on each provider that controls how ingress headers are forwarded upstream. Providers can now declare a blocklist for sensitive headers, explicit add/replace rules, and a list of headers to remove before the request leaves the gateway. This commit is schema-only: the runtime engine and middleware wiring are added in the following commits.
Implement the runtime header override engine and wire it into the proxy path. The engine applies provider-specific block, add, replace, and remove rules to ingress headers before forwarding them upstream. Provider implementations (OpenAI-compatible, OpenRouter, VLLM, and Ollama) now use the engine, and the server passthrough middleware exposes the resolved header set to downstream handlers.
Add unit tests for the header override engine, provider config/factory wiring, and passthrough middleware. Includes coverage for the OpenAI-compatible, OpenRouter, VLLM, Ollama, DeepSeek, Groq, Kimi Code, and xAI provider paths as well as server request snapshot handling.
Add tests for provider-level header override configuration parsing in config/providers_test.go and extend config/config_test.go to cover the new schema fields end-to-end.
Add a feature guide for per-provider custom upstream headers, including blocklist, add, replace, and remove examples. Update config.example.yaml, .env.template, and CLAUDE.md so users can discover and copy the new configuration.
The overlap warning is already fully exercised by TestApplyHeaderOverrides_PassthroughMode_LogsOverlappingStaticHeaders via a dedicated capturingSlogHandler. captureLogger was never called, so remove it and the now-unused bytes import to satisfy golangci-lint's unused check.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds per-provider header override support, filtered passthrough header capture, user-path header wiring, adapter updates, OpenRouter identity precedence, and related configuration and documentation changes. ChangesHeader Overrides Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant EchoMiddleware
participant RequestSnapshotCapture
participant ProviderFactory
participant Provider
participant UpstreamAPI
Client->>EchoMiddleware: HTTP request
EchoMiddleware->>RequestSnapshotCapture: capture snapshot context
RequestSnapshotCapture->>RequestSnapshotCapture: filter passthrough headers
EchoMiddleware->>ProviderFactory: create provider with header settings
ProviderFactory->>Provider: pass HeaderOverrides and UserPathHeader
Client->>Provider: ChatCompletion / Passthrough
Provider->>UpstreamAPI: forward merged headers
UpstreamAPI-->>Client: response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The example comments nested the header fields under a header_overrides block and used the shorthand skip_mode/skip_headers names, neither of which the parser accepts. Flatten the keys to the real schema (passthrough_user_headers, passthrough_user_headers_skip_mode, passthrough_user_headers_skip, custom_upstream_headers) in all three example providers (openai, groq, deepseek), and correct the stale header_overrides wording in .env.template.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/providers/init.go (1)
219-272: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove the passthrough flag behind successful provider creation
anyPassthroughUserHeadersis set beforefactory.Create, so a config withpassthrough_user_headers: truestill turns on header capture even if that provider fails to initialize and never becomes active. Move the check afterCreatesucceeds.🤖 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 `@internal/providers/init.go` around lines 219 - 272, The passthrough-user-headers flag is being recorded before provider creation succeeds, so failed providers can still enable header capture. In initializeProviders, move the HeaderOverrides.PassthroughUserHeaders check to after factory.Create returns successfully, and only then update anyPassthroughUserHeaders for the provider that actually initialized.
🤖 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 `@config/config.example.yaml`:
- Around line 269-279: The provider examples use a nested header_overrides block
with skip_mode and skip_headers, but RawProviderConfig expects flat top-level
fields instead. Update the example YAML in the openai, groq, and deepseek
sections to match the actual schema by moving the header fields to the top level
and using the real field names from config/providers.go
(custom_upstream_headers, passthrough_user_headers,
passthrough_user_headers_skip, passthrough_user_headers_skip_mode). Keep the
examples aligned with TestRawProviderConfig_YAMLHeaderFields so uncommenting
them produces the intended header behavior.
In `@internal/providers/config.go`:
- Around line 590-616: The comma-splitting in parseHeaderMapEnv cannot safely
represent header values that contain commas, so update the handling in
parseHeaderMapEnv/config parsing to use a safer format or separator (for example
JSON or another unambiguous encoding) and document the accepted format clearly
near the CUSTOM_UPSTREAM_HEADERS path. Make sure the new behavior is located
through parseHeaderMapEnv and any callers in config.go so operators don’t
silently get malformed header maps when values include commas.
- Around line 575-588: The parseBoolEnv helper currently fails open by treating
any unrecognized value as true, which can silently enable PassthroughUserHeaders
on bad env input. Update parseBoolEnv to use an explicit true-set and return
false or nil for anything else, so only clearly recognized enable values turn
the flag on. Keep the fix localized to parseBoolEnv in
internal/providers/config.go and preserve its existing trimming/lowercasing
behavior.
In `@internal/providers/headers.go`:
- Around line 169-204: Update the docstring for shouldForward to match the
implemented behavior: the current “Default-closed” wording is stale and
contradicts the default-open logic in the switch for mode "skip" and "" when
skipSet is empty. Rewrite the comment above shouldForward (and the nearby mode
handling note if needed) so it clearly states that empty skip sets forward all
headers except those blocked by IsHeaderBlocked or stripSet, and keep the
description consistent with shouldForward and
TestApplyHeaderOverrides_PassthroughMode_DefaultOpenSkipList.
- Around line 105-122: Update logStaticOverridden in
internal/providers/headers.go so it only warns for passthrough headers that
would actually be forwarded upstream after SkipMode filtering. Instead of
checking overlap against the raw PassthroughHeadersFromContext source alone, run
each candidate through the same shouldForward / allow-skip logic used when
building req.Header and only include names that would survive that filter. Keep
the warning and overlapping_names output, but base both on the effective
forwarded headers rather than the unfiltered source.
In `@internal/providers/kimicode/kimicode_headers_test.go`:
- Around line 55-83: TestNew_AppliesUserPathHeader only proves a request was
sent and does not verify the UserPathHeader behavior. Update the test to capture
request headers in the httptest handler, send a passthrough/user-path header
through the ChatCompletion call, and assert that the configured alias
X-Tenant-Path is not forwarded while the request still succeeds. Use the New
constructor, ProviderOptions.UserPathHeader, and provider.ChatCompletion to
locate the behavior.
In `@internal/providers/xai/xai_test.go`:
- Around line 489-499: The X-Custom-Header assertion in xai_test.go is
conditionally skipped when wantCustom is empty, so the default-case test does
not verify the header is absent. Update the header assertions in the relevant
test case to always check gotHeaders.Get("X-Custom-Header") against the expected
value, using the existing tc.wantCustom field and the surrounding gotHeaders
checks for X-Tenant-Path and X-User-Header as the reference points. Ensure the
no-options/default scenario explicitly expects an empty X-Custom-Header value so
stray headers are caught.
In `@internal/server/http.go`:
- Around line 289-293: The middleware wiring in the HTTP server is
double-processing passthrough headers because `RequestSnapshotCapture` is
already followed by `PassthroughHeaderCapture` when
`cfg.PassthroughUserHeadersEnabled` is true. Update the setup around
`e.Use(...)` in `http.go` so `RequestSnapshotCapture` is always called with
passthrough capture disabled, since `PassthroughHeaderCapture` already performs
the needed filtering and context overwrite for both ingress-managed and `/p/*`
routes. Keep the change localized to this middleware registration logic and
preserve the existing `RequestSnapshotCapture` and `PassthroughHeaderCapture`
behavior elsewhere.
---
Outside diff comments:
In `@internal/providers/init.go`:
- Around line 219-272: The passthrough-user-headers flag is being recorded
before provider creation succeeds, so failed providers can still enable header
capture. In initializeProviders, move the HeaderOverrides.PassthroughUserHeaders
check to after factory.Create returns successfully, and only then update
anyPassthroughUserHeaders for the provider that actually initialized.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9b0ed2b-e465-467d-91a2-6b2f914eae92
📒 Files selected for processing (38)
.env.templateCLAUDE.mdconfig/config.example.yamlconfig/config_test.goconfig/providers.goconfig/providers_test.godocs/features/custom-headers.mdxinternal/app/app.gointernal/providers/config.gointernal/providers/config_test.gointernal/providers/deepseek/deepseek_headers_test.gointernal/providers/deepseek/deepseek_test.gointernal/providers/factory.gointernal/providers/factory_test.gointernal/providers/groq/groq_test.gointernal/providers/headers.gointernal/providers/headers_test.gointernal/providers/init.gointernal/providers/init_test.gointernal/providers/kimicode/kimicode_headers_test.gointernal/providers/ollama/ollama.gointernal/providers/ollama/ollama_test.gointernal/providers/openai/compatible_provider.gointernal/providers/openai/compatible_provider_test.gointernal/providers/openrouter/openrouter.gointernal/providers/openrouter/openrouter_test.gointernal/providers/vllm/vllm.gointernal/providers/vllm/vllm_test.gointernal/providers/xai/xai_test.gointernal/server/auth_test.gointernal/server/http.gointernal/server/model_validation_test.gointernal/server/passthrough_headers.gointernal/server/passthrough_headers_test.gointernal/server/passthrough_semantic_enrichment_test.gointernal/server/request_snapshot.gointernal/server/request_snapshot_test.gorun/lifecycle_test.go
…ling - parseBoolEnv now requires an explicit true-set; any unrecognized value defaults to false so typos cannot silently enable passthrough. - logStaticOverridden only warns for headers that survive the same skip/allow and strip-set filtering applied during forwarding. - anyPassthroughUserHeaders is only set after factory.Create succeeds, so failed providers cannot enable the middleware. - Corrected the stale "Default-closed" docstring on shouldForward to reflect the actual default-open skip-mode behavior.
…ough capture - TestNew_AppliesUserPathHeader now captures request headers and asserts the configured alias is blocked and user headers are not forwarded by default. - xai_test no longer conditionally skips the X-Custom-Header assertion, so the default case explicitly verifies absence. - RequestSnapshotCapture in server setup is always called with capturePassthroughHeaders=false; PassthroughHeaderCapture already handles the filtering and context overwrite, avoiding double work on the hot path.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
internal/providers/config.go (1)
580-590: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winFail-closed fix looks correct; consider logging unrecognized values.
The explicit true-set plus
default → falseresolves the earlier fail-open concern. One residual gap: unrecognized values (e.g."tru","enalbed") are now silently coerced tofalse, so a fat-fingered value that was meant to enable passthrough disappears with no operator feedback. A warn log would surface the misconfiguration without changing the safe default.🪵 Optional: warn on unrecognized boolean env values
default: + slog.Warn("unrecognized boolean env value; defaulting to false", "value", value) b := false return &b }🤖 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 `@internal/providers/config.go` around lines 580 - 590, The boolean parsing in the config helper now fails closed, but unrecognized inputs are silently coerced to false. Update the parsing logic around the boolean env/value helper in config.go to emit a warning when the normalized value does not match the known true/false sets, while still returning false by default. Keep the existing behavior in the parsing function that handles trimmed values, and use the existing logger/config context so operators can spot typos like “tru” or “enalbed” without changing the safe default.
🤖 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.
Duplicate comments:
In `@internal/providers/config.go`:
- Around line 580-590: The boolean parsing in the config helper now fails
closed, but unrecognized inputs are silently coerced to false. Update the
parsing logic around the boolean env/value helper in config.go to emit a warning
when the normalized value does not match the known true/false sets, while still
returning false by default. Keep the existing behavior in the parsing function
that handles trimmed values, and use the existing logger/config context so
operators can spot typos like “tru” or “enalbed” without changing the safe
default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07bb5caf-ed5e-40b4-ba9f-e6253215baa2
📒 Files selected for processing (6)
internal/providers/config.gointernal/providers/headers.gointernal/providers/init.gointernal/providers/kimicode/kimicode_headers_test.gointernal/providers/xai/xai_test.gointernal/server/http.go
…eader overrides After resolving the earlier CodeRabbit review items, Greptile flagged two maintainability issues in the header override implementation that would leave misleading docs and dead parameters in the PR: - `internal/providers/headers.go`: `shouldForward` had a stale docstring that described the opposite forwarding behavior and a dead `case "only"` branch. `"only"` is rejected at config validation time, so it was unreachable. The function is now simplified to an if-chain and the docstring accurately reflects the default-open behavior. - `internal/server/request_snapshot.go`: `RequestSnapshotCapture` still carried a `capturePassthroughHeaders` bool and a conditional block that captured passthrough headers. On ingress-managed paths this value is always overwritten by the later `PassthroughHeaderCapture` middleware; on non-ingress-managed paths the block is never reached. Removed the parameter and block, updated every caller and test, and dropped the test that exercised the removed code path. All changes are within the existing PR's changed-file set and the package test suite (`go test ./internal/providers/... ./internal/server/... ./config/...`) passes.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…r overrides coverage - Extend config_test.go with coverage for parseBoolEnv, parseHeaderMapEnv, and collectProviderEnvValues header overrides - Extend headers_test.go with coverage for PassthroughHeadersFromContext, sourceHasHeader, logStaticOverridden, and applyPassthroughHeaders edge cases - Add unsuffixed, suffixed, and ambiguous-provider env overlay tests for header override fields - Integrate all coverage into existing test files instead of creating standalone test files
… skip mode - Add TestInit_PropagateInvalidHeaderOverridesSkipMode to verify buildProviderConfig error is returned through Init - Covers the error return path added by header overrides in both init.go and config.go
- Add TestServer_RegistersPassthroughHeaderCaptureWhenEnabled to ensure PassthroughUserHeadersEnabled causes the ingress capture middleware to run - Covers the server.New registration path added by the header overrides feature
- Add TestNew_AppliesHeaderOverrideWiring to verify SetUserPathHeader is called - Verify AnyPassthroughUserHeaders propagates from provider init to app state - Uses minimal config with SQLite temp file and disabled storage-backed features
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
…r overrides - Add TestInitializeProviders_PassthroughUserHeadersNotSetOnFailedProvider to guard the CodeRabbit finding that the passthrough flag must only be set after provider creation succeeds. - Add TestProviderEnvValues_RawConfig_PassthroughUserHeaders and TestProviderEnvValues_Empty_HeaderFields to cover the missing line and partial branch Codecov reported in internal/providers/config.go.
Contributor
Author
The passthrough flag is already set after Commit: @coderabbitai resume |
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
sed -n '190,280p' internal/providers/init.goLength of output: 3092 Confirmed — the ✅ Action performedReviews resumed. |
Contributor
Author
|
Converting to draft to fix a bug with the OpenRouter implementation. It currently overwrites headers even when 'User Headers Passthrough' is enabled. Note: Other providers' implementations are working perfectly based on my final manual E2E tests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch adds per-provider header configuration: inject static headers on every upstream request, forward selected client headers, or both. The four new provider keys are
custom_upstream_headers,passthrough_user_headers,passthrough_user_headers_skip, andpassthrough_user_headers_skip_mode. The feature is wired through the sharedCompatibleProvideradapter, so every OpenAI-compatible provider inherits it without per-provider changes. A hard-coded security floor always blocks credential headers, transport/hop-by-hop headers, and the internalX-GoModel-User-Pathheader. The default is closed:passthrough_user_headersisfalse, sopassthrough_user_headers_skipandpassthrough_user_headers_skip_modeonly matter when passthrough is explicitly enabled.Files to Review (38 changed, +3,253 / -149)
internal/providers/headers.go(start here)HeaderOverridesConfig,ApplyHeaderOverrides,IsHeaderBlocked, and all forwarding logic — the core engine.internal/providers/config.go<PROVIDER>_CUSTOM_UPSTREAM_HEADERS, etc.).internal/server/passthrough_headers.goPassthroughHeaderCapturemiddleware — captures and filters incoming headers into context.internal/providers/openai/compatible_provider.goHeaderOverrides/UserPathAliasinto the shared compatible client and wiresApplyHeaderOverridesinto the request closure.internal/providers/factory.goSetUserPathHeaderbuilder option and plumbing from config to provider options.internal/providers/openrouter/openrouter.goHTTP-Referer,X-OpenRouter-Title) respect static overrides via a localRequestMutator.docs/features/custom-headers.mdxWhy
OpenAI-compatible providers often require headers beyond
Authorizationto route requests correctly. Client headers such asX-TitleorX-Trace-Idcan influence provider-side throttling, logging, or model routing, and operators frequently need to inject static identity headers so internal tooling behind GoModel is identified upstream without every client sending them.The original request surfaced in issue #290 (comment-4707019144). An earlier combined implementation in PR #487 was split: the Kimi Code provider landed in PR #508, and the header-override engine is re-implemented here on top of the PR #486 provider rework.
Design Decisions
internal/providers/headers.goand is applied throughCompatibleProvider's request closure, so DeepSeek, Groq, Ollama, vLLM, xAI, OpenRouter, Kimi Code, and any future OpenAI-compatible provider get the feature automatically.<PROVIDER>_prefix convention.passthrough_user_headersdefaults tofalse; when it isfalse, no user headers are forwarded regardless ofpassthrough_user_headers_skip/passthrough_user_headers_skip_mode. An operator must explicitly enable passthrough to forward headers. When passthrough is enabled withpassthrough_user_headers_skip_modeskipor""(default) and an emptypassthrough_user_headers_skiplist, all non-blocked user headers are forwarded; withpassthrough_user_headers_skip_modeallowand an emptypassthrough_user_headers_skiplist, no user headers are forwarded.IsHeaderBlockedpermanently removes credential headers,X-GoModel-User-Path, the configureduser_pathalias, and hop-by-hop/transport headers. The passthrough path additionally consults the tagging middleware'sTaggingStripHeadersFromContextso headers markeddo_not_passin tagging rules are never forwarded.slog.Warnfires at runtime when both are configured and overlap.How
RawProviderConfiggains four flat header fields; these are packed intoHeaderOverridesConfigduring resolution. Env-var overlays apply on top of YAML.ProviderFactory.SetUserPathHeaderstores the alias thatIsHeaderBlockedchecks beforeproviders.Initruns.PassthroughHeaderCapturemiddleware captures the incoming request, strips hard-blocked headers, and stashes the filtered copy in context.CompatibleProviderreceivesHeaderOverridesandUserPathAliasin its config and callsApplyHeaderOverridesafter the provider-specificSetHeadershook.ApplyHeaderOverridesapplies static headers first, then passthrough headers if enabled.shouldForwardchecks the blocklist and strip-set before the skip/allow list.Notes
header_overrideskey. The env-var and YAML test suites use this flat layout.skipmode (emptypassthrough_user_headers_skiplist withpassthrough_user_headers_skip_modeskipor""), or configure an explicit allow list (passthrough_user_headers_skip_modeallowwith the desired headers listed).PassthroughHeaderCaptureis registered on the hot path; overhead is small but there is no feature flag.HTTP-Referer,X-OpenRouter-Title) through a localRequestMutator. Withcustom_upstream_headers, an operator can override those values.Tests
headers.gopaths: static apply, passthrough apply, skip/allow modes, blocklist, strip-set, default-skip-forward-all, allow-empty-forward-nothing, overlap warning.SetUserPathHeaderwiring,HeaderOverridesplumbing.PassthroughHeaderCapturefilters blocked headers and context storage.Run:
go test ./internal/providers/... ./internal/server/... ./config/...Follow-up
ApplyHeaderOverridesthere.openai.CompatibleProvideralready propagates overrides; the other compatible providers inherit without additional code.OPENROUTER_SITE_URL/OPENROUTER_APP_NAME) through the shared header setter instead of the localRequestMutator. Not implemented in this PR (but on request could be added in this or future PR).Links
Generated with creating-pull-requests Skill
Summary by CodeRabbit
CUSTOM_UPSTREAM_HEADERS.bailianfrom default passthrough provider routes.