commit changes so far

Author: mikolalysenko

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

@@ -236,10 +236,14 @@ async fn get_composer_home() -> Option<PathBuf> {
         }
     }
 
-    // Platform defaults
+    // Platform defaults. A set-but-empty HOME counts as unset: honoring
+    // `""` would turn the `.composer`/`.config/composer` probes below into
+    // CWD-relative paths inside the user's project (same rule as
+    // `utils::fs::home_dir`).
     let home_dir = std::env::var("HOME")
-        .or_else(|_| std::env::var("USERPROFILE"))
-        .ok()?;
+        .ok()
+        .filter(|h| !h.is_empty())
+        .or_else(|| std::env::var("USERPROFILE").ok().filter(|h| !h.is_empty()))?;
     let home = PathBuf::from(home_dir);
 
     let candidates = [

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

@@ -248,9 +248,14 @@ impl GoCrawler {
                 return Some(first.join("pkg").join("mod"));
             }
         }
+        // A set-but-empty HOME/USERPROFILE counts as unset, matching the
+        // GOMODCACHE and GOPATH guards above: honoring `""` would yield the
+        // RELATIVE path `go/pkg/mod`, pointing the crawl at a directory
+        // inside the user's project instead of a real module cache.
         let home = std::env::var("HOME")
-            .or_else(|_| std::env::var("USERPROFILE"))
-            .ok()?;
+            .ok()
+            .filter(|h| !h.is_empty())
+            .or_else(|| std::env::var("USERPROFILE").ok().filter(|h| !h.is_empty()))?;
         Some(PathBuf::from(home).join("go").join("pkg").join("mod"))
     }
 

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

