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,142 @@ 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
+}
+
+# Whether the VM container is gone, so no later samba start can flip the
+# share again. Only positive evidence ends the wait: a successful inspect
+# reporting anything but a live container (running, restarting, created, or
+# paused — a paused entrypoint resumes, so a flip may still come), or a
+# probe that positively reports no such container. Anything else — denied,
+# unreachable — keeps watching: exiting past a flip that may still come
+# would reopen the hole, while a lingering watcher is merely untidy. Status
+# and diagnostics travel separately: a warning on stderr must never poison
+# the stdout match and read a live container as gone.
+container_gone() {
+  local status err_file
+  err_file=$(mktemp) || return 1
+  status=$(docker inspect --format='{{.State.Status}}' "$CONTAINER" 2>"$err_file") || {
+    if grep -qE "No such object|No such container" "$err_file"; then
+      rm -f -- "$err_file"
+      return 0
+    fi
+    rm -f -- "$err_file"
+    return 1
+  }
+  rm -f -- "$err_file"
+  if [[ $status == "running" || $status == "restarting" || $status == "created" || $status == "paused" ]]; then
+    return 1
+  else
+    return 0
+  fi
+}
+
+# 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.
+watch_share_privacy() {
+  local dir="$1" i mode result
+  [[ -e $dir ]] || return 0
+  # Twice the documented download is a thin margin on a slow link, so budget
+  # hours rather than minutes — and stop depending on the clock alone. A
+  # download slower than any fixed budget would outlast the watcher while
+  # samba can still flip the share later, so leave only once the share is
+  # fixed, the container is gone, or the budget ends.
+  for i in {1..21600}; 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 ]] && return 0
+    fi
+    if ((i % 60 == 0)) && container_gone; then
+      break
+    fi
+    sleep 1
+  done
+  mode=$(stat -Lc '%a' "$dir" 2>/dev/null) || mode=""
+  if [[ $mode == 2777 || $mode == 777 ]]; then
+    chmod_private_dir "$dir" 2>/dev/null || true
+  fi
+  result=$(stat -Lc '%a' "$dir" 2>/dev/null) || result="missing"
+  logger -t omarchy-windows-vm \
+    "share privacy watcher exiting; $dir mode is $result" 2>/dev/null || true
+}
+
+schedule_share_privacy_restore() {
+  local dir="$HOME/Windows" self
+  [[ -e $dir ]] || return 0
+  self=$(readlink -f -- "${BASH_SOURCE[0]}") || self="${BASH_SOURCE[0]}"
+  # install runs inside omarchy-launch-floating-terminal-with-presentation,
+  # which is a uwsm-app systemd scope. Dismissing that window SIGTERMs leftover
+  # cgroup members; trap '' HUP does not cover that. A user unit is outside
+  # the scope, so the wait survives the terminal.
+  # Sourcing this file runs the dispatcher, so take the harmless help branch
+  # the way the shell tests do when sourcing it.
+  if omarchy-cmd-present systemd-run; then
+    systemctl --user reset-failed omarchy-windows-share-privacy.service 2>/dev/null || true
+    systemctl --user stop omarchy-windows-share-privacy.service 2>/dev/null || true
+    # Stash the service arguments first: `set -- help` would otherwise
+    # clobber them, sourcing "help" instead of the helper and starting the
+    # watcher with an empty directory (silently watching nothing while the
+    # successful unit launch skips the fallback below).
+    if systemd-run --user --quiet --collect \
+      --unit=omarchy-windows-share-privacy \
+      --description="Restore ~/Windows mode after dockur samba.sh" \
+      /bin/bash -c 'helper=$1; dir=$2; set -- help; source "$helper" >/dev/null; watch_share_privacy "$dir"' \
+      watcher "$self" "$dir"; then
+      return 0
+    fi
+  fi
+  # No user bus (tests, a stripped session): ignore HUP/TERM so a closing
+  # terminal cannot kill the wait the way the scope would.
+  (
+    trap '' HUP TERM
+    watch_share_privacy "$dir"
+  ) >/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,9 +738,7 @@ 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.
-  # The mode is symbolic because a numeric chmod keeps setuid/setgid on a
-  # directory, and dockur marks an initially empty /shared setgid (2777).
-  chmod u=rwx,go=,a-s -- "/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
@@ -898,7 +1054,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_caller_mounts 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.
@@ -916,11 +1090,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
@@ -1018,7 +1196,7 @@ prepare_user_mount_sources() {
     echo "omarchy-windows-vm: storage and shared must be different directories" >&2
     return 1
   }
-  chmod u=rwx,go=,a-s -- "$storage" "$shared"
+  chmod_private_dir "$storage" "$shared" || return 1
 }
 
 storage_space_path() {
@@ -1061,7 +1239,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; }
@@ -1337,6 +1515,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!"
@@ -1412,6 +1591,11 @@ launch_windows() {
 
   echo "Starting Windows VM (this may prompt for authorization)..."
   if ! priv up_wait; then
+    # The container may still be coming up in the background (slow download,
+    # slow guest): the one-shot priv-side restore in up_wait cannot cover a
+    # samba flip that lands after this failure, so arm the owner-side watcher
+    # for it before reporting.
+    schedule_share_privacy_restore
     echo "❌ Failed to start Windows VM!"
     echo "   Try checking: omarchy-windows-vm status"
     omarchy-notification-send -u critical "Windows VM" "Failed to start Windows VM"