Skip to content

Clear setgid before setting the Windows VM mount modes - #10411

Closed
srikat wants to merge 1 commit into
omacom:quattrofrom
srikat:fix/windows-vm-setgid-mount-source
Closed

srikat wants to merge 1 commit into
omacom:quattrofrom
srikat:fix/windows-vm-setgid-mount-source

Conversation

@srikat

@srikat srikat commented Sep 6, 2026 •

Copy link
Copy Markdown

omarchy-windows-vm launch works exactly once. After the first successful run every later launch fails silently and permanently: no output, no notification, no Docker contact - the daemon is never even started.

Cause

chmod preserves a directory's set-user-ID and set-group-ID bits when it is handed an octal mode. chmod 0700 dir on a 2777 directory leaves it at 2700:

$ mkdir t && chmod 2777 t && chmod 0700 t && stat -c %a t
2700

prepare_caller_mounts hardens both pinned sources and then requires the result to be exactly 700:

chmod 0700 -- "/proc/$BASHPID/fd/$storage_fd" "/proc/$BASHPID/fd/$shared_fd" || { ... }
storage_mode=$(stat -Lc '%a' "/proc/$BASHPID/fd/$storage_fd" 2>/dev/null) || storage_mode=""
...
if [[ $storage_mode != 700 || $shared_mode != 700 ]]; then ... return 1

The chmod succeeds, stat returns 2700, the check fails, and __priv_up_wait returns 1 before dc up -d. The desktop entry appears to do nothing.

Nothing in the tool can recover from it: the unprivileged prepare_user_mount_sources normalizes the same sources with the same octal chmod 0700, so the bit can never be cleared.

What sets the bit

The dockurr/windows image does, on every single run. Its Samba setup probes the share and then chmods it to 2777. Caught with an inotifywait on both sources across a full launch:

22:17:18 ATTRIB,ISDIR  ~/.windows/                        <- prepare_caller_mounts hardens both sources
22:17:18 ATTRIB,ISDIR  ~/Windows/
22:17:22 ATTRIB        ~/.windows/data.img                <- container starts
22:17:23 CREATE        ~/Windows/.samba-write-test.u8fzeK
22:17:23 DELETE        ~/Windows/.samba-write-test.u8fzeK
22:17:23 ATTRIB,ISDIR  ~/Windows/                         <- 0700 -> 2777

~/Windows is the bind source for /shared, so the container's chmod lands on the same inode the next bring-up checks. Verified on a real install: ~/Windows measured 0700 before the launch and 2777 after it, while ~/.windows stayed 700 throughout. The next launch then failed with no output at all.

There is a privacy consequence too. The 0700 hardening never actually holds: between runs the share sits world-writable at 2777, and the hardening chmod cannot bring it back because it cannot clear the setgid bit. With this fix the source really is 0700 at each bring-up.

Fix

Route every exact-mode chmod through a helper that clears the special bits first:

chmod_exact() {
  local mode="$1"
  shift
  chmod u-s,g-s,o-t -- "$@" && chmod "$mode" -- "$@"
}

Applied to the four sites that set a mode a later check compares against, or that are meant to normalize an inherited one: prepare_caller_mounts, prepare_user_mount_sources, prepare_boundary_component, and write_credentials. The chmod 0640/0600 "$tmp" calls are untouched: those are fresh mktemp regular files that cannot carry special bits.

Affected installs recover on their next launch, so no migration is needed.

Tests

A regression in each of the two existing harnesses:

  • windows-vm-mount-boundary-test.sh seeds g+s on both tmpfs sources before the root bind. Without the fix: not ok - root could not create verified production bind anchors.
  • windows-vm-compose-test.sh covers the unprivileged path - setgid sources must leave prepare_user_mount_sources at exactly 0700. Without the fix: not ok - setgid sources were not hardened to an exact 0700.

Both verified failing on stock quattro and passing with the fix. ./test/shell is otherwise unchanged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01E4RgepEpHPpP5i8Qcccpi8

chmod preserves a directory's set-user-ID and set-group-ID bits when it is
handed an octal mode, so `chmod 0700 ~/Windows` leaves a setgid source at
2700. prepare_caller_mounts compares the hardened mode against 700 exactly
and returns 1, so a source that picked up a setgid bit fails every launch
with no output at all, before Docker is ever contacted. The unprivileged
prepare_user_mount_sources could not clear it either, so the state was
permanent.

Route the exact-mode chmods through a helper that clears the special bits
first. Affected installs recover on their next launch; no migration needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xzqotBnVWaSdAMeqj87oy
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at 9df3382 by Claude Opus 5 in Claude Code, with a second opinion from Codex (gpt-5.6-sol) at xhigh reasoning pinned to the same SHA. Everything was measured on a disposable VM, never on the machine holding credentials. Nothing pushed to your branch.

The fix is correct and the two tests earn their place. Mutation-tested on a worker: with chmod_exact reduced back to a plain chmod "$mode" -- "$@" and both call sites left alone, windows-vm-compose-test.sh fails with not ok - setgid sources were not hardened to an exact 0700 and windows-vm-mount-boundary-test.sh fails with not ok - root could not create verified production bind anchors. Restored, both pass, along with windows-vm-test.sh and ./test/cli (112 ok, 0 failures). The mechanism reproduces directly on coreutils 9.11: chmod 0700 on a 2755 directory leaves 2700; chmod u-s,g-s,o-t then chmod 0700 lands on 700. Both assert_boundary_dir (:262) and priv_target (:73) test 8#$mode & 022, which a four-digit mode does not disturb, so the two exact comparisons — mounted_leaf_matches (:528) and prepare_caller_mounts (:600) — are the only ones that could break, and :593 now feeds both.

