From 206750f8d634769e29ba1eaf24deda8374c4f717 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 10:23:39 +0000 Subject: [PATCH 1/4] Start fix for #547 Assisted-by: Claude Code:claude-opus-5-5 From 4d945d9ee8b5a50ae29ff6cf741a0fff4750c736 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 10:35:08 +0000 Subject: [PATCH 2/4] Refuse vendored Hatch in %/#/?/; checkouts Vendored Hatch wiring points at {root:uri}/.socket/vendor/..., but Hatch expands {root:uri} escaping only spaces. In a checkout whose path holds %, #, ? or ; (a Jenkins multibranch workspace like app_feature%2Fx), pip decodes, truncates or rejects the URL, so every Hatch env failed to install while the scan reported success. vendor and scan --mode vendored now refuse such a root with pypi_hatch_unsupported before writing anything, including a re-run of a checkout that was vendored elsewhere and then moved. The hatch testing doc notes the relocation limit. Fixes #547 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/mode_migration_pypi.rs | 76 +++++++++++++++++++ crates/socket-patch-core/src/utils/hatch.rs | 13 ++++ .../src/vendor/pypi_hatch.rs | 60 ++++++++++++++- docs/testing/hatch.md | 6 +- 4 files changed, 152 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index 02197d8f0..95f773f11 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/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 + ); +} diff --git a/crates/socket-patch-core/src/utils/hatch.rs b/crates/socket-patch-core/src/utils/hatch.rs index 6ba38b4c2..5198e1dd7 100644 --- a/crates/socket-patch-core/src/utils/hatch.rs +++ b/crates/socket-patch-core/src/utils/hatch.rs @@ -11,6 +11,19 @@ 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 absolute path that Hatch's `{root:uri}` +/// expands into a broken file URL (#547). Hatchling's `path_to_uri` escapes +/// only spaces, so pip percent-decodes `%`, cuts the path at a `#` fragment +/// or `?` query, and the PEP 508 parser rejects a `;` in the URL. `None` +/// when the expansion is a usable URL. +pub fn root_uri_unsafe_char(root: &std::path::Path) -> Option { + let absolute = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); + absolute + .to_string_lossy() + .chars() + .find(|c| matches!(c, '%' | '#' | '?' | ';')) +} + pub fn is_hatch(files: &BTreeMap) -> bool { files.contains_key("hatch.toml") || files.get("pyproject.toml").is_some_and(|text| { diff --git a/crates/socket-patch-core/src/vendor/pypi_hatch.rs b/crates/socket-patch-core/src/vendor/pypi_hatch.rs index 36517ac44..ab7f6c32a 100644 --- a/crates/socket-patch-core/src/vendor/pypi_hatch.rs +++ b/crates/socket-patch-core/src/vendor/pypi_hatch.rs @@ -41,6 +41,7 @@ pub(super) async fn load( ) -> Result { 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,13 +108,15 @@ 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?; @@ -121,6 +124,20 @@ pub(super) async fn preflight(root: &Path, name: &str) -> Result<(), Failure> { 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 '%', '#', '?' or ';', 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,45 @@ mod tests { assert!(held(format!("\u{feff}\u{feff}{live}"))); } + /// #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", "a#b", "semi;colon"]; + if cfg!(unix) { + unsafe_names.push("q?x"); + } + 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}"); + assert_eq!( + preflight(&root, "one").await.map_err(|(code, _)| code), + Err("pypi_hatch_unsupported"), + "{name}" + ); + } + for name in ["at@sign", "quo'te", "sp ace"] { + 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(); diff --git a/docs/testing/hatch.md b/docs/testing/hatch.md index 0dd27b61e..4e7e9dac7 100644 --- a/docs/testing/hatch.md +++ b/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 `%`, `#`, `?` or `;`: 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 From 8d4e9eabcf82a0400d58b5393ad8eca40d008b53 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 10:40:37 +0000 Subject: [PATCH 3/4] Check the real Hatch root, %XX and whitespace Hatch builds {root:uri} from the real project path, so a checkout reached through a symlink is now checked at its target. Only a % that starts a %XX escape is refused (pip leaves 100% alone), and a tab or other non-space whitespace or control character, which the PEP 508 parser stops at, is refused too. A Windows \\?\ prefix no longer counts as a ?. Refs #547 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/utils/hatch.rs | 38 ++++++++++++++----- .../src/vendor/pypi_hatch.rs | 29 ++++++++++++-- docs/testing/hatch.md | 6 +-- 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-core/src/utils/hatch.rs b/crates/socket-patch-core/src/utils/hatch.rs index 5198e1dd7..b1cfea9e0 100644 --- a/crates/socket-patch-core/src/utils/hatch.rs +++ b/crates/socket-patch-core/src/utils/hatch.rs @@ -11,17 +11,37 @@ 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 absolute path that Hatch's `{root:uri}` -/// expands into a broken file URL (#547). Hatchling's `path_to_uri` escapes -/// only spaces, so pip percent-decodes `%`, cuts the path at a `#` fragment -/// or `?` query, and the PEP 508 parser rejects a `;` in the URL. `None` +/// 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 { - let absolute = std::path::absolute(root).unwrap_or_else(|_| root.to_path_buf()); - absolute - .to_string_lossy() - .chars() - .find(|c| matches!(c, '%' | '#' | '?' | ';')) + 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 = 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) -> bool { diff --git a/crates/socket-patch-core/src/vendor/pypi_hatch.rs b/crates/socket-patch-core/src/vendor/pypi_hatch.rs index ab7f6c32a..ec7f35b47 100644 --- a/crates/socket-patch-core/src/vendor/pypi_hatch.rs +++ b/crates/socket-patch-core/src/vendor/pypi_hatch.rs @@ -131,7 +131,7 @@ fn require_expandable_root(root: &Path) -> Result<(), Failure> { 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 '%', '#', '?' or ';', or use hosted or agent mode" + "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(()), @@ -456,14 +456,29 @@ 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", "a#b", "semi;colon"]; + let mut unsafe_names = vec!["feature%2Fx", "p%20q", "a#b", "semi;colon"]; if cfg!(unix) { - unsafe_names.push("q?x"); + unsafe_names.extend(["q?x", "tab\tbed"]); } for name in unsafe_names { let temp = tempfile::tempdir().unwrap(); @@ -477,13 +492,19 @@ mod tests { }; 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}" ); } - for name in ["at@sign", "quo'te", "sp ace"] { + // 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(); diff --git a/docs/testing/hatch.md b/docs/testing/hatch.md index 4e7e9dac7..248089223 100644 --- a/docs/testing/hatch.md +++ b/docs/testing/hatch.md @@ -6,9 +6,9 @@ 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, -except to a path containing `%`, `#`, `?` or `;`: 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 +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. From 272c0220e017d007a4c566afefca6f77e5758326 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:08:06 +0000 Subject: [PATCH 4/4] Port #1418 cache_env scrub snapshot fix The coverage job failed this PR's merge-queue run (2ea36d6). That job flakes in in_process_edge_cases (#1416): the cache_env scrub self-tests read the parent env twice while in-process tests set SOCKET_OFFLINE. Port #1418's fix (one snapshot of the parent env) so the queue run is not at the mercy of that race. It no-ops once #1418 lands. Refs #1416 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/common/cache_env.rs | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/crates/socket-patch-cli/tests/common/cache_env.rs b/crates/socket-patch-cli/tests/common/cache_env.rs index 8ba1f5879..d01fd7e4b 100644 --- a/crates/socket-patch-cli/tests/common/cache_env.rs +++ b/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 { + 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, 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 = 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 = 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");