From 19094d90320aa064ed6bd9efb41bec70793edde5 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 14:55:41 +0500 Subject: [PATCH 1/7] ci: run the engine tests on Windows and macOS, not only Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every job in this workflow ran on ubuntu, and the release workflow ships a Windows installer. So the platform-specific half of the engine — `default_excludes`, most of the rules database, quarantine's flattening of drive letters and backslashes — was only ever checked against string literals on Linux, and executed on Windows for the first time during the release build, after the tag was pushed. The test job is now a matrix over ubuntu, windows and macos, with `fail-fast` off so a break on two platforms reports as two. macOS is in it even though the release entry for macOS is still commented out: the rules describe `Library/Caches` and `Library/Logs` and the code has a `target_os = "macos"` arm, and none of that has ever run. `app-tauri` gains Windows for the same reason at a different layer: the Linux shell links webkitgtk and the GTK3 stack, the Windows one links WebView2, and a cfg-shaped mistake compiles on one and not the other. fmt and clippy move to their own `lint` job — they answer identically everywhere, and running them three times only slows the feedback. Note for whoever manages branch protection: the required check `test` becomes `test (ubuntu-22.04)`, `test (windows-latest)` and `test (macos-latest)`, and lint is now its own check. --- .github/workflows/ci.yml | 44 +++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6fff9c..535e260 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,9 @@ jobs: - 'Cargo.lock' - '.github/workflows/ci.yml' - test: + # Formatting and lints answer the same on every platform, so they run + # once rather than three times. + lint: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v7 @@ -42,10 +44,31 @@ jobs: with: components: clippy, rustfmt - uses: swatinem/rust-cache@v2 - # Core + CLI only: building the Tauri crate needs webkitgtk, which CI - # for library tests doesn't need. The release workflow covers the app. + # Core + CLI only: building the Tauri crate needs webkitgtk, which + # lints of the library don't need. `app-tauri` below covers the shell. - run: cargo fmt --check - run: cargo clippy -p diskern-core -p diskern-cli -- -D warnings + + # The engine's behaviour is platform-specific in ways a run on one OS + # cannot reach, and it ships on more than one: `default_excludes` has a + # `cfg` arm per platform, most of the rules database describes Windows + # and macOS paths, and quarantine flattens drive letters and separators + # that only exist on Windows. Until this matrix, all of that was checked + # against string literals on Linux and executed on Windows for the first + # time in the release build — after the tag was pushed. + # + # fail-fast off: when a change breaks two platforms, seeing one of them + # is half a bug report. + test: + strategy: + fail-fast: false + matrix: + os: [ubuntu-22.04, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + - uses: swatinem/rust-cache@v2 - run: cargo test -p diskern-core -p diskern-cli # Type-checks app/src-tauri against the current diskern-core. Until this @@ -56,16 +79,27 @@ jobs: # clippy, not a full `cargo build` — the goal is catching signature drift # and lint regressions in the shell; the release workflow still owns # producing an actual bundle. + # + # On both platforms the release workflow ships, because the shells differ: + # Linux links webkitgtk and the GTK3 stack, Windows links WebView2, and a + # `cfg`-shaped mistake compiles fine on one and not the other. Windows was + # previously compiled for the first time during the release build. app-tauri: needs: changes if: needs.changes.outputs.app == 'true' - runs-on: ubuntu-22.04 + strategy: + fail-fast: false + matrix: + os: [ubuntu-22.04, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7 # Same apt line as release.yml. Kept in sync by hand; there is no - # cheaper way to share it between workflows. + # cheaper way to share it between workflows. Windows needs none of + # this — WebView2 ships with the OS. - name: Linux system deps (Tauri v2 / webkitgtk 4.1) + if: matrix.os == 'ubuntu-22.04' run: | sudo apt-get update sudo apt-get install -y libwebkit2gtk-4.1-dev libappindicator3-dev \ From 2159c59899b39a0fb3b2c41a571aadaabd98a513 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 15:07:43 +0500 Subject: [PATCH 2/7] test: cover the behaviour that only differs off Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps the new matrix can now actually exercise. `an_exclude_matches_a_real_directory_whatever_its_case` walks a real tree with a `Cache` directory excluded as `cache`. Windows and macOS fold case in the filesystem, Linux does not, so on Linux it passes only because `is_within` folds — which is the half that has to be right everywhere. `a_quarantine_name_carries_no_separator_from_the_original` pins the flattening. On Windows the original path opens `C:\`, and a colon or a backslash surviving into a filename is not a wrong name, it is an impossible one. `a_windows_path_that_cannot_be_recorded_is_refused` is the Windows half of the non-UTF-8 data-loss guard. Windows filenames are UTF-16 and may hold an unpaired surrogate, which serde rejects the same way it rejects invalid UTF-8 on Unix, so the same loss was reachable there. Creating such a file through the Win32 API isn't dependable, so it pins the guard that refuses the record rather than the whole flow — narrower than the Unix test, and deliberately so. --- crates/diskern-core/src/actions.rs | 54 ++++++++++++++++++++++++++++++ crates/diskern-core/src/scanner.rs | 28 ++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/crates/diskern-core/src/actions.rs b/crates/diskern-core/src/actions.rs index 5d7f949..0d6f114 100644 --- a/crates/diskern-core/src/actions.rs +++ b/crates/diskern-core/src/actions.rs @@ -686,4 +686,58 @@ mod tests { assert_eq!(summary.bytes_removed, 0); assert!(summary.failed.is_empty()); } + + /// Quarantine names are built from the original path, and on Windows + /// that path opens `C:\` — a colon and backslashes, none of which are + /// legal in a filename. Nothing may survive the flattening. + #[test] + fn a_quarantine_name_carries_no_separator_from_the_original() { + let dir = tempfile::tempdir().unwrap(); + let q = dir.path().join("quarantine"); + let nested = dir.path().join("a").join("b"); + std::fs::create_dir_all(&nested).unwrap(); + let victim = nested.join("victim.txt"); + std::fs::write(&victim, b"data").unwrap(); + + let record = quarantine(&victim, Verdict::Safe, &q).unwrap(); + let name = record + .quarantined_to + .file_name() + .expect("a quarantined file has a name") + .to_string_lossy() + .into_owned(); + + for separator in ['/', '\\', ':'] { + assert!( + !name.contains(separator), + "{name} still carries {separator:?}" + ); + } + assert!(record.quarantined_to.exists()); + assert_eq!(list(&q).unwrap().len(), 1); + } + + /// The Windows half of `a_path_that_cannot_be_recorded_is_not_moved`. + /// + /// Windows filenames are UTF-16 and may hold an unpaired surrogate, + /// which serde rejects exactly as it rejects invalid UTF-8 on Unix — + /// so the same data loss was reachable here. Creating such a file + /// through the Win32 API is not dependable, so this pins the guard + /// that stops the move rather than the whole flow. + #[cfg(windows)] + #[test] + fn a_windows_path_that_cannot_be_recorded_is_refused() { + use std::os::windows::ffi::OsStringExt; + + // "a\u{D800}.dat" — a lone high surrogate in an ordinary name. + let name = std::ffi::OsString::from_wide(&[0x0061, 0xD800, 0x002E, 0x0064, 0x0061, 0x0074]); + let record = QuarantineRecord { + original: PathBuf::from(name), + quarantined_to: PathBuf::from("quarantine"), + at_epoch: 0, + }; + + let err = encode(&record).unwrap_err(); + assert!(err.to_string().contains("will not be moved"), "{err}"); + } } diff --git a/crates/diskern-core/src/scanner.rs b/crates/diskern-core/src/scanner.rs index 570429d..7efb74d 100644 --- a/crates/diskern-core/src/scanner.rs +++ b/crates/diskern-core/src/scanner.rs @@ -246,6 +246,34 @@ mod tests { assert!(!is_excluded(Path::new("/process-data/x"), &excludes)); } + /// The folding in `is_within` against a directory that really exists. + /// + /// Windows and macOS fold case in the filesystem itself, Linux does + /// not — so on Linux this passes only because the matcher folds, which + /// is the half that has to be right on all three. + #[test] + fn an_exclude_matches_a_real_directory_whatever_its_case() { + let dir = tempfile::tempdir().unwrap(); + let cache = dir.path().join("Cache"); + std::fs::create_dir(&cache).unwrap(); + std::fs::write(cache.join("blob.bin"), b"cached").unwrap(); + std::fs::write(dir.path().join("keep.txt"), b"keep").unwrap(); + + let opts = ScanOptions { + roots: vec![dir.path().to_path_buf()], + // Written lowercase; the directory on disk is not. + excludes: vec![cache.to_string_lossy().to_lowercase()], + ..Default::default() + }; + let entries = scan(&opts, Arc::new(ScanProgress::default())).unwrap(); + + assert_eq!(entries.len(), 1, "{entries:#?}"); + assert_eq!( + entries[0].path.file_name().unwrap().to_string_lossy(), + "keep.txt" + ); + } + #[test] fn scans_a_temp_tree() { let dir = tempfile::tempdir().unwrap(); From 60cfba723066c50aefd0a07ac30b840a7d7ecbd0 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 15:15:47 +0500 Subject: [PATCH 3/7] test: fix the two assumptions the new platforms caught MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The matrix found both on its first run, and both were in the tests rather than the engine. Windows: `a_referenced_store_is_more_cautious_than_an_abandoned_one` looked for a finding whose path contained `live/node_modules`. Windows separates with `\`, so it matched nothing and the `unwrap` panicked. It compares whole paths now, built with `join`, which is separator-correct everywhere. macOS: `a_path_that_cannot_be_recorded_is_not_moved` could not create its fixture — APFS validates filenames as UTF-8 and answers EILSEQ, so a non-UTF-8 name cannot exist there and the data loss it caused is not reachable on macOS at all. Linux imposes no such rule, so the test returns early only there, and a creation failure anywhere else is still a hard failure rather than a quiet pass. Worth recording, since it narrows the bug fixed in #63: that loss was Linux-only in practice on Unix, and reachable on Windows through unpaired surrogates instead. --- crates/diskern-core/src/actions.rs | 17 ++++++++++++++--- crates/diskern-core/src/report.rs | 13 ++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/crates/diskern-core/src/actions.rs b/crates/diskern-core/src/actions.rs index 0d6f114..e39c6ea 100644 --- a/crates/diskern-core/src/actions.rs +++ b/crates/diskern-core/src/actions.rs @@ -566,12 +566,23 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let q = dir.path().join("quarantine"); // Raw bytes no UTF-8 decoder accepts, in an otherwise ordinary - // name: a legal filename on this platform, and one a scan of a - // real disk turns up in downloads unpacked from old archives. + // name: a legal filename on Linux, and one a scan of a real disk + // turns up in downloads unpacked from old archives. let victim = dir .path() .join(std::ffi::OsStr::from_bytes(b"photo-\xff\xfe.dmg")); - std::fs::write(&victim, b"payload").unwrap(); + + match std::fs::write(&victim, b"payload") { + Ok(()) => {} + // macOS validates filenames as UTF-8 in the filesystem itself + // and answers EILSEQ, so a name like this cannot exist there + // and the loss it caused is not reachable. Linux imposes no + // such rule, which is why this is checked rather than skipped + // by a `cfg`: if the fixture ever stops being creatable on + // Linux, that is a failure and not a quiet pass. + Err(_) if cfg!(target_os = "macos") => return, + Err(e) => panic!("could not create the fixture: {e}"), + } let err = quarantine(&victim, Verdict::Review, &q).unwrap_err(); assert!( diff --git a/crates/diskern-core/src/report.rs b/crates/diskern-core/src/report.rs index 1275eef..4f9d33a 100644 --- a/crates/diskern-core/src/report.rs +++ b/crates/diskern-core/src/report.rs @@ -421,16 +421,19 @@ mod tests { ) .unwrap(); - let find = |needle: &str| { + // Compared as paths, not as substrings of one: Windows separates + // with `\`, so "live/node_modules" matches nothing there. + let find = |root: &std::path::Path| { + let wanted = root.join("node_modules").join("react").join("index.js"); report .findings .iter() - .find(|f| f.entry.path.to_string_lossy().contains(needle)) - .unwrap() + .find(|f| f.entry.path == wanted) + .unwrap_or_else(|| panic!("no finding for {}", wanted.display())) .clone() }; - let referenced = find("live/node_modules"); + let referenced = find(&live); assert_eq!(referenced.verdict, Verdict::Risky); assert!(referenced .reasons @@ -440,7 +443,7 @@ mod tests { // offer either. assert_eq!(referenced.reclaimable, 0); - let abandoned = find("dead/node_modules"); + let abandoned = find(&dead); assert_eq!(abandoned.verdict, Verdict::Review); assert!(!abandoned .reasons From 62426ad070f9e9e3500e69d7851a465fc6e285ca Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 15:47:41 +0500 Subject: [PATCH 4/7] docs(scanner): say correctly why the exclude test passes The comment claimed Windows and macOS fold case in the filesystem, so that only on Linux did the test depend on `is_within` folding. That is wrong in a way that invites damage: `is_excluded` compares two strings and never opens a file, so the filesystem's case rules cannot affect the outcome anywhere. Remove the folding and the test fails on all three. As written it read as licence either to skip the test off Linux or to treat the folding as redundant on a case-insensitive platform, both of which would quietly remove the coverage the test exists to give. --- crates/diskern-core/src/scanner.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/diskern-core/src/scanner.rs b/crates/diskern-core/src/scanner.rs index 7efb74d..3646117 100644 --- a/crates/diskern-core/src/scanner.rs +++ b/crates/diskern-core/src/scanner.rs @@ -248,9 +248,10 @@ mod tests { /// The folding in `is_within` against a directory that really exists. /// - /// Windows and macOS fold case in the filesystem itself, Linux does - /// not — so on Linux this passes only because the matcher folds, which - /// is the half that has to be right on all three. + /// The filesystem's own case rules have no bearing on this, on any + /// platform: `is_excluded` compares two strings and never opens a + /// file. Drop the ASCII folding and this fails on Windows, macOS and + /// Linux alike — which is what makes it worth running on all three. #[test] fn an_exclude_matches_a_real_directory_whatever_its_case() { let dir = tempfile::tempdir().unwrap(); From 7883198163744a4de840fe99466335f14e98e8a3 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 15:48:02 +0500 Subject: [PATCH 5/7] test(actions): let only the macOS UTF-8 refusal skip the fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Err(_) if cfg!(target_os = "macos")` treated any failure to create the fixture as the expected filesystem rejection, so a missing temp directory, a full disk or a permissions problem reported the test as passing having exercised nothing. The comment beside it already claimed a broken fixture would fail rather than pass quietly. Matching the errno APFS actually answers — EILSEQ, raw OS error 92, which is what the macOS run reported — makes that true instead of aspirational. --- crates/diskern-core/src/actions.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/diskern-core/src/actions.rs b/crates/diskern-core/src/actions.rs index e39c6ea..f965935 100644 --- a/crates/diskern-core/src/actions.rs +++ b/crates/diskern-core/src/actions.rs @@ -572,15 +572,20 @@ mod tests { .path() .join(std::ffi::OsStr::from_bytes(b"photo-\xff\xfe.dmg")); + // `EILSEQ` on macOS: the errno APFS answers a filename that is + // not valid UTF-8 with. + const MACOS_EILSEQ: i32 = 92; + match std::fs::write(&victim, b"payload") { Ok(()) => {} - // macOS validates filenames as UTF-8 in the filesystem itself - // and answers EILSEQ, so a name like this cannot exist there - // and the loss it caused is not reachable. Linux imposes no - // such rule, which is why this is checked rather than skipped - // by a `cfg`: if the fixture ever stops being creatable on - // Linux, that is a failure and not a quiet pass. - Err(_) if cfg!(target_os = "macos") => return, + // macOS validates filenames as UTF-8 in the filesystem itself, + // so a name like this cannot exist there and the loss it + // caused is not reachable. Linux imposes no such rule, which + // is why this is checked rather than skipped by a `cfg`: if + // the fixture ever stops being creatable, that is a failure + // and not a quiet pass. Only the one refusal that means + // "this platform forbids the name" is allowed through. + Err(e) if cfg!(target_os = "macos") && e.raw_os_error() == Some(MACOS_EILSEQ) => return, Err(e) => panic!("could not create the fixture: {e}"), } From 3ddd56049237ffc68da1cfa20a5802a4572d1082 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 15:49:03 +0500 Subject: [PATCH 6/7] ci: lint the test code too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo clippy` ran without `--all-targets`, so `#[cfg(test)]` code in diskern-core and diskern-cli was never linted — while `app-tauri` has always passed the flag. `cargo test` compiles test code but applies no lints to it, so a warning there was invisible on every platform. That matters more than it used to: the suite is now a good half of the crate by line count, and this workflow is the only thing looking at it. Clean as of this commit, so nothing to fix alongside — this is about the next one. --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 535e260..1495f0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,8 +46,12 @@ jobs: - uses: swatinem/rust-cache@v2 # Core + CLI only: building the Tauri crate needs webkitgtk, which # lints of the library don't need. `app-tauri` below covers the shell. + # + # --all-targets so tests are linted too. `cargo test` compiles test + # code but applies no lints to it, so without this the suite — by now + # a good half of the crate — drifts unlinted while CI stays green. - run: cargo fmt --check - - run: cargo clippy -p diskern-core -p diskern-cli -- -D warnings + - run: cargo clippy -p diskern-core -p diskern-cli --all-targets -- -D warnings # The engine's behaviour is platform-specific in ways a run on one OS # cannot reach, and it ships on more than one: `default_excludes` has a From 19df1d8dde8b86afb9f739268359d33966773e23 Mon Sep 17 00:00:00 2001 From: Muawiya Amir Date: Sat, 5 Sep 2026 15:49:16 +0500 Subject: [PATCH 7/7] ci: compile the Tauri shell on macOS as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `app-tauri` covered Linux and Windows while `test` covered macOS too, so the one crate whose whole job is platform-specific was the one platform short. The reason Windows is in this job applies to macOS unchanged: the shells differ — webkitgtk on Linux, WebView2 on Windows, WKWebView on macOS — and a cfg-shaped mistake compiles on one and not another. The macOS release entry is commented out today, so leaving it off would have handed whoever re-enables it exactly the failure this job exists to prevent: a first compile after the tag is pushed. It costs a few minutes on pull requests that touch `crates/` or `app/` and nothing at all on any other, since `changes` gates the whole job. --- .github/workflows/ci.yml | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1495f0d..5866f8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,17 +84,24 @@ jobs: # and lint regressions in the shell; the release workflow still owns # producing an actual bundle. # - # On both platforms the release workflow ships, because the shells differ: - # Linux links webkitgtk and the GTK3 stack, Windows links WebView2, and a - # `cfg`-shaped mistake compiles fine on one and not the other. Windows was - # previously compiled for the first time during the release build. + # Every platform the shell is built for, because the shells differ: + # Linux links webkitgtk and the GTK3 stack, Windows links WebView2, + # macOS links WKWebView, and a `cfg`-shaped mistake compiles fine on one + # and not another. Windows used to be compiled for the first time during + # the release build. + # + # macOS is here even though its release entry is still commented out. + # Leaving it off would hand whoever re-enables that entry the exact + # failure this job exists to prevent — a first compile after the tag is + # pushed. It costs a few minutes on PRs that touch `crates/` or `app/`, + # and nothing on any other PR: `changes` gates the whole job. app-tauri: needs: changes if: needs.changes.outputs.app == 'true' strategy: fail-fast: false matrix: - os: [ubuntu-22.04, windows-latest] + os: [ubuntu-22.04, windows-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7