Fix vendored requirements revert on edited comment (#977)

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

@@ -1787,6 +1787,126 @@ fn pip_requirements_vendor_fresh_checkout_no_index_and_revert() {
     );
 }
 
+/// #977: the `# socket-patch vendor:` comment is bookkeeping pip never
+/// reads. With it edited (respaced, a note appended) or stripped, a re-run
+/// of `vendor` is in sync instead of appending a second line for the same
+/// wheel, and `vendor --revert` still restores the registry pin.
+#[test]
+#[serial_test::serial]
+fn pip_requirements_revert_after_the_vendor_comment_is_edited() {
+    let Some(python) = find_python() else {
+        println!("SKIP e2e_vendor_pypi_build(pip #977): no python3/python on PATH");
+        return;
+    };
+    bake_leak_guards();
+    let tmp = tempfile::tempdir().unwrap();
+    let proj = tmp.path().join("proj");
+    std::fs::create_dir_all(&proj).unwrap();
+    std::fs::write(proj.join("requirements.txt"), "idna==3.7\nsix==1.16.0\n").unwrap();
+    let venv = proj.join(".venv");
+    let mkvenv = tool(Path::new(python), &proj, &["-m", "venv", ".venv"], &[]);
+    assert_tool_ok(&mkvenv, "python -m venv");
+    let install = tool(
+        &venv.join("bin/pip"),
+        &proj,
+        &[
+            "install",
+            "--disable-pip-version-check",
+            "--quiet",
+            "--no-cache-dir",
+            "-r",
+            "requirements.txt",
+        ],
+        &[],
+    );
+    if !install.status.success() {
+        println!(
+            "SKIP e2e_vendor_pypi_build(pip #977): `pip install` failed (PyPI \
+             unreachable?):\n{}",
+            String::from_utf8_lossy(&install.stderr)
+        );
+        return;
+    }
+    stage_patch(&proj, &site_packages(&venv).join("six.py"));
+    let requirements_before = std::fs::read(proj.join("requirements.txt")).unwrap();
+    let blobs = proj.join(".socket/blobs");
+    let blobs_before = tmp.path().join("blobs");
+    copy_dir_recursive(&blobs, &blobs_before);
+
+    let edits: [(&str, fn(&str) -> String); 3] = [
+        ("respaced", |l| {
+            l.replace("  # socket-patch vendor", " # socket-patch vendor")
+        }),
+        ("note appended", |l| format!("{l}  # CVE fix, do not bump")),
+        ("stripped", |l| {
+            l.split("  # socket-patch vendor")
+                .next()
+                .unwrap()
+                .to_string()
+        }),
+    ];
+    for (edit, apply_edit) in edits {
+        if !blobs.exists() {
+            copy_dir_recursive(&blobs_before, &blobs);
+        }
+        let (code, stdout, stderr) = run_vendored(&VendorDriver::VendorOffline, &proj);
+        assert_eq!(
+            code, 0,
+            "{edit}: vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        );
+        let wired = std::fs::read_to_string(proj.join("requirements.txt")).unwrap();
+        let line = wired
+            .lines()
+            .find(|l| l.contains(".socket/vendor/pypi/"))
+            .unwrap_or_else(|| panic!("{edit}: no vendor line:\n{wired}"))
+            .to_string();
+        let edited = wired.replace(&line, &apply_edit(&line));
+        assert_ne!(edited, wired, "{edit}");
+        std::fs::write(proj.join("requirements.txt"), &edited).unwrap();
+
+        // Re-vendor: in sync, no second wheel line.
+        let (code, stdout, stderr) = run_vendored(&VendorDriver::VendorOffline, &proj);
+        assert_eq!(
+            code, 0,
+            "{edit}: re-vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        );
+        let after = std::fs::read_to_string(proj.join("requirements.txt")).unwrap();
+        assert_eq!(
+            after, edited,
+            "{edit}: re-vendor must leave the wired file alone"
+        );
+
+        let (code, stdout, stderr) = run_socket(
+            &proj,
+            &[
+                "vendor",
+                "--revert",
+                "--json",
+                "--cwd",
+                proj.to_str().unwrap(),
+            ],
+        );
+        assert_eq!(
+            code, 0,
+            "{edit}: revert failed.\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        );
+        let renv = parse_envelope(&stdout);
+        assert_eq!(
+            renv["summary"]["removed"], 1,
+            "{edit}: one entry reverted: {renv}"
+        );
+        assert_eq!(
+            std::fs::read(proj.join("requirements.txt")).unwrap(),
+            requirements_before,
+            "{edit}: revert must restore the registry pin"
+        );
+        assert!(
+            !proj.join(".socket/vendor").exists(),
+            "{edit}: .socket/vendor must be removed after revert"
+        );
+    }
+}
+
 #[test]
 #[serial_test::serial]
 fn pip_vendored_requirements_evaluate_environment_markers() {

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

@@ -629,9 +629,9 @@ async fn inventory_pdm_lock(view: &ProjectView<'_>) -> Option<Vec<LockfileEntry>
 ///   `./.socket/vendor/pypi/<uuid>/<wheel> --hash=sha256:…
 ///   # socket-patch vendor: <name>==<ver>`
 ///   (`vendor::pypi_requirements::vendor_line`), whose requirement name
-///   lives ONLY in that comment tag
-///   (`utils::requirements::vendor_tag`, the reader
-///   `vex::discover::pypi_other` already uses).
+///   lives in that comment tag (`utils::requirements::vendor_tag`, the
+///   reader `vex::discover::pypi_other` already uses), or, with the tag
+///   edited away, in the wheel's own file name (#977).
 ///
 /// A user's OWN file/url/path reference is not ours to resolve and stays
 /// out.
@@ -689,22 +689,28 @@ async fn inventory_requirements_txt(view: &ProjectView<'_>) -> Option<Vec<Lockfi
             }
             None => {
                 let Some((raw_name, reference)) = crate::utils::requirements::direct_reference(t)
-                    .and_then(|(n, r)| Some((n, socket_reference_coords(r)?)))
+                    .and_then(|(n, r)| Some((n.to_string(), socket_reference_coords(r)?)))
                     .or_else(|| {
                         // The VENDORED writer's shape: a bare path line
-                        // whose requirement name lives only in the
+                        // whose requirement name lives in the
                         // `socket-patch vendor:` comment tag it appends.
+                        // pip never reads that comment, so a line whose tag
+                        // was edited away or stripped is named by its
+                        // wheel (#977).
                         let coords = socket_reference_coords(t.split_whitespace().next()?)?;
-                        let (tag_name, _) = crate::utils::requirements::vendor_tag(comment?)?;
-                        Some((tag_name, coords))
+                        let name = match comment.and_then(crate::utils::requirements::vendor_tag) {
+                            Some((tag_name, _)) => tag_name.to_string(),
+                            None => coords.0.clone(),
+                        };
+                        Some((name, coords))
                     })
                 else {
                     continue;
                 };
                 // The line's own name must agree with the artifact's:
                 // a reference whose coordinates contradict the requirement
                 // it stands on is not one of ours, whoever wrote it.
-                if canonicalize_pypi_name(raw_name) != reference.0 {
+                if canonicalize_pypi_name(&raw_name) != reference.0 {
                     continue;
                 }
                 reference

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

@@ -3254,6 +3254,37 @@ async fn already_vendored_requirements_lines_stay_in_the_inventory() {
     }
 }
 
+/// #977: pip never reads the `socket-patch vendor:` tag, so a vendored line
+/// whose tag was edited away or stripped still installs the package and is
+/// named by its wheel. A tag naming another package still keeps it out.
+#[tokio::test]
+async fn vendored_requirements_lines_without_their_tag_stay_in_the_inventory() {
+    const UUID: &str = "55555555-5555-5555-5555-555555555555";
+    let tmp = tempfile::tempdir().unwrap();
+    write(
+        tmp.path(),
+        "requirements.txt",
+        &format!(
+            "./.socket/vendor/pypi/{UUID}/requests-2.28.1-py3-none-any.whl \
+             --hash=sha256:{sha}\n\
+             ./.socket/vendor/pypi/{UUID}/six-1.16.0-py2.py3-none-any.whl  # CVE fix\n\
+             ./.socket/vendor/pypi/{UUID}/idna-3.7-py3-none-any.whl  \
+             # socket-patch vendor: urllib3==1.26.18\n",
+            sha = "c".repeat(64),
+        ),
+    )
+    .await;
+    let entries = inventory_pypi_locks(tmp.path()).await.unwrap();
+    assert_eq!(
+        sorted_pairs(&entries),
+        vec![
+            ("requests".to_string(), "2.28.1".to_string()),
+            ("six".to_string(), "1.16.0".to_string()),
+        ],
+        "{entries:?}"
+    );
+}
+
 /// The vendored writer's OWN output must re-inventory: build the line with
 /// the writer's formatter and feed it straight back in.
 #[tokio::test]

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

@@ -879,7 +879,8 @@ impl RevertOutcome {
             "vendor_artifact_kept",
             format!(
                 "kept {uuid_dir_rel}: some recorded lock entries were left alone (see the \
-                 vendor_lock_entry_drifted / vendor_lock_entry_removed warnings) and the vendored artifacts may still be \
+                 vendor_lock_entry_drifted / vendor_lock_entry_removed / \
+                 vendor_revert_line_drifted warnings) and the vendored artifacts may still be \
                  needed for a later restore; undo the drift (restore the vendored lock entries \
                  or re-vendor) and re-run `vendor --revert` to finish cleaning up"
             ),