From a0642a4851fb38dc047a11eb95a80a0b0fd22b81 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 02:23:25 +0000 Subject: [PATCH 1/7] Start fix for #1873 Assisted-by: Claude Code:claude-opus-5-5 From fbb2084b0d6008b95b46615c15a8c122f761378b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 02:32:47 +0000 Subject: [PATCH 2/7] Read ${VAR} in pip requirement lines pip expands ${NAME} environment variables on every requirements line before parsing it, so `six==${SIXV}` with SIXV=1.16.0 installs exactly six 1.16.0. socket-patch expanded them only in -r/-c include targets, so vendored scans and VEX discovery read the templated line as no pin. Beside a pylock.toml that meant vendor wired only the lock, left the line on PyPI and exited 0; `vendor --check` then verified the wiring and `vex` attested not_affected while `pip install -r` installed the unpatched release. Without a lock, vendor refused the line as "not pinned". Requirement lines are now expanded from the process environment the way pip does, in both the vendored planner and VEX discovery. A templated exact pin is wired like a literal one, and revert restores the templated line as written. An unset or empty variable stays literal, as in pip, and pins nothing. Fixes #1873 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/utils/requirements.rs | 7 +++ .../src/vendor/pypi_requirements.rs | 58 +++++++++++++++++-- .../src/vex/discover/pypi_other.rs | 54 +++++++++++++++++ 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index cc7384db3..6a6713894 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -288,6 +288,13 @@ pub(crate) fn strip_comment(text: &str) -> &str { split_comment(text).0 } +/// A line's code as pip parses it: [`expand_env_vars`] over the process +/// environment. pip expands every line, not only `-r` / `-c` targets, so +/// `six==${SIXV}` with `SIXV=1.16.0` is the exact pin `six==1.16.0` (#1873). +pub(crate) fn expand_process_env(code: &str) -> String { + expand_env_vars(code, |name| std::env::var(name).ok()) +} + /// pip's `expand_env_variables`: each `${NAME}` whose `NAME` is /// `[A-Z0-9_]+` is replaced by `lookup(NAME)`; an unset or empty variable /// leaves the reference as written. pip expands after stripping comments diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 8faf4bc10..8d72071a0 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -27,8 +27,8 @@ use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_string}; use crate::utils::line_endings::{self, terminator}; use crate::utils::requirements::{ - expand_env_vars, hash_options, logical_lines, parse_requirement, requirement_name, - requires_hashes, shlex_split, split_comment, strip_comment, vendor_tag, + expand_env_vars, expand_process_env, hash_options, logical_lines, parse_requirement, + requirement_name, requires_hashes, shlex_split, split_comment, strip_comment, vendor_tag, }; use super::common::refuse_symlinked; @@ -132,11 +132,12 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { let mut found_range = false; let mut other_branches = false; for ll in logical_lines(content) { - let code = strip_comment(&ll.text); - let Some(req) = parse_requirement(code) else { + let code = expand_process_env(strip_comment(&ll.text)); + let Some(req) = parse_requirement(&code) else { // A line that names the package but is no requirement pip // reads cleanly is never a pin to rewrite (fail closed). - if requirement_name(code).is_some_and(|name| canonicalize_pypi_name(name) == canon_name) + if requirement_name(&code) + .is_some_and(|name| canonicalize_pypi_name(name) == canon_name) { found_range = true; } @@ -191,7 +192,7 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { /// spec, extras or marker): a file that pins it is an install source of it. pub(super) fn names_package(content: &str, canon_name: &str) -> bool { logical_lines(content).into_iter().any(|ll| { - requirement_name(strip_comment(&ll.text)) + requirement_name(&expand_process_env(strip_comment(&ll.text))) .is_some_and(|name| canonicalize_pypi_name(name) == canon_name) }) } @@ -2171,6 +2172,51 @@ mod tests { } } + /// #1873: pip expands `${NAME}` on every line, so `six==${SIXV}` with + /// `SIXV=1.16.0` is the exact pin `six==1.16.0`. Vendor wires it (it + /// used to refuse it as "not pinned", or beside a pylock leave it on + /// PyPI while the lock wiring reported the package patched), and the + /// revert restores the templated line as written. + #[tokio::test] + async fn wires_and_reverts_an_env_templated_pin() { + std::env::set_var("SP_TEST_1873_PLAN_SIXV", "1.16.0"); + std::env::set_var("SP_TEST_1873_PLAN_OTHER", "1.17.0"); + for pin in [ + "six==${SP_TEST_1873_PLAN_SIXV}", + "six == ${SP_TEST_1873_PLAN_SIXV} # templated", + ] { + let original = format!("requests==2.31.0\n{pin}\n"); + let tmp = write_root(&original).await; + assert!( + sibling_pin_wirable(tmp.path(), "six", "1.16.0").await, + "{pin}" + ); + let wiring = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap_or_else(|err| panic!("{pin}: {err:?}")); + assert_eq!( + read_root(tmp.path()).await, + format!("requests==2.31.0\n{}\n", expected_unhashed_line()), + "{pin}" + ); + let outcome = revert_requirements(&entry_for(wiring), tmp.path(), false).await; + assert!(outcome.success, "{pin}"); + assert_eq!(read_root(tmp.path()).await, original, "{pin}"); + } + // A variable holding another release pins that release; an unset + // one stays literal, which pip refuses: neither is this pin. + for pin in [ + "six==${SP_TEST_1873_PLAN_OTHER}", + "six==${SP_TEST_1873_PLAN_UNSET}", + ] { + let tmp = write_root(&format!("{pin}\n")).await; + assert!( + !sibling_pin_wirable(tmp.path(), "six", "1.16.0").await, + "{pin}" + ); + } + } + #[tokio::test] async fn rewrites_hash_pinned_continuation_and_preserves_crlf() { // A hash-pinned requirement spanning two physical lines, CRLF file. diff --git a/crates/socket-patch-core/src/vex/discover/pypi_other.rs b/crates/socket-patch-core/src/vex/discover/pypi_other.rs index c6f55e57f..7575c559f 100644 --- a/crates/socket-patch-core/src/vex/discover/pypi_other.rs +++ b/crates/socket-patch-core/src/vex/discover/pypi_other.rs @@ -269,6 +269,8 @@ async fn extract_requirements(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { /// One logical requirements line (see the module docs for the two shapes). fn requirements_line(ctx: &DiscoverCtx<'_>, file: &str, line: &str, out: &mut Discovery) { let (code, comment) = split_comment(line); + // pip expands `${NAME}` on every line before parsing it (#1873). + let code = requirements::expand_process_env(code); let code = code.trim(); if let Some(location) = editable_location(code) { // pip installs an editable source checkout of whatever package it @@ -1873,6 +1875,58 @@ mod tests { } } + /// REGRESSION (#1873): pip expands `${NAME}` on every requirements + /// line before parsing it, so `six==${SIXV}` with `SIXV=1.16.0` is the + /// exact pin `six==1.16.0` and contests a pylock-only wiring of that + /// release. An unset or empty variable stays literal (pip refuses the + /// line), and one holding another release pins that release. + #[tokio::test] + async fn issue_1873_env_templated_pin_contests_wiring() { + std::env::set_var("SP_TEST_1873_SIXV", "1.16.0"); + std::env::set_var("SP_TEST_1873_OTHER", "1.17.0"); + std::env::set_var("SP_TEST_1873_EMPTY", ""); + std::env::remove_var("SP_TEST_1873_UNSET"); + const WHL: &str = "six-1.16.0-py2.py3-none-any.whl"; + let vendored = vendored_wheel(UUID_A, WHL); + let wired = pipfile_lock(serde_json::json!({ "default": { + "six": { "file": format!("./{vendored}"), "hashes": [format!("sha256:{SHA}")] }, + }})); + let discover = |requirements: &'static str| { + let wired = wired.clone(); + async move { + let p = Project::new(); + p.write("Pipfile.lock", &wired) + .write("Pipfile", "[packages]\nsix = \"*\"\n") + .write("requirements.txt", requirements); + p.discover().await + } + }; + for line in [ + "six==${SP_TEST_1873_SIXV}\n", + "idna==3.7\nsix == ${SP_TEST_1873_SIXV} # templated\n", + ] { + let out = discover(line).await; + assert!(out.refs.is_empty(), "{line:?}: {:#?}", out.refs); + let c = out + .vendored_contest("pkg:pypi/six@1.16.0", UUID_A) + .unwrap_or_else(|| panic!("{line:?}: not contested")); + assert_eq!( + c.other, + std::path::Path::new("requirements.txt"), + "{line:?}" + ); + } + for line in [ + "six==${SP_TEST_1873_OTHER}\n", + "six==${SP_TEST_1873_EMPTY}\n", + "six==${SP_TEST_1873_UNSET}\n", + "six==${sp_test_1873_sixv}\n", + ] { + let out = discover(line).await; + assert_eq!(out.refs.len(), 1, "{line:?}: {:#?}", out.diagnostics); + } + } + /// #1602: a lock tree that wires the release itself, under another /// PEP 440 spelling, is not a contester even when it also pins it: /// pip resolves the include's `==1.16.0` to the tree's own vendored From efe29519e10f7d227a26962fd3260d2cad0deb01 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 02:35:58 +0000 Subject: [PATCH 3/7] Test templated pip pin beside a pip lock (CLI) Covers #1873 end to end through the CLI: vendor wires a `six==${VAR}` line together with the pylock.toml, `vendor --check` is green, revert restores the templated line, and a pylock-only wiring the templated pin contests fails `vendor --check`. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/mode_migration_pypi.rs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index afac1baab..cf16de326 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -3289,6 +3289,66 @@ async fn pip_lock_vendor_wires_requirements_with_the_pylock() { assert!(!root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); } +/// #1873: pip expands `${NAME}` on every requirements line, so +/// `six==${SIXV}` with `SIXV=1.16.0` installs exactly six 1.16.0. Beside a +/// `pip lock` pylock.toml, vendor wires that line with the lock (it used +/// to wire only the lock, name requirements.txt an unpatched loser and +/// exit 0), and the revert restores the templated line as written. A +/// pylock-only wiring the templated pin later contests fails `vendor +/// --check`, as a literal pin does. +#[tokio::test] +async fn pip_lock_vendor_wires_an_env_templated_requirements_pin() { + const TEMPLATED: &str = "idna==3.7\nsix==${SP_E2E_1873_SIXV}\n"; + let env = [("SP_E2E_1873_SIXV", "1.16.0")]; + let (_tmp, root) = project(); + stage_pip_lock(&root); + std::fs::write(root.join("requirements.txt"), TEMPLATED).unwrap(); + stage_manifest(&root); + let needle = format!(".socket/vendor/pypi/{UUID}/"); + + let (code, out) = run_cli(&root, &["vendor"], &env); + assert_eq!(code, 0, "{out:#}"); + assert!( + !out.to_string().contains("pypi_multiple_lockfiles"), + "requirements.txt is wired, not an unpatched loser: {out:#}" + ); + for file in ["requirements.txt", "pylock.toml"] { + assert!( + std::fs::read_to_string(root.join(file)) + .unwrap() + .contains(&needle), + "{file} is wired: {out:#}" + ); + } + let (code, out) = run_cli(&root, &["vendor", "--check"], &env); + assert_eq!(code, 0, "both install sources are wired: {out:#}"); + + let (code, out) = run_cli(&root, &["vendor", "--revert"], &env); + assert_eq!(code, 0, "{out:#}"); + assert_eq!( + std::fs::read_to_string(root.join("requirements.txt")).unwrap(), + TEMPLATED, + "the templated line is restored as written" + ); + assert_eq!( + std::fs::read(root.join("pylock.toml")).unwrap(), + PIP_LOCK.as_bytes() + ); + + // A pylock-only wiring (vendored before six was pinned) is contested + // once the templated pin reads as six==1.16.0. + let (_tmp, root) = project(); + stage_pip_lock(&root); + std::fs::write(root.join("requirements.txt"), "idna==3.7\n").unwrap(); + stage_manifest(&root); + let (code, out) = run_cli(&root, &["vendor"], &env); + assert_eq!(code, 0, "{out:#}"); + std::fs::write(root.join("requirements.txt"), TEMPLATED).unwrap(); + let (code, out) = run_cli(&root, &["vendor", "--check"], &env); + assert_eq!(code, 1, "pip installs six from PyPI: {out:#}"); + assert!(out.to_string().contains("contested"), "{out:#}"); +} + /// #1758: a pylock vendored while requirements.txt did not pin six yet /// (a project vendored before the fix wired only the pylock). Once the /// file pins six==1.16.0, the next `vendor` wires it to the committed From 531f7f5e1c25f9b2ddc04d88d0bafbd765902e79 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:10:04 +0000 Subject: [PATCH 4/7] Refuse ${VAR} pip pins instead of expanding pip expands ${NAME} where it installs (a Docker ARG, a CI step), which socket-patch can't see. Reading `six==${SIXV}` through this process's environment lets `vendor --check` pass wherever SIXV is unset, and lets vendor rewrite the template away for every other environment. So a templated requirement now fails closed instead: - vendor refuses it with pypi_requirement_env_templated and writes nothing, in the root file or an include, also beside a pylock.toml or tool lock instead of naming requirements.txt an unpatched loser; - VEX discovery and `vendor --check` read it as possibly installing any release of the package from PyPI, so it contests every wiring of that package and VEX no longer attests it. A lowercase ${...} stays literal in pip and is still no pin. Fixes #1873 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/mode_migration_pypi.rs | 67 +++------ .../src/utils/requirements.rs | 12 +- crates/socket-patch-core/src/vendor/pypi.rs | 22 +++ .../src/vendor/pypi_requirements.rs | 142 ++++++++++++------ .../src/vex/discover/pypi_other.rs | 34 ++--- 5 files changed, 168 insertions(+), 109 deletions(-) diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index cf16de326..fad180c0b 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -3289,63 +3289,44 @@ async fn pip_lock_vendor_wires_requirements_with_the_pylock() { assert!(!root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); } -/// #1873: pip expands `${NAME}` on every requirements line, so -/// `six==${SIXV}` with `SIXV=1.16.0` installs exactly six 1.16.0. Beside a -/// `pip lock` pylock.toml, vendor wires that line with the lock (it used -/// to wire only the lock, name requirements.txt an unpatched loser and -/// exit 0), and the revert restores the templated line as written. A -/// pylock-only wiring the templated pin later contests fails `vendor -/// --check`, as a literal pin does. +/// #1873: pip expands `${NAME}` on every requirements line where it runs, +/// so what `six==${SIXV}` installs is unknowable to vendor. Beside a `pip +/// lock` pylock.toml, vendor refuses it and writes nothing (it used to wire +/// only the lock, name requirements.txt an unpatched loser and exit 0), +/// whatever this process's environment holds. A pylock-only wiring the +/// templated line later contests fails `vendor --check`. #[tokio::test] -async fn pip_lock_vendor_wires_an_env_templated_requirements_pin() { +async fn pip_lock_vendor_refuses_an_env_templated_requirements_pin() { const TEMPLATED: &str = "idna==3.7\nsix==${SP_E2E_1873_SIXV}\n"; - let env = [("SP_E2E_1873_SIXV", "1.16.0")]; - let (_tmp, root) = project(); - stage_pip_lock(&root); - std::fs::write(root.join("requirements.txt"), TEMPLATED).unwrap(); - stage_manifest(&root); - let needle = format!(".socket/vendor/pypi/{UUID}/"); - - let (code, out) = run_cli(&root, &["vendor"], &env); - assert_eq!(code, 0, "{out:#}"); - assert!( - !out.to_string().contains("pypi_multiple_lockfiles"), - "requirements.txt is wired, not an unpatched loser: {out:#}" - ); - for file in ["requirements.txt", "pylock.toml"] { + for env in [&[("SP_E2E_1873_SIXV", "1.16.0")][..], &[][..]] { + let (_tmp, root) = project(); + stage_pip_lock(&root); + std::fs::write(root.join("requirements.txt"), TEMPLATED).unwrap(); + stage_manifest(&root); + let before = tree(&root); + let (code, out) = run_cli(&root, &["vendor"], env); + assert_ne!(code, 0, "{env:?}: {out:#}"); assert!( - std::fs::read_to_string(root.join(file)) - .unwrap() - .contains(&needle), - "{file} is wired: {out:#}" + out.to_string().contains("pypi_requirement_env_templated"), + "{env:?}: {out:#}" ); + assert_eq!(tree(&root), before, "{env:?}: nothing written"); } - let (code, out) = run_cli(&root, &["vendor", "--check"], &env); - assert_eq!(code, 0, "both install sources are wired: {out:#}"); - - let (code, out) = run_cli(&root, &["vendor", "--revert"], &env); - assert_eq!(code, 0, "{out:#}"); - assert_eq!( - std::fs::read_to_string(root.join("requirements.txt")).unwrap(), - TEMPLATED, - "the templated line is restored as written" - ); - assert_eq!( - std::fs::read(root.join("pylock.toml")).unwrap(), - PIP_LOCK.as_bytes() - ); - // A pylock-only wiring (vendored before six was pinned) is contested - // once the templated pin reads as six==1.16.0. + // A pylock-only wiring (vendored before six was named) is contested + // once the templated line names six. + let env = [("SP_E2E_1873_SIXV", "1.16.0")]; let (_tmp, root) = project(); stage_pip_lock(&root); std::fs::write(root.join("requirements.txt"), "idna==3.7\n").unwrap(); stage_manifest(&root); let (code, out) = run_cli(&root, &["vendor"], &env); assert_eq!(code, 0, "{out:#}"); + let (code, out) = run_cli(&root, &["vendor", "--check"], &env); + assert_eq!(code, 0, "{out:#}"); std::fs::write(root.join("requirements.txt"), TEMPLATED).unwrap(); let (code, out) = run_cli(&root, &["vendor", "--check"], &env); - assert_eq!(code, 1, "pip installs six from PyPI: {out:#}"); + assert_eq!(code, 1, "pip may install six from PyPI: {out:#}"); assert!(out.to_string().contains("contested"), "{out:#}"); } diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index 6a6713894..cbcb0ec3a 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -288,11 +288,13 @@ pub(crate) fn strip_comment(text: &str) -> &str { split_comment(text).0 } -/// A line's code as pip parses it: [`expand_env_vars`] over the process -/// environment. pip expands every line, not only `-r` / `-c` targets, so -/// `six==${SIXV}` with `SIXV=1.16.0` is the exact pin `six==1.16.0` (#1873). -pub(crate) fn expand_process_env(code: &str) -> String { - expand_env_vars(code, |name| std::env::var(name).ok()) +/// Whether pip's [`expand_env_vars`] would substitute anything in `code`. +/// pip expands every line, not only `-r` / `-c` targets, so `six==${SIXV}` +/// pins whatever `SIXV` holds where pip runs: a Docker `ARG`, a CI step, +/// none of which this process sees. Such a line pins no knowable release +/// (#1873). +pub(crate) fn has_env_reference(code: &str) -> bool { + expand_env_vars(code, |_| Some("_".to_string())) != code } /// pip's `expand_env_variables`: each `${NAME}` whose `NAME` is diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 5d6d1c44b..24a4d5457 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -244,6 +244,24 @@ async fn requirements_pins_target(project_root: &Path, target: Option<(&str, &st } } +/// #1873: refuse wiring a lock beside a requirements set that names the +/// target through a `${NAME}` pip expands where it runs (`six==${SIXV}`). +/// The release it installs is unknowable here, so naming requirements.txt +/// a loser would let the lock wiring report the package patched while pip +/// installs it from the registry. +async fn refuse_templated_pin( + project_root: &Path, + target: Option<(&str, &str)>, +) -> Result<(), (&'static str, String)> { + let Some((name, _)) = target else { + return Ok(()); + }; + match super::pypi_requirements::templated_pin(project_root, name).await { + Some(refusal) => Err(refusal), + None => Ok(()), + } +} + /// Route the project to a wiring flavor, first match wins. Lockfiles are the /// authoritative "this tool manages installs" signal, so locks are compared /// with locks (precedence follows migration direction / ecosystem currency: @@ -396,6 +414,7 @@ async fn detect_pypi_flavor( } None => false, }; + refuse_templated_pin(project_root, target).await?; if !wired_with_lock && exists(PYPI_REQUIREMENTS).await { present.push(PYPI_REQUIREMENTS); } @@ -449,6 +468,9 @@ async fn detect_pypi_flavor( } _ => false, }; + if governing.is_some() { + refuse_templated_pin(project_root, target).await?; + } if governing.is_some() && !wired_with_lock && requirements_pins_target(project_root, target).await diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 8d72071a0..e477b1109 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -27,7 +27,7 @@ use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_string}; use crate::utils::line_endings::{self, terminator}; use crate::utils::requirements::{ - expand_env_vars, expand_process_env, hash_options, logical_lines, parse_requirement, + expand_env_vars, has_env_reference, hash_options, logical_lines, parse_requirement, requirement_name, requires_hashes, shlex_split, split_comment, strip_comment, vendor_tag, }; @@ -79,6 +79,8 @@ struct PinScan { /// environment-marker branch; whether that is ambiguous depends on the /// target pins of the whole requirement set ([`marker_split_ok`]). other_branches: bool, + /// Some occurrence carries a `${NAME}` pip expands where it runs. + templated: bool, } /// Whether the other version's marker branches (if any) in a requirement @@ -105,13 +107,16 @@ fn marker_split_ok(scans: &[PinScan]) -> bool { /// all carry a marker (`split_ok` from [`marker_split_ok`]). #[derive(Debug, PartialEq, Eq)] enum PinRefusal { + Templated, Extras, NotPinned, UnmarkedSplit, } fn pin_refusal(scan: &PinScan, split_ok: bool) -> Option { - if scan.extras { + if scan.templated { + Some(PinRefusal::Templated) + } else if scan.extras { Some(PinRefusal::Extras) } else if scan.range { Some(PinRefusal::NotPinned) @@ -131,13 +136,22 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { let mut found_extras = false; let mut found_range = false; let mut other_branches = false; + let mut found_templated = false; for ll in logical_lines(content) { - let code = expand_process_env(strip_comment(&ll.text)); - let Some(req) = parse_requirement(&code) else { + let code = strip_comment(&ll.text); + if has_env_reference(code) + && requirement_name(code).is_some_and(|name| canonicalize_pypi_name(name) == canon_name) + { + // pip expands `${NAME}` where it runs, so the release this + // line installs is unknowable here, and rewriting it would + // drop the template for every other environment (#1873). + found_templated = true; + continue; + } + let Some(req) = parse_requirement(code) else { // A line that names the package but is no requirement pip // reads cleanly is never a pin to rewrite (fail closed). - if requirement_name(&code) - .is_some_and(|name| canonicalize_pypi_name(name) == canon_name) + if requirement_name(code).is_some_and(|name| canonicalize_pypi_name(name) == canon_name) { found_range = true; } @@ -185,6 +199,7 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { extras: found_extras, range: found_range, other_branches, + templated: found_templated, } } @@ -192,7 +207,7 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { /// spec, extras or marker): a file that pins it is an install source of it. pub(super) fn names_package(content: &str, canon_name: &str) -> bool { logical_lines(content).into_iter().any(|ll| { - requirement_name(&expand_process_env(strip_comment(&ll.text))) + requirement_name(strip_comment(&ll.text)) .is_some_and(|name| canonicalize_pypi_name(name) == canon_name) }) } @@ -208,7 +223,9 @@ fn find_pin(content: &str, canon_name: &str, version: &str) -> PinSearch { let split_ok = marker_split_ok(std::slice::from_ref(&scan)); match pin_refusal(&scan, split_ok) { Some(PinRefusal::Extras) => return PinSearch::Extras, - Some(PinRefusal::NotPinned | PinRefusal::UnmarkedSplit) => return PinSearch::Range, + Some(PinRefusal::Templated | PinRefusal::NotPinned | PinRefusal::UnmarkedSplit) => { + return PinSearch::Range + } None => {} } match scan.exact.into_iter().next() { @@ -447,6 +464,37 @@ pub(super) async fn sibling_pin_wirable(root: &Path, canon_name: &str, version: && refuse_symlinked(root, &rels).await.is_ok() } +/// #1873: refuse a requirements line naming `canon_name` through a +/// `${NAME}` pip expands where it runs (`six==${SIXV}`). What it installs +/// is unknowable here, so the vendor neither rewrites it nor names the file +/// a loser beside a lock that it wires alone. +fn templated_refusal(rel: &str, canon_name: &str) -> (&'static str, String) { + ( + "pypi_requirement_env_templated", + format!( + "{rel}: the {canon_name} requirement uses a ${{NAME}} environment variable pip \ + expands at install time, so the release it installs is unknown; pin it \ + literally or use agent mode (`scan --mode agent` + `socket-patch apply`) \ + instead" + ), + ) +} + +/// [`templated_refusal`] for the first requirements file in the tree pip +/// reads from `root` that names `canon_name` through a `${NAME}`. +pub(super) async fn templated_pin(root: &Path, canon_name: &str) -> Option<(&'static str, String)> { + let files = collect_requirements_files(root).await.ok()?; + files.iter().find_map(|f| { + logical_lines(&f.content).into_iter().find_map(|ll| { + let code = strip_comment(&ll.text); + (has_env_reference(code) + && requirement_name(code) + .is_some_and(|name| canonicalize_pypi_name(name) == canon_name)) + .then(|| templated_refusal(&f.rel, canon_name)) + }) + }) +} + /// The `requirements_line` records of a mixed entry (a Pipenv or uv entry /// that also wired its exported requirements file, #612 / #1368). pub(super) const RECORD_KIND: &str = "requirements_line"; @@ -847,6 +895,7 @@ async fn plan_requirements( for (file, scan) in files.iter().zip(scans) { match pin_refusal(&scan, split_ok) { + Some(PinRefusal::Templated) => return Err(templated_refusal(&file.rel, canon_name)), Some(PinRefusal::Extras) => { return Err(( "pypi_extras_unsupported", @@ -2172,49 +2221,54 @@ mod tests { } } - /// #1873: pip expands `${NAME}` on every line, so `six==${SIXV}` with - /// `SIXV=1.16.0` is the exact pin `six==1.16.0`. Vendor wires it (it - /// used to refuse it as "not pinned", or beside a pylock leave it on - /// PyPI while the lock wiring reported the package patched), and the - /// revert restores the templated line as written. + /// #1873: pip expands `${NAME}` on every line where it runs, so what + /// `six==${SIXV}` installs is unknowable here. Vendor refuses it with + /// its own code (it used to say "not pinned", or beside a pylock leave + /// it on PyPI while the lock wiring reported the package patched) and + /// never rewrites the template away, in the root or an include. A + /// `${...}` pip does not expand (lowercase) is no template. #[tokio::test] - async fn wires_and_reverts_an_env_templated_pin() { - std::env::set_var("SP_TEST_1873_PLAN_SIXV", "1.16.0"); - std::env::set_var("SP_TEST_1873_PLAN_OTHER", "1.17.0"); - for pin in [ - "six==${SP_TEST_1873_PLAN_SIXV}", - "six == ${SP_TEST_1873_PLAN_SIXV} # templated", + async fn refuses_an_env_templated_pin() { + for (root, base) in [ + ("requests==2.31.0\nsix==${SIXV}\n", None), + ("six == ${SIXV} # templated\n", None), + ("six==1.16.0 --hash=sha256:${SIX_SHA}\n", None), + ("-r base.txt\nidna==3.7\n", Some("six==${SIXV}\n")), ] { - let original = format!("requests==2.31.0\n{pin}\n"); - let tmp = write_root(&original).await; - assert!( - sibling_pin_wirable(tmp.path(), "six", "1.16.0").await, - "{pin}" - ); - let wiring = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + let tmp = write_root(root).await; + if let Some(base) = base { + tokio::fs::write(tmp.path().join("base.txt"), base) + .await + .unwrap(); + } + let err = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) .await - .unwrap_or_else(|err| panic!("{pin}: {err:?}")); + .unwrap_err(); assert_eq!( - read_root(tmp.path()).await, - format!("requests==2.31.0\n{}\n", expected_unhashed_line()), - "{pin}" + err.0, "pypi_requirement_env_templated", + "{root:?}: {}", + err.1 ); - let outcome = revert_requirements(&entry_for(wiring), tmp.path(), false).await; - assert!(outcome.success, "{pin}"); - assert_eq!(read_root(tmp.path()).await, original, "{pin}"); - } - // A variable holding another release pins that release; an unset - // one stays literal, which pip refuses: neither is this pin. - for pin in [ - "six==${SP_TEST_1873_PLAN_OTHER}", - "six==${SP_TEST_1873_PLAN_UNSET}", - ] { - let tmp = write_root(&format!("{pin}\n")).await; - assert!( - !sibling_pin_wirable(tmp.path(), "six", "1.16.0").await, - "{pin}" + assert_eq!(read_root(tmp.path()).await, root, "nothing written"); + assert!(!sibling_pin_wirable(tmp.path(), "six", "1.16.0").await); + assert_eq!( + templated_pin(tmp.path(), "six").await.map(|r| r.0), + Some("pypi_requirement_env_templated"), + "{root:?}" ); + // Another package's line is untouched by the refusal. + assert!(templated_pin(tmp.path(), "idna").await.is_none()); } + let tmp = write_root("requests==${REQV}\nsix==1.16.0\n").await; + assert!(templated_pin(tmp.path(), "six").await.is_none()); + wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap(); + assert_eq!( + find_pin("six==${sixv}\n", "six", "1.16.0"), + PinSearch::Range, + "a lowercase reference stays literal and is no pin" + ); } #[tokio::test] diff --git a/crates/socket-patch-core/src/vex/discover/pypi_other.rs b/crates/socket-patch-core/src/vex/discover/pypi_other.rs index 7575c559f..02ba51b43 100644 --- a/crates/socket-patch-core/src/vex/discover/pypi_other.rs +++ b/crates/socket-patch-core/src/vex/discover/pypi_other.rs @@ -269,8 +269,6 @@ async fn extract_requirements(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { /// One logical requirements line (see the module docs for the two shapes). fn requirements_line(ctx: &DiscoverCtx<'_>, file: &str, line: &str, out: &mut Discovery) { let (code, comment) = split_comment(line); - // pip expands `${NAME}` on every line before parsing it (#1873). - let code = requirements::expand_process_env(code); let code = code.trim(); if let Some(location) = editable_location(code) { // pip installs an editable source checkout of whatever package it @@ -289,6 +287,15 @@ fn requirements_line(ctx: &DiscoverCtx<'_>, file: &str, line: &str, out: &mut Di return; } let Some((name, location)) = requirement_location(code) else { + if requirements::has_env_reference(code) { + // pip expands `${NAME}` where it runs, so a registry line + // carrying one may install any release of the package it + // names: evidence against every wiring of it (#1873). + if let Some(name) = requirements::requirement_name(code) { + out.resolved_elsewhere_any_version(file, pypi_name_purl(name)); + } + return; + } // A registry requirement; a pin that can only install one release // is evidence against another lock's wiring of that release. registry_pin(file, code, out); @@ -1876,16 +1883,12 @@ mod tests { } /// REGRESSION (#1873): pip expands `${NAME}` on every requirements - /// line before parsing it, so `six==${SIXV}` with `SIXV=1.16.0` is the - /// exact pin `six==1.16.0` and contests a pylock-only wiring of that - /// release. An unset or empty variable stays literal (pip refuses the - /// line), and one holding another release pins that release. + /// line where it runs (a Docker `ARG`, a CI step), so `six==${SIXV}` + /// may install any release, including the wired one, from PyPI. It + /// contests a lock's wiring of six whatever this process's environment + /// holds. A lowercase `${...}` stays literal in pip and pins nothing. #[tokio::test] async fn issue_1873_env_templated_pin_contests_wiring() { - std::env::set_var("SP_TEST_1873_SIXV", "1.16.0"); - std::env::set_var("SP_TEST_1873_OTHER", "1.17.0"); - std::env::set_var("SP_TEST_1873_EMPTY", ""); - std::env::remove_var("SP_TEST_1873_UNSET"); const WHL: &str = "six-1.16.0-py2.py3-none-any.whl"; let vendored = vendored_wheel(UUID_A, WHL); let wired = pipfile_lock(serde_json::json!({ "default": { @@ -1903,7 +1906,9 @@ mod tests { }; for line in [ "six==${SP_TEST_1873_SIXV}\n", - "idna==3.7\nsix == ${SP_TEST_1873_SIXV} # templated\n", + "idna==3.7\nSix == ${SIXV} # templated\n", + "six${SIX_SPEC}\n", + "six==1.16.0 --hash=sha256:${SIX_SHA}\n", ] { let out = discover(line).await; assert!(out.refs.is_empty(), "{line:?}: {:#?}", out.refs); @@ -1916,12 +1921,7 @@ mod tests { "{line:?}" ); } - for line in [ - "six==${SP_TEST_1873_OTHER}\n", - "six==${SP_TEST_1873_EMPTY}\n", - "six==${SP_TEST_1873_UNSET}\n", - "six==${sp_test_1873_sixv}\n", - ] { + for line in ["six==${sixv}\n", "idna==${IDNAV}\n"] { let out = discover(line).await; assert_eq!(out.refs.len(), 1, "{line:?}: {:#?}", out.diagnostics); } From 28a6249539827da354bedb9f1d509d4946afcba0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:12:02 +0000 Subject: [PATCH 5/7] Skip env-reference scan on lines without ${ Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/utils/requirements.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index cbcb0ec3a..939cb908f 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -294,7 +294,7 @@ pub(crate) fn strip_comment(text: &str) -> &str { /// none of which this process sees. Such a line pins no knowable release /// (#1873). pub(crate) fn has_env_reference(code: &str) -> bool { - expand_env_vars(code, |_| Some("_".to_string())) != code + code.contains("${") && expand_env_vars(code, |_| Some("_".to_string())) != code } /// pip's `expand_env_variables`: each `${NAME}` whose `NAME` is From d97f89ca26c7bf365e7ecfa7b4d28caf5088e87f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 05:55:54 +0000 Subject: [PATCH 6/7] Test the governing-lock refusal of a templated requirements pin refuse_templated_pin runs in two branches of detect_pypi_flavor, the pylock branch and the governing-lock branch. Only the pylock branch had a test: with the governing-lock call removed, every core and CLI test still passed. Add a Pipfile.lock project with requirements.txt pinning six==${SIXV}, which must refuse with pypi_requirement_env_templated. It fails without that call. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011f9aeS41XjBLw8LQsUUTYU --- crates/socket-patch-core/src/vendor/pypi.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 24a4d5457..c7b70795c 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -4216,6 +4216,21 @@ mod tests { } } + /// #1873: beside a governing tool lock, a requirements pin templated + /// with pip's `${VAR}` refuses instead of being named a loser: the + /// release it installs is unknown. + #[tokio::test] + async fn governing_lock_refuses_an_env_templated_requirements_pin() { + let tmp = tempfile::tempdir().unwrap(); + touch(tmp.path(), "Pipfile", "[packages]\nsix = \"==1.16.0\"\n").await; + touch(tmp.path(), "Pipfile.lock", "{}").await; + touch(tmp.path(), "requirements.txt", "six==${SIXV}\n").await; + let (code, detail) = detect_pypi_flavor(tmp.path(), Some(("six", "1.16.0"))) + .await + .unwrap_err(); + assert_eq!(code, "pypi_requirement_env_templated", "{detail}"); + } + /// #912: a pylock-only Pipenv checkout can't be vendored — Pipenv drops /// the `archive` entry the wiring writes and installs the upstream /// release — so it refuses with a `pipenv lock` pointer. Without a From ddc9e88e74098a3c98b5a8a82fa433f13b37d527 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 16:56:51 +0000 Subject: [PATCH 7/7] Contest ${VAR} pins in out-of-root includes too The #1851 merge added a second reader for -r includes outside the project root, and it still read `six==${SIXV}` as no pin. Once a lock was wired, adding such a line to an out-of-root include left `vendor --check` and VEX attesting the patch while pip installed six from PyPI. Both readers now go through one registry_line helper, so a templated line contests every wiring of the package it names wherever pip reads it. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vex/discover/pypi_locks.rs | 6 ++++ .../src/vex/discover/pypi_other.rs | 30 +++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/pypi_locks.rs b/crates/socket-patch-core/src/vex/discover/pypi_locks.rs index 8e7a78166..97bfe31c6 100644 --- a/crates/socket-patch-core/src/vex/discover/pypi_locks.rs +++ b/crates/socket-patch-core/src/vex/discover/pypi_locks.rs @@ -1420,10 +1420,16 @@ mod tests { ("-e git+https://example.com/click.git#egg=click\n", true), ("git+https://example.com/click.git#egg=click\n", true), ("../wheels/click-8.1.7-py3-none-any.whl\n", true), + // #1873: pip expands `${NAME}` where it installs, so a + // templated pin may install the wired release from PyPI. + ("click==${CLICK_VERSION}\n", true), + ("click${CLICK_SPEC}\n", true), ( "click @ https://files.pythonhosted.org/packages/click-8.1.6-py3-none-any.whl\n", false, ), + ("click==${click_version}\n", false), + ("idna==${IDNA_VERSION}\n", false), ("click==8.1.6\n", false), ("idna==3.7\n", false), ] { diff --git a/crates/socket-patch-core/src/vex/discover/pypi_other.rs b/crates/socket-patch-core/src/vex/discover/pypi_other.rs index 03f85047b..9a724b3e6 100644 --- a/crates/socket-patch-core/src/vex/discover/pypi_other.rs +++ b/crates/socket-patch-core/src/vex/discover/pypi_other.rs @@ -305,7 +305,7 @@ fn outside_root_line(ctx: &DiscoverCtx<'_>, file: &str, line: &str, out: &mut Di return; } let Some((name, location)) = requirement_location(code) else { - registry_pin(file, code, out); + registry_line(file, code, out); return; }; if matches!(classify(ctx, location, url_form(location)), Ok(None)) { @@ -342,18 +342,7 @@ fn requirements_line(ctx: &DiscoverCtx<'_>, file: &str, line: &str, out: &mut Di return; } let Some((name, location)) = requirement_location(code) else { - if requirements::has_env_reference(code) { - // pip expands `${NAME}` where it runs, so a registry line - // carrying one may install any release of the package it - // names: evidence against every wiring of it (#1873). - if let Some(name) = requirements::requirement_name(code) { - out.resolved_elsewhere_any_version(file, pypi_name_purl(name)); - } - return; - } - // A registry requirement; a pin that can only install one release - // is evidence against another lock's wiring of that release. - registry_pin(file, code, out); + registry_line(file, code, out); return; }; let tag = comment.and_then(vendor_tag); @@ -373,6 +362,21 @@ fn requirements_line(ctx: &DiscoverCtx<'_>, file: &str, line: &str, out: &mut Di ); } +/// A registry requirement line, in-root or in an out-of-root include. pip +/// expands `${NAME}` where it runs, so a line carrying one may install any +/// release of the package it names: evidence against every wiring of it +/// (#1873). Otherwise a pin that can only install one release is evidence +/// against another lock's wiring of that release ([`registry_pin`]). +fn registry_line(file: &str, code: &str, out: &mut Discovery) { + if requirements::has_env_reference(code) { + if let Some(name) = requirements::requirement_name(code) { + out.resolved_elsewhere_any_version(file, pypi_name_purl(name)); + } + return; + } + registry_pin(file, code, out); +} + /// Record a registry requirement that can only install one release as /// evidence against another lock's wiring of it: an exact /// `name[extras]==X` pin in any PEP 440 spelling (`==1.16.0.0`,