diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 95f4fecc48..9ec94496dc 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -865,6 +865,7 @@ dependencies = [ "anyhow", "clap", "codex-protocol", + "codex-utils-absolute-path", "mcp-types", "pretty_assertions", "schemars 0.8.22", @@ -1183,6 +1184,7 @@ dependencies = [ "codex-common", "codex-core", "codex-protocol", + "codex-utils-absolute-path", "core_test_support", "libc", "mcp-types", @@ -1446,6 +1448,7 @@ version = "0.0.0" dependencies = [ "anyhow", "codex-git", + "codex-utils-absolute-path", "codex-utils-image", "icu_decimal", "icu_locale_core", @@ -1544,6 +1547,7 @@ dependencies = [ "codex-file-search", "codex-login", "codex-protocol", + "codex-utils-absolute-path", "codex-windows-sandbox", "color-eyre", "crossterm", @@ -1613,6 +1617,7 @@ dependencies = [ "codex-login", "codex-protocol", "codex-tui", + "codex-utils-absolute-path", "codex-windows-sandbox", "color-eyre", "crossterm", @@ -1665,9 +1670,11 @@ name = "codex-utils-absolute-path" version = "0.0.0" dependencies = [ "path-absolutize", + "schemars 0.8.22", "serde", "serde_json", "tempfile", + "ts-rs", ] [[package]] diff --git a/codex-rs/app-server-protocol/Cargo.toml b/codex-rs/app-server-protocol/Cargo.toml index c1a251c656..1c21bd6ea0 100644 --- a/codex-rs/app-server-protocol/Cargo.toml +++ b/codex-rs/app-server-protocol/Cargo.toml @@ -15,6 +15,7 @@ workspace = true anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } mcp-types = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 853cb03b40..df39f8809e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -16,6 +16,7 @@ use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::TurnAbortReason; +use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -359,7 +360,7 @@ pub struct Tools { #[serde(rename_all = "camelCase")] pub struct SandboxSettings { #[serde(default)] - pub writable_roots: Vec, + pub writable_roots: Vec, pub network_access: Option, pub exclude_tmpdir_env_var: Option, pub exclude_slash_tmp: Option, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index edd3aefa74..a0772a17b8 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -24,6 +24,7 @@ use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::TokenUsage as CoreTokenUsage; use codex_protocol::protocol::TokenUsageInfo as CoreTokenUsageInfo; use codex_protocol::user_input::UserInput as CoreUserInput; +use codex_utils_absolute_path::AbsolutePathBuf; use mcp_types::ContentBlock as McpContentBlock; use mcp_types::Resource as McpResource; use mcp_types::ResourceTemplate as McpResourceTemplate; @@ -420,7 +421,7 @@ pub enum SandboxPolicy { #[ts(rename_all = "camelCase")] WorkspaceWrite { #[serde(default)] - writable_roots: Vec, + writable_roots: Vec, #[serde(default)] network_access: bool, #[serde(default)] @@ -668,10 +669,21 @@ pub struct CancelLoginAccountParams { pub login_id: String, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub enum CancelLoginAccountStatus { + Canceled, + NotFound, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] -pub struct CancelLoginAccountResponse {} +pub struct CancelLoginAccountResponse { + pub status: CancelLoginAccountStatus, +} #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index ee28637b2a..e9dee71271 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -65,6 +65,8 @@ Example (from OpenAI's official VSCode extension): - `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation). - `model/list` — list available models (with reasoning effort options). +- `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes. +- `mcpServers/list` — enumerate configured MCP servers with their tools, resources, resource templates, and auth status; supports cursor+limit pagination. - `feedback/upload` — submit a feedback report (classification + optional reason/logs and conversation_id); returns the tracking thread id. - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation). - `config/read` — fetch the effective config on disk after resolving config layering. @@ -366,6 +368,8 @@ The JSON-RPC auth/account surface exposes request/response methods plus server-i - `account/logout` — sign out; triggers `account/updated`. - `account/updated` (notify) — emitted whenever auth mode changes (`authMode`: `apikey`, `chatgpt`, or `null`). - `account/rateLimits/read` — fetch ChatGPT rate limits; updates arrive via `account/rateLimits/updated` (notify). +- `account/rateLimits/updated` (notify) — emitted whenever a user's ChatGPT rate limits change. +- `mcpServer/oauthLogin/completed` (notify) — emitted after a `mcpServer/oauth/login` flow finishes for a server; payload includes `{ name, success, error? }`. ### 1) Check auth state diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 3df940889f..c62734f24e 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -19,6 +19,7 @@ use codex_app_server_protocol::AuthMode; use codex_app_server_protocol::AuthStatusChangeNotification; use codex_app_server_protocol::CancelLoginAccountParams; use codex_app_server_protocol::CancelLoginAccountResponse; +use codex_app_server_protocol::CancelLoginAccountStatus; use codex_app_server_protocol::CancelLoginChatGptResponse; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::CommandExecParams; @@ -195,6 +196,11 @@ struct ActiveLogin { login_id: Uuid, } +#[derive(Clone, Copy, Debug)] +enum CancelLoginError { + NotFound(Uuid), +} + impl Drop for ActiveLogin { fn drop(&mut self) { self.shutdown_handle.shutdown(); @@ -828,7 +834,7 @@ impl CodexMessageProcessor { async fn cancel_login_chatgpt_common( &mut self, login_id: Uuid, - ) -> std::result::Result<(), JSONRPCErrorError> { + ) -> std::result::Result<(), CancelLoginError> { let mut guard = self.active_login.lock().await; if guard.as_ref().map(|l| l.login_id) == Some(login_id) { if let Some(active) = guard.take() { @@ -836,11 +842,7 @@ impl CodexMessageProcessor { } Ok(()) } else { - Err(JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("login id not found: {login_id}"), - data: None, - }) + Err(CancelLoginError::NotFound(login_id)) } } @@ -851,7 +853,12 @@ impl CodexMessageProcessor { .send_response(request_id, CancelLoginChatGptResponse {}) .await; } - Err(error) => { + Err(CancelLoginError::NotFound(missing_login_id)) => { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: format!("login id not found: {missing_login_id}"), + data: None, + }; self.outgoing.send_error(request_id, error).await; } } @@ -860,16 +867,14 @@ impl CodexMessageProcessor { async fn cancel_login_v2(&mut self, request_id: RequestId, params: CancelLoginAccountParams) { let login_id = params.login_id; match Uuid::parse_str(&login_id) { - Ok(uuid) => match self.cancel_login_chatgpt_common(uuid).await { - Ok(()) => { - self.outgoing - .send_response(request_id, CancelLoginAccountResponse {}) - .await; - } - Err(error) => { - self.outgoing.send_error(request_id, error).await; - } - }, + Ok(uuid) => { + let status = match self.cancel_login_chatgpt_common(uuid).await { + Ok(()) => CancelLoginAccountStatus::Canceled, + Err(CancelLoginError::NotFound(_)) => CancelLoginAccountStatus::NotFound, + }; + let response = CancelLoginAccountResponse { status }; + self.outgoing.send_response(request_id, response).await; + } Err(_) => { let error = JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index e417198994..be94dd822e 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -410,7 +410,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( cwd: first_cwd.clone(), approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::WorkspaceWrite { - writable_roots: vec![first_cwd.clone()], + writable_roots: vec![first_cwd.try_into()?], network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/app-server/tests/suite/config.rs b/codex-rs/app-server/tests/suite/config.rs index 88e74a6fb4..29af70181b 100644 --- a/codex-rs/app-server/tests/suite/config.rs +++ b/codex-rs/app-server/tests/suite/config.rs @@ -80,7 +80,7 @@ async fn get_config_toml_parses_all_fields() -> Result<()> { approval_policy: Some(AskForApproval::OnRequest), sandbox_mode: Some(SandboxMode::WorkspaceWrite), sandbox_settings: Some(SandboxSettings { - writable_roots: vec!["/tmp".into()], + writable_roots: vec!["/tmp".try_into()?], network_access: Some(true), exclude_tmpdir_env_var: Some(true), exclude_slash_tmp: Some(true), diff --git a/codex-rs/app-server/tests/suite/login.rs b/codex-rs/app-server/tests/suite/login.rs index c5470c3ec4..e252bcb0c0 100644 --- a/codex-rs/app-server/tests/suite/login.rs +++ b/codex-rs/app-server/tests/suite/login.rs @@ -1,8 +1,6 @@ use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; -use codex_app_server_protocol::CancelLoginChatGptParams; -use codex_app_server_protocol::CancelLoginChatGptResponse; use codex_app_server_protocol::GetAuthStatusParams; use codex_app_server_protocol::GetAuthStatusResponse; use codex_app_server_protocol::JSONRPCError; @@ -14,7 +12,6 @@ use codex_core::auth::AuthCredentialsStoreMode; use codex_login::login_with_api_key; use serial_test::serial; use std::path::Path; -use std::time::Duration; use tempfile::TempDir; use tokio::time::timeout; @@ -87,48 +84,6 @@ async fn logout_chatgpt_removes_auth() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -// Serialize tests that launch the login server since it binds to a fixed port. -#[serial(login_port)] -async fn login_and_cancel_chatgpt() -> Result<()> { - let codex_home = TempDir::new()?; - create_config_toml(codex_home.path())?; - - let mut mcp = McpProcess::new(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let login_id = mcp.send_login_chat_gpt_request().await?; - let login_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(login_id)), - ) - .await??; - let login: LoginChatGptResponse = to_response(login_resp)?; - - let cancel_id = mcp - .send_cancel_login_chat_gpt_request(CancelLoginChatGptParams { - login_id: login.login_id, - }) - .await?; - let cancel_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(cancel_id)), - ) - .await??; - let _ok: CancelLoginChatGptResponse = to_response(cancel_resp)?; - - // Optionally observe the completion notification; do not fail if it races. - let maybe_note = timeout( - Duration::from_secs(2), - mcp.read_stream_until_notification_message("codex/event/login_chat_gpt_complete"), - ) - .await; - if maybe_note.is_err() { - eprintln!("warning: did not observe login_chat_gpt_complete notification after cancel"); - } - Ok(()) -} - fn create_config_toml_forced_login(codex_home: &Path, forced_method: &str) -> std::io::Result<()> { let config_toml = codex_home.join("config.toml"); let contents = format!( diff --git a/codex-rs/app-server/tests/suite/v2/account.rs b/codex-rs/app-server/tests/suite/v2/account.rs index dd59270739..4d481f3959 100644 --- a/codex-rs/app-server/tests/suite/v2/account.rs +++ b/codex-rs/app-server/tests/suite/v2/account.rs @@ -241,7 +241,7 @@ async fn login_account_chatgpt_rejected_when_forced_api() -> Result<()> { #[tokio::test] // Serialize tests that launch the login server since it binds to a fixed port. #[serial(login_port)] -async fn login_account_chatgpt_start() -> Result<()> { +async fn login_account_chatgpt_start_can_be_cancelled() -> Result<()> { let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), CreateConfigTomlParams::default())?; diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index afc22c7072..1948487d14 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -532,7 +532,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { cwd: Some(first_cwd.clone()), approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { - writable_roots: vec![first_cwd.clone()], + writable_roots: vec![first_cwd.try_into()?], network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e0e6985a39..c8e7fe8646 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -40,9 +40,9 @@ use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::Verbosity; use codex_protocol::openai_models::ReasoningEffort; use codex_rmcp_client::OAuthCredentialsStoreMode; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; use dirs::home_dir; -use dunce::canonicalize; use serde::Deserialize; use similar::DiffableStr; use std::collections::BTreeMap; @@ -1022,13 +1022,10 @@ impl Config { } } }; - let additional_writable_roots: Vec = additional_writable_roots + let additional_writable_roots: Vec = additional_writable_roots .into_iter() - .map(|path| { - let absolute = resolve_path(&resolved_cwd, &path); - canonicalize(&absolute).unwrap_or(absolute) - }) - .collect(); + .map(|path| AbsolutePathBuf::resolve_path_against_base(path, &resolved_cwd)) + .collect::, _>>()?; let active_project = cfg .get_active_project(&resolved_cwd) .unwrap_or(ProjectConfig { trust_level: None }); @@ -1488,7 +1485,7 @@ exclude_slash_tmp = true resolution, SandboxPolicyResolution { policy: SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("/my/workspace")], + writable_roots: vec!["/my/workspace".try_into().unwrap()], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -1533,7 +1530,10 @@ trust_level = "trusted" resolution, SandboxPolicyResolution { policy: SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("/my/workspace")], + writable_roots: vec![ + AbsolutePathBuf::from_absolute_path("/my/workspace") + .expect("absolute path") + ], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -1565,7 +1565,10 @@ trust_level = "trusted" temp_dir.path().to_path_buf(), )?; - let expected_backend = canonicalize(&backend).expect("canonicalize backend directory"); + let expected_backend = AbsolutePathBuf::try_from( + dunce::canonicalize(&backend).expect("canonicalize backend directory"), + ) + .unwrap(); if cfg!(target_os = "windows") { assert!( config.forced_auto_mode_downgraded_on_windows, diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 6648e288a5..b542536ba7 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -406,7 +406,7 @@ impl Notice { #[derive(Deserialize, Debug, Clone, PartialEq, Default)] pub struct SandboxWorkspaceWrite { #[serde(default)] - pub writable_roots: Vec, + pub writable_roots: Vec, #[serde(default)] pub network_access: bool, #[serde(default)] diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/environment_context.rs index 54756bda2d..a014eefe1b 100644 --- a/codex-rs/core/src/environment_context.rs +++ b/codex-rs/core/src/environment_context.rs @@ -1,3 +1,4 @@ +use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; use strum_macros::Display as DeriveDisplay; @@ -27,7 +28,7 @@ pub(crate) struct EnvironmentContext { pub approval_policy: Option, pub sandbox_mode: Option, pub network_access: Option, - pub writable_roots: Option>, + pub writable_roots: Option>, pub shell: Shell, } @@ -203,7 +204,10 @@ mod tests { fn workspace_write_policy(writable_roots: Vec<&str>, network_access: bool) -> SandboxPolicy { SandboxPolicy::WorkspaceWrite { - writable_roots: writable_roots.into_iter().map(PathBuf::from).collect(), + writable_roots: writable_roots + .into_iter() + .map(|s| AbsolutePathBuf::try_from(s).unwrap()) + .collect(), network_access, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index e82e107453..eaa3e2a613 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -188,6 +188,7 @@ fn is_write_patch_constrained_to_writable_paths( #[cfg(test)] mod tests { use super::*; + use codex_utils_absolute_path::AbsolutePathBuf; use tempfile::TempDir; #[test] @@ -228,7 +229,7 @@ mod tests { // With the parent dir explicitly added as a writable root, the // outside write should be permitted. let policy_with_parent = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![parent], + writable_roots: vec![AbsolutePathBuf::try_from(parent).unwrap()], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 8ca7e4357e..5e01b041d0 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -63,7 +63,11 @@ pub(crate) fn create_seatbelt_command_args( for (index, wr) in writable_roots.iter().enumerate() { // Canonicalize to avoid mismatches like /var vs /private/var on macOS. - let canonical_root = wr.root.canonicalize().unwrap_or_else(|_| wr.root.clone()); + let canonical_root = wr + .root + .as_path() + .canonicalize() + .unwrap_or_else(|_| wr.root.to_path_buf()); let root_param = format!("WRITABLE_ROOT_{index}"); file_write_params.push((root_param.clone(), canonical_root)); @@ -75,7 +79,10 @@ pub(crate) fn create_seatbelt_command_args( let mut require_parts: Vec = Vec::new(); require_parts.push(format!("(subpath (param \"{root_param}\"))")); for (subpath_index, ro) in wr.read_only_subpaths.iter().enumerate() { - let canonical_ro = ro.canonicalize().unwrap_or_else(|_| ro.clone()); + let canonical_ro = ro + .as_path() + .canonicalize() + .unwrap_or_else(|_| ro.to_path_buf()); let ro_param = format!("WRITABLE_ROOT_{index}_RO_{subpath_index}"); require_parts .push(format!("(require-not (subpath (param \"{ro_param}\")))")); @@ -182,7 +189,10 @@ mod tests { // Build a policy that only includes the two test roots as writable and // does not automatically include defaults TMPDIR or /tmp. let policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![root_with_git, root_without_git], + writable_roots: vec![root_with_git, root_without_git] + .into_iter() + .map(|p| p.try_into().unwrap()) + .collect(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 94158df6d8..e11139090e 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -11,6 +11,7 @@ use codex_core::shell::Shell; use codex_core::shell::default_user_shell; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::user_input::UserInput; +use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::load_sse_fixture_with_id; use core_test_support::responses::mount_sse_once; use core_test_support::responses::start_mock_server; @@ -317,7 +318,7 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an cwd: None, approval_policy: Some(AskForApproval::Never), sandbox_policy: Some(SandboxPolicy::WorkspaceWrite { - writable_roots: vec![writable.path().to_path_buf()], + writable_roots: vec![writable.path().try_into().unwrap()], network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -507,7 +508,7 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Res cwd: new_cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::WorkspaceWrite { - writable_roots: vec![writable.path().to_path_buf()], + writable_roots: vec![AbsolutePathBuf::try_from(writable.path()).unwrap()], network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/tests/suite/seatbelt.rs b/codex-rs/core/tests/suite/seatbelt.rs index 52150b0511..286bc8791b 100644 --- a/codex-rs/core/tests/suite/seatbelt.rs +++ b/codex-rs/core/tests/suite/seatbelt.rs @@ -76,7 +76,7 @@ async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() { let tmp = TempDir::new().expect("should be able to create temp dir"); let test_scenario = create_test_scenario(&tmp); let policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![test_scenario.repo_parent.clone()], + writable_roots: vec![test_scenario.repo_parent.as_path().try_into().unwrap()], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -102,7 +102,7 @@ async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() { let tmp = TempDir::new().expect("should be able to create temp dir"); let test_scenario = create_test_scenario(&tmp); let policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![test_scenario.repo_root.clone()], + writable_roots: vec![test_scenario.repo_root.as_path().try_into().unwrap()], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index f26bec0e67..a6e5302de1 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -26,6 +26,7 @@ codex-common = { workspace = true, features = [ ] } codex-core = { workspace = true } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } mcp-types = { workspace = true } opentelemetry-appender-tracing = { workspace = true } owo-colors = { workspace = true } diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index f0faa8b438..d995d91030 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -1,6 +1,7 @@ #![cfg(unix)] use codex_core::protocol::SandboxPolicy; use codex_core::spawn::StdioPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; use std::future::Future; use std::io; @@ -58,14 +59,14 @@ async fn spawn_command_under_sandbox( async fn python_multiprocessing_lock_works_under_sandbox() { core_test_support::skip_if_sandbox!(); #[cfg(target_os = "macos")] - let writable_roots = Vec::::new(); + let writable_roots = Vec::::new(); // From https://man7.org/linux/man-pages/man7/sem_overview.7.html // // > On Linux, named semaphores are created in a virtual filesystem, // > normally mounted under /dev/shm. #[cfg(target_os = "linux")] - let writable_roots = vec![PathBuf::from("/dev/shm")]; + let writable_roots: Vec = vec!["/dev/shm".try_into().unwrap()]; let policy = SandboxPolicy::WorkspaceWrite { writable_roots, diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 46f030c60a..19ec1fcc36 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -13,7 +13,7 @@ workspace = true [dependencies] codex-git = { workspace = true } - +codex-utils-absolute-path = { workspace = true } codex-utils-image = { workspace = true } icu_decimal = { workspace = true } icu_locale_core = { workspace = true } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 73e2c9c877..238cd0fa26 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -23,6 +23,7 @@ use crate::openai_models::ReasoningEffort as ReasoningEffortConfig; use crate::parse_command::ParsedCommand; use crate::plan_tool::UpdatePlanArgs; use crate::user_input::UserInput; +use codex_utils_absolute_path::AbsolutePathBuf; use mcp_types::CallToolResult; use mcp_types::RequestId; use mcp_types::Resource as McpResource; @@ -34,6 +35,7 @@ use serde::Serialize; use serde_json::Value; use serde_with::serde_as; use strum_macros::Display; +use tracing::error; use ts_rs::TS; pub use crate::approvals::ApplyPatchApprovalRequestEvent; @@ -273,7 +275,7 @@ pub enum SandboxPolicy { /// Additional folders (beyond cwd and possibly TMPDIR) that should be /// writable from within the sandbox. #[serde(default, skip_serializing_if = "Vec::is_empty")] - writable_roots: Vec, + writable_roots: Vec, /// When set to `true`, outbound network access is allowed. `false` by /// default. @@ -299,11 +301,9 @@ pub enum SandboxPolicy { /// not modified by the agent. #[derive(Debug, Clone, PartialEq, Eq, JsonSchema)] pub struct WritableRoot { - /// Absolute path, by construction. - pub root: PathBuf, + pub root: AbsolutePathBuf, - /// Also absolute paths, by construction. - pub read_only_subpaths: Vec, + pub read_only_subpaths: Vec, } impl WritableRoot { @@ -385,16 +385,30 @@ impl SandboxPolicy { network_access: _, } => { // Start from explicitly configured writable roots. - let mut roots: Vec = writable_roots.clone(); + let mut roots: Vec = writable_roots.clone(); // Always include defaults: cwd, /tmp (if present on Unix), and // on macOS, the per-user TMPDIR unless explicitly excluded. - roots.push(cwd.to_path_buf()); + // TODO(mbolin): cwd param should be AbsolutePathBuf. + let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd); + match cwd_absolute { + Ok(cwd) => { + roots.push(cwd); + } + Err(e) => { + error!( + "Ignoring invalid cwd {:?} for sandbox writable root: {}", + cwd, e + ); + } + } // Include /tmp on Unix unless explicitly excluded. if cfg!(unix) && !exclude_slash_tmp { - let slash_tmp = PathBuf::from("/tmp"); - if slash_tmp.is_dir() { + #[allow(clippy::expect_used)] + let slash_tmp = + AbsolutePathBuf::from_absolute_path("/tmp").expect("/tmp is absolute"); + if slash_tmp.as_path().is_dir() { roots.push(slash_tmp); } } @@ -411,7 +425,15 @@ impl SandboxPolicy { && let Some(tmpdir) = std::env::var_os("TMPDIR") && !tmpdir.is_empty() { - roots.push(PathBuf::from(tmpdir)); + if let Ok(tmpdir_path) = + AbsolutePathBuf::from_absolute_path(PathBuf::from(&tmpdir)) + { + roots.push(tmpdir_path); + } else { + error!( + "Ignoring invalid TMPDIR value {tmpdir:?} for sandbox writable root", + ); + } } // For each root, compute subpaths that should remain read-only. @@ -419,8 +441,11 @@ impl SandboxPolicy { .into_iter() .map(|writable_root| { let mut subpaths = Vec::new(); - let top_level_git = writable_root.join(".git"); - if top_level_git.is_dir() { + #[allow(clippy::expect_used)] + let top_level_git = writable_root + .join(".git") + .expect(".git is a valid relative path"); + if top_level_git.as_path().is_dir() { subpaths.push(top_level_git); } WritableRoot { diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 4e5fad06b4..c440f09aeb 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -41,6 +41,7 @@ codex-feedback = { workspace = true } codex-file-search = { workspace = true } codex-login = { workspace = true } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } color-eyre = { workspace = true } crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] } derive_more = { workspace = true, features = ["is_variant"] } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index bd85a9edc8..4c0248d7ae 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -56,6 +56,7 @@ use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo; +use codex_utils_absolute_path::AbsolutePathBuf; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -1805,7 +1806,7 @@ fn preset_matching_ignores_extra_writable_roots() { .find(|p| p.id == "auto") .expect("auto preset exists"); let current_sandbox = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("C:\\extra")], + writable_roots: vec![AbsolutePathBuf::try_from("C:\\extra").unwrap()], network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/tui2/Cargo.toml b/codex-rs/tui2/Cargo.toml index 06308e996c..7ac1fb859c 100644 --- a/codex-rs/tui2/Cargo.toml +++ b/codex-rs/tui2/Cargo.toml @@ -40,6 +40,7 @@ codex-feedback = { workspace = true } codex-file-search = { workspace = true } codex-login = { workspace = true } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } codex-tui = { workspace = true } color-eyre = { workspace = true } crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] } diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index d9e242674b..11a5799280 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -56,6 +56,7 @@ use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo; +use codex_utils_absolute_path::AbsolutePathBuf; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -1806,7 +1807,7 @@ fn preset_matching_ignores_extra_writable_roots() { .find(|p| p.id == "auto") .expect("auto preset exists"); let current_sandbox = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("C:\\extra")], + writable_roots: vec![AbsolutePathBuf::try_from("C:\\extra").unwrap()], network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/utils/absolute-path/Cargo.toml b/codex-rs/utils/absolute-path/Cargo.toml index 486051fcb4..5af9dc23aa 100644 --- a/codex-rs/utils/absolute-path/Cargo.toml +++ b/codex-rs/utils/absolute-path/Cargo.toml @@ -10,7 +10,12 @@ workspace = true [dependencies] path-absolutize = { workspace = true } +schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } +ts-rs = { workspace = true, features = [ + "serde-json-impl", + "no-serde-warnings", +] } [dev-dependencies] serde_json = { workspace = true } diff --git a/codex-rs/utils/absolute-path/src/lib.rs b/codex-rs/utils/absolute-path/src/lib.rs index 5ea77b2b52..0d2fedb276 100644 --- a/codex-rs/utils/absolute-path/src/lib.rs +++ b/codex-rs/utils/absolute-path/src/lib.rs @@ -1,4 +1,5 @@ use path_absolutize::Absolutize; +use schemars::JsonSchema; use serde::Deserialize; use serde::Deserializer; use serde::Serialize; @@ -7,6 +8,7 @@ use std::cell::RefCell; use std::path::Display; use std::path::Path; use std::path::PathBuf; +use ts_rs::TS; /// A path that is guaranteed to be absolute and normalized (though it is not /// guaranteed to be canonicalized or exist on the filesystem). @@ -15,27 +17,27 @@ use std::path::PathBuf; /// using `AbsolutePathBufGuard::new(base_path)`. If no base path is set, the /// deserialization will fail unless the path being deserialized is already /// absolute. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, JsonSchema, TS)] pub struct AbsolutePathBuf(PathBuf); impl AbsolutePathBuf { - pub fn resolve_path_against_base(path: P, base_path: B) -> std::io::Result - where - P: AsRef, - B: AsRef, - { + pub fn resolve_path_against_base, B: AsRef>( + path: P, + base_path: B, + ) -> std::io::Result { let absolute_path = path.as_ref().absolutize_from(base_path.as_ref())?; Ok(Self(absolute_path.into_owned())) } - pub fn from_absolute_path

(path: P) -> std::io::Result - where - P: AsRef, - { + pub fn from_absolute_path>(path: P) -> std::io::Result { let absolute_path = path.as_ref().absolutize()?; Ok(Self(absolute_path.into_owned())) } + pub fn join>(&self, path: P) -> std::io::Result { + Self::resolve_path_against_base(path, &self.0) + } + pub fn as_path(&self) -> &Path { &self.0 } @@ -48,11 +50,47 @@ impl AbsolutePathBuf { self.0.clone() } + pub fn to_string_lossy(&self) -> std::borrow::Cow<'_, str> { + self.0.to_string_lossy() + } + pub fn display(&self) -> Display<'_> { self.0.display() } } +impl TryFrom<&Path> for AbsolutePathBuf { + type Error = std::io::Error; + + fn try_from(value: &Path) -> Result { + Self::from_absolute_path(value) + } +} + +impl TryFrom for AbsolutePathBuf { + type Error = std::io::Error; + + fn try_from(value: PathBuf) -> Result { + Self::from_absolute_path(value) + } +} + +impl TryFrom<&str> for AbsolutePathBuf { + type Error = std::io::Error; + + fn try_from(value: &str) -> Result { + Self::from_absolute_path(value) + } +} + +impl TryFrom for AbsolutePathBuf { + type Error = std::io::Error; + + fn try_from(value: String) -> Result { + Self::from_absolute_path(value) + } +} + thread_local! { static ABSOLUTE_PATH_BASE: RefCell> = const { RefCell::new(None) }; } diff --git a/docs/execpolicy.md b/docs/execpolicy.md index a2dc6d9add..e60e1cec6c 100644 --- a/docs/execpolicy.md +++ b/docs/execpolicy.md @@ -1,6 +1,6 @@ # Execpolicy quickstart -Codex can enforce your own rules-based execution policy before it runs shell commands. Policies live in `.execpolicy` files under `~/.codex/policy`. +Codex can enforce your own rules-based execution policy before it runs shell commands. Policies live in `.codexpolicy` files under `~/.codex/policy`. ## How to create and edit rules @@ -12,12 +12,12 @@ Codex CLI will present the option to whitelist commands when a command causes a Whitelisted commands will no longer require your permission to run in current and subsequent sessions. -Under the hood, when you approve and whitelist a command, codex will edit `~/.codex/policy/default.execpolicy`. +Under the hood, when you approve and whitelist a command, codex will edit `~/.codex/policy/default.codexpolicy`. -### Editing `.execpolicy` files +### Editing `.codexpolicy` files 1. Create a policy directory: `mkdir -p ~/.codex/policy`. -2. Add one or more `.execpolicy` files in that folder. Codex automatically loads every `.execpolicy` file in there on startup. +2. Add one or more `.codexpolicy` files in that folder. Codex automatically loads every `.codexpolicy` file in there on startup. 3. Write `prefix_rule` entries to describe the commands you want to allow, prompt, or block: ```starlark