From e90f49a2692e94e8b11be0263093547e1a74abd6 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 15 Oct 2026 21:20:28 +0000 Subject: [PATCH] Treat explicit connections to the verified default daemon as local (#54421) ## Why Explicitly selecting the default daemon socket previously classified the TUI connection as remote, changing how local configuration and thread parameters were handled. A socket at that path may also be an SSH forward, so the path alone cannot establish that the daemon is local. ## What changed - Verify the default daemon endpoint before classifying an explicit connection as local. On Unix, check the expected symlink target and the private socket directory's ownership and permissions, then connect to the protected endpoint directly. - Preserve local workspace behavior and local thread parameters for verified attachments while keeping directly bound Unix sockets classified as remote. - Report a missing explicitly requested socket, disable embedded fallback for explicit attachments, and restrict daemon auto-start to implicit connections. ## Testing Extend tests to cover explicit and implicit local target selection, verified socket resolution, and rejection of a directly bound socket as a local daemon endpoint. GitOrigin-RevId: ac2226ca1c5f60b26aad9571f945e3325d3bebfa --- codex-rs/app-server-client/src/lib.rs | 1 + codex-rs/app-server-transport/src/lib.rs | 1 + .../app-server-transport/src/transport/mod.rs | 1 + .../src/transport/unix_socket.rs | 30 ++++++++++++ .../src/transport/unix_socket_tests.rs | 13 ++++++ codex-rs/app-server/src/lib.rs | 1 + codex-rs/app-server/src/transport.rs | 1 + codex-rs/tui/src/lib.rs | 46 ++++++++++++------- codex-rs/tui/src/startup_orchestration.rs | 39 ++++++++++++++-- 9 files changed, 111 insertions(+), 22 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 75544173d1..e24ce78add 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -40,6 +40,7 @@ pub use codex_app_server::in_process::InProcessServerEvent; use codex_app_server::in_process::InProcessStartArgs; use codex_app_server::in_process::LogDbLayer; pub use codex_app_server::in_process::StateDbHandle; +pub use codex_app_server::verified_local_daemon_socket; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::ClientNotification; use codex_app_server_protocol::ClientRequest; diff --git a/codex-rs/app-server-transport/src/lib.rs b/codex-rs/app-server-transport/src/lib.rs index 946b0be967..1f08704ad1 100644 --- a/codex-rs/app-server-transport/src/lib.rs +++ b/codex-rs/app-server-transport/src/lib.rs @@ -43,3 +43,4 @@ pub use transport::start_remote_control; pub use transport::start_stdio_connection; pub use transport::start_websocket_acceptor; pub use transport::take_remote_control_disabled_env; +pub use transport::verified_local_daemon_socket; diff --git a/codex-rs/app-server-transport/src/transport/mod.rs b/codex-rs/app-server-transport/src/transport/mod.rs index 8ec79543e9..cd7557810b 100644 --- a/codex-rs/app-server-transport/src/transport/mod.rs +++ b/codex-rs/app-server-transport/src/transport/mod.rs @@ -47,6 +47,7 @@ pub use unix_socket::DaemonSocketFileGuard; pub use unix_socket::acquire_app_server_startup_lock; pub use unix_socket::bind_private_daemon_socket; pub use unix_socket::start_control_socket_acceptor; +pub use unix_socket::verified_local_daemon_socket; pub use websocket::start_websocket_acceptor; const INTERNAL_ERROR_CODE: i64 = -32603; diff --git a/codex-rs/app-server-transport/src/transport/unix_socket.rs b/codex-rs/app-server-transport/src/transport/unix_socket.rs index 89a2cace33..ae53c12db5 100644 --- a/codex-rs/app-server-transport/src/transport/unix_socket.rs +++ b/codex-rs/app-server-transport/src/transport/unix_socket.rs @@ -296,6 +296,36 @@ async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { tokio::fs::remove_file(socket_path).await } +/// Return a verified local socket endpoint, independent of the advertised Unix alias. +/// A socket bound directly at the advertised path (e.g. SSH forwarding) is not ours on Unix. +pub fn verified_local_daemon_socket(socket_path: &Path) -> IoResult> { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + + let protected = protected_socket_path(socket_path)?; + if std::fs::read_link(socket_path).ok().as_deref() != Some(protected.as_path()) { + return Ok(None); + } + let directory = std::fs::symlink_metadata(codex_uds::shared_daemon_socket_directory()?)?; + // Match the daemon's bind-time check without creating or repairing the directory. + if directory.is_dir() + && directory.uid() == unsafe { libc::geteuid() } + && directory.mode() & 0o777 == 0o700 + { + return AbsolutePathBuf::from_absolute_path(protected).map(Some); + } + Ok(None) + } + #[cfg(windows)] + { + // The TUI revalidates this directory and verifies the peer at connection time, + // using the same connect_local_daemon path as implicit startup. + let (path, _directory) = codex_uds::validate_private_socket_path(socket_path)?; + AbsolutePathBuf::from_absolute_path_checked(path).map(Some) + } +} + #[cfg(unix)] fn protected_socket_path(rendezvous_path: &Path) -> IoResult { use sha2::Digest; diff --git a/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs b/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs index 3172bb2e38..43dbee22f4 100644 --- a/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs +++ b/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs @@ -411,6 +411,10 @@ async fn control_socket_file_is_private_after_bind() { 0o755 ); let physical_path = std::fs::read_link(socket_path.as_path()).expect("rendezvous symlink"); + assert_eq!( + super::verified_local_daemon_socket(socket_path.as_path()).unwrap(), + Some(AbsolutePathBuf::from_absolute_path(physical_path.clone()).unwrap()) + ); assert_eq!( physical_path.parent(), Some( @@ -427,6 +431,15 @@ async fn control_socket_file_is_private_after_bind() { assert!(!physical_path.with_extension("lock").exists()); assert!(std::fs::symlink_metadata(socket_path.as_path()).is_err()); + // SSH/socat may bind directly at the same advertised path. + let forwarded = tokio::net::UnixListener::bind(socket_path.as_path()).unwrap(); + assert_eq!( + super::verified_local_daemon_socket(socket_path.as_path()).unwrap(), + None + ); + drop(forwarded); + std::fs::remove_file(socket_path.as_path()).unwrap(); + // Simulate a dangling rendezvous left by an interrupted cleanup. std::os::unix::fs::symlink(&physical_path, socket_path.as_path()).unwrap(); let (sender, _receiver) = mpsc::channel::(CHANNEL_CAPACITY); diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 9ef1b73a82..d34ea297bd 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -153,6 +153,7 @@ pub use crate::transport::AppServerTransport; pub use crate::transport::RemoteControlStartupMode; pub use crate::transport::app_server_control_socket_path; pub use crate::transport::take_remote_control_disabled_env; +pub use crate::transport::verified_local_daemon_socket; const LOG_FORMAT_ENV_VAR: &str = "LOG_FORMAT"; const OTEL_SERVICE_NAME: &str = "codex-app-server"; diff --git a/codex-rs/app-server/src/transport.rs b/codex-rs/app-server/src/transport.rs index 51df5036ec..98aeeeac84 100644 --- a/codex-rs/app-server/src/transport.rs +++ b/codex-rs/app-server/src/transport.rs @@ -34,6 +34,7 @@ pub(crate) use codex_app_server_transport::start_remote_control; pub(crate) use codex_app_server_transport::start_stdio_connection; pub(crate) use codex_app_server_transport::start_websocket_acceptor; pub use codex_app_server_transport::take_remote_control_disabled_env; +pub use codex_app_server_transport::verified_local_daemon_socket; pub(crate) struct ConnectionState { pub(crate) origin: ConnectionOrigin, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index e586a61fa4..3366b4b0a7 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1041,6 +1041,14 @@ fn app_server_target_for_launch( return Ok(AppServerTarget::Embedded); } Ok(match explicit_remote_endpoint { + Some(RemoteAppServerEndpoint::UnixSocket { socket_path }) + if default_daemon_socket.as_ref() == Some(&socket_path) => + { + AppServerTarget::LocalDaemon { + endpoint: RemoteAppServerEndpoint::UnixSocket { socket_path }, + allow_embedded_fallback: false, + } + } Some(endpoint) => AppServerTarget::Remote { endpoint }, // A shared daemon cannot adopt this invocation's executor selection. None if can_reuse_implicit_local_daemon && exec_server_url.is_none() => { @@ -3198,23 +3206,27 @@ requires_openai_auth = {requires_openai_auth} fn app_server_target_for_launch_uses_local_daemon_for_default_socket() -> color_eyre::Result<()> { let socket_path = AbsolutePathBuf::relative_to_current_dir("codex.sock")?; - let target = app_server_target_for_launch( - /*explicit_remote_endpoint*/ None, - Some(socket_path.clone()), - /*can_reuse_implicit_local_daemon*/ true, - /*workload_identity_selected*/ false, - /*exec_server_url*/ None, - )?; - - assert_eq!( - target, - AppServerTarget::LocalDaemon { - allow_embedded_fallback: true, - endpoint: RemoteAppServerEndpoint::UnixSocket { socket_path }, - } - ); - assert!(!target.uses_remote_workspace()); - assert_eq!(target.thread_params_mode(), ThreadParamsMode::Embedded); + let endpoint = RemoteAppServerEndpoint::UnixSocket { + socket_path: socket_path.clone(), + }; + for explicit in [false, true] { + let target = app_server_target_for_launch( + explicit.then_some(endpoint.clone()), + Some(socket_path.clone()), + /*can_reuse_implicit_local_daemon*/ !explicit, + /*workload_identity_selected*/ false, + /*exec_server_url*/ None, + )?; + assert_eq!( + target, + AppServerTarget::LocalDaemon { + allow_embedded_fallback: !explicit, + endpoint: endpoint.clone(), + } + ); + assert!(!target.uses_remote_workspace()); + assert_eq!(target.thread_params_mode(), ThreadParamsMode::Embedded); + } Ok(()) } diff --git a/codex-rs/tui/src/startup_orchestration.rs b/codex-rs/tui/src/startup_orchestration.rs index ca871087c4..3995e3de35 100644 --- a/codex-rs/tui/src/startup_orchestration.rs +++ b/codex-rs/tui/src/startup_orchestration.rs @@ -11,7 +11,7 @@ pub(super) async fn run_main_inner( mut cli: Cli, arg0_paths: Arg0DispatchPaths, loader_overrides: LoaderOverrides, - explicit_remote_endpoint: Option, + mut explicit_remote_endpoint: Option, ) -> std::io::Result { if cli.no_daemon && explicit_remote_endpoint.is_some() { return Err(std::io::Error::other( @@ -106,6 +106,33 @@ pub(super) async fn run_main_inner( } }; + // A bound socket at the default path may be an SSH forward. Keep a verified + // endpoint so replacing the advertised alias cannot redirect local config. + let explicit_local_daemon_socket = match &explicit_remote_endpoint { + Some(RemoteAppServerEndpoint::UnixSocket { socket_path }) + if codex_app_server_client::app_server_control_socket_path(&codex_home) + .is_ok_and(|default_socket| default_socket == *socket_path) => + { + // An explicit attachment requires the server to be present. Do not + // classify a missing daemon as remote during its restart window. + if !socket_path.as_path().try_exists()? { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "the requested app-server socket is not available", + )); + } + codex_app_server_client::verified_local_daemon_socket(socket_path.as_path()) + .ok() + .flatten() + } + _ => None, + }; + if let Some(socket_path) = &explicit_local_daemon_socket { + explicit_remote_endpoint = Some(RemoteAppServerEndpoint::UnixSocket { + socket_path: socket_path.clone(), + }); + } + let mut launch_loader_overrides = loader_overrides.clone(); if let Some(profile_v2) = cli.config_profile_v2.as_ref() { let user_config_path = resolve_profile_v2_config_path(&codex_home, profile_v2); @@ -119,7 +146,7 @@ pub(super) async fn run_main_inner( if !std::io::stdin().is_terminal() || !std::io::stdout().is_terminal() { let validation_target = app_server_target_for_launch( explicit_remote_endpoint.clone(), - /*default_daemon_socket*/ None, + explicit_local_daemon_socket.clone(), /*can_reuse_implicit_local_daemon*/ false, workload_identity_selected, std::env::var_os(codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR).as_deref(), @@ -228,7 +255,7 @@ pub(super) async fn run_main_inner( // remote selection and the environment without opening a server connection. let presentation_target = app_server_target_for_launch( explicit_remote_endpoint.clone(), - /*default_daemon_socket*/ None, + explicit_local_daemon_socket.clone(), reuse_implicit_local_daemon, workload_identity_selected, std::env::var_os(codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR).as_deref(), @@ -311,12 +338,13 @@ pub(super) async fn run_main_inner( screen, )?; - let default_daemon = if explicit_remote_endpoint.is_none() && reuse_implicit_local_daemon { + let implicit_connection = explicit_remote_endpoint.is_none(); + let default_daemon = if implicit_connection && reuse_implicit_local_daemon { startup_draft .run_until(maybe_probe_default_daemon_socket(&codex_home)) .await? } else { - None + explicit_local_daemon_socket }; let mut app_server_target = app_server_target_for_launch( explicit_remote_endpoint, @@ -518,6 +546,7 @@ pub(super) async fn run_main_inner( None }; let auto_start_daemon = config.features.enabled(Feature::DaemonAutoStart) + && implicit_connection && !cli.agents_overview && !cli.no_daemon && !app_server_target.uses_remote_workspace();