diff --git a/.changeset/print-mode-env-provider-key.md b/.changeset/print-mode-env-provider-key.md new file mode 100644 index 0000000000..237d9bbc37 --- /dev/null +++ b/.changeset/print-mode-env-provider-key.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix `kimi -p` refusing to start with an environment-supplied provider key: the auth readiness gate looked the vendor's declared `apiKeyEnv` up in the provider's `env` bag alone, so a key present only in the process environment was reported as `provider has no credential configured` even though the adapter issuing the request would have read it. The gate now falls back to the process environment, with an explicit `[providers..env]` entry still taking precedence. diff --git a/packages/agent-core-v2/src/kosong/model/modelAuth.ts b/packages/agent-core-v2/src/kosong/model/modelAuth.ts index e2a923edd4..340348b600 100644 --- a/packages/agent-core-v2/src/kosong/model/modelAuth.ts +++ b/packages/agent-core-v2/src/kosong/model/modelAuth.ts @@ -62,25 +62,35 @@ export function resolveModelAuthMaterial( } const providerAuthType = args.provider?.type ?? args.model.protocol; - const providerEndpoint = + + // Explicitly configured credentials are resolved first and in isolation: the + // provider's own `env` bag must not have to compete with ambient process env, + // or a vendor whose endpoint chain declares several keys (google-genai lists + // VERTEXAI_API_KEY ahead of GOOGLE_API_KEY) could resolve an ambient key in + // preference to the one the user configured. + const configuredEndpoint = providerAuthType === undefined ? {} : explainProviderEndpoint(providerAuthType, args.provider?.env ?? {}); - const providerApiKey = nonEmpty(args.provider?.apiKey) ?? nonEmpty(providerEndpoint.apiKey); - if (providerApiKey !== undefined && args.provider?.oauth !== undefined) { + const configuredApiKey = nonEmpty(args.provider?.apiKey) ?? nonEmpty(configuredEndpoint.apiKey); + + // Only explicitly configured credentials participate in the apiKey/oauth + // conflict check. An unrelated key that merely happens to exist in the shell + // must never invalidate a working oauth provider. + if (configuredApiKey !== undefined && args.provider?.oauth !== undefined) { throw authConflictError('Provider', args.providerName); } - if (providerApiKey !== undefined) { + if (configuredApiKey !== undefined) { trace?.record( 'resolved.auth', nonEmpty(args.provider?.apiKey) !== undefined ? { kind: 'config', detail: `provider '${args.providerName}' apiKey` } : { kind: 'env', - detail: `${providerEndpoint.apiKeyEnvName ?? '?'} (provider '${args.providerName}' env bag)`, + detail: `${configuredEndpoint.apiKeyEnvName ?? '?'} (provider '${args.providerName}' env bag)`, }, ); - return { apiKey: providerApiKey }; + return { apiKey: configuredApiKey }; } if (args.provider?.oauth !== undefined) { trace?.record('resolved.auth', { @@ -92,6 +102,20 @@ export function resolveModelAuthMaterial( oauthProviderKey: args.model.providerId ?? args.model.provider, }; } + + // Nothing was configured anywhere. Fall back to the ambient process env, which + // is what the adapters themselves read when they construct the request; without + // this the readiness gate is stricter than the code it guards. + const ambientEndpoint = + providerAuthType === undefined ? {} : explainProviderEndpoint(providerAuthType, process.env); + const ambientApiKey = nonEmpty(ambientEndpoint.apiKey); + if (ambientApiKey !== undefined) { + trace?.record('resolved.auth', { + kind: 'env', + detail: `${ambientEndpoint.apiKeyEnvName ?? '?'} (process env)`, + }); + return { apiKey: ambientApiKey }; + } trace?.record('resolved.auth', { kind: 'none', detail: 'no credential resolved at any layer (adapter construction may still read process.env)', diff --git a/packages/agent-core-v2/test/kosong/model/modelAuth.test.ts b/packages/agent-core-v2/test/kosong/model/modelAuth.test.ts index ec7540a765..682801be27 100644 --- a/packages/agent-core-v2/test/kosong/model/modelAuth.test.ts +++ b/packages/agent-core-v2/test/kosong/model/modelAuth.test.ts @@ -12,7 +12,7 @@ * profile — inferred only for vendors whose thinking is not trait-driven. */ -import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { ConfigErrors } from '#/app/config/errors'; import '#/kosong/provider/providers/kimi/kimi.contrib'; @@ -101,10 +101,85 @@ describe('resolveModelAuthMaterial', () => { ).toEqual({ apiKey: 'vertex-env-key' }); }); + it('falls back to the process environment when the provider declares no env bag', () => { + vi.stubEnv('OPENAI_API_KEY', 'process-env-key'); + expect(authMaterial({ model: { model: 'm' }, provider: { type: 'openai' } })).toEqual({ + apiKey: 'process-env-key', + }); + }); + + it('prefers the provider env bag and inline apiKey over the process environment', () => { + vi.stubEnv('OPENAI_API_KEY', 'process-env-key'); + expect( + authMaterial({ + model: { model: 'm' }, + provider: { type: 'openai', env: { OPENAI_API_KEY: 'bag-key' } }, + }), + ).toEqual({ apiKey: 'bag-key' }); + expect( + authMaterial({ + model: { model: 'm' }, + provider: { type: 'openai', apiKey: 'inline-key' }, + }), + ).toEqual({ apiKey: 'inline-key' }); + }); + + it('prefers a configured env-bag key over an ambient key declared earlier in the chain', () => { + // google-genai declares VERTEXAI_API_KEY ahead of GOOGLE_API_KEY, so an + // ambient Vertex key must not outrank the Gemini key the user configured. + vi.stubEnv('VERTEXAI_API_KEY', 'ambient-vertex-key'); + expect( + authMaterial({ + model: { model: 'm' }, + provider: { type: 'google-genai', env: { GOOGLE_API_KEY: 'configured-google-key' } }, + }), + ).toEqual({ apiKey: 'configured-google-key' }); + }); + + it('does not let an ambient key invalidate a provider configured for oauth', () => { + vi.stubEnv('OPENAI_API_KEY', 'ambient-unrelated-key'); + expect( + authMaterial({ + model: { model: 'm', providerId: 'p1' }, + provider: { type: 'openai', oauth: { storage: 'file', key: 'k' } }, + }), + ).toEqual({ oauth: { storage: 'file', key: 'k' }, oauthProviderKey: 'p1' }); + }); + + it('still rejects a configured apiKey alongside oauth', () => { + expect(() => + authMaterial({ + model: { model: 'm' }, + provider: { type: 'openai', apiKey: 'k', oauth: { storage: 'file', key: 'k' } }, + }), + ).toThrowError(expect.objectContaining({ code: ConfigErrors.codes.CONFIG_INVALID })); + expect(() => + authMaterial({ + model: { model: 'm' }, + provider: { + type: 'openai', + env: { OPENAI_API_KEY: 'bag-key' }, + oauth: { storage: 'file', key: 'k' }, + }, + }), + ).toThrowError(expect.objectContaining({ code: ConfigErrors.codes.CONFIG_INVALID })); + }); + + it('does not leak an unrelated vendor key from the process environment', () => { + vi.stubEnv('OPENAI_API_KEY', 'process-env-key'); + vi.stubEnv('ANTHROPIC_API_KEY', ''); + expect(authMaterial({ model: { model: 'm' }, provider: { type: 'anthropic' } })).toEqual({}); + }); + it('returns empty material when nothing is configured', () => { + vi.stubEnv('OPENAI_API_KEY', ''); expect(authMaterial({ model: { model: 'm' }, provider: { type: 'openai' } })).toEqual({}); expect(authMaterial({ model: { model: 'm' } })).toEqual({}); }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); }); describe('effectiveModelConfig', () => {