fix(security): harden remote pairing PIN persistence

[appergb]

User description

Summary

• harden remote pairing PIN reads around an already-opened file descriptor/handle before any read or permission repair
• reject symlinks/reparse points, hard links, directories, devices/FIFOs, and oversized files without touching their targets
• replace PINs atomically on Unix and with native Windows ReplaceFileW / MoveFileExW plus rollback backup handling
• make generated/reset PIN persistence fail closed, committing memory and refreshing the server only after durable persistence succeeds
• run the PIN persistence suite from the Rust-only cross-platform backend harness and pin the source/CFG contract in CI

Security behavior

Unix

• opens with O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC
• validates regular-file type, link count, and bounded size from fstat metadata
• repairs legacy permissions to 0600 through the opened descriptor
• performs a bounded post-validation read to catch concurrent growth
• creates the temporary file as 0600 before rename

Windows

• opens with FILE_FLAG_OPEN_REPARSE_POINT
• validates disk-file type, attributes, hard-link count, and size using handle-based Win32 APIs
• uses ReplaceFileW for existing destinations and MoveFileExW(MOVEFILE_WRITE_THROUGH) for first creation
• never deletes the destination before replacement; retains/restores a unique same-directory backup on replacement failure

State consistency

• generated PINs are returned only after persistence succeeds
• reset errors propagate through the Tauri command
• in-memory PIN and remote-server refresh remain unchanged when persistence fails

TDD and verification

Exact pushed head: d5b3d09d7833904f9b1b8ebbd419a72a7cb963a9
Rebased onto beta: 9991b325b8ea035ab167d486c5ba3c71688418a9

RED evidence included a FIFO-read timeout, oversized-file acceptance, missing atomic-replacement fault seam, and reset transaction compile failures before the implementation was added.

• cargo test --locked --manifest-path src-tauri/Cargo.toml --lib: 665 passed
• cargo test --locked --manifest-path src-tauri/backend-tests/Cargo.toml: 119 passed
• Windows MSVC backend test harness cargo check --locked ... --target x86_64-pc-windows-msvc: passed
• GitHub CI run 29392896847: Android, macOS, Linux, and native Windows checks passed; Windows included Rust backend unit test execution
• PIN persistence focused suite: 13 passed
• macOS capsule, hotkey injection, side-modifier, Windows lifecycle, PIN persistence, and Android updater contracts: passed
• npm run build including the latest beta prebuild contract: passed
• npm audit: 0 vulnerabilities
• cargo audit: 0 vulnerabilities; 18 existing allowed maintenance/unsoundness warnings
• rustfmt check, diff check, and changed-diff gitleaks scan: passed
• GitNexus change impact: low risk, 0 affected execution flows

No visible UI layout changes.

Closes #813

---

PR Type

Bug fix, Enhancement, Tests

---

Description

• Harden PIN file permissions on Unix to 0600, repair legacy modes

• Replace PIN atomically: temp file with 0600, then rename or ReplaceFileW

• Validate file type, reject symlinks, FIFOs, oversized files

• Propagate persistence errors to Tauri command and memory state

• Add tests for atomic replacement, permission repair, and invalid files

---

Diagram Walkthrough

flowchart LR
  A["load_or_create_pin"] --> B{File exists?}
  B -- Yes --> C["Open with security flags"]
  C --> D["Validate file metadata"]
  D --> E{Valid?}
  E -- Yes --> F["Read bounded PIN"]
  F --> G["Return PIN"]
  E -- No --> H["Generate new PIN"]
  B -- No --> H
  H --> I["Create temp file (0600)"]
  I --> J["Write PIN + sync"]
  J --> K["Atomic replace: rename or ReplaceFileW"]
  K --> L["Return new PIN"]

Loading

File Walkthrough

	Relevant files
Tests	2 files backend_rust.rs Add PIN persistence test module import                                      +2/-0      pin-persistence-security-contract.test.mjs Create security contract test for PIN persistence                +74/-0
Enhancement	4 files remote_input.rs Make regenerate_remote_pin return Result                                  +1/-1      coordinator.rs Add persist-and-commit transaction for PIN                              +80/-12  mod.rs Delegate PIN persistence to dedicated module                          +20/-22  pin_persistence.rs Implement secure PIN persistence with atomic replacement  +588/-0
Configuration changes	2 files ci.yml Add PIN persistence security check to CI                                  +3/-0      package.json Add npm script for PIN persistence check                                  +1/-0
Dependencies	2 files Cargo.toml Add dependencies for PIN persistence file operations          +6/-2      Cargo.toml Add dependencies for backend tests                                              +2/-0

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit d5b3d09)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 813 - Partially compliant Compliant requirements: Unix: temporary file is created with mode 0600; rename retains permissions; repair uses opened file descriptor. Atomic replacement: std::fs::rename on Unix, ReplaceFileW/MoveFileExW on Windows. Windows ACL behavior preserved (no permission change attempted). No logging of the PIN anywhere in the added code. Non-compliant requirements: (none) Requires further human verification: (none)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 Security concerns TOCTOU race: As described above, a path substitution between validation and rename on Unix could allow writing the PIN to an arbitrary file if an attacker has write access to the config directory. No other security concerns identified; the code properly avoids logging the PIN and uses bounded reads.

