Fix Windows VM helper rejecting setgid source directories

bin/omarchy-windows-vm

@@ -77,6 +77,24 @@ priv_target() {
   printf '%s\n' "$candidate"
 }
 
+# pkexec can only run the packaged copy at its fixed root-owned path, which a
+# dev-link checkout never shadows: the unprivileged half then runs the checkout
+# while the elevated half runs the package. All privileged mount work happens in
+# the elevated half, so a stale packaged copy would re-apply whatever chmod
+# semantics it shipped with and fail closed with no diagnostic (observed as a
+# launch refusing a 2777 share and leaving it at 2700). Refuse unless both
+# halves are the same build.
+privileged_copy_matches() {
+  local target="$1" target_hash self_hash
+  [[ -r ${BASH_SOURCE[0]} && -r $target ]] || return 1
+  # The common case runs both halves from the same file; only a dev checkout
+  # needs the content comparison.
+  [[ ${BASH_SOURCE[0]} -ef $target ]] && return 0
+  target_hash=$(sha256sum -- "$target") || return 1
+  self_hash=$(sha256sum -- "${BASH_SOURCE[0]}") || return 1
+  [[ ${target_hash%% *} == "${self_hash%% *}" ]]
+}
+
 # Run a privileged VM action. write_compose always elevates (the compose is
 # root-owned); the daemon operations run directly when sudoless Docker is on and
 # otherwise behind a polkit prompt. The stock org.freedesktop.policykit.exec
@@ -104,6 +122,10 @@ priv() {
     echo "omarchy-windows-vm: refusing to run a non-root-owned command as root" >&2
     return 1
   }
+  privileged_copy_matches "$target" || {
+    echo "omarchy-windows-vm: refusing to elevate: $target is not this command; refresh the installed omarchy package so root runs the same build" >&2
+    return 1
+  }
   pkexec "$target" __priv "$action" "$@"
 }
 
@@ -554,6 +576,81 @@ bind_mount_leaf() {
   }
 }
 
