cleanup: reject private issue shorthand

This commit is contained in:
Tianyi Cui
2026-08-08 01:40:14 +08:00
parent 6d09c315b9
commit 95c01e6e61
5 changed files with 19 additions and 16 deletions

View File

@@ -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: e9ca76ede06080f2b868f6436998d163e642adbc
2026-08-06-api-key-format-validation.zh.md: 5666a884d4c9478291072375681d8d3526b2632a
2026-08-06-api-key-format-validation.md: 2174cb466c6af72f15005ce1ba3dec8100de6f2f
2026-08-06-api-key-format-validation.zh.md: f9b7d6518fedc42e5264c46beaa1a78619139c58

View File

@@ -14,13 +14,11 @@ Pasting a key containing an emoji, CJK text, or a full-width punctuation mark in
Whitespace passed every check. `ProviderEditor` tested `keyDraft.length`, so a key of three spaces was stored and then authenticated as `Bearer` plus blanks. Neither adapter checked 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.
This single predicate covers every reported input: 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 failures share one definition rather than 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 — and requiring a non-`=` character after the separator keeps base64 padding clear of it too. It reports the same format failure as an illegal character rather than its own message: the reader's next move is identical either way, so a separate line would name a cause without changing what to do.
@@ -76,7 +74,7 @@ The client cannot import any of this: client packages reference only client pack
**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.
**Probing the provider at save time to prove the key works.** It would close the original complaint — 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

View File

@@ -14,13 +14,11 @@ Status: implemented
空白字符能通过每一道检查。`ProviderEditor` 判的是 `keyDraft.length`,于是三个空格构成的 Key 会被存下,随后以 `Bearer` 加若干空格去认证。两个适配器都不检查来自凭据或环境的 Key——而那正是 Models 页写入的路径,也就是用户真正走的路径。
来源deepseek-harness#1594#1595dsh-external#247#249#266#210
## Decision
一条规则定义什么是合法 Key**trim 之后非空,且每个字符都落在 `[\x21-\x7E]`**——可打印 ASCII不含空格。
这一个断言覆盖了来源列出的全部输入空值、首尾空白、中间空白、C0 控制字符、emoji、中文、全角标点。它同时正是造成 ByteString 失败的那条约束,所以两个 issue 收敛于同一个定义,而不是两个恰好相关的修复。
这一个断言覆盖了所有已报告的输入空值、首尾空白、中间空白、C0 控制字符、emoji、中文、全角标点。它同时正是造成 ByteString 失败的那条约束,所以这些故障收敛于同一个定义,而不是两个恰好相关的修复。
第二条更窄的规则用于识别整行粘贴的环境变量:匹配 `^[A-Z][A-Z0-9_]*=[^=]` 或首尾成对引号的输入会被拒绝。把前缀限定为全大写可以让真实 Key 与之绝缘——`sk-` 这类形态会在连字符处中断标识符匹配——而要求分隔符之后必须是非 `=` 字符,则让 base64 的 padding 也与之绝缘。它报出的是与非法字符相同的那条格式失败,而不是自己的一句:读到它的人下一步动作完全一样,因此单列一句只会点出一个原因,却不改变该怎么做。
@@ -76,7 +74,7 @@ Status: implemented
**让形状启发式也在 resolver 中运行。** 更对称,且能拦住直接写进 `.env` 的整行环境变量。因上文所述的锁死风险而否决resolver 中的一次误判会让用户无路可走,浏览器中的一次误判则仍留有环境变量这条路。
**在保存时探测 provider 以证明 Key 可用。** 它能关掉来源真正开篇抱怨的那件事——保存报成功、第一轮才失败。因超出范围而否决,且在当时的代码上无法建成:对 pi-ai 恰好自带 catalog 的那些 provider`discoverModels` 会在任何网络调用之前短路到内置 catalog因而对 Key 什么都验证不了;而 DeepSeek 卡片根本没有探测。验证器的价值在于分清「Key 被拒」与「无法连通」,而这正是本次改动让其变得可靠的区分;先建验证器只会得到一个分不清自身结果的验证器。同类产品也不在保存时验证,因此保存时的阻断式网络调用会是一个意外行为,而非一处缺失。
**在保存时探测 provider 以证明 Key 可用。** 它能关掉最初报告的那件事——保存报成功、第一轮才失败。因超出范围而否决,且在当时的代码上无法建成:对 pi-ai 恰好自带 catalog 的那些 provider`discoverModels` 会在任何网络调用之前短路到内置 catalog因而对 Key 什么都验证不了;而 DeepSeek 卡片根本没有探测。验证器的价值在于分清「Key 被拒」与「无法连通」,而这正是本次改动让其变得可靠的区分;先建验证器只会得到一个分不清自身结果的验证器。同类产品也不在保存时验证,因此保存时的阻断式网络调用会是一个意外行为,而非一处缺失。
## Consequences

View File

@@ -2,15 +2,18 @@ import { describe, expect, it } from 'vitest'
import { findInternalRepositoryReferences } from './verify-public-repository-links.ts'
describe('public repository link policy', () => {
it('rejects the internal remote and accepts the public home', () => {
const internalRepository = ['deepseek-harness', 'deepseek-harness'].join('/')
it('rejects internal repository references and accepts the public home', () => {
const internalOwner = ['deepseek', 'harness'].join('-')
const internalRepository = [internalOwner, internalOwner].join('/')
const source = [
'https://github.com/deepseek-ai/deepseek-harness-sdk',
`https://github.com/${internalRepository}/issues/1`,
`${internalOwner}#2`,
].join('\n')
expect(findInternalRepositoryReferences('subject.md', source)).toEqual([
{ file: 'subject.md', line: 2 },
{ file: 'subject.md', line: 3 },
])
})
})

View File

@@ -1,4 +1,4 @@
/** Reject tracked files that expose the internal repository remote. */
/** Reject tracked files that expose the internal repository identity. */
import { execFileSync } from 'node:child_process'
import { existsSync, lstatSync, readFileSync, readlinkSync } from 'node:fs'
@@ -6,7 +6,9 @@ import { resolve } from 'node:path'
import { pathToFileURL } from 'node:url'
const root = resolve(import.meta.dirname, '..')
const internalRepository = ['deepseek-harness', 'deepseek-harness'].join('/')
const internalOwner = ['deepseek', 'harness'].join('-')
const internalRepository = [internalOwner, internalOwner].join('/')
const internalIssueShorthand = `${internalOwner}#`
/** One tracked reference to the internal repository. */
export interface InternalRepositoryReference {
@@ -25,7 +27,9 @@ export interface InternalRepositoryReference {
export function findInternalRepositoryReferences(file: string, source: string): InternalRepositoryReference[] {
const references: InternalRepositoryReference[] = []
for (const [index, line] of source.split('\n').entries()) {
if (line.includes(internalRepository)) references.push({ file, line: index + 1 })
if (line.includes(internalRepository) || line.includes(internalIssueShorthand)) {
references.push({ file, line: index + 1 })
}
}
return references
}
@@ -55,7 +59,7 @@ const isMain = invokedPath !== undefined && import.meta.url === pathToFileURL(re
if (isMain) {
const references = scanRepository(root)
if (references.length === 0) {
console.log('verify-public-repository-links: tracked files expose no internal repository remote.')
console.log('verify-public-repository-links: tracked files expose no internal repository identity.')
} else {
console.error('verify-public-repository-links: internal repository references found:')
for (const reference of references) console.error(` ${reference.file}:${String(reference.line)}`)