diff --git a/CHANGELOG.md b/CHANGELOG.md index c905badf..b85d8bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -914,6 +914,22 @@ into the new version's section — see docs/releasing.md. ### Fixed +- **Hosted nuget redirects survive a `` in `nuget.config`.** The + Socket source (and, in an existing ``, its + mapping) was inserted ahead of the section's ``, which NuGet + applies to everything read before it: `dotnet restore` then failed + NU1100 / NU1101 for the patched package. Both now land after the last + ``. + +- **nuget redirects and vendoring edit the config NuGet actually reads.** + NuGet reads the first of `nuget.config`, `NuGet.config` and + `NuGet.Config` in a directory. Hosted mode only knew `nuget.config` + and vendored mode missed `NuGet.config`, so on a case-sensitive + filesystem they created a fresh `nuget.config` that shadowed the + project's own file: its sources and mappings vanished and private + packages failed restore. Both modes now edit the existing spelling in + place. + - **`rollback` fetches a before-blob that only a store peer variant needs.** The before-blob gate now probes every pnpm and vlt store variant copy the rollback restores, so an online rollback no longer fails diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 7cf3cc12..df813bf0 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -72,7 +72,11 @@ pub(crate) const REDIRECT_CANDIDATE_FILES: &[&str] = &[ // otherwise the `[registries.…]` block lands in a file cargo ignores. ".cargo/config", "composer.lock", + // Every spelling NuGet probes: the rewriter edits the one NuGet reads + // rather than shadowing it with a fresh `nuget.config`. "nuget.config", + "NuGet.config", + "NuGet.Config", "packages.lock.json", "Gemfile", "Gemfile.lock", diff --git a/crates/socket-patch-cli/src/hosted_memory/redirect.rs b/crates/socket-patch-cli/src/hosted_memory/redirect.rs index 7848425c..4a876341 100644 --- a/crates/socket-patch-cli/src/hosted_memory/redirect.rs +++ b/crates/socket-patch-cli/src/hosted_memory/redirect.rs @@ -178,7 +178,7 @@ fn file_ecosystem(rel: &str) -> Option<&'static str> { | "pyproject.toml" | "hatch.toml" => "pypi", "Cargo.toml" | "Cargo.lock" | "config.toml" | "config" => "cargo", "composer.lock" => "composer", - "nuget.config" | "packages.lock.json" => "nuget", + "nuget.config" | "NuGet.config" | "NuGet.Config" | "packages.lock.json" => "nuget", "Gemfile" | "Gemfile.lock" | "gems.rb" | "gems.locked" => "gem", "go.mod" | "go.sum" => "golang", "pom.xml" | "maven.config" | "checksums.sha256" => "maven", diff --git a/crates/socket-patch-cli/src/hosted_memory/roots.rs b/crates/socket-patch-cli/src/hosted_memory/roots.rs index 002cac15..bb5e15f8 100644 --- a/crates/socket-patch-cli/src/hosted_memory/roots.rs +++ b/crates/socket-patch-cli/src/hosted_memory/roots.rs @@ -48,7 +48,15 @@ pub(crate) const UNSUPPORTED_MARKERS: [(&str, &[&str]); 2] = [ "settings.gradle.kts", ], ), - ("nuget", &["packages.lock.json", "nuget.config"]), + ( + "nuget", + &[ + "packages.lock.json", + "nuget.config", + "NuGet.config", + "NuGet.Config", + ], + ), ]; /// Directory names whose subtrees never hold a project root: installed diff --git a/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs b/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs index 1a8c2a21..c563da33 100644 --- a/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs +++ b/crates/socket-patch-cli/tests/in_process_get_hosted_ecosystems.rs @@ -651,6 +651,72 @@ async fn nuget_hosted_wires_source_mapping_and_lock_hash() { .unwrap(); } +/// NuGet reads `NuGet.config` when no `nuget.config` exists: the grant must +/// wire that file in place, not author a `nuget.config` that shadows it. +#[tokio::test] +#[serial] +async fn nuget_hosted_wires_mixed_case_config_in_place() { + const UUID: &str = "c4c4c4c4-c4c4-4c4c-8c4c-c4c4c4c4c4c4"; + const PURL: &str = "pkg:nuget/Newtonsoft.Json@13.0.3"; + let index_url = format!("http://patch.test/patch-registry/nuget/{TOKEN}/{UUID}/index.json"); + let url = format!( + "http://patch.test/patch-registry/nuget/{TOKEN}/{UUID}/flat/newtonsoft.json/13.0.3/newtonsoft.json.13.0.3.nupkg" + ); + + let server = MockServer::start().await; + mock_view(&server, UUID, PURL).await; + mock_reference( + &server, + UUID, + PURL, + &url, + serde_json::json!({ "sha512": "sha512-NUGETPATCHED==" }), + serde_json::json!({ + "kind": "nuget-v3", + "indexUrl": index_url, + "identifiers": { + "name": "Newtonsoft.Json", + "version": "13.0.3", + "nugetIdLower": "newtonsoft.json", + "nugetVersionNorm": "13.0.3", + } + }), + ) + .await; + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("NuGet.config"), + r#" + + + + + +"#, + ) + .unwrap(); + let case_sensitive = !tmp.path().join("nuget.config").exists(); + + let code = + socket_patch_cli::commands::get::run(get_hosted_args(UUID, tmp.path(), server.uri())).await; + assert_eq!(code, 0, "get --mode hosted (nuget) should succeed"); + + let config = std::fs::read_to_string(tmp.path().join("NuGet.config")).unwrap(); + assert!( + config.contains(&format!( + r#""# + )) && config.contains(r#" String { "\n\n \n \n \n\n".to_string() } @@ -5152,7 +5157,10 @@ fn add_nuget_source(config: &str, reg: &str, index_url: &str, pkg_id: &str) -> O // socket-only and every other package would fail. Seed the implicit default // nuget.org source so the catch-all has a real target (unless the config // already has one). Only relevant when we are about to CREATE the mapping. - let creating_mapping = !out.contains(""); + // The open tag may carry whitespace or attributes (`` + // is valid XML); a literal probe reads it as absent and authors a + // DUPLICATE section. + let creating_mapping = nuget_mapping_open_end(&out).is_none(); // "Already has one" is decided by the parsed keys ALONE: // a whole-file "nuget.org" probe is satisfied by text that defines no // source (a defaultPushSource URL, a entry, a @@ -5173,11 +5181,11 @@ fn add_nuget_source(config: &str, reg: &str, index_url: &str, pkg_id: &str) -> O // A mapping already exists (e.g. a prior patched dep, or the project's // own): append ONLY this source's mapping — every other source is // already covered. - out = out.replacen( - "", - &format!("\n{socket_mapping}"), - 1, - ); + // After any `` in the section: NuGet drops every mapping + // read before one, leaving the patched id routed nowhere. + let open_end = nuget_mapping_open_end(&out)?; + let at = nuget_after_last_clear(&out, open_end, "packageSourceMapping"); + out = format!("{}\n{socket_mapping}{}", &out[..at], &out[at..]); } else { // Creating the mapping from scratch. Once ANY // exists, NuGet requires EVERY package to match some source's pattern, @@ -5249,7 +5257,9 @@ fn insert_nuget_source(config: &str, key: &str, url: &str) -> Option { // through to the from-scratch branch rather than insert outside it. .filter(|m| !m.as_str().ends_with("/>")) { - let end = m.end(); + // After any ``: NuGet drops every source read before one, + // so the mapping would point at an undefined source (NU1100). + let end = nuget_after_last_clear(config, m.end(), "packageSources"); Some(format!( "{}\n{source_line}{}", &config[..end], @@ -5271,6 +5281,46 @@ fn insert_nuget_source(config: &str, key: &str, url: &str) -> Option { } } +/// The offset just past the `` open tag (any whitespace +/// or attributes), or `None` when the config has no open/close section — a +/// self-closing `` holds no children to append to. +fn nuget_mapping_open_end(config: &str) -> Option { + static OPEN_RE: LazyLock = LazyLock::new(|| { + Regex::new(r"]*)?>") + .expect("static packageSourceMapping open-tag regex is valid") + }); + OPEN_RE + .find(config) + .filter(|m| !m.as_str().ends_with("/>")) + .map(|m| m.end()) +} + +/// The offset just past the last `` between `from` and the `section` +/// element's close tag (any whitespace before `>`), else `from`. Comments are +/// skipped: a commented-out `` clears nothing, and anchoring on it +/// would splice the new entry INSIDE the comment. +fn nuget_after_last_clear(config: &str, from: usize, section: &str) -> usize { + static CLEAR_RE: LazyLock = + LazyLock::new(|| Regex::new(r"").expect("static clear-tag regex is valid")); + static COMMENT_RE: LazyLock = + LazyLock::new(|| Regex::new(r"(?s)").expect("static comment regex is valid")); + // Blank comment bytes in place so offsets still index `config`. + let mut masked = config.as_bytes()[from..].to_vec(); + for m in COMMENT_RE.find_iter(&config[from..]) { + masked[m.range()].fill(b' '); + } + let masked = String::from_utf8(masked).expect("only whole comments are blanked"); + let close_re = + Regex::new(&format!(r"")).expect("section close-tag regex is valid"); + let Some(close) = close_re.find(&masked) else { + return from; + }; + CLEAR_RE + .find_iter(&masked[..close.start()]) + .last() + .map_or(from, |m| from + m.end()) +} + /// The `key` of every `` under `` (empty when there /// is no such element). Used to preserve resolution for non-patched packages /// when a `` is introduced. @@ -5327,13 +5377,19 @@ fn rewrite_nuget( if nuget.is_empty() { return; } + // NuGet reads the first of these spellings present in a directory; a + // fresh `nuget.config` beside a `NuGet.config` would shadow it. + let config_path = NUGET_CONFIG_FILE_NAMES + .into_iter() + .find(|name| files.contains_key(*name)) + .unwrap_or(NUGET_CONFIG_FILE_NAMES[0]); let mut config = files - .get("nuget.config") + .get(config_path) .cloned() .unwrap_or_else(default_nuget_config); // A config this run authors from scratch records its source edits as // `added` — the spelling every other rewriter uses for a created file. - let source_action = if files.contains_key("nuget.config") { + let source_action = if files.contains_key(config_path) { "rewritten" } else { "added" @@ -5412,7 +5468,7 @@ fn rewrite_nuget( config = updated; config_changed = true; result.edits.push(FileEdit { - path: "nuget.config".into(), + path: config_path.into(), kind: "redirect_nuget_source".into(), action: source_action.into(), key: Some(reg.clone()), @@ -5475,7 +5531,7 @@ fn rewrite_nuget( } if config_changed { - result.files.insert("nuget.config".into(), config); + result.files.insert(config_path.into(), config); } if lock_changed { if let Some(lock_val) = lock { @@ -8275,6 +8331,141 @@ mod tests { ); } + /// NuGet's `` drops every item read before it: a Socket source + /// or mapping inserted ahead of one vanishes and restore NU1100s. + #[test] + fn nuget_socket_entries_land_after_clear() { + let mut files = BTreeMap::new(); + files.insert( + "nuget.config".to_string(), + "\n\n \n \n \n \n\n" + .to_string(), + ); + let r = rewrite_registry_redirect(&files, &[nuget_override()]); + let out = r.files.get("nuget.config").expect("config rewritten"); + assert!( + out.contains( + " \n \n , ahead of the other sources: {out}" + ); + } + + #[test] + fn nuget_existing_mapping_socket_entry_lands_after_clear() { + let mut files = BTreeMap::new(); + files.insert( + "nuget.config".to_string(), + "\n\n \n \n \n \n \n \n \n \n \n \n \n \n \n\n" + .to_string(), + ); + let r = rewrite_registry_redirect(&files, &[nuget_override()]); + let out = r.files.get("nuget.config").expect("config rewritten"); + assert!( + out.contains(" \n : {out}" + ); + assert!( + out.contains( + " \n \n " + ), + "socket mapping after the mapping's : {out}" + ); + assert!( + out.contains(" \n \n "), + "a in another section is not an anchor: {out}" + ); + } + + /// Close tags with whitespace before `>` are valid XML: a literal probe + /// misses the section, falls back to the open tag and lands the Socket + /// entries ahead of the `` that drops them. + #[test] + fn nuget_after_clear_tolerates_spaced_tags() { + let mut files = BTreeMap::new(); + files.insert( + "nuget.config".to_string(), + "\n\n \n \n \n \n \n \n \n \n \n \n\n" + .to_string(), + ); + let r = rewrite_registry_redirect(&files, &[nuget_override()]); + let out = r.files.get("nuget.config").expect("config rewritten"); + assert!( + out.contains(" \n \n : {out}" + ); + assert!( + out.contains( + " \n \n " + ), + "socket mapping after the mapping's : {out}" + ); + assert_eq!( + out.matches("` clears nothing; anchoring on it would + /// splice the Socket source inside the comment. + #[test] + fn nuget_commented_clear_is_not_an_anchor() { + let mut files = BTreeMap::new(); + files.insert( + "nuget.config".to_string(), + "\n\n \n \n \n \n \n\n" + .to_string(), + ); + let r = rewrite_registry_redirect(&files, &[nuget_override()]); + let out = r.files.get("nuget.config").expect("config rewritten"); + assert!( + out.contains(" \n : {out}" + ); + assert!( + out.contains(" \n"), + "comment left intact: {out}" + ); + } + + /// NuGet reads the first of `nuget.config`, `NuGet.config`, + /// `NuGet.Config` present; authoring `nuget.config` beside another + /// spelling would shadow the project's sources. + #[test] + fn nuget_config_spelling_is_rewritten_in_place() { + let config = "\n\n \n \n \n\n"; + for (present, expected) in [ + (&["NuGet.config"][..], "NuGet.config"), + (&["NuGet.Config"][..], "NuGet.Config"), + (&["NuGet.Config", "NuGet.config"][..], "NuGet.config"), + (&["NuGet.Config", "nuget.config"][..], "nuget.config"), + ] { + let files: BTreeMap = present + .iter() + .map(|name| (name.to_string(), config.to_string())) + .collect(); + let r = rewrite_registry_redirect(&files, &[nuget_override()]); + assert_eq!( + r.files.keys().collect::>(), + vec![expected], + "{present:?}" + ); + assert!( + r.files[expected].contains("key=\"corp-feed\""), + "{present:?}: {}", + r.files[expected] + ); + let source_edit = r + .edits + .iter() + .find(|e| e.kind == "redirect_nuget_source") + .expect("source edit recorded"); + assert_eq!(source_edit.path, expected, "{present:?}"); + assert_eq!(source_edit.action, "rewritten", "{present:?}"); + } + } + fn berry_override(name: &str, version: &str, url: &str, checksum: &str) -> DepOverride { DepOverride { integrity: Integrity { diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index 61ed8c6d..37c3acae 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -1140,10 +1140,10 @@ struct ConfigEdit { mapping_fragment: String, } -/// Resolve the existing `nuget.config` (prefer lowercase `nuget.config`, then -/// `NuGet.Config`), or `None` when the project has none. +/// Resolve the existing config in NuGet's own probe order, or `None` when the +/// project has none. async fn existing_config_path(project_root: &Path) -> Option { - for name in ["nuget.config", "NuGet.Config"] { + for name in crate::patch::redirect::NUGET_CONFIG_FILE_NAMES { let p = project_root.join(name); // Answers from a group-committed run's capture: a config an earlier // package of this run created is not on disk yet. @@ -2535,6 +2535,41 @@ mod tests { ); } + /// NuGet reads `NuGet.config` when no `nuget.config` exists; creating + /// one beside it would shadow the project's own sources. + #[tokio::test] + async fn wiring_edits_mixed_case_nuget_config_in_place() { + let orig_cfg = "\n\ + \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \n"; + let (dir, blobs, installed, record) = fixture(true, None).await; + let root = dir.path(); + tokio::fs::write(root.join("NuGet.config"), orig_cfg) + .await + .unwrap(); + let case_sensitive = !root.join("nuget.config").exists(); + + let (_r, entry, _w) = + unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); + let entry = entry.unwrap(); + assert_eq!(entry.wiring[0].action, WiringAction::Rewritten); + let wired = tokio::fs::read_to_string(root.join("NuGet.config")) + .await + .unwrap(); + assert!(wired.contains(&format!("socket-patch-{UUID}")), "{wired}"); + assert!(wired.contains("key=\"corp\""), "{wired}"); + if case_sensitive { + assert_eq!(entry.wiring[0].file, "NuGet.config"); + assert!( + !root.join("nuget.config").exists(), + "no shadowing nuget.config created" + ); + } + } + #[tokio::test] async fn revert_excises_only_our_source_preserving_sibling() { // A pre-existing config. Vendor wires OUR source + mapping. Then a