diff --git a/codex-rs/core/src/guardian/test_host.rs b/codex-rs/core/src/guardian/test_host.rs index 75c42343b3..655f4e150a 100644 --- a/codex-rs/core/src/guardian/test_host.rs +++ b/codex-rs/core/src/guardian/test_host.rs @@ -73,7 +73,7 @@ pub(crate) fn install(session: &Session, config: &Config) { .insert(SessionIsolation::Isolated); options .thread_extension_init - .insert(codex_guardian_reviewer::reviewer_allowed_tools()); + .insert(codex_guardian_reviewer::reviewer_tool_policy()); let session_cancel = cancel.clone(); let until = async move { let _cancel_on_exit = cancel.clone().drop_guard(); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index ff2be9c566..92c1481375 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -72,7 +72,7 @@ pub(crate) struct Session { pub(super) features: ManagedFeatures, pub(crate) guardian_context_mode: GuardianContextMode, pub(super) isolation: codex_extension_api::SessionIsolation, - pub(crate) allowed_tools: Option>, + pub(crate) tool_policy: Arc, pub(crate) windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode, pub(super) multi_agent_version: OnceLock, @@ -959,12 +959,15 @@ impl Session { // Publish the already resolved model before extensions make startup decisions. // Turn construction refreshes this attachment when the selected model changes. thread_extension_init.insert(model_info); - let allowed_tools = thread_extension_init - .get::() - .or_else(|| { - // Older reviewer rollouts predate the explicit startup setting. - crate::guardian::is_basic_session_source(&session_configuration.session_source) - .then(|| Arc::new(codex_guardian_reviewer::reviewer_allowed_tools())) + let tool_policy = thread_extension_init + .get::() + .unwrap_or_else(|| { + // Older reviewer rollouts predate the explicit startup policy. + if crate::guardian::is_basic_session_source(&session_configuration.session_source) { + Arc::new(codex_guardian_reviewer::reviewer_tool_policy()) + } else { + Arc::default() + } }); let mcp_thread_init = thread_extension_init.clone(); let thread_extension_data = codex_extension_api::ExtensionData::new_with_init( @@ -1740,7 +1743,7 @@ impl Session { features: config.features.clone(), guardian_context_mode, isolation, - allowed_tools, + tool_policy, windows_sandbox_proxy_settings_mode, multi_agent_version, mcp_refresh: McpRefresh::new(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 5370f5c24d..fc1df5abf7 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -6260,7 +6260,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { features: config.features.clone(), guardian_context_mode: GuardianContextMode::from_features(&config.features), isolation: codex_extension_api::SessionIsolation::Inherit, - allowed_tools: None, + tool_policy: Arc::default(), windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile, multi_agent_version: OnceLock::from(config.multi_agent_version_from_features()), @@ -8518,7 +8518,7 @@ where features: config.features.clone(), guardian_context_mode: GuardianContextMode::from_features(&config.features), isolation: codex_extension_api::SessionIsolation::Inherit, - allowed_tools: None, + tool_policy: Arc::default(), windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile, multi_agent_version: OnceLock::from(config.multi_agent_version_from_features()), diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 101844f9ab..95d54e34c4 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -27,8 +27,8 @@ use crate::tools::router::tool_log_payload; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use crate::util::error_or_panic; use codex_analytics::ControlToolCallStatus; -use codex_extension_api::AllowedTools; use codex_extension_api::ToolCallOutcome; +use codex_extension_api::ToolPolicy; use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::models::FunctionCallOutputPayload; @@ -292,13 +292,13 @@ pub(crate) struct RegisteredTool { pub struct ToolRegistry { tools: IndexMap, first_collision: Option, - pub(crate) allowed_tools: Option>, + pub(crate) tool_policy: Arc, } impl ToolRegistry { - pub(crate) fn with_allowed_tools(allowed_tools: Option>) -> Self { + pub(crate) fn with_tool_policy(tool_policy: Arc) -> Self { Self { - allowed_tools, + tool_policy, ..Self::default() } } @@ -339,11 +339,7 @@ impl ToolRegistry { exposure: ToolExposure, ) { let tool_name = runtime.tool_name().with_default_namespace(); - if self - .allowed_tools - .as_ref() - .is_some_and(|allowed| !allowed.contains(&tool_name)) - { + if !self.tool_policy.allows(&tool_name) { return; } match self.tools.entry(tool_name) { @@ -359,11 +355,7 @@ impl ToolRegistry { pub(crate) fn prepend_trusted(&mut self, runtime: Arc) { let tool_name = runtime.tool_name().with_default_namespace(); - if self - .allowed_tools - .as_ref() - .is_some_and(|allowed| !allowed.contains(&tool_name)) - { + if !self.tool_policy.allows(&tool_name) { return; } if self.tools.contains_key(&tool_name) { @@ -387,11 +379,7 @@ impl ToolRegistry { exposure: ToolExposure, ) -> bool { let tool_name = runtime.tool_name().with_default_namespace(); - if self - .allowed_tools - .as_ref() - .is_some_and(|allowed| !allowed.contains(&tool_name)) - { + if !self.tool_policy.allows(&tool_name) { return false; } if tool_name.is_default_namespace() diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 90cfdf5c69..f5c88e3b92 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -106,6 +106,7 @@ const IMAGEGEN_TOOL_NAME: &str = "imagegen"; #[derive(Clone, Copy)] struct CoreToolPlanContext<'a> { + tool_policy: &'a codex_extension_api::ToolPolicy, turn_context: &'a TurnContext, model_info: &'a ModelInfo, environments: &'a TurnEnvironmentSnapshot, @@ -135,6 +136,7 @@ pub(crate) fn build_tool_router( .thread_extension_data .get::(); let context = CoreToolPlanContext { + tool_policy: &session.tool_policy, turn_context, model_info, environments, @@ -144,7 +146,7 @@ pub(crate) fn build_tool_router( default_agent_type_description: &default_agent_type_description, wait_agent_timeouts: wait_agent_timeout_options(turn_context), }; - let mut registry = ToolRegistry::with_allowed_tools(session.allowed_tools.clone()); + let mut registry = ToolRegistry::with_tool_policy(Arc::clone(&session.tool_policy)); add_core_tool_sources(&context, &mut registry); let registered_mcp_tools = session.services.mcp_handler_cache.append_mcp_tools( @@ -279,6 +281,7 @@ pub(crate) fn build_core_tool_registry( let default_agent_type_description = crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); let context = CoreToolPlanContext { + tool_policy: &Default::default(), turn_context, model_info, environments, @@ -349,9 +352,7 @@ pub(crate) fn finalize_tool_router( mut hosted_specs: Vec, tool_search_handler_cache: &ToolSearchHandlerCache, ) -> CodexResult { - if let Some(allowed) = ®istry.allowed_tools { - hosted_specs.retain(|spec| allowed.contains(&ToolName::plain(spec.name()))); - } + hosted_specs.retain(|spec| registry.tool_policy.allows(&ToolName::plain(spec.name()))); apply_direct_model_only_namespace_overrides(turn_context, &mut registry); let tool_mode = effective_tool_mode(turn_context, model_info); let code_mode_enabled = matches!(tool_mode, ToolMode::CodeMode | ToolMode::CodeModeOnly); @@ -966,9 +967,8 @@ fn code_mode_namespace_descriptions( #[instrument(level = "trace", skip_all)] fn add_core_tool_sources(context: &CoreToolPlanContext<'_>, registry: &mut ToolRegistry) { - // Preserve the reviewer's existing sandbox requirements. Tool selection is - // supplied separately by the extension through AllowedTools. - if crate::guardian::is_basic_session_source(&context.turn_context.session_source) + // The startup ceiling applies to the thread and every selected environment. + if context.tool_policy.require_managed_sandbox && (!matches!( context.turn_context.permission_profile(), PermissionProfile::Managed { .. } @@ -1041,12 +1041,11 @@ fn add_shell_tools(context: &CoreToolPlanContext<'_>, registry: &mut ToolRegistr } let allow_login_shell = any_environment_allows_login_shell(context.environments); - let is_guardian = crate::guardian::is_basic_session_source(&turn_context.session_source); - if is_guardian && !features.enabled(Feature::UnifiedExec) { + if context.tool_policy.require_unified_exec && !features.enabled(Feature::UnifiedExec) { return; } - let exec_permission_approvals_enabled = - features.enabled(Feature::ExecPermissionApprovals) && !is_guardian; + let exec_permission_approvals_enabled = features.enabled(Feature::ExecPermissionApprovals) + && context.tool_policy.expose_additional_permissions; let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); let options = ExecCommandHandlerOptions { allow_login_shell, diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index b50228703f..1518558409 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -24,7 +24,6 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ToolMode; use codex_protocol::openai_models::WebSearchToolType; use codex_protocol::protocol::EnvironmentConfigState; -use codex_protocol::protocol::InternalSessionSource; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -517,16 +516,16 @@ fn apply_patch_accepts_environment_id(spec: &ToolSpec) -> bool { #[tokio::test] async fn allowed_tools_filter_sources_before_code_mode_and_discovery() { use crate::tools::registry::ToolRegistry; - use codex_extension_api::AllowedTools; + use codex_extension_api::ToolPolicy; for allowed in [ None, - Some(AllowedTools(vec![ + Some(vec![ ToolName::namespaced("kept", "lookup"), ToolName::plain("exec"), ToolName::plain("wait"), - ])), - Some(AllowedTools::default()), + ]), + Some(Vec::new()), ] { let (_, mut turn) = make_session_and_context().await; set_feature(&mut turn, Feature::CodeMode, /*enabled*/ true); @@ -536,7 +535,10 @@ async fn allowed_tools_filter_sources_before_code_mode_and_discovery() { model.supports_search_tool = true; model.use_responses_lite = false; }); - let mut registry = ToolRegistry::with_allowed_tools(allowed.clone().map(Arc::new)); + let mut registry = ToolRegistry::with_tool_policy(Arc::new(ToolPolicy { + allowed_tools: allowed.clone(), + ..Default::default() + })); registry.add(crate::tools::handlers::PlanHandler); let hosted = append_source_tools( &turn, @@ -570,7 +572,7 @@ async fn allowed_tools_filter_sources_before_code_mode_and_discovery() { plan.assert_registered_contains(&["update_plan", "extension_echo", "dynamic_echo"]); plan.assert_visible_contains(&["web_search", "tool_search", "exec", "wait"]); } - Some(allowed) if allowed.0.is_empty() => { + Some(allowed) if allowed.is_empty() => { assert_eq!(plan.registered_names, Vec::::new()); assert_eq!(plan.visible_specs, Vec::::new()); assert_eq!(plan.code_mode_tool_names, BTreeMap::new()); @@ -597,10 +599,9 @@ async fn allowed_tools_filter_sources_before_code_mode_and_discovery() { } #[tokio::test] -async fn internal_guardian_sessions_exclude_optional_core_tools() { +async fn reviewer_tool_policy_exclude_optional_core_tools() { let (mut session, mut turn) = make_session_and_context().await; - turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); - session.allowed_tools = Some(Arc::new(codex_guardian_reviewer::reviewer_allowed_tools())); + session.tool_policy = Arc::new(codex_guardian_reviewer::reviewer_tool_policy()); set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); Arc::make_mut(&mut turn.config).update_plan_enabled = true; turn.multi_agent_version = MultiAgentVersion::V2; @@ -630,15 +631,14 @@ async fn internal_guardian_sessions_exclude_optional_core_tools() { } #[tokio::test] -async fn internal_guardian_sessions_respect_managed_shell_restrictions() { +async fn reviewer_tool_policy_respect_managed_shell_restrictions() { for (disabled_feature, shell_type) in [ (Some(Feature::ShellTool), ConfigShellToolType::UnifiedExec), (Some(Feature::UnifiedExec), ConfigShellToolType::UnifiedExec), (None, ConfigShellToolType::Disabled), ] { let (mut session, mut turn) = make_session_and_context().await; - turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); - session.allowed_tools = Some(Arc::new(codex_guardian_reviewer::reviewer_allowed_tools())); + session.tool_policy = Arc::new(codex_guardian_reviewer::reviewer_tool_policy()); set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); set_feature(&mut turn, Feature::CodeMode, /*enabled*/ true); if let Some(feature) = disabled_feature { @@ -689,10 +689,9 @@ async fn internal_guardian_sessions_respect_managed_shell_restrictions() { } #[tokio::test] -async fn internal_guardian_sessions_preserve_code_mode() { +async fn reviewer_tool_policy_preserve_code_mode() { let (mut session, mut turn) = make_session_and_context().await; - turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); - session.allowed_tools = Some(Arc::new(codex_guardian_reviewer::reviewer_allowed_tools())); + session.tool_policy = Arc::new(codex_guardian_reviewer::reviewer_tool_policy()); set_feature(&mut turn, Feature::CodeMode, /*enabled*/ true); let turn = Arc::new(turn); let step_context = StepContext::for_test(Arc::clone(&turn)); @@ -724,7 +723,7 @@ async fn internal_guardian_sessions_preserve_code_mode() { } #[tokio::test] -async fn internal_guardian_sessions_require_managed_secondary_environments() { +async fn reviewer_tool_policy_require_managed_secondary_environments() { for (secondary_profile, expected_tools) in [ ( codex_protocol::models::PermissionProfile::workspace_write(), @@ -736,8 +735,7 @@ async fn internal_guardian_sessions_require_managed_secondary_environments() { ), ] { let (mut session, mut turn) = make_session_and_context().await; - turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); - session.allowed_tools = Some(Arc::new(codex_guardian_reviewer::reviewer_allowed_tools())); + session.tool_policy = Arc::new(codex_guardian_reviewer::reviewer_tool_policy()); set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); let TurnEnvironmentState::Ready(primary) = turn .initial_environments diff --git a/codex-rs/core/tests/suite/managed_threads_tests.rs b/codex-rs/core/tests/suite/managed_threads_tests.rs index af24716213..72ece73831 100644 --- a/codex-rs/core/tests/suite/managed_threads_tests.rs +++ b/codex-rs/core/tests/suite/managed_threads_tests.rs @@ -9,9 +9,9 @@ use std::time::Duration; use anyhow::Context; use codex_core::StartThreadOptions; use codex_core::TurnInputRequest; -use codex_extension_api::AllowedTools; use codex_extension_api::SessionIsolation; use codex_extension_api::ToolName; +use codex_extension_api::ToolPolicy; use codex_protocol::protocol::EventMsg; use codex_protocol::user_input::UserInput; use codex_thread_store::ThreadStoreError; @@ -270,9 +270,10 @@ async fn startup_allowlist_controls_advertising_and_execution( options .thread_extension_init .insert(SessionIsolation::Isolated); - options - .thread_extension_init - .insert(AllowedTools(tools.clone())); + options.thread_extension_init.insert(ToolPolicy { + allowed_tools: Some(tools.clone()), + ..Default::default() + }); let cancelled = CancellationToken::new(); let tasks = TaskTracker::new(); let agent = fixture @@ -280,10 +281,10 @@ async fn startup_allowlist_controls_advertising_and_execution( .start_thread_until(options, cancelled.clone().cancelled_owned(), &tasks) .await?; // Replacing extension state after startup must not change the captured ceiling. - agent - .thread - .thread_extension_data() - .insert(AllowedTools(vec![ToolName::plain("exec_command")])); + agent.thread.thread_extension_data().insert(ToolPolicy { + allowed_tools: Some(vec![ToolName::plain("exec_command")]), + ..Default::default() + }); let response = responses::mount_sse_sequence( &server, vec![ @@ -347,3 +348,98 @@ async fn startup_allowlist_controls_advertising_and_execution( fixture.codex.shutdown_and_wait().await?; Ok(()) } + +#[test_case(None, Some(codex_guardian_reviewer::reviewer_tool_policy()), false; "explicit_policy")] +#[test_case(None, None, true; "ordinary_defaults")] +#[test_case(Some(codex_protocol::protocol::SessionSource::Internal( + codex_protocol::protocol::InternalSessionSource::Guardian)), None, false; "internal_guardian_fallback")] +#[test_case(Some(codex_protocol::protocol::SessionSource::SubAgent( + codex_protocol::protocol::SubAgentSource::Other("guardian".to_owned()))), None, false; "legacy_guardian_fallback")] +#[test_case(Some(codex_protocol::protocol::SessionSource::Internal( + codex_protocol::protocol::InternalSessionSource::Guardian)), Some(ToolPolicy::default()), true; "explicit_policy_overrides_identity")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn startup_tool_policy_controls_additional_permission_parameters( + source: Option, + policy: Option, + expose_additional_permissions: bool, +) -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + let server = responses::start_mock_server().await; + let fixture = test_codex() + .with_config(|config| { + config + .features + .disable(codex_features::Feature::CodeMode) + .expect("disable Code Mode for direct-tool assertions"); + config + .features + .enable(codex_features::Feature::ExecPermissionApprovals) + .expect("enable additional-permission schema for startup assertions"); + }) + .build_with_auto_env(&server) + .await?; + let mut options = StartThreadOptions::new(fixture.config.clone()); + options.environments = Some(fixture.codex.environment_selections().await); + options + .thread_extension_init + .insert(SessionIsolation::Isolated); + if let Some(policy) = policy { + options.thread_extension_init.insert(policy); + } + options.session_source = source; + let cancelled = CancellationToken::new(); + let tasks = TaskTracker::new(); + let agent = fixture + .thread_manager + .start_thread_until(options, cancelled.clone().cancelled_owned(), &tasks) + .await?; + // The policy is captured at startup, not read back from mutable extension state. + agent + .thread + .thread_extension_data() + .insert(ToolPolicy::default()); + let response = responses::mount_sse_once( + &server, + responses::sse(vec![responses::ev_completed("done")]), + ) + .await; + agent + .thread + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Inspect available tools".to_owned(), + text_elements: Vec::new(), + }])) + .await?; + wait_for_event(&agent.thread, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + let body = response.single_request().body_json(); + let properties = &body["tools"] + .as_array() + .expect("tool list") + .iter() + .find(|tool| tool["name"] == "exec_command") + .expect("shell tool")["parameters"]["properties"]; + assert_eq!( + properties.get("additional_permissions").is_some(), + expose_additional_permissions + ); + assert_eq!( + properties["sandbox_permissions"]["enum"], + if expose_additional_permissions { + json!([ + "use_default", + "with_additional_permissions", + "require_escalated" + ]) + } else { + json!(["use_default", "require_escalated"]) + } + ); + cancelled.cancel(); + tasks.close(); + tasks.wait().await; + fixture.codex.shutdown_and_wait().await?; + Ok(()) +} diff --git a/codex-rs/ext/extension-api/src/allowed_tools.rs b/codex-rs/ext/extension-api/src/allowed_tools.rs deleted file mode 100644 index 25abab2244..0000000000 --- a/codex-rs/ext/extension-api/src/allowed_tools.rs +++ /dev/null @@ -1,24 +0,0 @@ -//! A startup ceiling on the tools a thread may advertise or execute. - -use crate::ToolName; - -/// Supply through `ExtensionDataInit` before starting a thread. The host captures -/// this value once; changing extension state later cannot widen the tool set. -/// Callers must supply it again when resuming a thread. -/// -/// An absent value keeps ordinary tool setup. An empty list permits no tools. -/// Names include their namespace; a plain name uses the default namespace. -/// This only removes tools: configuration and permission checks still apply. -/// Generated tools such as Code Mode's `exec` and `wait` must also be listed. -#[derive(Clone, Debug, Default)] -pub struct AllowedTools(pub Vec); - -impl AllowedTools { - pub fn contains(&self, tool: &ToolName) -> bool { - self.0.iter().any(|allowed| { - allowed.name == tool.name - && (allowed.namespace == tool.namespace - || (allowed.is_default_namespace() && tool.is_default_namespace())) - }) - } -} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index 9cda9f62b2..44faae9472 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -1,14 +1,14 @@ -mod allowed_tools; mod capabilities; mod contributors; mod registry; mod session_isolation; mod state; +mod tool_policy; mod turn_admission; mod user_instructions; -pub use allowed_tools::AllowedTools; pub use session_isolation::SessionIsolation; +pub use tool_policy::ToolPolicy; pub use capabilities::ConversationHistorySnapshot; pub use capabilities::ExtensionEventSink; diff --git a/codex-rs/ext/extension-api/src/tool_policy.rs b/codex-rs/ext/extension-api/src/tool_policy.rs new file mode 100644 index 0000000000..71824bd42c --- /dev/null +++ b/codex-rs/ext/extension-api/src/tool_policy.rs @@ -0,0 +1,44 @@ +//! A startup ceiling on tool selection and construction, independent of session identity. +//! This policy only restricts tools; permission and approval checks still apply. + +use crate::ToolName; + +/// Supply through `ExtensionDataInit` before starting a thread. The host captures +/// the policy once; later extension-state changes cannot relax it. Callers must +/// supply it again when resuming a thread. +#[derive(Clone, Debug)] +pub struct ToolPolicy { + /// `None` keeps ordinary tool selection; an empty list permits no tools. + /// Names include their namespace; plain names use the default namespace. + /// Generated tools such as Code Mode's `exec` and `wait` must also be listed. + pub allowed_tools: Option>, + /// Omit core tools unless the thread and every ready environment use a managed sandbox. + pub require_managed_sandbox: bool, + /// Omit shell tools when unified exec is disabled, instead of using one-shot exec. + pub require_unified_exec: bool, + /// Advertise additional-permission arguments when the feature is enabled. + pub expose_additional_permissions: bool, +} + +impl Default for ToolPolicy { + fn default() -> Self { + Self { + allowed_tools: None, + require_managed_sandbox: false, + require_unified_exec: false, + expose_additional_permissions: true, + } + } +} + +impl ToolPolicy { + pub fn allows(&self, tool: &ToolName) -> bool { + self.allowed_tools.as_ref().is_none_or(|tools| { + tools.iter().any(|allowed| { + allowed.name == tool.name + && (allowed.namespace == tool.namespace + || (allowed.is_default_namespace() && tool.is_default_namespace())) + }) + }) + } +} diff --git a/codex-rs/ext/guardian-reviewer/src/lib.rs b/codex-rs/ext/guardian-reviewer/src/lib.rs index 6feff41201..29ce8edf5a 100644 --- a/codex-rs/ext/guardian-reviewer/src/lib.rs +++ b/codex-rs/ext/guardian-reviewer/src/lib.rs @@ -59,8 +59,8 @@ pub use execution::start_review_turn; pub use execution::wait_for_guardian_review; pub use settings::ReviewerConfig; pub use settings::ReviewerTurn; -pub use settings::reviewer_allowed_tools; pub use settings::reviewer_permission_profile; +pub use settings::reviewer_tool_policy; pub use feedback::FailedReviewFeedback; pub use feedback::ReviewFeedbackContext; diff --git a/codex-rs/ext/guardian-reviewer/src/settings.rs b/codex-rs/ext/guardian-reviewer/src/settings.rs index b26e6fa36f..619bf15eb0 100644 --- a/codex-rs/ext/guardian-reviewer/src/settings.rs +++ b/codex-rs/ext/guardian-reviewer/src/settings.rs @@ -3,8 +3,8 @@ use std::collections::HashMap; -use codex_extension_api::AllowedTools; use codex_extension_api::ToolName; +use codex_extension_api::ToolPolicy; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; @@ -27,15 +27,19 @@ use serde_json::Value; /// Keeping it in thread data avoids caching settings that can change between reviews. pub struct ReviewerConfig(pub fn(&C) -> anyhow::Result); -/// Reviewer tools, including the existing Code Mode dispatchers when enabled. -/// The host still applies feature flags and sandbox restrictions to these tools. -pub fn reviewer_allowed_tools() -> AllowedTools { - AllowedTools( - ["exec_command", "write_stdin", "view_image", "exec", "wait"] - .into_iter() - .map(ToolName::plain) - .collect(), - ) +/// Reviewer tool selection and construction restrictions, enforced by core. +pub fn reviewer_tool_policy() -> ToolPolicy { + ToolPolicy { + allowed_tools: Some( + ["exec_command", "write_stdin", "view_image", "exec", "wait"] + .into_iter() + .map(ToolName::plain) + .collect(), + ), + require_managed_sandbox: true, + require_unified_exec: true, + expose_additional_permissions: false, + } } /// Applies the same read-only ceiling to the reviewer and each inherited environment. diff --git a/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs b/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs index f1bf7772c8..d69bde20cd 100644 --- a/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs +++ b/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs @@ -104,7 +104,7 @@ impl ThreadLifecycleContributor for GuardianExtension { .insert(SessionIsolation::Isolated); options .thread_extension_init - .insert(codex_guardian_reviewer::reviewer_allowed_tools()); + .insert(codex_guardian_reviewer::reviewer_tool_policy()); let session_cancel = cancel.clone(); let until = async move { let _cancel_on_exit = cancel.clone().drop_guard();