From a89c26b6110420ff59582528839d01230c970e9a Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Thu, 6 Aug 2026 22:42:56 +0800 Subject: [PATCH] test(web): pin the API key field refusal end to end --- ...-08-06-api-key-format-validation.i18n.yaml | 6 + .../2026-08-06-api-key-format-validation.md | 105 ++++++++++++++++++ ...2026-08-06-api-key-format-validation.zh.md | 105 ++++++++++++++++++ ...-08-06-api-key-format-validation.i18n.yaml | 6 - .../2026-08-06-api-key-format-validation.md | 101 ----------------- ...2026-08-06-api-key-format-validation.zh.md | 101 ----------------- apps/web/tests/models-settings.e2e.ts | 19 ++++ 7 files changed, 235 insertions(+), 208 deletions(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md create mode 100644 .agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md delete mode 100644 .agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml delete mode 100644 .agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.md delete mode 100644 .agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.zh.md diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml new file mode 100644 index 0000000000..42b42a591a --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md +2026-08-06-api-key-format-validation.md: 9ec247cb2ba2578158759ec1115c5d3a95778cc4 +2026-08-06-api-key-format-validation.zh.md: 63c6a8c17ee93b4b68eb3505d5499756e9fb2401 diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md new file mode 100644 index 0000000000..9ec247cb2b --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.md @@ -0,0 +1,105 @@ +# Agent Note: Validate API key format before it reaches an HTTP header + +Status: implemented + +English | [中文](2026-08-06-api-key-format-validation.zh.md) + +## Problem + +An API key holding characters no HTTP header value can carry was accepted by every configuration surface and failed only when a request was built, far from the field that caused it. + +Pasting a key containing an emoji, CJK text, or a full-width punctuation mark into the web Models page reported a successful save. The first turn then failed with `Cannot convert argument to a ByteString because the character at index 7 has a value of 55357 which is greater than 255` — the index and code point are UTF-16 internals with no action attached, and they disclose the code point of one character of the key. `llm-deepseek` produced this because `fetch` builds the `Bearer` header inside the `try` in [adapter.ts](../../../../packages/llm/llm-deepseek/src/adapter.ts), whose `catch` labels every failure `TRANSPORT`; that label is in `DEFAULT_RETRYABLE_CODES`, so a permanent, deterministic fault was also retried three times. + +`llm-pi-ai` was worse on the same input. Its discovery probe builds the same header with a bare `fetch` in [discovery.ts](../../../../packages/llm/llm-pi-ai/src/discovery.ts) and wrapped every failure as `could not reach `, so a local key fault was reported as an unreachable network. The probe is reachable from the unsaved draft: `ProviderEditor` puts the typed `keyDraft` into its probe request, so the model-listing button sent an illegal key before anything was stored. + +Whitespace passed every check. `ProviderEditor` tested `keyDraft.length` and `resolveAdapterOptions` tested `config.apiKey.length`, so a key of three spaces stored and then authenticated as `Bearer` plus blanks. `llm-pi-ai` rejected an empty literal `apiKey` in `resolveProfiles`, but applied no check whatsoever to a credential- or environment-sourced key — the path the Models page writes, and therefore the path users actually take. + +Sources: deepseek-harness#1594 and #1595; dsh-external#247, #249, #266, and #210. + +## Decision + +One rule defines a legal key: **after trimming, non-empty, and every character within `[\x21-\x7E]`** — printable ASCII, space excluded. + +This single predicate covers every input the sources list: empty, leading and trailing whitespace, interior whitespace, C0 control characters, emoji, CJK text, and full-width punctuation. It is also exactly the constraint that produced the ByteString failure, so the two issues close on one definition rather than on two coincidentally related fixes. + +A second, narrower rule catches a pasted environment line: input matching `^[A-Z][A-Z0-9_]*=` or wrapped in matching quotes is refused. Restricting the prefix to upper-case keeps real keys clear of it — `sk-` forms break the identifier match at the hyphen. + +### Invariants belong at every layer; heuristics belong where the human is + +The charset rule is an invariant. A non-ASCII character *cannot* travel in a header value for any provider, so enforcing it in the browser, in each resolver, and on every credential read is consistent by construction rather than by agreement. + +The shape rule is a guess about how people paste, so it runs **only in the browser**. `llm-pi-ai` fronts OpenAI, Anthropic, and arbitrary hand-declared gateways whose key formats this repository does not own; a gateway issuing a key shaped like `TENANT1=abc` would, if the rule ran in the resolver, be locked out with no escape — the settings page would refuse it and a hand-written `.env` would be rejected on read. Confining the heuristic to the surface where the paste happens keeps the environment as the way through. + +### Absence is a configuration state, not a missing key + +"No API key" means three different things here, and only one of them is an error. The rule applies to a value that was *provided*; deciding whether one was provided at all stays with each caller. + +**Omitted.** A profile naming neither `apiKey` nor `apiKeyEnv` is authenticated by something other than a harness-held key. `routeAuth` in [provider.ts](../../../../packages/llm/llm-pi-ai/src/provider.ts) keeps the installed catalog provider's own auth precisely so provider-native ambient discovery survives, and `openai-codex` — shipped in that catalog — authenticates through OAuth and refuses an explicit key outright. `namesCredential` carries this distinction. In `llm-deepseek`, an absent `apiKey` likewise falls through to `apiKeyEnv`. Omission is never validated. + +**A blank field in the web UI.** The key input opens empty even for a provider whose key is already stored — the `keyStored` copy reads "Configured — enter a new value to replace" — so blank means *keep what is stored*. `ProviderEditor` skips `credentials.set` entirely when the draft is empty, and that stays a no-op: a blank field never blocks submit, or editing a base URL would demand re-entering the key. + +**Provided, but empty or whitespace-only.** This is the one error, because the user expressed an intent to set a key and supplied nothing. `llm-pi-ai` already worded it correctly in `resolveProfiles` — *has an empty apiKey; omit it to use ambient authentication* — and that shape, naming the legitimate alternative rather than just refusing, is what the other surfaces adopt. + +`normalizeApiKey` therefore takes `string`, never `string | undefined`. + +### Where the rule lives + +`normalizeApiKey` is a module of the `dsh-llm` seam, beside [attribution.ts](../../../../packages/llm/llm/src/attribution.ts), which already owns shared header concerns. Both adapters depend on the seam and both need the rule, so it has two current consumers rather than a speculative one. It returns the trimmed value or a reason (`empty`, `illegalCharacters`). + +Both adapters also need the identical "refuse a stored credential" diagnosis, differing only by package prefix. `LlmError` is declared in the seam's `index.ts`, so `assertUsableApiKey(raw, pkg, ref)` lives there beside it and neither adapter carries a local copy. The predicate module stays dependency-free: importing `LlmError` into `api-key.ts` would cycle with `index.ts`'s re-export of it. + +The client cannot import any of this: client packages reference only client packages, so `packages/client/ui-models` mirrors the predicate in its own `apiKey.ts` and owns the localized messages, exactly as `validateDeepSeekModels` mirrors the host's `catalogModel` schema. Each side names the other in a comment. + +### What each surface does + +| Surface | Behavior | +|---|---| +| `dsh-llm` | Owns `normalizeApiKey`, `assertUsableApiKey`, and `INVALID_CREDENTIAL_CODE`, which is deliberately outside `DEFAULT_RETRYABLE_CODES`. | +| `llm-deepseek` `resolveAdapterOptions` | Normalizes a present `apiKey`, throwing beside the other beyond-schema bounds; uses the trimmed value. An absent one falls through to `apiKeyEnv`. | +| `llm-deepseek` `resolveApiKey` | Normalizes what the credentials seam or environment returns, rejecting with `INVALID_CREDENTIAL` naming the Models page and never echoing the key. | +| `llm-pi-ai` `resolveProfiles` | Applies the shared rule, keeping its "omit it to use ambient authentication" wording, and writes the trimmed value into the resolved profile. | +| `llm-pi-ai` `resolveApiKey` | Normalizes the credential and environment paths. A profile naming no credential still returns `undefined`, so ambient and OAuth routes are unaffected. | +| `llm-pi-ai` `discoverModels` | Normalizes before building the header, so an illegal key is a credential fault rather than an unreachable endpoint. A probe carrying no key stays unauthenticated. | +| `ui-models` | Mirrors the charset rule, adds the shape heuristic, trims `keyDraft` before probe and `credentials.set`, and fixes the `stringAt` emptiness test. A blank field remains a no-op that submits; a field holding only whitespace is a field-level failure. Submit is gated and the failure renders on the field, matching the existing `modelFailure` pattern. | + +`ProviderEditor` serves both the DeepSeek and pi-ai layouts, so one client change covers both providers. `CustomProviderCard` carries the same judgement for a hand-declared route. + +`credentials-local` is deliberately untouched. It stores credentials generally, and printable-ASCII is a constraint of HTTP headers rather than of credential storage; its existing refusal of values no dotenv style can represent stands as it was. + +## Alternatives considered + +**A `.pattern()` on the `apiKey` schema field.** Vendored schemastery supports it, and the pattern would serialize to the browser with the rest of the namespace schema — one rule, delivered rather than mirrored. It lost because a pattern cannot trim first: `cordis.yml` would then reject a padded key while `.env` tolerated one, and the resolver would disagree with the schema about the same string. Validating in `resolveAdapterOptions` keeps every surface trim-then-validate, and that function is already where this package re-judges bounds the schema cannot express. + +**A validation module shared by client and host.** Rejected by the source-plane layout: client packages reference only client packages plus `vendor/cordis` and `support/invariants`, and widening that to reach a host package would collide the two `Context` merges the split exists to keep apart. Mirroring a one-line predicate with a test on each side is the established shape here. + +**A per-adapter thrower in each of `llm-deepseek` and `llm-pi-ai`.** The first plan gave each adapter its own, differing only by the package prefix in the message, with a duplication-gate exemption to excuse the pair. Rejected before implementation: `LlmError` is declared in the seam, so the seam can own the diagnosis outright, and an exemption there would have hidden exactly the duplication it was covering for. + +**Sniffing the `TypeError` in the adapter's `catch`.** This would classify the ByteString failure after the fact, leaving the header construction itself unguarded. It depends on the wording of a Node error message, so it degrades silently across runtime versions, and it cannot help `llm-pi-ai`, whose request header is built inside the pi-ai SDK. Refusing the key before handing it over works for both adapters and for the discovery probe. + +**Enforcing in `credentials-local.set`.** It would catch every writer at once, including a hand-edited file. It lost because that provider stores credentials of every kind, and a rule derived from HTTP header encoding does not belong to it. + +**Running the shape heuristic in the resolvers too.** Symmetric, and it would stop a pasted environment line written directly into `.env`. Rejected for the lockout described above: a false positive in a resolver leaves the user no working path, while a false positive in the browser leaves the environment open. + +**Probing the provider at save time to prove the key works.** It would close the complaint the sources actually open with — a save that reports success and fails at the first turn. Rejected as out of scope and, on the code as it stood, unbuildable: `discoverModels` short-circuits to the installed catalog before any network call for exactly the providers pi-ai ships catalogs for, so it verified nothing about the key, and the DeepSeek card has no probe at all. A verifier's value is distinguishing "key rejected" from "cannot reach", which is the distinction this change makes reliable; building it first would have produced a verifier unable to tell its own outcomes apart. Comparable products also do not verify on save, so a blocking network call there would be an unexpected behavior rather than a missing one. + +## Consequences + +A malformed key is refused at the field that holds it, and a malformed stored key fails as `INVALID_CREDENTIAL` with a message naming where to fix it and no fragment of the key. Because that code sits outside `DEFAULT_RETRYABLE_CODES`, a deterministic credential fault is no longer retried three times as a transport blip. `llm-pi-ai` discovery reports an illegal probe key as a credential fault instead of an unreachable endpoint. + +The shape heuristic can refuse a real key. Upper-case-identifier-then-`=` and matched surrounding quotes are shapes no known provider issues, and the rule runs only in the browser, so a user who hits it can still set the credential through the environment. The residual cost is a confusing refusal for a key nobody has yet reported. + +Restricting to printable ASCII is stricter than the transport requires: a header value may carry `\x80`–`\xFF`. Admitting latin-1 would let `é` through to return an opaque 401 instead of a local, explained refusal, so the stricter rule is deliberate. A provider that issues latin-1 keys would need this rule widened. + +The charset predicate exists twice, once per source plane. The layout forbids sharing it; each side carries its own test and names its twin. + +Keys already stored by an earlier build are read through `resolveApiKey`, so an illegal stored value fails at resolution rather than at request time. The diagnosis improves, but the failure moves earlier for anyone currently holding one. + +The costliest way to get this wrong would have been to treat absence as invalidity: a rule applied to `undefined` breaks every route authenticating through ambient discovery or OAuth, and a blank field that blocked submit makes editing any other setting demand re-entering the key. Both are pinned by tests rather than left to care. + +## Testing + +`packages/llm/llm/tests/api-key.spec.ts` drives `normalizeApiKey` and `assertUsableApiKey` over the whole input table — empty, whitespace-only, padded, interior-space, C0 control, emoji, CJK, full-width, latin-1, and the printable-ASCII boundary — and pins that a refusal carries `INVALID_CREDENTIAL` and no part of the key. + +`packages/llm/llm-deepseek/tests/` covers the literal-config path in `adapter.spec.ts` and the stored-credential path end to end in `dynamic-config.spec.ts`, through the real credentials seam rather than a stub. `packages/llm/llm-pi-ai/tests/` covers `resolveProfiles` — including that the trimmed value reaches the resolved profile, which the `...rest` spread would otherwise discard — and the discovery probe, including that a probe with no key sends no `authorization` header. + +`packages/client/ui-models/tests/` pins `apiKeyFailure` over the same table plus the paste-shape cases, and drives both cards: a blank field submits without writing a credential, a whitespace-only field fails on the field, an illegal or wrapped key blocks submit, a padded key is trimmed before `credentials.set` and before an interrogation, and a hand-declared route can be created with no key at all. diff --git a/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md new file mode 100644 index 0000000000..63c6a8c17e --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-06-api-key-format-validation.zh.md @@ -0,0 +1,105 @@ +# Agent Note: 在 API Key 进入 HTTP header 之前校验其格式 + +Status: implemented + +[English](2026-08-06-api-key-format-validation.md) | 中文 + +## Problem + +一个含有 HTTP header value 无法承载的字符的 API Key，曾被每一层配置界面接受，直到构造请求时才失败——离引发它的那个字段已经很远。 + +把含 emoji、中文或全角标点的 Key 粘进 Web 模型设置页，保存会报成功。第一轮对话随即失败于 `Cannot convert argument to a ByteString because the character at index 7 has a value of 55357 which is greater than 255`——其中的下标与码点是 UTF-16 内部细节，不附带任何可执行动作，却泄露了 Key 中某一个字符的码点。`llm-deepseek` 之所以产出这句，是因为 `fetch` 在 [adapter.ts](../../../../packages/llm/llm-deepseek/src/adapter.ts) 的 `try` 内部构造 `Bearer` header，而那个 `catch` 把一切失败都标为 `TRANSPORT`；该标签又在 `DEFAULT_RETRYABLE_CODES` 之中，于是一个永久且确定的故障还会被重试三次。 + +同样的输入在 `llm-pi-ai` 上更糟。它的探测路径在 [discovery.ts](../../../../packages/llm/llm-pi-ai/src/discovery.ts) 里用裸 `fetch` 构造同一个 header，并把一切失败包装成 `could not reach `，于是一个本地的 Key 故障被报成网络不可达。这条探测在保存之前就够得着：`ProviderEditor` 把用户输入的 `keyDraft` 直接放进探测请求，所以「获取模型列表」按钮会在任何东西落盘之前就把非法 Key 发出去。 + +空白字符能通过每一道检查。`ProviderEditor` 判的是 `keyDraft.length`，`resolveAdapterOptions` 判的是 `config.apiKey.length`，于是三个空格构成的 Key 会被存下，随后以 `Bearer` 加若干空格去认证。`llm-pi-ai` 在 `resolveProfiles` 中拒绝空的字面量 `apiKey`，却对来自凭据或环境的 Key 完全不做检查——而那正是模型设置页写入的路径，也就是用户真正走的路径。 + +来源：deepseek-harness#1594 与 #1595；dsh-external#247、#249、#266、#210。 + +## Decision + +一条规则定义什么是合法 Key：**trim 之后非空，且每个字符都落在 `[\x21-\x7E]`**——可打印 ASCII，不含空格。 + +这一个断言覆盖了来源列出的全部输入：空值、首尾空白、中间空白、C0 控制字符、emoji、中文、全角标点。它同时正是造成 ByteString 失败的那条约束，所以两个 issue 收敛于同一个定义，而不是两个恰好相关的修复。 + +第二条更窄的规则用于识别整行粘贴的环境变量：匹配 `^[A-Z][A-Z0-9_]*=` 或首尾成对引号的输入会被拒绝。把前缀限定为全大写可以让真实 Key 与之绝缘——`sk-` 这类形态会在连字符处中断标识符匹配。 + +### 不变量属于每一层，启发式属于人所在的那一层 + +字符集规则是不变量。非 ASCII 字符对任何 provider 都**不可能**在 header value 中传输，因此在浏览器、在各个 resolver、在每一次凭据读取上执行它，是结构上的一致而非约定上的一致。 + +形状规则是对人如何粘贴的猜测，因此**只在浏览器中运行**。`llm-pi-ai` 前面挂着 OpenAI、Anthropic 以及任意手工声明的网关，本仓库并不掌握它们的 Key 格式；若这条规则运行在 resolver 中，一个签发形如 `TENANT1=abc` 的网关会让用户被彻底锁死、无路可走——设置页拒绝它，手写的 `.env` 在读取时同样被拒。把启发式限制在粘贴动作发生的那一层，环境变量便始终是那条出路。 + +### 「没有 Key」是一种配置状态，不是缺失 + +在这里，「没有 API Key」意味着三件完全不同的事，其中只有一件是错误。规则作用于**已提供**的值；至于究竟有没有提供，由各个调用方自行判断。 + +**未指定。** 既不写 `apiKey` 也不写 `apiKeyEnv` 的 profile，是由 harness 所持有的 Key 之外的东西来鉴权的。[provider.ts](../../../../packages/llm/llm-pi-ai/src/provider.ts) 中的 `routeAuth` 保留内置 catalog provider 自身的鉴权，正是为了让 provider 原生的 ambient 发现得以存活；而该 catalog 附带的 `openai-codex` 通过 OAuth 鉴权，并会直接拒绝一个显式的 Key。`namesCredential` 承载着这一区分。在 `llm-deepseek` 中，缺省的 `apiKey` 同样会回落到 `apiKeyEnv`。未指定的情形永不参与校验。 + +**Web UI 中留空的输入框。** 即便某个 provider 的 Key 已经存好，该输入框也是空着打开的——`keyStored` 的文案写的是「已配置——输入新值以替换」——所以留空意味着*保持已存储的值*。`ProviderEditor` 在草稿为空时完全跳过 `credentials.set`，这一点保持不变：留空绝不拦截提交，否则改一个 base URL 都得重新输一遍 Key。 + +**已提供，但为空或纯空白。** 这是唯一的错误，因为用户表达了设置 Key 的意图却什么都没给。`llm-pi-ai` 在 `resolveProfiles` 中的措辞本就是对的——*has an empty apiKey; omit it to use ambient authentication*——这种指明合法替代路径而非单纯拒绝的形态，正是其他界面所采用的。 + +因此 `normalizeApiKey` 接受 `string`，而绝非 `string | undefined`。 + +### 规则住在哪里 + +`normalizeApiKey` 是 `dsh-llm` seam 的一个模块，与已经承担共享 header 事务的 [attribution.ts](../../../../packages/llm/llm/src/attribution.ts) 并列。两个适配器都依赖该 seam 且都需要这条规则，因此它拥有两个当前消费者而非一个预设消费者。它返回 trim 后的值，或一个原因（`empty`、`illegalCharacters`）。 + +两个适配器同样都需要那句完全相同的「拒绝一个已存储凭据」的诊断，差别仅在包名前缀。`LlmError` 声明在 seam 的 `index.ts` 中，因此 `assertUsableApiKey(raw, pkg, ref)` 就住在它旁边，两个适配器都不再各留一份。断言模块本身保持零依赖：把 `LlmError` 引入 `api-key.ts` 会与 `index.ts` 对它的再导出成环。 + +客户端无法引入其中任何一个：client 包只 reference client 包，因此 `packages/client/ui-models` 在自己的 `apiKey.ts` 中镜像这个断言并持有本地化文案，正如 `validateDeepSeekModels` 镜像 host 侧的 `catalogModel` schema。两侧在注释中互相指名。 + +### 各个界面各做什么 + +| 界面 | 行为 | +|---|---| +| `dsh-llm` | 拥有 `normalizeApiKey`、`assertUsableApiKey` 与 `INVALID_CREDENTIAL_CODE`，后者刻意不进 `DEFAULT_RETRYABLE_CODES`。 | +| `llm-deepseek` `resolveAdapterOptions` | 归一化已提供的 `apiKey`，与其他超出 schema 的边界检查并排抛错；使用 trim 后的值。缺省的 `apiKey` 回落到 `apiKeyEnv`。 | +| `llm-deepseek` `resolveApiKey` | 归一化凭据 seam 或环境返回的值，以 `INVALID_CREDENTIAL` 拒绝，消息指明模型设置页，绝不回显 Key。 | +| `llm-pi-ai` `resolveProfiles` | 施加这条共享规则，保留其「omit it to use ambient authentication」的措辞，并把 trim 后的值写进解析后的 profile。 | +| `llm-pi-ai` `resolveApiKey` | 归一化凭据与环境路径。不指定任何凭据的 profile 仍返回 `undefined`，ambient 与 OAuth 路由不受影响。 | +| `llm-pi-ai` `discoverModels` | 在构造 header 之前归一化，使非法 Key 成为凭据故障而非端点不可达。不带 Key 的探测保持未鉴权。 | +| `ui-models` | 镜像字符集规则，加入形状启发式，在探测与 `credentials.set` 之前 trim `keyDraft`，并修正 `stringAt` 的空值判断。留空的输入框仍是可以提交的空操作；只含空白的输入框则是字段级失败。提交受拦截，失败呈现在字段上，与既有的 `modelFailure` 模式一致。 | + +`ProviderEditor` 同时服务 DeepSeek 与 pi-ai 两种布局，因此一处客户端改动覆盖两个 provider。`CustomProviderCard` 为手工声明的路由承载同一套判定。 + +`credentials-local` 刻意不动。它存储各类凭据，而可打印 ASCII 是 HTTP header 的约束而非凭据存储的约束；它既有的、拒绝任何 dotenv 样式都无法表示的值的行为保持原样。 + +## Alternatives considered + +**在 `apiKey` schema 字段上加 `.pattern()`。** vendor 中的 schemastery 支持它，且该 pattern 会随命名空间 schema 一同序列化到浏览器——一条规则，投递而非镜像。它落败于 pattern 无法先行 trim：那样 `cordis.yml` 会拒绝带首尾空白的 Key 而 `.env` 却容忍，resolver 与 schema 会对同一个字符串给出分歧。在 `resolveAdapterOptions` 中校验可以让每一层都是 trim-then-validate，而该函数本就是本包重新裁定 schema 无法表达的边界之处。 + +**由 client 与 host 共享一个校验模块。** 被 source plane 布局否决：client 包只 reference client 包外加 `vendor/cordis` 与 `support/invariants`，把它放宽到够得着 host 包会撞上这一分割本就要隔开的两份 `Context` 合并。在两侧各镜像一行断言并各配一份测试，是此处的既定形态。 + +**在 `llm-deepseek` 与 `llm-pi-ai` 中各留一个抛错 helper。** 最初的计划正是各留一份，差别仅在消息中的包名前缀，并配一个重复检测豁免来放行这一对。在实现之前即被否决：`LlmError` 声明在 seam 中，因此 seam 完全可以自己拥有这句诊断，而那里的一个豁免恰恰会掩盖它本要遮掩的重复。 + +**在适配器的 `catch` 中嗅探 `TypeError`。** 这只是事后归类 ByteString 失败，header 构造本身仍无防护。它依赖 Node 错误消息的措辞，因而会随运行时版本静默失效；它也帮不到 `llm-pi-ai`——后者的请求 header 构造在 pi-ai SDK 内部。在交出 Key 之前就拒绝，则对两个适配器与探测路径同时有效。 + +**在 `credentials-local.set` 中执行。** 它能一次性拦住所有写入方，包括手工编辑的文件。它落败于该 provider 存储各种类型的凭据，而一条源自 HTTP header 编码的规则并不属于它。 + +**让形状启发式也在 resolver 中运行。** 更对称，且能拦住直接写进 `.env` 的整行环境变量。因上文所述的锁死风险而否决：resolver 中的一次误判会让用户无路可走，浏览器中的一次误判则仍留有环境变量这条路。 + +**在保存时探测 provider 以证明 Key 可用。** 它能关掉来源真正开篇抱怨的那件事——保存报成功、第一轮才失败。因超出范围而否决，且在当时的代码上无法建成：对 pi-ai 恰好自带 catalog 的那些 provider，`discoverModels` 会在任何网络调用之前短路到内置 catalog，因而对 Key 什么都验证不了；而 DeepSeek 卡片根本没有探测。验证器的价值在于分清「Key 被拒」与「无法连通」，而这正是本次改动让其变得可靠的区分；先建验证器只会得到一个分不清自身结果的验证器。同类产品也不在保存时验证，因此保存时的阻断式网络调用会是一个意外行为，而非一处缺失。 + +## Consequences + +格式错误的 Key 在持有它的那个字段上就被拒绝；格式错误的已存储 Key 以 `INVALID_CREDENTIAL` 失败，消息指明修复位置且不含 Key 的任何片段。由于该 code 位于 `DEFAULT_RETRYABLE_CODES` 之外，一个确定性的凭据故障不再被当作瞬时传输抖动重试三次。`llm-pi-ai` 的探测把非法 Key 报为凭据故障，而非端点不可达。 + +形状启发式可能拒绝一个真实的 Key。全大写标识符接 `=`、以及首尾成对引号，都是已知 provider 不会签发的形态，且该规则只在浏览器中运行，因此撞上它的用户仍可通过环境变量设置该凭据。残留代价是对一个尚无人报告过的 Key 给出一次令人困惑的拒绝。 + +限定为可打印 ASCII 比传输本身的要求更严：header value 是可以承载 `\x80`–`\xFF` 的。放行 latin-1 会让 `é` 通过并换回一个语焉不详的 401，而不是一次本地的、有解释的拒绝，因此从严是刻意的。若某个 provider 签发 latin-1 的 Key，这条规则需要放宽。 + +字符集断言存在两份，每个 source plane 一份。布局禁止共享它；两侧各自带测试并在注释中指名其孪生体。 + +早先版本已存下的 Key 会经 `resolveApiKey` 读取，因此一个非法的既存值将从解析时开始失败，而非到请求时才失败。诊断变好了，但对当前正持有这类值的人而言，失败点提前了。 + +把这件事做错的最大代价，会是把「未指定」当成「非法」：一条施加到 `undefined` 上的规则会打断每一条依赖 ambient 发现或 OAuth 鉴权的路由，而一个会拦截提交的空输入框，则会让改动任何其他设置都必须重新输入 Key。这两点都由测试钉住，而不是仅仰赖谨慎。 + +## Testing + +`packages/llm/llm/tests/api-key.spec.ts` 以整张输入表驱动 `normalizeApiKey` 与 `assertUsableApiKey`——空值、纯空白、带首尾空白、含中间空格、C0 控制字符、emoji、中文、全角、latin-1，以及可打印 ASCII 的边界字符——并钉住一次拒绝携带 `INVALID_CREDENTIAL` 且不含 Key 的任何部分。 + +`packages/llm/llm-deepseek/tests/` 在 `adapter.spec.ts` 中覆盖字面量配置路径，在 `dynamic-config.spec.ts` 中经真实凭据 seam（而非 stub）端到端覆盖已存储凭据路径。`packages/llm/llm-pi-ai/tests/` 覆盖 `resolveProfiles`——包括 trim 后的值确实到达解析后的 profile，否则会被 `...rest` 展开丢弃——以及探测路径，包括不带 Key 的探测不会发出 `authorization` 标头。 + +`packages/client/ui-models/tests/` 以同一张表加上形状用例钉住 `apiKeyFailure`，并驱动两张卡片：留空的输入框可提交且不写入凭据、只含空白的输入框在字段上失败、非法或被包裹的 Key 拦截提交、带首尾空白的 Key 在 `credentials.set` 与探测之前被 trim，以及手工声明的路由可以完全不带 Key 创建。 diff --git a/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml b/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml deleted file mode 100644 index f62a18e0eb..0000000000 --- a/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.i18n.yaml +++ /dev/null @@ -1,6 +0,0 @@ -# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each -# side as of the last confirmed-consistent state. Both languages carry equal authority; -# after editing either side, bring the other along and re-record with: -# pnpm run verify-translation-pairing --write .agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.md -2026-08-06-api-key-format-validation.md: dc19baa8b697998df2892f0840a35a8232cc92de -2026-08-06-api-key-format-validation.zh.md: 28073660b1d4868fecf5ce419726d6d997383392 diff --git a/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.md b/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.md deleted file mode 100644 index dc19baa8b6..0000000000 --- a/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.md +++ /dev/null @@ -1,101 +0,0 @@ -# Agent Note: Validate API key format before it reaches an HTTP header - -Status: proposed - -English | [中文](2026-08-06-api-key-format-validation.zh.md) - -## Problem - -An API key holding characters no HTTP header value can carry is accepted by every configuration surface and fails only when a request is built, far from the field that caused it. - -Paste a key containing an emoji, CJK text, or a full-width punctuation mark into the web Models page and the save reports success. The first turn then fails with `Cannot convert argument to a ByteString because the character at index 7 has a value of 55357 which is greater than 255` — the index and code point are UTF-16 internals with no action attached, and they disclose the code point of one character of the key. `llm-deepseek` produces this because `fetch` builds the `Bearer` header inside the `try` at [adapter.ts](../../../../packages/llm/llm-deepseek/src/adapter.ts), whose `catch` labels every failure `TRANSPORT`; that label is in `DEFAULT_RETRYABLE_CODES`, so a permanent, deterministic fault is also retried three times. - -`llm-pi-ai` is worse on the same input. Its discovery probe builds the same header with a bare `fetch` in [discovery.ts](../../../../packages/llm/llm-pi-ai/src/discovery.ts) and wraps every failure as `could not reach `, so a local key fault is reported as an unreachable network. The probe is reachable from the unsaved draft: `ProviderEditor` puts the typed `keyDraft` into its probe request, so the model-listing button sends an illegal key before anything is stored. - -Whitespace passes every check. `ProviderEditor` tests `keyDraft.length` and `resolveAdapterOptions` tests `config.apiKey.length`, so a key of three spaces stores and then authenticates as `Bearer` plus blanks. `llm-pi-ai` rejects an empty literal `apiKey` in `resolveProfiles`, but applies no check whatsoever to a credential- or environment-sourced key — which is the path the Models page writes, and therefore the path users actually take. - -Sources: deepseek-harness#1594 and #1595; dsh-external#247, #249, #266, and #210. - -## Proposal - -One rule defines a legal key: **after trimming, non-empty, and every character within `[\x21-\x7E]`** — printable ASCII, space excluded. - -This single predicate covers every input the sources list: empty, leading and trailing whitespace, interior whitespace, C0 control characters, emoji, CJK text, and full-width punctuation. It is also exactly the constraint that produced the ByteString failure, so the two issues close on one definition rather than on two coincidentally related fixes. - -A second, narrower rule catches a pasted environment line: reject input matching `^[A-Z][A-Z0-9_]*=` or wrapped in matching quotes. Restricting the prefix to upper-case keeps real keys clear of it — `sk-` forms break the identifier match at the hyphen. - -### Invariants belong at every layer; heuristics belong where the human is - -The charset rule is an invariant. A non-ASCII character *cannot* travel in a header value for any provider, so enforcing it in the browser, in each resolver, and on every credential read is consistent by construction rather than by agreement. - -The shape rule is a guess about how people paste, so it runs **only in the browser**. `llm-pi-ai` fronts OpenAI, Anthropic, and arbitrary hand-declared gateways whose key formats this repository does not own; a gateway issuing a key shaped like `TENANT1=abc` would, if the rule ran in the resolver, be locked out with no escape — the settings page would refuse it and a hand-written `.env` would be rejected on read. Confining the heuristic to the surface where the paste happens keeps the environment as the way through. - -### Absence is a configuration state, not a missing key - -"No API key" means three different things here, and only one of them is an error. The rule applies to a value that was *provided*; deciding whether one was provided at all stays with each caller. - -**Omitted.** A profile naming neither `apiKey` nor `apiKeyEnv` is authenticated by something other than a harness-held key. `routeAuth` in [provider.ts](../../../../packages/llm/llm-pi-ai/src/provider.ts) keeps the installed catalog provider's own auth precisely so provider-native ambient discovery survives, and `openai-codex` — shipped in that catalog — authenticates through OAuth and refuses an explicit key outright. `namesCredential` exists to carry this distinction. In `llm-deepseek`, an absent `apiKey` likewise falls through to `apiKeyEnv`. Omission is never validated. - -**A blank field in the web UI.** The key input opens empty even for a provider whose key is already stored — the `keyStored` copy reads "Configured — enter a new value to replace" — so blank means *keep what is stored*. `ProviderEditor` already skips `credentials.set` entirely when the draft is empty, and that stays a no-op: a blank field must never block submit, or editing a base URL would demand re-entering the key. - -**Provided, but empty or whitespace-only.** This is the one error, because the user expressed an intent to set a key and supplied nothing. `llm-pi-ai` already words it correctly in `resolveProfiles` — *has an empty apiKey; omit it to use ambient authentication* — and that shape, naming the legitimate alternative rather than just refusing, is what the other surfaces adopt. - -`normalizeApiKey` therefore takes `string`, never `string | undefined`. - -### Where the rule lives - -`normalizeApiKey` is a new module of the `dsh-llm` seam, beside [attribution.ts](../../../../packages/llm/llm/src/attribution.ts), which already owns shared header concerns. Both adapters depend on the seam and both need the rule, so it has two current consumers rather than a speculative one. It returns the trimmed value or a reason (`empty`, `illegalCharacters`). - -The client cannot import it: client packages reference only client packages, so `packages/client/ui-models` mirrors the predicate and owns the localized messages, exactly as `validateDeepSeekModels` mirrors the host's `catalogModel` schema today. Each side names the other in a comment. - -### What each surface does - -| Surface | Change | -|---|---| -| `dsh-llm` | Add `normalizeApiKey`; add `INVALID_CREDENTIAL`, deliberately outside `DEFAULT_RETRYABLE_CODES`. | -| `llm-deepseek` `resolveAdapterOptions` | Normalize a present `apiKey`, throwing beside the existing beyond-schema bounds; use the trimmed value. An absent one still falls through to `apiKeyEnv`. Closes dsh-external#210. | -| `llm-deepseek` `resolveApiKey` | Normalize what the credentials seam or environment returns; reject with `INVALID_CREDENTIAL` naming the Models page, never echoing the key. | -| `llm-pi-ai` `resolveProfiles` | Widen the existing emptiness check to the shared rule, keeping its "omit it to use ambient authentication" wording. | -| `llm-pi-ai` `resolveApiKey` | Normalize the credential and environment paths, which are unchecked today. A profile naming no credential still returns `undefined` untouched, so ambient and OAuth routes are unaffected. | -| `llm-pi-ai` `discoverModels` | Normalize before building the header, so an illegal key stops reporting as an unreachable endpoint. A probe carrying no key stays unauthenticated as it is today. | -| `ui-models` | Mirror the charset rule, add the shape heuristic, trim `keyDraft` before probe and `credentials.set`, and fix the `stringAt` emptiness test. A blank field remains a no-op that submits; a field holding only whitespace is a field-level failure, so typed input is never silently discarded. Gate submit and show the failure on the field, matching the existing `modelFailure` pattern. | - -`ProviderEditor` serves both the DeepSeek and pi-ai layouts, so one client change covers both providers. - -`credentials-local` is deliberately untouched. It stores credentials generally, and printable-ASCII is a constraint of HTTP headers rather than of credential storage; its existing refusal of values no dotenv style can represent stays as it is. - -## Alternatives considered - -**A `.pattern()` on the `apiKey` schema field.** Vendored schemastery supports it, and the pattern would serialize to the browser with the rest of the namespace schema — one rule, delivered rather than mirrored. It loses because a pattern cannot trim first: `cordis.yml` would then reject a padded key while `.env` tolerated one, and the resolver would disagree with the schema about the same string. Validating in `resolveAdapterOptions` keeps every surface trim-then-validate, and that function is already where this package re-judges bounds the schema cannot express. - -**A validation module shared by client and host.** Rejected by the source-plane layout: client packages reference only client packages plus `vendor/cordis` and `support/invariants`, and widening that to reach a host package would collide the two `Context` merges the split exists to keep apart. Mirroring a one-line predicate with a test on each side is the established shape here. - -**Sniffing the `TypeError` in the adapter's `catch`.** This would classify the ByteString failure after the fact, leaving the header construction itself unguarded. It depends on the wording of a Node error message, so it degrades silently across runtime versions, and it cannot help `llm-pi-ai`, whose header is built inside the pi-ai SDK. Refusing the key before handing it over works for both adapters and for the discovery probe. - -**Enforcing in `credentials-local.set`.** It would catch every writer at once, including a hand-edited file. It loses because that provider stores credentials of every kind, and a rule derived from HTTP header encoding does not belong to it. - -**Running the shape heuristic in the resolvers too.** Symmetric, and it would stop a pasted environment line written directly into `.env`. Rejected for the lockout described above: a false positive in a resolver leaves the user no working path, while a false positive in the browser leaves the environment open. - -**Probing the provider at save time to prove the key works.** It would close the complaint the sources actually open with — a save that reports success and fails at the first turn. Rejected as out of scope and, on today's code, unbuildable: `discoverModels` short-circuits to the installed catalog before any network call for exactly the providers pi-ai ships catalogs for, so it verifies nothing about the key, and the DeepSeek card has no probe at all. A verifier's value is distinguishing "key rejected" from "cannot reach", which is the distinction this note makes reliable; building it first would produce a verifier unable to tell its own outcomes apart. Comparable products also do not verify on save, so a blocking network call at save time would be an unexpected behavior rather than a missing one. - -## Acceptance criteria - -- The browser, both resolvers, and both credential reads accept and reject the same *provided* strings: whitespace-only, padded, interior-space, C0 control, emoji, CJK, and full-width inputs are refused; a printable-ASCII key is accepted, trimmed. -- A profile naming no credential still resolves to no key, and a route authenticating through the installed provider's own ambient discovery or OAuth keeps working untouched. -- A blank key field saves the rest of the card without writing a credential; a field holding only whitespace fails on the field instead of being silently dropped. -- A rejected key names the API key field in the web UI and blocks submit; nothing is written to settings or credentials. -- A key that reaches a resolver illegally fails as `INVALID_CREDENTIAL` with a message naming where to fix it, containing no part of the key, and is not retried. -- `llm-pi-ai` discovery reports an illegal key as a key fault, not as an unreachable endpoint. -- A legal key still travels the existing `credentials.set` path unchanged. - -## Risks - -The shape heuristic can refuse a real key. Upper-case-identifier-then-`=` and matched surrounding quotes are shapes no known provider issues, and the rule runs only in the browser, so a user who hits it can still set the credential through the environment. The residual cost is a confusing refusal for a key nobody has yet reported. - -Restricting to printable ASCII is stricter than the transport requires: a header value may carry `\x80`–`\xFF`. Admitting latin-1 would let `é` through to return an opaque 401 instead of a local, explained refusal, so the stricter rule is deliberate. A provider that issues latin-1 keys would need this rule widened. - -The charset predicate exists twice, once per source plane. The layout forbids sharing it, and the duplication gate may flag the pair; each side carries its own test and names its twin. - -The costliest way to get this wrong is to treat absence as invalidity. A rule applied to `undefined` would break every route authenticating through ambient discovery or OAuth — `openai-codex` cannot take a key at all — and a blank field that blocked submit would make editing any other setting demand re-entering the key. Both belong in the tests, not only in this note. - -Keys already stored by an earlier build are read through `resolveApiKey`, so an illegal stored value begins failing at resolution rather than at request time. That is the intent — the diagnosis improves — but it moves the failure earlier for anyone currently holding one. diff --git a/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.zh.md b/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.zh.md deleted file mode 100644 index 28073660b1..0000000000 --- a/.agents/notes/proposed/bug-fix/2026-08-06-api-key-format-validation.zh.md +++ /dev/null @@ -1,101 +0,0 @@ -# Agent Note: 在 API Key 进入 HTTP header 之前校验其格式 - -Status: proposed - -[English](2026-08-06-api-key-format-validation.md) | 中文 - -## Problem - -一个含有 HTTP header value 无法承载的字符的 API Key,会被每一层配置界面接受,直到构造请求时才失败——离引发它的那个字段已经很远。 - -把含 emoji、中文或全角标点的 Key 粘进 Web 模型设置页,保存会报成功。第一轮对话随即失败于 `Cannot convert argument to a ByteString because the character at index 7 has a value of 55357 which is greater than 255`——其中的下标与码点是 UTF-16 内部细节,不附带任何可执行动作,却泄露了 Key 中某一个字符的码点。`llm-deepseek` 之所以产出这句,是因为 `fetch` 在 [adapter.ts](../../../../packages/llm/llm-deepseek/src/adapter.ts) 的 `try` 内部构造 `Bearer` header,而那个 `catch` 把一切失败都标为 `TRANSPORT`;该标签又在 `DEFAULT_RETRYABLE_CODES` 之中,于是一个永久且确定的故障还会被重试三次。 - -同样的输入在 `llm-pi-ai` 上更糟。它的探测路径在 [discovery.ts](../../../../packages/llm/llm-pi-ai/src/discovery.ts) 里用裸 `fetch` 构造同一个 header,并把一切失败包装成 `could not reach `,于是一个本地的 Key 故障被报成网络不可达。这条探测在保存之前就够得着:`ProviderEditor` 把用户输入的 `keyDraft` 直接放进探测请求,所以「获取模型列表」按钮会在任何东西落盘之前就把非法 Key 发出去。 - -空白字符能通过每一道检查。`ProviderEditor` 判的是 `keyDraft.length`,`resolveAdapterOptions` 判的是 `config.apiKey.length`,于是三个空格构成的 Key 会被存下,随后以 `Bearer` 加若干空格去认证。`llm-pi-ai` 在 `resolveProfiles` 中拒绝空的字面量 `apiKey`,却对来自凭据或环境的 Key 完全不做检查——而那正是模型设置页写入的路径,也就是用户真正走的路径。 - -来源:deepseek-harness#1594 与 #1595;dsh-external#247、#249、#266、#210。 - -## Proposal - -一条规则定义什么是合法 Key:**trim 之后非空,且每个字符都落在 `[\x21-\x7E]`**——可打印 ASCII,不含空格。 - -这一个断言覆盖了来源列出的全部输入:空值、首尾空白、中间空白、C0 控制字符、emoji、中文、全角标点。它同时正是造成 ByteString 失败的那条约束,所以两个 issue 收敛于同一个定义,而不是两个恰好相关的修复。 - -第二条更窄的规则用于识别整行粘贴的环境变量:拒绝匹配 `^[A-Z][A-Z0-9_]*=` 或首尾成对引号的输入。把前缀限定为全大写可以让真实 Key 与之绝缘——`sk-` 这类形态会在连字符处中断标识符匹配。 - -### 不变量属于每一层,启发式属于人所在的那一层 - -字符集规则是不变量。非 ASCII 字符对任何 provider 都**不可能**在 header value 中传输,因此在浏览器、在各个 resolver、在每一次凭据读取上执行它,是结构上的一致而非约定上的一致。 - -形状规则是对人如何粘贴的猜测,因此**只在浏览器中运行**。`llm-pi-ai` 前面挂着 OpenAI、Anthropic 以及任意手工声明的网关,本仓库并不掌握它们的 Key 格式;若这条规则运行在 resolver 中,一个签发形如 `TENANT1=abc` 的网关会让用户被彻底锁死、无路可走——设置页拒绝它,手写的 `.env` 在读取时同样被拒。把启发式限制在粘贴动作发生的那一层,环境变量便始终是那条出路。 - -### 「没有 Key」是一种配置状态,不是缺失 - -在这里,「没有 API Key」意味着三件完全不同的事,其中只有一件是错误。规则作用于**已提供**的值;至于究竟有没有提供,由各个调用方自行判断。 - -**未指定。** 既不写 `apiKey` 也不写 `apiKeyEnv` 的 profile,是由 harness 所持有的 Key 之外的东西来鉴权的。[provider.ts](../../../../packages/llm/llm-pi-ai/src/provider.ts) 中的 `routeAuth` 保留内置 catalog provider 自身的鉴权,正是为了让 provider 原生的 ambient 发现得以存活;而该 catalog 附带的 `openai-codex` 通过 OAuth 鉴权,并会直接拒绝一个显式的 Key。`namesCredential` 的存在就是为了承载这一区分。在 `llm-deepseek` 中,缺省的 `apiKey` 同样会回落到 `apiKeyEnv`。未指定的情形永不参与校验。 - -**Web UI 中留空的输入框。** 即便某个 provider 的 Key 已经存好,该输入框也是空着打开的——`keyStored` 的文案写的是「已配置——输入新值以替换」——所以留空意味着*保持已存储的值*。`ProviderEditor` 在草稿为空时本就完全跳过 `credentials.set`,这一点保持不变:留空绝不能拦截提交,否则改一个 base URL 都得重新输一遍 Key。 - -**已提供,但为空或纯空白。** 这是唯一的错误,因为用户表达了设置 Key 的意图却什么都没给。`llm-pi-ai` 在 `resolveProfiles` 中的措辞本就是对的——*has an empty apiKey; omit it to use ambient authentication*——这种指明合法替代路径而非单纯拒绝的形态,正是其他界面要采用的。 - -因此 `normalizeApiKey` 接受 `string`,而绝非 `string | undefined`。 - -### 规则住在哪里 - -`normalizeApiKey` 是 `dsh-llm` seam 的新模块,与已经承担共享 header 事务的 [attribution.ts](../../../../packages/llm/llm/src/attribution.ts) 并列。两个适配器都依赖该 seam 且都需要这条规则,因此它拥有两个当前消费者而非一个预设消费者。它返回 trim 后的值,或一个原因(`empty`、`illegalCharacters`)。 - -客户端无法引入它:client 包只 reference client 包,因此 `packages/client/ui-models` 镜像这个断言并持有本地化文案,正如今天 `validateDeepSeekModels` 镜像 host 侧的 `catalogModel` schema。两侧在注释中互相指名。 - -### 各个界面各做什么 - -| 界面 | 改动 | -|---|---| -| `dsh-llm` | 新增 `normalizeApiKey`;新增 `INVALID_CREDENTIAL`,刻意不进 `DEFAULT_RETRYABLE_CODES`。 | -| `llm-deepseek` `resolveAdapterOptions` | 归一化已提供的 `apiKey`,与既有的超出 schema 的边界检查并排抛错;使用 trim 后的值。缺省的 `apiKey` 仍照旧回落到 `apiKeyEnv`。关闭 dsh-external#210。 | -| `llm-deepseek` `resolveApiKey` | 归一化凭据 seam 或环境返回的值;以 `INVALID_CREDENTIAL` 拒绝,消息指明模型设置页,绝不回显 Key。 | -| `llm-pi-ai` `resolveProfiles` | 把既有的空值检查扩展为这条共享规则,并保留其「omit it to use ambient authentication」的措辞。 | -| `llm-pi-ai` `resolveApiKey` | 归一化今天完全未受检的凭据与环境路径。不指定任何凭据的 profile 仍原样返回 `undefined`,ambient 与 OAuth 路由不受影响。 | -| `llm-pi-ai` `discoverModels` | 在构造 header 之前归一化,使非法 Key 不再被报成端点不可达。不带 Key 的探测照旧保持未鉴权。 | -| `ui-models` | 镜像字符集规则,加入形状启发式,在探测与 `credentials.set` 之前 trim `keyDraft`,并修正 `stringAt` 的空值判断。留空的输入框仍是可以提交的空操作;只含空白的输入框则以字段级失败呈现,使已输入的内容绝不被静默丢弃。按既有 `modelFailure` 的模式拦截提交并在字段上呈现失败。 | - -`ProviderEditor` 同时服务 DeepSeek 与 pi-ai 两种布局,因此一处客户端改动覆盖两个 provider。 - -`credentials-local` 刻意不动。它存储各类凭据,而可打印 ASCII 是 HTTP header 的约束而非凭据存储的约束;它既有的、拒绝任何 dotenv 样式都无法表示的值的行为保持原样。 - -## Alternatives considered - -**在 `apiKey` schema 字段上加 `.pattern()`。** vendor 中的 schemastery 支持它,且该 pattern 会随命名空间 schema 一同序列化到浏览器——一条规则,投递而非镜像。它落败于 pattern 无法先行 trim:那样 `cordis.yml` 会拒绝带首尾空白的 Key 而 `.env` 却容忍,resolver 与 schema 会对同一个字符串给出分歧。在 `resolveAdapterOptions` 中校验可以让每一层都是 trim-then-validate,而该函数本就是本包重新裁定 schema 无法表达的边界之处。 - -**由 client 与 host 共享一个校验模块。** 被 source plane 布局否决:client 包只 reference client 包外加 `vendor/cordis` 与 `support/invariants`,把它放宽到够得着 host 包会撞上这一分割本就要隔开的两份 `Context` 合并。在两侧各镜像一行断言并各配一份测试,是此处的既定形态。 - -**在适配器的 `catch` 中嗅探 `TypeError`。** 这只是事后归类 ByteString 失败,header 构造本身仍无防护。它依赖 Node 错误消息的措辞,因而会随运行时版本静默失效;它也帮不到 `llm-pi-ai`——后者的 header 构造在 pi-ai SDK 内部。在交出 Key 之前就拒绝,则对两个适配器与探测路径同时有效。 - -**在 `credentials-local.set` 中执行。** 它能一次性拦住所有写入方,包括手工编辑的文件。它落败于该 provider 存储各种类型的凭据,而一条源自 HTTP header 编码的规则并不属于它。 - -**让形状启发式也在 resolver 中运行。** 更对称,且能拦住直接写进 `.env` 的整行环境变量。因上文所述的锁死风险而否决:resolver 中的一次误判会让用户无路可走,浏览器中的一次误判则仍留有环境变量这条路。 - -**在保存时探测 provider 以证明 Key 可用。** 它能关掉来源真正开篇抱怨的那件事——保存报成功、第一轮才失败。因超出范围而否决,且在今天的代码上无法建成:对 pi-ai 恰好自带 catalog 的那些 provider,`discoverModels` 会在任何网络调用之前短路到内置 catalog,因而对 Key 什么都验证不了;而 DeepSeek 卡片根本没有探测。验证器的价值在于分清「Key 被拒」与「无法连通」,而这正是本 Agent Note 要让其变得可靠的区分;先建验证器只会得到一个分不清自身结果的验证器。同类产品也不在保存时验证,因此保存时的阻断式网络调用会是一个意外行为,而非一处缺失。 - -## Acceptance criteria - -- 浏览器、两个 resolver 与两处凭据读取接受与拒绝同一组**已提供**的字符串:纯空白、带首尾空白、含中间空格、C0 控制字符、emoji、中文、全角输入均被拒绝;可打印 ASCII 的 Key 被接受并 trim。 -- 不指定任何凭据的 profile 仍解析为「没有 Key」,通过内置 provider 自身的 ambient 发现或 OAuth 鉴权的路由原样可用。 -- 留空的 Key 输入框可以保存卡片其余部分而不写入凭据;只含空白的输入框则以字段级失败呈现,而不是被静默丢弃。 -- 被拒绝的 Key 在 Web UI 中定位到 API Key 字段并拦截提交;settings 与凭据均不写入。 -- 非法抵达 resolver 的 Key 以 `INVALID_CREDENTIAL` 失败,消息指明修复位置、不含 Key 的任何片段,且不被重试。 -- `llm-pi-ai` 的探测把非法 Key 报为 Key 故障,而非端点不可达。 -- 合法 Key 仍沿既有 `credentials.set` 路径原样通过。 - -## Risks - -形状启发式可能拒绝一个真实的 Key。全大写标识符接 `=`、以及首尾成对引号,都是已知 provider 不会签发的形态,且该规则只在浏览器中运行,因此撞上它的用户仍可通过环境变量设置该凭据。残留代价是对一个尚无人报告过的 Key 给出一次令人困惑的拒绝。 - -限定为可打印 ASCII 比传输本身的要求更严:header value 是可以承载 `\x80`–`\xFF` 的。放行 latin-1 会让 `é` 通过并换回一个语焉不详的 401,而不是一次本地的、有解释的拒绝,因此从严是刻意的。若某个 provider 签发 latin-1 的 Key,这条规则需要放宽。 - -字符集断言存在两份,每个 source plane 一份。布局禁止共享它,重复检测门禁可能会标记这一对;两侧各自带测试并在注释中指名其孪生体。 - -把这件事做错的最大代价,是把「未指定」当成「非法」。一条施加到 `undefined` 上的规则会打断每一条依赖 ambient 发现或 OAuth 鉴权的路由——`openai-codex` 根本无法接受 Key——而一个会拦截提交的空输入框,则会让改动任何其他设置都必须重新输入 Key。这两点都应落在测试里,而不只是写在本 Agent Note 中。 - -早先版本已存下的 Key 会经 `resolveApiKey` 读取,因此一个非法的既存值将从解析时开始失败,而非到请求时才失败。这正是意图所在——诊断变好了——但对当前正持有这类值的人而言,失败点提前了。 diff --git a/apps/web/tests/models-settings.e2e.ts b/apps/web/tests/models-settings.e2e.ts index 1d9117dc85..0468e0e9d1 100644 --- a/apps/web/tests/models-settings.e2e.ts +++ b/apps/web/tests/models-settings.e2e.ts @@ -75,6 +75,25 @@ describe('web e2e: Models settings page configures a dormant provider', () => { await compareOrRefreshGolden(EMPTY_EXPECTED, snapshot, MODE) }, 60_000) + it('refuses a key no HTTP header can carry before anything is written', async () => { + onTestFailed(() => saveFailureShot(page, 'web-e2e-models-illegal-key')) + const dialog = page.getByRole('dialog', { name: '设置' }) + const key = dialog.getByLabel('API 密钥') + const save = dialog.getByRole('button', { name: '保存', exact: true }) + + // The paste that used to save cleanly and then fail the first turn with a + // ByteString TypeError now names the field that holds it. + await key.fill('sk-\u{1F600}minimax') + await dialog.getByText('该 API 密钥含有无法发送的字符。请只粘贴原始密钥。').waitFor({ timeout: 10_000 }) + await expect.poll(async () => save.isEnabled(), { timeout: 10_000 }).toBe(false) + + // Clearing it restores submit: an empty field means "keep what is stored", + // never a refusal, or editing any other setting would demand the key. + await key.fill('') + await expect.poll(async () => save.isEnabled(), { timeout: 10_000 }).toBe(true) + expect(await dialog.getByText('该 API 密钥含有无法发送的字符。请只粘贴原始密钥。').count()).toBe(0) + }, 60_000) + it('stores the key under the derived reference and the route registers live', async () => { onTestFailed(() => saveFailureShot(page, 'web-e2e-models-add')) const dialog = page.getByRole('dialog', { name: '设置' })