From 3fce3bca01f190db70279a895b9a062d7fad3cb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 04:26:31 +0000 Subject: [PATCH 1/3] Start fix for #412, #523 Assisted-by: Claude Code:claude-opus-5-5 From e1a61984ba58efaac3f401a25d5063854927a3cf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 04:46:07 +0000 Subject: [PATCH 2/3] Read spaced requirements.txt pins as exact A requirements.txt pin written with spaces around `==` (`six == 1.15.0`), or in the legacy `six (==1.15.0)` form, was not recognized as an exact pin. On a fresh checkout with no venv, lock-only discovery never asked the patch API about the package. scan reported "No patches available" and pip installed the unpatched release. pip treats these forms exactly like `six==1.15.0`, and so does the hosted rewriter. exact_pin now parses the name, extras, optional parentheses and `==` the way pip does. Only options may follow the version, so a range such as `six==1.0,<2` is no longer mistaken for a pin. Lock-only `vex` evidence uses the same rule and gets the fix too. Fixes #523 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/utils/requirements.rs | 61 ++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index 5d3087e9b..b13f634d3 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -96,14 +96,38 @@ pub(crate) fn strip_comment(text: &str) -> &str { /// The `(name as spelled, version)` of an exact `name[extras]==X` registry /// requirement (a logical line's code part; an optional `; marker` and /// options may follow), `None` for anything else — ranges, `===`, wildcards -/// (`==1.*`), a version not starting with a digit. The ONE exact-pin rule -/// the lock inventory and lockfile discovery read requirements with. +/// (`==1.*`), a version not starting with a digit. Spelled as pip reads it: +/// whitespace may surround the extras and the `==` (`six == 1.0`, +/// `six[x] ==1.0`), and the legacy parenthesised form `six (==1.0)` is the +/// same pin. The ONE exact-pin rule the lock inventory and lockfile +/// discovery read requirements with. pub(crate) fn exact_pin(code: &str) -> Option<(&str, &str)> { - let spec = code.split(';').next()?.split_whitespace().next()?; - let (name, version) = spec.split_once("==")?; - let name = name.split('[').next()?.trim(); - let version = version.trim(); + let spec = code.split(';').next()?.trim_start(); + let name_end = spec + .find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))) + .unwrap_or(spec.len()); + let (name, mut rest) = spec.split_at(name_end); + rest = rest.trim_start(); + if rest.starts_with('[') { + rest = rest[rest.find(']')? + 1..].trim_start(); + } + let parenthesised = rest.starts_with('('); + if parenthesised { + rest = rest[1..].trim_start(); + } + rest = rest.strip_prefix("==")?.trim_start(); + let version_end = rest + .find(|c: char| c.is_whitespace() || matches!(c, ')' | ',')) + .unwrap_or(rest.len()); + let (version, mut rest) = rest.split_at(version_end); + rest = rest.trim_start(); + if parenthesised { + rest = rest.strip_prefix(')')?.trim_start(); + } + // Only options (`--hash=…`) may follow the specifier; anything else + // (`,<2`, a stray `)`, a second token) is not one exact pin. if name.is_empty() + || !(rest.is_empty() || rest.starts_with("--")) || version.starts_with('=') || version.contains('*') || !version.starts_with(|c: char| c.is_ascii_digit()) @@ -267,6 +291,23 @@ mod tests { exact_pin("requests[socks]==2.31.0; python_version < \"3.12\" --hash=sha256:ab"), Some(("requests", "2.31.0")) ); + // #523: pip's whitespace around `==` and the legacy parenthesised + // form are the same exact pin. + for code in [ + "six == 1.16.0", + "six ==1.16.0", + "six== 1.16.0", + "six\t==\t1.16.0", + "six (==1.16.0)", + "six ( == 1.16.0 )", + "six(==1.16.0)", + "six [x] == 1.16.0", + "six[x] == 1.16.0 ; python_version >= \"3.8\"", + "six == 1.16.0 --hash=sha256:ab", + "six (==1.16.0) --hash sha256:ab", + ] { + assert_eq!(exact_pin(code), Some(("six", "1.16.0")), "{code}"); + } for code in [ "six==1.*", "six==1.16.*", @@ -275,7 +316,13 @@ mod tests { "six>=1.0", "six", "==1.0", - "six == 1.0", + "six == 1.*", + "six (==1.0", + "six ==1.0)", + "six==1.0,<2", + "six == 1.0, <2", + "six==1.0 extra", + "six @ https://h/six-1.0-py3-none-any.whl", ] { assert_eq!(exact_pin(code), None, "{code}"); } From acb0abb5f4130b8d3ef52a48dcb6c2a8a53fa4f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 04:46:07 +0000 Subject: [PATCH 3/3] Discover pins in requirements.txt -r includes On a fresh checkout, lock-only discovery read only the root requirements.txt and skipped its `-r` include lines. A pin kept in an included file (`-r requirements/base.txt`) was never sent to the patch API. scan reported "No patches available", exited 0, and pip installed the unpatched release, in both hosted and vendored mode. The lock inventory now walks the same in-root include tree the vendored writer edits, using the writer's include parser. The walk runs through the project view, so the in-memory hosted engine sees it too. `-c` constraints and out-of-root includes are still not followed. An index option in any file of the tree now makes hashed pins unverifiable everywhere, matching how pip applies options globally. Fixes #412 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/scan_requirements_lock_only.rs | 150 ++++++++++++++++++ .../src/vendor/lock_inventory/pypi.rs | 60 ++++++- .../src/vendor/lock_inventory/tests.rs | 150 ++++++++++++++++++ .../src/vendor/pypi_requirements.rs | 45 +++--- 4 files changed, 380 insertions(+), 25 deletions(-) create mode 100644 crates/socket-patch-cli/tests/scan_requirements_lock_only.rs diff --git a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs new file mode 100644 index 000000000..9ab939607 --- /dev/null +++ b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs @@ -0,0 +1,150 @@ +//! Lock-only `scan` over a pip `requirements.txt` (a fresh checkout: no +//! virtualenv yet, the usual CI case). Discovery must read the pins the +//! way pip does, or the package never reaches the patch API and `scan` +//! reports "No patches available" while pip installs the unpatched +//! release: +//! +//! * #523: whitespace around `==` and the legacy `name (==X)` form; +//! * #412: pins reached through in-root `-r` includes. +//! +//! Driven through the built binary against a mock patch API; the +//! assertion is what discovery sends to the batch endpoint and the +//! `lockfileOnlyPackages` count in the JSON envelope, in both hosted and +//! vendored mode. The package names are fixtures no interpreter on the +//! machine has installed, so every hit is a lock-only one. + +use std::path::Path; +use std::process::Command; + +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const ORG_SLUG: &str = "test-org"; + +async fn mount_empty_batch(mock: &MockServer) { + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "packages": [], + "canAccessPaidPatches": false, + }))) + .mount(mock) + .await; +} + +fn run_scan(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, serde_json::Value) { + let mut argv = vec![ + "scan", + "--json", + "--yes", + "--api-url", + mock_uri, + "--api-token", + "fake-token", + "--org", + ORG_SLUG, + ]; + argv.extend_from_slice(extra); + let out = Command::new(env!("CARGO_BIN_EXE_socket-patch")) + .args(&argv) + .current_dir(root) + .env("SOCKET_TELEMETRY_DISABLED", "1") + .env_remove("VIRTUAL_ENV") + .env_remove("CONDA_PREFIX") + .output() + .expect("run socket-patch"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + let v = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("invalid JSON ({e}): stdout={stdout}; stderr={stderr}")); + (out.status.code().unwrap_or(-1), v) +} + +/// Every purl the scan sent to the batch endpoint. +async fn batch_purls(mock: &MockServer) -> Vec { + let mut purls: Vec = Vec::new(); + for req in mock.received_requests().await.unwrap_or_default() { + if !req.url.path().ends_with("/patches/batch") { + continue; + } + let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap_or_default(); + let found = body["components"] + .as_array() + .or_else(|| body["purls"].as_array()) + .cloned() + .unwrap_or_default(); + for c in found { + let purl = c["purl"] + .as_str() + .or_else(|| c.as_str()) + .map(str::to_string); + purls.extend(purl); + } + } + purls.sort(); + purls.dedup(); + purls +} + +async fn assert_lock_only_discovers(files: &[(&str, &str)], expected: &[&str]) { + for mode in [&[][..], &["--vendor"][..]] { + let mock = MockServer::start().await; + mount_empty_batch(&mock).await; + let tmp = tempfile::tempdir().unwrap(); + for (rel, content) in files { + let p = tmp.path().join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, content).unwrap(); + } + let (code, v) = run_scan(tmp.path(), &mock.uri(), mode); + assert_eq!(code, 0, "mode={mode:?}: {v}"); + assert_eq!( + v["lockfileOnlyPackages"].as_u64(), + Some(expected.len() as u64), + "mode={mode:?}: {v}" + ); + let purls = batch_purls(&mock).await; + for want in expected { + assert!( + purls.iter().any(|p| p == want), + "mode={mode:?}: {want} must reach the patch API; sent {purls:?}; {v}" + ); + } + } +} + +/// #523: spaced and parenthesised exact pins are discovered. +#[tokio::test] +async fn lock_only_scan_discovers_spaced_pins() { + assert_lock_only_discovers( + &[( + "requirements.txt", + "sp-fixture-a == 1.15.0\n\ + sp-fixture-b ==1.15.0\n\ + sp-fixture-c== 1.15.0\n\ + sp-fixture-d[x] == 1.15.0\n\ + sp-fixture-e (==1.15.0)\n", + )], + &[ + "pkg:pypi/sp-fixture-a@1.15.0", + "pkg:pypi/sp-fixture-b@1.15.0", + "pkg:pypi/sp-fixture-c@1.15.0", + "pkg:pypi/sp-fixture-d@1.15.0", + "pkg:pypi/sp-fixture-e@1.15.0", + ], + ) + .await; +} + +/// #412: pins in an in-root `-r` include are discovered. +#[tokio::test] +async fn lock_only_scan_discovers_included_pins() { + assert_lock_only_discovers( + &[ + ("requirements.txt", "-r requirements/base.txt\n"), + ("requirements/base.txt", "sp-fixture-six==1.16.0\n"), + ], + &["pkg:pypi/sp-fixture-six@1.16.0"], + ) + .await; +} diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs index 4960652e4..ad9f663fb 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs @@ -584,19 +584,34 @@ async fn inventory_pdm_lock(view: &ProjectView<'_>) -> Option /// /// A user's OWN file/url/path reference is not ours to resolve and stays /// out. +/// +/// The pins are read from the root `requirements.txt` AND every in-root +/// `-r` / `--requirement` include it reaches ([`requirements_tree`]) — the +/// tree the vendored writer edits, so a pin there is discovered on a fresh +/// checkout too (#412). async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option> { - let text = view.read_text("requirements.txt").await.ok()?; - let lines = crate::utils::requirements::logical_lines(&text); + let files = requirements_tree(view).await?; + let lines: Vec<_> = files + .iter() + .flat_map(|text| crate::utils::requirements::logical_lines(text)) + .collect(); // An exact pin's `--hash=sha256:` digests verify a PyPI download only // while the file resolves from the public index: an index option // (`-i` / `--index-url` / `--extra-index-url` / `-f`) may serve other // bytes under the same name, so it keeps every pin unverifiable (the - // Pipfile.lock `public_index` rule). + // Pipfile.lock `public_index` rule). pip applies an option from any + // file of the tree globally, so the rule spans the whole tree. let public_index = lines.iter().all(|line| { let code = crate::utils::requirements::strip_comment(&line.text).trim_start(); - !["-i", "--index-url", "--extra-index-url", "-f", "--find-links"] - .iter() - .any(|opt| code.starts_with(opt)) + ![ + "-i", + "--index-url", + "--extra-index-url", + "-f", + "--find-links", + ] + .iter() + .any(|opt| code.starts_with(opt)) }); let mut out = Vec::new(); for line in lines { @@ -660,3 +675,36 @@ async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option) -> Option> { + use crate::vendor::pypi_requirements::{is_in_root_rel, requirements_includes}; + const ROOT: &str = "requirements.txt"; + let root = view.read_text(ROOT).await.ok()?; + let mut visited = std::collections::HashSet::from([ROOT.to_string()]); + let mut stack: Vec = requirements_includes(ROOT, &root); + stack.reverse(); + let mut files = vec![root]; + while let Some(rel) = stack.pop() { + if !is_in_root_rel(&rel) || !visited.insert(rel.clone()) { + continue; + } + let Ok(text) = view.read_text(&rel).await else { + continue; + }; + let mut includes = requirements_includes(&rel, &text); + includes.reverse(); + stack.extend(includes); + files.push(text); + } + Some(files) +} diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index 3b793fa9c..e43f5d85a 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -2925,3 +2925,153 @@ async fn the_vendored_requirements_writers_own_output_reinventories() { "wet requirements.txt:\n{line}\nentries: {entries:?}" ); } + +/// #523: pip reads `six == 1.15.0`, `six ==1.15.0`, `six== 1.15.0`, +/// `six[x] == 1.15.0` and the legacy `six (==1.15.0)` exactly like +/// `six==1.15.0`, and so does the hosted rewriter — the lock-only +/// inventory must too, or a fresh checkout never asks for the patch. +#[tokio::test] +async fn requirements_spaced_and_parenthesised_pins_are_inventoried() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "requirements.txt", + "six == 1.15.0\n\ + idna ==3.7\n\ + attrs== 23.1.0\n\ + requests[socks] == 2.31.0 ; python_version >= \"3.8\"\n\ + certifi (==2024.2.2)\n\ + urllib3 ( == 1.26.18 ) # legacy parens\n\ + flask == 3.0.0 \\\n --hash=sha256:{sha}\n\ + jinja2 == 3.*\n\ + click == 8.0,<9\n" + .replace("{sha}", &"d".repeat(64)) + .as_str(), + ) + .await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("attrs".to_string(), "23.1.0".to_string()), + ("certifi".to_string(), "2024.2.2".to_string()), + ("flask".to_string(), "3.0.0".to_string()), + ("idna".to_string(), "3.7".to_string()), + ("requests".to_string(), "2.31.0".to_string()), + ("six".to_string(), "1.15.0".to_string()), + ("urllib3".to_string(), "1.26.18".to_string()), + ], + "{entries:?}" + ); + assert_eq!( + entry(&entries, "flask").integrity, + LockIntegrity::Sha256AnyOf(vec!["d".repeat(64)]), + "a spaced pin keeps its --hash digest" + ); +} + +/// #412: pins reached through in-root `-r` / `--requirement` includes +/// (resolved against the INCLUDING file's directory) join the lock-only +/// inventory — the same tree the vendored writer edits. `-c` constraints +/// and out-of-root includes are not followed; include cycles terminate. +#[tokio::test] +async fn requirements_in_root_includes_are_inventoried() { + let outer = tempfile::tempdir().unwrap(); + let root = outer.path().join("proj"); + tokio::fs::create_dir_all(&root).await.unwrap(); + write( + &root, + "requirements.txt", + "-r requirements/base.txt\n--requirement=requirements/dev.txt\n-c constraints.txt\n-r ../outside.txt\nflask==3.0.0\n", + ) + .await; + write_nested( + &root, + "requirements/base.txt", + "six==1.16.0\n-r common/extra.txt\n", + ) + .await; + // Relative to requirements/, not to the root. + write_nested( + &root, + "requirements/common/extra.txt", + "idna == 3.7\n-r ../base.txt\n", + ) + .await; + write_nested(&root, "requirements/dev.txt", "-rbase.txt\npytest==8.0.0\n").await; + write(&root, "constraints.txt", "attrs==23.1.0\n").await; + write(outer.path(), "outside.txt", "click==8.1.7\n").await; + + let entries = inventory_pypi_locks(&root).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("flask".to_string(), "3.0.0".to_string()), + ("idna".to_string(), "3.7".to_string()), + ("pytest".to_string(), "8.0.0".to_string()), + ("six".to_string(), "1.16.0".to_string()), + ], + "{entries:?}" + ); + + // A root file holding ONLY an include still yields the included pins + // (the #412 repro layout). + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "requirements.txt", "-r requirements/base.txt\n").await; + write_nested(tmp.path(), "requirements/base.txt", "six==1.16.0\n").await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![("six".to_string(), "1.16.0".to_string())] + ); + + // The in-memory hosted engine reads the same tree. + let mut project = MemoryProject::new(); + project.insert_text("requirements.txt", "-r requirements/base.txt\n"); + project.insert_text("requirements/base.txt", "six==1.16.0\n"); + let in_memory = super::pypi::inventory_pypi_locks_in(&ProjectView::Memory(&project)) + .await + .unwrap(); + assert_eq!(sorted_pairs(&in_memory), sorted_pairs(&entries)); +} + +/// pip applies an index option from ANY file of the tree globally, so an +/// `--index-url` inside an include keeps the root file's hashed pins +/// unverifiable too (the `public_index` rule spans the whole tree). +#[tokio::test] +async fn requirements_index_option_in_an_include_spans_the_tree() { + let sha = "e".repeat(64); + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "requirements.txt", + &format!("-r private.txt\nsix==1.16.0 --hash=sha256:{sha}\n"), + ) + .await; + write( + tmp.path(), + "private.txt", + &format!( + "--index-url https://pypi.internal.example/simple\nidna==3.7 --hash=sha256:{sha}\n" + ), + ) + .await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!(entry(&entries, "six").integrity, LockIntegrity::None); + assert_eq!(entry(&entries, "idna").integrity, LockIntegrity::None); + + // Control: no index option anywhere keeps the digests. + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "requirements.txt", "-r public.txt\n").await; + write( + tmp.path(), + "public.txt", + &format!("idna==3.7 --hash=sha256:{sha}\n"), + ) + .await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + entry(&entries, "idna").integrity, + LockIntegrity::Sha256AnyOf(vec![sha.clone()]) + ); +} diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 236e25dc0..aa45009c4 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -683,7 +683,7 @@ pub async fn requirements_include_names(root: &Path) -> std::io::Result bool { +pub(crate) fn is_in_root_rel(rel: &str) -> bool { !rel.starts_with("../") && !Path::new(rel).is_absolute() } @@ -711,24 +711,7 @@ async fn walk_requirements_tree( // Parse the includes BEFORE handing the content over (the visitor // takes it by value); nothing is pushed unless it asks to descend. let includes: Vec = match &read { - Ok(content) => { - let include_dir = match rel.rfind('/') { - Some(i) => rel[..i].to_string(), - None => String::new(), - }; - logical_lines(content) - .iter() - .filter_map(|ll| include_target(&ll.text)) - .map(|target| { - let joined = if include_dir.is_empty() { - target.to_string() - } else { - format!("{include_dir}/{target}") - }; - normalize_rel_path(&joined) - }) - .collect() - } + Ok(content) => requirements_includes(&rel, content), Err(_) => Vec::new(), }; if visit(&rel, &path, read)? { @@ -740,6 +723,30 @@ async fn walk_requirements_tree( Ok(()) } +/// The `-r`/`--requirement` includes of the requirements file `rel` +/// (root-relative) with `content`, in file order: each target resolved +/// against the INCLUDING file's directory and lexically normalized +/// (`requirements/../x.txt` → `x.txt`; an escape keeps its `../`). The one +/// include grammar behind the planner's walk and the lock inventory's. +pub(crate) fn requirements_includes(rel: &str, content: &str) -> Vec { + let include_dir = match rel.rfind('/') { + Some(i) => &rel[..i], + None => "", + }; + logical_lines(content) + .iter() + .filter_map(|ll| include_target(&ll.text)) + .map(|target| { + let joined = if include_dir.is_empty() { + target.to_string() + } else { + format!("{include_dir}/{target}") + }; + normalize_rel_path(&joined) + }) + .collect() +} + /// The `-r`/`--requirement` include target of a logical line, if any. fn include_target(text: &str) -> Option<&str> { let code = strip_comment(text).trim();