fix(desktop): normalize bundle restore target paths

[tt-a1i]

What this PR does

Makes restoreFiles() normalize its target directory before validating restore boundaries. The target path is now resolved with path.resolve(), and the containment check uses path.relative() (rejecting results that are .., start with .. + separator, or are absolute) instead of the previous fragile fullPath.startsWith(targetDir + sep) comparison. As a result, target directories that include a trailing path separator are accepted and restored correctly, while path-traversal protection is preserved. Adds regression coverage for the trailing-separator restore case.

Why it's needed

The previous boundary check compared the resolved file path against the raw targetDir string. When the caller passed a target directory with a trailing path separator (or an otherwise non-normalized form), the startsWith(targetDir + sep) comparison failed and a valid restore was incorrectly rejected as escaping the target directory. Normalizing both sides before comparison fixes this false positive without weakening the traversal guard. See #5518.

Reviewer Test Plan

How to verify

• bun test packages/desktop/packages/shared/src/utils/__tests__/bundle-files.test.ts
• bun run typecheck:shared in packages/desktop
• npx eslint src/utils/bundle-files.ts src/utils/__tests__/bundle-files.test.ts in packages/desktop/packages/shared
• npx prettier --check packages/desktop/packages/shared/src/utils/bundle-files.ts packages/desktop/packages/shared/src/utils/__tests__/bundle-files.test.ts
• git diff --check

Evidence (Before & After)

N/A — internal logic change covered by unit tests. New regression test asserts that restoreFiles() succeeds when the target directory has a trailing separator.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested locally; covered by CI
🐧 Linux	⚠️ not tested locally; covered by CI

Environment (optional)

Local macOS workspace; unit tests via npm/vitest.

Risk & Scope

• Main risk or tradeoff: low; scope-limited to restoreFiles() path normalization and its boundary check.
• Not validated / out of scope: manual e2e of the desktop bundle restore flow.
• Breaking changes / migration notes: none.

Linked Issues

Fixes #5518

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

中文说明

本 PR 做了什么

让 restoreFiles() 在校验恢复边界之前先对目标目录进行归一化。现在目标路径会通过 path.resolve() 解析，并且包含性检查改用 path.relative()（拒绝结果为 ..、以 .. + 分隔符开头、或为绝对路径的情况），取代之前脆弱的 fullPath.startsWith(targetDir + sep) 比较。因此，带有结尾路径分隔符的目标目录现在能够被正确接受并完成恢复，同时仍然保留了防止路径穿越（path traversal）的保护。新增了针对结尾分隔符恢复场景的回归测试覆盖。

为什么需要

之前的边界检查是把解析后的文件路径与原始的 targetDir 字符串做比较。当调用方传入带有结尾路径分隔符（或其他未归一化形式）的目标目录时，startsWith(targetDir + sep) 比较会失败，导致一次合法的恢复被错误地判定为越出目标目录而遭拒绝。在比较前对两边都进行归一化即可修复这一误报，且不会削弱穿越保护。详见 #5518。

评审测试计划

如何验证

• bun test packages/desktop/packages/shared/src/utils/__tests__/bundle-files.test.ts
• 在 packages/desktop 中执行 bun run typecheck:shared
• 在 packages/desktop/packages/shared 中执行 npx eslint src/utils/bundle-files.ts src/utils/__tests__/bundle-files.test.ts
• npx prettier --check packages/desktop/packages/shared/src/utils/bundle-files.ts packages/desktop/packages/shared/src/utils/__tests__/bundle-files.test.ts
• git diff --check

证据（前后对比）

N/A —— 这是内部逻辑改动，已由单元测试覆盖。新增的回归测试断言：当目标目录带有结尾分隔符时，restoreFiles() 能够成功执行。

测试平台

操作系统	状态
🍏 macOS	✅ 已测试
🪟 Windows	⚠️ 本地未测试；由 CI 覆盖
🐧 Linux	⚠️ 本地未测试；由 CI 覆盖

环境（可选）

本地 macOS 工作区；通过 npm/vitest 运行单元测试。

风险与范围

• 主要风险或权衡：低；范围仅限于 restoreFiles() 的路径归一化及其边界检查。
• 未验证 / 范围之外：桌面端 bundle 恢复流程的手动端到端测试。
• 破坏性改动 / 迁移说明：无。

关联 Issue

Fixes #5518

AI 协助声明

我使用了 Codex 来评审这些改动、对照既有模式核对实现，并帮助发现潜在的边界情况。

[wenshao]

📌 Same maintainer note — full version on #5509.

