fix(agent-core-v2): resolve provider credentials from process env in the auth gate - #2746
fix(agent-core-v2): resolve provider credentials from process env in the auth gate#2746JacobStephens2 wants to merge 3 commits into
Conversation
…the auth gate
resolveModelAuthMaterial passed `args.provider?.env ?? {}` to
explainProviderEndpoint, replacing that function's `process.env` default with an
empty map for every provider without a `[providers.<id>.env]` table. The vendor's
declared apiKeyEnv was therefore looked up in an empty map and
authService.ensureReady threw AuthTokenMissingError, even though the adapters that
issue the request do read process.env.
This was latent behind the experimental flag until 0.33.0 made agent-core-v2 the
default engine for `kimi -p`, which turned it into a user-visible regression:
print mode rejects an environment-supplied provider key that 0.32.0 accepted.
Merge the provider env bag over process.env so the gate and the request client
agree on where a key may come from, with explicit config still taking precedence
(model apiKey > provider apiKey > provider env bag > process env). Also report the
actual source in the resolution trace.
🦋 Changeset detectedLatest commit: 79ebe02 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f57262e276
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : explainProviderEndpoint(providerAuthType, { | ||
| ...process.env, | ||
| ...args.provider?.env, | ||
| }); |
There was a problem hiding this comment.
When a google-genai provider explicitly configures GOOGLE_API_KEY while the host also exports VERTEXAI_API_KEY, merging both maps before resolution lets the endpoint declaration order in standard.contrib.ts:59-60 select the ambient Vertex key first. This reverses the intended provider-env-over-process-env precedence and can send requests with the wrong credential; resolve the provider env bag first, then consult process.env only if the bag contains no usable declared key.
Useful? React with 👍 / 👎.
| : explainProviderEndpoint(providerAuthType, { | ||
| ...process.env, | ||
| ...args.provider?.env, | ||
| }); |
There was a problem hiding this comment.
When a provider has a valid oauth configuration and the corresponding vendor key happens to exist in process.env, this merge turns the ambient key into providerApiKey, so the conflict check immediately throws config.invalid. Such OAuth configurations worked before this change and the request path supplies the OAuth token explicitly, so an unrelated shell-level key should not make them unusable; only explicitly configured provider credentials should participate in the apiKey/OAuth conflict check.
Useful? React with 👍 / 👎.
…t fallback Addresses two review findings on the previous revision. Merging process.env into the provider env bag before resolution let ambient values compete with configured ones: - a provider that explicitly configures GOOGLE_API_KEY on a host that also exports VERTEXAI_API_KEY resolved the ambient Vertex key, because standard.contrib declares VERTEXAI_API_KEY earlier in the endpoint chain; - an unrelated vendor key present in the shell was treated as providerApiKey and tripped the apiKey/oauth mutual-exclusion check, breaking provider configurations that previously worked. The provider's own env bag is now resolved in isolation, oauth is consulted before any ambient lookup, and process.env is read only as a last resort when nothing is configured at any layer. Only explicitly configured credentials participate in the conflict check. Precedence is unchanged from the documented order except that provider oauth now outranks an ambient process.env key, which previously could not be reached at all.
|
Thanks — both findings were real, and I reproduced each as a failing test before changing anything. P1a, precedence inversion. P1b, OAuth providers. An unrelated vendor key in the shell became The merge-then-resolve approach can't avoid either, so I dropped it.
Only explicitly configured credentials take part in the Precedence is now Regression tests for both cases are in the same commit; each fails on |
|
Reopened — apologies for the noise. This was closed accidentally by a cross-repo closing keyword: a PR in an unrelated repository of mine referenced this one as "fix #2746", and merging it auto-closed this PR. Nothing here is withdrawn, and the branch is unchanged at 79ebe02. The reference has been corrected on my side so it can't happen again. |
1 participant
Fixes #2745.
Problem
resolveModelAuthMaterialpassesargs.provider?.env ?? {}toexplainProviderEndpoint:explainProviderEndpointdefaults that parameter toprocess.env, so passing{}replaces the default with an empty map for every provider that has no
[providers.<id>.env]table. The vendor's declaredapiKeyEnv(OPENAI_API_KEYfor
type = "openai") is then looked up in an empty map andAuthService.ensureReadythrowsAuthTokenMissingError.The adapters that actually issue the request do read the process environment —
firstProcessEnv(endpoint?.apiKeyEnv)inopenai-legacy.contrib.ts,openai-responses.contrib.ts,anthropic.contrib.tsandgoogle-genai.contrib.ts— so the readiness gate is stricter than the code itguards, and rejects configurations that would have worked.
This was latent behind the experimental flag until 0.33.0 inverted the engine
selection in
experimental-v2.ts(KIMI_CODE_EXPERIMENTAL_FLAGopt-in becameKIMI_CODE_LEGACY_FLAGopt-out), making agent-core-v2 the default forkimi -p.That turned it into a user-visible regression: print mode now rejects an
environment-supplied provider key that 0.32.0 accepted. Full bisect and root-cause
evidence in #2745.
Change
Credentials are resolved in three ordered phases, and the process environment is
consulted only when nothing is configured at any layer.
own
envbag is passed toexplainProviderEndpointon its own, exactly asbefore, so ambient variables never compete with configured ones.
process.env, as a last resort — this is the case that previouslythrew.
Only explicitly configured credentials take part in the
apiKey/oauthmutual-exclusion check.
Resulting precedence:
model.apiKey>model.oauth>provider.apiKey>provider.envbag >provider.oauth>process.envThat is the documented order, with one deliberate difference:
provider.oauthnow outranks an ambient
process.envkey. Nothing regresses, because before thischange
process.envcould not be reached from this function at all.The resolution trace previously labelled every env hit as coming from the
"env bag"; it now distinguishes the bag from the process environment, so
--traceoutput stays truthful.
Note on the first revision of this PR
The initial commit merged
process.envinto the bag before resolution(
{ ...process.env, ...args.provider?.env }). The Codex review correctly flaggedtwo P1 defects in that approach, both of which I reproduced as failing tests
before rewriting it:
standard.contrib.tsdeclaresVERTEXAI_API_KEYahead of
GOOGLE_API_KEY. A provider explicitly configuringGOOGLE_API_KEYon a host that also exported
VERTEXAI_API_KEYresolved the ambient Vertexkey, silently sending the wrong credential.
providerApiKeyand tripped the mutual-exclusion check, so a previously workingoauthprovider failed withconfig.invalid.Resolving the configured bag in isolation and moving the OAuth branch ahead of the
ambient lookup fixes both. Each has a regression test that fails on the previous
revision.
Tests
Six cases in
packages/agent-core-v2/test/kosong/model/modelAuth.test.ts, usingvi.stubEnvwithafterEach(() => vi.unstubAllEnvs()):apiKeyboth still win over the processenvironment;
GOOGLE_API_KEYbeats an ambientVERTEXAI_API_KEYdespite theendpoint chain declaring Vertex first;
oauth;apiKeyalongsideoauthis still rejected, whether the key comesfrom
provider.apiKeyor the provider'senvbag;type = "anthropic"provider does not pick up an unrelatedOPENAI_API_KEY.The first fails on
main; the third and fourth fail on this PR's first revision.packages/agent-core-v2/test/kosongandtest/app/authare green (359 tests), asare
pnpm lintandtsc --noEmitforagent-core-v2.Verification against the shipped CLI
Reproduced on Linux x86_64 / Node v22.22.2 with an
openai-type provider whosekey exists only in
OPENAI_API_KEY. A deliberately invalid key was usedthroughout, so a provider
401is the signal that the readiness gate was passedand the request was actually issued:
kimi -pwith the key in the environment401)provider oai has no credential configuredprovider oai has no credential configureddist/main.mjs401)Flipping the engine flag instead of the version moves the behaviour the same way,
which is what identifies 0.33.0's engine switch rather than any auth change as the
regression trigger:
KIMI_CODE_LEGACY_FLAG=1KIMI_CODE_EXPERIMENTAL_FLAG=1provider oai has no credential configuredA control on the patched bundle confirms the configured bag still wins: with
[providers.oai.env] OPENAI_API_KEYset and a different ambient value present,the request carries the bag's key.
Independent confirmation on two more platforms
The same checks were re-run on two additional machines. Every verdict matched:
So the regression is not specific to a platform, architecture, or Node major.
On macOS the session wire log was inspected directly. A gate-blocked run emits only
metadata,profile.bindandpermission.set_mode— noturn.promptand nollm.request— while a run that passes the gate addsturn.prompt,llm.tools_snapshot,llm.requestandturn.ended. The run aborts before the turnbegins, confirming at the trace level that no request is issued.
End-to-end with a real credential from a production secret store
The runs above use a deliberately invalid key, so they prove the gate's behaviour but
not that a valid environment-supplied credential reaches the model. This one does.
Ubuntu 24.04.3 / x86_64 / Node v22.22.0. The credential is resolved from Bitwarden
Secrets Manager and injected into the child environment by
vaulted-agent, a vaultlauncher whose entire mechanism is environment injection — the real-world shape this
regression breaks. The provider table contains no
api_key, so the environment isthe only possible source. All three probes ran under a single vault injection in
one child shell, so the environment is provably identical and only the binary/flag
differs:
provider oai has no credential configuredPONG— model answeredKIMI_CODE_LEGACY_FLAG=1PONGBecause B succeeds with no flag and no inline
api_key, the failure in A cannot beattributed to key validity, config path, provider wiring, or network reachability.
That transcript was produced with this PR's first revision. The rewrite does not
change behaviour for that configuration — a provider with no
envbag and nooauthreaches the same ambient lookup by both routes — and the dummy-key checkabove was re-run against the current revision on the shipped bundle. I have not
re-run the live-key probe against the rewrite.
The patch was applied to a copy of the shipped
dist/main.mjs; the targetexpression occurs exactly once in the 0.34.0 bundle.
Notes for reviewers
catalogService.tshas thesame
providerConfig.env ?? {}pattern for baseUrl resolution; that one doesnot block a turn, and changing it would let an ambient
OPENAI_BASE_URLredirect configured providers, so I left it alone. Happy to address it here or
in a follow-up if you'd prefer consistency.
resolveModelAuthMaterialthan readprocess.envat this layer, say the wordand I will rework it — the observable behaviour is the part I care about.
an inference from the engine-selection code, not a measurement.