Files
soft_quay/docs/review/phase3-review.md
T
ilaandClaude Fable 5 09f56478d3
Harness governance / validate (push) Has been cancelled
Phase 0 build gate / verify (push) Has been cancelled
Add Phase 3 review cross-check ruling
Adjudicate Codex's review correction: concede three overstatements in
the original review (M3 loop not actually complete - InstallService has
no production assembly and T-401 launch is unbuilt; scope 'no security
defect' to T-302/T-303; O2 mischaracterized - ENOSPC during io.Copy is
misclassified as zip_corrupt with a broken error chain, escalate to P1;
O1 fix was flawed - recheck IsRunning explicitly instead of inferring
app_running from rename failure). Add finding: disk-full surfaces at
write/sync/close with three different codes, so the O2 fix must span the
whole write->sync->close sequence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-19 20:06:55 +08:00

18 KiB

Phase 3 审查(T-301 ~ T-303:下载队列 / 安装整合 / 失败处理)

审查范围:8dad40f(T-301 可恢复下载队列,早前已实现)、14589ab(T-302 安装流程整合)、ae3f64c(T-303 失败处理与磁盘预检)。 本轮深读重点:T-302/T-303 的安装整合与失败处理(core/installer/verified_package.go、core/application/install/service.go);T-301 队列此前已在架构文档层核过(可恢复、崩溃对账),本轮不重复深挖。 审查视角:全栈开发工程师 + 安全审计。 审查方式:静态代码审计 + 攻击路径推演。因 WSL 无 Go,未执行测试。 日期:2026-07-19

总体判断

Phase 3 质量高,安全正确,未发现安全缺陷。 安装整合把 Phase 1 的原型(验签清单、安全解压、原子切换)串成完整可信链,并且在两个最容易被忽略的点上做对了:先验哈希再解析 ZIP、全程复用同一 file 句柄防 verify-then-use TOCTOU。失败处理有完整的稳定错误码枚举。M3(清单→列表→下载→安装→启动)核心闭环成立。以下均为可优化观察项,非缺陷。

逐模块核验

T-302 · 安装整合(installer/verified_package + application/install/service)

包验证链顺序正确(verified_package.go,对应 api.md 安装验证顺序):

  1. expectation.validate():Catalog 期望校验(size>0、SHA-256 32 字节、app 字段合法)。
  2. openArchiveFile(zipPath, expectation.Size) → verifyPackageSHA256 在解析任何 ZIP 结构之前用 io.NewSectionReader 全量哈希并 subtle.ConstantTimeCompare;哈希后 Stat 复查 Size/IsRegular 防并发改动。
  3. scanOpenedArchive(中央目录预扫描)→ zip.NewReader → preflight(路径/上限/结构)→ readPackageAppManifest(app.json,限 1 MiB)→ manifest.matches(expectation.App)。
  4. beforeExtract(环境预检)→ extractPlan(安全解压)。

两个关键安全属性:

  • 哈希先于解析:恶意 ZIP 在 SHA-256 通过前碰不到 zip parser,消除"解析未验证字节"的攻击面。
  • 单句柄贯穿:同一个 file 句柄用于哈希 → 预扫描 → zip.NewReader → 解压,攻击者无法在"验证后、使用前"替换文件(verify-then-use TOCTOU)。这是很多实现漏掉的点。

app.json 严格且与 Catalog 交叉比对: validateManifestObject 手工拒绝未知/重复/缺失字段与尾随数据;DisallowUnknownFields 二次解码;validate() 固定 schema_version=1、channel=stable、data_policy/update_policy 定值、working_directory 经 safepath;matches() 要求 ID/版本/通道/min_os/架构/entrypoint/requires_admin 全部等于已签名 Catalog 期望。

VerifiedPackage 不含 ZIP 句柄或目标路径,预检回调拿不到原始 archive/destination,无法绕过安全解压——好的 API 边界。

每文件 SHA-256 在解压流中计算(io.MultiWriter(output, digest),无二次读),写入 installed-app.json Files,为后续 files.json 修复铺路。

