Fix pylock wiring ignoring an out-of-root -r pin (#1851)

crates/socket-patch-cli/tests/mode_migration_pypi.rs

@@ -3289,6 +3289,69 @@ async fn pip_lock_vendor_wires_requirements_with_the_pylock() {
     assert!(!root.join(format!(".socket/vendor/pypi/{UUID}")).exists());
 }
 
+/// #1851: requirements.txt pins six only through a `-r` include outside
+/// the project root, beside the `pip lock` pylock.toml. `pip install -r
+/// requirements.txt` installs that pin, and vendor can't edit the
+/// include, so vendoring six refuses with `pypi_requirements_outside_root`
+/// (as it does without the pylock) and writes nothing. Before the fix only
+/// the pylock was wired and the run exited 0. A pylock already wired
+/// beside such a pin (vendored before the fix, or a pin added later)
+/// fails `vendor --check` as contested instead of passing.
+#[tokio::test]
+async fn pip_lock_beside_an_out_of_root_include_pin_refuses() {
+    let (tmp, root) = project();
+    stage_pip_lock(&root);
+    let shared = tmp.path().join("shared");
+    std::fs::create_dir_all(&shared).unwrap();
+    std::fs::write(shared.join("base.txt"), "six==1.16.0\n").unwrap();
+    std::fs::write(
+        root.join("requirements.txt"),
+        "-r ../shared/base.txt\nidna==3.7\n",
+    )
+    .unwrap();
+    stage_manifest(&root);
+
+    let (code, env) = run_cli(&root, &["vendor"], &[]);
+    assert_ne!(code, 0, "{env:#}");
+    assert!(
+        env.to_string().contains("pypi_requirements_outside_root"),
+        "{env:#}"
+    );
+    assert_eq!(
+        std::fs::read(root.join("pylock.toml")).unwrap(),
+        PIP_LOCK.as_bytes(),
+        "the pylock is not wired: {env:#}"
+    );
+    assert_eq!(
+        std::fs::read(root.join("requirements.txt")).unwrap(),
+        b"-r ../shared/base.txt\nidna==3.7\n"
+    );
+    assert_eq!(
+        std::fs::read(shared.join("base.txt")).unwrap(),
+        b"six==1.16.0\n"
+    );
+
+    // The include pins another release: six is wired in the pylock only.
+    std::fs::write(shared.join("base.txt"), "six==1.15.0\n").unwrap();
+    let (code, env) = run_cli(&root, &["vendor"], &[]);
+    assert_eq!(code, 0, "{env:#}");
+    assert!(
+        std::fs::read_to_string(root.join("pylock.toml"))
+            .unwrap()
+            .contains(&format!(".socket/vendor/pypi/{UUID}/")),
+        "{env:#}"
+    );
+    let (code, env) = run_cli(&root, &["vendor", "--check"], &[]);
+    assert_eq!(code, 0, "{env:#}");
+
+    // Once the include pins the vendored release, pip installs the
+    // upstream six again: the pylock wiring is contested.
+    std::fs::write(shared.join("base.txt"), "six==1.16.0\n").unwrap();
+    let (code, env) = run_cli(&root, &["vendor", "--check"], &[]);
+    assert_ne!(code, 0, "the out-of-root pin contests the pylock: {env:#}");
+    assert!(env.to_string().contains("contested"), "{env:#}");
+}
+
 /// #1758: a pylock vendored while requirements.txt did not pin six yet
 /// (a project vendored before the fix wired only the pylock). Once the
 /// file pins six==1.16.0, the next `vendor` wires it to the committed

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

@@ -381,6 +381,40 @@ pub(crate) fn shlex_split(text: &str) -> Option<Vec<String>> {
     Some(words)
 }
 
+/// The location of an editable requirement (`-e <loc>`, `-e<loc>`,
+/// `--editable <loc>`, `--editable=<loc>`), as pip's option parser reads it.
+pub(crate) fn editable_location(code: &str) -> Option<&str> {
+    let rest = if let Some(rest) = code.strip_prefix("--editable") {
+        match rest.strip_prefix('=') {
+            Some(rest) => rest,
+            None if rest.starts_with(char::is_whitespace) => rest,
+            None => return None,
+        }
+    } else {
+        // A short option takes its value attached or after a space.
+        let rest = code.strip_prefix("-e")?;
+        rest.strip_prefix('=').unwrap_or(rest)
+    };
+    rest.split_whitespace().next()
+}
+
+/// The package a VCS / url requirement names in its `#egg=<name>` fragment.
+/// A legacy `#egg=<name>-<version>` fragment names the package before its
+/// first `-<digit>`, as setuptools and older pip read it.
+pub(crate) fn egg_name(location: &str) -> Option<&str> {
+    let (_, fragment) = location.split_once('#')?;
+    let egg = fragment.split('&').find_map(|p| p.strip_prefix("egg="))?;
+    let end = egg
+        .find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')))
+        .unwrap_or(egg.len());
+    let egg = &egg[..end];
+    let end = egg
+        .match_indices('-')
+        .find(|(i, _)| egg[i + 1..].starts_with(|c: char| c.is_ascii_digit()))
+        .map_or(egg.len(), |(i, _)| i);
+    Some(&egg[..end]).filter(|name| !name.is_empty())
+}
+
 /// One requirements.txt requirement, read from a logical line's code part
 /// (comment already stripped) the way pip reads it. The ONE requirements
 /// grammar: the lock inventory, lockfile discovery, the hosted rewriter and

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

@@ -244,6 +244,25 @@ async fn requirements_pins_target(project_root: &Path, target: Option<(&str, &st
     }
 }
 
+/// #1851: refuse wiring a lock beside a requirements set whose exact pin of
+/// the target sits in an out-of-root `-r` include. pip installs that pin,
+/// vendor cannot edit the file, and naming requirements.txt a loser would
+/// let the lock wiring report the package patched.
+/// A pin the lock wiring rewrites with it ([`super::pypi_requirements::
+/// sibling_pin_wirable`]) never has one: the planner refuses it first.
+async fn refuse_pin_outside_root(
+    project_root: &Path,
+    target: Option<(&str, &str)>,
+) -> Result<(), (&'static str, String)> {
+    let Some((name, version)) = target else {
+        return Ok(());
+    };
+    match super::pypi_requirements::pin_outside_root(project_root, name, version).await {
+        Some(refusal) => Err(refusal),
+        None => Ok(()),
+    }
+}
+
 /// Route the project to a wiring flavor, first match wins. Lockfiles are the
 /// authoritative "this tool manages installs" signal, so locks are compared
 /// with locks (precedence follows migration direction / ecosystem currency:
@@ -396,6 +415,7 @@ async fn detect_pypi_flavor(
             }
             None => false,
         };
