Repository navigation
Clear set-ID bits when securing the Windows VM mount sources - #9605
omarchybot wants to merge 1 commit into
Conversation
`chmod 0700` does not clear S_ISGID on a directory: the kernel preserves it whenever the caller is in the file's group or holds CAP_FSETID, which root always does. A `~/Windows` or `~/.windows` carrying the setgid bit therefore stayed 2700 through the privacy step, and the exact `!= 700` check that follows rejected a source the same function had just been asked to secure — aborting launch and install with no message on stderr. Symbolic modes clear the set-ID bits, so the check now sees the mode the chmod was meant to produce. The mode-verification branch gains a diagnostic, because it is reachable for reasons a user can act on and previously returned silently. Co-Authored-By: Codex XHigh <codex@openai.com>
|
+1 from an affected user — 4.0.3-1, and the analysis here matches what I found independently on my machine. A few details that might be useful in the thread.
The install path breaks the same way, not just launch. On the numeric-mode behaviour, for anyone still wondering whether it's the kernel or the filesystem: it's coreutils. 9.11 here, package checksums clean. Against an existing 2700 directory: The raw syscall clears it, so it's the four-digits-or-fewer rule for directories — nothing btrfs-specific. Same applies to the other two Lastly, the silent |
|
Reviewed at Why #12323 over this one. Both make the same change at the two sites that matter,
Other open fixes for the same bug: #9783 rewrites far more than this bug needs. #12019 adds no tests. #13154 leaves the user-side Second opinion: Codex at medium effort was given all seven pull requests without our view and asked to pick one. It also picked #12323, for the same reasons. It read the same tree as this review, so that is agreement, not independent confirmation. This now waits on the maintainer. The recommendation is to merge #12323 and close this one in its favour. |
|
Closing this alternative after #12323 merged into 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 |
Closes #9374. Closes #9540. Closes #9567.
omarchy-windows-vm launchandinstallabort — the first with❌ Failed to start Windows VM!, the second with❌ Failed to write the Windows VM configuration.— when~/Windowsor~/.windowscarries the setgid bit. Nothing is written to stderr, sostatusreporting RUNNING and a working web console leave no way to tell what refused.prepare_caller_mounts()hardens both pinned sources withchmod 0700and then assertsstat -Lc '%a'is exactly700.chmodwith a numeric mode does not clearS_ISGIDon a directory: the kernel preserves it whenever the caller is in the file's group or holdsCAP_FSETID, which root always does. A directory at2700stays2700,chmodstill returns 0, and the!= 700branch rejects a source the same function had just been asked to secure — then returns 1 without a message.Symbolic modes clear the set-ID bits, so the check now sees the mode the chmod was meant to produce. The same substitution applies at the two other sites that make these directories private:
prepare_user_mount_sources(), which prepares the very directories the elevated path re-checks, andwrite_credentials(), where a set-ID bit would hand the directory holding the plaintext RDP password a group it was never meant to have.The mode-verification branch gains a diagnostic. It stays reachable for reasons a user can act on, and returning silently is what made this take three separate reports to characterise.
Reviewed by Codex at xhigh, which confirmed the mode arithmetic across every set-ID combination and pointed out that the existing suites start these directories at
0755and so pass against the broken chmod too; the regression cases added here start them at2700/6700and fail without the fix.