mirror of
https://github.com/openai/codex.git
synced 2026-04-13 19:11:40 +03:00
Compare commits
5 Commits
dev/shaqay
...
mikhail/wi
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ff0039b7ae | ||
|
|
86bd0bc95c | ||
|
|
bacb92b1d7 | ||
|
|
4ffe6c2ce6 | ||
|
|
6550007cca |
2
codex-rs/Cargo.lock
generated
2
codex-rs/Cargo.lock
generated
@@ -2098,6 +2098,7 @@ dependencies = [
|
||||
"async-trait",
|
||||
"base64 0.22.1",
|
||||
"codex-app-server-protocol",
|
||||
"codex-config",
|
||||
"codex-protocol",
|
||||
"codex-sandboxing",
|
||||
"codex-utils-absolute-path",
|
||||
@@ -2107,6 +2108,7 @@ dependencies = [
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serial_test",
|
||||
"tempfile",
|
||||
"test-case",
|
||||
"thiserror 2.0.18",
|
||||
|
||||
@@ -14,6 +14,7 @@ pub mod profile_toml;
|
||||
mod project_root_markers;
|
||||
mod requirements_exec_policy;
|
||||
pub mod schema;
|
||||
pub mod shell_environment;
|
||||
mod skills_config;
|
||||
mod state;
|
||||
pub mod types;
|
||||
|
||||
123
codex-rs/config/src/shell_environment.rs
Normal file
123
codex-rs/config/src/shell_environment.rs
Normal file
@@ -0,0 +1,123 @@
|
||||
use crate::types::EnvironmentVariablePattern;
|
||||
use crate::types::ShellEnvironmentPolicy;
|
||||
use crate::types::ShellEnvironmentPolicyInherit;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
pub const CODEX_THREAD_ID_ENV_VAR: &str = "CODEX_THREAD_ID";
|
||||
|
||||
/// Construct a shell environment from the supplied process environment and
|
||||
/// shell-environment policy.
|
||||
pub fn create_env(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
thread_id: Option<&str>,
|
||||
) -> HashMap<String, String> {
|
||||
create_env_from_vars(std::env::vars(), policy, thread_id)
|
||||
}
|
||||
|
||||
pub fn create_env_from_vars<I>(
|
||||
vars: I,
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
thread_id: Option<&str>,
|
||||
) -> HashMap<String, String>
|
||||
where
|
||||
I: IntoIterator<Item = (String, String)>,
|
||||
{
|
||||
let mut env_map = populate_env(vars, policy, thread_id);
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
// This is a workaround to address the failures we are seeing in the
|
||||
// following tests when run via Bazel on Windows:
|
||||
//
|
||||
// ```
|
||||
// suite::shell_command::unicode_output::with_login
|
||||
// suite::shell_command::unicode_output::without_login
|
||||
// ```
|
||||
//
|
||||
// Currently, we can only reproduce these failures in CI, which makes
|
||||
// iteration times long, so we include this quick fix for now to unblock
|
||||
// getting the Windows Bazel build running.
|
||||
if !env_map.keys().any(|k| k.eq_ignore_ascii_case("PATHEXT")) {
|
||||
env_map.insert("PATHEXT".to_string(), ".COM;.EXE;.BAT;.CMD".to_string());
|
||||
}
|
||||
}
|
||||
env_map
|
||||
}
|
||||
|
||||
pub fn populate_env<I>(
|
||||
vars: I,
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
thread_id: Option<&str>,
|
||||
) -> HashMap<String, String>
|
||||
where
|
||||
I: IntoIterator<Item = (String, String)>,
|
||||
{
|
||||
// Step 1 - determine the starting set of variables based on the
|
||||
// `inherit` strategy.
|
||||
let mut env_map: HashMap<String, String> = match policy.inherit {
|
||||
ShellEnvironmentPolicyInherit::All => vars.into_iter().collect(),
|
||||
ShellEnvironmentPolicyInherit::None => HashMap::new(),
|
||||
ShellEnvironmentPolicyInherit::Core => {
|
||||
let core_vars: HashSet<&str> = COMMON_CORE_VARS
|
||||
.iter()
|
||||
.copied()
|
||||
.chain(PLATFORM_CORE_VARS.iter().copied())
|
||||
.collect();
|
||||
let is_core_var = |name: &str| {
|
||||
if cfg!(target_os = "windows") {
|
||||
core_vars
|
||||
.iter()
|
||||
.any(|allowed| allowed.eq_ignore_ascii_case(name))
|
||||
} else {
|
||||
core_vars.contains(name)
|
||||
}
|
||||
};
|
||||
vars.into_iter().filter(|(k, _)| is_core_var(k)).collect()
|
||||
}
|
||||
};
|
||||
|
||||
// Internal helper - does `name` match any pattern in `patterns`?
|
||||
let matches_any = |name: &str, patterns: &[EnvironmentVariablePattern]| -> bool {
|
||||
patterns.iter().any(|pattern| pattern.matches(name))
|
||||
};
|
||||
|
||||
// Step 2 - Apply the default exclude if not disabled.
|
||||
if !policy.ignore_default_excludes {
|
||||
let default_excludes = vec![
|
||||
EnvironmentVariablePattern::new_case_insensitive("*KEY*"),
|
||||
EnvironmentVariablePattern::new_case_insensitive("*SECRET*"),
|
||||
EnvironmentVariablePattern::new_case_insensitive("*TOKEN*"),
|
||||
];
|
||||
env_map.retain(|k, _| !matches_any(k, &default_excludes));
|
||||
}
|
||||
|
||||
// Step 3 - Apply custom excludes.
|
||||
if !policy.exclude.is_empty() {
|
||||
env_map.retain(|k, _| !matches_any(k, &policy.exclude));
|
||||
}
|
||||
|
||||
// Step 4 - Apply user-provided overrides.
|
||||
for (key, val) in &policy.r#set {
|
||||
env_map.insert(key.clone(), val.clone());
|
||||
}
|
||||
|
||||
// Step 5 - If include_only is non-empty, keep only the matching vars.
|
||||
if !policy.include_only.is_empty() {
|
||||
env_map.retain(|k, _| matches_any(k, &policy.include_only));
|
||||
}
|
||||
|
||||
// Step 6 - Populate the thread ID environment variable when provided.
|
||||
if let Some(thread_id) = thread_id {
|
||||
env_map.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
|
||||
}
|
||||
|
||||
env_map
|
||||
}
|
||||
|
||||
const COMMON_CORE_VARS: &[&str] = &["PATH", "SHELL", "TMPDIR", "TEMP", "TMP"];
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
const PLATFORM_CORE_VARS: &[&str] = &["PATHEXT", "USERNAME", "USERPROFILE"];
|
||||
|
||||
#[cfg(unix)]
|
||||
const PLATFORM_CORE_VARS: &[&str] = &["HOME", "LANG", "LC_ALL", "LC_CTYPE", "LOGNAME", "USER"];
|
||||
@@ -658,7 +658,7 @@ impl From<SandboxWorkspaceWrite> for codex_app_server_protocol::SandboxSettings
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)]
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
pub enum ShellEnvironmentPolicyInherit {
|
||||
/// "Core" environment variables for the platform. On UNIX, this would
|
||||
|
||||
@@ -1525,7 +1525,7 @@ fn config_defaults_to_file_cli_auth_store_mode() -> std::io::Result<()> {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_honors_explicit_keyring_auth_store_mode() -> std::io::Result<()> {
|
||||
fn config_resolves_explicit_keyring_auth_store_mode() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cfg = ConfigToml {
|
||||
cli_auth_credentials_store: Some(AuthCredentialsStoreMode::Keyring),
|
||||
@@ -1540,14 +1540,17 @@ fn config_honors_explicit_keyring_auth_store_mode() -> std::io::Result<()> {
|
||||
|
||||
assert_eq!(
|
||||
config.cli_auth_credentials_store_mode,
|
||||
AuthCredentialsStoreMode::Keyring,
|
||||
resolve_cli_auth_credentials_store_mode(
|
||||
AuthCredentialsStoreMode::Keyring,
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_defaults_to_auto_oauth_store_mode() -> std::io::Result<()> {
|
||||
fn config_resolves_default_oauth_store_mode() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cfg = ConfigToml::default();
|
||||
|
||||
@@ -1559,12 +1562,66 @@ fn config_defaults_to_auto_oauth_store_mode() -> std::io::Result<()> {
|
||||
|
||||
assert_eq!(
|
||||
config.mcp_oauth_credentials_store_mode,
|
||||
OAuthCredentialsStoreMode::Auto,
|
||||
resolve_mcp_oauth_credentials_store_mode(
|
||||
OAuthCredentialsStoreMode::Auto,
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_dev_builds_force_file_cli_auth_store_modes() {
|
||||
assert_eq!(
|
||||
resolve_cli_auth_credentials_store_mode(
|
||||
AuthCredentialsStoreMode::Keyring,
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
AuthCredentialsStoreMode::File,
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_cli_auth_credentials_store_mode(
|
||||
AuthCredentialsStoreMode::Auto,
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
AuthCredentialsStoreMode::File,
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_cli_auth_credentials_store_mode(
|
||||
AuthCredentialsStoreMode::Ephemeral,
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
AuthCredentialsStoreMode::Ephemeral,
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_cli_auth_credentials_store_mode(AuthCredentialsStoreMode::Keyring, "1.2.3"),
|
||||
AuthCredentialsStoreMode::Keyring,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_dev_builds_force_file_mcp_oauth_store_modes() {
|
||||
assert_eq!(
|
||||
resolve_mcp_oauth_credentials_store_mode(
|
||||
OAuthCredentialsStoreMode::Keyring,
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
OAuthCredentialsStoreMode::File,
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_mcp_oauth_credentials_store_mode(
|
||||
OAuthCredentialsStoreMode::Auto,
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
OAuthCredentialsStoreMode::File,
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_mcp_oauth_credentials_store_mode(OAuthCredentialsStoreMode::Keyring, "1.2.3"),
|
||||
OAuthCredentialsStoreMode::Keyring,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feedback_enabled_defaults_to_true() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -1922,7 +1979,10 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> {
|
||||
)?;
|
||||
assert_eq!(
|
||||
final_config.mcp_oauth_credentials_store_mode,
|
||||
OAuthCredentialsStoreMode::Keyring,
|
||||
resolve_mcp_oauth_credentials_store_mode(
|
||||
OAuthCredentialsStoreMode::Keyring,
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
@@ -4514,7 +4574,10 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
cwd: fixture.cwd(),
|
||||
cli_auth_credentials_store_mode: Default::default(),
|
||||
mcp_servers: Constrained::allow_any(HashMap::new()),
|
||||
mcp_oauth_credentials_store_mode: Default::default(),
|
||||
mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode(
|
||||
Default::default(),
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
mcp_oauth_callback_port: None,
|
||||
mcp_oauth_callback_url: None,
|
||||
model_providers: fixture.model_provider_map.clone(),
|
||||
@@ -4660,7 +4723,10 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
cwd: fixture.cwd(),
|
||||
cli_auth_credentials_store_mode: Default::default(),
|
||||
mcp_servers: Constrained::allow_any(HashMap::new()),
|
||||
mcp_oauth_credentials_store_mode: Default::default(),
|
||||
mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode(
|
||||
Default::default(),
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
mcp_oauth_callback_port: None,
|
||||
mcp_oauth_callback_url: None,
|
||||
model_providers: fixture.model_provider_map.clone(),
|
||||
@@ -4804,7 +4870,10 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
cwd: fixture.cwd(),
|
||||
cli_auth_credentials_store_mode: Default::default(),
|
||||
mcp_servers: Constrained::allow_any(HashMap::new()),
|
||||
mcp_oauth_credentials_store_mode: Default::default(),
|
||||
mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode(
|
||||
Default::default(),
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
mcp_oauth_callback_port: None,
|
||||
mcp_oauth_callback_url: None,
|
||||
model_providers: fixture.model_provider_map.clone(),
|
||||
@@ -4934,7 +5003,10 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
cwd: fixture.cwd(),
|
||||
cli_auth_credentials_store_mode: Default::default(),
|
||||
mcp_servers: Constrained::allow_any(HashMap::new()),
|
||||
mcp_oauth_credentials_store_mode: Default::default(),
|
||||
mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode(
|
||||
Default::default(),
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
),
|
||||
mcp_oauth_callback_port: None,
|
||||
mcp_oauth_callback_url: None,
|
||||
model_providers: fixture.model_provider_map.clone(),
|
||||
|
||||
@@ -124,6 +124,7 @@ pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB
|
||||
pub(crate) const DEFAULT_AGENT_MAX_THREADS: Option<usize> = Some(6);
|
||||
pub(crate) const DEFAULT_AGENT_MAX_DEPTH: i32 = 1;
|
||||
pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option<u64> = None;
|
||||
const LOCAL_DEV_BUILD_VERSION: &str = "0.0.0";
|
||||
|
||||
pub const CONFIG_TOML_FILE: &str = "config.toml";
|
||||
|
||||
@@ -141,6 +142,32 @@ fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option<PathBuf> {
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_cli_auth_credentials_store_mode(
|
||||
configured: AuthCredentialsStoreMode,
|
||||
package_version: &str,
|
||||
) -> AuthCredentialsStoreMode {
|
||||
match (package_version, configured) {
|
||||
(
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
AuthCredentialsStoreMode::Keyring | AuthCredentialsStoreMode::Auto,
|
||||
) => AuthCredentialsStoreMode::File,
|
||||
(_, mode) => mode,
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_mcp_oauth_credentials_store_mode(
|
||||
configured: OAuthCredentialsStoreMode,
|
||||
package_version: &str,
|
||||
) -> OAuthCredentialsStoreMode {
|
||||
match (package_version, configured) {
|
||||
(
|
||||
LOCAL_DEV_BUILD_VERSION,
|
||||
OAuthCredentialsStoreMode::Keyring | OAuthCredentialsStoreMode::Auto,
|
||||
) => OAuthCredentialsStoreMode::File,
|
||||
(_, mode) => mode,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn test_config() -> Config {
|
||||
let codex_home = tempfile::tempdir().expect("create temp dir");
|
||||
@@ -2014,11 +2041,17 @@ impl Config {
|
||||
include_environment_context,
|
||||
// The config.toml omits "_mode" because it's a config file. However, "_mode"
|
||||
// is important in code to differentiate the mode from the store implementation.
|
||||
cli_auth_credentials_store_mode: cfg.cli_auth_credentials_store.unwrap_or_default(),
|
||||
cli_auth_credentials_store_mode: resolve_cli_auth_credentials_store_mode(
|
||||
cfg.cli_auth_credentials_store.unwrap_or_default(),
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
mcp_servers,
|
||||
// The config.toml omits "_mode" because it's a config file. However, "_mode"
|
||||
// is important in code to differentiate the mode from the store implementation.
|
||||
mcp_oauth_credentials_store_mode: cfg.mcp_oauth_credentials_store.unwrap_or_default(),
|
||||
mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode(
|
||||
cfg.mcp_oauth_credentials_store.unwrap_or_default(),
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
mcp_oauth_callback_port: cfg.mcp_oauth_callback_port,
|
||||
mcp_oauth_callback_url: cfg.mcp_oauth_callback_url.clone(),
|
||||
model_providers,
|
||||
|
||||
@@ -349,6 +349,7 @@ pub(crate) async fn execute_exec_request(
|
||||
command,
|
||||
cwd,
|
||||
env,
|
||||
exec_server_env_config: _,
|
||||
network,
|
||||
expiration,
|
||||
capture_policy,
|
||||
|
||||
@@ -1,11 +1,10 @@
|
||||
#[cfg(test)]
|
||||
use codex_config::types::EnvironmentVariablePattern;
|
||||
use codex_config::types::ShellEnvironmentPolicy;
|
||||
use codex_config::types::ShellEnvironmentPolicyInherit;
|
||||
use codex_protocol::ThreadId;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
pub const CODEX_THREAD_ID_ENV_VAR: &str = "CODEX_THREAD_ID";
|
||||
pub use codex_config::shell_environment::CODEX_THREAD_ID_ENV_VAR;
|
||||
|
||||
/// Construct an environment map based on the rules in the specified policy. The
|
||||
/// resulting map can be passed directly to `Command::envs()` after calling
|
||||
@@ -21,9 +20,11 @@ pub fn create_env(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
thread_id: Option<ThreadId>,
|
||||
) -> HashMap<String, String> {
|
||||
create_env_from_vars(std::env::vars(), policy, thread_id)
|
||||
let thread_id = thread_id.map(|thread_id| thread_id.to_string());
|
||||
codex_config::shell_environment::create_env(policy, thread_id.as_deref())
|
||||
}
|
||||
|
||||
#[cfg(all(test, target_os = "windows"))]
|
||||
fn create_env_from_vars<I>(
|
||||
vars: I,
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
@@ -32,35 +33,11 @@ fn create_env_from_vars<I>(
|
||||
where
|
||||
I: IntoIterator<Item = (String, String)>,
|
||||
{
|
||||
let mut env_map = populate_env(vars, policy, thread_id);
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
// This is a workaround to address the failures we are seeing in the
|
||||
// following tests when run via Bazel on Windows:
|
||||
//
|
||||
// ```
|
||||
// suite::shell_command::unicode_output::with_login
|
||||
// suite::shell_command::unicode_output::without_login
|
||||
// ```
|
||||
//
|
||||
// Currently, we can only reproduce these failures in CI, which makes
|
||||
// iteration times long, so we include this quick fix for now to unblock
|
||||
// getting the Windows Bazel build running.
|
||||
if !env_map.keys().any(|k| k.eq_ignore_ascii_case("PATHEXT")) {
|
||||
env_map.insert("PATHEXT".to_string(), ".COM;.EXE;.BAT;.CMD".to_string());
|
||||
}
|
||||
}
|
||||
env_map
|
||||
let thread_id = thread_id.map(|thread_id| thread_id.to_string());
|
||||
codex_config::shell_environment::create_env_from_vars(vars, policy, thread_id.as_deref())
|
||||
}
|
||||
|
||||
const COMMON_CORE_VARS: &[&str] = &["PATH", "SHELL", "TMPDIR", "TEMP", "TMP"];
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
const PLATFORM_CORE_VARS: &[&str] = &["PATHEXT", "USERNAME", "USERPROFILE"];
|
||||
|
||||
#[cfg(unix)]
|
||||
const PLATFORM_CORE_VARS: &[&str] = &["HOME", "LANG", "LC_ALL", "LC_CTYPE", "LOGNAME", "USER"];
|
||||
|
||||
#[cfg(test)]
|
||||
fn populate_env<I>(
|
||||
vars: I,
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
@@ -69,66 +46,8 @@ fn populate_env<I>(
|
||||
where
|
||||
I: IntoIterator<Item = (String, String)>,
|
||||
{
|
||||
// Step 1 – determine the starting set of variables based on the
|
||||
// `inherit` strategy.
|
||||
let mut env_map: HashMap<String, String> = match policy.inherit {
|
||||
ShellEnvironmentPolicyInherit::All => vars.into_iter().collect(),
|
||||
ShellEnvironmentPolicyInherit::None => HashMap::new(),
|
||||
ShellEnvironmentPolicyInherit::Core => {
|
||||
let core_vars: HashSet<&str> = COMMON_CORE_VARS
|
||||
.iter()
|
||||
.copied()
|
||||
.chain(PLATFORM_CORE_VARS.iter().copied())
|
||||
.collect();
|
||||
let is_core_var = |name: &str| {
|
||||
if cfg!(target_os = "windows") {
|
||||
core_vars
|
||||
.iter()
|
||||
.any(|allowed| allowed.eq_ignore_ascii_case(name))
|
||||
} else {
|
||||
core_vars.contains(name)
|
||||
}
|
||||
};
|
||||
vars.into_iter().filter(|(k, _)| is_core_var(k)).collect()
|
||||
}
|
||||
};
|
||||
|
||||
// Internal helper – does `name` match **any** pattern in `patterns`?
|
||||
let matches_any = |name: &str, patterns: &[EnvironmentVariablePattern]| -> bool {
|
||||
patterns.iter().any(|pattern| pattern.matches(name))
|
||||
};
|
||||
|
||||
// Step 2 – Apply the default exclude if not disabled.
|
||||
if !policy.ignore_default_excludes {
|
||||
let default_excludes = vec![
|
||||
EnvironmentVariablePattern::new_case_insensitive("*KEY*"),
|
||||
EnvironmentVariablePattern::new_case_insensitive("*SECRET*"),
|
||||
EnvironmentVariablePattern::new_case_insensitive("*TOKEN*"),
|
||||
];
|
||||
env_map.retain(|k, _| !matches_any(k, &default_excludes));
|
||||
}
|
||||
|
||||
// Step 3 – Apply custom excludes.
|
||||
if !policy.exclude.is_empty() {
|
||||
env_map.retain(|k, _| !matches_any(k, &policy.exclude));
|
||||
}
|
||||
|
||||
// Step 4 – Apply user-provided overrides.
|
||||
for (key, val) in &policy.r#set {
|
||||
env_map.insert(key.clone(), val.clone());
|
||||
}
|
||||
|
||||
// Step 5 – If include_only is non-empty, keep *only* the matching vars.
|
||||
if !policy.include_only.is_empty() {
|
||||
env_map.retain(|k, _| matches_any(k, &policy.include_only));
|
||||
}
|
||||
|
||||
// Step 6 – Populate the thread ID environment variable when provided.
|
||||
if let Some(thread_id) = thread_id {
|
||||
env_map.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
|
||||
}
|
||||
|
||||
env_map
|
||||
let thread_id = thread_id.map(|thread_id| thread_id.to_string());
|
||||
codex_config::shell_environment::populate_env(vars, policy, thread_id.as_deref())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -65,7 +65,7 @@ mod phase_one {
|
||||
/// Phase 2 (aka `Consolidation`).
|
||||
mod phase_two {
|
||||
/// Default model used for phase 2.
|
||||
pub(super) const MODEL: &str = "gpt-5.3-codex";
|
||||
pub(super) const MODEL: &str = "gpt-5.4";
|
||||
/// Default reasoning effort used for phase 2.
|
||||
pub(super) const REASONING_EFFORT: super::ReasoningEffort = super::ReasoningEffort::Medium;
|
||||
/// Lease duration (seconds) for phase-2 consolidation job ownership.
|
||||
|
||||
@@ -33,11 +33,18 @@ pub(crate) struct ExecOptions {
|
||||
pub(crate) capture_policy: ExecCapturePolicy,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) struct ExecServerEnvConfig {
|
||||
pub(crate) policy: codex_exec_server::ExecEnvPolicy,
|
||||
pub(crate) local_policy_env: HashMap<String, String>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ExecRequest {
|
||||
pub command: Vec<String>,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub env: HashMap<String, String>,
|
||||
pub(crate) exec_server_env_config: Option<ExecServerEnvConfig>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub expiration: ExecExpiration,
|
||||
pub capture_policy: ExecCapturePolicy,
|
||||
@@ -72,6 +79,7 @@ impl ExecRequest {
|
||||
command,
|
||||
cwd,
|
||||
env,
|
||||
exec_server_env_config: None,
|
||||
network,
|
||||
expiration,
|
||||
capture_policy,
|
||||
@@ -121,6 +129,7 @@ impl ExecRequest {
|
||||
command,
|
||||
cwd,
|
||||
env,
|
||||
exec_server_env_config: None,
|
||||
network,
|
||||
expiration,
|
||||
capture_policy,
|
||||
|
||||
@@ -162,6 +162,7 @@ pub(crate) async fn execute_user_shell_command(
|
||||
command: exec_command.clone(),
|
||||
cwd: cwd.clone(),
|
||||
env: exec_env_map,
|
||||
exec_server_env_config: None,
|
||||
network: turn_context.network.clone(),
|
||||
// TODO(zhao-oai): Now that we have ExecExpiration::Cancellation, we
|
||||
// should use that instead of an "arbitrarily large" timeout here.
|
||||
|
||||
@@ -126,6 +126,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
command,
|
||||
cwd: sandbox_cwd,
|
||||
env: sandbox_env,
|
||||
exec_server_env_config: _,
|
||||
network: sandbox_network,
|
||||
expiration: _sandbox_expiration,
|
||||
capture_policy: _capture_policy,
|
||||
@@ -734,6 +735,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
command: self.command.clone(),
|
||||
cwd: self.cwd.clone(),
|
||||
env: exec_env,
|
||||
exec_server_env_config: None,
|
||||
network: self.network.clone(),
|
||||
expiration: ExecExpiration::Cancellation(cancel_rx),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
|
||||
@@ -10,6 +10,7 @@ use crate::exec::ExecExpiration;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
use crate::sandboxing::ExecServerEnvConfig;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::shell::ShellType;
|
||||
use crate::tools::network_approval::NetworkApprovalMode;
|
||||
@@ -52,6 +53,7 @@ pub struct UnifiedExecRequest {
|
||||
pub process_id: i32,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub env: HashMap<String, String>,
|
||||
pub exec_server_env_config: Option<ExecServerEnvConfig>,
|
||||
pub explicit_env_overrides: HashMap<String, String>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub tty: bool,
|
||||
@@ -237,9 +239,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let exec_env = attempt
|
||||
let mut exec_env = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
match zsh_fork_backend::maybe_prepare_unified_exec(
|
||||
req,
|
||||
attempt,
|
||||
@@ -294,9 +297,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let exec_env = attempt
|
||||
let mut exec_env = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
let Some(environment) = ctx.turn.environment.as_ref() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
|
||||
@@ -66,10 +66,11 @@ pub(crate) struct OutputHandles {
|
||||
/// Transport-specific process handle used by unified exec.
|
||||
enum ProcessHandle {
|
||||
Local(Box<ExecCommandSession>),
|
||||
Remote(Arc<dyn ExecProcess>),
|
||||
ExecServer(Arc<dyn ExecProcess>),
|
||||
}
|
||||
|
||||
/// Unified wrapper over local PTY sessions and exec-server-backed processes.
|
||||
/// Unified wrapper over directly spawned PTY sessions and exec-server-backed
|
||||
/// processes.
|
||||
pub(crate) struct UnifiedExecProcess {
|
||||
process_handle: ProcessHandle,
|
||||
output_tx: broadcast::Sender<Vec<u8>>,
|
||||
@@ -135,7 +136,7 @@ impl UnifiedExecProcess {
|
||||
.send(data.to_vec())
|
||||
.await
|
||||
.map_err(|_| UnifiedExecError::WriteToStdin),
|
||||
ProcessHandle::Remote(process_handle) => {
|
||||
ProcessHandle::ExecServer(process_handle) => {
|
||||
match process_handle.write(data.to_vec()).await {
|
||||
Ok(response) => match response.status {
|
||||
WriteStatus::Accepted => Ok(()),
|
||||
@@ -179,7 +180,7 @@ impl UnifiedExecProcess {
|
||||
let state = self.state_rx.borrow().clone();
|
||||
match &self.process_handle {
|
||||
ProcessHandle::Local(process_handle) => state.has_exited || process_handle.has_exited(),
|
||||
ProcessHandle::Remote(_) => state.has_exited,
|
||||
ProcessHandle::ExecServer(_) => state.has_exited,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -189,7 +190,7 @@ impl UnifiedExecProcess {
|
||||
ProcessHandle::Local(process_handle) => {
|
||||
state.exit_code.or_else(|| process_handle.exit_code())
|
||||
}
|
||||
ProcessHandle::Remote(_) => state.exit_code,
|
||||
ProcessHandle::ExecServer(_) => state.exit_code,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -198,7 +199,7 @@ impl UnifiedExecProcess {
|
||||
self.output_closed_notify.notify_waiters();
|
||||
match &self.process_handle {
|
||||
ProcessHandle::Local(process_handle) => process_handle.terminate(),
|
||||
ProcessHandle::Remote(process_handle) => {
|
||||
ProcessHandle::ExecServer(process_handle) => {
|
||||
let process_handle = Arc::clone(process_handle);
|
||||
tokio::spawn(async move {
|
||||
let _ = process_handle.terminate().await;
|
||||
@@ -331,14 +332,14 @@ impl UnifiedExecProcess {
|
||||
Ok(managed)
|
||||
}
|
||||
|
||||
pub(super) async fn from_remote_started(
|
||||
pub(super) async fn from_exec_server_started(
|
||||
started: StartedExecProcess,
|
||||
sandbox_type: SandboxType,
|
||||
) -> Result<Self, UnifiedExecError> {
|
||||
let process_handle = ProcessHandle::Remote(Arc::clone(&started.process));
|
||||
let process_handle = ProcessHandle::ExecServer(Arc::clone(&started.process));
|
||||
let mut managed = Self::new(process_handle, sandbox_type, /*spawn_lifecycle*/ None);
|
||||
let output_handles = managed.output_handles();
|
||||
managed.output_task = Some(Self::spawn_remote_output_task(
|
||||
managed.output_task = Some(Self::spawn_exec_server_output_task(
|
||||
started,
|
||||
output_handles,
|
||||
managed.output_tx.clone(),
|
||||
@@ -366,7 +367,7 @@ impl UnifiedExecProcess {
|
||||
Ok(managed)
|
||||
}
|
||||
|
||||
fn spawn_remote_output_task(
|
||||
fn spawn_exec_server_output_task(
|
||||
started: StartedExecProcess,
|
||||
output_handles: OutputHandles,
|
||||
output_tx: broadcast::Sender<Vec<u8>>,
|
||||
|
||||
@@ -11,9 +11,11 @@ use tokio::time::Duration;
|
||||
use tokio::time::Instant;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
use crate::sandboxing::ExecRequest;
|
||||
use crate::sandboxing::ExecServerEnvConfig;
|
||||
use crate::tools::context::ExecCommandToolOutput;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
@@ -47,6 +49,7 @@ use crate::unified_exec::process::OutputBuffer;
|
||||
use crate::unified_exec::process::OutputHandles;
|
||||
use crate::unified_exec::process::SpawnLifecycleHandle;
|
||||
use crate::unified_exec::process::UnifiedExecProcess;
|
||||
use codex_config::types::ShellEnvironmentPolicy;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_output_truncation::approx_token_count;
|
||||
@@ -89,6 +92,70 @@ fn apply_unified_exec_env(mut env: HashMap<String, String>) -> HashMap<String, S
|
||||
env
|
||||
}
|
||||
|
||||
fn exec_env_policy_from_shell_policy(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
) -> codex_exec_server::ExecEnvPolicy {
|
||||
codex_exec_server::ExecEnvPolicy {
|
||||
inherit: policy.inherit.clone(),
|
||||
ignore_default_excludes: policy.ignore_default_excludes,
|
||||
exclude: policy
|
||||
.exclude
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect(),
|
||||
r#set: policy.r#set.clone(),
|
||||
include_only: policy
|
||||
.include_only
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn env_overlay_for_exec_server(
|
||||
request_env: &HashMap<String, String>,
|
||||
local_policy_env: &HashMap<String, String>,
|
||||
) -> HashMap<String, String> {
|
||||
request_env
|
||||
.iter()
|
||||
.filter(|(key, value)| local_policy_env.get(*key) != Some(*value))
|
||||
.map(|(key, value)| (key.clone(), value.clone()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn exec_server_env_for_request(
|
||||
request: &ExecRequest,
|
||||
) -> (
|
||||
Option<codex_exec_server::ExecEnvPolicy>,
|
||||
HashMap<String, String>,
|
||||
) {
|
||||
if let Some(exec_server_env_config) = &request.exec_server_env_config {
|
||||
(
|
||||
Some(exec_server_env_config.policy.clone()),
|
||||
env_overlay_for_exec_server(&request.env, &exec_server_env_config.local_policy_env),
|
||||
)
|
||||
} else {
|
||||
(None, request.env.clone())
|
||||
}
|
||||
}
|
||||
|
||||
fn exec_server_params_for_request(
|
||||
process_id: i32,
|
||||
request: &ExecRequest,
|
||||
tty: bool,
|
||||
) -> codex_exec_server::ExecParams {
|
||||
let (env_policy, env) = exec_server_env_for_request(request);
|
||||
codex_exec_server::ExecParams {
|
||||
process_id: exec_server_process_id(process_id).into(),
|
||||
argv: request.command.clone(),
|
||||
cwd: request.cwd.to_path_buf(),
|
||||
env_policy,
|
||||
env,
|
||||
tty,
|
||||
arg0: request.arg0.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Borrowed process state prepared for a `write_stdin` or poll operation.
|
||||
struct PreparedProcessHandles {
|
||||
process: Arc<UnifiedExecProcess>,
|
||||
@@ -587,12 +654,7 @@ impl UnifiedExecProcessManager {
|
||||
mut spawn_lifecycle: SpawnLifecycleHandle,
|
||||
environment: &codex_exec_server::Environment,
|
||||
) -> Result<UnifiedExecProcess, UnifiedExecError> {
|
||||
let (program, args) = request
|
||||
.command
|
||||
.split_first()
|
||||
.ok_or(UnifiedExecError::MissingCommandLine)?;
|
||||
let inherited_fds = spawn_lifecycle.inherited_fds();
|
||||
|
||||
if environment.is_remote() {
|
||||
if !inherited_fds.is_empty() {
|
||||
return Err(UnifiedExecError::create_process(
|
||||
@@ -602,19 +664,17 @@ impl UnifiedExecProcessManager {
|
||||
|
||||
let started = environment
|
||||
.get_exec_backend()
|
||||
.start(codex_exec_server::ExecParams {
|
||||
process_id: exec_server_process_id(process_id).into(),
|
||||
argv: request.command.clone(),
|
||||
cwd: request.cwd.to_path_buf(),
|
||||
env: request.env.clone(),
|
||||
tty,
|
||||
arg0: request.arg0.clone(),
|
||||
})
|
||||
.start(exec_server_params_for_request(process_id, request, tty))
|
||||
.await
|
||||
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
|
||||
return UnifiedExecProcess::from_remote_started(started, request.sandbox).await;
|
||||
spawn_lifecycle.after_spawn();
|
||||
return UnifiedExecProcess::from_exec_server_started(started, request.sandbox).await;
|
||||
}
|
||||
|
||||
let (program, args) = request
|
||||
.command
|
||||
.split_first()
|
||||
.ok_or(UnifiedExecError::MissingCommandLine)?;
|
||||
let spawn_result = if tty {
|
||||
codex_utils_pty::pty::spawn_process_with_inherited_fds(
|
||||
program,
|
||||
@@ -649,10 +709,20 @@ impl UnifiedExecProcessManager {
|
||||
cwd: AbsolutePathBuf,
|
||||
context: &UnifiedExecContext,
|
||||
) -> Result<(UnifiedExecProcess, Option<DeferredNetworkApproval>), UnifiedExecError> {
|
||||
let env = apply_unified_exec_env(create_env(
|
||||
let local_policy_env = create_env(
|
||||
&context.turn.shell_environment_policy,
|
||||
Some(context.session.conversation_id),
|
||||
));
|
||||
/*thread_id*/ None,
|
||||
);
|
||||
let mut env = local_policy_env.clone();
|
||||
env.insert(
|
||||
CODEX_THREAD_ID_ENV_VAR.to_string(),
|
||||
context.session.conversation_id.to_string(),
|
||||
);
|
||||
let env = apply_unified_exec_env(env);
|
||||
let exec_server_env_config = ExecServerEnvConfig {
|
||||
policy: exec_env_policy_from_shell_policy(&context.turn.shell_environment_policy),
|
||||
local_policy_env,
|
||||
};
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(
|
||||
self,
|
||||
@@ -680,6 +750,7 @@ impl UnifiedExecProcessManager {
|
||||
process_id: request.process_id,
|
||||
cwd,
|
||||
env,
|
||||
exec_server_env_config: Some(exec_server_env_config),
|
||||
explicit_env_overrides: context.turn.shell_environment_policy.r#set.clone(),
|
||||
network: request.network.clone(),
|
||||
tty: request.tty,
|
||||
|
||||
@@ -34,6 +34,92 @@ fn unified_exec_env_overrides_existing_values() {
|
||||
assert_eq!(env.get("PATH"), Some(&"/usr/bin".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
|
||||
let local_policy_env = HashMap::from([
|
||||
("HOME".to_string(), "/client-home".to_string()),
|
||||
("PATH".to_string(), "/client-path".to_string()),
|
||||
("SHELL_SET".to_string(), "policy".to_string()),
|
||||
]);
|
||||
let request_env = HashMap::from([
|
||||
("HOME".to_string(), "/client-home".to_string()),
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("SHELL_SET".to_string(), "policy".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
(
|
||||
"CODEX_SANDBOX_NETWORK_DISABLED".to_string(),
|
||||
"1".to_string(),
|
||||
),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
env_overlay_for_exec_server(&request_env, &local_policy_env),
|
||||
HashMap::from([
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
(
|
||||
"CODEX_SANDBOX_NETWORK_DISABLED".to_string(),
|
||||
"1".to_string()
|
||||
),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_params_use_env_policy_overlay_contract() {
|
||||
let request = ExecRequest {
|
||||
command: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()],
|
||||
cwd: std::env::current_dir()
|
||||
.expect("current dir")
|
||||
.try_into()
|
||||
.expect("absolute path"),
|
||||
env: HashMap::from([
|
||||
("HOME".to_string(), "/client-home".to_string()),
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
]),
|
||||
exec_server_env_config: Some(ExecServerEnvConfig {
|
||||
policy: codex_exec_server::ExecEnvPolicy {
|
||||
inherit: codex_config::types::ShellEnvironmentPolicyInherit::Core,
|
||||
ignore_default_excludes: false,
|
||||
exclude: Vec::new(),
|
||||
r#set: HashMap::new(),
|
||||
include_only: Vec::new(),
|
||||
},
|
||||
local_policy_env: HashMap::from([
|
||||
("HOME".to_string(), "/client-home".to_string()),
|
||||
("PATH".to_string(), "/client-path".to_string()),
|
||||
]),
|
||||
}),
|
||||
network: None,
|
||||
expiration: crate::exec::ExecExpiration::DefaultTimeout,
|
||||
capture_policy: crate::exec::ExecCapturePolicy::ShellTool,
|
||||
sandbox: codex_sandboxing::SandboxType::None,
|
||||
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
sandbox_policy: codex_protocol::protocol::SandboxPolicy::DangerFullAccess,
|
||||
file_system_sandbox_policy: codex_protocol::permissions::FileSystemSandboxPolicy::from(
|
||||
&codex_protocol::protocol::SandboxPolicy::DangerFullAccess,
|
||||
),
|
||||
network_sandbox_policy: codex_protocol::permissions::NetworkSandboxPolicy::Restricted,
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
let params =
|
||||
exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true);
|
||||
|
||||
assert_eq!(params.process_id.as_str(), "123");
|
||||
assert!(params.env_policy.is_some());
|
||||
assert_eq!(
|
||||
params.env,
|
||||
HashMap::from([
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_process_id_matches_unified_exec_process_id() {
|
||||
assert_eq!(exec_server_process_id(/*process_id*/ 4321), "4321");
|
||||
|
||||
@@ -76,7 +76,7 @@ async fn remote_process(write_status: WriteStatus) -> UnifiedExecProcess {
|
||||
}),
|
||||
};
|
||||
|
||||
UnifiedExecProcess::from_remote_started(started, SandboxType::None)
|
||||
UnifiedExecProcess::from_exec_server_started(started, SandboxType::None)
|
||||
.await
|
||||
.expect("remote process should start")
|
||||
}
|
||||
@@ -133,7 +133,7 @@ async fn remote_process_waits_for_early_exit_event() {
|
||||
let _ = wake_tx.send(1);
|
||||
});
|
||||
|
||||
let process = UnifiedExecProcess::from_remote_started(started, SandboxType::None)
|
||||
let process = UnifiedExecProcess::from_exec_server_started(started, SandboxType::None)
|
||||
.await
|
||||
.expect("remote process should observe early exit");
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ arc-swap = { workspace = true }
|
||||
async-trait = { workspace = true }
|
||||
base64 = { workspace = true }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-config = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-sandboxing = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
@@ -42,5 +43,6 @@ uuid = { workspace = true, features = ["v4"] }
|
||||
anyhow = { workspace = true }
|
||||
codex-utils-cargo-bin = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
serial_test = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
test-case = "3.3.1"
|
||||
|
||||
@@ -343,6 +343,7 @@ mod tests {
|
||||
process_id: ProcessId::from("default-env-proc"),
|
||||
argv: vec!["true".to_string()],
|
||||
cwd: std::env::current_dir().expect("read current dir"),
|
||||
env_policy: None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
|
||||
@@ -42,6 +42,7 @@ pub use process::ExecProcess;
|
||||
pub use process::StartedExecProcess;
|
||||
pub use process_id::ProcessId;
|
||||
pub use protocol::ExecClosedNotification;
|
||||
pub use protocol::ExecEnvPolicy;
|
||||
pub use protocol::ExecExitedNotification;
|
||||
pub use protocol::ExecOutputDeltaNotification;
|
||||
pub use protocol::ExecOutputStream;
|
||||
|
||||
@@ -5,6 +5,9 @@ use std::time::Duration;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_config::shell_environment;
|
||||
use codex_config::types::EnvironmentVariablePattern;
|
||||
use codex_config::types::ShellEnvironmentPolicy;
|
||||
use codex_utils_pty::ExecCommandSession;
|
||||
use codex_utils_pty::TerminalSize;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -19,6 +22,7 @@ use crate::ProcessId;
|
||||
use crate::StartedExecProcess;
|
||||
use crate::protocol::EXEC_CLOSED_METHOD;
|
||||
use crate::protocol::ExecClosedNotification;
|
||||
use crate::protocol::ExecEnvPolicy;
|
||||
use crate::protocol::ExecExitedNotification;
|
||||
use crate::protocol::ExecOutputDeltaNotification;
|
||||
use crate::protocol::ExecOutputStream;
|
||||
@@ -150,12 +154,13 @@ impl LocalProcess {
|
||||
process_map.insert(process_id.clone(), ProcessEntry::Starting);
|
||||
}
|
||||
|
||||
let env = child_env(¶ms);
|
||||
let spawned_result = if params.tty {
|
||||
codex_utils_pty::spawn_pty_process(
|
||||
program,
|
||||
args,
|
||||
params.cwd.as_path(),
|
||||
¶ms.env,
|
||||
&env,
|
||||
¶ms.arg0,
|
||||
TerminalSize::default(),
|
||||
)
|
||||
@@ -165,7 +170,7 @@ impl LocalProcess {
|
||||
program,
|
||||
args,
|
||||
params.cwd.as_path(),
|
||||
¶ms.env,
|
||||
&env,
|
||||
¶ms.arg0,
|
||||
)
|
||||
.await
|
||||
@@ -375,6 +380,36 @@ impl LocalProcess {
|
||||
}
|
||||
}
|
||||
|
||||
fn child_env(params: &ExecParams) -> HashMap<String, String> {
|
||||
let Some(env_policy) = ¶ms.env_policy else {
|
||||
return params.env.clone();
|
||||
};
|
||||
|
||||
let policy = shell_environment_policy(env_policy);
|
||||
let mut env = shell_environment::create_env(&policy, /*thread_id*/ None);
|
||||
env.extend(params.env.clone());
|
||||
env
|
||||
}
|
||||
|
||||
fn shell_environment_policy(env_policy: &ExecEnvPolicy) -> ShellEnvironmentPolicy {
|
||||
ShellEnvironmentPolicy {
|
||||
inherit: env_policy.inherit.clone(),
|
||||
ignore_default_excludes: env_policy.ignore_default_excludes,
|
||||
exclude: env_policy
|
||||
.exclude
|
||||
.iter()
|
||||
.map(|pattern| EnvironmentVariablePattern::new_case_insensitive(pattern))
|
||||
.collect(),
|
||||
r#set: env_policy.r#set.clone(),
|
||||
include_only: env_policy
|
||||
.include_only
|
||||
.iter()
|
||||
.map(|pattern| EnvironmentVariablePattern::new_case_insensitive(pattern))
|
||||
.collect(),
|
||||
use_profile: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ExecBackend for LocalProcess {
|
||||
async fn start(&self, params: ExecParams) -> Result<StartedExecProcess, ExecServerError> {
|
||||
@@ -618,3 +653,56 @@ fn notification_sender(inner: &Inner) -> Option<RpcNotificationSender> {
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_config::types::ShellEnvironmentPolicyInherit;
|
||||
|
||||
fn test_exec_params(env: HashMap<String, String>) -> ExecParams {
|
||||
ExecParams {
|
||||
process_id: ProcessId::from("env-test"),
|
||||
argv: vec!["true".to_string()],
|
||||
cwd: std::path::PathBuf::from("/tmp"),
|
||||
env_policy: None,
|
||||
env,
|
||||
tty: false,
|
||||
arg0: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_env_defaults_to_exact_env() {
|
||||
let params = test_exec_params(HashMap::from([("ONLY_THIS".to_string(), "1".to_string())]));
|
||||
|
||||
assert_eq!(
|
||||
child_env(¶ms),
|
||||
HashMap::from([("ONLY_THIS".to_string(), "1".to_string())])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_env_applies_policy_then_overlay() {
|
||||
let mut params = test_exec_params(HashMap::from([
|
||||
("OVERLAY".to_string(), "overlay".to_string()),
|
||||
("POLICY_SET".to_string(), "overlay-wins".to_string()),
|
||||
]));
|
||||
params.env_policy = Some(ExecEnvPolicy {
|
||||
inherit: ShellEnvironmentPolicyInherit::None,
|
||||
ignore_default_excludes: true,
|
||||
exclude: Vec::new(),
|
||||
r#set: HashMap::from([("POLICY_SET".to_string(), "policy".to_string())]),
|
||||
include_only: Vec::new(),
|
||||
});
|
||||
|
||||
let mut expected = HashMap::from([
|
||||
("OVERLAY".to_string(), "overlay".to_string()),
|
||||
("POLICY_SET".to_string(), "overlay-wins".to_string()),
|
||||
]);
|
||||
if cfg!(target_os = "windows") {
|
||||
expected.insert("PATHEXT".to_string(), ".COM;.EXE;.BAT;.CMD".to_string());
|
||||
}
|
||||
|
||||
assert_eq!(child_env(¶ms), expected);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::path::PathBuf;
|
||||
|
||||
use crate::FileSystemSandboxContext;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_config::types::ShellEnvironmentPolicyInherit;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
@@ -64,11 +65,23 @@ pub struct ExecParams {
|
||||
pub process_id: ProcessId,
|
||||
pub argv: Vec<String>,
|
||||
pub cwd: PathBuf,
|
||||
#[serde(default)]
|
||||
pub env_policy: Option<ExecEnvPolicy>,
|
||||
pub env: HashMap<String, String>,
|
||||
pub tty: bool,
|
||||
pub arg0: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ExecEnvPolicy {
|
||||
pub inherit: ShellEnvironmentPolicyInherit,
|
||||
pub ignore_default_excludes: bool,
|
||||
pub exclude: Vec<String>,
|
||||
pub r#set: HashMap<String, String>,
|
||||
pub include_only: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ExecResponse {
|
||||
|
||||
@@ -27,6 +27,7 @@ fn exec_params_with_argv(process_id: &str, argv: Vec<String>) -> ExecParams {
|
||||
process_id: ProcessId::from(process_id),
|
||||
argv,
|
||||
cwd: std::env::current_dir().expect("cwd"),
|
||||
env_policy: None,
|
||||
env: inherited_path_env(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
|
||||
@@ -390,6 +390,7 @@ mod tests {
|
||||
process_id,
|
||||
argv: sleep_then_print_argv(),
|
||||
cwd: std::env::current_dir().expect("cwd"),
|
||||
env_policy: None,
|
||||
env,
|
||||
tty: false,
|
||||
arg0: None,
|
||||
|
||||
@@ -47,6 +47,7 @@ pub(crate) async fn exec_server() -> anyhow::Result<ExecServerHarness> {
|
||||
child.stdin(Stdio::null());
|
||||
child.stdout(Stdio::piped());
|
||||
child.stderr(Stdio::inherit());
|
||||
child.kill_on_drop(true);
|
||||
let mut child = child.spawn()?;
|
||||
|
||||
let websocket_url = read_listen_url_from_stdout(&mut child).await?;
|
||||
@@ -140,6 +141,9 @@ impl ExecServerHarness {
|
||||
|
||||
pub(crate) async fn shutdown(&mut self) -> anyhow::Result<()> {
|
||||
self.child.start_kill()?;
|
||||
timeout(CONNECT_TIMEOUT, self.child.wait())
|
||||
.await
|
||||
.map_err(|_| anyhow!("timed out waiting for exec-server shutdown"))??;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ async fn assert_exec_process_starts_and_exits(use_remote: bool) -> Result<()> {
|
||||
process_id: ProcessId::from("proc-1"),
|
||||
argv: vec!["true".to_string()],
|
||||
cwd: std::env::current_dir()?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
@@ -127,6 +128,7 @@ async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> {
|
||||
"sleep 0.05; printf 'session output\\n'".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
@@ -156,6 +158,7 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> {
|
||||
"import sys; line = sys.stdin.readline(); sys.stdout.write(f'from-stdin:{line}'); sys.stdout.flush()".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: true,
|
||||
arg0: None,
|
||||
@@ -192,6 +195,7 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe(
|
||||
"printf 'queued output\\n'".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
@@ -210,6 +214,8 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe(
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
// Serialize tests that launch a real exec-server process through the full CLI.
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn remote_exec_process_reports_transport_disconnect() -> Result<()> {
|
||||
let mut context = create_process_context(/*use_remote*/ true).await?;
|
||||
let session = context
|
||||
@@ -222,6 +228,7 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> {
|
||||
"sleep 10".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
@@ -255,6 +262,8 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> {
|
||||
#[test_case(false ; "local")]
|
||||
#[test_case(true ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
// Serialize tests that launch a real exec-server process through the full CLI.
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn exec_process_starts_and_exits(use_remote: bool) -> Result<()> {
|
||||
assert_exec_process_starts_and_exits(use_remote).await
|
||||
}
|
||||
@@ -262,6 +271,8 @@ async fn exec_process_starts_and_exits(use_remote: bool) -> Result<()> {
|
||||
#[test_case(false ; "local")]
|
||||
#[test_case(true ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
// Serialize tests that launch a real exec-server process through the full CLI.
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn exec_process_streams_output(use_remote: bool) -> Result<()> {
|
||||
assert_exec_process_streams_output(use_remote).await
|
||||
}
|
||||
@@ -269,6 +280,8 @@ async fn exec_process_streams_output(use_remote: bool) -> Result<()> {
|
||||
#[test_case(false ; "local")]
|
||||
#[test_case(true ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
// Serialize tests that launch a real exec-server process through the full CLI.
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn exec_process_write_then_read(use_remote: bool) -> Result<()> {
|
||||
assert_exec_process_write_then_read(use_remote).await
|
||||
}
|
||||
@@ -276,6 +289,8 @@ async fn exec_process_write_then_read(use_remote: bool) -> Result<()> {
|
||||
#[test_case(false ; "local")]
|
||||
#[test_case(true ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
// Serialize tests that launch a real exec-server process through the full CLI.
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn exec_process_preserves_queued_events_before_subscribe(use_remote: bool) -> Result<()> {
|
||||
assert_exec_process_preserves_queued_events_before_subscribe(use_remote).await
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::de::DeserializeOwned;
|
||||
use sha2::Digest;
|
||||
use sha2::Sha256;
|
||||
use std::collections::HashMap;
|
||||
@@ -119,6 +120,14 @@ impl AuthStorageBackend for FileAuthStorage {
|
||||
|
||||
const KEYRING_SERVICE: &str = "Codex Auth";
|
||||
|
||||
// Const for each field of AuthDotJson, only when used on windows
|
||||
// if more fields are added to struct, then update these consts
|
||||
// and windows storage functions.
|
||||
const AUTH_MODE_FIELD: &str = "auth_mode";
|
||||
const OPENAI_API_KEY_FIELD: &str = "openai_api_key";
|
||||
const TOKENS_FIELD: &str = "tokens";
|
||||
const LAST_REFRESH_FIELD: &str = "last_refresh";
|
||||
|
||||
// turns codex_home path into a stable, short key string
|
||||
fn compute_store_key(codex_home: &Path) -> std::io::Result<String> {
|
||||
let canonical = codex_home
|
||||
@@ -175,19 +184,139 @@ impl KeyringAuthStorage {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn field_key(key: &str, field_name: &str) -> String {
|
||||
format!("{key}|{field_name}")
|
||||
}
|
||||
|
||||
fn save_windows_auth_to_keyring(&self, key: &str, auth: &AuthDotJson) -> std::io::Result<()> {
|
||||
let fields = [
|
||||
(
|
||||
AUTH_MODE_FIELD,
|
||||
serde_json::to_string(&auth.auth_mode).map_err(std::io::Error::other)?,
|
||||
),
|
||||
(
|
||||
OPENAI_API_KEY_FIELD,
|
||||
serde_json::to_string(&auth.openai_api_key).map_err(std::io::Error::other)?,
|
||||
),
|
||||
(
|
||||
TOKENS_FIELD,
|
||||
serde_json::to_string(&auth.tokens).map_err(std::io::Error::other)?,
|
||||
),
|
||||
(
|
||||
LAST_REFRESH_FIELD,
|
||||
serde_json::to_string(&auth.last_refresh).map_err(std::io::Error::other)?,
|
||||
),
|
||||
];
|
||||
|
||||
for (field_name, serialized) in fields {
|
||||
self.save_to_keyring(&Self::field_key(key, field_name), &serialized)?;
|
||||
}
|
||||
|
||||
if let Err(err) = self.keyring_store.delete(KEYRING_SERVICE, key) {
|
||||
warn!("failed to remove legacy CLI auth from keyring: {err}");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn load_windows_auth_from_keyring(&self, key: &str) -> std::io::Result<Option<AuthDotJson>> {
|
||||
fn load_auth_field_from_keyring<T>(
|
||||
keyring_store: &dyn KeyringStore,
|
||||
key: &str,
|
||||
) -> std::io::Result<Option<T>>
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
{
|
||||
match keyring_store.load(KEYRING_SERVICE, key) {
|
||||
Ok(Some(serialized)) => {
|
||||
serde_json::from_str(&serialized).map(Some).map_err(|err| {
|
||||
std::io::Error::other(format!(
|
||||
"failed to deserialize CLI auth field from keyring: {err}"
|
||||
))
|
||||
})
|
||||
}
|
||||
Ok(None) => Ok(None),
|
||||
Err(error) => Err(std::io::Error::other(format!(
|
||||
"failed to load CLI auth from keyring: {}",
|
||||
error.message()
|
||||
))),
|
||||
}
|
||||
}
|
||||
|
||||
let auth_mode = load_auth_field_from_keyring::<Option<AuthMode>>(
|
||||
self.keyring_store.as_ref(),
|
||||
&Self::field_key(key, AUTH_MODE_FIELD),
|
||||
)?;
|
||||
let openai_api_key = load_auth_field_from_keyring::<Option<String>>(
|
||||
self.keyring_store.as_ref(),
|
||||
&Self::field_key(key, OPENAI_API_KEY_FIELD),
|
||||
)?;
|
||||
let tokens = load_auth_field_from_keyring::<Option<TokenData>>(
|
||||
self.keyring_store.as_ref(),
|
||||
&Self::field_key(key, TOKENS_FIELD),
|
||||
)?;
|
||||
let last_refresh = load_auth_field_from_keyring::<Option<DateTime<Utc>>>(
|
||||
self.keyring_store.as_ref(),
|
||||
&Self::field_key(key, LAST_REFRESH_FIELD),
|
||||
)?;
|
||||
|
||||
if auth_mode.is_none()
|
||||
&& openai_api_key.is_none()
|
||||
&& tokens.is_none()
|
||||
&& last_refresh.is_none()
|
||||
{
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
Ok(Some(AuthDotJson {
|
||||
auth_mode: auth_mode.flatten(),
|
||||
openai_api_key: openai_api_key.flatten(),
|
||||
tokens: tokens.flatten(),
|
||||
last_refresh: last_refresh.flatten(),
|
||||
}))
|
||||
}
|
||||
|
||||
fn delete_windows_auth_from_keyring(&self, key: &str) -> std::io::Result<bool> {
|
||||
let field_keys = [
|
||||
Self::field_key(key, AUTH_MODE_FIELD),
|
||||
Self::field_key(key, OPENAI_API_KEY_FIELD),
|
||||
Self::field_key(key, TOKENS_FIELD),
|
||||
Self::field_key(key, LAST_REFRESH_FIELD),
|
||||
key.to_string(),
|
||||
];
|
||||
let mut removed = false;
|
||||
|
||||
for field_key in field_keys {
|
||||
removed |= self
|
||||
.keyring_store
|
||||
.delete(KEYRING_SERVICE, &field_key)
|
||||
.map_err(|err| {
|
||||
std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
|
||||
})?;
|
||||
}
|
||||
|
||||
Ok(removed)
|
||||
}
|
||||
}
|
||||
|
||||
impl AuthStorageBackend for KeyringAuthStorage {
|
||||
fn load(&self) -> std::io::Result<Option<AuthDotJson>> {
|
||||
let key = compute_store_key(&self.codex_home)?;
|
||||
self.load_from_keyring(&key)
|
||||
if cfg!(windows) {
|
||||
self.load_windows_auth_from_keyring(&key)
|
||||
} else {
|
||||
self.load_from_keyring(&key)
|
||||
}
|
||||
}
|
||||
|
||||
fn save(&self, auth: &AuthDotJson) -> std::io::Result<()> {
|
||||
let key = compute_store_key(&self.codex_home)?;
|
||||
// Simpler error mapping per style: prefer method reference over closure
|
||||
let serialized = serde_json::to_string(auth).map_err(std::io::Error::other)?;
|
||||
self.save_to_keyring(&key, &serialized)?;
|
||||
if cfg!(windows) {
|
||||
self.save_windows_auth_to_keyring(&key, auth)?;
|
||||
} else {
|
||||
let serialized = serde_json::to_string(auth).map_err(std::io::Error::other)?;
|
||||
self.save_to_keyring(&key, &serialized)?;
|
||||
}
|
||||
if let Err(err) = delete_file_if_exists(&self.codex_home) {
|
||||
warn!("failed to remove CLI auth fallback file: {err}");
|
||||
}
|
||||
@@ -196,12 +325,15 @@ impl AuthStorageBackend for KeyringAuthStorage {
|
||||
|
||||
fn delete(&self) -> std::io::Result<bool> {
|
||||
let key = compute_store_key(&self.codex_home)?;
|
||||
let keyring_removed = self
|
||||
.keyring_store
|
||||
.delete(KEYRING_SERVICE, &key)
|
||||
.map_err(|err| {
|
||||
std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
|
||||
})?;
|
||||
let keyring_removed = if cfg!(windows) {
|
||||
self.delete_windows_auth_from_keyring(&key)?
|
||||
} else {
|
||||
self.keyring_store
|
||||
.delete(KEYRING_SERVICE, &key)
|
||||
.map_err(|err| {
|
||||
std::io::Error::other(format!("failed to delete auth from keyring: {err}"))
|
||||
})?
|
||||
};
|
||||
let file_removed = delete_file_if_exists(&self.codex_home)?;
|
||||
Ok(keyring_removed || file_removed)
|
||||
}
|
||||
|
||||
@@ -126,6 +126,34 @@ where
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn seed_windows_keyring_with_auth(
|
||||
mock_keyring: &MockKeyringStore,
|
||||
key: &str,
|
||||
auth: &AuthDotJson,
|
||||
) -> anyhow::Result<()> {
|
||||
mock_keyring.save(
|
||||
KEYRING_SERVICE,
|
||||
&KeyringAuthStorage::field_key(key, AUTH_MODE_FIELD),
|
||||
&serde_json::to_string(&auth.auth_mode)?,
|
||||
)?;
|
||||
mock_keyring.save(
|
||||
KEYRING_SERVICE,
|
||||
&KeyringAuthStorage::field_key(key, OPENAI_API_KEY_FIELD),
|
||||
&serde_json::to_string(&auth.openai_api_key)?,
|
||||
)?;
|
||||
mock_keyring.save(
|
||||
KEYRING_SERVICE,
|
||||
&KeyringAuthStorage::field_key(key, TOKENS_FIELD),
|
||||
&serde_json::to_string(&auth.tokens)?,
|
||||
)?;
|
||||
mock_keyring.save(
|
||||
KEYRING_SERVICE,
|
||||
&KeyringAuthStorage::field_key(key, LAST_REFRESH_FIELD),
|
||||
&serde_json::to_string(&auth.last_refresh)?,
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn assert_keyring_saved_auth_and_removed_fallback(
|
||||
mock_keyring: &MockKeyringStore,
|
||||
key: &str,
|
||||
@@ -144,6 +172,44 @@ fn assert_keyring_saved_auth_and_removed_fallback(
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_windows_keyring_saved_auth_and_removed_fallback(
|
||||
mock_keyring: &MockKeyringStore,
|
||||
key: &str,
|
||||
codex_home: &Path,
|
||||
expected: &AuthDotJson,
|
||||
) {
|
||||
let auth_mode_key = KeyringAuthStorage::field_key(key, AUTH_MODE_FIELD);
|
||||
let openai_api_key_key = KeyringAuthStorage::field_key(key, OPENAI_API_KEY_FIELD);
|
||||
let tokens_key = KeyringAuthStorage::field_key(key, TOKENS_FIELD);
|
||||
let last_refresh_key = KeyringAuthStorage::field_key(key, LAST_REFRESH_FIELD);
|
||||
|
||||
assert_eq!(
|
||||
mock_keyring.saved_value(&auth_mode_key),
|
||||
Some(serde_json::to_string(&expected.auth_mode).expect("serialize auth_mode")),
|
||||
);
|
||||
assert_eq!(
|
||||
mock_keyring.saved_value(&openai_api_key_key),
|
||||
Some(serde_json::to_string(&expected.openai_api_key).expect("serialize openai_api_key")),
|
||||
);
|
||||
assert_eq!(
|
||||
mock_keyring.saved_value(&tokens_key),
|
||||
Some(serde_json::to_string(&expected.tokens).expect("serialize tokens")),
|
||||
);
|
||||
assert_eq!(
|
||||
mock_keyring.saved_value(&last_refresh_key),
|
||||
Some(serde_json::to_string(&expected.last_refresh).expect("serialize last_refresh")),
|
||||
);
|
||||
assert!(
|
||||
mock_keyring.saved_value(key).is_none(),
|
||||
"legacy whole-auth key should be removed after windows keyring save"
|
||||
);
|
||||
let auth_file = get_auth_file(codex_home);
|
||||
assert!(
|
||||
!auth_file.exists(),
|
||||
"fallback auth.json should be removed after keyring save"
|
||||
);
|
||||
}
|
||||
|
||||
fn id_token_with_prefix(prefix: &str) -> IdTokenInfo {
|
||||
#[derive(Serialize)]
|
||||
struct Header {
|
||||
@@ -275,6 +341,108 @@ fn keyring_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyring_auth_storage_windows_load_combines_split_entries() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
let mock_keyring = MockKeyringStore::default();
|
||||
let storage = KeyringAuthStorage::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(mock_keyring.clone()),
|
||||
);
|
||||
let expected = auth_with_prefix("windows-load");
|
||||
let key = compute_store_key(codex_home.path())?;
|
||||
seed_windows_keyring_with_auth(&mock_keyring, &key, &expected)?;
|
||||
|
||||
let loaded = storage.load_windows_auth_from_keyring(&key)?;
|
||||
|
||||
assert_eq!(Some(expected), loaded);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyring_auth_storage_windows_load_ignores_legacy_entry() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
let mock_keyring = MockKeyringStore::default();
|
||||
let storage = KeyringAuthStorage::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(mock_keyring.clone()),
|
||||
);
|
||||
let expected = auth_with_prefix("windows-legacy");
|
||||
let key = compute_store_key(codex_home.path())?;
|
||||
seed_keyring_with_auth(&mock_keyring, || Ok(key.clone()), &expected)?;
|
||||
|
||||
let loaded = storage.load_windows_auth_from_keyring(&key)?;
|
||||
|
||||
assert_eq!(None, loaded);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyring_auth_storage_windows_save_persists_split_entries() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
let mock_keyring = MockKeyringStore::default();
|
||||
let storage = KeyringAuthStorage::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(mock_keyring.clone()),
|
||||
);
|
||||
let auth_file = get_auth_file(codex_home.path());
|
||||
std::fs::write(&auth_file, "stale")?;
|
||||
let auth = auth_with_prefix("windows-save");
|
||||
let key = compute_store_key(codex_home.path())?;
|
||||
mock_keyring.save(KEYRING_SERVICE, &key, "legacy-auth")?;
|
||||
|
||||
storage.save_windows_auth_to_keyring(&key, &auth)?;
|
||||
if let Err(err) = delete_file_if_exists(codex_home.path()) {
|
||||
panic!("failed to remove fallback auth file in test: {err}");
|
||||
}
|
||||
|
||||
assert_windows_keyring_saved_auth_and_removed_fallback(
|
||||
&mock_keyring,
|
||||
&key,
|
||||
codex_home.path(),
|
||||
&auth,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keyring_auth_storage_windows_delete_removes_split_entries_and_file() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
let mock_keyring = MockKeyringStore::default();
|
||||
let storage = KeyringAuthStorage::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(mock_keyring.clone()),
|
||||
);
|
||||
let key = compute_store_key(codex_home.path())?;
|
||||
let auth = auth_with_prefix("windows-delete");
|
||||
seed_windows_keyring_with_auth(&mock_keyring, &key, &auth)?;
|
||||
mock_keyring.save(KEYRING_SERVICE, &key, "legacy-auth")?;
|
||||
let auth_file = get_auth_file(codex_home.path());
|
||||
std::fs::write(&auth_file, "stale")?;
|
||||
|
||||
let removed = storage.delete_windows_auth_from_keyring(&key)?;
|
||||
let file_removed = delete_file_if_exists(codex_home.path())?;
|
||||
|
||||
assert!(removed, "delete should report removal");
|
||||
assert!(file_removed, "fallback auth.json should be removed");
|
||||
assert!(!mock_keyring.contains(&KeyringAuthStorage::field_key(
|
||||
key.as_str(),
|
||||
AUTH_MODE_FIELD
|
||||
)));
|
||||
assert!(!mock_keyring.contains(&KeyringAuthStorage::field_key(
|
||||
key.as_str(),
|
||||
OPENAI_API_KEY_FIELD,
|
||||
)));
|
||||
assert!(!mock_keyring.contains(&KeyringAuthStorage::field_key(key.as_str(), TOKENS_FIELD)));
|
||||
assert!(!mock_keyring.contains(&KeyringAuthStorage::field_key(
|
||||
key.as_str(),
|
||||
LAST_REFRESH_FIELD,
|
||||
)));
|
||||
assert!(!mock_keyring.contains(&key));
|
||||
assert!(!auth_file.exists());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_auth_storage_load_prefers_keyring_value() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
|
||||
Reference in New Issue
Block a user