From bf7dd6d621dc826a3d00f9aca547d7cf38492acb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 03:22:24 +0000 Subject: [PATCH 1/4] Start fix for #1608 Assisted-by: Claude Code:claude-opus-5-5 From 696a49c9466549006a1d818c512701c7a61cb712 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 03:32:32 +0000 Subject: [PATCH 2/4] Show ledger keys plainly in revert warnings When a vendored PyPI project's wiring had drifted, `vendor --revert`, `rollback` and `remove` named the ledger entry in Rust debug form, in both human output and `--json` warning text: Pipfile.lock entry for Some("default:six") changed since vendoring The key is now printed as recorded (`default:six`, `requirements.txt:1`), or as `` for a ledger record without one. This covers the Pipenv, requirements.txt, uv and Poetry/PDM lock warnings (#1608). Warning codes and keep/restore behavior are unchanged. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vendor/common.rs | 4 +-- .../src/vendor/pypi_pipenv.rs | 30 ++++++++++++++----- .../src/vendor/pypi_poetry.rs | 8 +++++ .../src/vendor/pypi_requirements.rs | 12 ++++++-- .../socket-patch-core/src/vendor/pypi_uv.rs | 19 ++++++++---- crates/socket-patch-core/src/vendor/state.rs | 25 ++++++++++++++++ 6 files changed, 82 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/common.rs b/crates/socket-patch-core/src/vendor/common.rs index e770af59b..7490ddf4d 100644 --- a/crates/socket-patch-core/src/vendor/common.rs +++ b/crates/socket-patch-core/src/vendor/common.rs @@ -823,8 +823,8 @@ async fn revert_lock_fragment_splice_inner( warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( - "{lock_file} fragment for {:?} changed since vendoring; left untouched", - rec.key + "{lock_file} fragment for {} changed since vendoring; left untouched", + rec.key_label() ), )); } diff --git a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs index fd53b1685..23261cba6 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs @@ -587,8 +587,8 @@ pub(super) async fn revert_pipenv( VendorWarning::new( "vendor_lock_entry_drifted", format!( - "{LOCK_FILE} entry for {:?} changed since vendoring; left untouched", - rec.key + "{LOCK_FILE} entry for {} changed since vendoring; left untouched", + rec.key_label() ), ) }; @@ -625,7 +625,7 @@ pub(super) async fn revert_pipenv( { VendorWarning::new( "vendor_lock_entry_relocked", - format!("{LOCK_FILE} entry for {:?} was removed by a relock; the vendored reference is already gone, so the record is retired", rec.key), + format!("{LOCK_FILE} entry for {} was removed by a relock; the vendored reference is already gone, so the record is retired", rec.key_label()), ) } else { drifted() @@ -683,8 +683,8 @@ pub(super) async fn revert_pipenv( warnings.push(VendorWarning::new( "vendor_lock_entry_relocked", format!( - "{LOCK_FILE} entry for {:?} was regenerated to a registry reference by a relock; the vendored reference is already gone, so the record is retired", - rec.key + "{LOCK_FILE} entry for {} was regenerated to a registry reference by a relock; the vendored reference is already gone, so the record is retired", + rec.key_label() ), )); continue; @@ -1741,6 +1741,14 @@ mod tests { "drifted entry + unknown kind: {:?}", outcome.warnings ); + // The ledger key reads as recorded, never as `Some("…")` (#1608). + assert!( + outcome.warnings.iter().any(|w| w + .detail + .contains("Pipfile.lock entry for default:six changed since vendoring")), + "{:?}", + outcome.warnings + ); assert_eq!( read_lock(tmp.path()).await, drifted, @@ -2013,7 +2021,9 @@ mod tests { }; assert_eq!(outcome.warnings[0].code, expected, "{label}"); assert!( - outcome.warnings[0].detail.contains(key), + outcome.warnings[0] + .detail + .contains(&format!("entry for {key} ")), "{label}: {}", outcome.warnings[0].detail ); @@ -2062,7 +2072,13 @@ mod tests { assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings); assert_eq!(outcome.warnings[0].code, "vendor_lock_entry_relocked"); - assert!(outcome.warnings[0].detail.contains("docs:six")); + assert!( + outcome.warnings[0] + .detail + .contains("entry for docs:six was removed by a relock"), + "{}", + outcome.warnings[0].detail + ); assert_eq!(read_lock(tmp.path()).await, relocked_text); // The same record while another category still routes through the diff --git a/crates/socket-patch-core/src/vendor/pypi_poetry.rs b/crates/socket-patch-core/src/vendor/pypi_poetry.rs index eb02cbd3f..373fd8323 100644 --- a/crates/socket-patch-core/src/vendor/pypi_poetry.rs +++ b/crates/socket-patch-core/src/vendor/pypi_poetry.rs @@ -2077,6 +2077,14 @@ content-hash = "4b42a89b7ff7b26511b06acdc458dbd85312e5083db8f212b017482bc68cdd01 "drifted fragment + unknown kind: {:?}", outcome.warnings ); + // The ledger key reads as recorded, never as `Some("…")` (#1608). + assert!( + outcome.warnings.iter().any(|w| w + .detail + .contains("poetry.lock fragment for six changed since vendoring")), + "{:?}", + outcome.warnings + ); assert_eq!( read_lock(tmp.path()).await, drifted, diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index d14637d65..d52a78e68 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -589,8 +589,8 @@ fn drift_warning(file: &str, rec: &WiringRecord) -> VendorWarning { VendorWarning::new( "vendor_revert_line_drifted", format!( - "{file}: the vendor line for {:?} changed since vendoring; left untouched", - rec.key + "{file}: the vendor line for {} changed since vendoring; left untouched", + rec.key_label() ), ) } @@ -2354,6 +2354,14 @@ mod tests { .warnings .iter() .any(|w| w.code == "vendor_revert_line_drifted")); + // The ledger key reads as recorded, never as `Some("…")` (#1608). + assert!( + outcome.warnings.iter().any(|w| w.detail.contains( + "requirements.txt: the vendor line for requirements.txt:1 changed since vendoring" + )), + "{:?}", + outcome.warnings + ); // A drifted edit (still referencing the uuid dir) also raises the // residual-reference warning. assert!(outcome diff --git a/crates/socket-patch-core/src/vendor/pypi_uv.rs b/crates/socket-patch-core/src/vendor/pypi_uv.rs index d1e4bf9df..07574a8b2 100644 --- a/crates/socket-patch-core/src/vendor/pypi_uv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_uv.rs @@ -946,9 +946,10 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - VendorWarning::new( super::LOCK_ENTRY_REMOVED_CODE, format!( - "{} entry for {:?} no longer exists and nothing else references {needle} \ + "{} entry for {} no longer exists and nothing else references {needle} \ (the dependency was removed); nothing to restore", - rec.kind, rec.key + rec.kind, + rec.key_label() ), ) }) @@ -973,7 +974,7 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( - "uv.lock entry for {key:?} can't be restored to match \ + "uv.lock entry for {key} can't be restored to match \ pyproject.toml ({reason}); left untouched" ), )); @@ -988,8 +989,8 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - VendorWarning::new( "vendor_lock_entry_drifted", format!( - "{what} fragment for {:?} changed since vendoring; left untouched", - rec.key + "{what} fragment for {} changed since vendoring; left untouched", + rec.key_label() ), ) }; @@ -5253,6 +5254,14 @@ wheels = [ "{}", outcome.warnings[0].detail ); + // The ledger key reads as recorded, never as `Some("…")` (#1608). + assert!( + outcome.warnings[0] + .detail + .contains("fragment for six changed since vendoring"), + "{}", + outcome.warnings[0].detail + ); let (py, lock) = read_pair(tmp.path()).await; assert_eq!(py, tampered, "the drifted pyproject is left alone"); assert_ne!( diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index 0c13d0a3d..cffd3e3bd 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -151,6 +151,14 @@ pub struct WiringRecord { pub new: Option, } +impl WiringRecord { + /// The key as user-facing warning text: verbatim, or `` for a + /// record without one (an older or hand-written ledger). + pub fn key_label(&self) -> &str { + self.key.as_deref().unwrap_or("") + } +} + /// Original Cargo.lock fields removed by the path-dep surgery; not /// recomputable offline (the checksum is the sha256 of the registry `.crate` /// tarball, not of the extracted tree). @@ -1077,6 +1085,23 @@ mod tests { const UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; + /// Warnings name the ledger key as recorded, never in debug form + /// (`Some("default:six")`, #1608). + #[test] + fn key_label_is_plain_text() { + let mut rec = WiringRecord { + file: "Pipfile.lock".into(), + kind: "pipenv_lock_entry".into(), + action: WiringAction::Rewritten, + key: Some("default:six".into()), + original: None, + new: None, + }; + assert_eq!(rec.key_label(), "default:six"); + rec.key = None; + assert_eq!(rec.key_label(), ""); + } + fn sample_entry() -> VendorEntry { VendorEntry::new( "npm".into(), From 42e2a5c523bf8e5becb6f6c7098d1a52453fa827 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 03:47:18 +0000 Subject: [PATCH 3/4] Escape untrusted keys and flavors in warnings Follow-up from /code-review on #1608: - A ledger key with control characters is escaped instead of reaching the terminal raw, since state.json is committed and can be tampered with. An empty key reads as `` like a missing one. - A ledger with an unknown PyPI flavor failed with `unknown pypi vendor flavor Some("frobnicate")`; it now names the flavor plainly. - The uv `uv add` drift warning no longer quotes the package name. - The Composer, NuGet, Maven, gem and JVM warnings use the same `key_label()` instead of their own fallbacks. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/composer_lock.rs | 2 +- crates/socket-patch-core/src/vendor/gem.rs | 2 +- crates/socket-patch-core/src/vendor/jvm/apply.rs | 2 +- .../socket-patch-core/src/vendor/maven_repo.rs | 2 +- .../socket-patch-core/src/vendor/nuget_feed.rs | 4 ++-- crates/socket-patch-core/src/vendor/pypi.rs | 14 ++++++++------ crates/socket-patch-core/src/vendor/pypi_uv.rs | 2 +- crates/socket-patch-core/src/vendor/state.rs | 16 +++++++++++++--- 8 files changed, 28 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/composer_lock.rs b/crates/socket-patch-core/src/vendor/composer_lock.rs index 63e319d43..64d424584 100644 --- a/crates/socket-patch-core/src/vendor/composer_lock.rs +++ b/crates/socket-patch-core/src/vendor/composer_lock.rs @@ -556,7 +556,7 @@ pub async fn revert_composer_opts( "vendor_lock_entry_drifted", format!( "composer.lock entry for {} no longer points into .socket/vendor/composer/; left alone", - w.key.as_deref().unwrap_or("") + w.key_label() ), )), Err(e) => { diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 51bd83bf2..04fc21e60 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -1114,7 +1114,7 @@ pub async fn revert_gem_opts( continue; } }; - let key = w.key.as_deref().unwrap_or(""); + let key = w.key_label(); match restored { Ok(RecordRevert::Done) => {} Ok(RecordRevert::Drifted) => { diff --git a/crates/socket-patch-core/src/vendor/jvm/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index 2758a2317..e9d17f762 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -926,7 +926,7 @@ pub fn check_entry( if !gradle::metadata_record_present(text, w) { return Err(format!( "upstream verification entry drifted: {}", - w.key.as_deref().unwrap_or("") + w.key_label() )); } } diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 63bce27af..1b7afe0f4 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -251,7 +251,7 @@ pub async fn revert_maven_opts( format!( "{} no longer carries the vendored {}; left alone", w.file, - w.key.as_deref().unwrap_or("") + w.key_label() ), )), Err(e) => { diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index 452f7dfc3..213ace3d4 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -877,7 +877,7 @@ pub async fn revert_nuget_opts( .iter() .find(|c| c.kind == CONFIG_SOURCE_WIRING_KIND) .map_or("nuget.config", |c| c.file.as_str()), - w.key.as_deref().unwrap_or(""), + w.key_label(), w.file ), )); @@ -905,7 +905,7 @@ pub async fn revert_nuget_opts( format!( "{} no longer carries what vendor wrote for {}; left alone", w.file, - w.key.as_deref().unwrap_or("") + w.key_label() ), )), Err(e) => { diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 292109ebb..e7d72eacc 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -3332,8 +3332,7 @@ pub async fn revert_pypi_opts( .filter(|f| !KNOWN_PYPI_FLAVORS.contains(f)) { return RevertOutcome::failed(format!( - "unknown pypi vendor flavor {:?}; cannot revert", - Some(flavor) + "unknown pypi vendor flavor {flavor:?}; cannot revert" )); } let uuid_dir_rel = vendor_uuid_dir_rel("pypi", &entry.uuid) @@ -3360,9 +3359,12 @@ pub async fn revert_pypi_opts( match flavor { Some(flavor) => revert_by_record_kind(entry, project_root, flavor, dry_run).await, None => { - let other = entry.flavor.as_deref(); + let other = entry + .flavor + .as_deref() + .map_or_else(|| "".to_string(), |f| format!("{f:?}")); return RevertOutcome::failed(format!( - "unknown pypi vendor flavor {other:?}; cannot revert" + "unknown pypi vendor flavor {other}; cannot revert" )); } } @@ -6858,7 +6860,7 @@ wheels = [ outcome .error .as_deref() - .is_some_and(|e| e.contains("unknown pypi vendor flavor")), + .is_some_and(|e| e.contains("unknown pypi vendor flavor \"frobnicate\";")), "{outcome:?}" ); let mut wired = revert_entry("uv", &rel_wheel, Vec::new()); @@ -6877,7 +6879,7 @@ wheels = [ outcome .error .as_deref() - .is_some_and(|e| e.contains("unknown pypi vendor flavor")), + .is_some_and(|e| e.contains("unknown pypi vendor flavor \"frobnicate\";")), "{outcome:?}" ); } diff --git a/crates/socket-patch-core/src/vendor/pypi_uv.rs b/crates/socket-patch-core/src/vendor/pypi_uv.rs index 07574a8b2..58dda2a3d 100644 --- a/crates/socket-patch-core/src/vendor/pypi_uv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_uv.rs @@ -1181,7 +1181,7 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( - "uv.lock entry for {canon:?} that `uv add` routed through {needle} \ + "uv.lock entry for {canon} that `uv add` routed through {needle} \ can't be put back on the registry ({reason}); left both files \ untouched (pin it with `==` in pyproject.toml or remove it, then \ revert again)" diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index cffd3e3bd..aafe52f92 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -153,9 +153,15 @@ pub struct WiringRecord { impl WiringRecord { /// The key as user-facing warning text: verbatim, or `` for a - /// record without one (an older or hand-written ledger). - pub fn key_label(&self) -> &str { - self.key.as_deref().unwrap_or("") + /// record without one (an older or hand-written ledger). SECURITY: the + /// key comes from the committed, tamper-able state.json, so control + /// characters are escaped rather than written to the terminal. + pub fn key_label(&self) -> std::borrow::Cow<'_, str> { + match self.key.as_deref() { + None | Some("") => "".into(), + Some(key) if key.chars().any(char::is_control) => key.escape_debug().to_string().into(), + Some(key) => key.into(), + } } } @@ -1100,6 +1106,10 @@ mod tests { assert_eq!(rec.key_label(), "default:six"); rec.key = None; assert_eq!(rec.key_label(), ""); + rec.key = Some(String::new()); + assert_eq!(rec.key_label(), ""); + rec.key = Some("default:six\x1b[2K\nforged".into()); + assert_eq!(rec.key_label(), "default:six\\u{1b}[2K\\nforged"); } fn sample_entry() -> VendorEntry { From 4c9362f8e451a67ab6178f1e3eabfccab10d5776 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 05:20:40 +0000 Subject: [PATCH 4/4] Escape tamper-able names in uv revert drift warnings Bugbot on #1679: dropping Debug formatting left two uv revert warnings interpolating the ledger key and the canonical package name raw. Both come from the committed state.json, so control characters could reach the terminal. Route the key through key_label() and the name through a new shared terminal_safe() helper, which key_label() now also uses. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01CS1VGxJJE7pdpG9pXQNqGP --- crates/socket-patch-core/src/vendor/pypi_uv.rs | 12 +++++++----- crates/socket-patch-core/src/vendor/state.rs | 15 +++++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_uv.rs b/crates/socket-patch-core/src/vendor/pypi_uv.rs index 58dda2a3d..70f880e09 100644 --- a/crates/socket-patch-core/src/vendor/pypi_uv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_uv.rs @@ -46,7 +46,7 @@ use super::common::{ ensure_unchanged, item_get, pep508_name, pyproject_dependency_specs, record, refuse_symlinked, }; use super::parse_memo::ParseMemo; -use super::state::{UvMeta, VendorEntry, WiringAction, WiringRecord}; +use super::state::{terminal_safe, UvMeta, VendorEntry, WiringAction, WiringRecord}; use super::toml_surgery::{ balanced_span, find_unit_span, line_index, remove_exact_line, remove_substring, remove_table_if_empty, replace_fragment, split_top_level_commas, top_level_brace_groups, @@ -974,8 +974,9 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( - "uv.lock entry for {key} can't be restored to match \ - pyproject.toml ({reason}); left untouched" + "uv.lock entry for {} can't be restored to match \ + pyproject.toml ({reason}); left untouched", + rec.key_label() ), )); continue; @@ -1181,10 +1182,11 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( - "uv.lock entry for {canon} that `uv add` routed through {needle} \ + "uv.lock entry for {} that `uv add` routed through {needle} \ can't be put back on the registry ({reason}); left both files \ untouched (pin it with `==` in pyproject.toml or remove it, then \ - revert again)" + revert again)", + terminal_safe(&canon) ), )); } diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index aafe52f92..a579fa38f 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -159,12 +159,21 @@ impl WiringRecord { pub fn key_label(&self) -> std::borrow::Cow<'_, str> { match self.key.as_deref() { None | Some("") => "".into(), - Some(key) if key.chars().any(char::is_control) => key.escape_debug().to_string().into(), - Some(key) => key.into(), + Some(key) => terminal_safe(key), } } } +/// `s` with control characters escaped, for warning text built from +/// values read out of the committed, tamper-able state.json. +pub fn terminal_safe(s: &str) -> std::borrow::Cow<'_, str> { + if s.chars().any(char::is_control) { + s.escape_debug().to_string().into() + } else { + s.into() + } +} + /// Original Cargo.lock fields removed by the path-dep surgery; not /// recomputable offline (the checksum is the sha256 of the registry `.crate` /// tarball, not of the extracted tree). @@ -1110,6 +1119,8 @@ mod tests { assert_eq!(rec.key_label(), ""); rec.key = Some("default:six\x1b[2K\nforged".into()); assert_eq!(rec.key_label(), "default:six\\u{1b}[2K\\nforged"); + assert_eq!(terminal_safe("six"), "six"); + assert_eq!(terminal_safe("six\x1b[2K"), "six\\u{1b}[2K"); } fn sample_entry() -> VendorEntry {