Repository navigation
fix(enrollment): do not open a browser from a system-mode daemon - #231
Conversation
The daemon always tried to open its sign-in landing page in a browser when it started an enrollment attempt. As a system service that is wrong, and on macOS not merely a no-op: `open` run by the root LaunchDaemon reaches the console session, and since root has no default-browser preference, LaunchServices starts Safari on the daemon's landing page, next to the user's own browser that the desktop app opens on the IdP. On Linux services it is the stuck `xdg-open` attempt the detached spawn works around. Open the browser only when the daemon runs in the user's session: in `--user` mode, or when started from a terminal (`sudo agentdesktop daemon` during development). A LaunchDaemon, Windows service, or systemd unit now leaves the sign-in to the desktop app and `agentdesktop enrollment`, which already publish the URL, as the device-identity doc describes. Fixes agentdesktop-dev#230 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the enrollment browser-policy change.
No open issues 🎉
What changed in this PR
Addresses #230 by preventing background system daemons from launching an enrollment browser, avoiding unexpected Safari windows on macOS. Desktop and CLI clients retain access to the sign-in URL.
Changes:
- Allows automatic browser opening only in user mode or when attached to a terminal.
- Passes that policy through device enrollment without changing device-authorization or LLM gateway sign-in.
- Adds browser-policy unit coverage and updates the enrollment documentation.
| File | Description |
|---|---|
docs/device-identity.md |
Documents system-service behavior and the macOS browser issue. |
crates/agent/src/remote.rs |
Passes the browser policy through enrollment sessions and retries. |
crates/agent/src/oidc.rs |
Defines and tests the policy and applies it to browser-based enrollment. |
crates/agent/src/daemon.rs |
Determines browser permission from the resolved user mode and terminal attachment. |
Review effort: Balanced
yuval-k
left a comment
There was a problem hiding this comment.
over lgtm; but what does the std::io::stdin().is_terminal() check adds here?
The terminal check let `sudo agentdesktop daemon` open a browser, but that daemon is root too: on macOS it starts Safari just like the LaunchDaemon, and on Linux browsers refuse to run as root. Key the decision on `--user` alone; a system-mode daemon started from a terminal still prints the sign-in URL. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FTtqGMz13mdY1RVG9di2jW
|
@yuval-k thanks for the review, and good catch! 🙏 That check was meant to keep the auto-open for I've dropped it: the daemon now opens the browser only in |
`main` has failed `Test and lint` since a33f11e. #241 added `auth_recovery_tests.rs`, which calls `run_session` with 10 arguments, and #231 added an `open_browser: bool` parameter to `run_session`. Each passed CI on its own; together they don't compile: ``` error[E0061]: this function takes 11 arguments but 10 arguments were supplied --> crates/agent/src/remote/auth_recovery_tests.rs:205:13 ``` This passes `false`. The flag only decides whether `oidc::enroll` launches a browser, so it doesn't change what these tests check, and a test should never open one. The test module is Linux-only (`cfg(all(test, target_os = "linux"))`), so the break doesn't show up in a macOS build. It also turns `Test and lint` red on every open PR, since PR checks build against `main`.
Fixes #230.
Problem
oidc::wait_for_authorization_codealways passedopen_browser = true, so every enrollment attempt calledopen::that_detachedon the daemon's landing page, regardless of how the daemon runs.docs/device-identity.mdsays a system service "cannot open a browser; its attempt to do so fails and is only logged", but on macOS it does not fail:openrun by the root LaunchDaemon reaches the console session, and because root has no default-browser preference, LaunchServices starts Safari onhttp://127.0.0.1:51327/("Continue to sign in"). The user then has an unexpected Safari window on the landing page next to their own browser, which the desktop app's Enroll device opens straight on the IdP. On Linux services it is the stuckxdg-openattempt that the detached spawn comment already works around.Change
oidc::opens_browser(user_mode, attached_to_terminal): the daemon opens the browser only when it runs in the user's session, i.e.--usermode or started from a terminal (sudo agentdesktop daemon ...during development).daemon.rsevaluates it once from the resolveduserflag andstdin.is_terminal()and passes it throughremote::run→run_session→oidc::enroll→wait_for_authorization_code./v1/enrollmentfor the desktop app andagentdesktop enrollment.docs/device-identity.md: step 4 of the browser flow now describes the behaviour (and why the old attempt was not harmless on macOS).Testing
cargo clippy -p agentdesktop-agent --all-targets -- -D warnings,cargo fmt --all --checkcargo test -p agentdesktop-agent --lib -- oidc::tests remote::tests(48 passed, including the newonly_a_daemon_in_the_user_session_opens_a_browser)open_browserno longer being set for the LaunchDaemon (no--user, stdin is/dev/null).