Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions codex-rs/core/src/agent/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ impl AgentControl {
let inherited_shell_snapshot = self
.inherited_shell_snapshot_for_source(&state, session_source.as_ref())
.await;
let inherited_exec_policy = self
.inherited_exec_policy_for_source(&state, session_source.as_ref(), &config)
.await;
let session_source = match session_source {
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id,
Expand Down Expand Up @@ -189,6 +192,7 @@ impl AgentControl {
session_source,
/*persist_extended_history*/ false,
inherited_shell_snapshot,
inherited_exec_policy,
)
.await?
} else {
Expand All @@ -200,6 +204,7 @@ impl AgentControl {
/*persist_extended_history*/ false,
/*metrics_service_name*/ None,
inherited_shell_snapshot,
inherited_exec_policy,
)
.await?
}
Expand Down Expand Up @@ -271,6 +276,9 @@ impl AgentControl {
let inherited_shell_snapshot = self
.inherited_shell_snapshot_for_source(&state, Some(&session_source))
.await;
let inherited_exec_policy = self
.inherited_exec_policy_for_source(&state, Some(&session_source), &config)
.await;
let rollout_path =
find_thread_path_by_id_str(config.codex_home.as_path(), &thread_id.to_string())
.await?
Expand All @@ -283,6 +291,7 @@ impl AgentControl {
self.clone(),
session_source,
inherited_shell_snapshot,
inherited_exec_policy,
)
.await?;
reservation.commit(resumed_thread.thread_id);
Expand Down Expand Up @@ -499,6 +508,30 @@ impl AgentControl {
let parent_thread = state.get_thread(*parent_thread_id).await.ok()?;
parent_thread.codex.session.user_shell().shell_snapshot()
}

async fn inherited_exec_policy_for_source(
&self,
state: &Arc<ThreadManagerState>,
session_source: Option<&SessionSource>,
child_config: &crate::config::Config,
) -> Option<Arc<crate::exec_policy::ExecPolicyManager>> {
let Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id, ..
})) = session_source
else {
return None;
};

let parent_thread = state.get_thread(*parent_thread_id).await.ok()?;
let parent_config = parent_thread.codex.session.get_config().await;
if !crate::exec_policy::child_uses_parent_exec_policy(&parent_config, child_config) {
return None;
}

Some(Arc::clone(
&parent_thread.codex.session.services.exec_policy,
))
}
}
#[cfg(test)]
#[path = "control_tests.rs"]
Expand Down
16 changes: 11 additions & 5 deletions codex-rs/core/src/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ pub(crate) struct CodexSpawnArgs {
pub(crate) persist_extended_history: bool,
pub(crate) metrics_service_name: Option<String>,
pub(crate) inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
pub(crate) inherited_exec_policy: Option<Arc<ExecPolicyManager>>,
pub(crate) user_shell_override: Option<shell::Shell>,
pub(crate) parent_trace: Option<W3cTraceContext>,
}
Expand Down Expand Up @@ -423,6 +424,7 @@ impl Codex {
metrics_service_name,
inherited_shell_snapshot,
user_shell_override,
inherited_exec_policy,
parent_trace: _,
} = args;
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
Expand Down Expand Up @@ -479,11 +481,15 @@ impl Codex {
// Guardian review should rely on the built-in shell safety checks,
// not on caller-provided exec-policy rules that could shape the
// reviewer or silently auto-approve commands.
ExecPolicyManager::default()
Arc::new(ExecPolicyManager::default())
} else if let Some(exec_policy) = &inherited_exec_policy {
Arc::clone(exec_policy)
} else {
ExecPolicyManager::load(&config.config_layer_stack)
.await
.map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))?
Arc::new(
ExecPolicyManager::load(&config.config_layer_stack)
.await
.map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))?,
)
};

