Files
OJ2/docs/specs/phase3-review-rereview.md
yuetsh cb6d42363b docs(阶段3): 修复波复评通过,安全 findings 收口
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>
2026-08-07 06:26:20 -06:00

12 KiB
Raw Blame History

阶段 3 修复复评

日期2026-08-07 复评范围commit 9c04b00..f548aef4 个提交),对照 docs/specs/phase3-fix-list.md 的 6 条 findings + 2 条收尾修复F4b、F5b。 方法:读 diff + 读源码 + 独立实跑(不复用实施者的证据,除非明确说明)。所有测试数据均已清理并核对还原。


逐条核验

F1 —— 匿名可读任意用户完整档案【Critical】→ ADDRESSED

apps/api/src/routes/account.ts:101optionalAuth 之后立即 if (!c.get("user")) return success(c, null),早于任何数据库查询。

独立实跑:

匿名 GET /api/profiles/e2etest -> 200 {"data":null}

services/profile.tsshowRealName 参数链路未动,登录看自己档案不受影响(未重复验证,逻辑未改,风险低)。 前端 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(注册写库常量 nullservices/profile.ts:29F1 已核实的独立开关)外,没有任何路由手写 { ..., 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:199const full = isAdminRole(user),不再以 row.submission.userId === user.id 放行。/submissions/:idrequireAuth,匿名根本到不了这里,不存在新的匿名向量。

F4b —— contestId 未脱敏 → ADDRESSED已用真实比赛提交独立验证(原报告只有代码审查,本次补上实跑)

submission.ts:214contestId: 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.tsjudgeServerToken()env 存在则用 env不存在则 randomBytes(32) + console.warndocker/compose.dev.yml${OJ2_JUDGE_TOKEN:?...} 缺失时 compose 直接报错退出(未重新实跑 compose读码 + 报告证据一致,逻辑简单,风险低)。

F5b —— 仓库根 .env 读不到 → ADDRESSED加载器安全

见下方「重点判断 3」独立做了三组隔离测试shell env 优先级、cwd .env 优先级、缺文件不崩、畸形行不崩),并额外做了一次端到端真实提交(问题 1002Python3确认判题机用当前 .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.tssampleUser() 是唯一入口,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)默认就是关的,不会重犯。唯一的隐患是「有人手写字面量绕过 sampleUserdiff 里已经全部清干净,长期靠代码评审维持(无法用类型系统强制,值得记一条范围外观察)。

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, ...)userc.get("user")! 取得,requireAuth 保证非空匿名到不了这行。这是「Regular User 专属限制」的合法写法(限制普通用户只看自己的,不限制教师/管理员),语义与 F3 的场景不同——F3 的路由是 optionalAuth,匿名 user 可能为 null未在 diff 内,非本次改动引入,仅作范围外观察记录,不计入 findings。

其余 adminType 使用点(classroom.tsaccount.tscontest.tsinArray/sql 过滤,services/contest.tsisContestAdmin)均为「构造 SQL 过滤条件」或「非 null 保证下的角色判断」,没有第二个「匿名反而权限更大」的实例。

3. F5b 的 .env 加载器是否安全?

安全,三点分别独立验证:

  • 真实 env 优先于仓库根 .envJUDGE_SERVER_TOKEN=from-shell-env bun -e '...' 得到 from-shell-env,不是 .env 里的值。
  • cwd 下的 .envBun 自动加载)优先于仓库根 .envbun --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 时静默跳过readFileSyncENOENTcatch {} 吞掉,不影响启动——这是生产场景(真实环境变量注入)的必经路径,逻辑上有覆盖(未在容器里额外验证,风险低,纯 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.tsthrottlingDefaults 逐字段相同。

挂点位置对照 OnlineJudge/submission/views/oj.py:68SubmissionAPI.postthrottling()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 等端点,均 200sampleUser() 各调用点(含 row ?? { id, username: "" } 的 null 分支、{ id: row.creatorId, username: row.creatorUsername } 的裸对象分支)未见运行时异常。
  • apps/web 零改动diff stat 确认F1/F4 对前端的潜在影响(data:null 分支、info.data 表格不渲染)均有既存代码兜底或本就是回归到旧后端行为,读码确认不炸。
  • F4b 的 contestId 收口用真实比赛提交实测通过,未见回归。

范围外观察(仅记录,不阻塞)

  1. apps/api/src/routes/flowchart.ts:98 有一处外观相似的 user.adminType === "Regular User" 判断,但路由挂 requireAuth,不构成 F3 类陷阱。建议后续如果这条路由改成 optionalAuth,需要一并检查。
  2. .env 解析器不支持 export KEY=value 语法(会把 export KEY 当整个键名),本仓库当前 .env/.env.example 不用这种写法,暂无影响。
  3. F2 的「默认关闭」防线目前只靠约定(大家都调用 sampleUser()),没有类型系统强制。长期看这是 admin 侧 45 个端点铺开前值得补一道 lint/测试的地方,但不属于本次 findings。
  4. 限流的 429 是新引入的 HTTP 语义(旧后端全走 200+error 信封),前端目前没有针对 429 的专门处理(报告里已自述,非新发现)。

结论

All findings addressed: Yes

F1 / F2 / F3 / F4 / F4b / F5 / F5b / F6 —— 8 项全部 ADDRESSED均有独立实跑或读码验证支持diff 范围内未发现新引入的破坏。