[bug] scripts/release/pack.ts: --out 传空字符串会递归删除整个仓库目录

[Johnixr]

概述

scripts/release/pack.ts 对 --out 参数解析后没有空值/根路径守卫，rmSync(destination, { recursive: true, force: true }) 会直接递归删除仓库根目录。

位置

scripts/release/pack.ts:47-51（基于 master 47f943859）

const destination = resolve(root, values.out ?? DEFAULT_OUTPUT)
// ...
rmSync(destination, { recursive: true, force: true })
mkdirSync(destination, { recursive: true })

复现

Node 的 path.resolve(root, '') 返回值与 resolve(root, '.') 相同（即 root 本身），而 parseArgs 接受 --out ''：

tsx scripts/release/pack.ts --family dsh --out ''
# → rmSync(仓库根) 递归删除整个 checkout，包括未提交改动

CI 中 --out "$RELEASE_DIR" 在 RELEASE_DIR 为空环境变量时同样触发。

对比：仓库内其他破坏性脚本都有守卫，唯独此处缺失

• scripts/clean.ts：删除前断言目标为后代路径，并解析祖先符号链接防止逃逸
• scripts/release/verify-built-package-invariants.mjs：只删除自己 mkdtemp 出来的目录

建议修复

rmSync 之前拒绝 destination === resolve(root, '.')（以及空 values.out），守卫风格与 clean.ts 对齐即可，一行修复。

影响

数据丢失（本地未提交改动、CI 工作区），后果不可逆。触发前提是操作员失误或空环境变量，但 release 脚本正是最容易出现这类输入的地方。

---

本问题由多轮 AI 辅助代码审查发现，并经两个独立外部模型（Claude Opus 4.8 / GPT-5.6，均 high reasoning）交叉读码复核，三方一致确认。

[moonquake2004]

Confirmed against master's scripts/release/pack.ts: destination = resolve(root, values.out ?? DEFAULT_OUTPUT) — --out "" passes the empty string through (only undefined hits the default), and path.resolve(root, "") resolves to the repo root, so rmSync(destination, { recursive: true, force: true }) deletes the entire repository. High severity confirmed. Minimal fix: values.out?.trim() || DEFAULT_OUTPUT, or validate the resolved path is inside a known output root before rmSync.

[yha9806]

我基于当前 master 47f9438 完成了一个可直接审阅或 cherry-pick 的修复：

• Branch: https://github.com/yha9806/deepseek-harness/tree/codex/fix-release-pack-out-guard
• Commit: yha9806@e18f94a

修复在执行 rmSync 之前拒绝任何解析到仓库根目录或其祖先目录的输出路径，因此覆盖空字符串、点号、双点号以及文件系统根目录；正常的子目录和同级输出仍然可用。

回归测试不是 mock rmSync，而是在 mkdtemp 创建的一次性 fixture 中执行真实 TypeScript release 脚本。空 --out 和 --out .. 在修复前分别删除 fixture sentinel，修复后均 fail fast 并保留 sentinel。focused test 2/2 通过，pre-push typecheck 通过。

[moonquake2004]

Reviewed the fix — the ancestor-guard is stronger than the minimal ?.trim() || DEFAULT_OUTPUT I suggested: containsPath(destination, root) rejects ''/./../absolute-ancestor paths (anything whose deletion would include the repo root), while keeping normal subdirectories and sibling outputs usable; the non-mocked mkdtemp fixtures (sentinel survives after the fix, deleted before) are exactly the right regression shape. Nice work — this can be cherry-picked upstream when the PR channel reopens. One optional hardening note: the guard doesn't resolve symlinks, so a --out symlink pointing outside the repo would still pass — a realpath check before rmSync would close that too, but it's beyond the reported bug.

[yha9806]

Thanks — this prompted a deeper filesystem-semantics check. There is an important distinction:

• On the tested POSIX runtime, when the final --out entry itself is a symlink, recursive rmSync unlinks that entry and preserves its target. Windows junction behavior differs.
• When an intermediate ancestor is a symlink or junction, rmSync can traverse it.

I reproduced a worst-case bypass against the first commit. In a disposable repository, I created parent-link -> container and ran with --out parent-link/repository. The path is lexically below the checkout but physically resolves back to the checkout root. Before the follow-up fix, the real release script deleted the repository sentinel.

Follow-up commit: yha9806@b8f2957

Branch: https://github.com/yha9806/deepseek-harness/tree/codex/fix-release-pack-out-guard

The guard now checks both lexical containment and the canonical deletion target derived from the nearest existing destination parent, while deliberately not following the final path entry. This preserves ordinary missing output directories and final-link deletion semantics, but rejects ancestor symlink or junction paths that route deletion through the repository.

Verification:

• TDD red: the new real-script regression deleted the sentinel before the follow-up fix.
• Green: all 3 focused release-pack safety tests pass.
• A real safe pack to a previously missing dist/npm output succeeds.
• Documentation sync passes 28/28; full lint and the pre-push build/typecheck pass.

Remaining boundary: this provides fail-fast protection against static misconfiguration. A hostile same-user process could still replace an ancestor after validation, so it is not a race-free filesystem security boundary.

[moonquake2004]

The follow-up is excellent — my "optional hardening note" turned out to be a real hole, and you proved it: an intermediate-ancestor symlink (parent-link → container) makes a lexically-below path physically delete the repo root. The canonicalDeletionTarget approach is exactly right — realpath the nearest existing ancestor (which follows symlinks in existing components), append the missing suffix + final entry without following it — so missing output dirs and final-link unlink semantics are preserved while ancestor symlink/junction routing is rejected. The alternatives-rejected analysis (full realpath would break the missing-dir and final-link cases) is well-reasoned, the TDD red→green shape is right, and the honest TOCTOU framing ("fail-fast against static misconfiguration, not a race-free security boundary") is the correct scope. Nice touch folding it into docs/defensive-patterns.md — that's a reusable pattern beyond this one script.