ci: run the engine tests on Windows and macOS, not only Linux

.github/workflows/ci.yml

@@ -34,18 +34,45 @@ 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
       - uses: dtolnay/rust-toolchain@stable
         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.
+      #
+      # --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
+  # `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 +83,34 @@ 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.
+  #
+  # 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'
-    runs-on: ubuntu-22.04
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-22.04, windows-latest, macos-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 \

crates/diskern-core/src/actions.rs

@@ -566,12 +566,28 @@ 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();
+
+        // `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,
+            // 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}"),
+        }
 
         let err = quarantine(&victim, Verdict::Review, &q).unwrap_err();
         assert!(
@@ -686,4 +702,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}");
+    }
 }

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

crates/diskern-core/src/scanner.rs

@@ -246,6 +246,35 @@ mod tests {
         assert!(!is_excluded(Path::new("/process-data/x"), &excludes));
     }
 
+    /// The folding in `is_within` against a directory that really exists.
+    ///
+    /// 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();
+        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();