T-302 编排(install/service.go)

  • 信任边界清晰:InstallRequest 明确"untrusted completed download + trusted Catalog selection";信任根是 Catalog 的 Size/SHA256。本地对 .download/元数据的篡改被 SHA-256 + 签名复检拦下。
  • 先恢复再安装:installer.Recover(appRoot) 在开始新安装前解决遗留事务,与 switcher 的 ErrRecoveryRequired 守卫一致。
  • 强制安全依赖:NewInstallService 拒绝 nil 的 Records/Health/DiskSpace/TargetState——"no caller can silently bypass the pre-extract safety boundary"。
  • record 写入置于 switcher 健康回调内:写失败与健康失败走同一回滚路径(recordWriteErr 追踪 → InstallStageRecord),避免"切换成功但记录半写"的中间态。

T-303 · 失败处理与磁盘预检(preExtractCheck + 错误码)

  • 磁盘预检:required = payloadBytes + 64 MiB reserve,available < required 或 < 0 拒绝;在解压目标创建前 fail-fast。
  • 运行检测:TargetState.IsRunning 在预检拒绝正在运行的目标(ErrTargetRunning);接口注释明确"never starts/waits/terminates a process",符合"不强杀"。
  • 稳定错误码枚举:hash_mismatch/zip_path_escape/zip_corrupt/package_invalid/disk_full/disk_check_failed/app_running/target_state_unavailable/install_failed,failureCodeFor 用 errors.Is 映射;UI 只本地化码、不显原始错误。与 api.md 错误码一致,满足"各情况有确定结果与错误码"。

需优化项(按优先级,均非安全缺陷)

O1 · 运行检测存在 TOCTOU,switch 前不复查

现状: IsRunning 只在 preExtractCheck(解压前)执行;switcher.Switch 在 current → backup rename 前不复查。解压可能耗时数秒,期间用户可能启动该软件。

影响: Windows 对运行中 EXE 的文件锁能否可靠阻止父目录 rename,并无明确保证(取决于镜像 section 锁语义),不应默认"Windows 一定会挡住"。两种劣化路径:① rename 失败 → 回滚(较干净,但错误码是通用 switch 失败而非 app_running);② rename 成功但 commit 阶段删 backup 因运行中 EXE 锁失败 → 新版已装但 backup 滞留 + 报错。均非数据丢失,但都不干净;非 Windows(测试)路径无锁,TOCTOU 更实。

建议: 在 switcher current→backup 紧邻前再查一次 IsRunning(把窗口从"整个解压期"收窄到"切换瞬间");并确保"因占用导致 rename 失败"映射到清晰的 app_running 诊断而非通用错误。TOCTOU 无法在无 OS 级锁下完全关闭,但可显著收窄。

O2 · 磁盘预检是建议性,真正保证来自解压失败清理

现状: AvailableBytes 在解压前查,但其他进程可在"查"与"写"之间占用磁盘。

影响: 中途 ENOSPC 由 extractor 的失败清理(删 staging)兜底,只是错误路径与预检的 disk_full 码不同。预检是 fail-fast UX,不是保证。

建议: 明确文档表述"预检为 fail-fast,真正的原子性由 extractor 失败清理保证";可选:把中途 ENOSPC 也归一到 disk_full 码,避免同一现象两种码。

O3 · 已完成下载文件的生命周期归属未在安装服务体现

现状: Install 不删除 request.DownloadPath;安装成功后 ZIP 由谁清理未在此层体现。

建议: 明确下载 ZIP 的清理归属(下载队列 or 专门 GC),避免 downloads/ 无限增长。属生命周期职责,非缺陷。

O4 · verified_package 的 stage 标签命名不一致

现状: ZIP preflight() 失败标记为 PackageStageVerify,而 PackageStagePreflight 用于 beforeExtract;normalizeEntrypoint 失败标 PackageStageManifest。

影响: 纯观测性命名,不影响安全或行为;诊断时 stage 语义略含糊。优先级低。

O5 · StagingDiskReserveBytes 固定 64 MiB

