fix(web): silence the stale gate hint, clear whitespace fields, narrow the paste heuristic

2026-08-07 11:36:01 +08:00
parent 91e9d5afa2
commit 5a422337f8
@@ -2,5 +2,5 @@
 # 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: a0a99bfcace5422ed021d684c5d5aae48c197af7
-2026-08-06-api-key-format-validation.zh.md: b6dc836cbc7bc828f343d9d376dab6e5c3d424ee
+2026-08-06-api-key-format-validation.md: 4666f6197dbed060d00c77fdd6b87842141c10f4
+2026-08-06-api-key-format-validation.zh.md: 75c98bd29cf009e69ceb450432f540e3f49d99d0
@@ -86,7 +86,7 @@ The client cannot import any of this: client packages reference only client pack

 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.
+The shape heuristic can refuse a real key. The first draft matched any upper-case identifier followed by `=`, which review showed was broader than intended: an all-upper-case base64 key ending in padding (`ABCD==`) matched an assignment it does not resemble. Requiring a non-`=` character after the separator excludes padding, since base64 only ever pads at the end. What remains — an upper-case name, one `=`, then a value — is a shape no known provider issues, and the rule runs only in the browser, so a user who still hits it can 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.

@@ -86,7 +86,7 @@ Status: implemented

 格式错误的 Key 在持有它的那个字段上就被拒绝；格式错误的已存储 Key 以 `INVALID_CREDENTIAL` 失败，消息指明修复位置且不含 Key 的任何片段。由于该 code 位于 `DEFAULT_RETRYABLE_CODES` 之外，一个确定性的凭据故障不再被当作瞬时传输抖动重试三次。`llm-pi-ai` 的探测把非法 Key 报为凭据故障，而非端点不可达。

-形状启发式可能拒绝一个真实的 Key。全大写标识符接 `=`、以及首尾成对引号，都是已知 provider 不会签发的形态，且该规则只在浏览器中运行，因此撞上它的用户仍可通过环境变量设置该凭据。残留代价是对一个尚无人报告过的 Key 给出一次令人困惑的拒绝。
+形状启发式可能拒绝一个真实的 Key。最初的写法匹配任意「全大写标识符接 `=`」，评审指出其覆盖面比预期更宽：一个以 padding 结尾的全大写 base64 Key（`ABCD==`）会命中它并不像的赋值形态。要求分隔符之后必须是非 `=` 字符即可排除 padding——base64 的 padding 只出现在末尾。剩下的形态（大写名称、一个 `=`、然后是值）是已知 provider 不会签发的，且该规则只在浏览器中运行，因此仍撞上它的用户可通过环境变量设置该凭据。残留代价是对一个尚无人报告过的 Key 给出一次令人困惑的拒绝。

 限定为可打印 ASCII 比传输本身的要求更严：header value 是可以承载 `\x80`–`\xFF` 的。放行 latin-1 会让 `é` 通过并换回一个语焉不详的 401，而不是一次本地的、有解释的拒绝，因此从严是刻意的。若某个 provider 签发 latin-1 的 Key，这条规则需要放宽。

@@ -93,6 +93,10 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
  // because its own field already explains itself, and a satisfied card says
  // nothing at all rather than printing an empty paragraph.
  const hint = failure !== undefined || ready
+    // The key field prints its own failure directly beneath itself, so a card
+    // blocked only by the key stays silent here rather than answering with the
+    // next unmet gate — which is satisfied, and reads as a second, false fault.
+    || keyFailure !== undefined
    ? undefined
    : baseURL.length === 0
      ? t('customNeedsBaseUrl')
@@ -167,7 +167,12 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
    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))
+    // A value of nothing but whitespace is cleared, not stored: `stringAt`
+    // already reports it as absent, so the field would otherwise render empty
+    // while the draft still carried the spaces into `settings.yaml`, where
+    // both adapters would accept that non-empty string as a real value.
+    const value = next === undefined || next.trim().length === 0 ? undefined : next
+    setDraft(current => value === undefined ? deletePath(current, [key]) : setPath(current, [key], value))
  }

  // The model list is validated by the same per-row checker for both families,
@@ -12,13 +12,15 @@
 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.
+ * A pasted `NAME=value` environment line. Two narrowings keep real keys clear
+ * of it: the name must be upper-case, so `sk-` forms break at the hyphen, and
+ * the `=` must be followed by something other than another `=`, so base64
+ * padding on an all-upper-case key (`ABCD==`) is not mistaken for an
+ * assignment. 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_]*=/
+const ENV_LINE = /^[A-Z][A-Z0-9_]*=[^=]/

 /** Copy key naming why a typed key cannot be saved. */
 export type ApiKeyFailureKey = 'keyBlank' | 'keyIllegalCharacters' | 'keyLooksWrapped'
@@ -1092,6 +1092,8 @@ describe('apiKeyFailure', () => {
    ['a padded key, which the caller trims', '  sk-abc  '],
    ['the printable-ASCII boundary characters', '!~'],
    ['a hyphenated key carrying an equals sign', 'sk-ABC=xyz'],
+    ['an all-upper-case key ending in base64 padding', 'ABCD=='],
+    ['an all-upper-case key ending in one padding character', 'MNOPQRST='],
  ])('accepts %s', (_label, draft) => {
    expect(apiKeyFailure(draft)).toBeUndefined()
  })
@@ -880,6 +880,22 @@ describe('hand-declared providers', () => {
    expect(set).not.toHaveBeenCalled()
  })

+  it('stays silent about the other gates when only the key is refused', () => {
+    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}' } })
+
+    // Route, endpoint, and models are all satisfied, so answering with the
+    // next unmet gate would print a second, false fault beside the real one.
+    expect(screen.getByText(en.keyIllegalCharacters)).toBeTruthy()
+    expect(screen.queryByText(en.customNeedsModels)).toBeNull()
+    expect(screen.queryByText(en.customNeedsBaseUrl)).toBeNull()
+  })
+
  it('tells a whitespace-only key what a blank field means on a create card', () => {
    const { mutate } = mountCard()

@@ -927,6 +943,21 @@ describe('API key field', () => {
    expect(set).not.toHaveBeenCalled()
  })

+  it('clears a whitespace-only base URL instead of writing the spaces', async () => {
+    const { mutate } = await mountSection()
+    openEditor('openai')
+
+    // The field renders this as empty, so the draft must agree: storing the
+    // spaces would hand both adapters a non-empty string they accept as a URL.
+    fireEvent.change(screen.getByLabelText(en.baseUrl), { target: { value: '   ' } })
+    fireEvent.click(screen.getByText(en.apply))
+
+    await waitFor(() => { expect(mutate).toHaveBeenCalled() })
+    const ops = firstMutate(mutate).ops
+    expect(ops.some(op => op.op === 'set' && op.path.includes('baseURL'))).toBe(false)
+    expect(ops.some(op => op.op === 'unset' && op.path.includes('baseURL'))).toBe(true)
+  })
+
  it('blocks submit and names the field when the key holds only whitespace', async () => {
    const { mutate, set } = await mountSection()
    openEditor('openai')