mirror of
https://github.com/openai/codex.git
synced 2026-09-09 15:58:47 +00:00
Merge branch 'main' into xli-codex/fix-marketplace-local-source-windows
This commit is contained in:
@@ -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 <token>` 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:
|
||||
|
||||
|
||||
@@ -34,6 +34,10 @@ pub struct AppServerWebsocketAuthArgs {
|
||||
#[arg(long = "ws-token-file", value_name = "PATH")]
|
||||
pub ws_token_file: Option<PathBuf>,
|
||||
|
||||
/// Hex-encoded SHA-256 digest of the capability token.
|
||||
#[arg(long = "ws-token-sha256", value_name = "HEX")]
|
||||
pub ws_token_sha256: Option<String>,
|
||||
|
||||
/// 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<PathBuf>,
|
||||
@@ -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<WebsocketAuthMode>,
|
||||
@@ -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<WebsocketAuthPolicy> {
|
||||
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<AbsoluteP
|
||||
AbsolutePathBuf::try_from(path).with_context(|| format!("{flag_name} must be an absolute path"))
|
||||
}
|
||||
|
||||
fn sha256_digest_arg(flag_name: &str, value: &str) -> 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<u8> {
|
||||
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 {
|
||||
|
||||
@@ -251,8 +251,24 @@ pub fn build_exec_request(
|
||||
codex_linux_sandbox_exe: &Option<PathBuf>,
|
||||
use_legacy_landlock: bool,
|
||||
) -> Result<ExecRequest> {
|
||||
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<StdoutStream>,
|
||||
after_spawn: Option<Box<dyn FnOnce() + Send>>,
|
||||
#[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<RawExecToolCallOutput> {
|
||||
#[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<String> {
|
||||
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<StdoutStream>,
|
||||
after_spawn: Option<Box<dyn FnOnce() + Send>>,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
#[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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user