From 65b2bad085c666ac3387891f80b40fe127ff8862 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 19:44:31 +0200 Subject: [PATCH 1/9] docs(win): Phase 2 implementation plan (privilege elevation) --- ...26-06-26-windows-port-phase-2-elevation.md | 402 ++++++++++++++++++ 1 file changed, 402 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-26-windows-port-phase-2-elevation.md diff --git a/docs/superpowers/plans/2026-06-26-windows-port-phase-2-elevation.md b/docs/superpowers/plans/2026-06-26-windows-port-phase-2-elevation.md new file mode 100644 index 0000000..da60c9f --- /dev/null +++ b/docs/superpowers/plans/2026-06-26-windows-port-phase-2-elevation.md @@ -0,0 +1,402 @@ +# Windows Port — Phase 2: Privilege Elevation — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:subagent-driven-development. Steps use checkbox (`- [ ]`). + +> **Plan series.** Phase **2 of 8**. Phases 0–1 merged to `master`. Spec: +> `docs/superpowers/specs/2026-06-26-windows-port-design.md` §4. Branch: `feat/win-phase-2`. + +**Goal:** Privileged cleanup (root-owned items, e.g. `%WINDIR%\Temp`) works on Windows: the un-elevated UI relaunches itself **headless + elevated** to execute the validated plan, with one UAC prompt per batch. + +**Architecture:** Mirror the macOS osascript-admin pattern with PowerShell instead of raw WinAPI. The un-elevated app stages the root-only `DeletionPlan` to a temp file and relaunches its own exe via `powershell Start-Process -Verb RunAs -Wait` in a new `--apply` headless mode. The elevated child re-validates the plan against hard-coded Windows root zones and deletes, writing a report file the parent reads. Validation/execution is unified in a shared `core_trash::execute_root_plan` used by BOTH the Linux/macOS `privhelper` binary and the Windows in-process executor — single source of truth for the allowlist. No `unsafe`, no `windows` crate, no path-with-spaces in arguments (only the parent PID is passed; both sides derive `%TEMP%\fyd-apply--{plan,report}.json`). + +**Tech Stack:** Rust, `core-trash`/`core-ipc`, `std::process::Command` (powershell), filesystem IPC. + +## Global Constraints + +- **Target:** Windows 10 (1803+)/11 x64. Do NOT regress Linux or macOS. +- **License:** keep `// SPDX-License-Identifier: GPL-3.0-or-later` on every edited/new file. +- **cfg rule:** split any touched `#[cfg(not(target_os="macos"))]` into explicit `linux`/`windows` arms. +- **Security:** the elevated child re-validates EVERY path against hard-coded root zones and refuses the whole batch on any escape (all-or-nothing) — identical guarantee to `privhelper`. The shared `execute_root_plan` enforces this; callers must not bypass it. +- **No new crate dependency** in this phase (elevation via `powershell`, not the `windows` crate). +- **Elevation arg:** pass ONLY the parent PID (numeric, no spaces). Both parent and child derive the temp file paths from it. Single-quote the exe path in the PowerShell command (doubling embedded single quotes). +- **Verification:** MANDATORY local gate per task = `cargo fmt --all --check` + `cargo clippy -p freeyourdisk --all-targets -- -D warnings` + `cargo test -p core-trash -p freeyourdisk-helper` GREEN. Windows-only code (`#[cfg(target_os="windows")]`) is NOT compiled locally — it is authoritatively gated by the `windows` CI job on the PR; write it carefully. + +--- + +### Task 1: Shared `execute_root_plan` in core-trash; refactor privhelper to use it + +Unify the "validate every path against root zones (all-or-nothing) then delete" logic so the Linux/macOS helper and the future Windows executor share one implementation. + +**Files:** +- Modify: `crates/core-trash/src/lib.rs` (add `execute_root_plan`; it already exports `validate`, `to_trash`, `delete_permanent`, `Zones`) +- Modify: `crates/core-trash/Cargo.toml` (confirm `core-ipc` dep — it is already used via `ExecutionReport` return types; add it only if missing) +- Modify: `crates/privhelper/src/main.rs:251-281` (call the shared fn) + +**Interfaces:** +- Produces: `core_trash::execute_root_plan(plan: &core_ipc::DeletionPlan, zones: &Zones) -> Result` — `Ok` = executed report; `Err` = refusal report (nothing deleted, all-or-nothing). Consumed by privhelper (Task 1) and the Windows executor (Task 2). + +- [ ] **Step 1: Confirm core-trash depends on core-ipc** + +Run: `grep -n "core-ipc" crates/core-trash/Cargo.toml` +Expected: a `core-ipc = { path = "../core-ipc" }` line. If absent, add it under `[dependencies]` (it is needed for `DeletionPlan`/`Destination`/`ExecutionReport`/`ItemError`). (The crate already returns `ExecutionReport` from `to_trash`, so it is almost certainly present.) + +- [ ] **Step 2: Write a failing test for `execute_root_plan`** + +Add to the `#[cfg(test)] mod tests` in `crates/core-trash/src/lib.rs`: + +```rust + #[test] + fn execute_root_plan_refuses_whole_batch_on_escape() { + let zone = tempfile::tempdir().unwrap(); + let inside = zone.path().join("junk.tmp"); + std::fs::write(&inside, b"x").unwrap(); + let outside = tempfile::tempdir().unwrap(); + let escape = outside.path().join("keep.txt"); + std::fs::write(&escape, b"important").unwrap(); + + let zones = Zones(vec![zone.path().to_path_buf()]); + let plan = core_ipc::DeletionPlan { + items: vec![ + core_ipc::ScanItem { + id: inside.to_string_lossy().into_owned(), + path: inside.clone(), + size_bytes: 1, + last_access: None, + kind: core_ipc::ItemKind::File, + requires_root: true, + }, + core_ipc::ScanItem { + id: escape.to_string_lossy().into_owned(), + path: escape.clone(), + size_bytes: 9, + last_access: None, + kind: core_ipc::ItemKind::File, + requires_root: true, + }, + ], + destination: core_ipc::Destination::Permanent, + total_bytes: 10, + requires_root: true, + }; + + let result = execute_root_plan(&plan, &zones); + assert!(result.is_err(), "any escape must refuse the whole batch"); + assert!(inside.exists(), "nothing deleted on refusal"); + assert!(escape.exists(), "the out-of-zone file must never be touched"); + } + + #[test] + fn execute_root_plan_deletes_when_all_in_zone() { + let zone = tempfile::tempdir().unwrap(); + let f = zone.path().join("junk.tmp"); + std::fs::write(&f, vec![0u8; 50]).unwrap(); + let zones = Zones(vec![zone.path().to_path_buf()]); + let plan = core_ipc::DeletionPlan { + items: vec![core_ipc::ScanItem { + id: f.to_string_lossy().into_owned(), + path: f.clone(), + size_bytes: 50, + last_access: None, + kind: core_ipc::ItemKind::File, + requires_root: true, + }], + destination: core_ipc::Destination::Permanent, + total_bytes: 50, + requires_root: true, + }; + let result = execute_root_plan(&plan, &zones); + assert!(result.is_ok()); + assert!(!f.exists(), "in-zone file deleted"); + } +``` + +(If `core_ipc` test types need importing, add `use core_ipc::...` at the top of the test module or qualify as above.) + +- [ ] **Step 3: Run the test — expect FAIL (function not defined)** + +Run: `cargo test -p core-trash execute_root_plan 2>&1 | tail -15` +Expected: compile error `cannot find function execute_root_plan`. + +- [ ] **Step 4: Implement `execute_root_plan`** + +Add to `crates/core-trash/src/lib.rs` (public API, near `to_trash`/`delete_permanent`). Adjust the `use`/paths to match the crate's existing imports: + +```rust +/// Validate every path in `plan` against `zones` (all-or-nothing) and, if none +/// escape, execute the deletion. `Ok` = executed report; `Err` = refusal report +/// (nothing deleted). Shared by the privileged helper (Linux/macOS) and the +/// Windows elevated executor so the allowlist has a single source of truth. +pub fn execute_root_plan( + plan: &core_ipc::DeletionPlan, + zones: &Zones, +) -> Result { + use std::path::PathBuf; + let paths: Vec = plan.items.iter().map(|item| item.path.clone()).collect(); + + let refusals: Vec = paths + .iter() + .filter_map(|path| match validate(path, zones) { + Ok(_) => None, + Err(err) => Some(core_ipc::ItemError { + path: path.clone(), + message: err.to_string(), + }), + }) + .collect(); + + if !refusals.is_empty() { + return Err(core_ipc::ExecutionReport { + freed_bytes: 0, + deleted_count: 0, + errors: refusals, + }); + } + + let report = match plan.destination { + core_ipc::Destination::Trash => to_trash(&paths, zones), + core_ipc::Destination::Permanent => delete_permanent(&paths, zones), + }; + Ok(report) +} +``` + +- [ ] **Step 5: Run the tests — expect PASS** + +Run: `cargo test -p core-trash 2>&1 | tail -15` +Expected: all pass, including the two new tests. + +- [ ] **Step 6: Refactor privhelper to call the shared fn (preserve exit codes)** + +In `crates/privhelper/src/main.rs`, replace the block from `let zones = Zones(...)` through the end of `main` (lines ~251-280, the pre-validate loop + execute) with: + +```rust + let zones = Zones(ROOT_ZONES.iter().map(PathBuf::from).collect()); + match core_trash::execute_root_plan(&plan, &zones) { + Ok(report) => { + write_report(&report); + ExitCode::SUCCESS + } + Err(refusal) => { + write_report(&refusal); + ExitCode::from(3) + } + } +``` + +Remove now-unused imports (`validate`, `Destination`, `ItemError`, `to_trash`, `delete_permanent` if no longer referenced elsewhere in the file — `smart`/`install-deps` paths don't use them). Let clippy guide which imports to drop. + +- [ ] **Step 7: Verify privhelper + workspace** + +Run: `cargo fmt --all && cargo clippy -p freeyourdisk --all-targets -- -D warnings && cargo test -p core-trash -p freeyourdisk-helper 2>&1 | tail -20` +Expected: GREEN; existing privhelper validation tests still pass (behavior unchanged: same stdout report, exit 3 on refusal, 0 on success). If clippy errors about the Tauri dist, run `pnpm --dir ui install --frozen-lockfile && pnpm --dir ui build` once. + +- [ ] **Step 8: Commit** + +```bash +git add crates/core-trash/src/lib.rs crates/core-trash/Cargo.toml crates/privhelper/src/main.rs +git commit -m "feat: core_trash::execute_root_plan shared validator; privhelper uses it" +``` + +--- + +### Task 2: Windows elevated `--apply` executor in main.rs + +The callee side: a new headless mode the elevated child runs. It reads the staged plan, executes it against hard-coded Windows root zones via `execute_root_plan`, and writes the report. No WebView is created (arg dispatch happens before `tauri::Builder`). + +**Files:** +- Modify: `src-tauri/src/main.rs` (add `--apply` dispatch before the Tauri builder) +- Modify: `src-tauri/src/headless.rs` (add the `apply_elevated` function) OR add a small `src-tauri/src/elevated.rs` module. Use `headless.rs` to keep related code together. + +**Interfaces:** +- Consumes: `core_trash::execute_root_plan` (Task 1), `core_ipc::DeletionPlan`. +- Produces: `headless::apply_elevated(token: &str) -> i32` (process exit code). `main` calls it for `--apply ` on Windows. + +- [ ] **Step 1: Add `apply_elevated` to headless.rs** + +Add to `src-tauri/src/headless.rs` (Windows-only): + +```rust +/// Windows: the elevated child. Reads the plan staged by the un-elevated parent +/// at `%TEMP%\fyd-apply--plan.json`, re-validates against the hard-coded +/// Windows root zone (`%WINDIR%\Temp`), deletes, and writes the report to +/// `%TEMP%\fyd-apply--report.json`. `token` is the parent PID (no spaces). +#[cfg(target_os = "windows")] +pub fn apply_elevated(token: &str) -> i32 { + use core_trash::Zones; + let tmp = std::env::temp_dir(); + let plan_path = tmp.join(format!("fyd-apply-{token}-plan.json")); + let report_path = tmp.join(format!("fyd-apply-{token}-report.json")); + + let Ok(raw) = std::fs::read_to_string(&plan_path) else { + return 2; + }; + let Ok(plan) = serde_json::from_str::(&raw) else { + return 2; + }; + + // Hard-coded Windows privileged zone (must match temp.rs's requires_root root). + let windir = std::env::var_os("WINDIR") + .map(std::path::PathBuf::from) + .unwrap_or_else(|| std::path::PathBuf::from("C:\\Windows")); + let zones = Zones(vec![windir.join("Temp")]); + + let report = match core_trash::execute_root_plan(&plan, &zones) { + Ok(report) => report, + Err(refusal) => refusal, + }; + let json = serde_json::to_string(&report).unwrap_or_default(); + if std::fs::write(&report_path, json).is_err() { + return 1; + } + 0 +} +``` + +Ensure `core_ipc` and `core_trash` are usable from `src-tauri` (they are workspace deps of `freeyourdisk` already). Add `use core_ipc;` only if needed (the fully-qualified `core_ipc::DeletionPlan` above avoids an import). + +- [ ] **Step 2: Dispatch `--apply` in main.rs before the Tauri builder** + +In `src-tauri/src/main.rs`, the `main()` starts with the `--headless` check. Add the Windows `--apply` dispatch right after it: + +```rust + let args: Vec = std::env::args().collect(); + if args.iter().any(|arg| arg == "--headless") { + std::process::exit(headless::run(&args)); + } + #[cfg(target_os = "windows")] + if let Some(token) = args.iter().position(|a| a == "--apply").and_then(|i| args.get(i + 1)) { + std::process::exit(headless::apply_elevated(token)); + } +``` + +(Place the `#[cfg(target_os = "windows")] if let ...` block immediately after the existing `--headless` block, before `taskmgr::raise_priority();`.) + +- [ ] **Step 3: Verify** + +Run: `cargo fmt --all && cargo clippy -p freeyourdisk --all-targets -- -D warnings` +Expected: GREEN. (The `apply_elevated` fn is `#[cfg(target_os="windows")]` so it is not compiled on Linux; the `main.rs` dispatch is also cfg-gated. Read both carefully — only CI compiles them.) + +- [ ] **Step 4: Commit** + +```bash +git add src-tauri/src/headless.rs src-tauri/src/main.rs +git commit -m "feat(win): elevated --apply executor (headless child runs the validated root plan)" +``` + +--- + +### Task 3: Windows elevation in execute.rs (PowerShell RunAs) + +The caller side: replace the Linux `pkexec` helper invocation with a Windows arm that relaunches the app elevated via PowerShell and reads the report file. Split the touched `#[cfg(not(target_os="macos"))]` functions into `linux` + `windows`. + +**Files:** +- Modify: `src-tauri/src/execute.rs` (`pkexec_helper`, `pkexec_smart` — split `not(macos)` → `linux`/`windows`) + +**Interfaces:** +- Consumes: the elevated `--apply` executor (Task 2) via self-relaunch; `execute::execute_plan`'s `invoke_helper` closure already routes the root plan here. +- Produces: `#[cfg(target_os="windows")] pub fn pkexec_helper(plan: &DeletionPlan) -> ExecutionReport` and a `#[cfg(target_os="windows")] pub fn pkexec_smart(devices: &[String]) -> Vec` (stub — real Windows SMART is Phase 3). Same signatures as the Linux/macOS arms so `commands.rs` callers are unchanged. + +- [ ] **Step 1: Split `pkexec_helper` — change the Linux attribute and add the Windows arm** + +In `src-tauri/src/execute.rs`, change the existing `#[cfg(not(target_os = "macos"))]` on `pub fn pkexec_helper` (the pkexec version, ~line 117) to `#[cfg(target_os = "linux")]`. Then add this Windows arm immediately after it: + +```rust +/// Windows: relaunch THIS exe elevated (UAC) in headless `--apply` mode to run +/// the root plan. Only the parent PID is passed as an argument (no spaces); both +/// sides derive `%TEMP%\fyd-apply--{plan,report}.json`. Elevation uses +/// `powershell Start-Process -Verb RunAs -Wait` — the WinAPI-free analogue of the +/// macOS osascript-admin path. +#[cfg(target_os = "windows")] +pub fn pkexec_helper(plan: &DeletionPlan) -> ExecutionReport { + let json = match serde_json::to_string(plan) { + Ok(json) => json, + Err(err) => return err_report(plan, &err.to_string()), + }; + let token = std::process::id().to_string(); + let tmp = std::env::temp_dir(); + let plan_path = tmp.join(format!("fyd-apply-{token}-plan.json")); + let report_path = tmp.join(format!("fyd-apply-{token}-report.json")); + let _ = std::fs::remove_file(&report_path); + if std::fs::write(&plan_path, &json).is_err() { + return err_report(plan, "failed to stage deletion plan"); + } + + let exe = match std::env::current_exe() { + Ok(exe) => exe, + Err(err) => { + let _ = std::fs::remove_file(&plan_path); + return err_report(plan, &format!("cannot locate exe: {err}")); + } + }; + // Single-quote the exe path for PowerShell, doubling any embedded single quote. + let exe_ps = exe.to_string_lossy().replace('\'', "''"); + let ps = format!( + "Start-Process -FilePath '{exe_ps}' -ArgumentList '--apply','{token}' -Verb RunAs -Wait -WindowStyle Hidden" + ); + + let status = Command::new("powershell") + .args(["-NoProfile", "-NonInteractive", "-Command", &ps]) + .status(); + + let result = match status { + Ok(s) if s.success() => std::fs::read_to_string(&report_path) + .ok() + .and_then(|raw| serde_json::from_str(&raw).ok()) + .unwrap_or_else(|| err_report(plan, "elevated helper returned no report")), + Ok(_) => err_report(plan, "elevation cancelled or failed"), + Err(err) => err_report(plan, &format!("failed to launch elevation: {err}")), + }; + let _ = std::fs::remove_file(&plan_path); + let _ = std::fs::remove_file(&report_path); + result +} +``` + +- [ ] **Step 2: Split `pkexec_smart` — Linux arm + Windows stub** + +Change the existing `#[cfg(not(target_os = "macos"))]` on `pub fn pkexec_smart` (~line 153) to `#[cfg(target_os = "linux")]`. Add after it: + +```rust +/// Windows SMART is implemented in Phase 3 (bundled smartctl.exe via the elevated +/// executor). Until then, return no SMART data (the UI degrades gracefully). +#[cfg(target_os = "windows")] +pub fn pkexec_smart(_devices: &[String]) -> Vec { + Vec::new() +} +``` + +- [ ] **Step 3: Check the `InstallReport` import and `pkexec_install_deps`** + +`pkexec_install_deps` and the top `use core_ipc::InstallReport;` are `#[cfg(not(target_os = "macos"))]`. They compile on Windows (Command-based) and are only reached at runtime when `detect_manager()` returns `Some`, which is `None` on Windows — so they are inert on Windows. Leave them as `not(macos)` for this phase (Windows SMART-tool install is Phase 3). Do NOT change them. Confirm via clippy that nothing references a now-missing symbol. + +- [ ] **Step 4: Verify** + +Run: `cargo fmt --all && cargo clippy -p freeyourdisk --all-targets -- -D warnings` +Expected: GREEN on Linux (the Windows `pkexec_helper`/`pkexec_smart` arms are cfg-gated and not compiled here; the Linux arms are unchanged in behavior). Read the Windows arm carefully — CI compiles it. + +- [ ] **Step 5: Commit** + +```bash +git add src-tauri/src/execute.rs +git commit -m "feat(win): elevate via PowerShell RunAs self-relaunch (--apply); SMART stub for Phase 3" +``` + +--- + +## Self-Review + +**1. Spec coverage (Phase 2 = spec §4 elevation):** +- Whole-process elevation realized as headless `--apply` self-relaunch (no separate helper on Windows) — Tasks 2+3. ✓ +- UI never elevated; elevated child is headless (arg dispatch before Tauri builder) — Task 2. ✓ +- Plan staged to temp file; report read back (pipes don't cross UAC) — Task 3. ✓ +- One UAC per privileged batch (`Start-Process -Verb RunAs -Wait`) — Task 3. ✓ +- Single source of truth for the allowlist (`execute_root_plan`, used by privhelper + Windows executor) — Task 1. ✓ +- `privhelper` not bundled on Windows: unchanged (it's a separate binary built only for Linux/macOS bundles; not invoked on Windows). ✓ + +**2. Placeholder scan:** No TBD. `pkexec_smart` Windows returns `Vec::new()` — a deliberate Phase-3 stub (documented), not a placeholder. + +**3. Type consistency:** `execute_root_plan(&DeletionPlan, &Zones) -> Result` (Task 1) is consumed by privhelper (Task 1 Step 6) and `apply_elevated` (Task 2). `pkexec_helper(&DeletionPlan) -> ExecutionReport` / `pkexec_smart(&[String]) -> Vec` keep identical signatures across all OS arms, so `execute_plan` and `commands.rs` callers are unchanged. The parent (Task 3) and child (Task 2) agree on the temp path format `fyd-apply--{plan,report}.json` and the token = parent PID. + +## Notes for later phases +- Phase 3: real Windows SMART — bundle `smartctl.exe`, add a `--smart` elevated mode (same self-relaunch pattern), implement the Windows `pkexec_smart` arm. +- Assumption: elevated child shares the parent's `%TEMP%` (same-user UAC elevation — the standard case). If a separate admin account is used, the temp dir would differ; revisit if reported. From 7660a2cda6b552712e3b5738c32ad63651b9df86 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 19:59:36 +0200 Subject: [PATCH 2/9] feat: core_trash::execute_root_plan shared validator; privhelper uses it --- crates/core-trash/src/lib.rs | 103 ++++++++++++++++++++++++++++++++++ crates/privhelper/src/main.rs | 43 +++++--------- 2 files changed, 116 insertions(+), 30 deletions(-) diff --git a/crates/core-trash/src/lib.rs b/crates/core-trash/src/lib.rs index 0425075..10456c4 100644 --- a/crates/core-trash/src/lib.rs +++ b/crates/core-trash/src/lib.rs @@ -98,6 +98,42 @@ pub fn delete_permanent(paths: &[PathBuf], zones: &Zones) -> ExecutionReport { execute(paths, zones, |real| remove(real).map_err(|e| e.to_string())) } +/// Validate every path in `plan` against `zones` (all-or-nothing) and, if none +/// escape, execute the deletion. `Ok` = executed report; `Err` = refusal report +/// (nothing deleted). Shared by the privileged helper (Linux/macOS) and the +/// Windows elevated executor so the allowlist has a single source of truth. +pub fn execute_root_plan( + plan: &core_ipc::DeletionPlan, + zones: &Zones, +) -> Result { + let paths: Vec = plan.items.iter().map(|item| item.path.clone()).collect(); + + let refusals: Vec = paths + .iter() + .filter_map(|path| match validate(path, zones) { + Ok(_) => None, + Err(err) => Some(core_ipc::ItemError { + path: path.clone(), + message: err.to_string(), + }), + }) + .collect(); + + if !refusals.is_empty() { + return Err(core_ipc::ExecutionReport { + freed_bytes: 0, + deleted_count: 0, + errors: refusals, + }); + } + + let report = match plan.destination { + core_ipc::Destination::Trash => to_trash(&paths, zones), + core_ipc::Destination::Permanent => delete_permanent(&paths, zones), + }; + Ok(report) +} + fn execute( paths: &[PathBuf], zones: &Zones, @@ -209,4 +245,71 @@ mod tests { assert_eq!(report.errors.len(), 1); assert!(victim.exists()); } + + #[test] + fn execute_root_plan_refuses_whole_batch_on_escape() { + let zone = tempfile::tempdir().unwrap(); + let inside = zone.path().join("junk.tmp"); + std::fs::write(&inside, b"x").unwrap(); + let outside = tempfile::tempdir().unwrap(); + let escape = outside.path().join("keep.txt"); + std::fs::write(&escape, b"important").unwrap(); + + let zones = Zones(vec![zone.path().to_path_buf()]); + let plan = core_ipc::DeletionPlan { + items: vec![ + core_ipc::ScanItem { + id: inside.to_string_lossy().into_owned(), + path: inside.clone(), + size_bytes: 1, + last_access: None, + kind: core_ipc::ItemKind::File, + requires_root: true, + }, + core_ipc::ScanItem { + id: escape.to_string_lossy().into_owned(), + path: escape.clone(), + size_bytes: 9, + last_access: None, + kind: core_ipc::ItemKind::File, + requires_root: true, + }, + ], + destination: core_ipc::Destination::Permanent, + total_bytes: 10, + requires_root: true, + }; + + let result = execute_root_plan(&plan, &zones); + assert!(result.is_err(), "any escape must refuse the whole batch"); + assert!(inside.exists(), "nothing deleted on refusal"); + assert!( + escape.exists(), + "the out-of-zone file must never be touched" + ); + } + + #[test] + fn execute_root_plan_deletes_when_all_in_zone() { + let zone = tempfile::tempdir().unwrap(); + let f = zone.path().join("junk.tmp"); + std::fs::write(&f, vec![0u8; 50]).unwrap(); + let zones = Zones(vec![zone.path().to_path_buf()]); + let plan = core_ipc::DeletionPlan { + items: vec![core_ipc::ScanItem { + id: f.to_string_lossy().into_owned(), + path: f.clone(), + size_bytes: 50, + last_access: None, + kind: core_ipc::ItemKind::File, + requires_root: true, + }], + destination: core_ipc::Destination::Permanent, + total_bytes: 50, + requires_root: true, + }; + let result = execute_root_plan(&plan, &zones); + assert!(result.is_ok()); + assert!(!f.exists(), "in-zone file deleted"); + } } diff --git a/crates/privhelper/src/main.rs b/crates/privhelper/src/main.rs index a5f09c2..d42856f 100644 --- a/crates/privhelper/src/main.rs +++ b/crates/privhelper/src/main.rs @@ -11,8 +11,8 @@ //! //! Exit codes: 0 = success, 2 = invalid input, 3 = a path was refused. -use core_ipc::{DeletionPlan, Destination, ExecutionReport, InstallReport, ItemError, SmartInfo}; -use core_trash::{delete_permanent, to_trash, validate, Zones}; +use core_ipc::{DeletionPlan, ExecutionReport, InstallReport, SmartInfo}; +use core_trash::Zones; use std::io::{Read, Write}; use std::path::PathBuf; use std::process::{Command, ExitCode}; @@ -249,33 +249,16 @@ fn main() -> ExitCode { }; let zones = Zones(ROOT_ZONES.iter().map(PathBuf::from).collect()); - let paths: Vec = plan.items.iter().map(|item| item.path.clone()).collect(); - - // Pre-validate every path; refuse the whole batch on any escape. - let refusals: Vec = paths - .iter() - .filter_map(|path| match validate(path, &zones) { - Ok(_) => None, - Err(err) => Some(ItemError { - path: path.clone(), - message: err.to_string(), - }), - }) - .collect(); - - if !refusals.is_empty() { - write_report(&ExecutionReport { - freed_bytes: 0, - deleted_count: 0, - errors: refusals, - }); - return ExitCode::from(3); + // Validation + execution is the shared single source of truth (also used by + // the Windows elevated executor). All-or-nothing: any escape refuses the batch. + match core_trash::execute_root_plan(&plan, &zones) { + Ok(report) => { + write_report(&report); + ExitCode::SUCCESS + } + Err(refusal) => { + write_report(&refusal); + ExitCode::from(3) + } } - - let report = match plan.destination { - Destination::Trash => to_trash(&paths, &zones), - Destination::Permanent => delete_permanent(&paths, &zones), - }; - write_report(&report); - ExitCode::SUCCESS } From 318fd77d6a22312b93a2c858f3f882303d868b20 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:07:26 +0200 Subject: [PATCH 3/9] feat(win): elevated --apply executor (headless child runs the validated root plan) --- src-tauri/src/headless.rs | 35 +++++++++++++++++++++++++++++++++++ src-tauri/src/main.rs | 8 ++++++++ 2 files changed, 43 insertions(+) diff --git a/src-tauri/src/headless.rs b/src-tauri/src/headless.rs index c100494..8dc0970 100644 --- a/src-tauri/src/headless.rs +++ b/src-tauri/src/headless.rs @@ -131,6 +131,41 @@ pub fn run(args: &[String]) -> i32 { 0 } +/// Windows: the elevated child. Reads the plan staged by the un-elevated parent +/// at `%TEMP%\fyd-apply--plan.json`, re-validates against the hard-coded +/// Windows root zone (`%WINDIR%\Temp`), deletes, and writes the report to +/// `%TEMP%\fyd-apply--report.json`. `token` is the parent PID (no spaces). +#[cfg(target_os = "windows")] +pub fn apply_elevated(token: &str) -> i32 { + use core_trash::Zones; + let tmp = std::env::temp_dir(); + let plan_path = tmp.join(format!("fyd-apply-{token}-plan.json")); + let report_path = tmp.join(format!("fyd-apply-{token}-report.json")); + + let Ok(raw) = std::fs::read_to_string(&plan_path) else { + return 2; + }; + let Ok(plan) = serde_json::from_str::(&raw) else { + return 2; + }; + + // Hard-coded Windows privileged zone (must match temp.rs's requires_root root). + let windir = std::env::var_os("WINDIR") + .map(std::path::PathBuf::from) + .unwrap_or_else(|| std::path::PathBuf::from("C:\\Windows")); + let zones = Zones(vec![windir.join("Temp")]); + + let report = match core_trash::execute_root_plan(&plan, &zones) { + Ok(report) => report, + Err(refusal) => refusal, + }; + let json = serde_json::to_string(&report).unwrap_or_default(); + if std::fs::write(&report_path, json).is_err() { + return 1; + } + 0 +} + #[cfg(test)] mod tests { use super::*; diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index 3836cae..1e7f433 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -28,6 +28,14 @@ fn main() { if args.iter().any(|arg| arg == "--headless") { std::process::exit(headless::run(&args)); } + #[cfg(target_os = "windows")] + if let Some(token) = args + .iter() + .position(|a| a == "--apply") + .and_then(|i| args.get(i + 1)) + { + std::process::exit(headless::apply_elevated(token)); + } // Stay alive and responsive under memory pressure (best effort). taskmgr::raise_priority(); From 3819c7cc5efcaa9c60edc320c4f95046345059d7 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:12:08 +0200 Subject: [PATCH 4/9] fix(win/security): reject non-numeric --apply token (path-traversal hardening, elevated process) --- src-tauri/src/headless.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src-tauri/src/headless.rs b/src-tauri/src/headless.rs index 8dc0970..a267fb4 100644 --- a/src-tauri/src/headless.rs +++ b/src-tauri/src/headless.rs @@ -138,6 +138,12 @@ pub fn run(args: &[String]) -> i32 { #[cfg(target_os = "windows")] pub fn apply_elevated(token: &str) -> i32 { use core_trash::Zones; + // Hardening: this runs ELEVATED. `token` (the parent PID) is interpolated + // into a %TEMP% path — reject anything but ASCII digits so a crafted token + // can never traverse out of %TEMP% (arbitrary admin file read/write). + if token.is_empty() || !token.bytes().all(|b| b.is_ascii_digit()) { + return 2; + } let tmp = std::env::temp_dir(); let plan_path = tmp.join(format!("fyd-apply-{token}-plan.json")); let report_path = tmp.join(format!("fyd-apply-{token}-report.json")); From b02e4e4d1914e0dcfc004edad0bccd2f3d19c7fb Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:18:52 +0200 Subject: [PATCH 5/9] feat(win): elevate via PowerShell RunAs self-relaunch (--apply); SMART stub for Phase 3 --- src-tauri/src/execute.rs | 61 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/execute.rs b/src-tauri/src/execute.rs index 6ea8efe..ec0f11c 100644 --- a/src-tauri/src/execute.rs +++ b/src-tauri/src/execute.rs @@ -114,7 +114,7 @@ fn err_report(plan: &DeletionPlan, message: &str) -> ExecutionReport { } /// Real helper invocation: `pkexec `, plan on stdin, report on stdout. -#[cfg(not(target_os = "macos"))] +#[cfg(target_os = "linux")] pub fn pkexec_helper(plan: &DeletionPlan) -> ExecutionReport { let json = match serde_json::to_string(plan) { Ok(json) => json, @@ -148,9 +148,59 @@ pub fn pkexec_helper(plan: &DeletionPlan) -> ExecutionReport { } } +/// Windows: relaunch THIS exe elevated (UAC) in headless `--apply` mode to run +/// the root plan. Only the parent PID is passed as an argument (no spaces); both +/// sides derive `%TEMP%\fyd-apply--{plan,report}.json`. Elevation uses +/// `powershell Start-Process -Verb RunAs -Wait` — the WinAPI-free analogue of the +/// macOS osascript-admin path. +#[cfg(target_os = "windows")] +pub fn pkexec_helper(plan: &DeletionPlan) -> ExecutionReport { + let json = match serde_json::to_string(plan) { + Ok(json) => json, + Err(err) => return err_report(plan, &err.to_string()), + }; + let token = std::process::id().to_string(); + let tmp = std::env::temp_dir(); + let plan_path = tmp.join(format!("fyd-apply-{token}-plan.json")); + let report_path = tmp.join(format!("fyd-apply-{token}-report.json")); + let _ = std::fs::remove_file(&report_path); + if std::fs::write(&plan_path, &json).is_err() { + return err_report(plan, "failed to stage deletion plan"); + } + + let exe = match std::env::current_exe() { + Ok(exe) => exe, + Err(err) => { + let _ = std::fs::remove_file(&plan_path); + return err_report(plan, &format!("cannot locate exe: {err}")); + } + }; + // Single-quote the exe path for PowerShell, doubling any embedded single quote. + let exe_ps = exe.to_string_lossy().replace('\'', "''"); + let ps = format!( + "Start-Process -FilePath '{exe_ps}' -ArgumentList '--apply','{token}' -Verb RunAs -Wait -WindowStyle Hidden" + ); + + let status = Command::new("powershell") + .args(["-NoProfile", "-NonInteractive", "-Command", &ps]) + .status(); + + let result = match status { + Ok(s) if s.success() => std::fs::read_to_string(&report_path) + .ok() + .and_then(|raw| serde_json::from_str(&raw).ok()) + .unwrap_or_else(|| err_report(plan, "elevated helper returned no report")), + Ok(_) => err_report(plan, "elevation cancelled or failed"), + Err(err) => err_report(plan, &format!("failed to launch elevation: {err}")), + }; + let _ = std::fs::remove_file(&plan_path); + let _ = std::fs::remove_file(&report_path); + result +} + /// Read SMART for every device in one privileged call. Returns an empty vec if /// pkexec is cancelled or the helper/smartctl is unavailable (graceful). -#[cfg(not(target_os = "macos"))] +#[cfg(target_os = "linux")] pub fn pkexec_smart(devices: &[String]) -> Vec { if devices.is_empty() { return Vec::new(); @@ -166,6 +216,13 @@ pub fn pkexec_smart(devices: &[String]) -> Vec { } } +/// Windows SMART is implemented in Phase 3 (bundled smartctl.exe via the elevated +/// executor). Until then, return no SMART data (the UI degrades gracefully). +#[cfg(target_os = "windows")] +pub fn pkexec_smart(_devices: &[String]) -> Vec { + Vec::new() +} + // --------------------------------------------------------------------------- // macOS: privilege escalation via the native auth dialog // (`osascript … with administrator privileges`). The plan is staged to a temp From bb2995b93168cc2e84b9677f7d20267fee84a547 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:29:35 +0200 Subject: [PATCH 6/9] fix(win): gate Write/Stdio imports to cfg(linux) (unused on Windows/macOS after pkexec_helper split) --- src-tauri/src/execute.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/execute.rs b/src-tauri/src/execute.rs index ec0f11c..54093c8 100644 --- a/src-tauri/src/execute.rs +++ b/src-tauri/src/execute.rs @@ -8,9 +8,14 @@ use core_ipc::InstallReport; use core_ipc::{DeletionPlan, Destination, ExecutionReport, ItemError, ScanItem, SmartInfo}; use core_trash::Zones; +// `Write`/`Stdio` are used only by the Linux pkexec helper (stdin-piped IPC); +// macOS (osascript) and Windows (PowerShell) stage the plan via temp files. +#[cfg(target_os = "linux")] use std::io::Write; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Command; +#[cfg(target_os = "linux")] +use std::process::Stdio; /// Installed location of the privileged helper. pub const HELPER_PATH: &str = "/usr/lib/freeyourdisk/freeyourdisk-helper"; From fa0ffcb24d8269eacd93cfdc81cb244ba2619e33 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:40:50 +0200 Subject: [PATCH 7/9] fix(win/security): hard-coded canonicalized privileged zone (P2 final-review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - apply_elevated: derive the root zone from a hard-coded, canonicalized C:\Windows\Temp instead of %WINDIR% — env vars propagate across same-user UAC elevation (attacker-influenceable) and an env-derived zone is an admin-delete EoP. Canonicalizing also fixes the \?\-verbatim mismatch that made privileged cleanup a silent no-op (zone never matched validate()'s canonical paths). - execute.rs: launch PowerShell by absolute System32 path (anti PATH-poisoning). - main.rs: --apply always terminates (missing token rejected by digit guard), never falls through to launching the elevated GUI. - core-trash: #[cfg(windows)] regression test for canonical zone matching, run on real Windows via a new 'cargo test -p core-trash' CI step (clippy is compile-only). --- .github/workflows/ci.yml | 6 ++++++ crates/core-trash/src/lib.rs | 18 ++++++++++++++++++ src-tauri/src/execute.rs | 4 +++- src-tauri/src/headless.rs | 14 +++++++++----- src-tauri/src/main.rs | 14 +++++++++----- 5 files changed, 45 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef1706b..ca83cd6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,3 +93,9 @@ jobs: - name: clippy (app only — privhelper is Linux/macOS) run: cargo clippy -p freeyourdisk --all-targets -- -D warnings + + # Run the cross-platform core-trash tests on REAL Windows — including the + # zone-canonicalization regression guard for the elevated cleanup path. The + # clippy gate above is compile-only and cannot catch this runtime bug. + - name: test core-trash (Windows runtime) + run: cargo test -p core-trash diff --git a/crates/core-trash/src/lib.rs b/crates/core-trash/src/lib.rs index 10456c4..8ba503d 100644 --- a/crates/core-trash/src/lib.rs +++ b/crates/core-trash/src/lib.rs @@ -312,4 +312,22 @@ mod tests { assert!(result.is_ok()); assert!(!f.exists(), "in-zone file deleted"); } + + // Regression guard for the Phase-2 Windows `\\?\`-verbatim mismatch: + // validate() canonicalizes the candidate to a verbatim path, so a zone must + // ALSO be canonicalized or starts_with() never matches — which would make the + // elevated privileged cleanup a silent no-op. Runs on Windows CI via + // `cargo test -p core-trash`. + #[cfg(windows)] + #[test] + fn validate_accepts_in_canonicalized_zone_on_windows() { + let dir = tempfile::tempdir().unwrap(); + let f = dir.path().join("junk.tmp"); + std::fs::write(&f, b"x").unwrap(); + let zone = Zones(vec![std::fs::canonicalize(dir.path()).unwrap()]); + assert!( + validate(&f, &zone).is_ok(), + "a canonicalized zone must accept an in-zone path" + ); + } } diff --git a/src-tauri/src/execute.rs b/src-tauri/src/execute.rs index 54093c8..f25db33 100644 --- a/src-tauri/src/execute.rs +++ b/src-tauri/src/execute.rs @@ -186,7 +186,9 @@ pub fn pkexec_helper(plan: &DeletionPlan) -> ExecutionReport { "Start-Process -FilePath '{exe_ps}' -ArgumentList '--apply','{token}' -Verb RunAs -Wait -WindowStyle Hidden" ); - let status = Command::new("powershell") + // Absolute path, not a PATH lookup: a PATH-poisoned powershell.exe could + // control the RunAs target shown in the UAC prompt. Fixed on Win10/11. + let status = Command::new("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe") .args(["-NoProfile", "-NonInteractive", "-Command", &ps]) .status(); diff --git a/src-tauri/src/headless.rs b/src-tauri/src/headless.rs index a267fb4..f9723c4 100644 --- a/src-tauri/src/headless.rs +++ b/src-tauri/src/headless.rs @@ -155,11 +155,15 @@ pub fn apply_elevated(token: &str) -> i32 { return 2; }; - // Hard-coded Windows privileged zone (must match temp.rs's requires_root root). - let windir = std::env::var_os("WINDIR") - .map(std::path::PathBuf::from) - .unwrap_or_else(|| std::path::PathBuf::from("C:\\Windows")); - let zones = Zones(vec![windir.join("Temp")]); + // Hard-coded, canonicalized privileged zone. NOT derived from %WINDIR%: env + // vars propagate across same-user UAC elevation and are attacker-influenceable + // (classic windir UAC-bypass vector) — an env-derived zone would be an + // admin-delete EoP. Canonicalize the trusted constant so the zone matches + // validate()'s canonical (\\?\-verbatim) candidate paths; otherwise + // starts_with() never matches and every item is silently refused (no-op). + let zone_base = std::path::Path::new("C:\\Windows\\Temp"); + let zone = std::fs::canonicalize(zone_base).unwrap_or_else(|_| zone_base.to_path_buf()); + let zones = Zones(vec![zone]); let report = match core_trash::execute_root_plan(&plan, &zones) { Ok(report) => report, diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index 1e7f433..fe795ae 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -29,11 +29,15 @@ fn main() { std::process::exit(headless::run(&args)); } #[cfg(target_os = "windows")] - if let Some(token) = args - .iter() - .position(|a| a == "--apply") - .and_then(|i| args.get(i + 1)) - { + if args.iter().any(|a| a == "--apply") { + // `--apply` always terminates here (never falls through to the GUI). A + // missing/invalid token is rejected by apply_elevated's digit guard. + let token = args + .iter() + .position(|a| a == "--apply") + .and_then(|i| args.get(i + 1)) + .map(String::as_str) + .unwrap_or(""); std::process::exit(headless::apply_elevated(token)); } From 2337c23465f11195de760c6cce37556f29e3a074 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:49:59 +0200 Subject: [PATCH 8/9] fix(win): dunce-canonicalize zone matching in core-trash (root cause) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows cargo-test step (added with the security fix) revealed that ALL core-trash zone checks fail on Windows: validate() canonicalizes candidates to \?\-verbatim paths, but every Zones is built from plain paths, so starts_with() never matched — refusing ALL deletions (user cleanup from Phase 1 AND the new privileged cleanup). Root fix: use dunce::canonicalize (strips the verbatim prefix on Windows; delegates to fs::canonicalize on Linux/macOS). apply_elevated now uses a plain hard-coded C:\Windows\Temp zone (matches dunce candidates; stays EoP-safe — not env-derived). Also made validate_rejects_outside_zone cross-platform (was /etc/passwd, Unix-only). --- Cargo.lock | 1 + crates/core-trash/Cargo.toml | 3 +++ crates/core-trash/src/lib.rs | 34 ++++++++++------------------------ src-tauri/src/headless.rs | 16 +++++++--------- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5ce0176..cb85b17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -417,6 +417,7 @@ name = "core-trash" version = "0.4.1" dependencies = [ "core-ipc", + "dunce", "tempfile", "thiserror 2.0.18", "trash", diff --git a/crates/core-trash/Cargo.toml b/crates/core-trash/Cargo.toml index 302232e..92a014e 100644 --- a/crates/core-trash/Cargo.toml +++ b/crates/core-trash/Cargo.toml @@ -10,6 +10,9 @@ repository.workspace = true core-ipc = { path = "../core-ipc" } trash = { workspace = true } thiserror = { workspace = true } +# Normalizes Windows canonical paths (strips the \\?\ verbatim prefix) so zone +# membership (starts_with) works; on Linux/macOS it delegates to fs::canonicalize. +dunce = "1" [dev-dependencies] tempfile = { workspace = true } diff --git a/crates/core-trash/src/lib.rs b/crates/core-trash/src/lib.rs index 8ba503d..188a439 100644 --- a/crates/core-trash/src/lib.rs +++ b/crates/core-trash/src/lib.rs @@ -41,11 +41,11 @@ pub fn validate(path: &Path, zones: &Zones) -> Result { let lexical = match (path.parent(), path.file_name()) { (Some(parent), Some(name)) => { let parent_real = - fs::canonicalize(parent).map_err(|e| TrashError::Io(e.to_string()))?; + dunce::canonicalize(parent).map_err(|e| TrashError::Io(e.to_string()))?; parent_real.join(name) } // path is "/" or ends with "..": resolve it whole. - _ => fs::canonicalize(path).map_err(|e| TrashError::Io(e.to_string()))?, + _ => dunce::canonicalize(path).map_err(|e| TrashError::Io(e.to_string()))?, }; if !zones.contains(&lexical) { @@ -56,7 +56,7 @@ pub fn validate(path: &Path, zones: &Zones) -> Result { .map(|m| m.file_type().is_symlink()) .unwrap_or(false); if is_symlink { - let real = fs::canonicalize(path).map_err(|e| TrashError::Io(e.to_string()))?; + let real = dunce::canonicalize(path).map_err(|e| TrashError::Io(e.to_string()))?; if !zones.contains(&real) { return Err(TrashError::SymlinkEscape(path.to_path_buf())); } @@ -174,10 +174,14 @@ mod tests { #[test] fn validate_rejects_outside_zone() { - let dir = tempfile::tempdir().unwrap(); - let zones = Zones(vec![dir.path().to_path_buf()]); + // Cross-platform: a real file in a different tempdir is out of zone. + let zone_dir = tempfile::tempdir().unwrap(); + let outside_dir = tempfile::tempdir().unwrap(); + let outside = outside_dir.path().join("victim.txt"); + std::fs::write(&outside, b"x").unwrap(); + let zones = Zones(vec![zone_dir.path().to_path_buf()]); assert!(matches!( - validate(Path::new("/etc/passwd"), &zones), + validate(&outside, &zones), Err(TrashError::OutsideZone(_)) )); } @@ -312,22 +316,4 @@ mod tests { assert!(result.is_ok()); assert!(!f.exists(), "in-zone file deleted"); } - - // Regression guard for the Phase-2 Windows `\\?\`-verbatim mismatch: - // validate() canonicalizes the candidate to a verbatim path, so a zone must - // ALSO be canonicalized or starts_with() never matches — which would make the - // elevated privileged cleanup a silent no-op. Runs on Windows CI via - // `cargo test -p core-trash`. - #[cfg(windows)] - #[test] - fn validate_accepts_in_canonicalized_zone_on_windows() { - let dir = tempfile::tempdir().unwrap(); - let f = dir.path().join("junk.tmp"); - std::fs::write(&f, b"x").unwrap(); - let zone = Zones(vec![std::fs::canonicalize(dir.path()).unwrap()]); - assert!( - validate(&f, &zone).is_ok(), - "a canonicalized zone must accept an in-zone path" - ); - } } diff --git a/src-tauri/src/headless.rs b/src-tauri/src/headless.rs index f9723c4..c32032f 100644 --- a/src-tauri/src/headless.rs +++ b/src-tauri/src/headless.rs @@ -155,15 +155,13 @@ pub fn apply_elevated(token: &str) -> i32 { return 2; }; - // Hard-coded, canonicalized privileged zone. NOT derived from %WINDIR%: env - // vars propagate across same-user UAC elevation and are attacker-influenceable - // (classic windir UAC-bypass vector) — an env-derived zone would be an - // admin-delete EoP. Canonicalize the trusted constant so the zone matches - // validate()'s canonical (\\?\-verbatim) candidate paths; otherwise - // starts_with() never matches and every item is silently refused (no-op). - let zone_base = std::path::Path::new("C:\\Windows\\Temp"); - let zone = std::fs::canonicalize(zone_base).unwrap_or_else(|_| zone_base.to_path_buf()); - let zones = Zones(vec![zone]); + // Hard-coded privileged zone — NOT derived from %WINDIR%: env vars propagate + // across same-user UAC elevation and are attacker-influenceable (classic + // windir UAC-bypass vector), so an env-derived zone would be an admin-delete + // EoP. Plain path is correct: core_trash::validate normalizes candidates with + // dunce (no \\?\ verbatim prefix) so a plain zone matches; junctions inside + // the zone are still resolved and refused by validate's symlink check. + let zones = Zones(vec![std::path::PathBuf::from("C:\\Windows\\Temp")]); let report = match core_trash::execute_root_plan(&plan, &zones) { Ok(report) => report, From 02a5c61a7067528ecd7749e95e1d31fa3e914f99 Mon Sep 17 00:00:00 2001 From: rony Date: Fri, 26 Jun 2026 20:54:33 +0200 Subject: [PATCH 9/9] fix(win): canonicalize zones in Zones::contains (match canonical candidates on Windows + macOS /tmp) --- crates/core-trash/src/lib.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/core-trash/src/lib.rs b/crates/core-trash/src/lib.rs index 188a439..2b53c1e 100644 --- a/crates/core-trash/src/lib.rs +++ b/crates/core-trash/src/lib.rs @@ -16,7 +16,16 @@ pub struct Zones(pub Vec); impl Zones { fn contains(&self, path: &Path) -> bool { - self.0.iter().any(|zone| path.starts_with(zone)) + // Compare against each zone's *canonical* form so a plain zone matches a + // canonicalized candidate. `validate` resolves candidates to their real + // location (dunce strips Windows' \\?\ prefix and resolves macOS + // /tmp→/private/tmp); the zone must be resolved the same way or + // starts_with never matches. Falls back to the literal zone if it cannot + // be resolved (e.g. it does not exist on disk). + self.0.iter().any(|zone| { + let zone_real = dunce::canonicalize(zone).unwrap_or_else(|_| zone.clone()); + path.starts_with(&zone_real) + }) } }