Fix the riscv64 test sweep: hermetic collapse tests, unique sandbox names

crates/sandlock-cli/tests/cli_test.rs

@@ -429,21 +429,40 @@ fn sandlock_sleep_args(name: &str) -> Vec<String> {
 
 /// Start a background sandbox, wait for it to be listed by `ps`, then
 /// return its PID. The caller is responsible for killing it.
+///
+/// stderr is captured so wait_for_sandbox can report why a sandbox never
+/// appeared instead of a bare timeout.
 fn spawn_sandbox(name: &str) -> std::process::Child {
     let bin = env!("CARGO_BIN_EXE_sandlock");
     let args = sandlock_sleep_args(name);
     std::process::Command::new(bin)
         .args(&args)
         .stdout(std::process::Stdio::null())
-        .stderr(std::process::Stdio::null())
+        .stderr(std::process::Stdio::piped())
         .spawn()
         .expect("spawn sandlock")
 }
 
+/// Kill `child` if still running and return whatever it wrote to stderr.
+fn drain_child_stderr(child: &mut std::process::Child) -> String {
+    let _ = child.kill();
+    let _ = child.wait();
+    let mut stderr = String::new();
+    if let Some(mut pipe) = child.stderr.take() {
+        use std::io::Read;
+        let _ = pipe.read_to_string(&mut stderr);
+    }
+    stderr
+}
+
 /// Wait for a sandbox to appear in `sandlock ps` output.
-fn wait_for_sandbox(name: &str) -> Result<(), String> {
+///
+/// Fails fast with the child's exit status and stderr if the sandbox process
+/// dies before appearing. The 15s ceiling leaves room for slow hosts running
+/// several sandbox startups in parallel (riscv64 boards).
+fn wait_for_sandbox(child: &mut std::process::Child, name: &str) -> Result<(), String> {
     let bin = env!("CARGO_BIN_EXE_sandlock");
-    for _ in 0..10 {
+    for _ in 0..30 {
         let out = std::process::Command::new(bin)
             .args(["ps"])
             .output()
@@ -452,17 +471,26 @@ fn wait_for_sandbox(name: &str) -> Result<(), String> {
         if stdout.contains(name) {
             return Ok(());
         }
+        if let Ok(Some(status)) = child.try_wait() {
+            return Err(format!(
+                "sandbox '{}' exited ({}) before appearing in ps: {}",
+                name, status, drain_child_stderr(child)
+            ));
+        }
         std::thread::sleep(std::time::Duration::from_millis(500));
     }
-    Err(format!("sandbox '{}' did not appear in ps output", name))
+    Err(format!(
+        "sandbox '{}' did not appear in ps output within 15s: {}",
+        name, drain_child_stderr(child)
+    ))
 }
 
 #[test]
 fn test_ps_lists_running_sandbox() {
     let name = format!("test-ps-cli-{}", std::process::id());
     let mut child = spawn_sandbox(&name);
 
-    match wait_for_sandbox(&name) {
+    match wait_for_sandbox(&mut child, &name) {
         Ok(()) => {
             let out = sandlock_bin()
                 .args(["ps"])
@@ -514,7 +542,7 @@ fn test_config_returns_json_policy() {
     let name = format!("test-config-cli-{}", std::process::id());
     let mut child = spawn_sandbox(&name);
 
-    match wait_for_sandbox(&name) {
+    match wait_for_sandbox(&mut child, &name) {
         Ok(()) => {
             let out = sandlock_bin()
                 .args(["config", &name])
@@ -553,7 +581,7 @@ fn test_config_toml_flag_produces_toml() {
     let name = format!("test-config-toml-cli-{}", std::process::id());
     let mut child = spawn_sandbox(&name);
 
-    match wait_for_sandbox(&name) {
+    match wait_for_sandbox(&mut child, &name) {
         Ok(()) => {
             let out = sandlock_bin()
                 .args(["config", "--toml", &name])
@@ -601,7 +629,7 @@ fn test_kill_stops_sandbox() {
     let name = format!("test-kill-cli-{}", std::process::id());
     let mut child = spawn_sandbox(&name);
 
-    match wait_for_sandbox(&name) {
+    match wait_for_sandbox(&mut child, &name) {
         Ok(()) => {
             let out = sandlock_bin()
                 .args(["kill", &name])

crates/sandlock-cli/tests/learn_integration.rs

@@ -171,26 +171,44 @@ fn test_learn_then_run_unix_bind() {
 }
 
 /// Collapsed profile: directory grant covers files not individually observed during learn.
+///
+/// Uses a private tempdir rather than /usr/bin: learn canonicalizes observed
+/// paths, and on hosts where coreutils are symlinks into a multicall link farm
+/// (e.g. Ubuntu rust-coreutils on riscv64) the /usr/bin names resolve to
+/// parents that never reach the collapse threshold (issue #170).
 #[test]
 fn test_learn_then_run_collapse() {
     let profile = tempfile::NamedTempFile::new().expect("tempfile");
     let profile_path = profile.path().to_str().unwrap().to_owned();
+    let dir = tempfile::TempDir::new_in("/var/tmp").expect("tempdir in /var/tmp");
 
-    // Learn touches several /usr/bin files; --collapse folds them into /usr/bin.
-    let learn = sandlock_bin()
-        .args(["learn", "--collapse", "-o", &profile_path, "--", "sh", "-c",
-               "cat /usr/bin/cat /usr/bin/sh /usr/bin/ls /usr/bin/env"])
-        .output()
-        .expect("failed to run sandlock learn");
+    // Four observed files meet the pinned collapse threshold.
+    let observed: Vec<String> = (1..=4)
+        .map(|i| {
+            let p = dir.path().join(format!("observed{i}.txt"));
+            std::fs::write(&p, "collapse me\n").expect("write observed file");
+            p.to_str().unwrap().to_owned()
+        })
+        .collect();
+    let extra = dir.path().join("unobserved.txt");
+    std::fs::write(&extra, "covered by directory grant\n").expect("write unobserved file");
+
+    let mut learn_cmd = sandlock_bin();
+    learn_cmd.args(["learn", "--collapse=4", "-o", &profile_path, "--", "cat"]);
+    learn_cmd.args(&observed);
+    let learn = learn_cmd.output().expect("failed to run sandlock learn");
     assert!(learn.status.success(), "learn failed: {}", String::from_utf8_lossy(&learn.stderr));
 
-    // Run accesses /usr/bin/true which was not individually observed, the collapsed grant covers it.
+    // Run reads a file that was not individually observed; the collapsed
+    // directory grant covers it.
     let run = sandlock_bin()
-        .args(["run", "--profile-file", &profile_path, "--", "cat", "/usr/bin/true"])
+        .args(["run", "--profile-file", &profile_path, "--", "cat", extra.to_str().unwrap()])
         .output()
         .expect("failed to run sandlock run");
     assert!(run.status.success(),
         "run failed with collapsed profile: {}", String::from_utf8_lossy(&run.stderr));
+    assert_eq!(String::from_utf8_lossy(&run.stdout).trim(), "covered by directory grant",
+        "unexpected output from unobserved file");
 }
 
 /// Learned memory limit reflects actual anonymous allocation, not just the floor.

crates/sandlock-cli/tests/learn_test.rs

@@ -539,13 +539,27 @@ fn test_read_dedup_removes_leaf_when_ancestor_present() {
 }
 
 /// N-threshold collapse replaces individual files with their parent directory.
+///
+/// Uses a private tempdir rather than /usr/bin: learn canonicalizes observed
+/// paths, and on hosts where coreutils are symlinks into a multicall link farm
+/// (e.g. Ubuntu rust-coreutils on riscv64) the /usr/bin names scatter across
+/// parents that never reach the collapse threshold (issue #170).
 #[test]
 fn test_collapse_threshold_reads() {
-    let output = sandlock_bin()
-        .args(["learn", "--collapse", "--", "sh", "-c",
-               "cat /usr/bin/cat /usr/bin/sh /usr/bin/ls /usr/bin/env"])
-        .output()
-        .expect("failed to run sandlock learn");
+    let dir = tempfile::TempDir::new_in("/var/tmp").expect("tempdir in /var/tmp");
+    let dir_path = dir.path().canonicalize().expect("canonicalize tempdir");
+    let observed: Vec<String> = (1..=4)
+        .map(|i| {
+            let p = dir_path.join(format!("observed{i}.txt"));
+            std::fs::write(&p, "collapse me\n").expect("write observed file");
+            p.to_str().unwrap().to_owned()
+        })
+        .collect();
+
+    let mut cmd = sandlock_bin();
+    cmd.args(["learn", "--collapse=4", "--", "cat"]);
+    cmd.args(&observed);
+    let output = cmd.output().expect("failed to run sandlock learn");
     assert!(
         output.status.success(),
         "sandlock learn failed: stderr={}",
@@ -554,20 +568,35 @@ fn test_collapse_threshold_reads() {
     let stdout = String::from_utf8_lossy(&output.stdout);
     let read_line = stdout.lines().find(|l| l.starts_with("read = [")).unwrap_or("");
     assert!(
-        read_line.contains("/usr/bin\"") || read_line.contains("/usr/bin,"),
-        "expected /usr/bin collapsed in reads, got: {read_line}",
+        read_line.contains(&format!("\"{}\"", dir_path.display())),
+        "expected {} collapsed in reads, got: {read_line}",
+        dir_path.display(),
     );
-    assert!(!read_line.contains("/usr/bin/cat"),
-        "expected individual /usr/bin files removed after collapse, got: {read_line}");
+    assert!(!read_line.contains("observed1.txt"),
+        "expected individual files removed after collapse, got: {read_line}");
 }
 
 /// --collapse-prefix forces collapse regardless of the file count threshold.
+///
+/// Two observed files stay below the N=4 default threshold, so only the
+/// prefix can produce the directory grant. Hermetic for the same reason as
+/// test_collapse_threshold_reads (issue #170).
 #[test]
 fn test_collapse_prefix_forces_collapse() {
-    let output = sandlock_bin()
-        .args(["learn", "--collapse-prefix", "/usr/bin", "--", "cat", "/usr/bin/cat", "/usr/bin/sh"])
-        .output()
-        .expect("failed to run sandlock learn");
+    let dir = tempfile::TempDir::new_in("/var/tmp").expect("tempdir in /var/tmp");
+    let dir_path = dir.path().canonicalize().expect("canonicalize tempdir");
+    let observed: Vec<String> = (1..=2)
+        .map(|i| {
+            let p = dir_path.join(format!("observed{i}.txt"));
+            std::fs::write(&p, "collapse me\n").expect("write observed file");
+            p.to_str().unwrap().to_owned()
+        })
+        .collect();
+
+    let mut cmd = sandlock_bin();
+    cmd.args(["learn", "--collapse-prefix", dir_path.to_str().unwrap(), "--", "cat"]);
+    cmd.args(&observed);
+    let output = cmd.output().expect("failed to run sandlock learn");
     assert!(
         output.status.success(),
         "sandlock learn failed: stderr={}",
@@ -576,11 +605,12 @@ fn test_collapse_prefix_forces_collapse() {
     let stdout = String::from_utf8_lossy(&output.stdout);
     let read_line = stdout.lines().find(|l| l.starts_with("read = [")).unwrap_or("");
     assert!(
-        read_line.contains("/usr/bin\"") || read_line.contains("/usr/bin,"),
-        "expected /usr/bin collapsed in reads, got: {read_line}",
+        read_line.contains(&format!("\"{}\"", dir_path.display())),
+        "expected {} collapsed in reads, got: {read_line}",
+        dir_path.display(),
     );
-    assert!(!read_line.contains("/usr/bin/cat"),
-        "expected /usr/bin/cat removed after prefix collapse, got: {read_line}");
+    assert!(!read_line.contains("observed1.txt"),
+        "expected individual files removed after prefix collapse, got: {read_line}");
 }
 
 /// Guarded paths are not collapsed by the N-threshold; individual files are kept.

crates/sandlock-core/src/pipeline.rs

@@ -47,7 +47,10 @@ impl Stage {
     /// Run this single stage and return the result.
     pub async fn run(self, timeout: Option<Duration>) -> Result<RunResult, SandlockError> {
         let cmd_refs: Vec<&str> = self.args.iter().map(|s| s.as_str()).collect();
-        let mut sb = self.sandbox.with_name("stage");
+        // Names claim a per-UID runtime dir and a live collision is a hard
+        // error, so every internally assigned name carries a unique id.
+        let mut sb = self.sandbox.with_name(
+            format!("stage-{}", crate::sandbox::unique_instance_id()));
         if let Some(dur) = timeout {
             match tokio::time::timeout(dur, sb.run_interactive(&cmd_refs)).await {
                 Ok(result) => result,
@@ -180,11 +183,13 @@ async fn run_pipeline(stages: Vec<Stage>) -> Result<RunResult, SandlockError> {
     let (cap_stdout_r, cap_stdout_w) = make_pipe().map_err(SandboxRuntimeError::Io)?;
     let (cap_stderr_r, cap_stderr_w) = make_pipe().map_err(SandboxRuntimeError::Io)?;
 
-    // Spawn each stage
+    // Spawn each stage. Stage names share one unique run id so concurrent
+    // pipelines in the same UID never collide on their runtime dirs.
+    let run = crate::sandbox::unique_instance_id();
     let mut sandboxes: Vec<Sandbox> = Vec::with_capacity(n);
 
     for (i, stage) in stages.into_iter().enumerate() {
-        let name = format!("pipeline-stage-{}", i);
+        let name = format!("pipeline-{run}-stage-{i}");
         let mut sb = stage.sandbox.clone().with_name(name);
 
         // Determine stdin for this stage
@@ -363,10 +368,13 @@ async fn run_gather(
     let (cap_stdout_r, cap_stdout_w) = make_pipe().map_err(SandboxRuntimeError::Io)?;
     let (cap_stderr_r, cap_stderr_w) = make_pipe().map_err(SandboxRuntimeError::Io)?;
 
-    // Spawn producers: each writes stdout to its pipe
+    // Spawn producers: each writes stdout to its pipe. Source and consumer
+    // names share one unique run id so concurrent gathers in the same UID
+    // never collide on their runtime dirs.
+    let run = crate::sandbox::unique_instance_id();
     let mut sandboxes: Vec<Sandbox> = Vec::with_capacity(n + 1);
     for (i, ns) in sources.into_iter().enumerate() {
-        let name = format!("gather-source-{}", ns.name);
+        let name = format!("gather-{run}-source-{}", ns.name);
         let mut sb = ns.stage.sandbox.clone().with_name(name);
         let stdout_fd = source_pipes[i].1.as_raw_fd();
         let cmd_refs: Vec<&str> = ns.stage.args.iter().map(|s| s.as_str()).collect();
@@ -380,7 +388,7 @@ async fn run_gather(
     // Inject _SANDLOCK_GATHER env var
     consumer_sandbox.env.insert("_SANDLOCK_GATHER".to_string(), gather_env);
 
-    let mut consumer_sb = consumer_sandbox.clone().with_name("gather-consumer");
+    let mut consumer_sb = consumer_sandbox.clone().with_name(format!("gather-{run}-consumer"));
     let stdin_fd = source_pipes[n - 1].0.as_raw_fd();
 
     // Build extra fd mappings for non-stdin sources

crates/sandlock-core/src/sandbox.rs

@@ -2487,14 +2487,22 @@ static NEXT_SANDBOX_NAME: std::sync::atomic::AtomicU64 = std::sync::atomic::Atom
 fn sandbox_resolve_name(name: Option<&str>) -> Result<String, crate::error::SandlockError> {
     match name {
         Some(n) => sandbox_validate_name(n.to_string()),
-        None => Ok(format!(
-            "sandbox-{}-{}",
-            std::process::id(),
-            NEXT_SANDBOX_NAME.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
-        )),
+        None => Ok(format!("sandbox-{}", unique_instance_id())),
     }
 }
 
+/// A `<pid>-<counter>` suffix that makes an internally generated sandbox name
+/// unique across processes and within one. The runtime dir under
+/// /dev/shm/sandlock-$UID/ is claimed per name and a live collision is a hard
+/// error, so no internal caller may use a fixed name.
+pub(crate) fn unique_instance_id() -> String {
+    format!(
+        "{}-{}",
+        std::process::id(),
+        NEXT_SANDBOX_NAME.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
+    )
+}
+
 fn sandbox_validate_name(name: String) -> Result<String, crate::error::SandlockError> {
     use crate::error::SandboxRuntimeError;
     if name.is_empty() {

crates/sandlock-core/src/transaction.rs

@@ -673,10 +673,13 @@ async fn drive_txn_stages(
     // (best-effort). See `open_stderr_tee` for why it must not be a dup of fd 2.
     let tee_fd: Option<OwnedFd> = open_stderr_tee();
 
+    // Stage names share one unique run id so concurrent transactions in the
+    // same UID never collide on their runtime dirs.
+    let run = crate::sandbox::unique_instance_id();
     for (i, stage) in stages.into_iter().enumerate() {
         let at = |source: SandlockError| TxnError::Stage { index: i, source };
         let cmd_refs: Vec<&str> = stage.args.iter().map(|s| s.as_str()).collect();
-        let mut sb = stage.sandbox.with_name(format!("txn-stage-{i}"));
+        let mut sb = stage.sandbox.with_name(format!("txn-{run}-stage-{i}"));
         sb.set_shared_cow(shared.clone()).map_err(at)?;
 
         // Redirect the stage's fd 2 onto a pipe the coordinator drains, keeping

crates/sandlock-core/tests/integration/test_cow.rs

@@ -342,7 +342,7 @@ async fn test_seccomp_cow_legacy_stat_honors_whiteout() {
         .unwrap();
 
     let script = "rm gone.txt ; legacy-stat gone.txt ; legacy-lstat gone.txt ; legacy-access gone.txt";
-    let result = policy.clone().with_name("test")
+    let result = policy.clone()
         .run(&[helper.to_str().unwrap(), "sh", "-c", script]).await;
     match result {
         Ok(r) => {
@@ -1204,7 +1204,7 @@ async fn test_cow_child_rm_r_directory_stays_deleted() {
         .unwrap();
 
     let cmd = format!("rm -r {}/d", workdir.display());
-    let result = policy.clone().with_name("test").run(&["sh", "-c", &cmd]).await;
+    let result = policy.clone().run(&["sh", "-c", &cmd]).await;
     match result {
         Ok(r) => {
             assert!(r.success(), "rm -r should succeed, stderr: {}", r.stderr_str().unwrap_or(""));
@@ -1235,7 +1235,7 @@ async fn test_cow_child_mv_directory_preserves_contents() {
         .unwrap();
 
     let cmd = format!("mv {}/d {}/d2", workdir.display(), workdir.display());
-    let result = policy.clone().with_name("test").run(&["sh", "-c", &cmd]).await;
+    let result = policy.clone().run(&["sh", "-c", &cmd]).await;
     match result {
         Ok(r) => {
             assert!(r.success(), "mv should succeed, stderr: {}", r.stderr_str().unwrap_or(""));
@@ -1270,7 +1270,7 @@ async fn test_cow_child_rmdir_nonempty_fails() {
         .unwrap();
 
     let cmd = format!("rmdir {}/d", workdir.display());
-    let result = policy.clone().with_name("test").run(&["sh", "-c", &cmd]).await;
+    let result = policy.clone().run(&["sh", "-c", &cmd]).await;
     match result {
         Ok(r) => {
             assert!(!r.success(), "rmdir of a non-empty directory must fail");