This is the fourth path-boundary fix in the desktop batch — #5507, #5513, #5517, and now this one (#5519) all touch the same "normalize a path, then do a boundary check" logic, each as a separate issue + PR. Please collapse the path-boundary work into one desktop PR and route it through the existing shared helper (isSubpath / isPathWithinRoot in packages/core/src/utils/paths.ts, or the desktop-shared equivalent) instead of four near-duplicate checks (trailing-separator handling included). The change looks correct — this is purely grouping + DRY.

中文说明

📌 同一条维护者说明 —— 完整版见 #5509。

这是 desktop 这批里第四个路径边界修复了 —— #5507、#5513、#5517,加上现在这个 #5519,动的都是同一套"先 normalize 路径、再做边界检查"的逻辑,各自一个 issue + 一个 PR。请把路径边界相关的改动合并成一个 desktop PR,并统一走已有共享 helper(packages/core/src/utils/paths.ts 里的 isSubpath / isPathWithinRoot,或 desktop 对应实现),而不是四份几乎一样的检查(含尾分隔符处理)。改动看起来是对的 —— 这条纯粹是归并 + DRY。

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR! Unfortunately the PR body doesn't follow our pull request template. Several required sections are missing:

• What this PR does / Why it's needed — currently "Summary" covers the what but not the why in the expected format
• Reviewer Test Plan — with "How to verify", "Evidence (Before & After)", and "Tested on" (OS matrix)
• Risk & Scope — main risk/tradeoff, what's out of scope, breaking changes
• Linked Issues — use Fixes #5518 with a closing keyword for auto-close
• Chinese translation — the <details> block with a full translation of the English body

Please update the PR description to match the template.

---

