diff --git a/docs/design/nuget-vendoring-research/adversarial-env.md b/docs/design/nuget-vendoring-research/adversarial-env.md new file mode 100644 index 000000000..20b9e347b --- /dev/null +++ b/docs/design/nuget-vendoring-research/adversarial-env.md @@ -0,0 +1,162 @@ +# Adversarial review: NuGet vendoring v2 (unique-version fallback seed) on real environments and CI + +I found one blocker and seven majors. The blocker is that git line-ending normalization breaks the committed seed. On a fresh CI clone the build fails with SOCKETPATCH002, while it still passes on the author's machine. The design's core behaviour survived the attacks I ran: the redirects, SOCKETPATCH005, the Docker-style bypass and symlinked checkouts all behaved as designed. + +**How I tested.** SDK 8.0.131 with HOME, NUGET_PACKAGES, NUGET_HTTP_CACHE_PATH, TMPDIR and DOTNET_CLI_HOME isolated under `scratchpad/adv-env/`, one dotnet process at a time. I built a Lib/App sln fixture with locks. `adv-env/r1/.socket/vendor/nuget/socket-patch.targets` is the §6.3 targets file rendered almost exactly (seed-presence check, direct redirect and the full guard). I added only two `Message` lines for diagnostics. The seed is the design-D 13.0.1 seed under a uuid root. Clones of the fixture are in `adv-env/c1`, `c2` and `c3`. Git experiments are in `adv-env/git1`. + +Labels: **VERIFIED** means I ran it here. **REASONED** means I did not run it. + +--- + +## BLOCKER + +### B1. Git EOL normalization changes seed bytes, so every fresh clone fails SOCKETPATCH002 (VERIFIED) +- **Scenario.** + - The repo has `* text=auto` in `.gitattributes`. That is the Visual Studio template default and is common in .NET repos. + - Newtonsoft's nuspec, `LICENSE.md` and `lib/*/Newtonsoft.Json.xml` are CRLF. I checked: 38/38, 20/20 and 11305/11305 lines are CRLF. + - Git stores them as LF in the index. The author's working tree stays CRLF and `git status` is clean, so every local build passes. +- **Wrong outcome.** + - On a fresh CI clone the files check out as LF. The inventory check then fails: + - `error SOCKETPATCH002: seed modified: …/13.0.1.1843260417/LICENSE.md` + - VERIFIED in `adv-env/c1`, built after `git clone`. + - The same class of failure also occurs with: + - `core.autocrlf=true` (the Git for Windows installer default): LF text files become CRLF on checkout. VERIFIED: a committed LF `x.json` came back `\r\n` in the autocrlf clone. + - `core.autocrlf=input` on the author's machine. + - `*.xml text eol=crlf` rules. + - A root LFS rule such as `*.dll filter=lfs`. With `actions/checkout`'s default `lfs: false`, CI gets pointer files and fails SOCKETPATCH002. That failure is loud, but it is a surprise. +- **Fix.** + - The CLI writes `.socket/vendor/nuget/.gitattributes` **before** the user's first `git add`: + ``` + /*-*-*-*-*/** -text -filter -diff -merge + socket-patch.targets text eol=lf + .gitignore text eol=lf + ``` + - VERIFIED: `git check-attr` shows text, filter and eol all unset under a hostile root file (`* text=auto`, `*.dll filter=lfs`, `*.xml text eol=crlf`). An autocrlf=true clone then passes the hash check, and `dotnet build` succeeds (`adv-env/c2`). + - Blobs committed before the attribute existed stay normalized. I had to `git rm --cached` and re-add them. + - So `vendor --check` must compare the inventory against the committed blobs (`git cat-file` of `HEAD:`), not the working tree. It should also check `git ls-files --eol` and refuse or fix with `vendor_nuget_eol_normalized`. Otherwise the author's green local run hides the CI failure. + - Add a golden-image e2e leg: commit, then `git clone`, then build. Run it with both `text=auto` and `autocrlf=true`. + +--- + +## MAJOR + +### M1. The same V′ in the GPF (same bytes) fails SOCKETPATCH003 and prints destructive advice (VERIFIED) +- **Scenario.** A shared machine or self-hosted runner with a shared `~/.nuget/packages`, where one repo uses the feed tier (§10) or, later, hosted V′ (§11.2) for the same patch. Those tiers extract `//` into the GPF, and the GPF beats fallback folders at asset resolution. +- **Wrong outcome.** + - The fallback-tier repo resolves the GPF copy and fails: + - `error SOCKETPATCH003: foreign …/gpf/newtonsoft.json/13.0.1.1843260417/lib/netstandard2.0/Newtonsoft.Json.dll` + - This happens even with byte-identical files. VERIFIED in `adv-env/c2`: I copied the seed into the GPF, `restore --locked-mode` succeeded, and the build failed. + - The error text says "Delete that package folder". Doing that breaks the other repo, which re-extracts the package, and the two repos keep undoing each other. + - This contradicts the design's own rule that one V′ is always one byte sequence (§6.1). +- **Fix.** + - Key `SocketPatchNuGetHashes` by `//`, not by `/…`. Hash the consumed files wherever they resolved. + - Accept files from a foreign root when all of them match the inventory. Fail SOCKETPATCH003 only when they differ. This is also the feed-tier guard, so the two tiers share one code path. + - Alternatively, add a tier bit to V′ so fallback V′ never appears in any feed. That costs a bit of the 29-bit uuid space. + +### M2. The guard runs in design-time builds, contrary to §9 and G9 (VERIFIED) +- **Scenario.** VS, Rider or C# Dev Kit run a design-time build (`ResolveAssemblyReferencesDesignTime`, `DesignTimeBuild=true`). +- **Wrong outcome.** + - `ResolvePackageAssets` runs, so the `AfterTargets` guard runs too. VERIFIED: `SPGUARD-RAN DesignTimeBuild='true'`. + - Any SOCKETPATCH001/002/003/005 condition, such as a stale restore right after `git pull`, fails the design-time build. IntelliSense then shows "project load" errors and references go unresolved on every edit. + - §9 states "The guard runs only in real builds", which is incorrect. +- **Fix.** Add `and '$(DesignTimeBuild)' != 'true'` to the guard condition. Optionally, emit SOCKETPATCH005/003 as warnings when `$(BuildingInsideVisualStudio)` is true and keep the errors for the command line. Fix the §9 text. + +### M3. Windows paths are about 80 characters deeper than the GPF (REASONED, lengths computed) +- **Scenario.** The seed sits at `.socket\vendor\nuget\<36-char uuid>\\\…`. + - Under the GitHub Actions workspace, `D:\a\my-service-repo\my-service-repo\` plus the Microsoft.Extensions.DependencyInjection.Abstractions `lib/netstandard2.1/*.xml` gives **242** characters. + - The repo-relative part is only 205, so the §9 warning ("seed path > 240") never fires. + - `runtimes//native/…` and satellite paths are deeper still. +- **Wrong outcome.** + - Git for Windows (`core.longpaths` is false by default) fails checkout with "Filename too long". + - MSBuild.exe on .NET Framework and some tools hit MAX_PATH. The same package works fine from `%USERPROFILE%\.nuget\packages`. +- **Fix.** + - Use a uuid8 directory name (with collision refusal). + - Compute the warning on an **absolute** path under a pessimistic prefix: 60 characters for CI, or the real root. + - Refuse above 250 unless `--allow-long-paths`. + - Document `git config core.longpaths true`. + - Add the Windows leg to the prototype acceptance criteria, not only to promotion. + +### M4. EOL-sensitive byte comparisons in the hot path, `--check` and revert break on Windows and on mixed teams (REASONED; the conversion is VERIFIED in B1) +- **Scenario.** `fallback_in_sync` and `--check` require the targets file and `.gitignore` to be byte-equal to a fresh render. Lock and import records compare the live text with the recorded `new` or `original`. + - With autocrlf=true, a Windows checkout turns LF-committed files into CRLF (VERIFIED for `.json`). +- **Wrong outcome.** + - `vendor --check` fails on every Windows CI job. + - The hot path re-renders on every run. + - Lock and import revert on Windows treats every record as drift. It keeps the seed, and the revert is not clean. +- **Fix.** + - Pin `eol=lf` on the generated files via B1's `.gitattributes`. + - For user-tree files (locks, `Directory.Build.targets`), compare after normalizing CRLF to LF. Splice using the live file's detected EOL. + - Store records as EOL-neutral text plus the observed EOL. + +### M5. Mixed packages.config and SDK repos are left undefined (REASONED) +- **Scenario.** An enterprise solution has old packages.config web apps and new SDK libraries that use the same id@V. This is very common. +- **Wrong outcome.** + - §5 and §7.1 route packages.config to legacy and allow one tier per repo. `vendor_nuget_layout_mixed` refuses a same-id legacy entry. + - So either the whole patch is refused, or the legacy path runs. The legacy path brings back the nuget.config catch-all mapping and same-version GPF poisoning for the whole machine, which is exactly what v2 set out to remove. + - The design never says which of the two happens. +- **Fix.** + - Define a single entry with two wiring sets: the legacy feed for packages.config projects only, and the fallback tier for SDK projects. + - Alternatively, refuse explicitly with `vendor_nuget_mixed_project_styles` and describe the options. + - Either way, add a docker leg that uses nuget.exe under mono (for example the `mono` image), since no packages.config coverage exists today. + +### M6. `vendor --check` and the hot path depend on restore outputs (REASONED) +- **Scenario.** A typical CI job runs `git clean -fdx` (or starts from a fresh checkout), then `socket-patch vendor --check`, then `dotnet restore`. +- **Wrong outcome.** + - Lockless projects have no `obj/project.assets.json`. + - `plan_closure` then refuses with `vendor_nuget_unrestored`, or reports `closure_changed`, so the pre-restore check fails for no real reason. +- **Fix.** + - Persist the closure in state and make `--check` validate against it. + - Recompute the closure only when assets are present. + - Add a fallback that plans from `dotnet msbuild -getItem:PackageReference` without a restore. + +### M7. Renovate and Dependabot rewrite or misread the generated targets file (REASONED) +- **Renovate.** Renovate's nuget manager matches `\.(props|targets)$` and parses `PackageReference Update=… Version=`. It can propose `[13.0.1.N]` → `[13.0.3]` in `socket-patch.targets`. + - Renovate's default `ignorePaths` (`**/vendor/**`, dot-matching) probably covers `.socket/vendor/nuget/`. + - But any repo that sets `ignorePaths` replaces that default and is then exposed. +- **Dependabot.** Dependabot's native (MSBuild-evaluating) updater sees the evaluated version V′ and may try to edit the file where it is declared, or skip the dependency. +- **Wrong outcome.** + - The redirect silently pins some other version. + - `SocketPatchNuGetUpstream` only knows 13.0.1, so SOCKETPATCH005 does not fire. + - The patch drops out, and only the lock diff and a later `--check` show it. +- **Fix.** + - Put the versions in properties (`Version="$(_SpV_3f9a01bc)"`) so regex tools see no literal. + - Add a render-hash self-check: a `SocketPatchTargetsHash` property that the CLI verifies, plus a SOCKETPATCH006 build warning when the file was edited. + - Have `vendor` print or emit `ignorePaths: [".socket/**"]` for Renovate. + - Add a Q3 experiment with the Dependabot nuget updater container. + +--- + +## MINOR + +| # | Scenario | Outcome | Fix | +|---|---|---|---| +| m1 | Sparse checkout in cone mode on monorepos: root files are included, root directories are not. | `.socket/` is absent and every build fails MSB4019. VERIFIED (`adv-env/c3`). Loud, but a friction failure mode for every sparse user. | Emit a comment above the Import that names `git sparse-checkout add .socket/vendor/nuget`. `vendor` prints it. Document it for Scalar users. | +| m2 | Declared literal `[13.0.1]` or `13.0.1.0` in a new project. | The redirect does not engage and SOCKETPATCH005 fires. VERIFIED for both. The failure is loud, but CLI planning treats it as "literal known". | Emit one redirect condition per observed normalized-equal literal (`13.0.1`, `13.0.1.0`, `[13.0.1]`, `[13.0.1, )`). | +| m3 | Docker `COPY *.csproj` → `restore` without DBT or `.socket` → `COPY . .` → `build --no-restore`. | SOCKETPATCH005, so fail-closed as designed. VERIFIED. Every Dockerfile needs `COPY Directory.Build.targets .socket/vendor/nuget`, and the layer cache is invalidated per patch. | Document it. `vendor` scans for `Dockerfile*` with `dotnet restore` and warns `vendor_nuget_dockerfile_copy`. | +| m4 | `git clean -fdx` after `vendor` but before commit. | The untracked seed and targets are deleted. The tracked DBT edit stays, giving MSB4019. | Print "commit `.socket/vendor/nuget`" in the outcome. `--check` detects untracked seeds. | +| m5 | Seeds for big packages (native runtimes, analyzers, satellites). | GitHub hard-limits files to 100 MB and warns at 50 MB. The whole package goes into history once per uuid, and org pre-receive hooks may demand LFS. | Cap file and seed size with `vendor_nuget_seed_too_large`. Record a per-repo LFS opt-in only with `checkout lfs:true` documented. Future G-experiment: prune the seed to the closure's TFMs and RIDs (contentHash comes only from `.nupkg.metadata`). | +| m6 | Seed entries that differ only by case, on macOS or Windows. | Checkout collision and SOCKETPATCH002. | Refuse at vendor time on a case-folded inventory collision. | +| m7 | Seed missing, VS restore (no SOCKETPATCH001 there), lockless, and a source that serves V′. V′ is predictable from the committed uuid. | An attacker's `build/*.targets` imports at evaluation, before any guard. This needs control of the id on a source (private or virtual feeds, unreserved ids). | The server-side collision check must cover the org's configured upstreams, not just nuget.org. Refuse the fallback tier for ids that are not prefix-reserved on multi-upstream feeds, or make redirects depend on `Exists(seed)` combined with a hard evaluation error. | +| m8 | GitHub dependency graph, Trivy and CycloneDX read `packages.lock.json`. | They see `12.0.1.N`, so alerts persist and Dependabot security PRs bump the version and drop the patch. Tools that fetch nuspec metadata from nuget.org for V′ fail. | depscan SBOM mapping (already a GA blocker). Document the alert behaviour. Consider emitting Dependabot `ignore` entries. | +| m9 | actions/setup-dotnet `cache: true` keys on the hash of the lock files. | One cache miss per vendor or revert. Also, `NUGET_PACKAGES=${{github.workspace}}/.nuget/packages` puts the GPF inside the repo, and `discover_projects` does not skip `.nuget/` (template packages contain `*.csproj`). | Skip every dot-directory and any directory that is a configured package folder during discovery. | +| m10 | Concurrent restores or builds. | No writes to seed roots, so this is safe (REASONED). The pack hook writes `obj/socket-patch-pack/`, per project. | None needed. Add one parallel `-m` sln leg to the docker suite. | + +--- + +## Attacks that failed (the design holds) +- **Symlinked checkout path.** The resolved paths and `SocketPatchNuGetDir` stay consistent, so the guard passes. VERIFIED. +- **Guard cost.** 52 ms per project for the full 8.7 MB Newtonsoft seed, with `GetFileHash` at 10 ms. Stamp-free hashing is affordable. VERIFIED. +- **Guard targets as written.** They parse and work on SDK 8: the unquoted `StartsWith($(...))`, the `Substring` key and the `%(SpKey)` batching. That is partial G1 evidence, one root only. VERIFIED. +- **Direct redirect, auto-follow and locked-mode restore from the fallback root with a warm upstream 13.0.1 in the GPF.** VERIFIED. +- **Accepted from earlier research, not re-run here:** `NUGET_PACKAGES`, `--packages`, `RestorePackagesPath`, `locals all --clear` and offline for the patched package (VERIFIED-D and VERIFIED-C). + +## Required design changes, in priority order +1. Write the nested `.gitattributes` from B1, and have `--check` compare against committed blobs. +2. Make the hash guard location-independent, as in M1. +3. Skip the guard in design-time builds. +4. Use a uuid8 directory and compute the MAX_PATH check on absolute paths. +5. Compare EOL-neutrally in the hot path, `--check` and revert. +6. Define the mixed packages.config policy. +7. Make `--check` work before any restore. +8. Stop exposing literal versions in the targets file, and add a render-hash check. +9. Add real-clone e2e legs (`text=auto`, `autocrlf=true`, sparse, Windows) to the **prototype** acceptance criteria. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/adversarial-integrity.md b/docs/design/nuget-vendoring-research/adversarial-integrity.md new file mode 100644 index 000000000..24678743a --- /dev/null +++ b/docs/design/nuget-vendoring-research/adversarial-integrity.md @@ -0,0 +1,192 @@ +# Adversarial integrity review: NuGet vendoring v2 (unique-version fallback seed) + +**Verdict: not ready.** The design's integrity story has one gap that decides everything else. Nothing pins the set of files in the committed seed, and every check (lock `contentHash`, the `SOCKETPATCH002` tables, `.nupkg.metadata`, `state.json` `fileInventory`) points back at other files in the same repo. I proved these gaps with dotnet runs: +- a tampered seed, or one with files added, passes `--locked-mode`; +- files added to the seed run as MSBuild code before the guard runs, and can switch the guard off; +- an environment variable switches the guard off; +- a nested `Directory.Build.targets` added later silently builds upstream; +- git's `* text=auto` rewrites seed bytes, so `SOCKETPATCH002` fails on every clean clone. + +Most fixes are cheap. + +**Setup.** .NET SDK 8.0.131. HOME, NUGET_PACKAGES, NUGET_HTTP_CACHE_PATH, TMPDIR and DOTNET_CLI_HOME were isolated under `/adv-integrity/` (`env.sh`). The package feed was an offline local folder holding upstream Newtonsoft.Json 13.0.1. The seed was the design-D e1 seed `13.0.1.1843260417`, placed at `.socket/vendor/nuget/u1/`. The fixture repo is `adv-integrity/base`. It uses the design's §6.3 guard logic (redirect, `SOCKETPATCH005`/`003`/`002`, and the `SocketPatchSeedFile` hash check) almost unchanged. The other experiments ran in `x0` to `x5`. + +Labels: **VERIFIED** means I ran it here. **REASONED** means it follows from verified facts or from the code but I did not run it. **DOCS** means it comes from documentation only. + +--- + +## Blockers + +### B1. The seed's file set is not pinned. Added files are consumed, run before the guard, and can turn it off. The lock pins nothing. +- **Scenario.** A PR, a bad merge, or a compromised dependency bot changes a seed dll. It also adds `build/netstandard2.0/Newtonsoft.Json.props` and `.targets`, which are not in the nuspec, not in `fileInventory`, and not in `SocketPatchSeedFile`. +- **Result (VERIFIED, `x1`).** + - On a fresh clone, `dotnet restore --locked-mode` succeeds with the tampered dll. The lock `contentHash` is compared only with `.nupkg.metadata`, and that file sits in the seed and was not changed. + - NuGet enumerates the seed directory, so the added `build/` files go into `project.assets.json` and are imported: the props through `nuget.g.props`, the targets through `nuget.g.targets`. + - The props set `SocketPatchNuGetGuard=false`. `dotnet build` then prints `EVIL: injected seed build/ code executed` and `Build succeeded`. No `SOCKETPATCH002` is raised, even though the dll was changed. +- **What this means.** + - In the fallback tier the lock gives version identity only, not integrity. The claim in §2.5/§4 that the patched package is integrity-pinned is overstated. + - The design's inventory check hashes only the files it lists, never "these files and no others". It also runs only when `_SpLocal != ''`. + - A single added file in a large, binary-heavy seed diff is easy for a reviewer to miss. +- **Severity: blocker.** +- **Fixes (all needed):** + 1. **Set-equality check at restore time.** Add a target at `BeforeTargets="_GenerateRestoreGraph;CollectPackageReferences"` that globs `$(SocketPatchNuGetDir)/**` (including dotfiles) and fails if the glob differs from the inventory or any hash differs. Restore runs in its own evaluation before `nuget.g.*` is regenerated, so on a clean CI checkout the check runs before the planted code is imported. Run it unconditionally, not gated on `_SpLocal`. + 2. **The CLI checks set equality too.** `fallback_in_sync`, `verify.rs` and `vendor --check` should walk the directory, reject any extra file, symlink or non-regular file, and compare against `fileInventory`. + 3. **Anchor the seed outside the repo.** Add `vendor --check --online`, which rebuilds the expected tree from an independent source and diffs the committed seed against it byte for byte. The source is either the SRI-checked service `nupkg-socket-version`, or the upstream nupkg (with its signature or depscan `upstreamSha512` checked) plus the patch afterHashes. Recommend it as the CI gate. Without it, every pin is self-referential (see M6). + 4. **Harden the guard property.** Covered in M1. + +### B2. Git line-ending normalisation changes seed bytes, so SOCKETPATCH002 fails on clean clones. +- **Scenario.** The repo has `* text=auto` in `.gitattributes`, as the common VisualStudio template does, or a developer uses `core.autocrlf=true`, the Git for Windows default. +- **Result (VERIFIED, `x5`, `x4`).** + - With `* text=auto`, `git add` normalises CRLF to LF in `newtonsoft.json.nuspec`, `LICENSE.md` and `lib/*/Newtonsoft.Json.xml`. The clone's hashes differ from the originals (for example the nuspec goes from `a1d0fb81…` to `2fd73470…`). The dll is unchanged. + - With `autocrlf=true`, LF-only text members such as `socket-patched.txt` are rewritten on checkout. +- **Wrong outcome.** + - The developer who vendored still has the original bytes and passes. Every other clone and CI fails `SOCKETPATCH002` on the `SocketPatchSeedFile` rows. + - The hot path always sees drift, so `vendor` rewrites the seed every run. + - Legacy mode never hit this, because a `.nupkg` is binary to git. +- **Severity: blocker** (correctness and CI friction; it makes the integrity guard useless in practice). +- **Fix.** + - Generate `.socket/vendor/nuget/.gitattributes` containing `* -text -diff -merge -filter`. This also turns off LFS. + - After writing, run `git check-attr text eol filter -- `. Refuse with `vendor_artifact_git_transformed` if any file is still text or filtered. + - Add a case to the Docker test: a repo with `* text=auto`, then clone and build. + - If `*.dll filter=lfs` survives (the negation does not win), LFS pointer files in the checkout give a loud `SOCKETPATCH002` rather than a silent failure, but refuse at vendor time anyway. + +--- + +## Major + +### M1. The guard can be turned off by an environment variable, package props, or a Directory.Build.rsp. +- **VERIFIED (`x3`):** + - `SocketPatchNuGetGuard=false dotnet build` with a tampered seed dll ends in `Build succeeded`. MSBuild reads environment variables as properties. One stray variable in a CI template turns off every guard in the org. + - B1 showed that package-level props can do the same. +- **REASONED:** a committed `Directory.Build.rsp` containing `-p:SocketPatchNuGetGuard=false`, or `SocketPatchNuGetAllowUnpatched=true` in the environment, does the same thing and is easy to miss in review. +- **Fix (VERIFIED, `x1` second run).** + - Assign `true` unconditionally in `socket-patch.targets`. The `-pp` output shows `Directory.Build.targets` imported after the package `build/*.targets` (line 9218 vs 9173). A project or package cannot override that assignment, and neither can an environment variable. Only a global `-p:` can. With this change, the props attack was stopped and `SOCKETPATCH002` fired. + - Because our file is imported last, our `SocketPatchNuGetGuard` target definition also wins over a package that redefines the target (VERIFIED). A `BeforeTargets` hook in the planted code is still arbitrary code, which is why B1's restore-time check is the real defence. + - Replace the free `SocketPatchNuGetAllowUnpatched` property with a project allowlist generated by the CLI from `state.json`. + - Make `vendor --check` flag `Directory.Build.rsp` and `MSBuild.rsp` files that mention `SocketPatch*`. + +### M2. A nested Directory.Build.targets added later hides the import. The project builds upstream silently and SOCKETPATCH005 never runs. +- **VERIFIED (`x3`).** + - Adding `src/Directory.Build.targets`, which does not chain to the root, plus a new `src/New/New.csproj` that references 13.0.1, gives `Build succeeded`. + - A new lock is created with `resolved: 13.0.1` and the upstream hash. No `SOCKETPATCH*` error appears. +- **Why the design misses it.** §4 and §9 say `SOCKETPATCH005` covers new projects. It covers them only if the import is reached. `ImportDirectoryBuildTargets=false` added later behaves the same way. +- **Severity: major.** The build silently ships the known-vulnerable version. +- **Fix.** + - `vendor --check` (the CI gate) evaluates every discovered project with `dotnet msbuild -getProperty:SocketPatchNuGetTargetsImported` (batched, or with `-graph` from the sln). It fails if any project that resolves the patched id at V is missing the import. + - Also, when writing a nested DBT, prefer adding the `GetPathOfFileAbove` chain line. + - Document that the protection holds only up to the next nested DBT. + +### M3. A copy of V′ in the global packages folder takes priority over the committed seed. +The global packages folder (GPF) is NuGet's shared package cache. +- **VERIFIED (`x2`).** + - When `$NUGET_PACKAGES/newtonsoft.json/13.0.1.1843260417/` holds different dll bytes and a copy of the seed's `.nupkg.metadata`, `restore --locked-mode` succeeds and resolves from the GPF. + - The guard then fires `SOCKETPATCH003` correctly. + - If that GPF copy also carries a `build/*.props` that sets `SocketPatchNuGetGuard=false`, the build ends in `Build succeeded` with the other bytes in `bin/`. +- **Consequences.** + - (a) §9's "NU1403 when locked" holds only when the planted `.nupkg.metadata` hash differs. A copied directory, for example from a cache-sync tool, passes. With M1's fix, `SOCKETPATCH003` still catches it. + - (b) **Cross-tier false positive (REASONED from the verified priority).** Once any feed-tier repo, or the future hosted V′ (§11.2), puts builder-0 V′ into a shared CI or developer GPF, every fallback-tier repo using that uuid fails `SOCKETPATCH003` on that machine, although the bytes are identical. The GPF is never refreshed, so the failure persists until someone deletes the entry by hand. +- **Fix.** + - Make the resolved-root check depend on content. Accept a foreign root when the package's whole file set hashes equal the inventory (same set-equality as B1), and fail otherwise. + - Also narrow the `StartsWith` prefix from `SocketPatchNuGetDir` to the exact `` root. Today one uuid's V′ served from a different uuid directory passes (minor). + +### M4. Builder bit 1 does not map to a single byte sequence. In the feed tier this gives persistent NU1403 on a shared GPF. +- **REASONED.** Builder 1 covers three different byte producers: + - the CLI re-versioning the same-version service nupkg; + - `local_rebuild` from upstream; + - either of those produced by a different CLI version, where zip writer details change. +- **Consequence.** The §6.1 promise that one V′ equals one byte sequence everywhere is false for builder 1. + - In the feed tier on a shared runner GPF, the first writer wins. The second repo gets NU1403 on every locked restore on that runner. The research also verified that a failed locked restore still writes to the GPF, so the failure persists. + - Lockless projects silently use the other repo's bytes. `SOCKETPATCH002` catches this only if the table covers every consumed file. + - In the fallback tier the only cost is lock churn between developers. +- **Severity: major for the feed tier, minor for the fallback tier.** +- **Fix, any one of:** + - the feed tier requires builder 0 (service bytes) only; + - for local builds, derive N from the V′ nupkg's sha512 instead of the uuid, and record the uuid in state and in a nuspec field; + - freeze `reversion_nupkg` output with a cross-version golden, and forbid `local_rebuild` under builder 1 (give it a third code). + +### M5. Revert drift repair "from the signed-content-hash of a GPF copy" trusts a poisoned cache. +- **REASONED from the verified facts (first writer wins; the legacy layout wrote patched same-version bytes to `//`).** + - After legacy use, or on any shared or poisoned GPF, §7.4 step 1 computes the "upstream" hash from patched or tampered bytes and writes it into the committed lock under upstream V. + - The local locked restore then passes against the poisoned cache, which blesses it. CI either gets NU1403, or, if CI shares the cache, keeps the bad bytes. +- **Fix.** + - Repair only from depscan `upstreamContentHash`, or from a freshly downloaded nuget.org nupkg whose repository signature has been checked. + - Never repair from the GPF. If a GPF copy is used at all, require `.nupkg.metadata.source` to be nuget.org, a valid `.signature.p7s`, and `dotnet nuget verify` to pass. + +### M6. Provenance of the unpatched files is lost, and verify has no external anchor. +- **REASONED.** + - A normal restore checks nuget.org's repository signature on every extraction. + - Vendoring drops the signature once (it has to; NU3008). The fallback tier then never checks anything again (VERIFIED earlier: V-D9 and signing §b). + - For the unpatched members of the seed, integrity therefore rests on one HTTPS download on the vendoring machine. + - `verify` and `--check` compare state against seed against lock against targets. All of these are repo data. Only afterHashes (patched files only) come from the server. +- **Fix.** + - At vendor time, check the upstream nupkg's repository signature, or its sha512 against depscan `upstreamSha512` or the nuget.org catalog `packageHash`. + - Record `upstreamSha512` and afterHashes in state. + - `--check --online` (B1 fix 3) recomputes the full expected tree. + - Make depscan item #3 a GA requirement, not only a revert aid. + +### M7. Feed-tier trust: a repo-level, self-signed author trustedSigner applies to every package id. +- **DOCS and VERIFIED mechanics.** The signing research verified that trustedSigners merge across config levels and that `allowUntrustedRoot` passes `require`. NuGet documentation says `` trust has no `owners` or id scope. +- **Scenario.** Whoever holds the pfx (§10) can sign any package id, with any content, and it passes the enterprise's `require` in this repo. The design also normalises repos extending enterprise trust. A contributor could equally add their own fingerprint, and it would look like routine socket-patch churn. +- **Fix.** + - Generate an ephemeral key per uuid, sign, and destroy the private key before writing the fingerprint, so the trust covers exactly the bytes already signed. + - Better: never write trustedSigners. Print the `` block for the admin to add to machine-level config, or wait for depscan's CA-issued certificate with an RFC 3161 timestamp (item #8) and trust Socket's certificate once at org level. + - `vendor --check` flags any repo trustedSigner not recorded in state. + +### M8. Name handling: percent-decoding after validation allows path traversal; MSBuild metacharacters allow injection. +- **REASONED from the code.** + - `read_zip_members` (`crates/socket-patch-core/src/vendor/common.rs:336`) checks the **raw** name with `is_safe_relative_subpath`. §6.2 then percent-decodes. `%2E%2E%2Fx`, `lib%2F..%2F..%2Fx`, or `%5C` passes the raw check and decodes to traversal. Decoded names can also collide (`a%20b` and `a b`) or differ only in case. + - `is_plain_archive_name` allows `$ @ % ; ' ( )`. These are interpolated into `Include=`, `Condition=` and the `;path=hash;` `Contains` tables. `%XX` is unescaped by MSBuild, `;` splits items, `@(...)` and `$(Prop.Method())` are expanded. The result is a broken or forgeable inventory (a crafted name containing `=HASH;` can satisfy `Contains`) and property expansion at evaluation. + - `state.json` fields (id, versions, uuid, project paths) are rendered into the targets file by `regenerate_shared` straight from disk. +- **Fix.** + - Decode, then re-run `is_safe_relative_subpath`, `is_plain_archive_name` and `names_are_unambiguous`, case-folded. + - Refuse names containing `$ @ % ; ' = ( )`. Otherwise MSBuild-escape (`%24 %40 %25 %3B %27`) and XML-escape every interpolated value. + - Check id against `^[A-Za-z0-9_.-]+$`, versions against the parser, uuids against strict UUID format, and project paths on every render, not only in the prelude. + - Zip-bomb protection is already there (`MAX_ENTRIES`, per-entry and total caps enforced against the actual decompressed size). Reuse it and keep it. + +### M9. Guard coverage: packages without lib or ref assets are not checked at all. +- **REASONED.** + - `_SpCand` is built only from runtime, compile, native and resource items. For an analyzer-only or build-only package (source generators, SourceLink, MSBuild task packages), `_SpLocal` is empty. + - Then nothing is hashed, the `SocketPatchSeedFile` check is skipped (gated on `_SpLocal`), and `SOCKETPATCH003`/`005` never run. + - Even for lib packages, analyzers and `build/` files are not in `_SpCand`, although they run inside the compiler and MSBuild. +- **Fix.** + - Decide "patched package present" from the `project.assets.json` `libraries` and `targets` keys, not from asset items. + - Add `@(Analyzer)` to the candidates. + - Run the full-inventory check (B1) unconditionally at restore. + - Gate G1 on an analyzer-only fixture. + +--- + +## Minor + +- **m1. Symlinks (REASONED).** + - Committed symlinks inside the seed, or on its ancestors (`.socket`, `vendor`, `nuget`, ``), pass `SOCKETPATCH003`, because that check compares path strings without resolving symlinks. A seed linked outside the repo also opens a check-then-use race between hashing and compilation. + - Revert's "delete the seed and prune" through a symlinked ancestor deletes outside the repo. Rust's `remove_dir_all` does not follow a symlink passed as its own argument, but it does traverse symlinked ancestors in the path. + - Fix: apply the existing `refuse_symlinked`/`first_symlink` to the seed and all its ancestors in apply, verify and revert, and refuse non-regular files in the inventory walk. +- **m2. The signature policy is checked only at vendor time (REASONED).** A `require` added later to a repo, ancestor or user config is bypassed silently. The design documents only the CI-only case. Fix: `vendor --check` probes the policy again, and the restore-time target warns when it can see `signatureValidationMode` in `$(RestoreConfigFile)` or repo configs. +- **m3. Seed nuspec edits.** An added `` in the nuspec pulls in a new package. That is loud under locked mode (NU1004) and silent when lockless. B1's set-and-hash check at restore closes it. The current design only checks the nuspec after resolution, and only when `_SpLocal` is non-empty. +- **m4. Wording in §9.** "V′ present elsewhere → NU1403 when locked" holds only when the planted `.nupkg.metadata` hash differs (VERIFIED `x2`). The row should say that `SOCKETPATCH003`/`002` is the actual defence. +- **m5. Stale `obj/` (REASONED, low).** A pre-vendor `project.assets.json` followed by `build --no-restore` is caught by `SOCKETPATCH005`. After a revert, a stale assets file pointing at a deleted seed gives a loud missing-file error. Both are fine; keep them as test legs. +- **m6. Hash tables.** They depend on `GetFileHash` giving uppercase hex and on case-sensitive `String.Contains`. This works (VERIFIED `x0`: the metadata comparison passes, and editing `.nupkg.metadata` gives `SOCKETPATCH002`). Pin it with a golden test so a lowercase render never ships. + +--- + +## Verified experiment log + +| ID | Where | Result | +|---|---|---| +| x0 | `adv-integrity/x0` | Baseline passes: locked restore, `packageFolders` = GPF then the seed, guard `ok local=2`. The `SocketPatchSeedFile` hash check works: editing `.nupkg.metadata` gives `SOCKETPATCH002`. | +| x1 | `x1` | Tampered dll plus unlisted `build/*.props`/`.targets` in the seed: locked restore passes, the planted targets run, the guard is off, `Build succeeded`. With an unconditional `SocketPatchNuGetGuard=true` in our targets: `SOCKETPATCH002` fires, and a planted redefinition of the guard target is ignored. The `-pp` output shows the DBT imported after the package `build/*.targets`. | +| x2 | `x2` | V′ in the GPF with a copied `.nupkg.metadata` and other bytes: locked restore passes and resolves from the GPF, `SOCKETPATCH003` fires. Adding planted props that disable the guard: `Build succeeded` with the other bytes. | +| x3 | `x3` | A new nested `src/Directory.Build.targets` that does not chain: the new project builds upstream 13.0.1, a new lock is written, no error. The environment variable `SocketPatchNuGetGuard=false` hides a tampered seed. | +| x4/x5 | `x4`, `x5c` | `* text=auto` changes the hashes of the nuspec, LICENSE and xml members; `autocrlf=true` changes LF-only text members; the dll is unchanged. | + +## Priority fixes before the prototype is accepted +1. Restore-time full-set check of the seed, plus a CLI set-equality walk (B1). +2. Generated `.gitattributes` with `* -text -diff -merge -filter`, plus a `git check-attr` probe (B2). +3. Unconditional guard assignment and a CLI-managed allowlist (M1). +4. `vendor --check` evaluates every project for the import (M2) and supports `--online` rebuild verification (B1, M6). +5. A resolved-root check that also compares content hashes (M3). +6. Decode-then-validate names and escape MSBuild metacharacters (M8). +7. Candidates chosen from the assets file, including analyzers (M9). +8. Remove GPF-based drift repair (M5). +9. For the feed tier: builder 0 only, or content-derived V′ (M4), and no pfx-holder trust written at repo level (M7). \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/adversarial-lifecycle.md b/docs/design/nuget-vendoring-research/adversarial-lifecycle.md new file mode 100644 index 000000000..60883f9dd --- /dev/null +++ b/docs/design/nuget-vendoring-research/adversarial-lifecycle.md @@ -0,0 +1,305 @@ +# Adversarial lifecycle review: NuGet vendoring v2 (unique-version fallback seed) + +## Verdict + +The build-time mechanism holds up: V′, the fallback folder, evaluation-time redirects and the guards. The weak part is the lifecycle, which the design does not specify well enough to implement. + +The design adds generated files shared by every NuGet entry: `socket-patch.targets`, `.gitignore`, and one DBT import per directory. It then tracks them with per-entry records, inside a CLI that: +- saves the ledger and sweeps old uuid dirs *after* the backend returns; +- drops cross-uuid wiring on re-vendor; +- keeps "preserved" entries byte-identical. + +Those three behaviours cause the two lifecycle blockers: +- **Patch update:** the most common lifecycle event breaks restore for every project in the repo. +- **Revert after an update, or reverting in the wrong order:** leaves an import to a deleted targets file. Restore then quietly rewrites locks to upstream. + +A third blocker is the git line-ending problem: under the common `* text=auto` rule, the committed seed does not survive a clone byte-for-byte. + +With the fixes below the design is still workable. §15 should grow by one structural piece: a pass that regenerates the shared NuGet outputs from the final ledger state. + +## Experiments run (SDK 8.0.131) + +All runs used an isolated `HOME`, `NUGET_PACKAGES`, `NUGET_HTTP_CACHE_PATH`, `TMPDIR` and `DOTNET_CLI_HOME`, under `/adv-lifecycle/`. Env script: `adv-lifecycle/env.sh`. The fixture is a copy of design-D `e1/r` (Lib, App, Tool; 13.0.1 and 12.0.1 redirected to `.1843260417`). + +| ID | Experiment | Result | +|---|---|---| +| **V1** | Cold GPF, then `dotnet restore S.sln --locked-mode` on the vendored repo | **VERIFIED.** Afterwards the GPF holds only `newtonsoft.json.bson/1.0.2`. It holds **no** `newtonsoft.json` at any version, because upstream 13.0.1 and 12.0.1 are never downloaded. `packageFolders` = [gpf, `.socket/vendor/nuget/packages`]. | +| **V2** | Add one non-existent uuid root to `RestoreAdditionalProjectFallbackFolders` | **VERIFIED.** `error NU1301: The local source '…' doesn't exist` for **every** project that imports the targets: Lib, App and Tool, including projects that do not use the patched package. | +| **V3** | Unguarded `Import` of `socket-patch.targets` with the targets file deleted | **VERIFIED.** A plain `dotnet restore` **succeeds without error**, because NuGet's restore evaluation ignores missing imports. It **rewrites `Lib/packages.lock.json` from `13.0.1.1843260417` back to upstream `13.0.1`**. Only `dotnet build` then fails with MSB4019. A later `--locked-mode` restore passes against the rewritten upstream lock. | +| **V4** | Commit the extracted seed in a repo with `.gitattributes` `* text=auto`, then `git clone` | **VERIFIED.** In the clone, `newtonsoft.json.nuspec`, `LICENSE.md` and `lib/*/Newtonsoft.Json.xml` have different sha256 (CRLF became LF). The vendoring machine's tree still reports clean. The dll and `.nupkg.metadata` are unchanged. The fix below is also VERIFIED under both `text=auto` and `core.autocrlf=true`: a nested `.socket/vendor/nuget/.gitattributes` containing `* -text -diff -merge -filter` gives byte-identical clones. | + +Everything else below is **REASONED**, from the code as cited or from the verified facts above. + +## Findings + +### B1. Patch update to a new uuid breaks every restore, pins a dead V′, and makes revert unrecoverable (BLOCKER) + +**Scenario.** Patch A (uuid `3f9a…`, 13.0.1 → V′a) is vendored. The manifest moves to patch B (new uuid) for the same purl, and the user runs `socket-patch vendor`. + +**What the code does** (`vendor.rs` `record_vendor_entry` :1020, `sweep_stale_artifact` :1073): +1. The backend runs while A's entry is **still in the ledger**. +2. The CLI then replaces the entry under the same key. +3. The CLI deletes A's uuid dir. + +**What goes wrong:** +- §7.1 step 5 renders the targets from "state on disk minus *this* uuid (B) plus this entry". So it renders **both** A and B: + - A's root stays in `RestoreAdditionalProjectFallbackFolders`; + - there are two `Update` redirects for the same declared 13.0.1, and the later one wins; + - SOCKETPATCH001 checks A's seed. +- After `sweep_stale_artifact` deletes A's dir, every project fails with NU1301 (V2) or SOCKETPATCH001. Nothing regenerates the targets after the sweep. +- `plan_closure` and `plan_lock_edits` look for entries that resolve to **V**. After A, every lock resolves to V′a, so B finds no closure and makes no lock edits (or refuses). The targets redirect to V′b while the locks pin V′a, which gives NU1004 in locked mode. +- If B does edit V′a → V′b, it records `original` = the V′a text. `carry_forward_wiring` (`state.rs:418`) fills an original **only when `original` is None**. So B's revert would restore V′a, a version whose seed is gone (NU1301 or NU1102). +- The prelude checks `vendor_nuget_duplicate_patch` and `vendor_nuget_version_collision` against A's entry for the same purl and may refuse the update outright. + +**Fix:** +- The shared-output render must exclude every entry with the same ledger key or basePurl as the entry being written. +- More robustly, regenerate `socket-patch.targets` and `.gitignore` in a CLI step that runs **after** `persist_vendor_entry`, `sweep_stale_artifact` and each revert's `save_state` (see P1). +- The closure planner and lock planner must treat any `resolved` that `parse_socket_nuget_version` decodes to (id, V) as "ours". For those entries they should record `original: None`, so that `carry_forward_wiring` fills in the true upstream original from A (the key `##` is uuid-agnostic, so the match works). +- Duplicate and collision checks must ignore the entry being replaced. +- Add a docker leg: vendor A, update to B, run a locked restore, revert, then `git diff --exit-code`. + +### B2. Import records are "reference-counted" per entry, but the ledger cannot hold them, leaving the repo unbuildable after revert (BLOCKER) + +**Scenario 1 (patch update, then revert).** B's apply finds the imports already present. `inject_import` returns None, so nothing is recorded. `carry_forward_wiring` performs its union **only when `prev.uuid == entry.uuid`** (`state.rs:461`), so A's `nuget_msbuild_import` records are dropped. When B is reverted: +- no import record remains; +- step 2 deletes the targets because no entries are left; +- every DBT still imports a missing file. + +**Scenario 2 (two ids, reverted in creation order).** Entries X and Y exist, and X created the imports. `vendor --revert` walks keys in sorted order and saves after each entry (`rollback.rs:891-902`). X is not last, so its import records vanish with X's ledger entry. Y has none, so the imports stay. + +**Scenario 3 (crash).** The process is killed between the import write and the ledger save. The next run sees the imports as pre-existing and has no record to undo them. + +**Outcome.** Every `dotnet build` fails with MSB4019. Worse, by V3 every plain `dotnet restore` (Dependabot, Renovate, `dotnet list package`, IDE restore) **silently rewrites the locks to upstream**. A bot PR can then commit unpatched locks. + +**Fix:** +- Stop tracking imports in per-entry records. +- Make import removal deterministic and free of records: on the last `nuget-fallback` revert, discover every DBT (same discovery as apply) and excise exactly the line carrying `Label="socket-patch"`. +- Delete a DBT only if its whole text equals `created_dbt(import_rel)`. +- Optionally keep a state-level `nugetShared` object (an additive optional field on `VendorState`) with the created and edited DBT originals, for byte-exact restore. +- **Order:** remove the imports first, and delete the targets only after every import is gone. If any excision drifts, write a no-op stub `socket-patch.targets` (just ``) rather than deleting it. + +### B3. The committed seed is not byte-stable through git (BLOCKER for Windows and VS-template repos) + +**Scenario.** The repo has `.gitattributes` `* text=auto` (the stock Visual Studio template), `core.autocrlf=true` on Windows, or an LFS rule such as `*.dll filter=lfs`. + +**Outcome (V4).** Every fresh clone or CI checkout has different bytes for the nuspec, xml docs and LICENSE. Then: +- the seed-inventory hash in the guard fails with SOCKETPATCH002 on **every build except the vendoring machine's**; +- `fallback_in_sync` never holds, so each `socket-patch vendor` rewrites the seed, and git hides this because it normalises; +- patched text members (content files, `build/*.targets`, `.ps1`) also fail afterHash verification; +- with LFS, clones without LFS get pointer files. + +The legacy layout is immune because a `.nupkg` is binary. So this is a regression specific to the new layout. + +**Fix:** +- Generate `.socket/vendor/nuget/.gitattributes` with `* -text -diff -merge -filter` (V4 verified that it fixes `text=auto` and autocrlf). +- After writing, probe with `git check-attr text filter eol` on each seed file, and refuse with `vendor_artifact_git_transformed` if any file is still transformed. +- Add `.gitattributes` to the reserved names (§11.4) and the hot-path render check. + +### M1. Preserved, kept or failed-save entries are re-wired by the next render (MAJOR) + +**Scenario.** One of: +- `rollback --preserve-state` or `remove --preserve-state`: `VendorRevertStep::Preserved` keeps the entry byte-identical (`rollback.rs:860-864`); +- drift-keep (`Kept`); +- `LedgerWriteFailed`. + +After that, any later NuGet vendor or revert, or even the hot path of a *different* entry ("targets byte-equal to a fresh render"), renders from all `nuget-fallback` entries. The supposedly reverted patch comes back into the targets. + +**Outcome:** +- Locked projects: NU1004, because their locks were restored to upstream. +- Lockless projects: silently patched again. +- If the seed was deleted: NU1301 for every project (V2). + +**Fix.** Render only the entries that are actually wired. Add `nuget.wired: bool`, or `unwiredAt`, to `NugetMeta`, and set it false on Preserved and Kept. Never render an entry whose seed directory is missing; emit a warning instead. + +### M2. Revert step order defeats drift-keep, and deletes the targets before import removal can fail (MAJOR) + +**Scenario.** §7.4 step 2 regenerates the targets **without** this uuid. Step 4 then keeps the seed only if "the live targets file still names the uuid". That can never be true at step 4, so drift-keep never fires. The seed is deleted while a drifted lock still pins V′. The result is NU1004 when locked, and a dangling V′ pin for any tool that reads the lock. + +Step 2 also deletes the targets before step 3 tries to excise the imports, which is the B2 failure mode. + +**Fix.** Use this order: +1. lock records; +2. compute keep = any drifted record whose live text still contains V′; +3. if keep, leave the uuid in the targets, keep the seed, and return `kept_artifact`; +4. otherwise remove imports (last entry only, per B2); +5. regenerate or delete the targets; +6. delete the seed. + +### M3. On a clean machine the package never counts as installed: repair, update and migration have no pristine source (MAJOR) + +**Scenario.** CI or a teammate's clone. V1 shows the GPF never holds upstream `newtonsoft.json/13.0.1` again after vendoring. With §7.5, the crawler skips the seed folders. The loop (`vendor.rs:1840-1910`) therefore sees the purl as missing, and what happens next depends on the case: +- **Same uuid:** `ledger_covers` returns true for a directory artifact with `sha256: ""` (`state.rs:293` checks existence only), so the source becomes `Deferred`. The `vend_installed!` `debug_assert!(PackageSource::Installed)` (`vendor.rs:171`) then panics in debug and test builds. In release, `installed_dir` is a hint path. This does exist for legacy with a cold GPF, but legacy's first restore repairs it; in fallback it is permanent. +- **New uuid (patch update), `repair` (service None, `repair_vendor.rs:1632`), or `--offline`:** there is no fetch rung, so `package_not_installed` and exit 1. A NuGet patch can then only be updated where the service is reachable, or after revert, restore and re-vendor. + +**Fix:** +- Add a NuGet pristine-fetch rung: nuget.org flatcontainer, `//..nupkg`. +- Verify it fail-closed against the upstream `contentHash` that the ledger already holds in each `nuget_lock_entry_v2` `original`. For signed packages, compute the signed-content hash (zip without `.signature.p7s`, signing §e) instead of hashing the file. +- Treat that fragment like npm's "ledger-recovered pre-vendor registry fragment". +- Replace the `debug_assert` for nuget-fallback entries. The fallback hot path must never read `installed_dir`. + +### M4. The hot path and `--check` fail on CI before restore, and the closure re-plan misreads its own output (MAJOR) + +**Scenario.** §7.2 requires the recomputed closure plan to match `nuget.projects`, and §7.1 refuses `vendor_nuget_unrestored` when a project has neither a lock nor an assets file. + +**Outcome:** +- On a fresh clone (no `obj/`), lockless projects have neither, so `socket-patch vendor` or `--check` refuses or reports "closure changed" on every CI run. +- Even with locks, the post-vendor lock resolves **V′**, not V. A `pin` project's entry has become `Direct`, so without V′-awareness it looks like `direct`. `auto-follow` is ambiguous in the same way. The re-plan then "re-keys" and churns the targets. + +**Fix:** +- Persist the plan in state and treat it as authoritative. +- Re-plan only from csproj and CPM text plus the locks, with decoded V′ counted as V. Missing assets for a project already in the plan is not a refusal. +- Keep `vendor_nuget_unrestored` only for **new** projects that the plan has not seen. + +### M5. Older binaries (v4.0.0 is released) corrupt fallback entries; the "flavor gate in the same PR" protects only v5+ (MAJOR) + +**Scenario.** A teammate, or a CI action pinned to v4, runs `socket-patch vendor --revert` or `vendor` on a repo with `nuget-fallback` entries. + +**v4 revert** (`nuget_feed.rs:772-873`): +- every kind is unknown, so each produces a `vendor_lock_entry_drifted` warning; +- drift-keep probes only single-segment wiring files for the literal `.socket/vendor/nuget/`; +- `Directory.Build.targets` contains only the targets path, so the probe finds nothing; +- the seed is deleted and the call reports success; +- `revert_vendor_entry` drops the entry; +- the targets and locks still reference the uuid, so NU1301 in every project (V2), with no ledger entry left. + +**v4 vendor:** `config_wired` is false (there is no `socket-patch-` in `nuget.config`), so v4 runs a full legacy apply on top. It: +- writes a `.nupkg` into the same uuid dir; +- adds a source and catch-all to `nuget.config`; +- rewrites the root lock; +- writes a legacy entry. Because the uuid is the same, `carry_forward_wiring` merges the fallback records into it, and that entry then routes to the legacy revert. + +`load_state` has no version gate (`state.rs:562-590`), so bumping `VENDOR_STATE_VERSION` does not help. + +**Fix:** +- In the root DBT, emit one comment per uuid: ``. v4's drift-keep then keeps the seed and the entry, which is fail-safe. This also fixes M6's liveness probe. +- In v5, route any entry that carries `nuget_lock_entry_v2` or `nuget_msbuild_import` records to the fallback backend, whatever its `flavor`. +- Detect and heal a legacy `socket-patch-` source that sits beside a fallback seed. +- Document the minimum version. + +### M6. VEX silently stops attesting fallback-vendored patches (MAJOR, missing from the §15 module table) + +**Scenario.** `vex/discover/nuget.rs` is driven entirely by `nuget.config` (source plus mapping). The vendored liveness probe list for nuget is `CONFIG_NAMES` (`vex/discover/mod.rs:1750`). `vendored_wiring_in_files` looks for the literal `.socket/vendor/nuget/`. + +The fallback entry's recorded files are the DBT (which has only the targets path) and the locks (which have only V′). The targets file uses `$(SocketPatchNuGetDir)`, which is not the literal string. So `vendored_wiring_live` returns false and the entry reads as dead. + +**Outcome.** VEX output drops the patch without any error. + +**Fix:** +- Add a fallback extractor: parse `socket-patch.targets` (uuid roots plus `Update`/`Include` V′) and every discovered lock (decode V′ to purl@V plus the uuid prefix). +- Add `.socket/vendor/nuget/socket-patch.targets` and the discovered locks to the nuget probe files. +- Spell the uuid dir literally in the targets, as a comment or in the SOCKETPATCH001 path. +- Set `artifact_rel` to the seed directory. +- Put all of this in prototype scope. + +### M7. Partial-failure unwind is unsafe for shared outputs and reused seeds (MAJOR) + +**Scenario:** +- §7.1 step 4 may *reuse* an existing, already committed seed, for example on a closure re-plan. The unwind for steps 5–8 then "deletes the seed". +- Step 5 overwrites the shared targets and `.gitignore`, which have no recorded original. +- A lock edit fails at project k of N, for example on an unparseable lock. + +**Outcome.** A committed seed can be deleted while the targets still name it (NU1301 everywhere, V2). Alternatively, the new targets file is left behind with only some locks edited, which gives NU1004. + +**Fix:** +- Snapshot the bytes of `socket-patch.targets`, `.gitignore` and `.gitattributes` before step 5, and restore them on unwind. +- Delete the seed only if this run created it. +- Stage all lock splices in memory and write them last, each by temp file and rename. Unwind must restore every written file. + +### M8. Migration from the legacy layout destroys its own pristine source and leaves the repo unpatched on refusal (MAJOR) + +**Scenario.** §11.1 runs the legacy revert (step 1) before the fallback apply (step 2). + +**Problems:** +- The legacy revert deletes the committed patched `.nupkg`, which is the only local copy of the patched bytes. +- On a legacy machine the GPF `newtonsoft.json/13.0.1/*.nupkg` is **the patched one**, extracted from our feed. `local_rebuild` would therefore apply the patch to already-patched bytes, and the before-hash gate fails. +- Any fallback-only refusal (`exact_dependency`, `dbt_disabled`, `unrestored`, `version_literal_unknown`) that fires after step 1 leaves the repo **unpatched**. + +**Fix:** +1. Run `fallback_prelude` and `plan_closure` first. +2. Materialise V′ with `reversion_nupkg(committed_legacy_nupkg)`, builder Local, and verify the afterHashes. +3. Only then revert the legacy wiring and write the fallback wiring, as one unit that can be unwound. +4. The migration must fail as a whole and leave legacy intact. + +### M9. Restore does not fail closed when the targets file is missing (MAJOR; contradicts §6.4) + +§6.4 says an unguarded import means "it never degrades silently to upstream". V3 shows otherwise: +- `dotnet restore` ignores the missing import and **rewrites the locks to upstream**; +- only build fails. + +Restore-only pipelines therefore produce unpatched locks without any error: Dependabot/Renovate lock refresh, `dotnet restore` Docker layers without `.socket/`, and `dotnet list package --vulnerable`. + +**Fix.** Update §6.4 and §9 to describe this accurately. Recommend `socket-patch vendor --check` as a required CI step, and document that lock diffs from V′ to V in bot PRs mean the patch was dropped. In Docker, the COPY step must include `.socket/vendor/nuget/` whenever it copies `Directory.Build.targets`. + +### M10. Transitive pins keep themselves alive and never disengage (MAJOR) + +**Scenario.** Tool pins `[12.0.1.N]` because Bson 1.0.2 pulls in 12.0.1. Later Bson is removed, or bumped to a version that needs 13.x. + +**Outcome:** +- **Bson removed:** the pin's condition (the project has no PackageReference for the id) still holds. Tool keeps a direct dependency on patched 12.0.1 indefinitely, and `Publish="true"` ships the dll. +- **Bson needs ≥ 13:** NU1605 against the exact pin. +- The in-use probe ("any lock resolves V′") is satisfied by our own pin, so gc never reclaims the entry. + +**Fix.** The in-use probe and `vendor --check` must ignore edges the tool created itself. A pin counts as in use only if some other package in the assets or lock `dependencies` still lists the id at a range V′ satisfies. Otherwise, report `vendor_nuget_pin_orphaned` and drop the pin on the next `vendor`. + +### M11. The layout choice does not persist, so repos end up with mixed layouts (MAJOR) + +**Scenario.** +- `--nuget-layout` defaults to `feed`. +- Only *existing* entries are sticky. +- The flag is threaded only into `dispatch_vendor_one` and the preflight. It is not on `scan --mode vendored`, `get --mode vendored` or `repair`. + +**Outcome.** A teammate or CI adding a new NuGet patch without the flag vendors it in the legacy layout. The repo then has both a `nuget.config` catch-all and fallback targets. This is untested, and legacy GPF poisoning returns for that id. + +**Fix.** Infer the layout: if any `nuget-fallback` entry exists, new entries use fallback. Or persist a `nugetLayout` optional top-level field in the state. Put the flag on `GlobalArgs`. + +### Minor findings + +| # | Scenario | Outcome | Fix | +|---|---|---|---| +| m1 | A user edits `socket-patch.targets`, for example to add `SocketPatchNuGetAllowUnpatched` or exclude a project | The next render overwrites the edit without warning | Store the render sha in state. If the on-disk file differs from the last render, warn `vendor_nuget_targets_edited` and refuse without `--force`. Provide a supported user hook: an optional `socket-patch.user.targets` imported last, plus per-project properties. | +| m2 | "Import into every nearest DBT" reaches git submodules, `dotnet new` template content (`PackageType=Template`), samples, or vendored third-party trees | Submodule edits cannot be committed from the superproject, so projects there resolve upstream silently. Templates ship a broken import to their consumers. | Limit to projects in the same git worktree (`git rev-parse --show-toplevel`). Skip template content. Warn `vendor_nuget_import_skipped`. | +| m3 | A new subdirectory is added later with its own DBT that does not chain to the parent | The targets are never imported, so SOCKETPATCH005 cannot fire and the project is silently unpatched. §4's claim that 005 covers new projects is only partly true. | Say so explicitly. Have `vendor --check` list uncovered projects, and make `--check` part of the recommended CI step. | +| m4 | Developers with and without service access (builder 0 vs 1), and Q4 | Different V′ for the same uuid. Lock and targets churn as soon as anyone runs `--force` or a re-plan. | Hot path accepts either builder for the same uuid. Never switch builders without an explicit `--nuget-rebuild-service`. | +| m5 | Dry run | Earlier entries in the run are not persisted, so the preview render leaves them out and understates the diff | Render from the in-memory run state. | +| m6 | Directory artifact with `sha256: ""` | `ledger_covers`, `list` and orphan labelling rely on existence only. The `path.rs` leaf parser expects a `.nupkg` leaf. | Add a `fileInventory`-based intact check for nuget-fallback artifacts, and a nested leaf parser. | +| m7 | NU1903 on V′ | VERIFIED during V1: `NU1903 … 12.0.1.1843260417 has a known high severity vulnerability`. This is the same as before the patch, but still a false positive once patched. Under `TreatWarningsAsErrors` or `NuGetAuditLevel` it fails the build. NuGetAuditSuppress is out of prototype scope. | Bring NuGetAuditSuppress into the prototype for NuGet ≥ 6.11, or document the gap. | + +## P1. Can the prototype (§15) be built in the current code structure? + +Only with CLI changes that §15 does not list. What is missing: + +1. **No per-ecosystem finalize hook.** Shared outputs derived from the whole ledger need a `vendor::nuget_fallback::sync_shared(cwd, &VendorState)` call after every ledger mutation: + - `run_vendor` after `persist_vendor_entry` and `sweep_stale_artifact`; + - `run_revert`, `rollback`, `remove` (`revert_vendor_entry` after `save_state`); + - `run_vendor_gc`, `repair`, and scan/get vendored. + + Doing this inside the backend is what causes B1 and M1. +2. **`carry_forward_wiring` contract.** + - The backend must emit `original: None` for lock entries already at a Socket V′. + - Import wiring must move out of per-entry records (B2). + - A new-uuid re-vendor unions nothing. +3. **Layout plumbing is more than two call sites.** + - `vendor::service_preflight` (`vendor/mod.rs:918`) is a public 6-argument function with no ledger access, so `layout_for(entry_flavor, …)` cannot be evaluated there without a signature change. + - `dispatch_vendor_one` has callers at `vendor.rs:2636` and `repair_vendor.rs:1632`, and is also reached through `vendor_records` from scan and get. The flag belongs on `GlobalArgs`. +4. `vend_installed!` `debug_assert` and a NuGet fetch rung (M3). +5. VEX discovery and liveness (M6), the in-use probe with self-edge exclusion (M10), and crawler skipping all have to be in scope. Otherwise `vex`, `gc` and `list` regress as soon as the flag is used. +6. A v4 hybrid-entry router plus the DBT uuid comment (M5). + +Scale: the core modules in the §15 table look sized correctly. The CLI and VEX work above probably adds another 30–40% and touches 6–8 more files. None of it conflicts with the backend and dispatch shape. + +## Recommended design edits (summary) + +1. Add §6.7 "Shared outputs": + - `sync_shared` after every ledger save; + - render only wired entries whose seed is present; + - record-free import excision, removing imports before the targets; + - a stub targets file on drift; + - a generated `.gitattributes` checked with `git check-attr`. +2. Add §7.6 "Patch update": + - V′-aware planner; + - `original: None` carry-forward; + - exclude the entry being replaced; + - a docker leg for update, revert and a clean diff. +3. Reorder §7.4 revert as in M2. Reorder §11.1 migration as in M8. +4. Correct §6.4 and §9 on restore behaviour when the targets file is missing (V3), and make `vendor --check` the documented CI gate. +5. Extend §15 scope: VEX, the in-use probe, the fetch rung, the v4 hybrid router, layout inference, and the finalize hook. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/adversarial-shapes.md b/docs/design/nuget-vendoring-research/adversarial-shapes.md new file mode 100644 index 000000000..6ed578546 --- /dev/null +++ b/docs/design/nuget-vendoring-research/adversarial-shapes.md @@ -0,0 +1,161 @@ +# Adversarial review: NuGet vendoring v2 (unique-version fallback seed), project shapes + +**Environment.** .NET SDK 8.0.131 on Linux, with HOME, NUGET_PACKAGES, NUGET_HTTP_CACHE_PATH, TMPDIR and DOTNET_CLI_HOME isolated per experiment. I ran one dotnet process at a time. + +**Where everything is.** Experiments are under `/adv-shapes/exp/e{1,2,3,5,7,8,9}`. The generator is `adv-shapes/gen.py`: it renders the §6.3 targets as the design wrote them (guard, SOCKETPATCH001, pack guard) using design-D's real seeds (`13.0.1.1843260417`, `12.0.1.1843260417`). Lock "after" states come from `restore --force-evaluate`, which is the ideal output the CLI's edits would have to match. + +**Labels.** VERIFIED means I ran it. REASONED means I did not. + +## What held up (VERIFIED) +- The §6.3 guard works as written, with no MSB4096 metadata errors. + - SOCKETPATCH002 fires on a tampered dll that is consumed, and on a tampered seed file that is not. + - SOCKETPATCH005 fires. + - `GetFileHash` keeps the custom `Sha256` metadata. +- SOCKETPATCH001 (`BeforeTargets=CollectPackageReferences`) fires in normal restore and in static-graph restore, with and without `--locked-mode`. That covers part of G4. +- CPM with transitive pinning on, under a static-graph `--locked-mode` restore, passes (part of G7). +- The pack range restore fixes both a direct CPM reference and a CentralTransitive pin: the nuspec says `13.0.1`. +- Package ids that differ only in case are redirected (`newtonsoft.JSON`). F# (`.fsproj`) works. GlobalPackageReference and nuget.g.targets are imported before DBT, so `Update` reaches them. + +--- + +## Blockers + +### B1. An exact `[V′]` spreads through ProjectReference and breaks projects the closure never sees (VERIFIED, e2) +**Setup.** Lib has Newtonsoft 13.0.1 (patched). Lib12 has 12.0.1 (patched). Lib3 has 13.0.3 (unpatched). +- AppA references Lib and Lib3. It resolved 13.0.3 before vendoring. +- AppB references Lib and Lib12. It resolved 13.0.1. +- AppC has a direct 13.0.3 reference and references Lib. + +**Results after vendoring:** +- **AppA:** `NU1107 Version conflict: AppA -> Lib -> Newtonsoft.Json (= 13.0.1.1843260417) / AppA -> Lib3 -> (>= 13.0.3)`. This is a hard restore failure. AppA is not in the closure, because it resolves 13.0.3, not V, so the prelude never checks it. +- **AppB:** `NU1107` between `(= 13.0.1.N)` and `(= 12.0.1.N)`. Patching two versions of one id breaks any project that reaches both. This is exactly the "multi-version of the same id" shape the design says is Supported. +- **AppC:** `NU1608` (Lib requires `= 13.0.1.N` but 13.0.3 resolved). This breaks the build under `TreatWarningsAsErrors`. +- **Locks outside the closure change too.** AppA's and AppC's `packages.lock.json` Project entries (`"Newtonsoft.Json": "[13.0.1, )"` → `"[13.0.1.N, 13.0.1.N]"`) change even though those projects are not in the closure. The CLI only edits locks in the closure, so AppC gets **`NU1004 ... references lib whose dependencies has changed`** in locked mode. + +**Fix (the range part is VERIFIED):** +1. Redirect to `[V′, )` instead of `[V′]`. After that change: + - AppA resolves 13.0.3, as before. + - AppB resolves 13.0.1.N (lowest applicable). + - AppC resolves 13.0.3, with no NU1608. + - Lib and Lib12 resolve V′. + - No NU1107. +2. Close the drift that `[V′, )` opens with a new guard, SOCKETPATCH006. Put a marker item (``) inside the same conditioned ItemGroup as the redirect. The guard fails if a project where the redirect engaged resolved the id at anything other than V′. Do not make NU1603 a repo-wide error: that would break existing builds that already have approximate matches on other packages. +3. The planner must edit the `Project`-entry range in **every** lock whose graph includes a redirected project, not only locks in the closure. Record each edit for revert. +4. Update the range-restore replace string to the `[V′, )` form. + +### B2. With chained nested DBT files, the once-guard imports the targets too early (VERIFIED, e7) +**Setup.** A common pattern: `src/Directory.Build.targets` imports the root DBT on its first line through `GetPathOfFileAbove`, then declares ``. + +**Why it breaks.** The root DBT's import runs first and sets `SocketPatchNuGetTargetsImported`. The import the design injects as the last child of `src/Directory.Build.targets` is then skipped. The redirect ItemGroup is therefore evaluated before the PackageReference exists. + +**Results:** +- Restore resolves **upstream 13.0.1**. +- The build fails with SOCKETPATCH005, so it is loud. In locked mode it fails NU1004 against the edited lock. +- When the project was planned as `pin`, the misfire is worse. The pin condition `@(PackageReference…)==''` is true at that point, so the pin is added next to the later Include. The result is `NU1504 Duplicate 'PackageReference'`, and the version that wins is arbitrary. + +**Fix (VERIFIED on SDK 8).** Stop injecting into DBT files. Put one line in the root, or nearest, `Directory.Build.props`: +```xml +$(CustomAfterDirectoryBuildTargets);$(MSBuildThisFileDirectory).socket/vendor/nuget/socket-patch.targets +``` +- `Microsoft.Common.targets:55` imports that property after the whole DBT chain, so ordering no longer matters. With this line the redirect applied and the build passed. +- It is also imported without the `ImportDirectoryBuildTargets` condition, so `vendor_nuget_dbt_disabled` becomes unnecessary. +- This needs a gating experiment on SDK 6 and 7 (MSBuild 17.0 to 17.7) to confirm the property exists there. + +### B3. Windows `core.autocrlf=true`, the Git for Windows default, breaks every build (VERIFIED by simulation, e1/clone) +**Cause.** An extracted seed contains text files: `.nuspec`, `lib/**/*.xml`, `LICENSE.md`, `.nupkg.metadata`, and content, build and props files. With `autocrlf=true`, git rewrites them to CRLF on checkout. + +**Result.** Every affected project fails with `SOCKETPATCH002 vendored NuGet seed file modified`. Today's layout commits a binary `.nupkg` and is immune, so this is a regression specific to the new layout. + +**Fix (VERIFIED: the clone builds).** Generate `.socket/vendor/nuget/.gitattributes` containing `* -text -diff -merge`, and treat it as a reserved name. Also decide how to handle a root `.gitattributes` that puts `*.dll` under git LFS: a clone without LFS gets pointer files and fails SOCKETPATCH002. Either add `-filter` or refuse with `vendor_artifact_lfs`. + +--- + +## Major + +### M1. A multi-target transitive pin is not conditioned on TFM (VERIFIED, e3) +**Setup.** ToolA targets `net8.0;netstandard2.0`. Bson is referenced only for netstandard2.0. + +**Result.** The §6.3 pin has no `$(TargetFramework)` condition. It adds `Newtonsoft.Json [12.0.1.N]` as a new **Direct** dependency to `net8.0`, which previously had `{}`. +- The CLI plans edits only for the TFMs where the id resolves, so locked mode fails NU1004. +- Lockless builds start shipping a new dll for net8.0. +- If the other TFM resolved a higher version, this becomes an NU1605 downgrade. + +**Fix.** +- Add `and '$(TargetFramework)' == ''` to each pin condition. +- Map aliases to lock keys. The lock uses `.NETStandard,Version=v2.0`, not `netstandard2.0` (VERIFIED), so this needs a real alias-to-framework mapping. Refuse custom aliases. + +### M2. A consumer outside the `.socket` root breaks, and the B1 fix would make it silent (VERIFIED, e8) +**Setup.** A project outside the root (a sibling repo, or a monorepo where `.socket` lives in a subdirectory) references the patched Lib through ProjectReference. + +**Results:** +- With `[V′]`: `NU1102 Unable to find Newtonsoft.Json (= 13.0.1.N)`. The consumer does not import the targets, so it has no fallback folder. +- With `[V′, )`: `NU1603` and a silent resolve of **13.0.2**. + +**Fix.** +- The planner walks ProjectReferences in both directions. It refuses (`vendor_nuget_external_consumer`) when a project outside the root references a redirected project, or when a redirected project is listed in a `.sln` or `.slnf` above the root. +- Document that `.socket` must sit at the root of the solution. + +### M3. SOCKETPATCH004 fires after the leaking `.nupkg` is already written (VERIFIED, e5) +**Result.** With range restore disabled, `dotnet pack -o out3` fails SOCKETPATCH004, but `out3/Lib.1.0.0.nupkg` already exists and has ``. PackTask writes the nuspec and the nupkg in the same step. A later `nuget push out/*.nupkg`, or a retry, publishes a package that consumers cannot restore. The "fail-closed" claim is false. + +**Fix.** +- Add a pre-check `BeforeTargets="GenerateNuspec"` against the rewritten pack assets. +- On a post-check failure, delete `@(NuGetPackOutput)` before raising the error. + +### M4. The pack guard fails with MSB4184 when `obj` holds more than one nuspec (VERIFIED, e5) +**Result.** Pack once, then again with `-p:Version=1.0.1`. `$(NuspecOutputAbsolutePath)*.nuspec` matches both `Lib.1.0.0.nuspec` and `Lib.1.0.1.nuspec`, and `ReadAllText("a;b")` fails the pack. Local and CI version bumps hit this routinely. + +**Fix.** Read exactly `$(NuspecOutputAbsolutePath)$(PackageId).$(PackageVersion).nuspec`, or filter `@(NuGetPackOutput)` by extension. + +### M5. The guard does not run for non-SDK csproj files that use PackageReference (REASONED; no mono here) +**Cause.** Legacy WPF and WinForms csproj files with PackageReference import DBT, so the redirect applies. They do not have `ResolvePackageAssets` or `RuntimeCopyLocalItems`, so SOCKETPATCH002, 003 and 005 silently never run. + +**Fix.** Detect a project with no `Sdk` that uses PackageReference, and refuse it or route it to legacy. Alternatively, hook `ResolveNuGetPackageAssets` and `@(ReferenceCopyLocalPaths)` and gate that on the Windows leg (G8). + +### M6. NuGetAudit still flags V′ (VERIFIED, e1) +**Result.** `warning NU1903: Package 'Newtonsoft.Json' 12.0.1.1843260417 has a known high severity vulnerability`. §2.3's "no false NU1903" holds only when the advisory's fixed version is V's own next patch. Here the fix is in 13.0.1, so `12.0.1.N` still falls inside `< 13.0.1`. +- Repos that treat NU1903 as an error stay broken after patching. +- NuGetAuditSuppress is out of the prototype's scope and needs NuGet 6.11 or later (SDK 8.0.4xx). The 8.0.1xx SDK here cannot use it. + +**Fix.** State the limitation in the design. Ship NuGetAuditSuppress with the prototype. On older SDKs, emit an informational `vendor_nuget_audit_still_flags`. + +### M7. The closure planner misses whole project types and version spellings (VERIFIED and REASONED) +- **VERIFIED (e9):** `Version="13.0.1.0"` means the same to NuGet as 13.0.1, but the literal redirect does not match it. The result is SOCKETPATCH005, and NU1004 if the CLI edited the lock. Render one condition per recorded literal spelling, or refuse the non-canonical spelling. +- **REASONED:** the enumerator looks only at `*.csproj|fsproj|vbproj`. It misses NoTargets and Traversal `.proj` files (from `global.json` `msbuild-sdks`), `.sqlproj`, `.esproj`, and others. They import DBT, so they get redirected with no lock edits. Enumerate `*.*proj` plus the projects listed in the sln. +- **REASONED:** property-expanded versions (Arcade `eng/Versions.props`) and `NuGetLockFilePath=$(MSBuildProjectDirectory)/…` depend on `dotnet msbuild -getItem`/`-getProperty`, which need MSBuild 17.8, i.e. SDK 8. Repos whose `global.json` pins SDK 6 or 7 get refused. Also, a `global.json` SDK that is not installed on the vendoring machine breaks every `dotnet msbuild` call. + +--- + +## Minor + +1. **The lock golden table is wrong for CPM with pinning on (VERIFIED, e5).** With the uniform `PackageVersion Update`, the entry stays `CentralTransitive` and only `requested` becomes `[V′, V′]`. It does not turn into Direct. +2. **Pin conversion reorders lock entries (VERIFIED, e3).** NuGet writes Direct entries first. Plain and locked restores do not rewrite a lock whose order differs, but `--force-evaluate`, Dependabot relocks, and the `--nuget-relock` text-diff check all see churn. Compare semantically, and write the canonical order. +3. **One tampered seed file fails every project (VERIFIED, e1).** The whole-inventory `SocketPatchSeedFile` hash made Tool, which only uses the 12.0.1 seed, fail for a tampered 13.0.1 seed. The cost also scales with the total seed size times the number of projects; think of native-heavy packages such as SkiaSharp. Filter on `PackageKey` for the packages this project resolved. +4. **An exact-bracket declared literal `[13.0.1]` is widened by pack range restore to `[13.0.1, )` (REASONED).** Render the replacement from the recorded literal. +5. **F# FSharp.Core and other implicit, SDK-versioned references drift across SDK versions (REASONED; props file checked).** Their version comes from the SDK that `global.json` resolves (`Microsoft.FSharp.NetSdk.props:95`). Refuse ids with `IsImplicitlyDefined`. +6. **The `vendor_nuget_tool_manifest` refusal is aimed at the wrong thing (REASONED).** `dotnet-tools.json` lists tool packages, and their dependencies are bundled inside those packages. MSBuild never resolves them. Say that tools cannot be patched, and keep the crawler from reporting that they are covered. +7. **RID lock sections list only packages with RID-specific assets (VERIFIED, e3/Rid).** Newtonsoft is absent from `net8.0/linux-x64`. RID edits matter only for packages that ship `runtimes/`. Add goldens for such a package, for example SqlClient. +8. **Windows MAX_PATH.** A 36-character uuid directory plus a long id plus a 4-part V′ is about 225 characters under `C:\agent\_work\1\s`. Use a short uuid8 directory. +9. **Analyzer-only packages are invisible to the guard (G1 still open).** + +--- + +## How the verdict changes + +The D base is sound for GPF safety, source mapping, and CPM with static-graph restore. It is **not** ready for its own "Supported" rows: +- multiple versions of one id; +- a patched library consumed next to a higher-versioned sibling; +- nested DBT chaining; +- multi-TFM transitive-only projects; +- Windows checkouts. + +**Minimum changes before prototype acceptance:** +1. `[V′, )` redirect plus SOCKETPATCH006. +2. Project-range edits in every lock that consumes a redirected project. +3. Injection through `CustomAfterDirectoryBuildTargets`. +4. TFM-conditioned pins. +5. The seed `.gitattributes`. +6. Pack guard pre-check, deletion of the bad nupkg, and an exact nuspec path. +7. The external-consumer refusal. + +Add e2, e3, e7 and the autocrlf clone as docker and e2e legs. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/code-map.md b/docs/design/nuget-vendoring-research/code-map.md new file mode 100644 index 000000000..c96fd65ce --- /dev/null +++ b/docs/design/nuget-vendoring-research/code-map.md @@ -0,0 +1,228 @@ +# socket-patch NuGet implementation map (branch v5/nuget-vendoring, read-only) + +All paths are relative to `/home/user/socket-patch/crates/`. Where a line number carries a `~` it points to the right block but is not exact. + +## 1. Backend interface a new vendored layout must implement + +**There is no trait.** Backends are free functions with one shared signature. The CLI calls them through `match` arms. + +**Vendor entry** (the same 9 arguments for every backend), `socket-patch-core/src/vendor/nuget_feed.rs:399`: +```rust +pub async fn vendor_nuget(purl: &str, installed_dir: &Path, project_root: &Path, + record: &PatchRecord, sources: &PatchSources<'_>, vendored_at: &str, + dry_run: bool, force: bool, service: Option<&VendorServiceConfig>) -> VendorOutcome +``` + +**Revert entry**, `nuget_feed.rs:762` and `:772`: +```rust +pub async fn revert_nuget(entry: &VendorEntry, project_root: &Path, dry_run: bool) -> RevertOutcome +pub async fn revert_nuget_opts(entry: &VendorEntry, project_root: &Path, opts: RevertOpts) -> RevertOutcome +``` + +**Service preflight**, `nuget_feed.rs:372`: +```rust +pub(crate) async fn service_preflight(purl, project_root, record) -> Option +``` +- It must run the same refusal checks as `vendor_nuget`, so the download plan never asks the service for a package the loop will refuse. Today it shares `nuget_prelude` (`:243`) for this. +- It is routed from `vendor/mod.rs:918` (the nuget arm is at `:932`). + +**Dispatch points** in `socket-patch-cli/src/commands/vendor.rs`: +- `dispatch_vendor_one` (`:112`) uses the `vend_installed!` macro (`:168-190`). That macro asserts `PackageSource::Installed`, because NuGet and Maven have no registry-fetch rung. The nuget arm is at `:215`. +- `SERVICE_ECOSYSTEMS` includes nuget (`:135`). +- `dispatch_revert_one_opts` (`:237`), nuget arm at `:245`. +- `dispatch_in_use_one` (`:256`) returns `None` for nuget, so there is no in-use probe. + +**Types** +- `vendor/mod.rs`: + - `VendorWarning` `:142` + - `VendorServiceConfig` `:268` + - `VendorOutcome::{Refused{code,detail}, Done{result: ApplyResult, entry: Option, warnings}}` `:785` + - `RevertOpts{dry_run, keep_artifact}` `:801` + - `RevertOutcome{success, warnings, error, kept_artifact}` `:824` + - `force_apply_staged` `:732`, which applies the patch into a private stage and is hash-gated. +- `vendor/state.rs`: + - `VendorArtifact{path, sha256, size, platform_locked, file_inventory}` `:52` + - `WiringAction{Rewritten, Added}` `:83` + - `WiringRecord{file, kind, action, key, original, new}` `:97` + - `VendorEntry` `:212` + - `VendorState` `:324` + - `VENDOR_STATE_REL = ".socket/vendor/state.json"` `:43` + - `VENDOR_MARKER_FILE = "socket-patch.vendor.json"` `:764` + - `VendorEntry::committed_artifact_intact` (a sha256 check of file artifacts), right after `:212`. + +**Shared helpers to hook into** +- `vendor/common.rs`: `refused`, `done`, `already_patched_result`, `prepare_memory_repack`/`MemoryRepack`, `rebuild_zip`, `write_zip_entries`, `zip_bytes_match_after_hashes`, `any_live_file_references` (`:993`), `prune_empty_vendor_levels`. +- `vendor/path.rs`: + - `vendor_uuid_dir_rel("nuget", uuid)` + - leaf parser for `nuget` at `:280-286`, with `split_nuget_leaf` at `:207` + - ecosystem list at `:44` +- `vendor/ledger_snapshots.rs:56-62`: `WHOLE_FILE_KINDS` includes `"nuget_config_source"`. Whole-file values of 1024 bytes or more (`SNAPSHOT_MIN_BYTES`) are stored as diff ops, and those ledgers are written as version 2. +- `vendor/verify.rs:93-102, 488-498, 711`: `.nupkg` is treated as a single committed zip file, and its members are checked against the afterHashes. +- `vendor/reuse.rs:1-12`: the reuse path is for npm/pypi only. NuGet decides "in sync" from the committed artifact itself. +- `vendor/service_fetch.rs:155-240`: `service_archive_copy` returns `ServiceCopy::{Used, HardFail, FallBack}` (the Tier-A path). +- `socket-patch-cli/src/commands/repair_vendor.rs:105-129`: `WIRING_FILES` **does not include `nuget.config`** (see §7). + +## 2. Current on-disk layout, edits, state record and revert + +**Artifact** +- Path: `.socket/vendor/nuget//..nupkg` (`nupkg_leaf` `:172`, `normalize_nuget_version` `:125`). The normalizer mirrors the TS `normalizeNuGetVersion` and the two must stay in sync. +- Marker `socket-patch.vendor.json` is written beside it (`:664`). +- The uuid dir is the local folder feed itself (module doc `:8-18`). + +**Source key:** `socket-patch-` (prelude, around `:280`). + +**nuget.config** (`build_config_edit` `:1161`) +- Config lookup is root-only. `existing_config_path` (`:1145`) probes `nuget.config`, then `NuGet.Config`. It does not probe `NuGet.config`, although `nuget_config.rs:228` `CONFIG_NAMES` lists all three. +- **No config:** writes a fresh file (`:1171-1195`) containing the nuget.org source plus ours, and a mapping of `nuget.org → *` plus `socket → `. It is written to `project_root/nuget.config` (`:599`). +- **Existing config** (`:1196-1285`): + - Anchors are found on a comment-blanked view (`blank_comments` `:1292`). + - Our `` is inserted before ``. A self-closing `` is expanded (`:1390`), and if the section is missing it is created before ``. + - If `` exists, only our `` block is appended. + - Otherwise a new mapping is created that fans `*` out to every pre-existing source key (`parse_config_source_keys` `:1330`). nuget.org is seeded if there are none. + - It errors if there is no ``. + +**packages.lock.json** (`edit_lock` `:1545`) +- Only the root `project_root/packages.lock.json` is considered (`:93`). +- Every `dependencies..` entry (case-insensitive) whose `resolved` normalizes to the version (`locked_at` `:208`) gets `contentHash` replaced with `base64(sha512(nupkg))` (`content_hash` `:1116`). +- This is a string replace of the quoted old hash, so formatting is preserved. +- Entries that disagree on the hash cause a failure. An entry with no match gives the `vendor_nuget_lock_entry_absent` warning (`~:633`). A missing lock gives `vendor_nuget_no_lockfile` (`:653`). + +**Edit order:** artifact → config → lock. A failure in the lock step unwinds the config and deletes the uuid dir (`unwind_config` `:1641`, called around `:619` and `:646`). + +**state.json entry** (`nuget_entry` `:713`) +- Fields: `ecosystem:"nuget"`, `basePurl`, `uuid`, `artifact{path, sha256 (plain hex of the nupkg), size}`. All the extras (`lock`, `flavor`, `uv`, …) are `None`. +- The CLI adds `detached`/`record`. +- Wiring records, in application order (`:678-705`): + 1. `nuget_config_source`: `file` = the config basename; `Added` if we created the file, otherwise `Rewritten`; `key` = the source key; `original` = the whole pre-vendor file text (or none); `new` = the whole post-edit file text. This is the authoritative revert record. + 2. `nuget_config_mapping`: `Added`, `key` = the id, `new` = the mapping fragment. Audit only. + 3. `nuget_lock_entry`: `Rewritten`, `key` = the id, `original` = the old contentHash, `new` = the new one. +- Example: `socket-patch-cli/tests/fixtures/legacy-ledgers/nuget/wired/.socket/vendor/state.json`. + +**Hot path** (prelude, around `:318-345`; `vendor_nuget` `:437-535`) +- `config_wired` is a plain substring test: does the config text contain the source key? +- `in_sync` also requires the committed nupkg's members to match the afterHashes, and the lock to be pinned (or to have no matching entry). +- Wired and in sync: returns `AlreadyPatched` with no entry. +- Wired but stale: rebuilds only the artifact and re-pins the lock (with `original: None`; the CLI's `carry_forward_wiring` fills it in), and warns `vendor_artifact_rebuilt`. The config is never touched on this path. + +**Revert** (`revert_nuget_opts` `:772`) +- The uuid is validated first. +- Records are walked in reverse: + - Lock (`revert_lock_record` `:1606`): replaces our hash with the original. If ours is gone but the original is present, it counts as done. Otherwise it is drift. + - Mapping record: no-op. + - Config (`revert_config_record` `:1419`): + - The file name must be a safe single segment. + - If the live file is byte-identical to `new`: restore `original`, or delete the file if we created it. + - Otherwise: excise only our verbatim `` line and our `` block (`excise_source_mapping` `:1507`). + - If neither is present: drift (`Ok(false)`). + - The catch-all mapping is left in place. +- **Drift-keep** (`:844-860`): if any record drifted and a live wiring file still contains the uuid dir path, the artifact is kept (`kept_artifact`) so the exclusive mapping is not left pointing at a missing dir. +- Otherwise `remove_tree_and_prune` runs (`:871`). `keep_artifact` supports `--preserve-state`. + +## 3. Supported and refused project shapes + +**Explicit refusals** (prelude `:250-310`) +- Not a NuGet purl, non-canonical uuid, or id/version outside `[A-Za-z0-9._+-]` → `unsafe_coordinates`. +- Unreadable config → `vendor_nuget_config_unreadable`. +- Unreadable lock → `vendor_nuget_lock_unreadable`. +- No cached `.nupkg` → `vendor_nupkg_not_found` (`:962`). +- A config without `` → a failed result (not a refusal). + +**Everything else is accepted without detection:** + +| Shape | Handling today | +|---|---| +| **No lockfile** | Accepted with a warning. There is no content pin (`:653`; `docs/ecosystems.md:382-386`). | +| **Existing mapping** | Accepted. Our block is appended (`:1255-1260`). Nothing checks whether another source already maps the same exact id. NuGet then treats both sources as eligible, so the patched copy is not guaranteed. The vex doc lists this as a non-goal (`vex/discover/nuget.rs:59-65`). | +| **Multi-project / per-project locks** | Only the root `packages.lock.json` and root `nuget.config` are used (`:93`, `:1145`). Locks in sub-projects are never pinned. Parent-dir or user-level configs, ``, and custom `NuGetLockFilePath` are not handled (`vex/discover/nuget.rs:59-65`). The crawler reads `obj/project.assets.json` one level deep (`crawlers/nuget_crawler.rs:483-498`), but only to find package folders. | +| **CPM (`Directory.Packages.props`)** | Not referenced anywhere. No refusal and no special handling. A typical CPM solution keeps its locks per project, so it falls into the "no lockfile" warning path. | +| **packages.config** | Recognized as a .NET marker (`nuget_crawler.rs:419-436`), and the legacy `packages/./` dir is crawled. `vendor_nuget` accepts that dir as `installed_dir` (doc `:392-397`). No special handling and no test for restore under packages.config. | +| **Warm global cache** | Every real-restore test uses a cold `NUGET_PACKAGES` (`docker_e2e_vendor_nuget.rs:185-224`; `e2e_nuget_dotnet_build.rs` also uses a "cold `NUGET_PACKAGES`"). A same-id/version pristine copy already in `~/.nuget/packages` is untested. | + +## 4. How the patched .nupkg is built (`materialise_patched_nupkg` `:892`) + +1. **Service first.** `service_archive_copy(service, record, name, ".nupkg")` (`service_fetch.rs:169`). + - It POSTs `/v0/orgs/{slug}/patches/package` (or the public proxy's `/patch/package`) and GETs the grant URL (`api/client.rs:1106-1113`). + - The SRI is checked, and every member must hash to its afterHash, before the bytes are written verbatim. + - An integrity mismatch is always a hard failure. + - A miss under `auto` falls back to the local build. Under `--vendor-source=service` it is refused (`vendor_prebuilt_required`). +2. **Local rebuild** (`local_rebuild` `:949`). + - `locate_cached_nupkg(installed_dir)` (`:1123`) takes the first `*.nupkg` in the crawler's package dir. That is `~/.nuget/packages///` or `$NUGET_PACKAGES`, or the legacy `packages/./`. **The pristine bytes therefore come from the global packages folder** (or the legacy folder), never from a registry: NuGet has no fetch rung (`vendor.rs:168-170`). + - The in-memory repack (`prepare_memory_repack`, which also stages `.nupkg.metadata` and `*.nupkg.sha512`), then `force_apply_staged`, which also runs the sidecar fixup. + - Deterministic lexicographic re-zip that drops `.signature.p7s` (`rebuild_nupkg_bytes` `:1079`, `SIGNATURE_PART` `:108`). + - The upstream id and version are kept, with the same filename leaf. + +## 5. Hosted NuGet (`patch/redirect/mod.rs`) + +**Entry point:** `rewrite_nuget` (`:5317`), registered at `:449`. +- It reads only a root `nuget.config` (lowercase) and `packages.lock.json` (`scan/hosted.rs:75-76`, `mod.rs:5330`, `:5346`). If there is no config it starts from `default_nuget_config()` (`:5127`). +- An unparseable lock skips the whole NuGet rewrite (`redirect_nuget_lock_unparseable`). +- Per dep it requires a `nuget-v3` override and a sha512 (`:5362-5376`). + +**Config** +- `add_nuget_source` (`:5142`) adds ``. It seeds nuget.org when the new mapping would otherwise have no catch-all target. +- If there is no mapping, it creates socket-exact-id plus a `*` catch-all for each pre-existing source. If a mapping exists, it prepends only ours after ``. + +**Lock** +- For every framework entry whose id matches (**id only, not version**; `:5431`), it sets `resolved` = `nugetVersionNorm` and `contentHash` = the SRI with `sha512-` stripped. + +**URL form** (fixtures `socket-patch-core/tests/fixtures/redirect/nuget/packages-lock/{basic,empty-sources,empty-sources-selfclosing,no-preexisting-mapping}`): +- index: `https://patch.socket.dev/patch-registry/nuget///index.json` +- artifact: `…//flat///..nupkg` +- The data comes from `api_client.fetch_registry_references` (`scan/hosted.rs:1244`, `client.rs:748-776`, same POST `…/patches/package`). Integrity is taken from the reference's `tarball`-kind artifact (`hosted.rs:1281-1287`). + +**Why hosted revert is unsupported** +- The config `FileEdit` records only `new: {source, pattern}` with `original: None` (`:5413-5421`). No pre-edit snapshot exists, so it cannot be inverted. +- `replay.rs:181` classifies `redirect_nuget_source` and `redirect_nuget_lock` as `Inverse::Unsupported`; the module doc `:20-23` says these groups refuse with `hosted_revert_unsupported`. +- `takeover.rs:79-83` `redirect_revert_supported` covers only cargo, npm and golang. So vendored takeover of a hosted NuGet purl is also blocked (`vendor.rs:1585`, `:2416`), and `remove.rs:1359` reports `hosted_revert_unsupported`. +- The v5 plan (`docs/design/v5-plan.md` WS1) replaces this with a re-resolve from nuget v3. +- Hosted in-memory mode can't take inventory from NuGet either (`hosted_memory/roots.rs:51` `UNSUPPORTED_MARKERS`). + +## 6. Tests and how to run them + +Run everything from `/home/user/socket-patch`. + +| Test | Needs | Gate / command | +|---|---|---| +| Inline unit tests in `nuget_feed.rs` (from `:1662`, roughly 3.7k lines: config surgery, lock, revert, drift, hot path, service, tamper guards) | nothing | `cargo test -p socket-patch-core --lib nuget_feed` | +| `socket-patch-core/tests/covgap_vendor_nuget_feed.rs` (TMPDIR failure; `cfg(unix)`) | nothing | `cargo test -p socket-patch-core --test covgap_vendor_nuget_feed` | +| `crawler_nuget_e2e.rs`, `redirect_golden.rs` (shared TS goldens), redirect `mod.rs` tests `:7915+`, `replay.rs:3406` | nothing | `cargo test -p socket-patch-core` | +| `socket-patch-cli/tests/e2e_vex_lockfile/nuget.rs` (module in the `e2e_vex_lockfile` binary; hermetic, wiremock, no dotnet) | nothing | `cargo test -p socket-patch-cli --test e2e_vex_lockfile nuget` | +| `e2e_nuget.rs` (crawl only, wiremock proxy) | nothing | `#[ignore]` at `:182`, `:250`; run with `cargo test -p socket-patch-cli --test e2e_nuget -- --ignored` | +| `e2e_nuget_dotnet_build.rs` (hosted `:725` + vendored `:834`, real SDK, needs nuget.org) | host `dotnet` + network | `#[ignore]`; soft-skips without dotnet unless `SOCKET_PATCH_DOTNET_E2E_REQUIRED=1`; `SOCKET_PATCH_DOTNET_E2E_VERSION=8` picks the SDK; `cargo test -p socket-patch-cli --all-features --test e2e_nuget_dotnet_build -- --ignored` | +| `docker_e2e_nuget.rs` (apply chain, `:570`, `:607`) | Docker image `socket-patch-test-nuget:latest` | `#![cfg(feature="docker-e2e")]`; soft-skips if the image is missing (`:482`); `cargo test -p socket-patch-cli --features docker-e2e --test docker_e2e_nuget` | +| `docker_e2e_vendor_nuget.rs` (`:472`: 3-stage vendor → cold offline `--locked-mode` restore → RED/TAMPER(NU1403) → idempotence and revert) | Docker (SDK 8.0) | `--features docker-e2e --test docker_e2e_vendor_nuget` | +| `setup_matrix_nuget.rs` | host guard runs; `dotnet()` is `#[ignore]` (baseline gap) | `--features setup-e2e --test setup_matrix_nuget` | +| Other hermetic CLI tests (`in_process_get_hosted_ecosystems.rs:532`, `in_process_scan.rs:1374`, `in_process_rollback_all_ecosystems.rs:552`, `e2e_safety_advisories.rs:412-671`, `ecosystem_dispatch_e2e.rs:310,986`, `vendor_ecosystem_fixtures/mod.rs:748`, `e2e_vex_vendor.rs`, `e2e_vendored_production.rs`) | nothing (production suites are canaries) | normal `cargo test -p socket-patch-cli --test ` | + +**Docker images:** build `tests/docker/Dockerfile.base` tagged `socket-patch-test-base:latest`, then `tests/docker/Dockerfile.nuget` (FROM `mcr.microsoft.com/dotnet/sdk:8.0`, copies the binary from base) tagged `socket-patch-test-nuget:latest`. + +**CI (`.github/workflows/ci.yml`)** +- `coverage-docker` (`:455`, matrix `:479`) and `e2e-docker` (`:1390`, matrix `:1398`) run `docker_e2e_nuget`, adding `docker_e2e_vendor_nuget` at `:561` and `:1447`. +- `e2e` (`:675`) runs `suite: e2e_nuget` (`:696`) and `e2e_nuget_dotnet_build` for dotnet 6/7/8/9/10 on ubuntu and 8 on macOS (`:1057-1064`, setup-dotnet at `:1248`, env at `:1351`). The default filter is `--ignored` (`:1353`). +- `setup-matrix` (`:1634`) is non-blocking. + +## 7. Known TODOs and limitations + +- `v5-plan.md:30-31`: "a better vendored story for … NuGet (… NuGet feed is fragile). Keep the current behavior; do not invest further now." +- There are no literal `TODO`/`FIXME` comments in the NuGet files. Documented non-goals are in `vex/discover/nuget.rs:59-65`: parent/user configs, per-project locks, ``, a non-Socket source that also maps the id, and `NuGetLockFilePath`. +- **Orphan-sweep gap.** `nuget.config` is missing from `repair_vendor.rs` `WIRING_FILES` (`:105-129`). That list feeds both `repair`'s ledger reconstruction and `sweep_orphan_vendor_dirs` (`vendor.rs:283-326`). Separately, the config names the uuid dir rather than a leaf file. Together this likely means repair cannot recover a NuGet entry, and the sweep could delete a NuGet uuid dir that is still wired. This comes from reading the code only; no test was run to confirm it. +- Config-name mismatch: vendor probes 2 spellings, vex/crawler probe 3, hosted probes only `nuget.config`. +- The hosted lock rewrite matches on id regardless of version (`redirect/mod.rs:5431`). +- The same id+version as upstream means the global-cache collision is untested (every test uses a cold `NUGET_PACKAGES`). +- The signature is dropped, so the package reads as unsigned. This relies on NuGet's default signature validation mode being `accept` (`:15-18`). +- Sidecar (agent mode): `patch/sidecars/nuget.rs:1-18` deletes `.nupkg.metadata` and only advises when `.nupkg.sha512` is present. +- `dispatch_in_use_one` has no NuGet probe (`vendor.rs:256-265`). + +## 8. The NuGet crawler (`socket-patch-core/src/crawlers/nuget_crawler.rs`) + +- **Global mode** (`:37-49`): uses `--global-prefix` if given, else `NUGET_PACKAGES`, else `~/.nuget/packages` (`nuget_home` `:392`). +- **Local mode** is gated by `is_dotnet_project(cwd)` (`:406`): a root entry that is `*.csproj`, `*.fsproj`, `*.vbproj`, `*.sln` or `*.slnx`, or `nuget.config` / `packages.config` in any casing. When the gate passes, paths are returned in this order: + 1. `/packages/` (legacy) + 2. the global cache + 3. the `packageFolders` keys of `obj/project.assets.json` in cwd and one level of subdirectories (`:472-512`) +- **Layouts** (`classify_package_entry` `:228`): + - Global: `//`, where the version must start with a digit (`:286-328`). + - Legacy: `./`, split at the first `.`+digit (`:453`). + - A package is verified by `lib/` or a `*.nuspec` (`:331`). +- It crawls **every package in those folders**. It does not filter by the project's dependency graph (it does not read assets targets or the lock). +- The purl comes from the directory name, which is lowercased in the global cache. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/critic.md b/docs/design/nuget-vendoring-research/critic.md new file mode 100644 index 000000000..a1d8a80ef --- /dev/null +++ b/docs/design/nuget-vendoring-research/critic.md @@ -0,0 +1,84 @@ +# NuGet vendoring research: critic pass (contradictions, verified gaps, open questions) + +I ran six experiments myself on SDK 8.0.131. Two results change the design: +- **A committed repo-local package folder only survives if it is set through `RestorePackagesPath`.** If it is set through `nuget.config` `globalPackagesFolder`, a user's `dotnet nuget locals --clear` deletes it. +- **Once a package is in that folder, NuGet never looks at sources or `packageSourceMapping` for it.** That makes the folder the only offline mechanism that does not depend on source mapping. The catch is that it also skips signature checks. + +Full notes are in `/research/critic/FINDINGS.md`, with the helpers (`env.sh`, `mkpatch.py`) and `seed/`, `tmpl/`, `exp/` next to it. In the text below, "package folder" means NuGet's global packages folder (normally `~/.nuget/packages`), and "seed" means a patched package pre-extracted into a repo-local package folder. + +**Isolation incident.** My first experiment ran `dotnet nuget locals all --clear` before I had set `TMPDIR`. It cleared the shared `/tmp/NuGetScratchroot`, which is NuGet's temp directory. A restore running in another agent at that moment could have failed. `env.sh` now sets `TMPDIR`, and every later experiment used it. + +## 1. Contradictions and tensions between reports + +1. **Option C, the repo-local package folder, conflicts with the signing report.** + - The shapes and lock-cache reports recommend seeding a repo-local package folder. + - The signing report verified that anything already extracted into a package folder skips signature validation, even under `signatureValidationMode=require`. It says this "quietly gets around enterprise policy; should not document it". + - Consequence: if we adopt option C, the tool itself has to check the effective signature policy. It should refuse, or require a Socket author signer, when the policy is `require`. It cannot rely on NuGet to enforce it. +2. **Which unique-version form to use.** + - The sources report recommends `X.Y.Z.N-socket.M`; the shapes and lock-cache reports recommend `X.Y.Z.N`. + - The trade-off: plain `X.Y.Z.N` collides with a real upstream 4-part version. The prerelease form never collides, but `pack` warns about it: **VERIFIED** `warning NU5104: A stable release of a package should not have a prerelease dependency` for `[13.0.3.1-socket.1]`, and no warning for `[13.0.3.1]`. + - Both forms leak into the nuspec of a packed library: **VERIFIED** `version="[13.0.3.1-socket.1]"` and `version="[13.0.3.1]"`. +3. **How serious NU3005 is.** + - The shapes report says keeping the signature "does not work" and cites NU3005. + - The lock-cache and signing reports verified that NU3005 is only a warning and the package installs as unsigned. + - Reconciled: a malformed signature entry means "treated as unsigned". It fails only under `require` (NU3004) or when warnings are treated as errors (DOCS). +4. **Whether NuGet trusts `.nupkg.sha512`.** + - The sources report says its content is ignored in a hierarchical feed; the lock-cache report says its text is copied when `.nupkg.metadata` is rebuilt for an unsigned package in the package folder. + - Both are correct, in different contexts. Either way the file is not a trust anchor. +5. **Implications across reports for today's code.** + - The code map says vendoring appends an exact-id mapping to our feed. The shapes report verified that id-wide mapping breaks every other version of that id in the repo (NU1102 for Tool's 12.0.1). So current vendoring likely breaks multi-version repos whenever `nuget.config` already has a mapping. + - The hosted lock rewrite matches on id regardless of version (`redirect/mod.rs:5431`). Combined with the shapes finding that one id resolves to different versions per project, this would rewrite the wrong versions' entries. +6. **Different bytes from the two build routes.** + - depscan repacks STORE-only; the local rebuild uses its own deterministic re-zip. The signing report verified that every layout change changes `contentHash`. + - So the service route and the local route give different lock pins. Verify and revert must hash the committed artifact, never a rebuild. + +## 2. DOCS claims worth verifying + +| Claim | Status | +|---|---| +| A dot-prefixed folder is excluded from SDK default globs (shapes) | **VERIFIED.** `.socket/nuget-packages/zz/1.0.0/Broken.cs` under a root-level csproj: `-getItem:Compile` lists only `Program.cs` and the build succeeds. Control: the same file in `pkgs-visible/` fails with `error CS1040`. | +| Local folder feeds are not HTTP-cached (lock-cache) | **VERIFIED (partial).** After restoring from a folder feed there are no newtonsoft entries in `NUGET_HTTP_CACHE_PATH`, and `.nupkg.metadata` `source` is the folder path. | +| An exact `[x]` dependency against a 4-part version gives NU1608 | Not verified. It needs a real package with an exact-version dependency. It matters for option B together with `TreatWarningsAsErrors`. | +| CPM allows one `PackageVersion` per id | Not verified. Cheap to check. | +| `require` mode has no per-package exemption; an untimestamped signature expires with its certificate | Not verified. The first is probably right given the NU3004 behaviour. | +| nuget.exe behaviour for packages.config | Unverifiable here (see §4). | + +## 3. Research topics nobody covered, and my results + +Of the topics on the list, only packages.config is still open (§4). The rest are covered at least partly. These next ones were not on the list; the first four are now answered: + +- **V1: `dotnet nuget locals --clear` against a committed seed.** + - With `RestorePackagesPath=$(MSBuildThisFileDirectory).socket/nuget-packages` in `Directory.Build.props`, `locals all --list` does not show the repo folder, and `--clear` leaves the seed in place. **VERIFIED** + - With `nuget.config` `` it printed `Clearing NuGet global packages folder: .../r/.socket/nuget-packages` and **deleted the committed seed**. **VERIFIED** + - Design rule: never point `globalPackagesFolder` in `nuget.config` at committed data. +- **V2: a seed bypasses sources and mapping.** + - The mapping mapped only `Humanizer.*` to nuget.org and left Newtonsoft.Json unmapped: restore succeeded, with no NU1100, from the seed. **VERIFIED** + - The only source was `./does-not-exist`, `obj/` was deleted, and `dotnet restore --locked-mode` printed `Restored ... (in 252 ms)`. `dotnet run` printed `{"a":1}` and the output dll ends with `SOCKETPATCHED`. **VERIFIED** + - This confirms the lock-cache claim that mapping is never consulted once a package is in the folder. +- **V2b: restore does not touch the seed.** `diff -r` of the seed before and after restore, build and run found no differences, so the git tree stays clean. **VERIFIED** +- **V3: dot-folder globbing.** See the table in §2. A repo-local folder under a root csproj must be dot-prefixed, or covered by `DefaultItemExcludes`. **VERIFIED** +- **Still uncovered:** + - `NuGetLockFilePath` and the `RestoreLockedMode` property + - static-graph restore (`RestoreUseStaticGraphEvaluation`) + - whether Visual Studio or nuget.exe honour `RestorePackagesPath` + - `dotnet test` with a repo-local folder + - macOS and Windows case-insensitivity for the seed path + - Dependabot/Renovate rewriting the version or lock + +## 4. Remaining unknowns + +- **packages.config.** A legacy csproj that imports `Microsoft.Common.targets` with a `packages.config`, run through `dotnet msbuild -t:restore -p:RestorePackagesConfig=true`, printed `Nothing to do. None of the projects specified contain packages to restore.` **VERIFIED.** DOCS: packages.config restore exists only in desktop MSBuild or nuget.exe. mono and nuget.exe are not installed, so it cannot be tested on this machine; it needs a Windows or mono CI leg. +- **CI overrides.** A CI job that passes `-p:RestorePackagesPath` or `--packages` bypasses the seed. With the upstream hash in the lock this silently gives an unpatched build; with the patched hash it fails with NU1403 (shapes, VERIFIED). There is no way to prevent it, only to fail closed. +- **Signature policy with a seed** (see §1.1). We would need a detection rule for an effective `require` policy across the user, machine and repo configs. +- **Pack leak for option B.** There is no verified mitigation other than limiting option B to non-packable projects or transitive `PrivateAssets=all` redirects. + +## 5. Implications for vendoring a patched package with the upstream id+version + +1. The id+version can stay the same **only inside a package folder the repo owns**: + - **Location:** `RestorePackagesPath=$(MSBuildThisFileDirectory).socket/nuget-packages`. Import it from an Exists-guarded `.socket` props file that is chained into every nearest `Directory.Build.props`. The folder must be dot-prefixed (V3) and must not be set through `nuget.config` (V1). + - **Seed contents:** `//` holding `.nupkg.metadata`, the lowercase nuspec and `lib/`. No `.nupkg` or `.sha512` is needed. Git sees no changes after a restore (V2b). + - **Lock pin:** put the patched hash in both `.nupkg.metadata` and every lock in the closure, so bypassing the folder fails closed with NU1403. + - **Cost:** every other package also downloads into the repo folder, so `.gitignore` needs negation rules for the seed. +2. With the seed in place, **no `nuget.config` edit is needed for the patched package** (V2). That removes the fragile source-mapping surgery, including the catch-all `*` and the multi-version NU1102 breakage. Revert is: delete the seed, restore the original lock hashes, and remove the props import. There is no global-cache eviction, because the shared package folder is never used. +3. The seed skips signature validation. The tool must detect an effective `signatureValidationMode=require` and refuse, or sign the package and document the `` signer entry. Otherwise the tool silently weakens the organisation's policy. +4. Any shared-cache variant, meaning today's folder feed plus mapping, cannot be made safe. All reports agree on this: the first copy written wins, the wrong copy leaks both ways, and NU1403 repeats until the entry is evicted. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/depscan.md b/docs/design/nuget-vendoring-research/depscan.md new file mode 100644 index 000000000..cd26a6d4f --- /dev/null +++ b/docs/design/nuget-vendoring-research/depscan.md @@ -0,0 +1,204 @@ +# depscan backend research for NuGet vendoring (for socket-patch v5) + +## TL;DR +- depscan already builds **one prebuilt patched `.nupkg` per patch**. It keeps the **same id and version as upstream**, repacks the zip STORE-only, strips `.signature.p7s` and adds no signature. The bytes are written once to object storage, and their **sha512 SRI** is saved as `package_integrity`. +- You can get it two ways: + - **Artifact route:** `/patch/nuget/...`. The vendoring service (`POST /v0/orgs/{org}/patches/package`) returns this URL, and socket-patch's `service_fetch.rs` already downloads and checks it. + - **Single-package NuGet v3 feed:** `/patch-registry/nuget/{token}/{uuid}/index.json`, used by hosted mode. +- **For unsigned packages, NuGet's `contentHash` is the base64 sha512 of the nupkg bytes.** That equals `package_integrity` with the `sha512-` prefix removed. A dotnet e2e test proves this. +- **A patched-version suffix exists for Maven (`-socket.`) and Go (`-socketpatch.`). NuGet has none.** NuGet is always served under the upstream id+version. +- **Nothing is signed anywhere.** `sign.ts` is an identity stub, and there is no key custody. +- **No NuGet-specific vendoring artifact exists server-side**, beyond the plain `.nupkg`. For comparison, gem has a stub gemspec, Go has the gopatch zip and npm has a berry zip. + +--- + +## 1. Hosted NuGet serving (patch-server) + +**Server entry:** `depscan/workspaces/patches/src/services/patch-serving/server.ts:65-80` registers three route families: +- `/patch/...`: `registerPatchServeRoute` in `serve-route.ts` +- `/patch-registry/...`: `registerPatchRegistryRoutes` in `registry-routes.ts:32`, one regex route that parses `req.path` +- `/gopatch/...` + +**URL grammar:** `depscan/workspaces/lib/src/socket-patch/patch-url.ts:1-23`. These parsers are shared by the server and SBOM recognition. +- Artifact URL: `https://{host}/patch/{eco}/{name}/{version}/{token}/{patch-uuid}/{filename}` (`parsePatchServeUrl`, :82) +- Registry URL: `https://{host}/patch-registry/{eco}/{token}/{patch-uuid}/` (`parsePatchRegistryUrl`, :156) +- Vendored path: `file:.socket/vendor/{eco}/{patch-uuid}/{leaf}` (`parseSocketVendorPath`, :253, a TS port of socket-patch `vendor/path.rs`) + +**NuGet v3 feed:** `nugetDecision` in `patch-serving/registry-decision.ts:361-427`. This is a pure function. It serves a feed containing exactly one package version, pinned by the uuid in the URL: + +| tail | response | +|---|---| +| `index.json` | Service index with only two resources: `PackageBaseAddress/3.0.0` → `{base}/flat/` and `RegistrationsBaseUrl/3.6.0` → `{base}/reg/` (:370-387). No search, autocomplete or catalog. | +| `flat/{idLower}/index.json` | `{versions:[verNorm]}` (:389-396) | +| `flat/{idLower}/{verNorm}/{idLower}.{verNorm}.nupkg` | Streams the object-store bytes with `ETag: ""` (:397-406) | +| `reg/{idLower}/index.json` | Minimal registration: one page, one leaf, with `catalogEntry:{id,version}` and `packageContent`. **No `packageHash`, no dependency groups, no `listed`** (:407-425) | + +**Gates, before any of the above** (`evaluateRegistryDecision`, :190-222): +- `unpublished_at` set → 410 +- ecosystem mismatch → 404 +- `package_status !== 'built'` → 408, or 404 if the build failed + +**Artifact headers:** `archive-response-headers.ts:11` +- `Cache-Control: public, max-age=3600, no-transform`, deliberately not `immutable`, so that revoking a grant still has an effect. +- There is no `X-Socket-Patch-Unsigned` header, although `sign.ts` suggests one. + +**Bytes served:** the output of `nugetRepacker` in `patches/src/repack/repackers/nuget.ts:35-69`: +- Unzips the upstream nupkg and substitutes the patched files. +- Leaves the nuspec, id and version unchanged. +- Removes `.signature.p7s` via `stripSignedArchiveMetadata` (`patches-shared/src/archive/repack-utils.ts:1290-1314`). +- Re-zips with `store: true`, level 0 (`repack-utils.ts:696-701`). +- The doc comment (:24-33) says unsigned output is the "current v1 contract". + +**Upstream fetch:** `patches/src/repack/upstream/nuget.ts:13-62`. +- Downloads from `api.nuget.org/v3-flatcontainer` using the lowercased id and normalized version. +- Does **not** verify against the registry `packageHash`. Per the comment at :49-59, only the per-file `before_sha` check applies. + +**Hash computation:** `patches/src/services/patch-package/build.ts:309-320`. +- Computes `sha512-` (`package_integrity`), sha256 (`package_blob_file_hash`), sha1 and md5 over the archive. +- Storage is write-once (:235-240); `gcs-store.ts:194-205` re-hashes on read-back. + +**Proof that `contentHash` equals `package_integrity` without the prefix:** `patches/src/test/integration/installers/nuget.installer.e2e.test.ts:1-36, 166-261`. +- Uses a real `dotnet restore` against a folder feed with ``. +- Runs `--locked-mode` with fresh `NUGET_PACKAGES` and `NUGET_HTTP_CACHE_PATH`. +- A negative leg swaps in the upstream package and expects NU1403. +- Note: the test isolates the global packages folder precisely **because** the same id+version collides in the global packages cache. + +## 2. Vendoring service contract (server ↔ client pairing) + +**Endpoint:** `POST /v0/orgs/{org_slug}/patches/package`, defined in `depscan/workspaces/api-v0/src/endpoints/orgs/patches/package.ts:252-258` (operationId `getPatchPackages`). +- **Request** (:37-42): `{uuids[], freeOnly}`. +- **Response** (:137-169): `results[uuid] = {status, url, purl, artifacts[], registryOverride}`. + - `status` is one of `granted | reused | pending_build | build_failed | withdrawn | forbidden | not_found`. + - `artifacts[].kind` is one of `tarball | yarn-berry-zip | gem-stub-gemspec` (:121-134). + - Integrity fields: `{sha512, sha256, sha1, md5, dirhashH1, goModH1, yarnBerry10c0}` (:46-62). + +**Public proxy:** `POST /patch/package` in `depscan/workspaces/patches-api-proxy/src/server.ts:1003-1030` forwards to the same endpoint and forces `freeOnly`. + +**Core logic:** `depscan/workspaces/app/src/patches/patch-package-references.ts` +- `getPatchPackageReferences` (:933). +- `buildPatchPackageArtifacts` (:469-530): the tarball comes first and carries sha512, sha256, sha1 and md5. For NuGet that is the only artifact; it points at the `/patch/nuget/...` serve URL. +- `buildRegistryOverride` for NuGet (:668-683) returns: + - `kind: 'nuget-v3'` + - `indexUrl: {base}/index.json` + - `nugetIdLower` and `nugetVersionNorm` + - **no hash** in the identifiers; the rewriter uses the artifact's sha512. + +**Client side (socket-patch):** + +| Step | File and lines | +|---|---| +| Choose endpoint: authenticated vs `/patch/package` proxy | `crates/socket-patch-core/src/api/client.rs:742-743`, `vendor_package_url` :1373-1386 | +| Two-step request, then download; `patch_server_url` rewrites the download host | `fetch_vendor_package` :1115-1160, `_once` :1242-1350 | +| Secondary artifacts | client.rs :1310-1348 | +| Checks the sha512 SRI floor (plus Go `h1:`) and fails closed on mismatch | `vendor/service_fetch.rs:88-138` (`fetch_verified_archive`) | +| Maven/NuGet path: service first, else local rebuild; `--vendor-source=service` hard-fails | `service_fetch.rs:170-200` (`service_archive_copy`) | +| NuGet vendoring calls it | `vendor/nuget_feed.rs:906` | +| Hosted-mode rewriter reads `nuget_id_lower` / `nuget_version_norm` | `patch/redirect/mod.rs:126-127, 449, 5126-5280` | + +The hosted-mode rewriter is mirrored in depscan: `depscan/workspaces/app/src/patches/registry-rewrite/nuget.ts:1-232`. It: +- adds a source plus a `packageSourceMapping`, with a catch-all for existing sources (:160-211); +- sets `contentHash` to the sha512 without the prefix and `resolved` to `nugetVersionNorm` (:69-112). + +**Answer to (2): yes, a prebuilt patched `.nupkg` is already available for vendoring.** socket-patch already consumes it through `service_fetch`. There is no NuGet-specific variant: no renamed or re-versioned build and no signed build. + +## 3. Patched-version suffix and distinct-identity precedents + +**Maven: `-socket.`** +- Derivation: `depscan/workspaces/app/src/patches/maven-suffix.ts:1-65`. `suffixMavenPom` rewrites the pom's `` and literalizes `${project.version}`, or returns null so the caller falls back to the same GAV. +- Serve: `registry-decision.ts:~430-470`. When the pom rewrite succeeds, the artifact is served only under the suffixed version and the bare-upstream paths return 404. There is deliberately no `maven-metadata.xml`, so version ranges cannot resolve it (fail-closed). +- The pom's sha256 is published as `mavenPomSha256` (`package.ts:93-101`) for a trusted-checksum pin. +- Note: the jar bytes themselves are not re-versioned; only the pom is. + +**Go: `-socketpatch.`, with the module re-homed to `patch.socket.dev/gopatch/`** +- `depscan/workspaces/lib/src/go/gopatch.ts:21-64`. +- `GopatchFlavor` (`repack/ecosystem-repacker.ts:65-80`) is a **second stored artifact** with its own sha512 and `h1:`, stored in `build.ts:395-405, 485-500`. +- Changed content must bump ``, never reuse a version (`build.ts:487-491`; `patch-sync/import-db.ts:56`). + +**Other secondary vendoring artifacts:** +- gem stub gemspec: `repackers/gem-stub-gemspec.ts`, stored in `build.ts:412-419`. +- npm yarn-berry cache zip: `build.ts:344-390`. + +**NuGet:** none. There is no `-socket.N` prerelease, no `+socket` metadata and no distinct id. `normalizeNuGetVersion` actually **drops** `+metadata` (`repack/nuget-version.ts:1-35`). That makes `+socket` useless as a distinguisher, because NuGet also ignores build metadata for identity and for the global packages cache path. + +**Other ecosystems' vendored artifacts server-side:** +- There is exactly one pipeline: the converter build in `services/patch-package/build.ts` feeding the `published_patches.package_*` columns (`queue.ts:410`), then the serve route. +- There is no separate "vendor" build; vendoring reuses the hosted tarball. + +## 4. NuGet identity in depscan + +**PURL:** `depscan/workspaces/lib/src/purl/schema/nuget.ts:1-25`. +- `pkg:nuget/@`, with no namespace. +- The name keeps its case (spec: case-sensitive in the archive, case-insensitive for lookup). +- `purl-full-name.ts:258-263` sets namespace to null. + +**Version normalization (three implementations kept in sync):** +- `patches/src/repack/nuget-version.ts:15-35`: lowercase, drop `+meta`, strip leading zeros, pad to 3 parts, drop a zero 4th part. +- `app/src/patches/patch-package-references.ts:555-590`. +- socket-patch `vendor/nuget_feed.rs`. + +Serve paths, the upstream fetch and `nugetVersionNorm` all use the normalized form. The serve decision checks the tail against the row using `idLower` and `verNorm`. + +**SBOM:** the pipeline NuGet tasks (`pipeline/src/task/cs/nuget/**`) have **no Socket-patch reference detection**. That detection exists for npm, pypi and gem only (grep of `detectSocketPatchReference`/`parseSocketVendorPath` in `pipeline/src`). A patched NuGet dependency therefore shows up as plain upstream `name@version`. If the NuGet package id or version changed, SBOM/purl mapping would need new detection. + +## 5. Signing + +- `patches/src/repack/sign.ts:1-38`: `defaultSign` returns null. +- The comments list a future `authenticode-p7s` kind for NuGet, but state "no key custody / KMS / HSM exists yet". +- Columns `package_signature` and `package_signature_kind` exist (`ecosystem-repacker.ts:55-62`; `queue.ts:410-411`) and are always null. +- There are **no repository or author signatures** for any ecosystem. +- Consequence: consumers using `signatureValidationMode=require` or `` will reject Socket nupkgs. The nuget repacker comment (:27-31) acknowledges this. + +--- + +## What could live server-side for NuGet vendoring + +1. **Keep today's prebuilt `.nupkg` and add the missing integrity pieces.** + - Feasibility: exists; the additions are trivial. + - The artifact and its sha512 are already served. `contentHash` is the sha512 payload of that artifact. + - Cheap additions: + - put `packageHash`/`packageHashAlgorithm` into the registration leaf in `registry-decision.ts:407-425`; + - add dependency groups taken from the nuspec so the registration is spec-complete. + - This does not remove the global-packages-cache collision. + +2. **A Socket-suffixed NuGet flavor, `-socket.` (prerelease), as a second artifact.** + - Feasibility: moderate. The Maven and Go flavors are close templates: `GopatchFlavor` storage in `build.ts:395-500`, and the fail-closed serve/404 pattern for bare paths in `mavenDecision`. + - Work required: + - rewrite `` in the nuspec; + - rename the entry `{id}.nuspec` and possibly rebuild `[Content_Types].xml`/`_rels`; + - the flat path, registration and `normalizeNuGetVersion` then handle the new version as-is; + - add `nugetSuffixedVersion` to `RegistryOverrideIdentifiers` and a new `artifacts[].kind` (for example `nupkg-socket`). + - Benefit: removes the global-packages-cache collision, source-mapping ambiguity and fall-through to nuget.org. + - Costs: + - a prerelease version changes resolution semantics: floating ranges ignore prereleases, NU5104 warnings appear, and transitive `>= x` constraints are still satisfied; + - the consumer must change `` or use central package management (CPM) / `Directory.Packages.props`; + - SBOM must learn to map `-socket.` back to the upstream purl (see §4). + - Using `+socket` metadata instead of a prerelease is **not viable**, because NuGet drops it for identity. + +3. **A distinct package id, e.g. `Socket.Patched.`.** + - Feasibility: low. + - Assembly and type identity would not change, but every transitive dependent still references the original id. It would need a shim or `PackageReference` aliasing that NuGet does not support. Not recommended. + +4. **Serve the vendored feed metadata server-side.** + - Feasibility: exists for hosted mode. + - The single-package v3 feed can be "ejected": socket-patch already gets the bytes. A local feed only needs the nupkg (a folder feed needs no metadata). Nothing extra is needed from the server. + +5. **Repository signature (`.signature.p7s`) with a Socket certificate.** + - Feasibility: not feasible now. There is no key custody (`sign.ts`). + - It would also make `contentHash` follow the signed-package rules, so it would no longer equal the plain sha512. The e2e test's claim would change. + +6. **Upstream `packageHash` recording, for the restore-upstream / revert of workstream WS1.** + - Feasibility: small to moderate. + - The server does not fetch or store the upstream nuget.org `packageHash` (`upstream/nuget.ts:49-59` skips it). + - Persisting the upstream sha512 and signed `contentHash` would let the CLI restore the original `packages.lock.json` entry offline. This addresses "hosted revert unsupported". + +7. **SBOM recognition of patched NuGet dependencies.** + - Feasibility: moderate. It does not exist today for NuGet. + - Needed if option 2 or 3 ships. It is useful even for same-version packages, via the `nuget.config` source URL or a `.socket/vendor/nuget/` path. + +**Items that do not exist anywhere in depscan:** +- a NuGet version suffix or distinct id; +- any signing; +- `packageHash` in the served registration; +- an upstream NuGet digest check; +- a vendoring-specific NuGet artifact; +- NuGet patch detection in SBOM. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/lock-cache.md b/docs/design/nuget-vendoring-research/lock-cache.md new file mode 100644 index 000000000..19a751416 --- /dev/null +++ b/docs/design/nuget-vendoring-research/lock-cache.md @@ -0,0 +1,81 @@ +# NuGet lock-file and package-cache research (SDK 8.0.131, Linux) + +Every experiment ran with its own fresh HOME, NUGET_PACKAGES and NUGET_HTTP_CACHE_PATH under `/research/lock-cache/exp//`. Full notes, commands and outputs are in `/research/lock-cache/FINDINGS.md`. The helper scripts (`env.sh`, `mkapp.sh`, `lockhash.sh`, `work/mkver.py`) and the test feeds are in the same directory. + +Terms used below: +- **GPF** is the global packages folder, normally `~/.nuget/packages`. +- **Patched** means a copy of Newtonsoft.Json 13.0.3 with bytes appended to `lib/net6.0/Newtonsoft.Json.dll` and a `SOCKET_PATCHED.txt` file added. +- **Upstream** means the unmodified package from nuget.org. + +## (a) What the lock file contains +- **VERIFIED:** For a signed package, `contentHash` is the base64 sha512 of the .nupkg with the `.signature.p7s` entry removed. It is not the hash of the file as downloaded. `zip -d t.nupkg .signature.p7s; openssl dgst -sha512 -binary t.nupkg | base64` gave `HrC5BXdl…`, which matches the lock file. The hash of the raw file is `mbJSvHfR…`, which is what the GPF's `newtonsoft.json.13.0.3.nupkg.sha512` holds. +- **VERIFIED:** For an unsigned package, `contentHash` is the sha512 of the file bytes (`hNLqr27u…` for the patched copy, both ways). +- **VERIFIED:** `.nupkg.metadata` in the GPF holds `{"version":2,"contentHash":,"source":}`. +- **VERIFIED** entry shapes: + - Direct: `{"type":"Direct","requested":"[13.0.3, )","resolved":"13.0.3","contentHash":…,"dependencies":{…}}` + - Transitive: `{"type":"Transitive","resolved","contentHash"}` + - Project: `{"type":"Project","dependencies":{"Humanizer.Core":"[2.14.1, )"}}`, with no hash + - CentralTransitive: `{"type":"CentralTransitive","requested":"[13.0.3.1, )",…}`. This needs Central Package Management with `CentralPackageTransitivePinningEnabled`. + +## (b) What restore does when hashes don't match +- **VERIFIED, cold cache:** Empty GPF, feed has the patched package, lock pins the upstream hash, `--locked-mode`. It fails with `error NU1403: Package content hash validation failed for Newtonsoft.Json.13.0.3. The package is different than the last restore.` But the patched package has already been fully extracted into the GPF, `.nupkg.metadata` included. **A failed restore still poisons the cache.** +- **VERIFIED:** GPF has upstream, lock pins patched, feed has patched: NU1403. Nothing is re-downloaded and the GPF is unchanged. +- **VERIFIED:** GPF has patched, lock pins upstream: NU1403. +- **VERIFIED, what NuGet compares against:** + - I left the upstream bytes in the GPF and edited only the `contentHash` in `.nupkg.metadata` to the patched hash. A locked restore against the patched lock **succeeded**. + - Editing only `.nupkg.sha512` still gave NU1403. + - So when a package is already in the GPF, NuGet compares the lock hash with the `.nupkg.metadata` value only. It never rehashes the .nupkg, the extracted files, or the feed bytes. +- **VERIFIED:** NU1403 also happens **without** `--locked-mode` whenever a `packages.lock.json` exists and its hash differs. +- **VERIFIED, other error codes:** + - NU1004: locked mode after a PackageReference changed (`The package references have changed for net8.0…`). + - NU1605: a direct reference to 13.0.3-socket.1 while a dependency needs >=13.0.3 (`Detected package downgrade: Newtonsoft.Json from 13.0.3 to 13.0.3-socket.1`). The SDK treats this warning as an error, yet the lock file was still rewritten. + - NU1102: the lock pins a version the feed doesn't have. +- **VERIFIED, signatures:** + - Keeping a stale signature that is well-formed and is the last zip entry gives `error NU3008: The package integrity check failed. The package has changed since it was signed.` + - A misplaced or malformed signature entry gives only `warning NU3005` and the restore continues. + - Signatures are checked only when a package is extracted, not when it is already in the GPF. + +## (c) The GPF never refreshes an existing package +- **VERIFIED:** I tampered with a dll inside the GPF. The locked restore passed and the tampered dll was used. +- **VERIFIED:** `.nupkg.metadata` is the marker that a package is complete. + - Delete it but keep `.sha512`: NuGet recreates `.nupkg.metadata` from the local files (`"source": null`) and does not re-extract. + - How it recreates the hash: for a signed package it recomputes the hash from the .nupkg; for an unsigned one it copies the text of `.sha512`. + - Delete both files: the package is downloaded and extracted again (the tampered dll went back to its original size). +- **VERIFIED:** `dotnet restore --no-cache --force --force-evaluate` does not refresh a GPF entry. +- **VERIFIED, stale copies leak both ways without a lock file:** + - After the failed restore above left the patched copy in the GPF, an unrelated nuget.org project with no lock file built against it (its output dll ends in `SOCKETPATCH`). + - The other way round: GPF has upstream, feed has patched, no lock. Restore quietly used upstream and wrote a lock with the upstream hash. + +## (d) Repo-local GPF +- **VERIFIED:** A relative `globalPackagesFolder` in nuget.config resolves relative to the nuget.config file, not the current directory. Restoring the solution from `src/app` created the folder at the solution root. +- **VERIFIED:** Restore, build and publish all used it (`project.assets.json` packageFolders and `NuGetPackageRoot` in `nuget.g.props`). `$HOME/.nuget/packages` was never created. `dotnet test` was not run. +- **VERIFIED, which setting wins:** `RestorePackagesPath` (MSBuild property) beats the `NUGET_PACKAGES` env var, which beats nuget.config. So a CI job that sets `NUGET_PACKAGES` overrides a nuget.config setting, but not a `Directory.Build.props` property. +- **VERIFIED:** A relative `RestorePackagesPath` in `Directory.Build.props` resolves per project directory, giving one GPF per project. Use `$(MSBuildThisFileDirectory)`. +- **DOCS:** The HTTP cache stays machine-wide. Local folder feeds are not HTTP-cached. + +## (e) Fallback folders +- **VERIFIED:** I pointed nuget.config's `` at a folder holding the extracted patched package. It was used in place and not copied to the GPF (the GPF stayed empty), and the built dll was the patched one. The lock hash came from the fallback's `.nupkg.metadata` and was byte-identical to the lock produced from a feed. +- **VERIFIED:** The minimum a fallback entry needs is `.nupkg.metadata`, the nuspec and `lib/`. It needs neither the .nupkg nor `.sha512`, and it worked with the only source pointing at a folder that doesn't exist. Without `.nupkg.metadata` the entry is ignored and NuGet goes to the source (NU1301). +- **VERIFIED:** An upstream lock against a patched fallback gives NU1403. The hash check only trusts the value written in `.nupkg.metadata`. +- **VERIFIED, the GPF beats fallback folders:** if the GPF already holds upstream 13.0.3, a patched lock fails with NU1403, and with no lock the upstream copy is used silently. A committed fallback folder is therefore not a vendoring mechanism on its own. It only works together with an isolated GPF. + +## (f) The lock doesn't record the source +- **VERIFIED:** A lock generated from nuget.org passed a locked restore from a local folder feed holding the same bytes, and the lock stayed byte-identical. The lock only pins id, version and contentHash. + +## (g) Version suffixes +- **VERIFIED:** `13.0.3+socket.1` collides completely with 13.0.3. + - The GPF path is `newtonsoft.json/13.0.3`, the lock says `resolved "13.0.3"`, and it says `requested "[13.0.3, )"` even when the csproj says `Version="13.0.3+socket.1"`. + - A second nuget.org project on the same GPF then built against the patched dll. +- **VERIFIED:** `13.0.3-socket.1` gets its own GPF folder, but it sorts below 13.0.3. That causes NU1605 whenever something needs >=13.0.3. +- **VERIFIED:** A four-part `13.0.3.1` gets its own GPF folder and sorts above 13.0.3, so there's no downgrade error. It works as a direct reference and as a CentralTransitive pin. +- **DOCS:** A dependency with an exact `[13.0.3]` range would give NU1608. A real upstream 13.0.3.1 would collide, but that's rare. + +## What this means for vendoring a patched copy with the same id+version as upstream +1. **A shared GPF can't be made safe.** The first copy written wins and is never refreshed. Patched bytes leak to other projects and CI caches, and upstream bytes leak in. A failed locked restore still leaves the wrong copy behind. +2. **The lock hash detects the problem but can't fix it.** It only compares against `.nupkg.metadata`. The only recovery is deleting that GPF entry, so hosted or cached-CI revert needs cache eviction as well as rewriting the lock. +3. **Robust options:** + - **(a) Repo-local GPF** set with `RestorePackagesPath=$(MSBuildThisFileDirectory)…` in `Directory.Build.props`. It beats `NUGET_PACKAGES` and covers restore, build and publish. It could be pre-seeded with the extracted patched package (a `.nupkg.metadata` file plus a minimal file set) and gitignore everything else. + - **(b) A distinct four-part version** such as 13.0.3.1, with Central Package Management transitive pinning. This removes the collision entirely. Don't use `+metadata` (it collides) or `-prerelease` (NU1605). + - **(c) A committed fallback folder.** It gives offline use without copying into the GPF, but only if the GPF doesn't already have that id+version, so it depends on (a). +4. **Always drop `.signature.p7s`** (otherwise NU3008). The patched package's lock hash is then just the sha512 of the rebuilt file, and the current rewrite of the lock `contentHash` must use that. +5. **The current design's catch-all `*` source mapping doesn't help.** Once the package is in any GPF, the mapping is never consulted. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/shapes.md b/docs/design/nuget-vendoring-research/shapes.md new file mode 100644 index 000000000..59802b458 --- /dev/null +++ b/docs/design/nuget-vendoring-research/shapes.md @@ -0,0 +1,155 @@ +# NuGet vendoring across project shapes: findings + +All tests ran on .NET SDK 8.0.131 on Linux. Each experiment had its own HOME, NUGET_PACKAGES and NUGET_HTTP_CACHE_PATH under `exp//`. Patched packages were built by `mkpatch.py`: it appends `SOCKETPATCHED` to the netstandard2.0 dll, adds `socket-patched.txt`, drops the signature and can rewrite the nuspec ``. + +The main test solution has three projects: +- **Lib** references Newtonsoft.Json 13.0.1 directly. +- **App** has a ProjectReference to Lib. +- **Tool** references Newtonsoft.Json.Bson 1.0.2, which pulls in Newtonsoft.Json ≥12.0.1, resolved as 12.0.1. + +Full notes are in `/research/shapes/FINDINGS.md`. The templates, `env.sh`, `mkpatch.py` and the feeds are in the same directory. + +## 1. Multi-project solution with PackageReference and lock files + +**VERIFIED:** each project's lock lists the package itself: +- Lib: `"type": "Direct", "requested": "[13.0.1, )"`. +- App: `"Transitive"`, with its own contentHash, plus a `lib` entry of `"type": "Project"` whose dependencies are `{"Newtonsoft.Json": "[13.0.1, )"}`. +- Tool: `Transitive 12.0.1`. + +So one package id shows up in every lock in the closure, and it can resolve to a different version in each project. + +**A (same id+version):** +- **Source mapping works per package id, not per version.** + - VERIFIED: once Newtonsoft.Json was mapped only to the local feed, Tool failed with `NU1102 Unable to find package Newtonsoft.Json with version (>= 12.0.1) … Versions from nuget.org were not considered`. + - Every version of that id used anywhere in the repo has to be in the local feed, or the id has to be mapped to both sources. + - VERIFIED: mapping to both worked 3 out of 3 times, and the local feed won each time. + - DOCS: which source wins when both hold the same version is not guaranteed. +- **Every project in the closure needs its lock contentHash rewritten.** + - VERIFIED: without it, Lib and App failed with `NU1403 Package content hash validation failed`. + - For an unsigned rebuilt package, contentHash = base64(sha512(nupkg)). VERIFIED: restore passed after rewriting it that way. +- **Global cache poisoning is fatal.** VERIFIED: with upstream 13.0.1 already in the global packages folder, restore failed with `NU1403`, even with `--force`. +- **No-op restore can hide the problem.** VERIFIED: with `obj/` left from an earlier good restore, restore did nothing and hid the poisoned cache. +- **The patched copy leaks into other repos.** VERIFIED: an unrelated project with no lock file, using only nuget.org and sharing the same cache, silently got the patched 13.0.1. +- **Keeping the original signature does not work.** VERIFIED: `NU3005 … signature file entry is invalid … compression method (8)`. +- **Pack is correct.** VERIFIED: the nuspec says `version="13.0.1"`, so there is no leak. + +**B (unique version):** +- **A prerelease suffix is ruled out.** + - VERIFIED: `-socket.1` sorts below the release. It triggered a false vulnerability warning, `NU1903 … 13.0.1-socket.1 has a known high severity vulnerability`. + - VERIFIED: next to a dependency that needs ≥12.0.1 it fails with `NU1605 Detected package downgrade: … from 12.0.1 to 12.0.1-socket.1`. +- **Build metadata does not create a new identity.** VERIFIED: `13.0.1+socket.1` is stripped and resolves as 13.0.1, which is the same as A. +- **A 4th version part works.** VERIFIED: `12.0.1.1` / `13.0.1.1` had no NU1605 and no false NU1903. +- **Metadata conditions must go inside a target.** VERIFIED: a `%(Version)` condition on an item Update outside a target fails with `MSB4191`. +- **Working repo-wide redirect, no csproj edits** (VERIFIED): + - `Directory.Build.targets` imports `.socket/vendor/nuget/socket.targets`. + - That file has a target with `BeforeTargets="CollectPackageReferences"`, which rewrites any PackageReference with `'%(Identity)'=='Newtonsoft.Json' and '%(Version)'=='13.0.1'` to `13.0.1.1`. + - Resulting locks: Lib Direct `[13.0.1.1, )`; App Transitive 13.0.1.1, with its `lib` project entry changed to `[13.0.1.1, )`; a project with a direct 13.0.3 is left alone. + - Then a `--locked-mode` restore with an empty cache passes, and build/publish output contains the patched dll. +- **Old locks fail and must be regenerated for the whole closure.** VERIFIED: `NU1004 The package reference … version has changed`, and on App `NU1004 The project references lib whose dependencies has changed`. +- **If the vendor feed is unusable, restore silently moves to a different version.** + - VERIFIED: with a ≥13.0.1.1 reference and the feed unmapped, NuGet picked 13.0.2 from nuget.org with only `NU1601 … ended up with Newtonsoft.Json 13.0.2`. + - VERIFIED: exact brackets `[13.0.1.1]` turn this into a hard `NU1102`. +- **`RestoreAdditionalProjectSources` only helps when the repo has no source mapping.** + - VERIFIED: set from an imported props file, it adds the feed without touching nuget.config. + - VERIFIED: when the repo has source mapping, that feed is used only if mapped by its **absolute path**. A relative key failed (NU1102). So nuget.config has to be edited anyway. +- **Pack leaks the new version to consumers.** + - VERIFIED: the nuspec gets `version="13.0.1.1"` (or `[13.0.1.1]` with brackets). + - VERIFIED: a consumer with an empty cache then fails with `NU1102`. Without brackets it would drift to 13.0.2 with NU1601. + - VERIFIED: a transitive redirect with PrivateAssets=all does not leak. + +**C (repo-local package folder):** +- **`RestorePackagesPath` in Directory.Build.props wins.** + - VERIFIED: `RestorePackagesPath=$(MSBuildThisFileDirectory).socket/nuget-packages` beat the NUGET_PACKAGES env var (the env cache stayed empty). + - VERIFIED: a pre-extracted patched `newtonsoft.json/13.0.1` was used as is; other packages were downloaded into the repo folder. +- **The seed can be minimal.** VERIFIED: `.nupkg.metadata` + nuspec + lib/ is enough; the .nupkg and .sha512 are not needed and NuGet does not re-hash the files. +- **The seed can carry the upstream hash, so the locks stay as they are.** VERIFIED: a `.nupkg.metadata` with the upstream contentHash, with locks untouched, passed `--locked-mode` and the patched dll was built. +- **It survives forced restores.** VERIFIED: `restore --force --no-cache` left the seed in place. +- **A CI override bypasses it.** + - VERIFIED: with `-p:RestorePackagesPath=…` and the upstream hash in the locks, restore silently downloads the unpatched package. + - VERIFIED: with the patched hash in the locks, it fails closed with NU1403. +- **Fallback folders are unsafe.** VERIFIED: `RestoreFallbackFolders` works with an empty cache, but if the cache already holds upstream 13.0.1, the cache wins and the result is silently unpatched. +- **Folder name and CI cost** (DOCS): a dot-prefixed folder is excluded from SDK default globs. Every package then lives in the repo, so a `.gitignore` with negation rules is needed and CI caching of the global folder stops helping. +- **Pack is correct.** VERIFIED: the nuspec says 13.0.1. + +## 2. Central Package Management + +- **Lock entry types** (VERIFIED): + - Transitive packages that have a PackageVersion appear as `"CentralTransitive"` even with pinning off. + - Tool showed `requested [13.0.1, )` but `resolved 12.0.1`. + - A project with a VersionOverride shows as Direct. +- **Turning pinning on changes nothing until locks are re-evaluated.** VERIFIED: with existing locks, both `--locked-mode` and a plain restore passed and kept Tool at 12.0.1. Only `--force-evaluate` moved it to 13.0.1. +- **Redirect without editing Directory.Packages.props** (VERIFIED): + - A `` in the imported targets file works. A non-matching version check did not redirect. + - Lib and App moved to 13.0.1.1. The VersionOverride project was untouched. + - Without pinning, Tool's requested range became `[13.0.1.1, )` but it still resolved 12.0.1. + - With pinning, Tool moved to 13.0.1.1. + - Old locks fail with `NU1004 Mistmatch between the requestedVersion of a lock file dependency marked as CentralTransitive…`. +- **Transitive-only project without pinning** (VERIFIED): + - Needs `` scoped to that project. + - `Version=` fails with `NU1008`. If the repo sets `CentralPackageVersionOverrideEnabled=false`, it fails with `NU1013`. +- **Pinning should not be the patch mechanism.** CPM allows one PackageVersion per id (DOCS). Turning pinning on changes other projects' versions: VERIFIED, Tool went from 12.0.1 to 13.0.1. +- **Answer to "only Directory.Packages.props?"** No. An imported targets file can do the redirect, but every lock in the closure still has to change. + +## 3. packages.config + +- **Not testable here.** VERIFIED: `which mono nuget` finds nothing. `dotnet restore` on the solution, on the csproj, and `msbuild -t:restore -p:RestorePackagesConfig=true` all print `Nothing to do. None of the projects specified contain packages to restore.` +- **How nuget.exe works** (DOCS): + - It restores to `/packages/./`, or to `repositoryPath` if set in nuget.config. + - There is no lock file and no contentHash; projects reference dlls through HintPath. + - It skips any folder that already holds `..nupkg`. +- **Verdicts:** + - C (pre-seed or commit the `packages/` folder) is the practical option. + - B needs every packages.config and HintPath edited. + - A with a folder feed works, but the packages/ folder poisons it the same way the global cache does. + +## 4. Directory.Build.props/targets injection + +- **Only the nearest file is imported.** + - VERIFIED: a nested `src/Directory.Build.props` hides the root one. + - VERIFIED: chaining with `$([MSBuild]::GetPathOfFileAbove(Directory.Build.props, $(MSBuildThisFileDirectory)..))` restores it. + - VERIFIED: the root .targets file is still found independently of the .props. + - So the injector must add its Import to the nearest existing file for every project. +- **A separate imported file works.** VERIFIED: an Exists-guarded `.socket/vendor/nuget/socket.props|targets` worked for all the properties and items above. +- **Limits outside MSBuild** (DOCS): source mapping can only be set in nuget.config. `Directory.Build.rsp` affects the msbuild CLI only. + +## 5. Transitive redirect + +VERIFIED: a per-project PackageReference with PrivateAssets=all (or VersionOverride under CPM) changes the lock entry from Transitive or CentralTransitive to Direct, with the new requested range. The parent's dependency line stays the same (Bson still lists `"Newtonsoft.Json": "12.0.1"`). No csproj edits are needed. + +## 6. Build, publish and pack + +- **Build and publish:** VERIFIED patched dll in the output for A, B (4th part) and C. +- **Pack:** VERIFIED A and C are correct. B leaks the new version into the nuspec whenever the package is a direct dependency of a packable project. + +## 7. Floating versions and ranges + +- **Resolution:** VERIFIED `13.0.*` and `13.*` resolve 13.0.4 today; `[13.0.1,14.0)` resolves 13.0.1. +- **Lock files win:** VERIFIED a plain restore with an existing lock keeps the locked versions. +- **4th-part versions are not picked up on their own:** VERIFIED 13.0.1.1 was not chosen for the range with `--force-evaluate`, so B has to replace the floating spec with the exact version. +- **A and C patch whatever the lock resolved:** DOCS/inferred, re-evaluating after upstream publishes a newer version silently drops the patch. + +## Verdicts + +| Shape | A (same version) | B (4th-part version + redirect) | C (repo-local packages folder) | +|---|---|---|---| +| Multi-project sln | Fragile: id-wide mapping, hash rewrite in every lock, cache poisoning and leaks to other repos | Works with the imported-targets redirect and exact brackets; locks must be regenerated; pack leaks | Works and is isolated from the global cache; a CI override bypasses it | +| CPM | Same as sln | Works with a PackageVersion Update; transitive-only projects need VersionOverride; the pinning trap | Works | +| packages.config | Poor | Heavy edits | Best (seed `packages/`) | +| Transitive-only | Automatic (id-based) | Per-project redirect, lock type becomes Direct | Automatic | +| pack/consumers | Correct | Leaks the new version | Correct | +| Floating/ranges | Tied to the lock | Must pin exactly | Tied to the lock | + +## What this means for a patched copy with the upstream id+version + +- **Same id+version is inherently unsafe in any shared cache.** Both the global packages folder and the packages/ folder are keyed on id/version alone. Every A failure seen here comes from that: NU1403 poisoning, silent leaks into other repos, per-id mapping breaking other versions. +- **The same identity is only safe in a cache the repo owns (C).** + - Recommended form: an Exists-guarded `RestorePackagesPath` import chained into every nearest Directory.Build.props. + - Seed a minimal extracted package. + - Write the patched contentHash into both `.nupkg.metadata` and the locks, so that bypassing the folder fails closed (NU1403) instead of silently restoring upstream. + - Do not use fallback folders. +- **If a unique version (B) is used:** + - Use a 4th version part, never a prerelease suffix or `+metadata`. + - Pin it with exact brackets. + - Apply the redirect through an imported targets file, not csproj edits. + - Regenerate the locks for the whole closure. + - Handle the pack leak, for example by limiting B to non-packable projects. \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/signing.md b/docs/design/nuget-vendoring-research/signing.md new file mode 100644 index 000000000..810aa2eff --- /dev/null +++ b/docs/design/nuget-vendoring-research/signing.md @@ -0,0 +1,102 @@ +# NuGet signing and verification when vendoring a patched same-id+version nupkg + +**Headline:** once we change a package we have to drop its `.signature.p7s`. Keeping it fails restore with NU3008, even on Linux with no special settings. Dropping it works under the default policy but fails under `require` with NU3004. Signing, or not signing, never changes the lock `contentHash`. That means we can sign with a Socket certificate and keep lock pinning as it is. + +Setup: .NET SDK 8.0.131 on Linux. Each experiment had a fresh HOME, NUGET_PACKAGES and NUGET_HTTP_CACHE_PATH under `exp//`. The test package was Newtonsoft.Json 13.0.3 from nuget.org, which is author-signed and carries the nuget.org repository countersignature. The "patch" appends bytes to `lib/net6.0/Newtonsoft.Json.dll`. Restores used a single local folder feed (``) with `RestorePackagesWithLockFile`. + +## Environment +- **VERIFIED:** signature verification is on by default on Linux with SDK 8. `dotnet nuget verify` prints `X.509 certificate chain validation will use the fallback certificate bundle at '/usr/lib/dotnet/sdk/8.0.131/trustedroots/codesignctl.pem'`. A tampered signed package fails restore with no env var or config set. +- **VERIFIED:** the certificate revocation servers can't be reached from here. Genuine packages only get NU3018/NU3028 `RevocationStatusUnknown` warnings, even in `require` mode. +- **VERIFIED:** timestamp.digicert.com returns HTTP 403 through the proxy, so I could not test timestamped signing. + +## (a) Keeping vs dropping `.signature.p7s` (default mode, `accept`) +- **VERIFIED:** modified dll with the signature kept fails: `error NU3008: ... The package integrity check failed. The package has changed since it was signed.` +- **VERIFIED:** re-zipping with no content change and the signature kept also fails with NU3008. The signature covers the zip bytes, not just the file contents. +- **VERIFIED:** modified dll with the signature dropped installs cleanly, with no warnings. +- **VERIFIED:** `DOTNET_NUGET_SIGNATURE_VERIFICATION=false` turns verification off entirely. The tampered, still-signed package then installs. +- **VERIFIED:** if the signature entry has non-zero external file attributes (Python `zipfile` writes `0o600<<16` by default), restore only warns (`NU3005: The package signature file entry is invalid ... 'external file attributes' has an invalid value (25165824)`) and installs the package as if it were unsigned. + +## (b) `require` mode (enterprise setup) +The config was `signatureValidationMode=require` plus a `` entry with the three nuget.org repository certificate fingerprints (SHA256 `0E5F38F5…`, `5A2901D6…`, `1F4B311D…`). + +- **VERIFIED:** the genuine package installs even from a local folder feed. Repository trust is tied to the certificate, not the source URL. +- **VERIFIED:** the unsigned patched package fails: `error NU3004: ... signatureValidationMode is set to require, so packages are allowed only if signed by trusted signers; however, this package is unsigned.` +- **VERIFIED:** the patched package with the upstream signature kept fails with NU3008. +- **VERIFIED:** `DOTNET_NUGET_SIGNATURE_VERIFICATION=false` overrides `require`, and the unsigned package installs. +- **VERIFIED:** config precedence. With `require` in the user-level `~/.nuget/NuGet/NuGet.Config` and `accept` in the repo's `nuget.config`, the unsigned package installs, so a repo config can weaken the policy. With `require` at user level only, it fails with NU3004. +- **VERIFIED:** `trustedSigners` add up across config levels. The user level had `require` plus the nuget.org ``; the repo level added only an ``. Both the Socket-signed patched package and genuine nuget.org packages installed. +- **VERIFIED:** if the global packages folder already holds the extracted patched package, restore succeeds under `require` with no check at all. Signatures are only verified when a package is extracted into that folder. +- **DOCS:** `require` mode has no per-package or per-source exemption. `trustedSigners` entries are per certificate: an `` fingerprint, or a `` fingerprint with optional ``. `allowUntrustedRoot` only relaxes chain building for a listed certificate; it does not let unsigned packages through. + +## (c) Signing with a self-signed certificate (`dotnet nuget sign`) +I made the certificate with `openssl req -x509` (extended key use codeSigning, key use digitalSignature) and exported it to a pfx. + +- **VERIFIED:** signing works on Linux in about 1.1 s with exit code 0. `dotnet nuget sign moddrop.nupkg --certificate-path cert.pfx --certificate-password pw -o signed` warns NU3002 (no timestamper), NU3042 (root not in the codesignctl.pem bundle) and `NU3018: UntrustedRoot: self-signed certificate`. +- **VERIFIED:** restore results for the signed patched package: + +| Config | Result | +|---|---| +| Default `accept`, nothing trusted | Installs, with warnings NU3018 ("signing certificate is not trusted by the trust provider"), NU3027 (not timestamped) and NU3042 | +| `require` + `` fingerprint, `allowUntrustedRoot="false"` | `error NU3018` | +| `require` + `` fingerprint, `allowUntrustedRoot="true"` | Installs, only a NU3027 warning | +| `require`, no `` | `error NU3018` plus `error NU3034: This package is signed but not by a trusted signer` | + +- **VERIFIED:** re-signing a package that still has the upstream signature fails without `--overwrite` (`NU3001: The package already contains a signature`). With `--overwrite` it succeeds and installs under `require` with the author trusted. +- **VERIFIED:** `dotnet nuget sign` writes a valid signature entry even when the input zip has Python-style attributes. +- **VERIFIED (from `--help`):** the CLI can only create author signatures, so we cannot make a repository signature like nuget.org's. **DOCS:** repository signing exists only in the NuGet.Packaging API. +- **DOCS:** a signature without a timestamp stops being valid when the certificate expires. +- **Cost:** about a second per package, and no system trust store changes are needed. The user does have to add one `` entry with `allowUntrustedRoot="true"` if they use `require`. + +## (d) `dotnet nuget verify --all` (all VERIFIED) +- Unsigned: `error: NU3004: The package is not signed.` (exit 1) +- Modified with the upstream signature kept: prints `Signature type: Author` and `Signature type: Repository`, then `error: NU3008` (exit 1). +- Self-signed: `error: NU3018 ... not trusted by the trust provider` (exit 1). `--certificate-fingerprint` alone does not change that. With `--configfile` pointing at a config that has the `` entry, it exits 0 with only a NU3027 warning. +- Genuine: `Successfully verified package`, with revocation warnings. + +## (e) Global packages folder metadata and `contentHash` +- **VERIFIED:** the package's folder holds `.nupkg.metadata` (`{version:2, contentHash, source}`), `..nupkg.sha512`, a byte-identical copy of the nupkg, and an extracted `.signature.p7s`. The metadata records nothing about the signer. +- **VERIFIED: the lock `contentHash` does not depend on the signature file.** This corrects the assumption in the brief. + - For signed packages, `contentHash` is the SHA-512 of the archive with the `.signature.p7s` entry removed from both the file data and the central directory, and the end-of-archive record offsets and counts adjusted. My `signed_content_hash.py` reproduces the upstream `HrC5BXdl…zQ==` exactly. + - That differs from SHA-512 of the whole file (`mbJSvHfR…kg==`), which is what `.nupkg.sha512` stores. + - The unsigned patched package, the Socket-signed one and the re-signed one all produce the same `contentHash` (`Lb2f3WlJ…`), equal to SHA-512 of the unsigned file. + - `dotnet nuget sign` appends the signature and leaves every other byte unchanged. Swapping signed and unsigned files passes `--locked-mode`. +- **VERIFIED:** when the global packages folder already has upstream and the lock pins the patched hash, `--locked-mode` fails with `error NU1403: Package content hash validation failed`. Without a lock file, restore silently uses upstream and writes the upstream hash into the new lock. + +## (f) Zip layout and packaging metadata files (all VERIFIED; unsigned packages in `accept` mode all installed) +- These variants all restored fine: + - `.psmdcp` removed, even though `_rels/.rels` still points to it + - `_rels/.rels` and `.psmdcp` both removed + - `[Content_Types].xml` removed + - all three removed + - a new file with an extension not listed in `[Content_Types].xml` (`.patchmeta`, extracted to `lib/net6.0/`) + - entries in reverse order + - all entries stored without compression +- The version with all three files removed, then self-signed, installed under `require`. The console app built with `-m:1`, ran, and the copied dll ended with the patch marker. +- The global packages folder never contains `[Content_Types].xml`, `_rels` or `.psmdcp`. +- Every layout change produced a different `contentHash`, so the repack must be deterministic for the hash to be reproducible. +- **DOCS:** other tools that read these files the Office-document way (older `nuget.exe push`, Package Explorer) may still expect them. Keeping them, and adding a `Default` entry for any new extension, costs nothing. + +## Implications for each vendoring mechanism +1. **Same id+version, unsigned (current approach).** + - Works under `accept`. Always fails under `require` (NU3004), with no per-package exemption. + - The upstream `.signature.p7s` must always be removed (NU3008). A hand-written signature entry with non-zero attributes is silently treated as unsigned. + - The lock pin, plain SHA-512 of the file, is correct for unsigned packages. + - The main risk is a collision with an existing copy in the global packages folder (NU1403 in locked mode, silently using upstream otherwise). That comes from reusing id+version, not from signing. +2. **Same id+version, signed by Socket or a repo-local author certificate.** + - The lock hash is unaffected, and signing is cheap. + - Under `accept` the only cost is warnings: NU3018/NU3042 go away only if the certificate chains to a root in the SDK bundle, and NU3027 goes away only with a timestamp. + - Under `require` the user needs one `` entry with `allowUntrustedRoot="true"` for a self-signed certificate. It can go in the repo's `nuget.config` because it merges with the enterprise-level `trustedSigners`. This is the setup to document for enterprises. + - A self-signed or repo-local key is only as trustworthy as whoever can write to the repo. + - A CA-issued Socket code-signing certificate with a timestamp would remove `allowUntrustedRoot` and the warnings. Signing could happen on Socket's server, because the signature does not affect the lock hash. We cannot reproduce the nuget.org repository signature. +3. **Pre-filling the global packages folder or a fallback folder.** This skips verification even under `require`, which means it quietly gets around enterprise policy. We should not document it. +4. **A renamed id or new version (e.g. `13.0.3-socket.1`).** The signing situation is the same (unsigned fails NU3004 under `require` unless Socket-signed), but it removes the global-folder and lock collisions. This is inferred, not run. +5. **Overrides we should not recommend.** `DOTNET_NUGET_SIGNATURE_VERIFICATION=false` and a repo-level `signatureValidationMode=accept` both work, but both quietly weaken an organization's policy. The tool should detect an effective `require` and fail with a clear message pointing to the `` instruction. + +Everything is in `/research/signing/`: +- `FINDINGS.md` — full notes with commands and output +- `env.sh`, `run.sh` — setup and restore scripts +- `repack.py` — rebuilds the test packages +- `signed_content_hash.py` — the signed-package hash calculation +- `cert/` — the test certificate +- `v/`, `signed*/` — the package variants +- `exp//` — one folder per experiment \ No newline at end of file diff --git a/docs/design/nuget-vendoring-research/sources.md b/docs/design/nuget-vendoring-research/sources.md new file mode 100644 index 000000000..dae0627e2 --- /dev/null +++ b/docs/design/nuget-vendoring-research/sources.md @@ -0,0 +1,134 @@ +# NuGet sources, packageSourceMapping and nuget.config research (SDK 8.0.131, NuGet 6.8.2) + +The tool harness refused to let me write `FINDINGS.md`, so the notes are below instead of in that file. The experiment dirs, helpers (`env.sh`), test packages and the delayed-HTTP feed server are under `/research/sources/` (in `exp/`, `pkgs/` and `httpfeed/server.py`). Every experiment ran with a fresh HOME, NUGET_PACKAGES, NUGET_HTTP_CACHE_PATH and DOTNET_CLI_HOME. + +To tell which bytes won, I read `gpf///SOCKET_PATCHED.txt` and the `source` field in `.nupkg.metadata`. "gpf" below means the global packages folder (`~/.nuget/packages`). + +**Command not available:** `dotnet nuget config paths` does not exist in 8.0.131. It fails with `error: Unrecognized command or argument 'config'` (VERIFIED). I used `dotnet restore -v:n` (the "NuGet Config files used" and "Feeds used" lines) and strace instead. + +## (a) Local folder feeds + +**Flat feed** +- A file named `.*.nupkg` resolves. The filename match is case-insensitive (`Newtonsoft.Json.13.0.3.nupkg`, lowercase and UPPERCASE all worked). Even `Newtonsoft.Json.13.0.3-whatever.nupkg` worked, because the version comes from the nuspec inside the package (VERIFIED). +- These fail with NU1101 (VERIFIED): + - `zzz-random.nupkg` + - `Newtonsoft.Json.nupkg` + - a nupkg in a subdirectory (the flat feed is not recursive) + +**Hierarchical feed (`//`)** (VERIFIED) +- It needs three files: + - the lowercase nupkg, + - `.nupkg.sha512`, which acts as the existence marker (nupkg + nuspec without it gives NU1101), + - `.nuspec` (nupkg + sha512 without it gives `error NU5037: The package is missing the required nuspec file`). +- **Linux needs lowercase.** A `Newtonsoft.Json/13.0.3/Newtonsoft.Json.13.0.3.nupkg` layout gives NU1101. +- **The `.sha512` content is not trusted.** I put garbage in it, and the gpf and lock contentHash still came out as the real SHA512 of the nupkg (`dkeh7b…`). NuGet re-hashes the package on install. +- **Dependencies come from the loose `.nuspec`, not the one inside the nupkg.** A loose nuspec with an extra dependency made restore pull Humanizer.Core. The nuspec extracted into gpf had no such dependency. So the loose nuspec must be byte-identical to the inner one. + +**`dotnet nuget push -s `** (VERIFIED) +- An empty dir gets a flat file, `Newtonsoft.Json.13.0.3.nupkg`. +- A dir that already has one full hierarchical entry (nupkg + nuspec + sha512) gets a full expanded entry: lowercase dirs, `.nupkg.sha512`, nuspec, `.nupkg.metadata` and all extracted files. It is not a `nuget add`-style minimal entry. + +**Performance:** a flat feed with 1 vs 201 `Newtonsoft.Json.*` files restored in 236–263 ms either way (VERIFIED). The difference doesn't matter at our scale. + +**Signatures** +- Keeping the original `.signature.p7s` on modified contents gives `error NU3008: The package integrity check failed. The package has changed since it was signed.` This happens even under the default `accept` mode (VERIFIED). The signature must be dropped. +- An unsigned package under `signatureValidationMode=require` gives `error NU3004 ... this package is unsigned` (VERIFIED). + +**Global packages folder poisoning** (VERIFIED) +- I restored upstream 13.0.3, then switched the config to only the patched feed and ran `restore --force`. There was no error, the gpf entry stayed upstream (`source: https://api.nuget.org/...`), and the assets file kept the upstream hash. A matching id+version in gpf means the feeds are never asked. +- With the patched contentHash in the lock file, `--locked-mode` gives `error NU1403: Package content hash validation failed ... different than the last restore`. Upstream was still written to gpf. After that, restoring with the patched feed also fails NU1403 until the gpf entry is deleted. + +## (b) packageSourceMapping + +**Exclusivity** (VERIFIED) +- Once any mapping exists, a package that no pattern matches fails: `error NU1100: Unable to resolve 'Humanizer.Core (>= 2.14.1)' ... PackageSourceMapping is enabled, the following source(s) were not considered: loc, nuget.org.` +- The catch-all is skipped for an id that a more specific pattern matches. With `nuget.org:*`, `loc:Newtonsoft.Json` and an empty loc, the result is `NU1101 ... No packages exist with this id in source(s): loc ... not considered: nuget.org`. There is no fallback. + +**Pattern precedence** (VERIFIED) +- The longest prefix wins: `Newtonsoft.Js*` on loc beats `Newtonsoft.*` on nuget.org. +- An exact id beats a prefix: `Newtonsoft.Json` on nuget.org beats `Newtonsoft.*` on loc. +- If the same pattern is on two sources, both are eligible. With loc empty it silently falls back to nuget.org; with both holding the package, it becomes the race described in (d). +- Pattern matching is case-insensitive. The source key is case-sensitive: a mapping for `LOC` against a source `loc` gives NU1100. + +**Missing or disabled source** (VERIFIED) +- A mapping to a key with no `` entry gives NU1100. +- A mapping to a source that exists but is disabled also gives NU1100. + +**Multiple config files** (VERIFIED) +- **A user-level mapping silently hides a repo source that has no mapping.** User `~/.nuget/NuGet/NuGet.Config` mapped `nuget.org:*`; the repo added `loc` with no mapping. Upstream was installed with no error. +- **Different keys combine across files.** User `nuget.org:*` plus repo `loc:Newtonsoft.Json` gave the patched package. +- **The same key in a nearer file replaces the farther file's patterns.** User `nuget.org:{*, Newtonsoft.Json}` plus repo `nuget.org:{Humanizer.*}` made Newtonsoft.Json fail NU1100. +- **`` inside `` drops the user-level mappings.** Anything the repo file doesn't map then fails NU1100. + +## (c) nuget.config precedence + +**Discovery** (VERIFIED, strace): NuGet probes `nuget.config`, `NuGet.config` and `NuGet.Config` in every ancestor directory up to `/`. It then reads: +- `$HOME/.nuget/NuGet/NuGet.Config` +- `$HOME/.nuget/NuGet/config/` (a directory of extra user-level configs) +- `/etc/opt/NuGet/Config/` (machine-wide) +- `/etc/opt/NuGet/NuGetDefaults.config` + +**File-name casing on Linux** (VERIFIED) +- `nuget.config`, `NuGet.config` and `NuGet.Config` all work. `NUGET.CONFIG` and `Nuget.Config` are ignored. +- If several are in one directory, only one is read, in the order `nuget.config` > `NuGet.config` > `NuGet.Config`. + +**Merging** (VERIFIED) +- Configs are merged and applied from farthest to nearest. +- `` in `packageSources` removes only sources from farther files. In the child it removes the parent's and the user's sources; in the parent it removes only the user's. +- A relative source value (`./feed`) resolves against the directory of the config file that declares it. + +**`--source` and `RestoreSources`** (VERIFIED) +- Both replace the configured sources, but the mapping still applies. +- A `--source` value equal to a configured source's value takes that source's key and mapping. +- An unconfigured path is named by its path, so it matches no mapping and fails NU1100 (`not considered: /other, nuget.org`). +- `-p:RestoreSources=` drops loc, so Newtonsoft.Json fails NU1100. + +## (d) Same id+version from two sources, no mapping (VERIFIED) +- **Local folder vs nuget.org:** the local folder won every time, in both config orders and with a cold or warm HTTP cache (4 runs each). +- **Two local folders:** the first-listed one won 6/6 times, and swapping the order flipped the result. +- **Local HTTP feed:** with 0 s delay it won in both orders. With 3 s delay it lost to nuget.org in both orders. + +Conclusion: the first source to respond wins, so the result depends on timing and is nondeterministic on real networks. + +## (e) RestoreAdditionalProjectSources in Directory.Build.props (VERIFIED) +- **With no mapping anywhere:** the source is added (it shows in "Feeds used") and joins the race. +- **With any mapping** (e.g. `nuget.org:*`): it is ignored and upstream is installed silently. +- **A mapping keyed by the absolute path works.** A relative value is named by its resolved absolute path, so a mapping keyed `feed` fails NU1100. This setup can't be committed portably. + +## (f) Giving the patched package a unique version + +**It resolves only from our feed** (VERIFIED) +- `13.0.3-socket.1` resolved from loc with no mapping. +- `13.0.3.1-socket.1` resolved from an HTTP feed with 3 s delay even though nuget.org was faster (restore took 9.95 s). There is no race when only one source has the version. + +**When our feed is missing** (VERIFIED) +- An exact `[13.0.3.1]` or `[13.0.3-socket.1]` gives `error NU1102: Unable to find package Newtonsoft.Json with version (= 13.0.3.1) - Found 0 version(s) in loc - Found 86 version(s) in nuget.org [ Nearest version: 13.0.4-beta1 ]`. +- A plain `13.0.3-socket.1` gives `warning NU1603 ... approximate best match of Newtonsoft.Json 13.0.3 was resolved` (upstream, silently). +- A plain `13.0.3.1` gives NU1603 and resolves upstream **13.0.4**. + +**Interaction with other packages' dependency ranges** (VERIFIED) +- A dependency asking `>=13.0.1` plus a direct `13.0.3-socket.1` is fine. +- A dependency asking `>=13.0.3` plus a direct `13.0.3-socket.1` gives `error NU1605: Warning As Error: Detected package downgrade: Newtonsoft.Json from 13.0.3 to 13.0.3-socket.1`. A prerelease sorts below its base version. +- A dependency asking `>=13.0.3` works with a direct `13.0.3.1` and with `13.0.3.1-socket.1`. The latter sorts above 13.0.3 and below 13.0.4, and also below any future upstream 13.0.3.1. +- `13.0.3+socket.1` (build metadata) is treated as 13.0.3 and came from nuget.org, so it is useless. + +**Transitive-only use needs a pin** (VERIFIED) +- If only a dependency references it (`>=13.0.1`), restore picks upstream 13.0.1. +- Central Package Management works: `ManagePackageVersionsCentrally` + `CentralPackageTransitivePinningEnabled` + `PackageVersion 13.0.3.1-socket.1` gave the patched package with no direct reference. + +**No cache collision:** the unique version gets its own gpf dir (`newtonsoft.json/13.0.3.1-socket.1`), so it can't collide with a cached upstream 13.0.3 (VERIFIED). + +## What this means for vendoring a patched copy with the same id+version + +1. **Same id+version can't be made robust.** + - gpf wins silently, even with `--force` (VERIFIED). + - A poisoned gpf, CI cache or shared runner gives permanent NU1403 or silent unpatched builds (VERIFIED). + - Without a mapping the winner is timing-dependent (VERIFIED). + - A mapping can be undone by a user-level, CI or ancestor-dir config: an unmapped repo source, a replaced pattern list for the same key, ``, or `--source` / `RestoreSources` (all VERIFIED). + - The current catch-all `*` workaround turns every package no pattern matches into an NU1100 risk once another config adds its own mapping. +2. **A unique version is deterministic** and needs no `packageSourceMapping`. Use `X.Y.Z.N-socket.M` (4-part base plus prerelease tag), not `X.Y.Z-socket.N` (NU1605 against any `>=X.Y.Z` dependency) and not `+meta` (same identity as upstream). + - Reference it with an exact bracket `[v]` (via `Directory.Packages.props` with transitive pinning, or a direct PackageReference), so a missing vendor dir fails hard with NU1102 instead of the NU1603 silent upgrade to upstream (VERIFIED). + - Revert is just restoring the original version string; no gpf cleanup is needed. + - Trade-offs (DOCS/inference): the lock file changes version and contentHash, and packable libraries would publish a dependency on a version consumers can't get. +3. **Feed format:** prefer a flat folder with `..nupkg`, which needs no sidecar files (VERIFIED). A hierarchical feed needs a lowercase path, a `.sha512` marker (its content is ignored) and a loose nuspec byte-identical to the inner one (VERIFIED). Declare the feed with a relative path in a lowercase `nuget.config` at the repo root (VERIFIED). Don't use `RestoreAdditionalProjectSources` (VERIFIED). +4. **Signatures:** always strip `.signature.p7s` (NU3008 otherwise). Repos using `signatureValidationMode=require` will reject any rebuilt package (NU3004), so detect that and refuse. \ No newline at end of file diff --git a/docs/design/nuget-vendoring.md b/docs/design/nuget-vendoring.md new file mode 100644 index 000000000..67c3f3299 --- /dev/null +++ b/docs/design/nuget-vendoring.md @@ -0,0 +1,914 @@ +# NuGet vendoring v2: unique-version fallback seed + +**Status:** draft, revision 3. Revision 2 added the four adversarial reviews (§16). Revision 3 records where the prototype overrides the design (§0). Vendored NuGet redesign stays future work for v5 (see `v5-plan.md`). This document lands on its own; the prototype code lives on the branch [`v5/nuget-vendoring-prototype`](https://github.com/SocketDev/socket-patch/tree/v5/nuget-vendoring-prototype) and is not merged. + +--- + +## 0. Where the prototype overrides this design + +The prototype on the branch [`v5/nuget-vendoring-prototype`](https://github.com/SocketDev/socket-patch/tree/v5/nuget-vendoring-prototype) was built after the design review. Building it and testing it against real `dotnet` changed several decisions. When anything below conflicts with this table, **this table wins**. The full list is in §17. + +| Design text | Prototype (authoritative) | Why | +|---|---|---| +| `.socket/vendor/nuget` is listed as an "anchor" fallback folder, so restore fails with NU1301 when it is missing (§6.3, G12) | **Removed.** The DBP block now holds a `SocketPatchNuGetImportCheck` target that fails restore with **SOCKETPATCH007** when `socket-patch.targets` was not imported | With the anchor, anyone could plant `//` next to the uuid dirs and NuGet would restore it under `--locked-mode` with no seed check. The security review reproduced this on SDK 8. SOCKETPATCH007 is proven by e2e for both locked and unlocked restore. | +| V′ carries a builder bit: `N = 2^30 + ((u32 >> 3) << 1) + builder` (§6.1) | `N = 2^30 + (u32(uuid[0..8]) >> 2)`, with **no builder bit** | contentHash is `base64(sha512(canonical zip of the seed file set))`. It does not depend on whether the bytes came from the service or a local rebuild, so one uuid gives one V′ and one hash. | +| Seed dirs named by uuid8 (§6.2) | Full canonical uuid: `.socket/vendor/nuget////` | Keeps `vendor_uuid_dir_rel`, the orphan sweep and the path parsers unchanged. Long Windows paths are still an open item (G8). | +| `--nuget-layout` flag on `GlobalArgs`, and a `nugetShared` ledger section (§5, §6.7) | Opt in with `SOCKET_PATCH_NUGET_LAYOUT=fallback`, or automatically once the ledger has a `nuget-fallback` entry. A purl the ledger holds as a legacy feed entry keeps the feed layout. Shared outputs are rendered from the per-uuid markers, using the ledger `fileInventory` when there is one. | Keeps the prototype contained in the core backend with a few CLI routing lines. It needs no new ledger schema. | +| Lock revert from per-edit records (§6.5, §7.4) | One `nuget_lock_file_v2` record per lock. Revert **unsplices only this entry's V′ values**, back to the recorded originals. | Seeds that share a lock can be reverted in any order, and edits made after vendoring are kept. | +| SOCKETPATCH003/004/006, per-project transitive pins, `--check`, pack range restore, VEX extractor | Not in the prototype. Transitive-only use refuses with `vendor_nuget_transitive_only` (CPM with transitive pinning is supported). | Scope (§15 was too large for one PR). | + +## 1. Status and summary + +**Status.** Proposed. This design ships as an opt-in layout, `--nuget-layout=fallback`, next to today's feed layout. Today's layout stays the default until the promotion criteria in §15 are met. Hosted mode is unchanged. packages.config projects stay on the legacy layout. A repo that has both packages.config and SDK projects using the same id@V is refused (§5). + +**Summary.** The patched package gets its own version, V′, that nothing else can produce. For a stable 3-part upstream version, V′ adds a 4th part derived from the patch uuid, for example `13.0.1` → `13.0.1.1340506222`. The package is committed already extracted, as a NuGet fallback package folder at `.socket/vendor/nuget////`. + +The CLI adds one generated block to the nearest `Directory.Build.props` (DBP) of every SDK project. The block has three parts: +- It appends `.socket/vendor/nuget/socket-patch.targets` to `CustomAfterDirectoryBuildTargets`. MSBuild imports that property after the whole `Directory.Build.targets` chain. +- It adds a restore-time `SocketPatchNuGetImportCheck` target. If the targets file was not imported, for example because `.socket/vendor/nuget` is missing, restore fails with SOCKETPATCH007. (§0: this replaces the original anchor fallback folder.) +- It adds one literal uuid comment per patch. + +The generated targets file does five things: +1. It adds each uuid8 directory to `RestoreAdditionalProjectFallbackFolders`. +2. It redirects the planned references to the floor range `[V′, )`, using evaluation-time `Update` and `Include` items. Direct references, CPM `PackageVersion`, and TFM-conditioned transitive pins are all handled. An exact `[V′]` is not used, because it spreads through ProjectReference (§16, S-B1). +3. At restore time, it checks that the files in the seed are exactly the recorded set and have the recorded hashes (SOCKETPATCH001/002). +4. At build time, it checks the files that were actually consumed: + - where they resolved from, and whether their content matches (SOCKETPATCH003); + - whether the project resolved the unpatched V (SOCKETPATCH005); + - whether a project where a redirect engaged resolved anything other than V′ (SOCKETPATCH006). +5. It writes the original version range back into packed nuspecs, checks the nuspec before and after pack, and deletes a nupkg that leaks V′ (SOCKETPATCH004). + +The CLI also edits, entry by entry, every lock whose graph contains a redirected project. It writes `.socket/vendor/nuget/.gitattributes` so that git stores the seed bytes exactly as written. + +**Why this shape:** +- The global packages folder (GPF) is never read or written for the patched package. Every other package keeps its normal GPF and caches. +- `nuget.config` is never edited in the fallback tier. +- Source mapping, user, CI and ancestor configs, `--source`, `NUGET_PACKAGES`, `--packages` and `locals --clear` do not affect the patched package. +- Revert is deterministic: remove the generated block, restore the recorded lock entries, delete the seed. No cache eviction is needed. + +**What the lock gives, and what it does not.** In the fallback tier the lock `contentHash` is compared only with the committed `.nupkg.metadata`, so it pins version identity only. Byte integrity comes from three checks: +- the restore-time set-and-hash check of the seed; +- the build-time check of consumed files; +- `vendor --check --online`, which rebuilds the expected seed from an anchor outside the repo (§7.3). + +**Enterprise `signatureValidationMode=require`.** A fallback folder is never signature-verified. These repos need the feed tier (§10). That tier is out of prototype scope and waits on depscan's CA-issued signing. + +## 2. Background: verified NuGet facts + +The research reports cited below, for example `lock-cache §c`, `sources §b` and `adv-env`, are committed in [`nuget-vendoring-research/`](nuget-vendoring-research/). They are the raw notes of the parallel research agents and the adversarial reviewers. `` paths in them refer to the throwaway sandbox the experiments ran in. + +All runs used .NET SDK 8.0.131 (NuGet 6.8.2) on Linux. Labels: +- **VERIFIED**: run in the original research; the report is cited. +- **VERIFIED-D / -C**: run by the candidate authors. +- **VERIFIED-ADV**: run by the adversarial reviewers: `adv-shapes eN`, `adv-env`, `adv-integrity xN`, `adv-lifecycle VN`. +- **DOCS**: documentation only. +- **REASONED**: inferred from code or from verified facts; not run. +- **UNVERIFIED**: a proposal waiting on a gating experiment (§14). + +### 2.1 Why the same id+version cannot be made safe +| Fact | Status | +|---|---| +| The GPF is keyed by id/version only. The first writer wins, and the entry is never refreshed, even with `--force --no-cache --force-evaluate`. | VERIFIED lock-cache §c, sources §a | +| A failed `--locked-mode` restore (NU1403) still extracts the wrong copy into the GPF. | VERIFIED lock-cache §b | +| Lock `contentHash` is compared only with `.nupkg.metadata`. A tampered dll passes locked restore. | VERIFIED lock-cache §b/§c; VERIFIED-ADV x1 (a seed) | +| Patched bytes leak to other lockless projects on the same GPF, and the reverse happens too. | VERIFIED lock-cache §c, shapes §1A | +| For the **same** id+version, the GPF beats a fallback folder. This is also true for V′: a GPF copy of V′ wins over the seed. | VERIFIED lock-cache §e; VERIFIED-ADV x2, adv-env M1 | +| A fallback folder is used in place and never copied into the GPF. The minimum content is `.nupkg.metadata`, the nuspec and `lib/`. NuGet enumerates the whole directory, so extra `build/` files are imported. | VERIFIED lock-cache §e; VERIFIED-D V-D1/2; VERIFIED-ADV x1 | +| A missing folder in `RestoreAdditionalProjectFallbackFolders` gives NU1301 in every project that imports it. | VERIFIED-ADV V2 | +| Restore ignores missing imports. It rewrites locks to upstream, and only `build` fails with MSB4019. | VERIFIED-ADV V3 | + +### 2.2 Source routing is fragile +| Fact | Status | +|---|---| +| Source mapping is exclusive and matches by id, not version. Mapping an id to a local feed breaks the id's other versions (NU1102). | VERIFIED sources §b | +| When two sources hold the same id+version, the first to respond wins, so the result is nondeterministic. | VERIFIED sources §d | +| A user-level mapping hides an unmapped repo source. A nearer file replaces a farther file's patterns for the same key. `` drops farther levels. | VERIFIED sources §b/§c | +| Config probing covers 3 spellings in every ancestor, then user, `config/*.config` and machine files. | VERIFIED sources §c | + +### 2.3 Version identity +| Fact | Status | +|---|---| +| `X.Y.Z+meta` collides with X.Y.Z. `X.Y.Z-socket.N` sorts below X.Y.Z, which gives NU1605 and a false NU1903. | VERIFIED lock-cache §g, shapes §1B | +| A 4-part `X.Y.Z.N` gets its own GPF directory and sorts above X.Y.Z. It works as a direct reference and as a CentralTransitive pin. | VERIFIED lock-cache §g, shapes §1B | +| NuGetAudit still flags V′ whenever the advisory's vulnerable range includes V′. Example: `12.0.1.N` against the `< 13.0.1` advisory. "No false NU1903" holds only when the fix is V's next release. | VERIFIED-ADV e1, V1 (this corrects revision 1) | +| An exact `[V′]` spreads through ProjectReference. It causes NU1107 against a sibling's `>= 13.0.3`, NU1107 across two patched versions of one id, and NU1608 in consumers. | VERIFIED-ADV e2 | +| With `[V′, )` the declaring project resolves V′, AppA/AppC resolve 13.0.3 as before, and AppB resolves 13.0.1.N. No NU1107 or NU1608. | VERIFIED-ADV e2 | +| When V′ is unreachable (an external consumer), `[V′, )` silently resolves the next higher version (NU1603). | VERIFIED-ADV e8 | +| Floating ranges never select a 4th-part version. | VERIFIED shapes §7 | +| A unique version leaks into packed nuspecs. | VERIFIED shapes §1B | +| `Version="13.0.1.0"` means the same as 13.0.1 to NuGet but does not match the literal condition. | VERIFIED-ADV e9 | + +### 2.4 MSBuild and injection +| Fact | Status | +|---|---| +| Only the nearest DBP/DBT is imported. Chaining with `GetPathOfFileAbove` works. | VERIFIED shapes §4 | +| With chained DBTs, an import at the end of a nested DBT runs too early when it has a once-guard, so the redirect misses items declared later. | VERIFIED-ADV e7 | +| `CustomAfterDirectoryBuildTargets`, set in DBP, is imported after the whole DBT chain (`Microsoft.Common.targets:55`), without the `ImportDirectoryBuildTargets` condition. | VERIFIED-ADV e7 (SDK 8 only) | +| Our file, imported after DBT, comes after package `build/*.targets`. An unconditional property assignment there beats env vars and package props; only a global `-p:` beats it. | VERIFIED-ADV x1 | +| `@(Item->WithMetadataValue(...))` conditions on evaluation-time ItemGroups work for PackageReference and PackageVersion Update. GlobalPackageReference and case-variant ids are covered. | VERIFIED shapes §2; VERIFIED-D e1/e7; VERIFIED-ADV (shapes) | +| A PrivateAssets=all pin becomes a Direct lock entry and does not leak into pack. Without `Publish="true"` the dll is missing from publish output. | VERIFIED shapes §5; VERIFIED-D V-D11 | +| A pin without a `$(TargetFramework)` condition adds a new Direct dependency to every TFM. | VERIFIED-ADV e3 | +| The `AfterTargets=ResolvePackageAssets` guard runs in design-time builds. | VERIFIED-ADV (adv-env M2) | +| Evaluation-time redirects work under static-graph restore (sln, and CPM with pinning on). SOCKETPATCH001 at `BeforeTargets=CollectPackageReferences` fires in normal and static-graph restore. | VERIFIED-D V-D3b; VERIFIED-ADV (shapes) | +| The guard as written runs cleanly: it parses, batches, and preserves `GetFileHash` metadata. Cost is 52 ms per project for an 8.7 MB seed. | VERIFIED-ADV (shapes, env) | + +### 2.5 Integrity, signing and git +| Fact | Status | +|---|---| +| For an unsigned nupkg, contentHash = base64(sha512(file)). For a signed nupkg it is computed with `.signature.p7s` removed. Signing never changes contentHash. | VERIFIED lock-cache §a, signing §e | +| depscan's SRI with `sha512-` removed equals the unsigned contentHash. | VERIFIED depscan §1 | +| Keeping the upstream signature on changed bytes gives NU3008. An unsigned package under `require` gives NU3004. Package and fallback folders are never verified. | VERIFIED signing §a/§b; VERIFIED-D V-D9 | +| A self-signed author cert plus `` passes `require`. `` trust has no id scope. | VERIFIED signing §b/§c; DOCS | +| `* text=auto`, `core.autocrlf=true` and `eol` rules rewrite seed text members. The author's tree stays clean, and every clone gets different bytes. | VERIFIED-ADV adv-env B1, x4/x5, V4 | +| A nested `.gitattributes` with `* -text -diff -merge -filter` gives byte-identical clones under hostile root rules. | VERIFIED-ADV adv-env c2, V4 | +| Seed files added outside the nuspec are imported and can disable a guard whose assignment is conditional. | VERIFIED-ADV x1 | + +## 3. Candidates considered + +| Key | Summary | Judge mean (/100) | +|---|---|---| +| A (feed hardened) | Same-version local feed, chain-aware mapping, pins in every lock, gitignored per-generation `RestorePackagesPath` | 59.6 | +| B (unique version via feed) | `X.Y.Z.(D+1)-socket.p.` in a local feed, with per-project redirects | 70.5 | +| C (repo-local seed) | Same id+version seeded into a committed repo-wide `RestorePackagesPath` | 70.2 | +| D (unique-version fallback seed) | 4th-part V′ in a committed fallback folder; no config edits | **76.3** (chosen by 2 of 3 judges) | + +Mean judge scores per criterion (1–10; the total is out of 100, with correctness, GPF safety and integrity weighted double). Each judge scored through a different lens: shapes and environments, integrity and supply chain, and operability. + +| Candidate | correctness | integrity | gpf safety | diff size | revert | ci friction | impl cost | server side | total | +|---|---|---|---|---|---|---|---|---|---| +| A-feed-hardened | 5.7 | 7.0 | 7.7 | 5.7 | 6.3 | 3.7 | 3.7 | 5.7 | 59.6 | +| B-unique-version | 6.3 | 7.7 | 9.7 | 5.7 | 8.0 | 5.3 | 2.7 | 8.3 | 70.5 | +| C-repo-local-seed | 7.7 | 7.3 | 9.3 | 4.3 | 8.0 | 4.0 | 6.3 | 5.7 | 70.2 | +| D-wildcard | 7.7 | 7.3 | 9.7 | 5.7 | 8.7 | 8.3 | 4.7 | 7.7 | 76.3 | + +## 4. Decision + +**D is the base.** The grafts from the other candidates: +- **From C:** the resolved-root and consumed-file hash guard, now content-aware; `.gitignore` negations checked with `git check-ignore`; eviction of GPF entries leaked by the legacy layout, identified through `.nupkg.metadata` `source`. +- **From B:** an invertible V′ derivation shared with depscan; the declared-version guard; refusal when a dependency constraint excludes V′; server-recorded upstream hashes; a sandboxed relock; the pack-roots policy; NuGetAuditSuppress. +- **From A:** version-precise pins in every affected lock; config-chain discovery, used for signature policy only; the WIRING_FILES and sweep fix; the fix for the id-only hosted lock match at `redirect/mod.rs:5431`. + +**C was rejected** because a repo-wide `RestorePackagesPath` moves every package into the repo. That throws away the CI and developer GPF caches and breaks Docker layers and scripts, which is an operational regression for every repo. + +**Structural changes after adversarial review:** +1. The floor range `[V′, )` replaces `[V′]`. SOCKETPATCH006 is added, and lock edits reach every consumer of a redirected project. +2. The injection point moves from a DBT Import line to a DBP `CustomAfterDirectoryBuildTargets` block. +3. A generated `.gitattributes` is added, and `--check` compares against committed blobs. +4. A restore-time set-equality check of the seed is added. The guard property is assigned unconditionally. Exemptions come from a CLI-managed allowlist. +5. The foreign-root check depends on content, and hash keys are `//`. +6. Shared outputs are regenerated by a CLI finalize hook (`sync_shared`) that runs after every ledger mutation. Removing the block needs no records. +7. Patch updates, revert ordering, migration ordering and v4 compatibility are specified (§7.4–§7.7). +8. Directories are named by uuid8, and the long-path check uses absolute paths. + +## 5. Tiers and layout selection + +| Tier | When | Mechanism | +|---|---|---| +| **fallback** | SDK-style PackageReference projects with no visible require policy | Committed extracted seed. Prototype scope. | +| **feed** | A require policy is visible, the depscan org policy says require, or `--nuget-tier=feed` | Signed V′ nupkg in a per-uuid flat feed. Builder 0 (service bytes) only. Post-prototype, blocked on G6 and depscan #8 (§10). | +| **legacy** | packages.config-only repos, `--nuget-layout=feed`, and existing legacy entries | Today's `nuget_feed.rs`, unchanged. | + +**Selection rules:** +- The layout is selected by `--nuget-layout ` on `GlobalArgs`, so it applies to vendor, scan, get and repair. The environment variable is `SOCKET_NUGET_LAYOUT`. The default is `feed` until promotion. +- **Inference:** if any `nuget-fallback` entry exists, or the top-level optional `state.nugetLayout == "fallback"`, new NuGet entries use fallback whatever the flag says. The first fallback vendor sets `nugetLayout`. +- **Hybrid router:** an entry whose `flavor` is `nuget-fallback`, or that carries `nuget_lock_entry_v2` records, always routes to the fallback backend. This covers entries that an older binary relabelled (§11.3). +- **Refusals:** + - `vendor_nuget_tier_mixed`: more than one tier per repo. + - `vendor_nuget_mixed_project_styles`: packages.config and SDK projects resolve the same id@V. A dual-wiring entry is open question Q5. + +## 6. Mechanism + +### 6.1 Deriving V′ +The function is pure and must be identical in `vendor/nuget_version.rs` and in depscan `lib/src/nuget/socket-version.ts`, with shared golden vectors. Its inputs are the normalized upstream V, the uuid, and the builder (service = 0, local = 1). + +| Upstream V | V′ | Status | +|---|---|---| +| Stable, ≤ 3 parts or zero 4th part | `V.N`, where `N = 2^30 + ((u32(uuid[0..8]) >> 3) << 1) + builder`, in [2^30, 2^31−1] | Form VERIFIED-D (e1/e7) | +| 4-part with D > 0 | `A.B.C.(D+1)-socket.p.` | Refused (`vendor_nuget_version_unsuffixable`) until G5 | +| Prerelease | `X.Y.Z-pre.socket.p.` | Refused until G5 | + +- **Example:** uuid `3f9a01bc-…` gives `13.0.1.1340506222` for a service build, and `…223` for a local build. +- **Collision refusals:** + - `vendor_nuget_version_collision`: two entries for the same id@V derive the same V′. + - `vendor_nuget_version_gap`: a published version of the id lies in (V, V′]. For example, an upstream 4-part `13.0.1.5` would change floor-range resolution. The server checks nuget.org and the org's configured upstreams (adv-env m7). The CLI repeats the check against the flatcontainer index when it is online. +- **One V′, one byte sequence:** service artifacts are write-once per uuid. Builder 1 is not a single byte producer (adv-integrity M4), but in the fallback tier V′ bytes exist only in the committed seed, so the only cost is lock churn. The feed tier accepts builder 0 only. +- The hot path accepts either builder for an existing uuid. The builder changes only with `--nuget-rebuild-service` (Q4). + +### 6.2 On-disk layout (fallback tier) +``` +Directory.Build.props EDITED (socket-patch block) or CREATED +src/Directory.Build.props EDITED where it is the nearest DBP of an SDK project +src/Lib/packages.lock.json EDITED (entries for id@V and Project ranges only) +src/AppA/packages.lock.json EDITED (Project range only: consumer of a redirected project) +.socket/vendor/state.json flavor "nuget-fallback", nugetLayout, nugetShared +.socket/vendor/nuget/ + socket-patch.targets GENERATED by sync_shared (LF, -text) + .gitignore GENERATED (negations) + .gitattributes GENERATED + 3f9a01bc/ fallback root (uuid8; full uuid in marker + state) + socket-patch.vendor.json + newtonsoft.json/13.0.1.1340506222/ + .nupkg.metadata {"version":2,"contentHash":"","source":null} + newtonsoft.json.nuspec + lib/netstandard2.0/Newtonsoft.Json.dll (patched) + … +``` + +**`.gitattributes`:** +``` +* -text -diff -merge -filter +/socket-patch.targets -text diff +/.gitignore -text diff +/.gitattributes -text diff +``` + +**`.gitignore`:** +``` +!/3f9a01bc/ +!/3f9a01bc/** +!/socket-patch.targets +!/.gitattributes +``` + +**Post-write checks.** After writing, the CLI runs: +- `git check-ignore` on the seed files, refusing with `vendor_artifact_gitignored`; +- `git check-attr text eol filter` on the seed files, refusing with `vendor_artifact_git_transformed`, for example when a root LFS rule survives. + +**Seed extraction.** The seed is extracted from the V′ nupkg following NuGet's own rules: +- entry names are percent-decoded, **then** validated with `is_safe_relative_subpath`, `is_plain_archive_name` and `names_are_unambiguous`, compared case-folded; +- names containing any of `$ @ % ; ' = ( )` are refused (`vendor_nuget_unsafe_member`); +- OPC parts and `.signature.p7s` are dropped; +- the nuspec is renamed to `.nuspec`; +- no `.nupkg` or `.sha512` is written; +- the existing zip-bomb caps (`MAX_ENTRIES`, per-entry and total limits) are reused. + +**Size and path limits:** +- `vendor_nuget_seed_too_large` if any file is over 50 MB or the seed is over 200 MB (configurable). +- `vendor_nuget_long_path` warns when the absolute path, under a pessimistic 60-character CI prefix or the real root, exceeds 240 characters. Above 250 characters the CLI refuses unless `--allow-long-paths` is given. + +### 6.3 Directory.Build.props block +```xml + + + + $(CustomAfterDirectoryBuildTargets);$(MSBuildThisFileDirectory).socket/vendor/nuget/socket-patch.targets + $(RestoreAdditionalProjectFallbackFolders);$(MSBuildThisFileDirectory).socket/vendor/nuget + + +``` + +**Placement.** The block goes in the nearest DBP of **every** SDK project in the same git worktree, including projects that do not use the patched package. That lets SOCKETPATCH005 and 006 cover projects added later. Submodules, template content (`PackageType=Template`) and dot-directories are skipped, with warning `vendor_nuget_import_skipped`. Nested files use a relative `../` path. If the root has no DBP, the CLI creates `` containing the block and nothing else. + +**What the block does:** +- **Import ordering.** The targets are imported after the entire DBT chain, which fixes the nested-chain misfire (VERIFIED-ADV e7, SDK 8). The `Contains` guard dedupes chained DBPs. Whether the property exists on SDK 6 and 7 is G10. +- **Restore fail-closed.** Superseded (§0). The anchor folder let anyone plant packages. The prototype's `SocketPatchNuGetImportCheck` target fails restore with SOCKETPATCH007 instead. VERIFIED in the e2e. +- **v4 compatibility.** The literal uuid comments let v4 drift-keep and VEX liveness see the uuid (§11.3). + +**Refusals:** +- `vendor_nuget_dbp_disabled`: a project sets `ImportDirectoryBuildProps=false`. +- `vendor_nuget_non_sdk_project`: a non-SDK project uses PackageReference, because the guard would never run there (adv-shapes M5). + +### 6.4 `socket-patch.targets` (generated; illustrative, deterministic, sorted) +The example repo: +- Lib has a direct 13.0.1 reference. +- AppA references Lib and Lib3 (13.0.3). +- Tool gets 12.0.1 through Bson, for netstandard2.0 only. +- Web uses CPM. + +```xml + + + + + true + true + $([MSBuild]::NormalizeDirectory('$(MSBuildThisFileDirectory)')) + $([MSBuild]::NormalizeDirectory('$(MSBuildThisFileDirectory)', '..', '..', '..')) + $([MSBuild]::MakeRelative('$(SocketPatchRepoRoot)', '$(MSBuildProjectFullPath)').Replace('\', '/')) + <_SpV_3f9a01bc>13.0.1.1340506222 + <_SpV_7c21d0e4>12.0.1.1994350720 + $(RestoreAdditionalProjectFallbackFolders);$(SocketPatchNuGetDir)3f9a01bc;$(SocketPatchNuGetDir)7c21d0e4 + <_SpPatched>;newtonsoft.json/$(_SpV_3f9a01bc);newtonsoft.json/$(_SpV_7c21d0e4); + <_SpUpstream>;newtonsoft.json/13.0.1;newtonsoft.json/12.0.1; + <_SpAllowUnpatched>;src/Legacy/Legacy.csproj; + <_SpHashes>;newtonsoft.json/13.0.1.1340506222/lib/netstandard2.0/newtonsoft.json.dll=4F33…BD;…; + + + + + + + + + + + + + + + + + + + + + + + + + + + + <_SpOnDisk Include="$(SocketPatchNuGetDir)3f9a01bc/**;$(SocketPatchNuGetDir)7c21d0e4/**" Exclude="$(SocketPatchNuGetDir)*/socket-patch.vendor.json" /> + <_SpExtra Include="@(_SpOnDisk)" Exclude="@(SocketPatchSeedFile)" /> + <_SpMissing Include="@(SocketPatchSeedFile)" Condition="!Exists('%(FullPath)')" /> + + + + + <_SpSeedChecked>true + + + + + + <_SpCand Include="@(RuntimeCopyLocalItems);@(ResolvedCompileFileDefinitions);@(RuntimeTargetsCopyLocalItems);@(NativeCopyLocalItems);@(ResourceCopyLocalItems);@(Analyzer)" + SpKey=";$([System.String]::Copy('%(NuGetPackageId)/%(NuGetPackageVersion)').ToLowerInvariant());" /> + <_SpPatched Include="@(_SpCand)" Condition="$(_SpPatched.Contains('%(SpKey)'))" /> + <_SpUnpatched Include="@(_SpCand)" Condition="$(_SpUpstream.Contains('%(SpKey)'))" /> + <_SpDrift Include="@(_SpCand)" Condition="'@(SocketPatchRedirect)' != '' and '%(NuGetPackageId)' != '' and $([System.String]::Copy('@(SocketPatchRedirect)').ToLowerInvariant().Contains('$([System.String]::Copy('%(NuGetPackageId)').ToLowerInvariant())/')) and !$(_SpPatched.Contains('%(SpKey)'))" /> + + + + + + <_SpBad Include="@(_SpHashed)" Condition="!$(_SpHashes.Contains(';%(SpKeyPath)=%(FileHash);'))" /> + + + + + +``` + +**Notes on the file:** +- **Content-aware foreign root.** `SpKeyPath` is `//`, computed from the path relative to the package root it resolved in. The mechanics are G15. A byte-identical copy in the GPF passes. A different copy fails SOCKETPATCH003. The error message no longer tells the user to delete the folder. +- **Package presence.** Presence is decided from `@(_SpCand)`, which now includes `@(Analyzer)`. For packages with no lib, ref or analyzer assets, the guard also reads the `libraries` keys of `project.assets.json` (G1). +- **Guard escape hatch.** `SocketPatchNuGetGuard` is assigned unconditionally, so the only way to turn it off is a global `-p:SocketPatchNuGetGuard=false`. `vendor --check` flags `*.rsp` files that mention `SocketPatch*`. +- **SOCKETPATCH005 exemptions.** Projects are exempted only through `_SpAllowUnpatched`, which is rendered from `state.nuget.allowUnpatched` (`socket-patch vendor --nuget-allow-unpatched `). There is no free-form property any more. +- **Versions behind properties.** Versions sit in `_SpV_*` properties, so regex updaters see no literals. If a bot edits them anyway, SOCKETPATCH006 fires, and `--check` compares the render sha. +- **Split CPM.** When one central PackageVersion resolves differently across projects, the CLI emits the per-project form `('$(SocketPatchProject)'=='a' or …)` (VERIFIED-D e7). A CPM transitive-only project gets a Version-less `Include` pin plus `Publish="true"`. A `VersionOverride` equal to V gets `VersionOverride="[V′, )"`. VersionOverrides to other versions are left alone. +- **TFM conditions.** Pin TFM conditions use the alias taken from assets `project.frameworks`. A multi-TFM pin with no assets file, or with a custom alias, is refused (`vendor_nuget_tfm_alias_unknown`). + +### 6.5 Lock edits +**Which lock.** For each project, in order: +1. a static `NuGetLockFilePath` (a dynamic value is refused with `vendor_nuget_lockpath_dynamic`); +2. `packages..lock.json`; +3. `packages.lock.json`. + +**Edits.** Splices are byte-preserving and TFM/RID-scoped. The exact `[V′, )` text is G11; the goldens are re-captured from `--force-evaluate`. + +| Project role | Before | After | +|---|---|---| +| Direct / CPM direct | `Direct`, `requested "[13.0.1, )"`, `resolved 13.0.1` | `Direct`, `requested "[V′, )"`, `resolved V′`, `contentHash ` | +| CPM uniform, pinning on (CentralTransitive) | `CentralTransitive`, `requested "[13.0.1, )"` | stays `CentralTransitive`; `requested "[V′, )"` plus new resolved and hash (VERIFIED-ADV e5 corrects revision 1) | +| Transitive pin | `Transitive` | `Direct` with `requested` inserted. Written in NuGet's canonical order: Direct entries first (VERIFIED-ADV e3). | +| Auto-follow consumer that resolved V | `Transitive 13.0.1` | `Transitive V′` plus hash | +| Consumer that resolved R > V (AppA/AppC) | unchanged | unchanged. Its resolution is R because R > V′ is guaranteed by the gap check. | +| `Project` entry listing a redirected project | `"[13.0.1, )"` | `"[V′, )"`, in **every** lock whose graph contains that project | + +**Records.** +- One record per edit: `nuget_lock_entry_v2` (key `#[/]#`) or `nuget_lock_project_range`. +- Records are stored EOL-neutral, together with the observed EOL. +- Live-text comparisons normalize CRLF to LF, and splices use the live file's EOL. +- Relock comparison is semantic (parsed JSON), not textual. + +### 6.6 Pack +- **Range restore** (`AfterTargets=_GetAbsoluteOutputPathsForPack`, VERIFIED-D V-D6 and VERIFIED-ADV e5): `"[V′, )"` is replaced with the **recorded original literal range**, for example `[13.0.1]` stays exact. +- **Pre-check**, `BeforeTargets=GenerateNuspec`: fail SOCKETPATCH004 if the rewritten assets still contain any V′. +- **Post-check**, `AfterTargets=GenerateNuspec`: read exactly `$(NuspecOutputAbsolutePath)$(PackageId).$(PackageVersion).nuspec`. This avoids MSB4184 when several nuspecs are present (VERIFIED-ADV e5). On failure, `` runs before the error, because the nupkg was already written (VERIFIED-ADV e5). +- Both checks are G2. If G2 fails on any supported SDK, packable projects with a direct patched reference are refused (`vendor_nuget_packable_direct`). Opt-in: `--nuget-pack-policy=roots`. + +### 6.7 State +```json +{"ecosystem":"nuget","flavor":"nuget-fallback","basePurl":"pkg:nuget/Newtonsoft.Json@13.0.1","uuid":"3f9a01bc-…", + "artifact":{"path":".socket/vendor/nuget/3f9a01bc/newtonsoft.json/13.0.1.1340506222","sha256":"", + "fileInventory":{"lib/netstandard2.0/Newtonsoft.Json.dll":"4f33…","…":"…"}}, + "nuget":{"tier":"fallback","wired":true,"upstreamVersion":"13.0.1","patchedVersion":"13.0.1.1340506222", + "builder":"local","contentHash":"FRVi…","upstreamSha512":"…","upstreamContentHash":"…","packPolicy":"restore-ranges", + "literals":["13.0.1","13.0.1.0"],"allowUnpatched":[], + "projects":[{"path":"src/Lib/Lib.csproj","role":"direct","declared":"13.0.1","tfms":["net8.0"]}, + {"path":"src/AppA/AppA.csproj","role":"range-consumer","tfms":["net8.0"]}]}, + "wiring":[ + {"file":"src/Lib/packages.lock.json","kind":"nuget_lock_entry_v2","key":"…#net8.0#Newtonsoft.Json","original":"…","new":"…"}, + {"file":"Directory.Build.props","kind":"nuget_uuid_anchor","action":"Referenced"}]} +``` +Top-level additions to `VendorState`, all additive and optional: +- `nugetLayout`; +- `nugetShared`: `{renderSha256, props:[{file, created, original}]}`. + +**Rules:** +- The DBP block is owned by the state, not by any entry. `nuget_uuid_anchor` is a reference only: v5 never reverts from it, and it exists so v4 sees the file (§11.3). +- `nuget.projects` is the authoritative closure (§7.2). +- `wired=false` marks Preserved or Kept entries. `sync_shared` never renders them. +- `fileInventory` intactness replaces the existence-only `ledger_covers` check for directory artifacts. + +## 7. Algorithms + +### 7.1 Apply (`vendor_nuget_fallback`) +1. **Prelude** (`fallback_prelude`, shared with `service_preflight`). It refuses on: + - unsafe coordinates; + - `version_unsuffixable`, `version_gap`, `version_collision`; + - signature policy: config chain for every project directory, plus `DOTNET_NUGET_SIGNATURE_VERIFICATION`; + - `mixed_project_styles`, `dbp_disabled`, `non_sdk_project`; + - `external_consumer`: a project outside the root references a redirected project, or an `.sln`/`.slnf` above the root lists one; + - `exact_dependency`: a dependency constraint excludes V′; + - `version_literal_unknown`: property-expanded versions on an SDK without `-getItem` (< 8), which are refused; + - `sdk_unavailable`: the SDK that `global.json` pins is not installed; + - `implicit_reference`: `IsImplicitlyDefined`, for example FSharp.Core; + - `tool_only`: tool packages cannot be patched this way, and the crawler must not claim that they are covered; + - `tier_mixed`, `layout_mixed`. + + Entries that share this entry's ledger key or basePurl are excluded from the duplicate and collision checks. +2. **Closure plan** (`plan_closure`): + - Enumerate `*.*proj` plus the projects listed in the sln. Skip dot-directories, `bin`, `obj`, `node_modules` and configured package folders. + - Read assets, or else the lock, or else csproj and CPM text. + - Count a `resolved` that decodes (`parse_socket_nuget_version`) to (id, V) as V. + - Roles: `direct`, `cpm-uniform`, `cpm-split`, `auto-follow`, `pin`, and `range-consumer`. A range-consumer is any project whose graph reaches a redirected project; ProjectReferences are walked in both directions. + - Projects that are already planned and have no assets are not refused. `vendor_nuget_unrestored` applies only to **new** projects with neither a lock nor assets. +3. **Materialise the V′ nupkg:** + - from the service `nupkg-socket-version` artifact (builder 0), or + - by running `reversion_nupkg` on the SRI-verified same-version service nupkg or on `local_rebuild` output (builder 1). + + The upstream is verified by its repository signature, or by sha512 against depscan `upstreamSha512`. Every patched member is checked against its afterHash. +4. **Stage everything in memory:** the seed tree, all lock splices, and the new DBP blocks. +5. **Write, with snapshots:** + - Write the seed atomically (temp directory, then rename). Reuse it if it already matches `fileInventory`. + - Snapshot the bytes of every file about to be touched, then write the locks and DBPs, each by temp file and rename. +6. Optional `--nuget-relock`: a sandboxed `--force-evaluate` run with a throwaway `NUGET_PACKAGES`, then a semantic diff against the planned key set. +7. **Unwind** on any failure in steps 5–6: restore every snapshot, and delete the seed only if this run created it. +8. Return `VendorOutcome::Done`. The CLI then persists the entry, sweeps stale artifacts, and calls **`sync_shared`**. + +### 7.2 `sync_shared(cwd, &VendorState)`, the CLI finalize hook +It is called after every ledger mutation: `run_vendor` (after persist and sweep), revert, rollback, remove, gc, repair, and scan/get in vendored mode. It runs: +- in memory during `--dry-run`; +- idempotently; on failure it reports `vendor_nuget_shared_sync_failed` and exits non-zero. + +What it does: +1. It selects entries that are `nuget-fallback`, `wired`, and whose seed directory is present. For an entry that is wired but has a missing seed, it warns and does not render it. +2. If the selection is non-empty: + - it renders `socket-patch.targets`, `.gitignore` and `.gitattributes`, and writes the DBP blocks into every nearest DBP; + - it refuses without `--force` when the on-disk targets differ from `nugetShared.renderSha256` (`vendor_nuget_targets_edited`). +3. If the selection is empty: + - it removes the block from every discovered DBP, plus every DBP listed in `nugetShared.props`, without needing any per-entry records; + - it deletes a DBP whose text equals the created text; otherwise it restores `original` when the rest of the file is unchanged; + - it deletes the targets file only after every block is gone. If any excision drifts, it writes a stub `` targets file instead; + - it deletes `.gitignore` and `.gitattributes` last. + +### 7.3 Hot path, `--check`, `--check --online` +**In sync**, which returns `AlreadyPatched`, requires all of: +- the seed file set equals `fileInventory`, with no extra, symlinked or non-regular files, and the hashes match; +- `.nupkg.metadata` equals the recorded contentHash; +- the shared outputs are byte-equal to the render (they are `-text`, so this is safe); +- every lock record's live text equals `new`, compared EOL-neutral; +- a re-plan from csproj, CPM and locks, with V′ counted as V, matches `nuget.projects`. + +A changed closure is re-planned and re-keyed, and warns `vendor_nuget_closure_changed`. + +**`--check`** does everything above, plus: +- it compares seed hashes against the **committed** blobs (`git cat-file HEAD:`) and checks `git ls-files --eol`, reporting `vendor_nuget_eol_normalized`; +- it evaluates every discovered project with `dotnet msbuild -getProperty:SocketPatchNuGetTargetsImported` (batched). Any project that resolves the id at V without importing the targets fails (`vendor_nuget_project_uncovered`); +- it probes the signature policy again; +- it flags repo trustedSigners that are not in state, and `.rsp` overrides; +- it reports untracked seeds, stale keys, unused entries and orphaned pins. + +It works before any restore, because it relies on the persisted closure. + +**`--check --online`** also rebuilds the expected tree from an anchor outside the repo and diffs it byte for byte. The anchor is either the service artifact (SRI) or upstream plus afterHashes. This is the recommended required CI step. + +### 7.4 Revert +Steps, in order: +1. **Lock records.** + - Live text equal to `new`: splice `original` back. + - Live text equal to `original`: the record is done. + - Otherwise it is drift. Drift is repaired **only** from depscan `upstreamContentHash`, or from a freshly downloaded nuget.org nupkg whose repository signature was checked. It is never repaired from the GPF (adv-integrity M5). +2. **keep** = any drifted record whose live text still contains V′. If keep is set, the entry becomes `wired=true`, `kept_artifact`, and the process stops here. +3. The CLI drops the entry, or marks it `wired=false` for Preserved, then saves state and runs `sync_shared`. That regenerates the targets or removes the blocks, deleting the targets only after the blocks are gone. +4. It deletes the seed and prunes empty levels. It first runs `refuse_symlinked`/`first_symlink` on the seed and all its ancestors. +5. No GPF eviction is needed (VERIFIED-D V-D12). + +### 7.5 Patch update (a new uuid for the same purl) +- The planner treats the old V′a as V (step 2). Lock edits V′a → V′b record `original: None`, so `carry_forward_wiring` fills in A's true upstream original. The key has no uuid in it. +- `carry_forward_wiring` merges import state across uuids implicitly, because that state now lives in the state-level `nugetShared`. +- The old entry is excluded from the prelude checks. The CLI replaces the entry, sweeps A's seed, and then runs `sync_shared`, which renders only B. +- Docker leg: vendor A, update to B, locked restore, revert, then `git diff --exit-code`. + +### 7.6 Migration from the legacy layout (`--migrate-nuget`) +1. Run `fallback_prelude` and `plan_closure` first. A refusal leaves legacy intact. +2. Materialise V′ by running `reversion_nupkg` on the **committed legacy `.nupkg`** (builder 1), and verify the afterHashes. Local rebuild is not used, because the GPF holds patched bytes. +3. As one unwindable unit: run the legacy revert (config, catch-all when the live text equals the legacy `new`, root lock), then apply fallback. +4. Opt-in `--nuget-evict-legacy-cache`: delete `//` GPF entries whose `.nupkg.metadata` `source` is a `.socket/vendor/nuget/` path, and print the command for CI caches. + +### 7.7 Pristine source on clean machines +When the GPF never holds upstream V, a rung fetches `https://api.nuget.org/v3-flatcontainer///..nupkg`. It verifies the result fail-closed against the upstream contentHash recorded in the ledger (the `original` of any `nuget_lock_entry_v2`), using the signed-content hash. Repair, patch update and `--offline` use this rung. + +For fallback entries, `vend_installed!` no longer `debug_assert`s `Installed`, and the hot path never reads `installed_dir`. + +### 7.8 Crawler, VEX, in-use +- **Crawler:** `nuget_crawler.rs` skips `.socket/vendor/nuget/**` packageFolders and maps V′ to (upstream purl, uuid). +- **VEX:** the fallback extractor parses `socket-patch.targets` (literal uuid comments and `_SpV_*`) and the discovered locks (V′ decoded). The nuget probe files gain the targets file, the DBPs and the locks. +- **In use:** an entry is in use when a lock or assets file resolves V′ for a reason other than our own pin. A pin with no remaining parent edge gives `vendor_nuget_pin_orphaned` and is dropped on the next `vendor`. + +## 8. Supported shapes (fallback tier) + +| Shape | Status | Evidence / notes | +|---|---|---| +| SDK sln, per-project locks, `--locked-mode`, warm or cold GPF | Supported | VERIFIED-D V-D3/V-D10, VERIFIED-ADV (env). Goldens for the `[V′, )` form: G11. | +| Patched library consumed next to a higher sibling (AppA/AppC) | Supported | `[V′, )` plus Project-range edits in every consumer lock (VERIFIED-ADV e2) | +| Multiple patched versions of one id reached by one project | Supported | Resolves the higher V′ (VERIFIED-ADV e2) | +| CPM, pinning off/on, VersionOverride | Supported | VERIFIED-D V-D7; static graph with pinning on VERIFIED-ADV | +| Transitive-only, multi-TFM | Supported | TFM-conditioned pin (VERIFIED-ADV e3 shows the need; the fix is G11) | +| Nested and chained DBT/DBP | Supported | `CustomAfterDirectoryBuildTargets` (VERIFIED-ADV e7 on SDK 8; SDK 6/7 is G10) | +| No lockfile | Supported, warning `vendor_nuget_no_lockfile` | 001/002/003/005/006 | +| Spelling variants `13.0.1.0`, `[13.0.1]` | Supported | One condition per recorded spelling. A new spelling in a new project gives loud 005. | +| `*.proj` NoTargets/Traversal, `.fsproj`, `.sqlproj` | Supported if SDK-style | Enumerated through `*.*proj` plus the sln | +| Any source mapping, ``, `--source`, `NUGET_PACKAGES`, `--packages`, `locals --clear` | Supported | VERIFIED-D V-D2/4/5b/9 | +| Byte-identical V′ in a shared GPF | Supported | Content-aware 003 (G15) | +| Windows checkout with `autocrlf`, `* text=auto`, LFS rules | Supported | `.gitattributes` (VERIFIED-ADV c2/V4). A surviving LFS rule is refused. | +| Packable library with a direct patched dependency | Supported with warning, or refused until G2 | §6.6 | +| Project outside the `.socket` root referencing a redirected project | Refused: `vendor_nuget_external_consumer` | VERIFIED-ADV e8 | +| Non-SDK csproj with PackageReference | Refused: `vendor_nuget_non_sdk_project` | REASONED (no mono) | +| Mixed packages.config and SDK on the same id@V | Refused: `vendor_nuget_mixed_project_styles` | Q5 | +| packages.config only | Legacy layout | | +| Property-expanded version on SDK < 8, or `global.json` SDK missing | Refused | REASONED | +| Implicit SDK references (FSharp.Core) | Refused | REASONED | +| 4-part non-zero or prerelease upstream | Refused until G5 | | +| `require` policy visible | Refused (fallback tier) | Feed tier is post-prototype | +| Submodules, template content | Skipped with warning | | + +## 9. Failure modes + +| Situation | Result | Loud? | +|---|---|---| +| Seed file missing or added (for example planted `build/*.props`) | SOCKETPATCH001 at restore, before `nuget.g.*` is regenerated on a clean checkout | Yes (G13) | +| Seed file edited | SOCKETPATCH002 at restore and build | Yes | +| All of `.socket/vendor/nuget` missing | SOCKETPATCH007 at restore (prototype, VERIFIED in the e2e), then MSB4019 at build | Yes | +| Only `socket-patch.targets` missing | Restore **succeeds and rewrites locks to upstream** (V3). Build gives MSB4019, and `--check` fails. | Build only; the CI gate is `--check` | +| V′ elsewhere, different bytes | SOCKETPATCH003. NU1403 applies only if `.nupkg.metadata` differs (VERIFIED-ADV x2). | Yes | +| V′ elsewhere, same bytes | Accepted | — | +| New or renamed project that imports the targets | SOCKETPATCH005, and NU1004 when locked | Yes | +| New nested DBP that does not chain to the root | **Silent at build** (VERIFIED-ADV x3, DBT analogue). Caught by `--check` import evaluation. | CI gate | +| Redirect engaged but drifted (bot edit, parent bump, NU1603) | SOCKETPATCH006 | Yes | +| Bot bumps the declared version | Redirect disengages; the lock diff shows V′ → new; `--check` flags the unused entry | In PR | +| Env var or package props setting `SocketPatchNuGetGuard=false` | Ignored (unconditional assignment) | — | +| Global `-p:SocketPatchNuGetGuard=false` | Guards off (escape hatch) | User choice | +| Planted `build/*.targets` on a dev machine that already restored | Arbitrary code before the guard; residual risk | Accepted; mitigated by `--check --online` in CI | +| Design-time build | Redirects apply; the guard is skipped | — (G16) | +| Docker `COPY *.csproj` without DBP/.socket | Locked: NU1004. Lockless: SOCKETPATCH005 at full build. `vendor` warns `vendor_nuget_dockerfile_copy`. | Yes | +| Sparse checkout without `.socket/` | NU1301/MSB4019. The block comment names `git sparse-checkout add .socket/vendor/nuget`. | Yes | +| CI-only `require` policy | Silently bypassed; closed by depscan org policy #6 | No (accepted gap) | +| NuGetAudit when V′ is still in an advisory range | NuGetAuditSuppress (NuGet ≥ 6.11). Older SDKs emit info `vendor_nuget_audit_still_flags`. | Parity with pre-patch | +| SBOM and dependency-graph tools reading locks | They see V′; depscan mapping (#4); document Dependabot alert behaviour | Docs | +| Missing seed + VS nomination restore + lockless + attacker feed serving V′ | Possible code execution | Accepted residual. Server gap check covers org upstreams. | + +## 10. Signing and enterprise policy (feed tier; post-prototype) + +- **Fallback tier:** it never claims signature enforcement. It refuses under a visible `require`, never writes `accept`, and never sets `DOTNET_NUGET_SIGNATURE_VERIFICATION`. +- **Feed tier:** + - Builder 0 only, so shared GPFs stay safe (adv-integrity M4). + - It adds exactly one `` for the per-uuid feed. Id-exact mapping is added only where a repo mapping already exists. It refuses with `vendor_nuget_feed_mapping_conflict` when the only mapping is outside the repo and the repo uses several versions of the id. + - **Trust:** it never writes a pfx-holder `` to repo config by default. The default path is depscan's CA-issued certificate plus RFC3161 timestamp (#8), trusted once at org or machine level. The opt-in `--nuget-sign-ephemeral` generates a per-uuid key, signs, destroys the key, then writes the fingerprint, so trust covers exactly the bytes already signed. `--check` flags unrecorded trustedSigners. + - **Guard:** the same content-aware 003/002 as the fallback tier. + - Gated by G6. + +## 11. Compatibility + +### 11.1 Hosted +- Unchanged until WS1 (`hosted_revert_unsupported`). +- Later, hosted can serve V′ from v3 and reuse the targets file. +- Independent fix: version-precise hosted lock matching at `redirect/mod.rs:5431`. + +### 11.2 Repair and sweep +- WIRING_FILES gains the three `nuget.config` spellings, the discovered DBP and DBT files, and `packages*.lock.json`. +- Reserved names: `socket-patch.targets`, `.gitignore`, `.gitattributes`. +- A wiring file that names a uuid directory makes that directory live. + +### 11.3 Older binaries (v4.0.0 is released) +- **The risk (REASONED from code).** Without mitigation, v4's revert deletes the seed while the locks and targets still reference it. v4's vendor applies a legacy layout on top. +- **Mitigations:** + - The literal `.socket/vendor/nuget/` comments in the root DBP block, plus a `nuget_uuid_anchor` record, make v4's drift-keep keep the seed and the entry (fail-safe). + - The v5 hybrid router (§5) and a heal step: detect a legacy `socket-patch-` source next to a fallback seed, remove it, and restore the lock. + - `revert_nuget_opts` fails closed on an unknown flavor (v5). + - The minimum version is documented. +- **Test:** G14 runs the real v4 binary against a fallback repo. + +## 12. Server/CLI split (depscan) + +| # | depscan addition | Needed for | +|---|---|---| +| 1 | `lib/src/nuget/socket-version.ts` derive/parse, with golden vectors shared with Rust | GA | +| 2 | `nuget-socket-version` repacker (V′ nuspec, STORE-only, deterministic, write-once). Refuses versions in (V, V′] on nuget.org **and the org's configured upstreams**. | GA | +| 3 | `artifacts[].kind="nupkg-socket-version"`, `nugetPatchedVersion`, `upstreamContentHash`, `upstreamSha512` | **GA** (provenance, drift repair, `--check --online`) | +| 4 | SBOM mapping V′ → upstream purl plus uuid; recognise the fallback folders. Bump task versions, run `generate-task-metadata`, use `RunTask` helpers. | **GA blocker** | +| 5 | Fixed-advisory GHSA list per patch (NuGetAuditSuppress) | Prototype nice-to-have; the CLI can fall back to local advisory data | +| 6 | Org `nugetSignaturePolicy: require` | GA for enterprises | +| 7 | `nuget-seed` artifact (extracted layout plus inventory), tested with real dotnet | Post-GA | +| 8 | CA-issued Socket author signing plus timestamp | Feed tier | +| 9 | Hosted V′ in flat and registration indexes | Later | + +Identity and bytes (about 30% of the logic) belong to the server. All repo and machine wiring is 100% CLI. + +## 13. Test plan + +**Hermetic unit tests** (`cargo test -p socket-patch-core --lib nuget_`): +- `nuget_version`: vectors, round-trip, gap refusal. +- `nuget_lock`: goldens from real `--force-evaluate` output for `[V′, )`, covering: + - each role, including CentralTransitive with pinning on, range-consumers and canonical Direct ordering; + - CRLF and no trailing newline; + - RID goldens for a `runtimes/` package (SqlClient); + - EOL-neutral revert and drift. +- `nuget_targets`: render golden with uppercase-hex pinning; XML and MSBuild escaping; DBP block inject and excise on CRLF, BOM, ``, chained DBPs; stub rendering. +- `nuget_seed`: decode-then-validate traversal cases (`%2E%2E%2F`, `%5C`), metacharacter refusal, case-fold collisions, OPC drops, `reversion_nupkg` determinism. +- `nuget_projects`: closure on fixtures (e2 AppA/AppB/AppC, multi-TFM e3, external consumer e8, spellings e9, submodule, template). +- `nuget_policy`: chain merge. +- `sync_shared`: update, revert order, Preserved, dry run. + +**Real-dotnet e2e** (`e2e_nuget_dotnet_build.rs`; SDK 6–10 ubuntu, 8 macOS, **windows-latest**). Legs: +- `sln_locked`, `cpm_locked`; +- warm GPF; +- `RestorePackagesPath`/`NUGET_PACKAGES`; +- `locals --clear`; +- tamper (002) and planted file (001); +- env-var disable ignored; +- pack (range restore, 004 pre and post, output deleted); +- revert plus `git diff --exit-code`. + +**Docker** (`docker_e2e_vendor_nuget.rs`): +- static graph; no lockfile; +- 005 rename; 006 bot edit; +- anchor NU1301; `--packages` then `--no-restore`; +- hostile user mapping; +- require refusal; +- migration plus eviction; +- idempotence; +- G3 two roots; +- e2/e3/e7/e8 fixtures; +- same-bytes V′ in the GPF (003 passes) and different bytes (003 fails); +- patch update A→B then revert; +- two-id revert in both orders; +- crash between write and save (resume via `sync_shared`); +- v4 binary (G14); +- parallel `-m`; +- mono image for the packages.config refusal. + +**Real-clone legs** (prototype acceptance): commit, `git clone`, build under `* text=auto`, `autocrlf=true`, a `*.dll filter=lfs` root rule, and sparse checkout. + +**Regression:** every existing `nuget_feed` test passes unchanged with the default layout. + +## 14. Gating experiments and open questions + +| ID | Question | Blocks | +|---|---|---| +| G1 | Content-aware guard with multiple roots; `@(Analyzer)`; analyzer-only and build-only packages via the assets `libraries` | Prototype | +| G2 | Pack pre/post checks and `@(NuGetPackOutput)` deletion on SDK 6–10 | Packable support | +| G3 | Multiple uuid roots plus the anchor root together | Prototype | +| G4 | SOCKETPATCH005/006 on renamed projects (001 is VERIFIED-ADV) | Prototype | +| G5 | Prerelease-tagged V′ forms | 4-part/prerelease | +| G6 | Feed tier with V′ under `require`; shared GPF across repos | Feed tier | +| G7 | Static graph plus `dotnet test` across CPM variants | Promotion | +| G8 | macOS/Windows path case; nuget.exe packages.config | Promotion | +| G9 | VS/Rider design-time restore honours redirects | Docs | +| G10 | `CustomAfterDirectoryBuildTargets` on SDK 6/7; chained-DBP dedupe | **Prototype** | +| G11 | `[V′, )` lock goldens, TFM-conditioned pins, no NU1603 when the seed is present | **Prototype** | +| G12 | The anchor fallback folder gives NU1301 at restore when `.socket/vendor/nuget` is missing, and nothing when present | **Prototype** | +| G13 | Restore-time set check: glob includes dotfiles, runs under static graph, runs before a planted `build/` file on a clean checkout | **Prototype** | +| G14 | v4.0.0 revert and vendor against a fallback repo keep the seed (fail-safe) | **Prototype** | +| G15 | `SpKeyPath` computation for GPF-resolved files | Prototype | +| G16 | Guard skipped when `DesignTimeBuild=true` in VS, Rider and C# Dev Kit | Promotion | +| Q1 | Should SOCKETPATCH005/006 be warnings for one release? | Product | +| Q2 | Is org require policy (#6) enough, or should the feed tier be default for paid orgs? | Product | +| Q3 | Renovate and Dependabot behaviour on the targets file; emit `ignorePaths: [".socket/**"]`? | Docs | +| Q4 | Re-vendor builder-1 seeds to builder 0 once the service artifact ships? | Post-GA | +| Q5 | Dual wiring (legacy for packages.config plus fallback for SDK) for mixed repos? | Post-prototype | + +## 15. Prototype scope + +**Goal.** A working prototype for SDK sln with locks in locked mode, and for CPM, with no change to default behaviour. + +**Gating.** +- `--nuget-layout` on `GlobalArgs`, with the inference rules from §5. +- `layout_for(entry, state, run)` is used by `dispatch_vendor_one` (callers at `vendor.rs:2636` and `repair_vendor.rs:1632`, and via `vendor_records`). +- `service_preflight` changes signature to take `&VendorState`. +- The revert arm uses the hybrid router. + +**In scope:** +- Fallback tier; 3-part V′. +- All roles, including range-consumers. +- The DBP block, the anchor, and `sync_shared`. +- `.gitattributes` with probes. +- Guards 001–006 and the pack checks. +- NuGetAuditSuppress. +- Hot path, `--check` (including `--online`), revert. +- Patch update. +- The pristine fetch rung. +- VEX extractor, crawler skip, in-use probe. +- v4 anchor and hybrid router. +- Flavor gate, WIRING_FILES, sweep. +- Name validation and escaping. + +**Out of scope:** the feed tier and signing; 4-part and prerelease upstreams; packages.config (legacy, and mixed repos are refused); `--nuget-relock`; migration (refused with `vendor_nuget_layout_mixed`); hosted, apart from the `:5431` fix. + +**Modules** (under `crates/socket-patch-core/src/vendor/` unless noted): + +| Module | Items | +|---|---| +| `nuget_version.rs` | `NugetBuilder`, `SocketNugetVersion`, `socket_nuget_version`, `parse_socket_nuget_version`, `check_version_gap` | +| `nuget_projects.rs` | `NugetProject`, `discover_projects` (`*.*proj`, sln, worktree-bounded), `plan_closure` (V′-aware, bidirectional), `ProjectRole::{Direct, CpmUniform, CpmSplit, AutoFollow, Pin{tfm_alias, parent}, RangeConsumer}`, `excluding_constraints`, `external_consumers` | +| `nuget_lock.rs` | `LockEntryEdit`, `plan_lock_edits`, `splice` (EOL-aware), `revert_lock_entry_record`, `semantic_eq` | +| `nuget_seed.rs` | `reversion_nupkg`, `extract_seed` (decode then validate), `seed_inventory`, `walk_seed_strict`, `write_seed_atomic` | +| `nuget_targets.rs` | `render_targets`, `render_gitignore`, `render_gitattributes`, `render_dbp_block`, `inject_block`, `excise_block`, `msbuild_escape` | +| `nuget_shared.rs` | `sync_shared(cwd, &VendorState, dry_run)` | +| `nuget_policy.rs` | `effective_signature_mode` | +| `nuget_fallback.rs` | `vendor_nuget_fallback`, `revert_nuget_fallback_opts`, `service_preflight`, `fallback_prelude`, `fallback_in_sync`, `layout_for`, `fetch_pristine` | +| `state.rs` | `NugetMeta` (with `wired`, `literals`, `allowUnpatched`, upstream hashes), `nugetLayout`, `nugetShared`, `fileInventory` intactness in `ledger_covers` | +| `ledger_snapshots.rs`, `path.rs`, `verify.rs` | new kinds; uuid8 nested leaf parser; reserved names; set-equality verify | +| `nuget_feed.rs` | flavor gate; `CONFIG_NAMES` | +| `crawlers/nuget_crawler.rs`, `vex/discover/nuget.rs`, `vex/discover/mod.rs:1750` | skip seeds; fallback extractor; probe files | +| CLI `vendor.rs`, `rollback.rs`, `remove`, `gc`, `repair_vendor.rs`, scan/get | `sync_shared` calls; `vend_installed!` fix; in-use probe; `GlobalArgs` flag | + +The CLI and VEX work adds roughly 30–40% over the revision 1 estimate. + +**Prototype acceptance:** +- the §13 unit goldens; +- sln and CPM e2e on SDK 6–10, plus Windows and macOS; +- the docker suite and the real-clone legs; +- G1, G3, G4, G10–G15; +- zero changes to existing NuGet test outcomes. + +**Promotion to default** additionally requires G2, G7, G8 and G16; depscan #2–#4; and one release as opt-in. + +## 16. Adversarial review + +Dispositions: **Fixed** (design changed), **Refused** (moved to refused shapes), **Accepted** (risk kept, with rationale), **Open** (gating experiment or question). + +### Shapes +| ID | Finding | Disposition | +|---|---|---| +| S-B1 | `[V′]` spreads via ProjectReference (NU1107, NU1608, NU1004 outside the closure) | **Fixed:** `[V′, )`, SOCKETPATCH006, range-consumer lock edits, gap check (§6.1, §6.4, §6.5). Goldens G11. | +| S-B2 | Once-guard on a chained DBT imports too early; pin gives NU1504 | **Fixed:** DBP `CustomAfterDirectoryBuildTargets` (§6.3). SDK 6/7 is G10. | +| S-B3 | autocrlf breaks the seed | **Fixed:** generated `.gitattributes` plus `check-attr`; LFS refused (§6.2) | +| S-M1 | Pins not TFM-conditioned | **Fixed:** alias condition from assets; unknown alias refused | +| S-M2 | Consumer outside the root breaks, or goes silent under `[V′, )` | **Refused:** `vendor_nuget_external_consumer`; 006 covers in-root drift | +| S-M3 | 004 fires after the nupkg is written | **Fixed:** pre-check plus deletion of `NuGetPackOutput` (G2) | +| S-M4 | Several nuspecs give MSB4184 | **Fixed:** exact nuspec path | +| S-M5 | Non-SDK PackageReference projects are unguarded | **Refused:** `vendor_nuget_non_sdk_project` | +| S-M6 | NU1903 still fires on V′ | **Fixed:** §2.3 corrected; NuGetAuditSuppress in prototype; info code on < 6.11 | +| S-M7 | Literal spellings, `*.proj` types, SDK < 8 `-getItem` | **Fixed:** one condition per spelling, `*.*proj` plus sln; SDK < 8 property versions and missing SDK refused | +| S-minor | CPM golden, canonical order, per-package seed hashing, exact literal in pack, implicit refs, tool wording, RID goldens, MAX_PATH, analyzers | **Fixed:** §6.5, §6.6, §7.1, §13, uuid8. Seed hashing moved to a once-per-restore check (`_SpSeedChecked`). Analyzers are G1. | + +### Environments and CI +| ID | Finding | Disposition | +|---|---|---| +| E-B1 | EOL normalization fails fresh clones | **Fixed:** `.gitattributes`, `--check` against HEAD blobs, `ls-files --eol`, clone legs in acceptance | +| E-M1 | Same-bytes V′ in the GPF fails 003 with destructive advice | **Fixed:** content-aware 003, `/` keys, new message (G15) | +| E-M2 | Guard runs in design-time builds | **Fixed:** `DesignTimeBuild` condition; §9 corrected (G16) | +| E-M3 | Windows path depth | **Fixed:** uuid8 directory, absolute-path check with refusal, Windows leg in prototype | +| E-M4 | EOL-sensitive comparisons | **Fixed:** generated files `-text`; EOL-neutral records and splices | +| E-M5 | Mixed packages.config/SDK undefined | **Refused:** `vendor_nuget_mixed_project_styles`; dual wiring is Q5 | +| E-M6 | `--check` needs restore outputs | **Fixed:** persisted closure is authoritative; unrestored applies only to new projects | +| E-M7 | Renovate/Dependabot edit the targets | **Fixed:** property-indirected versions, render sha, 006 catches edits. Q3 stays open. | +| E-minor | Sparse checkout, Docker COPY, `git clean`, seed size, case collisions, predictable-V′ attack, SBOM alerts, setup-dotnet cache, concurrency | **Fixed/Accepted:** block comment; `vendor_nuget_dockerfile_copy`; untracked-seed check; size caps; case-fold refusal; server gap check over org upstreams (residual accepted, §9); depscan #4; dot-directory skip; no fix needed for concurrency | + +### Integrity +| ID | Finding | Disposition | +|---|---|---| +| I-B1 | Seed file set unpinned; planted files disable the guard; lock pins nothing | **Fixed:** restore-time set and hash check (G13), strict CLI walk, `--check --online`, unconditional guard. §1 wording narrowed. **Accepted residual:** a dev machine that already restored a planted seed runs its code; CI `--online` is the defence. | +| I-B2 | Git transforms seed bytes | **Fixed** (same as E-B1) | +| I-M1 | Env var, props or rsp disable the guard | **Fixed:** unconditional assignment (VERIFIED-ADV x1), CLI allowlist, rsp flagging | +| I-M2 | New nested DBT hides the import silently | **Fixed/Accepted:** `--check` evaluates every project's import; documented as the CI gate. Build-time silence is accepted. | +| I-M3 | GPF V′ beats the seed | **Fixed:** content-aware 003; §9 wording corrected; exact per-uuid root is no longer needed because keys are content-based | +| I-M4 | Builder 1 is not one byte sequence | **Fixed:** feed tier accepts builder 0 only. **Accepted** for the fallback tier: only lock churn. | +| I-M5 | Drift repair from a poisoned GPF | **Fixed:** repair only from depscan or a signature-verified download | +| I-M6 | Upstream provenance lost | **Fixed:** signature or sha512 check at vendor; `upstreamSha512` recorded; depscan #3 is GA | +| I-M7 | Repo-level pfx author trust is id-unscoped | **Fixed:** feed tier defaults to CA cert; ephemeral-key opt-in; `--check` flags trustedSigners | +| I-M8 | Percent-decoding traversal and MSBuild injection | **Fixed:** decode then validate, metacharacter refusal, escaping, strict validation at render | +| I-M9 | Analyzer and build-only packages unguarded | **Fixed:** `@(Analyzer)`, assets `libraries`, unconditional restore check (G1) | +| I-minor | Symlinks, late `require`, nuspec edits, §9 wording, stale obj, hex-case golden | **Fixed:** symlink refusal on ancestors; `--check` policy re-probe; set check; wording; test legs; golden | + +### Lifecycle +| ID | Finding | Disposition | +|---|---|---| +| L-B1 | Patch update renders both uuids, pins a dead V′, breaks revert | **Fixed:** `sync_shared` after persist and sweep; V′-aware planner; `original: None` carry-forward; replaced entry excluded (§7.5) | +| L-B2 | Import records lost across uuids, order and crashes | **Fixed:** state-owned block, record-free excision, blocks removed before the targets, stub on drift (§7.2) | +| L-B3 | Seed not byte-stable through git | **Fixed** (E-B1) | +| L-M1 | Preserved or Kept entries re-wired | **Fixed:** `wired` flag; missing seeds are never rendered | +| L-M2 | Revert order defeats drift-keep | **Fixed:** §7.4 order | +| L-M3 | No pristine source on clean machines; `debug_assert` panic | **Fixed:** fetch rung verified against ledger hashes; assert removed for fallback (§7.7) | +| L-M4 | Hot path and `--check` before restore; re-plan misreads V′ | **Fixed:** persisted plan; V′ decoded as V | +| L-M5 | v4 binaries corrupt fallback entries | **Fixed/Open:** literal uuid anchor, hybrid router, heal step; G14 | +| L-M6 | VEX silently drops fallback patches | **Fixed:** extractor and probe files in prototype scope | +| L-M7 | Unsafe unwind of shared outputs and reused seeds | **Fixed:** staged writes, snapshots, delete only what this run created | +| L-M8 | Migration destroys its own source | **Fixed:** prelude first, re-version the committed legacy nupkg, one atomic unit (§7.6) | +| L-M9 | Restore does not fail closed on a missing targets file | **Fixed/Accepted:** anchor NU1301 when `.socket` is gone (G12); targets-only deletion accepted, with `--check` as CI gate; §9 corrected | +| L-M10 | Transitive pins keep themselves alive | **Fixed:** pin conditioned on the direct parent; in-use probe excludes self-edges; `pin_orphaned` | +| L-M11 | Layout choice does not persist | **Fixed:** `GlobalArgs` flag, inference, `nugetLayout` | +| L-P1 | Prototype needs CLI hooks not in §15 | **Fixed:** §15 scope and module table extended (+30–40%) | +| L-minor | User edits of the targets, submodules/templates, builder churn, dry run, directory `ledger_covers`, NU1903 | **Fixed:** render sha plus `targets_edited`; worktree bound plus skip; builder sticky; in-memory render; inventory intactness; S-M6 | + +## 17. Prototype (follow-up branch `v5/nuget-vendoring-prototype`): what was built and how it deviates + +**Code:** `crates/socket-patch-core/src/vendor/nuget_{version,seed,lock,targets,fallback}.rs`, plus small routing hooks in `vendor/mod.rs`, `commands/vendor.rs` and `commands/repair_vendor.rs`. + +**Tests:** +- unit tests in each module; +- lock fixtures captured from real `dotnet` in `crates/socket-patch-core/tests/fixtures/nuget-fallback/{sln,cpm,cpmpin,cpm-tool}`. NuGet's own `--force-evaluate` output is the golden for the splice; +- `crates/socket-patch-cli/tests/e2e_nuget_fallback_dotnet.rs`, which is `#[ignore]` and runs against the real SDK and nuget.org. Its legs are `sln_locked`, `cpm_locked`, `cpm_pinning` and `sln_patch_update`. + +The default layout is unchanged: every existing NuGet test passes untouched. + +Implementation: `crates/socket-patch-core/src/vendor/nuget_{version,lock,seed,targets,fallback}.rs`. + +### Behaviour deviations (deliberate) + +1. **Lock rule for a range at V that resolved elsewhere.** The spec only edits entries whose `resolved` is V. NuGet also rewrites `requested "[V, )"` → `"[V′, )"` on a `CentralTransitive` (or `Direct`) entry that resolved to some other version (cpm-tool `Tool`: `resolved 12.0.1`). Without this edit, `--locked-mode` fails NU1004. The splice now makes the requested-only edit (`resolved` and `contentHash` are left unchanged), and the golden `cpm-tool` fixture proves it. +2. **`CentralTransitive` at V counts for transitive-only too.** Under CPM *without* pinning, a `CentralTransitive` V entry is refused `vendor_nuget_transitive_only` when the project has no `Project` reference to a redirected project. This follows from NuGet itself: without pinning the central version is not applied, so the entry resolves upstream V and the build guard would fire. With `CentralPackageTransitivePinningEnabled` it is redirected. Pinning is detected by a repo-global text scan. +3. **Layout selection is per purl.** The env var or any `nuget-fallback` ledger entry selects the fallback layout, except for a purl the ledger already holds as a legacy feed entry: that purl keeps routing to `nuget_feed` (vendor and service preflight), so an in-sync legacy entry never fails `vendor_nuget_layout_mixed` once the repo opts in. The refusal stays in the backend for direct callers. The earlier "targets file exists" signal was dropped: `.socket/` writes are outside the group commit, so a failed or crashed run could leave the targets behind and silently opt the repo in. +4. **DBP block text.** + - The begin comment is `(generated; remove with: socket-patch vendor revert)`. An XML comment cannot contain `--`, and a DBP that fails to load is silently ignored by restore. + - `` is rendered with its trailing slash (`""` / `../`), which gives `$(MSBuildThisFileDirectory).socket/...` and never `//.socket`. + - **The anchor line is gone.** Registering `.socket/vendor/nuget` itself as a fallback folder let anyone plant `//` beside the uuid dirs and have NuGet restore it under `--locked-mode` with no seed check (security review, reproduced on SDK 8). The block now holds a `SocketPatchNuGetImportCheck` target instead (`BeforeTargets="_GenerateRestoreGraph;CollectPackageReferences"`, condition `'$(SocketPatchNuGetTargetsImported)' != 'true'`), which fails restore with **SOCKETPATCH007** when the targets were not imported (the vendored tree is missing; restore ignores the missing import). The e2e proves it for locked and unlocked restore. `-p:SocketPatchNuGetGuard=false` disables it like the other guards. + - A props file socket-patch **created** carries `socket-patch:begin created` on its begin line. Only such a file is deleted on the last revert, when nothing but its block was added (compared with BOM and CRLF normalised). A user's own `\n\n` is kept, and a CRLF checkout of a created file is still deleted. Re-rendering a block keeps the tag. + - The `` insertion point (and a self-closing ``) is found with comments blanked, so a `` inside a trailing comment is never used. +5. **The render inputs live in the marker.** + - `socket-patch.vendor.json` gains an optional `nuget` section: `id`, `version`, `socketVersion`, `contentHash`, `spellings`, `inventory`, and `wired` (default true). + - `sync_shared` renders the seed inventory from the marker, never from the disk, so a tampered seed is never re-baselined. + - The marker is not trusted alone: when the ledger has a `nuget-fallback` entry for the uuid, its `artifact.file_inventory` replaces the marker's inventory in the render. The one exception is the seed the current vendor run just wrote (`RenderScope::fresh`). The markers are also diffable in `.gitattributes` (`/*/socket-patch.vendor.json -text diff`), so a changed hash shows in review. + - The `AlreadyPatched` fast path also requires every patched file in the seed to hash to its `afterHash`. + - A `--preserve-state` revert (`keep_artifact`) writes `wired: false`. The seed stays on disk but is no longer rendered into the targets. +6. **Extra refusal codes** beyond the spec list: + - `vendor_nuget_nuspec_patched`: the patch rewrites the root nuspec, which the layout re-versions. + - `vendor_nuget_version_literal_unknown`: a `$(Property)` or item version for the id. + - `vendor_nuget_lock_unreadable`: an unparseable or unreadable lock. + - `vendor_nuget_seed_failed`: the canonical zip failed. + - `vendor_nuget_symlink_unsupported`: a project file, `Directory.Build.props` or `packages.lock.json` under the root is a symlink (an atomic rewrite would replace the link, and a skipped linked props file would silently shadow the wiring). `sync_shared` and revert refuse the same way before writing anything. + - `vendor_nuget_repo_too_large`: the discovery walk hit its 100,000-entry cap. Vendor refuses, and `sync_shared` / revert fail before writing, instead of silently wiring only part of the tree. + - Coordinates containing `--` refuse `unsafe_coordinates` (they are rendered into XML comments); `.` / `..` and `--` never render from a marker either. + - Floating (`*`) or bracketed ranges naming V in csproj/props text refuse `vendor_nuget_range_unsupported`, the same code as lock ranges. +7. **Member-name validation.** + - Names are percent-decoded first. + - They must then pass `is_safe_relative_subpath` + `is_plain_archive_name` + `names_are_unambiguous` (printable ASCII; no `\ : * ? " < > |`; no case-fold collisions). + - Names containing `$ @ % ; '` are refused instead of escaped. + - Members named `.nupkg.metadata`, `*.nupkg` or `*.nupkg.sha512` are refused. + - OPC parts (`[Content_Types].xml`, `_rels/`, `package/`) and `.signature.p7s` are dropped, compared case-insensitively. + - The nuspec `` must match the purl id. +8. **`vendor_nuget_no_lockfile`** is emitted when **no** discovered `packages.lock.json` references the id (at V, V′ or an older Socket version of V), not per project. +9. **Signature policy is conservative.** + - The CLI refuses if *any* config sets `signatureValidationMode=require`. It checks every dir from each SDK project dir up to the root, the root and its ancestors (all three config spellings), `~/.nuget/NuGet/NuGet.Config`, and `%APPDATA%/NuGet/NuGet.Config`. + - NuGet's nearest-wins override is not modelled, and `DOTNET_NUGET_SIGNATURE_VERIFICATION` is not read. +10. **contentHash bytes.** The canonical zip is built with the existing `write_zip_entries` (zip-crate deflate level 6, fixed DOS time, mode 0644), so its hash differs from the Python-built fixture hash (`DDiM…`). NuGet only string-compares lock ↔ `.nupkg.metadata`, and the real-dotnet e2e proves the Rust value works. +11. **Wiring record shape and per-entry revert.** + - One `nuget_lock_file_v2` record per lock that pins the id after the splice (edited now or already spliced): `key` is the id, `original` and `new` hold the whole text. Recording unchanged-but-spliced locks keeps their revert across a partial re-vendor. + - The kind is registered in `ledger_snapshots::WHOLE_FILE_KINDS`, so large `new` values are diff-encoded (ledger version 2). + - When the pre-edit lock already names a Socket version of V anywhere (a re-vendor, same or older uuid), `original` is `None` and `carry_forward_wiring` fills it from the replaced entry. + - **The revert does not restore whole files.** `nuget_lock::unsplice_lock` puts back only this entry's values (V′ in `resolved`, `requested`, Project `[V′, )` ranges, and the matching `contentHash`), each to the value the recorded `original` held at the same JSON path (else plain V / `[V, )`; a hash only from `original`). Another seed's splice, a package or framework added since, and EOLs are untouched, so seeds sharing a lock revert in any order and a stale carried-forward original cannot discard later edits. When no upstream hash is recorded the entry is left at V′ and reported `vendor_lock_entry_drifted` (the seed is then kept). +12. **Seed rebuild.** An existing but stale seed dir for the same uuid is rebuilt through `.socket-stage` + `swap_stage_into_place`. If a later step fails, the unwind deletes the seed only when this run created it, so a replaced stale seed is not restored. +13. **Self-closing `` DBP.** It is expanded to `…`, and excision leaves `\n` rather than byte-restoring the self-closing spelling (the file is kept, never deleted, since socket-patch did not create it). The same applies to a `` sharing a line with other content, where one extra EOL remains. +14. **Seed materialisation.** + - The new `nuget_feed::patched_nupkg_bytes` wraps the unchanged `materialise_patched_nupkg` (service download → local rebuild) in a private temp stage. + - `config_wired=true` is passed only so a failed build does not prune the stage's parents. + - The seed is then extracted from those same-version bytes. +15. **Patch update (new uuid).** `splice_lock` treats any value at an *older* Socket version of the same V (resolved, requested, Project range) as a re-splice target and moves it to the new V′ and hash. The vendor run leaves the superseded uuid's seed out of the render (`RenderScope::exclude`), and the CLI re-runs `sync_shared` after `sweep_stale_artifact` deletes the old uuid dir. The new `sln_patch_update` e2e leg proves the flow with real dotnet. + +### Verified with real dotnet (SDK 8.0.131, nuget.org) +`e2e_nuget_fallback_dotnet` covers `sln_locked`, `cpm_locked`, `cpm_pinning` and `sln_patch_update`. The first three each do the following: +- runs the real `socket-patch scan --mode vendored --vendor-source build` with `SOCKET_PATCH_NUGET_LAYOUT=fallback`; +- on a fresh copy, runs `restore --locked-mode` with a cold GPF: the locks are unchanged and the App (and Tool) bin dll carries the patched bytes; +- does the same with a warm GPF that holds upstream 13.0.1; +- tampers a seed file and checks for `SOCKETPATCH002`, then restores the committed bytes and checks that restore passes; +- removes `.socket/vendor/nuget` from a checkout and checks that locked and unlocked restore fail `SOCKETPATCH007`; +- re-runs vendor **without** the env var (the ledger alone selects the layout) and checks that no project file changes; +- runs `vendor --revert` without the env var and checks that the tree outside `.socket/` is byte-identical and no `.socket/vendor/` remains; +- runs a locked restore and build of the reverted tree and checks that the pristine dll is back. + +`sln_patch_update` vendors uuid 1, then a newer patch (uuid 2) without the env var, and checks: no `vendor_nuget_no_lockfile`, both locks at V′₂ only, the targets name only uuid 2, the uuid 1 dir is gone, a cold locked restore leaves the locks unchanged and builds the uuid 2 bytes, and `vendor --revert` restores the pre-vendor tree byte for byte. + +### Known gaps (out of scope / follow-ups) +- The following are not implemented: + - SOCKETPATCH003/004/006, pack range restore, per-project transitive pins, packages.config, the feed tier and signing, migration from the legacy layout (refused as `vendor_nuget_layout_mixed` for the same purl only), and `--check`. + - Post-write git probes: `vendor_artifact_gitignored` / `_git_transformed`. + - Long-path warning, size caps beyond the existing zip-bomb caps, and the version-gap/collision checks against nuget.org. +- **Lock discovery** reads only `/packages.lock.json`. `NuGetLockFilePath` and `packages..lock.json` are ignored. +- **Version literals** are found only as `Version=` attributes or `` children of `PackageReference` / `PackageVersion` / `GlobalPackageReference` in `*.csproj|fsproj|vbproj|props|targets` text. MSBuild evaluation is not used. +- **DBP placement** only considers DBPs inside the root. A `Directory.Build.props` *above* the repo root that a project relied on is shadowed by the created root DBP, because the created file does not import the parent. +- **Repair.** It can re-synthesise a fallback entry from the targets file (flavor stamped `nuget-fallback`), but not its lock wiring records. A revert of such a reconstructed entry deletes the seed and leaves the locks spliced. +- **Service download path.** Every e2e vendor uses `--vendor-source build`; the fallback's use of a downloaded service artifact (`patched_nupkg_bytes` → service copy, fallback `service_preflight`) is covered only by unit tests of the shared feed pipeline. +- **SOCKETPATCH005** (the build guard) is not exercised by the e2e. +- **Marker trust.** A uuid dir with no ledger entry (state.json lost, or before the entry is committed) still renders from its own marker. +- **Orphan sweep.** It now treats uuid dirs named in `socket-patch.targets` as wired. +- **Migration.** A purl held by a legacy feed entry keeps the feed layout even with the env var set; migrating it needs a `vendor --revert` first. +- **VEX.** There is no fallback extractor. Discovery returns nothing for fallback repos (the golden `vex-discover-golden/nuget-fallback.json` records empty results; there is no crash). Ledger-based VEX verifies the seed dir through the existing dir-artifact path, members plus `fileInventory`. +- **Crawler.** After a restore, `obj/project.assets.json` lists the fallback folder as a package folder, so the crawler may report `newtonsoft.json@13.0.1.` as an installed package. This is not mapped back to the upstream purl. +- **Pre-existing failures, unrelated.** + - `cargo clippy --workspace --all-targets --all-features -- -D warnings` fails only in the untouched `crates/socket-patch-cli/tests/covgap_commands_rollback.rs:152` (dead fields `before_hash`/`after_hash`). + - 9 core lib tests fail in this environment regardless of this change (it runs as root: read-only-permission tests and an RSS measurement). + - `cargo fmt --all` reformats ~75 unrelated files on HEAD. Those changes were reverted; only the files touched here are rustfmt-clean. +