From e74185866f76d15253c22b7c3b04fe327d96df8b Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Mon, 7 Sep 2026 19:18:07 +0500 Subject: [PATCH 1/6] fix(scanner): resolve scan roots to absolute paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rules are written against absolute paths, and the ones anchored at the filesystem root are anchored deliberately: `/tmp/**` stops there so it cannot reach `~/tmp/tax-return.pdf`, which is what #41 was about. jwalk builds every entry's path from the root as it was handed in, so `diskern scan tmp` from `/var` produced `tmp/systemd-private/x` and no anchored pattern could match it. The walk found the files and the rules could not tell where they were, so the scan printed "Scanned 1 files" and then nothing to clean — a plausible empty result for a directory full of matches, which is the #82 failure arriving by another route. absolute() rather than canonicalize(): no filesystem access, works on a path that does not exist, and leaves symlinks alone. canonicalize would rewrite /var/tmp to /private/var/tmp on macOS and scan somewhere other than what was asked for. It leaves `..` in place, because a/../b is only b when a is not a symlink; that limit is written down beside the code. Every path the scanner yields is now absolute, which also settles what `actions::quarantine` records as a file's original location — a relative original could not be restored from a different directory. --- crates/diskern-core/src/scanner.rs | 44 +++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/crates/diskern-core/src/scanner.rs b/crates/diskern-core/src/scanner.rs index 3646117..66c1405 100644 --- a/crates/diskern-core/src/scanner.rs +++ b/crates/diskern-core/src/scanner.rs @@ -68,11 +68,35 @@ pub fn scan(opts: &ScanOptions, progress: Arc) -> Result Result { + std::path::absolute(root).map_err(|source| crate::GenomeError::Io { + path: root.to_path_buf(), + source, + }) +} + fn walk_root( root: &Path, opts: &ScanOptions, @@ -275,6 +299,24 @@ mod tests { ); } + /// Issue #103. Anchored rules only match absolute paths, so a relative + /// root has to be resolved before the walk, not after. + #[test] + fn a_relative_root_is_made_absolute() { + let cwd = std::env::current_dir().unwrap(); + assert_eq!(absolute_root(Path::new("tmp")).unwrap(), cwd.join("tmp")); + assert_eq!(absolute_root(Path::new(".")).unwrap(), cwd); + } + + /// An absolute root is already what the rules expect and must survive + /// untouched — in particular `/var/tmp` must not become the symlink + /// target `/private/var/tmp` that `canonicalize` would produce on macOS. + #[test] + fn an_absolute_root_is_left_alone() { + let root = Path::new("/var/tmp"); + assert_eq!(absolute_root(root).unwrap(), root); + } + #[test] fn scans_a_temp_tree() { let dir = tempfile::tempdir().unwrap(); From 2e6d799fa6fa69f6a8814e060c7613184ff47a37 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Mon, 7 Sep 2026 19:18:07 +0500 Subject: [PATCH 2/6] test(cli): pin the relative-root regression to the real binary Changing the process working directory inside a test would race every other test in the same binary, so this drives the installed binary with `current_dir` instead and asks for `.` as the root. The rule it uses is anchored at the filesystem root, like the shipped `/tmp/**`, because an unanchored pattern matches either way and would pass with the bug still present. Verified against the unfixed scanner: the relative case fails, the absolute control passes. Unix only. Windows paths normalize to `c:/...`, so a `/`-anchored pattern cannot match there and neither can the bug. --- crates/diskern-cli/tests/relative_root.rs | 78 +++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 crates/diskern-cli/tests/relative_root.rs diff --git a/crates/diskern-cli/tests/relative_root.rs b/crates/diskern-cli/tests/relative_root.rs new file mode 100644 index 0000000..90fbd94 --- /dev/null +++ b/crates/diskern-cli/tests/relative_root.rs @@ -0,0 +1,78 @@ +//! Issue #103. A relative scan root produced entries no anchored rule could +//! match, so the scan reported nothing to clean for a directory full of +//! matches. Driving the real binary with a working directory is the only +//! honest way to pin this: changing the current directory inside a test would +//! race every other test in the same binary. + +use serde_json::json; +use std::fs; +use std::process::Command; +use tempfile::tempdir; + +/// A rule anchored at the filesystem root, like the shipped `/tmp/**` and +/// `/var/log/**`. Windows paths normalize to `c:/...`, so a `/`-anchored +/// pattern cannot match there and neither can the bug. +#[cfg(unix)] +fn write_anchored_rules(path: &std::path::Path) { + let rules = json!({ + "version": 1, + "rules": [{ + "id": "anchored-marker", + "patterns": ["/**/*.marker"], + "category": "temp_file", + "verdict": "review", + "description": "Anchored rule for the relative-root regression." + }] + }); + fs::write(path, serde_json::to_vec(&rules).unwrap()).unwrap(); +} + +#[cfg(unix)] +#[test] +fn a_relative_root_still_reaches_anchored_rules() { + let root = tempdir().unwrap(); + fs::write(root.path().join("scratch.marker"), b"temp").unwrap(); + let rules = root.path().join("rules.json"); + write_anchored_rules(&rules); + + // "." as the root, resolved against the child's working directory. + let output = Command::new(env!("CARGO_BIN_EXE_diskern")) + .current_dir(root.path()) + .args(["scan", ".", "--rules", "rules.json", "--top", "0"]) + .output() + .expect("diskern should start"); + + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("matched rule anchored-marker"), "{stdout}"); + // The printed path is the one the manifest would record on quarantine. + assert!(stdout.contains("/scratch.marker"), "{stdout}"); +} + +#[cfg(unix)] +#[test] +fn an_absolute_root_reaches_the_same_rule() { + let root = tempdir().unwrap(); + fs::write(root.path().join("scratch.marker"), b"temp").unwrap(); + let rules = root.path().join("rules.json"); + write_anchored_rules(&rules); + + let output = Command::new(env!("CARGO_BIN_EXE_diskern")) + .args([ + "scan", + &root.path().to_string_lossy(), + "--rules", + &rules.to_string_lossy(), + "--top", + "0", + ]) + .output() + .expect("diskern should start"); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("matched rule anchored-marker"), "{stdout}"); +} From e16e6ebb9ee44c57112cc13759710b03416bf777 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Mon, 7 Sep 2026 19:18:17 +0500 Subject: [PATCH 3/6] docs(changelog): note the relative-root fix First entry under Unreleased since v0.2.0 shipped. --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a068c06..e6bc4b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to Diskern are documented here. The format follows ## [Unreleased] +### Fixed + +- A relative scan root no longer hides every finding a root-anchored rule + would have made. `diskern scan tmp` from `/var` reported nothing to + clean; roots are resolved to absolute paths before the walk + ## [0.2.0] — 2026-09-07 ### Added From 1bc8a0c0069d5707c88fb482cea0d3ec6cbe14cb Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Mon, 7 Sep 2026 19:21:57 +0500 Subject: [PATCH 4/6] test(scanner): state the absolute-root guarantee per platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/var/tmp` is not an absolute path on Windows. It is rooted but carries no drive, so `absolute` resolves it against the current one and returns `D:\var\tmp` — correct behaviour, and what the Windows CI job caught. The guarantee being tested is that a root the rules already understand survives untouched, so each platform states it with a path that is actually absolute there. --- crates/diskern-core/src/scanner.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/diskern-core/src/scanner.rs b/crates/diskern-core/src/scanner.rs index 66c1405..8cb1742 100644 --- a/crates/diskern-core/src/scanner.rs +++ b/crates/diskern-core/src/scanner.rs @@ -311,12 +311,24 @@ mod tests { /// An absolute root is already what the rules expect and must survive /// untouched — in particular `/var/tmp` must not become the symlink /// target `/private/var/tmp` that `canonicalize` would produce on macOS. + #[cfg(unix)] #[test] fn an_absolute_root_is_left_alone() { let root = Path::new("/var/tmp"); assert_eq!(absolute_root(root).unwrap(), root); } + /// The same guarantee on Windows, where it needs a different path to + /// state. `/var/tmp` is not absolute there — it is rooted but has no + /// drive, so `absolute` resolves it against the current one, which is + /// the right answer and not the one this test is about. + #[cfg(windows)] + #[test] + fn an_absolute_root_is_left_alone() { + let root = Path::new(r"C:\Users\example\AppData\Local\Temp"); + assert_eq!(absolute_root(root).unwrap(), root); + } + #[test] fn scans_a_temp_tree() { let dir = tempfile::tempdir().unwrap(); From e5ff1dae321a9a23ac0458c86e5d2d519bc13b82 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Mon, 7 Sep 2026 19:31:52 +0500 Subject: [PATCH 5/6] test(cli): gate the relative-root test at module level Every item carried `#[cfg(unix)]` but the imports did not, so on Windows all four were unused. CI never saw it: the lint job runs clippy on ubuntu, and the Windows test job treats unused imports as warnings. A Windows contributor running the clippy line CONTRIBUTING asks for would have got four errors out of a file they had not touched. --- crates/diskern-cli/tests/relative_root.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/diskern-cli/tests/relative_root.rs b/crates/diskern-cli/tests/relative_root.rs index 90fbd94..133420d 100644 --- a/crates/diskern-cli/tests/relative_root.rs +++ b/crates/diskern-cli/tests/relative_root.rs @@ -3,6 +3,13 @@ //! matches. Driving the real binary with a working directory is the only //! honest way to pin this: changing the current directory inside a test would //! race every other test in the same binary. +//! +//! Unix only, and gated at the module level so the imports go with it: +//! Windows paths normalize to `c:/...`, so a `/`-anchored pattern cannot +//! match there and neither can the bug. Gating each item instead left these +//! imports unused on Windows, which is four errors under the `-D warnings` +//! clippy run CONTRIBUTING asks for. +#![cfg(unix)] use serde_json::json; use std::fs; @@ -10,9 +17,7 @@ use std::process::Command; use tempfile::tempdir; /// A rule anchored at the filesystem root, like the shipped `/tmp/**` and -/// `/var/log/**`. Windows paths normalize to `c:/...`, so a `/`-anchored -/// pattern cannot match there and neither can the bug. -#[cfg(unix)] +/// `/var/log/**`. fn write_anchored_rules(path: &std::path::Path) { let rules = json!({ "version": 1, @@ -27,7 +32,6 @@ fn write_anchored_rules(path: &std::path::Path) { fs::write(path, serde_json::to_vec(&rules).unwrap()).unwrap(); } -#[cfg(unix)] #[test] fn a_relative_root_still_reaches_anchored_rules() { let root = tempdir().unwrap(); @@ -53,7 +57,6 @@ fn a_relative_root_still_reaches_anchored_rules() { assert!(stdout.contains("/scratch.marker"), "{stdout}"); } -#[cfg(unix)] #[test] fn an_absolute_root_reaches_the_same_rule() { let root = tempdir().unwrap(); From 8e0f757931e3f406f43fdd79903e5bdd830d1f24 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Mon, 7 Sep 2026 19:31:52 +0500 Subject: [PATCH 6/6] fix(scanner): refuse a scan root the exclude list covers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making roots absolute brought them into range of the exclude list, which is right — `is_within` matches on whole components, so `/run` covers `/run/user/1000/cache`, and that is what the list is for. But the walk then dropped every entry and the report came back empty, which reads as a clean disk. `$XDG_RUNTIME_DIR` lives under `/run` and does hold caches, so this is reachable rather than theoretical. An empty report that means "I refused to look here" is the same wrong answer as #82 and #103: plausible, silent, and indistinguishable from the real thing. It names the exclude instead. The exclude list is normalized once for the whole scan now rather than per root, since the root check and the walk both need it. --- CHANGELOG.md | 2 + crates/diskern-core/src/lib.rs | 2 + crates/diskern-core/src/scanner.rs | 80 ++++++++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6bc4b3..e302bc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ All notable changes to Diskern are documented here. The format follows - A relative scan root no longer hides every finding a root-anchored rule would have made. `diskern scan tmp` from `/var` reported nothing to clean; roots are resolved to absolute paths before the walk +- Scanning a root inside an excluded directory says so, instead of walking + it to an empty report that reads like a clean disk ## [0.2.0] — 2026-09-07 diff --git a/crates/diskern-core/src/lib.rs b/crates/diskern-core/src/lib.rs index 25a5013..3f105b0 100644 --- a/crates/diskern-core/src/lib.rs +++ b/crates/diskern-core/src/lib.rs @@ -107,6 +107,8 @@ pub enum GenomeError { #[source] source: std::io::Error, }, + #[error("scan root {path} is inside excluded directory {exclude}")] + ExcludedRoot { path: PathBuf, exclude: String }, #[error("scan cancelled")] Cancelled, #[error("rules database error: {0}")] diff --git a/crates/diskern-core/src/scanner.rs b/crates/diskern-core/src/scanner.rs index 8cb1742..b67a237 100644 --- a/crates/diskern-core/src/scanner.rs +++ b/crates/diskern-core/src/scanner.rs @@ -63,16 +63,40 @@ impl ScanProgress { /// so the UI can render results while the scan runs. pub fn scan(opts: &ScanOptions, progress: Arc) -> Result> { let mut out = Vec::new(); + // Normalized once for the whole scan: the exclude list never changes, and + // both the root check below and every directory the walk opens use it. + let excludes: Vec = opts.excludes.iter().map(|e| normalize_exclude(e)).collect(); for root in &opts.roots { if progress.cancelled.load(Ordering::Relaxed) { return Err(crate::GenomeError::Cancelled); } - walk_root(&absolute_root(root)?, opts, &progress, &mut out)?; + let root = absolute_root(root)?; + // Say so, rather than walking a root whose every entry the exclude + // list will drop. `/run/user/` holds real caches and sits under + // the `/run` exclude, so this is reachable — and an empty report is + // indistinguishable from a clean disk, which is the answer issue #103 + // and #82 are both about not giving. + if let Some(exclude) = excluded_by(&root, &excludes) { + return Err(crate::GenomeError::ExcludedRoot { + path: root, + exclude: exclude.to_string(), + }); + } + walk_root(&root, &excludes, opts, &progress, &mut out)?; } Ok(out) } +/// Which exclude, if any, contains `path`. +fn excluded_by<'a>(path: &Path, excludes: &'a [String]) -> Option<&'a str> { + let path = path.to_string_lossy(); + excludes + .iter() + .find(|ex| is_within(&path, ex)) + .map(String::as_str) +} + /// Issue #103. The rules are written against absolute paths, and the ones /// anchored at the filesystem root — `/tmp/**`, `/var/log/**` — are anchored /// on purpose: that is what keeps `/tmp` out of `~/tmp`. `jwalk` builds every @@ -99,13 +123,14 @@ fn absolute_root(root: &Path) -> Result { fn walk_root( root: &Path, + excludes: &[String], opts: &ScanOptions, progress: &ScanProgress, out: &mut Vec, ) -> Result<()> { - // Normalized once, not per directory: `process_read_dir` runs on every - // directory the walk opens, and the exclude list never changes. - let excludes: Vec = opts.excludes.iter().map(|e| normalize_exclude(e)).collect(); + // `process_read_dir` runs on every directory the walk opens, so the list + // arrives already normalized rather than being rebuilt here. + let excludes = excludes.to_vec(); let walker = jwalk::WalkDir::new(root) .follow_links(opts.follow_symlinks) @@ -329,6 +354,53 @@ mod tests { assert_eq!(absolute_root(root).unwrap(), root); } + /// A root the exclude list covers is an error, not an empty scan. The + /// walk would drop every entry and the report would look like a clean + /// disk, which is the answer #103 exists to stop giving. + #[test] + fn a_root_inside_an_exclude_is_refused() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.bin"), b"x").unwrap(); + + let err = scan( + &ScanOptions { + roots: vec![dir.path().to_path_buf()], + excludes: vec![dir.path().to_string_lossy().into_owned()], + ..Default::default() + }, + Arc::new(ScanProgress::default()), + ) + .unwrap_err(); + + assert!( + matches!(err, crate::GenomeError::ExcludedRoot { .. }), + "{err:?}" + ); + assert!(err.to_string().contains("excluded directory"), "{err}"); + } + + /// The check is about containment, not a shared prefix: `/runtime-data` + /// is not inside `/run`, the same property `is_within` is written for. + #[test] + fn a_root_merely_sharing_a_prefix_with_an_exclude_is_scanned() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("runtime-data"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("a.bin"), b"x").unwrap(); + + let entries = scan( + &ScanOptions { + roots: vec![root], + excludes: vec![dir.path().join("run").to_string_lossy().into_owned()], + ..Default::default() + }, + Arc::new(ScanProgress::default()), + ) + .unwrap(); + + assert_eq!(entries.len(), 1); + } + #[test] fn scans_a_temp_tree() { let dir = tempfile::tempdir().unwrap();