+# Make a directory mode 700, including leftover setuid/setgid. GNU chmod keeps
+# those bits on directories for numeric modes of four digits or fewer, so
+# `chmod 0700` cannot satisfy the exact-700 checks when ~/Windows was created
+# setgid (omacom/omarchy#9698).
+chmod_private_dir() {
+  chmod a-s,u=rwx,go= -- "$@"
+}
+
+# dockur samba.sh chmod 2777s an empty /shared at container start. Re-harden
+# only the protected anchor, which sits in the root-owned boundary tree the
+# caller cannot write: it is a bind of the same inode as $LEGACY_SHARED, so this
+# is what ~/Windows ends up at. Never chmod $LEGACY_SHARED by pathname — the
+# caller can swap it for a symlink between the test and the chmod.
+# Best-effort: a failed chmod must not fail a VM that already started.
+restore_shared_privacy() {
+  [[ -n $EXPECTED_SHARED && -d $EXPECTED_SHARED && ! -L $EXPECTED_SHARED ]] || return 0
+  chmod_private_dir "$EXPECTED_SHARED" || true
+}
+
+# Root-only: the mounts tree is 0711, so an unprivileged caller cannot list it
+# and the glob below would silently match nothing. After `dc down` there is no
+# PKEXEC_UID on a direct `sudo ... stop`, and a second user on the box would
+# resolve a different per-uid anchor, so root walks the tree instead. A
+# sudoless caller restores its own anchor through restore_shared_privacy.
+restore_all_shared_privacy() {
+  local dir canonical prefix
+  ((EUID == 0)) || return 0
+  # Same refusal prepare_runtime_tree applies: a non-standard privileged
+  # runtime is supported only for unprivileged tests/development.
+  [[ $RUNTIME_DIR == /var/lib/omarchy/windows ]] || return 0
+  prefix=$(realpath -e -- "$RUNTIME_DIR/mounts/users" 2>/dev/null) || return 0
+  for dir in "$prefix"/*/shared; do
+    [[ -d $dir && ! -L $dir ]] || continue
+    canonical=$(realpath -e -- "$dir" 2>/dev/null) || continue
+    [[ $canonical == "$dir" ]] || continue
+    [[ $canonical == "$prefix/"*"/shared" ]] || continue
+    chmod_private_dir "$canonical" || true
+  done
+}
+
+# Unprivileged: samba.sh runs only after dockur's ISO download (10-15 minutes
+# on a fresh install). Do not hold a polkit session open for that. Watch the
+# caller's own share and chmod it as the owner once it becomes 2777.
+schedule_share_privacy_restore() {
+  local dir="$HOME/Windows"
+  [[ -e $dir ]] || return 0
+  (
+    # disown only keeps bash from hupping this on exit. The watcher is in the
+    # install terminal's foreground process group, so closing that terminal
+    # still hangs it up from the kernel side, seconds after install returns.
+    trap '' HUP
+    local i mode
+    # 2x the documented 10-15 minute download is a thin margin on a slow link,
+    # so budget an hour; on expiry the final restore below still closes the
+    # 2777 window for anything that happened before it.
+    for i in {1..3600}; do
+      mode=$(stat -Lc '%a' "$dir" 2>/dev/null) || mode=""
+      if [[ $mode == 2777 || $mode == 777 ]]; then
+        chmod_private_dir "$dir" 2>/dev/null || true
+        # Do not exit on a failed chmod: a transient failure (a swapped path,
+        # an unwritable moment) must not end the watcher permanently while the
+        # share is still exposed. Leave only once the mode is really 700.
+        [[ $(stat -Lc '%a' "$dir" 2>/dev/null) == 700 ]] && exit 0
+      fi
+      sleep 1
+    done
+    chmod_private_dir "$dir" 2>/dev/null || true
+    # The subshell output goes to /dev/null, so the only visible trace of an
+    # expiry is this journal entry.
+    logger -t omarchy-windows-vm \
+      "share privacy watcher timed out; applied a final restore to $dir" 2>/dev/null || true
+  ) >/dev/null 2>&1 &
+  disown || true
+}
+
 prepare_caller_mounts() {
   local storage_fd storage_id shared_fd shared_id storage_mode shared_mode
   CALLER_MOUNTS_NEW_STORAGE=0
@@ -580,14 +677,15 @@ prepare_caller_mounts() {
   # Privacy is an explicit preflight step for both already-pinned sources, not
   # a side effect halfway through the two-mount transaction. Old umask-022
   # installs are hardened together before either Docker-facing anchor changes.
-  chmod 0700 -- "/proc/$BASHPID/fd/$storage_fd" "/proc/$BASHPID/fd/$shared_fd" || {
+  chmod_private_dir "/proc/$BASHPID/fd/$storage_fd" "/proc/$BASHPID/fd/$shared_fd" || {
     exec {storage_fd}<&-
     exec {shared_fd}<&-
     return 1
   }
   storage_mode=$(stat -Lc '%a' "/proc/$BASHPID/fd/$storage_fd" 2>/dev/null) || storage_mode=""
   shared_mode=$(stat -Lc '%a' "/proc/$BASHPID/fd/$shared_fd" 2>/dev/null) || shared_mode=""
   if [[ $storage_mode != 700 || $shared_mode != 700 ]]; then
+    echo "omarchy-windows-vm: VM source directories must be mode 700 (storage=$storage_mode shared=$shared_mode)" >&2
     exec {storage_fd}<&-
     exec {shared_fd}<&-
     return 1
@@ -895,7 +993,25 @@ assert_mounts_safe() {
 
 __priv_up() { assert_mounts_safe && dc up -d; }
 
-__priv_down() { dc down; }
+__priv_down() {
+  local rc=0
+  dc down || rc=$?
+  if ((EUID == 0)); then
+    restore_all_shared_privacy
+  else
+    # A sudoless-Docker stop runs unelevated, where the mounts tree is not
+    # listable. restore_shared_privacy still works here: the anchor sits under
+    # the root-owned boundary tree the caller cannot rename, and while the
+    # bind exists it is the caller's own inode, so the owner chmod succeeds.
+    # If the bind is gone (fresh reboot) the chmod fails and the next launch
+    # re-hardens through prepare_user_mount_sources instead. Best-effort: the
+    # container is already down, so a failed restore must not fail the stop.
+    if resolve_caller; then
+      restore_shared_privacy
+    fi
+  fi
+  return "$rc"
+}
 
 # Bring the VM up and wait until the guest reports it is ready, all under a
 # single elevation so the readiness poll does not prompt on every iteration.
@@ -913,11 +1029,15 @@ __priv_up_wait() {
   while true; do
     started_at=$(docker inspect --format='{{.State.StartedAt}}' "$CONTAINER" 2>/dev/null)
     if [[ -n $started_at ]] && docker logs --since "$started_at" "$CONTAINER" 2>&1 | grep -qi "windows started successfully"; then
+      # samba.sh has already run by the time the guest reports ready, so this
+      # chmod lands after the 2777 rather than racing `dc up -d`.
+      restore_shared_privacy
       return 0
     fi
     sleep 2
     ((++count > 60)) && {
       echo "Timeout: Windows VM did not report ready within 2 minutes" >&2
+      restore_shared_privacy
       return 1
     }
   done
@@ -1015,7 +1135,7 @@ prepare_user_mount_sources() {
     echo "omarchy-windows-vm: storage and shared must be different directories" >&2
     return 1
   }
-  chmod 0700 -- "$storage" "$shared"
+  chmod_private_dir "$storage" "$shared" || return 1
 }
 
 storage_space_path() {
@@ -1058,7 +1178,7 @@ write_credentials() {
   local username="$1" password="$2" old_umask dir tmp
   dir=$(dirname -- "$CREDENTIALS_FILE")
   mkdir -p "$dir" || return 1
-  chmod 0700 "$dir" || return 1
+  chmod_private_dir "$dir" || return 1
   old_umask=$(umask)
   umask 077
   tmp=$(mktemp "$dir/.credentials.XXXXXX") || { umask "$old_umask"; return 1; }
@@ -1334,6 +1454,7 @@ EOF
     echo "   - Port already in use: check if another VM is running"
     exit 1
   fi
+  schedule_share_privacy_restore
 
   echo ""
   echo "Windows VM is starting up!"

test/shell.d/windows-vm-mount-boundary-test.sh

@@ -19,6 +19,22 @@ trap 'rm -rf "$test_tmp"' EXIT
 # mount-safe copy of the helper before the mounts land.
 cp "$ROOT/bin/omarchy-windows-vm" "$test_tmp/omarchy-windows-vm"
 
+# A host VM leaves its anchors mounted at exactly the production anchor paths
+# this test re-creates for its fixture uid. The tmpfs below hides them from the
+# filesystem, but the user-ns copy of the mount table keeps them listed in
+# /proc/self/mountinfo — and they are MNT_LOCKED, so they cannot be detached —
+# while mountpoint(1) matches entries by path: the fresh anchors then look like
+# existing mounts and prepare_mount_anchor short-circuits without creating
+# them. Pick fixture uids whose anchor paths cannot collide with whatever the
+# host VM left behind.
+TEST_UID=4242
+TEST_UID_OTHER=$((TEST_UID + 1))
+while grep -qE "mounts/users/(${TEST_UID}|${TEST_UID_OTHER})/(storage|shared) " /proc/self/mountinfo; do
+  TEST_UID=$((TEST_UID + 2))
+  TEST_UID_OTHER=$((TEST_UID + 1))
+done
+export TEST_UID TEST_UID_OTHER
+
 # Hide host state before creating the production paths used by the root helper.
 mount -t tmpfs -o mode=0755,size=8m run-test /run
 mkdir -p /run/lock
@@ -42,8 +58,8 @@ stat() {
 
 TEST_PASSWD_HOME=/home/alice
 getent() {
-  if [[ $1 == passwd && ${2:-} == 1000 ]]; then
-    printf 'alice:x:1000:1000::%s:/bin/bash\n' "$TEST_PASSWD_HOME"
+  if [[ $1 == passwd && ${2:-} == ${TEST_UID} ]]; then
+    printf "alice:x:${TEST_UID}:${TEST_UID}::%s:/bin/bash\n" "$TEST_PASSWD_HOME"
     return 0
   fi
   return 2
@@ -63,22 +79,22 @@ assert_no_runtime_mutation "zero PKEXEC_UID"
 PKEXEC_UID=not-a-number
 resolve_caller 2>/dev/null && fail "root accepted nonnumeric PKEXEC_UID"
 assert_no_runtime_mutation "nonnumeric PKEXEC_UID"
-PKEXEC_UID=1001
+PKEXEC_UID=${TEST_UID_OTHER}
 resolve_caller 2>/dev/null && fail "root accepted uid absent from passwd"
 assert_no_runtime_mutation "missing passwd entry"
 
-PKEXEC_UID=1000
+PKEXEC_UID=${TEST_UID}
 resolve_caller 2>/dev/null && fail "root accepted a home not owned by caller"
 assert_no_runtime_mutation "wrong-owned home"
-chown 1000:1000 /home/alice
+chown ${TEST_UID}:${TEST_UID} /home/alice
 
 chmod 0777 /home
 resolve_caller 2>/dev/null && fail "root accepted writable home parent"
 assert_no_runtime_mutation "writable parent"
 chmod 0755 /home
 
 mkdir /home/real-alice
-chown 1000:1000 /home/real-alice
+chown ${TEST_UID}:${TEST_UID} /home/real-alice
 ln -s /home/real-alice /home/link-alice
 TEST_PASSWD_HOME=/home/link-alice
 resolve_caller 2>/dev/null && fail "root accepted symlinked passwd home"
@@ -90,14 +106,14 @@ pass "root dispatch rejects missing/invalid uid, passwd, owner, symlink, and wri
 # Put each familiar source on its own filesystem. Both start with legacy 0755
 # permissions and world-readable payloads to prove migration hardens the leaves.
 mkdir /home/storage-target /home/shared-target
-mount -t tmpfs -o uid=1000,gid=1000,mode=0755,size=3g storage-test /home/storage-target
-mount -t tmpfs -o uid=1000,gid=1000,mode=0755,size=64m shared-test /home/shared-target
+mount -t tmpfs -o uid=${TEST_UID},gid=${TEST_UID},mode=0755,size=3g storage-test /home/storage-target
+mount -t tmpfs -o uid=${TEST_UID},gid=${TEST_UID},mode=0755,size=64m shared-test /home/shared-target
 ln -s /home/storage-target /home/alice/.windows
 ln -s /home/shared-target /home/alice/Windows
-chown -h 1000:1000 /home/alice/.windows /home/alice/Windows
+chown -h ${TEST_UID}:${TEST_UID} /home/alice/.windows /home/alice/Windows
 printf disk >/home/storage-target/disk.img
 printf shared >/home/shared-target/shared.txt
-chown 1000:1000 /home/storage-target/disk.img /home/shared-target/shared.txt
+chown ${TEST_UID}:${TEST_UID} /home/storage-target/disk.img /home/shared-target/shared.txt
 chmod 0644 /home/storage-target/disk.img /home/shared-target/shared.txt
 
 home_dev=$(command stat -Lc '%d' /home/alice)
@@ -113,16 +129,71 @@ resolve_caller
 [[ $(command stat -Lc '%d' "$CALLER_DATA_ROOT") != "$storage_dev" ]] || fail "Docker boundary unexpectedly shares the storage filesystem"
 [[ $(command stat -Lc '%u:%a' "$MOUNT_ROOT") == 0:711 &&
   $(command stat -Lc '%u:%a' "$CALLER_DATA_ROOT") == 0:711 ]] || fail "production ancestors are not root-owned/private-boundary modes"
-[[ $(command stat -Lc '%u:%a' "$EXPECTED_STORAGE") == 1000:700 &&
-  $(command stat -Lc '%u:%a' "$EXPECTED_SHARED") == 1000:700 ]] || fail "migrated leaves are not caller-owned 0700"
-if setpriv --reuid=1001 --regid=1001 --clear-groups cat "$EXPECTED_STORAGE/disk.img" >/dev/null 2>&1; then
+[[ $(command stat -Lc '%u:%a' "$EXPECTED_STORAGE") == ${TEST_UID}:700 &&
+  $(command stat -Lc '%u:%a' "$EXPECTED_SHARED") == ${TEST_UID}:700 ]] || fail "migrated leaves are not caller-owned 0700"
+if setpriv --reuid=${TEST_UID_OTHER} --regid=${TEST_UID_OTHER} --clear-groups cat "$EXPECTED_STORAGE/disk.img" >/dev/null 2>&1; then
   fail "another local account read the VM disk through its anchor"
 fi
-if setpriv --reuid=1001 --regid=1001 --clear-groups cat "$EXPECTED_SHARED/shared.txt" >/dev/null 2>&1; then
+if setpriv --reuid=${TEST_UID_OTHER} --regid=${TEST_UID_OTHER} --clear-groups cat "$EXPECTED_SHARED/shared.txt" >/dev/null 2>&1; then
   fail "another local account read shared files through their anchor"
 fi
 pass "cross-filesystem symlink sources bind by identity and migrated 0700 leaves deny another account"
 
+# GNU chmod 0700 leaves directory setgid; leftover g+s used to fail the
+# exact-700 check and block every privileged action (omacom/omarchy#9698).
+chmod 2700 /home/storage-target
+chmod 2777 /home/shared-target
+chown ${TEST_UID}:${TEST_UID} /home/storage-target /home/shared-target
+with_vm_lock prepare_caller_mounts || fail "root could not harden setgid VM source directories"
+[[ $(command stat -Lc '%a' /home/storage-target) == 700 ]] || fail "storage still had special bits after hardening"
+[[ $(command stat -Lc '%a' /home/shared-target) == 700 ]] || fail "shared still had special bits after hardening"
+[[ $(command stat -Lc '%u:%a' "$EXPECTED_STORAGE") == ${TEST_UID}:700 &&
+  $(command stat -Lc '%u:%a' "$EXPECTED_SHARED") == ${TEST_UID}:700 ]] || fail "setgid hardening did not leave caller-owned 0700 anchors"
+pass "setgid VM source directories harden to exactly 700"
+
+# Stop-time restore must cover every caller shape. Root walks the mounts tree
+# for a direct `sudo ... stop`; a sudoless-Docker stop is unelevated, where the
+# 0711 tree is not listable, and must restore the caller's own anchor
+# owner-side instead. A second account's anchor is never another user's to
+# chmod, and nothing runs through a non-standard privileged runtime path.
+mkdir -p "$USERS_DIR/${TEST_UID_OTHER}/shared" "$test_tmp/mounts/users/${TEST_UID}/shared"
+chmod 2777 "$USERS_DIR/${TEST_UID_OTHER}/shared" "$EXPECTED_SHARED" "$test_tmp/mounts/users/${TEST_UID}/shared"
+RUNTIME_DIR=$test_tmp restore_all_shared_privacy
+[[ $(command stat -Lc '%a' "$EXPECTED_SHARED") == 2777 &&
+  $(command stat -Lc '%a' "$USERS_DIR/${TEST_UID_OTHER}/shared") == 2777 &&
+  $(command stat -Lc '%a' "$test_tmp/mounts/users/${TEST_UID}/shared") == 2777 ]] ||
+  fail "root restored anchors through a non-standard runtime path"
+install -d -m 0755 /var/vm-test-bin
+install -m 0644 "$test_tmp/omarchy-windows-vm" /var/vm-test-bin/omarchy-windows-vm
+child_rc=0
+setpriv --reuid=${TEST_UID} --regid=${TEST_UID} --clear-groups bash -c '
+  # The namespace has no passwd entry for uid ${TEST_UID}, so mirror the stub above.
+  getent() {
+    if [[ $1 == passwd && $2 == ${TEST_UID} ]]; then
+      printf "alice:x:${TEST_UID}:${TEST_UID}::/home/alice:/bin/bash\n"
+      return 0
+    fi
+    return 2
+  }
+  set -- help
+  source /var/vm-test-bin/omarchy-windows-vm >/dev/null 2>&1
+  resolve_caller || exit 10
+  restore_all_shared_privacy || exit 11
+  [[ $(command stat -Lc "%a" "$USERS_DIR/${TEST_UID}/shared") == 2777 ]] || exit 12
+  [[ $(command stat -Lc "%a" "$USERS_DIR/${TEST_UID_OTHER}/shared") == 2777 ]] || exit 13
+  restore_shared_privacy || exit 14
+  [[ $(command stat -Lc "%a" "$USERS_DIR/${TEST_UID}/shared") == 700 ]] || exit 15
+  [[ $(command stat -Lc "%a" "$USERS_DIR/${TEST_UID_OTHER}/shared") == 2777 ]] || exit 16
+' || child_rc=$?
+[[ $child_rc == 0 ]] || fail "sudoless stop restore misbehaved (rc=$child_rc)"
+chmod 2777 "$EXPECTED_SHARED"
+restore_all_shared_privacy
+[[ $(command stat -Lc '%a' "$EXPECTED_SHARED") == 700 &&
+  $(command stat -Lc '%a' "$USERS_DIR/${TEST_UID_OTHER}/shared") == 700 ]] ||
+  fail "root walk did not restore every anchor to 700"
+rm -rf /var/vm-test-bin
+pass "stop-time restore walks the tree as root and restores only the caller's anchor unprivileged"
+
 # Existing production boundary components are never repaired in place when
 # their ownership or write permissions are unsafe. Both the preparation path
 # and the final pre-Docker guard must fail closed without disturbing the binds.
@@ -132,10 +203,10 @@ mounts_ready 2>/dev/null && fail "final guard accepted a group-writable mount bo
 [[ $(command stat -Lc '%a' "$MOUNT_ROOT") == 731 ]] || fail "rejection unexpectedly changed the writable boundary"
 chmod 0711 "$MOUNT_ROOT"
 
-chown 1000:1000 "$USERS_DIR"
+chown ${TEST_UID}:${TEST_UID} "$USERS_DIR"
 with_vm_lock prepare_caller_mounts 2>/dev/null && fail "root repaired a caller-owned mount boundary instead of rejecting it"
 mounts_ready 2>/dev/null && fail "final guard accepted a caller-owned mount boundary"
-[[ $(command stat -Lc '%u' "$USERS_DIR") == 1000 ]] || fail "rejection unexpectedly changed the boundary owner"
+[[ $(command stat -Lc '%u' "$USERS_DIR") == ${TEST_UID} ]] || fail "rejection unexpectedly changed the boundary owner"
 chown root:root "$USERS_DIR"
 
 [[ $(mount_layer_count "$EXPECTED_STORAGE") == 1 &&
@@ -182,7 +253,7 @@ umount "$EXPECTED_SHARED"
 umount "$EXPECTED_STORAGE"
 rm /home/alice/Windows
 ln -s / /home/alice/Windows
-chown -h 1000:1000 /home/alice/Windows
+chown -h ${TEST_UID}:${TEST_UID} /home/alice/Windows
 with_vm_lock prepare_caller_mounts 2>/dev/null && fail "root accepted a non-caller-owned second source"
 [[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "failed second-source preflight left a partial bind"
 [[ $(readlink /home/alice/Windows) == / ]] || fail "failed preflight consumed or quarantined symlink"
@@ -192,7 +263,7 @@ pass "root preflights both sources before mounting either and preserves rejectio
 # root-planted anchor symlink to the expected mounted source is rejected.
 rm /home/alice/Windows
 ln -s /home/shared-target /home/alice/Windows
-chown -h 1000:1000 /home/alice/Windows
+chown -h ${TEST_UID}:${TEST_UID} /home/alice/Windows
 rmdir "$EXPECTED_STORAGE"
 ln -s /home/storage-target "$EXPECTED_STORAGE"
 storage_id=$(command stat -Lc '%d:%i' /home/storage-target)