fix(restore): keep the executable bit on custom-harness test binaries

src/wrapper.rs

@@ -1669,6 +1669,28 @@ fn rewrite_depinfo_outputs(artifacts: &ArtifactSet, anchor: &Path, mode: link::D
     }
 }
 
+/// How to materialize one restored artifact.
+///
+/// `kind` comes from the compile context, which does not always identify an
+/// executable. A `[[test]] harness = false` target supplies its own `main`, so
+/// cargo invokes rustc with neither `--test` nor `--crate-type`; its
+/// extensionless output classifies as `Other("rustc:unknown")`, whose strategy
+/// is `Hardlink` — no `0o755` on restore, and cargo then fails the run with
+/// "Permission denied (os error 13)".
+///
+/// The executable bit recorded at insert time is the reliable signal, and the
+/// insert side already trusts it over the filename (`store::hardlink_eligible`
+/// refuses to hardlink anything carrying a mode bit). Restore trusts it the
+/// same way, which also keeps executables on the independent-inode path so a
+/// post-build `strip` or codesign cannot reach back into the shared blob.
+fn restore_link_strategy(kind: ArtifactKind, executable: bool) -> link::LinkStrategy {
+    if executable {
+        link::LinkStrategy::Copy
+    } else {
+        kind.link_strategy()
+    }
+}
+
 /// Materialize one cached blob at its invocation-specific output path.
 ///
 /// The caller owns target-path resolution because that is compiler-specific
@@ -1736,21 +1758,21 @@ fn materialize_cached_artifact(
         }
     };
 
+    let strategy = restore_link_strategy(kind, cached_file.executable);
+
     match transformed {
         Some(content) => {
-            link::write_restored(target_path, &content, kind.link_strategy())
+            link::write_restored(target_path, &content, strategy)
                 .with_context(|| format!("{context}: writing {}", target_path.display()))?;
         }
         None => {
-            link::link_to_target(&store_path, target_path, kind.link_strategy()).with_context(
-                || {
-                    format!(
-                        "{context}: linking {} -> {}",
-                        store_path.display(),
-                        target_path.display()
-                    )
-                },
-            )?;
+            link::link_to_target(&store_path, target_path, strategy).with_context(|| {
+                format!(
+                    "{context}: linking {} -> {}",
+                    store_path.display(),
+                    target_path.display()
+                )
+            })?;
         }
     }
 
@@ -3853,6 +3875,90 @@ mod tests {
         );
     }
 
+    /// A `[[test]] harness = false` target is compiled without `--test` and
+    /// without `--crate-type`, so its extensionless output classifies as
+    /// `Other("rustc:unknown")` — the compile context simply never says
+    /// "executable". The mode bit recorded at insert time does, and restore
+    /// must honour it: otherwise the restored test binary comes back 0o644 and
+    /// cargo fails the run with "Permission denied (os error 13)".
+    #[cfg(unix)]
+    #[test]
+    fn materialize_cached_artifact_restores_executable_bit_recorded_at_insert() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let dir = tempfile::tempdir().unwrap();
+        let config = test_config(dir.path().join("cache"));
+        let store = Store::open(&config).unwrap();
+        let hash = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
+        create_blob(&store, hash, b"\x7fELF harness=false test binary");
+        // Store blobs are read-only and carry no executable bit, so a restore
+        // that never chmods cannot produce a runnable file.
+        std::fs::set_permissions(
+            store.blob_path(hash),
+            std::fs::Permissions::from_mode(0o444),
+        )
+        .unwrap();
+
+        let mut cached = cached_file("harness-a1b2c3d4e5f60718", hash);
+        cached.executable = true;
+        let target = dir.path().join("target").join("harness-a1b2c3d4e5f60718");
+        let platform = platform::current();
+
+        materialize_cached_artifact(
+            &BlobSource::Store(&store),
+            &cached,
+            &target,
+            ArtifactKind::Other("rustc:unknown"),
+            dir.path(),
+            &*platform,
+            "test restore",
+        )
+        .unwrap();
+
+        let mode = std::fs::metadata(&target).unwrap().permissions().mode();
+        assert_ne!(
+            mode & 0o111,
+            0,
+            "restored test binary must stay executable, got {mode:o}"
+        );
+    }
+
+    /// The converse: an artifact that was not executable at insert time must
+    /// not acquire the bit on restore.
+    #[cfg(unix)]
+    #[test]
+    fn materialize_cached_artifact_leaves_non_executable_artifacts_unexecutable() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let dir = tempfile::tempdir().unwrap();
+        let config = test_config(dir.path().join("cache"));
+        let store = Store::open(&config).unwrap();
+        let hash = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc";
+        create_blob(&store, hash, b"rlib bytes");
+
+        let cached = cached_file("libfoo.rlib", hash);
+        let target = dir.path().join("target").join("libfoo.rlib");
+        let platform = platform::current();
+
+        materialize_cached_artifact(
+            &BlobSource::Store(&store),
+            &cached,
+            &target,
+            ArtifactKind::Library,
+            dir.path(),
+            &*platform,
+            "test restore",
+        )
+        .unwrap();
+
+        let mode = std::fs::metadata(&target).unwrap().permissions().mode();
+        assert_eq!(
+            mode & 0o111,
+            0,
+            "library must not become executable, got {mode:o}"
+        );
+    }
+
     // ── fallback wrapper ─────────────────────────────────────────────
 
     #[test]

