diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index c85da0f5f2..a63f124bb8 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -16,7 +16,7 @@ use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_protocol::config_types::SandboxMode; use codex_protocol::permissions::NetworkSandboxPolicy; -use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; +use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_permission_profile; #[cfg(target_os = "macos")] use codex_sandboxing::seatbelt::CreateSeatbeltCommandArgsParams; #[cfg(target_os = "macos")] @@ -222,16 +222,11 @@ async fn run_command_under_sandbox( .codex_linux_sandbox_exe .expect("codex-linux-sandbox executable not found"); let use_legacy_landlock = config.features.use_legacy_landlock(); - let file_system_sandbox_policy = config.permissions.file_system_sandbox_policy(); let network_sandbox_policy = config.permissions.network_sandbox_policy(); - let args = create_linux_sandbox_command_args_for_policies( + let args = create_linux_sandbox_command_args_for_permission_profile( command, cwd.as_path(), - &config - .permissions - .legacy_sandbox_policy(sandbox_policy_cwd.as_path()), - &file_system_sandbox_policy, - network_sandbox_policy, + &config.permissions.permission_profile(), sandbox_policy_cwd.as_path(), use_legacy_landlock, /*allow_network_for_proxy*/ false, diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 5f2877de26..b2d37b17bc 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -186,11 +186,45 @@ pub struct ToolSuggestDiscoverable { pub id: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct ToolSuggestDisabledTool { + #[serde(rename = "type")] + pub kind: ToolSuggestDiscoverableType, + pub id: String, +} + +impl ToolSuggestDisabledTool { + pub fn plugin(id: impl Into) -> Self { + Self { + kind: ToolSuggestDiscoverableType::Plugin, + id: id.into(), + } + } + + pub fn connector(id: impl Into) -> Self { + Self { + kind: ToolSuggestDiscoverableType::Connector, + id: id.into(), + } + } + + pub fn normalized(&self) -> Option { + let id = self.id.trim(); + (!id.is_empty()).then(|| Self { + kind: self.kind, + id: id.to_string(), + }) + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct ToolSuggestConfig { #[serde(default)] pub discoverables: Vec, + #[serde(default)] + pub disabled_tools: Vec, } /// Memories settings loaded from config.toml. diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index c971895e04..a17426d508 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2169,6 +2169,13 @@ "ToolSuggestConfig": { "additionalProperties": false, "properties": { + "disabled_tools": { + "default": [], + "items": { + "$ref": "#/definitions/ToolSuggestDisabledTool" + }, + "type": "array" + }, "discoverables": { "default": [], "items": { @@ -2179,6 +2186,22 @@ }, "type": "object" }, + "ToolSuggestDisabledTool": { + "additionalProperties": false, + "properties": { + "id": { + "type": "string" + }, + "type": { + "$ref": "#/definitions/ToolSuggestDiscoverableType" + } + }, + "required": [ + "id", + "type" + ], + "type": "object" + }, "ToolSuggestDiscoverable": { "additionalProperties": false, "properties": { diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 02a897d5df..23becfa580 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -46,6 +46,7 @@ use codex_config::types::NotificationMethod; use codex_config::types::Notifications; use codex_config::types::SandboxWorkspaceWrite; use codex_config::types::SkillsConfig; +use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverableType; use codex_config::types::Tui; use codex_config::types::TuiKeymap; @@ -8227,6 +8228,7 @@ discoverables = [ id: " ".to_string(), }, ], + disabled_tools: Vec::new(), }) ); @@ -8251,11 +8253,118 @@ discoverables = [ id: "plugin_alpha@openai-curated".to_string(), }, ], + disabled_tools: Vec::new(), } ); Ok(()) } +#[tokio::test] +async fn tool_suggest_disabled_tools_load_from_config_toml() -> std::io::Result<()> { + let cfg: ConfigToml = toml::from_str( + r#" +[tool_suggest] +disabled_tools = [ + { type = "connector", id = " connector_calendar " }, + { type = "connector", id = "connector_calendar" }, + { type = "connector", id = " " }, + { type = "plugin", id = "slack@openai-curated" } +] +"#, + ) + .expect("TOML deserialization should succeed"); + + assert_eq!( + cfg.tool_suggest, + Some(ToolSuggestConfig { + discoverables: Vec::new(), + disabled_tools: vec![ + ToolSuggestDisabledTool::connector(" connector_calendar "), + ToolSuggestDisabledTool::connector("connector_calendar"), + ToolSuggestDisabledTool::connector(" "), + ToolSuggestDisabledTool::plugin("slack@openai-curated"), + ], + }) + ); + + let codex_home = TempDir::new()?; + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.tool_suggest, + ToolSuggestConfig { + discoverables: Vec::new(), + disabled_tools: vec![ + ToolSuggestDisabledTool::connector("connector_calendar"), + ToolSuggestDisabledTool::plugin("slack@openai-curated"), + ], + } + ); + Ok(()) +} + +#[tokio::test] +async fn tool_suggest_disabled_tools_merge_across_config_layers() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let workspace = TempDir::new()?; + let workspace_key = workspace.path().to_string_lossy().replace('\\', "\\\\"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + format!( + r#" +[projects."{workspace_key}"] +trust_level = "trusted" + +[tool_suggest] +disabled_tools = [ + {{ type = "connector", id = " user_connector " }}, + {{ type = "plugin", id = "shared_plugin" }}, + {{ type = "connector", id = "project_connector" }}, +] +"# + ), + )?; + + let project_config_dir = workspace.path().join(".codex"); + std::fs::create_dir_all(&project_config_dir)?; + std::fs::write( + project_config_dir.join(CONFIG_TOML_FILE), + r#" +[tool_suggest] +disabled_tools = [ + { type = "connector", id = "project_connector" }, + { type = "plugin", id = "project_plugin" }, + { type = "plugin", id = "shared_plugin" }, +] +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + cwd: Some(workspace.path().to_path_buf()), + ..Default::default() + }) + .build() + .await?; + + assert_eq!( + config.tool_suggest.disabled_tools, + vec![ + ToolSuggestDisabledTool::connector("user_connector"), + ToolSuggestDisabledTool::plugin("shared_plugin"), + ToolSuggestDisabledTool::connector("project_connector"), + ToolSuggestDisabledTool::plugin("project_plugin"), + ] + ); + Ok(()) +} + #[tokio::test] async fn experimental_realtime_start_instructions_load_from_config_toml() -> std::io::Result<()> { let cfg: ConfigToml = toml::from_str( diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index f86e0d925b..80b54aeaf0 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -3,6 +3,7 @@ use crate::path_utils::write_atomically; use anyhow::Context; use codex_config::CONFIG_TOML_FILE; use codex_config::types::McpServerConfig; +use codex_config::types::ToolSuggestDisabledTool; use codex_features::FEATURES; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ServiceTier; @@ -10,6 +11,7 @@ use codex_protocol::config_types::TrustLevel; use codex_protocol::openai_models::ReasoningEffort; use std::collections::BTreeMap; use std::collections::HashMap; +use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; use tokio::task; @@ -57,6 +59,8 @@ pub enum ConfigEdit { RecordModelMigrationSeen { from: String, to: String }, /// Replace the entire `[mcp_servers]` table. ReplaceMcpServers(BTreeMap), + /// Add a disabled tool suggestion under `[tool_suggest].disabled_tools`. + AddToolSuggestDisabledTool(ToolSuggestDisabledTool), /// Set or clear a skill config entry under `[[skills.config]]` by path. SetSkillConfig { path: PathBuf, enabled: bool }, /// Set or clear a skill config entry under `[[skills.config]]` by name. @@ -180,10 +184,13 @@ mod document_helpers { use codex_config::types::McpServerEnvVar; use codex_config::types::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; + use codex_config::types::ToolSuggestDisabledTool; + use codex_config::types::ToolSuggestDiscoverableType; use toml_edit::Array as TomlArray; use toml_edit::InlineTable; use toml_edit::Item as TomlItem; use toml_edit::Table as TomlTable; + use toml_edit::Value as TomlValue; use toml_edit::value; pub(super) fn ensure_table_for_write(item: &mut TomlItem) -> Option<&mut TomlTable> { @@ -379,6 +386,57 @@ mod document_helpers { table } + pub(super) fn parse_tool_suggest_disabled_tool( + value: &TomlValue, + ) -> Option { + let table = value.as_inline_table()?; + let kind = match table.get("type").and_then(TomlValue::as_str) { + Some("connector") => ToolSuggestDiscoverableType::Connector, + Some("plugin") => ToolSuggestDiscoverableType::Plugin, + _ => return None, + }; + let id = table.get("id").and_then(TomlValue::as_str)?; + Some(ToolSuggestDisabledTool { + kind, + id: id.to_string(), + }) + } + + pub(super) fn parse_tool_suggest_disabled_tool_table( + table: &TomlTable, + ) -> Option { + let kind = match table.get("type").and_then(TomlItem::as_str) { + Some("connector") => ToolSuggestDiscoverableType::Connector, + Some("plugin") => ToolSuggestDiscoverableType::Plugin, + _ => return None, + }; + let id = table.get("id").and_then(TomlItem::as_str)?; + Some(ToolSuggestDisabledTool { + kind, + id: id.to_string(), + }) + } + + pub(super) fn tool_suggest_disabled_tools_value( + disabled_tools: &[ToolSuggestDisabledTool], + ) -> TomlItem { + let mut array = TomlArray::new(); + for disabled_tool in disabled_tools { + let mut table = InlineTable::new(); + table.insert( + "type", + match disabled_tool.kind { + ToolSuggestDiscoverableType::Connector => "connector", + ToolSuggestDiscoverableType::Plugin => "plugin", + } + .into(), + ); + table.insert("id", disabled_tool.id.clone().into()); + array.push(table); + } + TomlItem::Value(array.into()) + } + fn array_from_iter(iter: I) -> TomlItem where I: Iterator, @@ -552,6 +610,9 @@ impl ConfigDocument { value(*acknowledged), )), ConfigEdit::ReplaceMcpServers(servers) => Ok(self.replace_mcp_servers(servers)), + ConfigEdit::AddToolSuggestDisabledTool(disabled_tool) => { + Ok(self.add_tool_suggest_disabled_tool(disabled_tool)) + } ConfigEdit::SetSkillConfig { path, enabled } => { Ok(self.set_skill_config(SkillConfigSelector::Path(path.clone()), *enabled)) } @@ -590,6 +651,41 @@ impl ConfigDocument { self.remove(&resolved) } + fn add_tool_suggest_disabled_tool(&mut self, disabled_tool: &ToolSuggestDisabledTool) -> bool { + let disabled_tools_item = self + .doc + .get("tool_suggest") + .and_then(|item| item.as_table_like()) + .and_then(|table| table.get("disabled_tools")); + let existing_from_array = disabled_tools_item + .and_then(|item| item.as_value()) + .and_then(|value| value.as_array()) + .into_iter() + .flat_map(|array| array.iter()) + .filter_map(document_helpers::parse_tool_suggest_disabled_tool); + let existing_from_tables = disabled_tools_item + .and_then(|item| match item { + TomlItem::ArrayOfTables(array) => Some(array), + _ => None, + }) + .into_iter() + .flat_map(|array| array.iter()) + .filter_map(document_helpers::parse_tool_suggest_disabled_tool_table); + + let mut seen = HashSet::new(); + let disabled_tools = existing_from_array + .chain(existing_from_tables) + .chain(std::iter::once(disabled_tool.clone())) + .filter_map(|disabled_tool| disabled_tool.normalized()) + .filter(|disabled_tool| seen.insert(disabled_tool.clone())) + .collect::>(); + self.write_value( + Scope::Global, + &["tool_suggest", "disabled_tools"], + document_helpers::tool_suggest_disabled_tools_value(&disabled_tools), + ) + } + fn clear_owned(&mut self, segments: &[String]) -> bool { self.remove(segments) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 43e35e308f..4357ea8ae0 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -46,6 +46,7 @@ use codex_config::types::OtelConfig; use codex_config::types::OtelConfigToml; use codex_config::types::OtelExporterKind; use codex_config::types::ToolSuggestConfig; +use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverable; use codex_config::types::TuiKeymap; use codex_config::types::TuiNotificationSettings; @@ -96,6 +97,7 @@ use codex_utils_absolute_path::AbsolutePathBufGuard; use serde::Deserialize; use std::collections::BTreeMap; use std::collections::HashMap; +use std::collections::HashSet; use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; @@ -1440,10 +1442,29 @@ pub struct AgentRoleConfig { pub nickname_candidates: Option>, } -fn resolve_tool_suggest_config(config_toml: &ConfigToml) -> ToolSuggestConfig { - let discoverables = config_toml - .tool_suggest - .as_ref() +fn resolve_tool_suggest_config( + config_toml: &ConfigToml, + config_layer_stack: &ConfigLayerStack, +) -> ToolSuggestConfig { + resolve_tool_suggest_config_from_config(config_toml.tool_suggest.as_ref(), config_layer_stack) +} + +pub(crate) fn resolve_tool_suggest_config_from_layer_stack( + config_layer_stack: &ConfigLayerStack, +) -> ToolSuggestConfig { + let tool_suggest = config_layer_stack + .effective_config() + .get("tool_suggest") + .cloned() + .and_then(|value| value.try_into::().ok()); + resolve_tool_suggest_config_from_config(tool_suggest.as_ref(), config_layer_stack) +} + +fn resolve_tool_suggest_config_from_config( + tool_suggest: Option<&ToolSuggestConfig>, + config_layer_stack: &ConfigLayerStack, +) -> ToolSuggestConfig { + let discoverables = tool_suggest .into_iter() .flat_map(|tool_suggest| tool_suggest.discoverables.iter()) .filter_map(|discoverable| { @@ -1458,8 +1479,47 @@ fn resolve_tool_suggest_config(config_toml: &ConfigToml) -> ToolSuggestConfig { } }) .collect(); + let mut seen_disabled_tools = HashSet::new(); + let mut disabled_tools = Vec::new(); + let mut add_disabled_tool = |disabled_tool: ToolSuggestDisabledTool| { + if let Some(disabled_tool) = disabled_tool.normalized() + && seen_disabled_tools.insert(disabled_tool.clone()) + { + disabled_tools.push(disabled_tool); + } + }; - ToolSuggestConfig { discoverables } + let layers = config_layer_stack.get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ false, + ); + if layers.is_empty() { + for disabled_tool in tool_suggest + .into_iter() + .flat_map(|tool_suggest| tool_suggest.disabled_tools.iter().cloned()) + { + add_disabled_tool(disabled_tool); + } + } else { + for layer in layers { + let Some(tool_suggest) = layer + .config + .get("tool_suggest") + .cloned() + .and_then(|value| value.try_into::().ok()) + else { + continue; + }; + for disabled_tool in tool_suggest.disabled_tools { + add_disabled_tool(disabled_tool); + } + } + } + + ToolSuggestConfig { + discoverables, + disabled_tools, + } } fn thread_store_config( @@ -1864,7 +1924,7 @@ impl Config { .clone(), None => ConfigProfile::default(), }; - let tool_suggest = resolve_tool_suggest_config(&cfg); + let tool_suggest = resolve_tool_suggest_config(&cfg, &config_layer_stack); let feature_overrides = FeatureOverrides { include_apply_patch_tool: include_apply_patch_tool_override, web_search_request: override_tools_web_search_request, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index d06ea24609..c8da969837 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -419,6 +419,14 @@ async fn tool_suggest_connector_ids(config: &Config) -> HashSet { .filter(|discoverable| discoverable.kind == ToolSuggestDiscoverableType::Connector) .map(|discoverable| discoverable.id.clone()), ); + let disabled_connector_ids = config + .tool_suggest + .disabled_tools + .iter() + .filter(|disabled_tool| disabled_tool.kind == ToolSuggestDiscoverableType::Connector) + .map(|disabled_tool| disabled_tool.id.as_str()) + .collect::>(); + connector_ids.retain(|connector_id| !disabled_connector_ids.contains(connector_id.as_str())); connector_ids } diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 885b573dac..b3538d1ff0 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -1112,6 +1112,35 @@ discoverables = [ ); } +#[tokio::test] +async fn tool_suggest_connector_ids_exclude_disabled_tool_suggestions() { + let codex_home = tempdir().expect("tempdir should succeed"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +[tool_suggest] +discoverables = [ + { type = "connector", id = "connector_calendar" }, + { type = "connector", id = "connector_gmail" } +] +disabled_tools = [ + { type = "connector", id = "connector_calendar" } +] +"#, + ) + .expect("write config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should load"); + + assert_eq!( + tool_suggest_connector_ids(&config).await, + HashSet::from(["connector_gmail".to_string()]) + ); +} + #[test] fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstalled_apps() { let filtered = filter_tool_suggest_discoverable_connectors( diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 56059f8eee..c117f706e1 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -3,10 +3,9 @@ use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; use codex_network_proxy::NetworkProxy; use codex_protocol::models::PermissionProfile; -use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_sandboxing::landlock::allow_network_for_proxy; -use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; +use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_permission_profile; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; use std::path::Path; @@ -17,9 +16,8 @@ use tokio::process::Child; /// isolation plus seccomp for network restrictions. /// /// Unlike macOS Seatbelt where we directly embed the policy text, the Linux -/// helper is a separate executable. We pass both the canonical split -/// filesystem/network policies and a compatibility legacy projection as JSON -/// until the helper protocol no longer needs the legacy field. +/// helper is a separate executable. We pass the canonical permission profile +/// as JSON and let the helper derive the runtime filesystem/network policies. #[allow(clippy::too_many_arguments)] pub async fn spawn_command_under_linux_sandbox

