diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4c22f4a66a36..f216f9e0f7a9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2564,6 +2564,7 @@ version = "0.0.0" dependencies = [ "anyhow", "codex-app-server-protocol", + "codex-config", "pretty_assertions", "serde", "serde_json", diff --git a/codex-rs/connectors/Cargo.toml b/codex-rs/connectors/Cargo.toml index 1ebdae32dc1b..417b3e48a7df 100644 --- a/codex-rs/connectors/Cargo.toml +++ b/codex-rs/connectors/Cargo.toml @@ -10,6 +10,7 @@ workspace = true [dependencies] anyhow = { workspace = true } codex-app-server-protocol = { workspace = true } +codex-config = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha1 = { workspace = true } diff --git a/codex-rs/connectors/src/app_tool_policy.rs b/codex-rs/connectors/src/app_tool_policy.rs new file mode 100644 index 000000000000..5f1e512beccd --- /dev/null +++ b/codex-rs/connectors/src/app_tool_policy.rs @@ -0,0 +1,207 @@ +use codex_config::AppsRequirementsToml; +use codex_config::ConfigLayerStack; +use codex_config::types::AppToolApproval; +use codex_config::types::AppsConfigToml; +use serde::Deserialize; + +/// The effective enablement and approval policy for one app tool. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AppToolPolicy { + pub enabled: bool, + pub approval: AppToolApproval, +} + +impl Default for AppToolPolicy { + fn default() -> Self { + Self { + enabled: true, + approval: AppToolApproval::Auto, + } + } +} + +/// Connector-owned metadata used to evaluate one app tool. +#[derive(Debug, Clone, Copy)] +pub struct AppToolPolicyInput<'a> { + pub connector_id: Option<&'a str>, + pub tool_name: &'a str, + pub tool_title: Option<&'a str>, + pub destructive_hint: Option, + pub open_world_hint: Option, +} + +/// Resolves app tool policy against one immutable config snapshot. +/// +/// Callers should construct one evaluator and reuse it for every tool in the +/// same exposure build so config layers are merged and decoded only once. +pub struct AppToolPolicyEvaluator<'a> { + apps_config: Option, + requirements_apps_config: Option<&'a AppsRequirementsToml>, +} + +impl<'a> AppToolPolicyEvaluator<'a> { + pub fn new(config_layer_stack: &'a ConfigLayerStack) -> Self { + let apps_config = apps_config_from_layer_stack(config_layer_stack); + let requirements_apps_config = config_layer_stack.requirements_toml().apps.as_ref(); + Self::from_parts(apps_config, requirements_apps_config) + } + + pub fn policy(&self, input: AppToolPolicyInput<'_>) -> AppToolPolicy { + let managed_approval = managed_app_tool_approval( + self.requirements_apps_config, + input.connector_id, + input.tool_name, + ); + app_tool_policy_from_apps_config(self.apps_config.as_ref(), input, managed_approval) + } + + fn from_parts( + apps_config: Option, + requirements_apps_config: Option<&'a AppsRequirementsToml>, + ) -> Self { + Self { + apps_config: effective_apps_config(apps_config, requirements_apps_config), + requirements_apps_config, + } + } +} + +/// Reads the merged, unmanaged Apps configuration from a config-layer stack. +pub fn apps_config_from_layer_stack( + config_layer_stack: &ConfigLayerStack, +) -> Option { + config_layer_stack + .effective_config() + .as_table() + .and_then(|table| table.get("apps")) + .cloned() + .and_then(|value| AppsConfigToml::deserialize(value).ok()) +} + +pub fn app_is_enabled(apps_config: &AppsConfigToml, connector_id: Option<&str>) -> bool { + let default_enabled = apps_config + .default + .as_ref() + .map(|defaults| defaults.enabled) + .unwrap_or(true); + + connector_id + .and_then(|connector_id| apps_config.apps.get(connector_id)) + .map(|app| app.enabled) + .unwrap_or(default_enabled) +} + +fn effective_apps_config( + apps_config: Option, + requirements_apps_config: Option<&AppsRequirementsToml>, +) -> Option { + let had_apps_config = apps_config.is_some(); + let mut apps_config = apps_config.unwrap_or_default(); + apply_requirements_apps_constraints(&mut apps_config, requirements_apps_config); + if had_apps_config || apps_config.default.is_some() || !apps_config.apps.is_empty() { + Some(apps_config) + } else { + None + } +} + +fn apply_requirements_apps_constraints( + apps_config: &mut AppsConfigToml, + requirements_apps_config: Option<&AppsRequirementsToml>, +) { + let Some(requirements_apps_config) = requirements_apps_config else { + return; + }; + + for (app_id, requirement) in &requirements_apps_config.apps { + if requirement.enabled == Some(false) { + let app = apps_config.apps.entry(app_id.clone()).or_default(); + app.enabled = false; + } + } +} + +fn managed_app_tool_approval( + requirements_apps_config: Option<&AppsRequirementsToml>, + connector_id: Option<&str>, + tool_name: &str, +) -> Option { + let connector_id = connector_id?; + requirements_apps_config? + .apps + .get(connector_id)? + .tools + .as_ref()? + .tools + .get(tool_name)? + .approval_mode +} + +fn app_tool_policy_from_apps_config( + apps_config: Option<&AppsConfigToml>, + input: AppToolPolicyInput<'_>, + managed_approval: Option, +) -> AppToolPolicy { + let Some(apps_config) = apps_config else { + return AppToolPolicy { + approval: managed_approval.unwrap_or(AppToolApproval::Auto), + ..Default::default() + }; + }; + + let app = input + .connector_id + .and_then(|connector_id| apps_config.apps.get(connector_id)); + let tools = app.and_then(|app| app.tools.as_ref()); + let tool_config = tools.and_then(|tools| { + tools + .tools + .get(input.tool_name) + .or_else(|| input.tool_title.and_then(|title| tools.tools.get(title))) + }); + let approval = managed_approval + .or_else(|| tool_config.and_then(|tool| tool.approval_mode)) + .or_else(|| app.and_then(|app| app.default_tools_approval_mode)) + .unwrap_or(AppToolApproval::Auto); + + if !app_is_enabled(apps_config, input.connector_id) { + return AppToolPolicy { + enabled: false, + approval, + }; + } + + if let Some(enabled) = tool_config.and_then(|tool| tool.enabled) { + return AppToolPolicy { enabled, approval }; + } + + if let Some(enabled) = app.and_then(|app| app.default_tools_enabled) { + return AppToolPolicy { enabled, approval }; + } + + let app_defaults = apps_config.default.as_ref(); + let destructive_enabled = app + .and_then(|app| app.destructive_enabled) + .unwrap_or_else(|| { + app_defaults + .map(|defaults| defaults.destructive_enabled) + .unwrap_or(true) + }); + let open_world_enabled = app + .and_then(|app| app.open_world_enabled) + .unwrap_or_else(|| { + app_defaults + .map(|defaults| defaults.open_world_enabled) + .unwrap_or(true) + }); + let destructive_hint = input.destructive_hint.unwrap_or(true); + let open_world_hint = input.open_world_hint.unwrap_or(true); + let enabled = + (destructive_enabled || !destructive_hint) && (open_world_enabled || !open_world_hint); + + AppToolPolicy { enabled, approval } +} + +#[cfg(test)] +#[path = "app_tool_policy_tests.rs"] +mod tests; diff --git a/codex-rs/connectors/src/app_tool_policy_tests.rs b/codex-rs/connectors/src/app_tool_policy_tests.rs new file mode 100644 index 000000000000..45e01cabdf6f --- /dev/null +++ b/codex-rs/connectors/src/app_tool_policy_tests.rs @@ -0,0 +1,724 @@ +use std::collections::BTreeMap; +use std::collections::HashMap; + +use codex_config::AbsolutePathBuf; +use codex_config::AppRequirementToml; +use codex_config::AppToolRequirementToml; +use codex_config::AppToolsRequirementsToml; +use codex_config::AppsRequirementsToml; +use codex_config::CONFIG_TOML_FILE; +use codex_config::ConfigLayerStack; +use codex_config::ConfigRequirements; +use codex_config::ConfigRequirementsToml; +use codex_config::TomlValue; +use codex_config::types::AppConfig; +use codex_config::types::AppToolApproval; +use codex_config::types::AppToolConfig; +use codex_config::types::AppToolsConfig; +use codex_config::types::AppsConfigToml; +use codex_config::types::AppsDefaultConfig; +use pretty_assertions::assert_eq; + +use super::*; + +#[test] +fn evaluator_reuses_one_snapshot_across_tools() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "calendar".to_string(), + AppConfig { + enabled: true, + default_tools_enabled: Some(false), + tools: Some(AppToolsConfig { + tools: HashMap::from([( + "events/create".to_string(), + AppToolConfig { + enabled: Some(true), + approval_mode: Some(AppToolApproval::Prompt), + }, + )]), + }), + ..Default::default() + }, + )]), + }; + let requirements = AppsRequirementsToml { + apps: BTreeMap::from([( + "calendar".to_string(), + AppRequirementToml { + enabled: None, + tools: Some(AppToolsRequirementsToml { + tools: BTreeMap::from([( + "events/create".to_string(), + AppToolRequirementToml { + approval_mode: Some(AppToolApproval::Approve), + }, + )]), + }), + }, + )]), + }; + let evaluator = AppToolPolicyEvaluator::from_parts(Some(apps_config), Some(&requirements)); + + assert_eq!( + [ + evaluator.policy(input("events/create", /*tool_title*/ None)), + evaluator.policy(input("events/list", /*tool_title*/ None)), + evaluator.policy(input("calendar_events/create", Some("events/create"))), + ], + [ + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Approve, + }, + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + }, + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Prompt, + }, + ] + ); +} + +#[test] +fn evaluator_uses_global_defaults_for_destructive_hints() { + let apps_config = AppsConfigToml { + default: Some(defaults( + /*enabled*/ true, /*destructive_enabled*/ false, + /*open_world_enabled*/ true, + )), + apps: HashMap::new(), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/create", + /*tool_title*/ None, + Some(true), + /*open_world_hint*/ None, + /*managed_approval*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn evaluator_defaults_missing_destructive_hint_to_true() { + let apps_config = AppsConfigToml { + default: Some(defaults( + /*enabled*/ true, /*destructive_enabled*/ false, + /*open_world_enabled*/ true, + )), + apps: HashMap::new(), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/create", + /*tool_title*/ None, + /*destructive_hint*/ None, + Some(false), + /*managed_approval*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn evaluator_defaults_missing_open_world_hint_to_true() { + let apps_config = AppsConfigToml { + default: Some(defaults( + /*enabled*/ true, /*destructive_enabled*/ true, + /*open_world_enabled*/ false, + )), + apps: HashMap::new(), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/create", + /*tool_title*/ None, + Some(false), + /*open_world_hint*/ None, + /*managed_approval*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn app_enablement_uses_defaults_and_per_app_overrides() { + let apps_config = AppsConfigToml { + default: Some(defaults( + /*enabled*/ false, /*destructive_enabled*/ true, + /*open_world_enabled*/ true, + )), + apps: HashMap::from([( + "calendar".to_string(), + AppConfig { + enabled: true, + ..Default::default() + }, + )]), + }; + + assert_eq!( + [ + app_is_enabled(&apps_config, Some("calendar")), + app_is_enabled(&apps_config, Some("drive")), + app_is_enabled(&apps_config, /*connector_id*/ None), + ], + [true, false, false] + ); +} + +#[test] +fn managed_disable_overrides_enabled_app() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "connector_123123".to_string(), + AppConfig { + enabled: true, + ..Default::default() + }, + )]), + }; + let requirements = app_enabled_requirement("connector_123123", /*enabled*/ false); + + assert_eq!( + policy_from_config_parts( + Some(&apps_config), + Some(&requirements), + Some("connector_123123"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn managed_enable_does_not_override_disabled_app() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "connector_123123".to_string(), + AppConfig { + enabled: false, + ..Default::default() + }, + )]), + }; + let requirements = app_enabled_requirement("connector_123123", /*enabled*/ true); + + assert_eq!( + policy_from_config_parts( + Some(&apps_config), + Some(&requirements), + Some("connector_123123"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn managed_disable_applies_without_apps_config() { + let requirements = app_enabled_requirement("connector_123123", /*enabled*/ false); + + assert_eq!( + policy_from_config_parts( + /*apps_config*/ None, + Some(&requirements), + Some("connector_123123"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn evaluator_honors_default_app_enabled_false() { + let apps_config = AppsConfigToml { + default: Some(defaults( + /*enabled*/ false, /*destructive_enabled*/ true, + /*open_world_enabled*/ true, + )), + apps: HashMap::new(), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + /*managed_approval*/ None, + ), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Auto, + } + ); +} + +#[test] +fn evaluator_allows_per_app_enable_when_default_is_disabled() { + let apps_config = AppsConfigToml { + default: Some(defaults( + /*enabled*/ false, /*destructive_enabled*/ true, + /*open_world_enabled*/ true, + )), + apps: HashMap::from([( + "calendar".to_string(), + AppConfig { + enabled: true, + ..Default::default() + }, + )]), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + /*managed_approval*/ None, + ), + AppToolPolicy::default() + ); +} + +#[test] +fn evaluator_uses_managed_approval_without_apps_config() { + assert_eq!( + policy_from_apps_config( + /*apps_config*/ None, + Some("calendar"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + Some(AppToolApproval::Approve), + ), + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Approve, + } + ); +} + +#[test] +fn managed_approval_uses_raw_tool_name() { + let requirements = app_tool_requirements( + "connector_123123", + "calendar/list_events", + AppToolApproval::Approve, + ); + + assert_eq!( + [ + policy_from_config_parts( + /*apps_config*/ None, + Some(&requirements), + Some("connector_123123"), + "calendar/list_events", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + ), + policy_from_config_parts( + /*apps_config*/ None, + Some(&requirements), + Some("connector_123123"), + "calendar/create_event", + Some("calendar/list_events"), + /*destructive_hint*/ None, + /*open_world_hint*/ None, + ), + ], + [ + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Approve, + }, + AppToolPolicy::default(), + ] + ); +} + +#[test] +fn managed_approval_overrides_user_tool_approval() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "connector_123123".to_string(), + AppConfig { + enabled: true, + tools: Some(AppToolsConfig { + tools: HashMap::from([( + "calendar/list_events".to_string(), + AppToolConfig { + enabled: None, + approval_mode: Some(AppToolApproval::Prompt), + }, + )]), + }), + ..Default::default() + }, + )]), + }; + let requirements = app_tool_requirements( + "connector_123123", + "calendar/list_events", + AppToolApproval::Approve, + ); + + assert_eq!( + policy_from_config_parts( + Some(&apps_config), + Some(&requirements), + Some("connector_123123"), + "calendar/list_events", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + ), + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Approve, + } + ); +} + +#[test] +fn per_tool_enable_overrides_app_level_hints() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "calendar".to_string(), + AppConfig { + enabled: true, + destructive_enabled: Some(false), + open_world_enabled: Some(false), + tools: Some(AppToolsConfig { + tools: HashMap::from([( + "events/create".to_string(), + AppToolConfig { + enabled: Some(true), + approval_mode: None, + }, + )]), + }), + ..Default::default() + }, + )]), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/create", + /*tool_title*/ None, + Some(true), + Some(true), + /*managed_approval*/ None, + ), + AppToolPolicy::default() + ); +} + +#[test] +fn default_tools_enable_overrides_app_level_hints() { + let mut app = AppConfig { + enabled: true, + destructive_enabled: Some(false), + open_world_enabled: Some(false), + default_tools_enabled: Some(true), + ..Default::default() + }; + let apps_config = |app: AppConfig| AppsConfigToml { + default: None, + apps: HashMap::from([("calendar".to_string(), app)]), + }; + + let enabled_policy = policy_from_apps_config( + Some(&apps_config(app.clone())), + Some("calendar"), + "events/create", + /*tool_title*/ None, + Some(true), + Some(true), + /*managed_approval*/ None, + ); + app.destructive_enabled = Some(true); + app.open_world_enabled = Some(true); + app.default_tools_enabled = Some(false); + app.default_tools_approval_mode = Some(AppToolApproval::Approve); + let disabled_policy = policy_from_apps_config( + Some(&apps_config(app)), + Some("calendar"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + /*managed_approval*/ None, + ); + + assert_eq!( + [enabled_policy, disabled_policy], + [ + AppToolPolicy::default(), + AppToolPolicy { + enabled: false, + approval: AppToolApproval::Approve, + }, + ] + ); +} + +#[test] +fn evaluator_uses_default_tools_approval_mode() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "calendar".to_string(), + AppConfig { + enabled: true, + default_tools_approval_mode: Some(AppToolApproval::Prompt), + tools: Some(AppToolsConfig { + tools: HashMap::new(), + }), + ..Default::default() + }, + )]), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "events/list", + /*tool_title*/ None, + /*destructive_hint*/ None, + /*open_world_hint*/ None, + /*managed_approval*/ None, + ), + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Prompt, + } + ); +} + +#[test] +fn evaluator_matches_tool_title_for_user_config() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([( + "calendar".to_string(), + AppConfig { + enabled: true, + destructive_enabled: Some(false), + open_world_enabled: Some(false), + default_tools_approval_mode: Some(AppToolApproval::Auto), + default_tools_enabled: Some(false), + tools: Some(AppToolsConfig { + tools: HashMap::from([( + "events/create".to_string(), + AppToolConfig { + enabled: Some(true), + approval_mode: Some(AppToolApproval::Approve), + }, + )]), + }), + ..Default::default() + }, + )]), + }; + + assert_eq!( + policy_from_apps_config( + Some(&apps_config), + Some("calendar"), + "calendar_events/create", + Some("events/create"), + Some(true), + Some(true), + /*managed_approval*/ None, + ), + AppToolPolicy { + enabled: true, + approval: AppToolApproval::Approve, + } + ); +} + +fn input<'a>(tool_name: &'a str, tool_title: Option<&'a str>) -> AppToolPolicyInput<'a> { + AppToolPolicyInput { + connector_id: Some("calendar"), + tool_name, + tool_title, + destructive_hint: Some(true), + open_world_hint: Some(true), + } +} + +fn policy_from_apps_config( + apps_config: Option<&AppsConfigToml>, + connector_id: Option<&str>, + tool_name: &str, + tool_title: Option<&str>, + destructive_hint: Option, + open_world_hint: Option, + managed_approval: Option, +) -> AppToolPolicy { + let requirements = managed_approval.map(|approval| { + app_tool_requirements( + connector_id.expect("managed approval requires a connector id"), + tool_name, + approval, + ) + }); + policy_from_config_parts( + apps_config, + requirements.as_ref(), + connector_id, + tool_name, + tool_title, + destructive_hint, + open_world_hint, + ) +} + +fn policy_from_config_parts( + apps_config: Option<&AppsConfigToml>, + requirements_apps_config: Option<&AppsRequirementsToml>, + connector_id: Option<&str>, + tool_name: &str, + tool_title: Option<&str>, + destructive_hint: Option, + open_world_hint: Option, +) -> AppToolPolicy { + let requirements = ConfigRequirementsToml { + apps: requirements_apps_config.cloned(), + ..Default::default() + }; + let config_layer_stack = + ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) + .expect("config layer stack"); + let config_layer_stack = if let Some(apps_config) = apps_config { + let mut user_config = TomlValue::Table(Default::default()); + user_config + .as_table_mut() + .expect("user config table") + .insert( + "apps".to_string(), + TomlValue::try_from(apps_config).expect("serialize apps config"), + ); + let config_toml_path = + AbsolutePathBuf::try_from(std::env::temp_dir().join(CONFIG_TOML_FILE)) + .expect("absolute config path"); + config_layer_stack.with_user_config(&config_toml_path, user_config) + } else { + config_layer_stack + }; + AppToolPolicyEvaluator::new(&config_layer_stack).policy(AppToolPolicyInput { + connector_id, + tool_name, + tool_title, + destructive_hint, + open_world_hint, + }) +} + +fn app_enabled_requirement(app_id: &str, enabled: bool) -> AppsRequirementsToml { + AppsRequirementsToml { + apps: BTreeMap::from([( + app_id.to_string(), + AppRequirementToml { + enabled: Some(enabled), + tools: None, + }, + )]), + } +} + +fn app_tool_requirements( + app_id: &str, + tool_name: &str, + approval_mode: AppToolApproval, +) -> AppsRequirementsToml { + AppsRequirementsToml { + apps: BTreeMap::from([( + app_id.to_string(), + AppRequirementToml { + enabled: None, + tools: Some(AppToolsRequirementsToml { + tools: BTreeMap::from([( + tool_name.to_string(), + AppToolRequirementToml { + approval_mode: Some(approval_mode), + }, + )]), + }), + }, + )]), + } +} + +fn defaults( + enabled: bool, + destructive_enabled: bool, + open_world_enabled: bool, +) -> AppsDefaultConfig { + AppsDefaultConfig { + enabled, + approvals_reviewer: None, + destructive_enabled, + open_world_enabled, + } +} diff --git a/codex-rs/connectors/src/lib.rs b/codex-rs/connectors/src/lib.rs index c2bf8911153a..a75120c0a457 100644 --- a/codex-rs/connectors/src/lib.rs +++ b/codex-rs/connectors/src/lib.rs @@ -12,11 +12,17 @@ use serde::Deserialize; use serde::Serialize; pub mod accessible; +mod app_tool_policy; mod directory_cache; pub mod filter; pub mod merge; pub mod metadata; +pub use app_tool_policy::AppToolPolicy; +pub use app_tool_policy::AppToolPolicyEvaluator; +pub use app_tool_policy::AppToolPolicyInput; +pub use app_tool_policy::app_is_enabled; +pub use app_tool_policy::apps_config_from_layer_stack; pub use directory_cache::ConnectorDirectoryCacheContext; pub const CONNECTORS_CACHE_TTL: Duration = Duration::from_secs(3600); diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index f21912fdbf9b..cd0c3a7bbf5a 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -11,12 +11,12 @@ pub use codex_app_server_protocol::AppInfo; pub use codex_app_server_protocol::AppMetadata; use codex_connectors::ConnectorDirectoryCacheContext; use codex_connectors::ConnectorDirectoryCacheKey; +use codex_connectors::app_is_enabled; +use codex_connectors::apps_config_from_layer_stack; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecServerRuntimePaths; use codex_protocol::models::PermissionProfile; use codex_tools::DiscoverableTool; -use rmcp::model::ToolAnnotations; -use serde::Deserialize; use tokio_util::sync::CancellationToken; use tracing::warn; @@ -24,10 +24,7 @@ use crate::config::Config; use crate::mcp::McpManager; use crate::plugins::list_tool_suggest_discoverable_plugins; use crate::session::INITIAL_SUBMIT_ID; -use codex_config::AppsRequirementsToml; -use codex_config::types::AppToolApproval; use codex_config::types::ApprovalsReviewer; -use codex_config::types::AppsConfigToml; use codex_config::types::ToolSuggestDiscoverableType; use codex_core_plugins::PluginsManager; use codex_features::Feature; @@ -47,21 +44,6 @@ use codex_mcp::tool_plugin_provenance; const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30); -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) struct AppToolPolicy { - pub enabled: bool, - pub approval: AppToolApproval, -} - -impl Default for AppToolPolicy { - fn default() -> Self { - Self { - enabled: true, - approval: AppToolApproval::Auto, - } - } -} - #[derive(Clone, PartialEq, Eq)] struct AccessibleConnectorsCacheKey { chatgpt_base_url: String, @@ -516,7 +498,7 @@ pub(crate) fn accessible_connectors_from_mcp_tools(mcp_tools: &[ToolInfo]) -> Ve } pub fn with_app_enabled_state(mut connectors: Vec, config: &Config) -> Vec { - let user_apps_config = read_user_apps_config(config); + let user_apps_config = apps_config_from_layer_stack(&config.config_layer_stack); let requirements_apps_config = config.config_layer_stack.requirements_toml().apps.as_ref(); if user_apps_config.is_none() && requirements_apps_config.is_none() { return connectors; @@ -553,51 +535,13 @@ pub fn with_app_plugin_sources( connectors } -pub(crate) fn app_tool_policy( - config: &Config, - connector_id: Option<&str>, - tool_name: &str, - tool_title: Option<&str>, - annotations: Option<&ToolAnnotations>, -) -> AppToolPolicy { - let apps_config = read_apps_config(config); - let managed_approval = managed_app_tool_approval( - config.config_layer_stack.requirements_toml().apps.as_ref(), - connector_id, - tool_name, - ); - app_tool_policy_from_apps_config( - apps_config.as_ref(), - connector_id, - tool_name, - tool_title, - annotations, - managed_approval, - ) -} - -pub(crate) fn codex_app_tool_is_enabled(config: &Config, tool_info: &ToolInfo) -> bool { - if tool_info.server_name != CODEX_APPS_MCP_SERVER_NAME { - return true; - } - - app_tool_policy( - config, - tool_info.connector_id.as_deref(), - &tool_info.tool.name, - tool_info.tool.title.as_deref(), - tool_info.tool.annotations.as_ref(), - ) - .enabled -} - pub(crate) fn mcp_approvals_reviewer( config: &Config, server_name: &str, connector_id: Option<&str>, ) -> ApprovalsReviewer { let app_reviewer = if server_name == CODEX_APPS_MCP_SERVER_NAME { - read_user_apps_config(config).and_then(|apps_config| { + apps_config_from_layer_stack(&config.config_layer_stack).and_then(|apps_config| { connector_id .and_then(|connector_id| apps_config.apps.get(connector_id)) .and_then(|app| app.approvals_reviewer) @@ -625,146 +569,6 @@ pub(crate) fn mcp_approvals_reviewer( config.approvals_reviewer } -fn read_apps_config(config: &Config) -> Option { - let apps_config = read_user_apps_config(config); - let had_apps_config = apps_config.is_some(); - let mut apps_config = apps_config.unwrap_or_default(); - apply_requirements_apps_constraints( - &mut apps_config, - config.config_layer_stack.requirements_toml().apps.as_ref(), - ); - if had_apps_config || apps_config.default.is_some() || !apps_config.apps.is_empty() { - Some(apps_config) - } else { - None - } -} - -fn read_user_apps_config(config: &Config) -> Option { - config - .config_layer_stack - .effective_config() - .as_table() - .and_then(|table| table.get("apps")) - .cloned() - .and_then(|value| AppsConfigToml::deserialize(value).ok()) -} - -fn apply_requirements_apps_constraints( - apps_config: &mut AppsConfigToml, - requirements_apps_config: Option<&AppsRequirementsToml>, -) { - let Some(requirements_apps_config) = requirements_apps_config else { - return; - }; - - for (app_id, requirement) in &requirements_apps_config.apps { - if requirement.enabled == Some(false) { - let app = apps_config.apps.entry(app_id.clone()).or_default(); - app.enabled = false; - } - } -} - -fn managed_app_tool_approval( - requirements_apps_config: Option<&AppsRequirementsToml>, - connector_id: Option<&str>, - tool_name: &str, -) -> Option { - let connector_id = connector_id?; - requirements_apps_config? - .apps - .get(connector_id)? - .tools - .as_ref()? - .tools - .get(tool_name)? - .approval_mode -} - -fn app_is_enabled(apps_config: &AppsConfigToml, connector_id: Option<&str>) -> bool { - let default_enabled = apps_config - .default - .as_ref() - .map(|defaults| defaults.enabled) - .unwrap_or(true); - - connector_id - .and_then(|connector_id| apps_config.apps.get(connector_id)) - .map(|app| app.enabled) - .unwrap_or(default_enabled) -} - -fn app_tool_policy_from_apps_config( - apps_config: Option<&AppsConfigToml>, - connector_id: Option<&str>, - tool_name: &str, - tool_title: Option<&str>, - annotations: Option<&ToolAnnotations>, - managed_approval: Option, -) -> AppToolPolicy { - let Some(apps_config) = apps_config else { - return AppToolPolicy { - approval: managed_approval.unwrap_or(AppToolApproval::Auto), - ..Default::default() - }; - }; - - let app = connector_id.and_then(|connector_id| apps_config.apps.get(connector_id)); - let tools = app.and_then(|app| app.tools.as_ref()); - let tool_config = tools.and_then(|tools| { - tools - .tools - .get(tool_name) - .or_else(|| tool_title.and_then(|title| tools.tools.get(title))) - }); - let approval = managed_approval - .or_else(|| tool_config.and_then(|tool| tool.approval_mode)) - .or_else(|| app.and_then(|app| app.default_tools_approval_mode)) - .unwrap_or(AppToolApproval::Auto); - - if !app_is_enabled(apps_config, connector_id) { - return AppToolPolicy { - enabled: false, - approval, - }; - } - - if let Some(enabled) = tool_config.and_then(|tool| tool.enabled) { - return AppToolPolicy { enabled, approval }; - } - - if let Some(enabled) = app.and_then(|app| app.default_tools_enabled) { - return AppToolPolicy { enabled, approval }; - } - - let app_defaults = apps_config.default.as_ref(); - let destructive_enabled = app - .and_then(|app| app.destructive_enabled) - .unwrap_or_else(|| { - app_defaults - .map(|defaults| defaults.destructive_enabled) - .unwrap_or(true) - }); - let open_world_enabled = app - .and_then(|app| app.open_world_enabled) - .unwrap_or_else(|| { - app_defaults - .map(|defaults| defaults.open_world_enabled) - .unwrap_or(true) - }); - let destructive_hint = annotations - .and_then(|annotations| annotations.destructive_hint) - .unwrap_or(true); - let open_world_hint = annotations - .and_then(|annotations| annotations.open_world_hint) - .unwrap_or(true); - let enabled = - (destructive_enabled || !destructive_hint) && (open_world_enabled || !open_world_hint); - - AppToolPolicy { enabled, approval } -} - #[cfg(test)] #[path = "connectors_tests.rs"] mod tests; diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 337e69c8d5fd..4e6cb55aaa36 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -2,18 +2,12 @@ use super::*; use crate::config::CONFIG_TOML_FILE; use crate::config::ConfigBuilder; use codex_config::AppRequirementToml; -use codex_config::AppToolRequirementToml; -use codex_config::AppToolsRequirementsToml; use codex_config::AppsRequirementsToml; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::test_support::CloudConfigBundleFixture; -use codex_config::types::AppConfig; -use codex_config::types::AppToolConfig; -use codex_config::types::AppToolsConfig; use codex_config::types::ApprovalsReviewer; -use codex_config::types::AppsDefaultConfig; use codex_connectors::merge::plugin_connector_to_app_info; use codex_connectors::metadata::connector_install_url; use codex_connectors::metadata::sanitize_name; @@ -21,26 +15,14 @@ use codex_features::Feature; use codex_login::CodexAuth; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo; -use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use rmcp::model::JsonObject; use rmcp::model::Tool; use std::collections::BTreeMap; -use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; use tempfile::tempdir; -fn annotations(destructive_hint: Option, open_world_hint: Option) -> ToolAnnotations { - ToolAnnotations::from_raw( - /*title*/ None, - /*read_only_hint*/ None, - destructive_hint, - /*idempotent_hint*/ None, - open_world_hint, - ) -} - fn app(id: &str) -> AppInfo { AppInfo { id: id.to_string(), @@ -264,139 +246,6 @@ fn accessible_connectors_from_mcp_tools_preserves_description() { ); } -#[test] -fn app_tool_policy_uses_global_defaults_for_destructive_hints() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: false, - open_world_enabled: true, - }), - apps: HashMap::new(), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/create", - /*tool_title*/ None, - Some(&annotations(Some(true), /*open_world_hint*/ None)), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_defaults_missing_destructive_hint_to_true() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: false, - open_world_enabled: true, - }), - apps: HashMap::new(), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/create", - /*tool_title*/ None, - Some(&annotations(/*destructive_hint*/ None, Some(false))), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_defaults_missing_open_world_hint_to_true() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: true, - open_world_enabled: false, - }), - apps: HashMap::new(), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/create", - /*tool_title*/ None, - Some(&annotations(Some(false), /*open_world_hint*/ None)), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_is_enabled_uses_default_for_unconfigured_apps() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: false, - approvals_reviewer: None, - destructive_enabled: true, - open_world_enabled: true, - }), - apps: HashMap::new(), - }; - - assert!(!app_is_enabled(&apps_config, Some("calendar"))); - assert!(!app_is_enabled(&apps_config, /*connector_id*/ None)); -} - -#[test] -fn app_is_enabled_prefers_per_app_override_over_default() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: false, - approvals_reviewer: None, - destructive_enabled: true, - open_world_enabled: true, - }), - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: None, - open_world_enabled: None, - default_tools_approval_mode: None, - default_tools_enabled: None, - tools: None, - }, - )]), - }; - - assert!(app_is_enabled(&apps_config, Some("calendar"))); - assert!(!app_is_enabled(&apps_config, Some("drive"))); -} - #[tokio::test] async fn app_approvals_reviewer_uses_app_then_default_then_global() { for (global, app_default, app, expected_global, expected_default, expected_app) in [ @@ -522,247 +371,6 @@ approvals_reviewer = "user" ); } -#[test] -fn requirements_disabled_connector_overrides_enabled_connector() { - let mut effective_apps = AppsConfigToml { - default: None, - apps: HashMap::from([( - "connector_123123".to_string(), - AppConfig { - enabled: true, - ..Default::default() - }, - )]), - }; - let requirements_apps = AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_123123".to_string(), - AppRequirementToml { - enabled: Some(false), - tools: None, - }, - )]), - }; - - apply_requirements_apps_constraints(&mut effective_apps, Some(&requirements_apps)); - - assert_eq!( - effective_apps - .apps - .get("connector_123123") - .map(|app| app.enabled), - Some(false) - ); -} - -#[test] -fn requirements_enabled_does_not_override_disabled_connector() { - let mut effective_apps = AppsConfigToml { - default: None, - apps: HashMap::from([( - "connector_123123".to_string(), - AppConfig { - enabled: false, - ..Default::default() - }, - )]), - }; - let requirements_apps = AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_123123".to_string(), - AppRequirementToml { - enabled: Some(true), - tools: None, - }, - )]), - }; - - apply_requirements_apps_constraints(&mut effective_apps, Some(&requirements_apps)); - - assert_eq!( - effective_apps - .apps - .get("connector_123123") - .map(|app| app.enabled), - Some(false) - ); -} - -#[tokio::test] -async fn cloud_config_bundle_disable_connector_overrides_user_apps_config() { - let codex_home = tempdir().expect("tempdir should succeed"); - std::fs::write( - codex_home.path().join(CONFIG_TOML_FILE), - r#" -[apps.connector_123123] -enabled = true -"#, - ) - .expect("write config"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .cloud_config_bundle( - CloudConfigBundleFixture::loader_with_enterprise_requirement( - r#" -[apps.connector_123123] -enabled = false -"#, - ), - ) - .build() - .await - .expect("config should build"); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "events.list", - /*tool_title*/ None, - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[tokio::test] -async fn cloud_config_bundle_disable_connector_applies_without_user_apps_table() { - let codex_home = tempdir().expect("tempdir should succeed"); - std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), "").expect("write config"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .cloud_config_bundle( - CloudConfigBundleFixture::loader_with_enterprise_requirement( - r#" -[apps.connector_123123] -enabled = false -"#, - ), - ) - .build() - .await - .expect("config should build"); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "events.list", - /*tool_title*/ None, - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[tokio::test] -async fn local_requirements_disable_connector_overrides_user_apps_config() { - let codex_home = tempdir().expect("tempdir should succeed"); - let config_toml_path = - AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE)).expect("abs path"); - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .build() - .await - .expect("config should build"); - - let requirements = ConfigRequirementsToml { - apps: Some(AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_123123".to_string(), - AppRequirementToml { - enabled: Some(false), - tools: None, - }, - )]), - }), - ..Default::default() - }; - config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack") - .with_user_config( - &config_toml_path, - toml::from_str::( - r#" -[apps.connector_123123] -enabled = true -"#, - ) - .expect("apps config"), - ); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "events.list", - /*tool_title*/ None, - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[tokio::test] -async fn local_requirements_disable_connector_applies_without_user_apps_table() { - let codex_home = tempdir().expect("tempdir should succeed"); - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .build() - .await - .expect("config should build"); - - let requirements = ConfigRequirementsToml { - apps: Some(AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_123123".to_string(), - AppRequirementToml { - enabled: Some(false), - tools: None, - }, - )]), - }), - ..Default::default() - }; - config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack"); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "events.list", - /*tool_title*/ None, - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - #[tokio::test] async fn with_app_enabled_state_preserves_unrelated_disabled_connector() { let codex_home = tempdir().expect("tempdir should succeed"); @@ -801,483 +409,6 @@ async fn with_app_enabled_state_preserves_unrelated_disabled_connector() { ); } -#[test] -fn app_tool_policy_honors_default_app_enabled_false() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: false, - approvals_reviewer: None, - destructive_enabled: true, - open_world_enabled: true, - }), - apps: HashMap::new(), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/list", - /*tool_title*/ None, - Some(&annotations( - /*destructive_hint*/ None, /*open_world_hint*/ None, - )), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_uses_managed_approval_without_apps_config() { - let policy = app_tool_policy_from_apps_config( - /*apps_config*/ None, - Some("calendar"), - "events/list", - /*tool_title*/ None, - /*annotations*/ None, - Some(AppToolApproval::Approve), - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Approve, - } - ); -} - -fn app_tool_requirements( - app_id: &str, - tool_name: &str, - approval_mode: AppToolApproval, -) -> AppsRequirementsToml { - AppsRequirementsToml { - apps: BTreeMap::from([( - app_id.to_string(), - AppRequirementToml { - enabled: None, - tools: Some(AppToolsRequirementsToml { - tools: BTreeMap::from([( - tool_name.to_string(), - AppToolRequirementToml { - approval_mode: Some(approval_mode), - }, - )]), - }), - }, - )]), - } -} - -#[test] -fn managed_app_tool_approval_uses_raw_tool_name() { - let requirements_apps = app_tool_requirements( - "connector_123123", - "calendar/list_events", - AppToolApproval::Approve, - ); - - assert_eq!( - managed_app_tool_approval( - Some(&requirements_apps), - Some("connector_123123"), - "calendar/list_events", - ), - Some(AppToolApproval::Approve) - ); - assert_eq!( - managed_app_tool_approval( - Some(&requirements_apps), - Some("connector_123123"), - "calendar/create_event", - ), - None - ); -} - -#[tokio::test] -async fn cloud_config_bundle_tool_approval_overrides_user_apps_config() { - let codex_home = tempdir().expect("tempdir should succeed"); - std::fs::write( - codex_home.path().join(CONFIG_TOML_FILE), - r#" -[apps.connector_123123.tools."calendar/list_events"] -approval_mode = "prompt" -"#, - ) - .expect("write config"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .cloud_config_bundle( - CloudConfigBundleFixture::loader_with_enterprise_requirement( - r#" -[apps.connector_123123.tools."calendar/list_events"] -approval_mode = "approve" -"#, - ), - ) - .build() - .await - .expect("config should build"); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "calendar/list_events", - /*tool_title*/ None, - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Approve, - } - ); -} - -#[tokio::test] -async fn local_requirements_tool_approval_overrides_user_apps_config() { - let codex_home = tempdir().expect("tempdir should succeed"); - let config_toml_path = - AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE)).expect("abs path"); - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .build() - .await - .expect("config should build"); - - let requirements = ConfigRequirementsToml { - apps: Some(app_tool_requirements( - "connector_123123", - "calendar/list_events", - AppToolApproval::Approve, - )), - ..Default::default() - }; - config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack") - .with_user_config( - &config_toml_path, - toml::from_str::( - r#" -[apps.connector_123123.tools."calendar/list_events"] -approval_mode = "prompt" -"#, - ) - .expect("apps config"), - ); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "calendar/list_events", - /*tool_title*/ None, - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Approve, - } - ); -} - -#[tokio::test] -async fn local_requirements_tool_approval_does_not_match_tool_title() { - let codex_home = tempdir().expect("tempdir should succeed"); - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .build() - .await - .expect("config should build"); - - let requirements = ConfigRequirementsToml { - apps: Some(app_tool_requirements( - "connector_123123", - "calendar/list_events", - AppToolApproval::Approve, - )), - ..Default::default() - }; - config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack"); - - let policy = app_tool_policy( - &config, - Some("connector_123123"), - "calendar/create_event", - Some("calendar/list_events"), - /*annotations*/ None, - ); - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_allows_per_app_enable_when_default_is_disabled() { - let apps_config = AppsConfigToml { - default: Some(AppsDefaultConfig { - enabled: false, - approvals_reviewer: None, - destructive_enabled: true, - open_world_enabled: true, - }), - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: None, - open_world_enabled: None, - default_tools_approval_mode: None, - default_tools_enabled: None, - tools: None, - }, - )]), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/list", - /*tool_title*/ None, - Some(&annotations( - /*destructive_hint*/ None, /*open_world_hint*/ None, - )), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_per_tool_enabled_true_overrides_app_level_disable_flags() { - let apps_config = AppsConfigToml { - default: None, - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: Some(false), - open_world_enabled: Some(false), - default_tools_approval_mode: None, - default_tools_enabled: None, - tools: Some(AppToolsConfig { - tools: HashMap::from([( - "events/create".to_string(), - AppToolConfig { - enabled: Some(true), - approval_mode: None, - }, - )]), - }), - }, - )]), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/create", - /*tool_title*/ None, - Some(&annotations(Some(true), Some(true))), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_default_tools_enabled_true_overrides_app_level_tool_hints() { - let apps_config = AppsConfigToml { - default: None, - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: Some(false), - open_world_enabled: Some(false), - default_tools_approval_mode: None, - default_tools_enabled: Some(true), - tools: None, - }, - )]), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/create", - /*tool_title*/ None, - Some(&annotations(Some(true), Some(true))), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Auto, - } - ); -} - -#[test] -fn app_tool_policy_default_tools_enabled_false_overrides_app_level_tool_hints() { - let apps_config = AppsConfigToml { - default: None, - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: Some(true), - open_world_enabled: Some(true), - default_tools_approval_mode: Some(AppToolApproval::Approve), - default_tools_enabled: Some(false), - tools: None, - }, - )]), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/list", - /*tool_title*/ None, - Some(&annotations( - /*destructive_hint*/ None, /*open_world_hint*/ None, - )), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: false, - approval: AppToolApproval::Approve, - } - ); -} - -#[test] -fn app_tool_policy_uses_default_tools_approval_mode() { - let apps_config = AppsConfigToml { - default: None, - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: None, - open_world_enabled: None, - default_tools_approval_mode: Some(AppToolApproval::Prompt), - default_tools_enabled: None, - tools: Some(AppToolsConfig { - tools: HashMap::new(), - }), - }, - )]), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "events/list", - /*tool_title*/ None, - Some(&annotations( - /*destructive_hint*/ None, /*open_world_hint*/ None, - )), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Prompt, - } - ); -} - -#[test] -fn app_tool_policy_matches_prefix_stripped_tool_name_for_tool_config() { - let apps_config = AppsConfigToml { - default: None, - apps: HashMap::from([( - "calendar".to_string(), - AppConfig { - enabled: true, - approvals_reviewer: None, - destructive_enabled: Some(false), - open_world_enabled: Some(false), - default_tools_approval_mode: Some(AppToolApproval::Auto), - default_tools_enabled: Some(false), - tools: Some(AppToolsConfig { - tools: HashMap::from([( - "events/create".to_string(), - AppToolConfig { - enabled: Some(true), - approval_mode: Some(AppToolApproval::Approve), - }, - )]), - }), - }, - )]), - }; - - let policy = app_tool_policy_from_apps_config( - Some(&apps_config), - Some("calendar"), - "calendar_events/create", - Some("events/create"), - Some(&annotations(Some(true), Some(true))), - /*managed_approval*/ None, - ); - - assert_eq!( - policy, - AppToolPolicy { - enabled: true, - approval: AppToolApproval::Approve, - } - ); -} - #[tokio::test] async fn tool_suggest_connector_ids_include_configured_tool_suggest_discoverables() { let codex_home = tempdir().expect("tempdir should succeed"); diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index e3f7d5faf5d1..9b2e99332eff 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -33,6 +33,9 @@ use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_config::types::AppToolApproval; use codex_config::types::ApprovalsReviewer; +use codex_connectors::AppToolPolicy; +use codex_connectors::AppToolPolicyEvaluator; +use codex_connectors::AppToolPolicyInput; use codex_features::Feature; use codex_hooks::PermissionRequestDecision; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; @@ -148,21 +151,24 @@ pub(crate) async fn handle_mcp_tool_call( .and_then(|metadata| metadata.plugin_id.clone()), }; let app_tool_policy = if server == CODEX_APPS_MCP_SERVER_NAME { - connectors::app_tool_policy( - &turn_context.config, - metadata - .as_ref() - .and_then(|metadata| metadata.connector_id.as_deref()), - &tool_name, - metadata - .as_ref() - .and_then(|metadata| metadata.tool_title.as_deref()), - metadata - .as_ref() - .and_then(|metadata| metadata.annotations.as_ref()), + let annotations = metadata + .as_ref() + .and_then(|metadata| metadata.annotations.as_ref()); + AppToolPolicyEvaluator::new(&turn_context.config.config_layer_stack).policy( + AppToolPolicyInput { + connector_id: metadata + .as_ref() + .and_then(|metadata| metadata.connector_id.as_deref()), + tool_name: &tool_name, + tool_title: metadata + .as_ref() + .and_then(|metadata| metadata.tool_title.as_deref()), + destructive_hint: annotations.and_then(|annotations| annotations.destructive_hint), + open_world_hint: annotations.and_then(|annotations| annotations.open_world_hint), + }, ) } else { - connectors::AppToolPolicy::default() + AppToolPolicy::default() }; let approval_mode = if server == CODEX_APPS_MCP_SERVER_NAME { app_tool_policy.approval diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs index d430851570ff..d62880615f4a 100644 --- a/codex-rs/core/src/mcp_tool_exposure.rs +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -1,5 +1,7 @@ use std::collections::HashSet; +use codex_connectors::AppToolPolicyEvaluator; +use codex_connectors::AppToolPolicyInput; use codex_features::Feature; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo as McpToolInfo; @@ -70,6 +72,7 @@ fn filter_codex_apps_mcp_tools( .iter() .map(|connector| connector.id.as_str()) .collect(); + let app_tool_policy = AppToolPolicyEvaluator::new(&config.config_layer_stack); mcp_tools .iter() @@ -83,7 +86,19 @@ fn filter_codex_apps_mcp_tools( let Some(connector_id) = tool.connector_id.as_deref() else { return false; }; - allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool) + let annotations = tool.tool.annotations.as_ref(); + allowed.contains(connector_id) + && app_tool_policy + .policy(AppToolPolicyInput { + connector_id: Some(connector_id), + tool_name: &tool.tool.name, + tool_title: tool.tool.title.as_deref(), + destructive_hint: annotations + .and_then(|annotations| annotations.destructive_hint), + open_world_hint: annotations + .and_then(|annotations| annotations.open_world_hint), + }) + .enabled }) .cloned() .collect() diff --git a/codex-rs/core/src/mcp_tool_exposure_test.rs b/codex-rs/core/src/mcp_tool_exposure_test.rs index 4918f768aa68..63a2e33f3121 100644 --- a/codex-rs/core/src/mcp_tool_exposure_test.rs +++ b/codex-rs/core/src/mcp_tool_exposure_test.rs @@ -11,8 +11,11 @@ use rmcp::model::Meta; use rmcp::model::Tool; use super::*; +use crate::config::CONFIG_TOML_FILE; +use crate::config::ConfigBuilder; use crate::config::test_config; use crate::connectors::AppInfo; +use tempfile::tempdir; fn make_connector(id: &str, name: &str) -> AppInfo { AppInfo { @@ -182,6 +185,57 @@ async fn excludes_tools_hidden_from_model_exposure() { assert!(exposure.deferred_tools.is_none()); } +#[tokio::test] +async fn applies_per_tool_app_policy_across_the_exposure_build() { + let codex_home = tempdir().expect("tempdir should succeed"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +[apps.calendar] +default_tools_enabled = false + +[apps.calendar.tools."events/create"] +enabled = true +"#, + ) + .expect("write config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should build"); + let enabled_tool = make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "events/create", + "mcp__codex_apps__calendar", + "create", + Some("calendar"), + Some("Calendar"), + ); + let disabled_tool = make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "events/list", + "mcp__codex_apps__calendar", + "list", + Some("calendar"), + Some("Calendar"), + ); + let connectors = vec![make_connector("calendar", "Calendar")]; + + let exposure = build_mcp_tool_exposure( + &[enabled_tool.clone(), disabled_tool], + Some(connectors.as_slice()), + &config, + /*search_tool_enabled*/ false, + ); + + assert_eq!( + tool_names(&exposure.direct_tools), + tool_names(&[enabled_tool]) + ); + assert!(exposure.deferred_tools.is_none()); +} + #[tokio::test] async fn searches_large_effective_tool_sets() { let config = test_config().await;