现状: 固定预留,不随包大小缩放。更新场景峰值磁盘为"旧 current + 新 staging 并存"(current→backup、staging→current 均为同卷 rename,无额外空间),预检只保证新 payload+reserve 的空间,假定旧 current 已在盘。

影响: 该假定对更新/首装都成立,heuristic 合理。仅提醒:若未来 backup 策略改变(非同卷 / 复制而非 rename),需重估预留模型。

结论

Phase 3 可作为启动/更新/授权阶段的可信安装基线。安装整合的安全属性(哈希先于解析、单句柄防 TOCTOU、app.json 与 Catalog 交叉比对、untrusted 下载 + Catalog 信任根、强制不可绕过的预检)都到位,失败码体系完整。建议处理顺序:

  1. [可靠性] O1:switch 前复查 IsRunning 收窄 TOCTOU,并把占用导致的 rename 失败映射到 app_running 码。
  2. [一致性] O2:统一中途 ENOSPC 与预检的 disk_full 码;文档写清预检为 fail-fast。
  3. [生命周期] O3:明确已完成下载 ZIP 的清理归属。
  4. [低优先] O4/O5:统一 package stage 命名;预留模型随 backup 策略复核。

声明:本报告为静态审计与攻击路径推演,逐项核了实现(含 verifyPackageSHA256 单句柄链、io.MultiWriter 每文件哈希、preExtractCheck 强制性、failureCodeFor 映射);因 WSL 无 Go 未执行测试,双 workspace 闸门通过为 Codex 自述。O1 的 Windows 文件锁语义需真机验证。

Codex 复核修正(2026-07-19)

本节保留上文 Claude Code 的静态审计结论,并根据当前代码、任务边界和路线图作出补充/修正;没有修改 T-301~T-303 的实现。复核时额外执行了 go -C core test -count=1 ./installer ./application/install,两个包均通过。

修正后的总体判断

上文对 T-302/T-303 的核心信任边界判断成立:已验签 Catalog selection、同一普通文件句柄上的 size/SHA-256、ZIP/app manifest 验证、安全 staging、switch/rollback 以及 staging 前环境预检的顺序正确,未发现从未验证下载字节到执行/切换的直接绕过路径。

但“M3(清单→列表→下载→安装→启动)核心闭环成立”不成立。路线图把 M3 定义为 Phase 2~4 前半的闭环,而 T-401 的 Toolhelp 进程检测、启动前检查、WorkingDirectory 与实际启动尚未落成;当前 NewInstallService/TargetStateChecker 只在测试中构造或调用,尚无命令层生产装配。因此准确表述应为:T-302/T-303 的无头 core 可信安装边界已完成;端到端安装编排和启动闭环尚未完成。

同理,“Phase 3 未发现安全缺陷”应限定为“本轮深审的 T-302/T-303 未发现新的信任边界绕过”。本报告明确未重新深审 T-301,不能据此替代对整个 Phase 3 的完整安全结论。

O1 · 接受,并转入 T-401 的切换临界区契约

TargetStateChecker.IsRunning 目前只在解压前执行;解压完成到 Switcher 的 current → backup 之间存在时间窗口,用户可在此期间启动旧版本。该观察成立,优先级应为 P1 可靠性整改。

处理方式不应把所有 rename 失败一概映射为 app_running:权限、杀毒软件、目录损坏等错误会产生错误归因。应在 current → backup 的紧邻前增加可注入的最后一次运行状态检查;只有该检查明确返回“仍在运行”时才返回 app_running。T-401 落成时应显式消费 T-303 的 TargetStateChecker 契约,并补充“解压期间启动旧版”的测试;Windows 文件锁/映像节语义仍需 T-601 真机/VM 验证。

O2 · 提升为 P1:中途磁盘写满会误报且丢失根因

上文正确指出磁盘预检只是 fail-fast 建议,不能消除“查询后被其他进程占满”的窗口;docs/api.md 已经写明预检不能替代写入、同步、切换和回滚阶段的 fail-closed I/O 处理。

