Fix/header override suppress empty#4996
Open
yingfengli wants to merge 3 commits into
Open
Conversation
Setting a header_override entry to "" is the natural way to express
"do not forward this header to the upstream", but the previous code
silently dropped empty entries:
- sanitizeHeaderOverrideMap discarded keys whose value was empty unless
the key happened to be a passthrough rule key ("*", "re:", "regex:").
- applyHeaderOverridePlaceholders returned include=false for empty
templates.
As a result, a header that the channel adaptor already wrote into the
outgoing request from c.Request.Header (notably anthropic-beta, which
claude.CommonClaudeHeadersOperation copies for the AWS Bedrock channel)
would still be forwarded to the upstream regardless of the override.
This caused requests routed through a Bedrock channel to fail with
"ValidationException: invalid beta flag" whenever the client sent any
of the beta flags Bedrock does not accept (prompt-caching-2024-07-31,
extended-cache-ttl-2025-04-11, etc.) - which is the default behavior of
clients like Claude Code. Operators had no configuration-only way to
fix it.
Fix:
- sanitizeHeaderOverrideMap now preserves empty values for any key
(passthrough rule contract is unchanged).
- applyHeaderOverridePlaceholders returns include=true for empty
templates so the suppression marker reaches the consumers.
- All four header_override consumers - applyHeaderOverrideToRequest,
DoWssRequest's loop, the AWS Bedrock relay, and channel test header
building in controller/channel.go - now call Header.Del when the
override value is "" instead of writing the empty string.
This restores the obvious mental model:
| header_override value | upstream behavior |
| ------------------------------- | ------------------- |
| "anthropic-beta": "value" | Set to "value" |
| "anthropic-beta": "" | Removed (NEW) |
| (no entry) | Pass-through |
Backward compatibility: production overrides almost universally use
non-empty values, and intentionally storing an empty value to mean
"no-op" was never a documented contract; the new semantics match
operator intent.
Tests:
- TestProcessHeaderOverride_EmptyValueIsExplicitSuppression
- TestApplyHeaderOverrideToRequest_EmptyValueDeletesHeader
Both regress against the previous behavior; all existing
header_override tests still pass.
…override
Before this change, GetEffectiveHeaderOverride returned the runtime
override map verbatim whenever info.UseRuntimeHeadersOverride was true,
completely ignoring the channel-level header_override configured in the
admin UI. The runtime override map is populated by upstream features
such as channel affinity rules (e.g. the built-in "claude cli trace"
rule, which copies anthropic-beta and other Claude CLI headers into the
runtime map). As a result, an operator setting
header_override = {"anthropic-beta": ""} on a Bedrock channel had no
effect: the affinity rule re-injected the client's anthropic-beta into
the runtime map, downstream consumers wrote it into the upstream
request, and Bedrock rejected the request with
"ValidationException: invalid beta flag" for any flag it does not
support.
Fix:
- Merge the channel-level header_override on top of the runtime override
map, with channel entries winning on conflicting keys. The admin UI
is now the authoritative source for header policy on a per-channel
basis, and runtime-only headers (those the affinity rule injects but
the channel does not redefine) still flow through unchanged.
- Combined with the previous suppression-marker change, this means
setting header_override = {"anthropic-beta": ""} on a Bedrock channel
now reliably strips the header before the upstream call, even when an
affinity rule attempts to copy it from the client request.
Tests:
- TestGetEffectiveHeaderOverrideMergesChannelOnTopOfRuntime replaces the
previous TestGetEffectiveHeaderOverrideUsesRuntimeOverrideAsFinalResult
to encode the new merge semantics.
- TestProcessHeaderOverride_ChannelOverrideWinsOverRuntime replaces the
previous TestProcessHeaderOverride_RuntimeOverrideIsFinalHeaderMap.
- All other header_override tests continue to pass.
Manually verified end-to-end with the official Claude Code CLI routed
to a Bedrock channel: prior to this change, the request failed with
"invalid beta flag"; after this change, the upstream payload no longer
contains anthropic_beta and the request succeeds.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughTreat empty-string header overrides as explicit delete directives and merge channel-level overrides over runtime overrides. Changes update sanitization, HTTP/WebSocket application, controller and AWS request handling, and tests. ChangesHeader Override Empty-String Suppression
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
…is PR
Adds package-level Go doc comments to the seven functions modified by this
branch so they explain the empty-value suppression contract and the
channel-over-runtime merge semantics directly at the function definition,
rather than only at the call sites.
No behavior change. Only doc comments added. Build clean, all existing
header_override tests still pass.
Functions documented:
- controller/channel.go: buildFetchModelsHeaders
- relay/channel/api_request.go: applyHeaderOverridePlaceholders,
ResolveHeaderOverride,
applyHeaderOverrideToRequest,
DoWssRequest
- relay/channel/aws/relay-aws.go: doAwsClientRequest
- relay/common/override.go: sanitizeHeaderOverrideMap,
GetEffectiveHeaderOverride
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two related bugs that prevent operators from suppressing a header on a channel via the
header_overridesetting.Bug 1: empty value silently dropped
header_override = {"anthropic-beta": ""}was a no-op.sanitizeHeaderOverrideMapandapplyHeaderOverridePlaceholdersboth treated empty strings as "no entry," so the channel's existing header (e.g.anthropic-betacopied from the client byCommonClaudeHeadersOperation) leaked through unchanged.This commit treats an empty value as an explicit suppression marker: the entry is retained through sanitization, and downstream consumers (
applyHeaderOverrideToRequest,DoWssRequest,doAwsClientRequest,buildFetchModelsHeaders) callHeader.Del(key)instead ofHeader.Set(key, ""). The passthrough rule keys (*,re:...,regex:...) historically also use empty values; their behavior is preserved by the same fall-through.Bug 2: runtime override silently replaced channel override
When
info.UseRuntimeHeadersOverridewas true (set by upstream features such as the built-inclaude cli tracechannel-affinity rule),GetEffectiveHeaderOverridereturned onlyRuntimeHeadersOverrideand discarded the operator-configured channelheader_overrideentirely.For the Bedrock case this meant the affinity rule re-injected
anthropic-betafrom the client request after the operator had explicitly tried to suppress it on the channel.This commit merges the two: runtime first, channel-level on top. Channel-level entries (including empty-string suppression markers) win for keys defined in both layers, on the principle that the admin UI is the authoritative source of header policy. Runtime-only entries are still included via the union merge.
Why this matters
Concrete failure mode this fixes: Claude Code → New API → AWS Bedrock channel.
Bedrock rejects beta flags Anthropic ships first-party (
prompt-caching-2024-07-31,claude-code-20250219, etc.) withValidationException: invalid beta flag. Without this fix there is no way for an operator to suppress those headers on the Bedrock channel, even though the UI exposes a setting that appears to do exactly that.Test plan
go build ./...cleanheader_overridetests passTestProcessHeaderOverride_EmptyValueIsExplicitSuppression— sanitizer keeps empty valuesTestApplyHeaderOverrideToRequest_EmptyValueDeletesHeader—Del()is called instead ofSet("", "")TestProcessHeaderOverride_ChannelOverrideWinsOverRuntime— channel beats runtime in the mergeTestGetEffectiveHeaderOverrideMergesChannelOnTopOfRuntime— empty-string suppression marker survives the mergeheader_override = {"anthropic-beta": ""}on the Bedrock channel returns 200 OK; affinity rule still triggers but channel override wins;anthropic_betano longer appears in upstream Bedrock body.Files changed
relay/common/override.go— sanitizer +GetEffectiveHeaderOverridemergerelay/common/override_test.go— merge test updatedrelay/channel/api_request.go— placeholder expansion +applyHeaderOverrideToRequest+DoWssRequestDel logicrelay/channel/api_request_test.go— new suppression / merge testsrelay/channel/aws/relay-aws.go— Bedrock Del logiccontroller/channel.go— channel-test path Del logicNotes
header_override(not just Bedrock).Summary by CodeRabbit
Bug Fixes
New Features