Commit 35805d85168333349d3859d44b1500b32846d898
1 parent
3bd46d05
fix(tests): 三处「隔离是表演」的测试污染;清掉共享用户库里 358 行夹具
起因是一个看起来无害的问题:<repo>/data/users.sqlite 为什么是 0 字节。 答案是没人用它——user-db 走三级上跳落在仓库外的 ~lemonAI/data(跨兄弟仓库 共享的平台数据,与 skill-db / benchmark-db 一致;仓库内 data/ 放 graph/companies/ devloop 这类租户数据)。那个空文件是遗留,已删。 顺着查下去发现三处同型缺陷,形态都是「产生 → 传递 → 落库 → 读回」漏掉读回, 而且是最有欺骗性的一种:测试设了隔离用的 env,没有人读,于是数据全写进 **共享的生产库**,测试照样全绿,afterAll 删的 /tmp 文件从来没被创建过、清理还“成功”。 1) USERS_DB_PATH 压根不存在 user-db.test.js 只能直写共享用户库(注释老实写着 "uses hardcoded data/users.sqlite"),afterAll 是空的、拿「邮箱按 UUID 唯一所以不会撞」 当成了不用清理的理由。不撞是真的,不堆积是假的:357 个用户里 356 个是夹具。 加 USERS_DB_PATH 覆盖口(口径同 graph-db 的 GRAPH_DB_PATH),测试改为 先设 env 再清 require.cache 再 require(DB_PATH 是加载时求值的常量, 顺序错了照样静默失效),afterAll 真删库文件。 2) BENCHMARK_DB_PATH 有人设、没人读 benchmark-db.test.js:13 与 integration/api-routes.test.js:43 都在设它, 而 benchmark-db.js 那一行不读 → 共享 benchmark 库攒了 342 行 skill_name=benchmark_test。补上读取后,两个测试文件一行不改就真隔离了 (日志里 Initialized SQLite at /tmp/... 第一次是真的)。 3) SKILLS_DIR 是写错的变量名 config.js:13 读的是 SKILLS_CODE_DIR,所以 skillsCodeDir 一直指向那个 2.7 万条的真实共享 skills 目录。两个测试文件的变量名改正。 新增 guard server/tests/unit/test-isolation-env.test.js: 测试赋值过的**路径类** env(_DIR/_PATH/_ROOT 结尾)必须有生产读者。 只管路径类是刻意的——那是会造成静默数据污染的地方,而 HOME(os.homedir 间接读)、 JWT_SECRET、PORT 这类天然不在范围内,不需要维护会腐烂的白名单。 顺带钉住「共享 data 在仓库外」这个有意的分层,免得被“修”回仓库内。 guard 第一版把自己文档注释里的举例当成真赋值、自己判红了,所以两侧都先剥注释—— 凡是静态扫源码的检查都要先想清楚该量的是代码还是文本。 数据清理:共享 ~lemonAI/data/users.sqlite 删掉 358 行夹具 (不是 356——修好之前又跑了两轮测试,每轮 +2,正好印证泄漏一直活跃), 删前备份 + 计数闭合校验(保留行 + 匹配行 == 总数才动手), 删后真人 bboonnee@gmail.com 完好,总数 1。 验证:854 全绿(84 文件)。跑完全套后 users / benchmark_logs / skills 三个共享库的 sha256 逐位不变——修好之前这不可能。 3 处变异逐一精确变红(去掉任一 env 读取、把变量名改回错的)后逐字还原。 未动:JWT_SECRET / API_KEYS 生产侧无读者或名字不符(config.js 读 OPEN_API_KEYS), 不造成数据污染,另报;共享 benchmark 库那 342 行夹具未清,等确认。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Showing
6 changed files
with
173 additions
and
6 deletions
server/services/benchmark-db.js
| @@ -11,7 +11,12 @@ const fs = require('fs'); | @@ -11,7 +11,12 @@ const fs = require('fs'); | ||
| 11 | 11 | ||
| 12 | // 数据目录 | 12 | // 数据目录 |
| 13 | const DATA_DIR = path.resolve(__dirname, '../../../data'); | 13 | const DATA_DIR = path.resolve(__dirname, '../../../data'); |
| 14 | -const DB_PATH = path.join(DATA_DIR, 'benchmark_logs.sqlite'); | 14 | +// BENCHMARK_DB_PATH 覆盖口。**两个测试文件早就在设这个变量了** |
| 15 | +// (benchmark-db.test.js:13 与 integration/api-routes.test.js:43),而这一行以前不读它 —— | ||
| 16 | +// 于是它们的隔离是表演:数据全写进共享的生产 benchmark 库(攒了 342 行 skill_name=benchmark_test), | ||
| 17 | +// 而 afterAll 删的那个 /tmp 文件从来没被创建过,所以清理还“成功”了。 | ||
| 18 | +// 设了没人读 = 「产生→传递→落库→读回」漏掉读回那一环,这是本项目最常见的缺陷形状。 | ||
| 19 | +const DB_PATH = process.env.BENCHMARK_DB_PATH || path.join(DATA_DIR, 'benchmark_logs.sqlite'); | ||
| 15 | 20 | ||
| 16 | let db = null; | 21 | let db = null; |
| 17 | 22 |
server/services/user-db.js
| @@ -10,8 +10,14 @@ const path = require('path'); | @@ -10,8 +10,14 @@ const path = require('path'); | ||
| 10 | const fs = require('fs'); | 10 | const fs = require('fs'); |
| 11 | const crypto = require('crypto'); | 11 | const crypto = require('crypto'); |
| 12 | 12 | ||
| 13 | +// 注意这里是**三级**上跳:从 server/services/ 落到仓库外的 ~lemonAI/data, | ||
| 14 | +// 与 skill-db / benchmark-db 一致 —— 用户、skill、benchmark 是跨兄弟仓库共享的平台数据。 | ||
| 15 | +// 仓库内的 <repo>/data 放的是本仓库自己的租户数据(graph / companies / devloop)。 | ||
| 13 | const DATA_DIR = path.resolve(__dirname, '../../../data'); | 16 | const DATA_DIR = path.resolve(__dirname, '../../../data'); |
| 14 | -const DB_PATH = path.join(DATA_DIR, 'users.sqlite'); | 17 | +// USERS_DB_PATH 覆盖口(口径同 graph-db.js 的 GRAPH_DB_PATH)。 |
| 18 | +// 没有它之前,user-db.test.js 无法隔离,每跑一次就往**共享的生产用户库**里插两行; | ||
| 19 | +// 那个库里 357 个用户有 356 个是这么攒出来的测试夹具(178 次运行),真人只有 1 个。 | ||
| 20 | +const DB_PATH = process.env.USERS_DB_PATH || path.join(DATA_DIR, 'users.sqlite'); | ||
| 15 | 21 | ||
| 16 | let db = null; | 22 | let db = null; |
| 17 | 23 |
server/tests/integration/api-routes.test.js
| @@ -42,7 +42,7 @@ describe('API Integration Tests', () => { | @@ -42,7 +42,7 @@ describe('API Integration Tests', () => { | ||
| 42 | process.env.SKILLS_DB_PATH = TEST_DB_PATH; | 42 | process.env.SKILLS_DB_PATH = TEST_DB_PATH; |
| 43 | process.env.BENCHMARK_DB_PATH = '/tmp/test-integration-bench.sqlite'; | 43 | process.env.BENCHMARK_DB_PATH = '/tmp/test-integration-bench.sqlite'; |
| 44 | process.env.PORT = '0'; | 44 | process.env.PORT = '0'; |
| 45 | - process.env.SKILLS_DIR = TEST_SKILLS_DIR; | 45 | + process.env.SKILLS_CODE_DIR = TEST_SKILLS_DIR; |
| 46 | process.env.SKILLS_DATA_DIR = TEST_DATA_DIR; | 46 | process.env.SKILLS_DATA_DIR = TEST_DATA_DIR; |
| 47 | process.env.JWT_SECRET = 'test-secret-key-for-integration'; | 47 | process.env.JWT_SECRET = 'test-secret-key-for-integration'; |
| 48 | process.env.API_KEYS = 'test-api-key-123'; | 48 | process.env.API_KEYS = 'test-api-key-123'; |
server/tests/unit/skill-dir-manager.test.js
| @@ -16,7 +16,7 @@ describe('skill-dir-manager', () => { | @@ -16,7 +16,7 @@ describe('skill-dir-manager', () => { | ||
| 16 | fs.mkdirSync(path.join(TEST_DATA_DIR, 'my-test-skill'), { recursive: true }); | 16 | fs.mkdirSync(path.join(TEST_DATA_DIR, 'my-test-skill'), { recursive: true }); |
| 17 | fs.writeFileSync(path.join(TEST_DATA_DIR, 'my-test-skill', 'SKILL.md'), '# Test'); | 17 | fs.writeFileSync(path.join(TEST_DATA_DIR, 'my-test-skill', 'SKILL.md'), '# Test'); |
| 18 | 18 | ||
| 19 | - process.env.SKILLS_DIR = TEST_CODE_DIR; | 19 | + process.env.SKILLS_CODE_DIR = TEST_CODE_DIR; |
| 20 | process.env.SKILLS_DATA_DIR = TEST_DATA_DIR; | 20 | process.env.SKILLS_DATA_DIR = TEST_DATA_DIR; |
| 21 | delete require.cache[require.resolve('../../config')]; | 21 | delete require.cache[require.resolve('../../config')]; |
| 22 | delete require.cache[require.resolve('../../services/skill-dir-manager')]; | 22 | delete require.cache[require.resolve('../../services/skill-dir-manager')]; |
server/tests/unit/test-isolation-env.test.js
0 → 100644
| 1 | +/** | ||
| 2 | + * Unit Tests — 防漂移闸:**测试用来隔离的路径 env,生产代码必须真的读它** | ||
| 3 | + * | ||
| 4 | + * 挡的是一类已经实际造成数据污染的缺陷,而且是最有欺骗性的那种形态: | ||
| 5 | + * 测试在 beforeAll 里 `process.env.XXX_DB_PATH = '/tmp/...'`,以为自己隔离了, | ||
| 6 | + * 而生产代码那一行压根不读这个变量 —— 于是数据全写进**共享的生产库**, | ||
| 7 | + * 测试照样全绿,afterAll 删的那个 /tmp 文件从来没被创建过、所以清理还“成功”了。 | ||
| 8 | + * | ||
| 9 | + * 已经发生过三次: | ||
| 10 | + * - `BENCHMARK_DB_PATH`:两个测试文件设它,benchmark-db.js 不读 → | ||
| 11 | + * 共享 benchmark 库攒了 342 行 skill_name=benchmark_test | ||
| 12 | + * - `USERS_DB_PATH`:根本没有这个覆盖口,user-db.test.js 只能直写共享用户库 → | ||
| 13 | + * 357 个用户里 356 个是测试夹具(178 次运行),真人只有 1 个 | ||
| 14 | + * - `SKILLS_DIR`:**变量名写错**,config.js 读的是 `SKILLS_CODE_DIR` → | ||
| 15 | + * skillsCodeDir 一直指向那个 2.7 万条的真实共享 skills 目录 | ||
| 16 | + * | ||
| 17 | + * 这就是本项目反复出现的「产生 → 传递 → 落库 → 读回」漏掉一环: | ||
| 18 | + * 测试把值**产生**出来了,没有人**读回**。而它的隐蔽性在于—— | ||
| 19 | + * 缺了读回那一环,测试不会红,只有共享数据在悄悄变脏。 | ||
| 20 | + * | ||
| 21 | + * ── 规则 ──────────────────────────────────────────────────────── | ||
| 22 | + * 只管**路径类**变量(名字以 _DIR / _PATH / _ROOT 结尾):那是会造成 | ||
| 23 | + * 静默数据污染的地方。`HOME`(由 os.homedir() 间接读)、`JWT_SECRET`、`PORT` | ||
| 24 | + * 这类非路径变量天然不在范围内,不需要维护白名单 —— 白名单会随时间腐烂, | ||
| 25 | + * 而后缀规则不会。 | ||
| 26 | + * | ||
| 27 | + * ponytail: 静态正则扫源码,不 require 任何模块(require 会拉起 sqlite 连接)。 | ||
| 28 | + * 上限:只认 `process.env.X` 这种字面写法,动态取(`process.env[name]`)查不出来; | ||
| 29 | + * 也不检查测试是否**在 require 之前**设 env(顺序错了同样静默失效,那一环靠各文件 | ||
| 30 | + * 自己的「写的是隔离库」断言兜——见 user-db.test.js)。 | ||
| 31 | + */ | ||
| 32 | +import { describe, it, expect } from 'vitest'; | ||
| 33 | +import fs from 'fs'; | ||
| 34 | +import path from 'path'; | ||
| 35 | + | ||
| 36 | +const ROOT = path.resolve(__dirname, '../../..'); | ||
| 37 | +const PATH_ENV = /^[A-Z0-9_]*(_DIR|_PATH|_ROOT)$/; | ||
| 38 | + | ||
| 39 | +function walk(dir, out = []) { | ||
| 40 | + if (!fs.existsSync(dir)) return out; | ||
| 41 | + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { | ||
| 42 | + if (e.name === 'node_modules') continue; | ||
| 43 | + const p = path.join(dir, e.name); | ||
| 44 | + if (e.isDirectory()) walk(p, out); | ||
| 45 | + else if (p.endsWith('.js')) out.push(p); | ||
| 46 | + } | ||
| 47 | + return out; | ||
| 48 | +} | ||
| 49 | + | ||
| 50 | +/** | ||
| 51 | + * 剥掉注释再扫。 | ||
| 52 | + * | ||
| 53 | + * 不剥的话本文件的文档注释里那句举例(`process.env.XXX_DB_PATH = ...`)会被当成真的赋值, | ||
| 54 | + * guard 自己把自己判红 —— 实测踩过。凡是「静态扫源码」的检查都要先想清楚 | ||
| 55 | + * **该量的是代码还是文本**:这里该量的是会执行的代码。 | ||
| 56 | + */ | ||
| 57 | +function stripComments(src) { | ||
| 58 | + return src | ||
| 59 | + .replace(/\/\*[\s\S]*?\*\//g, '') | ||
| 60 | + .split('\n') | ||
| 61 | + .map((l) => l.replace(/\/\/.*$/, '')) | ||
| 62 | + .join('\n'); | ||
| 63 | +} | ||
| 64 | + | ||
| 65 | +/** 测试里被**赋值**的 env 名 → 哪些文件在设它 */ | ||
| 66 | +function assignedInTests() { | ||
| 67 | + const found = new Map(); | ||
| 68 | + for (const f of walk(path.join(ROOT, 'server/tests'))) { | ||
| 69 | + const src = stripComments(fs.readFileSync(f, 'utf-8')); | ||
| 70 | + for (const m of src.matchAll(/process\.env\.([A-Z0-9_]+)\s*=(?!=)/g)) { | ||
| 71 | + if (!found.has(m[1])) found.set(m[1], new Set()); | ||
| 72 | + found.get(m[1]).add(path.relative(ROOT, f)); | ||
| 73 | + } | ||
| 74 | + } | ||
| 75 | + return found; | ||
| 76 | +} | ||
| 77 | + | ||
| 78 | +/** 生产代码里被**读取**的 env 名 */ | ||
| 79 | +function readInProduction() { | ||
| 80 | + const dirs = ['server/services', 'server/src', 'server/routes', 'server/middleware', 'packages']; | ||
| 81 | + const files = dirs.flatMap((d) => walk(path.join(ROOT, d))); | ||
| 82 | + files.push(path.join(ROOT, 'server/config.js'), path.join(ROOT, 'server/index.js')); | ||
| 83 | + | ||
| 84 | + const found = new Set(); | ||
| 85 | + for (const f of files) { | ||
| 86 | + if (!fs.existsSync(f)) continue; | ||
| 87 | + // 生产侧同样剥注释:注释里提到一个变量名不等于代码读了它 | ||
| 88 | + // (这一侧漏剥的方向更坏 —— 会把「其实没人读」误判成「有人读」,guard 变空跑) | ||
| 89 | + const src = stripComments(fs.readFileSync(f, 'utf-8')); | ||
| 90 | + for (const m of src.matchAll(/process\.env\.([A-Z0-9_]+)/g)) { | ||
| 91 | + // 生产代码里自己赋值不算「读」(几乎没有,但别让它自证) | ||
| 92 | + const after = src.slice(m.index + m[0].length, m.index + m[0].length + 3); | ||
| 93 | + if (/^\s*=(?!=)/.test(after)) continue; | ||
| 94 | + found.add(m[1]); | ||
| 95 | + } | ||
| 96 | + } | ||
| 97 | + return found; | ||
| 98 | +} | ||
| 99 | + | ||
| 100 | +describe('测试隔离:路径类 env 必须有生产读者', () => { | ||
| 101 | + const assigned = assignedInTests(); | ||
| 102 | + const read = readInProduction(); | ||
| 103 | + | ||
| 104 | + it('两侧都不是空的(防止 guard 因为路径/正则失效而静默空跑)', () => { | ||
| 105 | + expect(assigned.size).toBeGreaterThan(3); | ||
| 106 | + expect(read.size).toBeGreaterThan(3); | ||
| 107 | + // 已知一定存在的两个,任一消失说明扫描口径坏了 | ||
| 108 | + expect([...assigned.keys()]).toContain('GRAPH_DB_PATH'); | ||
| 109 | + expect(read.has('GRAPH_DB_PATH')).toBe(true); | ||
| 110 | + }); | ||
| 111 | + | ||
| 112 | + it('测试用来隔离的每个 _DIR/_PATH/_ROOT,生产代码里都有人读', () => { | ||
| 113 | + const orphans = [...assigned] | ||
| 114 | + .filter(([name]) => PATH_ENV.test(name)) | ||
| 115 | + .filter(([name]) => !read.has(name)) | ||
| 116 | + .map(([name, files]) => `${name} —— 设于 ${[...files].join(', ')},但生产代码零处读取`); | ||
| 117 | + | ||
| 118 | + // 报出全部而不是第一个:一次修完比来回跑三轮省事 | ||
| 119 | + expect(orphans).toEqual([]); | ||
| 120 | + }); | ||
| 121 | + | ||
| 122 | + it('共享数据目录在仓库外,这一点是有意的(顺带钉住,免得被“修”回仓库内)', () => { | ||
| 123 | + // user-db / skill-db / benchmark-db 用三级上跳落到 ~lemonAI/data(跨兄弟仓库共享); | ||
| 124 | + // graph-db / company-service 用两级落到 <repo>/data(本仓库租户数据)。 | ||
| 125 | + // 深度随文件所在目录变化,改动时容易改错方向,所以这里钉住两组的实际去向。 | ||
| 126 | + const shared = ['server/services/user-db.js', 'server/services/skill-db.js', 'server/services/benchmark-db.js']; | ||
| 127 | + const local = ['server/services/graph-db.js', 'server/services/company-service.js']; | ||
| 128 | + | ||
| 129 | + for (const rel of shared) { | ||
| 130 | + const resolved = path.resolve(path.dirname(path.join(ROOT, rel)), '../../../data'); | ||
| 131 | + expect(resolved).toBe(path.resolve(ROOT, '../data')); | ||
| 132 | + } | ||
| 133 | + for (const rel of local) { | ||
| 134 | + const resolved = path.resolve(path.dirname(path.join(ROOT, rel)), '../../data'); | ||
| 135 | + expect(resolved).toBe(path.join(ROOT, 'data')); | ||
| 136 | + } | ||
| 137 | + }); | ||
| 138 | +}); |
server/tests/unit/user-db.test.js
| @@ -11,15 +11,33 @@ describe('user-db', () => { | @@ -11,15 +11,33 @@ describe('user-db', () => { | ||
| 11 | const uid = crypto.randomUUID().substring(0, 8); | 11 | const uid = crypto.randomUUID().substring(0, 8); |
| 12 | const uniqueEmail = `test-${uid}@example.com`; | 12 | const uniqueEmail = `test-${uid}@example.com`; |
| 13 | const oauthEmail = `gh-${uid}@example.com`; | 13 | const oauthEmail = `gh-${uid}@example.com`; |
| 14 | + // 这个库跑完就删。以前没有覆盖口,这里写的是**共享的生产用户库** | ||
| 15 | + // (~lemonAI/data/users.sqlite,跨兄弟仓库共用,里面有真实 GitHub 用户), | ||
| 16 | + // 每跑一次留两行、从不清理,攒到 356 行夹具。 | ||
| 17 | + const TEST_DB_PATH = `/tmp/test-users-${uid}.sqlite`; | ||
| 18 | + const DB_FILES = [TEST_DB_PATH, `${TEST_DB_PATH}-shm`, `${TEST_DB_PATH}-wal`]; | ||
| 14 | 19 | ||
| 15 | beforeAll(() => { | 20 | beforeAll(() => { |
| 16 | - // user-db uses hardcoded data/users.sqlite — just clear module cache | 21 | + for (const f of DB_FILES) { try { fs.unlinkSync(f); } catch {} } |
| 22 | + // 先设 env 再清 require.cache 再 require:DB_PATH 是模块加载时求值的常量, | ||
| 23 | + // 顺序错了就还是打到共享库上(而且测试照样全绿,看不出来) | ||
| 24 | + process.env.USERS_DB_PATH = TEST_DB_PATH; | ||
| 17 | Object.keys(require.cache).filter(k => k.includes('user-db')).forEach(k => delete require.cache[k]); | 25 | Object.keys(require.cache).filter(k => k.includes('user-db')).forEach(k => delete require.cache[k]); |
| 18 | userDb = require(path.resolve(__dirname, '../../services/user-db')); | 26 | userDb = require(path.resolve(__dirname, '../../services/user-db')); |
| 19 | }); | 27 | }); |
| 20 | 28 | ||
| 21 | afterAll(() => { | 29 | afterAll(() => { |
| 22 | - // Clean up test users (emails are unique per run via crypto UUID) | 30 | + for (const f of DB_FILES) { try { fs.unlinkSync(f); } catch {} } |
| 31 | + delete process.env.USERS_DB_PATH; | ||
| 32 | + Object.keys(require.cache).filter(k => k.includes('user-db')).forEach(k => delete require.cache[k]); | ||
| 33 | + }); | ||
| 34 | + | ||
| 35 | + it('写的是隔离库,不是共享的生产用户库', () => { | ||
| 36 | + // 这条是这个文件里唯一防止「隔离静默失效」的东西:DB_PATH 不导出, | ||
| 37 | + // 所以从「文件有没有被创建出来」这一侧验 —— 打到共享库时这个文件不会存在。 | ||
| 38 | + // 先做一次读操作把 getDb() 触发起来:库是懒创建的,光 require 还没落文件。 | ||
| 39 | + userDb.findByEmail('force-getdb@example.com'); | ||
| 40 | + expect(fs.existsSync(TEST_DB_PATH)).toBe(true); | ||
| 23 | }); | 41 | }); |
| 24 | 42 | ||
| 25 | describe('createUser + findByEmail', () => { | 43 | describe('createUser + findByEmail', () => { |