From d58a314c6e237eef01518e2d01d72b42e28f14e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 19:57:17 +0000 Subject: [PATCH 1/5] Start refactor for #592 Assisted-by: Claude Code:claude-opus-5-5 From 7b1ef3fee2bc3c5ced8f895a88be5b0af213441b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:12:59 +0000 Subject: [PATCH 2/5] Read crawler project files FIFO-safely A FIFO planted in a checkout at obj/project.assets.json or at vendor//Cargo.toml blocked open(2) forever, so scan, get and apply hung on that repository. The NuGet assets read, both Cargo crate-manifest reads and the Pipenv .venv read now go through the shared FIFO-safe readers in utils::fs; an unreadable entry is skipped exactly as before. Fixes #592. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/cargo_crawler.rs | 91 ++++++++++++++++++- .../src/crawlers/nuget_crawler.rs | 82 ++++++++++++++++- .../src/crawlers/python_crawler.rs | 2 +- 3 files changed, 171 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs index 808876b76..6eacff7c3 100644 --- a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs @@ -286,7 +286,7 @@ impl CargoCrawler { /// name and version. async fn verify_crate_at_path(&self, path: &Path, name: &str, version: &str) -> bool { let cargo_toml_path = path.join("Cargo.toml"); - let content = match tokio::fs::read_to_string(&cargo_toml_path).await { + let content = match crate::utils::fs::read_regular_to_string(&cargo_toml_path).await { Ok(c) => c, Err(_) => return false, }; @@ -398,7 +398,7 @@ fn scan_crate_source(src_path: &Path, seen: &mut HashSet) -> Vec Option<(String, String)> { let cargo_toml_path = crate_path.join("Cargo.toml"); - let content = std::fs::read_to_string(&cargo_toml_path).ok()?; + let content = crate::utils::fs::read_regular_to_string_sync(&cargo_toml_path).ok()?; // Fallback: parse directory name as - parse_cargo_toml_name_version(&content) @@ -1231,4 +1231,91 @@ version = "fake" assert_eq!(rows(&new), rows(&old)); } } + + /// mkfifo(2) directly (no child process; spawning `mkfifo` flakes + /// under heavy parallel load). + #[cfg(unix)] + fn make_fifo(path: &Path) { + use std::os::unix::ffi::OsStrExt; + let c_path = + std::ffi::CString::new(path.as_os_str().as_bytes()).expect("fifo path has no NUL"); + let rc = unsafe { libc::mkfifo(c_path.as_ptr(), 0o644) }; + assert_eq!( + rc, + 0, + "mkfifo(2) failed: {}", + std::io::Error::last_os_error() + ); + } + + /// On timeout the open is wedged in a blocking-pool thread that the + /// runtime waits for on shutdown; connecting a writer releases it so + /// the test FAILS instead of hanging the whole suite. + #[cfg(unix)] + async fn within_deadline(fifo: &Path, what: &str, fut: F) -> F::Output { + match tokio::time::timeout(std::time::Duration::from_secs(5), fut).await { + Ok(out) => out, + Err(_) => { + let _ = std::fs::OpenOptions::new().write(true).open(fifo); + panic!("{what} must not block on a FIFO at {}", fifo.display()); + } + } + } + + /// Regression (#592): a FIFO at `vendor//Cargo.toml` in the + /// project tree used to wedge `crawl_all` (blocking-pool + /// `read_crate_cargo_toml`) and `find_by_purls` (`verify_crate_at_path`) + /// in open(2). Both now go through the FIFO-safe reader and skip the + /// unreadable crate, exactly like any other unreadable manifest. + #[cfg(unix)] + #[tokio::test] + async fn fifo_crate_manifest_is_skipped_not_blocked_on() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write( + dir.path().join("Cargo.toml"), + "[package]\nname = \"root\"\nversion = \"0.1.0\"\n", + ) + .await + .unwrap(); + let vendor = dir.path().join("vendor"); + let left = vendor.join("left-1.0.0"); + tokio::fs::create_dir_all(&left).await.unwrap(); + let fifo = left.join("Cargo.toml"); + make_fifo(&fifo); + // A readable crate beside the FIFO proves the crawl continues. + let serde_dir = vendor.join("serde"); + tokio::fs::create_dir_all(&serde_dir).await.unwrap(); + tokio::fs::write( + serde_dir.join("Cargo.toml"), + "[package]\nname = \"serde\"\nversion = \"1.0.200\"\n", + ) + .await + .unwrap(); + + let crawler = CargoCrawler::new(); + let options = CrawlerOptions { + cwd: dir.path().to_path_buf(), + global: false, + global_prefix: None, + }; + let packages = within_deadline(&fifo, "crawl_all", crawler.crawl_all(&options)).await; + let purls: Vec<_> = packages.iter().map(|p| p.purl.as_str()).collect(); + assert_eq!(purls, vec!["pkg:cargo/serde@1.0.200"]); + + let found = within_deadline( + &fifo, + "find_by_purls", + crawler.find_by_purls( + &vendor, + &[ + "pkg:cargo/left@1.0.0".to_string(), + "pkg:cargo/serde@1.0.200".to_string(), + ], + ), + ) + .await + .unwrap(); + assert!(!found.contains_key("pkg:cargo/left@1.0.0")); + assert!(found.contains_key("pkg:cargo/serde@1.0.200")); + } } diff --git a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs index 582277309..bc70a1245 100644 --- a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs @@ -503,7 +503,7 @@ async fn discover_paths_from_assets(cwd: &Path) -> Vec { /// The file is a JSON object with a `packageFolders` key containing /// folder paths as keys, e.g.: `{"packageFolders": {"/home/user/.nuget/packages/": {}}}`. async fn parse_project_assets_package_folders(path: &Path) -> Option> { - let content = tokio::fs::read_to_string(path).await.ok()?; + let content = crate::utils::fs::read_regular_to_string(path).await.ok()?; let json: serde_json::Value = serde_json::from_str(&content).ok()?; let folders = json.get("packageFolders")?.as_object()?; Some(folders.keys().map(PathBuf::from).collect()) @@ -1513,4 +1513,84 @@ mod tests { ); } } + + /// mkfifo(2) directly (no child process; spawning `mkfifo` flakes + /// under heavy parallel load). + #[cfg(unix)] + fn make_fifo(path: &Path) { + use std::os::unix::ffi::OsStrExt; + let c_path = + std::ffi::CString::new(path.as_os_str().as_bytes()).expect("fifo path has no NUL"); + let rc = unsafe { libc::mkfifo(c_path.as_ptr(), 0o644) }; + assert_eq!( + rc, + 0, + "mkfifo(2) failed: {}", + std::io::Error::last_os_error() + ); + } + + /// On timeout the open is wedged in a blocking-pool thread that the + /// runtime waits for on shutdown; connecting a writer releases it so + /// the test FAILS instead of hanging the whole suite. + #[cfg(unix)] + async fn within_deadline(fifo: &Path, what: &str, fut: F) -> F::Output { + match tokio::time::timeout(std::time::Duration::from_secs(5), fut).await { + Ok(out) => out, + Err(_) => { + let _ = std::fs::OpenOptions::new().write(true).open(fifo); + panic!("{what} must not block on a FIFO at {}", fifo.display()); + } + } + } + + /// Regression (#592): a FIFO at `obj/project.assets.json` used to + /// wedge `get_nuget_package_paths` (and so every NuGet crawl) in + /// open(2). It now goes through the FIFO-safe reader and is skipped + /// like any other unreadable assets file; a sibling sub-project's + /// readable assets file is still discovered. + #[cfg(unix)] + #[tokio::test] + async fn fifo_project_assets_is_skipped_not_blocked_on() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("App.csproj"), "") + .await + .unwrap(); + let obj_dir = dir.path().join("obj"); + tokio::fs::create_dir_all(&obj_dir).await.unwrap(); + let fifo = obj_dir.join("project.assets.json"); + make_fifo(&fifo); + + let pkg_folder = dir.path().join("nuget-cache"); + tokio::fs::create_dir_all(&pkg_folder).await.unwrap(); + let sub_obj = dir.path().join("Lib").join("obj"); + tokio::fs::create_dir_all(&sub_obj).await.unwrap(); + tokio::fs::write( + sub_obj.join("project.assets.json"), + serde_json::to_string(&serde_json::json!({ + "packageFolders": { pkg_folder.to_string_lossy().to_string(): {} } + })) + .unwrap(), + ) + .await + .unwrap(); + + let crawler = NuGetCrawler::new(); + let options = CrawlerOptions { + cwd: dir.path().to_path_buf(), + global: false, + global_prefix: None, + }; + let paths = within_deadline( + &fifo, + "get_nuget_package_paths", + crawler.get_nuget_package_paths(&options), + ) + .await + .unwrap(); + assert!( + paths.contains(&pkg_folder), + "the readable sub-project assets file must still be discovered, got {paths:?}" + ); + } } diff --git a/crates/socket-patch-core/src/crawlers/python_crawler.rs b/crates/socket-patch-core/src/crawlers/python_crawler.rs index dfdee4f52..a66fb7de4 100644 --- a/crates/socket-patch-core/src/crawlers/python_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/python_crawler.rs @@ -1212,7 +1212,7 @@ async fn find_pipenv_virtualenv_site_packages_with( // name under WORKON_HOME; an empty file means the default placement. let dot_venv = cwd.join(".venv"); if dot_venv.is_file() { - if let Ok(text) = std::fs::read_to_string(&dot_venv) { + if let Ok(text) = crate::utils::fs::read_regular_to_string_sync(&dot_venv) { let name = text.trim(); if !name.is_empty() { if name.contains('/') || name.contains('\\') { From a20aade95f84339753dcd531dad52a11d4ab1c0e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:12:59 +0000 Subject: [PATCH 3/5] Guard crawlers against bare file reads An architecture test now fails when crawler production code reads a file with a bare read_to_string, fs::read or File::open, so the FIFO hang fixed for #592 cannot come back. The two machine-repository Maven POM reads are allowlisted by count. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/crawlers/mod.rs | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/crates/socket-patch-core/src/crawlers/mod.rs b/crates/socket-patch-core/src/crawlers/mod.rs index b0c257f50..a63925181 100644 --- a/crates/socket-patch-core/src/crawlers/mod.rs +++ b/crates/socket-patch-core/src/crawlers/mod.rs @@ -28,3 +28,59 @@ pub use pkg_managers::{detect_npm_pkg_manager, NpmPkgManager}; pub use python_crawler::PythonCrawler; pub use ruby_crawler::RubyCrawler; pub use types::*; + +/// ARCHITECTURE GUARD (#592): crawlers read project-tree files only through +/// the FIFO-safe `utils::fs::read_regular_*` readers, so a FIFO planted in a +/// checkout can never wedge `scan`, `get` or `apply` in open(2). +#[cfg(test)] +mod architecture_tests { + use std::path::Path; + + const BARE_READS: [&str; 3] = ["fs::read_to_string(", "fs::read(", "File::open("]; + + /// Reads of the machine-wide Maven repository (not the project tree), + /// out of scope for #592: file name and number of allowed bare reads. + const ALLOWED: [(&str, usize); 1] = [("maven_crawler.rs", 2)]; + + #[test] + fn crawlers_read_project_files_fifo_safely() { + let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/crawlers"); + let mut checked = 0; + for entry in std::fs::read_dir(&dir).expect("read crawlers dir") { + let path = entry.expect("dir entry").path(); + if path.extension().is_none_or(|e| e != "rs") { + continue; + } + let name = path.file_name().unwrap().to_string_lossy().into_owned(); + let src = std::fs::read_to_string(&path).expect("read crawler source"); + // Production code ends at the first in-file test module + // (`mod tests`, or this guard in `mod.rs`); earlier + // `#[cfg(test)] mod oracle;` declarations are only one line. + let prod_end = [ + "#[cfg(test)]\nmod tests", + "#[cfg(test)]\nmod architecture_tests", + ] + .iter() + .filter_map(|marker| src.find(marker)) + .min() + .unwrap_or(src.len()); + let prod = &src[..prod_end]; + let bare = prod + .lines() + .filter(|l| !l.trim_start().starts_with("//")) + .filter(|l| BARE_READS.iter().any(|r| l.contains(r))) + .count(); + let allowed = ALLOWED + .iter() + .find(|(file, _)| *file == name) + .map_or(0, |(_, n)| *n); + assert_eq!( + bare, allowed, + "{name} has {bare} bare file reads in production code (allowed {allowed}); \ + use utils::fs::read_regular_to_string{{,_sync}} instead" + ); + checked += 1; + } + assert!(checked >= 10, "only {checked} crawler files found"); + } +} From ac9becea5258f4641bedef544786df6916d55c0a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:38:09 +0000 Subject: [PATCH 4/5] Release FIFO test watchdog without blocking The timeout cleanup in the #592 FIFO regression tests opened the FIFO for writing in blocking mode. If no reader was blocked on it (the timeout had another cause), that open itself hung the suite. Open it with O_NONBLOCK so it fails with ENXIO instead. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/crawlers/cargo_crawler.rs | 9 ++++++++- crates/socket-patch-core/src/crawlers/nuget_crawler.rs | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs index 6eacff7c3..35cf18d64 100644 --- a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs @@ -1256,7 +1256,14 @@ version = "fake" match tokio::time::timeout(std::time::Duration::from_secs(5), fut).await { Ok(out) => out, Err(_) => { - let _ = std::fs::OpenOptions::new().write(true).open(fifo); + // O_NONBLOCK: with no reader blocked on the FIFO (the + // timeout had another cause) a blocking writer open would + // itself hang; non-blocking it just fails with ENXIO. + use std::os::unix::fs::OpenOptionsExt; + let _ = std::fs::OpenOptions::new() + .write(true) + .custom_flags(libc::O_NONBLOCK) + .open(fifo); panic!("{what} must not block on a FIFO at {}", fifo.display()); } } diff --git a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs index bc70a1245..4e2fcd1b2 100644 --- a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs @@ -1538,7 +1538,14 @@ mod tests { match tokio::time::timeout(std::time::Duration::from_secs(5), fut).await { Ok(out) => out, Err(_) => { - let _ = std::fs::OpenOptions::new().write(true).open(fifo); + // O_NONBLOCK: with no reader blocked on the FIFO (the + // timeout had another cause) a blocking writer open would + // itself hang; non-blocking it just fails with ENXIO. + use std::os::unix::fs::OpenOptionsExt; + let _ = std::fs::OpenOptions::new() + .write(true) + .custom_flags(libc::O_NONBLOCK) + .open(fifo); panic!("{what} must not block on a FIFO at {}", fifo.display()); } } From a024719ab281f48223153f1ec641e07aa450d585 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 21:09:04 +0000 Subject: [PATCH 5/5] Match test-module markers on CRLF checkouts On a Windows core.autocrlf checkout the crawler guard's "#[cfg(test)]\nmod tests" marker never matched, so it counted a test-only Maven read as production code and failed the Windows test job. Normalize CRLF to LF before locating the test module. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/crawlers/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/crawlers/mod.rs b/crates/socket-patch-core/src/crawlers/mod.rs index a63925181..fde14bdec 100644 --- a/crates/socket-patch-core/src/crawlers/mod.rs +++ b/crates/socket-patch-core/src/crawlers/mod.rs @@ -52,7 +52,11 @@ mod architecture_tests { continue; } let name = path.file_name().unwrap().to_string_lossy().into_owned(); - let src = std::fs::read_to_string(&path).expect("read crawler source"); + // A Windows (core.autocrlf) checkout has CRLF lines; normalize so + // the `\n`-joined test-module markers below still match. + let src = std::fs::read_to_string(&path) + .expect("read crawler source") + .replace("\r\n", "\n"); // Production code ends at the first in-file test module // (`mod tests`, or this guard in `mod.rs`); earlier // `#[cfg(test)] mod oracle;` declarations are only one line.