Skip to content

fix(windows-vm): accept dockur 2777 shared mount mode on launch - #10113

Open
lushprey wants to merge 1 commit into
omacom:quattrofrom
lushprey:fix/windows-vm-2777-mode
Open

lushprey wants to merge 1 commit into
omacom:quattrofrom
lushprey:fix/windows-vm-2777-mode

Conversation

@lushprey

@lushprey lushprey commented Sep 4, 2026

Copy link
Copy Markdown

Problem

omarchy windows vm launch intermittently fails to start the Windows VM, aborting with ❌ Failed to start Windows VM!.

Root cause (verified on Arch/btrfs):

  1. dockur's container runs /run/samba.sh at every start; because ~/Windows (the /shared source) is empty, it runs chmod 2777 /shared plus the Samba config directory mask = 02777, so the shared source becomes mode 2777.
  2. mounted_leaf_matches and prepare_caller_mounts demanded the exact private 0700 ($mode == 700) for both sources.
  3. That strict check made mounts_ready false, so launch fell back to pkexec, which cannot prompt in a headless/agent context → failure.
  4. Worse, btrfs does not clear the setgid bit with a plain octal chmod 0700 (2700 persists), so even the hardening step cannot reliably produce 700 on btrfs.

Fix

Relax the two mount-mode checks to require only that the caller owns the source and retains full owner rwx, i.e. accept any mode where (mode & 0700) == 0700 (0700, 2700, 2777). The FD-pinned identity/ownership checks remain strict.

  • bin/omarchy-windows-vm mounted_leaf_matches() and prepare_caller_mounts()
  • Add a boundary test asserting a dockur-style 2777 shared source still passes the final guard, while exact 0700 remains accepted.

Verification

  • Locally the patched script now launches the VM: with ~/Windows at mode 2777, omarchy windows vm launch proceeds through with_vm_lockdc up -d (no pkexec fallback) and the container reports Windows VM Status: RUNNING.
  • test/shell.d/windows-vm-test.sh (3 ok) and test/shell.d/windows-vm-compose-test.sh (22 ok) pass. The mount-boundary suite exercises the new 2777-tolerance assertion on systems where nested user/mount namespace tests can run.

Related issue: #10114 (fixes the reported launch failure).

dockur resets an empty /shared bind to mode 2777 for Samba guest access on
every container start, and btrfs does not clear the setgid bit with an octal
chmod (2777/2700 persist). The launch preflight demanded the exact private
0700 for both sources, so a freshly tracked Windows VM with an empty ~/Windows
failed mounts_ready and fell back to pkexec, which cannot prompt in headless
environments, aborting the VM start.

Relax the two mode checks to require only caller-owner full rwx (accepting
setgid and guest-write bits), keeping the FD-pinned identity and ownership
checks strict. Add a boundary test asserting a 2777 shared source still passes
the final guard while exact 0700 remains accepted.
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed by Claude Opus 5 at high effort, with an independent second opinion from Codex at xhigh reasoning, and exercised on a disposable Omarchy VM rather than only read. The launch failure you are fixing is real and I reproduced it. The diagnosis behind the fix is off by one step, though, and the correct one leads to a change that does not require loosening the guard — so this is a report rather than a patch to your branch.

The btrfs attribution is wrong, and it is load-bearing

Point 4 of the description says btrfs does not clear the setgid bit with a plain octal chmod 0700, and the two new comments in bin/omarchy-windows-vm repeat it. That is GNU chmod, not btrfs. Coreutils documents it: a command like chmod does not affect a directory's set-user-ID or set-group-ID bits unless they are mentioned in a symbolic mode, or in an operator numeric mode such as =755, or cleared in a numeric mode of five or more octal digits (Directories and the Set-User-ID and Set-Group-ID Bits). Measured with coreutils 9.11 on both tmpfs and btrfs, a directory starting at 2777: chmod 0700 gives 2700 on both, chmod 00700 gives 700 on both, chmod g-s afterwards gives 700 on both.

That matters beyond pedantry, because chmod 0700 does narrow the permission bits — 2777 becomes 2700, so the group and other bits are already gone. The only thing surviving the hardening step is setgid, which is exactly what the exact-700 check then trips over. The claim that "even the hardening step cannot reliably produce 700 on btrfs" is not the case: one extra leading zero produces exactly 700 on btrfs.

I ran that through your own boundary test as the harness, on a worker, against three builds of the helper with a 2777 shared source:

helper launch guard on 2777 mode after prepare_caller_mounts final guard unrelated local uid reading the share
quattro unchanged rejects 2700 rejects — launch fails here denied
quattro with chmod 00700 only rejects, falls back to pkexec 700 accepts denied
this PR accepts (fast path never chmods) accepts read and write

The middle row is the bug fixed with the mode left private. Every route back into a launch passes through prepare_caller_mounts before any exact-mode check, so a hardening-step fix resolves the recurring rejection and not only the first launch.