但现状比“中途 ENOSPC 变成通用 switch 失败”更严重:core/installer/extractor.go 的 io.Copy 失败统一返回 ErrArchiveCorrupt,并把原始 copyErr 作为 %v 格式化文本而非 %w 错误链。若目标文件写入返回 ENOSPC,调用方会得到 zip_corrupt,且不能用 errors.Is 识别原始 I/O 根因;这与 T-303 对稳定错误码和可识别根因的目标不符。

整改应同时做到:区分 ZIP 输入/CRC 读取失败与 staging 输出写入失败、保留原始 error chain、把可识别的“磁盘已满”归一为 disk_full。跨 Windows/非 Windows 的磁盘满判定不得在 core 直接依赖 Windows API;应由明确的、可测试的接口或平台适配提供分类。os.RemoveAll(destinationRoot) 当前是忽略错误的尽力清理,旧版本安全来自尚未进入 switch,而不是清理本身保证原子性;清理失败的恢复语义也应在整改中明确。

O3 · 接受,但属于后续编排/生命周期任务

已完成 .download 的生命周期目前没有生产消费者:下载任务注释只说明 completed 文件“等待后续 workflow consume/remove”,InstallService 只接收候选路径且不应删除任意外部路径。应在正式安装编排中定义安全消费协议:仅安装成功后,由下载队列或以 request_id 派生路径的编排层删除完成文件和任务元数据;安装失败保留文件用于重试/诊断。该项不应通过让 InstallService 直接删除 DownloadPath 来解决。

O4 · 不建议作为独立整改项

PackageStageVerify 覆盖 ZIP 的可信性/安全验证,而 PackageStagePreflight 专指验证完成后的环境 hook;二者语义可区分,当前错误码也依赖根因而非 stage。命名略有认知成本,但不影响安全、行为或用户可见码。除非后续观测系统需要更细粒度指标,否则不应为此单独改动稳定 stage API。

O5 · 接受,维持为低优先级架构假设

当前 app root 内的 current → backup 与 staging → current 都是同卷 rename;旧 current 与完成下载包已占用的空间会反映在预检时的可用空间中,额外需求为新 payload 加 64 MiB 是合理的 v1 启发式。若未来改成跨卷复制、保留多份 backup 或改变下载/安装卷布局,再重新评估该模型。

建议的后续顺序

  1. 在下一次正式整改前,先落成任务规格,关闭 O2 的输出 I/O 错误保留与 disk_full 分类问题。
  2. 落成 T-401 时,把运行状态适配、启动协议和 O1 的切换临界区复查作为同一契约实现,并将 T-303 明确列为其依赖/接口前提。
  3. 在下载完成到安装的生产编排任务中,定义 O3 的 completed 文件消费和失败保留规则。

交叉复核裁定(定稿)

复核视角:Claude 全栈开发工程师,对 Codex 复核修正逐条核验后裁定。 核验方式:grep NewInstallService 生产调用者;读 extractor.go 第 203-235 行 io.Copy/fsync/close 的错误包装与码映射;读 06-tasks.md M3 定义;因 WSL 无 Go,未执行测试。 日期:2026-07-19

承认原审核错误

本轮 Codex 复核比原审核更准。原审核有两处 overstate 与一处有缺陷建议,应更正:

  • "M3 核心闭环成立" 撤回——原审核错。 NewInstallService 无任何生产调用者(仅定义/测试);M3 路线图定义含"启动(Phase 2-4 前半)",而 T-401(Toolhelp 进程检测 + 启动)未落成。准确表述:T-302/T-303 的无头 core 可信安装边界已完成;端到端编排与启动闭环未完成。
  • "Phase 3 未发现安全缺陷" 应限定为 T-302/T-303。 本轮未重新深审 T-301,不能替代整个 Phase 3 的安全结论。
  • 原 O2 定性错误(位置 + 严重性)。 ENOSPC 发生在解压阶段的 io.Copy 写,不是"通用 switch 失败";它被包成 ErrArchiveCorrupt + %v → 映射为 zip_corrupt(误导为"包坏了")且断了 error chain。这直接违反 T-303 "可识别根因"目标。O2 升 P1 成立。
  • 原 O1 建议有缺陷,收回。 "把占用导致的 rename 失败映射到 app_running" 会错误归因(权限/杀软/目录损坏也会致 rename 失败);正确做法是 switch 紧邻前显式复查 IsRunning,只有其返回运行才报 app_running。

