diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index bd74d83ad6..76188da5b8 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1540,7 +1540,6 @@ dependencies = [ "anyhow", "clap", "codex-experimental-api-macros", - "codex-git-utils", "codex-protocol", "codex-shell-command", "codex-utils-absolute-path", @@ -1866,12 +1865,11 @@ dependencies = [ "codex-app-server-protocol", "codex-execpolicy", "codex-features", - "codex-git-utils", "codex-model-provider-info", "codex-network-proxy", "codex-protocol", "codex-utils-absolute-path", - "dunce", + "codex-utils-path", "futures", "multimap", "pretty_assertions", @@ -2264,6 +2262,8 @@ name = "codex-git-utils" version = "0.0.0" dependencies = [ "assert_matches", + "codex-exec-server", + "codex-protocol", "codex-utils-absolute-path", "futures", "once_cell", @@ -2334,6 +2334,7 @@ dependencies = [ "codex-protocol", "codex-sandboxing", "codex-utils-absolute-path", + "globset", "landlock", "libc", "pkg-config", @@ -2620,7 +2621,6 @@ dependencies = [ "chrono", "codex-async-utils", "codex-execpolicy", - "codex-git-utils", "codex-network-proxy", "codex-utils-absolute-path", "codex-utils-image", @@ -2760,6 +2760,7 @@ dependencies = [ "dunce", "libc", "pretty_assertions", + "regex-lite", "serde_json", "tempfile", "tokio", diff --git a/codex-rs/app-server-protocol/Cargo.toml b/codex-rs/app-server-protocol/Cargo.toml index d9ed5e8730..0cb50d8549 100644 --- a/codex-rs/app-server-protocol/Cargo.toml +++ b/codex-rs/app-server-protocol/Cargo.toml @@ -15,7 +15,6 @@ workspace = true anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-experimental-api-macros = { workspace = true } -codex-git-utils = { workspace = true } codex-protocol = { workspace = true } codex-shell-command = { workspace = true } codex-utils-absolute-path = { workspace = true } diff --git a/codex-rs/app-server-protocol/src/lib.rs b/codex-rs/app-server-protocol/src/lib.rs index d5c2f4b243..46f0c9ae41 100644 --- a/codex-rs/app-server-protocol/src/lib.rs +++ b/codex-rs/app-server-protocol/src/lib.rs @@ -4,7 +4,6 @@ mod jsonrpc_lite; mod protocol; mod schema_fixtures; -pub use codex_git_utils::GitSha; pub use experimental_api::*; pub use export::GenerateTsOptions; pub use export::generate_internal_json_schema; @@ -30,6 +29,7 @@ pub use protocol::v1::GetConversationSummaryParams; pub use protocol::v1::GetConversationSummaryResponse; pub use protocol::v1::GitDiffToRemoteParams; pub use protocol::v1::GitDiffToRemoteResponse; +pub use protocol::v1::GitSha; pub use protocol::v1::InitializeCapabilities; pub use protocol::v1::InitializeParams; pub use protocol::v1::InitializeResponse; diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 6aa2e9fa30..d642e7fab9 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -1,7 +1,6 @@ use std::collections::HashMap; use std::path::PathBuf; -use codex_git_utils::GitSha; use codex_protocol::ThreadId; use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::config_types::ReasoningSummary; @@ -11,6 +10,7 @@ use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::FileChange; +pub use codex_protocol::protocol::GitSha; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 0f3b8bf522..6a9dfb4042 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2441,9 +2441,9 @@ impl CodexMessageProcessor { | codex_protocol::protocol::SandboxPolicy::ExternalSandbox { .. } )) { - let trust_target = resolve_root_git_project_for_trust(config.cwd.as_path()) + let trust_target = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &config.cwd) .await - .unwrap_or_else(|| config.cwd.to_path_buf()); + .unwrap_or_else(|| config.cwd.clone()); let cli_overrides_with_trust; let cli_overrides_for_reload = if let Err(err) = codex_core::config::set_project_trust_level( @@ -6155,6 +6155,7 @@ impl CodexMessageProcessor { } }; let config_layer_stack = match load_config_layers_state( + LOCAL_FS.as_ref(), &self.config.codex_home, Some(cwd_abs.clone()), &cli_overrides, diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index 0a38f3b37e..b0ff8888a4 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -7,7 +7,11 @@ use app_test_support::McpProcess; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; use axum::Router; -use codex_app_server_protocol::JSONRPCError; +use codex_app_server::in_process; +use codex_app_server::in_process::InProcessStartArgs; +use codex_app_server_protocol::ClientInfo; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::InitializeParams; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::McpResourceContent; use codex_app_server_protocol::McpResourceReadParams; @@ -15,7 +19,14 @@ use codex_app_server_protocol::McpResourceReadResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; +use codex_arg0::Arg0DispatchPaths; use codex_config::types::AuthCredentialsStoreMode; +use codex_core::config::ConfigBuilder; +use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::LoaderOverrides; +use codex_exec_server::EnvironmentManager; +use codex_feedback::CodexFeedback; +use codex_protocol::protocol::SessionSource; use core_test_support::responses; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; @@ -149,24 +160,57 @@ stream_max_retries = 0 #[tokio::test] async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { let codex_home = TempDir::new()?; - let mut mcp = McpProcess::new(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let request_id = mcp - .send_mcp_resource_read_request(McpResourceReadParams { - thread_id: "00000000-0000-4000-8000-000000000000".to_string(), - server: "codex_apps".to_string(), - uri: TEST_RESOURCE_URI.to_string(), - }) + let loader_overrides = LoaderOverrides::without_managed_config_for_tests(); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .loader_overrides(loader_overrides.clone()) + .build() .await?; - let error: JSONRPCError = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_error_message(RequestId::Integer(request_id)), - ) - .await??; + // This negative-path test does not need the stdio subprocess; keeping it + // in-process avoids child-process teardown timing in nextest leak detection. + let client = in_process::start(InProcessStartArgs { + arg0_paths: Arg0DispatchPaths::default(), + config: Arc::new(config), + cli_overrides: Vec::new(), + loader_overrides, + cloud_requirements: CloudRequirementsLoader::default(), + feedback: CodexFeedback::new(), + log_db: None, + environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), + config_warnings: Vec::new(), + session_source: SessionSource::Cli, + enable_codex_api_key_env: false, + initialize: InitializeParams { + client_info: ClientInfo { + name: "codex-app-server-tests".to_string(), + title: None, + version: "0.1.0".to_string(), + }, + capabilities: None, + }, + channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + }) + .await?; + let response = client + .request(ClientRequest::McpResourceRead { + request_id: RequestId::Integer(1), + params: McpResourceReadParams { + thread_id: "00000000-0000-4000-8000-000000000000".to_string(), + server: "codex_apps".to_string(), + uri: TEST_RESOURCE_URI.to_string(), + }, + }) + .await; + client.shutdown().await?; + + let error = match response? { + Ok(result) => anyhow::bail!("expected thread-not-found error, got response: {result:?}"), + Err(error) => error, + }; assert!( - error.error.message.contains("thread not found"), + error.message.contains("thread not found"), "expected thread-not-found error, got: {error:?}" ); diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index ab514252e4..508c77f7ff 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -1,6 +1,7 @@ use anyhow::Result; use app_test_support::ChatGptAuthFixture; use app_test_support::McpProcess; +use app_test_support::PathBufExt; use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; @@ -20,6 +21,7 @@ use codex_app_server_protocol::ThreadStatus; use codex_app_server_protocol::ThreadStatusChangedNotification; use codex_config::types::AuthCredentialsStoreMode; use codex_core::config::set_project_trust_level; +use codex_exec_server::LOCAL_FS; use codex_git_utils::resolve_root_git_project_for_trust; use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_protocol::config_types::ServiceTier; @@ -716,10 +718,11 @@ model_reasoning_effort = "high" assert_eq!(reasoning_effort, Some(ReasoningEffort::High)); let config_toml = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - let trusted_root = resolve_root_git_project_for_trust(workspace.path()) + let workspace_abs = workspace.path().to_path_buf().abs(); + let trusted_root = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &workspace_abs) .await - .unwrap_or_else(|| workspace.path().to_path_buf()); - assert!(config_toml.contains(&persisted_trust_path(&trusted_root))); + .unwrap_or(workspace_abs); + assert!(config_toml.contains(&persisted_trust_path(trusted_root.as_path()))); assert!(config_toml.contains("trust_level = \"trusted\"")); Ok(()) @@ -754,10 +757,11 @@ async fn thread_start_with_nested_git_cwd_trusts_repo_root() -> Result<()> { .await??; let config_toml = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - let trusted_root = resolve_root_git_project_for_trust(&nested) + let nested_abs = nested.abs(); + let trusted_root = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested_abs) .await .expect("git root should resolve"); - assert!(config_toml.contains(&persisted_trust_path(&trusted_root))); + assert!(config_toml.contains(&persisted_trust_path(trusted_root.as_path()))); assert!(!config_toml.contains(&persisted_trust_path(&nested))); Ok(()) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 25bf80a6e3..32fb25ad3d 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -111,8 +111,8 @@ enum Subcommand { /// Manage external MCP servers for Codex. Mcp(McpCli), - /// Manage plugin marketplaces for Codex. - Marketplace(MarketplaceCli), + /// Manage Codex plugins. + Plugin(PluginCli), /// Start Codex as an MCP server (stdio). McpServer, @@ -170,6 +170,21 @@ enum Subcommand { Features(FeaturesCli), } +#[derive(Debug, Parser)] +struct PluginCli { + #[clap(flatten)] + pub config_overrides: CliConfigOverrides, + + #[command(subcommand)] + subcommand: PluginSubcommand, +} + +#[derive(Debug, clap::Subcommand)] +enum PluginSubcommand { + /// Manage plugin marketplaces for Codex. + Marketplace(MarketplaceCli), +} + #[derive(Debug, Parser)] struct CompletionCommand { /// Shell to generate completions for @@ -726,17 +741,23 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { prepend_config_flags(&mut mcp_cli.config_overrides, root_config_overrides.clone()); mcp_cli.run().await?; } - Some(Subcommand::Marketplace(mut marketplace_cli)) => { + Some(Subcommand::Plugin(plugin_cli)) => { reject_remote_mode_for_subcommand( root_remote.as_deref(), root_remote_auth_token_env.as_deref(), - "marketplace", + "plugin", )?; - prepend_config_flags( - &mut marketplace_cli.config_overrides, - root_config_overrides.clone(), - ); - marketplace_cli.run().await?; + let PluginCli { + mut config_overrides, + subcommand, + } = plugin_cli; + prepend_config_flags(&mut config_overrides, root_config_overrides.clone()); + match subcommand { + PluginSubcommand::Marketplace(mut marketplace_cli) => { + prepend_config_flags(&mut marketplace_cli.config_overrides, config_overrides); + marketplace_cli.run().await?; + } + } } Some(Subcommand::AppServer(app_server_cli)) => { let AppServerCommand { @@ -1690,6 +1711,35 @@ mod tests { assert!(matches!(cli.subcommand, Some(Subcommand::Responses(_)))); } + #[test] + fn plugin_marketplace_add_parses_under_plugin() { + let cli = + MultitoolCli::try_parse_from(["codex", "plugin", "marketplace", "add", "owner/repo"]) + .expect("parse"); + + assert!(matches!(cli.subcommand, Some(Subcommand::Plugin(_)))); + } + + #[test] + fn plugin_marketplace_upgrade_parses_under_plugin() { + let cli = + MultitoolCli::try_parse_from(["codex", "plugin", "marketplace", "upgrade", "debug"]) + .expect("parse"); + + assert!(matches!(cli.subcommand, Some(Subcommand::Plugin(_)))); + } + + #[test] + fn marketplace_no_longer_parses_at_top_level() { + let add_result = + MultitoolCli::try_parse_from(["codex", "marketplace", "add", "owner/repo"]); + assert!(add_result.is_err()); + + let upgrade_result = + MultitoolCli::try_parse_from(["codex", "marketplace", "upgrade", "debug"]); + assert!(upgrade_result.is_err()); + } + fn sample_exit_info(conversation_id: Option<&str>, thread_name: Option<&str>) -> AppExitInfo { let token_usage = TokenUsage { output_tokens: 2, diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index e04cfb5791..e4256f52b2 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -48,7 +48,7 @@ async fn marketplace_add_local_directory_source() -> Result<()> { codex_command(codex_home.path())? .current_dir(source_parent) - .args(["marketplace", "add", source_arg.as_str()]) + .args(["plugin", "marketplace", "add", source_arg.as_str()]) .assert() .success(); @@ -78,7 +78,12 @@ async fn marketplace_add_rejects_local_manifest_file_source() -> Result<()> { let manifest_path = source.path().join(".agents/plugins/marketplace.json"); codex_command(codex_home.path())? - .args(["marketplace", "add", manifest_path.to_str().unwrap()]) + .args([ + "plugin", + "marketplace", + "add", + manifest_path.to_str().unwrap(), + ]) .assert() .failure() .stderr(contains( @@ -87,3 +92,27 @@ async fn marketplace_add_rejects_local_manifest_file_source() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn marketplace_add_rejects_sparse_for_local_directory_source() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source(source.path(), "local ref")?; + + codex_command(codex_home.path())? + .args([ + "plugin", + "marketplace", + "add", + "--sparse", + ".agents", + source.path().to_str().unwrap(), + ]) + .assert() + .failure() + .stderr(contains( + "--sparse is only supported for git marketplace sources", + )); + + Ok(()) +} diff --git a/codex-rs/cli/tests/marketplace_upgrade.rs b/codex-rs/cli/tests/marketplace_upgrade.rs new file mode 100644 index 0000000000..081203ebef --- /dev/null +++ b/codex-rs/cli/tests/marketplace_upgrade.rs @@ -0,0 +1,36 @@ +use anyhow::Result; +use predicates::str::contains; +use std::path::Path; +use tempfile::TempDir; + +fn codex_command(codex_home: &Path) -> Result { + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); + cmd.env("CODEX_HOME", codex_home); + Ok(cmd) +} + +#[tokio::test] +async fn marketplace_upgrade_runs_under_plugin() -> Result<()> { + let codex_home = TempDir::new()?; + + codex_command(codex_home.path())? + .args(["plugin", "marketplace", "upgrade"]) + .assert() + .success() + .stdout(contains("No configured Git marketplaces to upgrade.")); + + Ok(()) +} + +#[tokio::test] +async fn marketplace_upgrade_no_longer_runs_at_top_level() -> Result<()> { + let codex_home = TempDir::new()?; + + codex_command(codex_home.path())? + .args(["marketplace", "upgrade"]) + .assert() + .failure() + .stderr(contains("unexpected argument 'upgrade' found")); + + Ok(()) +} diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 9532d74d00..93ca283b86 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -12,12 +12,11 @@ anyhow = { workspace = true } codex-app-server-protocol = { workspace = true } codex-execpolicy = { workspace = true } codex-features = { workspace = true } -codex-git-utils = { workspace = true } codex-model-provider-info = { workspace = true } codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } -dunce = { workspace = true } +codex-utils-path = { workspace = true } futures = { workspace = true, features = ["alloc", "std"] } multimap = { workspace = true } schemars = { workspace = true } diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index ff68564e6e..6556ca73e3 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -29,7 +29,6 @@ use crate::types::WindowsToml; use codex_app_server_protocol::Tools; use codex_app_server_protocol::UserSavedConfig; use codex_features::FeaturesToml; -use codex_git_utils::resolve_root_git_project_for_trust; use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID; use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID; use codex_model_provider_info::ModelProviderInfo; @@ -51,6 +50,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path::normalize_for_path_comparison; use schemars::JsonSchema; use serde::Deserialize; use serde::Deserializer; @@ -601,7 +601,7 @@ impl ConfigToml { sandbox_mode_override: Option, profile_sandbox_mode: Option, windows_sandbox_level: WindowsSandboxLevel, - resolved_cwd: &Path, + active_project: Option<&ProjectConfig>, sandbox_policy_constraint: Option<&crate::Constrained>, ) -> SandboxPolicy { let sandbox_mode_was_explicit = sandbox_mode_override.is_some() @@ -616,7 +616,7 @@ impl ConfigToml { // If no sandbox_mode is set but this directory has a trust decision, // default to workspace-write except on unsandboxed Windows where we // default to read-only. - self.get_active_project(resolved_cwd).await.and_then(|p| { + active_project.and_then(|p| { if p.is_trusted() || p.is_untrusted() { if cfg!(target_os = "windows") && windows_sandbox_level == WindowsSandboxLevel::Disabled @@ -677,8 +677,13 @@ impl ConfigToml { } /// Resolves the cwd to an existing project, or returns None if ConfigToml - /// does not contain a project corresponding to cwd or a git repo for cwd - pub async fn get_active_project(&self, resolved_cwd: &Path) -> Option { + /// does not contain a project corresponding to cwd or the resolved git repo + /// root for cwd. + pub fn get_active_project( + &self, + resolved_cwd: &Path, + repo_root: Option<&Path>, + ) -> Option { let projects = self.projects.as_ref()?; for normalized_cwd in normalized_project_lookup_keys(resolved_cwd) { @@ -687,11 +692,8 @@ impl ConfigToml { } } - // If cwd lives inside a git repo/worktree, check whether the root git project - // (the primary repository working directory) is trusted. This lets - // worktrees inherit trust from the main project. - if let Some(repo_root) = resolve_root_git_project_for_trust(resolved_cwd).await { - for normalized_repo_root in normalized_project_lookup_keys(&repo_root) { + if let Some(repo_root) = repo_root { + for normalized_repo_root in normalized_project_lookup_keys(repo_root) { if let Some(project_config_for_root) = project_config_for_lookup_key(projects, &normalized_repo_root) { @@ -731,7 +733,7 @@ impl ConfigToml { fn normalized_project_lookup_keys(path: &Path) -> Vec { let normalized_path = normalize_project_lookup_key(path.to_string_lossy().to_string()); let normalized_canonical_path = normalize_project_lookup_key( - dunce::canonicalize(path) + normalize_for_path_comparison(path) .unwrap_or_else(|_| path.to_path_buf()) .to_string_lossy() .to_string(), diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index fcc3e006b1..cee68d7abb 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -31,6 +31,10 @@ pub struct PermissionProfileToml { #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] pub struct FilesystemPermissionsToml { + /// Optional maximum depth for expanding unreadable glob patterns on + /// platforms that snapshot glob matches before sandbox startup. + #[schemars(range(min = 1))] + pub glob_scan_max_depth: Option, #[serde(flatten)] pub entries: BTreeMap, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 966555856b..67d051a699 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -673,6 +673,14 @@ ] }, "FilesystemPermissionsToml": { + "properties": { + "glob_scan_max_depth": { + "description": "Optional maximum depth for expanding unreadable glob patterns on platforms that snapshot glob matches before sandbox startup.", + "format": "uint", + "minimum": 1.0, + "type": "integer" + } + }, "type": "object" }, "ForcedLoginMethod": { diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 61847f49ba..9569c02d71 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -18,6 +18,7 @@ use crate::config_loader::resolve_relative_paths_in_config_toml; use anyhow::anyhow; use codex_app_server_protocol::ConfigLayerSource; use codex_config::config_toml::ConfigToml; +use codex_exec_server::LOCAL_FS; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::path::Path; @@ -168,6 +169,7 @@ mod reload { } let mut next_config = Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), merged_config, reload_overrides(config, preserve_current_provider), config.codex_home.clone(), diff --git a/codex-rs/core/src/codex/handlers.rs b/codex-rs/core/src/codex/handlers.rs index 88f3f38b65..7e0ba45dad 100644 --- a/codex-rs/core/src/codex/handlers.rs +++ b/codex-rs/core/src/codex/handlers.rs @@ -22,6 +22,7 @@ use crate::realtime_context::REALTIME_TURN_TOKEN_BUDGET; use crate::realtime_context::truncate_realtime_text_to_token_budget; use crate::realtime_conversation::REALTIME_USER_TEXT_PREFIX; use crate::realtime_conversation::prefix_realtime_v2_text; +use codex_exec_server::LOCAL_FS; use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; @@ -526,6 +527,7 @@ pub async fn list_skills(sess: &Session, sub_id: String, cwds: Vec, for } }; let config_layer_stack = match load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd_abs.clone()), empty_cli_overrides, diff --git a/codex-rs/core/src/config/agent_roles.rs b/codex-rs/core/src/config/agent_roles.rs index 24d26ebf47..b1d28cf838 100644 --- a/codex-rs/core/src/config/agent_roles.rs +++ b/codex-rs/core/src/config/agent_roles.rs @@ -4,6 +4,7 @@ use crate::config_loader::ConfigLayerStackOrdering; use codex_config::config_toml::AgentRoleToml; use codex_config::config_toml::AgentsToml; use codex_config::config_toml::ConfigToml; +use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; use serde::Deserialize; @@ -14,7 +15,8 @@ use std::path::Path; use std::path::PathBuf; use toml::Value as TomlValue; -pub(crate) fn load_agent_roles( +pub(crate) async fn load_agent_roles( + fs: &dyn ExecutorFileSystem, cfg: &ConfigToml, config_layer_stack: &ConfigLayerStack, startup_warnings: &mut Vec, @@ -24,7 +26,7 @@ pub(crate) fn load_agent_roles( /*include_disabled*/ false, ); if layers.is_empty() { - return load_agent_roles_without_layers(cfg); + return load_agent_roles_without_layers(fs, cfg).await; } let mut roles: BTreeMap = BTreeMap::new(); @@ -40,13 +42,14 @@ pub(crate) fn load_agent_roles( }; if let Some(agents_toml) = agents_toml { for (declared_role_name, role_toml) in &agents_toml.roles { - let (role_name, role) = match read_declared_role(declared_role_name, role_toml) { - Ok(role) => role, - Err(err) => { - push_agent_role_warning(startup_warnings, err); - continue; - } - }; + let (role_name, role) = + match read_declared_role(fs, declared_role_name, role_toml).await { + Ok(role) => role, + Err(err) => { + push_agent_role_warning(startup_warnings, err); + continue; + } + }; if let Some(config_file) = role.config_file.clone() { declared_role_files.insert(config_file); } @@ -68,10 +71,13 @@ pub(crate) fn load_agent_roles( if let Some(config_folder) = layer.config_folder() { for (role_name, role) in discover_agent_roles_in_dir( - config_folder.as_path().join("agents").as_path(), + fs, + &config_folder.join("agents"), &declared_role_files, startup_warnings, - )? { + ) + .await? + { if layer_roles.contains_key(&role_name) { push_agent_role_warning( startup_warnings, @@ -113,13 +119,14 @@ fn push_agent_role_warning(startup_warnings: &mut Vec, err: std::io::Err startup_warnings.push(message); } -fn load_agent_roles_without_layers( +async fn load_agent_roles_without_layers( + fs: &dyn ExecutorFileSystem, cfg: &ConfigToml, ) -> std::io::Result> { let mut roles = BTreeMap::new(); if let Some(agents_toml) = cfg.agents.as_ref() { for (declared_role_name, role_toml) in &agents_toml.roles { - let (role_name, role) = read_declared_role(declared_role_name, role_toml)?; + let (role_name, role) = read_declared_role(fs, declared_role_name, role_toml).await?; validate_required_agent_role_description(&role_name, role.description.as_deref())?; if roles.insert(role_name.clone(), role).is_some() { @@ -134,14 +141,17 @@ fn load_agent_roles_without_layers( Ok(roles) } -fn read_declared_role( +async fn read_declared_role( + fs: &dyn ExecutorFileSystem, declared_role_name: &str, role_toml: &AgentRoleToml, ) -> std::io::Result<(String, AgentRoleConfig)> { - let mut role = agent_role_config_from_toml(declared_role_name, role_toml)?; + let mut role = agent_role_config_from_toml(fs, declared_role_name, role_toml).await?; let mut role_name = declared_role_name.to_string(); if let Some(config_file) = role.config_file.as_deref() { - let parsed_file = read_resolved_agent_role_file(config_file, Some(declared_role_name))?; + let config_file = AbsolutePathBuf::from_absolute_path(config_file)?; + let parsed_file = + read_resolved_agent_role_file(fs, &config_file, Some(declared_role_name)).await?; role_name = parsed_file.role_name; role.description = parsed_file.description.or(role.description); role.nickname_candidates = parsed_file.nickname_candidates.or(role.nickname_candidates); @@ -171,12 +181,17 @@ fn agents_toml_from_layer(layer_toml: &TomlValue) -> std::io::Result std::io::Result { - let config_file = role.config_file.as_ref().map(AbsolutePathBuf::to_path_buf); - validate_agent_role_config_file(role_name, config_file.as_deref())?; + let config_file = role + .config_file + .as_ref() + .map(AbsolutePathBuf::from_absolute_path) + .transpose()?; + validate_agent_role_config_file(fs, role_name, config_file.as_ref()).await?; let description = normalize_agent_role_description( &format!("agents.{role_name}.description"), role.description.as_deref(), @@ -188,7 +203,7 @@ fn agent_role_config_from_toml( Ok(AgentRoleConfig { description, - config_file, + config_file: config_file.map(AbsolutePathBuf::into_path_buf), nickname_candidates, }) } @@ -293,15 +308,17 @@ pub(crate) fn parse_agent_role_file_contents( }) } -fn read_resolved_agent_role_file( - path: &Path, +async fn read_resolved_agent_role_file( + fs: &dyn ExecutorFileSystem, + path: &AbsolutePathBuf, role_name_hint: Option<&str>, ) -> std::io::Result { - let contents = std::fs::read_to_string(path)?; + let contents = fs.read_file_text(path, /*sandbox*/ None).await?; + let config_base_dir = path.parent().unwrap_or_else(|| path.clone()); parse_agent_role_file_contents( &contents, - path, - path.parent().unwrap_or(path), + path.as_path(), + config_base_dir.as_path(), role_name_hint, ) } @@ -359,31 +376,35 @@ fn validate_agent_role_file_developer_instructions( } } -fn validate_agent_role_config_file( +async fn validate_agent_role_config_file( + fs: &dyn ExecutorFileSystem, role_name: &str, - config_file: Option<&Path>, + config_file: Option<&AbsolutePathBuf>, ) -> std::io::Result<()> { let Some(config_file) = config_file else { return Ok(()); }; - let metadata = std::fs::metadata(config_file).map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "agents.{role_name}.config_file must point to an existing file at {}: {e}", - config_file.display() - ), - ) - })?; - if metadata.is_file() { + let metadata = fs + .get_metadata(config_file, /*sandbox*/ None) + .await + .map_err(|e| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "agents.{role_name}.config_file must point to an existing file at {}: {e}", + config_file.as_path().display() + ), + ) + })?; + if metadata.is_file { Ok(()) } else { Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, format!( "agents.{role_name}.config_file must point to a file: {}", - config_file.display() + config_file.as_path().display() ), )) } @@ -441,19 +462,20 @@ fn normalize_agent_role_nickname_candidates( Ok(Some(normalized_candidates)) } -fn discover_agent_roles_in_dir( - agents_dir: &Path, +async fn discover_agent_roles_in_dir( + fs: &dyn ExecutorFileSystem, + agents_dir: &AbsolutePathBuf, declared_role_files: &BTreeSet, startup_warnings: &mut Vec, ) -> std::io::Result> { let mut roles = BTreeMap::new(); - for agent_file in collect_agent_role_files(agents_dir)? { - if declared_role_files.contains(&agent_file) { + for agent_file in collect_agent_role_files(fs, agents_dir).await? { + if declared_role_files.contains(agent_file.as_path()) { continue; } let parsed_file = - match read_resolved_agent_role_file(&agent_file, /*role_name_hint*/ None) { + match read_resolved_agent_role_file(fs, &agent_file, /*role_name_hint*/ None).await { Ok(parsed_file) => parsed_file, Err(err) => { push_agent_role_warning(startup_warnings, err); @@ -468,7 +490,7 @@ fn discover_agent_roles_in_dir( std::io::ErrorKind::InvalidInput, format!( "duplicate agent role name `{role_name}` discovered in {}", - agents_dir.display() + agents_dir.as_path().display() ), ), ); @@ -478,7 +500,7 @@ fn discover_agent_roles_in_dir( role_name, AgentRoleConfig { description: parsed_file.description, - config_file: Some(agent_file), + config_file: Some(agent_file.to_path_buf()), nickname_candidates: parsed_file.nickname_candidates, }, ); @@ -487,36 +509,36 @@ fn discover_agent_roles_in_dir( Ok(roles) } -fn collect_agent_role_files(dir: &Path) -> std::io::Result> { +async fn collect_agent_role_files( + fs: &dyn ExecutorFileSystem, + dir: &AbsolutePathBuf, +) -> std::io::Result> { let mut files = Vec::new(); - collect_agent_role_files_recursive(dir, &mut files)?; - files.sort(); - Ok(files) -} + let mut dirs = vec![dir.clone()]; + while let Some(dir) = dirs.pop() { + let entries = match fs.read_directory(&dir, /*sandbox*/ None).await { + Ok(entries) => entries, + Err(err) if err.kind() == ErrorKind::NotFound => continue, + Err(err) => return Err(err), + }; -fn collect_agent_role_files_recursive(dir: &Path, files: &mut Vec) -> std::io::Result<()> { - let read_dir = match std::fs::read_dir(dir) { - Ok(read_dir) => read_dir, - Err(err) if err.kind() == ErrorKind::NotFound => return Ok(()), - Err(err) => return Err(err), - }; - - for entry in read_dir { - let entry = entry?; - let path = entry.path(); - let file_type = entry.file_type()?; - if file_type.is_dir() { - collect_agent_role_files_recursive(&path, files)?; - continue; - } - if file_type.is_file() - && path - .extension() - .is_some_and(|extension| extension == "toml") - { - files.push(path); + for entry in entries { + let path = dir.join(entry.file_name); + if entry.is_directory { + dirs.push(path); + continue; + } + if entry.is_file + && path + .as_path() + .extension() + .is_some_and(|extension| extension == "toml") + { + files.push(path); + } } } - Ok(()) + files.sort(); + Ok(files) } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 97ba40f7ad..fb1a56f6d6 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -45,6 +45,7 @@ use codex_config::types::SkillsConfig; use codex_config::types::ToolSuggestDiscoverableType; use codex_config::types::Tui; use codex_config::types::TuiNotificationSettings; +use codex_exec_server::LOCAL_FS; use codex_features::Feature; use codex_features::FeaturesToml; use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID; @@ -429,6 +430,7 @@ allow_upstream_proxy = false "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([ ( ":minimal".to_string(), @@ -483,6 +485,7 @@ async fn permissions_profiles_network_populates_runtime_network_proxy_spec() -> "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":minimal".to_string(), FilesystemPermissionToml::Access(FileSystemAccessMode::Read), @@ -532,6 +535,7 @@ async fn permissions_profiles_network_disabled_by_default_does_not_start_proxy() "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":minimal".to_string(), FilesystemPermissionToml::Access(FileSystemAccessMode::Read), @@ -577,6 +581,7 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([ ( ":minimal".to_string(), @@ -672,6 +677,7 @@ async fn project_root_glob_none_compiles_to_filesystem_pattern_entry() -> std::i "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: Some(2), entries: BTreeMap::from([( ":project_roots".to_string(), FilesystemPermissionToml::Scoped(BTreeMap::from([ @@ -694,6 +700,13 @@ async fn project_root_glob_none_compiles_to_filesystem_pattern_entry() -> std::i ) .await?; + assert_eq!( + config + .permissions + .file_system_sandbox_policy + .glob_scan_max_depth, + Some(2) + ); let expected_pattern = AbsolutePathBuf::resolve_path_against_base("**/*.env", cwd.path()) .to_string_lossy() .into_owned(); @@ -739,6 +752,7 @@ async fn permissions_profiles_require_default_permissions() -> std::io::Result<( "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":minimal".to_string(), FilesystemPermissionToml::Access(FileSystemAccessMode::Read), @@ -782,6 +796,7 @@ async fn permissions_profiles_reject_writes_outside_workspace_root() -> std::io: "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( external_write_path.to_string(), FilesystemPermissionToml::Access(FileSystemAccessMode::Write), @@ -825,6 +840,7 @@ async fn permissions_profiles_reject_nested_entries_for_non_project_roots() -> s "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":minimal".to_string(), FilesystemPermissionToml::Scoped(BTreeMap::from([( @@ -884,6 +900,7 @@ async fn load_workspace_permission_profile( async fn permissions_profiles_allow_unknown_special_paths() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":future_special_path".to_string(), FilesystemPermissionToml::Access(FileSystemAccessMode::Read), @@ -930,6 +947,7 @@ async fn permissions_profiles_allow_unknown_special_paths_with_nested_entries() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":future_special_path".to_string(), FilesystemPermissionToml::Scoped(BTreeMap::from([( @@ -997,6 +1015,7 @@ async fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io async fn permissions_profiles_allow_empty_filesystem_with_warning() -> std::io::Result<()> { let config = load_workspace_permission_profile(PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::new(), }), network: None, @@ -1031,6 +1050,7 @@ async fn permissions_profiles_reject_project_root_parent_traversal() -> std::io: "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":project_roots".to_string(), FilesystemPermissionToml::Scoped(BTreeMap::from([( @@ -1076,6 +1096,7 @@ async fn permissions_profiles_allow_network_enablement() -> std::io::Result<()> "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":minimal".to_string(), FilesystemPermissionToml::Access(FileSystemAccessMode::Read), @@ -1175,7 +1196,7 @@ network_access = false # This should be ignored. sandbox_mode_override, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &PathBuf::from("/tmp/test"), + /*active_project*/ None, /*sandbox_policy_constraint*/ None, ) .await; @@ -1196,7 +1217,7 @@ network_access = true # This should be ignored. sandbox_mode_override, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &PathBuf::from("/tmp/test"), + /*active_project*/ None, /*sandbox_policy_constraint*/ None, ) .await; @@ -1213,6 +1234,9 @@ writable_roots = [ ] exclude_tmpdir_env_var = true exclude_slash_tmp = true + +[projects."/tmp/test"] +trust_level = "trusted" "#, serde_json::json!(writable_root) ); @@ -1225,7 +1249,7 @@ exclude_slash_tmp = true sandbox_mode_override, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &PathBuf::from("/tmp/test"), + /*active_project*/ None, /*sandbox_policy_constraint*/ None, ) .await; @@ -1254,9 +1278,6 @@ writable_roots = [ ] exclude_tmpdir_env_var = true exclude_slash_tmp = true - -[projects."/tmp/test"] -trust_level = "trusted" "#, serde_json::json!(writable_root) ); @@ -1269,7 +1290,7 @@ trust_level = "trusted" sandbox_mode_override, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &PathBuf::from("/tmp/test"), + /*active_project*/ None, /*sandbox_policy_constraint*/ None, ) .await; @@ -2066,6 +2087,7 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> { let cwd = codex_home.path().abs(); let config_layer_stack = load_config_layers_state( + LOCAL_FS.as_ref(), codex_home.path(), Some(cwd), &Vec::new(), @@ -2199,6 +2221,7 @@ async fn managed_config_wins_over_cli_overrides() -> anyhow::Result<()> { let cwd = codex_home.path().abs(); let config_layer_stack = load_config_layers_state( + LOCAL_FS.as_ref(), codex_home.path(), Some(cwd), &[("model".to_string(), TomlValue::String("cli".to_string()))], @@ -3467,6 +3490,7 @@ async fn load_config_uses_requirements_guardian_policy_config() -> std::io::Resu .map_err(std::io::Error::other)?; let config = Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), ConfigToml::default(), ConfigOverrides { cwd: Some(codex_home.path().to_path_buf()), @@ -3499,6 +3523,7 @@ async fn load_config_ignores_empty_requirements_guardian_policy_config() -> std: .map_err(std::io::Error::other)?; let config = Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), ConfigToml::default(), ConfigOverrides { cwd: Some(codex_home.path().to_path_buf()), @@ -5311,6 +5336,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() .expect("config layer stack"); let config = Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), fixture.cfg.clone(), ConfigOverrides { cwd: Some(fixture.cwd_path()), @@ -5451,7 +5477,10 @@ async fn active_project_does_not_match_configured_alias_for_canonical_cwd() -> a ..Default::default() }; - assert_eq!(config.get_active_project(&project_root).await, None); + assert_eq!( + config.get_active_project(&project_root, /*repo_root*/ None), + None + ); Ok(()) } @@ -5545,13 +5574,16 @@ trust_level = "untrusted" let cfg = toml::from_str::(config_with_untrusted) .expect("TOML deserialization should succeed"); + let active_project = ProjectConfig { + trust_level: Some(TrustLevel::Untrusted), + }; let resolution = cfg .derive_sandbox_policy( /*sandbox_mode_override*/ None, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &PathBuf::from("/tmp/test"), + Some(&active_project), /*sandbox_policy_constraint*/ None, ) .await; @@ -5587,6 +5619,9 @@ async fn derive_sandbox_policy_falls_back_to_constraint_value_for_implicit_defau )])), ..Default::default() }; + let active_project = ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }; let constrained = Constrained::new(SandboxPolicy::DangerFullAccess, |candidate| { if matches!(candidate, SandboxPolicy::DangerFullAccess) { Ok(()) @@ -5605,7 +5640,7 @@ async fn derive_sandbox_policy_falls_back_to_constraint_value_for_implicit_defau /*sandbox_mode_override*/ None, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &project_path, + Some(&active_project), Some(&constrained), ) .await; @@ -5629,6 +5664,9 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb )])), ..Default::default() }; + let active_project = ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }; let constrained = Constrained::new(SandboxPolicy::new_workspace_write_policy(), |candidate| { if matches!(candidate, SandboxPolicy::WorkspaceWrite { .. }) { Ok(()) @@ -5647,7 +5685,7 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb /*sandbox_mode_override*/ None, /*profile_sandbox_mode*/ None, WindowsSandboxLevel::Disabled, - &project_path, + Some(&active_project), Some(&constrained), ) .await; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index c4217bdf2a..47a8bdf304 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -47,6 +47,8 @@ use codex_config::types::ToolSuggestDiscoverable; use codex_config::types::TuiNotificationSettings; use codex_config::types::UriBasedFileOpener; use codex_config::types::WindowsSandboxModeToml; +use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::LOCAL_FS; use codex_features::Feature; use codex_features::FeatureConfigSource; use codex_features::FeatureOverrides; @@ -54,6 +56,7 @@ use codex_features::FeatureToml; use codex_features::Features; use codex_features::FeaturesToml; use codex_features::MultiAgentV2ConfigToml; +use codex_git_utils::resolve_root_git_project_for_trust; use codex_login::AuthManagerConfig; use codex_mcp::McpConfig; use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID; @@ -689,6 +692,7 @@ impl ConfigBuilder { }; harness_overrides.cwd = Some(cwd.to_path_buf()); let config_layer_stack = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &cli_overrides, @@ -718,6 +722,7 @@ impl ConfigBuilder { } }; Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), config_toml, harness_overrides, codex_home, @@ -812,6 +817,7 @@ impl Config { let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home)?; let config_toml = deserialize_config_toml_with_base(merged, &codex_home)?; Self::load_config_with_layer_stack( + LOCAL_FS.as_ref(), config_toml, ConfigOverrides::default(), codex_home, @@ -849,6 +855,7 @@ pub async fn load_config_as_toml_with_cli_overrides( cli_overrides: Vec<(String, TomlValue)>, ) -> std::io::Result { let config_layer_stack = load_config_layers_state( + LOCAL_FS.as_ref(), codex_home, cwd.cloned(), &cli_overrides, @@ -1019,6 +1026,7 @@ pub async fn load_global_mcp_servers( // MCP servers defined in in-repo .codex/ folders. let cwd: Option = None; let config_layer_stack = load_config_layers_state( + LOCAL_FS.as_ref(), codex_home, cwd, &cli_overrides, @@ -1420,10 +1428,18 @@ impl Config { ) -> std::io::Result { // Note this ignores requirements.toml enforcement for tests. let config_layer_stack = ConfigLayerStack::default(); - Self::load_config_with_layer_stack(cfg, overrides, codex_home, config_layer_stack).await + Self::load_config_with_layer_stack( + LOCAL_FS.as_ref(), + cfg, + overrides, + codex_home, + config_layer_stack, + ) + .await } pub(crate) async fn load_config_with_layer_stack( + fs: &dyn ExecutorFileSystem, cfg: ConfigToml, overrides: ConfigOverrides, codex_home: AbsolutePathBuf, @@ -1545,9 +1561,12 @@ impl Config { .into_iter() .map(|path| AbsolutePathBuf::resolve_path_against_base(path, resolved_cwd.as_path())) .collect(); + let repo_root = resolve_root_git_project_for_trust(fs, &resolved_cwd).await; let active_project = cfg - .get_active_project(resolved_cwd.as_path()) - .await + .get_active_project( + resolved_cwd.as_path(), + repo_root.as_ref().map(AbsolutePathBuf::as_path), + ) .unwrap_or(ProjectConfig { trust_level: None }); let permission_config_syntax = resolve_permission_config_syntax( &config_layer_stack, @@ -1643,7 +1662,7 @@ impl Config { sandbox_mode, config_profile.sandbox_mode, windows_sandbox_level, - resolved_cwd.as_path(), + Some(&active_project), Some(&constrained_sandbox_policy), ) .await; @@ -1712,7 +1731,8 @@ impl Config { let multi_agent_v2 = resolve_multi_agent_v2_config(&cfg, &config_profile); let agent_roles = - agent_roles::load_agent_roles(&cfg, &config_layer_stack, &mut startup_warnings)?; + agent_roles::load_agent_roles(fs, &cfg, &config_layer_stack, &mut startup_warnings) + .await?; let openai_base_url = cfg .openai_base_url @@ -1861,8 +1881,12 @@ impl Config { .model_instructions_file .as_ref() .or(cfg.model_instructions_file.as_ref()); - let file_base_instructions = - Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?; + let file_base_instructions = Self::try_read_non_empty_file( + fs, + model_instructions_path, + "model instructions file", + ) + .await?; let base_instructions = base_instructions.or(file_base_instructions); let developer_instructions = developer_instructions.or(cfg.developer_instructions); let include_permissions_instructions = config_profile @@ -1893,9 +1917,11 @@ impl Config { .as_ref() .or(cfg.experimental_compact_prompt_file.as_ref()); let file_compact_prompt = Self::try_read_non_empty_file( + fs, experimental_compact_prompt_path, "experimental compact prompt file", - )?; + ) + .await?; let compact_prompt = compact_prompt.or(file_compact_prompt); let js_repl_node_path = js_repl_node_path_override .or(config_profile.js_repl_node_path.map(Into::into)) @@ -2218,7 +2244,8 @@ impl Config { /// If `path` is `Some`, attempts to read the file at the given path and /// returns its contents as a trimmed `String`. If the file is empty, or /// is `Some` but cannot be read, returns an `Err`. - fn try_read_non_empty_file( + async fn try_read_non_empty_file( + fs: &dyn ExecutorFileSystem, path: Option<&AbsolutePathBuf>, context: &str, ) -> std::io::Result> { @@ -2226,12 +2253,15 @@ impl Config { return Ok(None); }; - let contents = std::fs::read_to_string(path).map_err(|e| { - std::io::Error::new( - e.kind(), - format!("failed to read {context} {}: {e}", path.display()), - ) - })?; + let contents = fs + .read_file_text(path, /*sandbox*/ None) + .await + .map_err(|e| { + std::io::Error::new( + e.kind(), + format!("failed to read {context} {}: {e}", path.display()), + ) + })?; let s = contents.trim().to_string(); if s.is_empty() { diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 1609b696d8..943c82c0e9 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -66,6 +66,14 @@ pub(crate) fn compile_permission_profile( ), ); } + for pattern in unbounded_unreadable_globstar_paths(filesystem) { + push_warning( + startup_warnings, + format!( + "Filesystem deny-read glob `{pattern}` uses `**`. Non-macOS sandboxing does not support unbounded `**` natively; set `glob_scan_max_depth` in this filesystem profile to cap Linux glob expansion and silence this warning, or enumerate explicit depths such as `*.env`, `*/*.env`, and `*/*/*.env`." + ), + ); + } } for (path, permission) in &filesystem.entries { entries.extend(compile_filesystem_permission( @@ -82,12 +90,17 @@ pub(crate) fn compile_permission_profile( missing_filesystem_entries_warning(profile_name), ); } + let glob_scan_max_depth = validate_glob_scan_max_depth( + profile + .filesystem + .as_ref() + .and_then(|filesystem| filesystem.glob_scan_max_depth), + )?; let network_sandbox_policy = compile_network_sandbox_policy(profile.network.as_ref()); - Ok(( - FileSystemSandboxPolicy::restricted(entries), - network_sandbox_policy, - )) + let mut file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(entries); + file_system_sandbox_policy.glob_scan_max_depth = glob_scan_max_depth; + Ok((file_system_sandbox_policy, network_sandbox_policy)) } /// Returns a list of paths that must be readable by shell tools in order @@ -333,6 +346,42 @@ fn unsupported_read_write_glob_paths(filesystem: &FilesystemPermissionsToml) -> patterns } +fn unbounded_unreadable_globstar_paths(filesystem: &FilesystemPermissionsToml) -> Vec { + if filesystem.glob_scan_max_depth.is_some() { + return Vec::new(); + } + + let mut patterns = Vec::new(); + for (path, permission) in &filesystem.entries { + match permission { + FilesystemPermissionToml::Access(FileSystemAccessMode::None) => { + if path.contains("**") { + patterns.push(path.clone()); + } + } + FilesystemPermissionToml::Access(_) => {} + FilesystemPermissionToml::Scoped(scoped_entries) => { + for (subpath, access) in scoped_entries { + if *access == FileSystemAccessMode::None && subpath.contains("**") { + patterns.push(format!("{path}/{subpath}")); + } + } + } + } + } + patterns +} + +fn validate_glob_scan_max_depth(max_depth: Option) -> io::Result> { + match max_depth { + Some(0) => Err(io::Error::new( + io::ErrorKind::InvalidInput, + "glob_scan_max_depth must be at least 1", + )), + _ => Ok(max_depth), + } +} + fn contains_glob_chars(path: &str) -> bool { path.chars().any(|ch| matches!(ch, '*' | '?' | '[' | ']')) } diff --git a/codex-rs/core/src/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index 7116e32e70..e22376b214 100644 --- a/codex-rs/core/src/config/permissions_tests.rs +++ b/codex-rs/core/src/config/permissions_tests.rs @@ -55,6 +55,7 @@ async fn restricted_read_implicitly_allows_helper_executables() -> std::io::Resu "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::new(), }), network: None, @@ -226,6 +227,7 @@ fn network_toml_overlays_unix_socket_permissions_by_path() { #[test] fn read_write_glob_warnings_skip_supported_deny_read_globs_and_trailing_subpaths() { let filesystem = FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([ ( "/tmp/**/*.log".to_string(), @@ -256,6 +258,47 @@ fn read_write_glob_warnings_skip_supported_deny_read_globs_and_trailing_subpaths ); } +#[test] +fn unreadable_globstar_warning_is_suppressed_when_scan_depth_is_configured() { + let filesystem = FilesystemPermissionsToml { + glob_scan_max_depth: None, + entries: BTreeMap::from([( + ":project_roots".to_string(), + FilesystemPermissionToml::Scoped(BTreeMap::from([ + ("**/*.env".to_string(), FileSystemAccessMode::None), + ("*.pem".to_string(), FileSystemAccessMode::None), + ])), + )]), + }; + + assert_eq!( + unbounded_unreadable_globstar_paths(&filesystem), + vec![":project_roots/**/*.env".to_string()] + ); + + let configured_filesystem = FilesystemPermissionsToml { + glob_scan_max_depth: Some(2), + ..filesystem + }; + assert_eq!( + unbounded_unreadable_globstar_paths(&configured_filesystem), + Vec::::new() + ); +} + +#[test] +fn glob_scan_max_depth_must_be_positive() { + let err = validate_glob_scan_max_depth(Some(0)) + .expect_err("zero depth would silently skip deny-read glob expansion"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!(err.to_string(), "glob_scan_max_depth must be at least 1"); + assert_eq!( + validate_glob_scan_max_depth(Some(2)).expect("depth should be valid"), + Some(2) + ); +} + #[test] fn read_write_trailing_glob_suffix_compiles_as_subpath() -> std::io::Result<()> { let cwd = TempDir::new()?; @@ -266,6 +309,7 @@ fn read_write_trailing_glob_suffix_compiles_as_subpath() -> std::io::Result<()> "workspace".to_string(), PermissionProfileToml { filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, entries: BTreeMap::from([( ":project_roots".to_string(), FilesystemPermissionToml::Scoped(BTreeMap::from([( diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index 6c7f071f2b..51288e60b9 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -29,6 +29,7 @@ use codex_app_server_protocol::OverriddenMetadata; use codex_app_server_protocol::WriteStatus; use codex_config::CONFIG_TOML_FILE; use codex_config::config_toml::ConfigToml; +use codex_exec_server::LOCAL_FS; use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value as JsonValue; use std::borrow::Cow; @@ -424,6 +425,7 @@ impl ConfigService { async fn load_thread_agnostic_config(&self) -> std::io::Result { let cwd: Option = None; load_config_layers_state( + LOCAL_FS.as_ref(), &self.codex_home, cwd, &self.cli_overrides, diff --git a/codex-rs/core/src/config_loader/README.md b/codex-rs/core/src/config_loader/README.md index 04b72e4ca1..44a514a10a 100644 --- a/codex-rs/core/src/config_loader/README.md +++ b/codex-rs/core/src/config_loader/README.md @@ -10,7 +10,7 @@ This module is the canonical place to **load and describe Codex configuration la Exported from `codex_core::config_loader`: -- `load_config_layers_state(codex_home, cwd_opt, cli_overrides, overrides, cloud_requirements) -> ConfigLayerStack` +- `load_config_layers_state(fs, codex_home, cwd_opt, cli_overrides, overrides, cloud_requirements) -> ConfigLayerStack` - `ConfigLayerStack` - `effective_config() -> toml::Value` - `origins() -> HashMap` @@ -38,18 +38,22 @@ computing the effective config and origins metadata. This is what Most callers want the effective config plus metadata: ```rust -use codex_core::config_loader::{load_config_layers_state, LoaderOverrides}; +use codex_core::config_loader::{ + CloudRequirementsLoader, LoaderOverrides, load_config_layers_state, +}; +use codex_exec_server::LOCAL_FS; use codex_utils_absolute_path::AbsolutePathBuf; use toml::Value as TomlValue; let cli_overrides: Vec<(String, TomlValue)> = Vec::new(); let cwd = AbsolutePathBuf::current_dir()?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &cli_overrides, LoaderOverrides::default(), - None, + CloudRequirementsLoader::default(), ).await?; let effective = layers.effective_config(); diff --git a/codex-rs/core/src/config_loader/layer_io.rs b/codex-rs/core/src/config_loader/layer_io.rs index af77bdafa5..6bd9a9130f 100644 --- a/codex-rs/core/src/config_loader/layer_io.rs +++ b/codex-rs/core/src/config_loader/layer_io.rs @@ -5,11 +5,11 @@ use super::macos::ManagedAdminConfigLayer; use super::macos::load_managed_admin_config_layer; use codex_config::config_error_from_toml; use codex_config::io_error_from_config_error; +use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use std::io; use std::path::Path; use std::path::PathBuf; -use tokio::fs; use toml::Value as TomlValue; #[cfg(unix)] @@ -36,6 +36,7 @@ pub(super) struct LoadedConfigLayers { } pub(super) async fn load_config_layers_internal( + fs: &dyn ExecutorFileSystem, codex_home: &Path, overrides: LoaderOverrides, ) -> io::Result { @@ -57,7 +58,7 @@ pub(super) async fn load_config_layers_internal( )?; let managed_config = - read_config_from_path(&managed_config_path, /*log_missing_as_info*/ false) + read_config_from_path(fs, &managed_config_path, /*log_missing_as_info*/ false) .await? .map(|managed_config| MangedConfigFromFile { managed_config, @@ -89,15 +90,16 @@ fn map_managed_admin_layer(layer: ManagedAdminConfigLayer) -> ManagedConfigFromM } pub(super) async fn read_config_from_path( - path: impl AsRef, + fs: &dyn ExecutorFileSystem, + path: &AbsolutePathBuf, log_missing_as_info: bool, ) -> io::Result> { - match fs::read_to_string(path.as_ref()).await { + match fs.read_file_text(path, /*sandbox*/ None).await { Ok(contents) => match toml::from_str::(&contents) { Ok(value) => Ok(Some(value)), Err(err) => { - tracing::error!("Failed to parse {}: {err}", path.as_ref().display()); - let config_error = config_error_from_toml(path.as_ref(), &contents, err.clone()); + tracing::error!("Failed to parse {}: {err}", path.as_path().display()); + let config_error = config_error_from_toml(path.as_path(), &contents, err.clone()); Err(io_error_from_config_error( io::ErrorKind::InvalidData, config_error, @@ -107,14 +109,14 @@ pub(super) async fn read_config_from_path( }, Err(err) if err.kind() == io::ErrorKind::NotFound => { if log_missing_as_info { - tracing::info!("{} not found, using defaults", path.as_ref().display()); + tracing::info!("{} not found, using defaults", path.as_path().display()); } else { - tracing::debug!("{} not found", path.as_ref().display()); + tracing::debug!("{} not found", path.as_path().display()); } Ok(None) } Err(err) => { - tracing::error!("Failed to read {}: {err}", path.as_ref().display()); + tracing::error!("Failed to read {}: {err}", path.as_path().display()); Err(err) } } diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 8beab2c3d2..5eab0402c4 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -11,6 +11,7 @@ use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigRequirementsWithSources; use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; +use codex_exec_server::ExecutorFileSystem; use codex_git_utils::resolve_root_git_project_for_trust; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::SandboxMode; @@ -118,6 +119,7 @@ pub(crate) async fn first_layer_config_error_from_entries( /// thread-agnostic config loading (e.g., for the app server's `/config` /// endpoint) should `cwd` be `None`. pub async fn load_config_layers_state( + fs: &dyn ExecutorFileSystem, codex_home: &Path, cwd: Option, cli_overrides: &[(String, TomlValue)], @@ -142,11 +144,12 @@ pub async fn load_config_layers_state( // Honor the system requirements.toml location. let requirements_toml_file = system_requirements_toml_file()?; - load_requirements_toml(&mut config_requirements_toml, requirements_toml_file).await?; + load_requirements_toml(fs, &mut config_requirements_toml, &requirements_toml_file).await?; // Make a best-effort to support the legacy `managed_config.toml` as a // requirements specification. - let loaded_config_layers = layer_io::load_config_layers_internal(codex_home, overrides).await?; + let loaded_config_layers = + layer_io::load_config_layers_internal(fs, codex_home, overrides).await?; load_requirements_from_legacy_scheme( &mut config_requirements_toml, loaded_config_layers.clone(), @@ -173,7 +176,7 @@ pub async fn load_config_layers_state( // if it exists. let system_config_toml_file = system_config_toml_file()?; let system_layer = - load_config_toml_for_required_layer(&system_config_toml_file, |config_toml| { + load_config_toml_for_required_layer(fs, &system_config_toml_file, |config_toml| { ConfigLayerEntry::new( ConfigLayerSource::System { file: system_config_toml_file.clone(), @@ -188,7 +191,7 @@ pub async fn load_config_layers_state( // exists, but is malformed, then this error should be propagated to the // user. let user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home); - let user_layer = load_config_toml_for_required_layer(&user_file, |config_toml| { + let user_layer = load_config_toml_for_required_layer(fs, &user_file, |config_toml| { ConfigLayerEntry::new( ConfigLayerSource::User { file: user_file.clone(), @@ -222,6 +225,7 @@ pub async fn load_config_layers_state( } }; let project_trust_context = match project_trust_context( + fs, &merged_so_far, &cwd, &project_root_markers, @@ -247,6 +251,7 @@ pub async fn load_config_layers_state( } }; let project_layers = load_project_layers( + fs, &cwd, &project_trust_context.project_root, &project_trust_context, @@ -320,22 +325,23 @@ pub async fn load_config_layers_state( /// - If there is an error reading the file or parsing the TOML, returns an /// error. async fn load_config_toml_for_required_layer( - config_toml: impl AsRef, + fs: &dyn ExecutorFileSystem, + toml_file: &AbsolutePathBuf, create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry, ) -> io::Result { - let toml_file = config_toml.as_ref(); - let toml_value = match tokio::fs::read_to_string(toml_file).await { + let toml_value = match fs.read_file_text(toml_file, /*sandbox*/ None).await { Ok(contents) => { let config: TomlValue = toml::from_str(&contents).map_err(|err| { - let config_error = config_error_from_toml(toml_file, &contents, err.clone()); + let config_error = + config_error_from_toml(toml_file.as_path(), &contents, err.clone()); io_error_from_config_error(io::ErrorKind::InvalidData, config_error, Some(err)) })?; - let config_parent = toml_file.parent().ok_or_else(|| { + let config_parent = toml_file.as_path().parent().ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidData, format!( "Config file {} has no parent directory", - toml_file.display() + toml_file.as_path().display() ), ) })?; @@ -347,7 +353,10 @@ async fn load_config_toml_for_required_layer( } else { Err(io::Error::new( e.kind(), - format!("Failed to read config file {}: {e}", toml_file.display()), + format!( + "Failed to read config file {}: {e}", + toml_file.as_path().display() + ), )) } } @@ -360,12 +369,14 @@ async fn load_config_toml_for_required_layer( /// `requirements.toml` location to `config_requirements_toml` by filling in /// any unset fields. async fn load_requirements_toml( + fs: &dyn ExecutorFileSystem, config_requirements_toml: &mut ConfigRequirementsWithSources, - requirements_toml_file: impl AsRef, + requirements_toml_file: &AbsolutePathBuf, ) -> io::Result<()> { - let requirements_toml_file = - AbsolutePathBuf::from_absolute_path(requirements_toml_file.as_ref())?; - match tokio::fs::read_to_string(&requirements_toml_file).await { + match fs + .read_file_text(requirements_toml_file, /*sandbox*/ None) + .await + { Ok(contents) => { let requirements_config: ConfigRequirementsToml = toml::from_str(&contents).map_err(|e| { @@ -373,7 +384,7 @@ async fn load_requirements_toml( io::ErrorKind::InvalidData, format!( "Error parsing requirements file {}: {e}", - requirements_toml_file.as_ref().display(), + requirements_toml_file.as_path().display(), ), ) })?; @@ -390,7 +401,7 @@ async fn load_requirements_toml( e.kind(), format!( "Failed to read requirements file {}: {e}", - requirements_toml_file.as_ref().display(), + requirements_toml_file.as_path().display(), ), )); } @@ -643,6 +654,7 @@ fn project_layer_entry( } async fn project_trust_context( + fs: &dyn ExecutorFileSystem, merged_config: &TomlValue, cwd: &AbsolutePathBuf, project_root_markers: &[String], @@ -657,7 +669,7 @@ async fn project_trust_context( .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))? }; - let project_root = find_project_root(cwd, project_root_markers).await?; + let project_root = find_project_root(fs, cwd, project_root_markers).await?; let projects = project_trust_config.projects.unwrap_or_default(); let project_root_lookup_keys = normalized_project_trust_keys(project_root.as_path()); @@ -665,10 +677,10 @@ async fn project_trust_context( .first() .cloned() .unwrap_or_else(|| normalized_project_trust_key(project_root.as_path())); - let repo_root = resolve_root_git_project_for_trust(cwd.as_path()).await; + let repo_root = resolve_root_git_project_for_trust(fs, cwd).await; let repo_root_lookup_keys = repo_root .as_ref() - .map(|root| normalized_project_trust_keys(root)); + .map(|root| normalized_project_trust_keys(root.as_path())); let repo_root_key = repo_root_lookup_keys .as_ref() .and_then(|keys| keys.first().cloned()); @@ -802,6 +814,7 @@ fn copy_shape_from_original(original: &TomlValue, resolved: &TomlValue) -> TomlV } async fn find_project_root( + fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, project_root_markers: &[String], ) -> io::Result { @@ -809,11 +822,15 @@ async fn find_project_root( return Ok(cwd.clone()); } - for ancestor in cwd.as_path().ancestors() { + for ancestor in cwd.ancestors() { for marker in project_root_markers { let marker_path = ancestor.join(marker); - if tokio::fs::metadata(&marker_path).await.is_ok() { - return AbsolutePathBuf::from_absolute_path(ancestor); + if fs + .get_metadata(&marker_path, /*sandbox*/ None) + .await + .is_ok() + { + return Ok(ancestor); } } } @@ -826,6 +843,7 @@ async fn find_project_root( /// starting from folders closest to `project_root` (which is the lowest /// precedence) to those closest to `cwd` (which is the highest precedence). async fn load_project_layers( + fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, project_root: &AbsolutePathBuf, trust_context: &ProjectTrustContext, @@ -835,13 +853,12 @@ async fn load_project_layers( let codex_home_normalized = normalize_path(codex_home_abs.as_path()).unwrap_or_else(|_| codex_home_abs.to_path_buf()); let mut dirs = cwd - .as_path() .ancestors() .scan(false, |done, a| { if *done { None } else { - if a == project_root.as_path() { + if &a == project_root { *done = true; } Some(a) @@ -852,26 +869,25 @@ async fn load_project_layers( let mut layers = Vec::new(); for dir in dirs { - let dot_codex = dir.join(".codex"); - if !tokio::fs::metadata(&dot_codex) + let dot_codex_abs = dir.join(".codex"); + if !fs + .get_metadata(&dot_codex_abs, /*sandbox*/ None) .await - .map(|meta| meta.is_dir()) + .map(|metadata| metadata.is_directory) .unwrap_or(false) { continue; } - let layer_dir = AbsolutePathBuf::from_absolute_path(dir)?; - let decision = trust_context.decision_for_dir(&layer_dir); + let decision = trust_context.decision_for_dir(&dir); let disabled_reason = trust_context.disabled_reason_for_decision(&decision); - let dot_codex_abs = AbsolutePathBuf::from_absolute_path(&dot_codex)?; let dot_codex_normalized = normalize_path(dot_codex_abs.as_path()).unwrap_or_else(|_| dot_codex_abs.to_path_buf()); if dot_codex_abs == codex_home_abs || dot_codex_normalized == codex_home_normalized { continue; } let config_file = dot_codex_abs.join(CONFIG_TOML_FILE); - match tokio::fs::read_to_string(&config_file).await { + match fs.read_file_text(&config_file, /*sandbox*/ None).await { Ok(contents) => { let config: TomlValue = match toml::from_str(&contents) { Ok(config) => config, diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 63eebbb623..d024b57f15 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -16,6 +16,7 @@ use crate::config_loader::version_for_toml; use codex_config::CONFIG_TOML_FILE; use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; +use codex_exec_server::LOCAL_FS; use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::WebSearchMode; use codex_protocol::protocol::AskForApproval; @@ -92,6 +93,7 @@ async fn returns_config_error_for_invalid_user_config_toml() { let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); let err = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(cwd), &[] as &[(String, TomlValue)], @@ -119,6 +121,7 @@ async fn returns_config_error_for_invalid_managed_config_toml() { let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); let err = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(cwd), &[] as &[(String, TomlValue)], @@ -203,6 +206,7 @@ extra = true let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); let state = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(cwd), &[] as &[(String, TomlValue)], @@ -235,6 +239,7 @@ async fn returns_empty_when_all_layers_missing() { let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); let layers = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(cwd), &[] as &[(String, TomlValue)], @@ -327,6 +332,7 @@ flag = false let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); let state = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(cwd), &[] as &[(String, TomlValue)], @@ -428,6 +434,7 @@ allowed_sandbox_modes = ["read-only"] ); let state = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(AbsolutePathBuf::try_from(tmp.path())?), &[] as &[(String, TomlValue)], @@ -489,6 +496,7 @@ allowed_approval_policies = ["never"] ); let state = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(AbsolutePathBuf::try_from(tmp.path())?), &[] as &[(String, TomlValue)], @@ -529,8 +537,14 @@ personality = true ) .await?; + let requirements_file = AbsolutePathBuf::try_from(requirements_file)?; let mut config_requirements_toml = ConfigRequirementsWithSources::default(); - load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?; + load_requirements_toml( + LOCAL_FS.as_ref(), + &mut config_requirements_toml, + &requirements_file, + ) + .await?; assert_eq!( config_requirements_toml @@ -620,6 +634,7 @@ allowed_approval_policies = ["on-request"] ), ); let state = load_config_layers_state( + LOCAL_FS.as_ref(), tmp.path(), Some(AbsolutePathBuf::try_from(tmp.path())?), &[] as &[(String, TomlValue)], @@ -691,7 +706,12 @@ allowed_approval_policies = ["on-request"] guardian_policy_config: None, }, ); - load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?; + load_requirements_toml( + LOCAL_FS.as_ref(), + &mut config_requirements_toml, + &AbsolutePathBuf::try_from(requirements_file)?, + ) + .await?; assert_eq!( config_requirements_toml @@ -735,6 +755,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> let cloud_requirements = CloudRequirementsLoader::new(async move { Ok(Some(requirements)) }); let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &[] as &[(String, TomlValue)], @@ -771,6 +792,7 @@ async fn load_config_layers_fails_when_cloud_requirements_loader_fails() -> anyh let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?; let err = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &[] as &[(String, TomlValue)], @@ -823,6 +845,7 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> { .await?; let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &[] as &[(String, TomlValue)], @@ -967,6 +990,7 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s .await?; let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &[] as &[(String, TomlValue)], @@ -1006,6 +1030,7 @@ async fn codex_home_is_not_loaded_as_project_layer_from_home_dir() -> std::io::R let cwd = AbsolutePathBuf::from_absolute_path(&home_dir)?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &[] as &[(String, TomlValue)], @@ -1062,6 +1087,7 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &project_dot_codex, Some(cwd), &[] as &[(String, TomlValue)], @@ -1132,6 +1158,7 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< .await?; let layers_untrusted = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home_untrusted, Some(cwd.clone()), &[] as &[(String, TomlValue)], @@ -1170,6 +1197,7 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< .await?; let layers_unknown = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home_unknown, Some(cwd), &[] as &[(String, TomlValue)], @@ -1235,6 +1263,7 @@ async fn project_trust_does_not_match_configured_alias_for_canonical_cwd() -> st .await?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(AbsolutePathBuf::from_absolute_path(&project_root)?), &[] as &[(String, TomlValue)], @@ -1387,6 +1416,7 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io:: } let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd.clone()), &[] as &[(String, TomlValue)], @@ -1454,6 +1484,7 @@ async fn project_layer_without_config_toml_is_disabled_when_untrusted_or_unknown } let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd.clone()), &[] as &[(String, TomlValue)], @@ -1513,6 +1544,7 @@ async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io )]; load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &cli_overrides, @@ -1555,6 +1587,7 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<() let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; let layers = load_config_layers_state( + LOCAL_FS.as_ref(), &codex_home, Some(cwd), &[] as &[(String, TomlValue)], diff --git a/codex-rs/core/src/git_info_tests.rs b/codex-rs/core/src/git_info_tests.rs index e3ef1fa62f..0ffdd1c4df 100644 --- a/codex-rs/core/src/git_info_tests.rs +++ b/codex-rs/core/src/git_info_tests.rs @@ -1,3 +1,4 @@ +use codex_exec_server::LOCAL_FS; use codex_git_utils::GitInfo; use codex_git_utils::GitSha; use codex_git_utils::collect_git_info; @@ -5,6 +6,9 @@ use codex_git_utils::get_has_changes; use codex_git_utils::git_diff_to_remote; use codex_git_utils::recent_commits; use codex_git_utils::resolve_root_git_project_for_trust; +use codex_utils_path::normalize_for_path_comparison; +use core_test_support::PathBufExt; +use core_test_support::PathExt; use core_test_support::skip_if_sandbox; use std::fs; use std::path::PathBuf; @@ -430,7 +434,7 @@ async fn test_get_git_working_tree_state_branch_fallback() { async fn resolve_root_git_project_for_trust_returns_none_outside_repo() { let tmp = TempDir::new().expect("tempdir"); assert!( - resolve_root_git_project_for_trust(tmp.path()) + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &tmp.path().abs()) .await .is_none() ); @@ -439,18 +443,17 @@ async fn resolve_root_git_project_for_trust_returns_none_outside_repo() { #[tokio::test] async fn resolve_root_git_project_for_trust_regular_repo_returns_repo_root() { let temp_dir = TempDir::new().expect("Failed to create temp dir"); - let repo_path = create_test_git_repo(&temp_dir).await; - let expected = std::fs::canonicalize(&repo_path).unwrap(); + let repo_path = create_test_git_repo(&temp_dir).await.abs(); assert_eq!( - resolve_root_git_project_for_trust(&repo_path).await, - Some(expected.clone()) + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &repo_path).await, + Some(repo_path.clone()) ); let nested = repo_path.join("sub/dir"); - std::fs::create_dir_all(&nested).unwrap(); + std::fs::create_dir_all(nested.as_path()).unwrap(); assert_eq!( - resolve_root_git_project_for_trust(&nested).await, - Some(expected) + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested).await, + Some(repo_path) ); } @@ -473,17 +476,27 @@ async fn resolve_root_git_project_for_trust_detects_worktree_and_returns_main_ro .output() .expect("git worktree add"); - let expected = std::fs::canonicalize(&repo_path).ok(); - let got = resolve_root_git_project_for_trust(&wt_root) - .await - .and_then(|p| std::fs::canonicalize(p).ok()); - assert_eq!(got, expected); + let expected = normalize_for_path_comparison(&repo_path).unwrap(); + let wt_root = wt_root.abs(); + let got = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &wt_root).await; + assert_eq!( + got.as_ref() + .map(normalize_for_path_comparison) + .transpose() + .unwrap(), + Some(expected.clone()) + ); let nested = wt_root.join("nested/sub"); - std::fs::create_dir_all(&nested).unwrap(); - let got_nested = resolve_root_git_project_for_trust(&nested) - .await - .and_then(|p| std::fs::canonicalize(p).ok()); - assert_eq!(got_nested, expected); + std::fs::create_dir_all(nested.as_path()).unwrap(); + let got_nested = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested).await; + assert_eq!( + got_nested + .as_ref() + .map(normalize_for_path_comparison) + .transpose() + .unwrap(), + Some(expected) + ); } #[tokio::test] @@ -502,13 +515,15 @@ async fn resolve_root_git_project_for_trust_detects_worktree_pointer_without_git ) .unwrap(); - let expected = std::fs::canonicalize(&repo_root).unwrap(); + let expected = repo_root.abs(); + let worktree_root = worktree_root.abs(); assert_eq!( - resolve_root_git_project_for_trust(&worktree_root).await, + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &worktree_root).await, Some(expected.clone()) ); + let nested = worktree_root.join("nested"); assert_eq!( - resolve_root_git_project_for_trust(&worktree_root.join("nested")).await, + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested).await, Some(expected) ); } @@ -529,9 +544,15 @@ async fn resolve_root_git_project_for_trust_non_worktrees_gitdir_returns_none() ) .unwrap(); - assert!(resolve_root_git_project_for_trust(&proj).await.is_none()); + let proj = proj.abs(); assert!( - resolve_root_git_project_for_trust(&proj.join("nested")) + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &proj) + .await + .is_none() + ); + let nested = proj.join("nested"); + assert!( + resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested) .await .is_none() ); diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 485f222551..2712a94039 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -16,6 +16,7 @@ use crate::config_loader::RequirementSource; use crate::config_loader::Sourced; use crate::test_support; use codex_config::config_toml::ConfigToml; +use codex_exec_server::LOCAL_FS; use codex_network_proxy::NetworkProxyConfig; use codex_protocol::ThreadId; use codex_protocol::approvals::NetworkApprovalProtocol; @@ -1740,6 +1741,7 @@ async fn guardian_review_session_config_uses_requirements_guardian_policy_config ) .expect("config layer stack"); let parent_config = Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), ConfigToml::default(), ConfigOverrides { cwd: Some(workspace.path().to_path_buf()), @@ -1776,6 +1778,7 @@ async fn guardian_review_session_config_uses_default_guardian_policy_without_req ConfigLayerStack::new(Vec::new(), Default::default(), Default::default()) .expect("config layer stack"); let parent_config = Config::load_config_with_layer_stack( + LOCAL_FS.as_ref(), ConfigToml::default(), ConfigOverrides { cwd: Some(workspace.path().to_path_buf()), diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index b5eaf26b7b..af4280bfb4 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -16,6 +16,7 @@ use codex_config::CONFIG_TOML_FILE; use codex_config::permissions_toml::NetworkToml; use codex_config::permissions_toml::PermissionsToml; use codex_config::permissions_toml::overlay_network_domain_permissions; +use codex_exec_server::LOCAL_FS; use codex_network_proxy::ConfigReloader; use codex_network_proxy::ConfigState; use codex_network_proxy::NetworkProxyConfig; @@ -46,6 +47,7 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec Result<()> { + let codex_home = TempDir::new()?; + let source_root = TempDir::new()?; + write_marketplace_source(source_root.path(), "local copy")?; + + let err = add_marketplace_sync_with_cloner( + codex_home.path(), + MarketplaceAddRequest { + source: source_root.path().display().to_string(), + ref_name: None, + sparse_paths: vec![".agents".to_string()], + }, + |_url, _ref_name, _sparse_paths, _destination| { + panic!("git cloner should not be called for local marketplace sources") + }, + ) + .unwrap_err(); + + assert_eq!( + err.to_string(), + "--sparse is only supported for git marketplace sources" + ); + assert!( + !codex_home + .path() + .join(codex_config::CONFIG_TOML_FILE) + .exists() + ); + Ok(()) + } + #[test] fn add_marketplace_sync_treats_existing_local_directory_source_as_already_added() -> Result<()> { diff --git a/codex-rs/core/src/realtime_context.rs b/codex-rs/core/src/realtime_context.rs index c23394fa37..129a7de642 100644 --- a/codex-rs/core/src/realtime_context.rs +++ b/codex-rs/core/src/realtime_context.rs @@ -2,12 +2,14 @@ use crate::codex::Session; use crate::compact::content_items_to_text; use crate::event_mapping::is_contextual_user_message_content; use chrono::Utc; +use codex_exec_server::LOCAL_FS; use codex_git_utils::resolve_root_git_project_for_trust; use codex_protocol::models::ResponseItem; use codex_thread_store::ListThreadsParams; use codex_thread_store::StoredThread; use codex_thread_store::ThreadSortKey; use codex_thread_store::ThreadStore; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::truncate_text; use dirs::home_dir; @@ -145,18 +147,26 @@ async fn load_recent_threads(sess: &Session) -> Vec { } } -async fn build_recent_work_section(cwd: &Path, recent_threads: &[StoredThread]) -> Option { +async fn build_recent_work_section( + cwd: &AbsolutePathBuf, + recent_threads: &[StoredThread], +) -> Option { let mut groups: HashMap> = HashMap::new(); for entry in recent_threads { - let group = resolve_root_git_project_for_trust(&entry.cwd) - .await - .unwrap_or_else(|| entry.cwd.clone()); + let group = match AbsolutePathBuf::from_absolute_path(entry.cwd.as_path()) { + Ok(entry_cwd) => resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &entry_cwd) + .await + .map(AbsolutePathBuf::into_path_buf) + .unwrap_or_else(|| entry.cwd.clone()), + Err(_) => entry.cwd.clone(), + }; groups.entry(group).or_default().push(entry); } - let current_group = resolve_root_git_project_for_trust(cwd) + let current_group = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), cwd) .await - .unwrap_or_else(|| cwd.to_path_buf()); + .map(AbsolutePathBuf::into_path_buf) + .unwrap_or_else(|| cwd.clone().into_path_buf()); let mut groups = groups.into_iter().collect::>(); groups.sort_by(|(left_group, left_entries), (right_group, right_entries)| { let left_latest = left_entries @@ -309,18 +319,19 @@ pub(crate) fn truncate_realtime_text_to_token_budget(text: &str, budget_tokens: } async fn build_workspace_section_with_user_root( - cwd: &Path, + cwd: &AbsolutePathBuf, user_root: Option, ) -> Option { - let git_root = resolve_root_git_project_for_trust(cwd).await; - let cwd_tree = render_tree(cwd); + let cwd_path = cwd.as_path(); + let git_root = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), cwd).await; + let cwd_tree = render_tree(cwd_path); let git_root_tree = git_root .as_ref() - .filter(|git_root| git_root.as_path() != cwd) - .and_then(|git_root| render_tree(git_root)); + .filter(|git_root| git_root.as_path() != cwd_path) + .and_then(|git_root| render_tree(git_root.as_path())); let user_root_tree = user_root .as_ref() - .filter(|user_root| user_root.as_path() != cwd) + .filter(|user_root| user_root.as_path() != cwd_path) .filter(|user_root| { git_root .as_ref() @@ -333,8 +344,8 @@ async fn build_workspace_section_with_user_root( } let mut lines = vec![ - format!("Current working directory: {}", cwd.display()), - format!("Working directory name: {}", file_name_string(cwd)), + format!("Current working directory: {}", cwd_path.display()), + format!("Working directory name: {}", file_name_string(cwd_path)), ]; if let Some(git_root) = &git_root { @@ -465,14 +476,19 @@ async fn format_thread_group( entries: Vec<&StoredThread>, ) -> Option { let latest = entries.first()?; - let group_label = if resolve_root_git_project_for_trust(latest.cwd.as_path()) - .await - .is_some() - { - format!("### Git repo: {}", group.display()) - } else { - format!("### Directory: {}", group.display()) - }; + let group_label = + if let Ok(latest_cwd) = AbsolutePathBuf::from_absolute_path(latest.cwd.as_path()) { + if resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &latest_cwd) + .await + .is_some() + { + format!("### Git repo: {}", group.display()) + } else { + format!("### Directory: {}", group.display()) + } + } else { + format!("### Directory: {}", group.display()) + }; let mut lines = vec![ group_label, format!("Recent sessions: {}", entries.len()), diff --git a/codex-rs/core/src/realtime_context_tests.rs b/codex-rs/core/src/realtime_context_tests.rs index 99cb00f0de..4096a1692a 100644 --- a/codex-rs/core/src/realtime_context_tests.rs +++ b/codex-rs/core/src/realtime_context_tests.rs @@ -19,6 +19,8 @@ use codex_protocol::protocol::GitInfo; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_thread_store::StoredThread; +use core_test_support::PathBufExt; +use core_test_support::PathExt; use pretty_assertions::assert_eq; use std::fs; use std::path::PathBuf; @@ -234,7 +236,7 @@ fn fixed_section_budgets_apply_per_section_without_total_blob_truncation() { async fn workspace_section_requires_meaningful_structure() { let cwd = TempDir::new().expect("tempdir"); assert_eq!( - build_workspace_section_with_user_root(cwd.path(), /*user_root*/ None).await, + build_workspace_section_with_user_root(&cwd.path().abs(), /*user_root*/ None).await, None ); } @@ -245,9 +247,10 @@ async fn workspace_section_includes_tree_when_entries_exist() { fs::create_dir(cwd.path().join("docs")).expect("create docs dir"); fs::write(cwd.path().join("README.md"), "hello").expect("write readme"); - let section = build_workspace_section_with_user_root(cwd.path(), /*user_root*/ None) - .await - .expect("workspace section"); + let section = + build_workspace_section_with_user_root(&cwd.path().abs(), /*user_root*/ None) + .await + .expect("workspace section"); assert!(section.contains("Working directory tree:")); assert!(section.contains("- docs/")); assert!(section.contains("- README.md")); @@ -267,7 +270,7 @@ async fn workspace_section_includes_user_root_tree_when_distinct() { fs::create_dir_all(user_root.join("code")).expect("create user root child"); fs::write(user_root.join(".zshrc"), "export TEST=1").expect("write home file"); - let section = build_workspace_section_with_user_root(cwd.as_path(), Some(user_root)) + let section = build_workspace_section_with_user_root(&cwd.abs(), Some(user_root)) .await .expect("workspace section"); assert!(section.contains("User root tree:")); @@ -309,9 +312,9 @@ async fn recent_work_section_groups_threads_by_cwd() { stored_thread(outside.to_string_lossy().as_ref(), "", "Inspect flaky test"), ]; let current_cwd = workspace_a; - let repo = fs::canonicalize(repo).expect("canonicalize repo"); + let repo = repo.abs(); - let section = build_recent_work_section(current_cwd.as_path(), &recent_threads) + let section = build_recent_work_section(¤t_cwd.abs(), &recent_threads) .await .expect("recent work section"); assert!(section.contains(&format!("### Git repo: {}", repo.display()))); diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 99c7e4a195..f2604a57d6 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -20,8 +20,10 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::UnifiedExecContext; +use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::WriteStdinRequest; +use crate::unified_exec::generate_chunk_id; use codex_features::Feature; use codex_otel::SessionTelemetry; use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC; @@ -30,6 +32,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::TerminalInteractionEvent; use codex_shell_command::is_safe_command::is_known_safe_command; use codex_tools::UnifiedExecShellMode; +use codex_utils_output_truncation::approx_token_count; use serde::Deserialize; use std::path::PathBuf; use std::sync::Arc; @@ -309,7 +312,8 @@ impl ToolHandler for UnifiedExecHandler { } emit_unified_exec_tty_metric(&turn.session_telemetry, tty); - manager + let session_command = command.clone(); + match manager .exec_command( ExecCommandRequest { command, @@ -330,11 +334,31 @@ impl ToolHandler for UnifiedExecHandler { &context, ) .await - .map_err(|err| { - FunctionCallError::RespondToModel(format!( + { + Ok(response) => response, + Err(UnifiedExecError::SandboxDenied { output, .. }) => { + let output_text = output.aggregated_output.text; + let original_token_count = approx_token_count(&output_text); + ExecCommandToolOutput { + event_call_id: context.call_id.clone(), + chunk_id: generate_chunk_id(), + wall_time: output.duration, + raw_output: output_text.into_bytes(), + max_output_tokens, + // Sandbox denial is terminal, so there is no live + // process for write_stdin to resume. + process_id: None, + exit_code: Some(output.exit_code), + original_token_count: Some(original_token_count), + session_command: Some(session_command), + } + } + Err(err) => { + return Err(FunctionCallError::RespondToModel(format!( "exec_command failed for `{command_for_display}`: {err:?}" - )) - })? + ))); + } + } } "write_stdin" => { let args: WriteStdinArgs = parse_arguments(&arguments)?; diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index aaaa7076f4..9f9189abcc 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -26,6 +26,7 @@ use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; use crate::tools::sandboxing::ToolCtx; +use crate::tools::sandboxing::ToolError; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::MAX_UNIFIED_EXEC_PROCESSES; use crate::unified_exec::MAX_YIELD_TIME_MS; @@ -50,6 +51,8 @@ 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::error::CodexErr; +use codex_protocol::error::SandboxErr; use codex_protocol::protocol::ExecCommandSource; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::approx_token_count; @@ -778,7 +781,19 @@ impl UnifiedExecProcessManager { ) .await .map(|result| (result.output, result.deferred_network_approval)) - .map_err(|e| UnifiedExecError::create_process(format!("{e:?}"))) + .map_err(|err| match err { + ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => { + let output = *output; + let message = if output.aggregated_output.text.is_empty() { + let exit_code = output.exit_code; + format!("Process exited with code {exit_code}") + } else { + output.aggregated_output.text.clone() + }; + UnifiedExecError::sandbox_denied(message, output) + } + other => UnifiedExecError::create_process(format!("{other:?}")), + }) } pub(super) async fn collect_output_until_deadline( diff --git a/codex-rs/core/tests/suite/deprecation_notice.rs b/codex-rs/core/tests/suite/deprecation_notice.rs index 26a56c86e2..dc7280ea32 100644 --- a/codex-rs/core/tests/suite/deprecation_notice.rs +++ b/codex-rs/core/tests/suite/deprecation_notice.rs @@ -161,3 +161,45 @@ async fn emits_deprecation_notice_for_web_search_feature_flag_values() -> anyhow Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn emits_deprecation_notice_for_use_legacy_landlock() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + + let mut builder = test_codex().with_config(|config| { + let mut entries = BTreeMap::new(); + entries.insert("use_legacy_landlock".to_string(), true); + let mut features = config.features.get().clone(); + features.apply_map(&entries); + config + .features + .set(features) + .expect("test config should allow managed feature map updates"); + }); + + let TestCodex { codex, .. } = builder.build(&server).await?; + + let notice = wait_for_event_match(&codex, |event| match event { + EventMsg::DeprecationNotice(ev) + if ev.summary.contains("[features].use_legacy_landlock") => + { + Some(ev.clone()) + } + _ => None, + }) + .await; + + let DeprecationNoticeEvent { summary, details } = notice; + assert_eq!( + summary, + "`[features].use_legacy_landlock` is deprecated and will be removed soon.".to_string(), + ); + assert_eq!( + details.as_deref(), + Some("Remove this setting to stop opting into the legacy Linux sandbox behavior."), + ); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index bc286ce5ea..795aa1c847 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -9,10 +9,15 @@ use std::time::Instant; use anyhow::Context; use anyhow::Result; +use codex_config::Constrained; use codex_config::types::McpServerConfig; use codex_config::types::McpServerTransportConfig; use codex_core::sandboxing::SandboxPermissions; use codex_features::Feature; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use core_test_support::assert_regex_match; @@ -26,6 +31,7 @@ use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; +use core_test_support::skip_if_sandbox; use core_test_support::stdio_server_bin; use core_test_support::test_codex::test_codex; use regex_lite::Regex; @@ -379,7 +385,7 @@ async fn sandbox_denied_shell_returns_original_output() -> Result<()> { ]; let args = json!({ "command": command, - "timeout_ms": 1_000, + "timeout_ms": 5_000, }); let responses = vec![ @@ -449,6 +455,109 @@ async fn sandbox_denied_shell_returns_original_output() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_enforces_glob_deny_read_policy() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + let read_only_policy = SandboxPolicy::new_read_only_policy(); + let read_only_policy_for_config = read_only_policy.clone(); + let mut builder = test_codex() + .with_model("gpt-5.1-codex") + .with_config(move |config| { + config.permissions.sandbox_policy = Constrained::allow_any(read_only_policy_for_config); + let mut file_system_sandbox_policy = FileSystemSandboxPolicy::default(); + file_system_sandbox_policy + .entries + .push(FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: format!("{}/**/*.env", config.cwd.as_path().display()), + }, + access: FileSystemAccessMode::None, + }); + config.permissions.file_system_sandbox_policy = file_system_sandbox_policy; + }); + let fixture = builder.build(&server).await?; + + let fixture_dir = fixture.workspace_path("glob-deny-read"); + fs::create_dir_all(&fixture_dir).context("create glob deny-read fixture directory")?; + let denied_path = fixture_dir.join("secret.env"); + let allowed_path = fixture_dir.join("notes.txt"); + let secret = "shell glob deny-read secret"; + let allowed = "shell glob deny-read allowed"; + fs::write(&denied_path, format!("{secret}\n")).context("write denied fixture")?; + fs::write(&allowed_path, format!("{allowed}\n")).context("write allowed fixture")?; + + let call_id = "shell-glob-deny-read"; + let command = vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "status=0; cat \"$1\" || status=$?; cat \"$2\"; exit \"$status\"".to_string(), + "sh".to_string(), + denied_path.to_string_lossy().into_owned(), + allowed_path.to_string_lossy().into_owned(), + ]; + let args = json!({ + "command": command, + "timeout_ms": 1_000, + }); + + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ]; + let mock = mount_sse_sequence(&server, responses).await; + + fixture + .submit_turn_with_policy("read the fixture files", read_only_policy) + .await?; + + let output_text = mock + .function_call_output_text(call_id) + .context("shell output present")?; + let exit_code_line = output_text + .lines() + .next() + .context("exit code line present")?; + let exit_code = exit_code_line + .strip_prefix("Exit code: ") + .context("exit code prefix present")? + .trim() + .parse::() + .context("exit code is integer")?; + + assert_ne!( + exit_code, 0, + "glob deny-read should surface a non-zero exit code" + ); + assert!( + output_text.contains(allowed), + "expected allowed file contents in shell output: {output_text}" + ); + assert!( + !output_text.contains(secret), + "denied file contents leaked into shell output: {output_text}" + ); + let output_lower = output_text.to_lowercase(); + let has_denial = output_lower.contains("permission denied") + || output_lower.contains("operation not permitted") + || output_lower.contains("read-only file system"); + assert!( + has_denial, + "expected sandbox denial details in shell output: {output_text}" + ); + + Ok(()) +} + async fn collect_tools(use_unified_exec: bool) -> Result> { let server = start_mock_server().await; diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index b14c930999..77a15e92c0 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -5,8 +5,13 @@ use std::sync::OnceLock; use anyhow::Context; use anyhow::Result; +use codex_config::Constrained; use codex_exec_server::CreateDirectoryOptions; use codex_features::Feature; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandSource; @@ -2341,6 +2346,127 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> { Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn unified_exec_enforces_glob_deny_read_policy() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + let read_only_policy = SandboxPolicy::new_read_only_policy(); + let read_only_policy_for_config = read_only_policy.clone(); + let mut builder = test_codex().with_config(move |config| { + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config.permissions.sandbox_policy = Constrained::allow_any(read_only_policy_for_config); + let mut file_system_sandbox_policy = FileSystemSandboxPolicy::default(); + file_system_sandbox_policy + .entries + .push(FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: format!("{}/**/*.env", config.cwd.as_path().display()), + }, + access: FileSystemAccessMode::None, + }); + config.permissions.file_system_sandbox_policy = file_system_sandbox_policy; + }); + let TestCodex { + codex, + cwd, + session_configured, + .. + } = builder.build(&server).await?; + + let fixture_dir = cwd.path().join("glob-deny-read"); + fs::create_dir_all(&fixture_dir).context("create glob deny-read fixture directory")?; + let denied_path = fixture_dir.join("secret.env"); + let allowed_path = fixture_dir.join("notes.txt"); + let secret = "unified exec glob deny-read secret"; + let allowed = "unified exec glob deny-read allowed"; + fs::write(&denied_path, format!("{secret}\n")).context("write denied fixture")?; + fs::write(&allowed_path, format!("{allowed}\n")).context("write allowed fixture")?; + + let call_id = "uexec-glob-deny-read"; + let cmd = format!( + "read_status=0; cat {denied_path:?} || read_status=$?; cat {allowed_path:?}; exit $read_status" + ); + let args = serde_json::json!({ + "cmd": cmd, + "yield_time_ms": 5_000, + }); + + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ]; + let request_log = mount_sse_sequence(&server, responses).await; + + let session_model = session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "read the fixture files".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + approvals_reviewer: None, + sandbox_policy: read_only_policy, + model: session_model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; + + wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + + let requests = request_log.requests(); + assert!(!requests.is_empty(), "expected at least one POST request"); + let bodies = requests + .into_iter() + .map(|request| request.body_json()) + .collect::>(); + + let outputs = collect_tool_outputs(&bodies)?; + let output = outputs.get(call_id).expect("missing output"); + + assert!( + output.exit_code.is_some_and(|code| code != 0), + "glob deny-read should surface a non-zero exit code: {output:?}" + ); + assert!( + output.output.contains(allowed), + "expected allowed file contents in unified exec output: {output:?}" + ); + assert!( + !output.output.contains(secret), + "denied file contents leaked into unified exec output: {output:?}" + ); + let output_lower = output.output.to_lowercase(); + let has_denial = output_lower.contains("permission denied") + || output_lower.contains("operation not permitted") + || output_lower.contains("read-only file system"); + assert!( + has_denial, + "expected sandbox denial details in unified exec output: {output:?}" + ); + + Ok(()) +} + #[cfg(target_os = "macos")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> { diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index a2e656be44..586c366850 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -371,6 +371,12 @@ impl Features { "image_detail_original" => { continue; } + "use_legacy_landlock" => { + self.record_legacy_usage_force( + "features.use_legacy_landlock", + Feature::UseLegacyLandlock, + ); + } _ => {} } match feature_for_key(k) { @@ -456,6 +462,19 @@ fn legacy_usage_notice(alias: &str, feature: Feature) -> (String, Option format!("`{label}` is deprecated because web search is enabled by default."); (summary, Some(web_search_details().to_string())) } + Feature::UseLegacyLandlock => { + let label = match alias { + "features.use_legacy_landlock" | "use_legacy_landlock" => { + "[features].use_legacy_landlock" + } + _ => alias, + }; + let summary = format!("`{label}` is deprecated and will be removed soon."); + let details = + "Remove this setting to stop opting into the legacy Linux sandbox behavior." + .to_string(); + (summary, Some(details)) + } _ => { let label = if alias.contains('.') || alias.starts_with('[') { alias.to_string() @@ -728,7 +747,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::UseLegacyLandlock, key: "use_legacy_landlock", - stage: Stage::Stable, + stage: Stage::Deprecated, default_enabled: false, }, FeatureSpec { diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index d2f73cedad..fc81c73085 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -42,8 +42,8 @@ fn default_enabled_features_are_stable() { } #[test] -fn use_legacy_landlock_is_stable_and_disabled_by_default() { - assert_eq!(Feature::UseLegacyLandlock.stage(), Stage::Stable); +fn use_legacy_landlock_is_deprecated_and_disabled_by_default() { + assert_eq!(Feature::UseLegacyLandlock.stage(), Stage::Deprecated); assert_eq!(Feature::UseLegacyLandlock.default_enabled(), false); } @@ -166,6 +166,28 @@ fn image_generation_is_stable_and_enabled_by_default() { assert_eq!(Feature::ImageGeneration.default_enabled(), true); } +#[test] +fn use_legacy_landlock_config_records_deprecation_notice() { + let mut entries = BTreeMap::new(); + entries.insert("use_legacy_landlock".to_string(), true); + + let mut features = Features::with_defaults(); + features.apply_map(&entries); + + let usages = features.legacy_feature_usages().collect::>(); + assert_eq!(usages.len(), 1); + assert_eq!(usages[0].alias, "features.use_legacy_landlock"); + assert_eq!(usages[0].feature, Feature::UseLegacyLandlock); + assert_eq!( + usages[0].summary, + "`[features].use_legacy_landlock` is deprecated and will be removed soon." + ); + assert_eq!( + usages[0].details.as_deref(), + Some("Remove this setting to stop opting into the legacy Linux sandbox behavior.") + ); +} + #[test] fn image_detail_original_is_a_removed_feature_key() { assert_eq!( diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index 7ecc72dfa9..5500546db7 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -9,6 +9,8 @@ readme = "README.md" workspace = true [dependencies] +codex-exec-server = { workspace = true } +codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } futures = { workspace = true, features = ["alloc"] } once_cell = { workspace = true } diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index e28a0bae42..e7642a8658 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -4,6 +4,7 @@ use std::ffi::OsStr; use std::path::Path; use std::path::PathBuf; +use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use futures::future::join_all; use schemars::JsonSchema; @@ -618,30 +619,38 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option { /// `[get_git_repo_root]`, but resolves to the root of the main /// repository. Handles worktrees via filesystem inspection without invoking /// the `git` executable. -pub async fn resolve_root_git_project_for_trust(cwd: &Path) -> Option { - let base = if cwd.is_dir() { cwd } else { cwd.parent()? }; - let (repo_root, dot_git) = find_ancestor_git_entry(base)?; - if dot_git.is_dir() { - return Some(canonicalize_or_raw(repo_root)); +pub async fn resolve_root_git_project_for_trust( + fs: &dyn ExecutorFileSystem, + cwd: &AbsolutePathBuf, +) -> Option { + let base = match fs.get_metadata(cwd, /*sandbox*/ None).await { + Ok(metadata) if metadata.is_directory => cwd.clone(), + _ => cwd.parent()?, + }; + let (repo_root, dot_git) = find_ancestor_git_entry_with_fs(fs, &base).await?; + if fs + .get_metadata(&dot_git, /*sandbox*/ None) + .await + .ok()? + .is_directory + { + return Some(repo_root); } - let git_dir_s = std::fs::read_to_string(&dot_git).ok()?; + let git_dir_s = fs.read_file_text(&dot_git, /*sandbox*/ None).await.ok()?; let git_dir_rel = git_dir_s.trim().strip_prefix("gitdir:")?.trim(); if git_dir_rel.is_empty() { return None; } - let git_dir_path = canonicalize_or_raw( - AbsolutePathBuf::resolve_path_against_base(git_dir_rel, &repo_root).into_path_buf(), - ); + let git_dir_path = AbsolutePathBuf::resolve_path_against_base(git_dir_rel, repo_root.as_path()); let worktrees_dir = git_dir_path.parent()?; - if worktrees_dir.file_name() != Some(OsStr::new("worktrees")) { + if worktrees_dir.as_path().file_name() != Some(OsStr::new("worktrees")) { return None; } let common_dir = worktrees_dir.parent()?; - let main_repo_root = common_dir.parent()?; - Some(canonicalize_or_raw(main_repo_root.to_path_buf())) + common_dir.parent() } fn find_ancestor_git_entry(base_dir: &Path) -> Option<(PathBuf, PathBuf)> { @@ -663,8 +672,17 @@ fn find_ancestor_git_entry(base_dir: &Path) -> Option<(PathBuf, PathBuf)> { None } -fn canonicalize_or_raw(path: PathBuf) -> PathBuf { - std::fs::canonicalize(&path).unwrap_or(path) +async fn find_ancestor_git_entry_with_fs( + fs: &dyn ExecutorFileSystem, + base_dir: &AbsolutePathBuf, +) -> Option<(AbsolutePathBuf, AbsolutePathBuf)> { + for dir in base_dir.ancestors() { + let dot_git = dir.join(".git"); + if fs.get_metadata(&dot_git, /*sandbox*/ None).await.is_ok() { + return Some((dir, dot_git)); + } + } + None } /// Returns a list of local git branches. diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index 8f188e4bf0..d9d61d8650 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -1,6 +1,3 @@ -use std::fmt; -use std::path::PathBuf; - mod apply; mod branch; mod errors; @@ -16,6 +13,8 @@ pub use apply::extract_paths_from_patch; pub use apply::parse_git_apply_output; pub use apply::stage_paths; pub use branch::merge_base_with_head; +pub use codex_protocol::models::GhostCommit; +pub use codex_protocol::protocol::GitSha; pub use errors::GitToolingError; pub use ghost_commits::CreateGhostCommitOptions; pub use ghost_commits::GhostSnapshotConfig; @@ -45,72 +44,3 @@ pub use info::local_git_branches; pub use info::recent_commits; pub use info::resolve_root_git_project_for_trust; pub use platform::create_symlink; -use schemars::JsonSchema; -use serde::Deserialize; -use serde::Serialize; -use ts_rs::TS; - -type CommitID = String; - -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema, TS)] -#[serde(transparent)] -#[ts(type = "string")] -pub struct GitSha(pub String); - -impl GitSha { - pub fn new(sha: &str) -> Self { - Self(sha.to_string()) - } -} - -/// Details of a ghost commit created from a repository state. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] -pub struct GhostCommit { - id: CommitID, - parent: Option, - preexisting_untracked_files: Vec, - preexisting_untracked_dirs: Vec, -} - -impl GhostCommit { - /// Create a new ghost commit wrapper from a raw commit ID and optional parent. - pub fn new( - id: CommitID, - parent: Option, - preexisting_untracked_files: Vec, - preexisting_untracked_dirs: Vec, - ) -> Self { - Self { - id, - parent, - preexisting_untracked_files, - preexisting_untracked_dirs, - } - } - - /// Commit ID for the snapshot. - pub fn id(&self) -> &str { - &self.id - } - - /// Parent commit ID, if the repository had a `HEAD` at creation time. - pub fn parent(&self) -> Option<&str> { - self.parent.as_deref() - } - - /// Untracked or ignored files that already existed when the snapshot was captured. - pub fn preexisting_untracked_files(&self) -> &[PathBuf] { - &self.preexisting_untracked_files - } - - /// Untracked or ignored directories that already existed when the snapshot was captured. - pub fn preexisting_untracked_dirs(&self) -> &[PathBuf] { - &self.preexisting_untracked_dirs - } -} - -impl fmt::Display for GhostCommit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.id) - } -} diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index 751d6d5d17..c624251e63 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -20,6 +20,7 @@ clap = { workspace = true, features = ["derive"] } codex-protocol = { workspace = true } codex-sandboxing = { workspace = true } codex-utils-absolute-path = { workspace = true } +globset = { workspace = true } landlock = { workspace = true } libc = { workspace = true } seccompiler = { workspace = true } diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 3a004934b2..5745f4816c 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -7,7 +7,7 @@ This crate is responsible for producing: - the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox` - this should also be true of the `codex` multitool CLI -On Linux, the bubblewrap pipeline prefers the first `bwrap` found on `PATH` +On Linux, Codex prefers the first `bwrap` found on `PATH` outside the current working directory whenever it is available. If `bwrap` is present but too old to support `--argv0`, the helper keeps using system bubblewrap and switches to a @@ -23,7 +23,7 @@ commands that would enter the bubblewrap path. **Current Behavior** - Legacy `SandboxPolicy` / `sandbox_mode` configs remain supported. -- Bubblewrap is the default filesystem sandbox pipeline. +- Bubblewrap is the default filesystem sandbox. - If `bwrap` is present on `PATH` outside the current working directory, the helper uses it. - If `bwrap` is present but too old to support `--argv0`, the helper uses a @@ -46,32 +46,51 @@ commands that would enter the bubblewrap path. - Split-only filesystem policies that do not round-trip through the legacy `SandboxPolicy` model stay on bubblewrap so nested read-only or denied carveouts are preserved. -- When the default bubblewrap pipeline is active, the helper applies `PR_SET_NO_NEW_PRIVS` and a +- When bubblewrap is active, the helper applies `PR_SET_NO_NEW_PRIVS` and a seccomp network filter in-process. -- When the default bubblewrap pipeline is active, the filesystem is read-only by default via `--ro-bind / /`. -- When the default bubblewrap pipeline is active, writable roots are layered with `--bind `. -- When the default bubblewrap pipeline is active, protected subpaths under writable roots (for +- When bubblewrap is active, the filesystem is read-only by default via `--ro-bind / /`. +- When bubblewrap is active, writable roots are layered with `--bind `. +- When bubblewrap is active, protected subpaths under writable roots (for example `.git`, resolved `gitdir:`, and `.codex`) are re-applied as read-only via `--ro-bind`. -- When the default bubblewrap pipeline is active, overlapping split-policy +- When bubblewrap is active, overlapping split-policy entries are applied in path-specificity order so narrower writable children can reopen broader read-only or denied parents while narrower denied subpaths still win. For example, `/repo = write`, `/repo/a = none`, `/repo/a/b = write` keeps `/repo` writable, denies `/repo/a`, and reopens `/repo/a/b` as writable again. -- When the default bubblewrap pipeline is active, symlink-in-path and non-existent protected paths inside +- When bubblewrap is active, unreadable glob entries are expanded before + launching the sandbox and matching files are masked in bubblewrap: + + ```text + Prefer: rg --files --hidden --no-ignore --glob -- + Fallback: internal globset walker when rg is not installed + Failure: any other rg failure aborts sandbox construction + ``` + + Users can cap the scan depth per permissions profile: + + ```toml + [permissions.workspace.filesystem] + glob_scan_max_depth = 2 + + [permissions.workspace.filesystem.":project_roots"] + "**/*.env" = "none" + ``` + +- When bubblewrap is active, symlink-in-path and non-existent protected paths inside writable roots are blocked by mounting `/dev/null` on the symlink or first missing component. -- When the default bubblewrap pipeline is active, the helper explicitly isolates the user namespace via +- When bubblewrap is active, the helper explicitly isolates the user namespace via `--unshare-user` and the PID namespace via `--unshare-pid`. -- When the default bubblewrap pipeline is active and network is restricted without proxy routing, the helper also +- When bubblewrap is active and network is restricted without proxy routing, the helper also isolates the network namespace via `--unshare-net`. - In managed proxy mode, the helper uses `--unshare-net` plus an internal TCP->UDS->TCP routing bridge so tool traffic reaches only configured proxy endpoints. - In managed proxy mode, after the bridge is live, seccomp blocks new AF_UNIX/socketpair creation for the user command. -- When the default bubblewrap pipeline is active, it mounts a fresh `/proc` via `--proc /proc` by default, but +- When bubblewrap is active, it mounts a fresh `/proc` via `--proc /proc` by default, but you can skip this in restrictive container environments with `--no-proc`. **Notes** diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 91d6621029..64d1342bef 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -9,17 +9,27 @@ //! The overall Linux sandbox is composed of: //! - seccomp + `PR_SET_NO_NEW_PRIVS` applied in-process, and //! - bubblewrap used to construct the filesystem view before exec. +use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashSet; +use std::ffi::OsString; use std::fs; use std::fs::File; +use std::io; use std::os::fd::AsRawFd; +use std::os::unix::ffi::OsStringExt; use std::path::Path; use std::path::PathBuf; +use std::process::Command; +use codex_protocol::error::CodexErr; use codex_protocol::error::Result; use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::WritableRoot; use codex_utils_absolute_path::AbsolutePathBuf; +use globset::GlobBuilder; +use globset::GlobSet; +use globset::GlobSetBuilder; /// Linux "platform defaults" that keep common system binaries and dynamic /// libraries readable when `ReadOnlyAccess::Restricted` requests them. @@ -37,6 +47,8 @@ const LINUX_PLATFORM_DEFAULT_READ_ROOTS: &[&str] = &[ "/run/current-system/sw", ]; +const MAX_UNREADABLE_GLOB_MATCHES: usize = 8192; + /// Options that control how bubblewrap is invoked. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct BwrapOptions { @@ -47,6 +59,11 @@ pub(crate) struct BwrapOptions { pub mount_proc: bool, /// How networking should be configured inside the bubblewrap sandbox. pub network_mode: BwrapNetworkMode, + /// Optional maximum depth for expanding unreadable glob patterns with ripgrep. + /// + /// Keep this uncapped by default so existing nested deny-read matches are + /// masked before the sandboxed command starts. + pub glob_scan_max_depth: Option, } impl Default for BwrapOptions { @@ -54,6 +71,7 @@ impl Default for BwrapOptions { Self { mount_proc: true, network_mode: BwrapNetworkMode::FullAccess, + glob_scan_max_depth: None, } } } @@ -99,7 +117,11 @@ pub(crate) fn create_bwrap_command_args( command_cwd: &Path, options: BwrapOptions, ) -> Result { - if file_system_sandbox_policy.has_full_disk_write_access() { + let unreadable_globs = + file_system_sandbox_policy.get_unreadable_globs_with_cwd(sandbox_policy_cwd); + // Full disk write normally skips bwrap, but unreadable glob patterns still + // need concrete bwrap masks for the matches expanded below. + if file_system_sandbox_policy.has_full_disk_write_access() && unreadable_globs.is_empty() { return if options.network_mode == BwrapNetworkMode::FullAccess { Ok(BwrapArgs { args: command, @@ -157,7 +179,13 @@ fn create_bwrap_flags( let BwrapArgs { args: filesystem_args, preserved_files, - } = create_filesystem_args(file_system_sandbox_policy, sandbox_policy_cwd)?; + } = create_filesystem_args( + file_system_sandbox_policy, + sandbox_policy_cwd, + options + .glob_scan_max_depth + .or(file_system_sandbox_policy.glob_scan_max_depth), + )?; let normalized_command_cwd = normalize_command_cwd_for_bwrap(command_cwd); let mut args = Vec::new(); args.push("--new-session".to_string()); @@ -210,16 +238,41 @@ fn create_bwrap_flags( fn create_filesystem_args( file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, + glob_scan_max_depth: Option, ) -> Result { + let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd); // Bubblewrap requires bind mount targets to exist. Skip missing writable // roots so mixed-platform configs can keep harmless paths for other // environments without breaking Linux command startup. - let writable_roots = file_system_sandbox_policy + let mut writable_roots = file_system_sandbox_policy .get_writable_roots_with_cwd(cwd) .into_iter() .filter(|writable_root| writable_root.root.as_path().exists()) .collect::>(); - let unreadable_roots = file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd); + if writable_roots.is_empty() + && file_system_sandbox_policy.has_full_disk_write_access() + && !unreadable_globs.is_empty() + { + writable_roots.push(WritableRoot { + root: AbsolutePathBuf::from_absolute_path("/")?, + read_only_subpaths: Vec::new(), + }); + } + let mut unreadable_roots = file_system_sandbox_policy + .get_unreadable_roots_with_cwd(cwd) + .into_iter() + .map(AbsolutePathBuf::into_path_buf) + .collect::>(); + // Bubblewrap can only mask concrete paths. Expand unreadable glob patterns + // to the existing matches we can see before constructing the mount overlay; + // core tool helpers still evaluate the original patterns directly at read time. + unreadable_roots.extend( + expand_unreadable_globs_with_ripgrep(&unreadable_globs, cwd, glob_scan_max_depth)? + .into_iter() + .map(AbsolutePathBuf::into_path_buf), + ); + unreadable_roots.sort(); + unreadable_roots.dedup(); let mut args = if file_system_sandbox_policy.has_full_disk_read_access() { // Read-only root, then mount a minimal device tree. @@ -303,10 +356,7 @@ fn create_filesystem_args( allowed_write_paths.push(target); } } - let unreadable_paths: HashSet = unreadable_roots - .iter() - .map(|path| path.as_path().to_path_buf()) - .collect(); + let unreadable_paths: HashSet = unreadable_roots.iter().cloned().collect(); let mut sorted_writable_roots = writable_roots; sorted_writable_roots.sort_by_key(|writable_root| path_depth(writable_root.root.as_path())); // Mask only the unreadable ancestors that sit outside every writable root. @@ -323,7 +373,7 @@ fn create_filesystem_args( .iter() .any(|root| root.starts_with(unreadable_root)) }) - .map(|path| path.as_path().to_path_buf()) + .cloned() .collect(); unreadable_ancestors_of_writable_roots.sort_by_key(|path| path_depth(path)); @@ -343,7 +393,7 @@ fn create_filesystem_args( // target parents before binding the narrower writable descendant. if let Some(masking_root) = unreadable_roots .iter() - .map(AbsolutePathBuf::as_path) + .map(PathBuf::as_path) .filter(|unreadable_root| root.starts_with(unreadable_root)) .max_by_key(|unreadable_root| path_depth(unreadable_root)) { @@ -366,12 +416,12 @@ fn create_filesystem_args( } read_only_subpaths.sort_by_key(|path| path_depth(path)); for subpath in read_only_subpaths { - append_read_only_subpath_args(&mut args, &subpath, &allowed_write_paths); + append_read_only_subpath_args(&mut args, &subpath, &allowed_write_paths)?; } let mut nested_unreadable_roots: Vec = unreadable_roots .iter() - .filter(|path| path.as_path().starts_with(root)) - .map(|path| path.as_path().to_path_buf()) + .filter(|path| path.starts_with(root)) + .cloned() .collect(); if let Some(target) = &symlink_target { nested_unreadable_roots = @@ -396,7 +446,7 @@ fn create_filesystem_args( .iter() .any(|root| unreadable_root.starts_with(root) || root.starts_with(unreadable_root)) }) - .map(|path| path.as_path().to_path_buf()) + .cloned() .collect(); rootless_unreadable_roots.sort_by_key(|path| path_depth(path)); for unreadable_root in rootless_unreadable_roots { @@ -414,6 +464,244 @@ fn create_filesystem_args( }) } +fn expand_unreadable_globs_with_ripgrep( + patterns: &[String], + cwd: &Path, + max_depth: Option, +) -> Result> { + if patterns.is_empty() || max_depth == Some(0) { + return Ok(Vec::new()); + } + + // Group each pattern by the static path prefix before its first glob + // metacharacter. That keeps scans narrow, avoids searching from `/`, and + // lets one `rg --files` call handle all patterns under the same root. + let mut patterns_by_search_root: BTreeMap> = BTreeMap::new(); + for pattern in patterns { + if let Some((search_root, glob)) = split_pattern_for_ripgrep(pattern, cwd) + && search_root.as_path().is_dir() + { + patterns_by_search_root + .entry(search_root) + .or_default() + .push(glob); + } + } + + // Record both the logical match and any canonical symlink target. The bwrap + // overlay needs the resolved target to prevent a readable symlink path from + // bypassing an unreadable glob match. + let mut expanded_paths = BTreeSet::new(); + for (search_root, globs) in patterns_by_search_root { + for path in ripgrep_files(search_root.as_path(), &globs, max_depth)? { + if let Some(target) = canonical_target_if_symlinked_path(path.as_path()) { + expanded_paths.insert(AbsolutePathBuf::from_absolute_path_checked(target)?); + } + expanded_paths.insert(path); + if expanded_paths.len() > MAX_UNREADABLE_GLOB_MATCHES { + return Err(CodexErr::Fatal(format!( + "unreadable glob expansion for {} matched more than {MAX_UNREADABLE_GLOB_MATCHES} paths", + search_root.display() + ))); + } + } + } + + Ok(expanded_paths.into_iter().collect()) +} + +fn split_pattern_for_ripgrep(pattern: &str, cwd: &Path) -> Option<(AbsolutePathBuf, String)> { + // Resolve relative patterns once, then split at the first glob + // metacharacter. The prefix becomes the search root and the suffix stays as + // the ripgrep glob. Root-level glob scans are intentionally skipped because + // they are too broad for startup-time sandbox construction. + let absolute_pattern = AbsolutePathBuf::resolve_path_against_base(pattern, cwd); + let pattern = absolute_pattern.to_string_lossy(); + let first_glob_index = pattern + .char_indices() + .find_map(|(index, ch)| matches!(ch, '*' | '?' | '[' | ']').then_some(index))?; + let static_prefix = &pattern[..first_glob_index]; + if static_prefix.is_empty() || static_prefix == "/" { + return None; + } + let search_root_end = if static_prefix.ends_with('/') { + static_prefix.len() - 1 + } else { + static_prefix.rfind('/').unwrap_or(0) + }; + let search_root = if search_root_end == 0 { + PathBuf::from("/") + } else { + PathBuf::from(&pattern[..search_root_end]) + }; + let search_root = AbsolutePathBuf::from_absolute_path_checked(search_root).ok()?; + let glob = escape_unclosed_glob_classes(&pattern[search_root_end + 1..]); + (!glob.is_empty()).then_some((search_root, glob)) +} + +fn escape_unclosed_glob_classes(glob: &str) -> String { + // The filesystem policy accepts an unclosed `[` as a literal. Ripgrep treats + // that as invalid glob syntax, so escape only the unclosed class opener. + let mut escaped = String::with_capacity(glob.len()); + let mut chars = glob.chars(); + + while let Some(ch) = chars.next() { + if ch != '[' { + escaped.push(ch); + continue; + } + + let mut class = String::new(); + let mut closed = false; + for class_ch in chars.by_ref() { + if class_ch == ']' { + closed = true; + break; + } + class.push(class_ch); + } + + if closed { + escaped.push('['); + escaped.push_str(&class); + escaped.push(']'); + } else { + escaped.push_str(r"\["); + escaped.push_str(&class); + } + } + + escaped +} + +fn ripgrep_files( + search_root: &Path, + globs: &[String], + max_depth: Option, +) -> Result> { + // Use `rg --files` rather than shell expansion so dotfiles and ignored files + // are still considered. A status 1 with no stderr is ripgrep's "no matches" + // case, not a sandbox construction error. + let mut command = Command::new("rg"); + command + .arg("--files") + .arg("--hidden") + .arg("--no-ignore") + .arg("--null"); + if let Some(max_depth) = max_depth { + command.arg("--max-depth").arg(max_depth.to_string()); + } + for glob in globs { + command.arg("--glob").arg(glob); + } + command.arg("--").arg(search_root); + + /* + * Prefer ripgrep for unreadable glob expansion because it is fast and + * already implements the file-walking semantics we want here: include + * dotfiles, ignore ignore files, and do not recurse through symlinked + * directories. If `rg` is not installed in the runtime environment, fall + * back to the internal globset walker so sandbox construction still masks + * matching paths. Other ripgrep failures stay fatal so deny-read does not + * silently weaken. + */ + let output = match command.output() { + Ok(output) => output, + Err(err) if err.kind() == io::ErrorKind::NotFound => { + return glob_files(search_root, globs, max_depth); + } + Err(err) => return Err(err.into()), + }; + if !output.status.success() { + if output.status.code() == Some(1) && output.stderr.is_empty() { + return Ok(Vec::new()); + } + + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(CodexErr::Fatal(format!( + "ripgrep unreadable glob scan failed for {}: {stderr}", + search_root.display() + ))); + } + + let paths = output + .stdout + .split(|byte| *byte == b'\0') + .filter(|path| !path.is_empty()) + .map(|path| { + let path = PathBuf::from(OsString::from_vec(path.to_vec())); + if path.is_absolute() { + path + } else { + search_root.join(path) + } + }) + .map(AbsolutePathBuf::from_absolute_path_checked) + .collect::>>()?; + Ok(paths) +} + +fn glob_files( + search_root: &Path, + globs: &[String], + max_depth: Option, +) -> Result> { + let mut builder = GlobSetBuilder::new(); + for glob in globs { + let glob = GlobBuilder::new(glob) + .literal_separator(true) + .allow_unclosed_class(true) + .build() + .map_err(|err| { + CodexErr::Fatal(format!( + "unreadable glob pattern is invalid for {}: {err}", + search_root.display() + )) + })?; + builder.add(glob); + } + let glob_set = builder.build().map_err(|err| { + CodexErr::Fatal(format!( + "unreadable glob matcher failed for {}: {err}", + search_root.display() + )) + })?; + + let mut paths = Vec::new(); + collect_glob_files(search_root, search_root, &glob_set, max_depth, &mut paths)?; + Ok(paths) +} + +fn collect_glob_files( + search_root: &Path, + dir: &Path, + glob_set: &GlobSet, + remaining_depth: Option, + paths: &mut Vec, +) -> Result<()> { + for entry in fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + let file_type = entry.file_type()?; + let relative = path.strip_prefix(search_root).unwrap_or(path.as_path()); + + if (file_type.is_file() || file_type.is_symlink()) && glob_set.is_match(relative) { + paths.push(AbsolutePathBuf::from_absolute_path_checked(&path)?); + } + + if !file_type.is_dir() { + continue; + } + let remaining_depth = match remaining_depth { + Some(0 | 1) => continue, + Some(depth) => Some(depth - 1), + None => None, + }; + collect_glob_files(search_root, &path, glob_set, remaining_depth, paths)?; + } + Ok(()) +} + fn path_to_string(path: &Path) -> String { path.to_string_lossy().to_string() } @@ -423,6 +711,9 @@ fn path_depth(path: &Path) -> usize { } fn canonical_target_if_symlinked_path(path: &Path) -> Option { + // Return the fully resolved target only when some path component is a + // symlink. Callers use this to bind/mask the real filesystem location while + // leaving ordinary paths in their logical form. let mut current = PathBuf::new(); for component in path.components() { use std::path::Component; @@ -498,17 +789,19 @@ fn append_read_only_subpath_args( args: &mut Vec, subpath: &Path, allowed_write_paths: &[PathBuf], -) { - if let Some(target) = canonical_target_for_symlink_in_path(subpath, allowed_write_paths) { - // bwrap takes `--ro-bind `. Use the resolved target - // for both operands so a protected symlinked directory is remounted - // read-only in place instead of binding onto the symlink path itself. - let mount_source = path_to_string(&target); - let mount_destination = path_to_string(&target); - args.push("--ro-bind".to_string()); - args.push(mount_source); - args.push(mount_destination); - return; +) -> Result<()> { + if let Some(symlink) = first_writable_symlink_component_in_path(subpath, allowed_write_paths) { + /* + * A read-only carveout under a writable symlink cannot be made reliable + * with bwrap path arguments. Binding the symlink's current target would + * only protect a startup-time snapshot; the sandboxed process could + * replace the writable symlink before it reads through the logical path. + */ + return Err(CodexErr::Fatal(format!( + "cannot enforce sandbox read-only path {} because it crosses writable symlink {}", + subpath.display(), + symlink.display() + ))); } if !subpath.exists() { @@ -519,7 +812,7 @@ fn append_read_only_subpath_args( args.push("/dev/null".to_string()); args.push(path_to_string(&first_missing_component)); } - return; + return Ok(()); } if is_within_allowed_write_paths(subpath, allowed_write_paths) { @@ -527,6 +820,7 @@ fn append_read_only_subpath_args( args.push(path_to_string(subpath)); args.push(path_to_string(subpath)); } + Ok(()) } fn append_unreadable_root_args( @@ -535,16 +829,21 @@ fn append_unreadable_root_args( unreadable_root: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { - if let Some(target) = canonical_target_for_symlink_in_path(unreadable_root, allowed_write_paths) + if let Some(symlink) = + first_writable_symlink_component_in_path(unreadable_root, allowed_write_paths) { - // Apply unreadable handling to the resolved symlink target, not the - // logical symlink path, to avoid file-vs-directory bind mismatches. - return append_existing_unreadable_path_args( - args, - preserved_files, - &target, - allowed_write_paths, - ); + /* + * Deny-read masks must fail closed when the protected path crosses a + * symlink that remains writable to the sandboxed process. Resolving and + * masking the symlink's current target is a TOCTTOU snapshot: bwrap would + * protect the old target while the logical path could later point + * somewhere else. + */ + return Err(CodexErr::Fatal(format!( + "cannot enforce sandbox deny-read path {} because it crosses writable symlink {}", + unreadable_root.display(), + symlink.display() + ))); } if !unreadable_root.exists() { @@ -621,10 +920,16 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) - .any(|root| path.starts_with(root)) } -fn canonical_target_for_symlink_in_path( +fn first_writable_symlink_component_in_path( target_path: &Path, allowed_write_paths: &[PathBuf], ) -> Option { + /* + * Walk the logical path and report the first symlink component that lives + * under a writable root. These symlinks are mutable from inside the sandbox, + * so any mount or mask based on their resolved target would be racing a path + * the sandboxed process can change. + */ let mut current = PathBuf::new(); for component in target_path.components() { @@ -651,7 +956,7 @@ fn canonical_target_for_symlink_in_path( if metadata.file_type().is_symlink() && is_within_allowed_write_paths(¤t, allowed_write_paths) { - return fs::canonicalize(¤t).ok(); + return Some(current); } } @@ -703,6 +1008,26 @@ mod tests { use pretty_assertions::assert_eq; use tempfile::TempDir; + const NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH: Option = None; + + #[test] + fn default_unreadable_glob_scan_has_no_depth_cap() { + assert_eq!(BwrapOptions::default().glob_scan_max_depth, None); + } + + fn unreadable_glob_entry(pattern: String) -> FileSystemSandboxEntry { + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { pattern }, + access: FileSystemAccessMode::None, + } + } + + fn default_policy_with_unreadable_glob(pattern: String) -> FileSystemSandboxPolicy { + let mut policy = FileSystemSandboxPolicy::default(); + policy.entries.push(unreadable_glob_entry(pattern)); + policy + } + #[test] fn full_disk_write_full_network_returns_unwrapped_command() { let command = vec!["/bin/true".to_string()]; @@ -714,6 +1039,7 @@ mod tests { BwrapOptions { mount_proc: true, network_mode: BwrapNetworkMode::FullAccess, + ..Default::default() }, ) .expect("create bwrap args"); @@ -732,6 +1058,7 @@ mod tests { BwrapOptions { mount_proc: true, network_mode: BwrapNetworkMode::ProxyOnly, + ..Default::default() }, ) .expect("create bwrap args"); @@ -755,6 +1082,42 @@ mod tests { ); } + #[test] + fn full_disk_write_with_unreadable_glob_still_wraps_and_masks_match() { + if !ripgrep_available() { + return; + } + + let temp_dir = TempDir::new().expect("temp dir"); + let root_env = temp_dir.path().join(".env"); + std::fs::write(&root_env, "secret").expect("write env"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + unreadable_glob_entry(format!("{}/**/*.env", temp_dir.path().display())), + ]); + let command = vec!["/bin/true".to_string()]; + + let args = create_bwrap_command_args( + command.clone(), + &policy, + temp_dir.path(), + temp_dir.path(), + BwrapOptions::default(), + ) + .expect("create bwrap args"); + + assert_ne!( + args.args, command, + "full-write policy with unreadable globs must still use bwrap" + ); + assert_file_masked(&args.args, &root_env); + } + #[cfg(unix)] #[test] fn restricted_policy_chdirs_to_canonical_command_cwd() { @@ -859,7 +1222,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); assert!(args.args.windows(3).any(|window| { window == ["--bind", real_root_str.as_str(), real_root_str.as_str()] @@ -902,7 +1267,9 @@ mod tests { access: FileSystemAccessMode::Write, }]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); assert!(args.args.windows(3).any(|window| { window @@ -924,7 +1291,7 @@ mod tests { #[cfg(unix)] #[test] - fn protected_symlinked_directory_subpaths_bind_target_read_only() { + fn protected_symlinked_directory_subpaths_fail_closed() { let temp_dir = TempDir::new().expect("temp dir"); let root = temp_dir.path().join("root"); let agents_target = root.join("agents-target"); @@ -934,28 +1301,26 @@ mod tests { let root = AbsolutePathBuf::from_absolute_path(&root).expect("absolute root"); let agents_link_str = path_to_string(&agents_link); - let agents_target_str = path_to_string(&agents_target); let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { path: root }, access: FileSystemAccessMode::Write, }]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let err = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect_err("protected symlinked subpath should fail closed"); + let message = err.to_string(); - assert!(args.args.windows(3).any(|window| { - window - == [ - "--ro-bind", - agents_target_str.as_str(), - agents_target_str.as_str(), - ] - })); - assert!(!args.args.iter().any(|arg| arg == agents_link_str.as_str())); + assert!( + message.contains("cannot enforce sandbox read-only path"), + "{message}" + ); + assert!(message.contains(&agents_link_str), "{message}"); } #[cfg(unix)] #[test] - fn symlinked_writable_roots_mask_nested_symlink_escape_paths_without_binding_targets() { + fn symlinked_writable_roots_nested_symlink_escape_paths_fail_closed() { let temp_dir = TempDir::new().expect("temp dir"); let real_root = temp_dir.path().join("real"); let link_root = temp_dir.path().join("link"); @@ -971,7 +1336,6 @@ mod tests { AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); let link_private = link_root.join("linked-private"); let real_linked_private_str = path_to_string(&linked_private); - let outside_str = path_to_string(&outside); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { path: link_root }, @@ -983,25 +1347,16 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let err = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect_err("deny-read path crossing writable symlink should fail closed"); + let message = err.to_string(); - assert!(args.args.windows(6).any(|window| { - window - == [ - "--perms", - "000", - "--tmpfs", - outside_str.as_str(), - "--remount-ro", - outside_str.as_str(), - ] - })); assert!( - !args - .args - .iter() - .any(|arg| arg == real_linked_private_str.as_str()) + message.contains("cannot enforce sandbox deny-read path"), + "{message}" ); + assert!(message.contains(&real_linked_private_str), "{message}"); } #[test] @@ -1022,8 +1377,12 @@ mod tests { exclude_slash_tmp: true, }; - let args = create_filesystem_args(&FileSystemSandboxPolicy::from(&policy), temp_dir.path()) - .expect("filesystem args"); + let args = create_filesystem_args( + &FileSystemSandboxPolicy::from(&policy), + temp_dir.path(), + NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, + ) + .expect("filesystem args"); let existing_root = path_to_string(&existing_root); let missing_root = path_to_string(&missing_root); @@ -1052,6 +1411,7 @@ mod tests { let args = create_filesystem_args( &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), + NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, ) .expect("bwrap fs args"); assert_eq!( @@ -1100,8 +1460,12 @@ mod tests { network_access: false, }; - let args = create_filesystem_args(&FileSystemSandboxPolicy::from(&policy), temp_dir.path()) - .expect("filesystem args"); + let args = create_filesystem_args( + &FileSystemSandboxPolicy::from(&policy), + temp_dir.path(), + NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, + ) + .expect("filesystem args"); assert_eq!(args.args[0..4], ["--tmpfs", "/", "--dev", "/dev"]); @@ -1130,8 +1494,12 @@ mod tests { // `ReadOnlyAccess::Restricted` always includes `cwd` as a readable // root. Using `"/"` here would intentionally collapse to broad read // access, so use a non-root cwd to exercise the restricted path. - let args = create_filesystem_args(&FileSystemSandboxPolicy::from(&policy), temp_dir.path()) - .expect("filesystem args"); + let args = create_filesystem_args( + &FileSystemSandboxPolicy::from(&policy), + temp_dir.path(), + NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, + ) + .expect("filesystem args"); assert!( args.args @@ -1171,7 +1539,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); assert!(args.args.windows(3).any(|window| { window @@ -1248,7 +1618,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); let docs_str = path_to_string(docs.as_path()); let docs_public_str = path_to_string(docs_public.as_path()); let docs_ro_index = args @@ -1300,7 +1672,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); let blocked_str = path_to_string(blocked.as_path()); let allowed_str = path_to_string(allowed.as_path()); let blocked_none_index = args @@ -1367,7 +1741,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); let blocked_str = path_to_string(blocked.as_path()); let allowed_dir_str = path_to_string(allowed_dir.as_path()); let allowed_file_str = path_to_string(allowed_file.as_path()); @@ -1442,7 +1818,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); let blocked_none_index = args .args .windows(4) @@ -1487,7 +1865,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); let blocked_str = path_to_string(blocked.as_path()); assert!( @@ -1529,7 +1909,9 @@ mod tests { }, ]); - let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); let blocked_file_str = path_to_string(blocked_file.as_path()); assert_eq!(args.preserved_files.len(), 1); @@ -1540,4 +1922,100 @@ mod tests { && window[4] == blocked_file_str })); } + + #[test] + fn unreadable_globs_expand_existing_matches_with_configured_depth() { + if !ripgrep_available() { + return; + } + + let temp_dir = TempDir::new().expect("temp dir"); + let root_env = temp_dir.path().join(".env"); + let nested_env = temp_dir.path().join("app").join(".env"); + let too_deep_env = temp_dir.path().join("app").join("deep").join(".env"); + std::fs::create_dir_all(too_deep_env.parent().expect("parent")).expect("create parent"); + std::fs::write(temp_dir.path().join(".gitignore"), ".env\n").expect("write gitignore"); + std::fs::write(&root_env, "secret").expect("write root env"); + std::fs::write(&nested_env, "secret").expect("write nested env"); + std::fs::write(&too_deep_env, "secret").expect("write deep env"); + let policy = + default_policy_with_unreadable_glob(format!("{}/**/*.env", temp_dir.path().display())); + + let args = + create_filesystem_args(&policy, temp_dir.path(), Some(2)).expect("filesystem args"); + + assert_file_masked(&args.args, &root_env); + assert_file_masked(&args.args, &nested_env); + assert!( + !args + .args + .iter() + .any(|arg| arg == &path_to_string(&too_deep_env)), + "max depth should keep deeper matches out of bwrap args: {:#?}", + args.args + ); + } + + #[cfg(unix)] + #[test] + fn unreadable_globs_add_canonical_targets_for_symlink_matches() { + if !ripgrep_available() { + return; + } + + let temp_dir = TempDir::new().expect("temp dir"); + let real_root = temp_dir.path().join("real"); + let link_root = temp_dir.path().join("link"); + let real_secret = real_root.join("secret.env"); + std::fs::create_dir_all(&real_root).expect("create real root"); + std::fs::write(&real_secret, "secret").expect("write real secret"); + std::os::unix::fs::symlink(&real_root, &link_root).expect("create symlink"); + let policy = + default_policy_with_unreadable_glob(format!("{}/**/*.env", link_root.display())); + + let args = + create_filesystem_args(&policy, temp_dir.path(), Some(2)).expect("filesystem args"); + + assert_file_masked(&args.args, &real_secret); + } + + #[test] + fn root_prefix_unreadable_globs_are_too_broad_for_linux_expansion() { + assert_eq!( + split_pattern_for_ripgrep("/**/*.env", Path::new("/tmp")), + None + ); + } + + #[test] + fn unclosed_character_classes_are_escaped_for_ripgrep() { + let (search_root, glob) = + split_pattern_for_ripgrep("/tmp/[*.env", Path::new("/")).expect("split pattern"); + + assert_eq!(search_root.as_path(), Path::new("/tmp")); + assert_eq!(glob, r"\[*.env"); + } + + fn ripgrep_available() -> bool { + Command::new("rg") + .arg("--version") + .output() + .is_ok_and(|output| output.status.success()) + } + + /// Assert that `path` is masked due to a bwrap arg sequence like: + /// + /// `bwrap ... --perms 000 --ro-bind-data FD PATH` + fn assert_file_masked(args: &[String], path: &Path) { + let path = path_to_string(path); + assert!( + args.windows(5).any(|window| { + window[0] == "--perms" + && window[1] == "000" + && window[2] == "--ro-bind-data" + && window[4] == path + }), + "expected file mask for {path}: {args:#?}" + ); + } } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 8f39837de1..958d8645bd 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -427,6 +427,7 @@ fn run_bwrap_with_proc_fallback( let options = BwrapOptions { mount_proc, network_mode, + ..Default::default() }; let mut bwrap_args = build_bwrap_argv( inner, @@ -547,6 +548,7 @@ fn build_preflight_bwrap_argv( BwrapOptions { mount_proc: true, network_mode, + ..Default::default() }, ) } diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index 96c3c8a737..0ed10717f2 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -48,6 +48,7 @@ fn inserts_bwrap_argv0_before_command_separator() { BwrapOptions { mount_proc: true, network_mode: BwrapNetworkMode::FullAccess, + ..Default::default() }, ) .args; @@ -90,6 +91,7 @@ fn rewrites_inner_command_path_when_bwrap_lacks_argv0() { BwrapOptions { mount_proc: true, network_mode: BwrapNetworkMode::FullAccess, + ..Default::default() }, ) .args; @@ -157,6 +159,7 @@ fn inserts_unshare_net_when_network_isolation_requested() { BwrapOptions { mount_proc: true, network_mode: BwrapNetworkMode::Isolated, + ..Default::default() }, ) .args; @@ -174,6 +177,7 @@ fn inserts_unshare_net_when_proxy_only_network_mode_requested() { BwrapOptions { mount_proc: true, network_mode: BwrapNetworkMode::ProxyOnly, + ..Default::default() }, ) .args; diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index f70dc94585..2bd46d7d5a 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -16,7 +16,6 @@ chardetng = { workspace = true } chrono = { workspace = true, features = ["serde"] } codex-async-utils = { workspace = true } codex-execpolicy = { workspace = true } -codex-git-utils = { workspace = true } codex-network-proxy = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-image = { workspace = true } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 2200456527..8c55d056cd 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1,5 +1,7 @@ use std::collections::HashMap; +use std::fmt; use std::path::Path; +use std::path::PathBuf; use std::sync::LazyLock; use codex_utils_image::PromptImageMode; @@ -25,7 +27,6 @@ use crate::protocol::SandboxPolicy; use crate::protocol::WritableRoot; use crate::user_input::UserInput; use codex_execpolicy::Policy; -use codex_git_utils::GhostCommit; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_image::ImageProcessingError; use schemars::JsonSchema; @@ -45,6 +46,60 @@ static SANDBOX_MODE_READ_ONLY_TEMPLATE: LazyLock