Fix Windows VM helper rejecting setgid source directories

[ekollof]

Fixes #9698
Fixes #9374

GNU chmod 0700 leaves setuid/setgid on directories when the numeric mode is four digits or fewer. prepare_caller_mounts then requires mode 700 exactly, so a leftover g+s on ~/Windows (often 2700 or 2777) makes every privileged VM action fail closed with no diagnostic.

This hardens those directories with chmod a-s,u=rwx,go= (which does clear the special bits) and prints the observed modes if the check still fails.

dockur samba.sh also chmod 2777s an empty /shared bind at container start, so a successful launch immediately undoes the host privacy mode. A hidden .omarchy-keep sentinel keeps the share non-empty, and the helper re-hardens ~/Windows after docker compose up.

Covered by a user-side prepare_user_mount_sources test and a root-namespace case that starts from 2700/2777 sources.

[omarchybot]

Reviewed by Claude Opus 5, with an independent second opinion from Codex at xhigh. Both reached the same verdict, and Codex contributed two mechanisms the first pass had not worked out; its independence is not currently guaranteed, so the agreement is worth less than those two findings.

What was checked. The diff against quattro at 8915169; every place in bin/omarchy-windows-vm that reads or asserts a mode (prepare_caller_mounts 609/616, mounted_leaf_matches 518, mounts_ready 650, assert_mounts_safe 919, and the unprivileged fast path at 89-100); the chmod semantics themselves, measured rather than assumed; and dockur/windows src/samba.sh addShare(), because the sentinel rests on a claim about it. Tests ran on a disposable VM, not locally: windows-vm-test.sh (4 passed), windows-vm-compose-test.sh (22), windows-vm-mount-boundary-test.sh (10), windows-key-test.sh (7) and ./test/cli, all green. Reverting chmod_private_dir to chmod 0700 on that VM makes both of your new cases fail — storage mode is 2700, expected 700, and root could not harden setgid VM source directories — so they are regression tests that actually regress.

The mode fix is correct, and it is the right kind of repair. On coreutils 9.11, chmod a-s,u=rwx,go= yields exactly 700 from 2700, 2777, 6700, 1700, 1777 and 3777; plain chmod 0700 yields 2700, 2700, 6700, 700, 700 and 2700 from the same starts. Nothing in the diff loosens a check: the exact-700 comparisons at 616 and 518 are untouched, and 617 adds a diagnostic rather than a tolerance. Extending the hardener to write_credentials (1099) is right as well.

The sentinel is where the defects are. Its premise holds — dockur's addShare() decides empty="Y" with [ -z "$(ls -A "$dir")" ], and ls -A counts a dotfile, so a .omarchy-keep does suppress the chmod 2777. The implementation is the problem.

1. ensure_shared_sentinel gives the caller a root-write primitive (high). At line 574 it does [[ -e $sentinel ]] || : >"$sentinel", and at line 622 that runs as root inside a directory the unprivileged caller owns. : > opens with O_CREAT|O_TRUNC and no O_NOFOLLOW, so a dangling symlink at ~/Windows/.omarchy-keep is followed. Demonstrated in the mount-boundary test's own namespace on the worker: with .omarchy-keep -> /var/secret/created-by-root planted by uid 1000, prepare_caller_mounts created that file root:root 0644 outside the caller's home. An existing target is not clobbered, because -e short-circuits first — but -e and the redirect are two syscalls with an attacker-controlled path between them, so swapping the symlink in that window turns it into a truncate of any root-owned file. Pinning the parent by FD does not make the child lookup safe. This is the only place in the file that reaches outside the boundary the rest of the function is built to enforce, and it is worth weighing against everything else the function does to avoid exactly this.

2. On an existing install the sentinel is created by root, in the user's home (low). launch does not call prepare_user_mount_sources: migrate_legacy_compose returns at 1135 as soon as $COMPOSE_FILE exists, so the first launch after this lands goes straight to priv up_wait and the root half creates the file. Confirmed on the worker: root:root 0644. The user can delete it, and the next privileged run puts it back root-owned. remove also leaves it behind, since rm -rf at 1418 covers ~/.windows but ~/Windows is deliberately kept.

3. A sentinel that cannot be written now fails every privileged action (low). Line 574 returns 1 and line 622 aborts prepare_caller_mounts, which up (1368), up_wait (1449), remove (1410) and write_compose (1078) all reach through assert_mounts_safe. A full or read-only ~/Windows previously did not stop removal; now it does. Losing a defensive sentinel probably should not be fatal, least of all on the cleanup path.

4. The sudoless-Docker path never creates a sentinel, and restore_shared_privacy races (low). Codex traced this one: on a box with sudoless Docker and an already-empty share with valid 700 anchors, the fast path at 95 finds mounts_ready true and runs __priv_up as the caller; assert_mounts_safe skips prepare_caller_mounts at 919 because EUID != 0, so ensure_shared_sentinel never runs before Docker starts. The same holds after the user or the guest deletes the file — the share is exported to the guest at 754-756, so it is deletable from inside Windows. That leaves only restore_shared_privacy, and dc up -d returns when the container is started, not when its entrypoint has reached samba.sh, so the restore can land before dockur's chmod 2777 rather than after it. The comment at 565-567 reads as though the ordering were guaranteed.

5. restore_shared_privacy supplies __priv_up's exit status (low). It is the last command at 931, so a failed chmod makes launch print ❌ Failed to start Windows VM! at 1450 over a container that started fine and is left running.

Neither of the two behaviours that distinguish this PR — the sentinel under the privileged path, and restore_shared_privacy — is covered by a test. The boundary suite's shared directory is already non-empty from shared.txt at line 99, and the unit test only asserts the sentinel exists; both would still pass if restore_shared_privacy were deleted.

Nothing was pushed to your branch. Finding 1 needs a design decision rather than a patch — drop the privileged creation, or open with O_NOFOLLOW/O_EXCL — and there are five open pull requests fixing this same root cause, so editing one of them ahead of that choice would not help. #9322, #9414 and #9504 change the same two chmod sites and stop there; #9605 also covers write_credentials and adds the diagnostic; this one is the only one that addresses the container re-adding the bit after start, which is what #9746 reports. On the shared question of which moments are covered: all five clear the bits at both the user preflight and the privileged mount preparation, and all five therefore fix #9334, #9374, #9540, #9567 and #9698, and also fix the #9746 launch failure, because the next privileged run re-clears before the exact-700 check. What the other four leave standing is the exposure rather than the failure — ~/Windows sits at 2777, world-writable, from container start until the next privileged action, which is the thing the 0700 hardening exists to prevent, and a sudoless-Docker user drops out of the fast path into a polkit prompt on every launch after the container has run.

Waiting on the maintainer, both for that choice and for a decision on the sentinel.

[ekollof]

Addressing the review by dropping privileged sentinel creation.

ensure_shared_sentinel ran as root in a caller-owned directory with a create/truncate and no O_NOFOLLOW, which is a write primitive the rest of prepare_caller_mounts exists to prevent. A missing sentinel also should not fail remove.

This revision:

• keeps chmod_private_dir and the exact-700 checks
• removes all privileged (and user-side) sentinel creation
• restores share mode after windows started successfully, so it lands after dockur samba.sh rather than racing dc up -d
• treats that restore as best-effort so a chmod failure cannot report a successful start as Failed to start Windows VM

The world-writable window is now only during guest boot, not until the next privileged action.