Repository navigation
fix(mcl.utils.deploy-state): stop the ancestor guard rejecting what the walk accepted - #598
Merged
Merged
Conversation
…n walk c85f2a0 hardened the deployment state directory against symlink and ownership attacks. Two of its rules cannot be satisfied by the places deployments actually keep state, and every deploy_state-using check has been red since. 1. openOrCreateStateDirectory() re-validated the parent descriptor that openOrCreateTrustedParentDirectory() had *just* validated, but under a different policy: validateDirectoryDescriptor() has no sticky-directory exemption. So the walk accepted /tmp (root-owned, 01777) as an ancestor and the very next statement rejected it as a parent. Any `--state-dir /tmp/<name>` — which is what deployment-direct-ssh-success-vm, deployment-reconciler-supersession-vm and deployment-production-cutover-simulation-vm all pass — died with "Deployment state directory parent /tmp is writable by its group or other users." The parent is an ancestor; validate it once, by the ancestor rule. 2. The walk anchor was required to be owned by root or by the effective deployment user. The anchor is "/" (or the process working directory), never a component the caller picked, and inside a chroot or user namespace it is legitimately owned by neither: in the Nix Linux build sandbox "/" is uid 65534, mode 0750. That made *every* absolute state directory unreachable from a Linux sandbox and broke deployment-darwin-activation-integration on x86_64-linux, while the same check stayed green on aarch64-darwin where the sandbox has a real root. Validate the anchor for what it can actually promise — that no other user may plant entries in it — and keep the full ownership check on every named component below it. Add a regression test placing the state directory directly inside a world-writable sticky parent, which is the shape all of the above use and which no existing test covered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Thanks for your Pull Request! Below you will find a summary of the cachix status of each package, for each supported platform.
|
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
1 participant
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.
Four CI checks have been red on
devsincec85f2a00(2026-08-11). They are one bug with two faces.Face 1 — the parent is validated twice, under two different policies
openOrCreateStateDirectoryre-validates the parent descriptor thatopenOrCreateTrustedParentDirectoryhas just validated, but under a stricter policy with no sticky-bit exemption. So the ancestor walk accepts/tmpand the very next statement rejects it:Fails:
deployment-direct-ssh-success-vm,deployment-reconciler-supersession-vm,deployment-production-cutover-simulation-vm.Face 2 — the walk anchor is unreachable in the Nix Linux sandbox
The walk required its anchor to be owned by root or the deploy user. Inside the sandbox
/isuid 65534, mode 0750(probed directly), so every absolute state directory is unreachable:This is why
deployment-darwin-activation-integrationpasses onaarch64-darwinand fails onx86_64-linux— same code, different sandbox.Verification
All 134
mclunit tests pass, including a new regression test for a state directory sitting directly inside a sticky parent. All four checks build and pass locally.Classification was proved rather than assumed: each check's
drvPathis byte-identical ondevand on the GARM PR that surfaced them (e.g.deployment-direct-ssh-success-vm=hhbckrxnbc90h20ma8b7xvid3c7749kaon both), and that hash matches the one in the CI failure log. PRs #593, #595 and #577 — none related — fail the identical set.