From 92630055336f74d898c59a1e2124f7cd907e171f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 15:00:36 +0000 Subject: [PATCH 1/6] Start refactor for #594 Assisted-by: Claude Code:claude-opus-5-5 From 3f929fd3a95e122588353464aad9171a24f9a2a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 15:16:29 +0000 Subject: [PATCH 2/6] Wire vendored nuget.config via formats::nuget Vendored NuGet now reads the source keys and finds the , and anchors through formats::nuget::parse_config, the reader that hosted, upstream restore and VEX already use. The private substring scanner (blank_comments, parse_config_source_keys, attr_value, self_closing_package_sources, insert_at_line) is deleted. User impact: - A close tag written with whitespace () is now the section that gets extended; vendor used to append a second section NuGet ignores, so restore failed NU1100/NU1403 (#685). - An empty is expanded in place instead of left beside a second mapping section. - A section opened and closed on one line receives the source inside it, not before its open tag. - Catch-all keys are written XML-encoded, so a key with & or a quote keeps its identity. - Malformed XML or a repeated section is refused with "malformed XML or a repeated section; not wired" instead of being spliced at the first substring match, as hosted already does. Output bytes for well-formed configs are unchanged. Fixes #685 Refs #594 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/nuget/mod.rs | 14 + .../src/vendor/nuget_feed.rs | 529 +++++++++++------- 2 files changed, 335 insertions(+), 208 deletions(-) diff --git a/crates/socket-patch-core/src/formats/nuget/mod.rs b/crates/socket-patch-core/src/formats/nuget/mod.rs index fdf41875c..1499a27d2 100644 --- a/crates/socket-patch-core/src/formats/nuget/mod.rs +++ b/crates/socket-patch-core/src/formats/nuget/mod.rs @@ -291,6 +291,20 @@ fn decode_entities(raw: &str) -> String { out } +/// Encode `value` for a double-quoted attribute: the inverse of +/// [`parse_config`]'s decoding, so a key read as `a&b` is written back as +/// `a&b` and keeps its identity. +pub(crate) fn xml_attribute(value: &str) -> String { + value + .replace('&', "&") + .replace('"', """) + .replace('<', "<") + // Literal XML attribute whitespace would be normalized to spaces. + .replace('\t', " ") + .replace('\n', " ") + .replace('\r', " ") +} + #[cfg(test)] mod tests { #[test] diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index 70d6be479..13c335489 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -256,7 +256,8 @@ async fn nuget_prelude( // key merely mentioned elsewhere — is not wiring NuGet reads. let config_wired = config_text .as_deref() - .is_some_and(|t| parse_config_source_keys(&blank_comments(t)).contains(&source_key)); + .and_then(crate::formats::nuget::parse_config) + .is_some_and(|parsed| parsed.sources.iter().any(|(key, _)| *key == source_key)); let in_sync = config_wired && { // One guarded read of the committed nupkg serves both the member-hash // check and the lock's content-hash pin. @@ -891,17 +892,24 @@ fn build_config_edit( }) } Some(text) => { - // Every anchor find and source scan runs against the - // comment-blanked view (same length, so offsets splice into - // `text`). NuGet never reads a comment: a commented-out section - // must not capture an insert (the wired source would be invisible - // and restore would silently serve the UNPATCHED package), and a - // commented-out `` must not become a catch-all target (the - // mapping would fan `*` out to a source that does not exist). - let visible = blank_comments(text); + // Keys and anchors come from the one `nuget.config` reader that + // hosted, restore and VEX use. NuGet never reads a comment, CDATA + // or an element outside `configuration/
`: a commented-out + // section must not capture an insert (the wired source would be + // invisible and restore would silently serve the UNPATCHED + // package), and a commented-out `` must not become a + // catch-all target (the mapping would fan `*` out to a source that + // does not exist). Malformed XML or a repeated section has no + // single live anchor, so it is refused rather than guessed at. + let parsed = parse_wirable_config(text)?; // Whether we are about to CREATE the mapping section (vs. extend an - // existing one) — decided against the pre-edit text. - let creating_mapping = !visible.contains(""); + // existing one) — decided against the pre-edit text. An empty + // self-closing `` maps nothing, so it is + // created (expanded in place) too. + let creating_mapping = parsed + .source_mapping + .as_ref() + .is_none_or(|section| section.close_start.is_none()); // The pre-existing sources the catch-all fans `*` out to. When the // config has NONE and we are creating a mapping from scratch, a // socket-only mapping would NU1100 every other package, so seed the @@ -912,7 +920,12 @@ fn build_config_edit( // suppressing the seed on it recreates the exact socket-only // mapping the seed exists to prevent. Mirrors // redirect::add_nuget_source. - let mut catch_all_keys = parse_config_source_keys(&visible); + let mut catch_all_keys: Vec = Vec::new(); + for (key, _) in &parsed.sources { + if !catch_all_keys.contains(key) { + catch_all_keys.push(key.clone()); + } + } let seed_nuget_org = creating_mapping && catch_all_keys.is_empty(); let source_add = format!(" \n"); @@ -931,45 +944,59 @@ fn build_config_edit( // self-closing `` carries no children, so // expand it in place into an open/close pair rather than leaving // it dangling beside a duplicate element. - let with_source = if let Some((start, end)) = self_closing_package_sources(&visible) { - let mut expanded = String::with_capacity(text.len() + injected_sources.len() + 40); - expanded.push_str(&text[..start]); - expanded.push_str(&format!( - "\n{injected_sources} " - )); - expanded.push_str(&text[end..]); - expanded - } else if let Some(at) = visible.find("") { - insert_at_line(text, at, &injected_sources) - } else if let Some(at) = visible.find("") { - let block = format!(" \n{injected_sources} \n"); - insert_at_line(text, at, &block) - } else { - return Err("nuget.config has no to edit".to_string()); + let no_root = || "nuget.config has no to edit".to_string(); + let with_source = match &parsed.package_sources { + Some(section) => { + insert_children(text, section, "packageSources", &injected_sources) + } + None => { + let root = parsed.configuration.as_ref().ok_or_else(no_root)?; + let block = + format!(" \n{injected_sources} \n"); + insert_before_close(text, root, &block).ok_or_else(no_root)? + } }; // 2. Mapping: extend an existing section, or create one over the - // pre-existing sources (the load-bearing catch-all). The blanked - // view is recomputed — step 1 shifted the offsets. - let visible_ws = blank_comments(&with_source); + // pre-existing sources (the load-bearing catch-all). Step 1 + // shifted the offsets, so the edited text is re-read through + // the same tokenizer. + let updated = parse_wirable_config(&with_source)?; let new_text = if !creating_mapping { - let at = visible_ws.find("").ok_or_else(|| { + let section = updated.source_mapping.as_ref().ok_or_else(|| { "could not locate to insert the mapping".to_string() })?; - insert_at_line(&with_source, at, &mapping_fragment) + insert_children( + &with_source, + section, + "packageSourceMapping", + &mapping_fragment, + ) } else { - let mut block = String::from(" \n"); + let mut inner = String::new(); for key in &catch_all_keys { - block.push_str(&format!( - " \n \n \n" + inner.push_str(&format!( + " \n \n \n", + crate::formats::nuget::xml_attribute(key) )); } - block.push_str(&mapping_fragment); - block.push_str(" \n"); - let at = visible_ws.find("").ok_or_else(|| { - "could not locate to insert a packageSourceMapping section" - .to_string() - })?; - insert_at_line(&with_source, at, &block) + inner.push_str(&mapping_fragment); + match &updated.source_mapping { + Some(section) => { + insert_children(&with_source, section, "packageSourceMapping", &inner) + } + None => { + let block = + format!(" \n{inner} \n"); + updated + .configuration + .as_ref() + .and_then(|root| insert_before_close(&with_source, root, &block)) + .ok_or_else(|| { + "could not locate to insert a packageSourceMapping section" + .to_string() + })? + } + } }; Ok(ConfigEdit { new_text, @@ -979,121 +1006,58 @@ fn build_config_edit( } } -/// `text` with every `` comment blanked to spaces (newlines kept), -/// preserving length so offsets found in the blanked view splice into the -/// original. NuGet never reads a comment, so anchors and source keys inside -/// one must be invisible to the wiring logic — the nuget twin of maven's -/// `find_wireable_anchor` comment masking. An unterminated comment blanks -/// through EOF (fail-closed). -fn blank_comments(text: &str) -> String { - let mut out = text.as_bytes().to_vec(); - let mut from = 0; - while let Some(rel) = text[from..].find("") { - Some(rel_end) => start + 4 + rel_end + 3, - None => text.len(), - }; - for b in &mut out[start..end] { - if *b != b'\n' { - *b = b' '; - } - } - from = end; - } - // Every replaced byte became ASCII space; newlines are never continuation - // bytes, so the result is valid UTF-8. - String::from_utf8(out).expect("blanking preserves UTF-8") -} - -/// Insert `insertion` (already newline-terminated) at the start of the line -/// containing byte offset `at` — the offset comes from the comment-blanked -/// view, which shares offsets with `text`. -fn insert_at_line(text: &str, at: usize, insertion: &str) -> String { - let line_start = text[..at].rfind('\n').map(|n| n + 1).unwrap_or(0); - let mut out = String::with_capacity(text.len() + insertion.len()); - out.push_str(&text[..line_start]); - out.push_str(insertion); - out.push_str(&text[line_start..]); - out +/// `text` through [`crate::formats::nuget::parse_config`], or the refusal +/// when it has no single live layout to wire into. +fn parse_wirable_config(text: &str) -> Result { + crate::formats::nuget::parse_config(text) + .filter(|parsed| !parsed.repeated_sections) + .ok_or_else(|| { + "nuget.config has malformed XML or a repeated section; not wired".to_string() + }) } -/// Extract the `key` attribute of every `` element inside -/// ``. Deliberately minimal (no XML parser dependency): scans -/// the packageSources span for `` elements. These are -/// the "pre-existing sources" the catch-all maps `*` to. Callers pass the -/// comment-blanked text so a commented-out source never contributes a key. -fn parse_config_source_keys(text: &str) -> Vec { - let mut out = Vec::new(); - let Some(start) = text.find("` - // (valid, common) or a malformed config NuGet itself would reject. Scanning - // to EOF instead would harvest `` entries from unrelated - // sections (``, ``, …) as phantom catch-all - // sources — mapping `*` to a key NuGet has no source for hard-fails every - // restore. - let Some(end) = text[start..].find("").map(|e| start + e) else { +/// Insert `children` (newline-terminated lines) as the last children of +/// `section`, or expand a self-closing `section` in place (its attributes +/// kept) into an open/close pair holding them. +fn insert_children( + text: &str, + section: &crate::formats::nuget::ConfigSection, + name: &str, + children: &str, +) -> String { + if let Some(out) = insert_before_close(text, section, children) { return out; - }; - let span = &text[start..end]; - let mut rest = span; - while let Some(add_at) = rest.find("'. - let elem_end = after.find('>').unwrap_or(after.len()); - let elem = &after[..elem_end]; - if let Some(key) = attr_value(elem, "key") { - if !out.contains(&key) { - out.push(key); - } - } - rest = &after[elem_end..]; } + let head = text[section.open.start..section.open.end - 2].trim_end(); + let mut out = String::with_capacity(text.len() + children.len() + 2 * name.len() + 8); + out.push_str(&text[..section.open.start]); + out.push_str(&format!("{head}>\n{children} ")); + out.push_str(&text[section.open.end..]); out } -/// The value of `="..."` inside an element's attribute text, if present. -/// Tolerates whitespace around `=` (`key = "nuget.org"` is valid XML NuGet -/// parses): a real source the scan misses would read as "no sources", -/// triggering a duplicate nuget.org seed and leaving the missed source out of -/// the catch-all fan-out. -fn attr_value(elem: &str, attr: &str) -> Option { - let mut rest = elem; - loop { - let at = rest.find(attr)?; - let after = rest[at + attr.len()..].trim_start(); - if let Some(eq) = after.strip_prefix('=') { - // NuGet accepts either XML quote style; tolerate both, like the - // redirect twin (patch/redirect/mod.rs nuget key harvesting). - let val = eq.trim_start(); - for quote in ['"', '\''] { - if let Some(quoted) = val.strip_prefix(quote) { - let close = quoted.find(quote)?; - return Some(quoted[..close].to_string()); - } - } - } - rest = &rest[at + attr.len()..]; - } -} - -/// The `[start, end)` byte span of a self-closing `` element -/// (any whitespace before `/>`), or `None` if the config has no such element. -/// Deliberately minimal (no XML parser dependency), matching the rest of this -/// module's scanning style. -fn self_closing_package_sources(text: &str) -> Option<(usize, usize)> { - let start = text.find("` for a - // self-closing element — anything else (`>` or an attribute) is a normal - // open tag, which the caller handles separately. - let after_name = &text[start + "`, so a - // `` open tag or `")?; - let end = text.len() - rest.len(); - Some((start, end)) +/// Insert `insertion` (newline-terminated lines) at the start of the line +/// holding `section`'s close tag, so it lands indented like its siblings, or +/// right before the close tag when other markup shares its line (a section +/// opened and closed on one line still receives it inside). `None` for a +/// self-closing section. +fn insert_before_close( + text: &str, + section: &crate::formats::nuget::ConfigSection, + insertion: &str, +) -> Option { + let close = section.close_start?; + let line_start = text[..close].rfind('\n').map(|n| n + 1).unwrap_or(0); + let at = if text[line_start..close].trim().is_empty() { + line_start + } else { + close + }; + let mut out = String::with_capacity(text.len() + insertion.len()); + out.push_str(&text[..at]); + out.push_str(insertion); + out.push_str(&text[at..]); + Some(out) } /// Revert our `nuget.config` wiring. `Ok(true)` = reverted (or would be on dry @@ -1547,12 +1511,30 @@ mod tests { assert_eq!(t.matches("").count(), 1); } + /// A section opened and closed on one line receives the insert inside + /// it: the line-start anchor never reaches back before the open tag. #[test] - fn parse_config_source_keys_reads_adds() { + fn one_line_sections_receive_their_children_inside() { let text = "\ \ "; - assert_eq!(parse_config_source_keys(text), vec!["a", "b"]); + let edit = build_config_edit( + Some(text), + &source_key(), + &format!(".socket/vendor/nuget/{UUID}"), + "Newtonsoft.Json", + ) + .unwrap(); + let parsed = crate::formats::nuget::parse_config(&edit.new_text).unwrap(); + let keys: Vec<&str> = parsed.sources.iter().map(|(k, _)| k.as_str()).collect(); + assert_eq!(keys, ["a", "b", source_key().as_str()], "{}", edit.new_text); + let mapped: Vec<&str> = parsed.mappings.iter().map(|(k, _)| k.as_str()).collect(); + assert_eq!( + mapped, + ["a", "b", source_key().as_str()], + "{}", + edit.new_text + ); } #[test] @@ -1570,9 +1552,11 @@ mod tests { \x20 \n\ \x20 \n\ \n"; - assert_eq!( - parse_config_source_keys(orig), - Vec::::new(), + assert!( + crate::formats::nuget::parse_config(orig) + .unwrap() + .sources + .is_empty(), "a self-closing packageSources carries no source keys" ); let edit = build_config_edit( @@ -2107,7 +2091,11 @@ mod tests { .await .unwrap(); assert!( - parse_config_source_keys(&blank_comments(&rewired)).contains(&source_key()), + crate::formats::nuget::parse_config(&rewired) + .unwrap() + .sources + .iter() + .any(|(key, _)| *key == source_key()), "the re-run wires a live source: {rewired}" ); } @@ -3460,7 +3448,7 @@ mod tests { .error .as_deref() .unwrap_or("") - .contains("no "), + .contains("malformed XML"), "{:?}", result.error ); @@ -3515,10 +3503,10 @@ mod tests { assert!(t.trim_end().ends_with("")); } - /// A config whose only anchor is `` (step 1 lands) but - /// with no `` fails creating the mapping section. + /// A `` outside a `` root is not a section + /// NuGet reads, so it is no anchor: the edit fails on the missing root. #[test] - fn config_without_configuration_close_errs_on_mapping_section() { + fn config_without_configuration_root_errs() { let orig = "\n\n"; let err = build_config_edit( Some(orig), @@ -3527,13 +3515,13 @@ mod tests { "Newtonsoft.Json", ) .err() - .expect("a config without must fail the mapping insert"); - assert!(err.contains("packageSourceMapping section"), "{err}"); + .expect("a config without must fail the edit"); + assert!(err.contains("no to edit"), "{err}"); } - /// No usable anchor at all: fail-closed with the `` error. + /// An unclosed root is malformed XML: fail-closed before any splice. #[test] - fn config_without_any_anchor_errs() { + fn config_with_unclosed_root_errs() { let err = build_config_edit( Some(""), &source_key(), @@ -3542,7 +3530,7 @@ mod tests { ) .err() .expect("an anchorless config must fail the edit"); - assert!(err.contains("no to edit"), "{err}"); + assert!(err.contains("malformed XML"), "{err}"); } // ── marker write failure is a warning, not a failure ─────────────────── @@ -4236,32 +4224,180 @@ mod tests { ); } - // ── comment blanking + key scan edges ────────────────────────────────── + // ── shared-reader edges (formats::nuget::parse_config) ───────────────── + + fn wire(text: &str) -> Result { + build_config_edit( + Some(text), + &source_key(), + &format!(".socket/vendor/nuget/{UUID}"), + "Newtonsoft.Json", + ) + } + /// An unterminated comment, a mismatched close tag or a repeated section + /// has no single live anchor: the writer refuses instead of splicing into + /// whichever copy a substring search finds first. #[test] - fn blank_comments_unterminated_blanks_through_eof() { - let input = "keep \n\ + \x20 \n \n\n", + // commented before the real one + "\n \n \n\ + \x20 \n \n\n", + // commented + "\n \n \n\ + \x20 \n \n\ + \n", + // self-closing sections + "\n \n \n\n", + // single-quoted and spaced attributes, CRLF + "\r\n \r\n \r\n\ + \x20 \r\n\r\n", + // in a lookalike section outside packageSources + "\n \n \n\ + \x20 \n\n", + ]; + for text in cases { + let before = crate::formats::nuget::parse_config(text).unwrap(); + let t = wire(text) + .unwrap_or_else(|e| panic!("{e}: {text:?}")) + .new_text; + let after = crate::formats::nuget::parse_config(&t) + .unwrap_or_else(|| panic!("unparseable output: {t:?}")); + assert!(!after.repeated_sections, "{t}"); + let mut expected: Vec = before.sources.iter().map(|(k, _)| k.clone()).collect(); + if expected.is_empty() + && before + .source_mapping + .is_none_or(|m| m.close_start.is_none()) + { + expected.push(NUGET_ORG_SOURCE_KEY.to_string()); + } + let wired = after + .sources + .iter() + .map(|(k, _)| k.clone()) + .filter(|k| *k != source_key()) + .collect::>(); + let mut wired_sorted = wired.clone(); + wired_sorted.sort(); + let mut expected_sorted = expected.clone(); + expected_sorted.sort(); + assert_eq!(wired_sorted, expected_sorted, "{t}"); + assert!(after.sources.iter().any(|(k, _)| *k == source_key()), "{t}"); + let catch_all: Vec<&str> = after + .mappings + .iter() + .filter(|(_, p)| p == &["*"]) + .map(|(k, _)| k.as_str()) + .collect(); + assert_eq!(catch_all.len(), expected.len(), "{t}"); + assert!( + after + .mappings + .iter() + .any(|(k, p)| *k == source_key() && p == &["Newtonsoft.Json"]), + "{t}" + ); + } } // ── permission-failure unwinds (unix) ────────────────────────────────── @@ -4450,29 +4586,6 @@ mod tests { // ── remaining prod arms ──────────────────────────────────────────────── - /// `attr_value` scanning edges: a substring hit on the attribute NAME - /// (`keyring`) and a malformed unquoted value both advance the scan to the - /// next occurrence instead of aborting the harvest, and a config with no - /// properly quoted attribute at all terminates with `None`. - #[test] - fn attr_value_skips_name_lookalikes_and_unquoted_values() { - // "keyring" contains "key" but is not the attribute: the real quoted - // `key` later in the element must still be harvested. - assert_eq!( - attr_value(" keyring=\"x\" key=\"real\" /", "key").as_deref(), - Some("real") - ); - // An unquoted value (malformed XML NuGet would reject anyway) is not - // harvested; the scan moves on to the next, properly quoted match. - assert_eq!( - attr_value("key=bare key='q2'", "key").as_deref(), - Some("q2") - ); - // Lookalikes only ("keyring", "monkeys") and no quoted value → None, - // not an infinite loop. - assert_eq!(attr_value("keyring monkeys", "key"), None); - } - /// Tier A (service prebuilt) write failure: the served bytes cannot land /// because a regular FILE squats the uuid dir path → the hard /// `vendor_prebuilt_write_failed` refusal, before any wiring. On this From 35fd42ca8e67643b334c8bd4ae70378a7682d1c2 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 12:46:13 -0400 Subject: [PATCH 3/6] Start NuGet fix: nuget-content-hash Draft placeholder while the fix is written. Co-Authored-By: Claude Opus 5.5 (1M context) From a5742c81aaef2400eaaed5037faead9703f52c56 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 13:19:49 -0400 Subject: [PATCH 4/6] Restore NuGet contentHash, not catalog hash Undoing a hosted NuGet patch (remove, rollback, and the hosted to vendored takeover) put nuget.org's catalog packageHash back into packages.lock.json. That is the SHA-512 of the signed .nupkg file, while NuGet's contentHash excludes the repository signature, so every later dotnet restore failed NU1403 for practically every nuget.org package. The upstream restore now downloads the .nupkg from nuget.org's flat container and computes the content hash the way NuGet does (SignedPackageArchiveUtility.GetPackageContentHash: the archive hashed as if the .signature.p7s entry were absent). Verified against the live Newtonsoft.Json 13.0.3 package: HrC5BXdl...gPa+zQ==, the value dotnet restore writes. Fixes #624. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/formats/nuget/mod.rs | 2 + .../src/formats/nuget/package.rs | 264 ++++++++++++++++++ .../src/patch/redirect/upstream/client.rs | 80 +----- .../src/patch/redirect/upstream/nuget.rs | 63 +++-- .../tests/upstream_restore_golden.rs | 68 +++-- 5 files changed, 369 insertions(+), 108 deletions(-) create mode 100644 crates/socket-patch-core/src/formats/nuget/package.rs diff --git a/crates/socket-patch-core/src/formats/nuget/mod.rs b/crates/socket-patch-core/src/formats/nuget/mod.rs index 1499a27d2..4e354779e 100644 --- a/crates/socket-patch-core/src/formats/nuget/mod.rs +++ b/crates/socket-patch-core/src/formats/nuget/mod.rs @@ -10,6 +10,8 @@ //! and DOCTYPEs are skipped; an unterminated tag or comment, an unquoted //! attribute or a mismatched close tag makes the whole file `None`. +pub(crate) mod package; + use std::collections::BTreeSet; use std::ops::Range; diff --git a/crates/socket-patch-core/src/formats/nuget/package.rs b/crates/socket-patch-core/src/formats/nuget/package.rs new file mode 100644 index 000000000..380405cc0 --- /dev/null +++ b/crates/socket-patch-core/src/formats/nuget/package.rs @@ -0,0 +1,264 @@ +//! A `.nupkg`'s content hash: the `contentHash` NuGet writes into +//! `packages.lock.json` and `.nupkg.metadata` (#624). +//! +//! For an unsigned package it is the base64 SHA-512 of the file. For a +//! signed package — nuget.org repository-signs every package — NuGet hashes +//! the archive AS IF the `.signature.p7s` entry were absent +//! (`PackageArchiveReader.GetContentHash` → +//! `SignedPackageArchiveUtility.GetPackageContentHash`): +//! +//! 1. the bytes before the first (non-signature) local file entry; +//! 2. every non-signature file entry (local header, data, data +//! descriptor), in archive order; +//! 3. every non-signature central directory record, in directory order, +//! with its local-header offset moved back by the signature entry's size +//! when the entry it points at follows the signature; +//! 4. the end-of-central-directory record with the entry counts one lower, +//! the directory size less the signature's record and the directory +//! offset less the signature entry's size, then the rest of the file. +//! +//! So the catalog `packageHash` (SHA-512 of the signed file as served) is +//! NOT a lock's `contentHash`, and pinning it fails every restore NU1403. +//! Zip64 archives are refused rather than guessed at. + +use sha2::{Digest, Sha512}; + +/// The signature entry NuGet excludes (`SigningSpecifications.SignaturePath`). +const SIGNATURE_PATH: &[u8] = b".signature.p7s"; + +const EOCD_SIG: u32 = 0x0605_4b50; +const ZIP64_LOCATOR_SIG: u32 = 0x0706_4b50; +const CENTRAL_SIG: u32 = 0x0201_4b50; +const LOCAL_SIG: u32 = 0x0403_4b50; +const DESCRIPTOR_SIG: u32 = 0x0807_4b50; +const EOCD_LEN: usize = 22; + +fn u16_at(b: &[u8], at: usize) -> Result { + b.get(at..at + 2) + .map(|s| u16::from_le_bytes([s[0], s[1]])) + .ok_or_else(|| truncated(at)) +} + +fn u32_at(b: &[u8], at: usize) -> Result { + b.get(at..at + 4) + .map(|s| u32::from_le_bytes([s[0], s[1], s[2], s[3]])) + .ok_or_else(|| truncated(at)) +} + +fn truncated(at: usize) -> String { + format!("the package archive is truncated at byte {at}") +} + +/// One central directory record and the file entry it describes. +struct Record { + /// Offset of the central directory record. + position: usize, + header_size: usize, + local_offset: usize, + /// Local header + data + data descriptor. + entry_size: usize, + is_signature: bool, +} + +/// The base64 SHA-512 NuGet records as `contentHash` for `nupkg`. +pub(crate) fn package_content_hash(nupkg: &[u8]) -> Result { + use base64::Engine as _; + let eocd = find_eocd(nupkg)?; + if eocd >= 20 && u32_at(nupkg, eocd - 20)? == ZIP64_LOCATOR_SIG { + return Err("zip64 package archives are not supported".to_string()); + } + let entries_disk = u16_at(nupkg, eocd + 8)?; + let entries = u16_at(nupkg, eocd + 10)?; + let cd_size = u32_at(nupkg, eocd + 12)?; + let cd_offset = u32_at(nupkg, eocd + 16)?; + if entries == u16::MAX || cd_size == u32::MAX || cd_offset == u32::MAX { + return Err("zip64 package archives are not supported".to_string()); + } + if entries_disk != entries || u16_at(nupkg, eocd + 4)? != 0 || u16_at(nupkg, eocd + 6)? != 0 { + return Err("multi-disk package archives are not supported".to_string()); + } + let mut records = Vec::with_capacity(entries as usize); + let mut at = cd_offset as usize; + for _ in 0..entries { + if u32_at(nupkg, at)? != CENTRAL_SIG { + return Err(format!("no central directory record at byte {at}")); + } + let flags = u16_at(nupkg, at + 8)?; + let compressed = u32_at(nupkg, at + 20)? as usize; + let name_len = u16_at(nupkg, at + 28)? as usize; + let extra_len = u16_at(nupkg, at + 30)? as usize; + let comment_len = u16_at(nupkg, at + 32)? as usize; + let local_offset = u32_at(nupkg, at + 42)? as usize; + let name = nupkg + .get(at + 46..at + 46 + name_len) + .ok_or_else(|| truncated(at + 46))?; + if u32_at(nupkg, local_offset)? != LOCAL_SIG { + return Err(format!("no local file header at byte {local_offset}")); + } + let local_header = 30 + + u16_at(nupkg, local_offset + 26)? as usize + + u16_at(nupkg, local_offset + 28)? as usize; + let mut entry_size = local_header + compressed; + if flags & 0x0008 != 0 { + // A data descriptor follows the data, with or without its + // optional signature. + let d = local_offset + entry_size; + entry_size += if u32_at(nupkg, d)? == DESCRIPTOR_SIG { + 16 + } else { + 12 + }; + } + if local_offset + entry_size > nupkg.len() { + return Err(truncated(local_offset + entry_size)); + } + let header_size = 46 + name_len + extra_len + comment_len; + records.push(Record { + position: at, + header_size, + local_offset, + entry_size, + is_signature: name == SIGNATURE_PATH, + }); + at += header_size; + } + let mut signatures = records.iter().filter(|r| r.is_signature); + let signature = match (signatures.next(), signatures.next()) { + (None, _) => return Ok(crate::utils::digest::sha512_base64_of(nupkg)), + (Some(sig), None) => (sig.local_offset, sig.entry_size, sig.header_size), + (Some(_), Some(_)) => return Err("the package has two signature entries".to_string()), + }; + let (sig_offset, sig_entry_size, sig_header_size) = signature; + let mut rest: Vec<&Record> = records.iter().filter(|r| !r.is_signature).collect(); + if rest.is_empty() { + return Err("the package holds nothing but its signature".to_string()); + } + + let mut hash = Sha512::new(); + rest.sort_by_key(|r| r.local_offset); + hash.update(&nupkg[..rest[0].local_offset]); + for r in &rest { + hash.update(&nupkg[r.local_offset..r.local_offset + r.entry_size]); + } + rest.sort_by_key(|r| r.position); + for r in &rest { + hash.update(&nupkg[r.position..r.position + 42]); + let offset = if r.local_offset > sig_offset { + r.local_offset - sig_entry_size + } else { + r.local_offset + }; + hash.update((offset as u32).to_le_bytes()); + hash.update(&nupkg[r.position + 46..r.position + r.header_size]); + } + hash.update(&nupkg[eocd..eocd + 8]); + hash.update((entries_disk - 1).to_le_bytes()); + hash.update((entries - 1).to_le_bytes()); + hash.update((cd_size - sig_header_size as u32).to_le_bytes()); + hash.update((cd_offset - sig_entry_size as u32).to_le_bytes()); + hash.update(&nupkg[eocd + 20..]); + Ok(base64::engine::general_purpose::STANDARD.encode(hash.finalize())) +} + +/// Offset of the end-of-central-directory record: the last signature whose +/// comment length reaches exactly to the end of the file. +fn find_eocd(b: &[u8]) -> Result { + if b.len() < EOCD_LEN { + return Err("the package is not a zip archive".to_string()); + } + let floor = b.len().saturating_sub(EOCD_LEN + u16::MAX as usize); + (floor..=b.len() - EOCD_LEN) + .rev() + .find(|&at| { + u32_at(b, at) == Ok(EOCD_SIG) + && u16_at(b, at + 20).is_ok_and(|c| at + EOCD_LEN + c as usize == b.len()) + }) + .ok_or_else(|| "the package is not a zip archive".to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write as _; + + fn zip(entries: &[(&str, &[u8])], descriptor_free: bool) -> Vec { + let mut zw = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default() + .last_modified_time(zip::DateTime::default()) + .compression_method(if descriptor_free { + zip::CompressionMethod::Stored + } else { + zip::CompressionMethod::Deflated + }); + for (name, data) in entries { + zw.start_file(*name, opts).unwrap(); + zw.write_all(data).unwrap(); + } + zw.finish().unwrap().into_inner() + } + + const FILES: [(&str, &[u8]); 3] = [ + ("[Content_Types].xml", b""), + ( + "pkg.nuspec", + b"Pkg", + ), + ( + "lib/net8.0/Pkg.dll", + b"MZ-not-really-an-assembly-but-long-enough", + ), + ]; + + #[test] + fn unsigned_package_hashes_the_whole_file() { + let bytes = zip(&FILES, true); + assert_eq!( + package_content_hash(&bytes).unwrap(), + crate::utils::digest::sha512_base64_of(&bytes) + ); + } + + /// A signature appended last (where NuGet places it) hashes exactly like + /// the same archive written without it. + #[test] + fn signed_package_hashes_as_if_unsigned() { + for stored in [true, false] { + let unsigned = zip(&FILES, stored); + let mut with_sig: Vec<(&str, &[u8])> = FILES.to_vec(); + with_sig.push((".signature.p7s", b"PKCS7-signature-bytes")); + let signed = zip(&with_sig, stored); + let hash = package_content_hash(&signed).unwrap(); + assert_ne!(hash, crate::utils::digest::sha512_base64_of(&signed)); + assert_eq!(hash, crate::utils::digest::sha512_base64_of(&unsigned)); + } + } + + /// A signature that is not the last entry: the entries after it have + /// their offsets moved back by its size. + #[test] + fn signature_in_the_middle_is_excluded_with_offsets_fixed() { + let unsigned = zip(&FILES, true); + let signed = zip( + &[ + FILES[0], + (".signature.p7s", b"PKCS7-signature-bytes"), + FILES[1], + FILES[2], + ], + true, + ); + assert_eq!( + package_content_hash(&signed).unwrap(), + crate::utils::digest::sha512_base64_of(&unsigned) + ); + } + + #[test] + fn malformed_archives_are_refused() { + assert!(package_content_hash(b"").is_err()); + assert!(package_content_hash(b"not a zip at all, just some bytes").is_err()); + let mut bytes = zip(&FILES, true); + bytes.truncate(bytes.len() / 2); + assert!(package_content_hash(&bytes).is_err()); + } +} diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs index 1c055b800..70c658377 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs @@ -548,10 +548,12 @@ impl UpstreamClient { result } - /// The `packages.lock.json` `contentHash` of `id@version` on nuget.org - /// (base64 sha512 of the `.nupkg`): the `packageHash` of the catalog - /// entry its registration leaf points at — the hash nuget.org computed - /// over the repository-signed package, without downloading it. + /// The `packages.lock.json` `contentHash` of `id@version` on nuget.org: + /// NuGet's content hash of the `.nupkg` its flat container serves, + /// which excludes the repository signature + /// ([`crate::formats::nuget::package::package_content_hash`]). The + /// catalog's `packageHash` is the hash of the signed file as served, so + /// it never matches a lock's `contentHash` (#624). pub(crate) async fn nuget_content_hash( &self, id: &str, @@ -566,67 +568,26 @@ impl UpstreamClient { return Err(OFFLINE.to_string()); } let (id_lower, version_lower) = &key; + let (id_seg, version_seg) = ( + crate::utils::uri::encode_uri_component(id_lower), + crate::utils::uri::encode_uri_component(version_lower), + ); let url = format!( - "{}/v3/registration5-gz-semver2/{}/{}.json", + "{}/v3-flatcontainer/{id_seg}/{version_seg}/{id_seg}.{version_seg}.nupkg", nuget_api_base(), - crate::utils::uri::encode_uri_component(id_lower), - crate::utils::uri::encode_uri_component(version_lower) ); - let leaf = self.get_json_maybe_gzip(&url).await?; - let catalog = leaf - .get("catalogEntry") - .and_then(Value::as_str) - .ok_or_else(|| format!("{url} names no catalog entry"))?; - let entry = self.get_json_maybe_gzip(catalog).await?; - let same_id = entry - .get("id") - .and_then(Value::as_str) - .is_some_and(|i| i.eq_ignore_ascii_case(id_lower)); - let same_version = entry - .get("version") - .and_then(Value::as_str) - .is_some_and(|v| { - crate::vendor::nuget_feed::normalize_nuget_version(v) - .eq_ignore_ascii_case(version_lower) - }); - if !same_id || !same_version { - return Err(format!( - "{catalog} is not the catalog entry of {id} {version}" - )); - } - let sha512 = entry - .get("packageHashAlgorithm") - .and_then(Value::as_str) - .is_some_and(|a| a.eq_ignore_ascii_case("SHA512")); - match entry.get("packageHash").and_then(Value::as_str) { - Some(hash) if sha512 && is_base64_digest(hash) => Ok(hash.to_string()), - _ => Err(format!("{catalog} records no SHA512 packageHash")), - } + let bytes = crate::vendor::registry_fetch::download(&self.http, &url).await?; + crate::formats::nuget::package::package_content_hash(&bytes) + .map_err(|why| format!("{url}: {why}")) } .await; self.nuget.lock().await.insert(key, result.clone()); result } - - /// A JSON document that nuget.org may serve gzip-encoded whatever the - /// request asked for (the `registration5-gz-*` hives). - async fn get_json_maybe_gzip(&self, url: &str) -> Result { - use std::io::Read as _; - let mut bytes = crate::vendor::registry_fetch::download(&self.http, url).await?; - if bytes.starts_with(&[0x1f, 0x8b]) { - let mut plain = Vec::new(); - flate2::read::GzDecoder::new(bytes.as_slice()) - .take(crate::vendor::registry_fetch::MAX_DOWNLOAD_BYTES) - .read_to_end(&mut plain) - .map_err(|e| format!("{url}: bad gzip body: {e}"))?; - bytes = plain; - } - serde_json::from_slice(&bytes).map_err(|e| format!("{url} is not JSON: {e}")) - } } /// nuget.org's API host; `SOCKET_NUGET_URL` names another (tests, mirrors -/// serving the same `/v3/registration5-gz-semver2/` hive). +/// serving the same `/v3-flatcontainer/` hive). pub(crate) const DEFAULT_NUGET_API: &str = "https://api.nuget.org"; fn nuget_api_base() -> String { @@ -637,17 +598,6 @@ fn nuget_api_base() -> String { .unwrap_or_else(|| DEFAULT_NUGET_API.to_string()) } -/// A base64 digest token (the alphabet and padding only: the lock stores -/// whatever nuget.org recorded, so its length is not second-guessed). -fn is_base64_digest(s: &str) -> bool { - let body = s.trim_end_matches('='); - !body.is_empty() - && s.len() - body.len() <= 2 - && body - .bytes() - .all(|b| b.is_ascii_alphanumeric() || b == b'+' || b == b'/') -} - /// The release files of a PyPI JSON API version document, sorted by /// filename. fn pypi_release_files(doc: &Value) -> Result, String> { diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs b/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs index 37d370c33..15fd0a077 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs @@ -10,8 +10,9 @@ //! one the user wrote), and a config it created from scratch (identical to //! a user's default config, so it is kept and a warning says so). //! -//! Every lock entry of the id gets nuget.org's `contentHash` back (the -//! catalog `packageHash`, see [`UpstreamClient::nuget_content_hash`]) — only +//! Every lock entry of the id gets nuget.org's `contentHash` back (NuGet's +//! signature-excluded content hash of the `.nupkg` nuget.org serves — not +//! the catalog `packageHash`, see [`UpstreamClient::nuget_content_hash`]) — only //! when the restored config resolves the id from nuget.org alone: another //! feed (or several) may serve different bytes, and socket-patch cannot //! tell which one the original lock came from, so such a pin is refused. @@ -426,8 +427,6 @@ mod tests { use wiremock::{Mock, MockServer, ResponseTemplate}; const UUID: &str = "66666666-6666-6666-6666-666666666666"; - const UPSTREAM: &str = - "ckEKf1MtNHGmiyXVMOQUWA1NhmENd95EZ8h2znGTaccCdgF/RgjlfKWRH+iEdgEx68wOpY+UFhWisuq3tHFA=="; const PATCHED: &str = "PATCHEDcontenthashPATCHEDcontenthashAA=="; fn index_url() -> String { @@ -454,27 +453,41 @@ mod tests { ) } + /// A tiny `.nupkg`, with or without a repository signature entry + /// (appended last, where NuGet's signer puts it). + fn nupkg(signed: bool) -> Vec { + use std::io::Write as _; + let mut zw = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = + zip::write::SimpleFileOptions::default().last_modified_time(zip::DateTime::default()); + let mut files: Vec<(&str, &[u8])> = vec![ + ("newtonsoft.json.nuspec", b""), + ("LICENSE.md", b"The MIT License (MIT)"), + ]; + if signed { + files.push((".signature.p7s", b"repository-signature")); + } + for (name, data) in files { + zw.start_file(name, opts).unwrap(); + zw.write_all(data).unwrap(); + } + zw.finish().unwrap().into_inner() + } + + /// The lock's original `contentHash`: NuGet's content hash, which + /// excludes the signature, i.e. the hash of the unsigned archive. + fn upstream() -> String { + crate::utils::digest::sha512_base64_of(&nupkg(false)) + } + + /// nuget.org's flat container serving the SIGNED package. async fn nuget_org() -> MockServer { let server = MockServer::start().await; - let catalog = format!("{}/catalog0/data/newtonsoft.json.13.0.3.json", server.uri()); Mock::given(method("GET")) .and(path( - "/v3/registration5-gz-semver2/newtonsoft.json/13.0.3.json", + "/v3-flatcontainer/newtonsoft.json/13.0.3/newtonsoft.json.13.0.3.nupkg", )) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(serde_json::json!({ "catalogEntry": catalog })), - ) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path("/catalog0/data/newtonsoft.json.13.0.3.json")) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "id": "Newtonsoft.Json", - "version": "13.0.3", - "packageHash": UPSTREAM, - "packageHashAlgorithm": "SHA512", - }))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(nupkg(true))) .mount(&server) .await; server @@ -515,7 +528,13 @@ mod tests { outcome.pins ); assert_eq!(config, USER_MAPPING); - assert_eq!(lock_after, lock(UPSTREAM)); + // #624: the signature-excluded content hash, never the hash of the + // signed file as served (what the catalog's packageHash records). + assert_ne!( + upstream(), + crate::utils::digest::sha512_base64_of(&nupkg(true)) + ); + assert_eq!(lock_after, lock(&upstream())); assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); } @@ -526,7 +545,7 @@ mod tests { let (outcome, config, lock_after) = run(&hosted_config(&default), false).await; assert_eq!(outcome.pins[0].status, PinStatus::Restored); assert_eq!(config, default); - assert_eq!(lock_after, lock(UPSTREAM)); + assert_eq!(lock_after, lock(&upstream())); assert!(outcome .warnings .iter() diff --git a/crates/socket-patch-core/tests/upstream_restore_golden.rs b/crates/socket-patch-core/tests/upstream_restore_golden.rs index a6049c0b1..ab6c57e7b 100644 --- a/crates/socket-patch-core/tests/upstream_restore_golden.rs +++ b/crates/socket-patch-core/tests/upstream_restore_golden.rs @@ -2394,36 +2394,60 @@ async fn maven_config_merge_keeps_the_resolver_lines() { ); } -/// Serve nuget.org's registration leaf and catalog entry for every package -/// the `input/` lock pins, with the contentHash it records. -async fn nuget_mock(case: &Case) -> MockServer { +/// A deterministic repository-signed `.nupkg` for `id@version` and its +/// NuGet content hash (the signature excluded, so: the hash of the same +/// archive without the signature entry, #624). +fn signed_nupkg(id: &str, version: &str) -> (Vec, String) { + use std::io::Write as _; + let build = |signed: bool| { + let mut zw = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = + zip::write::SimpleFileOptions::default().last_modified_time(zip::DateTime::default()); + zw.start_file(format!("{id}.nuspec"), opts).unwrap(); + write!( + zw, + "{id}{version}" + ) + .unwrap(); + if signed { + zw.start_file(".signature.p7s", opts).unwrap(); + zw.write_all(b"repository-signature").unwrap(); + } + zw.finish().unwrap().into_inner() + }; + let unsigned = build(false); + let hash = { + use base64::Engine as _; + use sha2::Digest as _; + base64::engine::general_purpose::STANDARD.encode(sha2::Sha512::digest(&unsigned)) + }; + (build(true), hash) +} + +/// Serve nuget.org's flat-container `.nupkg` for every package the +/// `input/` lock pins, and re-key the case's locks to that package's +/// content hash. +async fn nuget_mock(case: &mut Case) -> MockServer { let server = MockServer::start().await; let lock: serde_json::Value = serde_json::from_str(case.input.get("packages.lock.json").unwrap()).unwrap(); for fw in lock["dependencies"].as_object().unwrap().values() { for (id, entry) in fw.as_object().unwrap() { let (id, version) = (id.to_lowercase(), entry["resolved"].as_str().unwrap()); - let catalog = format!("{}/catalog0/data/{id}.{version}.json", server.uri()); + let (bytes, hash) = signed_nupkg(&id, version); Mock::given(method("GET")) .and(path(format!( - "/v3/registration5-gz-semver2/{id}/{version}.json" + "/v3-flatcontainer/{id}/{version}/{id}.{version}.nupkg" ))) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(serde_json::json!({ "catalogEntry": catalog })), - ) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path(format!("/catalog0/data/{id}.{version}.json"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "id": id, - "version": version, - "packageHash": entry["contentHash"], - "packageHashAlgorithm": "SHA512", - }))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(bytes)) .mount(&server) .await; + let original = entry["contentHash"].as_str().unwrap(); + for files in [&mut case.input, &mut case.expected] { + for text in files.values_mut() { + *text = text.replace(original, &hash); + } + } } } server @@ -2450,7 +2474,8 @@ async fn nuget_goldens_round_trip() { if NUGET_NOT_INVERTIBLE.contains(&name.as_str()) { continue; } - let server = nuget_mock(&case).await; + let mut case = case; + let server = nuget_mock(&mut case).await; let _env = EnvGuard::set(&[("SOCKET_NUGET_URL", server.uri())]); let (after, statuses) = run_case(&case).await; assert_round_trip(&case, &after, &statuses); @@ -2467,7 +2492,8 @@ async fn nuget_non_invertible_goldens_restore_or_refuse_as_documented() { .into_iter() .find(|c| c.dir.ends_with(name)) .unwrap(); - let server = nuget_mock(&case).await; + let mut case = case; + let server = nuget_mock(&mut case).await; let _env = EnvGuard::set(&[("SOCKET_NUGET_URL", server.uri())]); let (after, statuses) = run_case(&case).await; let [(_, status)] = &statuses[..] else { From 320ce9e3e145f24dd8bbceb88b5371d189549cd4 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 14:40:37 -0400 Subject: [PATCH 5/6] Refuse NuGet archives with out-of-bounds records A central-directory record whose extra or comment length ran past the end of the archive, or a signature entry inconsistent with the directory sizes, could panic the content-hash computation (Bugbot on #1343). Both now refuse the archive instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/formats/nuget/package.rs | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-core/src/formats/nuget/package.rs b/crates/socket-patch-core/src/formats/nuget/package.rs index 380405cc0..ecaf1f11d 100644 --- a/crates/socket-patch-core/src/formats/nuget/package.rs +++ b/crates/socket-patch-core/src/formats/nuget/package.rs @@ -113,6 +113,10 @@ pub(crate) fn package_content_hash(nupkg: &[u8]) -> Result { return Err(truncated(local_offset + entry_size)); } let header_size = 46 + name_len + extra_len + comment_len; + // The whole record is hashed below: it must lie inside the archive. + if at + header_size > nupkg.len() { + return Err(truncated(at + header_size)); + } records.push(Record { position: at, header_size, @@ -134,6 +138,8 @@ pub(crate) fn package_content_hash(nupkg: &[u8]) -> Result { return Err("the package holds nothing but its signature".to_string()); } + let inconsistent = + || "the package's signature entry is inconsistent with its directory".to_string(); let mut hash = Sha512::new(); rest.sort_by_key(|r| r.local_offset); hash.update(&nupkg[..rest[0].local_offset]); @@ -148,14 +154,26 @@ pub(crate) fn package_content_hash(nupkg: &[u8]) -> Result { } else { r.local_offset }; - hash.update((offset as u32).to_le_bytes()); + hash.update( + u32::try_from(offset) + .map_err(|_| inconsistent())? + .to_le_bytes(), + ); hash.update(&nupkg[r.position + 46..r.position + r.header_size]); } hash.update(&nupkg[eocd..eocd + 8]); hash.update((entries_disk - 1).to_le_bytes()); hash.update((entries - 1).to_le_bytes()); - hash.update((cd_size - sig_header_size as u32).to_le_bytes()); - hash.update((cd_offset - sig_entry_size as u32).to_le_bytes()); + let cd_size = u32::try_from(sig_header_size) + .ok() + .and_then(|n| cd_size.checked_sub(n)) + .ok_or_else(inconsistent)?; + let cd_offset = u32::try_from(sig_entry_size) + .ok() + .and_then(|n| cd_offset.checked_sub(n)) + .ok_or_else(inconsistent)?; + hash.update(cd_size.to_le_bytes()); + hash.update(cd_offset.to_le_bytes()); hash.update(&nupkg[eocd + 20..]); Ok(base64::engine::general_purpose::STANDARD.encode(hash.finalize())) } @@ -260,5 +278,14 @@ mod tests { let mut bytes = zip(&FILES, true); bytes.truncate(bytes.len() / 2); assert!(package_content_hash(&bytes).is_err()); + // A central-directory record whose extra/comment lengths run past + // the end of the archive is refused, not sliced out of bounds. + let mut with_sig: Vec<(&str, &[u8])> = FILES.to_vec(); + with_sig.push((".signature.p7s", b"sig")); + let mut bytes = zip(&with_sig, true); + let eocd = find_eocd(&bytes).unwrap(); + let cd = u32_at(&bytes, eocd + 16).unwrap() as usize; + bytes[cd + 32..cd + 34].copy_from_slice(&u16::MAX.to_le_bytes()); + assert!(package_content_hash(&bytes).is_err()); } } From 1862bafc5c632209bbf66fdce114e3053f6d70f5 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Sat, 10 Oct 2026 15:13:06 -0400 Subject: [PATCH 6/6] Use upstream() in #1340's member-lock restore test #1340 (NuGet lock discovery for member and named locks) added a_member_project_lock_is_restored_with_the_root_config, which asserts against the UPSTREAM const. This PR (#624) replaced that const with upstream(), the computed contentHash of the unsigned nupkg, so the merge group failed to compile the core lib tests. Point the new test at upstream() like the rest of the module. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs b/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs index 1516de4cd..387dc006d 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs @@ -632,7 +632,7 @@ mod tests { ); assert_eq!( std::fs::read_to_string(app.join(PACKAGES_LOCK)).unwrap(), - lock(UPSTREAM) + lock(&upstream()) ); }