mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Decouple tool restrictions from session identity with ToolPolicy (#46999)
## Why Tool allowlists alone cannot express the reviewer's sandbox and shell restrictions, which previously depended on session identity. Threads need to supply these restrictions explicitly at startup. ## What changed - Replace `AllowedTools` with `ToolPolicy`, combining an optional allowlist with managed-sandbox requirements, unified-exec requirements, and control over additional-permission arguments. - Capture the policy once at startup and apply it during tool registration and construction, including hosted tool filtering. Later extension-state changes cannot relax it. - Supply the reviewer policy explicitly while retaining the fallback for reviewer sessions without a supplied policy. Explicit policies take precedence over session identity; ordinary defaults preserve existing tool setup. ## Testing Update tool-planning tests to use explicit policies and add startup coverage for additional-permission schemas, ordinary defaults, reviewer fallbacks, explicit-policy precedence, and policy capture despite later extension-state changes. GitOrigin-RevId: 71e561d76ea24d961f5b29c7c619bdb1356c216a
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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<Arc<codex_extension_api::AllowedTools>>,
|
||||
pub(crate) tool_policy: Arc<codex_extension_api::ToolPolicy>,
|
||||
pub(crate) windows_sandbox_proxy_settings_mode:
|
||||
codex_sandboxing::WindowsSandboxProxySettingsMode,
|
||||
pub(super) multi_agent_version: OnceLock<MultiAgentVersion>,
|
||||
@@ -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::<codex_extension_api::AllowedTools>()
|
||||
.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::<codex_extension_api::ToolPolicy>()
|
||||
.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(),
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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<ToolName, RegisteredTool>,
|
||||
first_collision: Option<ToolName>,
|
||||
pub(crate) allowed_tools: Option<Arc<AllowedTools>>,
|
||||
pub(crate) tool_policy: Arc<ToolPolicy>,
|
||||
}
|
||||
|
||||
impl ToolRegistry {
|
||||
pub(crate) fn with_allowed_tools(allowed_tools: Option<Arc<AllowedTools>>) -> Self {
|
||||
pub(crate) fn with_tool_policy(tool_policy: Arc<ToolPolicy>) -> 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<dyn CoreToolRuntime>) {
|
||||
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()
|
||||
|
||||
@@ -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::<crate::WaitForEnvironmentToolConfig>();
|
||||
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<ToolSpec>,
|
||||
tool_search_handler_cache: &ToolSearchHandlerCache,
|
||||
) -> CodexResult<ToolRouter> {
|
||||
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,
|
||||
|
||||
@@ -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::<String>::new());
|
||||
assert_eq!(plan.visible_specs, Vec::<ToolSpec>::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
|
||||
|
||||
@@ -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<codex_protocol::protocol::SessionSource>,
|
||||
policy: Option<ToolPolicy>,
|
||||
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(())
|
||||
}
|
||||
|
||||
@@ -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<ToolName>);
|
||||
|
||||
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()))
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Vec<ToolName>>,
|
||||
/// 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()))
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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<C>(pub fn(&C) -> anyhow::Result<C>);
|
||||
|
||||
/// 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.
|
||||
|
||||
@@ -104,7 +104,7 @@ impl ThreadLifecycleContributor<Config> 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();
|
||||
|
||||
Reference in New Issue
Block a user