已确认的事实(可作为后续任务前提)

项 结论 证据
InstallService 无生产装配 属实(M3 撤回) NewInstallService 仅 service.go:129 定义,无非测试调用者
M3 含"启动"且未完成 属实 06-tasks.md:114 M3=「…安装→启动」Phase 2-4 前半;T-401 未落成
ENOSPC 误分类为 zip_corrupt 属实(O2 升级) extractor.go:210 io.Copy 失败包 ErrArchiveCorrupt+%v;failureCodeFor → zip_corrupt;消息误标 "read"
磁盘满有三种落点三种码(本轮新增) 属实 write→zip_corrupt(210);syncFileWithFence→默认 install_failed(230-231);Close→默认 install_failed(235)。ENOSPC 常延迟到 fsync/close 暴露
RemoveAll 忽略错误 属实 失败清理 _ = os.RemoveAll(...);旧版本安全来自"尚未进 switch"而非清理原子性

采纳 Codex 的修正与 sharpen

  • 接受 M3 表述撤回、"无安全缺陷"限定到 T-302/T-303。
  • 接受 O1 精化:switch 紧邻前可注入的最后一次 IsRunning 复查;不把 rename 失败一概映射为 app_running;与启动协议一并在 T-401 落成,列 T-303 为接口前提;Windows 映像节/文件锁语义留 T-601 真机验证。
  • 接受 O2 升 P1:区分 ZIP 读/解压失败与 staging 写失败、保留原始 error chain(%w)、可识别"磁盘满"归一 disk_full;分类判定经接口/平台适配注入,不在 core 直接依赖 Windows API。
  • 接受 O3 精化(定义安全消费协议:仅成功后由下载队列/编排层按 request_id 派生路径删除,失败保留;不让 InstallService 删任意外部路径)、O4 降级(verify=可信性验证 / preflight=环境 hook,语义可区分,不改稳定 stage API)、O5 维持低优先。

新增(对 O2 的补强):磁盘满整改需覆盖 write→sync→close 全序列

O2 的修复不能只改 io.Copy 那一行。ENOSPC 在 Linux 常因延迟分配推迟到 write/fsync/close 任一处暴露,当前三处给出 zip_corrupt/install_failed/install_failed 三种码。整改需:

  • 用平台感知 ENOSPC 判定(POSIX syscall.ENOSPC / Windows ERROR_DISK_FULL,经注入接口,不进 core)在 write/sync/close 三处统一归一 disk_full。
  • 把 output 包一层"记录写错误的 writer"以区分写失败 vs 读/解压失败(io.Copy 本身不告知哪侧失败)。
  • 明确清理失败(RemoveAll 报错)的恢复语义。

最终处理顺序(定稿)

  1. [可靠性 · P1] O1:switch 紧邻前显式复查 IsRunning(收窄 TOCTOU),随 T-401 启动协议同契约落成,列 T-303 为前提;真机文件锁验证归 T-601。
  2. [错误可诊断性 · P1] O2:write→sync→close 全序列统一 ENOSPC→disk_full,保留 error chain,区分读/写失败,分类经平台适配注入;明确清理失败恢复语义。
  3. [生命周期] O3:正式安装编排定义 completed 下载文件的安全消费与失败保留协议。
  4. [低优先] O4/O5:stage 命名维持现状;预留模型随 backup 策略复核。
  5. [文档] 修正 current-state.md / 相关文档中 M3 表述:无头 core 安装边界已完成,端到端编排与启动闭环待 T-401。

裁定:本轮 Codex 复核成立且优于原审核——撤回 M3 闭环 overstate、纠正 O2 定性(位置+严重性)、收回 O1 有缺陷建议、限定"无安全缺陷"范围。安全边界总结论(T-302/T-303 无绕过)双方一致,不需回滚;新增"磁盘满三落点"补强并入处理顺序第 2 步。