Fix vendored Hatch refs in %/#/;/? checkouts (#547)

crates/socket-patch-cli/tests/common/cache_env.rs

@@ -59,6 +59,7 @@
 
 #![allow(dead_code)]
 
+use std::ffi::OsString;
 use std::path::PathBuf;
 use std::process::Command;
 
@@ -285,8 +286,22 @@ pub fn is_ambient_bun_var(name: &str) -> bool {
 /// rule in the module docs: the scrub iterates the parent env and would
 /// otherwise remove what those seed).
 pub fn scrub_ambient_bun_env(cmd: &mut Command) {
-    for (k, _) in std::env::vars_os() {
-        if is_ambient_bun_var(&k.to_string_lossy()) {
+    scrub_parent_vars(cmd, parent_var_names(), is_ambient_bun_var);
+}
+
+/// The names of the PARENT environment, read once.
+fn parent_var_names() -> Vec<OsString> {
+    std::env::vars_os().map(|(k, _)| k).collect()
+}
+
+/// Remove every name in `parent` that `ambient` matches from `cmd`. The
+/// scrubs and their self-tests share it so a self-test can check the
+/// result against the same snapshot of the parent env the scrub saw:
+/// in-process tests in the same binary mirror flags into the env
+/// (`--offline` sets `SOCKET_OFFLINE`), so two separate reads race.
+fn scrub_parent_vars(cmd: &mut Command, parent: Vec<OsString>, ambient: fn(&str) -> bool) {
+    for k in parent {
+        if ambient(&k.to_string_lossy()) {
             cmd.env_remove(&k);
         }
     }
@@ -316,11 +331,7 @@ pub fn is_ambient_vlt_var(name: &str) -> bool {
 /// Remove every [`is_ambient_vlt_var`] variable of the PARENT environment
 /// from `cmd` (call it before [`isolate`], see the module docs).
 pub fn scrub_ambient_vlt_env(cmd: &mut Command) {
-    for (k, _) in std::env::vars_os() {
-        if is_ambient_vlt_var(&k.to_string_lossy()) {
-            cmd.env_remove(&k);
-        }
-    }
+    scrub_parent_vars(cmd, parent_var_names(), is_ambient_vlt_var);
 }
 
 // ── Self-tests ────────────────────────────────────────────────────────
@@ -373,8 +384,9 @@ mod cache_env_selftests {
     /// nothing else is touched, and no matching parent variable is missed.
     #[test]
     fn ambient_bun_scrub_removes_only_matching_parent_vars() {
+        let parent = parent_var_names();
         let mut cmd = Command::new("true");
-        scrub_ambient_bun_env(&mut cmd);
+        scrub_parent_vars(&mut cmd, parent.clone(), is_ambient_bun_var);
         let removed: Vec<String> = cmd
             .get_envs()
             .filter(|(_, value)| value.is_none())
@@ -386,7 +398,7 @@ mod cache_env_selftests {
                 "{name} was removed but is not an ambient bun var"
             );
         }
-        for (name, _) in std::env::vars_os() {
+        for name in &parent {
             let name = name.to_string_lossy();
             if is_ambient_bun_var(&name) {
                 assert!(
@@ -435,8 +447,9 @@ mod cache_env_selftests {
 
     #[test]
     fn ambient_vlt_scrub_removes_only_matching_parent_vars() {
+        let parent = parent_var_names();
         let mut cmd = Command::new("true");
-        scrub_ambient_vlt_env(&mut cmd);
+        scrub_parent_vars(&mut cmd, parent.clone(), is_ambient_vlt_var);
         let removed: Vec<String> = cmd
             .get_envs()
             .filter(|(_, value)| value.is_none())
@@ -445,7 +458,7 @@ mod cache_env_selftests {
         for name in &removed {
             assert!(is_ambient_vlt_var(name), "{name} was removed");
         }
-        for (name, _) in std::env::vars_os() {
+        for name in &parent {
             let name = name.to_string_lossy();
             if is_ambient_vlt_var(&name) {
                 assert!(removed.iter().any(|r| *r == name), "{name} kept");

crates/socket-patch-cli/tests/mode_migration_pypi.rs

@@ -3096,3 +3096,79 @@ async fn pipenv_residual_export_takeover_refusal_names_the_export() {
         "the takeover reclaims the vendored wheel: {env:#}"
     );
 }
+
+/// The checkout directory names #547 reproduced. Hatch expands `{root:uri}`
+/// with hatchling's `path_to_uri`, which escapes only spaces, so pip reads
+/// `%XX` as an escape, `#` as a fragment, `?` as a query, and the PEP 508
+/// parser rejects `;`. (`?` cannot be a Windows file name.)
+fn unsafe_hatch_roots() -> Vec<&'static str> {
+    let mut names = vec!["app_feature%2Fsix-patch", "p%20q", "a#b", "semi;colon"];
+    if cfg!(unix) {
+        names.push("q?x");
+    }
+    names
+}
+
+/// #547: vendoring a Hatch project whose checkout path contains a character
+/// Hatch's `{root:uri}` leaves unescaped is refused before anything is
+/// written, so the scan no longer reports success for wiring no Hatch env
+/// can install. Paths Hatch does expand correctly (`@`, `'`, a space) still
+/// vendor.
+#[tokio::test]
+async fn hatch_vendored_refuses_a_root_hatch_cannot_expand() {
+    for name in unsafe_hatch_roots() {
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path().join(name);
+        std::fs::create_dir_all(&root).unwrap();
+        let files = stage_hatch(&root);
+        let original = std::fs::read_to_string(root.join(files[0])).unwrap();
+        stage_manifest(&root);
+        let (code, env) = run_cli(&root, &["vendor"], &[]);
+        assert_eq!(code, 1, "{name}: {env:#}");
+        let rendered = env.to_string();
+        assert!(
+            rendered.contains("pypi_hatch_unsupported") && rendered.contains("{root:uri}"),
+            "{name}: the root-path guard is the reported refusal: {env:#}"
+        );
+        assert_eq!(
+            std::fs::read_to_string(root.join(files[0])).unwrap(),
+            original,
+            "{name}: pyproject.toml untouched"
+        );
+        assert!(
+            !root.join(".socket/vendor/state.json").exists(),
+            "{name}: nothing recorded"
+        );
+    }
+    for name in ["at@sign", "quo'te", "sp ace"] {
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path().join(name);
+        std::fs::create_dir_all(&root).unwrap();
+        let files = stage_hatch(&root);
+        vendor_project(&root, files);
+    }
+}
+
+/// #547: a vendored Hatch checkout moved to a path Hatch cannot expand
+/// (a Jenkins multibranch workspace such as `app_feature%2Fsix-patch`)
+/// fails the next `vendor` run instead of passing it as in sync, and the
+/// committed wiring is left as it was.
+#[tokio::test]
+async fn hatch_vendored_rerun_refuses_after_relocation_to_unsafe_root() {
+    let (tmp, root) = project();
+    let files = stage_hatch(&root);
+    vendor_project(&root, files);
+    let wired = std::fs::read_to_string(root.join(files[0])).unwrap();
+    let moved = tmp.path().join("app_feature%2Fsix-patch");
+    std::fs::rename(&root, &moved).unwrap();
+    let (code, env) = run_cli(&moved, &["vendor"], &[]);
+    assert_eq!(code, 1, "{env:#}");
+    assert!(
+        env.to_string().contains("pypi_hatch_unsupported"),
+        "{env:#}"
+    );
+    assert_eq!(
+        std::fs::read_to_string(moved.join(files[0])).unwrap(),
+        wired
+    );
+}

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

@@ -11,6 +11,39 @@ use crate::vendor::common::{pep508_name, pyproject_dependency_specs, DeclTable};
 /// exactly these from the candidate set, so the lists cannot drift.
 pub const HATCH_FILES: [&str; 2] = ["pyproject.toml", "hatch.toml"];
 
+/// The first character of `root`'s path that Hatch's `{root:uri}` expands
+/// into a broken file URL (#547). Hatchling's `path_to_uri` escapes only
+/// the space, so pip percent-decodes a `%XX` escape, cuts the path at a
+/// `#` fragment or `?` query, and the PEP 508 parser stops at a `;`, a tab
+/// or any other whitespace or control character. Hatch builds the URL from
+/// the real path (symlinks resolved), so that is the path checked; only its
+/// named components are, so a Windows `\\?\` prefix never counts. `None`
+/// when the expansion is a usable URL.
+pub fn root_uri_unsafe_char(root: &std::path::Path) -> Option<char> {
+    let real = std::fs::canonicalize(root)
+        .or_else(|_| std::path::absolute(root))
+        .unwrap_or_else(|_| root.to_path_buf());
+    real.components()
+        .filter_map(|component| match component {
+            std::path::Component::Normal(name) => Some(name.to_string_lossy()),
+            _ => None,
+        })
+        .find_map(|name| {
+            let chars: Vec<char> = name.chars().collect();
+            chars.iter().enumerate().find_map(|(i, &c)| {
+                let escape = c == '%'
+                    && chars
+                        .get(i + 1..i + 3)
+                        .is_some_and(|hex| hex.iter().all(char::is_ascii_hexdigit));
+                (escape
+                    || matches!(c, '#' | '?' | ';')
+                    || c.is_control()
+                    || (c.is_whitespace() && c != ' '))
+                    .then_some(c)
+            })
+        })
+}
+
 pub fn is_hatch(files: &BTreeMap<String, String>) -> bool {
     files.contains_key("hatch.toml")
         || files.get("pyproject.toml").is_some_and(|text| {

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

@@ -41,6 +41,7 @@ pub(super) async fn load(
 ) -> Result<HatchProject, Failure> {
     let files = read_files(root).await?;
     require_pip_installer()?;
+    require_expandable_root(root)?;
     let prefix = format!("{{root:uri}}/.socket/vendor/pypi/{uuid}/");
     let state = super::state::load_state_shared(root)
         .await
@@ -107,20 +108,36 @@ pub(super) async fn load(
 }
 
 /// The guards vendoring applies whatever the project's wiring: the pip
-/// installer (environment variable and environment settings), and
-/// Hatch >=1.2 when `name` is an environment dependency. Checked before a
+/// installer (environment variable and environment settings), a project
+/// path `{root:uri}` can expand, and Hatch >=1.2 when `name` is an
+/// environment dependency. Checked before a
 /// superseded patch's wiring is unwound, so a refusal unrelated to that
 /// wiring never touches the project's files.
 pub(super) async fn preflight(root: &Path, name: &str) -> Result<(), Failure> {
     let files = read_files(root).await?;
     require_pip_installer()?;
+    require_expandable_root(root)?;
     hatch::require_pip_installer(&files).map_err(|error| ("pypi_hatch_unsupported", error))?;
     if hatch::has_environment_dependency(&files, name) {
         require_environment_context_support(root).await?;
     }
     Ok(())
 }
 
+/// Refuse a project root Hatch's `{root:uri}` cannot expand into a usable
+/// file URL (#547): every vendored reference would fail to install there.
+fn require_expandable_root(root: &Path) -> Result<(), Failure> {
+    match hatch::root_uri_unsafe_char(root) {
+        Some(c) => Err((
+            "pypi_hatch_unsupported",
+            format!(
+                "the project path contains {c:?}, which Hatch's {{root:uri}} does not escape, so vendored references would not install from this checkout; move the checkout to a path without it, or use hosted or agent mode"
+            ),
+        )),
+        None => Ok(()),
+    }
+}
+
 fn require_pip_installer() -> Result<(), Failure> {
     if std::env::var("HATCH_ENV_TYPE_VIRTUAL_UV_PATH").is_ok_and(|path| !path.is_empty()) {
         return Err(("pypi_hatch_unsupported", "vendored Hatch wheels require the pip installer: uv does not enforce local wheel fragment hashes".into()));
@@ -439,6 +456,66 @@ mod tests {
         assert!(held(format!("\u{feff}\u{feff}{live}")));
     }
 
+    /// A symlink in `dir` to `target` (Unix), else `target` itself.
+    fn linked(dir: &Path, target: &Path) -> std::path::PathBuf {
+        #[cfg(unix)]
+        {
+            let link = dir.join("link");
+            std::os::unix::fs::symlink(target, &link).unwrap();
+            link
+        }
+        #[cfg(not(unix))]
+        {
+            let _ = dir;
+            target.to_path_buf()
+        }
+    }
+
+    /// #547: `load` and `preflight` refuse a project root Hatch's
+    /// `{root:uri}` expands into a broken URL, and accept the characters it
+    /// handles (`@`, `'`, a space).
+    #[tokio::test]
+    async fn root_hatch_cannot_expand_is_refused() {
+        let mut unsafe_names = vec!["feature%2Fx", "p%20q", "a#b", "semi;colon"];
+        if cfg!(unix) {
+            unsafe_names.extend(["q?x", "tab\tbed"]);
+        }
+        for name in unsafe_names {
+            let temp = tempfile::tempdir().unwrap();
+            let root = temp.path().join(name);
+            tokio::fs::create_dir_all(&root).await.unwrap();
+            tokio::fs::write(root.join("pyproject.toml"), ORIGINAL)
+                .await
+                .unwrap();
+            let Err((code, detail)) = load(&root, "one", "1", UUID).await else {
+                panic!("{name}: load must refuse");
+            };
+            assert_eq!(code, "pypi_hatch_unsupported", "{name}");
+            assert!(detail.contains("{root:uri}"), "{name}: {detail}");
+            let Err((_, through_link)) = load(&linked(temp.path(), &root), "one", "1", UUID).await
+            else {
+                panic!("{name}: a symlink to the checkout must not hide its real path");
+            };
+            assert_eq!(through_link, detail, "{name}");
+            assert_eq!(
+                preflight(&root, "one").await.map_err(|(code, _)| code),
+                Err("pypi_hatch_unsupported"),
+                "{name}"
+            );
+        }
+        // pip leaves a `%` that starts no `%XX` escape as it is.
+        for name in ["at@sign", "quo'te", "sp ace", "100%", "50%off"] {
+            let temp = tempfile::tempdir().unwrap();
+            let root = temp.path().join(name);
+            tokio::fs::create_dir_all(&root).await.unwrap();
+            tokio::fs::write(root.join("pyproject.toml"), ORIGINAL)
+                .await
+                .unwrap();
+            assert!(load(&root, "one", "1", UUID).await.is_ok(), "{name}");
+            assert!(preflight(&root, "one").await.is_ok(), "{name}");
+        }
+    }
+
     #[tokio::test]
     async fn recorded_pin_drift_missing_artifact_and_ledgerless_source() {
         let temp = tempfile::tempdir().unwrap();

docs/testing/hatch.md

@@ -5,7 +5,11 @@ PEP 508 declarations in `project.dependencies`, optional dependencies, and
 Hatch environment `dependencies` / `extra-dependencies`. External
 `hatch.toml` tables override the corresponding top-level `tool.hatch` keys.
 Project references enable Hatchling's `allow-direct-references` setting.
-Vendored references use `{root:uri}` so checkouts remain relocatable.
+Vendored references use `{root:uri}` so checkouts remain relocatable,
+except to a path containing a `%XX` escape, `#`, `?`, `;`, or whitespace
+other than a space: Hatch's `{root:uri}` escapes only spaces, so pip reads
+the expanded URL as an escape, fragment or query, or rejects it (#547). A vendored scan or `vendor` run from such a path
+is refused before writing; hosted and agent mode are unaffected.
 Vendored environment references require Hatch >=1.2 on PATH; preflight verifies the
 installed version because Hatch 1.0 and 1.1 do not expand that context.
 Vendored Hatch requires the pip installer; uv currently ignores hash