From 213ae949701e4475dc38af6244efa654d7f063e7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 11 Nov 2026 14:49:03 -0500 Subject: [PATCH 1/2] Roll back a vendored PDM 0.x dependency declared with extras `rollback` refused to undo socket-patch's own vendoring of a PyPI package in a PDM 0.x project that declares it with extras (`urllib3 = {version = "==1.26.18", extras = ["socks"]}`): refusing to revert pkg:pypi/urllib3@1.26.18: for pdm.lock, the ledger record for it would change entries other than urllib3's own, which is not an edit socket-patch makes PDM's lock_version 2 format keeps file hashes in `[metadata.files]`, keyed ` `, and a unit with extras has a key of its own, `[] `. Vendoring rewrites both. The revert's confinement check, new in 5.1.0 (#2052), read a key's package as its first word, so `urllib3[socks]` counted as another package's entry and the restore was refused. Nothing was written: the project stayed vendored until its lock was restored from version control. The check now drops the `[]` suffix before it compares names. Another package's extras key is still refused. Test: confinement_allows_a_pdm_revert_in_every_native_format runs the PDM lock writer over every native lock fixture and holds the check to accepting the revert. It fails on 0.12.3-extras without this change. Found by depscan's PDM patch backtest on the 5.1.0 bump (PDM 0.12.3, extras, vendored). The same cell passes with this build. Co-Authored-By: Claude Fable 5.2 --- .../socket-patch-core/src/vendor/pypi_lock.rs | 70 +++++++++++++++++-- 1 file changed, 65 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index 531158e95..0f6fe7b45 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -1209,19 +1209,27 @@ fn toml_json(item: &Item) -> serde_json::Value { /// of the key `parent`: array elements that name the package (a /// requirement string, a `{ name = … }` entry, a lock unit), its key in a /// `sources` table or in a lock's `files` / `hashes` table (Poetry's and -/// PDM's older formats, the latter keyed ` `), the Hatch +/// PDM's older formats, the latter keyed ` `, or +/// `[] ` for a unit with extras), the Hatch /// direct-reference permission, and then every table and array left /// empty. fn drop_package(value: &mut serde_json::Value, parent: &str, canon: &str) { use serde_json::Value as Json; let names = |name: &str| canonicalize_pypi_name(name) == canon; + // The package a `files` / `hashes` key is for: its first word, less the + // `[]` PDM's older formats append for an extras unit. + let keys = |key: &str| { + let unit = key.split_whitespace().next().unwrap_or_default(); + let name = unit + .strip_suffix(']') + .and_then(|open| open.split_once('[')) + .map_or(unit, |(name, _)| name); + names(name) + }; match value { Json::Object(map) => { let keyed = matches!(parent, "sources" | "files" | "hashes"); - map.retain(|key, _| { - key != "allow-direct-references" - && !(keyed && names(key.split_whitespace().next().unwrap_or_default())) - }); + map.retain(|key, _| key != "allow-direct-references" && !(keyed && keys(key))); for (key, child) in map.iter_mut() { drop_package(child, key, canon); } @@ -3676,6 +3684,58 @@ mod tests { } } + /// A revert of what the PDM writer wrote is the package's own in every + /// native lock format. PDM 0.x keys an extras unit's files + /// `[] `: read as another package's key, it made + /// `rollback` refuse to undo its own vendoring of a dependency declared + /// with extras. + #[test] + fn confinement_allows_a_pdm_revert_in_every_native_format() { + const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; + for fixture in [ + "0.12.3", + "0.12.3-extras", + "2.8.2", + "2.9.3", + "2.10.4", + "2.11.2", + "2.17.3", + "2.29.2", + "2.29.2-extras", + ] { + let original = std::fs::read_to_string( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/pdm-native") + .join(format!("{fixture}.lock")), + ) + .unwrap(); + let vendored = crate::utils::pdm_lock::rewrite_pdm_lock( + &original, + "urllib3", + "1.26.18", + ("path", &format!("./.socket/vendor/pypi/u/{WHEEL}")), + WHEEL, + &"a".repeat(64), + ) + .unwrap(); + assert_ne!(vendored, original, "{fixture}"); + assert_eq!( + confinement_refusal(&vendored, &original, "urllib3"), + None, + "{fixture}" + ); + } + + // Another package's extras key is still not the package's own. + let wired = "[metadata]\ncontent_hash = \"h\"\n\n[metadata.files]\n\"six[socks] 1.16.0\" = [{ file = \"six.whl\", hash = \"sha256:cc\" }]\n\"idna[socks] 3.7\" = [{ file = \"idna.whl\", hash = \"sha256:dd\" }]\n"; + let restored = wired.replace("sha256:cc", "sha256:bb"); + assert_eq!(confinement_refusal(wired, &restored, "six"), None); + assert!( + confinement_refusal(wired, &restored.replace("sha256:dd", "sha256:ee"), "six") + .is_some() + ); + } + /// ...and what it may not: another package, a setting, or anything in a /// file it cannot read as TOML. #[test] From 0baea26923fca499443cf0325615f500bc36da67 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 11 Nov 2026 14:59:45 -0500 Subject: [PATCH 2/2] Read the PDM files key beside the code that writes it Review follow-up for the extras rollback fix. The check's reading of a `[metadata.files]` key is now `pdm_lock::files_key_package`, next to `legacy_files_key`, which builds the key, with a test that every key the writer builds for the PDM 0.x fixtures reads back as its unit's package. The two can no longer drift apart unnoticed, which is how the 5.1.0 refusal shipped. The `[]` suffix is dropped only for `files` / `hashes` keys. A `sources` key is read as before, so `"six[socks]"` under `[tool.uv.sources]` is not six's own entry. Tests: revert_round_trip_restores_a_legacy_lock_with_extras vendors and reverts urllib3 in the native PDM 0.12.3 extras lock through wire_pdm and revert_pdm, the path that failed. Without the fix it fails with the refusal users saw. The confinement sweep takes its fixture list from pdm_lock's tests instead of a copy. Co-Authored-By: Claude Fable 5.2 --- .../socket-patch-core/src/utils/pdm_lock.rs | 67 +++++++++++++++---- .../socket-patch-core/src/vendor/pypi_lock.rs | 43 ++++-------- .../socket-patch-core/src/vendor/pypi_pdm.rs | 60 +++++++++++++++++ 3 files changed, 128 insertions(+), 42 deletions(-) diff --git a/crates/socket-patch-core/src/utils/pdm_lock.rs b/crates/socket-patch-core/src/utils/pdm_lock.rs index f063d8448..db6526f41 100644 --- a/crates/socket-patch-core/src/utils/pdm_lock.rs +++ b/crates/socket-patch-core/src/utils/pdm_lock.rs @@ -114,6 +114,16 @@ pub fn legacy_files_key(package: &Table) -> Option { Some(format!("{name}{suffix} {version}")) } +/// The package a `[metadata.files]` key is for: [`legacy_files_key`] read +/// back (` `, or `[] ` for a unit with +/// extras). A key that is a bare name, as Poetry's are, is its own answer. +pub fn files_key_package(key: &str) -> &str { + let unit = key.split_whitespace().next().unwrap_or_default(); + unit.strip_suffix(']') + .and_then(|open| open.split_once('[')) + .map_or(unit, |(name, _)| name) +} + pub fn files_for<'a>(lock: &'a Table, package: &'a Table) -> Option<&'a Array> { package.get("files").and_then(Item::as_array).or_else(|| { lock.get("metadata")? @@ -497,7 +507,20 @@ pub(crate) mod tests { const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; const PATH: &str = "./.socket/vendor/pypi/e828efa5-5c6d-43f3-9909-03f5ac232b98/urllib3-1.26.18-py2.py3-none-any.whl"; - fn fixture(name: &str) -> String { + /// The native locks the rewriter takes (tests/fixtures/pdm-native). + pub(crate) const REWRITABLE: [&str; 9] = [ + "0.12.3", + "2.8.2", + "2.9.3", + "2.10.4", + "2.11.2", + "2.17.3", + "2.29.2", + "0.12.3-extras", + "2.29.2-extras", + ]; + + pub(crate) fn fixture(name: &str) -> String { std::fs::read_to_string( std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .join("tests/fixtures/pdm-native") @@ -508,17 +531,7 @@ pub(crate) mod tests { #[test] fn native_formats_rewrite_and_reverse_byte_exactly() { - for version in [ - "0.12.3", - "2.8.2", - "2.9.3", - "2.10.4", - "2.11.2", - "2.17.3", - "2.29.2", - "0.12.3-extras", - "2.29.2-extras", - ] { + for version in REWRITABLE { for ending in ["\n", "\r\n"] { let original = fixture(version).replace('\n', ending); for (kind, source) in [("path", PATH), ("path", &PATH.replace('/', "\\")), ("url", "https://patch.socket.dev/patch/pypi/urllib3/1.26.18/7e52b8b6-53f2-4dc8-860a-1ae7ebd8be0e/e828efa5-5c6d-43f3-9909-03f5ac232b98/urllib3-1.26.18-py2.py3-none-any.whl")] { @@ -545,6 +558,36 @@ pub(crate) mod tests { } } + /// The key the writer builds for a unit reads back as the unit's + /// package, with extras or without: the revert's confinement check + /// depends on it. + #[test] + fn files_key_package_reads_back_every_legacy_key() { + for version in ["0.12.3", "0.12.3-extras"] { + let lock: DocumentMut = fixture(version).parse().unwrap(); + let packages = lock["package"].as_array_of_tables().unwrap(); + assert!(!packages.is_empty(), "{version}"); + for package in packages { + let key = legacy_files_key(package).unwrap(); + assert!(lock["metadata"]["files"].get(&key).is_some(), "{key}"); + assert_eq!( + files_key_package(&key), + pdm_files_key_name(package["name"].as_str().unwrap()), + "{key}" + ); + } + } + for (key, package) in [ + ("six", "six"), + ("zope.interface 5.0", "zope.interface"), + ("urllib3[brotli,socks] 1.26.18", "urllib3"), + ("odd] 1.0", "odd]"), + ("", ""), + ] { + assert_eq!(files_key_package(key), package, "{key:?}"); + } + } + #[test] fn native_installers_that_lose_source_identity_refuse() { for version in ["1.15.5", "2.0.3", "2.1.5", "2.3.4", "2.6.1", "2.7.4"] { diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index 0f6fe7b45..9e7acffb0 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -1216,20 +1216,14 @@ fn toml_json(item: &Item) -> serde_json::Value { fn drop_package(value: &mut serde_json::Value, parent: &str, canon: &str) { use serde_json::Value as Json; let names = |name: &str| canonicalize_pypi_name(name) == canon; - // The package a `files` / `hashes` key is for: its first word, less the - // `[]` PDM's older formats append for an extras unit. - let keys = |key: &str| { - let unit = key.split_whitespace().next().unwrap_or_default(); - let name = unit - .strip_suffix(']') - .and_then(|open| open.split_once('[')) - .map_or(unit, |(name, _)| name); - names(name) - }; match value { Json::Object(map) => { - let keyed = matches!(parent, "sources" | "files" | "hashes"); - map.retain(|key, _| key != "allow-direct-references" && !(keyed && keys(key))); + let own = |key: &str| match parent { + "sources" => names(key.split_whitespace().next().unwrap_or_default()), + "files" | "hashes" => names(crate::utils::pdm_lock::files_key_package(key)), + _ => false, + }; + map.retain(|key, _| key != "allow-direct-references" && !own(key)); for (key, child) in map.iter_mut() { drop_package(child, key, canon); } @@ -3691,24 +3685,10 @@ mod tests { /// with extras. #[test] fn confinement_allows_a_pdm_revert_in_every_native_format() { + use crate::utils::pdm_lock::tests::{fixture as native_lock, REWRITABLE}; const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; - for fixture in [ - "0.12.3", - "0.12.3-extras", - "2.8.2", - "2.9.3", - "2.10.4", - "2.11.2", - "2.17.3", - "2.29.2", - "2.29.2-extras", - ] { - let original = std::fs::read_to_string( - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures/pdm-native") - .join(format!("{fixture}.lock")), - ) - .unwrap(); + for fixture in REWRITABLE { + let original = native_lock(fixture); let vendored = crate::utils::pdm_lock::rewrite_pdm_lock( &original, "urllib3", @@ -3726,7 +3706,8 @@ mod tests { ); } - // Another package's extras key is still not the package's own. + // Another package's extras key is still not the package's own, and + // a `sources` key is a name: nothing is read off the end of one. let wired = "[metadata]\ncontent_hash = \"h\"\n\n[metadata.files]\n\"six[socks] 1.16.0\" = [{ file = \"six.whl\", hash = \"sha256:cc\" }]\n\"idna[socks] 3.7\" = [{ file = \"idna.whl\", hash = \"sha256:dd\" }]\n"; let restored = wired.replace("sha256:cc", "sha256:bb"); assert_eq!(confinement_refusal(wired, &restored, "six"), None); @@ -3734,6 +3715,8 @@ mod tests { confinement_refusal(wired, &restored.replace("sha256:dd", "sha256:ee"), "six") .is_some() ); + let sourced = "[tool.uv.sources]\n\"six[socks]\" = { path = \"a\" }\n"; + assert!(confinement_refusal(sourced, &sourced.replace("\"a\"", "\"b\""), "six").is_some()); } /// ...and what it may not: another package, a setting, or anything in a diff --git a/crates/socket-patch-core/src/vendor/pypi_pdm.rs b/crates/socket-patch-core/src/vendor/pypi_pdm.rs index 76e90a797..8c6f6309d 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pdm.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pdm.rs @@ -1328,6 +1328,66 @@ distribution = false } } + /// PDM 0.x keeps an extras unit's file hashes under a key of its own, + /// `[] `. Vendoring rewrites both keys and the + /// revert takes both back (5.1.0 refused it as a change to another + /// package's entry). + #[tokio::test] + async fn revert_round_trip_restores_a_legacy_lock_with_extras() { + const WHEEL: &str = "urllib3-1.26.18-py2.py3-none-any.whl"; + let rel_wheel = format!(".socket/vendor/pypi/{UUID}/{WHEEL}"); + let before = crate::utils::pdm_lock::tests::fixture("0.12.3-extras"); + let tmp = write_project( + &before, + "[tool.pdm]\nname = \"legacy-extras\"\nversion = \"0.0.0\"\npython_requires = \">=3.8\"\n\n[tool.pdm.dependencies]\nurllib3 = {version = \"==1.26.18\", extras = [\"socks\"]}\n", + ) + .await; + let p = load_pdm_project(tmp.path()).await.unwrap(); + let (wiring, meta) = wire_pdm( + &p, + tmp.path(), + "urllib3", + "1.26.18", + &rel_wheel, + WHEEL, + WHEEL_SHA, + UUID, + &[WHEEL_SHA], + ) + .await + .unwrap(); + let vendored = read_lock(tmp.path()).await; + assert_eq!(vendored.matches(&rel_wheel).count(), 2, "{vendored}"); + assert_eq!( + vendored.matches(&format!("sha256:{WHEEL_SHA}")).count(), + 2, + "{vendored}" + ); + let entry = VendorEntry { + flavor: Some("pdm".into()), + pdm: Some(meta), + ..VendorEntry::new( + "pypi".into(), + "pkg:pypi/urllib3@1.26.18".into(), + UUID.into(), + VendorArtifact { + yarn_berry10c0: None, + path: rel_wheel.clone(), + sha256: WHEEL_SHA.into(), + size: Some(11053), + platform_locked: None, + file_inventory: None, + }, + wiring, + ) + }; + + let outcome = revert_pdm(&entry, tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + assert_eq!(read_lock(tmp.path()).await, before, "byte-identical revert"); + } + /// The lock file is user-owned: wiring the splice must not reset its /// permission bits (see `atomic_write_bytes_preserving_mode`; the revert /// leg is covered in common.rs).