feat(llm): apikey_header_only auth scheme + record invoked LLM URL on llm_call#358
Conversation
…ader suppressed apikey_header (#302) sends the provider-native header (x-api-key / Authorization) IN ADDITION to the gateway header. That breaks gateways that INJECT the upstream credential via an add-if-absent transform (Kong request-transformer `add`): Forge's native header blocks the injection, so the gateway key reaches the provider and 401s. New auth_scheme: apikey_header_only sends only the gateway header and suppresses the native one, mirroring how aws_sigv4 skips it. The gateway becomes the sole injector of the real upstream key. apikey_header stays additive (unchanged) for gateways that REPLACE the native header or pass a valid provider key through. - suppress native x-api-key (anthropic) / Authorization (openai) for the new scheme - setGatewayAPIKeyHeader fires for both gateway schemes - validate: accept apikey_header_only; collision + auth_header_name checks via isGatewayScheme - tests: native-suppressed + gateway-header-set for both providers; validate accepts scheme - docs: schema reference, runtime-engine, forge.md forge-core build/vet/lint clean; llm + validate suites green.
…capture independent)
The llm_call audit event recorded model/provider/tokens but not WHERE the
request went, so a gateway/base-URL misroute (e.g. an ANTHROPIC_BASE_URL
pointing at Kong) was invisible without enabling payload capture. And a hard
failure (401) emitted no llm_call at all.
- llm.ChatResponse gains an Endpoint field (json:"-"), set by the openai and
anthropic clients to the URL they POSTed to (base URL + provider path).
- registerAuditHooks records fields.url on every llm_call, independent of
payload capture — the URL is header-authed metadata, carries no secret.
- provider errors now name the invoked URL ("... calling <url>: ..."), so a
401 misroute is debuggable straight from the agent-loop error log.
Tests: endpoint recorded (openai + anthropic), error includes URL. Docs:
audit-logging llm_call row. forge-core + forge-cli build/vet/lint clean.
initializ-mk
left a comment
There was a problem hiding this comment.
Review: apikey_header_only scheme + record invoked LLM URL
Change 1 (auth scheme) is clean and correct. Change 2 (URL recording) is a genuinely useful debugging aid with one security issue and one coverage gap. Merge-ready after finding 1.
Change 1 — apikey_header_only: correct, no regression ✅
- Native-header suppression is symmetric and right: anthropic suppresses
x-api-key, openai suppressesAuthorization: Bearerforapikey_header_only(joiningaws_sigv4), whilesetGatewayAPIKeyHeaderfires for both gateway schemes. apikey_headerstays additive and untouched — no behavior change for existing #302/#303 deployments.- Validate wiring is tidy:
isGatewaySchemeunifies the native-header-collision guard andauth_header_nameapplicability, the new scheme is added toknownModelAuthSchemes, and the provider warning is extended. The key rides in exactly one header.
Change 2 — findings (see inline)
- Low/Medium (security): the recorded URL isn't sanitized, so a base URL with userinfo (
https://user:pass@gateway) leaks a credential into the audit stream (fields.url) andagent loop errorlogs. The "carries no secret" comment holds only for a clean base URL — and userinfo-in-URL is a legitimate gateway config, i.e. exactly this PR's audience. - Low (completeness): only non-streaming
ChatsetsEndpoint; the streaming path doesn't, so streamingllm_callevents (the common agent path) carry nofields.url— which is where you'd most want it during a gateway misroute.
Also good
Endpoint string json:"-"is the right call — internal-only, not serialized into response payloads, explicitly surfaced into the audit field.- Naming the URL in the error string genuinely fixes the hard-401 case (no
llm_callfires there), the motivating bug. - CI fully green; tests + docs included.
| // the agent hit is auditable even when payload capture is disabled. | ||
| // Essential for debugging gateway/base-URL routing. | ||
| if hctx.Response != nil && hctx.Response.Endpoint != "" { | ||
| fields = map[string]any{"url": hctx.Response.Endpoint} |
There was a problem hiding this comment.
Low/Medium (security): sanitize before recording. The comment above says the URL "carries no secret," but that's only true for a clean base URL. Endpoint = c.baseURL + "/path", and baseURL comes from model.base_url / ANTHROPIC_BASE_URL / OPENAI_BASE_URL. A gateway configured with userinfo — https://user:pass@gateway.internal (a legitimate setup; Go's http.NewRequestWithContext extracts it into basic auth) — makes Endpoint = https://user:pass@gateway.internal/v1/messages, and that password then lands in the audit stream here (typically shipped to SIEM + retained) and, via the provider error strings (anthropic.go / openai.go ... calling <endpoint>: ...), in agent loop error logs.
Strip userinfo before recording, in one shared spot ideally:
safe := endpoint
if u, err := url.Parse(endpoint); err == nil {
u.User = nil // or u.Redacted() to mask the password
safe = u.String()
}Apply the same to the two provider error strings. That turns "carries no secret" from an assumption into an invariant.
| return c.parseAnthropicResponse(resp.Body) | ||
| result, err := c.parseAnthropicResponse(resp.Body) | ||
| if result != nil { | ||
| result.Endpoint = endpoint |
There was a problem hiding this comment.
Low (completeness): the streaming path never sets Endpoint. Chat sets result.Endpoint here, but ChatStream doesn't — its aggregated ChatResponse builder (anthropic.go:371, and openai's at openai.go:281) omits it, and ChatStream even re-inlines c.baseURL+"/v1/messages" rather than reusing an endpoint var. So streaming llm_call events carry no fields.url — and streaming is the common path for interactive agents, exactly where you'd want the URL when a gateway misroutes. Hoist the URL into an endpoint var in ChatStream like Chat does and assign it on the aggregated response. Same in the openai client. (Whatever sanitization lands for finding 1 should cover this path too.)
… stream aggregate (#358 review) - [Low/Medium security] strip userinfo (user:pass@) from the URL before it is recorded on ChatResponse.Endpoint (→ llm_call fields.url) or named in a provider error string. A gateway base URL with embedded credentials (https://user:pass@gateway) is a legitimate setup, and the raw URL would otherwise leak the password into the audit stream + agent-loop error logs. New shared sanitizeEndpoint helper; requests still use the raw URL so Basic auth works, but every recorded/logged form is sanitized. Covers the streaming error strings too. - [Low completeness] ResponsesClient.Chat aggregates a stream into a ChatResponse (the OpenAI OAuth path) — now sets Endpoint on it, so streaming llm_call events carry fields.url like non-streaming. ChatStream paths hoist the endpoint into a var (no more re-inlined URL) and name it in errors. OllamaClient inherits the fix (embeds *OpenAIClient). Tests: userinfo stripped from Endpoint + error, sanitizeEndpoint table, responses aggregator endpoint. llm suite green; vet + lint clean.
|
Addressed both findings in 3c3c746: Finding 1 (Low/Medium security — sanitize userinfo). Added a shared Finding 2 (Low completeness — streaming). You were right that an aggregated Worth noting on scope: Forges executor (
|
initializ-mk
left a comment
There was a problem hiding this comment.
Re-review: fix commit 3c3c746d — both findings resolved
Verified against the diffs and the audit data-flow. Build / Lint / Test / Doc-link green (Integration still running — unrelated; this diff is llm provider-only).
Finding 1 (userinfo leak) — ✅ resolved, cleanly
New shared sanitizeEndpoint does url.Parse → u.User = nil → u.String(), stripping the entire userinfo (stronger than Redacted(), which keeps the username). The separation is exactly right: the HTTP request still uses the raw endpoint (so user:pass@ becomes a working Basic auth header), while every recorded/logged form uses the sanitized one:
result.Endpoint(→fields.url) in openai/anthropicChatandResponsesClient.Chat- all provider error strings, including the
ChatStreamerror paths (which now also name the sanitized URL — a real debuggability win even though they don't feedllm_call) OllamaClientinherits it via embedded*OpenAIClient
Sanitizing at the source rather than only in the audit hook is the better call — it covers the error-log vector too. The url.Parse-error → return-raw fallback is a non-issue in practice (url.Parse doesn't error on http(s) URLs with userinfo).
Finding 2 (streaming Endpoint) — ✅ resolved at the correct path
I traced the audit path end-to-end: the executor loop calls e.client.Chat and fires AfterLLMCall with that response, so llm_call is only ever fed by .Chat. The one streaming path reaching it is ResponsesClient.Chat (OAuth/Responses, which streams + aggregates internally), and the fix sets Endpoint there. My original inline anchor pointed at the plain ChatStream aggregate, but that response never feeds llm_call — so this diagnosed the real audit-feeding path more precisely than my comment and fixed exactly it. Streaming llm_call events now carry fields.url on the path that actually emits them.
Verdict: both findings resolved, sanitization applied at every sink, streaming coverage on the genuinely correct path. Nothing dangling — merge-ready.
…ge.yaml) Two silent drops made a gateway base URL never take effect, so the anthropic client fell back to api.anthropic.com and sent the gateway key as x-api-key (401 invalid x-api-key): 1. The runner built envVars from only the .env file + the secret overlay; os.Environ was never merged, and the overlay fetches only secret keys. Non-secret model-config keys (ANTHROPIC_BASE_URL / OPENAI_BASE_URL / OLLAMA_BASE_URL / FORGE_MODEL_PROVIDER / MODEL_NAME / OPENAI_ORG_ID / AWS_REGION / FORGE_MODEL_FALLBACKS) injected into a deployed pod's env were invisible to ResolveModelConfig. New mergeModelConfigEnv fills them from the process environment as a fallback (.env / secrets still win). 2. model.base_url in forge.yaml was never copied to the client — it only fed the build-time egress allowlist. ResolveModelConfig now applies cfg.Model.BaseURL (before env overrides, so *_BASE_URL still wins). Together: a deployment's OPENAI_BASE_URL/ANTHROPIC_BASE_URL injection contract and the declarative model.base_url now both actually route the client. Tests: base_url from yaml + env override precedence; mergeModelConfigEnv fills from os env, .env wins, secret keys excluded. Build/vet/lint clean.
|
Added the actual root cause fix in 62cd126 (per the deployment forensics): a gateway base URL never reached the model client, so the anthropic client fell back to
This means any deployed agent using a gateway via the anthropic provider was broken the same way, and the platform’s Tests: base_url-from-yaml + env-override precedence; |
initializ-mk
left a comment
There was a problem hiding this comment.
Review: commit 62cd1265 — gateway base URL now reaches the client
This is the root-cause fix behind the original 401 (the earlier commits were debuggability + the apikey_header_only workaround; this makes the base URL actually take effect). Correct, well-scoped, properly tested. No security concern. Full CI green.
Why it's safe
mergeModelConfigEnvfills model-config keys fromos.Environas a fallback, but iterates a fixed 8-key allowlist (*_BASE_URL,FORGE_MODEL_PROVIDER,MODEL_NAME,OPENAI_ORG_ID,AWS_REGION,FORGE_MODEL_FALLBACKS) — not the whole environment, so no env-surface broadening, and all eight are non-secret config.model.base_urlis now copied tomc.Client.BaseURL(previously it only fed the build-time egress allowlist).
Precedence is correct and the tests pin every edge:
mergeModelConfigEnvskips keys already inenvVars, so.env/secrets win overos.Environ.- Secret keys are explicitly excluded — the test asserts
ANTHROPIC_API_KEYdoes not flow this path (stays withoverlaySecrets). This is the key safety invariant and it's locked. model.base_urlis applied before the*_BASE_URLenv override, so per-deploy env wins over yaml (tested both directions).- The CLI
--modeloverride still beatsos.EnvironMODEL_NAME(merge runs before the override block).
Nice synergy
This closes the loop on the PR: base_url now routes the client, and the sanitized fields.url audit from 3c3c746d records where it went — so a misroute is both fixable and visible. And since the key now reaches the configured gateway instead of the provider's public host, it complements apikey_header_only correctly.
Minor behavior-change note (not a finding)
Env now overrides yaml base_url including from the ambient shell — a stray OPENAI_BASE_URL/ANTHROPIC_BASE_URL in a dev's local shell will silently redirect the client where before it was ignored. Standard 12-factor env-over-config convention and defensible, and the new audit URL surfaces it if it happens — just worth being aware it's a local-dev behavior change, not only deployed pods.
Verdict: correct root-cause fix, scoped allowlist, secret-exclusion tested, precedence tested, CI green. No issues.
… stream aggregate (#358 review) - [Low/Medium security] strip userinfo (user:pass@) from the URL before it is recorded on ChatResponse.Endpoint (→ llm_call fields.url) or named in a provider error string. A gateway base URL with embedded credentials (https://user:pass@gateway) is a legitimate setup, and the raw URL would otherwise leak the password into the audit stream + agent-loop error logs. New shared sanitizeEndpoint helper; requests still use the raw URL so Basic auth works, but every recorded/logged form is sanitized. Covers the streaming error strings too. - [Low completeness] ResponsesClient.Chat aggregates a stream into a ChatResponse (the OpenAI OAuth path) — now sets Endpoint on it, so streaming llm_call events carry fields.url like non-streaming. ChatStream paths hoist the endpoint into a var (no more re-inlined URL) and name it in errors. OllamaClient inherits the fix (embeds *OpenAIClient). Tests: userinfo stripped from Endpoint + error, sanitizeEndpoint table, responses aggregator endpoint. llm suite green; vet + lint clean.
Two related gateway-debuggability changes, motivated by an agent behind a Kong AI Gateway 401ing on
invalid x-api-key.1.
auth_scheme: apikey_header_only(gateway header, native header suppressed)apikey_header(#302) sends the provider-native header (x-api-key/Authorization) in addition to the gateway header. That is correct when the gateway replaces the upstream credential (Kongrequest-transformerreplace, orai-proxywithallow_override: true) or the key passes through. But when the gateway injects the native header with an add-if-absent transform (Kongrequest-transformeradd), Forges native header blocks the injection, so the gateway key reaches the provider and it 401s.New
model.auth_scheme: apikey_header_onlysends only the gateway header and suppresses the provider-native header, mirroring howaws_sigv4already skips it. The gateway becomes the sole injector of the real upstream credential.x-api-key(anthropic) /Authorization: Bearer(openai) suppressed for the new scheme;setGatewayAPIKeyHeaderfires for both gateway schemes.apikey_headerstays additive and unchanged — no behavior change or broken tests for existing deployments (the LLM client: apikey-header auth scheme for Kong AI Gateway (key-auth) #302/feat(llm): apikey_header auth scheme for Kong AI Gateway key-auth #303 additive contract and its tests are preserved).forge validateaccepts the new scheme; the native-header-collision guard andauth_header_nameapplicability extend to it via a sharedisGatewaySchemehelper.Pick
apikey_headerwhen the gateway replaces the native header;apikey_header_onlywhen it adds.2. Record the invoked LLM URL on
llm_call(payload-capture independent)The
llm_callaudit event recorded model/provider/tokens but not the URL, so a base-URL/gateway misroute (e.g.ANTHROPIC_BASE_URLpointing at Kong) was invisible unless you enabled payload capture, and a hard 401 emitted nollm_callat all.llm.ChatResponsegains an internalEndpointfield (json:"-"), set by the openai + anthropic clients to the URL they POSTed to ({base}/v1/messages,{base}/chat/completions).registerAuditHooksrecordsfields.urlon everyllm_call, independent of payload capture — the URL is header-authed metadata and carries no secret.... calling <url>: ...), so a 401 misroute is debuggable straight from theagent loop errorlog even though nollm_callfires.Tests
apikey_header_only: native header suppressed + gateway header set (openai + anthropic); customauth_header_namehonored; validate accepts the scheme + collision guard.Docs
docs/reference/forge-yaml-schema.md,docs/core-concepts/runtime-engine.md,.claude/skills/forge.md— the new scheme + additive-vs-suppress guidance.docs/security/audit-logging.md— thellm_callfields.url.forge-core+forge-clibuild/vet/lint clean;llm+validatesuites green.