let config = Arc::new(config);
Expand Down Expand Up @@ -1376,7 +1382,7 @@ impl Session {
config: Arc<Config>,
auth_manager: Arc<AuthManager>,
models_manager: Arc<ModelsManager>,
exec_policy: ExecPolicyManager,
exec_policy: Arc<ExecPolicyManager>,
tx_event: Sender<Event>,
agent_status: watch::Sender<AgentStatus>,
initial_history: InitialHistory,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/codex_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub(crate) async fn run_codex_thread_interactive(
metrics_service_name: None,
inherited_shell_snapshot: None,
user_shell_override: None,
inherited_exec_policy: Some(Arc::clone(&parent_session.services.exec_policy)),
parent_trace: None,
})
.await?;
Expand Down
6 changes: 3 additions & 3 deletions codex-rs/core/src/codex_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2365,7 +2365,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
Arc::clone(&config),
auth_manager,
models_manager,
ExecPolicyManager::default(),
Arc::new(ExecPolicyManager::default()),
tx_event,
agent_status_tx,
InitialHistory::New,
Expand Down Expand Up @@ -2401,7 +2401,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
CollaborationModesConfig::default(),
));
let agent_control = AgentControl::default();
let exec_policy = ExecPolicyManager::default();
let exec_policy = Arc::new(ExecPolicyManager::default());
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let model = ModelsManager::get_model_offline_for_tests(config.model.as_deref());
let model_info = ModelsManager::construct_model_info_offline_for_tests(model.as_str(), &config);
Expand Down Expand Up @@ -3193,7 +3193,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
CollaborationModesConfig::default(),
));
let agent_control = AgentControl::default();
let exec_policy = ExecPolicyManager::default();
let exec_policy = Arc::new(ExecPolicyManager::default());
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let model = ModelsManager::get_model_offline_for_tests(config.model.as_deref());
let model_info = ModelsManager::construct_model_info_offline_for_tests(model.as_str(), &config);
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/codex_tests_guardian.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
persist_extended_history: false,
metrics_service_name: None,
inherited_shell_snapshot: None,
inherited_exec_policy: Some(Arc::new(parent_exec_policy)),
user_shell_override: None,
parent_trace: None,
})
Expand Down
48 changes: 44 additions & 4 deletions codex-rs/core/src/exec_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ use tracing::instrument;

use crate::bash::parse_shell_lc_plain_commands;
use crate::bash::parse_shell_lc_single_command_prefix;
use crate::config::Config;
use crate::sandboxing::SandboxPermissions;
use crate::tools::sandboxing::ExecApprovalRequirement;
use codex_utils_absolute_path::AbsolutePathBuf;
use shlex::try_join as shlex_try_join;

const PROMPT_CONFLICT_REASON: &str =
Expand Down Expand Up @@ -94,6 +96,24 @@ static BANNED_PREFIX_SUGGESTIONS: &[&[&str]] = &[
&["osascript"],
];

pub(crate) fn child_uses_parent_exec_policy(parent_config: &Config, child_config: &Config) -> bool {
fn exec_policy_config_folders(config: &Config) -> Vec<AbsolutePathBuf> {
config
.config_layer_stack
.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ false,
)
.into_iter()
.filter_map(codex_config::ConfigLayerEntry::config_folder)
.collect()
}

exec_policy_config_folders(parent_config) == exec_policy_config_folders(child_config)
&& parent_config.config_layer_stack.requirements().exec_policy
== child_config.config_layer_stack.requirements().exec_policy
}

