From 35805d85168333349d3859d44b1500b32846d898 Mon Sep 17 00:00:00 2001 From: yibo Date: Mon, 17 Aug 2026 19:36:06 +0800 Subject: [PATCH] fix(tests): 三处「隔离是表演」的测试污染；清掉共享用户库里 358 行夹具 --- server/services/benchmark-db.js | 7 ++++++- server/services/user-db.js | 8 +++++++- server/tests/integration/api-routes.test.js | 2 +- server/tests/unit/skill-dir-manager.test.js | 2 +- server/tests/unit/test-isolation-env.test.js | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ server/tests/unit/user-db.test.js | 22 ++++++++++++++++++++-- 6 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 server/tests/unit/test-isolation-env.test.js diff --git a/server/services/benchmark-db.js b/server/services/benchmark-db.js index af668f1..3c2d5ed 100644 --- a/server/services/benchmark-db.js +++ b/server/services/benchmark-db.js @@ -11,7 +11,12 @@ const fs = require('fs'); // 数据目录 const DATA_DIR = path.resolve(__dirname, '../../../data'); -const DB_PATH = path.join(DATA_DIR, 'benchmark_logs.sqlite'); +// BENCHMARK_DB_PATH 覆盖口。**两个测试文件早就在设这个变量了** +// （benchmark-db.test.js:13 与 integration/api-routes.test.js:43），而这一行以前不读它 —— +// 于是它们的隔离是表演：数据全写进共享的生产 benchmark 库（攒了 342 行 skill_name=benchmark_test）， +// 而 afterAll 删的那个 /tmp 文件从来没被创建过，所以清理还“成功”了。 +// 设了没人读 = 「产生→传递→落库→读回」漏掉读回那一环，这是本项目最常见的缺陷形状。 +const DB_PATH = process.env.BENCHMARK_DB_PATH || path.join(DATA_DIR, 'benchmark_logs.sqlite'); let db = null; diff --git a/server/services/user-db.js b/server/services/user-db.js index ddbc28d..8bee01f 100644 --- a/server/services/user-db.js +++ b/server/services/user-db.js @@ -10,8 +10,14 @@ const path = require('path'); const fs = require('fs'); const crypto = require('crypto'); +// 注意这里是**三级**上跳：从 server/services/ 落到仓库外的 ~lemonAI/data， +// 与 skill-db / benchmark-db 一致 —— 用户、skill、benchmark 是跨兄弟仓库共享的平台数据。 +// 仓库内的 /data 放的是本仓库自己的租户数据（graph / companies / devloop）。 const DATA_DIR = path.resolve(__dirname, '../../../data'); -const DB_PATH = path.join(DATA_DIR, 'users.sqlite'); +// USERS_DB_PATH 覆盖口（口径同 graph-db.js 的 GRAPH_DB_PATH）。 +// 没有它之前，user-db.test.js 无法隔离，每跑一次就往**共享的生产用户库**里插两行； +// 那个库里 357 个用户有 356 个是这么攒出来的测试夹具（178 次运行），真人只有 1 个。 +const DB_PATH = process.env.USERS_DB_PATH || path.join(DATA_DIR, 'users.sqlite'); let db = null; diff --git a/server/tests/integration/api-routes.test.js b/server/tests/integration/api-routes.test.js index 9f4e7f3..7a3c580 100644 --- a/server/tests/integration/api-routes.test.js +++ b/server/tests/integration/api-routes.test.js @@ -42,7 +42,7 @@ describe('API Integration Tests', () => { process.env.SKILLS_DB_PATH = TEST_DB_PATH; process.env.BENCHMARK_DB_PATH = '/tmp/test-integration-bench.sqlite'; process.env.PORT = '0'; - process.env.SKILLS_DIR = TEST_SKILLS_DIR; + process.env.SKILLS_CODE_DIR = TEST_SKILLS_DIR; process.env.SKILLS_DATA_DIR = TEST_DATA_DIR; process.env.JWT_SECRET = 'test-secret-key-for-integration'; process.env.API_KEYS = 'test-api-key-123'; diff --git a/server/tests/unit/skill-dir-manager.test.js b/server/tests/unit/skill-dir-manager.test.js index 76d63a3..2bf92a1 100644 --- a/server/tests/unit/skill-dir-manager.test.js +++ b/server/tests/unit/skill-dir-manager.test.js @@ -16,7 +16,7 @@ describe('skill-dir-manager', () => { fs.mkdirSync(path.join(TEST_DATA_DIR, 'my-test-skill'), { recursive: true }); fs.writeFileSync(path.join(TEST_DATA_DIR, 'my-test-skill', 'SKILL.md'), '# Test'); - process.env.SKILLS_DIR = TEST_CODE_DIR; + process.env.SKILLS_CODE_DIR = TEST_CODE_DIR; process.env.SKILLS_DATA_DIR = TEST_DATA_DIR; delete require.cache[require.resolve('../../config')]; delete require.cache[require.resolve('../../services/skill-dir-manager')]; diff --git a/server/tests/unit/test-isolation-env.test.js b/server/tests/unit/test-isolation-env.test.js new file mode 100644 index 0000000..f2679ea --- /dev/null +++ b/server/tests/unit/test-isolation-env.test.js @@ -0,0 +1,138 @@ +/** + * Unit Tests — 防漂移闸：**测试用来隔离的路径 env，生产代码必须真的读它** + * + * 挡的是一类已经实际造成数据污染的缺陷，而且是最有欺骗性的那种形态： + * 测试在 beforeAll 里 `process.env.XXX_DB_PATH = '/tmp/...'`，以为自己隔离了， + * 而生产代码那一行压根不读这个变量 —— 于是数据全写进**共享的生产库**， + * 测试照样全绿，afterAll 删的那个 /tmp 文件从来没被创建过、所以清理还“成功”了。 + * + * 已经发生过三次： + * - `BENCHMARK_DB_PATH`：两个测试文件设它，benchmark-db.js 不读 → + * 共享 benchmark 库攒了 342 行 skill_name=benchmark_test + * - `USERS_DB_PATH`：根本没有这个覆盖口，user-db.test.js 只能直写共享用户库 → + * 357 个用户里 356 个是测试夹具（178 次运行），真人只有 1 个 + * - `SKILLS_DIR`：**变量名写错**，config.js 读的是 `SKILLS_CODE_DIR` → + * skillsCodeDir 一直指向那个 2.7 万条的真实共享 skills 目录 + * + * 这就是本项目反复出现的「产生 → 传递 → 落库 → 读回」漏掉一环： + * 测试把值**产生**出来了，没有人**读回**。而它的隐蔽性在于—— + * 缺了读回那一环，测试不会红，只有共享数据在悄悄变脏。 + * + * ── 规则 ──────────────────────────────────────────────────────── + * 只管**路径类**变量（名字以 _DIR / _PATH / _ROOT 结尾）：那是会造成 + * 静默数据污染的地方。`HOME`（由 os.homedir() 间接读）、`JWT_SECRET`、`PORT` + * 这类非路径变量天然不在范围内，不需要维护白名单 —— 白名单会随时间腐烂， + * 而后缀规则不会。 + * + * ponytail: 静态正则扫源码，不 require 任何模块（require 会拉起 sqlite 连接）。 + * 上限：只认 `process.env.X` 这种字面写法，动态取（`process.env[name]`）查不出来； + * 也不检查测试是否**在 require 之前**设 env（顺序错了同样静默失效，那一环靠各文件 + * 自己的「写的是隔离库」断言兜——见 user-db.test.js）。 + */ +import { describe, it, expect } from 'vitest'; +import fs from 'fs'; +import path from 'path'; + +const ROOT = path.resolve(__dirname, '../../..'); +const PATH_ENV = /^[A-Z0-9_]*(_DIR|_PATH|_ROOT)$/; + +function walk(dir, out = []) { + if (!fs.existsSync(dir)) return out; + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { + if (e.name === 'node_modules') continue; + const p = path.join(dir, e.name); + if (e.isDirectory()) walk(p, out); + else if (p.endsWith('.js')) out.push(p); + } + return out; +} + +/** + * 剥掉注释再扫。 + * + * 不剥的话本文件的文档注释里那句举例（`process.env.XXX_DB_PATH = ...`）会被当成真的赋值， + * guard 自己把自己判红 —— 实测踩过。凡是「静态扫源码」的检查都要先想清楚 + * **该量的是代码还是文本**：这里该量的是会执行的代码。 + */ +function stripComments(src) { + return src + .replace(/\/\*[\s\S]*?\*\//g, '') + .split('\n') + .map((l) => l.replace(/\/\/.*$/, '')) + .join('\n'); +} + +/** 测试里被**赋值**的 env 名 → 哪些文件在设它 */ +function assignedInTests() { + const found = new Map(); + for (const f of walk(path.join(ROOT, 'server/tests'))) { + const src = stripComments(fs.readFileSync(f, 'utf-8')); + for (const m of src.matchAll(/process\.env\.([A-Z0-9_]+)\s*=(?!=)/g)) { + if (!found.has(m[1])) found.set(m[1], new Set()); + found.get(m[1]).add(path.relative(ROOT, f)); + } + } + return found; +} + +/** 生产代码里被**读取**的 env 名 */ +function readInProduction() { + const dirs = ['server/services', 'server/src', 'server/routes', 'server/middleware', 'packages']; + const files = dirs.flatMap((d) => walk(path.join(ROOT, d))); + files.push(path.join(ROOT, 'server/config.js'), path.join(ROOT, 'server/index.js')); + + const found = new Set(); + for (const f of files) { + if (!fs.existsSync(f)) continue; + // 生产侧同样剥注释：注释里提到一个变量名不等于代码读了它 + // （这一侧漏剥的方向更坏 —— 会把「其实没人读」误判成「有人读」，guard 变空跑） + const src = stripComments(fs.readFileSync(f, 'utf-8')); + for (const m of src.matchAll(/process\.env\.([A-Z0-9_]+)/g)) { + // 生产代码里自己赋值不算「读」（几乎没有，但别让它自证） + const after = src.slice(m.index + m[0].length, m.index + m[0].length + 3); + if (/^\s*=(?!=)/.test(after)) continue; + found.add(m[1]); + } + } + return found; +} + +describe('测试隔离：路径类 env 必须有生产读者', () => { + const assigned = assignedInTests(); + const read = readInProduction(); + + it('两侧都不是空的（防止 guard 因为路径/正则失效而静默空跑）', () => { + expect(assigned.size).toBeGreaterThan(3); + expect(read.size).toBeGreaterThan(3); + // 已知一定存在的两个，任一消失说明扫描口径坏了 + expect([...assigned.keys()]).toContain('GRAPH_DB_PATH'); + expect(read.has('GRAPH_DB_PATH')).toBe(true); + }); + + it('测试用来隔离的每个 _DIR/_PATH/_ROOT，生产代码里都有人读', () => { + const orphans = [...assigned] + .filter(([name]) => PATH_ENV.test(name)) + .filter(([name]) => !read.has(name)) + .map(([name, files]) => `${name} —— 设于 ${[...files].join(', ')}，但生产代码零处读取`); + + // 报出全部而不是第一个：一次修完比来回跑三轮省事 + expect(orphans).toEqual([]); + }); + + it('共享数据目录在仓库外，这一点是有意的（顺带钉住，免得被“修”回仓库内）', () => { + // user-db / skill-db / benchmark-db 用三级上跳落到 ~lemonAI/data（跨兄弟仓库共享）； + // graph-db / company-service 用两级落到 /data（本仓库租户数据）。 + // 深度随文件所在目录变化，改动时容易改错方向，所以这里钉住两组的实际去向。 + const shared = ['server/services/user-db.js', 'server/services/skill-db.js', 'server/services/benchmark-db.js']; + const local = ['server/services/graph-db.js', 'server/services/company-service.js']; + + for (const rel of shared) { + const resolved = path.resolve(path.dirname(path.join(ROOT, rel)), '../../../data'); + expect(resolved).toBe(path.resolve(ROOT, '../data')); + } + for (const rel of local) { + const resolved = path.resolve(path.dirname(path.join(ROOT, rel)), '../../data'); + expect(resolved).toBe(path.join(ROOT, 'data')); + } + }); +}); diff --git a/server/tests/unit/user-db.test.js b/server/tests/unit/user-db.test.js index 682049f..c98a86b 100644 --- a/server/tests/unit/user-db.test.js +++ b/server/tests/unit/user-db.test.js @@ -11,15 +11,33 @@ describe('user-db', () => { const uid = crypto.randomUUID().substring(0, 8); const uniqueEmail = `test-${uid}@example.com`; const oauthEmail = `gh-${uid}@example.com`; + // 这个库跑完就删。以前没有覆盖口，这里写的是**共享的生产用户库** + // （~lemonAI/data/users.sqlite，跨兄弟仓库共用，里面有真实 GitHub 用户）， + // 每跑一次留两行、从不清理，攒到 356 行夹具。 + const TEST_DB_PATH = `/tmp/test-users-${uid}.sqlite`; + const DB_FILES = [TEST_DB_PATH, `${TEST_DB_PATH}-shm`, `${TEST_DB_PATH}-wal`]; beforeAll(() => { - // user-db uses hardcoded data/users.sqlite — just clear module cache + for (const f of DB_FILES) { try { fs.unlinkSync(f); } catch {} } + // 先设 env 再清 require.cache 再 require：DB_PATH 是模块加载时求值的常量， + // 顺序错了就还是打到共享库上（而且测试照样全绿，看不出来） + process.env.USERS_DB_PATH = TEST_DB_PATH; Object.keys(require.cache).filter(k => k.includes('user-db')).forEach(k => delete require.cache[k]); userDb = require(path.resolve(__dirname, '../../services/user-db')); }); afterAll(() => { - // Clean up test users (emails are unique per run via crypto UUID) + for (const f of DB_FILES) { try { fs.unlinkSync(f); } catch {} } + delete process.env.USERS_DB_PATH; + Object.keys(require.cache).filter(k => k.includes('user-db')).forEach(k => delete require.cache[k]); + }); + + it('写的是隔离库，不是共享的生产用户库', () => { + // 这条是这个文件里唯一防止「隔离静默失效」的东西：DB_PATH 不导出， + // 所以从「文件有没有被创建出来」这一侧验 —— 打到共享库时这个文件不会存在。 + // 先做一次读操作把 getDb() 触发起来：库是懒创建的，光 require 还没落文件。 + userDb.findByEmail('force-getdb@example.com'); + expect(fs.existsSync(TEST_DB_PATH)).toBe(true); }); describe('createUser + findByEmail', () => { -- libgit2 0.21.4