( codex_linux_sandbox_exe: P, @@ -35,20 +33,11 @@ pub async fn spawn_command_under_linux_sandbox

( where P: AsRef, { - let (file_system_sandbox_policy, network_sandbox_policy) = - permission_profile.to_runtime_permissions(); - let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( - permission_profile, - &file_system_sandbox_policy, - network_sandbox_policy, - sandbox_policy_cwd.as_path(), - ); - let args = create_linux_sandbox_command_args_for_policies( + let network_sandbox_policy = permission_profile.network_sandbox_policy(); + let args = create_linux_sandbox_command_args_for_permission_profile( command, command_cwd.as_path(), - &sandbox_policy, - &file_system_sandbox_policy, - network_sandbox_policy, + permission_profile, sandbox_policy_cwd, use_legacy_landlock, allow_network_for_proxy(/*enforce_managed_network*/ false), diff --git a/codex-rs/core/src/plugins/discoverable.rs b/codex-rs/core/src/plugins/discoverable.rs index 8340f50493..5a1a3588fd 100644 --- a/codex-rs/core/src/plugins/discoverable.rs +++ b/codex-rs/core/src/plugins/discoverable.rs @@ -43,6 +43,13 @@ pub(crate) async fn list_tool_suggest_discoverable_plugins( .filter(|discoverable| discoverable.kind == ToolSuggestDiscoverableType::Plugin) .map(|discoverable| discoverable.id.as_str()) .collect::>(); + let disabled_plugin_ids = config + .tool_suggest + .disabled_tools + .iter() + .filter(|disabled_tool| disabled_tool.kind == ToolSuggestDiscoverableType::Plugin) + .map(|disabled_tool| disabled_tool.id.as_str()) + .collect::>(); let marketplaces = plugins_manager .list_marketplaces_for_config(config, &[]) .context("failed to list plugin marketplaces for tool suggestions")? @@ -56,6 +63,7 @@ pub(crate) async fn list_tool_suggest_discoverable_plugins( for plugin in marketplace.plugins { if plugin.installed + || disabled_plugin_ids.contains(plugin.id.as_str()) || (!TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST.contains(&plugin.id.as_str()) && !configured_plugin_ids.contains(plugin.id.as_str())) { diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index fb25b62f0c..20a03e0c84 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -230,6 +230,31 @@ async fn list_tool_suggest_discoverable_plugins_omits_installed_curated_plugins( assert_eq!(discoverable_plugins, Vec::::new()); } +#[tokio::test] +async fn list_tool_suggest_discoverable_plugins_omits_disabled_tool_suggestions() { + let codex_home = tempdir().expect("tempdir should succeed"); + let curated_root = curated_plugins_repo_path(codex_home.path()); + write_openai_curated_marketplace(&curated_root, &["slack"]); + write_file( + &codex_home.path().join(crate::config::CONFIG_TOML_FILE), + r#"[features] +plugins = true + +[tool_suggest] +disabled_tools = [ + { type = "plugin", id = "slack@openai-curated" } +] +"#, + ); + + let config = load_plugins_config(codex_home.path()).await; + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + .await + .unwrap(); + + assert_eq!(discoverable_plugins, Vec::::new()); +} + #[tokio::test] async fn list_tool_suggest_discoverable_plugins_includes_configured_plugin_ids() { let codex_home = tempdir().expect("tempdir should succeed"); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 2c6ed17aa5..7f3f6aa49d 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -18,6 +18,7 @@ use crate::build_available_skills; use crate::commit_attribution::commit_message_trailer_instruction; use crate::compact; use crate::config::ManagedFeatures; +use crate::config::resolve_tool_suggest_config_from_layer_stack; use crate::connectors; use crate::context::ApprovedCommandPrefixSaved; use crate::context::AppsInstructions; @@ -1409,6 +1410,8 @@ impl Session { config.config_layer_stack = config .config_layer_stack .with_user_config(&config_toml_path, user_config); + config.tool_suggest = + resolve_tool_suggest_config_from_layer_stack(&config.config_layer_stack); state.session_configuration.original_config_do_not_use = Arc::new(config); self.services.skills_manager.clear_cache(); self.services.plugins_manager.clear_cache(); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index a344021b75..fab67a8f36 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -19,6 +19,7 @@ use codex_config::NetworkDomainPermissionsToml; use codex_config::RequirementSource; use codex_config::Sourced; use codex_config::loader::project_trust_key; +use codex_config::types::ToolSuggestDisabledTool; use codex_features::Feature; use codex_features::Features; @@ -1155,6 +1156,35 @@ async fn reload_user_config_layer_updates_effective_apps_config() { assert_eq!(app.destructive_enabled, Some(false)); } +#[tokio::test] +async fn reload_user_config_layer_updates_effective_tool_suggest_config() { + let (session, _turn_context) = make_session_and_context().await; + let codex_home = session.codex_home().await; + std::fs::create_dir_all(&codex_home).expect("create codex home"); + let config_toml_path = codex_home.join(CONFIG_TOML_FILE); + std::fs::write( + &config_toml_path, + r#"[tool_suggest] +disabled_tools = [ + { type = "connector", id = " calendar " }, + { type = "plugin", id = "slack@openai-curated" }, +] +"#, + ) + .expect("write user config"); + + session.reload_user_config_layer().await; + + let config = session.get_config().await; + assert_eq!( + config.tool_suggest.disabled_tools, + vec![ + ToolSuggestDisabledTool::connector("calendar"), + ToolSuggestDisabledTool::plugin("slack@openai-curated"), + ] + ); +} + #[test] fn filter_connectors_for_input_skips_duplicate_slug_mentions() { let connectors = vec![ diff --git a/codex-rs/core/src/tools/handlers/tool_suggest.rs b/codex-rs/core/src/tools/handlers/tool_suggest.rs index 5c51c90d66..e0fa9156f4 100644 --- a/codex-rs/core/src/tools/handlers/tool_suggest.rs +++ b/codex-rs/core/src/tools/handlers/tool_suggest.rs @@ -1,11 +1,15 @@ use std::collections::HashSet; use codex_app_server_protocol::AppInfo; +use codex_config::types::ToolSuggestDisabledTool; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_rmcp_client::ElicitationAction; +use codex_rmcp_client::ElicitationResponse; use codex_tools::DiscoverableTool; use codex_tools::DiscoverableToolAction; use codex_tools::DiscoverableToolType; +use codex_tools::TOOL_SUGGEST_PERSIST_ALWAYS_VALUE; +use codex_tools::TOOL_SUGGEST_PERSIST_KEY; use codex_tools::TOOL_SUGGEST_TOOL_NAME; use codex_tools::ToolSuggestArgs; use codex_tools::ToolSuggestResult; @@ -14,8 +18,11 @@ use codex_tools::build_tool_suggestion_elicitation_request; use codex_tools::filter_tool_suggest_discoverable_tools_for_client; use codex_tools::verified_connector_suggestion_completed; use rmcp::model::RequestId; +use serde_json::Value; use tracing::warn; +use crate::config::edit::ConfigEdit; +use crate::config::edit::ConfigEditsBuilder; use crate::connectors; use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; @@ -123,6 +130,9 @@ impl ToolHandler for ToolSuggestHandler { let response = session .request_mcp_server_elicitation(turn.as_ref(), request_id, params) .await; + if let Some(response) = response.as_ref() { + maybe_persist_tool_suggest_disable(&session, &turn, &tool, response).await; + } let user_confirmed = response .as_ref() .is_some_and(|response| response.action == ElicitationAction::Accept); @@ -158,6 +168,63 @@ impl ToolHandler for ToolSuggestHandler { } } +async fn maybe_persist_tool_suggest_disable( + session: &crate::session::session::Session, + turn: &crate::session::turn_context::TurnContext, + tool: &DiscoverableTool, + response: &ElicitationResponse, +) { + if !tool_suggest_response_requests_persistent_disable(response) { + return; + } + + if let Err(err) = persist_tool_suggest_disable(&turn.config.codex_home, tool).await { + warn!( + error = %err, + tool_id = tool.id(), + "failed to persist disabled tool suggestion" + ); + return; + } + + session.reload_user_config_layer().await; +} + +fn tool_suggest_response_requests_persistent_disable(response: &ElicitationResponse) -> bool { + if response.action != ElicitationAction::Decline { + return false; + } + + response + .meta + .as_ref() + .and_then(Value::as_object) + .and_then(|meta| meta.get(TOOL_SUGGEST_PERSIST_KEY)) + .and_then(Value::as_str) + == Some(TOOL_SUGGEST_PERSIST_ALWAYS_VALUE) +} + +async fn persist_tool_suggest_disable( + codex_home: &codex_utils_absolute_path::AbsolutePathBuf, + tool: &DiscoverableTool, +) -> anyhow::Result<()> { + ConfigEditsBuilder::new(codex_home) + .with_edits([ConfigEdit::AddToolSuggestDisabledTool( + disabled_tool_suggestion(tool), + )]) + .apply() + .await +} + +fn disabled_tool_suggestion(tool: &DiscoverableTool) -> ToolSuggestDisabledTool { + match tool { + DiscoverableTool::Connector(connector) => { + ToolSuggestDisabledTool::connector(connector.id.as_str()) + } + DiscoverableTool::Plugin(plugin) => ToolSuggestDisabledTool::plugin(plugin.id.as_str()), + } +} + async fn verify_tool_suggestion_completed( session: &crate::session::session::Session, turn: &crate::session::turn_context::TurnContext, diff --git a/codex-rs/core/src/tools/handlers/tool_suggest_tests.rs b/codex-rs/core/src/tools/handlers/tool_suggest_tests.rs index 5290d113f5..2d482874ca 100644 --- a/codex-rs/core/src/tools/handlers/tool_suggest_tests.rs +++ b/codex-rs/core/src/tools/handlers/tool_suggest_tests.rs @@ -5,8 +5,20 @@ use crate::plugins::test_support::load_plugins_config; use crate::plugins::test_support::write_curated_plugin_sha; use crate::plugins::test_support::write_openai_curated_marketplace; use crate::plugins::test_support::write_plugins_feature_config; +use codex_config::CONFIG_TOML_FILE; +use codex_config::config_toml::ConfigToml; +use codex_config::types::ToolSuggestConfig; +use codex_config::types::ToolSuggestDisabledTool; +use codex_config::types::ToolSuggestDiscoverable; +use codex_config::types::ToolSuggestDiscoverableType; use codex_core_plugins::startup_sync::curated_plugins_repo_path; +use codex_rmcp_client::ElicitationResponse; +use codex_tools::DiscoverablePluginInfo; use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::PathExt; +use pretty_assertions::assert_eq; +use rmcp::model::ElicitationAction; +use serde_json::json; use tempfile::tempdir; #[tokio::test] @@ -44,3 +56,155 @@ async fn verified_plugin_suggestion_completed_requires_installed_plugin() { &plugins_manager, )); } + +#[test] +fn tool_suggest_response_persists_only_decline_always_mode() { + assert!(tool_suggest_response_requests_persistent_disable( + &ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ TOOL_SUGGEST_PERSIST_KEY: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE })), + } + )); + assert!(!tool_suggest_response_requests_persistent_disable( + &ElicitationResponse { + action: ElicitationAction::Accept, + content: None, + meta: Some(json!({ TOOL_SUGGEST_PERSIST_KEY: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE })), + } + )); + assert!(!tool_suggest_response_requests_persistent_disable( + &ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ TOOL_SUGGEST_PERSIST_KEY: "session" })), + } + )); + assert!(!tool_suggest_response_requests_persistent_disable( + &ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: None, + } + )); +} + +#[tokio::test] +async fn persist_tool_suggest_disable_writes_connector_config() { + let codex_home = tempdir().expect("tempdir should succeed"); + let tool = connector_tool("connector_calendar", "Google Calendar"); + + persist_tool_suggest_disable(&codex_home.path().abs(), &tool) + .await + .expect("persist connector disable"); + + let contents = + std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).expect("read config"); + let parsed: ConfigToml = toml::from_str(&contents).expect("parse config"); + assert_eq!( + parsed.tool_suggest, + Some(ToolSuggestConfig { + discoverables: Vec::new(), + disabled_tools: vec![ToolSuggestDisabledTool::connector("connector_calendar")], + }) + ); +} + +#[tokio::test] +async fn persist_tool_suggest_disable_writes_plugin_config() { + let codex_home = tempdir().expect("tempdir should succeed"); + let tool = DiscoverableTool::Plugin(Box::new(DiscoverablePluginInfo { + id: "slack@openai-curated".to_string(), + name: "Slack".to_string(), + description: None, + has_skills: true, + mcp_server_names: Vec::new(), + app_connector_ids: Vec::new(), + })); + + persist_tool_suggest_disable(&codex_home.path().abs(), &tool) + .await + .expect("persist plugin disable"); + + let contents = + std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).expect("read config"); + let parsed: ConfigToml = toml::from_str(&contents).expect("parse config"); + assert_eq!( + parsed.tool_suggest, + Some(ToolSuggestConfig { + discoverables: Vec::new(), + disabled_tools: vec![ToolSuggestDisabledTool::plugin("slack@openai-curated")], + }) + ); +} + +#[tokio::test] +async fn persist_tool_suggest_disable_dedupes_existing_disabled_tools() { + let codex_home = tempdir().expect("tempdir should succeed"); + let tool = connector_tool("connector_calendar", "Google Calendar"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +[tool_suggest] +discoverables = [ + { type = "plugin", id = "sample@openai-curated" } +] + +[[tool_suggest.disabled_tools]] +type = "connector" +id = " connector_calendar " + +[[tool_suggest.disabled_tools]] +type = "connector" +id = "connector_calendar" + +[[tool_suggest.disabled_tools]] +type = "connector" +id = " " + +[[tool_suggest.disabled_tools]] +type = "plugin" +id = "slack@openai-curated" +"#, + ) + .expect("write config"); + + persist_tool_suggest_disable(&codex_home.path().abs(), &tool) + .await + .expect("persist connector disable"); + + let contents = + std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).expect("read config"); + let parsed: ConfigToml = toml::from_str(&contents).expect("parse config"); + assert_eq!( + parsed.tool_suggest, + Some(ToolSuggestConfig { + discoverables: vec![ToolSuggestDiscoverable { + kind: ToolSuggestDiscoverableType::Plugin, + id: "sample@openai-curated".to_string(), + }], + disabled_tools: vec![ + ToolSuggestDisabledTool::connector("connector_calendar"), + ToolSuggestDisabledTool::plugin("slack@openai-curated"), + ], + }) + ); +} + +fn connector_tool(id: &str, name: &str) -> DiscoverableTool { + DiscoverableTool::Connector(Box::new(AppInfo { + id: id.to_string(), + name: name.to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: false, + is_enabled: true, + plugin_display_names: Vec::new(), + })) +} diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index e191a924d3..ff40f3663d 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -1036,7 +1036,6 @@ mod tests { use codex_protocol::protocol::FileSystemSandboxEntry; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::FileSystemSpecialPath; - use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -1066,7 +1065,7 @@ mod tests { let command = vec!["/bin/true".to_string()]; let args = create_bwrap_command_args( command.clone(), - &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + &FileSystemSandboxPolicy::unrestricted(), Path::new("/"), Path::new("/"), BwrapOptions { @@ -1085,7 +1084,7 @@ mod tests { let command = vec!["/bin/true".to_string()]; let args = create_bwrap_command_args( command, - &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + &FileSystemSandboxPolicy::unrestricted(), Path::new("/"), Path::new("/"), BwrapOptions { @@ -1399,22 +1398,18 @@ mod tests { let missing_root = temp_dir.path().join("missing"); std::fs::create_dir(&existing_root).expect("create existing root"); - let policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![ + let policy = FileSystemSandboxPolicy::workspace_write( + &[ AbsolutePathBuf::try_from(existing_root.as_path()).expect("absolute existing root"), AbsolutePathBuf::try_from(missing_root.as_path()).expect("absolute missing root"), ], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); - let args = create_filesystem_args( - &FileSystemSandboxPolicy::from(&policy), - temp_dir.path(), - NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, - ) - .expect("filesystem args"); + let args = + create_filesystem_args(&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); @@ -1532,15 +1527,14 @@ mod tests { #[test] fn mounts_dev_before_writable_dev_binds() { - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![AbsolutePathBuf::try_from(Path::new("/dev")).expect("/dev path")], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; + let sandbox_policy = FileSystemSandboxPolicy::workspace_write( + &[AbsolutePathBuf::try_from(Path::new("/dev")).expect("/dev path")], + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); let args = create_filesystem_args( - &FileSystemSandboxPolicy::from(&sandbox_policy), + &sandbox_policy, Path::new("/"), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, ) diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index 50ea87bd1d..3f95224ab6 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -8,8 +8,8 @@ use std::path::Path; use codex_protocol::error::CodexErr; use codex_protocol::error::Result; use codex_protocol::error::SandboxErr; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::NetworkSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use landlock::ABI; @@ -39,14 +39,15 @@ use seccompiler::apply_filter; /// - installing the network seccomp filter when network access is disabled. /// /// Filesystem restrictions are intentionally handled by bubblewrap. -pub(crate) fn apply_sandbox_policy_to_current_thread( - sandbox_policy: &SandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, +pub(crate) fn apply_permission_profile_to_current_thread( + permission_profile: &PermissionProfile, cwd: &Path, apply_landlock_fs: bool, allow_network_for_proxy: bool, proxy_routed_network: bool, ) -> Result<()> { + let (file_system_sandbox_policy, network_sandbox_policy) = + permission_profile.to_runtime_permissions(); let network_seccomp_mode = network_seccomp_mode( network_sandbox_policy, allow_network_for_proxy, @@ -58,7 +59,7 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( // we avoid this unless we need seccomp or we are explicitly using the // legacy Landlock filesystem pipeline. if network_seccomp_mode.is_some() - || (apply_landlock_fs && !sandbox_policy.has_full_disk_write_access()) + || (apply_landlock_fs && !file_system_sandbox_policy.has_full_disk_write_access()) { set_no_new_privs()?; } @@ -67,15 +68,15 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( install_network_seccomp_filter_on_current_thread(mode)?; } - if apply_landlock_fs && !sandbox_policy.has_full_disk_write_access() { - if !sandbox_policy.has_full_disk_read_access() { + if apply_landlock_fs && !file_system_sandbox_policy.has_full_disk_write_access() { + if !file_system_sandbox_policy.has_full_disk_read_access() { return Err(CodexErr::UnsupportedOperation( "Restricted read-only access is not supported by the legacy Linux Landlock filesystem backend." .to_string(), )); } - let writable_roots = sandbox_policy + let writable_roots = file_system_sandbox_policy .get_writable_roots_with_cwd(cwd) .into_iter() .map(|writable_root| writable_root.root) diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 0eede8bb81..7c27b2de9f 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -10,14 +10,14 @@ use std::path::PathBuf; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; use crate::bwrap::create_bwrap_command_args; -use crate::landlock::apply_sandbox_policy_to_current_thread; +use crate::landlock::apply_permission_profile_to_current_thread; use crate::launcher::exec_bwrap; use crate::launcher::preferred_bwrap_supports_argv0; use crate::proxy_routing::activate_proxy_routes_in_netns; use crate::proxy_routing::prepare_host_proxy_route_spec; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; #[derive(Debug, Parser)] @@ -41,18 +41,13 @@ pub struct LandlockCommand { #[arg(long = "command-cwd", hide = true)] pub command_cwd: Option, - /// Legacy compatibility policy. - /// - /// Newer callers pass split filesystem/network policies as well so the - /// helper can migrate incrementally without breaking older invocations. - #[arg(long = "sandbox-policy", hide = true)] - pub sandbox_policy: Option, - - #[arg(long = "file-system-sandbox-policy", hide = true)] - pub file_system_sandbox_policy: Option, - - #[arg(long = "network-sandbox-policy", hide = true)] - pub network_sandbox_policy: Option, + /// Canonical runtime permissions for the command. + #[arg( + long = "permission-profile", + hide = true, + value_parser = parse_permission_profile + )] + pub permission_profile: Option, /// Opt-in: use the legacy Landlock Linux sandbox fallback. /// @@ -102,9 +97,7 @@ pub fn run_main() -> ! { let LandlockCommand { sandbox_policy_cwd, command_cwd, - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, + permission_profile, use_legacy_landlock, apply_seccomp_then_exec, allow_network_for_proxy, @@ -117,17 +110,11 @@ pub fn run_main() -> ! { panic!("No command specified to execute."); } ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_legacy_landlock); - let EffectiveSandboxPolicies { - sandbox_policy, + let EffectivePermissions { + permission_profile, file_system_sandbox_policy, network_sandbox_policy, - } = resolve_sandbox_policies( - sandbox_policy_cwd.as_path(), - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, - ) - .unwrap_or_else(|err| panic!("{err}")); + } = resolve_permission_profile(permission_profile).unwrap_or_else(|err| panic!("{err}")); ensure_legacy_landlock_mode_supports_policy( use_legacy_landlock, &file_system_sandbox_policy, @@ -147,9 +134,8 @@ pub fn run_main() -> ! { } } let proxy_routing_active = allow_network_for_proxy; - if let Err(e) = apply_sandbox_policy_to_current_thread( - &sandbox_policy, - network_sandbox_policy, + if let Err(e) = apply_permission_profile_to_current_thread( + &permission_profile, &sandbox_policy_cwd, /*apply_landlock_fs*/ false, allow_network_for_proxy, @@ -161,9 +147,8 @@ pub fn run_main() -> ! { } if file_system_sandbox_policy.has_full_disk_write_access() && !allow_network_for_proxy { - if let Err(e) = apply_sandbox_policy_to_current_thread( - &sandbox_policy, - network_sandbox_policy, + if let Err(e) = apply_permission_profile_to_current_thread( + &permission_profile, &sandbox_policy_cwd, /*apply_landlock_fs*/ false, allow_network_for_proxy, @@ -189,9 +174,7 @@ pub fn run_main() -> ! { let inner = build_inner_seccomp_command(InnerSeccompCommandArgs { sandbox_policy_cwd: &sandbox_policy_cwd, command_cwd: command_cwd.as_deref(), - sandbox_policy: &sandbox_policy, - file_system_sandbox_policy: &file_system_sandbox_policy, - network_sandbox_policy, + permission_profile: &permission_profile, allow_network_for_proxy, proxy_route_spec, command, @@ -208,9 +191,8 @@ pub fn run_main() -> ! { } // Legacy path: Landlock enforcement only, when bwrap sandboxing is not enabled. - if let Err(e) = apply_sandbox_policy_to_current_thread( - &sandbox_policy, - network_sandbox_policy, + if let Err(e) = apply_permission_profile_to_current_thread( + &permission_profile, &sandbox_policy_cwd, /*apply_landlock_fs*/ true, allow_network_for_proxy, @@ -222,164 +204,41 @@ pub fn run_main() -> ! { } #[derive(Debug, Clone)] -struct EffectiveSandboxPolicies { - sandbox_policy: SandboxPolicy, +struct EffectivePermissions { + permission_profile: PermissionProfile, file_system_sandbox_policy: FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, } #[derive(Debug, PartialEq, Eq)] -enum ResolveSandboxPoliciesError { - PartialSplitPolicies, - SplitPoliciesRequireDirectRuntimeEnforcement(String), - FailedToDeriveLegacyPolicy(String), - MismatchedLegacyPolicy { - provided: SandboxPolicy, - derived: SandboxPolicy, - }, +enum ResolvePermissionProfileError { MissingConfiguration, } -impl fmt::Display for ResolveSandboxPoliciesError { +impl fmt::Display for ResolvePermissionProfileError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::PartialSplitPolicies => { - write!( - f, - "file-system and network sandbox policies must be provided together" - ) - } - Self::SplitPoliciesRequireDirectRuntimeEnforcement(err) => { - write!( - f, - "split sandbox policies require direct runtime enforcement and cannot be paired with legacy sandbox policy: {err}" - ) - } - Self::FailedToDeriveLegacyPolicy(err) => { - write!( - f, - "failed to derive legacy sandbox policy from split policies: {err}" - ) - } - Self::MismatchedLegacyPolicy { provided, derived } => { - write!( - f, - "legacy sandbox policy must match split sandbox policies: provided={provided:?}, derived={derived:?}" - ) - } - Self::MissingConfiguration => write!(f, "missing sandbox policy configuration"), + Self::MissingConfiguration => write!(f, "missing permission profile configuration"), } } } -fn resolve_sandbox_policies( - sandbox_policy_cwd: &Path, - sandbox_policy: Option, - file_system_sandbox_policy: Option, - network_sandbox_policy: Option, -) -> Result { - // Accept either a fully legacy policy, a fully split policy pair, or all - // three views together. Reject partial split-policy input so the helper - // never runs with mismatched filesystem/network state. - let split_policies = match (file_system_sandbox_policy, network_sandbox_policy) { - (Some(file_system_sandbox_policy), Some(network_sandbox_policy)) => { - Some((file_system_sandbox_policy, network_sandbox_policy)) - } - (None, None) => None, - _ => return Err(ResolveSandboxPoliciesError::PartialSplitPolicies), - }; - - match (sandbox_policy, split_policies) { - (Some(sandbox_policy), Some((file_system_sandbox_policy, network_sandbox_policy))) => { - if file_system_sandbox_policy - .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd) - { - return Ok(EffectiveSandboxPolicies { - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, - }); - } - let derived_legacy_policy = file_system_sandbox_policy - .to_legacy_sandbox_policy(network_sandbox_policy, sandbox_policy_cwd) - .map_err(|err| { - ResolveSandboxPoliciesError::SplitPoliciesRequireDirectRuntimeEnforcement( - err.to_string(), - ) - })?; - if !legacy_sandbox_policies_match_semantics( - &sandbox_policy, - &derived_legacy_policy, - sandbox_policy_cwd, - ) { - return Err(ResolveSandboxPoliciesError::MismatchedLegacyPolicy { - provided: sandbox_policy, - derived: derived_legacy_policy, - }); - } - Ok(EffectiveSandboxPolicies { - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, - }) - } - (Some(sandbox_policy), None) => Ok(EffectiveSandboxPolicies { - file_system_sandbox_policy: FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - &sandbox_policy, - sandbox_policy_cwd, - ), - network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), - sandbox_policy, - }), - (None, Some((file_system_sandbox_policy, network_sandbox_policy))) => { - let sandbox_policy = file_system_sandbox_policy - .to_legacy_sandbox_policy(network_sandbox_policy, sandbox_policy_cwd) - .map_err(|err| { - ResolveSandboxPoliciesError::FailedToDeriveLegacyPolicy(err.to_string()) - })?; - Ok(EffectiveSandboxPolicies { - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, - }) - } - (None, None) => Err(ResolveSandboxPoliciesError::MissingConfiguration), - } +fn parse_permission_profile(value: &str) -> std::result::Result { + serde_json::from_str(value).map_err(|err| format!("invalid permission profile JSON: {err}")) } -fn legacy_sandbox_policies_match_semantics( - provided: &SandboxPolicy, - derived: &SandboxPolicy, - sandbox_policy_cwd: &Path, -) -> bool { - NetworkSandboxPolicy::from(provided) == NetworkSandboxPolicy::from(derived) - && file_system_sandbox_policies_match_semantics( - &FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - provided, - sandbox_policy_cwd, - ), - &FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - derived, - sandbox_policy_cwd, - ), - sandbox_policy_cwd, - ) -} - -fn file_system_sandbox_policies_match_semantics( - provided: &FileSystemSandboxPolicy, - derived: &FileSystemSandboxPolicy, - sandbox_policy_cwd: &Path, -) -> bool { - provided.has_full_disk_read_access() == derived.has_full_disk_read_access() - && provided.has_full_disk_write_access() == derived.has_full_disk_write_access() - && provided.include_platform_defaults() == derived.include_platform_defaults() - && provided.get_readable_roots_with_cwd(sandbox_policy_cwd) - == derived.get_readable_roots_with_cwd(sandbox_policy_cwd) - && provided.get_writable_roots_with_cwd(sandbox_policy_cwd) - == derived.get_writable_roots_with_cwd(sandbox_policy_cwd) - && provided.get_unreadable_roots_with_cwd(sandbox_policy_cwd) - == derived.get_unreadable_roots_with_cwd(sandbox_policy_cwd) +fn resolve_permission_profile( + permission_profile: Option, +) -> Result { + let permission_profile = + permission_profile.ok_or(ResolvePermissionProfileError::MissingConfiguration)?; + let (file_system_sandbox_policy, network_sandbox_policy) = + permission_profile.to_runtime_permissions(); + Ok(EffectivePermissions { + permission_profile, + file_system_sandbox_policy, + network_sandbox_policy, + }) } fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_legacy_landlock: bool) { @@ -399,7 +258,7 @@ fn ensure_legacy_landlock_mode_supports_policy( .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd) { panic!( - "split sandbox policies requiring direct runtime enforcement are incompatible with --use-legacy-landlock" + "permission profiles requiring direct runtime enforcement are incompatible with --use-legacy-landlock" ); } } @@ -656,9 +515,7 @@ fn is_proc_mount_failure(stderr: &str) -> bool { struct InnerSeccompCommandArgs<'a> { sandbox_policy_cwd: &'a Path, command_cwd: Option<&'a Path>, - sandbox_policy: &'a SandboxPolicy, - file_system_sandbox_policy: &'a FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, + permission_profile: &'a PermissionProfile, allow_network_for_proxy: bool, proxy_route_spec: Option, command: Vec, @@ -669,9 +526,7 @@ fn build_inner_seccomp_command(args: InnerSeccompCommandArgs<'_>) -> Vec let InnerSeccompCommandArgs { sandbox_policy_cwd, command_cwd, - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, + permission_profile, allow_network_for_proxy, proxy_route_spec, command, @@ -680,17 +535,9 @@ fn build_inner_seccomp_command(args: InnerSeccompCommandArgs<'_>) -> Vec Ok(path) => path, Err(err) => panic!("failed to resolve current executable path: {err}"), }; - let policy_json = match serde_json::to_string(sandbox_policy) { + let permission_profile_json = match serde_json::to_string(permission_profile) { Ok(json) => json, - Err(err) => panic!("failed to serialize sandbox policy: {err}"), - }; - let file_system_policy_json = match serde_json::to_string(file_system_sandbox_policy) { - Ok(json) => json, - Err(err) => panic!("failed to serialize filesystem sandbox policy: {err}"), - }; - let network_policy_json = match serde_json::to_string(&network_sandbox_policy) { - Ok(json) => json, - Err(err) => panic!("failed to serialize network sandbox policy: {err}"), + Err(err) => panic!("failed to serialize permission profile: {err}"), }; let mut inner = vec![ @@ -703,12 +550,8 @@ fn build_inner_seccomp_command(args: InnerSeccompCommandArgs<'_>) -> Vec inner.push(command_cwd.to_string_lossy().to_string()); } inner.extend([ - "--sandbox-policy".to_string(), - policy_json, - "--file-system-sandbox-policy".to_string(), - file_system_policy_json, - "--network-sandbox-policy".to_string(), - network_policy_json, + "--permission-profile".to_string(), + permission_profile_json, "--apply-seccomp-then-exec".to_string(), ]); if allow_network_for_proxy { 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 89dd127512..b6062e0935 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -1,16 +1,24 @@ #[cfg(test)] use super::*; #[cfg(test)] +use codex_protocol::models::PermissionProfile; +#[cfg(test)] use codex_protocol::protocol::FileSystemSandboxPolicy; #[cfg(test)] use codex_protocol::protocol::NetworkSandboxPolicy; #[cfg(test)] -use codex_protocol::protocol::SandboxPolicy; -#[cfg(test)] use codex_utils_absolute_path::AbsolutePathBuf; #[cfg(test)] use pretty_assertions::assert_eq; +fn read_only_permission_profile() -> PermissionProfile { + PermissionProfile::read_only() +} + +fn read_only_file_system_policy() -> FileSystemSandboxPolicy { + read_only_permission_profile().file_system_sandbox_policy() +} + #[test] fn detects_proc_mount_invalid_argument_failure() { let stderr = "bwrap: Can't mount proc on /newroot/proc: Invalid argument"; @@ -37,10 +45,10 @@ fn ignores_non_proc_mount_errors() { #[test] fn inserts_bwrap_argv0_before_command_separator() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let file_system_sandbox_policy = read_only_file_system_policy(); let mut argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &FileSystemSandboxPolicy::from(&sandbox_policy), + &file_system_sandbox_policy, Path::new("/"), Path::new("/"), BwrapOptions { @@ -80,10 +88,10 @@ fn inserts_bwrap_argv0_before_command_separator() { #[test] fn rewrites_inner_command_path_when_bwrap_lacks_argv0() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let file_system_sandbox_policy = read_only_file_system_policy(); let mut argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &FileSystemSandboxPolicy::from(&sandbox_policy), + &file_system_sandbox_policy, Path::new("/"), Path::new("/"), BwrapOptions { @@ -148,10 +156,10 @@ fn rewrites_bwrap_helper_command_not_nested_user_command_when_current_exe_appear #[test] fn inserts_unshare_net_when_network_isolation_requested() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let file_system_sandbox_policy = read_only_file_system_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &FileSystemSandboxPolicy::from(&sandbox_policy), + &file_system_sandbox_policy, Path::new("/"), Path::new("/"), BwrapOptions { @@ -166,10 +174,10 @@ fn inserts_unshare_net_when_network_isolation_requested() { #[test] fn inserts_unshare_net_when_proxy_only_network_mode_requested() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let file_system_sandbox_policy = read_only_file_system_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &FileSystemSandboxPolicy::from(&sandbox_policy), + &file_system_sandbox_policy, Path::new("/"), Path::new("/"), BwrapOptions { @@ -250,7 +258,7 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { let argv = build_preflight_bwrap_argv( Path::new("/"), Path::new("/"), - &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + &FileSystemSandboxPolicy::unrestricted(), mode, ) .args; @@ -259,13 +267,11 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { #[test] fn managed_proxy_inner_command_includes_route_spec() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = read_only_permission_profile(); let args = build_inner_seccomp_command(InnerSeccompCommandArgs { sandbox_policy_cwd: Path::new("/tmp"), command_cwd: Some(Path::new("/tmp/link")), - sandbox_policy: &sandbox_policy, - file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), - network_sandbox_policy: NetworkSandboxPolicy::Restricted, + permission_profile: &permission_profile, allow_network_for_proxy: true, proxy_route_spec: Some("{\"routes\":[]}".to_string()), command: vec!["/bin/true".to_string()], @@ -276,21 +282,18 @@ fn managed_proxy_inner_command_includes_route_spec() { } #[test] -fn inner_command_includes_split_policy_flags() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); +fn inner_command_includes_permission_profile_flag() { + let permission_profile = read_only_permission_profile(); let args = build_inner_seccomp_command(InnerSeccompCommandArgs { sandbox_policy_cwd: Path::new("/tmp"), command_cwd: Some(Path::new("/tmp/link")), - sandbox_policy: &sandbox_policy, - file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), - network_sandbox_policy: NetworkSandboxPolicy::Restricted, + permission_profile: &permission_profile, allow_network_for_proxy: false, proxy_route_spec: None, command: vec!["/bin/true".to_string()], }); - assert!(args.iter().any(|arg| arg == "--file-system-sandbox-policy")); - assert!(args.iter().any(|arg| arg == "--network-sandbox-policy")); + assert!(args.iter().any(|arg| arg == "--permission-profile")); assert!( args.windows(2) .any(|window| { window == ["--command-cwd", "/tmp/link"] }) @@ -299,13 +302,11 @@ fn inner_command_includes_split_policy_flags() { #[test] fn non_managed_inner_command_omits_route_spec() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = read_only_permission_profile(); let args = build_inner_seccomp_command(InnerSeccompCommandArgs { sandbox_policy_cwd: Path::new("/tmp"), command_cwd: Some(Path::new("/tmp/link")), - sandbox_policy: &sandbox_policy, - file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), - network_sandbox_policy: NetworkSandboxPolicy::Restricted, + permission_profile: &permission_profile, allow_network_for_proxy: false, proxy_route_spec: None, command: vec!["/bin/true".to_string()], @@ -317,13 +318,11 @@ fn non_managed_inner_command_omits_route_spec() { #[test] fn managed_proxy_inner_command_requires_route_spec() { let result = std::panic::catch_unwind(|| { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let permission_profile = read_only_permission_profile(); build_inner_seccomp_command(InnerSeccompCommandArgs { sandbox_policy_cwd: Path::new("/tmp"), command_cwd: Some(Path::new("/tmp/link")), - sandbox_policy: &sandbox_policy, - file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), - network_sandbox_policy: NetworkSandboxPolicy::Restricted, + permission_profile: &permission_profile, allow_network_for_proxy: true, proxy_route_spec: None, command: vec!["/bin/true".to_string()], @@ -333,89 +332,28 @@ fn managed_proxy_inner_command_requires_route_spec() { } #[test] -fn resolve_sandbox_policies_derives_split_policies_from_legacy_policy() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); +fn resolve_permission_profile_derives_runtime_policies() { + let permission_profile = read_only_permission_profile(); + let resolved = resolve_permission_profile(Some(permission_profile.clone())) + .expect("profile should resolve"); - let resolved = resolve_sandbox_policies( - Path::new("/tmp"), - Some(sandbox_policy.clone()), - /*file_system_sandbox_policy*/ None, - /*network_sandbox_policy*/ None, - ) - .expect("legacy policy should resolve"); - - assert_eq!(resolved.sandbox_policy, sandbox_policy); + assert_eq!(resolved.permission_profile, permission_profile); assert_eq!( resolved.file_system_sandbox_policy, - FileSystemSandboxPolicy::from(&sandbox_policy) + read_only_file_system_policy() ); assert_eq!( resolved.network_sandbox_policy, - NetworkSandboxPolicy::from(&sandbox_policy) + NetworkSandboxPolicy::Restricted ); } #[test] -fn resolve_sandbox_policies_derives_legacy_policy_from_split_policies() { - let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - - let resolved = resolve_sandbox_policies( - Path::new("/tmp"), - /*sandbox_policy*/ None, - Some(file_system_sandbox_policy.clone()), - Some(network_sandbox_policy), - ) - .expect("split policies should resolve"); - - assert_eq!(resolved.sandbox_policy, sandbox_policy); - assert_eq!( - resolved.file_system_sandbox_policy, - file_system_sandbox_policy - ); - assert_eq!(resolved.network_sandbox_policy, network_sandbox_policy); -} - -#[test] -fn resolve_sandbox_policies_rejects_partial_split_policies() { - let err = resolve_sandbox_policies( - Path::new("/tmp"), - Some(SandboxPolicy::new_read_only_policy()), - Some(FileSystemSandboxPolicy::default()), - /*network_sandbox_policy*/ None, - ) - .expect_err("partial split policies should fail"); - - assert_eq!(err, ResolveSandboxPoliciesError::PartialSplitPolicies); -} - -#[test] -fn resolve_sandbox_policies_rejects_mismatched_legacy_and_split_inputs() { - let err = resolve_sandbox_policies( - Path::new("/tmp"), - Some(SandboxPolicy::new_read_only_policy()), - Some(FileSystemSandboxPolicy::unrestricted()), - Some(NetworkSandboxPolicy::Enabled), - ) - .expect_err("mismatched legacy and split policies should fail"); - - assert!( - matches!( - err, - ResolveSandboxPoliciesError::MismatchedLegacyPolicy { .. } - ), - "{err}" - ); -} - -#[test] -fn resolve_sandbox_policies_accepts_split_policies_requiring_direct_runtime_enforcement() { +fn resolve_permission_profile_preserves_direct_runtime_profile() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); let docs = temp_dir.path().join("docs"); std::fs::create_dir_all(&docs).expect("create docs"); let docs = AbsolutePathBuf::from_absolute_path(&docs).expect("absolute docs"); - let sandbox_policy = SandboxPolicy::new_read_only_policy(); let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ codex_protocol::permissions::FileSystemSandboxEntry { path: codex_protocol::permissions::FileSystemPath::Special { @@ -428,16 +366,14 @@ fn resolve_sandbox_policies_accepts_split_policies_requiring_direct_runtime_enfo access: codex_protocol::permissions::FileSystemAccessMode::Write, }, ]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); + let resolved = resolve_permission_profile(Some(permission_profile.clone())) + .expect("profile should resolve"); - let resolved = resolve_sandbox_policies( - temp_dir.path(), - Some(sandbox_policy.clone()), - Some(file_system_sandbox_policy.clone()), - Some(NetworkSandboxPolicy::Restricted), - ) - .expect("split-only policy should preserve provided legacy fallback"); - - assert_eq!(resolved.sandbox_policy, sandbox_policy); + assert_eq!(resolved.permission_profile, permission_profile); assert_eq!( resolved.file_system_sandbox_policy, file_system_sandbox_policy @@ -449,37 +385,10 @@ fn resolve_sandbox_policies_accepts_split_policies_requiring_direct_runtime_enfo } #[test] -fn resolve_sandbox_policies_accepts_semantically_equivalent_workspace_write_inputs() { - let temp_dir = tempfile::TempDir::new().expect("tempdir"); - let workspace = temp_dir.path().join("workspace"); - std::fs::create_dir_all(&workspace).expect("create workspace"); - let workspace = AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace"); - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![workspace], - network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - }; - let file_system_sandbox_policy = - FileSystemSandboxPolicy::from(&SandboxPolicy::new_workspace_write_policy()); +fn resolve_permission_profile_rejects_missing_configuration() { + let err = resolve_permission_profile(None).expect_err("missing profile should fail"); - let resolved = resolve_sandbox_policies( - temp_dir.path().join("workspace").as_path(), - Some(sandbox_policy.clone()), - Some(file_system_sandbox_policy.clone()), - Some(NetworkSandboxPolicy::Restricted), - ) - .expect("semantically equivalent legacy workspace-write policy should resolve"); - - assert_eq!(resolved.sandbox_policy, sandbox_policy); - assert_eq!( - resolved.file_system_sandbox_policy, - file_system_sandbox_policy - ); - assert_eq!( - resolved.network_sandbox_policy, - NetworkSandboxPolicy::Restricted - ); + assert_eq!(err, ResolvePermissionProfileError::MissingConfiguration); } #[test] diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index d1e84b89ef..3b97e933bb 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -11,14 +11,12 @@ use codex_protocol::error::CodexErr; use codex_protocol::error::Result; use codex_protocol::error::SandboxErr; use codex_protocol::models::PermissionProfile; -use codex_protocol::models::SandboxEnforcement; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -83,37 +81,32 @@ async fn run_cmd_result_with_writable_roots( use_legacy_landlock: bool, network_access: bool, ) -> Result { - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: writable_roots - .iter() - .map(|p| AbsolutePathBuf::try_from(p.as_path()).unwrap()) - .collect(), - network_access, + let writable_roots = writable_roots + .iter() + .map(|path| AbsolutePathBuf::try_from(path.as_path()).unwrap()) + .collect::>(); + let permission_profile = PermissionProfile::workspace_write_with( + &writable_roots, + if network_access { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + }, // Exclude tmp-related folders from writable roots because we need a // folder that is writable by tests but that we intentionally disallow // writing to in the sandbox. - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; - let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - run_cmd_result_with_policies( - cmd, - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, - timeout_ms, - use_legacy_landlock, - ) - .await + /*exclude_tmpdir_env_var*/ + true, + /*exclude_slash_tmp*/ true, + ); + run_cmd_result_with_permission_profile(cmd, permission_profile, timeout_ms, use_legacy_landlock) + .await } #[expect(clippy::expect_used)] -async fn run_cmd_result_with_policies( +async fn run_cmd_result_with_permission_profile( cmd: &[&str], - sandbox_policy: SandboxPolicy, - file_system_sandbox_policy: FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, + permission_profile: PermissionProfile, timeout_ms: u64, use_legacy_landlock: bool, ) -> Result { @@ -134,11 +127,6 @@ async fn run_cmd_result_with_policies( }; let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); - let permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ); process_exec_tool_call( params, @@ -396,10 +384,9 @@ async fn assert_network_blocked(cmd: &[&str]) { arg0: None, }; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe: Option = Some(PathBuf::from(sandbox_program)); - let permission_profile = PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy); + let permission_profile = PermissionProfile::read_only(); let result = process_exec_tool_call( params, &permission_profile, @@ -561,12 +548,6 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { .expect("sandbox helper should have a parent") .to_path_buf(); - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir")], - network_access: true, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -594,16 +575,18 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { access: FileSystemAccessMode::None, }, ]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + ); let output = expect_denied( - run_cmd_result_with_policies( + run_cmd_result_with_permission_profile( &[ "bash", "-lc", &format!("echo denied > {}", blocked_target.to_string_lossy()), ], - sandbox_policy, - file_system_sandbox_policy, - NetworkSandboxPolicy::Enabled, + permission_profile, LONG_TIMEOUT_MS, /*use_legacy_landlock*/ false, ) @@ -633,12 +616,6 @@ async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() { .expect("sandbox helper should have a parent") .to_path_buf(); - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir")], - network_access: true, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -672,7 +649,11 @@ async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() { access: FileSystemAccessMode::Write, }, ]); - let output = run_cmd_result_with_policies( + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + ); + let output = run_cmd_result_with_permission_profile( &[ "bash", "-lc", @@ -682,9 +663,7 @@ async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() { allowed_target.to_string_lossy() ), ], - sandbox_policy, - file_system_sandbox_policy, - NetworkSandboxPolicy::Enabled, + permission_profile, LONG_TIMEOUT_MS, /*use_legacy_landlock*/ false, ) @@ -708,9 +687,6 @@ async fn sandbox_blocks_root_read_carveouts_under_bwrap() { let blocked_target = blocked.join("secret.txt"); std::fs::write(&blocked_target, "secret").expect("seed blocked file"); - let sandbox_policy = SandboxPolicy::ReadOnly { - network_access: true, - }; let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -725,16 +701,18 @@ async fn sandbox_blocks_root_read_carveouts_under_bwrap() { access: FileSystemAccessMode::None, }, ]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + ); let output = expect_denied( - run_cmd_result_with_policies( + run_cmd_result_with_permission_profile( &[ "bash", "-lc", &format!("cat {}", blocked_target.to_string_lossy()), ], - sandbox_policy, - file_system_sandbox_policy, - NetworkSandboxPolicy::Enabled, + permission_profile, LONG_TIMEOUT_MS, /*use_legacy_landlock*/ false, ) diff --git a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs index e906facace..932b7981d3 100644 --- a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs +++ b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs @@ -3,7 +3,7 @@ use codex_core::exec_env::create_env; use codex_protocol::config_types::ShellEnvironmentPolicy; -use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::models::PermissionProfile; use pretty_assertions::assert_eq; use std::collections::HashMap; use std::io::Read; @@ -65,7 +65,7 @@ async fn should_skip_bwrap_tests() -> bool { let output = run_linux_sandbox_direct( &["bash", "-c", "true"], - &SandboxPolicy::new_read_only_policy(), + &PermissionProfile::read_only(), /*allow_network_for_proxy*/ false, env, NETWORK_TIMEOUT_MS, @@ -91,7 +91,7 @@ async fn managed_proxy_skip_reason() -> Option { let output = run_linux_sandbox_direct( &["bash", "-c", "true"], - &SandboxPolicy::DangerFullAccess, + &PermissionProfile::Disabled, /*allow_network_for_proxy*/ true, env, NETWORK_TIMEOUT_MS, @@ -114,7 +114,7 @@ async fn managed_proxy_skip_reason() -> Option { async fn run_linux_sandbox_direct( command: &[&str], - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, allow_network_for_proxy: bool, env: HashMap, timeout_ms: u64, @@ -123,16 +123,16 @@ async fn run_linux_sandbox_direct( Ok(cwd) => cwd, Err(err) => panic!("cwd should exist: {err}"), }; - let policy_json = match serde_json::to_string(sandbox_policy) { - Ok(policy_json) => policy_json, - Err(err) => panic!("policy should serialize: {err}"), + let permission_profile_json = match serde_json::to_string(permission_profile) { + Ok(permission_profile_json) => permission_profile_json, + Err(err) => panic!("permission profile should serialize: {err}"), }; let mut args = vec![ "--sandbox-policy-cwd".to_string(), cwd.to_string_lossy().to_string(), - "--sandbox-policy".to_string(), - policy_json, + "--permission-profile".to_string(), + permission_profile_json, ]; if allow_network_for_proxy { args.push("--allow-network-for-proxy".to_string()); @@ -170,7 +170,7 @@ async fn managed_proxy_mode_fails_closed_without_proxy_env() { let output = run_linux_sandbox_direct( &["bash", "-c", "true"], - &SandboxPolicy::DangerFullAccess, + &PermissionProfile::Disabled, /*allow_network_for_proxy*/ true, env, NETWORK_TIMEOUT_MS, @@ -225,7 +225,7 @@ async fn managed_proxy_mode_routes_through_bridge_and_blocks_direct_egress() { "-c", "proxy=\"${HTTP_PROXY#*://}\"; host=\"${proxy%%:*}\"; port=\"${proxy##*:}\"; exec 3<>/dev/tcp/${host}/${port}; printf 'GET http://example.com/ HTTP/1.1\\r\\nHost: example.com\\r\\n\\r\\n' >&3; IFS= read -r line <&3; printf '%s\\n' \"$line\"", ], - &SandboxPolicy::DangerFullAccess, + &PermissionProfile::Disabled, /*allow_network_for_proxy*/ true, env.clone(), NETWORK_TIMEOUT_MS, @@ -256,7 +256,7 @@ async fn managed_proxy_mode_routes_through_bridge_and_blocks_direct_egress() { let direct_egress_output = run_linux_sandbox_direct( &["bash", "-c", "echo hi > /dev/tcp/192.0.2.1/80"], - &SandboxPolicy::DangerFullAccess, + &PermissionProfile::Disabled, /*allow_network_for_proxy*/ true, env, NETWORK_TIMEOUT_MS, @@ -294,7 +294,7 @@ async fn managed_proxy_mode_denies_af_unix_creation_for_user_command() { "-c", "import socket,sys\ntry:\n socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)\nexcept PermissionError:\n sys.exit(0)\nexcept OSError:\n sys.exit(2)\nsys.exit(1)\n", ], - &SandboxPolicy::DangerFullAccess, + &PermissionProfile::Disabled, /*allow_network_for_proxy*/ true, env, NETWORK_TIMEOUT_MS, diff --git a/codex-rs/sandboxing/src/landlock.rs b/codex-rs/sandboxing/src/landlock.rs index d948d23d1f..0ff3f6977f 100644 --- a/codex-rs/sandboxing/src/landlock.rs +++ b/codex-rs/sandboxing/src/landlock.rs @@ -1,6 +1,4 @@ -use codex_protocol::permissions::FileSystemSandboxPolicy; -use codex_protocol::permissions::NetworkSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::models::PermissionProfile; use std::path::Path; /// Basename used when the Codex executable self-invokes as the Linux sandbox @@ -14,30 +12,24 @@ pub fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { enforce_managed_network } -/// Converts the sandbox policies into the CLI invocation for +/// Converts the permission profile into the CLI invocation for /// `codex-linux-sandbox`. /// -/// The helper performs the actual sandboxing (bubblewrap by default + seccomp) after -/// parsing these arguments. Policy JSON flags are emitted before helper feature -/// flags so the argv order matches the helper's CLI shape. See +/// The helper performs the actual sandboxing (bubblewrap by default + seccomp) +/// after parsing these arguments. The profile JSON flag is emitted before +/// helper feature flags so the argv order matches the helper's CLI shape. See /// `docs/linux_sandbox.md` for the Linux semantics. #[allow(clippy::too_many_arguments)] -pub fn create_linux_sandbox_command_args_for_policies( +pub fn create_linux_sandbox_command_args_for_permission_profile( command: Vec, command_cwd: &Path, - sandbox_policy: &SandboxPolicy, - file_system_sandbox_policy: &FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, + permission_profile: &PermissionProfile, sandbox_policy_cwd: &Path, use_legacy_landlock: bool, allow_network_for_proxy: bool, ) -> Vec { - let sandbox_policy_json = serde_json::to_string(sandbox_policy) - .unwrap_or_else(|err| panic!("failed to serialize sandbox policy: {err}")); - let file_system_policy_json = serde_json::to_string(file_system_sandbox_policy) - .unwrap_or_else(|err| panic!("failed to serialize filesystem sandbox policy: {err}")); - let network_policy_json = serde_json::to_string(&network_sandbox_policy) - .unwrap_or_else(|err| panic!("failed to serialize network sandbox policy: {err}")); + let permission_profile_json = serde_json::to_string(permission_profile) + .unwrap_or_else(|err| panic!("failed to serialize permission profile: {err}")); let sandbox_policy_cwd = sandbox_policy_cwd .to_str() .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) @@ -52,12 +44,8 @@ pub fn create_linux_sandbox_command_args_for_policies( sandbox_policy_cwd, "--command-cwd".to_string(), command_cwd, - "--sandbox-policy".to_string(), - sandbox_policy_json, - "--file-system-sandbox-policy".to_string(), - file_system_policy_json, - "--network-sandbox-policy".to_string(), - network_policy_json, + "--permission-profile".to_string(), + permission_profile_json, ]; if use_legacy_landlock { linux_cmd.push("--use-legacy-landlock".to_string()); diff --git a/codex-rs/sandboxing/src/landlock_tests.rs b/codex-rs/sandboxing/src/landlock_tests.rs index b1dd6236ab..14b1c047eb 100644 --- a/codex-rs/sandboxing/src/landlock_tests.rs +++ b/codex-rs/sandboxing/src/landlock_tests.rs @@ -52,20 +52,16 @@ fn proxy_flag_is_included_when_requested() { } #[test] -fn split_policy_flags_are_included() { +fn permission_profile_flag_is_included() { let command = vec!["/bin/true".to_string()]; let command_cwd = Path::new("/tmp/link"); let cwd = Path::new("/tmp"); - let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); + let permission_profile = PermissionProfile::read_only(); - let args = create_linux_sandbox_command_args_for_policies( + let args = create_linux_sandbox_command_args_for_permission_profile( command, command_cwd, - &sandbox_policy, - &file_system_sandbox_policy, - network_sandbox_policy, + &permission_profile, cwd, /*use_legacy_landlock*/ true, /*allow_network_for_proxy*/ false, @@ -73,12 +69,7 @@ fn split_policy_flags_are_included() { assert_eq!( args.windows(2) - .any(|window| { window[0] == "--file-system-sandbox-policy" && !window[1].is_empty() }), - true - ); - assert_eq!( - args.windows(2) - .any(|window| window[0] == "--network-sandbox-policy" && window[1] == "\"restricted\""), + .any(|window| { window[0] == "--permission-profile" && !window[1].is_empty() }), true ); assert_eq!( diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 900130ee68..82c49f7908 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -4,7 +4,7 @@ use crate::bwrap::WSL1_BWRAP_WARNING; use crate::bwrap::is_wsl1; use crate::landlock::CODEX_LINUX_SANDBOX_ARG0; use crate::landlock::allow_network_for_proxy; -use crate::landlock::create_linux_sandbox_command_args_for_policies; +use crate::landlock::create_linux_sandbox_command_args_for_permission_profile; use crate::policy_transforms::effective_permission_profile; use crate::policy_transforms::should_require_platform_sandbox; use codex_network_proxy::NetworkProxy; @@ -186,12 +186,6 @@ impl SandboxManager { effective_permission_profile(permissions, additional_permissions.as_ref()); let (effective_file_system_policy, effective_network_policy) = effective_permission_profile.to_runtime_permissions(); - let effective_policy = compatibility_sandbox_policy_for_permission_profile( - &effective_permission_profile, - &effective_file_system_policy, - effective_network_policy, - sandbox_policy_cwd, - ); let mut argv = Vec::with_capacity(1 + command.args.len()); argv.push(command.program); argv.extend(command.args.into_iter().map(OsString::from)); @@ -231,12 +225,10 @@ impl SandboxManager { allow_proxy_network, is_wsl1(), )?; - let mut args = create_linux_sandbox_command_args_for_policies( + let mut args = create_linux_sandbox_command_args_for_permission_profile( os_argv_to_strings(argv), command.cwd.as_path(), - &effective_policy, - &effective_file_system_policy, - effective_network_policy, + &effective_permission_profile, sandbox_policy_cwd, use_legacy_landlock, allow_proxy_network, diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 516bda6859..7c638ba675 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -140,6 +140,8 @@ pub use tool_spec::create_local_shell_tool; pub use tool_spec::create_tools_json_for_responses_api; pub use tool_spec::create_web_search_tool; pub use tool_suggest::TOOL_SUGGEST_APPROVAL_KIND_VALUE; +pub use tool_suggest::TOOL_SUGGEST_PERSIST_ALWAYS_VALUE; +pub use tool_suggest::TOOL_SUGGEST_PERSIST_KEY; pub use tool_suggest::ToolSuggestArgs; pub use tool_suggest::ToolSuggestMeta; pub use tool_suggest::ToolSuggestResult; diff --git a/codex-rs/tools/src/tool_suggest.rs b/codex-rs/tools/src/tool_suggest.rs index 7a6e87ff84..86e81dbbfe 100644 --- a/codex-rs/tools/src/tool_suggest.rs +++ b/codex-rs/tools/src/tool_suggest.rs @@ -14,6 +14,8 @@ use crate::DiscoverableToolAction; use crate::DiscoverableToolType; pub const TOOL_SUGGEST_APPROVAL_KIND_VALUE: &str = "tool_suggestion"; +pub const TOOL_SUGGEST_PERSIST_KEY: &str = "persist"; +pub const TOOL_SUGGEST_PERSIST_ALWAYS_VALUE: &str = "always"; #[derive(Debug, Deserialize)] pub struct ToolSuggestArgs { @@ -37,6 +39,7 @@ pub struct ToolSuggestResult { #[derive(Debug, Serialize, PartialEq, Eq)] pub struct ToolSuggestMeta<'a> { pub codex_approval_kind: &'static str, + pub persist: &'static str, pub tool_type: DiscoverableToolType, pub suggest_type: DiscoverableToolAction, pub suggest_reason: &'a str, @@ -111,6 +114,7 @@ fn build_tool_suggestion_meta<'a>( ) -> ToolSuggestMeta<'a> { ToolSuggestMeta { codex_approval_kind: TOOL_SUGGEST_APPROVAL_KIND_VALUE, + persist: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE, tool_type, suggest_type: action_type, suggest_reason, diff --git a/codex-rs/tools/src/tool_suggest_tests.rs b/codex-rs/tools/src/tool_suggest_tests.rs index d3c37bf110..056ef70151 100644 --- a/codex-rs/tools/src/tool_suggest_tests.rs +++ b/codex-rs/tools/src/tool_suggest_tests.rs @@ -48,6 +48,7 @@ fn build_tool_suggestion_elicitation_request_uses_expected_shape() { request: McpServerElicitationRequest::Form { meta: Some(json!(ToolSuggestMeta { codex_approval_kind: TOOL_SUGGEST_APPROVAL_KIND_VALUE, + persist: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE, tool_type: DiscoverableToolType::Connector, suggest_type: DiscoverableToolAction::Install, suggest_reason: "Plan and reference events from your calendar", @@ -104,6 +105,7 @@ fn build_tool_suggestion_elicitation_request_for_plugin_omits_install_url() { request: McpServerElicitationRequest::Form { meta: Some(json!(ToolSuggestMeta { codex_approval_kind: TOOL_SUGGEST_APPROVAL_KIND_VALUE, + persist: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE, tool_type: DiscoverableToolType::Plugin, suggest_type: DiscoverableToolAction::Install, suggest_reason: "Use the sample plugin's skills and MCP server", @@ -138,6 +140,7 @@ fn build_tool_suggestion_meta_uses_expected_shape() { meta, ToolSuggestMeta { codex_approval_kind: TOOL_SUGGEST_APPROVAL_KIND_VALUE, + persist: TOOL_SUGGEST_PERSIST_ALWAYS_VALUE, tool_type: DiscoverableToolType::Connector, suggest_type: DiscoverableToolAction::Install, suggest_reason: "Find and reference emails from your inbox", diff --git a/docs/config.md b/docs/config.md index d583122e01..8dda2b6393 100644 --- a/docs/config.md +++ b/docs/config.md @@ -48,6 +48,16 @@ Use `$` in the composer to insert a ChatGPT connector; the popover lists accessi apps. The `/apps` command lists available and installed apps. Connected apps appear first and are labeled as connected; others are marked as can be installed. +Codex stores "never show again" choices for tool suggestions in `config.toml`: + +```toml +[tool_suggest] +disabled_tools = [ + { type = "plugin", id = "slack@openai-curated" }, + { type = "connector", id = "connector_google_calendar" }, +] +``` + ## Notify Codex can run a notification hook when the agent finishes a turn. See the configuration reference for the latest notification settings: