Fix requirements -r include tree counted as a competing lock (#1086)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -381,7 +381,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-<uuid>` 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 `<add>`, pom `<repository>`, 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 `<profile>` 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). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). 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, Bun or yarn 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` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) 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). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). 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, Bun or yarn 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` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy.
 * **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Two cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says.
 * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-<uuid>`, 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).
 

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).

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;
 
@@ -546,6 +546,12 @@ pub struct Discovery {
     pub unwired_copies: Vec<UnwiredCopy>,
     /// Refs dropped because another lock contests them ([`ContestedRef`]).
     pub contested: Vec<ContestedRef>,
+    /// 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<PathBuf, PathBuf>,
     /// Every file an extractor read through the guarded reads
     /// ([`DiscoverCtx::read_text`] / [`DiscoverCtx::read_bytes`]), with the
     /// ecosystem whose extractor read it — sorted, deduped. "No ref wires
@@ -787,22 +793,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();
@@ -837,6 +848,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) {
@@ -4312,6 +4340,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).

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

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,
         read: _,
         withheld: _,
         // Already folded into `unattested` by the time a run returns.
@@ -296,6 +297,13 @@ fn render(out: &Discovery, root: &Path) -> Value {
             .collect::<Vec<_>>()
             .into();
     }
+    if !install_trees.is_empty() {
+        rendered["install_trees"] = install_trees
+            .iter()
+            .map(|(file, root)| (path_str(file), Value::from(path_str(root))))
+            .collect::<Map<_, _>>()
+            .into();
+    }
     rendered
 }