broker: forward all client headers, only override the auth slot#133
Merged
dangtony98 merged 2 commits intomainfrom Apr 26, 2026
Merged
broker: forward all client headers, only override the auth slot#133dangtony98 merged 2 commits intomainfrom
dangtony98 merged 2 commits intomainfrom
Conversation
Removes the 8-header allowlist that dropped non-allowlisted client headers (anthropic-version, anthropic-beta, If-Match, vendor tracing headers, ...) on credentialed services, breaking upstreams like the Anthropic API that mandate vendor request headers. Both ingress paths now use a single denylist in ApplyInjection: client headers pass through except hop-by-hop (RFC 7230, now including Proxy-Connection), broker-scoped (X-Vault, Proxy-Authorization), the keys of inject.Headers (the auth slot for the configured auth type), and any names in extraStrip. Pre-stripping inject.Headers keys before the copy loop is what preserves the "injected always wins" invariant. The schema is unchanged. Behavior is a strict loosening — no requests that succeed today will fail. Cookie now also flows through on credentialed services; Set-Cookie response stripping is unchanged. InjectResult.Passthrough is removed (its only consumer was the dispatch in ApplyInjection); res.Headers == nil is the new passthrough signal. CopyPassthroughRequestHeaders is inlined and deleted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
💬 Discussion in Slack: #pr-review-agent-vault-133-broker-forward-all-client-headers-only-override-the-au Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
A proxy must treat any header named in the Connection field as hop-by-hop for that connection. Pre-PR this was hidden by the 8-header allowlist; with all client headers now passing through, an unhandled `Connection: X-Custom-Hop` would let X-Custom-Hop reach the upstream. Honors the dynamic strip alongside the static HopByHopHeaders set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closes #104.
Summary
Content-Type,Content-Encoding,Accept,Accept-Encoding,Accept-Language,User-Agent,Idempotency-Key,X-Request-Id) that dropped non-allowlisted client headers on credentialed services. This was breaking upstreams that mandate vendor headers — most notably the Anthropic API (anthropic-versionis required on every/v1/messagesrequest)./proxyand transparent MITM) now share a single denylist inApplyInjection. Client headers flow through except for hop-by-hop (RFC 7230, now includingProxy-Connection), broker-scoped (X-Vault,Proxy-Authorization), the keys ofinject.Headers(the auth slot for the configured auth type), and any names passed viaextraStrip(the ingress's session-token header).inject.Headerskeys from the copy loop is what preserves the injected always wins invariant — same outcome as the old allowlist approach, derived from data instead of a hardcoded list.InjectResult.Passthroughis removed (vestigial after dispatch unified).res.Headers == nilis the new passthrough signal.Approach vs. the issue's suggested fix
#104 suggested a per-service
extra_passthrough_headersopt-in. This PR takes the inverse approach: forward everything by default, since header policy is a separate concern the broker had implicitly taken on, and the credential-brokering role only requires control over the headers the broker is itself injecting. A permissive default is the flexible foundation; restrictive opt-ins (strip_headers,allow_headers) can be added later if real customer need surfaces. The reverse is not true.Behavior delta
anthropic-version,anthropic-beta,If-Match,Prefer, custom tracing/signing headers, etc. — now reach the upstream on every auth type.Set-Cookieresponse stripping is unchanged, so upstream-set cookies still don't reach the client.Auth-slot stripping by auth type
bearerAuthorizationAuthorization: Bearer <token>basicAuthorizationAuthorization: Basic <b64>api-keyauth.header<auth.header>: <prefix><value>customauth.headersmap keyspassthroughTest plan
go test ./...— full suite green, including newTestApplyInjection_*matrix andTest{MITM,Proxy}BearerForwardsArbitraryClientHeadersintegration tests on both ingresses.bearerservice forapi.anthropic.com, real/v1/messagesPOST withanthropic-version: 2023-06-01. Expect 200 (today: 400 missing-version).Authorization: Bearer wrongreturns 200 — broker's stored credential shadows the client value on both/proxyand MITM ingresses.Test{Proxy,MITM}PassthroughForwards*integration tests pass without source changes.Docs
Per CLAUDE.md lockstep:
cmd/skill_cli.md,cmd/skill_http.md,docs/learn/services.mdx(new "Header forwarding" section),docs/learn/security.mdx,docs/agents/protocol.mdx,docs/index.mdx,docs/quickstart/custom-agent.mdx,docs/guides/connect-custom-agent.mdx,docs/reference/cli.mdxall updated. UI tooltip for the passthrough auth type rephrased to match.🤖 Generated with Claude Code