From 76b032aabcdce3454bd20402f39283635041db72 Mon Sep 17 00:00:00 2001 From: "ismet.balat" Date: Sun, 8 Nov 2026 18:57:21 +0300 Subject: [PATCH 1/2] fix(enrollment): do not open a browser from a system-mode daemon 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 #230 Co-Authored-By: Claude Fable 5.1 --- crates/agent/src/daemon.rs | 7 ++++++- crates/agent/src/oidc.rs | 32 ++++++++++++++++++++++++++++++-- crates/agent/src/remote.rs | 5 ++++- docs/device-identity.md | 12 +++++++----- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/crates/agent/src/daemon.rs b/crates/agent/src/daemon.rs index 766c0a2d..0658612f 100644 --- a/crates/agent/src/daemon.rs +++ b/crates/agent/src/daemon.rs @@ -1,5 +1,6 @@ use std::{ future::Future, + io::IsTerminal, net::SocketAddr, path::{Path, PathBuf}, sync::Arc, @@ -38,7 +39,7 @@ use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use crate::windows_security::SecurityDescriptor; use crate::{ api, enrollment::EnrollmentState, gateway_oidc, inventory_watch::InventoryWatch, llm_proxy, - reconcile, remote, secure_fs, + oidc, reconcile, remote, secure_fs, }; #[cfg(unix)] @@ -609,12 +610,16 @@ where let remote_discovery = inventory.clone(); let state_dir = args.state_dir.clone(); let oidc_callback_listen = args.oidc_callback_listen; + // A system service has no user session to open a browser in; the + // desktop app opens the sign-in URL there (see oidc::opens_browser). + let open_browser = oidc::opens_browser(args.user, std::io::stdin().is_terminal()); let remote_enrollment = enrollment.clone(); tokio::spawn(remote::run( controller, remote_discovery, state_dir, oidc_callback_listen, + open_browser, reconciler, remote_enrollment, controller_status.clone(), diff --git a/crates/agent/src/oidc.rs b/crates/agent/src/oidc.rs index 253593c8..25d8b3d6 100644 --- a/crates/agent/src/oidc.rs +++ b/crates/agent/src/oidc.rs @@ -285,10 +285,26 @@ pub(crate) struct TokenResponse { pub(crate) token_type: String, } +/// Whether the daemon may open the sign-in page in a browser itself. +/// +/// A system service (LaunchDaemon, Windows service, systemd) runs outside any +/// user session, so the desktop app opens the URL published on +/// `/v1/enrollment` instead. Trying from the service is not harmless: on macOS +/// `open` run by the root daemon still reaches the console session, but it +/// starts Safari (root has no default browser) on the daemon's landing page +/// rather than the user's browser, and on Linux `xdg-open` has no display. +/// The daemon opens the browser only when it runs in the user's session: +/// `--user` mode, or started from a terminal (for example +/// `sudo agentdesktop daemon` during development). +pub(crate) fn opens_browser(user_mode: bool, attached_to_terminal: bool) -> bool { + user_mode || attached_to_terminal +} + pub async fn enroll( controller: &ControllerConnectionConfig, enrollment: &EnrollmentState, callback_listen: Option, + open_browser: bool, ) -> anyhow::Result { if controller.device_authorization { return enroll_with_device_authorization(controller, enrollment).await; @@ -315,6 +331,7 @@ pub async fn enroll( &redirect_uri, std::mem::take(&mut begin.state), callback_listen, + open_browser, ) .await?; enrollment.set("enrolling").await; @@ -631,6 +648,7 @@ pub(crate) async fn wait_for_authorization_code( redirect_uri: &Url, expected_state: String, callback_listen: Option, + open_browser: bool, ) -> anyhow::Result { wait_for_authorization_code_with_page( authorization_url, @@ -641,7 +659,7 @@ pub(crate) async fn wait_for_authorization_code( subscription_available: false, github_available: false, }, - true, + open_browser, ) .await? .context("required authorization was skipped") @@ -956,10 +974,20 @@ mod tests { use super::{ AuthorizationPage, DevicePoll, TokenResponse, apply_refreshed_tokens, - classify_device_token_error, continue_subscription_page, poll_device_token, + classify_device_token_error, continue_subscription_page, opens_browser, poll_device_token, }; use crate::identity::{Identity, OAuthCredentials}; + // A LaunchDaemon, Windows service, or systemd unit has neither a user + // session nor a terminal: the desktop app opens the sign-in URL instead. + #[test] + fn only_a_daemon_in_the_user_session_opens_a_browser() { + assert!(!opens_browser(false, false)); + assert!(opens_browser(true, false)); + assert!(opens_browser(false, true)); + assert!(opens_browser(true, true)); + } + // The poll interval is provider-controlled and grows on every slow_down, so it can // exceed what is left of expires_in. Sleeping the whole interval kept the task, and // the pending login it stands for, alive long after the code had expired. diff --git a/crates/agent/src/remote.rs b/crates/agent/src/remote.rs index 23dfe192..3e00b4a7 100644 --- a/crates/agent/src/remote.rs +++ b/crates/agent/src/remote.rs @@ -184,6 +184,7 @@ pub async fn run( mut discovered: watch::Receiver>, state_dir: PathBuf, oidc_callback_listen: Option, + open_browser: bool, reconciler: Reconciler, enrollment: EnrollmentState, controller_status: ControllerConnectionState, @@ -207,6 +208,7 @@ pub async fn run( &mut discovered, &state_dir, oidc_callback_listen, + open_browser, &reconciler, &enrollment, &controller_status, @@ -304,6 +306,7 @@ async fn run_session( discovered: &mut watch::Receiver>, state_dir: &Path, oidc_callback_listen: Option, + open_browser: bool, reconciler: &Reconciler, enrollment: &EnrollmentState, controller_status: &ControllerConnectionState, @@ -338,7 +341,7 @@ async fn run_session( enrollment, logout, INITIAL_RETRY_DELAY, - || oidc::enroll(controller, enrollment, oidc_callback_listen), + || oidc::enroll(controller, enrollment, oidc_callback_listen, open_browser), ) .await; identity::save(&identity_path, &identity)?; diff --git a/docs/device-identity.md b/docs/device-identity.md index d1f3587d..f2ef2d79 100644 --- a/docs/device-identity.md +++ b/docs/device-identity.md @@ -218,11 +218,13 @@ native app (RFC 8252): authorization URL on `/v1/enrollment`. How the browser gets there depends on how the daemon runs: - **As a system service** (the normal install: LaunchDaemon, Windows - service, systemd), the daemon runs outside any user session and cannot - open a browser. Its attempt to do so fails and is only logged. The user - clicks sign-in in the desktop app, which reads the URL from - `/v1/enrollment` and opens it in the user's browser. The browser goes - straight to the IdP. + service, systemd), the daemon runs outside any user session and does not + try to open a browser (it is neither in `--user` mode nor attached to a + terminal). Such an attempt would not simply fail: on macOS, `open` run by + the root daemon starts Safari in the console session, on the daemon's + landing page, instead of the user's browser. The user clicks sign-in in + the desktop app, which reads the URL from `/v1/enrollment` and opens it + in the user's browser. The browser goes straight to the IdP. - **In the user's session** (`sudo` from a terminal, `--user` mode, development), the daemon opens the browser on its own landing page, and **Continue to sign in** redirects to the IdP. From 0ae10b74995a8065a25293f68081410cb42ecc33 Mon Sep 17 00:00:00 2001 From: "ismet.balat" Date: Thu, 12 Nov 2026 18:33:37 +0300 Subject: [PATCH 2/2] fix(enrollment): open the browser only in user mode 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 Claude-Session: https://claude.ai/code/session_01FTtqGMz13mdY1RVG9di2jW --- crates/agent/src/daemon.rs | 8 +++----- crates/agent/src/oidc.rs | 27 +-------------------------- docs/device-identity.md | 23 +++++++++++------------ 3 files changed, 15 insertions(+), 43 deletions(-) diff --git a/crates/agent/src/daemon.rs b/crates/agent/src/daemon.rs index 0658612f..cb5eb91a 100644 --- a/crates/agent/src/daemon.rs +++ b/crates/agent/src/daemon.rs @@ -1,6 +1,5 @@ use std::{ future::Future, - io::IsTerminal, net::SocketAddr, path::{Path, PathBuf}, sync::Arc, @@ -39,7 +38,7 @@ use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use crate::windows_security::SecurityDescriptor; use crate::{ api, enrollment::EnrollmentState, gateway_oidc, inventory_watch::InventoryWatch, llm_proxy, - oidc, reconcile, remote, secure_fs, + reconcile, remote, secure_fs, }; #[cfg(unix)] @@ -610,9 +609,8 @@ where let remote_discovery = inventory.clone(); let state_dir = args.state_dir.clone(); let oidc_callback_listen = args.oidc_callback_listen; - // A system service has no user session to open a browser in; the - // desktop app opens the sign-in URL there (see oidc::opens_browser). - let open_browser = oidc::opens_browser(args.user, std::io::stdin().is_terminal()); + // A system-mode daemon is root: its browser would not be the user's. + let open_browser = args.user; let remote_enrollment = enrollment.clone(); tokio::spawn(remote::run( controller, diff --git a/crates/agent/src/oidc.rs b/crates/agent/src/oidc.rs index 25d8b3d6..4dbf5211 100644 --- a/crates/agent/src/oidc.rs +++ b/crates/agent/src/oidc.rs @@ -285,21 +285,6 @@ pub(crate) struct TokenResponse { pub(crate) token_type: String, } -/// Whether the daemon may open the sign-in page in a browser itself. -/// -/// A system service (LaunchDaemon, Windows service, systemd) runs outside any -/// user session, so the desktop app opens the URL published on -/// `/v1/enrollment` instead. Trying from the service is not harmless: on macOS -/// `open` run by the root daemon still reaches the console session, but it -/// starts Safari (root has no default browser) on the daemon's landing page -/// rather than the user's browser, and on Linux `xdg-open` has no display. -/// The daemon opens the browser only when it runs in the user's session: -/// `--user` mode, or started from a terminal (for example -/// `sudo agentdesktop daemon` during development). -pub(crate) fn opens_browser(user_mode: bool, attached_to_terminal: bool) -> bool { - user_mode || attached_to_terminal -} - pub async fn enroll( controller: &ControllerConnectionConfig, enrollment: &EnrollmentState, @@ -974,20 +959,10 @@ mod tests { use super::{ AuthorizationPage, DevicePoll, TokenResponse, apply_refreshed_tokens, - classify_device_token_error, continue_subscription_page, opens_browser, poll_device_token, + classify_device_token_error, continue_subscription_page, poll_device_token, }; use crate::identity::{Identity, OAuthCredentials}; - // A LaunchDaemon, Windows service, or systemd unit has neither a user - // session nor a terminal: the desktop app opens the sign-in URL instead. - #[test] - fn only_a_daemon_in_the_user_session_opens_a_browser() { - assert!(!opens_browser(false, false)); - assert!(opens_browser(true, false)); - assert!(opens_browser(false, true)); - assert!(opens_browser(true, true)); - } - // The poll interval is provider-controlled and grows on every slow_down, so it can // exceed what is left of expires_in. Sleeping the whole interval kept the task, and // the pending login it stands for, alive long after the code had expired. diff --git a/docs/device-identity.md b/docs/device-identity.md index f2ef2d79..5e7920c8 100644 --- a/docs/device-identity.md +++ b/docs/device-identity.md @@ -72,13 +72,13 @@ sequenceDiagram Controller-->>Daemon: enrollment_id, authorization_url, state, redirect_uri, token_endpoint, client_id Daemon->>Daemon: Listen on loopback redirect_uri (default 127.0.0.1:51327) Daemon->>Daemon: Publish authorization_url on GET /v1/enrollment and wait up to 10 min - alt Daemon runs as a system service (normal install) - Note over Daemon: Cannot open a browser. It runs outside any user session. + alt Daemon runs in system mode (normal install) + Note over Daemon: Does not open a browser. It runs as root, outside the user's session. User->>App: Sign in App->>Daemon: GET /v1/enrollment Daemon-->>App: authorization_url App->>Browser: Open authorization_url - else Daemon runs in the user's session (sudo, user mode, development) + else Daemon runs in user mode (--user, development) Daemon->>Browser: Open the local sign-in page User->>Browser: Continue to sign in end @@ -217,17 +217,16 @@ native app (RFC 8252): (default `http://127.0.0.1:51327/callback`) and publishes the authorization URL on `/v1/enrollment`. How the browser gets there depends on how the daemon runs: - - **As a system service** (the normal install: LaunchDaemon, Windows - service, systemd), the daemon runs outside any user session and does not - try to open a browser (it is neither in `--user` mode nor attached to a - terminal). Such an attempt would not simply fail: on macOS, `open` run by - the root daemon starts Safari in the console session, on the daemon's - landing page, instead of the user's browser. The user clicks sign-in in + - **In system mode** (the normal install: LaunchDaemon, Windows service, + systemd; also `sudo agentdesktop daemon` from a terminal), the daemon + runs as root and does not open a browser. Such an attempt would not + simply fail: on macOS, `open` run by root starts Safari on the daemon's + landing page instead of the user's browser. The user clicks sign-in in the desktop app, which reads the URL from `/v1/enrollment` and opens it in the user's browser. The browser goes straight to the IdP. - - **In the user's session** (`sudo` from a terminal, `--user` mode, - development), the daemon opens the browser on its own landing page, and - **Continue to sign in** redirects to the IdP. + - **In user mode** (`--user`, development), the daemon opens the browser + on its own landing page, and **Continue to sign in** redirects to the + IdP. `agentdesktop enrollment` prints the same URL. 5. After the user authenticates, the IdP redirects to the loopback callback.