Fix Poetry lock writer read three ways (#1847)

crates/socket-patch-core/src/patch/redirect/poetry.rs

@@ -9,39 +9,11 @@ use serde_json::Value;
 use super::{DepOverride, FileEdit, RewriteResult, RewriteWarning};
 use crate::crawlers::python_crawler::canonicalize_pypi_name;
 use crate::utils::poetry_lock::{
-    generated_by_version, lock_version, lock_wiring, pyproject_source_constraints,
-    rewrite_poetry_lock_all_with_version, source_constraint_advisory, source_constraint_break,
-    source_constraint_refusal, LockBatch, LockStep, PoetryLockDep,
+    lock_wiring, lock_writer, pyproject_source_constraints, rewrite_poetry_lock_all_with_version,
+    source_constraint_advisory, source_constraint_break, source_constraint_refusal, LockBatch,
+    LockStep, LockWriter, PoetryLockDep,
 };
 
-/// Whether the lock was written by a Poetry release older than 1.4. Those
-/// installers neither verify hashes for local wheels nor replace an
-/// already-installed package at the same version, so a warm virtualenv keeps
-/// serving the upstream bytes after the redirect. Formats `0`/`1.0`/`1.1` are
-/// only written by such releases; lock `2.0` is written by 1.3 through 1.8, so
-/// the `@generated by Poetry X.Y.Z` header (present from 1.4) decides there.
-///
-/// `version` is the lock's `lock_version` when the caller already has it;
-/// otherwise the lock is parsed for it.
-fn pre_1_4_writer(lock_text: &str, version: Option<&str>) -> Option<&'static str> {
-    let parsed;
-    let version = match version {
-        Some(version) => version,
-        None => {
-            // The batch already parsed (and shared) these bytes.
-            parsed = crate::utils::lock_fragments::parse_lock(lock_text).ok()?;
-            lock_version(&parsed).ok()?
-        }
-    };
-    match version {
-        "0" => Some("0"),
-        "1.0" => Some("1.0"),
-        "1.1" => Some("1.1"),
-        "2.0" if !matches!(generated_by_version(lock_text), Some(v) if v >= (1, 4)) => Some("2.0"),
-        _ => None,
-    }
-}
-
 pub(super) fn rewrite_poetry(
     files: &BTreeMap<String, String>,
     overrides: &[DepOverride],
@@ -139,10 +111,10 @@ pub(super) fn rewrite_poetry(
             batch_lock_version,
         ) = rewrite_poetry_lock_all_with_version(original, &deps);
         let mut stale_warned = false;
-        // `pre_1_4_writer` reads only the first line and `[metadata]
+        // `lock_writer` reads only the first line and `[metadata]
         // lock-version`, which no package rewrite touches: judged once, from
         // the batch's parse when it has one.
-        let mut writer_format: Option<Option<&'static str>> = None;
+        let mut stale_writer: Option<Option<LockWriter>> = None;
         for (&(dep, _), step) in usable.iter().zip(steps) {
             match step {
                 LockStep::Rewritten(edits) => {
@@ -171,16 +143,17 @@ pub(super) fn rewrite_poetry(
                         });
                     }
                     if !stale_warned {
-                        if let Some(format) = *writer_format.get_or_insert_with(|| {
-                            pre_1_4_writer(&content, batch_lock_version.as_deref())
+                        if let Some(writer) = *stale_writer.get_or_insert_with(|| {
+                            lock_writer(&content, batch_lock_version.as_deref())
+                                .filter(|writer| writer.is_pre_1_4())
                         }) {
                             stale_warned = true;
                             // Poetry 1.0 installs url sources through pip, which reads
                             // the hash from the URL fragment; pip 22.3–23.0 take the
                             // rest of the fragment (`&#egg=<name>`, appended by Poetry)
                             // as part of the digest and refuse the install (measured;
                             // pip <= 22.2 and >= 23.1 install and verify).
-                            let pip_note = if format == "1.0" {
+                            let pip_note = if writer == LockWriter::Format("1.0") {
                                 " Poetry 1.0 installs through pip: pip 22.3–23.0 misparse the \
                                  hash fragment and refuse the install (fail-closed) — use pip \
                                  <= 22.2 or >= 23.1 in the virtualenv."

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

@@ -179,7 +179,7 @@ fn is_2x(version: &str) -> bool {
 /// Advisory only — a lock can be consumed by a different Poetry than the one
 /// that wrote it — but it lets warnings about the WRITER's installer be
 /// precise instead of blaming every lock-2.0 project for Poetry 1.3.
-pub fn generated_by_version(lock_text: &str) -> Option<(u64, u64)> {
+pub(crate) fn generated_by_version(lock_text: &str) -> Option<(u64, u64)> {
     let first = lock_text.lines().next()?;
     let rest = first.split("@generated by Poetry ").nth(1)?;
     let version = rest.split_whitespace().next()?;
@@ -360,19 +360,62 @@ pub fn source_constraint_advisory(
     )
 }
 
-/// The `(major, minor)` of the Poetry that wrote the lock, when the lock
-/// says: the version in the `@generated by Poetry X.Y` stamp (1.4 on), else
-/// 1.3 for `lock-version = "2.0"`. Only 1.3 – 1.8 write lock 2.0, and 1.4 –
-/// 1.8 stamp their version, so an unversioned 2.0 lock is 1.3's (it stamps
-/// the sentence without a version, #1447) or one whose stamp was edited
-/// away; both read as the oldest writer of that format, as
-/// `pre_1_4_writer` reads them. `None` for older formats.
-pub fn lock_writer_version(lock_text: &str) -> Option<(u64, u64)> {
-    if let Some(version) = generated_by_version(lock_text) {
-        return Some(version);
+/// Who wrote a `poetry.lock`, as far as the lock itself says: see
+/// [`lock_writer`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum LockWriter {
+    /// The `(major, minor)` of the Poetry that wrote it.
+    Poetry(u64, u64),
+    /// A lock in a format only Poetry < 1.3 writes, stamped or not: `"0"`
+    /// (the legacy `[metadata.hashes]` layout), `"1.0"` or `"1.1"`.
+    Format(&'static str),
+}
+
+impl LockWriter {
+    /// Whether that Poetry is older than 1.4. Those installers neither
+    /// verify hashes for local wheels nor replace an already-installed
+    /// package at the same version, so a warm virtualenv keeps serving the
+    /// upstream bytes after the lock is rewired.
+    pub fn is_pre_1_4(self) -> bool {
+        match self {
+            LockWriter::Poetry(major, minor) => (major, minor) < (1, 4),
+            LockWriter::Format(_) => true,
+        }
     }
-    let lock = lock_text.parse::<DocumentMut>().ok()?;
-    (lock_version(&lock).ok()? == "2.0").then_some((1, 3))
+}
+
+/// The Poetry that wrote the lock, when the lock says. A format older than
+/// 2.0 decides first: only Poetry < 1.3 write formats 0, 1.0 and 1.1, so
+/// they read as [`LockWriter::Format`] whatever the first line claims (a
+/// stamp is advisory, and a hand-merged header must not hide an old lock).
+/// Otherwise the version in the `@generated by Poetry X.Y` stamp (1.4 on)
+/// decides. An unstamped `lock-version = "2.0"` reads as 1.3: only 1.3 –
+/// 1.8 write that format, and 1.4 – 1.8 stamp their version, so it is
+/// 1.3's (it stamps the sentence without a version, #1447) or one whose
+/// stamp was edited away, read as the oldest writer of that format. An
+/// unstamped 2.1+ lock (or an unreadable one) has no known writer.
+///
+/// `known_format` is the lock's [`lock_version`] when the caller already
+/// has it; otherwise the lock is parsed for it.
+pub fn lock_writer(lock_text: &str, known_format: Option<&str>) -> Option<LockWriter> {
+    let parsed;
+    let format = match known_format {
+        Some(format) => Some(format),
+        None => {
+            parsed = crate::utils::lock_fragments::parse_lock(lock_text).ok();
+            parsed.as_ref().and_then(|lock| lock_version(lock).ok())
+        }
+    };
+    match format {
+        Some("0") => return Some(LockWriter::Format("0")),
+        Some("1.0") => return Some(LockWriter::Format("1.0")),
+        Some("1.1") => return Some(LockWriter::Format("1.1")),
+        _ => {}
+    }
+    if let Some((major, minor)) = generated_by_version(lock_text) {
+        return Some(LockWriter::Poetry(major, minor));
+    }
+    (format == Some("2.0")).then_some(LockWriter::Poetry(1, 3))
 }
 
 /// Why a lock can't source a pyproject-source-pinned package from a URL or
@@ -394,27 +437,22 @@ pub enum SourcePinBreak {
 /// Whether a lock that sources a pyproject `source = "…"`-pinned package
 /// from a URL or file breaks `poetry install` ("Repository \"pypi\" does
 /// not exist."; 1.3 and 1.4 report a solver failure), and why: its writer
-/// is Poetry 1.3 – 2.2 ([`lock_writer_version`]), or its format is older
+/// is Poetry 1.3 – 2.2 ([`lock_writer`]), or its format is older
 /// than 2.1. Poetry 2.3+'s own locks install it. The writer is usually but
 /// not always the installer, so a 2.1 lock is admitted (with the
 /// [`source_constraint_advisory`]); a 1.0 / 1.1 lock is refused whichever
 /// Poetry installs it, since only Poetry 1.0 – 1.2 would.
 ///
-/// A versioned stamp decides without parsing (only 1.4+ write one, and
-/// none of them writes a format older than 2.0); otherwise the lock is
-/// parsed once for its `lock-version`, which also names Poetry 1.3 (the
-/// only unstamped writer of 2.0, as [`lock_writer_version`] reads it).
+/// The lock is parsed once for its `lock-version`; a versioned stamp then
+/// names the writer of a 2.x lock, and an unstamped 2.0 lock is Poetry
+/// 1.3's (as [`lock_writer`] reads it).
 pub fn source_constraint_break(lock_text: &str) -> Option<SourcePinBreak> {
-    if let Some(writer) = generated_by_version(lock_text) {
-        return ((1, 3)..(2, 3))
-            .contains(&writer)
-            .then_some(SourcePinBreak::Writer);
-    }
-    let lock = lock_text.parse::<DocumentMut>().ok()?;
-    match lock_version(&lock).ok()? {
-        "2.0" => Some(SourcePinBreak::Writer),
-        format @ ("1.0" | "1.1") => Some(SourcePinBreak::Format(format.into())),
-        _ => None,
+    match lock_writer(lock_text, None)? {
+        LockWriter::Poetry(major, minor) => ((1, 3)..(2, 3))
+            .contains(&(major, minor))
+            .then_some(SourcePinBreak::Writer),
+        LockWriter::Format(format @ ("1.0" | "1.1")) => Some(SourcePinBreak::Format(format.into())),
+        LockWriter::Format(_) => None,
     }
 }
 
@@ -991,6 +1029,8 @@ grouped = {version = "*", source = "pypi"}
         }
         let versionless = "# This file is automatically @generated by Poetry and should not be \
                            changed by hand.\n\n";
+        let stamped_1_4 = "# This file is automatically @generated by Poetry 1.4.0 and should not \
+                           be changed by hand.\n\n";
         let lock_body = |lock_version: &str| {
             format!("[metadata]\nlock-version = \"{lock_version}\"\npython-versions = \"*\"\ncontent-hash = \"x\"\n")
         };
@@ -1000,6 +1040,9 @@ grouped = {version = "*", source = "pypi"}
             ("", "2.0", Some(Writer)),
             ("# a comment above the stamp\n", "2.0", Some(Writer)),
             (versionless, "1.1", format("1.1")),
+            // A 1.4+ stamp on a pre-2.1 format doesn't hide it.
+            (stamped_1_4, "1.1", format("1.1")),
+            (stamped_1_4, "1.0", format("1.0")),
             ("", "1.1", format("1.1")),
             ("", "1.0", format("1.0")),
             (versionless, "2.1", None),
@@ -1435,6 +1478,83 @@ grouped = {version = "*", source = "pypi"}
         assert_eq!(generated_by_version(&fixture("1.2.2")), None);
     }
 
+    /// One reader answers "who wrote this lock" for the hosted and vendored
+    /// pre-1.4 advisories and the source-pin refusal (#1847): a
+    /// pre-2.0 format first, then the stamp, else 1.3 for an unstamped 2.0
+    /// lock.
+    #[test]
+    fn lock_writer_reads_old_formats_then_the_stamp() {
+        use LockWriter::{Format, Poetry};
+        for (version, want) in [
+            ("0.12.17", Some(Format("0"))),
+            ("1.0.10", Some(Format("1.0"))),
+            ("1.1.15", Some(Format("1.1"))),
+            ("1.2.2", Some(Format("1.1"))),
+            ("1.3.2", Some(Poetry(1, 3))),
+            ("1.4.2", Some(Poetry(1, 4))),
+            ("1.5.1", Some(Poetry(1, 5))),
+            ("1.6.1", Some(Poetry(1, 6))),
+            ("1.7.1", Some(Poetry(1, 7))),
+            ("1.8.5", Some(Poetry(1, 8))),
+            ("2.0.1", Some(Poetry(2, 0))),
+            ("2.1.4", Some(Poetry(2, 1))),
+            ("2.2.1", Some(Poetry(2, 2))),
+            ("2.3.4", Some(Poetry(2, 3))),
+            ("2.4.3", Some(Poetry(2, 4))),
+        ] {
+            let lock = fixture(version);
+            assert_eq!(lock_writer(&lock, None), want, "{version}");
+            let format = lock_version(&crate::utils::lock_fragments::parse_lock(&lock).unwrap())
+                .unwrap()
+                .to_owned();
+            assert_eq!(lock_writer(&lock, Some(&format)), want, "{version}");
+            let pre_1_4 = matches!(version, "0.12.17" | "1.0.10" | "1.1.15" | "1.2.2" | "1.3.2");
+            assert_eq!(want.unwrap().is_pre_1_4(), pre_1_4, "{version}");
+        }
+
+        let stamp = |v: &str| {
+            format!("# This file is automatically @generated by Poetry {v} and should not be changed by hand.\n")
+        };
+        let body = |format: &str| format!("[metadata]\nlock-version = \"{format}\"\n");
+        for (lock, want) in [
+            // An unstamped (or versionless-stamped) 2.0 lock is 1.3's.
+            (body("2.0"), Some(Poetry(1, 3))),
+            (format!("{}{}", stamp(""), body("2.0")), Some(Poetry(1, 3))),
+            // A stamp decides without the format.
+            (stamp("1.4.0"), Some(Poetry(1, 4))),
+            (stamp("2.3.0"), Some(Poetry(2, 3))),
+            (
+                format!("{}{}", stamp("2.3.0"), body("2.1")),
+                Some(Poetry(2, 3)),
+            ),
+            // A pre-2.0 format decides over a (hand-merged) 1.4+ stamp.
+            (
+                format!("{}{}", stamp("1.4.0"), body("1.1")),
+                Some(Format("1.1")),
+            ),
+            (
+                format!("{}{}", stamp("2.2.1"), body("1.0")),
+                Some(Format("1.0")),
+            ),
+            // An unstamped 2.1+ lock, or an unreadable one, has no known
+            // writer.
+            (body("2.1"), None),
+            (body("2.2"), None),
+            ("[[package]]\n".to_owned(), None),
+            ("not = [toml".to_owned(), None),
+        ] {
+            assert_eq!(lock_writer(&lock, None), want, "{lock:?}");
+            assert_eq!(
+                lock_writer(&lock.replace('\n', "\r\n"), None),
+                want,
+                "CRLF {lock:?}"
+            );
+        }
+        assert!(Poetry(1, 3).is_pre_1_4());
+        assert!(!Poetry(1, 4).is_pre_1_4());
+        assert!(!Poetry(2, 3).is_pre_1_4());
+    }
+
     /// The edits a rewrite hands back are exactly `poetry_lock_edits` of its
     /// input and output — whether reused from the rewrite or re-derived.
     #[test]

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

@@ -125,17 +125,9 @@ pub(super) async fn load_poetry_project(
     }
     // Poetry < 1.4 neither verifies local wheel hashes nor replaces an
     // already-installed package at the same version (a warm virtualenv keeps
-    // the upstream bytes after the lock is rewired). Formats 0/1.0/1.1 are
-    // only written by those releases; lock 2.0 is written by 1.3 through 1.8,
-    // so the `@generated by Poetry X.Y.Z` header (present from 1.4) decides.
-    let pre_1_4_writer = match lock_version.as_str() {
-        "0" | "1.0" | "1.1" => true,
-        "2.0" => !matches!(
-            crate::utils::poetry_lock::generated_by_version(&lock_text),
-            Some(v) if v >= (1, 4)
-        ),
-        _ => false,
-    };
+    // the upstream bytes after the lock is rewired).
+    let pre_1_4_writer = poetry_lock::lock_writer(&lock_text, Some(&lock_version))
+        .is_some_and(|writer| writer.is_pre_1_4());
     if pre_1_4_writer {
         warnings.push(VendorWarning::new(
             "pypi_poetry_integrity_unverified",