fix(agent): apply the breeze group to the macOS IPC socket, not just create it (#3137) - #3422
Conversation
…create it On macOS the agent created a `breeze` group but never applied it to `/Library/Application Support/Breeze/agent.sock`, and never put any user in it. The socket therefore came up `srw-rw---- root:admin` on every daemon start: BSD semantics give a new file the *parent directory's* group, and the config dir is group `admin`. Mode 0660 + group `admin` means a Standard (non-admin) console user's desktop helper can never dial the socket, so `com.breeze.desktop-helper-user` crash-loops on `dial unix .../agent.sock: permission denied` (reporter observed runs≈133, last exit 1), which cascades into the TCC symptoms: Screen Recording reads Granted in System Settings while the helper reports screenRecording=false / captureGranted=false, and the loginwindow helper re-prompts endlessly. Admin users worked only incidentally, and the only workaround was `chmod 666` reapplied after every install, restart and reboot — setupSocket re-chmods to 0660 on every start. Both halves of the invariant are now enforced, and both self-heal: - **The socket is group-owned.** setupSocket resolves the `breeze` GID and chowns the socket to root:breeze before chmod 0660. Because this runs on every daemon start, the socket comes back correct after a reboot, after `launchctl kickstart -k system/com.breeze.agent`, and after a self-update — not just at first install. Deliberately NOT 0666: peer-credential + binary-path verification stays the real admission gate, but the socket must not be reachable by every local UID. - **The console user is a member.** The daemon adds every logged-in GUI user to `breeze` whenever it touches the helper LaunchAgents (login, session switch, post-update kickstart), always *before* any launchctl kickstart/bootstrap, since a helper inherits its group list at start. Both macOS installers do the same for users logged in at install time, mirroring the loop install-linux.sh has always had. Service accounts (uid < 500) and root are excluded. Supporting fixes: - The group is re-ensured on every daemon start, so a host installed by a build that predated the group — or where an admin removed it — recovers instead of staying broken forever. - `breeze-agent service install` created the group *after* bootstrapping the helper LaunchAgents; that ordering is reversed, and the group-creation shell script now has a single Go home (sessionbroker.EnsureIPCGroup) instead of being duplicated in agentapp. - A group that cannot be resolved logs a WARN naming the remedy and leaves the socket at 0660 rather than failing the broker. Losing the broker would take remote desktop, backup IPC and the watchdog link with it — strictly worse than the pre-fix root-only socket. - dscl lookups are bounded by a timeout; a wedged opendirectoryd cannot hold daemon startup. Usernames are validated before reaching a privileged argv. Linux had the same latent defect — the installer chowns /run/breeze to root:breeze but the socket itself was created root:root 0660 — and the same setupSocket chown fixes it. Tests: CI has no macOS runner (`test-agent` is ubuntu, `test-agent-windows` is windows), so a //go:build darwin test would run nowhere (#3019/#3046). The ownership decision, the dscl argv builders/parsers and the UID-eligibility predicate are pure and untagged, and the real chown/chmod syscalls are exercised against a real unix socket from a `!windows` file. The socket tests chown to a *supplementary* group so the assertion cannot pass vacuously, and were mutation-checked: forcing GID -1 fails them. Refs #3133, #3134, #3137 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
54260b9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96019c9e.breeze-9te.pages.dev |
| Branch Preview URL: | https://fix-3137-macos-sock-breeze-g.breeze-9te.pages.dev |
Addresses the three review passes on #3422. Lint (blocking): - Check the deferred listener Close() returns in the new socket tests (errcheck, 4 findings on the required Lint Agent (Go) job). Correctness / availability: - Split applySocketOwner's outcome into ChownErr and ChmodErr, which have different severities. A chown failure is now non-fatal — losing the group leaves a root-only socket (the pre-fix status quo), whereas failing the broker would take remote desktop, backup IPC and the watchdog link down with it, the exact tradeoff the lookup-failure path already reasoned about. A chmod failure stays fatal: net.Listen creates the socket at 0777&^umask, so an unapplied mode is the one outcome that could be MORE permissive than intended, and that must fail closed. The chmod is now attempted even after a failed chown, for the same reason. Silent failures: - Surface the membership read error that EnsureIPCGroupMember tolerates. The benign case (a freshly created group has no GroupMembership key) is indistinguishable here from a real read failure, so it is logged rather than discarded; in the benign case it fires at most once per host. - Verify the membership append in both macOS installers by re-reading the group instead of trusting dscl's exit status — the shell was claiming success on exactly the signal the Go path was hardened against trusting. - Warn when the config-dir chgrp fails instead of discarding it silently. Testability (the reviewers' highest-ROI gaps): - Extract the dscl orchestration into untagged, injectable functions (ensureGroupMemberViaDscl, lookupGroupIDViaDscl) so the six membership branches and the dscl-then-os/user fallback composition are table-tested on a REQUIRED job. The darwin files keep only the exec.Command calls. - Pin the install ordering that is the whole bug: installIPCPrereqsThenHelpers (untagged) enforces group -> members -> bootstrap helpers, with a test asserting the call order and that nothing proceeds when the group cannot be created. Previously that invariant was protected by prose alone. Comment accuracy (I had this wrong): - CI does have a macOS runner — test-agent-race on macos-latest. It is simply absent from ci-success's needs list, so it cannot block a merge. The rationale for keeping tests untagged is "the gating jobs are ubuntu/windows", not "no macOS runner exists". Also corrected: helper_group_test.go does NOT run on test-agent-windows, which names an explicit package list omitting internal/heartbeat (#2523). - Corrected the claim that `[ ... ] && continue` would abort under `set -e`. Verified empirically that it does not — set -e exempts a non-final command in an && list. The load-bearing guard is the `|| continue` on the id lookup, since a bare failing assignment does abort. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Review run: Findings: 9 raised → 8 addressed in Addressed:
Deferred (noted, not done in this PR):
Tests: full agent suite green — CI: all agent jobs green, including Mutation-checked, twice: forcing Status: review-clean, awaiting maintainer merge. One thing a maintainer must decide before promoting a build: the socket-ownership half is deterministic and covered by real-syscall tests, but membership taking effect for an already-logged-in user is not verified — macOS resolves group membership through |
Problem
On macOS the agent creates a
breezegroup but never applies it to the IPC socket, and never puts any user in it. So/Library/Application Support/Breeze/agent.sockcomes upsrw-rw---- root:adminon every daemon start — BSD semantics give a new file the parent directory's group, and the config dir is groupadmin.Mode
0660+ groupadminmeans a Standard (non-admin) console user's desktop helper can never dial the socket.com.breeze.desktop-helper-userthen crash-loops ondial unix ".../Breeze/agent.sock": permission denied(reporter observed runs≈133, last exit 1), which cascades into the TCC symptoms in the linked threads:screenRecording=false/captureGranted=false(ScreenCaptureKit timeout) — the probe runs in the helper's context, and the helper is dead.Admin users worked only incidentally (they happen to be in
admin). The only workaround waschmod 666on the socket, reapplied after every install, restart and reboot, sincesetupSocketre-chmods to0660on every start.The
breezegroup creation already existed in all three install paths (installer/macos/postinstall,scripts/install/install-darwin.sh,breeze-agent service install). Only the application of it was missing — the mirror-image ofscripts/install/install-linux.sh, which both chowns the IPC dir toroot:breezeand adds every logged-in user to the group.Fix
Both halves of the invariant are now enforced, and both self-heal.
1. The socket is group-owned.
sessionbroker.setupSocketresolves thebreezeGID and chowns the socket toroot:breezebeforechmod 0660. It runs on every daemon start, so the socket comes back correct after a reboot, afterlaunchctl kickstart -k system/com.breeze.agent, and after a self-update — the reporter's "sock likely recreated tight again after reboot" case.Deliberately not
0666. Peer-credential + binary-path verification (internal/ipc) stays the real admission gate, but a world-writable socket lets any local process connect and spend the broker's admission budget, and it is a footgun if the peer checks are ever loosened.TestIPCSocketModeIsNotWorldAccessiblepins this so a future "simpler fix" cannot land0666as shipped behaviour.2. The console user is a member. Group-owning the socket is useless alone: an admin installs the agent, a Standard user logs in later, and that user is in no group the socket grants. So:
breezewhenever it touches the helper LaunchAgents — login, session switch, post-update kickstart — viaensureDarwinHelperPrereqs. This covers users who first log in months after install, which the installers cannot.install-linux.shloop.launchctl kickstart/bootstrap: a helper inherits its group list when it starts._windowserverand friends.Supporting fixes
breeze-agent service installcreated the group after bootstrapping the helper LaunchAgents. Ordering reversed. The group-creation shell script now has one Go home (sessionbroker.EnsureIPCGroup) rather than being duplicated inagentapp.WARNnaming the remedy and leaves the socket at0660instead of failing the broker. Aborting would leave the agent with no IPC socket, taking remote desktop, backup IPC and the watchdog link with it — strictly worse than the pre-fix root-only socket.dsclcalls are bounded by a timeout, so a wedgedopendirectoryd(a real macOS failure mode) cannot hold daemon startup — and therefore the whole session broker — hostage. Usernames are validated before reaching a privileged argv.dsclGID lookup is Directory-Services-first, withos/useras fallback. This is load-bearing: release darwin binaries are builtCGO_ENABLED=0(scripts/build-edition.shdefaults it to 0), and cgo-lessos/userparses/etc/group, which never contains adscl-created group.os/useralone would have failed on exactly the shipped configuration.Linux had the same latent defect —
install-linux.shchowns/run/breezetoroot:breeze, but the socket itself was createdroot:root 0660, so non-root helpers were locked out there too. The samesetupSocketchown fixes it, andos/userresolves the group correctly from/etc/groupwithout cgo.Tests
CI has no macOS runner —
test-agentisubuntu-latest,test-agent-windowsiswindows-latest— so anything in a//go:build darwinfile compiles and runs nowhere (the #3019/#3046 lesson). The coverage is deliberately placed to actually run:internal/sessionbroker/ipc_group_test.go(untagged — runs on both the ubuntu and windows jobs): the ownership decision, the mode invariant, thedsclargv builders,PrimaryGroupID/GroupMembershipparsing (inline + dscl's folded output shape, missing group, missing key), whole-token membership matching (jingmust not matchjingxie), and username validation.internal/sessionbroker/ipc_group_unix_test.go(!windows— runs on the ubuntu job): the realchown/chmodsyscalls against a real unix socket viasetupSocket, covering the happy path, the missing-group path, parent-directory traversability, and a stale socket left by a killed daemon.internal/heartbeat/helper_group_test.go(untagged): the UID-eligibility predicate.Two things worth noting about the socket tests:
owner.GID = -1insetupSocket(pre-fix behaviour) failsTestSetupSocketAppliesGroupAndRestrictiveModeandTestSetupSocketReplacesAStaleSocket. Reverting makes them pass.Status
go test ./...(agent,CGO_ENABLED=0, CI mode): greengo test -raceoninternal/sessionbroker+internal/heartbeat: greengo vet+go buildfor darwin arm64/amd64, linux amd64, windows amd64: clean (the darwin-tagged files CI never compiles were cross-vetted explicitly)gofmtclean on every touched file;shellcheck -S warningclean on both modified installersNot verified here — needs a real Mac
I have no macOS host with the agent installed, so the end-to-end behaviour is not field-verified. Specifically:
agent.socklandsroot:breeze 0660and survives reboot/kickstart.opendirectoryd, and it is not certain a helper restarted mid-session picks up a group added after that GUI session began. Worst case it takes effect at the user's next login — still strictly better than today, where it never works at all, and no worse than achmodthat must be reapplied after every restart. A Standard-user login test on a real Mac is the thing to confirm before promoting the build.For @SemoTech
Once this ships in an agent build, please remove both workarounds and retest:
chmod 666onagent.sockafter install/restart — the socket should now besrw-rw---- root:breezeon its own, and stay that way across reboot andlaunchctl kickstart -k system/com.breeze.agent. Please confirm withls -l "/Library/Application Support/Breeze/agent.sock".com.breeze.desktop-helper-loginwindow.plist— the prompt loop was downstream of the dead user-session helper. (Note the separate point from the earlier thread: login-window WebRTC desktop control cannot work on macOS regardless, since Apple blocks synthetic input there without a private entitlement; that path falls back to the VNC relay.)The check that matters for a Standard (non-admin) user:
id -Gn <user>should listbreeze, and theircom.breeze.desktop-helper-userLaunchAgent should stop crash-looping. If a user who was already logged in when the agent upgraded still cannot connect, please log them out and back in and report whether that fixes it — that is exactly the uncertainty in point 2 above, and your answer decides whether a forced helper re-login is also needed.Refs #3133
Refs #3134
Refs #3137
(Not
Closes— these are Discord-relayed community reports, so they stay open until the reporter verifies on real hardware.)🤖 Generated with Claude Code