From 2e3c68467db2a5bccd1a1fff2ee69fcf44bd365d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:22:14 +0000 Subject: [PATCH 1/2] Start fix for #1086 Assisted-by: Claude Code:claude-opus-5-5 From 17811174f86174e029dd2f2c88619512bc6cbeb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:43:40 +0000 Subject: [PATCH 2/2] Treat a requirements include tree as one lock pip reads requirements.txt and every -r file it includes as one requirement set, where a Socket direct reference wins over a compatible == pin of the same version. Discovery treated each include as a separate lock, so a duplicate pin in dev.txt "contested" the hosted or vendored wiring in the same tree. After a hosted scan, vex then attested nothing (exit 2) and rollback refused the project (exit 1), although pip installs the patched wheel. Discovery now records which install tree each requirements file belongs to, and the cross-lock contest compares trees instead of file names. A lock outside the tree (uv.lock, Pipfile.lock, pylock) still contests the wiring as before. Fixes #1086 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/in_process_rollback_hosted.rs | 31 +++- .../socket-patch-core/src/vex/discover/mod.rs | 153 +++++++++++++++++- .../src/vex/discover/pypi_other.rs | 3 + .../src/vex/discover/testing/golden.rs | 8 + 5 files changed, 188 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index b02fa44fa..3b7adda3f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -379,7 +379,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. -* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. Another entry of the **same** npm or Bun lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install`) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. +* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. Another entry of the **same** npm or Bun lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install`) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). **Record resolution.** A candidate's record must carry the patch uuid the lockfile actually **wires**. It is taken from the first source that has one: the manifest (matched qualifier-insensitively), the hosted records above (this run's, then a pre-v5 ledger's), then the vendor ledger's embedded records. If none has it and the run is online, `vex` fetches the patch view by uuid from the patch API — for a v5 hosted checkout this is the normal path. The fetch uses `get`'s API client: the public proxy when no token is configured, and a one-shot 401/403 fallback to the proxy (free patches only). At most 10 fetches run concurrently. Fetched records stay in memory: `vex` never writes the manifest. A candidate still has no record under `--offline`, after a transport error or a 404, or when the patch is refused (paid without an entitled token); it is then omitted as `record_unavailable`, and the run is not aborted. A record whose uuid or package disagrees with the wiring is omitted as `record_mismatch`. The informational `socket-patch.vendor.json` marker is never a record source. When the lockfile wires a package to patch U, a manifest or ledger record for that package under another uuid is superseded, and a human-mode `Note:` says so. diff --git a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs index 5091adb59..5f7c76c23 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs @@ -757,6 +757,24 @@ async fn npm_hosted_round_trip_envelope() { #[tokio::test] #[serial] async fn pypi_requirements_hosted_round_trip() { + pypi_requirements_round_trip("flask==2.0.1\nrequests==2.31.0\n", &[]).await; +} + +/// REGRESSION (#1086): the same round trip when an in-root `-r` include +/// pins the same `requests==2.31.0` (split base/dev files), with the `-r` +/// line before and after the root pin. pip reads the root and its includes +/// as one requirement set, where the hosted direct reference wins, so the +/// include's compatible pin is not a competing lock: `vex` attests the +/// patch and `rollback` restores the root line (it used to exit 2 and 1). +#[tokio::test] +#[serial] +async fn pypi_requirements_hosted_round_trip_with_a_duplicate_include_pin() { + let dev = [("dev.txt", "requests==2.31.0\n")]; + pypi_requirements_round_trip("-r dev.txt\nflask==2.0.1\nrequests==2.31.0\n", &dev).await; + pypi_requirements_round_trip("flask==2.0.1\nrequests==2.31.0\n-r dev.txt\n", &dev).await; +} + +async fn pypi_requirements_round_trip(pristine: &'static str, includes: &[(&str, &str)]) { const PY_UUID: &str = "a1a1a1a1-a1a1-4a1a-8a1a-a1a1a1a1a1a1"; const PY_PURL: &str = "pkg:pypi/requests@2.31.0"; const SHA256: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; @@ -811,8 +829,10 @@ async fn pypi_requirements_hosted_round_trip() { .await; let tmp = tempfile::tempdir().unwrap(); - let pristine = "flask==2.0.1\nrequests==2.31.0\n"; std::fs::write(tmp.path().join("requirements.txt"), pristine).unwrap(); + for (file, text) in includes { + std::fs::write(tmp.path().join(file), text).unwrap(); + } let get_args = socket_patch_cli::commands::get::GetArgs { common: socket_patch_cli::args::GlobalArgs { @@ -889,6 +909,10 @@ async fn pypi_requirements_hosted_round_trip() { !tmp.path().join(".socket").exists(), "a fully unwound hosted project keeps no .socket/ residue" ); + for (file, text) in includes { + let after = std::fs::read_to_string(tmp.path().join(file)).unwrap(); + assert_eq!(&after, text, "hosted mode never edits the include {file}"); + } // After the rollback nothing references the patch any more: VEX finds // nothing to attest (online, the API would still vouch for the uuid). @@ -1144,8 +1168,9 @@ async fn a_git_pattern_hosted_pin_is_refused_not_restored_to_the_registry() { assert!( envelope["hosted"]["failed"][0]["error"] .as_str() - .is_some_and(|e| e.contains("installs from git") - && e.contains("`git checkout -- yarn.lock`")), + .is_some_and( + |e| e.contains("installs from git") && e.contains("`git checkout -- yarn.lock`") + ), "{envelope}" ); assert_eq!( diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index ba855e20b..ae243afb9 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -179,7 +179,7 @@ //! `vlt-lock.json`), so discovery and the inventory see one entry walk //! per format (an architecture test in this module enforces it). -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; use std::sync::Mutex; @@ -481,6 +481,12 @@ pub struct Discovery { pub unattested: Vec, /// Refs dropped because another lock contests them ([`ContestedRef`]). pub contested: Vec, + /// Root-relative files that are not locks of their own but part of + /// another file's install tree, mapped to that tree's root file: the + /// in-root `-r` includes of `requirements.txt`, which pip reads with the + /// root as one requirement set ([`Discovery::same_install_tree`]). A + /// file absent here is its own tree. + pub install_trees: BTreeMap, } impl Discovery { @@ -694,22 +700,27 @@ impl Discovery { /// 11). This generalizes the npm extractor's shrinkwrap/package-lock /// rule to every pair of locks. PEP 723 script locks (`*.py.lock`) /// neither contest nor are contested: each is scoped to its own script's - /// install. + /// install. Files of one install tree ([`Discovery::install_trees`]) are + /// one lock here: pip resolves a direct reference and a compatible `==` + /// pin of the same version in `requirements.txt` and its `-r` includes + /// to the reference (#1086). fn contest_across_locks(&mut self) { let wiring: BTreeSet<(String, PathBuf)> = self .refs .iter() - .map(|r| (r.purl.clone(), r.source_file.clone())) + .map(|r| (r.purl.clone(), self.tree_of(&r.source_file).to_path_buf())) .collect(); let mut contested = Vec::new(); let refs = std::mem::take(&mut self.refs); for r in refs { let other = (!is_script_lock(&r.source_file)) .then(|| { + let tree = self.tree_of(&r.source_file); self.elsewhere.iter().find(|e| { + let other = self.tree_of(&e.file); e.purl == r.purl - && e.file != r.source_file - && !wiring.contains(&(e.purl.clone(), e.file.clone())) + && other != tree + && !wiring.contains(&(e.purl.clone(), other.to_path_buf())) }) }) .flatten(); @@ -744,6 +755,23 @@ impl Discovery { } } + /// Record that root-relative `file` is installed as part of the tree + /// rooted at `root` (see [`Discovery::install_trees`]). + pub(crate) fn install_tree(&mut self, file: &str, root: &str) { + if file != root { + self.install_trees + .insert(PathBuf::from(file), PathBuf::from(root)); + } + } + + /// The install tree `file` belongs to: its tree's root file, or itself. + fn tree_of<'a>(&'a self, file: &'a Path) -> &'a Path { + self.install_trees + .get(file) + .map(PathBuf::as_path) + .unwrap_or(file) + } + /// Record a lockless Socket pin (see [`UnlockedPin`]). pub(crate) fn unlocked_pin(&mut self, pin: UnlockedPin) { if is_canonical_uuid(&pin.uuid) && !self.unlocked_pins.contains(&pin) { @@ -4016,6 +4044,121 @@ mod tests { } } + /// REGRESSION (#1086): the root `requirements.txt` and its in-root `-r` + /// includes are ONE install tree (`pip install -r requirements.txt` + /// reads them as one requirement set, where a direct reference beats a + /// compatible `==` pin of the same version). A duplicate exact pin in an + /// include therefore never contests a wiring elsewhere in the same tree, + /// whichever file carries the wiring and wherever the `-r` line sits. A + /// lock outside the tree (`uv.lock`) still contests it. + #[tokio::test] + async fn a_requirements_include_tree_is_one_lock_in_the_contest() { + const SHA: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + let wheel = "vexdemo-1.2.3-py3-none-any.whl"; + let pypi_url = hosted_url("pypi", "vexdemo", "1.2.3", UUID_A, wheel); + let hosted = format!("vexdemo @ {pypi_url} --hash=sha256:{SHA}\n"); + let vendored = format!("./.socket/vendor/pypi/{UUID_B}/{wheel} --hash=sha256:{SHA}\n"); + let pin = "vexdemo==1.2.3\n".to_string(); + type Files = Vec<(&'static str, String)>; + let cases: Vec<(&str, Files, &str, WiringMode, &str)> = vec![ + ( + "hosted root, include first", + vec![ + ("requirements.txt", format!("-r dev.txt\n{hosted}")), + ("dev.txt", pin.clone()), + ], + UUID_A, + WiringMode::Hosted, + "requirements.txt", + ), + ( + "hosted root, include last", + vec![ + ("requirements.txt", format!("{hosted}-r dev.txt\n")), + ("dev.txt", pin.clone()), + ], + UUID_A, + WiringMode::Hosted, + "requirements.txt", + ), + ( + "hosted root, pin in a nested include", + vec![ + ( + "requirements.txt", + format!("{hosted}--requirement reqs/dev.txt\n"), + ), + ("reqs/dev.txt", "-r more.txt\n".to_string()), + ("reqs/more.txt", pin.clone()), + ], + UUID_A, + WiringMode::Hosted, + "requirements.txt", + ), + ( + "vendored include, pin in the root", + vec![ + ("requirements.txt", format!("{pin}-r dev.txt\n")), + ("dev.txt", vendored.clone()), + ], + UUID_B, + WiringMode::Vendored, + "dev.txt", + ), + ]; + for (name, files, uuid, mode, wired_in) in cases { + let p = Project::new(); + for (file, text) in &files { + p.write(file, text); + } + let out = p.discover().await; + assert_eq!( + out.refs.len(), + 1, + "{name}: {:#?} {:#?}", + out.refs, + out.diagnostics + ); + let r = &out.refs[0]; + assert_eq!( + (r.purl.as_str(), r.uuid.as_str(), r.mode), + ("pkg:pypi/vexdemo@1.2.3", uuid, mode), + "{name}" + ); + assert_eq!(r.source_file, Path::new(wired_in), "{name}"); + assert!(out.contested.is_empty(), "{name}: {:#?}", out.contested); + assert!( + !out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE), + "{name}: {:#?}", + out.diagnostics + ); + if mode == WiringMode::Hosted { + assert_eq!( + out.hosted_claim("pkg:pypi/vexdemo@1.2.3", uuid), + Some(true), + "{name}" + ); + } + + // A lock OUTSIDE the tree still contests the same wiring, and + // names itself, not the include. + p.write( + "uv.lock", + format!( + "version = 1\nrequires-python = \">=3.8\"\n\n[[package]]\nname = \"vexdemo\"\n\ + version = \"1.2.3\"\nsource = {{ registry = \"https://pypi.org/simple\" }}\n\ + wheels = [{{ url = \"https://files.pythonhosted.org/packages/{wheel}\", hash = \"sha256:{SHA}\" }}]\n" + ), + ); + let out = p.discover().await; + assert!(out.refs.is_empty(), "{name} + uv.lock: {:#?}", out.refs); + assert_eq!(out.contested.len(), 1, "{name} + uv.lock"); + assert_eq!(out.contested[0].other, Path::new("uv.lock"), "{name}"); + } + } + /// What does NOT contest: a lock that never mentions the package, a lock /// that wires the same patch too, another version of the package, and a /// PEP 723 script lock (scoped to its script's own install). 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 4372d887e..94b64afec 100644 --- a/crates/socket-patch-core/src/vex/discover/pypi_other.rs +++ b/crates/socket-patch-core/src/vex/discover/pypi_other.rs @@ -234,6 +234,9 @@ async fn extract_requirements(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { } }; for file in files.iter().take(MAX_REQUIREMENTS_FILES) { + // pip installs the root and every include it reaches as ONE + // requirement set, so they contest other locks as one (#1086). + out.install_tree(file, ROOT_REQUIREMENTS); let Some(text) = ctx.read_text(file, out).await else { continue; }; diff --git a/crates/socket-patch-core/src/vex/discover/testing/golden.rs b/crates/socket-patch-core/src/vex/discover/testing/golden.rs index 36b15632e..24901c65c 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -123,6 +123,7 @@ fn render(out: &Discovery, root: &Path) -> Value { unpatched_copies, unattested, contested, + install_trees, } = out; let refs: Vec = refs .iter() @@ -270,6 +271,13 @@ fn render(out: &Discovery, root: &Path) -> Value { .collect::>() .into(); } + if !install_trees.is_empty() { + rendered["install_trees"] = install_trees + .iter() + .map(|(file, root)| (path_str(file), Value::from(path_str(root)))) + .collect::>() + .into(); + } rendered }