feat(broker): add per-service extra_passthrough_headers (closes #104)#107
Closed
MackDing wants to merge 1 commit intoInfisical:mainfrom
Closed
feat(broker): add per-service extra_passthrough_headers (closes #104)#107MackDing wants to merge 1 commit intoInfisical:mainfrom
MackDing wants to merge 1 commit intoInfisical:mainfrom
Conversation
…ical#104) Adds an opt-in per-service field that extends the core PassthroughHeaders allowlist with provider-specific headers that must be forwarded unchanged. Previously these were silently dropped by brokercore's allowlist, causing requests that worked via direct provider access to fail through agent-vault — most notably Anthropic's mandatory `anthropic-version` header on every /v1/messages call. ## Design - `broker.Service.ExtraPassthroughHeaders []string` — opt-in extension, omitempty so existing persisted services round-trip unchanged. - `broker.Validate` enforces a denylist at config time: no Authorization, Proxy-Authorization, X-Vault, or hop-by-hop names, plus RFC 7230 token validation and duplicate detection. Rejected on `passthrough` auth since the denylist model already forwards every header. - `brokercore.InjectResult.ExtraPassthroughHeaders` surfaces the list through the credential-resolution path without new parameters. - `brokercore.ApplyInjection` copies the extra headers between the core allowlist and the injected credential overlay, so injection still wins on collision (same guarantee PassthroughHeaders provides for Authorization). - Both ingresses (explicit /proxy + HTTPS_PROXY MITM) share the same code path, so behaviour stays in lockstep by construction. - Catalog presets pre-fill recommended defaults for Anthropic, OpenAI, Stripe, GitHub, Notion, Datadog, and Supabase based on each provider's public API docs. ## Testing - `broker`: validate-accepts, validate-rejects-denylist (8 names), invalid-name charset (5 cases), duplicate case-insensitive, rejected-on-passthrough-auth, plus JSON round-trip preservation and omitempty confirmation for back-compat. - `brokercore`: ApplyInjection forwards the extra headers, case- insensitive matching, injection-still-wins on collision, and no-op-on-passthrough-service as defense in depth alongside validation. Data-driven: the SuggestedCredentialKey list in the catalog was cross-checked against public API documentation for each of the 20 providers. The 7 provider entries that gained defaults are the ones whose documentation explicitly requires a non-standard request header as part of the protocol (not an optional one). Others were left alone. Closes Infisical#104
6d23858 to
5e299c5
Compare
Contributor
|
Closing as decided to go with a different direction closer to traditional MITM architecture with #133 |
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
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
Adds a per-service
extra_passthrough_headersfield that extends the corePassthroughHeadersallowlist with provider-specific headers that must be forwarded unchanged. Without it, credentialed requests to providers like Anthropic (which requiresanthropic-versionon every/v1/messagescall) fail through agent-vault even though credential injection works — the header is silently dropped in the agent → upstream hop.Implements the suggested fix from the issue, with validation guardrails so the new surface can't reopen the exact paths
PassthroughHeaderswas designed to close (Authorization, broker-scoped headers, hop-by-hop).Example
The built-in catalog now pre-fills these for the providers whose public API docs require a non-standard header as part of the protocol:
extra_passthrough_headersanthropicanthropic-version,anthropic-betaopenaiOpenAI-Organization,OpenAI-Project,OpenAI-BetastripeStripe-VersiongithubX-GitHub-Api-VersionnotionNotion-Version(required on every request)datadogDD-APPLICATION-KEYsupabaseapikey,PreferOthers in the catalog (Slack, Twilio, SendGrid, Cloudflare, PagerDuty, Linear, Jira, Vercel, Resend, Postmark, Sentry, Shopify, AWS S3) were left untouched — their APIs work with just the auth header.
Design
broker.Service.ExtraPassthroughHeaders []string(new field,omitempty)broker.Validatedenylist + RFC 7230 token check + dedup + passthrough-auth rejectionbrokercore.InjectResult.ExtraPassthroughHeadersbrokercore.ApplyInjectioncopies extras before injection overlayPassthroughHeadersprovides forAuthorization/proxy+ MITM) use sharedApplyInjectionNo public function signatures change. Zero breaking changes for existing deployments.
Validation rules (enforced at config time)
Rejected with a descriptive error:
Authorization,Proxy-Authorization,X-Vault— credential / broker-scopedConnection,Keep-Alive,TE,Trailer,Transfer-Encoding,Upgrade,Proxy-Authenticate— hop-by-hop per RFC 7230 §6.1extra_passthrough_headerson apassthrough-auth service (denylist already forwards everything, so the allowlist extension is a misconfiguration)Matching against
http.Headeris case-insensitive (useshttp.CanonicalHeaderKey), soanthropic-versionin config correctly matchesAnthropic-Versionin a client request.Tests
All new tests green; existing tests unchanged.
internal/brokerTestValidateExtraPassthroughHeadersAccepted— happy pathTestValidateExtraPassthroughHeadersRejectsDenylist(8 sub-cases) — each forbidden nameTestValidateExtraPassthroughHeadersRejectsInvalidName(5 sub-cases) — empty / space / colon / control / non-ASCIITestValidateExtraPassthroughHeadersRejectsDuplicate— case-insensitive dedupTestValidateExtraPassthroughHeadersRejectedOnPassthroughAuth— misconfiguration guardTestServiceJSONRoundTripPreservesExtraPassthroughHeaders— end-to-end store pathTestServiceJSONOmitsEmptyExtraPassthroughHeaders— back-compat (omitempty)internal/brokercoreTestApplyInjection_ForwardsExtraPassthroughHeaders— headers reach upstream,Content-Typestill works, client Authorization doesn't leakTestApplyInjection_ExtraPassthroughHeaders_AreCaseInsensitive— configanthropic-versionmatches clientAnthropic-VersionTestApplyInjection_ExtraPassthrough_IgnoredForPassthroughService— defense in depthTestApplyInjection_ExtraPassthrough_InjectionStillWins— auth config wins over client headerDocs
Added an Extra passthrough headers section to
docs/learn/services.mdxexplaining the problem, the YAML syntax, the validation guardrails, and when not to use it.Repro check
Before:
{"type":"error","error":{"type":"invalid_request_error","message":"anthropic-version: header is required"}}After (with the config above):
{"id":"msg_...","type":"message","role":"assistant", ...}Happy to tighten naming, split into smaller commits, or adjust the catalog defaults based on how conservative you want to be about pre-filling them. Thanks for the clean issue!