From 5514dd2bd6409d076bfa963a1c835fdd97e3f61b Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Thu, 6 Aug 2026 21:39:57 +0800 Subject: [PATCH] fix(llm-deepseek): refuse an API key no header can carry --- docs/config-catalog.md | 6 +++- packages/llm/llm-deepseek/src/index.ts | 27 +++++++++++++---- .../llm/llm-deepseek/tests/adapter.spec.ts | 30 +++++++++++++++++++ .../llm-deepseek/tests/dynamic-config.spec.ts | 21 ++++++++++++- 4 files changed, 77 insertions(+), 7 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 89f1529387..a3e63d994d 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -642,7 +642,11 @@ Requires: `llm` * reasoning effort resolves to `high`. */ export interface Config { - /** Literal API key; prefer {@link apiKeyEnv} so no secret enters configuration files. */ + /** + * Literal API key; prefer {@link apiKeyEnv} so no secret enters configuration files. Trimmed + * and format-checked by {@link resolveAdapterOptions}; a value no HTTP header can carry fails + * there rather than inside `fetch`. + */ apiKey?: string /** Credential reference (environment-variable name) resolved per request; defaults to `DEEPSEEK_API_KEY`. */ apiKeyEnv?: string diff --git a/packages/llm/llm-deepseek/src/index.ts b/packages/llm/llm-deepseek/src/index.ts index cd2bb9a24e..6aaab15573 100644 --- a/packages/llm/llm-deepseek/src/index.ts +++ b/packages/llm/llm-deepseek/src/index.ts @@ -13,7 +13,7 @@ import type { Context } from 'cordis' import z from 'schemastery' -import { LlmError, resolveRetryPolicy, RetryPolicySchema } from '@deepseek-ai/dsh-llm' +import { assertUsableApiKey, LlmError, normalizeApiKey, resolveRetryPolicy, RetryPolicySchema } from '@deepseek-ai/dsh-llm' import type { RetryPolicyConfig } from '@deepseek-ai/dsh-llm' import { credentialRef } from '@deepseek-ai/dsh-credentials' import { deepEqualJson, installSettingsSection, settingsNamespace } from '@deepseek-ai/dsh-settings' @@ -58,7 +58,11 @@ const DEFAULT_MODELS: DeepSeekCatalogModel[] = [ * reasoning effort resolves to `high`. */ export interface Config { - /** Literal API key; prefer {@link apiKeyEnv} so no secret enters configuration files. */ + /** + * Literal API key; prefer {@link apiKeyEnv} so no secret enters configuration files. Trimmed + * and format-checked by {@link resolveAdapterOptions}; a value no HTTP header can carry fails + * there rather than inside `fetch`. + */ apiKey?: string /** Credential reference (environment-variable name) resolved per request; defaults to `DEEPSEEK_API_KEY`. */ apiKeyEnv?: string @@ -174,8 +178,21 @@ export function resolveAdapterOptions(config: Config): ResolvedDeepSeekOptions { `llm-deepseek: streamIdleTimeoutMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`, ) } + // An absent apiKey is not a failure: it falls through to apiKeyEnv below. + // A supplied one must be usable, so a malformed literal fails here beside + // the other beyond-schema bounds instead of inside `fetch`. + let apiKey: string | undefined + if (config.apiKey !== undefined) { + const checked = normalizeApiKey(config.apiKey) + if (!checked.ok) { + throw new Error(checked.reason === 'empty' + ? 'llm-deepseek: apiKey is empty; omit it to resolve the key from apiKeyEnv' + : 'llm-deepseek: apiKey contains characters no HTTP header can carry; paste the raw key only') + } + apiKey = checked.value + } return { - ...config.apiKey !== undefined && config.apiKey.length > 0 ? { apiKey: config.apiKey } : {}, + ...apiKey === undefined ? {} : { apiKey }, apiKeyEnv: credentialRef(config.apiKeyEnv ?? DEFAULT_API_KEY_ENV), baseURL: config.baseURL ?? process.env.DEEPSEEK_BASE_URL ?? PUBLIC_BASE_URL, defaults: { @@ -223,12 +240,12 @@ export function apply(ctx: Context, config: Config): void { const credentials = ctx.get('credentials') if (credentials !== undefined) { const hit = await credentials.resolve(ref) - if (hit !== undefined) return hit.value + if (hit !== undefined) return assertUsableApiKey(hit.value, 'llm-deepseek', ref) } else { // Without the seam, keep the historical ambient fallback so a plain // cordis.yml composition works from the environment alone. const ambient = process.env[ref] - if (ambient !== undefined && ambient.length > 0) return ambient + if (ambient !== undefined && ambient.length > 0) return assertUsableApiKey(ambient, 'llm-deepseek', ref) } throw new LlmError( `llm-deepseek: no API key for provider route "${PROVIDER}"; store ${ref} through the credentials` diff --git a/packages/llm/llm-deepseek/tests/adapter.spec.ts b/packages/llm/llm-deepseek/tests/adapter.spec.ts index 9d104ace08..56ac3eb138 100644 --- a/packages/llm/llm-deepseek/tests/adapter.spec.ts +++ b/packages/llm/llm-deepseek/tests/adapter.spec.ts @@ -991,3 +991,33 @@ describe('plugin registration and config', () => { expect(ctx.llm.listProviders()).toEqual([]) }) }) + +describe('API key format', () => { + it('trims a padded literal apiKey', () => { + expect(resolveAdapterOptions({ apiKey: ' sk-abc ' }).apiKey).toBe('sk-abc') + }) + + it('leaves an omitted apiKey absent so apiKeyEnv still resolves it', () => { + expect(resolveAdapterOptions({}).apiKey).toBeUndefined() + }) + + it('rejects a literal apiKey of whitespace only', () => { + expect(() => resolveAdapterOptions({ apiKey: ' ' })) + .toThrow(/apiKey is empty; omit it/) + }) + + it('rejects a literal apiKey no header can carry', () => { + expect(() => resolveAdapterOptions({ apiKey: 'sk-\u{1F600}' })) + .toThrow(/no HTTP header can carry/) + }) + + it('never echoes the key in the rejection', () => { + const secret = 'sk-\u{1F600}supersecret' + expect(() => resolveAdapterOptions({ apiKey: secret })).toThrow() + try { + resolveAdapterOptions({ apiKey: secret }) + } catch (error) { + expect((error as Error).message).not.toContain('supersecret') + } + }) +}) diff --git a/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts b/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts index 11df9e1d81..e593e3a61d 100644 --- a/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts +++ b/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts @@ -3,7 +3,7 @@ import { Context } from 'cordis' import { mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import LlmService from '@deepseek-ai/dsh-llm' +import LlmService, { INVALID_CREDENTIAL_CODE } from '@deepseek-ai/dsh-llm' import { credentialRef } from '@deepseek-ai/dsh-credentials' import { CredentialsLocal } from '@deepseek-ai/dsh-credentials-local' import { settingsNamespace } from '@deepseek-ai/dsh-settings' @@ -103,6 +103,25 @@ describe('request-level dynamic configuration', () => { expect(server.headers[0]?.authorization).toBe('Bearer sk-arrived') }) + it('rejects a stored credential no header can carry, never echoing it in the failure', async () => { + vi.stubEnv('DEEPSEEK_API_KEY', '') + const dir = await home() + const { ctx } = await boot(dir, { baseURL: 'http://127.0.0.1:1' }) + const secret = 'sk-\u{1F600}supersecret' + + // The real credentials seam (the path the web Models page writes through), + // not a hand-built stub: this package's own dynamic-config harness already + // boots one, and round-tripping the value through its actual store/read + // path is stronger evidence than a canned in-memory return would be. + await ctx.credentials.set(KEY_REF, secret) + const result = await prompt(ctx) + expect(result.finish).toMatchObject({ kind: 'error', failure: { code: INVALID_CREDENTIAL_CODE } }) + if (result.finish.kind !== 'error') throw new Error('expected an error finish') + expect(result.finish.failure.message).not.toContain(secret) + expect(result.finish.failure.message).not.toContain('supersecret') + expect(result.finish.failure.message).not.toContain('ByteString') + }) + it('advertises a live settings catalog without re-registration', async () => { const dir = await home() const { ctx } = await boot(dir, { apiKey: 'k', baseURL: 'http://127.0.0.1:1' })