Repository navigation
nuget-home heal quarantines a healthy umask-002 ~/.nuget on every project build, breaking host builds mid-restore - #761
Merged
Merged
Conversation
Avoid mistakenly quarantining healthy user-owned NuGet cache directories solely due to group write bits under standard Linux umask 002 (user-private groups). Accept owner == current UID regardless of group write bits, reserving quarantine for foreign-owned or other-writable (o+w) trees. Coordinate quarantine operations under parallel MSBuild: - Skip per-project InitialTargets during solution builds so destructive repair only runs once at the solution level prior to restore. - Use advisory file locking and completion markers under TMPDIR to bound quarantine output to at most once per build and prevent concurrent nodes from renaming directories another node is reading. - Log quarantine actions to stderr. Extend EnsureWritableNuGetHomeTests and MsBuildNuGetHealTests to verify: - Preservation of UID-owned group-writable trees with packages intact. - Quarantine of foreign-owned and unusable trees. - Concurrent parallel invocations produce at most one quarantine directory and never remove a healthy cache. Update docs/development/build-environment.md to document the permission model and parallel coordination. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
Relocate the quarantine lock and completion marker from world-writable /tmp to verified private storage in $_codeybox_cli_home (.codeybox-quarantine.lock and .codeybox-quarantine.done) to prevent symlink manipulation, squats, and DoS attacks by unprivileged co-tenants. Verify lock acquisition before executing the destructive quarantine critical section so acquisition timeouts safely fall through to the redirect fallback without uncoordinated renames. Extract the retry limit magic literal into _codeybox_lock_max_attempts=100, remove the unused CODEYBOX_NO_DESTRUCTIVE_REPAIR branch, and update documentation. Extract duplicated umask 002 chmod calls across MsBuildNuGetHealTests into the SetUmask002PermissionsAsync helper, and seed the NuGet migration marker under test homes to prevent CoreCLR named-mutex races under concurrent execs. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
The quarantine completion marker was written unconditionally once the advisory lock was held, including when the `mv` of the unusable .nuget tree failed. A transient rename failure therefore suppressed every later quarantine attempt forever; for the MSBuild InitialTargets caller the DOTNET_CLI_HOME redirect cannot propagate back to dotnet, so the broken home would strand all subsequent builds. Write the marker only when the broken tree was actually renamed aside (or confirmed absent) — there is nothing left to quarantine then, and a failed home recreation is the redirect fallback's job. When the rename itself fails, leave the marker unset so a later invocation can retry the quarantine. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
Audit rework: uid-owned group-writable storage is now admitted only when the gid is provably private to the account (no foreign members, no other primary-gid holder) via _codeybox_group_is_private, applied to both _codeybox_secure_dir and the package-cache scan; shared-group, foreign, and other-writable entries still fail closed. Quarantine paths echoed to stderr are control-stripped, the quarantine marker now clears once a usable home is observed so later breakage re-arms the heal, the mkdir lock records its owner pid for stale reaping with named bounded backoff, and stale policy comments/docs/MSBuild notes are corrected. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
The NuGet-home self-heal preamble legitimately contains `command -v getent`, so substring probe matching treated wrapped dotnet execs as tool-availability probes and skipped the audited command. Match ShellCommandAuditor's exact probe argv instead. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
_codeybox_group_is_private inspected only the first getent//etc/group entry per gid and silently degraded to files-db-only checks when getent was absent, so a shared group with a second member-bearing entry — or any directory-service group invisible to /etc — could be admitted as "private" and uid-owned group-writable storage trusted for co-tenant writers. The proof now requires the gid to be one the account itself holds (id -G), getent-based NSS enumeration (absent getent fails closed), member lists evaluated across EVERY group-db entry for the gid, and the primary-gid check over the full getent+files passwd union. _codeybox_secure_package_cache now verifies every distinct gid carried by uid-owned group-writable descendants instead of only the root gid and process egid, so a healthy cache under another provably-private group is no longer declined. Also: pidless quarantine locks are reaped by age (-mmin +1) and a lock whose pid cannot be recorded is released immediately, so a holder killed mid-acquire can no longer suppress quarantine forever; the lock retry loop and repeated usable-check are extracted into _codeybox_acquire_quarantine_lock/_codeybox_cli_home_usable; the duplicated tool-probe argv predicate is single-sourced in TestSupport.IsToolPresenceProbe; ambiguous `packages` fixture names are renamed; misleading "foreign-owned" test names now describe the unusable trees they actually build; docs updated. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
…acy across all NSS modules Per-project InitialTargets no longer run the destructive quarantine: they export CODEYBOX_NUGET_HEAL_NO_QUARANTINE and rely on the in-process NuGetHomeGuard redirect, while Directory.Solution.props marks the single solution-level invocation allowed to rename an unusable .nuget aside. The quarantine notice now actually reaches the operator — Exec drops plain stderr unless IgnoreStandardErrorWarningFormat is set, so both hooks raise stderr importance to high with parsing disabled. The private-group proof now enumerates the whole group db via keyless getent group (setgrent/getgrent covers every NSS module, unlike a keyed getgrgid lookup that returns only the first match) unioned with /etc/group, and a gid with no group-db entry is admitted only as the account's primary gid, closing the vacuous-pass hole. The package-cache size bound now runs before the per-entry gid scan, the quarantine episode is extracted into a single helper, the mutating ensure helper is honestly named, and the pidless stale-lock age is a named constant. Tests exercise the repository's real Directory.* wiring end-to-end: solution builds quarantine exactly once with a visible notice, per-project builds never quarantine, umask-002 uid-owned trees stay intact, and a genuinely foreign-owned .nuget (privileged chown) is still quarantined. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
…for non-solution builds Two audit findings shared one root: the private-group proof trusted a keyless getent dump as complete coverage, and the non-solution fallback only checked NuGet.Config readability while RestorePackagesPath stayed pinned at an unverified cache. - _codeybox_group_is_private now requires _codeybox_nss_enumeration_complete before trusting getent group/passwd dumps: every source configured for both databases in nsswitch.conf must be enumerable (files/compat/db/nis/ nisplus/yp/systemd directly; sss only when enumerate=true is proven for every served domain across sssd.conf and sssd.conf.d drop-ins; anything else fails closed). - scripts/nuget-home-heal.sh emits a machine-readable verdict (CODEYBOX_HEAL_VERDICT/DOTNET_CLI_HOME/HOME/NUGET_PACKAGES) when CODEYBOX_NUGET_HEAL_REPORT=1; CODEYBOX_NUGET_HEAL_NO_QUARANTINE now suppresses quarantine only on the literal value "1". - build/NuGetHomeGuard.targets dot-sources the heal script in report mode and applies its verdict inside the MSBuild process: redirecting DOTNET_CLI_HOME/HOME and repointing the auto-pinned RestorePackagesPath (recorded as _CodeyBoxAutoRestorePackagesPath so operator-supplied values are never overridden) at the same whole-tree-verified storage the heal uses. Falls back to the original readability check when sh/script is unavailable. - The once-per-solution hook now unsets CODEYBOX_NUGET_HEAL_NO_QUARANTINE before dot-sourcing so an ambient value cannot disable mandated repair. - Tests: NSS enumeration fixtures (files-only, sss without/with enumerate=true, ldap, mixed domains), report-mode verdict contract, literal-"1" suppression, a held-gid probe that isolates proof 0 via a fixture group db, and an end-to-end MSBuild test proving RestorePackagesPath is repointed away from a co-tenant-writable cache. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
The non-solution MSBuild guard now treats the heal report as mandatory on POSIX: unavailable reports, nonzero exits, unknown verdicts, and failed verdicts stop the build instead of falling back to config readability or repointing RestorePackagesPath at an unverified cache. The heal report emits the exact verified package path for redirected homes so the guard no longer reconstructs NuGet layout policy in C#. SSSD enumeration proof now follows the real config hierarchy (/etc/sssd/ sssd.conf or the vendor fallback, plus /etc/sssd/conf.d snippets) and the privileged foreign-owned fixture is excluded from the unprivileged CI leg and run separately as root. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
The in-process MSBuild guard now strips CODEYBOX_NUGET_HOME_UNDER_TEST from the heal-report child so the verdict describes the home MSBuild will actually consume, and usable verdicts report/apply the canonical verified packages path instead of leaving the auto-pin on a retargetable symlink spelling. The ensure-writable fallback no longer has a raw-HOME logging path, the duplicated heal Exec is collapsed, and the heal/report tests now pin canonical usable paths, rejected-cache repointing, and the held-gid probe. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox <noreply@codeybox.invalid>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated via CodeyBox — work item 53053a48939345e8b960e587d8e5bbae
Initiated by CodeyBox operator
Co-Authored-By: CodeyBox noreply@codeybox.invalid
🤖 Generated with CodeyBox