The Samba case recovers, at the cost of one prompt per launch. Probed on the worker: with the shared source chmodded to 2777 behind a live bind (what samba.sh in dockurr/windows does to /shared), mounts_ready fails on both this branch and stock quattro, so the sudoless fast path at :95 drops through to pkexec. From there this branch re-hardens the pinned inode and mounts_ready passes on the retry — 2777 → 700; stock quattro goes 2777 → 2700 and never recovers. So the permanent failure is gone, but a user whose guest has touched the share still gets an authorization prompt on every subsequent launch. That is the ground #9783 covers separately, not a defect here.

One low-severity note, not pushed. Splitting one chmod into two opens a window the single call did not have. On a 1777 source, chmod u-s,g-s,o-t yields 0777 — verified; a numeric chmod already clears the sticky bit, so o-t is doing nothing the second call would not — and until the second chmod lands, another local user can unlink entries in a still-world-writable directory. Separately, when the first chmod fails on one of two operands, && skips the second for both, leaving the operand that would have succeeded unhardened, where a single chmod 0700 -- a b would have hardened it. Both paths are narrow, and both close with a single umask-insensitive call: chmod "=$mode" -- "$@" gives 700 from 2755, 1777 and 4700 alike, and still processes every operand when one fails. Your call whether it is worth the churn.

On #10262, which claims a different root cause for the same failure. Its premise is that $BASHPID inside $(stat ...) names a subshell with "a different descriptor table", so the mode check reads the wrong fd. A command-substitution subshell is a fork, so it inherits the descriptor exec {fd}<dir allocated in the parent, and /proc/<subshell-pid>/fd/N resolves to the same inode. Measured on a worker: parent /proc/<parent>/fd/10 and the substitution's /proc/<child>/fd/10 both give 56:767, and the mode read through the substitution is the true current mode. The xtrace in #10035 shows the same thing — storage_mode=700 came back correct through a subshell PID, and only shared_mode read 2700, because only the shared directory carried setgid. Codex reached this independently and ran its own descriptor demonstration with the same result; its read scope is not currently confined, so treat agreement as corroboration rather than a clean-room check, but the demonstration stands on its own. So this PR and #10262 are not two halves of one problem: #10262 keeps chmod 0700 and the exact != 700 check untouched, and cannot fix any of these reports.

Waiting on the maintainer, who has four open pull requests over this file to choose between: this one, #9783, #10338 and #10262.

@Chessing234

Copy link
Copy Markdown
Contributor

hey, this is already covered in #12264 (clear setgid when hardening windows vm mounts). mind closing this as a duplicate?

@omarchybot omarchybot added the bug Something isn't working label Sep 27, 2026
@omarchybot omarchybot added the verified Omarchy Triage has verified that this issue is ready for final review label Oct 2, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Re-checked at 9df3382 against today's quattro (821ae58) by Claude Opus 5.5 in Claude Code, with a second opinion from Codex at medium effort. Everything ran on a disposable VM. Nothing was pushed to your branch.

It still fixes the bug on current quattro. The branch merges cleanly. Merged, windows-vm-compose-test.sh (23), windows-vm-mount-boundary-test.sh (9) and windows-vm-test.sh (3) all pass. With quattro's bin/omarchy-windows-vm put back under the same tests, the two new tests fail with not ok - setgid sources were not hardened to an exact 0700 and not ok - root could not create verified production bind anchors. On coreutils 9.11, a plain chmod 0700 on a 2777 directory gives 2700, and chmod_exact 0700 gives 700. ./test/cli passes except vscode generated theme references current theme file, which fails the same way with this change reverted, so it belongs to quattro and not to this branch.

On the duplicate suggestion: #12264 was opened eleven days after this one, and it also changes when the launcher is created, so this pull request stays open. Both are left for the maintainer.

Choosing between the fixes. Codex was asked which open fix to merge and was not told our answer. It found that #10411, #10338, #9783 and #12264 all clear the setgid bit, and that #10262 does not: it keeps chmod 0700 and the exact 700 check. Its pick was this one, because it is the only proportionate fix whose regression tests start from setgid sources and assert an exact 700. Its runner-up was #10338: one chmod call instead of two, and clearer error messages, but no tests. We had reached the same conclusion first, and the reviewer can read this session, so treat its agreement as corroboration rather than an independent check. It found no defect in this branch. The earlier note about the two-step chmod still stands as a low-severity choice: chmod "=$mode" would do it in one call.

#10338 already carries ready, so this one is marked verified and not ready: two ready fixes for one bug would be two merges that conflict. It is waiting on the maintainer to choose between them.

@omarchybot omarchybot removed the verified Omarchy Triage has verified that this issue is ready for final review label Oct 4, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Closing this alternative after #12323 merged into quattro. The merged fix explicitly clears setuid/setgid in both mount-source hardening paths, keeps the exact mode-700 boundary, adds a diagnostic, and covers this launch/install failure. This alternative is no longer needed for that bug.

Scope checked by GPT-6 in Codex and Codex Medium against the discussion and landed change; review independence is not guaranteed. No tests were rerun for this cleanup.

Omabot on behalf of DHH

@omarchybot omarchybot closed this Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants