diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 8135a5cad1..eefc0fc9d9 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -36,7 +36,7 @@ When running with `--listen ws://IP:PORT`, the same listener also serves basic H Websocket transport is currently experimental and unsupported. Do not rely on it for production workloads. -Pass `--code-mode-host wss://HOST/PATH` to connect this app-server process to a remote code-mode host instead of starting a local host. This outbound connection is independent of `--listen` and is shared by the process's threads. Use `ws://` for a local code-mode host. +Pass `--code-mode-host URL` to connect this app-server process to a remote code-mode host instead of starting a local host. Use `ws://` or `wss://` for the WebSocket protocol, or a root `http://` or `https://` URL without a path or query for gRPC. Remote hosts require the `code_mode_host` feature. This outbound connection is independent of `--listen` and is shared by the process's threads. The unix socket transport is intended for local app-server control-plane clients. `codex app-server proxy` opens exactly one raw stream connection to `$CODEX_HOME/app-server-control/app-server-control.sock` diff --git a/codex-rs/app-server/src/code_mode_host.rs b/codex-rs/app-server/src/code_mode_host.rs index fb3c7e9139..6a0ceae5e9 100644 --- a/codex-rs/app-server/src/code_mode_host.rs +++ b/codex-rs/app-server/src/code_mode_host.rs @@ -1,4 +1,8 @@ +use std::ffi::OsStr; + use clap::Args; +use clap::builder::TypedValueParser; +use clap::error::ErrorKind; use url::Url; /// Selects the code-mode host for a single app-server process. @@ -7,8 +11,8 @@ pub struct AppServerCodeModeHostArgs { /// Connect to a remote code-mode host instead of starting a local host. #[arg( long = "code-mode-host", - value_name = "WS_URL", - value_parser = parse_websocket_url + value_name = "URL", + value_parser = RedactedHostUrlParser )] pub code_mode_host: Option, } @@ -21,25 +25,64 @@ pub enum CodeModeHostTransport { Local, /// Share a connection to the specified remote code-mode host. WebSocket(Url), + /// Share an HTTP/2 gRPC connection to the specified remote code-mode host. + Grpc(Url), } impl From for CodeModeHostTransport { fn from(args: AppServerCodeModeHostArgs) -> Self { match args.code_mode_host { + Some(url) if matches!(url.scheme(), "http" | "https") => Self::Grpc(url), Some(url) => Self::WebSocket(url), None => Self::Local, } } } -fn parse_websocket_url(value: &str) -> Result { - let url = Url::parse(value).map_err(|error| format!("invalid websocket URL: {error}"))?; - if !matches!(url.scheme(), "ws" | "wss") || url.host_str().is_none() { - return Err("code-mode host URL must use ws:// or wss:// with a host".to_string()); +#[derive(Clone)] +struct RedactedHostUrlParser; + +impl TypedValueParser for RedactedHostUrlParser { + type Value = Url; + + fn parse_ref( + &self, + command: &clap::Command, + _argument: Option<&clap::Arg>, + value: &OsStr, + ) -> Result { + let value = value.to_str().ok_or_else(|| { + clap::Error::raw( + ErrorKind::InvalidUtf8, + "code-mode host URL must contain valid UTF-8", + ) + .with_cmd(command) + })?; + + parse_host_url(value) + .map_err(|error| clap::Error::raw(ErrorKind::ValueValidation, error).with_cmd(command)) + } +} + +fn parse_host_url(value: &str) -> Result { + let url = Url::parse(value).map_err(|error| format!("invalid code-mode host URL: {error}"))?; + if !matches!(url.scheme(), "ws" | "wss" | "http" | "https") || url.host_str().is_none() { + return Err( + "code-mode host URL must use ws://, wss://, http://, or https:// with a host" + .to_string(), + ); } if url.fragment().is_some() { return Err("code-mode host URL must not contain a fragment".to_string()); } + if matches!(url.scheme(), "http" | "https") { + if !url.username().is_empty() || url.password().is_some() { + return Err("gRPC code-mode host URL must not contain credentials".to_string()); + } + if url.path() != "/" || url.query().is_some() { + return Err("gRPC code-mode host URL must not contain a path or query".to_string()); + } + } Ok(url) } diff --git a/codex-rs/app-server/src/code_mode_host_tests.rs b/codex-rs/app-server/src/code_mode_host_tests.rs index 400406a93c..6fe3c7f8e3 100644 --- a/codex-rs/app-server/src/code_mode_host_tests.rs +++ b/codex-rs/app-server/src/code_mode_host_tests.rs @@ -1,30 +1,63 @@ use super::AppServerCodeModeHostArgs; use super::CodeModeHostTransport; -use super::parse_websocket_url; +use super::parse_host_url; use pretty_assertions::assert_eq; use url::Url; #[test] fn websocket_host_accepts_local_and_secure_endpoints() { - for endpoint in ["ws://127.0.0.1:8765", "wss://example.test/code-mode"] { + for endpoint in [ + "ws://127.0.0.1:8765", + "wss://example.test/code-mode", + "ws://alice:secret@example.test/code-mode", + "wss://alice:secret@example.test/code-mode", + ] { assert_eq!( - parse_websocket_url(endpoint), + parse_host_url(endpoint), Ok(Url::parse(endpoint).expect("test endpoint should parse")) ); } } #[test] -fn websocket_host_rejects_invalid_endpoints() { +fn grpc_host_accepts_local_and_secure_endpoints() { + for endpoint in ["http://127.0.0.1:8765", "https://example.test"] { + assert_eq!( + parse_host_url(endpoint), + Ok(Url::parse(endpoint).expect("test endpoint should parse")) + ); + } +} + +#[test] +fn grpc_host_rejects_credentials_without_disclosing_them() { for endpoint in [ - "http://127.0.0.1:8765", - "https://example.test/code-mode", + "http://alice:secret@example.test", + "https://alice:secret@example.test", + "https://alice@example.test", + "https://:secret@example.test", + ] { + let error = parse_host_url(endpoint).expect_err("gRPC credentials should be rejected"); + + assert!(error.contains("must not contain credentials")); + assert!(!error.contains("alice")); + assert!(!error.contains("secret")); + } +} + +#[test] +fn code_mode_host_rejects_invalid_endpoints() { + for endpoint in [ + "ftp://127.0.0.1:8765", "ws://", - "not a websocket", + "not a host endpoint", "wss://example.test/code-mode#fragment", + "https://example.test/code-mode#fragment", + "https://example.test/code-mode", + "http://example.test/?token=secret", ] { assert!( - parse_websocket_url(endpoint).is_err(), + parse_host_url(endpoint).is_err(), "invalid code-mode host endpoint should be rejected: {endpoint}" ); } @@ -49,3 +82,15 @@ fn explicit_websocket_host_selects_remote_transport() { CodeModeHostTransport::WebSocket(url) ); } + +#[test] +fn explicit_grpc_host_selects_remote_transport() { + let url = Url::parse("https://example.test").expect("test endpoint should parse"); + + assert_eq!( + CodeModeHostTransport::from(AppServerCodeModeHostArgs { + code_mode_host: Some(url.clone()), + }), + CodeModeHostTransport::Grpc(url) + ); +} diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 7fef5cf838..934e00ee0a 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -3,6 +3,7 @@ use codex_arg0::Arg0DispatchPaths; use codex_code_mode::CodeModeSessionProvider; +use codex_code_mode::GrpcCodeModeSessionProvider; use codex_code_mode::WebSocketCodeModeSessionProvider; use codex_config::LoaderOverrides; use codex_config::NoopThreadConfigLoader; @@ -558,6 +559,20 @@ pub async fn run_main_with_transport_options( ), )) } + CodeModeHostTransport::Grpc(url) => { + if !config.features.enabled(Feature::CodeModeHost) { + return Err(std::io::Error::new( + ErrorKind::InvalidInput, + "remote code-mode host requires the code_mode_host feature to be enabled", + )); + } + Some(Arc::new( + GrpcCodeModeSessionProvider::with_http_client_factory( + url.to_string(), + config.http_client_factory(), + ), + )) + } }; let environment_manager = if ignore_user_config { EnvironmentManager::from_env(Some(local_runtime_paths), config.http_client_factory()).await diff --git a/codex-rs/app-server/src/main_tests.rs b/codex-rs/app-server/src/main_tests.rs index 9eb8d6fd53..3334fe9ac1 100644 --- a/codex-rs/app-server/src/main_tests.rs +++ b/codex-rs/app-server/src/main_tests.rs @@ -57,17 +57,42 @@ fn app_server_accepts_process_scoped_code_mode_host() { assert_eq!(args.config_overrides.raw_overrides, Vec::::new()); } +#[test] +fn app_server_accepts_process_scoped_grpc_code_mode_host() { + let args = AppServerArgs::try_parse_from([ + "codex-app-server", + "--code-mode-host", + "https://example.test", + "--listen", + "off", + ]) + .expect("parse gRPC app-server args"); + + assert_eq!( + args.code_mode_host.code_mode_host, + Some(Url::parse("https://example.test").expect("test endpoint should parse")) + ); + assert_eq!(args.listen, AppServerTransport::Off); +} + #[test] fn app_server_rejects_invalid_code_mode_host() { for endpoint in [ - "http://127.0.0.1:8765", + "ftp://127.0.0.1:8765", "ws://", "wss://example.test/code-mode#fragment", + "https://example.test/code-mode", + "http://alice:secret@example.test", + "https://alice:secret@example.test", + "http://example.test/?token=secret", ] { let error = AppServerArgs::try_parse_from(["codex-app-server", "--code-mode-host", endpoint]) .expect_err("invalid code-mode host endpoint should fail startup argument parsing"); assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation); + let rendered_error = error.to_string(); + assert!(!rendered_error.contains("alice")); + assert!(!rendered_error.contains("secret")); } } diff --git a/codex-rs/app-server/tests/suite/v2/code_mode_host.rs b/codex-rs/app-server/tests/suite/v2/code_mode_host.rs index 6cfa20492a..534c34878f 100644 --- a/codex-rs/app-server/tests/suite/v2/code_mode_host.rs +++ b/codex-rs/app-server/tests/suite/v2/code_mode_host.rs @@ -26,24 +26,33 @@ const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn app_server_shares_flag_selected_code_mode_host_across_threads() -> Result<()> { + assert_shared_remote_code_mode_host("ws://127.0.0.1:0").await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn app_server_shares_flag_selected_grpc_code_mode_host_across_threads() -> Result<()> { + assert_shared_remote_code_mode_host("grpc://127.0.0.1:0").await +} + +async fn assert_shared_remote_code_mode_host(listen_url: &str) -> Result<()> { let host_program = codex_utils_cargo_bin::cargo_bin("codex-code-mode-host")?; - let mut websocket_host = Command::new(host_program) - .args(["--listen", "ws://127.0.0.1:0"]) + let mut code_mode_host = Command::new(host_program) + .args(["--listen", listen_url]) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) .kill_on_drop(true) .spawn() - .context("failed to start websocket code-mode host")?; - let stdout = websocket_host + .context("failed to start remote code-mode host")?; + let stdout = code_mode_host .stdout .take() - .context("websocket code-mode host stdout was not captured")?; + .context("remote code-mode host stdout was not captured")?; let mut lines = BufReader::new(stdout).lines(); - let websocket_url = timeout(DEFAULT_READ_TIMEOUT, lines.next_line()) + let host_url = timeout(DEFAULT_READ_TIMEOUT, lines.next_line()) .await - .context("timed out waiting for websocket code-mode host URL")?? - .context("websocket code-mode host exited before publishing its URL")?; + .context("timed out waiting for remote code-mode host URL")?? + .context("remote code-mode host exited before publishing its URL")?; let model_server = responses::start_mock_server().await; let response_mock = responses::mount_sse_sequence( @@ -86,7 +95,7 @@ async fn app_server_shares_flag_selected_code_mode_host_across_threads() -> Resu let original_config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) - .with_args(&["--code-mode-host", &websocket_url]) + .with_args(&["--code-mode-host", &host_url]) .build_initialized_with_timeout(DEFAULT_READ_TIMEOUT) .await?; diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 6bd783f2b8..b1532f1fd2 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -4070,18 +4070,45 @@ mod tests { ); } + #[test] + fn app_server_grpc_code_mode_host_url_parses_independently_of_listen_transport() { + let app_server = app_server_from_args( + [ + "codex", + "app-server", + "--code-mode-host", + "https://example.test", + "--listen", + "ws://127.0.0.1:4500", + ] + .as_ref(), + ); + + assert_eq!( + app_server.code_mode_host.code_mode_host, + Some(url::Url::parse("https://example.test").expect("test endpoint should parse")) + ); + } + #[test] fn app_server_rejects_invalid_code_mode_host_urls() { for endpoint in [ - "http://127.0.0.1:8765", + "ftp://127.0.0.1:8765", "ws://", "wss://example.test/code-mode#fragment", + "https://example.test/code-mode", + "http://alice:secret@example.test", + "https://alice:secret@example.test", + "http://example.test/?token=secret", ] { let error = MultitoolCli::try_parse_from(["codex", "app-server", "--code-mode-host", endpoint]) .expect_err("invalid code-mode host endpoint should fail argument parsing"); assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation); + let rendered_error = error.to_string(); + assert!(!rendered_error.contains("alice")); + assert!(!rendered_error.contains("secret")); } } diff --git a/codex-rs/code-mode-host/tests/grpc.rs b/codex-rs/code-mode-host/tests/grpc.rs index 2d30be4a2c..67e8b95aad 100644 --- a/codex-rs/code-mode-host/tests/grpc.rs +++ b/codex-rs/code-mode-host/tests/grpc.rs @@ -175,6 +175,27 @@ async fn start_active_wait( Ok(wait) } +#[tokio::test] +async fn grpc_endpoints_reject_credentials_without_disclosing_them() { + for endpoint in [ + "http://alice:secret@host.example", + "https://alice:secret@host.example", + "https://alice@host.example", + "https://:secret@host.example", + ] { + let provider = GrpcCodeModeSessionProvider::new(endpoint); + let error = provider + .create_session(Arc::new(NoopCodeModeSessionDelegate)) + .await + .err() + .expect("gRPC credentials should be rejected"); + + assert!(error.contains("must not include credentials")); + assert!(!error.contains("alice")); + assert!(!error.contains("secret")); + } +} + #[tokio::test] async fn tcp_session_persists_values_and_forwards_tools_notifications_and_closure() -> Result<()> { let host = HostHarness::start("grpc://127.0.0.1:0").await?; diff --git a/codex-rs/code-mode/src/grpc_session/transport.rs b/codex-rs/code-mode/src/grpc_session/transport.rs index 0ca665125b..bd9d4b835e 100644 --- a/codex-rs/code-mode/src/grpc_session/transport.rs +++ b/codex-rs/code-mode/src/grpc_session/transport.rs @@ -72,6 +72,11 @@ impl SharedTransport { if !matches!(target.scheme(), "http" | "https") { return Err("gRPC code-mode host URL must use http or https".to_string()); } + if !target.username().is_empty() || target.password().is_some() { + return Err( + "gRPC code-mode host URL must not include credentials".to_string(), + ); + } if target.path() != "/" || target.query().is_some() || target.fragment().is_some()