+        refuse_pin_outside_root(project_root, target).await?;
         if !wired_with_lock && exists(PYPI_REQUIREMENTS).await {
             present.push(PYPI_REQUIREMENTS);
         }
@@ -449,6 +469,9 @@ async fn detect_pypi_flavor(
         }
         _ => false,
     };
+    if governing.is_some() {
+        refuse_pin_outside_root(project_root, target).await?;
+    }
     if governing.is_some()
         && !wired_with_lock
         && requirements_pins_target(project_root, target).await
@@ -4138,6 +4161,132 @@ mod tests {
         }
     }
 
+    /// #1851: pip installs an exact pin from a `-r` include outside the
+    /// project root, which vendor cannot edit. Wiring only the lock beside
+    /// it would leave `pip install -r requirements.txt` on the upstream
+    /// release, so every lock flavor refuses it as the requirements-only
+    /// planner does, instead of naming requirements.txt a loser.
+    #[tokio::test]
+    async fn exact_pin_in_an_out_of_root_include_refuses_beside_a_lock() {
+        let pylock = PIPENV_PYLOCK.replace("pipenv", "pip");
+        for (lock, content) in [
+            ("pylock.toml", pylock.as_str()),
+            ("uv.lock", "version = 1\n"),
+            ("Pipfile.lock", "{}"),
+            ("poetry.lock", ""),
+        ] {
+            let tmp = tempfile::tempdir().unwrap();
+            let root = tmp.path().join("app");
+            tokio::fs::create_dir_all(&root).await.unwrap();
+            touch(tmp.path(), "base.txt", "six==1.16.0\n").await;
+            touch(&root, lock, content).await;
+            touch(&root, "requirements.txt", "-r ../base.txt\nidna==3.7\n").await;
+            let (code, detail) = detect_pypi_flavor(&root, Some(("six", "1.16.0")))
+                .await
+                .unwrap_err();
+            assert_eq!(code, "pypi_requirements_outside_root", "{lock}: {detail}");
+            assert!(detail.contains("../base.txt"), "{lock}: {detail}");
+
+            // Another release, or a package only the in-root file names,
+            // is not refused.
+            touch(tmp.path(), "base.txt", "six==1.15.0\n").await;
+            assert!(
+                detect_pypi_flavor(&root, Some(("six", "1.16.0")))
+                    .await
+                    .is_ok(),
+                "{lock}"
+            );
+            assert!(
+                detect_pypi_flavor(&root, Some(("idna", "3.7")))
+                    .await
+                    .is_ok(),
+                "{lock}"
+            );
+        }
+    }
+
+    /// #1851: every shape through which pip installs the release from an
+    /// out-of-root include refuses, as discovery would contest the wiring
+    /// it leaves: extras, `===`, per-requirement options, a direct
+    /// reference, an editable checkout, and a UTF-16 `pip freeze` export.
+    #[tokio::test]
+    async fn every_installing_shape_in_an_out_of_root_include_refuses() {
+        let pylock = PIPENV_PYLOCK.replace("pipenv", "pip");
+        let utf16: Vec<u8> = [0xFF, 0xFE]
+            .into_iter()
+            .chain("six==1.16.0\r\n".encode_utf16().flat_map(u16::to_le_bytes))
+            .collect();
+        let mut shapes: Vec<(&str, Vec<u8>)> = [
+            "six[socks]==1.16.0\n",
+            "six===1.16.0\n",
+            "Six==1.16.0 --config-settings=x=y\n",
+            "six @ https://files.pythonhosted.org/packages/six-1.16.0-py2.py3-none-any.whl\n",
+            "-e git+https://example.com/six.git#egg=six\n",
+            "--editable=git+https://example.com/six.git#egg=six\n",
+            "git+https://example.com/six.git#egg=six\n",
+            "https://files.pythonhosted.org/packages/six-1.16.0-py2.py3-none-any.whl\n",
+            "../wheels/six-1.16.0-py2.py3-none-any.whl\n",
+        ]
+        .into_iter()
+        .map(|text| (text, text.as_bytes().to_vec()))
+        .collect();
+        shapes.push(("utf-16 six==1.16.0", utf16));
+        for (shape, bytes) in shapes {
+            let tmp = tempfile::tempdir().unwrap();
+            let root = tmp.path().join("app");
+            tokio::fs::create_dir_all(&root).await.unwrap();
+            tokio::fs::write(tmp.path().join("base.txt"), &bytes)
+                .await
+                .unwrap();
+            touch(&root, "pylock.toml", &pylock).await;
+            touch(&root, "requirements.txt", "-r ../base.txt\n").await;
+            let err = detect_pypi_flavor(&root, Some(("six", "1.16.0"))).await;
+            assert_eq!(
+                err.map_err(|(code, _)| code).map(|(flavor, _)| flavor),
+                Err("pypi_requirements_outside_root"),
+                "{shape}"
+            );
+        }
+
+        // An include pip may read but this walk can't decode: its pins are
+        // unknowable, so it refuses too.
+        {
+            let tmp = tempfile::tempdir().unwrap();
+            let root = tmp.path().join("app");
+            tokio::fs::create_dir_all(&root).await.unwrap();
+            tokio::fs::write(tmp.path().join("base.txt"), b"\x80\x81six==1.16.0\n")
+                .await
+                .unwrap();
+            touch(&root, "pylock.toml", &pylock).await;
+            touch(&root, "requirements.txt", "-r ../base.txt\n").await;
+            let (code, detail) = detect_pypi_flavor(&root, Some(("six", "1.16.0")))
+                .await
+                .unwrap_err();
+            assert_eq!(code, "pypi_requirements_outside_root", "{detail}");
+            assert!(detail.contains("cannot be read"), "{detail}");
+        }
+
+        // A bare artifact of another release, or of another package, is
+        // not this pin.
+        for other in [
+            "https://files.pythonhosted.org/packages/six-1.15.0-py2.py3-none-any.whl\n",
+            "../wheels/idna-3.7-py3-none-any.whl\n",
+        ] {
+            let tmp = tempfile::tempdir().unwrap();
+            let root = tmp.path().join("app");
+            tokio::fs::create_dir_all(&root).await.unwrap();
+            touch(tmp.path(), "base.txt", other).await;
+            touch(&root, "pylock.toml", &pylock).await;
+            touch(&root, "requirements.txt", "-r ../base.txt\n").await;
+            assert!(
+                detect_pypi_flavor(&root, Some(("six", "1.16.0")))
+                    .await
+                    .is_ok(),
+                "{other}"
+            );
+        }
+    }
+
     /// A pylock `pipenv lock` writes with `[pipenv] use_pylock = true`.
     const PIPENV_PYLOCK: &str = "lock-version = \"1.0\"\ncreated-by = \"pipenv\"\n\n[[packages]]\nname = \"six\"\nversion = \"1.16.0\"\nindex = \"https://pypi.org/simple\"\nwheels = [{ name = \"six-1.16.0-py2.py3-none-any.whl\", url = \"https://files.pythonhosted.org/packages/six-1.16.0-py2.py3-none-any.whl\", hashes = { sha256 = \"8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254\" } }]\n\n[tool.pipenv]\ngenerated_from = \"Pipfile.lock\"\n";