diff --git a/Cargo.lock b/Cargo.lock index 0b6cb52c6..528868b18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14358,6 +14358,7 @@ dependencies = [ "tokio", "tokio-util", "toml 0.8.23", + "toml_edit 0.25.6+spec-1.1.0", "tracing", "typed-path 0.10.0", "ui_components", diff --git a/app/Cargo.toml b/app/Cargo.toml index 740316757..ad77329e5 100644 --- a/app/Cargo.toml +++ b/app/Cargo.toml @@ -199,6 +199,7 @@ tikv-jemallocator = { version = "0.6", optional = true, features = [ "override_allocator_on_supported_platforms", ] } toml = "0.8.13" +toml_edit.workspace = true tracing.workspace = true ui_components.workspace = true unicase = "2.7.0" diff --git a/app/src/ai/agent_sdk/driver/harness/codex.rs b/app/src/ai/agent_sdk/driver/harness/codex.rs index 1560dbede..37ccc1da6 100644 --- a/app/src/ai/agent_sdk/driver/harness/codex.rs +++ b/app/src/ai/agent_sdk/driver/harness/codex.rs @@ -6,6 +6,8 @@ use std::sync::Arc; use anyhow::{Context, Result}; use async_trait::async_trait; use parking_lot::Mutex; +use serde::{Deserialize, Serialize}; +use serde_json::{Map, Value}; use tempfile::NamedTempFile; use warp_cli::agent::Harness; use warp_managed_secrets::ManagedSecretValue; @@ -20,6 +22,7 @@ use crate::terminal::CLIAgent; use super::super::terminal::{CommandHandle, TerminalDriver}; use super::super::{AgentDriver, AgentDriverError}; +use super::json_utils::read_json_file_or_default; use super::{write_temp_file, HarnessRunner, ResumePayload, SavePoint, ThirdPartyHarness}; pub(crate) struct CodexHarness; @@ -46,11 +49,11 @@ impl ThirdPartyHarness for CodexHarness { fn prepare_environment_config( &self, - _working_dir: &Path, + working_dir: &Path, system_prompt: Option<&str>, - _secrets: &HashMap, + secrets: &HashMap, ) -> Result<(), AgentDriverError> { - prepare_codex_environment_config(system_prompt).map_err(|error| { + prepare_codex_environment_config(working_dir, system_prompt, secrets).map_err(|error| { AgentDriverError::HarnessConfigSetupFailed { harness: self.cli_agent().command_prefix().to_owned(), error, @@ -213,14 +216,41 @@ impl HarnessRunner for CodexHarnessRunner { const CODEX_CONFIG_DIR: &str = ".codex"; const CODEX_AGENTS_OVERRIDE_FILE_NAME: &str = "AGENTS.override.md"; +const CODEX_AUTH_FILE_NAME: &str = "auth.json"; +const CODEX_CONFIG_TOML_FILE_NAME: &str = "config.toml"; +const OPENAI_API_KEY_ENV: &str = "OPENAI_API_KEY"; +const CODEX_AUTH_MODE_API_KEY: &str = "apikey"; +/// Lowercase string Codex's `TrustLevel` enum serializes to (codex +/// `protocol/src/config_types.rs::TrustLevel`). +const CODEX_TRUST_LEVEL_TRUSTED: &str = "trusted"; +/// Top-level config key codex reads to override the built-in `openai` provider's base URL +/// (codex `core/src/config/mod.rs`). +const CODEX_OPENAI_BASE_URL_KEY: &str = "openai_base_url"; +/// US data-residency endpoint. Our OpenAI keys are issued under a US-residency project, +/// which rejects requests to the global host with `401 incorrect_hostname`. +/// TODO(REMOTE-1509): plumb a region-tagged auth secret instead of hardcoding the URL. +const CODEX_OPENAI_BASE_URL: &str = "https://us.api.openai.com/v1"; -fn prepare_codex_environment_config(system_prompt: Option<&str>) -> Result<()> { - let Some(prompt) = system_prompt else { - return Ok(()); - }; +fn prepare_codex_environment_config( + working_dir: &Path, + system_prompt: Option<&str>, + secrets: &HashMap, +) -> Result<()> { let home_dir = dirs::home_dir().ok_or_else(|| anyhow::anyhow!("could not determine home directory"))?; - write_codex_agents_override(&home_dir.join(CODEX_CONFIG_DIR), prompt) + let codex_dir = home_dir.join(CODEX_CONFIG_DIR); + + if let Some(prompt) = system_prompt { + write_codex_agents_override(&codex_dir, prompt)?; + } + + match resolve_openai_api_key(secrets) { + Some(api_key) => prepare_codex_auth(&codex_dir.join(CODEX_AUTH_FILE_NAME), &api_key)?, + None => log::info!("No OPENAI_API_KEY available; skipping Codex auth.json seed"), + } + + prepare_codex_config_toml(&codex_dir.join(CODEX_CONFIG_TOML_FILE_NAME), working_dir)?; + Ok(()) } fn write_codex_agents_override(codex_dir: &Path, system_prompt: &str) -> Result<()> { @@ -241,3 +271,182 @@ fn write_codex_agents_override(codex_dir: &Path, system_prompt: &str) -> Result< ) }) } + +/// Mirrors the subset of Codex's `AuthDotJson` (codex `login/src/auth/storage.rs`) that we +/// need to seed. Unknown fields (`tokens`, `last_refresh`, `agent_identity`, ...) are +/// preserved via `extra` so we don't clobber an existing login. +#[derive(Default, Deserialize, Serialize, Debug)] +struct CodexAuthDotJson { + #[serde(default, skip_serializing_if = "Option::is_none")] + auth_mode: Option, + #[serde( + rename = "OPENAI_API_KEY", + default, + skip_serializing_if = "Option::is_none" + )] + openai_api_key: Option, + #[serde(flatten)] + extra: Map, +} + +fn prepare_codex_auth(auth_path: &Path, api_key: &str) -> Result<()> { + let mut auth: CodexAuthDotJson = read_json_file_or_default(auth_path)?; + auth.openai_api_key = Some(api_key.to_owned()); + if auth.auth_mode.is_none() { + auth.auth_mode = Some(CODEX_AUTH_MODE_API_KEY.to_owned()); + } + write_codex_auth_json(auth_path, &auth) +} + +/// Write Codex's `auth.json` with restrictive (0o600) permissions, mirroring how +/// codex sets up this file itself. +fn write_codex_auth_json(path: &Path, auth: &CodexAuthDotJson) -> Result<()> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("Failed to create {}", parent.display()))?; + } + let bytes = serde_json::to_vec_pretty(auth).context("Failed to serialize Codex auth.json")?; + + #[cfg(unix)] + { + use std::io::Write as _; + use std::os::unix::fs::OpenOptionsExt; + let mut file = fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path) + .with_context(|| format!("Failed to open {} for writing", path.display()))?; + file.write_all(&bytes) + .with_context(|| format!("Failed to write {}", path.display()))?; + } + #[cfg(not(unix))] + fs::write(path, &bytes).with_context(|| format!("Failed to write {}", path.display()))?; + + Ok(()) +} + +/// Returns the OpenAI API key for Codex auth, preferring the `OPENAI_API_KEY` env +/// var so the seeded `auth.json` matches the credential the launched Codex process +/// will see. [`AgentDriver::new`] skips a managed `OPENAI_API_KEY` secret when the +/// env var is already set, so we mirror that precedence here. +fn resolve_openai_api_key(secrets: &HashMap) -> Option { + if let Ok(value) = std::env::var(OPENAI_API_KEY_ENV) { + let trimmed = value.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_owned()); + } + } + if let Some(ManagedSecretValue::RawValue { value }) = secrets.get(OPENAI_API_KEY_ENV) { + let trimmed = value.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_owned()); + } + } + None +} + +/// Edit `~/.codex/config.toml` via `toml_edit` to seed the harness defaults +/// while preserving anything that might already exist there. We handle: +/// - project trust: for a working dir and all of its git repo subdirectories, +/// set the projects to `trusted`. +/// - base URL: set `openai_base_url = ""` so we +/// hit the regional host our API keys require. +fn prepare_codex_config_toml(config_toml_path: &Path, working_dir: &Path) -> Result<()> { + let existing = match fs::read_to_string(config_toml_path) { + Ok(content) => content, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(e) => { + return Err(anyhow::Error::from(e).context(format!( + "Failed to read Codex config.toml at {}", + config_toml_path.display() + ))); + } + }; + let mut doc: toml_edit::DocumentMut = existing.parse().with_context(|| { + format!( + "Failed to parse Codex config.toml at {}", + config_toml_path.display() + ) + })?; + + set_codex_openai_base_url(&mut doc, CODEX_OPENAI_BASE_URL); + + let canonical = working_dir.canonicalize().with_context(|| { + format!( + "Failed to canonicalize Codex working dir at {}", + working_dir.display() + ) + })?; + let project_key = canonical.to_string_lossy().into_owned(); + set_codex_project_trust_level(&mut doc, &project_key, CODEX_TRUST_LEVEL_TRUSTED); + + // Codex's trust check is not recursive (see openai/codex#19426) -- since we + // clone the git repos into workspace/ for cloud agents, we usually have git + // repo children that we also want to trust. + for child_repo in find_child_git_repos(&canonical) { + let key = child_repo.to_string_lossy().into_owned(); + set_codex_project_trust_level(&mut doc, &key, CODEX_TRUST_LEVEL_TRUSTED); + } + + if let Some(parent) = config_toml_path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!("Failed to create Codex config dir at {}", parent.display()) + })?; + } + fs::write(config_toml_path, doc.to_string()).with_context(|| { + format!( + "Failed to write Codex config.toml at {}", + config_toml_path.display() + ) + }) +} + +/// Set the top-level `openai_base_url` key, overwriting any existing value. +fn set_codex_openai_base_url(doc: &mut toml_edit::DocumentMut, base_url: &str) { + doc[CODEX_OPENAI_BASE_URL_KEY] = toml_edit::value(base_url); +} + +/// Return immediate subdirectories of `dir` that contain a `.git`. +fn find_child_git_repos(dir: &Path) -> Vec { + let Ok(entries) = fs::read_dir(dir) else { + return Vec::new(); + }; + entries + .flatten() + .filter_map(|entry| { + let path = entry.path(); + (path.is_dir() && path.join(".git").exists()).then_some(path) + }) + .collect() +} + +/// Insert/update `[projects.""] trust_level = `. +/// +/// Codex itself always writes `projects` as an explicit table, so we don't +/// handle the inline-table form here. +fn set_codex_project_trust_level( + doc: &mut toml_edit::DocumentMut, + project_key: &str, + trust_level: &str, +) { + if !doc.contains_table("projects") { + let mut projects_tbl = toml_edit::Table::new(); + projects_tbl.set_implicit(true); + doc.insert("projects", toml_edit::Item::Table(projects_tbl)); + } + let proj_tbl = doc["projects"] + .as_table_mut() + .expect("projects table inserted above") + .entry(project_key) + .or_insert_with(toml_edit::table) + .as_table_mut() + .expect("project entry is a table"); + proj_tbl.set_implicit(false); + proj_tbl["trust_level"] = toml_edit::value(trust_level); +} + +#[cfg(test)] +#[path = "codex_tests.rs"] +mod tests; diff --git a/app/src/ai/agent_sdk/driver/harness/codex_tests.rs b/app/src/ai/agent_sdk/driver/harness/codex_tests.rs new file mode 100644 index 000000000..c477d71f4 --- /dev/null +++ b/app/src/ai/agent_sdk/driver/harness/codex_tests.rs @@ -0,0 +1,326 @@ +use std::collections::HashMap; +use std::fs; + +use serde_json::Value; +use tempfile::TempDir; + +use super::*; + +#[test] +fn prepare_codex_auth_writes_fresh_file_with_api_key_mode() { + let tmp = TempDir::new().unwrap(); + let auth_path = tmp.path().join(".codex/auth.json"); + + prepare_codex_auth(&auth_path, "sk-test-key").unwrap(); + + let auth: Value = serde_json::from_slice(&fs::read(&auth_path).unwrap()).unwrap(); + assert_eq!(auth["OPENAI_API_KEY"], "sk-test-key"); + assert_eq!(auth["auth_mode"], "apikey"); +} + +#[test] +fn prepare_codex_auth_preserves_unrelated_fields() { + let tmp = TempDir::new().unwrap(); + let auth_path = tmp.path().join("auth.json"); + fs::write( + &auth_path, + r#"{"tokens":{"access_token":"tok"},"last_refresh":"2026-01-01T00:00:00Z"}"#, + ) + .unwrap(); + + prepare_codex_auth(&auth_path, "sk-new-key").unwrap(); + + let auth: Value = serde_json::from_slice(&fs::read(&auth_path).unwrap()).unwrap(); + assert_eq!(auth["OPENAI_API_KEY"], "sk-new-key"); + assert_eq!(auth["auth_mode"], "apikey"); + assert_eq!(auth["tokens"]["access_token"], "tok"); + assert_eq!(auth["last_refresh"], "2026-01-01T00:00:00Z"); +} + +#[test] +fn prepare_codex_auth_does_not_overwrite_existing_auth_mode() { + let tmp = TempDir::new().unwrap(); + let auth_path = tmp.path().join("auth.json"); + fs::write(&auth_path, r#"{"auth_mode":"Chatgpt"}"#).unwrap(); + + prepare_codex_auth(&auth_path, "sk-new-key").unwrap(); + + let auth: Value = serde_json::from_slice(&fs::read(&auth_path).unwrap()).unwrap(); + assert_eq!(auth["auth_mode"], "Chatgpt"); + assert_eq!(auth["OPENAI_API_KEY"], "sk-new-key"); +} + +#[test] +fn prepare_codex_auth_overwrites_stale_openai_api_key() { + let tmp = TempDir::new().unwrap(); + let auth_path = tmp.path().join("auth.json"); + fs::write( + &auth_path, + r#"{"auth_mode":"apikey","OPENAI_API_KEY":"sk-old"}"#, + ) + .unwrap(); + + prepare_codex_auth(&auth_path, "sk-new").unwrap(); + + let auth: Value = serde_json::from_slice(&fs::read(&auth_path).unwrap()).unwrap(); + assert_eq!(auth["OPENAI_API_KEY"], "sk-new"); +} + +#[cfg(unix)] +#[test] +fn prepare_codex_auth_writes_with_0600_perms() { + use std::os::unix::fs::PermissionsExt; + let tmp = TempDir::new().unwrap(); + let auth_path = tmp.path().join(".codex/auth.json"); + + prepare_codex_auth(&auth_path, "sk-test-key").unwrap(); + + let mode = fs::metadata(&auth_path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); +} + +#[test] +fn resolve_openai_api_key_returns_value_from_raw_value_secret() { + let secrets = HashMap::from([( + "OPENAI_API_KEY".to_string(), + ManagedSecretValue::raw_value("sk-from-secret"), + )]); + assert_eq!( + resolve_openai_api_key(&secrets).as_deref(), + Some("sk-from-secret") + ); +} + +#[test] +#[serial_test::serial] +fn resolve_openai_api_key_falls_back_to_env_var() { + let prev = std::env::var(OPENAI_API_KEY_ENV).ok(); + std::env::set_var(OPENAI_API_KEY_ENV, "sk-from-env"); + + let result = resolve_openai_api_key(&HashMap::new()); + + match prev { + Some(v) => std::env::set_var(OPENAI_API_KEY_ENV, v), + None => std::env::remove_var(OPENAI_API_KEY_ENV), + } + assert_eq!(result.as_deref(), Some("sk-from-env")); +} + +#[test] +#[serial_test::serial] +fn resolve_openai_api_key_returns_none_when_secrets_and_env_empty() { + let prev = std::env::var(OPENAI_API_KEY_ENV).ok(); + std::env::remove_var(OPENAI_API_KEY_ENV); + + let result = resolve_openai_api_key(&HashMap::new()); + + if let Some(v) = prev { + std::env::set_var(OPENAI_API_KEY_ENV, v); + } + assert_eq!(result, None); +} + +#[test] +#[serial_test::serial] +fn resolve_openai_api_key_prefers_env_over_secret() { + // Mirrors `AgentDriver::new`'s precedence: an existing `OPENAI_API_KEY` env var + // wins over a managed secret so `auth.json` matches the launched process's env. + let prev = std::env::var(OPENAI_API_KEY_ENV).ok(); + std::env::set_var(OPENAI_API_KEY_ENV, "sk-from-env"); + let secrets = HashMap::from([( + "OPENAI_API_KEY".to_string(), + ManagedSecretValue::raw_value("sk-from-secret"), + )]); + + let result = resolve_openai_api_key(&secrets); + + match prev { + Some(v) => std::env::set_var(OPENAI_API_KEY_ENV, v), + None => std::env::remove_var(OPENAI_API_KEY_ENV), + } + assert_eq!(result.as_deref(), Some("sk-from-env")); +} + +#[test] +#[serial_test::serial] +fn resolve_openai_api_key_uses_secret_when_env_empty() { + let prev = std::env::var(OPENAI_API_KEY_ENV).ok(); + std::env::set_var(OPENAI_API_KEY_ENV, " "); + let secrets = HashMap::from([( + "OPENAI_API_KEY".to_string(), + ManagedSecretValue::raw_value("sk-from-secret"), + )]); + + let result = resolve_openai_api_key(&secrets); + + match prev { + Some(v) => std::env::set_var(OPENAI_API_KEY_ENV, v), + None => std::env::remove_var(OPENAI_API_KEY_ENV), + } + assert_eq!(result.as_deref(), Some("sk-from-secret")); +} + +fn read_codex_config(path: &std::path::Path) -> toml::Table { + let content = fs::read_to_string(path).unwrap(); + toml::from_str(&content).unwrap() +} + +#[test] +fn prepare_codex_config_toml_writes_fresh_config() { + let tmp = TempDir::new().unwrap(); + let config_path = tmp.path().join(".codex/config.toml"); + let working_dir = tmp.path().join("workspace/proj"); + fs::create_dir_all(&working_dir).unwrap(); + + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + + let canonical = working_dir.canonicalize().unwrap(); + let key = canonical.to_string_lossy().into_owned(); + let cfg = read_codex_config(&config_path); + assert_eq!( + cfg["projects"][&key]["trust_level"].as_str(), + Some("trusted") + ); + assert_eq!(cfg["openai_base_url"].as_str(), Some(CODEX_OPENAI_BASE_URL)); +} + +#[test] +fn prepare_codex_config_toml_preserves_unrelated_keys() { + let tmp = TempDir::new().unwrap(); + let config_path = tmp.path().join("config.toml"); + let working_dir = tmp.path().join("workspace"); + fs::create_dir_all(&working_dir).unwrap(); + fs::write( + &config_path, + "model = \"gpt-5\"\n\n[projects.\"/other/path\"]\ntrust_level = \"trusted\"\n", + ) + .unwrap(); + + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + + let canonical = working_dir.canonicalize().unwrap(); + let key = canonical.to_string_lossy().into_owned(); + let cfg = read_codex_config(&config_path); + assert_eq!(cfg["model"].as_str(), Some("gpt-5")); + assert_eq!( + cfg["projects"]["/other/path"]["trust_level"].as_str(), + Some("trusted") + ); + assert_eq!( + cfg["projects"][&key]["trust_level"].as_str(), + Some("trusted") + ); +} + +#[test] +fn prepare_codex_config_toml_is_idempotent() { + let tmp = TempDir::new().unwrap(); + let config_path = tmp.path().join("config.toml"); + let working_dir = tmp.path().join("workspace"); + fs::create_dir_all(&working_dir).unwrap(); + + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + let after_first = fs::read_to_string(&config_path).unwrap(); + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + let after_second = fs::read_to_string(&config_path).unwrap(); + + assert_eq!(after_first, after_second); + let canonical = working_dir.canonicalize().unwrap(); + let key = canonical.to_string_lossy().into_owned(); + let cfg: toml::Table = toml::from_str(&after_second).unwrap(); + let projects = cfg["projects"].as_table().unwrap(); + assert_eq!(projects.len(), 1); + assert_eq!(projects[&key]["trust_level"].as_str(), Some("trusted")); + assert_eq!(cfg["openai_base_url"].as_str(), Some(CODEX_OPENAI_BASE_URL)); +} + +#[test] +fn prepare_codex_config_toml_upgrades_untrusted_entry() { + let tmp = TempDir::new().unwrap(); + let config_path = tmp.path().join("config.toml"); + let working_dir = tmp.path().join("workspace"); + fs::create_dir_all(&working_dir).unwrap(); + let canonical = working_dir.canonicalize().unwrap(); + let key = canonical.to_string_lossy().into_owned(); + // Use a TOML literal-string key ('...') so Windows backslashes in `key` + // (e.g. `\\?\C:\...`) are not interpreted as escape sequences. + fs::write( + &config_path, + format!("[projects.'{key}']\ntrust_level = \"untrusted\"\n"), + ) + .unwrap(); + + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + + let cfg = read_codex_config(&config_path); + assert_eq!( + cfg["projects"][&key]["trust_level"].as_str(), + Some("trusted") + ); +} + +#[test] +fn prepare_codex_config_toml_trusts_multiple_child_repos() { + let tmp = TempDir::new().unwrap(); + let config_path = tmp.path().join("config.toml"); + let working_dir = tmp.path().join("workspace"); + let repo_a = working_dir.join("a"); + let repo_b = working_dir.join("b"); + fs::create_dir_all(repo_a.join(".git")).unwrap(); + fs::create_dir_all(repo_b.join(".git")).unwrap(); + + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + + let cfg = read_codex_config(&config_path); + let projects = cfg["projects"].as_table().unwrap(); + let canonical_a = repo_a.canonicalize().unwrap(); + let canonical_b = repo_b.canonicalize().unwrap(); + assert_eq!( + projects[canonical_a.to_str().unwrap()]["trust_level"].as_str(), + Some("trusted") + ); + assert_eq!( + projects[canonical_b.to_str().unwrap()]["trust_level"].as_str(), + Some("trusted") + ); +} + +#[test] +fn prepare_codex_config_toml_overwrites_stale_openai_base_url() { + let tmp = TempDir::new().unwrap(); + let config_path = tmp.path().join("config.toml"); + let working_dir = tmp.path().join("workspace"); + fs::create_dir_all(&working_dir).unwrap(); + fs::write( + &config_path, + "openai_base_url = \"https://api.openai.com/v1\"\n", + ) + .unwrap(); + + prepare_codex_config_toml(&config_path, &working_dir).unwrap(); + + let cfg = read_codex_config(&config_path); + assert_eq!(cfg["openai_base_url"].as_str(), Some(CODEX_OPENAI_BASE_URL)); +} + +#[test] +fn find_child_git_repos_returns_only_repo_children() { + let tmp = TempDir::new().unwrap(); + let workspace = tmp.path().join("workspace"); + let repo = workspace.join("repo"); + let other = workspace.join("other"); + fs::create_dir_all(repo.join(".git")).unwrap(); + fs::create_dir_all(&other).unwrap(); + + let found = find_child_git_repos(&workspace); + let canonical_repo = repo.canonicalize().unwrap(); + assert_eq!(found.len(), 1); + assert_eq!(found[0].canonicalize().unwrap(), canonical_repo); +} + +#[test] +fn find_child_git_repos_returns_empty_when_dir_missing() { + let tmp = TempDir::new().unwrap(); + let missing = tmp.path().join("does-not-exist"); + assert!(find_child_git_repos(&missing).is_empty()); +}