fn is_policy_match(rule_match: &RuleMatch) -> bool {
match rule_match {
RuleMatch::PrefixRuleMatch { .. } => true,
Expand Down Expand Up @@ -170,6 +190,7 @@ pub enum ExecPolicyUpdateError {

pub(crate) struct ExecPolicyManager {
policy: ArcSwap<Policy>,
update_lock: tokio::sync::Mutex<()>,
}

pub(crate) struct ExecApprovalRequest<'a> {
Expand All @@ -185,6 +206,7 @@ impl ExecPolicyManager {
pub(crate) fn new(policy: Arc<Policy>) -> Self {
Self {
policy: ArcSwap::from(policy),
update_lock: tokio::sync::Mutex::new(()),
}
}

Expand Down Expand Up @@ -292,11 +314,11 @@ impl ExecPolicyManager {
codex_home: &Path,
amendment: &ExecPolicyAmendment,
) -> Result<(), ExecPolicyUpdateError> {
let _update_guard = self.update_lock.lock().await;
let policy_path = default_policy_path(codex_home);
let prefix = amendment.command.clone();
spawn_blocking({
let policy_path = policy_path.clone();
let prefix = prefix.clone();
let prefix = amendment.command.clone();
move || blocking_append_allow_prefix_rule(&policy_path, &prefix)
})
.await
Expand All @@ -306,8 +328,25 @@ impl ExecPolicyManager {
source,
})?;

let mut updated_policy = self.current().as_ref().clone();
updated_policy.add_prefix_rule(&prefix, Decision::Allow)?;
let current_policy = self.current();
let match_options = MatchOptions {
resolve_host_executables: true,
};
let existing_evaluation = current_policy.check_multiple_with_options(
[&amendment.command],
&|_| Decision::Forbidden,
&match_options,
);
let already_allowed = existing_evaluation.decision == Decision::Allow
&& existing_evaluation.matched_rules.iter().any(|rule_match| {
is_policy_match(rule_match) && rule_match.decision() == Decision::Allow
});
if already_allowed {
return Ok(());
}

let mut updated_policy = current_policy.as_ref().clone();
updated_policy.add_prefix_rule(&amendment.command, Decision::Allow)?;
self.policy.store(Arc::new(updated_policy));
Ok(())
}
Expand All @@ -320,6 +359,7 @@ impl ExecPolicyManager {
decision: Decision,
justification: Option<String>,
) -> Result<(), ExecPolicyUpdateError> {
let _update_guard = self.update_lock.lock().await;
let policy_path = default_policy_path(codex_home);
let host = host.to_string();
spawn_blocking({
Expand Down
94 changes: 94 additions & 0 deletions codex-rs/core/src/exec_policy_tests.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
use super::*;
use crate::config::Config;
use crate::config::ConfigBuilder;
use crate::config_loader::ConfigLayerEntry;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use crate::config_loader::LoaderOverrides;
use crate::config_loader::RequirementSource;
use crate::config_loader::Sourced;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::RequirementsExecPolicy;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
Expand All @@ -17,6 +24,7 @@ use std::fs;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use tempfile::TempDir;
use tempfile::tempdir;
use toml::Value as TomlValue;

Expand Down Expand Up @@ -73,6 +81,92 @@ fn unrestricted_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
FileSystemSandboxPolicy::unrestricted()
}

async fn test_config() -> (TempDir, Config) {
let home = TempDir::new().expect("create temp dir");
let config = ConfigBuilder::default()
.codex_home(home.path().to_path_buf())
.loader_overrides(LoaderOverrides {
#[cfg(target_os = "macos")]
managed_preferences_base64: Some(String::new()),
macos_managed_config_requirements_base64: Some(String::new()),
..LoaderOverrides::default()
})
.build()
.await
.expect("load default test config");
(home, config)
}

#[tokio::test]
async fn child_uses_parent_exec_policy_when_layer_stack_matches() {
let (_home, parent_config) = test_config().await;
let child_config = parent_config.clone();

assert!(child_uses_parent_exec_policy(&parent_config, &child_config));
}

#[tokio::test]
async fn child_uses_parent_exec_policy_when_non_exec_policy_layers_differ() {
let (_home, parent_config) = test_config().await;
let mut child_config = parent_config.clone();
let mut layers: Vec<_> = child_config
.config_layer_stack
.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true)
.into_iter()
.cloned()
.collect();
layers.push(ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
TomlValue::Table(Default::default()),
));
child_config.config_layer_stack = ConfigLayerStack::new(
layers,
child_config.config_layer_stack.requirements().clone(),
child_config.config_layer_stack.requirements_toml().clone(),
)
.expect("config layer stack");

assert!(child_uses_parent_exec_policy(&parent_config, &child_config));
}

#[tokio::test]
async fn child_does_not_use_parent_exec_policy_when_requirements_exec_policy_differs() {
let (_home, parent_config) = test_config().await;
let mut child_config = parent_config.clone();
let mut requirements = ConfigRequirements {
exec_policy: child_config
.config_layer_stack
.requirements()
.exec_policy
.clone(),
..ConfigRequirements::default()
};
let mut policy = Policy::empty();
policy
.add_prefix_rule(&["rm".to_string()], Decision::Forbidden)
.expect("add prefix rule");
requirements.exec_policy = Some(Sourced::new(
RequirementsExecPolicy::new(policy),
RequirementSource::Unknown,
));
child_config.config_layer_stack = ConfigLayerStack::new(
child_config
.config_layer_stack
.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true)
.into_iter()
.cloned()
.collect(),
requirements,
child_config.config_layer_stack.requirements_toml().clone(),
)
.expect("config layer stack");

assert!(!child_uses_parent_exec_policy(
&parent_config,
&child_config
));
}

#[tokio::test]
async fn returns_empty_policy_when_no_policy_files_exist() {
let temp_dir = tempdir().expect("create temp dir");
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/state/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub(crate) struct SessionServices {
pub(crate) user_shell: Arc<crate::shell::Shell>,
pub(crate) shell_snapshot_tx: watch::Sender<Option<Arc<crate::shell_snapshot::ShellSnapshot>>>,
pub(crate) show_raw_agent_reasoning: bool,
pub(crate) exec_policy: ExecPolicyManager,
pub(crate) exec_policy: Arc<ExecPolicyManager>,
pub(crate) auth_manager: Arc<AuthManager>,
pub(crate) models_manager: Arc<ModelsManager>,
pub(crate) session_telemetry: SessionTelemetry,
Expand Down
Loading
Loading