8 条(F1-F6 + 收尾 F4b/F5b)全部 ADDRESSED,修复 diff 内无新引入破坏。
复评补上了控制方没验充分的 F4b:真造了一条比赛提交,确认数据库存真值
而 API 返回给本人的是 info:{} / ip:null / contestId:null。
另核实 sampleUser() 是真正的默认关闭开关(覆盖全部 14 个下发点)、
isRegularUser 全仓确实只有一个调用点、.env 加载器优先级正确、
限流参数与旧后端 options/options.py 逐值吻合。
评审产物从 gitignore 的 .superpowers/sdd/ 归档进 docs/specs/ 以免丢失。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
12 KiB
阶段 3 修复复评
日期:2026-08-07
复评范围:commit 9c04b00..f548aef(4 个提交),对照 docs/specs/phase3-fix-list.md 的 6 条 findings + 2 条收尾修复(F4b、F5b)。
方法:读 diff + 读源码 + 独立实跑(不复用实施者的证据,除非明确说明)。所有测试数据均已清理并核对还原。
逐条核验
F1 —— 匿名可读任意用户完整档案【Critical】→ ADDRESSED
apps/api/src/routes/account.ts:101 在 optionalAuth 之后立即 if (!c.get("user")) return success(c, null),早于任何数据库查询。
独立实跑:
匿名 GET /api/profiles/e2etest -> 200 {"data":null}
services/profile.ts 的 showRealName 参数链路未动,登录看自己档案不受影响(未重复验证,逻辑未改,风险低)。
前端 apps/web/src/shared/api.ts:28 已有 data === null 分支,兼容确认(读码确认,未跑前端)。
F2 —— 学生真名无条件下发【Critical】→ ADDRESSED(默认关闭开关,非逐处删字段)
见下方「重点判断 1」,结论:14 个下发点全部经过统一的 sampleUser(),默认 realName: null,唯一显式打开的是 contest.ts:209(比赛榜单,且区分 admin/非 admin)。
独立实跑(与 diff 里的证据不同的端点,避免只是复读实施者的话):
匿名 GET /api/rankings/users?limit=2 -> [{"username":"student","realName":null},{"username":"e2etest","realName":null}]
匿名 GET /api/problems?limit=2 createdBy -> {"id":1,"username":"devadmin","realName":null} (x2)
匿名 GET /api/problems/1002 createdBy -> {"id":1,"username":"devadmin","realName":null}
grep -rn "realName" apps/api/src --include=*.ts 复查:除 sampleUser 内部实现、db.select() 取列、account.ts:94(注册写库常量 null)、services/profile.ts:29(F1 已核实的独立开关)外,没有任何路由手写 { ..., realName } 对象绕过 sampleUser()。
F3 —— 匿名绕过提交可见性守卫【Critical】→ ADDRESSED
submission.ts:227:!(await getBooleanOption(...)) && !isAdminRole(user),isAdminRole(null) 返回 false,!false = true,匿名正确落入受限分支。
独立实跑(临时把 submission_list_show_all 写为 false,测后 DELETE 该行,确认恢复为 []):
匿名 GET /api/submissions -> total = 0
登录学生 GET /api/submissions -> total = 0
两者一致地受限,F3 描述的「匿名权限大于登录用户」的错位已消除。
F4 —— 提交详情返回 info/ip【Important】→ ADDRESSED
submission.ts:199:const full = isAdminRole(user),不再以 row.submission.userId === user.id 放行。/submissions/:id 挂 requireAuth,匿名根本到不了这里,不存在新的匿名向量。
F4b —— contestId 未脱敏 → ADDRESSED,已用真实比赛提交独立验证(原报告只有代码审查,本次补上实跑)
submission.ts:214:contestId: full ? row.submission.contestId : null。
独立实跑(造了一场临时比赛 + 一条挂在该比赛下的真实提交,验证后已删除,见下方清理记录):
DB 实际存储:{"contestId":7,"ip":"9.9.9.9"}
提交所有者 (Regular User) GET /api/submissions/<id> ->
{"info":{},"ip":null,"contestId":null, ...} ← 三个字段全部脱敏
contestId 确实是被脱敏成 null,而不是碰巧本来就是 null(数据库里明确存的是 7)。控制方文档里点名"没验充分"的这一条已经补齐。
F5 —— 判题机 token 弱默认值【Important】→ ADDRESSED
config.ts 的 judgeServerToken():env 存在则用 env;不存在则 randomBytes(32) + console.warn。docker/compose.dev.yml 的 ${OJ2_JUDGE_TOKEN:?...} 缺失时 compose 直接报错退出(未重新实跑 compose,读码 + 报告证据一致,逻辑简单,风险低)。
F5b —— 仓库根 .env 读不到 → ADDRESSED,加载器安全
见下方「重点判断 3」,独立做了三组隔离测试(shell env 优先级、cwd .env 优先级、缺文件不崩、畸形行不崩),并额外做了一次端到端真实提交(问题 1002,Python3),确认判题机用当前 .env 里的 token 正常认证、提交离开 PENDING/JUDGING(拿到 result=-2,非 PENDING/JUDGING,证明判题机-后端握手成功)。测试提交与计数器已回滚,见清理记录。
F6 —— 提交接口缺限流【Important】→ ADDRESSED,参数与旧后端逐字对齐(独立验证)
见下方「重点判断 4」。独立绕过 HTTP 层直接调用 consumeToken() 12 次(避免污染 submission 计数):
#1~#10 allowed:true
#11 allowed:false, wait≈33.32s
#12 allowed:false, wait≈33.32s
wait = (1 - 0) / 0.03 ≈ 33.33,与旧后端 fill_rate=0.03 精确吻合,default_capacity=10 也吻合(第 11 次才被挡)。
四个重点判断
1. F2 是否真的做成了「默认关闭的开关」?
是。 apps/api/src/routes/helpers.ts 的 sampleUser() 是唯一入口,options.includeRealName === true 才下发真名,默认 false。核对了全仓 14 个下发点(比 findings 文档的 13 处多一个,多出的是 problem.ts:333,原先就写死 null,现在统一走 sampleUser 入口,不影响结论):
| 下发点 | 状态 |
|---|---|
| rankings/users、announcements(x2)、messages、tutorials/:id、problems 列表(x2)、problem-sets(x2)、problemset progress、contest creator(x2)、contest problems(x2) | 全部 sampleUser(user, realName),默认关 |
contest.ts:209(比赛榜单) |
唯一 { includeRealName: admin },对齐旧后端 contest/serializers.py:84 |
grep -rn "realName:" apps/api/src/routes/*.ts 复查无遗漏(本次独立复跑,非照抄报告)。不是逐处删字段——sampleUser() 是统一的序列化函数,下次新增端点如果照抄现有写法(调用 sampleUser)默认就是关的,不会重犯。唯一的隐患是「有人手写字面量绕过 sampleUser」,diff 里已经全部清干净,长期靠代码评审维持(无法用类型系统强制,值得记一条范围外观察)。
2. isRegularUser 是否真的只有一处调用?删除后是否有其它同类空值陷阱?
只有一处,属实。 grep -rn "isRegularUser" --include=*.ts .(排除 node_modules,覆盖整个仓库而非只有 apps/、packages/)只命中 helpers.ts 里的警示注释,无任何遗留调用点。
搜了其它同类模式(对 null 用户取 adminType 做权限判断),命中 apps/api/src/routes/flowchart.ts:98:
if (c.req.query("myself") === "1" || (!username && user.adminType === "Regular User")) ...
这处不是同类陷阱:该路由挂在 flowchartRoutes.get("/flowcharts", requireAuth, ...),user 由 c.get("user")! 取得,requireAuth 保证非空,匿名到不了这行。这是「Regular User 专属限制」的合法写法(限制普通用户只看自己的,不限制教师/管理员),语义与 F3 的场景不同——F3 的路由是 optionalAuth,匿名 user 可能为 null。未在 diff 内,非本次改动引入,仅作范围外观察记录,不计入 findings。
其余 adminType 使用点(classroom.ts、account.ts、contest.ts 的 inArray/sql 过滤,services/contest.ts 的 isContestAdmin)均为「构造 SQL 过滤条件」或「非 null 保证下的角色判断」,没有第二个「匿名反而权限更大」的实例。
3. F5b 的 .env 加载器是否安全?
安全,三点分别独立验证:
- 真实 env 优先于仓库根
.env:JUDGE_SERVER_TOKEN=from-shell-env bun -e '...'得到from-shell-env,不是.env里的值。 - cwd 下的
.env(Bun 自动加载)优先于仓库根.env:bun --env-file=.env.test-cwd模拟 cwd 优先加载后,loadRepoRootEnv()因为process.env[key] !== undefined而跳过,结果仍是 cwd 的值。 - 解析不会被畸形行搞崩:构造了空值(
=novalue)、无等号行、前导空格键、单/双引号、值里带等号、export FOO=bar语法、行内#注释、CRLF 结尾等混合样本喂给等价解析逻辑,全部正常跳过或按字面处理,无异常抛出。(export FOO=bar会被解析成键名"export FOO",即该变量实际上不会被正确加载——是一个小的解析局限,不是崩溃,本仓库的.env/.env.example都不用这种写法,记为范围外观察。) - 根目录无
.env时静默跳过:readFileSync抛ENOENT,catch {}吞掉,不影响启动——这是生产场景(真实环境变量注入)的必经路径,逻辑上有覆盖(未在容器里额外验证,风险低,纯 try/catch 结构)。 - 端到端:仓库根
.env当前有真实JUDGE_SERVER_TOKEN,直接提交一条真实代码(问题 1002/Python3),判题机在数秒内返回非 PENDING/JUDGING 结果,证明 token 握手成功、判题闭环工作,不是只停留在配置读取层面。
4. 限流参数是否真的对齐旧后端?
是,非拍脑袋。 对照 OnlineJudge/options/options.py:120:
throttling = {"ip": {"capacity": 100, "fill_rate": 0.1, "default_capacity": 50},
"user": {"capacity": 20, "fill_rate": 0.03, "default_capacity": 10}}
与 apps/api/src/services/throttling.ts 的 throttlingDefaults 逐字段相同。
挂点位置对照 OnlineJudge/submission/views/oj.py:68(SubmissionAPI.post):throttling() 在 check_contest_permission(比赛权限校验)之后、取 Problem 之前调用——apps/api/src/routes/submission.ts 的挂点(比赛权限校验后、db.select(schema.problem) 前)位置一致。
独立数值验证:绕开 HTTP 直接调用 consumeToken() 12 次,第 11 次起被拒,wait≈33.32s,与 (1 token 缺口) / (fill_rate=0.03) ≈ 33.33s 吻合,default_capacity=10 与观察到的"第 11 次才拒绝"一致。参数和算法都对得上,不是抄了个数字但算法跑偏。
(Lua 脚本把旧实现「非线程安全」的读改写做成了原子操作,这是双方都认可的合理增强,不是风险点。)
修复 diff 内新引入的破坏
无。
bunx tsc --noEmit独立重跑,0 错误。- 独立冒烟测试
/api/contests、/api/problems、/api/problem-sets、/api/rankings/users、/api/announcements、/api/submissions、/api/problems/1002等端点,均 200,sampleUser()各调用点(含row ?? { id, username: "" }的 null 分支、{ id: row.creatorId, username: row.creatorUsername }的裸对象分支)未见运行时异常。 apps/web零改动(diff stat 确认),F1/F4 对前端的潜在影响(data:null分支、info.data表格不渲染)均有既存代码兜底或本就是回归到旧后端行为,读码确认不炸。- F4b 的
contestId收口用真实比赛提交实测通过,未见回归。
范围外观察(仅记录,不阻塞)
apps/api/src/routes/flowchart.ts:98有一处外观相似的user.adminType === "Regular User"判断,但路由挂requireAuth,不构成 F3 类陷阱。建议后续如果这条路由改成optionalAuth,需要一并检查。.env解析器不支持export KEY=value语法(会把export KEY当整个键名),本仓库当前.env/.env.example不用这种写法,暂无影响。- F2 的「默认关闭」防线目前只靠约定(大家都调用
sampleUser()),没有类型系统强制。长期看这是 admin 侧 45 个端点铺开前值得补一道 lint/测试的地方,但不属于本次 findings。 - 限流的 429 是新引入的 HTTP 语义(旧后端全走 200+error 信封),前端目前没有针对 429 的专门处理(报告里已自述,非新发现)。
结论
All findings addressed: Yes
F1 / F2 / F3 / F4 / F4b / F5 / F5b / F6 —— 8 项全部 ADDRESSED,均有独立实跑或读码验证支持,diff 范围内未发现新引入的破坏。