What accepting the mode gives away, and to whom

mounted_leaf_matches is the only mode check on the direct docker-group path: priv() gates up/up_wait on mounts_ready, and assert_mounts_safe skips prepare_caller_mounts when EUID != 0, so on that path nothing chmods anything. The description's stated benefit — "proceeds through with_vm_lockdc up -d (no pkexec fallback)" — is delivered by removing the one step that would have re-narrowed the mode. For a sudoless-Docker or docker-group install the source then stays at 2777 across every subsequent launch.

(mode & 0700) == 0700 admits 512 modes: every mode whose owner digit is 7, with group, other, setuid, setgid and sticky entirely unchecked. 0777, 0755, 1777, 4777 and 7777 all pass. That leaves the leaf held to a weaker standard than its own parents, which assert_boundary_dir still requires to have no group or other write bit.

The anchor is reachable. prepare_runtime_tree creates /var/lib/omarchy/windows at 0755 and mounts, users and users/$UID at 0711, all root-owned — searchable by any local uid, at a path that is entirely predictable. On a stock Omarchy install /etc/login.defs sets HOME_MODE 0700, so ~/Windows is not reachable through the home directory at all; the bind anchor is the only path another account has, and the leaf's own 0700 was the only thing closing it. Your test file already asserts exactly this at lines 118-126, where setpriv --reuid=1001 must fail to read through the anchor.

Re-running that same probe inside your new block, on a worker:

PROBE READ: EXPOSED - uid 1001 read /var/lib/omarchy/windows/mounts/users/1000/shared/shared.txt at mode 2777
PROBE WRITE: EXPOSED - uid 1001 created /var/lib/omarchy/windows/mounts/users/1000/shared/attacker-planted.exe at mode 2777
ok - dockur 2777 shared mode is tolerated while exact 0700 stays accepted

Concretely, for any other local account on the machine — a second user, a service account, anything running outside the user's own uid: read every file exchanged with Windows, and plant an executable in the folder the user browses from inside the guest. The same guard covers EXPECTED_STORAGE, where a world-writable directory means the VM disk image can be unlinked and replaced even if the image file itself is not writable.

The container does widen the mode — that part of the description checks out. dockur/windows src/samba.sh runs chmod 2777 "$dir" on the share, guarded by an empty-directory test, which is why the reports cluster on empty shared folders (#10114). The behaviour is real; what follows from it is a question about whether the helper corrects the mode or accepts it. Under the base branch, and under the other open PRs on this file, the next launch narrows it back to 700. Under this change nothing does.

The new test's last assertion does not test what it says

Codex caught this and I measured it. Line 155 does chmod 0700 /home/shared-target to restore the exact private mode before line 156 asserts it is still accepted — but the directory is setgid by then, so that numeric chmod leaves 2700:

MODE after chmod 2777: 2777
MODE after the test's own chmod 0700: 2700

So "exact 0700 stays accepted" is really asserting that 2700 is accepted; 0700 is never exercised, and neither is rejection of 0777 or of an unsafe storage mode. The test reproduces the bug it is written about, one line below the comment describing it.

An empty mode is now a fatal bash error rather than a refusal

Lines 596-597 explicitly permit an empty mode with || storage_mode="", and line 598 then feeds it to 8#. On bash 5.3.15 that is (8#: invalid integer constant, and the expansion error unwinds out of prepare_caller_mounts and past its caller's || return 1 — so the intended return 1, the exec {fd}<&- cleanups and __priv_write_compose's exit 2 never run; the privileged helper dies mid-transaction instead of refusing. It fails closed, but [[ $storage_mode != 700 ]] handled the same case as designed. ${storage_mode:-0} or keeping a string comparison closes it.

Separately, AGENTS.md asks for (( )) for numeric tests rather than arithmetic expansion embedded in [[ ]]; the file's own idiom is ((8#$mode & 022)), used only where a failed stat has already returned.

Where this leaves it

Not merged, not approved, nothing pushed to your branch — the approach is a maintainer decision, not something to rewrite under you. Six open PRs now change this file: #9564, #9605, #9783, #9989, #10046 and this one. The other five all fix the hardening step so the mode ends up at 700, and leave both guards exact; this one is the only one that changes the guards instead, and the only one under which a widened mode is never corrected. Which lands is the maintainer's call.

What ran: your test/shell.d/windows-vm-mount-boundary-test.sh (10 passed), windows-vm-compose-test.sh (22), windows-vm-test.sh (3) and ./test/cli, all green on a worker VM at this head; the three-way helper comparison, the cross-account probe and the mode measurements above were run there too. Codex reviewed at xhigh and agreed on the guard's reach, the ancestor traversal, the btrfs misattribution and the empty-mode error — conclusions I had already reached, and its independence is not currently guaranteed, since its sandbox restricts writes and not reads. The 2700 finding in the test is its own, and I confirmed it by measurement before reporting it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants