Bound crawler and tool probes through one spawn deadline (#845)

crates/socket-patch-core/src/crawlers/gradle_cache.rs

@@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool {
 /// Whether `bytes` are the pristine download Gradle stored in the hash
 /// directory `dir_name` (their sha1 names it).
 pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool {
-    use sha1::{Digest, Sha1};
-    hash_eq(dir_name, &hex::encode(Sha1::digest(bytes)))
+    hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes))
 }
 
 /// Whether `path` is a version directory of a `files-2.1` tree
@@ -432,8 +431,6 @@ impl DerivedIndex {
     /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes
     /// hash to `pristine_sha1`.
     pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies {
-        use sha1::{Digest, Sha1};
-
         let instrumented = format!("instrumented-{jar_leaf}");
         let mut out = DerivedCopies {
             incomplete: self.incomplete,
@@ -460,7 +457,9 @@ impl DerivedIndex {
                 out.stale.push(path.clone());
             } else if name == jar_leaf || name == instrumented {
                 match crate::utils::fs::read_regular_to_bytes_sync(path) {
-                    Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => {
+                    Ok(bytes)
+                        if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) =>
+                    {
                         out.stale.push(path.clone())
                     }
                     Ok(_) => out.unknown.push(path.clone()),

crates/socket-patch-core/src/patch/jvm_jar.rs

@@ -25,8 +25,6 @@
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
-use sha1::Digest as _;
-
 use crate::crawlers::gradle_cache;
 use crate::hash::git_sha256::compute_git_sha256_from_bytes;
 use crate::manifest::schema::PatchFileInfo;
@@ -353,12 +351,11 @@ fn unpatched_members(
 }
 
 fn sha256_hex(bytes: &[u8]) -> String {
-    use sha2::Digest as _;
-    hex::encode(sha2::Sha256::digest(bytes))
+    crate::utils::digest::sha256_hex_of(bytes)
 }
 
 fn sha1_hex(bytes: &[u8]) -> String {
-    hex::encode(sha1::Sha1::digest(bytes))
+    crate::utils::digest::sha1_hex_of(bytes)
 }
 
 /// `<socket_dir>/jvm-originals/<sha256>.jar`.

crates/socket-patch-core/src/patch/sidecars/maven.rs

@@ -17,8 +17,6 @@
 
 use std::path::{Path, PathBuf};
 
-use sha1::Digest as _;
-
 use super::{
     SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction,
     SidecarPayload, SidecarSeverity,
@@ -44,7 +42,7 @@ impl Algo {
 
     fn digest(self, bytes: &[u8]) -> String {
         match self {
-            Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)),
+            Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes),
             Algo::Md5 => hex::encode(md5(bytes)),
         }
     }

crates/socket-patch-core/src/update/download.rs

@@ -265,19 +265,21 @@ async fn sanity_exec(
     const ETXTBSY_ATTEMPTS: u64 = 10;
     let mut attempt = 0u64;
     let output = loop {
-        let mut cmd = tokio::process::Command::new(staged);
-        cmd.arg("--version")
-            .stdin(std::process::Stdio::null())
-            .stdout(std::process::Stdio::piped())
-            .stderr(std::process::Stdio::null())
-            .kill_on_drop(true);
-        let result = tokio::time::timeout(std::time::Duration::from_secs(10), cmd.output())
-            .await
-            .map_err(|_| {
-                UpdateError::VerifyFailed(
+        let mut cmd = std::process::Command::new(staged);
+        cmd.arg("--version");
+        let result = match crate::utils::fs::run_blocking(move || {
+            crate::utils::process::output_within(cmd, crate::utils::process::PROBE_TIMEOUT)
+        })
+        .await
+        {
+            Ok(output) => Ok(output),
+            Err(crate::utils::process::BoundedError::Spawn(e)) => Err(e),
+            Err(crate::utils::process::BoundedError::TimedOut) => {
+                return Err(UpdateError::VerifyFailed(
                     "downloaded binary hung during its --version self-check".to_string(),
-                )
-            })?;
+                ))
+            }
+        };
         match result {
             Ok(output) => break output,
             Err(e)

crates/socket-patch-core/src/utils/pipenv.rs

@@ -65,7 +65,7 @@ pub async fn installed_major(root: &Path) -> Option<u32> {
     // The RESOLVED path is spawned; a Windows `.bat` / `.cmd` shim is run by
     // `std` itself through cmd.exe with correct quoting (see the shared
     // launcher's docs).
-    let mut command = tokio::process::Command::from(crate::utils::process::command_for(&program));
+    let mut command = crate::utils::process::command_for(&program);
     // The version banner does not depend on a project, so the probe runs in a
     // NEUTRAL directory: with the scanned repository as cwd, Pipenv would read
     // its `.env`, `Pipfile` and `.venv` pointer — committed, attacker-shaped
@@ -76,12 +76,12 @@ pub async fn installed_major(root: &Path) -> Option<u32> {
         .current_dir(std::env::temp_dir())
         .env("PIPENV_DONT_LOAD_ENV", "1")
         .env("PIPENV_NOSPIN", "1")
-        .env("PIPENV_IGNORE_VIRTUALENVS", "1")
-        .kill_on_drop(true);
-    let output = tokio::time::timeout(std::time::Duration::from_secs(10), command.output())
-        .await
-        .ok()?
-        .ok()?;
+        .env("PIPENV_IGNORE_VIRTUALENVS", "1");
+    let output = crate::utils::fs::run_blocking(move || {
+        crate::utils::process::output_within(command, crate::utils::process::PROBE_TIMEOUT)
+    })
+    .await
+    .ok()?;
     if !output.status.success() {
         return None;
     }

crates/socket-patch-core/src/utils/process.rs

@@ -17,8 +17,81 @@
 //! runner or thread a singleton.
 
 use std::ffi::OsString;
+use std::io::Read;
 use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::process::{Command, Output, Stdio};
+use std::time::{Duration, Instant};
+
+/// How long a crawler probe (`gem env gemdir`, `npm root -g`, `python3
+/// --version`, ...) may run before it is killed and answers "no
+/// information". The same budget the Pipenv and Hatch version probes and the
+/// self-update `--version` check use.
+pub const PROBE_TIMEOUT: Duration = Duration::from_secs(10);
+
+/// Why [`output_within`] produced no [`Output`].
+#[derive(Debug)]
+pub enum BoundedError {
+    /// The child could not be spawned (missing program, ETXTBSY, ...).
+    Spawn(std::io::Error),
+    /// The child did not exit and close stdout within the budget; it was
+    /// killed.
+    TimedOut,
+}
+
+/// Run `command` to completion within `budget`: the one bounded spawn every
+/// probe goes through.
+///
+/// stdin is null (the child can't wait for input), stdout is captured and
+/// stderr is discarded. When the child has not exited and closed stdout by
+/// the deadline it is killed and reaped, and the call returns
+/// [`BoundedError::TimedOut`] without waiting for a grandchild that still
+/// holds the pipe (a `#!/bin/sh` shim's `sleep`). A wedged toolchain shim (a
+/// version manager prompting for an install, a Ruby waiting on a network
+/// gem home) therefore costs at most `budget`, never the whole run.
+///
+/// Blocking: async callers run it through `utils::fs::run_blocking`.
+pub fn output_within(mut command: Command, budget: Duration) -> Result<Output, BoundedError> {
+    let deadline = Instant::now() + budget;
+    let mut child = command
+        .stdin(Stdio::null())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::null())
+        .spawn()
+        .map_err(BoundedError::Spawn)?;
+    let mut stdout = child.stdout.take().expect("stdout is piped");
+    let (sender, receiver) = std::sync::mpsc::channel();
+    std::thread::spawn(move || {
+        let mut bytes = Vec::new();
+        let _ = stdout.read_to_end(&mut bytes);
+        let _ = sender.send(bytes);
+    });
+    let timed_out = |child: &mut std::process::Child| {
+        let _ = child.kill();
+        let _ = child.wait();
+        Err(BoundedError::TimedOut)
+    };
+    let Ok(stdout) = receiver.recv_timeout(deadline.saturating_duration_since(Instant::now()))
+    else {
+        return timed_out(&mut child);
+    };
+    loop {
+        match child.try_wait() {
+            Ok(Some(status)) => {
+                return Ok(Output {
+                    status,
+                    stdout,
+                    stderr: Vec::new(),
+                })
+            }
+            Ok(None) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(5)),
+            Ok(None) => return timed_out(&mut child),
+            Err(error) => {
+                let _ = child.kill();
+                return Err(BoundedError::Spawn(error));
+            }
+        }
+    }
+}
 
 /// The executable `name` on ABSOLUTE `PATH` entries only, or `None` when
 /// no entry holds one.
@@ -212,6 +285,16 @@ pub(crate) fn neutral_probe_dir_with(var: &impl Fn(&str) -> Option<OsString>) ->
 /// names a path is spawned as given) with `args`, optionally from `cwd`,
 /// and return its trimmed stdout under the [`CommandRunner`] contract.
 fn run_resolved(bin: &str, args: &[&str], cwd: Option<&Path>) -> Option<String> {
+    run_resolved_within(bin, args, cwd, PROBE_TIMEOUT)
+}
+
+/// [`run_resolved`] under an explicit budget (tests).
+fn run_resolved_within(
+    bin: &str,
+    args: &[&str],
+    cwd: Option<&Path>,
+    budget: Duration,
+) -> Option<String> {
     let program = if Path::new(bin).components().count() > 1 {
         PathBuf::from(bin)
     } else {
@@ -233,7 +316,19 @@ fn run_resolved(bin: &str, args: &[&str], cwd: Option<&Path>) -> Option<String>
     if let Some(cwd) = cwd {
         command.current_dir(cwd);
     }
-    let output = command.output().ok()?;
+    let output = match output_within(command, budget) {
+        Ok(output) => output,
+        Err(BoundedError::TimedOut) => {
+            if crate::utils::env_compat::is_debug_enabled() {
+                eprintln!(
+                    "[socket-patch debug] probe `{bin} {}` did not answer within {budget:?}; treating it as absent",
+                    args.join(" ")
+                );
+            }
+            return None;
+        }
+        Err(BoundedError::Spawn(_)) => return None,
+    };
     if !output.status.success() {
         return None;
     }
@@ -260,6 +355,79 @@ mod tests {
         assert_eq!(out, "hello");
     }
 
+    /// A probe that never answers (a wedged `gem` shim) is killed at the
+    /// budget and answers "no information" instead of hanging the crawl.
+    /// On `main` `run_resolved` waited on `output()` with no deadline.
+    #[cfg(unix)]
+    #[test]
+    fn a_hung_probe_answers_none_within_its_budget() {
+        let tmp = tempfile::tempdir().unwrap();
+        let shim = tmp.path().join("gem");
+        std::fs::write(&shim, "#!/bin/sh\nexec sleep 30\n").unwrap();
+        set_executable(&shim);
+        let start = Instant::now();
+        let out = run_resolved_within(
+            shim.to_str().unwrap(),
+            &["env", "gemdir"],
+            None,
+            Duration::from_millis(300),
+        );
+        assert_eq!(out, None);
+        assert!(
+            start.elapsed() < Duration::from_secs(10),
+            "the budget must bound the probe, took {:?}",
+            start.elapsed()
+        );
+    }
+
+    /// A shim that forks its child (no `exec`) leaves a grandchild holding
+    /// stdout after the shim is killed: the deadline still returns.
+    #[cfg(unix)]
+    #[test]
+    fn output_within_does_not_wait_for_a_grandchild_holding_stdout() {
+        let mut command = Command::new("sh");
+        command.args(["-c", "sleep 30; echo late"]);
+        let start = Instant::now();
+        let result = output_within(command, Duration::from_millis(300));
+        assert!(matches!(result, Err(BoundedError::TimedOut)), "{result:?}");
+        assert!(
+            start.elapsed() < Duration::from_secs(10),
+            "{:?}",
+            start.elapsed()
+        );
+    }
+
+    /// The bounded spawn keeps the `output()` contract the former callers
+    /// relied on: stdout captured, stderr dropped, exit status reported,
+    /// stdin null, a missing program surfaced as a spawn error.
+    #[cfg(unix)]
+    #[test]
+    fn output_within_reports_status_stdout_and_spawn_errors() {
+        let mut ok = Command::new("sh");
+        ok.args(["-c", "printf out; printf err >&2"]);
+        let output = output_within(ok, PROBE_TIMEOUT).unwrap();
+        assert!(output.status.success());
+        assert_eq!(output.stdout, b"out");
+        assert!(output.stderr.is_empty());
+
+        let mut failing = Command::new("sh");
+        failing.args(["-c", "printf partial; exit 3"]);
+        let output = output_within(failing, PROBE_TIMEOUT).unwrap();
+        assert_eq!(output.status.code(), Some(3));
+        assert_eq!(output.stdout, b"partial");
+
+        let mut reads_stdin = Command::new("sh");
+        reads_stdin.args(["-c", "cat; printf done"]);
+        let output = output_within(reads_stdin, PROBE_TIMEOUT).unwrap();
+        assert_eq!(output.stdout, b"done", "stdin is null, so `cat` sees EOF");
+
+        let missing = Command::new("/definitely/not/a/real/binary-1234567");
+        assert!(matches!(
+            output_within(missing, PROBE_TIMEOUT),
+            Err(BoundedError::Spawn(_))
+        ));
+    }
+
     /// Spawn failure → None. The binary name is intentionally one
     /// that should never be on PATH.
     #[test]

crates/socket-patch-core/src/vendor/pypi_hatch.rs

@@ -129,21 +129,18 @@ async fn require_environment_context_support_with(
     var: &impl Fn(&str) -> Option<std::ffi::OsString>,
 ) -> Result<(), Failure> {
     let output = match crate::utils::process::resolve_tool_with("hatch", var) {
-        Some(program) => Some(
-            tokio::time::timeout(
-                std::time::Duration::from_secs(10),
-                tokio::process::Command::from(crate::utils::process::command_for(&program))
-                    .arg("--version")
-                    .current_dir(root)
-                    .stdin(std::process::Stdio::null())
-                    .kill_on_drop(true)
-                    .output(),
-            )
-            .await,
-        ),
+        Some(program) => {
+            let mut command = crate::utils::process::command_for(&program);
+            command.arg("--version").current_dir(root);
+            crate::utils::fs::run_blocking(move || {
+                crate::utils::process::output_within(command, crate::utils::process::PROBE_TIMEOUT)
+            })
+            .await
+            .ok()
+        }
         None => None,
     };
-    if let Some(Ok(Ok(output))) = output {
+    if let Some(output) = output {
         if output.status.success()
             && String::from_utf8_lossy(&output.stdout)
                 .split_whitespace()
@@ -922,4 +919,32 @@ mod tests {
         .unwrap();
         assert!(marker.exists(), "the resolved hatch was not run");
     }
+
+    /// A `hatch` that never answers is killed at the shared probe budget
+    /// and takes the same refusal as one too old.
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn a_hung_hatch_is_refused_within_the_probe_budget() {
+        use std::os::unix::fs::PermissionsExt;
+        let temp = tempfile::tempdir().unwrap();
+        let root = temp.path().join("project");
+        let bin = temp.path().join("bin");
+        std::fs::create_dir(&root).unwrap();
+        std::fs::create_dir(&bin).unwrap();
+        let hatch = bin.join("hatch");
+        std::fs::write(&hatch, "#!/bin/sh\nexec sleep 60\n").unwrap();
+        std::fs::set_permissions(&hatch, std::fs::Permissions::from_mode(0o755)).unwrap();
+        let path = std::env::join_paths([bin.as_path()]).unwrap();
+        let start = std::time::Instant::now();
+        let result = require_environment_context_support_with(&root, &|var| {
+            (var == "PATH").then(|| path.clone())
+        })
+        .await;
+        assert_eq!(result.unwrap_err().0, "pypi_hatch_unsupported");
+        assert!(
+            start.elapsed() < std::time::Duration::from_secs(40),
+            "the probe budget must bound hatch, took {:?}",
+            start.elapsed()
+        );
+    }
 }