diff --git a/client/electron/test/criteria-hygiene.test.mjs b/client/electron/test/criteria-hygiene.test.mjs index 2d6a4f8..153c5df 100644 --- a/client/electron/test/criteria-hygiene.test.mjs +++ b/client/electron/test/criteria-hygiene.test.mjs @@ -33,12 +33,34 @@ const HERE = dirname(fileURLToPath(import.meta.url)); const RELECTRON = join(HERE, '..'); // test/ 的上一级就是 client/electron const SELF = join(HERE, 'lib', 'read.mjs'); -/** 判据文件清单：`test/**` 下会跑的判据 + 编排器；不含 lib/ 与 manual/ */ +/** + * 仓库目录名 —— 判"某条绝对路径是不是落在仓库内"用的**值**特征。 + * + * 为什么不从 `ROOT` 推：这个字面量本身就是"仓库根在哪"的**事实**， + * 而本判据禁止的正是"把它写进代码"。这里写它，是因为判据**必须**知道要找什么。 + */ +const REPO_NAME = 'agentmail'; + +/** 某段文本（`needle`）在原始源码里出现在第几行（1-based）；找不到返回 0 */ +function lineOf(raw, needle) { + const i = raw.indexOf(needle); + return i < 0 ? 0 : raw.slice(0, i).split('\n').length; +} + +/** + * 判据文件清单：`test/**` 下会跑的判据 + 编排器 + **共享助手（`lib/`）**。 + * + * ★ 为什么 `lib/` **必须**在射程内（pi 2026-09-15 指出的洞）：我原来把 `lib/` 与 `manual/` + * 一起跳过了，理由是"`lib/read.mjs` 是共享助手"。但那正是**最可能的下一次复发点** —— + * 硬编码的仓库根**挪进 `test/lib/`**（一个"路径助手"最该待的地方）就完全不在本判据射程内。 + * 射程靠"这个目录看起来像什么"来裁，等于给逃逸指了路。 + * `manual/` 不一样：那是人工跑的脚本，**不进套件**，留在射程外的理由与它是否"助手"无关。 + */ function criteriaFiles(dir = HERE, out = []) { for (const e of readdirSync(dir, { withFileTypes: true })) { const p = join(dir, e.name); if (e.isDirectory()) { - if (e.name === 'lib' || e.name === 'manual' || e.name === 'node_modules') continue; + if (e.name === 'manual' || e.name === 'node_modules') continue; criteriaFiles(p, out); } else if (/\.(test\.mjs|test\.ts|test\.tsx|mjs)$/.test(e.name) && !e.name.endsWith('.d.ts')) { out.push(p); @@ -75,7 +97,16 @@ test('★ 判据目录里不得出现裸 readFileSync（必须走 code/prose/byt const offenders = []; for (const f of criteriaFiles()) { if (f === SELF) continue; // 读取器的实现自己当然要用它 - const src = prose(f); // 扫的是"文本里有没有这个写法"，所以读原文 + /* + * ★ 判的是**代码**，不是文本 —— 这里必须用 `code()`（剥注释）。 + * 原来用的是 `prose()`（原文），理由是"扫的是文本里有没有这个写法"。 + * 但那样一来，**注释里提到这个名字**就会被判违规 —— 我自己立刻撞上了： + * 在注释里写下"这个正则的源码里会出现 `readFileSync`"之后，这条判据就红了， + * 而红的原因**不是代码裸用了它，是我把规则写进了注释**。 + * 这正是本仓那条纪律的另一面：**注释说明禁令 ≠ 违反禁令**。 + * 不剥注释的判据会退化成"逼人别解释",与"理由要写清"直接冲突。 + */ + const src = code(f); if (BARE.test(src)) { const line = src.split('\n').findIndex(l => BARE.test(l)) + 1; offenders.push(`${relative(RELECTRON, f)}:${line}`); @@ -106,9 +137,18 @@ test('★ 用到 code/prose/bytes 就必须 import（不许靠运行时才发现 const EXPORTS = ['code', 'prose', 'bytes']; const problems = []; const SELF_PATH = fileURLToPath(import.meta.url); + /** 判据文件清单里，哪个文件是这些函数的**定义处**（它当然是"用了但不 import"） */ + const DEFINES_THEM = SELF; // test/lib/read.mjs for (const f of criteriaFiles()) { // 它自己的源码里就写着 code/prose/bytes 这几个名字（EXPORTS 列表），跳过自己 if (f === SELF_PATH) continue; + /* + * ★ `lib/read.mjs` 是这些函数的**定义处** —— 它"用了但不 import"是必然的、不是缺陷。 + * 这条豁免**必须按"是不是定义处"判，不能按"是不是在 lib/ 下"判**： + * 否则我把仓库根硬编码挪进 `test/lib/` 那个洞就会被同一条豁免再放行一次 + * （pi 2026-09-15 指出的形状：**射程/豁免按目录名裁，等于给逃逸指路**）。 + */ + if (f === DEFINES_THEM) continue; const src = prose(f); if (src.includes("from './lib/read.mjs'") || src.includes("from '../lib/read.mjs'")) { const m = /import \{([^}]*)\} from '\.\.?\/lib\/read\.mjs'/.exec(src); @@ -149,26 +189,97 @@ test('★ 用到 code/prose/bytes 就必须 import（不许靠运行时才发现 */ test('★ 判据不许把仓库根硬编码成绝对路径（必须从本文件位置推）', () => { /* - * 例外：**工具链/SDK 的绝对路径是合法的** —— 那些东西本来就不在仓库里， - * 推不出来（`TOOLCHAIN_ROOT = '/opt/huawei/command-line-tools'`）。 - * 所以按**名字**放行含 `TOOLCHAIN`/`SDK`/`HAP` 的常量，而不是按值的白名单 —— - * 值白名单会逼着下一个人为了过判据去改那个路径的写法。 - * 另一半保证：仓库**内部**的路径一律不许硬编码，那才是"读错树"的来源。 + * ★ 判法是**按值**，不是按名字 —— 这是 pi 2026-09-15 抓到的第一个洞： + * 我原来写的是 `if (looksLikeRepo && !TOOLCHAIN_OK.test(name))`， + * 也就是**名字白名单压过了值判断** ⇒ `const SDK_ROOT = '/home/program/agentmail'` + * 和 `const HDC_BASE = '/home/program/agentmail'` **直接放行**（实测：两条都过）。 + * 那正是 `CRITERIA.md` 里"allow-list"那条要防的形状：**换个变量名就过**。 + * 我当时的理由是"按值白名单会逼下一个人改路径写法" —— 取舍应该反过来： + * **值在仓库里 ⇒ 一律拒；例外只给"值本来就在仓库外"**（`/opt/`、`/usr/` 这类）。 + * 这样既不逼人改写法，也堵掉"换个名字就过"。 + * + * ★ 字面量形态也放宽了（第二个洞）：原来只认**单引号**的 `const/let/var` 赋值， + * 于是双引号、模板串、`path.join(...)`、内联参数、数组元素、`process.chdir(...)` + * 全都逃逸。现在改成：**扫真代码里任何字符串字面量**（三种引号）， + * 只要它的值落在仓库内就报 —— 不依赖"它被赋给了哪个变量"。 */ - const TOOLCHAIN_OK = /TOOLCHAIN|_SDK|SDK_|HAP_|EMULATOR|HDC/i; const problems = []; + const seen = new Set(); + const add = (msg) => { if (!seen.has(msg)) { seen.add(msg); problems.push(msg); } }; for (const f of criteriaFiles()) { + const raw = prose(f); const src = code(f); - // 只看**真的在赋值绝对路径**的那些行；注释已被剥掉，不会拿说明文字误报 - for (const m of src.matchAll(/(?:const|let|var)\s+(\w+)\s*=\s*'(\/[^']*)'/g)) { - const [, name, val] = m; - const looksLikeRepo = new RegExp(`(^|/)${relative(RELECTRON, f).split('/')[0]}|agentmail`, 'i').test(val) - || /PROJECT|REPO|WORKSPACE/i.test(name); - if (looksLikeRepo && !TOOLCHAIN_OK.test(name)) { - problems.push(`${relative(RELECTRON, f)}：\`${name} = '${val}'\` —— 这是**仓库内**的路径，` - + `必须从 \`import.meta.url\` 推（\`join(dirname(fileURLToPath(import.meta.url)), '..', …)\`），` - + `否则这个判据读的不是它自己那棵树`); - } + const rel = relative(RELECTRON, f); + /* + * 判法分两层，**都按值**： + * + * (A) **绑定**成常量的仓库内绝对路径（`const X = "…/agentmail…"`，三种引号）。 + * 命中即报 —— 这正是把判据从"读自己那棵树"改成"读固定那棵树"的动作。 + * 例外只给"值本来就在仓库外"（`/opt/`、`/usr/`）：那是**工具链/SDK**路径， + * 仓库里推不出来，所以按值放行是对的（按**名字**放行就是 pi 抓到的后门）。 + * + * (B) **直接**把仓库内绝对路径喂给取值/读盘函数（`readFileSync(…)`、`prose(…)`、 + * 内联 `join(…)`、`process.chdir(…)`）—— 覆盖 pi 指出的 + * "内联参数/数组元素/path.join"那几种逃逸。 + * + * ★ 为什么不再"扫一切字符串字面量"（我第一版那样）：`'/home/program/agentmail'` + * 在本仓有**正当用途** —— 测试数据。实测误报： + * `test/components/PermissionPanel.test.tsx:27 from_workspace: '/home/program/agentmail'` + * `test/components/replyTarget.test.tsx:307 expect(formatAddress('pi', '/home/program/agentmail', …))` + * 那是"地址长这样"，不是"去读那棵树"。**判据要抓的是"拿它去读文件"，不是"提到它"。** + * 用行内容判"是不是注释"来豁免也不行 —— 那是按形状裁，不是按风险裁。 + */ + /* + * ★ 判**整条赋值表达式**，不是只看第一个字面量。 + * 为什么（我自己测出来的漏）：`const ROOT = join('/home/program', 'agentmail')` + * 里**没有任何一个**字面量同时"以 / 开头"且"含仓库名" —— 仓库名被拆成了两个片段， + * 于是老写法直接放行。拼接所有片段后再判，才抓得到。 + */ + const LIT = /(['"`])((?:\\.|(?!\1)[^\\])*)\1/g; + const BIND = /(?:const|let|var)\s+(\w+)\s*=\s*([^\n;]+)/g; + for (const m of src.matchAll(BIND)) { + const [, name, rhs] = m; + const lits = [...rhs.matchAll(LIT)].map(x => x[2]); + const whole = lits.join(''); // 拼起来看"合起来是不是仓库路径" + const joined = lits.length > 1; + /* + * ★ 两个**各自独立**的触发条件，命中任一即报： + * + * (i) **值**落在仓库里（`whole`/`lits` 含仓库名，且是绝对路径）； + * (ii) **名字**读起来像"仓库根/工作区根"，且它绑的是一个**绝对路径**。 + * + * 为什么 (ii) 必须留着 —— 这是我改完 (i) 之后自己测出来漏掉的形状： + * `const WORKSPACE_ROOT = '/srv/ci/build/checkout';` + * 仓库被复制/检出到**别的目录名**下时，值里就没有 `agentmail` 了， + * 可它**仍然是"把判据钉死在一条绝对路径上"** —— 换棵树照样读错。 + * 我原来的版本靠 (ii) 抓这种，改成纯值判断后**把它丢了**（实测：改前红、改后绿）。 + * ⇒ pi 说的"按名字放行是 allow-list 要防的形状"是对的，但**结论不是"把名字判断删掉"**， + * 而是**把它降级**：名字不再能**豁免**任何东西（那才是后门）， + * 但它仍然可以**和值判据并列为一条独立的触发线**。豁免只按值给（`/opt/`、`/usr/`）。 + */ + const abs = lits.some(v => v.startsWith('/')); + const repoByValue = abs && (joined ? whole.includes(REPO_NAME) : lits.some(v => v.includes(REPO_NAME))); + const repoByName = /\b(PROJECT|REPO|WORKSPACE|CHECKOUT)\b|_ROOT$|^ROOT$/i.test(name); + if (!repoByValue && !(repoByName && abs)) continue; + if (lits.some(v => v.startsWith('/opt/') || v.startsWith('/usr/'))) continue; // 工具链，仓库外 + add(`${rel}:${lineOf(raw, m[0])} \`${name} = ${rhs.trim().slice(0, 60)}\` —— 这是**仓库内**的绝对路径。` + + `\n 必须从 \`import.meta.url\` 推：\`join(dirname(fileURLToPath(import.meta.url)), '..', …)\`，` + + `否则这个判据读的不是它自己那棵树（会静默读另一棵并报绿）`); + } + /* + * 这个正则的**源码里**会出现 `readFileSync` 这个词 —— 而本文件上面那条"不许裸用 + * readFileSync"的判据是扫源码文本的，会把它当违规（我自己先撞了一次）。 + * 所以用 `new RegExp` 把名字拼出来，让**字面量**不出现在源码里。 + */ + const FEEDS = new RegExp( + '(?:readFile' + 'Sync|readdirSync|prose|code|bytes|chdir|existsSync|statSync)\\s*\\(([^)]{0,240})\\)', 'g'); + for (const m of src.matchAll(FEEDS)) { + const lits = [...m[1].matchAll(LIT)].map(x => x[2]); + const whole = lits.join(''); + if (!whole.includes(REPO_NAME)) continue; + if (lits.some(v => v.startsWith('/opt/') || v.startsWith('/usr/'))) continue; + add(`${rel}:${lineOf(raw, m[0])} 读盘调用里直接写死了仓库内路径（\`${lits.join(' + ')}\`）` + + `\n 读盘用的路径必须从本文件位置推，否则换一棵树就读错`); } } assert.deepEqual(problems, [], @@ -184,10 +295,27 @@ test('★ 判据不许把仓库根硬编码成绝对路径（必须从本文件 * 实测（我自己的 `harmony-arkts` 报违规时）：报出 64/47，**真实文件是 80/63**， * 读者第一步就得先猜"这是剥过的还是没剥的"。 * - * 判据做法：造一个含多行块注释的样本，断言剥完**行数不变**、且行号仍然对得上； - * 再断言注释内容确实被去掉了（别为了保行号把注释留下）。 + * ★ 判据做法（pi 2026-09-15 指出的第四个洞）：我原来只对一个**手写合成样本**断言， + * 而它要修的故障**是从真实文件里来的**。合成样本过、真实文件错位，这个形状完全可能 + * （某个文件里有我没料到的注释写法）。所以现在**对每一个判据文件都断言** —— + * 合成样本留在下面当"探针没坏"的正例自检，**真实文件那层才是主体**。 */ -test('★ stripComments 必须保持行号（否则判据报的行号全是错的）', () => { +test('★ stripComments 必须保持行号（对所有真实判据文件，不只是合成样本）', () => { + // (1) 主体：**每一个真实文件**剥完之后行数必须一模一样 + const misaligned = []; + const countLines = (t) => t.split('\n').length; + for (const f of criteriaFiles()) { + const src = prose(f); + if (countLines(stripComments(src)) !== countLines(src)) { + misaligned.push(`${relative(RELECTRON, f)}（${countLines(src)} -> ${countLines(stripComments(src))} 行）`); + } + } + assert.deepEqual(misaligned, [], + '这些文件剥完注释后**行数变了** —— 它们报出的行号会整体错位，' + + '而全仓都用 `文件:行号` 定位（grep / 编辑器跳转 / git show 核对）：\n ' + + misaligned.join('\n ')); + + // (2) 正例自检：合成样本上"必须能抓到错位"（否则 (1) 全绿可能只是探针坏了） const sample = [ '/*', ' * 多行块注释', @@ -207,4 +335,13 @@ test('★ stripComments 必须保持行号（否则判据报的行号全是错 '剥完之后第 5 行不再是原来的第 5 行'); assert.match(out.split('\n')[6], /const b = 2;/, '单行块注释所在的那一行之后，行号错位了'); + + /* + * ★ 已知限制（记在这里，免得下一个人以为它是完整实现 —— pi 2026-09-15 指出）： + * `stripComments` 的 `//` 分支是 `(^|[^:])\/\/[^\n]*`，只保护了 `x://` 这种。 + * 于是**普通字符串里的 `//` 会被当成注释剥掉** —— `const s = 'a//b'` 会变成 `const s = 'a`。 + * 今天无害（没有判据靠这种字符串），但它与"剥注释剥多/剥少"是同一族。 + * 真要修得先有词法状态机，而不是再加一条正则 —— 那是另一件事，不在这里顺手补。 + * **这条限制没有判据**（写不出不靠词法分析就能判的形状），所以只能留成文字。 + */ });