fix(chat): securely support external Windows history paths#270
fix(chat): securely support external Windows history paths#270End0rph1nww wants to merge 2 commits into
Conversation
Reviewer's Guide by Ameath小爱把阅读路径顺了一下，可以从这里开始查看。 核查覆盖 文件级变更
关联 Issue
Tips and commands
Original review guide in EnglishThe review traced |
ameath-review
Bot
changed the title
![ameath-review[bot]](https://avatars.githubusercontent.com/in/4276791?s=60&v=4){kind=small}
There was a problem hiding this comment.
小爱把需要留意的位置放在这里，方便逐处确认。
小爱已完成全部变更审查，共发布 2 条行级建议（高风险 0，中风险 1）。
合并前关注点（风险 1 · 建议测试 1）
风险
- 唯一关联的 #269 在 PR 元数据中标记为
possible，其内容聚焦聊天 UI/文档困惑，并明确称跨盘路径故障尚未确认；目前没有与本次外部 Windows 聊天历史路径变更直接对应的已关联 issue。
建议测试
- 在 Windows 上补充
\\?\UNC\...历史路径经_chat_history_path、_launch_chat的--history参数到main.py加载/保存的端到端往返测试；现有长路径用例只直接读写Path，跨盘启动用例替换了_launch_chat。
Prompt for AI Agents
请逐项处理以下审查问题；修改前先核实上下文，修改后运行相关测试。
1. 原生清理可能恢复旧分支历史 / Native clear can resurrect branch history
core/sprite/chat_ui_service.py:129 (RIGHT)
<issue_to_address>此处在 clear_chat_history() 已删除 active.json 后才进行尽力而为的清理。remove_chat_history_storage() 会吞掉 branches.json 的删除错误；若该文件被 Windows 上的其他进程锁定，或进程在两步之间退出，旧分支树会保留。原生模式退出时只写入空的 active.json，下次启动 reconcile_active_branch_state() 会把空 active 当作缺失，并从残留分支树恢复旧的 messages/history，使已清空的聊天重新出现。请持久化可恢复的空分支状态（或清除标记），且不要将元数据删除失败报告为清理成功。
English: active.json is removed before this best-effort cleanup. If deleting branches.json fails or the process exits between the two steps, native shutdown writes only an empty active file; recovery then restores the stale branch payload, so cleared history reappears.</issue_to_address>
2. 离线 UNC 验证仍会阻塞请求 / Offline UNC validation still blocks requests
frontend_bridge_core/history_paths.py:79 (RIGHT)
<issue_to_address>注释说明这里应保持词法处理以避免离线 UNC 共享阻塞，但 Path.exists() 已经会访问该共享；后续的 is_file()、is_dir() 和 iterdir() 也一样。用户保存的 UNC 历史在共享断开后，启动、恢复或无实时会话时的快照轮询都可能等待 Windows 网络超时，而不是及时返回“不可访问”。请对 UNC 目标使用不触发 I/O 的验证，或将有界的可访问性检查移出同步请求路径。
English: Despite the lexical-path comment, exists() and the following checks access the UNC share. A disconnected saved share can block launch, resume, or snapshot polling until the Windows network timeout.</issue_to_address>
| ) | ||
| ) | ||
| if history_file: | ||
| remove_chat_history_storage(history_file) |
There was a problem hiding this comment.
[MEDIUM · correctness] 原生清理可能恢复旧分支历史 / Native clear can resurrect branch history
此处在 clear_chat_history() 已删除 active.json 后才进行尽力而为的清理。remove_chat_history_storage() 会吞掉 branches.json 的删除错误；若该文件被 Windows 上的其他进程锁定，或进程在两步之间退出，旧分支树会保留。原生模式退出时只写入空的 active.json，下次启动 reconcile_active_branch_state() 会把空 active 当作缺失，并从残留分支树恢复旧的 messages/history，使已清空的聊天重新出现。请持久化可恢复的空分支状态（或清除标记），且不要将元数据删除失败报告为清理成功。
English: active.json is removed before this best-effort cleanup. If deleting branches.json fails or the process exits between the two steps, native shutdown writes only an empty active file; recovery then restores the stale branch payload, so cleared history reappears.
置信度 96% · 可直接回复讨论 · 👍/👎 会影响后续审查
| def _validate_history_storage_target(path: Path) -> Path: | ||
| """Reject an existing unrelated file/directory as a history storage root.""" | ||
|
|
||
| if not path.exists(): |
There was a problem hiding this comment.
[LOW · performance] 离线 UNC 验证仍会阻塞请求 / Offline UNC validation still blocks requests
注释说明这里应保持词法处理以避免离线 UNC 共享阻塞，但 Path.exists() 已经会访问该共享；后续的 is_file()、is_dir() 和 iterdir() 也一样。用户保存的 UNC 历史在共享断开后，启动、恢复或无实时会话时的快照轮询都可能等待 Windows 网络超时，而不是及时返回“不可访问”。请对 UNC 目标使用不触发 I/O 的验证，或将有界的可访问性检查移出同步请求路径。
Original in English
Despite the lexical-path comment, exists() and the following checks access the UNC share. A disconnected saved share can block launch, resume, or snapshot polling until the Windows network timeout.
置信度 93% · 可直接回复讨论 · 👍/👎 会影响后续审查
|
@Ame4th review |
ameath-review
Bot
changed the title
There was a problem hiding this comment.
小爱沿着实现细节逐处看过，下面是需要留意的地方。
小爱已完成全部变更审查，共发布 2 条行级建议（高风险 0，中风险 2）。
Prompt for AI Agents
请逐项处理以下审查问题；修改前先核实上下文，修改后运行相关测试。
1. 保留失败清理的恢复临时文件 / Preserve recovery data on a failed clear
llm/history_manager.py:176 (RIGHT)
<issue_to_address>delete_tmp() 在空活动快照写入成功前执行。若 write_text("[]") 因权限或 Windows 文件占用失败，清理会报错且旧 active.json 仍在，但增量恢复用的 .tmp 已被删除。LLMManager 会把新增消息追加到该文件，因此最后尚未合并的对话无法恢复。应在新的空状态可持久恢复后才丢弃 .tmp，并为提交失败保留恢复路径。
delete_tmp() runs before the empty active snapshot is committed. If write_text("[]") fails because of permissions or a Windows sharing violation, the clear reports an error with the old active.json still present but the incremental recovery .tmp already deleted. LLMManager appends new messages there, so the latest unmerged turns cannot be recovered. Retain the recovery file until the new empty state is durably recoverable, with a failure-recovery path.</issue_to_address>
2. 在删除前验证 UNC 历史目录归属 / Validate UNC history ownership before deletion
frontend_bridge_core/history_paths.py:93 (RIGHT)
<issue_to_address>UNC 路径在此直接返回，因而完全跳过后续目录归属检查。用户在启动时提供 \\server\share\folder（甚至共享根）后，resetHistory 会调用 remove_chat_history_storage()，后续清理也会调用它，并删除该目录中仅因名称匹配的 active.json、active.json.tmp 和 branches.json。这些文件可能属于其他应用，而外部 UNC 正是本 PR 新增支持的范围。应将离线 UNC 的可启动判定与执行破坏性操作前的所有权验证分开。
UNC paths return here directly and therefore skip all later directory-ownership checks. After a user supplies \\server\share\folder (even a share root), resetHistory and later clearing call remove_chat_history_storage(), which deletes name-matched active.json, active.json.tmp, and branches.json from that directory. Those files may belong to another application, and external UNC storage is newly supported by this PR. Separate the offline-UNC launch decision from ownership validation before destructive operations.</issue_to_address>
2 participants
Summary
\\?\spelling for long-path I/ORoot cause
The desktop project-root selector canonicalizes the selected directory. On Windows this can produce a
\\?\path, while other callers may still use the regular drive spelling.ntpath.commonpathtreats those spellings as different drives.Chat history also reused the generic project-contained path resolver, even though explicit external chat-history locations are supported. As a result, both same-drive verbatim/plain spelling mismatches and legitimate cross-drive histories could fail with
Paths don't have the same drive.User impact
Users can launch and resume chats whose histories live outside the selected data directory without weakening the containment rules for general project assets and downloads. Existing extended-length paths retain long-path compatibility.
Validation
C:project plusD:external-history I/O smoke test passedSummary by Ameath
小爱先把本次核对的重点整理出来，方便快速查看。
本 PR 将聊天历史与通用项目内路径策略拆分，支持 Windows 外部盘符、UNC 和受支持的 verbatim 路径，并调整下载、清理和空历史恢复；审查发现失败清理会丢失恢复临时文件，且 UNC 路径可绕过目录归属校验后参与删除。
New Features
Enhancements
Tests
Original summary in English
This PR separates chat-history handling from the generic project-contained path policy, adds support for external Windows drives, UNC paths, and supported verbatim paths, and changes download, cleanup, and empty-history recovery. The review found that a failed clear can discard recovery data, and that UNC paths can bypass directory-ownership validation before deletion.
New Features
Enhancements
Tests