Skip to content

Commit d63ae5f

Browse files
mikolalysenkoclaude
mikolalysenko
and
claude
authored
Fix lock-only requirements.txt discovery (#412, #523) (#530)
* Start fix for #412, #523 Assisted-by: Claude Code:claude-opus-5-5 * 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 * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8eec03a commit d63ae5f

5 files changed

Lines changed: 434 additions & 32 deletions

File tree

‎crates/socket-patch-cli/tests/scan_requirements_lock_only.rs‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
//! Lock-only `scan` over a pip `requirements.txt` (a fresh checkout: no
2+
//! virtualenv yet, the usual CI case). Discovery must read the pins the
3+
//! way pip does, or the package never reaches the patch API and `scan`
4+
//! reports "No patches available" while pip installs the unpatched
5+
//! release:
6+
//!
7+
//! * #523: whitespace around `==` and the legacy `name (==X)` form;
8+
//! * #412: pins reached through in-root `-r` includes.
9+
//!
10+
//! Driven through the built binary against a mock patch API; the
11+
//! assertion is what discovery sends to the batch endpoint and the
12+
//! `lockfileOnlyPackages` count in the JSON envelope, in both hosted and
13+
//! vendored mode. The package names are fixtures no interpreter on the
14+
//! machine has installed, so every hit is a lock-only one.
15+
16+
use std::path::Path;
17+
use std::process::Command;
18+
19+
use wiremock::matchers::{method, path};
20+
use wiremock::{Mock, MockServer, ResponseTemplate};
21+
22+
const ORG_SLUG: &str = "test-org";
23+
24+
async fn mount_empty_batch(mock: &MockServer) {
25+
Mock::given(method("POST"))
26+
.and(path(format!("/v0/orgs/{ORG_SLUG}/patches/batch")))
27+
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
28+
"packages": [],
29+
"canAccessPaidPatches": false,
30+
})))
31+
.mount(mock)
32+
.await;
33+
}
34+
35+
fn run_scan(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, serde_json::Value) {
36+
let mut argv = vec![
37+
"scan",
38+
"--json",
39+
"--yes",
40+
"--api-url",
41+
mock_uri,
42+
"--api-token",
43+
"fake-token",
44+
"--org",
45+
ORG_SLUG,
46+
];
47+
argv.extend_from_slice(extra);
48+
let out = Command::new(env!("CARGO_BIN_EXE_socket-patch"))
49+
.args(&argv)
50+
.current_dir(root)
51+
.env("SOCKET_TELEMETRY_DISABLED", "1")
52+
.env_remove("VIRTUAL_ENV")
53+
.env_remove("CONDA_PREFIX")
54+
.output()
55+
.expect("run socket-patch");
56+
let stdout = String::from_utf8_lossy(&out.stdout);
57+
let stderr = String::from_utf8_lossy(&out.stderr);
58+
let v = serde_json::from_str(stdout.trim())
59+
.unwrap_or_else(|e| panic!("invalid JSON ({e}): stdout={stdout}; stderr={stderr}"));
60+
(out.status.code().unwrap_or(-1), v)
61+
}
62+
63+
/// Every purl the scan sent to the batch endpoint.
64+
async fn batch_purls(mock: &MockServer) -> Vec<String> {
65+
let mut purls: Vec<String> = Vec::new();
66+
for req in mock.received_requests().await.unwrap_or_default() {
67+
if !req.url.path().ends_with("/patches/batch") {
68+
continue;
69+
}
70+
let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap_or_default();
71+
let found = body["components"]
72+
.as_array()
73+
.or_else(|| body["purls"].as_array())
74+
.cloned()
75+
.unwrap_or_default();
76+
for c in found {
77+
let purl = c["purl"]
78+
.as_str()
79+
.or_else(|| c.as_str())
80+
.map(str::to_string);
81+
purls.extend(purl);
82+
}
83+
}
84+
purls.sort();
85+
purls.dedup();
86+
purls
87+
}
88+
89+
async fn assert_lock_only_discovers(files: &[(&str, &str)], expected: &[&str]) {
90+
for mode in [&[][..], &["--vendor"][..]] {
91+
let mock = MockServer::start().await;
92+
mount_empty_batch(&mock).await;
93+
let tmp = tempfile::tempdir().unwrap();
94+
for (rel, content) in files {
95+
let p = tmp.path().join(rel);
96+
std::fs::create_dir_all(p.parent().unwrap()).unwrap();
97+
std::fs::write(p, content).unwrap();
98+
}
99+
let (code, v) = run_scan(tmp.path(), &mock.uri(), mode);
100+
assert_eq!(code, 0, "mode={mode:?}: {v}");
101+
assert_eq!(
102+
v["lockfileOnlyPackages"].as_u64(),
103+
Some(expected.len() as u64),
104+
"mode={mode:?}: {v}"
105+
);
106+
let purls = batch_purls(&mock).await;
107+
for want in expected {
108+
assert!(
109+
purls.iter().any(|p| p == want),
110+
"mode={mode:?}: {want} must reach the patch API; sent {purls:?}; {v}"
111+
);
112+
}
113+
}
114+
}
115+
116+
/// #523: spaced and parenthesised exact pins are discovered.
117+
#[tokio::test]
118+
async fn lock_only_scan_discovers_spaced_pins() {
119+
assert_lock_only_discovers(
120+
&[(
121+
"requirements.txt",
122+
"sp-fixture-a == 1.15.0\n\
123+
sp-fixture-b ==1.15.0\n\
124+
sp-fixture-c== 1.15.0\n\
125+
sp-fixture-d[x] == 1.15.0\n\
126+
sp-fixture-e (==1.15.0)\n",
127+
)],
128+
&[
129+
"pkg:pypi/sp-fixture-a@1.15.0",
130+
"pkg:pypi/sp-fixture-b@1.15.0",
131+
"pkg:pypi/sp-fixture-c@1.15.0",
132+
"pkg:pypi/sp-fixture-d@1.15.0",
133+
"pkg:pypi/sp-fixture-e@1.15.0",
134+
],
135+
)
136+
.await;
137+
}
138+
139+
/// #412: pins in an in-root `-r` include are discovered.
140+
#[tokio::test]
141+
async fn lock_only_scan_discovers_included_pins() {
142+
assert_lock_only_discovers(
143+
&[
144+
("requirements.txt", "-r requirements/base.txt\n"),
145+
("requirements/base.txt", "sp-fixture-six==1.16.0\n"),
146+
],
147+
&["pkg:pypi/sp-fixture-six@1.16.0"],
148+
)
149+
.await;
150+
}

