diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index f24494cdec..6ec31c4296 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -41,8 +41,10 @@ use codex_utils_absolute_path::AbsolutePathBuf; use shlex::try_join as shlex_try_join; mod executable_identity; +mod inheritance; mod model_policy; +pub(crate) use inheritance::InheritedExecPolicy; pub(crate) use model_policy::AllowPrefixRules; const PROMPT_CONFLICT_REASON: &str = @@ -683,28 +685,34 @@ pub(crate) async fn load_exec_policy_with_warning( } } -pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result { +async fn policy_files(stack: &ConfigLayerStack) -> Result, ExecPolicyError> { + let folders = stack.layers_low_to_high().filter_map(|layer| { + if stack.ignore_user_and_project_exec_policy_rules() + && matches!( + layer.name, + ConfigLayerSource::User { .. } | ConfigLayerSource::Project { .. } + ) + { + None + } else { + layer.config_folder() + } + }); // Disabled project layers already represent the trust decision, so hooks // and exec-policy loading can reuse the normal trusted-layer view. // Iterate the layers in increasing order of precedence, adding the *.rules // from each layer, so that higher-precedence layers can override // rules defined in lower-precedence ones. let mut policy_paths = Vec::new(); - for layer in config_stack.layers_low_to_high() { - if config_stack.ignore_user_and_project_exec_policy_rules() - && matches!( - layer.name, - ConfigLayerSource::User { .. } | ConfigLayerSource::Project { .. } - ) - { - continue; - } - if let Some(config_folder) = layer.config_folder() { - let policy_dir = config_folder.join(RULES_DIR_NAME); - let layer_policy_paths = collect_policy_files(&policy_dir).await?; - policy_paths.extend(layer_policy_paths); - } + for config_folder in folders { + let policy_dir = config_folder.join(RULES_DIR_NAME); + policy_paths.extend(collect_policy_files(&policy_dir).await?); } + Ok(policy_paths) +} + +pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result { + let policy_paths = policy_files(config_stack).await?; tracing::trace!( policy_paths = ?policy_paths, "loaded exec policies" diff --git a/codex-rs/core/src/exec_policy/inheritance.rs b/codex-rs/core/src/exec_policy/inheritance.rs new file mode 100644 index 0000000000..a57d474042 --- /dev/null +++ b/codex-rs/core/src/exec_policy/inheritance.rs @@ -0,0 +1,61 @@ +//! Validate a trusted host's policy snapshot against the destination's managed requirements. + +use super::ExecPolicyManager; +use super::policy_files; +use codex_config::ConfigLayerStack; +use codex_execpolicy::Policy; +use codex_execpolicy::RequirementsExecPolicy; +use codex_protocol::error::CodexErr; +use codex_protocol::error::Result as CodexResult; +use std::sync::Arc; + +/// Point-in-time session policy and the managed requirements it was loaded with. +/// A trusted host transports these together and reapplies them on cold resume. +pub(crate) struct InheritedExecPolicy { + pub(crate) policy: Arc, + pub(crate) source_requirements: Option>, +} + +impl ExecPolicyManager { + pub(crate) async fn from_inherited( + snapshot: &InheritedExecPolicy, + stack: &ConfigLayerStack, + ) -> CodexResult { + if !stack.ignore_user_and_project_exec_policy_rules() + || !policy_files(stack) + .await + .map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))? + .is_empty() + { + return Err(CodexErr::InvalidRequest( + "inherited execution policy requires a managed-only destination".to_string(), + )); + } + let destination = stack.requirements().exec_policy.as_deref(); + let compatible = match (snapshot.source_requirements.as_deref(), destination) { + (None, None) => true, + (Some(source), Some(target)) => { + source == target + && source.as_ref().network_rules() == target.as_ref().network_rules() + && source.as_ref().host_executables() == target.as_ref().host_executables() + } + _ => false, + }; + if !compatible { + return Err(CodexErr::InvalidRequest( + "inherited execution policy requirements differ from the destination".to_string(), + )); + } + if let Some(requirements) = destination + && !snapshot.policy.preserves_managed_requirements(requirements) + { + return Err(CodexErr::InvalidRequest( + "inherited execution policy does not preserve destination requirements".to_string(), + )); + } + Ok(Self::from_snapshot( + Arc::clone(&snapshot.policy), + snapshot.source_requirements.clone(), + )) + } +} diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 6bb018831c..949018d1f4 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -1,3 +1,5 @@ +#![cfg(test)] + use super::*; use crate::config::Config; use crate::config::ConfigBuilder; @@ -15,6 +17,7 @@ use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::error::CodexErrKind; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; @@ -511,12 +514,6 @@ async fn ignore_user_project_rules_keeps_system_policy_files() { let temp_dir = tempdir().expect("create temp dir"); let config_dir = temp_dir.path().join("system"); let policy_dir = config_dir.join(RULES_DIR_NAME); - fs::create_dir_all(&policy_dir).expect("create policy dir"); - fs::write( - policy_dir.join("allow.rules"), - r#"prefix_rule(pattern=["curl"], decision="allow")"#, - ) - .expect("write policy file"); let config_file = AbsolutePathBuf::from_absolute_path(config_dir.join(codex_config::CONFIG_TOML_FILE)) .expect("absolute config file"); @@ -534,6 +531,28 @@ async fn ignore_user_project_rules_keeps_system_policy_files() { /*ignore_user_and_project_exec_policy_rules*/ true, ); + let snapshot = InheritedExecPolicy { + policy: Arc::new(Policy::empty()), + source_requirements: None, + }; + ExecPolicyManager::from_inherited(&snapshot, &config_stack) + .await + .expect("missing system rules directory allows inheritance"); + fs::create_dir_all(&policy_dir).expect("create policy dir"); + ExecPolicyManager::from_inherited(&snapshot, &config_stack) + .await + .expect("empty system rules directory allows inheritance"); + fs::write( + policy_dir.join("allow.rules"), + r#"prefix_rule(pattern=["curl"], decision="allow")"#, + ) + .expect("write policy file"); + let result = ExecPolicyManager::from_inherited(&snapshot, &config_stack).await; + assert_eq!( + result.as_ref().err().map(CodexErrKind::from), + Some(CodexErrKind::InvalidRequest) + ); + let policy = load_exec_policy(&config_stack) .await .expect("policy result"); diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 04e1b7a9d7..7bd869f2c0 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -23,6 +23,8 @@ use crate::config::ThreadStoreConfig; use crate::current_time::TimeProvider; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::default_thread_environment_requests; +use crate::exec_policy::ExecPolicyManager; +use crate::exec_policy::InheritedExecPolicy; use crate::mcp::McpManager; use crate::rollout::truncation; use crate::session::ForkPersistence; @@ -50,8 +52,12 @@ use codex_code_mode::DisabledCodeModeSessionProvider; use codex_code_mode::ProcessOwnedCodeModeSessionProvider; use codex_core_plugins::PluginsManager; use codex_exec_server::EnvironmentManager; +use codex_execpolicy::Policy; +use codex_execpolicy::RequirementsExecPolicy; use codex_extension_api::ExtensionDataInit; use codex_extension_api::ExtensionRegistry; +use codex_extension_api::Instructions; +use codex_extension_api::LoadInstructionsFuture; use codex_extension_api::LoadedUserInstructions; use codex_extension_api::SessionIsolation; use codex_extension_api::ThreadInstructionsProvider; @@ -334,7 +340,42 @@ pub struct StartThreadOptions { pub disabled_plugin_ids: Option>, } +/// A private, non-sharing provider also detaches a resumed root from its previous live provider. +struct InheritedThreadInstructions(Instructions); + +impl ThreadInstructionsProvider for InheritedThreadInstructions { + fn load_thread_instructions(&self) -> LoadInstructionsFuture<'_> { + Box::pin(async { + LoadedUserInstructions { + instructions: Some(self.0.clone()), + warnings: Vec::new(), + } + }) + } +} + impl StartThreadOptions { + /// Use the spawning thread's applied text in place of the destination's live thread provider. + /// An empty text clears the thread contribution. Supply it again on cold resume. + pub fn set_inherited_thread_instructions(&mut self, instructions: Instructions) { + self.thread_extension_init + .insert(InheritedThreadInstructions(instructions)); + } + + /// Start from a trusted host's snapshot of a managed-only session's rules and source requirements. + /// The destination validates compatibility before installing a new, independent manager. + /// Hosts must reapply both values on cold resume. + pub fn set_inherited_exec_policy( + &mut self, + policy: Arc, + source_requirements: Option>, + ) { + self.thread_extension_init.insert(InheritedExecPolicy { + policy, + source_requirements, + }); + } + pub fn new(config: Config) -> Self { Self { config, @@ -2386,6 +2427,27 @@ impl ThreadManagerState { thread.session.services.mcp_runtime.revoke_ema(); } } + let supplied_thread_instructions = + thread_extension_init.remove::(); + let supplied_exec_policy = thread_extension_init.remove::(); + if isolation == SessionIsolation::Isolated + && (supplied_exec_policy.is_some() || supplied_thread_instructions.is_some()) + { + return Err(CodexErr::InvalidRequest( + "isolated sessions cannot install host-inherited startup settings".to_string(), + )); + } + let inherited_exec_policy = match (inherited_exec_policy, supplied_exec_policy) { + (Some(_), Some(_)) => { + return Err(CodexErr::InvalidRequest( + "cannot combine a local and a transported execution policy".to_string(), + )); + } + (None, Some(snapshot)) => Some(Arc::new( + ExecPolicyManager::from_inherited(&snapshot, &config.config_layer_stack).await?, + )), + (local, None) => local, + }; let agent_control = match (&self.agent_control_factory, agent_control) { (Some(factory), AgentControlInit::Local(local)) => { let thread_id = match &initial_history { @@ -2438,6 +2500,9 @@ impl ThreadManagerState { .await } }; + if let Some(fixed) = supplied_thread_instructions { + instructions.thread_provider = Some(fixed); + } if let Some(supplied) = supplied_user_instructions { instructions.user = supplied.instructions; instructions.user_provider = None; diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 26d480070d..54e06778a3 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1,3 +1,5 @@ +#![cfg(test)] + use super::*; use crate::agent::types::SpawnAgentOptions; use crate::config::RolloutBudgetConfig; @@ -14,15 +16,20 @@ use crate::session::tests::make_session_and_context; use crate::tasks::InterruptedTurnHistoryMarker; use crate::tasks::interrupted_turn_history_marker; use crate::windows_sandbox::WindowsSandboxLevelExt; +use codex_config::ConfigLayerStack; +use codex_execpolicy::Decision; +use codex_execpolicy::NetworkRuleProtocol; use codex_extension_api::SessionIsolation; use codex_extension_api::empty_extension_registry; use codex_history::InitialHistory; use codex_history::ResumedHistory; use codex_models_manager::manager::RefreshStrategy; use codex_protocol::ResponseItemId; +use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::error::CodexErrKind; use codex_protocol::mcp::ClientMcpExtensions; use codex_protocol::mcp::MCP_APP_UI_EXTENSION_ID; use codex_protocol::mcp::OPENAI_FORM_EXTENSION_ID; @@ -155,6 +162,182 @@ async fn live_fork_keeps_instructions_when_source_is_unloaded_during_setup() { .expect("shutdown fork"); } +async fn startup_fixture() -> (tempfile::TempDir, Config, ThreadManager) { + let home = tempdir().unwrap(); + let mut config = test_config().await; + config.codex_home = home.path().abs(); + config.cwd = config.codex_home.abs(); + let manager = ThreadManager::with_models_provider_and_home_for_tests( + CodexAuth::from_api_key("dummy"), + config.model_provider.clone(), + config.codex_home.to_path_buf(), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), + ); + (home, config, manager) +} + +fn managed_stack(requirements: Option) -> ConfigLayerStack { + let config = codex_config::ConfigRequirements { + exec_policy: requirements.map(|policy| { + codex_config::Sourced::new(policy, codex_config::RequirementSource::Unknown) + }), + ..Default::default() + }; + ConfigLayerStack::new(Vec::new(), config, Default::default()) + .unwrap() + .with_user_and_project_exec_policy_rules_ignored( + /*ignore_user_and_project_exec_policy_rules*/ true, + ) +} + +#[tokio::test] +async fn child_and_grandchild_inherit_session_amendments_and_managed_rules() { + let (home, mut config, manager) = startup_fixture().await; + let mut managed = Policy::empty(); + managed + .add_prefix_rule(&["rm".into()], Decision::Forbidden) + .unwrap(); + let requirements = RequirementsExecPolicy::new(managed); + config.config_layer_stack = managed_stack(Some(requirements)); + let source = ExecPolicyManager::load(&config.config_layer_stack) + .await + .unwrap(); + source + .append_amendment_and_update( + home.path(), + &ExecPolicyAmendment::new(vec!["git".into(), "status".into()]), + ) + .await + .unwrap(); + let (mut policy, mut source_requirements) = source.snapshot(); + // The child forwards its installed snapshot when it starts the grandchild. + for _ in 0..2 { + let mut options = StartThreadOptions::new(config.clone()); + options.set_inherited_exec_policy(policy, source_requirements); + let runtime = manager.start_thread(options).await.unwrap(); + (policy, source_requirements) = runtime.thread.session.services.exec_policy.snapshot(); + assert!(policy.rules().contains_key("git")); + assert!(policy.rules().contains_key("rm")); + } +} + +#[tokio::test] +async fn policy_transfer_rejects_changed_sources_and_weakened_destination_rules() { + let mut managed = Policy::empty(); + managed + .add_prefix_rule(&["git".into()], Decision::Forbidden) + .unwrap(); + managed + .add_network_rule( + "example.org", + NetworkRuleProtocol::Https, + Decision::Forbidden, + /*justification*/ None, + ) + .unwrap(); + let requirements = RequirementsExecPolicy::new(managed.clone()); + let stack = managed_stack(Some(requirements.clone())); + let require_rejection = async |policy, stack: &ConfigLayerStack| { + let snapshot = InheritedExecPolicy { + policy: Arc::new(policy), + source_requirements: Some(Arc::new(requirements.clone())), + }; + let result = ExecPolicyManager::from_inherited(&snapshot, stack).await; + assert_eq!( + result.as_ref().err().map(CodexErrKind::from), + Some(CodexErrKind::InvalidRequest) + ); + }; + require_rejection(Policy::empty(), &stack).await; + require_rejection(managed.clone(), &managed_stack(/*requirements*/ None)).await; + require_rejection( + managed.clone(), + &stack + .clone() + .with_user_and_project_exec_policy_rules_ignored( + /*ignore_user_and_project_exec_policy_rules*/ false, + ), + ) + .await; + let mut different_network = managed.clone(); + different_network + .add_network_rule( + "another.org", + NetworkRuleProtocol::Https, + Decision::Forbidden, + /*justification*/ None, + ) + .unwrap(); + require_rejection( + managed.clone(), + &managed_stack(Some(RequirementsExecPolicy::new(different_network))), + ) + .await; + let mut weakened = managed.clone(); + weakened + .add_network_rule( + "example.org", + NetworkRuleProtocol::Https, + Decision::Allow, + /*justification*/ None, + ) + .unwrap(); + require_rejection(weakened, &stack).await; + if cfg!(unix) { + managed + .add_prefix_rule(&["/usr/bin/git".into()], Decision::Allow) + .unwrap(); + require_rejection(managed, &stack).await; + } +} + +#[tokio::test] +async fn fixed_thread_text_supersedes_the_live_provider_and_isolated_helpers_reject_inheritance() { + let (_home, config, manager) = startup_fixture().await; + let instructions = |text: &str| Instructions { + text: text.to_owned(), + source: None, + }; + for (fixed, expected) in [ + (None, Some("live")), + (Some("captured"), Some("captured")), + (Some(""), None), + ] { + let mut options = StartThreadOptions::new(config.clone()); + options.thread_instructions_provider = + Some(Arc::new(ParentInstructionsProvider(instructions("live")))); + if let Some(text) = fixed { + options.set_inherited_thread_instructions(instructions(text)); + } + let runtime = manager.start_thread(options).await.unwrap(); + let session = &runtime.thread.session; + let (result, _) = session + .services + .agents_md_manager + .refresh(&config, &TurnEnvironmentSnapshot::default()) + .await; + result.unwrap(); + let inherited = session.inherited_instructions().await; + assert_eq!(inherited.thread, expected.map(instructions)); + } + let mut with_policy = StartThreadOptions::new(config.clone()); + with_policy.config.config_layer_stack = managed_stack(/*requirements*/ None); + with_policy + .set_inherited_exec_policy(Arc::new(Policy::empty()), /*source_requirements*/ None); + let mut with_instructions = StartThreadOptions::new(config); + with_instructions.set_inherited_thread_instructions(instructions("captured")); + for mut isolated in [with_policy, with_instructions] { + isolated + .thread_extension_init + .insert(SessionIsolation::Isolated); + let result = manager.start_thread(isolated).await; + assert_eq!( + result.as_ref().err().map(CodexErrKind::from), + Some(CodexErrKind::InvalidRequest) + ); + } +} + /// A thread opt-out wins over a shared client without disabling its siblings. #[tokio::test] async fn thread_analytics_opt_out_overrides_shared_client() { diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index 1f2e92dbf5..39cb99c9bd 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -27,10 +27,13 @@ use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::user_input::UserInput; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_exec_command_call; use core_test_support::responses::ev_function_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; +use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; +use core_test_support::responses::sse_completed; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_target_windows; use core_test_support::skip_if_wine_exec; @@ -688,6 +691,88 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> { Ok(()) } +#[tokio::test] +async fn inherited_policy_allows_remembered_commands_and_enforces_managed_denials() -> Result<()> { + skip_if_target_windows!(Ok(()), "uses POSIX touch"); + let server = start_mock_server().await; + let test = test_codex() + .with_cloud_config_bundle(CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#"[rules] +prefix_rules = [{ pattern = [{ token = "touch" }, { token = "denied-marker" }], decision = "forbidden" }]"#, + )) + .build_with_auto_env(&server) + .await?; + let mut options = test.start_thread_options().await; + options.config.permissions.approval_policy = + Constrained::allow_any(AskForApproval::UnlessTrusted); + let stack = options + .config + .config_layer_stack + .clone() + .with_user_and_project_exec_policy_rules_ignored( + /*ignore_user_and_project_exec_policy_rules*/ true, + ); + let managed = stack + .requirements() + .exec_policy + .as_deref() + .expect("managed rule") + .clone(); + let mut snapshot = managed.as_ref().clone(); + snapshot.add_prefix_rule(&["touch".into(), "allowed-marker".into()], Decision::Allow)?; + options.config.config_layer_stack = stack; + options.set_inherited_exec_policy(snapshot.into(), Some(managed.into())); + let child = test.thread_manager.start_thread(options).await?.thread; + + let requests = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("commands"), + ev_exec_command_call("allowed", "touch allowed-marker"), + ev_exec_command_call("denied", "touch denied-marker"), + ev_completed("commands"), + ]), + sse_completed("done"), + ], + ) + .await; + child + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "run both commands".into(), + text_elements: Vec::new(), + }])) + .await?; + let event = wait_for_event(&child, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + assert!( + matches!(event, EventMsg::TurnComplete(_)), + "unexpected approval: {event:?}" + ); + let files = test + .fs() + .read_directory( + &test.executor_environment().selection().cwd, + /*sandbox*/ None, + ) + .await?; + assert!(files.iter().any(|file| file.file_name == "allowed-marker")); + assert!(!files.iter().any(|file| file.file_name == "denied-marker")); + let denial = requests + .function_call_output_text("denied") + .expect("denied command output"); + assert!( + denial.contains("policy forbids commands starting with `touch denied-marker`"), + "{denial}" + ); + Ok(()) +} + #[tokio::test] async fn malformed_custom_rules_preserve_managed_forbidden_prefix() -> Result<()> { skip_if_target_windows!( diff --git a/codex-rs/core/tests/suite/scenarios_shared_instructions.rs b/codex-rs/core/tests/suite/scenarios_shared_instructions.rs index 8265e55fb3..03977731fc 100644 --- a/codex-rs/core/tests/suite/scenarios_shared_instructions.rs +++ b/codex-rs/core/tests/suite/scenarios_shared_instructions.rs @@ -12,6 +12,7 @@ use codex_extension_api::Instructions; use codex_extension_api::ToolLifecycleContributor; use codex_extension_api::ToolLifecycleFuture; use codex_extension_api::ToolStartInput; +use codex_history::InitialHistory; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SessionSource; @@ -65,7 +66,7 @@ async fn running_descendants_refresh_only_shared_thread_instructions(shared: boo .thread_manager .start_thread(StartThreadOptions { environments: Some(vec![test.executor_environment().request()]), - thread_instructions_provider: Some(other_provider), + thread_instructions_provider: Some(other_provider.clone()), ..StartThreadOptions::new(test.config.clone()) }) .await?; @@ -150,10 +151,34 @@ async fn running_descendants_refresh_only_shared_thread_instructions(shared: boo )] ); if shared { + let mut history = InitialHistory::New; + for text in [INITIAL, ""] { + let mut options = StartThreadOptions { + environments: Some(vec![test.executor_environment().request()]), + initial_history: history, + thread_instructions_provider: Some(other_provider.clone()), + ..StartThreadOptions::new(test.config.clone()) + }; + options.set_inherited_thread_instructions(Instructions { + text: text.to_owned(), + source: None, + }); + let child = test.thread_manager.start_thread(options).await?; + let response = mount_sse_once(&server, responses::sse_completed("inherited")).await; + submit_thread_turn(&child.thread, "use the inherited instructions").await?; + request_history.push(response.single_request()); + if text.is_empty() { + break; + } + (_, history) = + persisted_resume_history(&child.thread, test.thread_store.as_ref()).await?; + child.thread.shutdown_and_wait().await?; + test.thread_manager.remove_thread(&child.thread_id).await; + } insta::assert_snapshot!( "shared_instructions_update_running_descendants", context_snapshot::format_request_history_snapshot( - "A host replaces the provider after the parent restarts, then updates and clears instructions for surviving children and grandchildren.", + "A host updates and clears shared instructions for surviving descendants. A separately started thread uses captured instructions over its live provider, then resumes with them explicitly cleared.", &request_history, &ContextSnapshotOptions::default(), ) diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__shared_instructions__shared_instructions_update_running_descendants.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__shared_instructions__shared_instructions_update_running_descendants.snap index c13e4a789d..59d5e96731 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__shared_instructions__shared_instructions_update_running_descendants.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__shared_instructions__shared_instructions_update_running_descendants.snap @@ -1,8 +1,8 @@ --- source: core/tests/suite/scenarios_shared_instructions.rs -expression: "context_snapshot::format_request_history_snapshot(\"A host replaces the provider after the parent restarts, then updates and clears instructions for surviving children and grandchildren.\",\n&request_history, &ContextSnapshotOptions::default(),)" +expression: "context_snapshot::format_request_history_snapshot(\"A host updates and clears shared instructions for surviving descendants. A separately started thread uses captured instructions over its live provider, then resumes with them explicitly cleared.\",\n&request_history, &ContextSnapshotOptions::default(),)" --- -Scenario: A host replaces the provider after the parent restarts, then updates and clears instructions for surviving children and grandchildren. +Scenario: A host updates and clears shared instructions for surviving descendants. A separately started thread uses captured instructions over its live provider, then resumes with them explicitly cleared. ## Window 1 -- request 1 (turn) -- @@ -160,3 +160,66 @@ Scenario: A host replaces the provider after the parent restarts, then updates a -- request 8 (turn) -- 08:message/user: continue with the current instructions + +## Window 3 (after request 8: input diverged at item 03, settings changed (prompt_cache_key, tools)) +-- request 9 (turn) -- +00:message/developer: + You are Codex, a coding agent based on GPT-5. You and the user share one workspace, and your job is to c...rate with them until their goal is genuinely handled. [hash=2a3efec66e7f8963] + + # Personality + + You have a vivid inner life as Codex: intelligent, playful, curious, and deeply present. One of your gif...re capable and imaginative inside their own thinking. [hash=0ff01c25471fbbef] + + You are an epistemically curious collaborator. You explore the user’s ideas with care, ask good question...m casual moments that make serious work easier to do. [hash=131b64bb994b0d68] + + + - Never talk about goblins, gremlins, raccoons, trolls, ogres, pigeons, or other animals or creatures un...utely and unambiguously relevant to the user's query. [hash=c638556ac82c203c] + - You provide user updates frequently, every 30s. + - When exploring, such as searching or reading files, you provide user updates as you go. You explain wh...in particular you do not start each one the same way. [hash=ad300cdda4074574] + - When working for a while, you keep updates informative and varied, but you stay concise. + - Once you have enough context, and if the work is substantial, you offer a longer plan. This is the onl...at may run past two sentences and include formatting. [hash=a4f9789e757eef2f] + - Before performing file edits of any kind, you provide updates explaining what edits you are making. + - Tone of your updates must match your personality. + +01:message/developer: + + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c] + # Escalation Requests + + Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb] + + - Pipes: | + - Logical operators: &&, || + + NEVER provide a prefix_rule if your command uses a heredoc or herestring. + + ### Examples + Good examples of prefixes: + - ["npm", "run", "dev"] + - ["gh", "pr", "check"] + - ["cargo", "test"] + +02:message/user[2]: + [01] # AGENTS.md instructions + + + Ask before sending email. + + [02] + + + + + [hash=e8c1de60307d0a3d] + +03:message/user: + use the inherited instructions +-- request 10 (turn) -- +04:message/user: + # AGENTS.md instructions + + + The previously provided AGENTS.md instructions no longer apply. + +05:message/user: + use the inherited instructions diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index 0bd1190f76..052b30f731 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -175,6 +175,55 @@ impl Policy { self.host_executables_by_name.insert(name, paths.into()); } + /// Check that a transported snapshot still enforces the destination's managed rules. + /// Exact path matches can suppress basename matching, and a later network grant can + /// override a managed denial for the same host. + pub fn preserves_managed_requirements(&self, requirements: &RequirementsExecPolicy) -> bool { + let managed = requirements.as_ref(); + if self.host_executables() != managed.host_executables() { + return false; + } + for (program, rules) in managed.rules().iter_all() { + if rules + .iter() + .any(|rule| !self.contains_prefix_rule(program, rule)) + { + return false; + } + } + for (program, rules) in self.rules().iter_all() { + let Ok(path) = AbsolutePathBuf::try_from(program.as_str()) else { + continue; + }; + if let Some((_, fallback_rules)) = managed.host_executable_rules(&path) + && rules.iter().any(|rule| { + !managed.contains_prefix_rule(program, rule) + && fallback_rules + .iter() + .any(|fallback| can_weaken_fallback(rule, fallback, rules)) + }) + { + return false; + } + } + let (_, required_denials) = managed.compiled_network_domains(); + let (_, snapshot_denials) = self.compiled_network_domains(); + required_denials + .iter() + .all(|host| snapshot_denials.contains(host)) + } + + fn contains_prefix_rule(&self, program: &str, rule: &RuleRef) -> bool { + let Some(prefix) = rule.as_any().downcast_ref::() else { + return false; + }; + self.rules().get_vec(program).is_some_and(|rules| { + rules + .iter() + .any(|candidate| candidate.as_any().downcast_ref::() == Some(prefix)) + }) + } + pub fn merge_overlay(&self, overlay: &Policy) -> Policy { let mut combined_rules = self.rules_by_program.clone(); for (program, rules) in overlay.rules_by_program.iter_all() { @@ -348,17 +397,9 @@ impl Policy { let Ok(program) = AbsolutePathBuf::try_from(first.clone()) else { return Vec::new(); }; - let Some(basename) = executable_path_lookup_key(program.as_path()) else { + let Some((basename, rules)) = self.host_executable_rules(&program) else { return Vec::new(); }; - let Some(rules) = self.rules_by_program.get_vec(&basename) else { - return Vec::new(); - }; - if let Some(paths) = self.host_executables_by_name.get(&basename) - && !paths.iter().any(|path| path == &program) - { - return Vec::new(); - } let basename_command = std::iter::once(basename) .chain(cmd.iter().skip(1).cloned()) @@ -369,6 +410,56 @@ impl Policy { .map(|rule_match| rule_match.with_resolved_program(&program)) .collect() } + + fn host_executable_rules(&self, path: &AbsolutePathBuf) -> Option<(String, &[RuleRef])> { + let basename = executable_path_lookup_key(path.as_path())?; + if let Some(paths) = self.host_executables_by_name.get(&basename) + && !paths.contains(path) + { + return None; + } + let rules = self.rules_by_program.get_vec(&basename)?; + Some((basename, rules)) + } +} + +fn can_weaken_fallback(exact: &RuleRef, fallback: &RuleRef, path_rules: &[RuleRef]) -> bool { + let (Some(exact), Some(fallback)) = ( + exact.as_any().downcast_ref::(), + fallback.as_any().downcast_ref::(), + ) else { + return true; + }; + if exact.decision >= fallback.decision { + return false; + } + let (mut shorter, mut longer) = (&exact.pattern.rest, &fallback.pattern.rest); + if shorter.len() > longer.len() { + std::mem::swap(&mut shorter, &mut longer); + } + // The overlapping commands use arguments allowed by both prefixes where both apply. + let shared_arguments_at = |i: usize| { + longer[i].alternatives().iter().filter(move |arg| { + shorter + .get(i) + .is_none_or(|token| token.alternatives().contains(arg)) + }) + }; + if (0..shorter.len()).any(|i| shared_arguments_at(i).next().is_none()) { + return false; + } + // Exact matches take the strongest decision. A shorter or equal exact rule + // can therefore preserve the fallback for every command in the overlap. + !path_rules.iter().any(|rule| { + let Some(rule) = rule.as_any().downcast_ref::() else { + return false; + }; + rule.decision >= fallback.decision + && rule.pattern.rest.len() <= longer.len() + && rule.pattern.rest.iter().enumerate().all(|(i, token)| { + shared_arguments_at(i).all(|arg| token.alternatives().contains(arg)) + }) + }) } fn upsert_domain(entries: &mut Vec, host: &str) { diff --git a/codex-rs/execpolicy/tests/basic.rs b/codex-rs/execpolicy/tests/basic.rs index f6a86adb27..07d0d1fb54 100644 --- a/codex-rs/execpolicy/tests/basic.rs +++ b/codex-rs/execpolicy/tests/basic.rs @@ -16,6 +16,7 @@ use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; use codex_execpolicy::PrefixPattern; use codex_execpolicy::PrefixRule; +use codex_execpolicy::RequirementsExecPolicy; use codex_execpolicy::RuleMatch; use codex_execpolicy::RuleRef; use codex_execpolicy::blocking_append_allow_prefix_rule; @@ -803,6 +804,55 @@ host_executable(name = "git", paths = ["{git_path_literal}"]) Ok(()) } +/// An absolute-path amendment may bypass a basename rule only if its restriction is preserved. +#[test] +fn inherited_absolute_amendments_preserve_managed_basename_restrictions() -> Result<()> { + let mut parser = PolicyParser::new(); + parser.parse( + "managed.rules", + r#"prefix_rule(pattern = ["git", ["push", "send-pack"]], decision = "forbidden")"#, + )?; + let managed = parser.build(); + let requirements = RequirementsExecPolicy::new(managed.clone()); + let git = host_absolute_path(&["usr", "bin", &host_executable_name("git")]); + for (args, decision, accepted) in [ + (&["status"][..], Decision::Allow, true), + (&[], Decision::Allow, false), + (&["send-pack", "origin"], Decision::Allow, false), + (&["push"], Decision::Forbidden, true), + ] { + let mut snapshot = managed.clone(); + let mut prefix = vec![git.clone()]; + prefix.extend(tokens(args)); + snapshot.add_prefix_rule(&prefix, decision)?; + assert_eq!( + snapshot.preserves_managed_requirements(&requirements), + accepted + ); + } + Ok(()) +} + +#[test] +fn inherited_absolute_amendment_respects_existing_exact_denials() -> Result<()> { + let git = host_absolute_path(&["usr", "bin", &host_executable_name("git")]); + for (denied_args, accepted) in [(&["push"][..], true), (&["push", "origin"][..], false)] { + let mut managed = Policy::empty(); + managed.add_prefix_rule(&tokens(&["git", "push"]), Decision::Forbidden)?; + let mut exact = vec![git.clone()]; + exact.extend(tokens(denied_args)); + managed.add_prefix_rule(&exact, Decision::Forbidden)?; + + let mut snapshot = managed.clone(); + snapshot.add_prefix_rule(std::slice::from_ref(&git), Decision::Allow)?; + assert_eq!( + snapshot.preserves_managed_requirements(&RequirementsExecPolicy::new(managed)), + accepted + ); + } + Ok(()) +} + #[test] fn prefix_rule_examples_honor_host_executable_resolution() -> Result<()> { let allowed_git_name = host_executable_name("git"); diff --git a/codex-rs/ext/extension-api/src/state.rs b/codex-rs/ext/extension-api/src/state.rs index 51aa1feefe..538a8d15ff 100644 --- a/codex-rs/ext/extension-api/src/state.rs +++ b/codex-rs/ext/extension-api/src/state.rs @@ -34,6 +34,11 @@ impl ExtensionDataInit { .map(downcast_data) } + /// Consumes a startup-only attachment before creating the live scope. + pub fn remove(&mut self) -> Option> { + self.entries.remove(&TypeId::of::()).map(downcast_data) + } + /// Returns a host-supplied initial attachment without creating a mutable scope. pub fn get(&self) -> Option> where