fix(llm-pi-ai): refuse an unusable API key before the header is built
This commit is contained in:
@@ -19,7 +19,7 @@ import z from 'schemastery'
|
||||
import { credentialRef } from '@deepseek-ai/dsh-credentials'
|
||||
import type { CredentialRef } from '@deepseek-ai/dsh-credentials'
|
||||
import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
|
||||
import { resolveRetryPolicy, RetryPolicySchema } from '@deepseek-ai/dsh-llm'
|
||||
import { normalizeApiKey, resolveRetryPolicy, RetryPolicySchema } from '@deepseek-ai/dsh-llm'
|
||||
import type { ResolvedRetryPolicy, RetryPolicyConfig } from '@deepseek-ai/dsh-llm'
|
||||
import { resolveRouteModels } from './catalog.ts'
|
||||
import type { PiAiModelProfile } from './catalog.ts'
|
||||
@@ -38,7 +38,11 @@ export type { PiAiModelProfile } from './catalog.ts'
|
||||
|
||||
/** Configuration for one pi-ai provider route; the `providers` dict key IS the route. */
|
||||
export interface PiAiProviderProfile {
|
||||
/** Literal provider credential; prefer {@link apiKeyEnv}. With both absent pi-ai uses its provider-native ambient discovery. */
|
||||
/**
|
||||
* Literal provider credential; prefer {@link apiKeyEnv}. With both absent pi-ai uses its
|
||||
* provider-native ambient discovery. Trimmed and format-checked by {@link resolveProfiles}; a
|
||||
* value no HTTP header can carry fails there rather than inside `fetch`.
|
||||
*/
|
||||
apiKey?: string
|
||||
/** Credential reference (environment-variable name) resolved per request through `ctx.credentials`. */
|
||||
apiKeyEnv?: string
|
||||
@@ -220,8 +224,18 @@ export function resolveProfiles(
|
||||
for (const [provider, source] of entries) {
|
||||
rejectRemovedFields(provider, source)
|
||||
if (provider.length === 0) throw new Error('llm-pi-ai: provider names must be non-empty')
|
||||
if (source.apiKey !== undefined && source.apiKey.trim().length === 0) {
|
||||
throw new Error(`llm-pi-ai: provider "${provider}" has an empty apiKey; omit it to use ambient authentication`)
|
||||
// Omission selects the installed provider's own auth — ambient discovery
|
||||
// or OAuth — so only a supplied key is judged.
|
||||
let apiKey: string | undefined
|
||||
if (source.apiKey !== undefined) {
|
||||
const checked = normalizeApiKey(source.apiKey)
|
||||
if (!checked.ok) {
|
||||
throw new Error(checked.reason === 'empty'
|
||||
? `llm-pi-ai: provider "${provider}" has an empty apiKey; omit it to use ambient authentication`
|
||||
: `llm-pi-ai: provider "${provider}" has an apiKey containing characters no HTTP header can carry;`
|
||||
+ ' paste the raw key only')
|
||||
}
|
||||
apiKey = checked.value
|
||||
}
|
||||
if (source.baseURL !== undefined && source.baseURL.length === 0) {
|
||||
throw new Error(`llm-pi-ai: provider "${provider}" has an empty baseURL`)
|
||||
@@ -252,6 +266,7 @@ export function resolveProfiles(
|
||||
const { apiKeyEnv, retryPolicy, models: _models, displayName: _displayName, ...rest } = source
|
||||
resolved.set(provider, {
|
||||
...rest,
|
||||
...apiKey === undefined ? {} : { apiKey },
|
||||
provider,
|
||||
displayName,
|
||||
...apiKeyEnv === undefined ? {} : { apiKeyEnv: credentialRef(apiKeyEnv) },
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
* @module dsh-llm-pi-ai/discovery
|
||||
*/
|
||||
|
||||
import { LlmError } from '@deepseek-ai/dsh-llm'
|
||||
import { INVALID_CREDENTIAL_CODE, LlmError, normalizeApiKey } from '@deepseek-ai/dsh-llm'
|
||||
import type { LlmDiscoveredModel, LlmModelDiscoveryRequest } from '@deepseek-ai/dsh-llm'
|
||||
import { attributionHeaders } from '@deepseek-ai/dsh-llm'
|
||||
import { catalogModels } from './catalog.ts'
|
||||
@@ -161,6 +161,25 @@ function readListing(body: unknown): LlmDiscoveredModel[] {
|
||||
return models
|
||||
}
|
||||
|
||||
/**
|
||||
* Accept one probe key, or refuse it before the header is built. Without this
|
||||
* the `fetch` below would throw a ByteString `TypeError` that this function's
|
||||
* catch reports as `could not reach <url>` — blaming the network for a local,
|
||||
* deterministic fault.
|
||||
* @param raw - the key typed into the form or read from storage.
|
||||
* @returns the trimmed, usable key.
|
||||
*/
|
||||
function usableProbeKey(raw: string): string {
|
||||
const checked = normalizeApiKey(raw)
|
||||
if (checked.ok) return checked.value
|
||||
throw new LlmError(
|
||||
checked.reason === 'empty'
|
||||
? 'this provider\'s API key is blank; enter it on the Models page, or clear it to probe unauthenticated'
|
||||
: 'this provider\'s API key contains characters no HTTP header can carry; paste the raw key only',
|
||||
INVALID_CREDENTIAL_CODE,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Interrogate one draft provider endpoint for the models it advertises.
|
||||
* @param request - the endpoint, protocol, and one-shot credential to use.
|
||||
@@ -216,7 +235,10 @@ export async function discoverModels(
|
||||
// stored one is only asked for here, past the catalog short-circuit and the
|
||||
// protocol check, so a route answered from the registry costs no credential
|
||||
// lookup — and no diagnostic about a credential it never needed.
|
||||
const apiKey = request.apiKey ?? await storedApiKey?.()
|
||||
// A probe carrying no key stays unauthenticated, which is how a route that
|
||||
// relies on the provider's own ambient discovery is meant to be asked.
|
||||
const supplied = request.apiKey ?? await storedApiKey?.()
|
||||
const apiKey = supplied === undefined ? undefined : usableProbeKey(supplied)
|
||||
let response: Response
|
||||
try {
|
||||
response = await fetch(url, {
|
||||
|
||||
@@ -43,7 +43,7 @@
|
||||
*/
|
||||
|
||||
import type { Context } from 'cordis'
|
||||
import { LlmError } from '@deepseek-ai/dsh-llm'
|
||||
import { assertUsableApiKey, LlmError } from '@deepseek-ai/dsh-llm'
|
||||
import type { AdapterRegistrationHandle, DirectoryRegistrationHandle, LlmConfigurableProvider } from '@deepseek-ai/dsh-llm'
|
||||
import { deepEqualJson, installSettingsSection, settingsNamespace } from '@deepseek-ai/dsh-settings'
|
||||
import { PiAiAdapter } from './adapter.ts'
|
||||
@@ -145,7 +145,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
// Without the seam, read exactly the named variable so a plain
|
||||
// cordis.yml composition works from the environment alone.
|
||||
: process.env[ref]
|
||||
if (hit !== undefined && hit.length > 0) return hit
|
||||
if (hit !== undefined && hit.length > 0) return assertUsableApiKey(hit, 'llm-pi-ai', ref)
|
||||
throw new LlmError(
|
||||
`llm-pi-ai: no credential for provider route "${provider}"; its profile resolves ${ref}, which is not`
|
||||
+ ` set — store ${ref} through the credentials service (the web Models page writes it) or export it,`
|
||||
|
||||
24
packages/llm/llm-pi-ai/tests/config.spec.ts
Normal file
24
packages/llm/llm-pi-ai/tests/config.spec.ts
Normal file
@@ -0,0 +1,24 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveProfiles } from '../src/config.ts'
|
||||
|
||||
describe('API key format', () => {
|
||||
it('trims a padded literal apiKey into the resolved profile', () => {
|
||||
const resolved = resolveProfiles({ openai: { apiKey: ' sk-abc ', baseURL: 'https://acme.test' } })
|
||||
expect(resolved.get('openai')?.apiKey).toBe('sk-abc')
|
||||
})
|
||||
|
||||
it('keeps an omitted apiKey absent so ambient authentication still applies', () => {
|
||||
const resolved = resolveProfiles({ openai: { baseURL: 'https://acme.test' } })
|
||||
expect(resolved.get('openai')?.apiKey).toBeUndefined()
|
||||
})
|
||||
|
||||
it('still tells an empty apiKey to omit itself', () => {
|
||||
expect(() => resolveProfiles({ openai: { apiKey: ' ', baseURL: 'https://acme.test' } }))
|
||||
.toThrow(/omit it to use ambient authentication/)
|
||||
})
|
||||
|
||||
it('rejects an apiKey no header can carry', () => {
|
||||
expect(() => resolveProfiles({ openai: { apiKey: 'sk-\u{1F600}', baseURL: 'https://acme.test' } }))
|
||||
.toThrow(/no HTTP header can carry/)
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,6 @@
|
||||
import { createServer } from 'node:http'
|
||||
import type { IncomingMessage, Server, ServerResponse } from 'node:http'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import LlmService, { userAgent } from '@deepseek-ai/dsh-llm'
|
||||
import * as LlmPiAi from '@deepseek-ai/dsh-llm-pi-ai'
|
||||
@@ -12,6 +12,9 @@ const servers: Server[] = []
|
||||
const touchedEnv: string[] = []
|
||||
|
||||
afterEach(async () => {
|
||||
// A no-op when the test never stubbed `fetch`; only 'probe key format'
|
||||
// below installs one.
|
||||
vi.unstubAllGlobals()
|
||||
for (const name of touchedEnv.splice(0)) Reflect.deleteProperty(process.env, name)
|
||||
await Promise.all(servers.splice(0).map(server => new Promise(resolve => server.close(resolve))))
|
||||
})
|
||||
@@ -311,3 +314,43 @@ describe('draft-provider model discovery', () => {
|
||||
.rejects.toMatchObject({ code: 'NO_DISCOVERY' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('probe key format', () => {
|
||||
it('reports an illegal probe key as a credential fault, not an unreachable endpoint', async () => {
|
||||
await expect(discoverModels({
|
||||
baseURL: 'https://acme.test',
|
||||
api: 'openai-completions',
|
||||
apiKey: 'sk-\u{1F600}',
|
||||
})).rejects.toMatchObject({ code: 'INVALID_CREDENTIAL' })
|
||||
})
|
||||
|
||||
it('reports a blank probe key as a credential fault too', async () => {
|
||||
// A cleared form field arrives as '', not an absent key; it must fail the
|
||||
// same way a typed-in illegal key does, rather than probing unauthenticated.
|
||||
await expect(discoverModels({
|
||||
baseURL: 'https://acme.test',
|
||||
api: 'openai-completions',
|
||||
apiKey: '',
|
||||
})).rejects.toMatchObject({ code: 'INVALID_CREDENTIAL' })
|
||||
})
|
||||
|
||||
it('leaves a probe with no key unauthenticated', async () => {
|
||||
// The file's other cases capture headers through a real local HTTP server
|
||||
// (`listingServer`); this one has no route or stored key to resolve, so
|
||||
// the smallest real double is a `fetch` stub, scoped to this test and
|
||||
// unstubbed by the shared `afterEach` above.
|
||||
const requests: RequestInit[] = []
|
||||
vi.stubGlobal('fetch', async (_url: string | URL, init?: RequestInit) => {
|
||||
requests.push(init ?? {})
|
||||
return new Response(JSON.stringify({ data: [] }), {
|
||||
status: 200,
|
||||
headers: { 'content-type': 'application/json' },
|
||||
})
|
||||
})
|
||||
|
||||
await discoverModels({ baseURL: 'https://acme.test', api: 'openai-completions' })
|
||||
|
||||
const headers = new Headers(requests[0]?.headers)
|
||||
expect(headers.has('authorization')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user