‎crates/socket-patch-core/src/utils/requirements.rs‎

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,38 @@ pub(crate) fn strip_comment(text: &str) -> &str {
9696
/// The `(name as spelled, version)` of an exact `name[extras]==X` registry
9797
/// requirement (a logical line's code part; an optional `; marker` and
9898
/// options may follow), `None` for anything else — ranges, `===`, wildcards
99-
/// (`==1.*`), a version not starting with a digit. The ONE exact-pin rule
100-
/// the lock inventory and lockfile discovery read requirements with.
99+
/// (`==1.*`), a version not starting with a digit. Spelled as pip reads it:
100+
/// whitespace may surround the extras and the `==` (`six == 1.0`,
101+
/// `six[x] ==1.0`), and the legacy parenthesised form `six (==1.0)` is the
102+
/// same pin. The ONE exact-pin rule the lock inventory and lockfile
103+
/// discovery read requirements with.
101104
pub(crate) fn exact_pin(code: &str) -> Option<(&str, &str)> {
102-
let spec = code.split(';').next()?.split_whitespace().next()?;
103-
let (name, version) = spec.split_once("==")?;
104-
let name = name.split('[').next()?.trim();
105-
let version = version.trim();
105+
let spec = code.split(';').next()?.trim_start();
106+
let name_end = spec
107+
.find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')))
108+
.unwrap_or(spec.len());
109+
let (name, mut rest) = spec.split_at(name_end);
110+
rest = rest.trim_start();
111+
if rest.starts_with('[') {
112+
rest = rest[rest.find(']')? + 1..].trim_start();
113+
}
114+
let parenthesised = rest.starts_with('(');
115+
if parenthesised {
116+
rest = rest[1..].trim_start();
117+
}
118+
rest = rest.strip_prefix("==")?.trim_start();
119+
let version_end = rest
120+
.find(|c: char| c.is_whitespace() || matches!(c, ')' | ','))
121+
.unwrap_or(rest.len());
122+
let (version, mut rest) = rest.split_at(version_end);
123+
rest = rest.trim_start();
124+
if parenthesised {
125+
rest = rest.strip_prefix(')')?.trim_start();
126+
}
127+
// Only options (`--hash=…`) may follow the specifier; anything else
128+
// (`,<2`, a stray `)`, a second token) is not one exact pin.
106129
if name.is_empty()
130+
|| !(rest.is_empty() || rest.starts_with("--"))
107131
|| version.starts_with('=')
108132
|| version.contains('*')
109133
|| !version.starts_with(|c: char| c.is_ascii_digit())
@@ -267,6 +291,23 @@ mod tests {
267291
exact_pin("requests[socks]==2.31.0; python_version < \"3.12\" --hash=sha256:ab"),
268292
Some(("requests", "2.31.0"))
269293
);
294+
// #523: pip's whitespace around `==` and the legacy parenthesised
295+
// form are the same exact pin.
296+
for code in [
297+
"six == 1.16.0",
298+
"six ==1.16.0",
299+
"six== 1.16.0",
300+
"six\t==\t1.16.0",
301+
"six (==1.16.0)",
302+
"six ( == 1.16.0 )",
303+
"six(==1.16.0)",
304+
"six [x] == 1.16.0",
305+
"six[x] == 1.16.0 ; python_version >= \"3.8\"",
306+
"six == 1.16.0 --hash=sha256:ab",
307+
"six (==1.16.0) --hash sha256:ab",
308+
] {
309+
assert_eq!(exact_pin(code), Some(("six", "1.16.0")), "{code}");
310+
}
270311
for code in [
271312
"six==1.*",
272313
"six==1.16.*",
@@ -275,7 +316,13 @@ mod tests {
275316
"six>=1.0",
276317
"six",
277318
"==1.0",
278-
"six == 1.0",
319+
"six == 1.*",
320+
"six (==1.0",
321+
"six ==1.0)",
322+
"six==1.0,<2",
323+
"six == 1.0, <2",
324+
"six==1.0 extra",
325+
"six @ https://h/six-1.0-py3-none-any.whl",
279326
] {
280327
assert_eq!(exact_pin(code), None, "{code}");
281328
}

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

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -584,19 +584,34 @@ async fn inventory_pdm_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEntry>
584584
///
585585
/// A user's OWN file/url/path reference is not ours to resolve and stays
586586
/// out.
587+
///
588+
/// The pins are read from the root `requirements.txt` AND every in-root
589+
/// `-r` / `--requirement` include it reaches ([`requirements_tree`]) — the
590+
/// tree the vendored writer edits, so a pin there is discovered on a fresh
591+
/// checkout too (#412).
587592
async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option<Vec<LockfileEntry>> {
588-
let text = view.read_text("requirements.txt").await.ok()?;
589-
let lines = crate::utils::requirements::logical_lines(&text);
593+
let files = requirements_tree(view).await?;
594+
let lines: Vec<_> = files
595+
.iter()
596+
.flat_map(|text| crate::utils::requirements::logical_lines(text))
597+
.collect();
590598
// An exact pin's `--hash=sha256:` digests verify a PyPI download only
591599
// while the file resolves from the public index: an index option
592600
// (`-i` / `--index-url` / `--extra-index-url` / `-f`) may serve other
593601
// bytes under the same name, so it keeps every pin unverifiable (the
594-
// Pipfile.lock `public_index` rule).
602+
// Pipfile.lock `public_index` rule). pip applies an option from any
603+
// file of the tree globally, so the rule spans the whole tree.
595604
let public_index = lines.iter().all(|line| {
596605
let code = crate::utils::requirements::strip_comment(&line.text).trim_start();
597-
!["-i", "--index-url", "--extra-index-url", "-f", "--find-links"]
598-
.iter()
599-
.any(|opt| code.starts_with(opt))
606+
![
607+
"-i",
608+
"--index-url",
609+
"--extra-index-url",
610+
"-f",
611+
"--find-links",
612+
]
613+
.iter()
614+
.any(|opt| code.starts_with(opt))
600615
});
601616
let mut out = Vec::new();
602617
for line in lines {
@@ -660,3 +675,36 @@ async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option<Vec<Lockfi
660675
}
661676
Some(out)
662677
}
678+
679+
/// The text of the root `requirements.txt` (first) and of each in-root
680+
/// `-r` / `--requirement` include it reaches: depth-first, each target
681+
/// resolved against the INCLUDING file's directory, visited-set cycle
682+
/// guard — the vendored planner's include grammar
683+
/// ([`crate::vendor::pypi_requirements::requirements_includes`]) over a
684+
/// [`ProjectView`], so the in-memory engine reads the same tree. `-c`
685+
/// constraints never introduce requirements and are not followed;
686+
/// out-of-root and absolute includes are not ours to edit and are not
687+
/// read; an unreadable include is pip's error to report and is skipped.
688+
/// `None` when the root file itself cannot be read.
689+
async fn requirements_tree(view: &ProjectView<'_>) -> Option<Vec<String>> {
690+
use crate::vendor::pypi_requirements::{is_in_root_rel, requirements_includes};
691+
const ROOT: &str = "requirements.txt";
692+
let root = view.read_text(ROOT).await.ok()?;
693+
let mut visited = std::collections::HashSet::from([ROOT.to_string()]);
694+
let mut stack: Vec<String> = requirements_includes(ROOT, &root);
695+
stack.reverse();
696+
let mut files = vec![root];
697+
while let Some(rel) = stack.pop() {
698+
if !is_in_root_rel(&rel) || !visited.insert(rel.clone()) {
699+
continue;
700+
}
701+
let Ok(text) = view.read_text(&rel).await else {
702+
continue;
703+
};
704+
let mut includes = requirements_includes(&rel, &text);
705+
includes.reverse();
706+
stack.extend(includes);
707+
files.push(text);
708+
}
709+
Some(files)
710+
}

0 commit comments

Comments
 (0)