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/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}"); } 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();