mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
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
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Option<AbsolutePathBuf>> {
|
||||
#[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<std::path::PathBuf> {
|
||||
use sha2::Digest;
|
||||
|
||||
@@ -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::<TransportEvent>(CHANNEL_CAPACITY);
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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,
|
||||
|
||||
+29
-17
@@ -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(())
|
||||
}
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ pub(super) async fn run_main_inner(
|
||||
mut cli: Cli,
|
||||
arg0_paths: Arg0DispatchPaths,
|
||||
loader_overrides: LoaderOverrides,
|
||||
explicit_remote_endpoint: Option<RemoteAppServerEndpoint>,
|
||||
mut explicit_remote_endpoint: Option<RemoteAppServerEndpoint>,
|
||||
) -> std::io::Result<AppExitInfo> {
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user