Fix NuGet mapping exclusivity and inherited sources (#354, #462)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #354
Fixes #462

Summary

The packageSourceMapping that vendored and hosted NuGet write is now both inherit-safe (#354) and exclusive (#462).

Root cause

Both writers computed the mapping from the single nuget.config they edit, and assumed "most specific pattern wins":

• NuGet nuget.config authored by vendored/hosted mode maps '*' only to nuget.org, cutting off sources inherited from parent or user-level configs (NU1101) #354: a created mapping fans * out only to that file's sources. NuGet merges the user config and every parent directory's config, and once a mapping exists it drops every source no pattern names. Feeds defined outside the file (dotnet nuget add source, a repo-root config above the project) then failed NU1101. A fresh config also re-seeded nuget.org even when a parent had <clear />ed it for a mirror.
• Hosted/vendored NuGet mapping isn't exclusive when nuget.config already maps the exact package id to another source, so restore races the Socket feed against nuget.org (NU1403 with a lock, silently unpatched without one) #462: when another source already names the patched id exactly (Visual Studio's mapping UI writes such lists), the Socket source ties with it. NuGet then takes the package from whichever feed answers first: NU1403 with a lock, silently unpatched without one.

Fix

• formats::nuget: the reader records sources_cleared. A <clear /> drops the sources the file defined before it. effective_source_keys merges a chain of configs, farthest first. It also records mapping_spans (byte ranges of each <packageSource> and its pattern tags).
• vendor::nuget_config::inherited_source_keys reads the chain: the user config (DOTNET_CLI_HOME or HOME / .nuget/NuGet/NuGet.Config, %APPDATA% on Windows; when absent, NuGet's implicit nuget.org), then each parent directory's config from the root down.
• NuGet nuget.config authored by vendored/hosted mode maps '*' only to nuget.org, cutting off sources inherited from parent or user-level configs (NU1101) #354: a created catch-all also names the inherited sources, unless the file itself <clear />s them. nuget.org is seeded only when the inherited set has it or is empty. In the common case (the user config has only nuget.org) the output is byte-identical to before. The hosted engine passes the inherited keys to the pure rewriter through a synthetic candidate key (as sbt does). The in-memory engine keeps the file-only reading. The upstream restore drops a pure * fan-out that covers the file's sources (it may name inherited ones too).
• Hosted/vendored NuGet mapping isn't exclusive when nuget.config already maps the exact package id to another source, so restore races the Socket feed against nuget.org (NU1403 with a lock, silently unpatched without one) #462: set_aside_competing_patterns comments out every other (non-Socket) source's exact pattern for the id, in <!-- socket-patch-<uuid> moved: … -->. When that pattern was the element's only one, the whole <packageSource> is commented, since NuGet rejects an element with no pattern. restore_set_aside turns the comments back into the original bytes. Hosted warns redirect_nuget_mapping_set_aside, and vendored warns vendor_nuget_mapping_set_aside. Markup that can't go in a comment is skipped with redirect_nuget_mapping_conflict (vendored fails the package). The vendored whole-file revert and its excision path, and the hosted upstream restore (remove / rollback), all restore the pattern. VEX already treats only an exclusive mapping as live, so it now sees the wiring as exclusive.
• CLI_CONTRACT.md documents the new codes and behavior.

Per-issue tests

• NuGet nuget.config authored by vendored/hosted mode maps '*' only to nuget.org, cutting off sources inherited from parent or user-level configs (NU1101) #354: vendor::nuget_config::tests::inherited_sources_follow_nugets_merge_order (user config, parent config, <clear />, implicit default); vendor::nuget_feed::tests::created_catch_all_names_inherited_sources, fresh_config_follows_the_inherited_sources, vendor_keeps_a_parent_directorys_feed_routable (end to end through vendor_nuget); patch::redirect::tests::nuget_created_catch_all_names_inherited_sources; formats::nuget::tests::clear_drops_earlier_and_farther_sources.
• Hosted/vendored NuGet mapping isn't exclusive when nuget.config already maps the exact package id to another source, so restore races the Socket feed against nuget.org (NU1403 with a lock, silently unpatched without one) #462: formats::nuget::tests::competing_exact_patterns_are_set_aside_and_restored; patch::redirect::tests::nuget_competing_exact_pattern_is_set_aside (the issue's config, plus an idempotent re-run); vendor::nuget_feed::tests::competing_exact_mapping_is_set_aside_and_restored (revert through the excision path); upstream::nuget::tests::a_set_aside_pattern_is_restored.

Red→green: before the fix, the inherited-source tests get a catch-all of the file's keys only, and the #462 tests leave two sources with the exact id.

Commands run

• cargo test -p socket-patch-core --no-fail-fast: green
• cargo test -p socket-patch-cli --all-features --test e2e_nuget (21) and --test e2e_nuget_dotnet_build -- --ignored (2, local SDK 8.0.129): green
• cargo clippy --workspace --all-features -- -D warnings: clean. cargo fmt --check: clean for touched files.

Review follow-ups

• An inherited config that already has a packageSourceMapping gets no * catch-all, only the Socket pattern, because NuGet merges inherited mappings and a catch-all would widen a source they restrict.
• The set-aside exempts only the source this run wires. A socket-patch-* lookalike key or a stale uuid that names the id is set aside like any other competitor (security review).
• A <package> element written with a close tag is set aside whole (Bugbot).
• The hosted engine reads the user and parent configs outside the project view and records each probed path with the view, so a re-scan's read cache keeps recording.

Notes for review

• Machine-wide configs (/etc/opt/NuGet/Config, %ProgramFiles(x86)%\NuGet\Config) and ~/.nuget/config/*.config aren't read. A source added only at restore time on another machine (e.g. CI's dotnet nuget add source under a name the scanning machine lacks) still needs a mapping entry.
• Includes Wire vendored nuget.config through formats::nuget (#594) #1288's commits (approved, about to merge).

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes how nuget.config and hosted redirects are authored—incorrect mapping could break restores or route packages to the wrong feed—but behavior is fail-closed with explicit warnings and broad unit/e2e coverage.

Overview
NuGet packageSourceMapping is now written to match how NuGet actually merges configs and resolves patterns, for both vendored feeds and hosted redirects.

When creating a catch-all * mapping, the writers now include package sources inherited from the user config and parent-directory NuGet.Config files (honoring <clear />), and only seed nuget.org when that inherited set still has it—so private feeds above the project stay routable and mirrors that cleared nuget.org are not reintroduced. Hosted scan loads those keys from disk via a synthetic candidate; vendoring uses the same inherited_source_keys helper.

When another feed already maps the patched package id exactly (a common Visual Studio pattern), that competing pattern is temporarily commented out in <!-- socket-patch-<uuid> moved: … --> while Socket wiring is active, with restore on rollback/remove/vendor revert. Un-commentable markup skips the redirect with redirect_nuget_mapping_conflict; successful set-asides warn as redirect_nuget_mapping_set_aside / vendor_nuget_mapping_set_aside.

The shared formats::nuget parser now tracks sources_cleared, mapping byte spans, and replaces vendored config editing’s ad-hoc XML scanning with the same tokenizer hosted/restore/VEX use. CLI_CONTRACT.md documents the new redirect codes and vendored NuGet behavior.

^{Reviewed by Cursor Bugbot for commit e5e3c5a. Configure here.}

[Mikola Lysenko (mikolalysenko)]

BugBot review

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e5e3c5a. Configure here.}

[cursor]

Set-aside omits package close tags

Medium Severity

Pattern spans record only the open tag of a <package> element. When that tag is not self-closing, set_aside_competing_patterns comments the opener and leaves a dangling </package>, which makes the config unparseable while the patch is wired.

Additional Locations (1)

• crates/socket-patch-core/src/formats/nuget/mod.rs#L397-L401

 

^{Reviewed by Cursor Bugbot for commit e5e3c5a. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[agent] Fixed in c696181: the pattern span now runs through </package> for a non-self-closing element, so the whole element is set aside and restored byte-exact (test extended with the open/close spelling).

[Tanmay Singla (Tanmay182003)]

[agent] This fans the created * catch-all out to every inherited source key, but nuget_config::inherited_source_keys (nuget_config.rs:94) only collects inherited sources, not inherited <packageSourceMapping>. NuGet merges mappings across the config chain, so if a parent/user NuGet.Config already maps e.g. nuget.org to Newtonsoft.* only and * to corp, the child's new <packageSource key="nuget.org"><package pattern="*" /> widens nuget.org to every package — re-opening the dependency-confusion path the parent mapping closes. Fix: when an inherited mapping exists, don't fan * to keys it already covers (or leave the inherited mapping governing non-socket packages and only add the socket pattern).

[Mikola Lysenko (mikolalysenko)]

[agent] Fixed in 8e1d8e4. (1) Inherited mappings: inherited_sources now also reports whether any inherited config maps packages; when one does, neither writer writes a * catch-all (only the Socket pattern), since NuGet merges the inherited patterns and a catch-all here would widen a restricted source. Tests: inherited_mapping_gets_no_catch_all, inherited_mapping_is_reported, hosted nuget_created_catch_all_names_inherited_sources. (2) Set-aside: only the key this run wires is exempt; socket-patch-* lookalikes and stale uuids are set aside like any other competitor (test in competing_exact_patterns_are_set_aside_and_restored).

[cursor]

🔒 Agentic Security Review

Severity: MEDIUM

Description: Hosted NuGet set-aside writes the patch-service uuid into an XML comment with no canonical-uuid or -- check. A uuid containing --> closes that comment and the remainder is committed as live nuget.config markup. Attribute encoding on the <add key> / <packageSource key> writes does not cover this prefix.

Impact: A hosted NuGet grant is only supposed to add one source and an exact-id mapping. On a file that already has another source's exact pattern, a non-canonical patch_uuid can rewrite that packageSourceMapping in place: <clear /> plus a <packageSource> whose key is not a real source makes restore skip every configured feed for every package id. NuGet keeps only the first packageSources and first packageSourceMapping element, so a second <packageSources><add> after an existing sources section is ignored and does not become an extra feed. When packageSourceMapping is ordered before packageSources, the same breakout can emit the first packageSources element (clear plus an attacker URL) and empty the mapping, so that URL is the only feed restore uses. That is a larger blast radius than the single package the grant was allowed to redirect. Cargo rewrite and vendored NuGet already reject a non-canonical uuid before writing config.

Remediation: In rewrite_nuget, skip the dependency unless patch_uuid passes is_canonical_uuid, matching rewrite_cargo and vendor_nuget. Also fail set_aside_competing_patterns when the generated key contains --, so the comment prefix cannot close itself.