Fix hosted NuGet lock walk: BOM and other versions (#593, #623) by mikolalysenko · Pull Request #1339 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-09T16:45:42Z

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

Fixes #593
Fixes #623

Summary

NuGet's packages.lock.json was walked by four separate readers (vendored pin, hosted redirect, hosted upstream restore, VEX). The hosted ones parsed with a bare serde_json::from_str and matched entries by id only. They now share one reader, formats::nuget::lock:

parse_lock reads past a leading UTF-8 BOM, as dotnet does.
locked_at / locked_at_mut match the id (case-insensitive) and the normalized resolved version.
other_versions names every other version the lock resolves the id at.

Root cause

Hosted NuGet rewrites packages.lock.json entries at other versions of the patched id #593: hosted rewrite_nuget and hosted restore matched lock entries by id alone. In a multi-targeting lock (net6.0 → 12.0.3, net8.0 → 13.0.3) hosted re-pinned net6.0 to 13.0.3 with the patched hash, so restore silently gave that framework the patched 13.0.3. Vendored pinned only 13.0.3, but its exact-id mapping still routed every version of the id to a feed that serves one, so net6.0 failed NU1102.
Hosted and vendored NuGet reject a packages.lock.json with a UTF-8 BOM that dotnet restores fine: hosted skips the redirect and exits 0 success, vendored fails apply_failed #623: serde_json rejects a BOM. Hosted then skipped the whole redirect with redirect_nuget_lock_unparseable (exit 0, nothing patched), and vendored failed apply_failed.

Fix

Both writers refuse a lock that also resolves the patched id at another version, before writing anything: redirect_nuget_lock_other_version (hosted warning, dep skipped) and vendor_nuget_lock_other_version (vendored refusal). This is the shared gate the issue proposed. packageSourceMapping patterns name ids, never versions, so wiring this lock can't be done correctly.
Hosted re-pins only the entries at the patched version, and changes only contentHash, never resolved. Upstream restore matches the same way.
Hosted writes the lock back in its own layout (serialize_json_like: BOM, indent, line endings), not LF pretty-print. The vendored and restore edits already keep it.
CLI_CONTRACT.md documents both new codes and the BOM handling.

Per-issue tests

Hosted NuGet rewrites packages.lock.json entries at other versions of the patched id #593: patch::redirect::tests::nuget_lock_with_the_id_at_another_version_is_refused and nuget_lock_repins_only_entries_at_the_patched_version; vendor::nuget_feed::tests::lock_with_the_id_at_another_version_is_refused_untouched; formats::nuget::lock::tests::*.
Hosted and vendored NuGet reject a packages.lock.json with a UTF-8 BOM that dotnet restores fine: hosted skips the redirect and exits 0 success, vendored fails apply_failed #623: patch::redirect::tests::nuget_bom_lock_is_redirected_and_keeps_its_bom; vendor::nuget_feed::tests::bom_lock_is_pinned_and_reverted_byte_identically; formats::nuget::lock::tests::bom_lock_parses_like_dotnet_reads_it.

Red→green: before the fix, the hosted BOM test hits redirect_nuget_lock_unparseable, and the other-version test re-pins net6.0 (2 lock edits, no warning).

Commands run

cargo test -p socket-patch-core --no-fail-fast (lib + all integration suites, including redirect_golden and upstream_restore_golden): green
cargo test -p socket-patch-cli --all-features --test e2e_nuget: 21/21
cargo test -p socket-patch-cli --all-features --test e2e_nuget_dotnet_build -- --ignored (local SDK 8.0.129): 2/2
cargo clippy --workspace --all-features -- -D warnings: clean
cargo fmt --all -- --check: clean for touched files. upstream/mod.rs shows a pre-existing local-rustfmt diff that this PR doesn't touch.

Includes #1288's commits (approved, about to merge). Follow-up #1340 (member and named locks) builds on this reader.

🤖 Generated with Claude Code

Note

Medium Risk
Changes NuGet hosted redirect and vendored wiring for multi-targeting locks and BOM locks; incorrect behavior could break restores, but the change is fail-closed with new refusal paths and targeted tests.

Overview
Unifies NuGet packages.lock.json handling across hosted redirect, vendored pin, upstream restore, and VEX via a shared formats::nuget::lock reader that strips UTF-8 BOMs (like dotnet), matches entries by id + normalized version, and detects when the same id is locked at another version.

Hosted and vendored behavior changes (#593, #623): Locks that resolve the patched package id at a different version are refused before any write (redirect_nuget_lock_other_version / vendor_nuget_lock_other_version), because exact-id packageSourceMapping cannot serve multiple versions. Hosted re-pins only matching-version entries and updates contentHash only (never resolved); lock output preserves BOM, indent, and line endings. BOM-prefixed locks are no longer treated as unparseable.

Vendored nuget.config wiring now uses the same formats::nuget::parse_config tokenizer as hosted/restore (replacing ad-hoc comment blanking and key scans), with stricter refusal on malformed or repeated sections.

CLI_CONTRACT.md documents the new refusal codes and BOM behavior.

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

Assisted-by: Claude Code:claude-opus-5-5

Vendored NuGet now reads the source keys and finds the <packageSources>, <packageSourceMapping> and <configuration> 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 (</packageSources >) is now the section that gets extended; vendor used to append a second section NuGet ignores, so restore failed NU1100/NU1403 (#685). - An empty <packageSourceMapping /> 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

…-config' into agent/v5-nuget-locks

Draft placeholder while the fix is written. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Hosted NuGet rewrote every packages.lock.json entry of the patched id, whatever version it resolved, so a multi-targeting project silently got the patched version's bytes in another framework (#593). Vendored only pinned the matching version but still routed every version of the id to a feed serving one, so the other framework failed NU1102. Both modes now refuse such a lock (redirect_ / vendor_nuget_lock_other_version) before writing anything, and only entries at the patched version are re-pinned. A lock starting with a UTF-8 BOM, which dotnet restores fine, made hosted skip the redirect with exit 0 and vendored fail apply (#623). The lock is now read past the BOM, and the BOM and layout are kept on write. All four lock walkers (vendored pin, hosted redirect, upstream restore, VEX) now share one reader in formats::nuget::lock. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) · 2026-10-09T17:15:33Z

BugBot review

Mikola Lysenko (mikolalysenko) · 2026-10-09T17:54:14Z

[final reviewer] I disarmed auto-merge at cbd7bcb7 because ci-ok is red on this head. Only hosted-e2e and e2e (ubuntu-latest, e2e_safety_pnpm) fail, which is the main-wide minimist@1.2.2 failure (#1293), not this PR. #1302 fixes it and is in the merge queue now. Once main carries it, merge main in here (no other commits needed) and I'll re-arm auto-merge after CI goes green. The approval still covers this head.

Generated by Claude Code

…eader # Conflicts: # crates/socket-patch-core/src/vendor/nuget_feed.rs

# Conflicts: # crates/socket-patch-cli/CLI_CONTRACT.md

Resolve conflicts with #1339 (squash-merged NuGet lock walk: BOM, other versions) by keeping this branch's multi-lock generalization, which already carries #1339's BOM read and other-version refusal per lock. CLI_CONTRACT.md merged word-wise with both sides' additions. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Claude (claude) and others added 5 commits October 9, 2026 15:00

Start refactor for #594

9263005

Assisted-by: Claude Code:claude-opus-5-5

Merge remote-tracking branch 'origin/arch-refactor/594-vendored-nuget…

5fa2be6

…-config' into agent/v5-nuget-locks

Start NuGet fix: nuget-lock-reader

f72fc26

Draft placeholder while the fix is written. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 9, 2026 17:15

Mikola Lysenko (mikolalysenko) enabled auto-merge October 9, 2026 17:15

Tanmay Singla (Tanmay182003) approved these changes Oct 9, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) disabled auto-merge October 9, 2026 17:53

This was referenced Oct 9, 2026

Fix NuGet lock discovery for member and named locks (#353, #514) #1340

Merged

Stop warm NuGet cache shadowing patches (#352) #1344

Merged

Merge branch 'main' into agent/v5-nuget-lock-reader

c04be5c

Mikola Lysenko (mikolalysenko) enabled auto-merge October 9, 2026 18:23

Merge remote-tracking branch 'origin/main' into agent/v5-nuget-lock-r…

122bd8d

…eader # Conflicts: # crates/socket-patch-core/src/vendor/nuget_feed.rs

Mikola Lysenko (mikolalysenko) disabled auto-merge October 9, 2026 20:41

Tanmay Singla (Tanmay182003) approved these changes Oct 9, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into HEAD

fe2c56d

# Conflicts: # crates/socket-patch-cli/CLI_CONTRACT.md

Mikola Lysenko (mikolalysenko) enabled auto-merge October 9, 2026 22:07

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026

Mikola Lysenko (mikolalysenko) removed this pull request from the merge queue due to a manual request Oct 9, 2026

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026

Merged via the queue into main with commit 1ae1e11 Oct 9, 2026
53 checks passed

Mikola Lysenko (mikolalysenko) deleted the agent/v5-nuget-lock-reader branch October 9, 2026 22:59

Navigation Menu

Repository navigation

Fix hosted NuGet lock walk: BOM and other versions (#593, #623) - #1339

Fix hosted NuGet lock walk: BOM and other versions (#593, #623)#1339
Mikola Lysenko (mikolalysenko) merged 8 commits into
mainfrom
agent/v5-nuget-lock-reader

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026 •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Fix

Per-issue tests

Commands run

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 9, 2026 •

edited

Loading