From 46c780abadd700b8c22b496dc8e283543004c277 Mon Sep 17 00:00:00 2001 From: yibo Date: Mon, 17 Aug 2026 19:57:23 +0800 Subject: [PATCH] fix(security): api-key 鉴权把原始用户行挂上 req.user；补 requireApiKey 的 4 条分支 --- server/middleware/api-key-auth.js | 8 +++++++- server/tests/integration/api-routes.test.js | 1 - server/tests/unit/api-key-auth.test.js | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 3 deletions(-) diff --git a/server/middleware/api-key-auth.js b/server/middleware/api-key-auth.js index e6c95b8..a37f308 100644 --- a/server/middleware/api-key-auth.js +++ b/server/middleware/api-key-auth.js @@ -47,7 +47,13 @@ function requireApiKey(req, res, next) { try { const userDb = require('../services/user-db'); - validUser = userDb.findByApiKey(key); + const row = userDb.findByApiKey(key); + // 过 toPublicUser 再挂上去：findByApiKey 返回的是**原始数据库行**，带 password_hash。 + // JWT 那条路（jwt-auth.extractUser）一直是脱敏后才挂的，这里以前不是 —— + // 两条鉴权路径行为不一致，而 req.user 是 handler 唯一的身份来源： + // 将来任何一个 res.json({ user: req.user }) 都会把密码哈希发给外部 API 调用方。 + // 目前所有 handler 只读 req.user.id / .email，还没真漏，但这个口子不该留着。 + validUser = row ? userDb.toPublicUser(row) : null; } catch (err) { console.error('[OpenAPI] DB check failed for API key:', err); } diff --git a/server/tests/integration/api-routes.test.js b/server/tests/integration/api-routes.test.js index 7a3c580..aa0aee9 100644 --- a/server/tests/integration/api-routes.test.js +++ b/server/tests/integration/api-routes.test.js @@ -45,7 +45,6 @@ describe('API Integration Tests', () => { 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'; // Clear module cache for clean state Object.keys(require.cache).filter(k => diff --git a/server/tests/unit/api-key-auth.test.js b/server/tests/unit/api-key-auth.test.js index 88e2a56..e543337 100644 --- a/server/tests/unit/api-key-auth.test.js +++ b/server/tests/unit/api-key-auth.test.js @@ -1,7 +1,7 @@ /** * Unit Tests — middleware/api-key-auth.js */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; const { extractApiKey } = require('../../middleware/api-key-auth'); @@ -50,3 +50,121 @@ describe('api-key-auth', () => { }); }); }); + +/** + * requireApiKey —— **做鉴权决定的那一半**。 + * + * 上面那组 7 条全在测 extractApiKey（怎么从 header/query 里取出字符串）， + * 而真正决定放行还是 401 的 requireApiKey 此前**一条用例都没有**。 + * 后果不是假设：`OPEN_API_KEYS` 那条分支从来没被执行过，于是 + * integration 测试里把变量名写成 `API_KEYS`（生产读的是 `OPEN_API_KEYS`） + * 存活了很久也没人发现 —— 没有测试执行过的路径，喂给它什么名字都不会有人知道。 + * + * 四条分支各一条，覆盖「拦」和「放」两侧： + * 无 key → 401 / key 无效 → 401 / 命中系统 key 列表 → 放行 / 命中用户 key → 放行且挂 req.user + * + * 隔离两处，缺任一条这组用例就会打到真实环境： + * - `OPEN_API_KEYS`：config.js 在模块加载时求值，所以要先设 env 再清 require.cache + * （config 与本中间件都要清 —— 中间件在顶层 require config） + * - `USERS_DB_PATH`：requireApiKey 内部**惰性** require user-db 来查用户 key。 + * 这个覆盖口是刚补上的；在它存在之前，这组用例会往**共享的生产用户库**里 + * 插一个测试用户（那个库正是这么攒到 356 行夹具的）。 + */ +describe('requireApiKey — 鉴权判定（此前零覆盖）', () => { + const path = require('path'); + const fs = require('fs'); + + const SYSTEM_KEY = 'sk-system-test-key'; + const TEST_DB_PATH = `/tmp/test-apikey-users-${process.pid}.sqlite`; + const DB_FILES = [TEST_DB_PATH, `${TEST_DB_PATH}-shm`, `${TEST_DB_PATH}-wal`]; + const MODULES = ['../../config', '../../middleware/api-key-auth', '../../services/user-db']; + + let requireApiKey; + let userKey; + let prevOpenApiKeys; + + beforeAll(() => { + for (const f of DB_FILES) { try { fs.unlinkSync(f); } catch {} } + prevOpenApiKeys = process.env.OPEN_API_KEYS; + process.env.OPEN_API_KEYS = `${SYSTEM_KEY}, sk-another-one`; // 带空格：config 会 trim + process.env.USERS_DB_PATH = TEST_DB_PATH; + for (const m of MODULES) delete require.cache[require.resolve(m)]; + + const userDb = require('../../services/user-db'); + const user = userDb.createUser({ + email: `apikey-${process.pid}@example.com`, + passwordHash: 'x', + displayName: 'API Key User', + provider: 'local', + }); + userKey = user.api_key; // createUser 自己生成 sk-user- + expect(userKey).toMatch(/^sk-user-/); + + ({ requireApiKey } = require('../../middleware/api-key-auth')); + }); + + afterAll(() => { + for (const f of DB_FILES) { try { fs.unlinkSync(f); } catch {} } + if (prevOpenApiKeys === undefined) delete process.env.OPEN_API_KEYS; + else process.env.OPEN_API_KEYS = prevOpenApiKeys; + delete process.env.USERS_DB_PATH; + for (const m of MODULES) delete require.cache[require.resolve(m)]; + }); + + /** 只记录结果，不真起 express */ + function run(headers) { + const req = { headers, query: {} }; + const res = { + statusCode: null, + body: null, + status(code) { this.statusCode = code; return this; }, + json(payload) { this.body = payload; return this; }, + }; + let passed = false; + requireApiKey(req, res, () => { passed = true; }); + return { req, res, passed }; + } + + it('隔离生效：用的是临时用户库，不是共享的生产库', () => { + expect(fs.existsSync(TEST_DB_PATH)).toBe(true); + }); + + it('① 完全不带 key → 401「需要 key」，且不调 next', () => { + const { res, passed } = run({}); + expect(passed).toBe(false); + expect(res.statusCode).toBe(401); + expect(res.body.error).toBe('Unauthorized'); + // 必须断言到**文案**而不是只断言 401：去掉这道门之后，key=null 会落到 + // 下面那道「key 无效」的门，照样回 401 —— 只看状态码的话这个变异测不出来 + // （实测过：把 if (!key) 改成 if (false)，只断言 401 的版本全绿）。 + // 而两者要分得开：一个是「你忘带 key」，一个是「你的 key 不对」，排查时全靠它。 + expect(res.body.message).toMatch(/API Key is required/); + }); + + it('② key 既不在系统列表也不属于任何用户 → 401', () => { + const { res, passed } = run({ 'x-api-key': 'sk-not-a-real-key' }); + expect(passed).toBe(false); + expect(res.statusCode).toBe(401); + // 与「没带 key」是两种不同的失败，文案要分得开（排查时全靠这个） + expect(res.body.message).toMatch(/Invalid API Key/); + }); + + it('③ 命中 OPEN_API_KEYS 系统列表 → 放行，挂 req.apiKey，不挂 req.user', () => { + // 这条就是那条从来没被执行过的分支：它存在的话，把变量名写成 API_KEYS 会当场红 + const { req, res, passed } = run({ authorization: `Bearer ${SYSTEM_KEY}` }); + expect(passed).toBe(true); + expect(res.statusCode).toBeNull(); + expect(req.apiKey).toBe(SYSTEM_KEY); + expect(req.user).toBeUndefined(); // 系统 key 不代表任何用户，不该凭空造一个身份 + }); + + it('④ 命中库里某个用户的 api_key → 放行并把该用户挂上 req.user', () => { + const { req, passed } = run({ 'x-api-key': userKey }); + expect(passed).toBe(true); + expect(req.apiKey).toBe(userKey); + expect(req.user).toBeTruthy(); + expect(req.user.email).toBe(`apikey-${process.pid}@example.com`); + // 走的是 toPublicUser，密码哈希不该跟着挂到请求上 + expect(req.user).not.toHaveProperty('password_hash'); + }); +}); -- libgit2 0.21.4