⚡ Recommended focus areas for review TOCTOU race A race exists between validate_existing_pin_path (line 147) and the subsequent std::fs::rename (line 195). An attacker with write access to the config directory could replace the validated regular file with a symlink to an arbitrary file after validation and before rename, causing the new PIN to be written to the attacker-chosen path. The probability is low because it requires precise timing and write access, but the potential impact is redirection of the PIN to an attacker-controlled location. The current implementation does not use file-descriptor-based rename to prevent this. validate_existing_pin_path(path)?; let file_name = path .file_name() .map(|name| name.to_string_lossy().into_owned()) .unwrap_or_else(|| "remote-input-pin.txt".to_string()); let temp_path = path.with_file_name(format!( ".{file_name}.tmp-{}", uuid::Uuid::new_v4().simple() )); let result = (|| { let mut options = std::fs::OpenOptions::new(); options.write(true).create_new(true); #[cfg(unix)] { use std::os::unix::fs::OpenOptionsExt; options.mode(0o600); } let mut temp = options.open(&temp_path)?; temp.write_all(pin.as_bytes())?; temp.sync_all()?; validate_and_repair_pin_file(&temp)?; drop(temp); replace(&temp_path, path)?; sync_pin_parent(path)?; Ok(()) })(); if result.is_err() { let _ = std::fs::remove_file(&temp_path); } result } fn validate_existing_pin_path(path: &Path) -> std::io::Result<bool> { match open_pin_file(path) { Ok(file) => { validate_and_repair_pin_file(&file)?; Ok(true) } Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), Err(error) => Err(error), } } #[cfg(unix)] fn replace_pin_file(temp_path: &Path, path: &Path) -> std::io::Result<()> { std::fs::rename(temp_path, path)

[appergb]

Senior security review — exact head dec4d6d186548c573f050b6f33f48a0819eaead0

Strengths

• 改动聚焦在 remote_server/mod.rs，没有前端/UI 改动，也没有记录 PIN；日志只包含路径与错误。
• Unix 新文件使用同目录 UUID 临时文件、create_new(true) 与 mode(0o600)，然后 write_all → file sync_all → rename → parent sync_all。正常路径不会出现 world-readable 创建窗口，且同文件系统 rename 的结构合理。
• legacy 0644、普通 symlink/hardlink、无效/partial PIN 与最终替换路径都有测试。detached exact-head focused run：remote_server::pin_persistence_tests 6/6 passed；changed-file rustfmt check 与 git diff --check passed。
• exact head 的 5 个 check runs 全部 success：Android cargo check、macOS checks、Windows checks、Linux checks、PR-Agent。

Critical

• None.

Important

1. Unix 的 nonregular/link 防护发生得太晚，并且 permission repair 存在 pathname TOCTOU。 read_valid_persisted_pin 在任何 symlink_metadata/regular-file 检查前先执行 read_to_string（lines 172-184）。因此 FIFO 会在启动路径无限阻塞，设备/超大 regular file 也会先产生 I/O 或无界内存开销；symlink target 也会先被打开。随后 repair_pin_permissions 是 symlink_metadata(path) 再 set_permissions(path)（lines 245-259），攻击者可在两步间把 path 换成 symlink/hardlink，使 chmod 落到另一个 inode。我的文件系统 contract 复现了两点：FIFO reader 在 250ms 后仍阻塞；通过检查后把 path 换成 symlink，path chmod 将 target 从 0644 改为 0600。请在 Unix 使用 O_NOFOLLOW 打开一次文件，在同一 fd 上 fstat/检查 regular+nlink、File::set_permissions，并限定最多读取 PIN 所需字节；不要在验证后重新按 pathname 打开/chmod。
2. Windows fallback 不是原子替换，并会在第二次 rename 失败时删除唯一旧 PIN。 replace_pin_file lines 231-240 在 destination 存在时先 remove_file(path)，再 rename temp；这中间有可见缺口，且第二次失败会丢失旧文件并错误地返回第一次 rename 的 error。调用链又在 Coordinator::regenerate_remote_pin 先更新内存 PIN，save_pin 吞掉持久化错误后仍 refresh server（coordinator.rs:1557-1564），所以 reset 可报告/启用新 PIN，但磁盘凭据已丢，重启后再变化。这违反 #813 的 atomic replacement 与 persistent/reset semantics。请使用 Windows 原生 replace primitive（例如 ReplaceFileW/等价安全封装），并让 reset 只在持久化成功后提交内存/server 状态。
3. PR 的“reject symlink and hard-link PIN paths”在 non-Unix 上并未实现。 repair_pin_permissions 的 cfg(not(unix)) 分支无条件 Ok(())（lines 263-265），所以 Windows 上一个指向六位文本的 symlink 会被读取并接受；Windows CI 只对主 lib tests --no-run，没有执行这组 Windows 行为测试。至少应跨平台拒绝 symlink/nonregular；若 hard-link rejection 仅承诺 Unix，需要在 PR/代码契约中明确并为 Windows replacement/error path 添加可执行测试。

Minor

• new_pin_file_is_owner_only_from_creation 只观察 rename 后的最终 mode，测试本身不能证明“from creation”；这一点目前由 OpenOptionsExt::mode(0o600) 的代码审查支持。建议用 umask/同步观察或封装后的 open helper 测试强化该断言。
• PR 当前仍是 Draft，PR base 记录为 fa53f47f…；current beta 已到 c645b168…，exact head 相对 current beta 为 1 behind / 1 ahead。修复后应同步 beta 并重新跑五项 checks。
• 全仓 cargo fmt --all -- --check 受大量与本 PR 无关的既有格式 drift 影响而失败；本次只据 changed-file rustfmt --check 通过作判断，没有裸跑会写文件的 cargo fmt。

Ready-to-merge verdict

COMMENTED — logical REQUEST_CHANGES. 0600 创建、正常 Unix rename 与 legacy repair 主方向正确，但 pathname TOCTOU/nonregular pre-read 与 Windows delete-then-rename 会留下安全和数据持久化缺口。上述 Important 项闭环并补相应失败路径测试前，不建议转 Ready 或合并。

[github-actions]

Persistent review updated to latest commit aaa0348

[github-actions]

Persistent review updated to latest commit 0e7493e

[github-actions]

Persistent review updated to latest commit d5b3d09

[appergb]

Independent security re-review — logical verdict: APPROVE

Reviewed exact head d5b3d09d7833904f9b1b8ebbd419a72a7cb963a9 against current beta 472b914985ec525bb082582fd7968a43c7d9ce41. The current conflict-free merge tree is ddd143c8baa94071aec18f401d1194b1407b268b.

Critical

None.

Important

None. The three prior Important blockers are closed.

Minor

• The Windows runner executes the real successful ReplaceFileW path plus symlink, hard-link, size, and generic replacement-failure behavior, but the native backup-restoration branch is still verified by code review/source contract rather than a deterministic Win32 fault-injection test. A future seam covering ERROR_UNABLE_TO_MOVE_REPLACEMENT_2 would make the rollback proof stronger. This is non-blocking because the implementation retains a unique same-directory backup and restores it whenever the destination is absent, matching Microsoft's documented failure state: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-replacefilew

Security evidence

• Unix opens once with O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC, then uses metadata from that descriptor to require a regular single-link file and enforce the 64-byte limit before reading. Permission repair is descriptor-based (File::set_permissions), and the post-validation read is capped at 65 bytes to detect concurrent growth.
• The Unix behavior suite proves nonblocking FIFO rejection, device/directory rejection, symlink-to-FIFO and ordinary symlink rejection without target access, hard-link rejection without target mutation, path-swap resistance, bounded oversized-file handling, descriptor-based legacy 0600 repair, owner-only temporary creation, and atomic replacement cleanup.
• Windows opens with FILE_FLAG_OPEN_REPARSE_POINT and validates the opened handle using GetFileType and GetFileInformationByHandle, rejecting directories/reparse points, multiple links, non-disk files, and oversized files before reading.
• Existing Windows destinations are replaced with ReplaceFileW and a unique backup; first creation uses MoveFileExW(MOVEFILE_WRITE_THROUGH). There is no delete-old-then-rename window. Documented partial replacement states restore the backup when the destination disappeared, while temporary files are cleaned on failure.
• save_pin and load_or_create_pin now return Result; the Tauri reset command propagates failure. Reset persists first, commits the in-memory PIN second, and refreshes the server last. Focused tests prove a persistence error leaves memory/server state unchanged and success exposes the committed PIN to refresh.

Verification

• Unix PIN persistence: 13/13; focused reset transaction: 2/2.
• Local Rust-only backend harness: 119/119; exact-head Windows backend harness: 130/130, including all 7 Windows PIN tests. Windows MSVC cross-target backend check passed.
• Current merge-tree Rust library suite: 687/687; cargo check --locked --lib passed.
• npm run build, PIN security contract, 9 standalone TypeScript tests, macOS capsule, Windows UI/startup, Android updater, and settings-modal contracts passed.
• npm audit: 0 vulnerabilities. cargo audit: 0 vulnerabilities and 18 repository-allowed warnings. Gitleaks found no leaks across all four PR commits. git diff --check passed.
• GitNexus reports low impact, 55 changed symbols, and 0 affected execution flows.
• All five exact-head GitHub checks are green: Android cargo check, Linux checks, Windows checks, macOS checks, and pr_agent_job.

Exact-head CI ran against the PR's recorded base 9991b325...; beta later advanced by one commit. The local build, cross-checks, and full suites above were run on the current merge tree. The PR remains Draft/Open and was not merged. Because the authenticated account is the PR author, this is submitted as COMMENTED; the semantic verdict is APPROVE.

[github-actions]

Persistent review updated to latest commit d5b3d09