Separately, a maintainer has already flagged that this is the fourth path-boundary fix in the desktop batch (#5507, #5513, #5517, #5519). Before spending more time on this individual PR, please consider consolidating all path-boundary fixes into a single PR and routing through the existing shared helper — isSubpath in packages/core/src/utils/paths.ts or isWithinRoot in packages/core/src/utils/fileUtils.ts — instead of duplicating near-identical boundary-check logic. The fix itself looks correct, but grouping + DRY would be more maintainable.

中文说明

感谢贡献！不过 PR 描述没有按照我们的PR 模板填写，缺少以下必填部分：

• What this PR does / Why it's needed — 目前的 "Summary" 覆盖了做了什么，但没有按模板格式说明为什么
• Reviewer Test Plan — 包含 "How to verify"、"Evidence (Before & After)" 和 "Tested on"（操作系统矩阵）
• Risk & Scope — 主要风险/权衡、不在范围内的内容、破坏性变更
• Linked Issues — 使用 Fixes #5518 关闭关键词自动关闭 issue
• 中文翻译 — <details> 块中包含英文正文的完整翻译

请按照模板更新 PR 描述。

---

另外，维护者已经指出这是 desktop 这批里第四个路径边界修复（#5507、#5513、#5517、#5519）。在继续单独推进这个 PR 之前，请考虑将所有路径边界修复合并成一个 PR，并统一使用已有的共享 helper —— packages/core/src/utils/paths.ts 中的 isSubpath 或 packages/core/src/utils/fileUtils.ts 中的 isWithinRoot —— 而不是重复几乎相同的边界检查逻辑。修复本身是正确的，但归并 + DRY 会更好维护。

— Qwen Code · qwen3.7-max

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

On direction: this is a legitimate bug fix — trailing separators causing false path-traversal rejection is a real issue, and the fix aligns with the project's reliability goals.

On approach: the implementation is correct in isolation, but @wenshao already flagged the bigger picture in the comments — this is the fourth path-boundary fix PR in the desktop batch, and packages/core/src/utils/paths.ts already has a well-tested isSubpath() helper that handles trailing separators, Windows paths, and all the edge cases this PR addresses manually. The right move is to consolidate: import isSubpath (or a desktop-local equivalent) and replace the inline startsWith / relative checks across all four call sites, rather than fixing each one individually.

The code change here is technically sound, but the DRY concern is the real blocker — not the logic itself.

Flagging this for consolidation before approving. 🔧

中文说明

感谢贡献！

模板完整 ✓

方向：这是一个合理的 bug 修复——尾部分隔符导致误判路径穿越是真实存在的问题，修复方向与项目的可靠性目标一致。

方案：单独来看实现是正确的，但 @wenshao 已经在评论中指出了更大的问题——这是 desktop 这批里第四个路径边界修复 PR，而 packages/core/src/utils/paths.ts 中已经有一个经过充分测试的 isSubpath() helper，它已经处理了尾部分隔符、Windows 路径以及本 PR 手动处理的所有边界情况。正确的做法是归并：引入 isSubpath（或 desktop 本地等价实现），替换四个调用点各自内联的 startsWith / relative 检查，而不是逐个修复。

这里的代码改动本身技术上是可靠的，但 DRY 问题才是真正的阻塞点——不是逻辑本身。

在批准之前先标记需要归并。🔧

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal (before reading the diff): I'd import isSubpath from packages/core/src/utils/paths.ts and use it to replace the inline boundary check. isSubpath already uses path.relative() + startsWith('..' + sep) + isAbsolute guards — the exact same logic this PR adds manually — and it handles trailing separators, Windows paths, and case normalization. One import, one call, zero new code.

PR's approach: re-implements the same containment check inline with resolve() + relative() + manual guard clauses. The logic is correct, but it's a near-duplicate of isSubpath that lives 30 lines away in the core package. The packages/desktop/packages/shared package doesn't currently import from core utils, which may be why the author went inline — but the right fix is to either import isSubpath or create a desktop-shared re-export, not to duplicate the boundary logic a fourth time.

The existing isSubpath test suite (paths.test.ts:290–388) already covers the trailing-separator case:

isSubpath('/a/b/', '/a/b/c')    → true
isSubpath('/a/b', '/a/b/c/')    → true
isSubpath('/a/b/', '/a/b/c/')   → true

No critical blockers — the logic is sound. The concern is purely duplication/DRY, which is the consolidation issue flagged in Stage 1.

Testing

Unit tests: bun test is not available in the CI environment, so the bundle-files.test.ts suite (including the new trailing-separator regression test) could not be executed here. The test itself looks correct — it creates a target dir, appends sep, and asserts restoreFiles succeeds.

tmux real-scenario testing: not applicable — restoreFiles() is an internal desktop bundle utility, not a CLI-facing feature. There is no qwen -p invocation that exercises this code path. The PR's test plan also acknowledges "N/A — internal logic change covered by unit tests."

中文说明

代码审查

独立方案（读 diff 之前）：我会从 packages/core/src/utils/paths.ts 引入 isSubpath，用它替换内联的边界检查。isSubpath 已经使用了 path.relative() + startsWith('..' + sep) + isAbsolute 防护——和本 PR 手动添加的逻辑完全一致——而且它还处理了尾部分隔符、Windows 路径和大小写归一化。一个 import，一次调用，零新增代码。

PR 的方案： 用 resolve() + relative() + 手动守卫条件内联重新实现了相同的包含性检查。逻辑是正确的，但几乎是 core 包里 isSubpath 的复制品。packages/desktop/packages/shared 目前没有从 core utils 导入，这可能是作者选择内联的原因——但正确的做法是引入 isSubpath 或创建 desktop-shared 的再导出，而不是第四次重复边界逻辑。

现有的 isSubpath 测试套件（paths.test.ts:290–388）已经覆盖了尾部分隔符的场景。

无严重阻塞——逻辑可靠。问题纯粹是重复/DRY，与 Stage 1 中标记的归并问题一致。

测试

单元测试： CI 环境中没有 bun，无法执行 bundle-files.test.ts（包括新增的尾部分隔符回归测试）。测试本身看起来是正确的。

tmux 真实场景测试： 不适用——restoreFiles() 是桌面端内部的 bundle 工具函数，不是 CLI 面向用户的功能，无法通过 qwen -p 调用触发。PR 的测试计划也说明了"N/A——由单元测试覆盖的内部逻辑改动"。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Verdict: Changes Requested

The fix is technically correct — path.resolve() + path.relative() is the right way to handle the trailing-separator boundary check. But stepping back, this is the fourth PR in a batch that each re-implements the same containment check in a slightly different way, while a well-tested shared helper (isSubpath in core) already covers all these cases.

My independent proposal was simpler: one import, one call, no new logic. The PR's approach works but duplicates what already exists.

The maintainer's ask is clear and reasonable: consolidate the four path-boundary fixes into a single PR that routes through isSubpath (or a desktop-shared equivalent). This PR should wait for that consolidation rather than merge as-is — it would introduce a fourth near-identical inline check that becomes tech debt the moment the consolidation lands.

No approval. Requesting changes to align with the consolidation direction. 🙏

中文说明

结论：需要修改

修复本身技术上是正确的——path.resolve() + path.relative() 是处理尾部分隔符边界检查的正确方式。但退一步看，这是这批中第四个 PR，每个都以略有不同的方式重新实现了相同的包含性检查，而 core 中已有一个经过充分测试的共享 helper（isSubpath）可以覆盖所有这些场景。

我的独立方案更简单：一个 import，一次调用，不需要新逻辑。PR 的方案可行，但重复了已有的实现。

维护者的要求清晰且合理：将四个路径边界修复合并为一个 PR，统一走 isSubpath（或 desktop-shared 等价实现）。这个 PR 应该等待那次归并，而不是按现状合入——否则它会引入第四个几乎相同的内联检查，归并落地时立刻变成技术债。

不予批准。请求修改以对齐归并方向。🙏

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Needs consolidation — see triage comments above. The fix is correct but duplicates the existing isSubpath helper. Please align with the maintainer's request to collapse the four path-boundary PRs into one. 🙏

[wenshao]

✅ Local verification report (real build + tmux test run) — commit efa92ee

Ran the full check matrix locally under bun (the runtime CI couldn't execute), in an isolated git worktree at the PR head. Environment: bun 1.3.14, Node 22, Linux; all commands run inside tmux.

TL;DR — the fix is correct, the new regression test genuinely guards the bug, path‑traversal protection is fully preserved, and every CI gate passes locally. The only open item is the consolidation already raised in the thread — and I verified that isSubpath is a behavior‑identical drop‑in, so that refactor is low‑risk.

1. Unit tests

bun test src/utils/__tests__/bundle-files.test.ts → 24 pass / 0 fail. The new case runs and passes in isolation (-t "trailing separator" → 1 pass / 23 filtered).

2. A/B — does the new test actually catch the bug? (yes)

Overlaid the merge‑base (pre‑fix) bundle-files.ts while keeping the PR's new test:

Source under test	Result
merge‑base (old startsWith(targetDir + sep) check)	23 pass, 1 fail
PR efa92ee	24 pass, 0 fail

The failing test on the old code is exactly the new one, failing at the old boundary line:

error: Path escapes target directory: test.txt
(fail) bundle-files > restoreFiles > restores files when target directory has a trailing separator

→ #5518 is a real false‑positive and the regression test has teeth.

3. Path‑traversal guard preserved (adversarial probe — 14 / 14 pass)

Exercised the modified trailing‑separator code path directly:

• Accepted (correct): nested file, target// (double sep), non‑normalized x/../target/, plain target.
• Rejected (correct): ../escape.txt, a/../../escape.txt, /etc/escape.txt, ..\escape.txt — against both plain and trailing‑separator targets.
• Nothing escaped to disk for any payload (sentinel files outside the target never appeared).
• Layering: traversal is caught upstream by validateBundleFile (Invalid bundle file: …), so the boundary check is defense‑in‑depth. The fix changes a false‑positive, not the traversal guard.

4. Consolidation check — isSubpath is a verified drop‑in (differential — 23 / 23 pass)

Re: routing through isSubpath. I imported the real isSubpath from packages/core/src/utils/paths.ts and compared !isSubpath(parent, child) against the PR's exact inline reject predicate over an explicit corpus plus a 72‑pair fuzz corpus → zero disagreements, including trailing‑separator cases, the classic sibling‑prefix trap (/a/b vs /a/bc), and traversal escapes.
→ if (!isSubpath(resolvedTargetDir, fullPath)) throw … (keeping the two resolve() calls) is behavior‑identical to the inline block, and isSubpath additionally centralizes the Windows path.win32 handling. The suggested consolidation is safe.

5. Static gates

Check	Result
git diff --check	✅ clean
eslint (the lint:shared gate) on both files	✅ exit 0
typecheck:shared (tsc --noEmit, whole package)	✅ 0 errors
prettier --check (test‑plan line)	⚠️ N/A — see note

Note on the test plan's prettier --check: it's moot for desktop files. packages/desktop/ is listed in the root .prettierignore (line 25); the desktop sub‑app is formatted/linted via its own ESLint flat config, not prettier. Running prettier directly picks up the root semi: true config and false‑flags the intentional no‑semi desktop style — not a real issue.

Recommendation

Correctness, the regression test, and traversal safety are all green — nothing blocks this on the merits. The only substantive item is the consolidation already flagged: this would be the 4th near‑duplicate inline boundary check, and the differential above confirms isSubpath replaces it 1:1. Preference is to land the consolidation (a single desktop PR routing all four call sites through isSubpath) rather than merge a 4th copy. If #5518 needs an immediate fix, this PR is safe to merge as‑is and refactor later — but consolidating now avoids creating the debt the moment it lands.

中文说明

✅ 本地验证报告（真实构建 + tmux 跑测试）—— 提交 efa92ee

在 PR 头部对应的独立 git worktree 中，使用 bun（CI 运行环境无法执行 bun）在本地完整跑了一遍检查矩阵。环境： bun 1.3.14、Node 22、Linux；所有命令均在 tmux 中执行。

结论速览 —— 修复是正确的，新增回归测试确实能守住这个 bug，路径穿越保护完整保留，本地所有 CI 门禁均通过。唯一待办项就是评论区已经提出的归并问题 —— 我验证了 isSubpath 是行为完全等价的直接替换，因此该重构风险很低。

1. 单元测试

bun test src/utils/__tests__/bundle-files.test.ts → 24 通过 / 0 失败。新增用例可单独运行并通过（-t "trailing separator" → 1 通过 / 23 过滤）。

2. A/B —— 新测试是否真的能抓到 bug？（能）

在保留 PR 新测试的前提下，覆盖回 merge‑base（修复前）的 bundle-files.ts：

被测源码	结果
merge‑base（旧的 startsWith(targetDir + sep) 检查）	23 通过，1 失败
PR efa92ee	24 通过，0 失败

旧代码下失败的正是这条新用例，且失败发生在旧的边界检查行：

error: Path escapes target directory: test.txt
(fail) bundle-files > restoreFiles > restores files when target directory has a trailing separator

→ #5518 是真实存在的误报，回归测试是有效的（有"牙齿"）。

3. 路径穿越保护已保留（对抗性探针 —— 14 / 14 通过）

直接针对被改动的尾分隔符代码路径进行测试：

• 正确接受： 嵌套文件、target//（双分隔符）、未归一化的 x/../target/、普通目标目录。
• 正确拒绝： ../escape.txt、a/../../escape.txt、/etc/escape.txt、..\escape.txt —— 在普通目标目录和尾分隔符目标目录两种情况下都拒绝。
• 任何 payload 都没有写到目标目录之外（目标目录外的哨兵文件从未出现）。
• 分层防护： 穿越类 payload 由上游的 validateBundleFile 拦截（Invalid bundle file: …），因此边界检查属于纵深防御。本次修复改的是误报，而非穿越保护本身。

4. 归并核验 —— isSubpath 是经过验证的直接替换（差分测试 —— 23 / 23 通过）

针对"改用 isSubpath"这一建议：我从 packages/core/src/utils/paths.ts 导入了真实的 isSubpath，在一组显式语料 外加 72 组模糊语料上，把 !isSubpath(parent, child) 与 PR 内联拒绝判定逐一比对 → 零分歧，覆盖尾分隔符、经典的同级前缀陷阱（/a/b vs /a/bc）以及穿越逃逸场景。
→ if (!isSubpath(resolvedTargetDir, fullPath)) throw …（保留两处 resolve() 调用）与内联代码块行为完全一致，且 isSubpath 还统一封装了 Windows 的 path.win32 处理。建议的归并是安全的。

5. 静态门禁

检查	结果
git diff --check	✅ 干净
eslint（即 lint:shared 门禁）作用于两个文件	✅ 退出码 0
typecheck:shared（tsc --noEmit，整包）	✅ 0 报错
prettier --check（测试计划中的那条）	⚠️ 不适用 —— 见说明

关于测试计划里的 prettier --check： 对 desktop 文件而言这条是无意义的。packages/desktop/ 被列入了根目录 .prettierignore（第 25 行）；desktop 子应用是通过它自己的 ESLint flat config 来格式化/检查的，并不走 prettier。直接运行 prettier 会读取到根目录的 semi: true 配置，从而误报 desktop 故意采用的无分号风格 —— 并非真实问题。

建议

正确性、回归测试、穿越安全性都是绿灯 —— 从技术本身看没有任何阻塞。唯一实质性事项就是已经被标记的归并问题：这将是第 4 个几乎重复的内联边界检查，而上面的差分测试已确认 isSubpath 可 1:1 替换它。倾向于先落地归并（用一个 desktop PR 把四个调用点统一改走 isSubpath），而不是再合入第 4 份副本。如果 #5518 需要立刻修，本 PR 按现状合入是安全的、之后再重构也可以 —— 但现在就归并能避免它一落地就变成技术债。

[tt-a1i]

consolidated this into #5545 so the desktop path-boundary fixes share one helper. closing this one to avoid duplicate review.