@@ -32,7 +32,18 @@ struct PackageJsonPartial {
 
 /// Read and parse a `package.json` file, returning `(name, version)` if valid.
 pub async fn read_package_json(pkg_json_path: &Path) -> Option<(String, String)> {
-    let content = tokio::fs::read_to_string(pkg_json_path).await.ok()?;
+    use tokio::io::AsyncReadExt;
+
+    // The path lives inside the (untrusted) package tree: a planted FIFO
+    // would make a plain `read_to_string` open block forever waiting for a
+    // writer, wedging scan (crawl_all) and apply (find_by_purls). Open via
+    // `open_regular_file` — non-blocking on Unix, rejecting
+    // FIFOs/devices/directories (see its docs).
+    let (mut file, metadata) = crate::utils::fs::open_regular_file(pkg_json_path)
+        .await
+        .ok()?;
+    let mut content = String::with_capacity(metadata.len() as usize);
+    file.read_to_string(&mut content).await.ok()?;
     // npm and Node both tolerate a leading UTF-8 BOM in package.json
     // (Windows-authored packages ship them), but serde_json rejects it —
     // a BOM'd install would be invisible to scan and unpatchable.

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

@@ -424,6 +424,10 @@ pub async fn get_global_python_site_packages() -> Vec<PathBuf> {
     // uv tools — platform-specific install root.
     #[cfg(target_os = "macos")]
     {
+        // Legacy/secondary location only: uv follows XDG conventions on
+        // macOS (`uv tool dir` → ~/.local/share/uv/tools, covered by the
+        // not(windows) scan below), but older layouts used the platform
+        // data dir, so keep scanning it too.
         let uv_base = home_dir
             .join("Library")
             .join("Application Support")
@@ -446,8 +450,11 @@ pub async fn get_global_python_site_packages() -> Vec<PathBuf> {
             }
         }
     }
-    #[cfg(all(not(target_os = "macos"), not(windows)))]
+    #[cfg(not(windows))]
     {
+        // uv uses XDG paths on BOTH Linux and macOS (`uv tool dir` →
+        // ~/.local/share/uv/tools; verified against a real uv install —
+        // macOS does NOT get an Application Support tool dir).
         let uv_base = home_dir
             .join(".local")
             .join("share")
@@ -509,13 +516,18 @@ pub async fn get_global_python_site_packages() -> Vec<PathBuf> {
 ///   * `requirements.txt` — pip-compile / bare requirements
 ///   * `uv.lock` — uv-managed projects (PEP 751 export sibling is
 ///     `pylock.toml` but in practice `uv.lock` is what ships)
+///   * `Pipfile` / `Pipfile.lock` — pipenv projects, which commonly
+///     ship NEITHER pyproject.toml nor requirements.txt and keep
+///     their venvs out-of-tree (`~/.local/share/virtualenvs`)
 pub async fn is_python_project(cwd: &Path) -> bool {
     let markers = [
         "pyproject.toml",
         "setup.py",
         "setup.cfg",
         "requirements.txt",
         "uv.lock",
+        "Pipfile",
+        "Pipfile.lock",
     ];
     for m in &markers {
         if tokio::fs::metadata(cwd.join(m)).await.is_ok() {
@@ -614,11 +626,16 @@ impl PythonCrawler {
     ) -> Result<HashMap<String, CrawledPackage>, std::io::Error> {
         let mut result = HashMap::new();
 
-        // Build lookup: canonicalized-name@version -> purl
+        // Build lookup: canonicalized-name@version -> purl. The API serves
+        // purls percent-encoded (a PEP 440 local/epoch version carries
+        // `+`/`!`, arriving as `%2B`/`%21`), so decode the coordinates
+        // before keying or the installed package never matches.
         let mut purl_lookup: HashMap<String, &str> = HashMap::new();
         for purl in purls {
             if let Some((name, version)) = crate::utils::purl::parse_pypi_purl(purl) {
-                let key = format!("{}@{}", canonicalize_pypi_name(name), version);
+                let name = crate::utils::purl::percent_decode_purl_component(name);
+                let version = crate::utils::purl::percent_decode_purl_component(version);
+                let key = format!("{}@{}", canonicalize_pypi_name(&name), version);
                 purl_lookup.insert(key, purl.as_str());
             }
         }

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

@@ -120,7 +120,25 @@ pub async fn verify_file_rollback(
             }
             Ok(_) => {}
         }
-        let current_hash = compute_file_git_sha256(&filepath).await.unwrap_or_default();
+        // A hash failure (directory/FIFO planted at the path, unreadable
+        // file, dangling symlink target) is the same unverifiable state as a
+        // stat failure above and must fail closed with the real error — a
+        // swallowed error would misreport "modified after patching" with a
+        // fabricated empty hash, and would compare equal to an empty
+        // `after_hash`, wrongly clearing the entry for deletion.
+        let current_hash = match compute_file_git_sha256(&filepath).await {
+            Ok(h) => h,
+            Err(e) => {
+                return VerifyRollbackResult {
+                    file: file_name.to_string(),
+                    status: VerifyRollbackStatus::NotFound,
+                    message: Some(format!("Failed to hash file: {}", e)),
+                    current_hash: None,
+                    expected_hash: None,
+                    target_hash: None,
+                };
+            }
+        };
         if current_hash == file_info.after_hash {
             return VerifyRollbackResult {
                 file: file_name.to_string(),

crates/socket-patch-core/src/pth_hook/edit.rs

@@ -189,11 +189,30 @@ fn pyproject_add(content: &str) -> Result<Option<String>, String> {
         .and_then(|t| t.get("poetry"))
         .and_then(Item::as_table)
         .is_some();
+    // PEP 621 forbids a field that is both listed in `dynamic` and set
+    // statically, so a project with `dynamic = ["dependencies"]` (setuptools/
+    // hatch dynamic metadata, or Poetry 2.x keeping its dependency surface in
+    // `[tool.poetry.dependencies]`) must not gain a static array — every
+    // backend would refuse to build the manifest.
+    let dynamic_deps = doc
+        .get("project")
+        .and_then(Item::as_table)
+        .and_then(|t| t.get("dynamic"))
+        .and_then(Item::as_array)
+        .map(|a| a.iter().any(|v| v.as_str() == Some("dependencies")))
+        .unwrap_or(false);
 
-    let changed = if has_poetry && !real_pep621 {
+    let changed = if has_poetry && (!real_pep621 || dynamic_deps) {
         poetry_add(&mut doc)?
-    } else if real_pep621 {
+    } else if real_pep621 && !dynamic_deps {
         pep621_add(&mut doc)?
+    } else if dynamic_deps {
+        return Err(
+            "pyproject.toml declares `[project].dependencies` as dynamic; adding a static \
+             dependencies array would make the manifest invalid — declare the hook in the \
+             source the dynamic metadata is resolved from (or use requirements.txt) instead"
+                .to_string(),
+        );
     } else {
         // Neither surface exists (e.g. a `[build-system]`-only or tool-config-only
         // pyproject.toml of a setup.py/setup.cfg project). Synthesizing a
@@ -948,7 +967,12 @@ mod tests {
             .unwrap();
         let res = remove_hook_dependency(&req, ManifestKind::Requirements, false).await;
         assert_eq!(res.status, PthStatus::Updated);
-        let mode = tokio::fs::metadata(&req).await.unwrap().permissions().mode() & 0o777;
+        let mode = tokio::fs::metadata(&req)
+            .await
+            .unwrap()
+            .permissions()
+            .mode()
+            & 0o777;
         assert_eq!(mode, 0o744, "remove must not reset requirements.txt mode");
     }
 

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

@@ -148,16 +148,21 @@ pub(crate) async fn entry_file_type(entry: &DirEntry) -> Option<FileType> {
 
 /// Resolve the user's home directory: `HOME`, then `USERPROFILE`
 /// (Windows), then a literal `"~"` — a harmless non-existent path so
-/// downstream joins probe nothing rather than panic. The shared
+/// downstream joins probe nothing rather than panic. A set-but-empty
+/// variable counts as unset: honoring `""` would turn every
+/// `home_dir().join(…)` probe into a CWD-relative path, pointing the
+/// crawlers at directories inside the user's project. The shared
 /// fallback chain for every crawler that scans well-known per-user
 /// package roots (`~/.cargo`, `~/.m2`, `~/.nuget`, …) and for
 /// telemetry's home-dir redaction; previously copy-pasted into each.
 /// The go/composer crawlers deliberately use a stricter
 /// no-home-means-no-path chain instead.
 pub(crate) fn home_dir() -> PathBuf {
     let home = std::env::var("HOME")
-        .or_else(|_| std::env::var("USERPROFILE"))
-        .unwrap_or_else(|_| "~".to_string());
+        .ok()
+        .filter(|h| !h.is_empty())
+        .or_else(|| std::env::var("USERPROFILE").ok().filter(|h| !h.is_empty()))
+        .unwrap_or_else(|| "~".to_string());
     PathBuf::from(home)
 }
 
@@ -474,6 +479,35 @@ mod tests {
         assert_eq!(tokio::fs::read(&fresh).await.unwrap(), b"x");
     }
 
+    /// Regression: a set-but-empty `HOME` (stripped CI/container/sudo
+    /// environments) must be treated as unset, exactly like the documented
+    /// no-home fallback. Honoring `""` made `home_dir()` return an empty
+    /// `PathBuf`, so every `home_dir().join(".cargo")`-style probe became a
+    /// CWD-relative path and the crawlers scanned directories inside the
+    /// user's project as if they were the per-user package roots.
+    #[test]
+    #[serial_test::serial]
+    fn home_dir_treats_empty_home_as_unset() {
+        let prev_home = std::env::var("HOME").ok();
+        let prev_profile = std::env::var("USERPROFILE").ok();
+        std::env::set_var("HOME", "");
+        std::env::set_var("USERPROFILE", "");
+        let home = home_dir();
+        match prev_home {
+            Some(v) => std::env::set_var("HOME", v),
+            None => std::env::remove_var("HOME"),
+        }
+        match prev_profile {
+            Some(v) => std::env::set_var("USERPROFILE", v),
+            None => std::env::remove_var("USERPROFILE"),
+        }
+        assert_eq!(
+            home,
+            PathBuf::from("~"),
+            "empty HOME/USERPROFILE must fall back to the harmless `~` sentinel"
+        );
+    }
+
     /// `entry_file_type` is the symlink-aware counterpart: it reports
     /// the link itself (`is_symlink`), never the resolved target.
     #[cfg(unix)]

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

@@ -177,10 +177,16 @@ pub fn sanitize_error_message(message: &str) -> String {
     // `home_dir()` falls back to a literal `"~"` when no home is set, and
     // replacing `"~"` with `"~"` is a no-op. A set-but-empty HOME must be
     // skipped explicitly — replacing `""` would splice `~` between every byte.
+    // Trailing separators are trimmed so a `HOME=/home/user/` redaction keeps
+    // the separator (`~/.cache`, not `~.cache`); a home that trims to nothing
+    // (`HOME=/`, common for unmapped-UID containers) is a filesystem root with
+    // no user-identifying prefix to redact — replacing it would splice `~`
+    // between every path segment in the message.
+    let home = home.trim_end_matches(['/', '\\']);
     if home.is_empty() {
         return message.to_string();
     }
-    message.replace(home.as_ref(), "~")
+    message.replace(home, "~")
 }
 
 /// Build a telemetry event. `error` is an `(error_type, message)` pair; the