test-projects/custom-harness/Cargo.toml

@@ -0,0 +1,15 @@
+[package]
+name = "custom-harness"
+version = "0.1.0"
+edition = "2021"
+
+# Empty `[workspace]` — see test-projects/multi-dep/Cargo.toml for why.
+[workspace]
+
+# The point of this fixture: a test target that supplies its own `main`.
+# Cargo compiles it with neither `--test` nor `--crate-type`, so the
+# extensionless output carries no "this is an executable" signal in the rustc
+# argv — only the mode bit on the file itself.
+[[test]]
+name = "harness"
+harness = false

test-projects/custom-harness/src/lib.rs

@@ -0,0 +1,3 @@
+pub fn answer() -> u32 {
+    42
+}

test-projects/custom-harness/tests/harness.rs

@@ -0,0 +1,7 @@
+//! A test target with `harness = false`: cargo runs this binary directly, so
+//! it must come back from the cache executable.
+
+fn main() {
+    assert_eq!(custom_harness::answer(), 42);
+    println!("custom harness ok");
+}

tests/common/mod.rs

@@ -97,3 +97,22 @@ fn bootstrap_kache() -> Result<PathBuf, String> {
 pub fn isolated_config_path(cache_dir: &Path) -> PathBuf {
     cache_dir.join("config.toml")
 }
+
+/// A scratch directory on the *repository's* filesystem, for tests whose
+/// subject is how artifacts are materialized.
+///
+/// `TempDir::new()` lands in `TMPDIR`, so a cache placed there sits on a
+/// different filesystem from the build — or on tmpfs, which supports no
+/// reflink. Either way the store ingest and the restore take different paths
+/// than they would in a real build, and those paths differ in what file mode
+/// they leave behind. Anchoring here keeps such tests on whatever filesystem
+/// the developer or CI actually builds on, and `cargo clean` reclaims it.
+///
+/// Only the tests that materialize artifacts need this, and each integration
+/// test binary compiles this module separately, so the rest see it as dead.
+#[allow(dead_code)]
+pub fn scratch_dir() -> PathBuf {
+    let dir = bootstrap_target_dir().with_file_name("kache-test-scratch");
+    std::fs::create_dir_all(&dir).expect("creating scratch dir");
+    dir
+}

tests/custom_harness_test.rs

@@ -0,0 +1,127 @@
+//! End-to-end guard for restoring `[[test]] harness = false` binaries.
+//!
+//! A custom-harness test target supplies its own `main`, so cargo compiles it
+//! with neither `--test` nor `--crate-type`. Its extensionless output therefore
+//! classifies as `Other("rustc:unknown")`, whose link strategy is `Hardlink` —
+//! and the restore path used to derive the output's permissions from that
+//! strategy alone. The restored binary came back without its executable bit and
+//! cargo failed the run with "Permission denied (os error 13)".
+//!
+//! The build here is a real one through the real wrapper: warm the cache, throw
+//! the target directory away so every artifact has to come back from the cache,
+//! then let cargo execute the restored binary.
+//!
+//! The restored mode depends on which materialization path runs, and only one
+//! of them preserves the bit by accident:
+//!
+//! | path | when | mode |
+//! |---|---|---|
+//! | reflink | cache and target on one CoW filesystem | fresh file at the umask |
+//! | hardlink | one filesystem, blob copy-ingested | blob's `0o555` — survives |
+//! | hardlink | one filesystem, blob reflink-ingested | blob's `0o444` |
+//! | copy | cache and target on different filesystems | `0o644`, hardcoded |
+//!
+//! So the only configuration that accidentally works is a same-filesystem
+//! hardlink from a copy-ingested blob — which is what a plain ext4 checkout
+//! with `TMPDIR` alongside it produces, and why this went unnoticed.
+//!
+//! Running under `TMPDIR` would land in exactly that blind spot, so this test
+//! uses the repository's filesystem instead. `wrapper::tests` asserts the same
+//! contract directly, without depending on any of it.
+
+use std::path::Path;
+
+use tempfile::TempDir;
+
+mod common;
+use common::{build_kache, isolated_config_path, kache_binary, scratch_dir};
+
+fn copy_dir(src: &Path, dst: &Path) {
+    std::fs::create_dir_all(dst).unwrap();
+    for entry in std::fs::read_dir(src).unwrap() {
+        let entry = entry.unwrap();
+        let dest_path = dst.join(entry.file_name());
+        if entry.file_type().unwrap().is_dir() {
+            copy_dir(&entry.path(), &dest_path);
+        } else {
+            std::fs::copy(entry.path(), &dest_path).unwrap();
+        }
+    }
+}
+
+fn copy_fixture() -> TempDir {
+    let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("test-projects/custom-harness");
+    let tmp = TempDir::new_in(scratch_dir()).unwrap();
+    copy_dir(&fixture, tmp.path());
+    tmp
+}
+
+/// Runs `cargo test --test harness` through the kache wrapper.
+fn cargo_test(project: &Path, cache_dir: &Path, target_dir: &Path) -> std::process::Output {
+    std::process::Command::new("cargo")
+        .args(["test", "--test", "harness"])
+        .current_dir(project)
+        .env("RUSTC_WRAPPER", kache_binary())
+        .env("KACHE_CACHE_DIR", cache_dir)
+        .env("KACHE_CONFIG", isolated_config_path(cache_dir))
+        .env("CARGO_TARGET_DIR", target_dir)
+        .env("CARGO_INCREMENTAL", "0")
+        .output()
+        .expect("failed to run cargo test")
+}
+
+/// The restored test binary must still be runnable.
+#[test]
+fn custom_harness_test_binary_is_executable_after_restore() {
+    build_kache();
+    let project = copy_fixture();
+    let cache = TempDir::new_in(scratch_dir()).unwrap();
+    let target = TempDir::new_in(scratch_dir()).unwrap();
+
+    let cold = cargo_test(project.path(), cache.path(), target.path());
+    assert!(
+        cold.status.success(),
+        "cold build failed.\nstdout: {}\nstderr: {}",
+        String::from_utf8_lossy(&cold.stdout),
+        String::from_utf8_lossy(&cold.stderr),
+    );
+
+    // Drop every build output so the second run has to restore the test binary
+    // from the cache rather than reuse the one the linker just produced.
+    std::fs::remove_dir_all(target.path()).unwrap();
+    std::fs::create_dir_all(target.path()).unwrap();
+
+    let warm = cargo_test(project.path(), cache.path(), target.path());
+    assert!(
+        warm.status.success(),
+        "restored harness=false test binary could not be run.\nstdout: {}\nstderr: {}",
+        String::from_utf8_lossy(&warm.stdout),
+        String::from_utf8_lossy(&warm.stderr),
+    );
+
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+
+        let binary = std::fs::read_dir(target.path().join("debug").join("deps"))
+            .unwrap()
+            .filter_map(Result::ok)
+            .map(|entry| entry.path())
+            .find(|path| {
+                path.extension().is_none()
+                    && path
+                        .file_name()
+                        .and_then(|name| name.to_str())
+                        .is_some_and(|name| name.starts_with("harness-"))
+            })
+            .expect("restored harness test binary");
+
+        let mode = std::fs::metadata(&binary).unwrap().permissions().mode();
+        assert_ne!(
+            mode & 0o111,
+            0,
+            "{} was restored without its executable bit (mode {mode:o})",
+            binary.display()
+        );
+    }
+}