feat(web): refuse an unusable API key on the field that holds it
This commit is contained in:
6 files changed
+233
-8
No files matched your search
@@ -18,6 +18,7 @@
|
||||
import { useState } from 'react'
|
||||
import type { ReactNode } from 'react'
|
||||
import type { IApiClient } from '@deepseek-ai/dsh-client-connection/client'
|
||||
import { apiKeyFailure } from './apiKey.ts'
|
||||
import { EditorFooter } from './EditorFooter.tsx'
|
||||
import { validateDeepSeekModels } from './DeepSeekModelsEditor.tsx'
|
||||
import { ModelListEditor } from './ModelListEditor.tsx'
|
||||
@@ -80,8 +81,14 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
|
||||
// bad row is named by its position here too. Capacities have route-level
|
||||
// fallbacks; what a route cannot default is at least one model.
|
||||
const modelFailure = validateDeepSeekModels(models)
|
||||
const keyFailure = apiKeyFailure(keyDraft)
|
||||
// The typed key with paste whitespace removed. A blank field yields an empty
|
||||
// string, which the create path reads as "no key supplied" — a route may
|
||||
// legitimately authenticate through the provider's own ambient discovery.
|
||||
const keyValue = keyDraft.trim()
|
||||
const ready = route.length > 0 && !routeInvalid && !routeTaken
|
||||
&& baseURL.length > 0 && models.length > 0 && modelFailure === undefined
|
||||
&& keyFailure === undefined
|
||||
// The one blocked gate worth a line under the form. The route id is omitted
|
||||
// because its own field already explains itself, and a satisfied card says
|
||||
// nothing at all rather than printing an empty paragraph.
|
||||
@@ -112,8 +119,8 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
|
||||
expectedRevision: openedAt,
|
||||
})
|
||||
if (!response.result.ok) return response.result.error.message
|
||||
if (keyDraft.length > 0) {
|
||||
const stored = await api.credentials.set({ ref: keyRef, value: keyDraft })
|
||||
if (keyValue.length > 0) {
|
||||
const stored = await api.credentials.set({ ref: keyRef, value: keyValue })
|
||||
// The profile landed; saying the key did not is the only honest report,
|
||||
// and the row is now editable so the key can be entered again there.
|
||||
if (!stored.result.ok) return stored.result.error.message
|
||||
@@ -208,6 +215,7 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
|
||||
disabled={disabled}
|
||||
onChange={(event) => { setKeyDraft(event.target.value) }}
|
||||
/>
|
||||
{keyFailure === undefined ? null : <p className={styles['error']}>{t(keyFailure)}</p>}
|
||||
</div>
|
||||
<ModelListEditor
|
||||
models={models}
|
||||
@@ -216,7 +224,7 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
|
||||
settingsNs: NS,
|
||||
baseURL,
|
||||
api: protocol,
|
||||
...keyDraft.length === 0 ? {} : { apiKey: keyDraft },
|
||||
...keyValue.length === 0 ? {} : { apiKey: keyValue },
|
||||
}}
|
||||
api={api}
|
||||
t={t}
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
import {
|
||||
DeepSeekModelsEditor, modelDrafts, validateDeepSeekModels,
|
||||
} from './DeepSeekModelsEditor.tsx'
|
||||
import { apiKeyFailure } from './apiKey.ts'
|
||||
import { EditorFooter } from './EditorFooter.tsx'
|
||||
import { ModelListEditor } from './ModelListEditor.tsx'
|
||||
import { deriveKeyRef, messageOf } from './store.ts'
|
||||
@@ -163,7 +164,7 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
||||
|
||||
const stringAt = (source: unknown, key: string): string | undefined => {
|
||||
const value = getPath(source, [key])
|
||||
return typeof value === 'string' && value.length > 0 ? value : undefined
|
||||
return typeof value === 'string' && value.trim().length > 0 ? value : undefined
|
||||
}
|
||||
const setField = (key: string, next: string | undefined): void => {
|
||||
setDraft(current => next === undefined ? deletePath(current, [key]) : setPath(current, [key], next))
|
||||
@@ -172,6 +173,12 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
||||
// The model list is validated by the same per-row checker for both families,
|
||||
// so a bad row is named by its position rather than by a blanket message.
|
||||
const modelFailure = validateDeepSeekModels(getPath(draft, ['models']))
|
||||
const keyFailure = apiKeyFailure(keyDraft)
|
||||
// What a probe or a write must carry: the typed key with paste whitespace
|
||||
// removed. A blank field yields an empty string, which both call sites read
|
||||
// as "no key supplied" rather than as a key — that is how a card whose
|
||||
// provider already has a stored key is edited without re-entering it.
|
||||
const keyValue = keyDraft.trim()
|
||||
// What the form currently shows, which is what an interrogation must ask:
|
||||
// an edited-but-unsaved endpoint, and a key typed but not yet stored.
|
||||
const probeApi = stringAt(draft, 'api') ?? stringAt(fallback, 'api')
|
||||
@@ -183,7 +190,7 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
||||
provider: props.provider,
|
||||
...probeBaseURL === undefined ? {} : { baseURL: probeBaseURL },
|
||||
...probeApi === undefined ? {} : { api: probeApi },
|
||||
...keyDraft.length === 0 ? {} : { apiKey: keyDraft },
|
||||
...keyValue.length === 0 ? {} : { apiKey: keyValue },
|
||||
}
|
||||
/**
|
||||
* The write for this card, or a failure message. Every edit travels as
|
||||
@@ -226,8 +233,8 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
||||
: response.result.error.message
|
||||
}
|
||||
}
|
||||
if (keyDraft.length > 0) {
|
||||
const stored = await api.credentials.set({ ref: keyRef, value: keyDraft })
|
||||
if (keyValue.length > 0) {
|
||||
const stored = await api.credentials.set({ ref: keyRef, value: keyValue })
|
||||
if (!stored.result.ok) return stored.result.error.message
|
||||
}
|
||||
setKeyDraft('')
|
||||
@@ -313,6 +320,7 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
||||
disabled={disabled || keyLocked}
|
||||
onChange={(event) => { setKeyDraft(event.target.value) }}
|
||||
/>
|
||||
{keyFailure === undefined ? null : <p className={styles['error']}>{t(keyFailure)}</p>}
|
||||
</div>
|
||||
<details className={styles['customized']}>
|
||||
<summary className={styles['customizedSummary']}>{t('customized')}</summary>
|
||||
@@ -396,7 +404,8 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
||||
<EditorFooter
|
||||
t={t}
|
||||
busy={busy}
|
||||
submitDisabled={disabled || layout === 'unknown' || modelFailure !== undefined}
|
||||
submitDisabled={disabled || layout === 'unknown' || modelFailure !== undefined
|
||||
|| keyFailure !== undefined}
|
||||
submitLabel="apply"
|
||||
submitBusyLabel="applying"
|
||||
onCancel={() => { props.onClose(false) }}
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
/**
|
||||
* Browser-side judgement of a typed API key.
|
||||
* @module @deepseek-ai/dsh-client-ui-models/apiKey
|
||||
*/
|
||||
|
||||
/**
|
||||
* Twin of `normalizeApiKey` in `@deepseek-ai/dsh-llm`: printable ASCII, space
|
||||
* excluded. Client packages reference only client packages, so the charset
|
||||
* rule is mirrored here rather than imported; keep the two in step, as
|
||||
* `validateDeepSeekModels` is kept in step with the host's `catalogModel`.
|
||||
*/
|
||||
const LEGAL_API_KEY = /^[\x21-\x7E]+$/
|
||||
|
||||
/**
|
||||
* A pasted `NAME=value` environment line. Restricted to an upper-case
|
||||
* identifier so a real key cannot match: `sk-` forms break at the hyphen.
|
||||
* This heuristic runs only here — a resolver applying it could lock a user
|
||||
* out of a gateway whose key legitimately takes this shape, with the
|
||||
* environment refusing it too and no way through.
|
||||
*/
|
||||
const ENV_LINE = /^[A-Z][A-Z0-9_]*=/
|
||||
|
||||
/** Copy key naming why a typed key cannot be saved. */
|
||||
export type ApiKeyFailureKey = 'keyBlank' | 'keyIllegalCharacters' | 'keyLooksWrapped'
|
||||
|
||||
/** Whether a value is wrapped in one matching pair of quotes. */
|
||||
function isQuoted(value: string): boolean {
|
||||
const first = value[0]
|
||||
if (first !== '"' && first !== '\'' && first !== '`') return false
|
||||
return value.length > 1 && value.endsWith(first)
|
||||
}
|
||||
|
||||
/**
|
||||
* Judge the key input's current value.
|
||||
*
|
||||
* An empty field is not a failure: every card opens with it empty even when a
|
||||
* key is already stored, where it means keep that one. A field holding only
|
||||
* whitespace is a failure rather than an empty field, so typed input is never
|
||||
* silently discarded.
|
||||
* @param draft - the key input's current value, untrimmed.
|
||||
* @returns the copy key for a field-level failure, or `undefined` to allow submit.
|
||||
*/
|
||||
export function apiKeyFailure(draft: string): ApiKeyFailureKey | undefined {
|
||||
if (draft.length === 0) return undefined
|
||||
const value = draft.trim()
|
||||
if (value.length === 0) return 'keyBlank'
|
||||
if (ENV_LINE.test(value) || isQuoted(value)) return 'keyLooksWrapped'
|
||||
if (!LEGAL_API_KEY.test(value)) return 'keyIllegalCharacters'
|
||||
return undefined
|
||||
}
|
||||
@@ -46,6 +46,9 @@ export const en = {
|
||||
addModel: 'Add model',
|
||||
removeModel: 'Delete model',
|
||||
modelsEmpty: 'No models will be shown in the selector. Unlisted IDs can still be sent directly.',
|
||||
keyBlank: 'Enter the API key, or leave the field empty to keep the stored one.',
|
||||
keyIllegalCharacters: 'This API key contains characters that cannot be sent. Paste the raw key only.',
|
||||
keyLooksWrapped: 'Paste only the key itself — not a NAME=value line, and without surrounding quotes.',
|
||||
modelIdRequired: 'Model ID is required.',
|
||||
modelIdDuplicate: 'Model ID must be unique.',
|
||||
modelNameInvalid: 'Display name cannot be empty.',
|
||||
@@ -130,6 +133,9 @@ export const zh: typeof en = {
|
||||
addModel: '添加模型',
|
||||
removeModel: '删除模型',
|
||||
modelsEmpty: '模型选择器中将不显示任何模型;目录外 ID 仍可直接发送。',
|
||||
keyBlank: '请输入 API 密钥;留空则保持已存储的密钥。',
|
||||
keyIllegalCharacters: '该 API 密钥含有无法发送的字符。请只粘贴原始密钥。',
|
||||
keyLooksWrapped: '请只粘贴密钥本身——不要带 NAME=value 整行,也不要带引号。',
|
||||
modelIdRequired: '模型 ID 不能为空。',
|
||||
modelIdDuplicate: '模型 ID 不能重复。',
|
||||
modelNameInvalid: '显示名称不能为空。',
|
||||
|
||||
@@ -11,6 +11,7 @@ import { pathOps } from '../src/client/ProviderEditor.tsx'
|
||||
import {
|
||||
DeepSeekModelsEditor, formatCapacity, modelDrafts, parseCapacity, validateDeepSeekModels,
|
||||
} from '../src/client/DeepSeekModelsEditor.tsx'
|
||||
import { apiKeyFailure } from '../src/client/apiKey.ts'
|
||||
import { deriveKeyRef, ModelsSettingsStore } from '../src/client/store.ts'
|
||||
import type { ProviderRow } from '../src/client/store.ts'
|
||||
import { en } from '../src/client/locales.ts'
|
||||
@@ -1080,3 +1081,52 @@ describe('ModelsSection', () => {
|
||||
expect(failure).toBe('connection lost')
|
||||
})
|
||||
})
|
||||
|
||||
describe('apiKeyFailure', () => {
|
||||
it('treats a blank field as no failure — it means keep the stored key', () => {
|
||||
expect(apiKeyFailure('')).toBeUndefined()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['a printable-ASCII key', 'sk-0123456789'],
|
||||
['a padded key, which the caller trims', ' sk-abc '],
|
||||
['the printable-ASCII boundary characters', '!~'],
|
||||
['a hyphenated key carrying an equals sign', 'sk-ABC=xyz'],
|
||||
])('accepts %s', (_label, draft) => {
|
||||
expect(apiKeyFailure(draft)).toBeUndefined()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['spaces', ' '],
|
||||
['a tab', '\t'],
|
||||
])('fails a field holding only %s instead of silently dropping it', (_label, draft) => {
|
||||
expect(apiKeyFailure(draft)).toBe('keyBlank')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['an emoji', 'sk-\u{1F600}'],
|
||||
['CJK text', 'sk-你好'],
|
||||
['full-width punctuation', 'sk-abc,'],
|
||||
['an interior space', 'sk-abc def'],
|
||||
['a C0 control character', 'sk-abc\x01'],
|
||||
['a latin-1 character', 'sk-café'],
|
||||
])('fails %s as illegal characters', (_label, draft) => {
|
||||
expect(apiKeyFailure(draft)).toBe('keyIllegalCharacters')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['a pasted environment line', 'DEEPSEEK_API_KEY=sk-abc'],
|
||||
['double quotes', '"sk-abc"'],
|
||||
['single quotes', '\'sk-abc\''],
|
||||
['backticks', '`sk-abc`'],
|
||||
])('fails %s as wrapped', (_label, draft) => {
|
||||
expect(apiKeyFailure(draft)).toBe('keyLooksWrapped')
|
||||
})
|
||||
|
||||
it('needs a matching closing quote before it calls a value wrapped', () => {
|
||||
// A lone quote and an unbalanced one are legal printable ASCII, so the
|
||||
// heuristic leaves them alone rather than guessing at a paste error.
|
||||
expect(apiKeyFailure('"')).toBeUndefined()
|
||||
expect(apiKeyFailure('"a')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -862,4 +862,106 @@ describe('hand-declared providers', () => {
|
||||
await waitFor(() => { expect(screen.queryByText(en.customTitle)).toBeNull() })
|
||||
expect(screen.getByRole('button', { name: en.customAdd })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('refuses an unusable key on the field and blocks creation', () => {
|
||||
const { mutate, set } = mountCard()
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.customRoute), { target: { value: 'acme-gateway' } })
|
||||
fireEvent.change(screen.getByLabelText(en.baseUrl), { target: { value: 'https://gateway.acme.example/v1' } })
|
||||
fireEvent.click(screen.getByRole('button', { name: en.addModel }))
|
||||
fireEvent.change(screen.getByLabelText(`${en.modelId} 1`), { target: { value: 'acme-large' } })
|
||||
fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: 'sk-\u{1F600}' } })
|
||||
|
||||
// A hand-declared route reaches the same judgement as an edited one, so a
|
||||
// key that no header can carry never becomes a profile plus a bad secret.
|
||||
expect(screen.getByText(en.keyIllegalCharacters)).toBeTruthy()
|
||||
expect(buttonNamed(en.create).disabled).toBe(true)
|
||||
expect(mutate).not.toHaveBeenCalled()
|
||||
expect(set).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('creates without a key when the route authenticates some other way', async () => {
|
||||
const { set, onClose } = mountCard()
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.customRoute), { target: { value: 'ambient-gateway' } })
|
||||
fireEvent.change(screen.getByLabelText(en.baseUrl), { target: { value: 'https://gateway.acme.example/v1' } })
|
||||
fireEvent.click(screen.getByRole('button', { name: en.addModel }))
|
||||
fireEvent.change(screen.getByLabelText(`${en.modelId} 1`), { target: { value: 'acme-large' } })
|
||||
fireEvent.click(screen.getByText(en.create))
|
||||
|
||||
await waitFor(() => { expect(onClose).toHaveBeenCalledWith(true) })
|
||||
expect(set).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('API key field', () => {
|
||||
it('submits with a blank key field without writing a credential', async () => {
|
||||
const { mutate, set } = await mountSection()
|
||||
openEditor('openai')
|
||||
|
||||
// The field opens empty even for a provider whose key is stored, where it
|
||||
// means "keep that one" — so editing anything else must not require it.
|
||||
fireEvent.change(screen.getByLabelText(en.baseUrl), { target: { value: 'https://moved.example/v1' } })
|
||||
expect(buttonNamed(en.apply).disabled).toBe(false)
|
||||
fireEvent.click(screen.getByText(en.apply))
|
||||
|
||||
await waitFor(() => { expect(mutate).toHaveBeenCalled() })
|
||||
expect(set).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('blocks submit and names the field when the key holds only whitespace', async () => {
|
||||
const { mutate, set } = await mountSection()
|
||||
openEditor('openai')
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: ' ' } })
|
||||
|
||||
expect(screen.getByText(en.keyBlank)).toBeTruthy()
|
||||
expect(buttonNamed(en.apply).disabled).toBe(true)
|
||||
expect(mutate).not.toHaveBeenCalled()
|
||||
expect(set).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('blocks submit when the key contains characters no header can carry', async () => {
|
||||
const { set } = await mountSection()
|
||||
openEditor('openai')
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: 'sk-\u{1F600}' } })
|
||||
|
||||
expect(screen.getByText(en.keyIllegalCharacters)).toBeTruthy()
|
||||
expect(buttonNamed(en.apply).disabled).toBe(true)
|
||||
expect(set).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('blocks submit when a whole NAME=value line was pasted', async () => {
|
||||
await mountSection()
|
||||
openEditor('openai')
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: 'OPENAI_API_KEY=sk-abc' } })
|
||||
|
||||
expect(screen.getByText(en.keyLooksWrapped)).toBeTruthy()
|
||||
expect(buttonNamed(en.apply).disabled).toBe(true)
|
||||
})
|
||||
|
||||
it('trims a padded key before storing it', async () => {
|
||||
const { set } = await mountSection()
|
||||
openEditor('openai')
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: ' sk-abc ' } })
|
||||
expect(buttonNamed(en.apply).disabled).toBe(false)
|
||||
fireEvent.click(screen.getByText(en.apply))
|
||||
|
||||
await waitFor(() => { expect(set).toHaveBeenCalled() })
|
||||
expect((set.mock.calls[0]?.[0] as { value: string }).value).toBe('sk-abc')
|
||||
})
|
||||
|
||||
it('carries the trimmed key into an interrogation, not the padded draft', async () => {
|
||||
const { discover } = await mountSection()
|
||||
openEditor('openai')
|
||||
|
||||
fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: ' sk-abc ' } })
|
||||
fireEvent.click(screen.getByRole('button', { name: en.fetchModels }))
|
||||
|
||||
await waitFor(() => { expect(discover).toHaveBeenCalled() })
|
||||
expect(firstProbe(discover)).toMatchObject({ apiKey: 'sk-abc' })
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user