diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index e00bbefc91..7673567427 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -41,8 +41,11 @@ Security note: - Non-loopback websocket listeners currently allow unauthenticated connections by default during rollout. If you expose one remotely, configure websocket auth explicitly now. - Supported auth modes are app-server flags: - `--ws-auth capability-token --ws-token-file /absolute/path` + - `--ws-auth capability-token --ws-token-sha256 HEX` - `--ws-auth signed-bearer-token --ws-shared-secret-file /absolute/path` for HMAC-signed JWT/JWS bearer tokens, with optional `--ws-issuer`, `--ws-audience`, `--ws-max-clock-skew-seconds` - Clients present the credential as `Authorization: Bearer ` during the websocket handshake. Auth is enforced before JSON-RPC `initialize`. +- When starting `codex app-server` manually, prefer `--ws-token-file` over passing raw bearer tokens on the command line. Store a high-entropy token in a file readable only by your user, then have your client present that token in the websocket `Authorization` header. +- `--ws-token-sha256` is intended for clients that keep the raw token in a separate local secret store and only need the server to know the SHA-256 verifier. The hash may appear in process listings, but it is not sufficient to authenticate; clients still need the original raw token. Only use this mode with randomly generated high-entropy tokens, not passwords or other guessable values. Tracing/log output: diff --git a/codex-rs/app-server/src/transport/auth.rs b/codex-rs/app-server/src/transport/auth.rs index a67c692b7d..45f44a36c9 100644 --- a/codex-rs/app-server/src/transport/auth.rs +++ b/codex-rs/app-server/src/transport/auth.rs @@ -34,6 +34,10 @@ pub struct AppServerWebsocketAuthArgs { #[arg(long = "ws-token-file", value_name = "PATH")] pub ws_token_file: Option, + /// Hex-encoded SHA-256 digest of the capability token. + #[arg(long = "ws-token-sha256", value_name = "HEX")] + pub ws_token_sha256: Option, + /// Absolute path to the shared secret file for signed JWT bearer tokens. #[arg(long = "ws-shared-secret-file", value_name = "PATH")] pub ws_shared_secret_file: Option, @@ -65,7 +69,7 @@ pub struct AppServerWebsocketAuthSettings { #[derive(Debug, Clone, PartialEq, Eq)] pub enum AppServerWebsocketAuthConfig { CapabilityToken { - token_file: AbsolutePathBuf, + source: AppServerWebsocketCapabilityTokenSource, }, SignedBearerToken { shared_secret_file: AbsolutePathBuf, @@ -75,6 +79,12 @@ pub enum AppServerWebsocketAuthConfig { }, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AppServerWebsocketCapabilityTokenSource { + TokenFile { token_file: AbsolutePathBuf }, + TokenSha256 { token_sha256: [u8; 32] }, +} + #[derive(Clone, Debug, Default)] pub(crate) struct WebsocketAuthPolicy { pub(crate) mode: Option, @@ -144,17 +154,34 @@ impl AppServerWebsocketAuthArgs { "`--ws-shared-secret-file`, `--ws-issuer`, `--ws-audience`, and `--ws-max-clock-skew-seconds` require `--ws-auth signed-bearer-token`" ); } - let token_file = self.ws_token_file.context( - "`--ws-token-file` is required when `--ws-auth capability-token` is set", - )?; - Some(AppServerWebsocketAuthConfig::CapabilityToken { - token_file: absolute_path_arg("--ws-token-file", token_file)?, - }) + let source = match (self.ws_token_file, self.ws_token_sha256) { + (Some(_), Some(_)) => { + anyhow::bail!( + "`--ws-token-file` and `--ws-token-sha256` are mutually exclusive" + ); + } + (Some(token_file), None) => { + AppServerWebsocketCapabilityTokenSource::TokenFile { + token_file: absolute_path_arg("--ws-token-file", token_file)?, + } + } + (None, Some(token_sha256)) => { + AppServerWebsocketCapabilityTokenSource::TokenSha256 { + token_sha256: sha256_digest_arg("--ws-token-sha256", &token_sha256)?, + } + } + (None, None) => { + anyhow::bail!( + "`--ws-token-file` or `--ws-token-sha256` is required when `--ws-auth capability-token` is set" + ); + } + }; + Some(AppServerWebsocketAuthConfig::CapabilityToken { source }) } Some(WebsocketAuthCliMode::SignedBearerToken) => { - if self.ws_token_file.is_some() { + if self.ws_token_file.is_some() || self.ws_token_sha256.is_some() { anyhow::bail!( - "`--ws-token-file` requires `--ws-auth capability-token`, not `signed-bearer-token`" + "`--ws-token-file` and `--ws-token-sha256` require `--ws-auth capability-token`, not `signed-bearer-token`" ); } let shared_secret_file = self.ws_shared_secret_file.context( @@ -174,6 +201,7 @@ impl AppServerWebsocketAuthArgs { } None => { if self.ws_token_file.is_some() + || self.ws_token_sha256.is_some() || self.ws_shared_secret_file.is_some() || self.ws_issuer.is_some() || self.ws_audience.is_some() @@ -195,12 +223,19 @@ pub(crate) fn policy_from_settings( settings: &AppServerWebsocketAuthSettings, ) -> io::Result { let mode = match settings.config.as_ref() { - Some(AppServerWebsocketAuthConfig::CapabilityToken { token_file }) => { - let token = read_trimmed_secret(token_file.as_ref())?; - Some(WebsocketAuthMode::CapabilityToken { - token_sha256: sha256_digest(token.as_bytes()), - }) - } + Some(AppServerWebsocketAuthConfig::CapabilityToken { source }) => match source { + AppServerWebsocketCapabilityTokenSource::TokenFile { token_file } => { + let token = read_trimmed_secret(token_file.as_ref())?; + Some(WebsocketAuthMode::CapabilityToken { + token_sha256: sha256_digest(token.as_bytes()), + }) + } + AppServerWebsocketCapabilityTokenSource::TokenSha256 { token_sha256 } => { + Some(WebsocketAuthMode::CapabilityToken { + token_sha256: *token_sha256, + }) + } + }, Some(AppServerWebsocketAuthConfig::SignedBearerToken { shared_secret_file, issuer, @@ -387,6 +422,30 @@ fn absolute_path_arg(flag_name: &str, path: PathBuf) -> anyhow::Result anyhow::Result<[u8; 32]> { + let trimmed = value.trim(); + if trimmed.len() != 64 { + anyhow::bail!("{flag_name} must be a 64-character hex SHA-256 digest"); + } + + let mut digest = [0u8; 32]; + for (index, pair) in trimmed.as_bytes().chunks_exact(2).enumerate() { + let high = hex_nibble(flag_name, pair[0])?; + let low = hex_nibble(flag_name, pair[1])?; + digest[index] = (high << 4) | low; + } + Ok(digest) +} + +fn hex_nibble(flag_name: &str, byte: u8) -> anyhow::Result { + match byte { + b'0'..=b'9' => Ok(byte - b'0'), + b'a'..=b'f' => Ok(byte - b'a' + 10), + b'A'..=b'F' => Ok(byte - b'A' + 10), + _ => anyhow::bail!("{flag_name} must be a 64-character hex SHA-256 digest"), + } +} + fn sha256_digest(input: &[u8]) -> [u8; 32] { let mut digest = [0u8; 32]; digest.copy_from_slice(&Sha256::digest(input)); @@ -403,6 +462,7 @@ fn unauthorized(message: &'static str) -> WebsocketAuthError { #[cfg(test)] mod tests { use super::*; + use axum::http::HeaderValue; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use hmac::Hmac; @@ -443,19 +503,98 @@ mod tests { } #[test] - fn capability_token_args_require_token_file() { + fn capability_token_args_require_token_file_or_hash() { let err = AppServerWebsocketAuthArgs { ws_auth: Some(WebsocketAuthCliMode::CapabilityToken), ..Default::default() } .try_into_settings() - .expect_err("capability-token mode should require a token file"); + .expect_err("capability-token mode should require a token source"); assert!( - err.to_string().contains("--ws-token-file"), + err.to_string().contains("--ws-token-file") + && err.to_string().contains("--ws-token-sha256"), "unexpected error: {err}" ); } + #[test] + fn capability_token_args_accept_token_hash() { + let settings = AppServerWebsocketAuthArgs { + ws_auth: Some(WebsocketAuthCliMode::CapabilityToken), + ws_token_sha256: Some("ab".repeat(32)), + ..Default::default() + } + .try_into_settings() + .expect("capability-token hash args should parse"); + + assert_eq!( + settings, + AppServerWebsocketAuthSettings { + config: Some(AppServerWebsocketAuthConfig::CapabilityToken { + source: AppServerWebsocketCapabilityTokenSource::TokenSha256 { + token_sha256: [0xab; 32], + }, + }), + } + ); + } + + #[test] + fn capability_token_args_reject_multiple_token_sources() { + let err = AppServerWebsocketAuthArgs { + ws_auth: Some(WebsocketAuthCliMode::CapabilityToken), + ws_token_file: Some(PathBuf::from("/tmp/token")), + ws_token_sha256: Some("ab".repeat(32)), + ..Default::default() + } + .try_into_settings() + .expect_err("capability-token mode should reject multiple token sources"); + assert!( + err.to_string().contains("mutually exclusive"), + "unexpected error: {err}" + ); + } + + #[test] + fn capability_token_args_reject_malformed_token_hash() { + let err = AppServerWebsocketAuthArgs { + ws_auth: Some(WebsocketAuthCliMode::CapabilityToken), + ws_token_sha256: Some("not-a-sha256".to_string()), + ..Default::default() + } + .try_into_settings() + .expect_err("capability-token mode should reject malformed token hashes"); + assert!( + err.to_string().contains("64-character hex"), + "unexpected error: {err}" + ); + } + + #[test] + fn capability_token_hash_policy_authorizes_matching_bearer_token() { + let settings = AppServerWebsocketAuthSettings { + config: Some(AppServerWebsocketAuthConfig::CapabilityToken { + source: AppServerWebsocketCapabilityTokenSource::TokenSha256 { + token_sha256: sha256_digest(b"super-secret-token"), + }, + }), + }; + let policy = policy_from_settings(&settings).expect("hash policy should build"); + let mut headers = HeaderMap::new(); + headers.insert( + AUTHORIZATION, + HeaderValue::from_static("Bearer super-secret-token"), + ); + authorize_upgrade(&headers, &policy).expect("matching token should authorize"); + + headers.insert( + AUTHORIZATION, + HeaderValue::from_static("Bearer wrong-token"), + ); + let err = authorize_upgrade(&headers, &policy).expect_err("wrong token should fail"); + assert_eq!(err.status_code(), StatusCode::UNAUTHORIZED); + } + #[test] fn signed_bearer_args_require_mode_when_mode_specific_flags_are_set() { let err = AppServerWebsocketAuthArgs { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index d5b96902dc..10a1dc40ff 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -251,8 +251,24 @@ pub fn build_exec_request( codex_linux_sandbox_exe: &Option, use_legacy_landlock: bool, ) -> Result { - let windows_sandbox_level = params.windows_sandbox_level; - let enforce_managed_network = params.network.is_some(); + let ExecParams { + command, + cwd, + mut env, + expiration, + capture_policy, + network, + windows_sandbox_level, + windows_sandbox_private_desktop, + + // TODO: Should arg0 be set on the ExecRequest that is returned? + arg0: _, + // These fields are related to approvals, so can be ignored here. + justification: _, + sandbox_permissions: _, + } = params; + + let enforce_managed_network = network.is_some(); let sandbox_type = select_process_exec_tool_sandbox_type( file_system_sandbox_policy, network_sandbox_policy, @@ -261,19 +277,6 @@ pub fn build_exec_request( ); tracing::debug!("Sandbox type: {sandbox_type:?}"); - let ExecParams { - command, - cwd, - mut env, - expiration, - capture_policy, - network, - sandbox_permissions: _, - windows_sandbox_level, - windows_sandbox_private_desktop, - justification: _, - arg0: _, - } = params; if let Some(network) = network.as_ref() { network.apply_to_env(&mut env); } @@ -357,7 +360,8 @@ pub(crate) async fn execute_exec_request( windows_sandbox_level, windows_sandbox_private_desktop, sandbox_policy, - file_system_sandbox_policy, + // TODO(mbolin): Use file_system_sandbox_policy instead of sandbox_policy. + file_system_sandbox_policy: _, network_sandbox_policy, windows_sandbox_filesystem_overrides, arg0, @@ -378,21 +382,40 @@ pub(crate) async fn execute_exec_request( }; let start = Instant::now(); - let raw_output_result = exec( + let raw_output_result = get_raw_output_result( params, - sandbox, - &sandbox_policy, - &file_system_sandbox_policy, - windows_sandbox_filesystem_overrides.as_ref(), network_sandbox_policy, stdout_stream, after_spawn, + sandbox, + &sandbox_policy, + windows_sandbox_filesystem_overrides.as_ref(), ) .await; let duration = start.elapsed(); finalize_exec_result(raw_output_result, sandbox, duration) } +async fn get_raw_output_result( + params: ExecParams, + network_sandbox_policy: NetworkSandboxPolicy, + stdout_stream: Option, + after_spawn: Option>, + #[cfg_attr(not(windows), allow(unused_variables))] sandbox: SandboxType, + #[cfg_attr(not(windows), allow(unused_variables))] sandbox_policy: &SandboxPolicy, + #[cfg_attr(not(windows), allow(unused_variables))] windows_sandbox_filesystem_overrides: Option< + &WindowsSandboxFilesystemOverrides, + >, +) -> Result { + #[cfg(target_os = "windows")] + if sandbox == SandboxType::WindowsRestrictedToken { + return exec_windows_sandbox(params, sandbox_policy, windows_sandbox_filesystem_overrides) + .await; + } + + exec(params, network_sandbox_policy, stdout_stream, after_spawn).await +} + #[cfg(target_os = "windows")] fn extract_create_process_as_user_error_code(err: &str) -> Option { let marker = "CreateProcessAsUserW failed: "; @@ -799,26 +822,24 @@ fn aggregate_output( } } -#[allow(clippy::too_many_arguments)] +/// This is a general-purpose function for executing a command specified by +/// [ExecParams]. Events are reported via `stdout_stream`, if specified, and +/// `after_spawn` is invoked once the child process has been spawned, before +/// output consumption begins. +/// +/// `network_sandbox_policy` is used to determine whether +/// CODEX_SANDBOX_NETWORK_DISABLED=1 is added to the environment of the spawned +/// process. +/// +/// Note this command does not apply any sandboxing logic. The caller is +/// responsible for constructing [ExecParams::command] to include any sandboxing +/// wrapper args, as appropriate. async fn exec( params: ExecParams, - _sandbox: SandboxType, - _sandbox_policy: &SandboxPolicy, - _file_system_sandbox_policy: &FileSystemSandboxPolicy, - _windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>, network_sandbox_policy: NetworkSandboxPolicy, stdout_stream: Option, after_spawn: Option>, ) -> Result { - #[cfg(target_os = "windows")] - if _sandbox == SandboxType::WindowsRestrictedToken { - return exec_windows_sandbox( - params, - _sandbox_policy, - _windows_sandbox_filesystem_overrides, - ) - .await; - } let ExecParams { command, cwd, @@ -827,8 +848,14 @@ async fn exec( arg0, expiration, capture_policy, + + // If applicable, these fields should have been honored upstream of + // this exec call. windows_sandbox_level: _, - .. + windows_sandbox_private_desktop: _, + // These fields are related to approvals, so can be ignored here. + sandbox_permissions: _, + justification: _, } = params; if let Some(network) = network.as_ref() { network.apply_to_env(&mut env); diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 796062b3fc..acc92f5585 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -277,10 +277,6 @@ async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> { justification: None, arg0: None, }, - SandboxType::None, - &SandboxPolicy::DangerFullAccess, - &FileSystemSandboxPolicy::unrestricted(), - /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -317,10 +313,6 @@ async fn exec_full_buffer_capture_keeps_io_drain_timeout_when_descendant_holds_p justification: None, arg0: None, }, - SandboxType::None, - &SandboxPolicy::DangerFullAccess, - &FileSystemSandboxPolicy::unrestricted(), - /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -931,10 +923,6 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()> let output = exec( params, - SandboxType::None, - &SandboxPolicy::new_read_only_policy(), - &FileSystemSandboxPolicy::from(&SandboxPolicy::new_read_only_policy()), - /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Restricted, /*stdout_stream*/ None, /*after_spawn*/ None,