Fix lock-only requirements.txt discovery (#412, #523)

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<String> {
+    let mut purls: Vec<String> = 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;
+}

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

crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs

@@ -584,19 +584,34 @@ async fn inventory_pdm_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEntry>
 ///
 /// 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<Vec<LockfileEntry>> {
-    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<Vec<Lockfi
     }
     Some(out)
 }
+
+/// The text of the root `requirements.txt` (first) and of each in-root
+/// `-r` / `--requirement` include it reaches: depth-first, each target
+/// resolved against the INCLUDING file's directory, visited-set cycle
+/// guard — the vendored planner's include grammar
+/// ([`crate::vendor::pypi_requirements::requirements_includes`]) over a
+/// [`ProjectView`], so the in-memory engine reads the same tree. `-c`
+/// constraints never introduce requirements and are not followed;
+/// out-of-root and absolute includes are not ours to edit and are not
+/// read; an unreadable include is pip's error to report and is skipped.
+/// `None` when the root file itself cannot be read.
+async fn requirements_tree(view: &ProjectView<'_>) -> Option<Vec<String>> {
+    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<String> = 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)
+}