From 358792d956cecf9240769e76cda9fddb5cab7a99 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:57:54 +0000 Subject: [PATCH 1/3] Start refactor for #845 Assisted-by: Claude Code:claude-opus-5-5 From b036f7d89ae5c4639a3703641dd11756edc04bcf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 19:06:51 +0000 Subject: [PATCH 2/3] Bound crawler and tool probes by one deadline A version-manager shim that never answers (`gem`, `python3`, `npm`, `composer` behind rbenv/asdf, a Ruby waiting on a network gem home) used to hang `scan`, `apply`, `vex` and every other crawling command forever with no output: the crawler probes waited on `output()` with no deadline. Every probe now runs through one `utils::process::output_within` primitive: null stdin, captured stdout, dropped stderr, and the child killed and reaped at the deadline without waiting on a grandchild that still holds the pipe. Crawler probes get the same 10 s budget that the Pipenv and Hatch version probes and the self-update `--version` check already used, and those three sites drop their hand-rolled `tokio::time::timeout` + `kill_on_drop` blocks for it. Refs #845. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/update/download.rs | 26 +-- crates/socket-patch-core/src/utils/pipenv.rs | 14 +- crates/socket-patch-core/src/utils/process.rs | 172 +++++++++++++++++- .../src/vendor/pypi_hatch.rs | 51 ++++-- 4 files changed, 229 insertions(+), 34 deletions(-) diff --git a/crates/socket-patch-core/src/update/download.rs b/crates/socket-patch-core/src/update/download.rs index ea7339f63..dcd322fbb 100644 --- a/crates/socket-patch-core/src/update/download.rs +++ b/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) diff --git a/crates/socket-patch-core/src/utils/pipenv.rs b/crates/socket-patch-core/src/utils/pipenv.rs index 8ef12a463..0e7836827 100644 --- a/crates/socket-patch-core/src/utils/pipenv.rs +++ b/crates/socket-patch-core/src/utils/pipenv.rs @@ -65,7 +65,7 @@ pub async fn installed_major(root: &Path) -> Option { // 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 { .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; } diff --git a/crates/socket-patch-core/src/utils/process.rs b/crates/socket-patch-core/src/utils/process.rs index c69552a73..380babbcd 100644 --- a/crates/socket-patch-core/src/utils/process.rs +++ b/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 { + 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) -> /// 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 { + 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 { 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 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] diff --git a/crates/socket-patch-core/src/vendor/pypi_hatch.rs b/crates/socket-patch-core/src/vendor/pypi_hatch.rs index 43d1cf2d6..6916e6e39 100644 --- a/crates/socket-patch-core/src/vendor/pypi_hatch.rs +++ b/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, ) -> 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() + ); + } } From 7b2eabdf7639320a34459e2ad5e0f99b47476ec2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 19:49:57 +0000 Subject: [PATCH 3/3] Port #878: Gradle digests through utils::digest `main` fails `utils::digest::tests::production_digests_go_through_the_ helpers` because #646 left inline sha1/sha256 calls in `gradle_cache.rs`, `jvm_jar.rs` and `sidecars/maven.rs`, which turns `test`, `test-release` and `coverage` red on every PR. This is #878's change verbatim; it no-ops once #878 merges. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/crawlers/gradle_cache.rs | 9 ++++----- crates/socket-patch-core/src/patch/jvm_jar.rs | 7 ++----- crates/socket-patch-core/src/patch/sidecars/maven.rs | 4 +--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/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()), diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/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) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/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)), } }