permissions: remove core legacy policy round trips

This commit is contained in:
Michael Bolin
2026-04-25 20:01:30 -07:00
parent b23793dc3c
commit a0e6a97cae
13 changed files with 230 additions and 304 deletions
+9 -8
View File
@@ -2182,24 +2182,25 @@ fn web_search_mode_disabled_overrides_legacy_request() {
#[test]
fn web_search_mode_for_turn_uses_preference_for_read_only() {
let web_search_mode = Constrained::allow_any(WebSearchMode::Cached);
let mode =
resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::new_read_only_policy());
let permission_profile =
PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::new_read_only_policy());
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &permission_profile);
assert_eq!(mode, WebSearchMode::Cached);
}
#[test]
fn web_search_mode_for_turn_prefers_live_for_danger_full_access() {
fn web_search_mode_for_turn_prefers_live_for_disabled_permissions() {
let web_search_mode = Constrained::allow_any(WebSearchMode::Cached);
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::DangerFullAccess);
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &PermissionProfile::Disabled);
assert_eq!(mode, WebSearchMode::Live);
}
#[test]
fn web_search_mode_for_turn_respects_disabled_for_danger_full_access() {
fn web_search_mode_for_turn_respects_disabled_for_disabled_permissions() {
let web_search_mode = Constrained::allow_any(WebSearchMode::Disabled);
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::DangerFullAccess);
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &PermissionProfile::Disabled);
assert_eq!(mode, WebSearchMode::Disabled);
}
@@ -2219,7 +2220,7 @@ fn web_search_mode_for_turn_falls_back_when_live_is_disallowed() -> anyhow::Resu
})
}
})?;
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::DangerFullAccess);
let mode = resolve_web_search_mode_for_turn(&web_search_mode, &PermissionProfile::Disabled);
assert_eq!(mode, WebSearchMode::Cached);
Ok(())
@@ -6783,7 +6784,7 @@ async fn requirements_web_search_mode_overrides_danger_full_access_default() ->
assert_eq!(
resolve_web_search_mode_for_turn(
&config.web_search_mode,
config.permissions.sandbox_policy.get(),
&config.permissions.permission_profile(),
),
WebSearchMode::Cached,
);
+2 -2
View File
@@ -1606,11 +1606,11 @@ fn multi_agent_v2_toml_config(features: Option<&FeaturesToml>) -> Option<&MultiA
pub(crate) fn resolve_web_search_mode_for_turn(
web_search_mode: &Constrained<WebSearchMode>,
sandbox_policy: &SandboxPolicy,
permission_profile: &PermissionProfile,
) -> WebSearchMode {
let preferred = web_search_mode.value();
if matches!(sandbox_policy, SandboxPolicy::DangerFullAccess)
if matches!(permission_profile, PermissionProfile::Disabled)
&& preferred != WebSearchMode::Disabled
{
for mode in [
+33 -9
View File
@@ -20,10 +20,10 @@ use codex_execpolicy::RuleMatch;
use codex_execpolicy::blocking_append_allow_prefix_rule;
use codex_execpolicy::blocking_append_network_rule;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use codex_shell_command::is_dangerous_command::command_might_be_dangerous;
use codex_shell_command::is_safe_command::is_known_safe_command;
use thiserror::Error;
@@ -204,8 +204,9 @@ pub(crate) struct ExecPolicyManager {
pub(crate) struct ExecApprovalRequest<'a> {
pub(crate) command: &'a [String],
pub(crate) approval_policy: AskForApproval,
pub(crate) sandbox_policy: &'a SandboxPolicy,
pub(crate) permission_profile: PermissionProfile,
pub(crate) file_system_sandbox_policy: &'a FileSystemSandboxPolicy,
pub(crate) sandbox_cwd: &'a Path,
pub(crate) sandbox_permissions: SandboxPermissions,
pub(crate) prefix_rule: Option<Vec<String>>,
}
@@ -238,8 +239,9 @@ impl ExecPolicyManager {
let ExecApprovalRequest {
command,
approval_policy,
sandbox_policy,
permission_profile,
file_system_sandbox_policy,
sandbox_cwd,
sandbox_permissions,
prefix_rule,
} = req;
@@ -252,8 +254,9 @@ impl ExecPolicyManager {
let exec_policy_fallback = |cmd: &[String]| {
render_decision_for_unmatched_command(
approval_policy,
sandbox_policy,
&permission_profile,
file_system_sandbox_policy,
sandbox_cwd,
cmd,
sandbox_permissions,
used_complex_parsing,
@@ -580,8 +583,9 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy,
/// If a command is not matched by any execpolicy rule, derive a [`Decision`].
pub fn render_decision_for_unmatched_command(
approval_policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
permission_profile: &PermissionProfile,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
sandbox_cwd: &Path,
command: &[String],
sandbox_permissions: SandboxPermissions,
used_complex_parsing: bool,
@@ -592,8 +596,12 @@ pub fn render_decision_for_unmatched_command(
// On Windows, ReadOnly sandbox is not a real sandbox, so special-case it
// here.
let environment_lacks_sandbox_protections =
cfg!(windows) && matches!(sandbox_policy, SandboxPolicy::ReadOnly { .. });
let environment_lacks_sandbox_protections = cfg!(windows)
&& profile_is_managed_read_only(
permission_profile,
file_system_sandbox_policy,
sandbox_cwd,
);
// If the command is flagged as dangerous or we have no sandbox protection,
// we should never allow it to run without approval.
@@ -605,8 +613,8 @@ pub fn render_decision_for_unmatched_command(
return match approval_policy {
AskForApproval::Never => {
let sandbox_is_explicitly_disabled = matches!(
sandbox_policy,
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. }
permission_profile,
PermissionProfile::Disabled | PermissionProfile::External { .. }
);
if sandbox_is_explicitly_disabled {
// If the sandbox is explicitly disabled, we should allow the command to run
@@ -670,6 +678,22 @@ pub fn render_decision_for_unmatched_command(
}
}
fn profile_is_managed_read_only(
permission_profile: &PermissionProfile,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
sandbox_cwd: &Path,
) -> bool {
matches!(permission_profile, PermissionProfile::Managed { .. })
&& matches!(
file_system_sandbox_policy.kind,
FileSystemSandboxKind::Restricted
)
&& !file_system_sandbox_policy.has_full_disk_write_access()
&& file_system_sandbox_policy
.get_writable_roots_with_cwd(sandbox_cwd)
.is_empty()
}
fn default_policy_path(codex_home: &Path) -> PathBuf {
codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE)
}
+112 -14
View File
@@ -15,10 +15,12 @@ use codex_config::RequirementsExecPolicy;
use codex_config::config_toml::ConfigToml;
use codex_config::config_toml::ProjectConfig;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::SandboxPolicy;
@@ -116,6 +118,10 @@ fn external_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
FileSystemSandboxPolicy::external_sandbox()
}
fn permission_profile_from_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile {
PermissionProfile::from_legacy_sandbox_policy(sandbox_policy)
}
async fn test_config() -> (TempDir, Config) {
let home = TempDir::new().expect("create temp dir");
let config = ConfigBuilder::without_managed_config_for_tests()
@@ -954,8 +960,9 @@ fn unmatched_granular_policy_still_prompts_for_restricted_sandbox_escalation() {
request_permissions: true,
mcp_elicitations: true,
}),
&SandboxPolicy::new_read_only_policy(),
&permission_profile_from_sandbox_policy(&SandboxPolicy::new_read_only_policy()),
&read_only_file_system_sandbox_policy(),
Path::new("/tmp"),
&command,
SandboxPermissions::RequireEscalated,
/*used_complex_parsing*/ false,
@@ -972,8 +979,9 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
Decision::Prompt,
render_decision_for_unmatched_command(
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
&PermissionProfile::Disabled,
&restricted_file_system_policy,
Path::new("/tmp"),
&command,
SandboxPermissions::RequireEscalated,
/*used_complex_parsing*/ false,
@@ -981,6 +989,65 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
);
}
#[test]
fn managed_cwd_write_profile_is_not_read_only() {
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
},
]);
let permission_profile = PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
NetworkSandboxPolicy::Restricted,
);
assert!(!profile_is_managed_read_only(
&permission_profile,
&file_system_sandbox_policy,
Path::new("/tmp/project")
));
}
#[test]
fn managed_unresolvable_write_profile_is_still_read_only() {
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::unknown(
":future_special_path",
/*subpath*/ None,
),
},
access: FileSystemAccessMode::Write,
},
]);
let permission_profile = PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
NetworkSandboxPolicy::Restricted,
);
assert!(profile_is_managed_read_only(
&permission_profile,
&file_system_sandbox_policy,
Path::new("/tmp/project")
));
}
#[tokio::test]
async fn exec_approval_requirement_prompts_for_inline_additional_permissions_under_on_request() {
assert_exec_approval_requirement_for_command(
@@ -1058,8 +1125,11 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision()
request_permissions: true,
mcp_elicitations: true,
}),
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::RequireEscalated,
prefix_rule: None,
})
@@ -1095,8 +1165,11 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled(
request_permissions: true,
mcp_elicitations: true,
}),
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::RequireEscalated,
prefix_rule: None,
})
@@ -1119,8 +1192,11 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
@@ -1144,8 +1220,11 @@ async fn empty_bash_lc_script_falls_back_to_original_command() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
@@ -1173,8 +1252,11 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
@@ -1202,8 +1284,11 @@ async fn request_rule_uses_prefix_rule() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::RequireEscalated,
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
})
@@ -1234,8 +1319,9 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands(
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::DangerFullAccess,
permission_profile: PermissionProfile::Disabled,
file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::RequireEscalated,
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
})
@@ -1273,8 +1359,9 @@ async fn heuristics_apply_when_other_commands_match_policy() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::DangerFullAccess,
permission_profile: PermissionProfile::Disabled,
file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
@@ -1759,8 +1846,11 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &sneaky_command,
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: permissions,
prefix_rule: None,
})
@@ -1783,8 +1873,11 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &dangerous_command,
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: permissions,
prefix_rule: None,
})
@@ -1803,8 +1896,11 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &dangerous_command,
approval_policy: AskForApproval::Never,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions: permissions,
prefix_rule: None,
})
@@ -1897,12 +1993,14 @@ async fn assert_exec_approval_requirement_for_command(
None => Arc::new(Policy::empty()),
};
let permission_profile = permission_profile_from_sandbox_policy(&sandbox_policy);
let requirement = ExecPolicyManager::new(policy)
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy,
sandbox_policy: &sandbox_policy,
permission_profile,
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: Path::new("/tmp"),
sandbox_permissions,
prefix_rule,
})
+2 -2
View File
@@ -7849,15 +7849,15 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
turn_context_mut.permission_profile = PermissionProfile::Disabled;
let file_system_sandbox_policy = turn_context.file_system_sandbox_policy();
let sandbox_policy = turn_context.sandbox_policy();
let exec_approval_requirement = session
.services
.exec_policy
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &params.command,
approval_policy: turn_context.approval_policy.value(),
sandbox_policy: &sandbox_policy,
permission_profile: turn_context.permission_profile(),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: turn_context.cwd.as_path(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
+3 -3
View File
@@ -384,10 +384,10 @@ impl Session {
per_turn_config.permissions.permission_profile =
session_configuration.permission_profile.clone();
let sandbox_policy = session_configuration.sandbox_policy();
per_turn_config.permissions.sandbox_policy =
Constrained::allow_only(sandbox_policy.clone());
per_turn_config.permissions.sandbox_policy = Constrained::allow_only(sandbox_policy);
let permission_profile = session_configuration.permission_profile();
let resolved_web_search_mode =
resolve_web_search_mode_for_turn(&per_turn_config.web_search_mode, &sandbox_policy);
resolve_web_search_mode_for_turn(&per_turn_config.web_search_mode, &permission_profile);
if let Err(err) = per_turn_config
.web_search_mode
.set(resolved_web_search_mode)
+2 -2
View File
@@ -514,15 +514,15 @@ impl ShellHandler {
emitter.begin(event_ctx).await;
let file_system_sandbox_policy = turn.file_system_sandbox_policy();
let sandbox_policy = turn.sandbox_policy();
let exec_approval_requirement = session
.services
.exec_policy
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &exec_params.command,
approval_policy: turn.approval_policy.value(),
sandbox_policy: &sandbox_policy,
permission_profile: turn.permission_profile(),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: turn.cwd.as_path(),
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
codex_protocol::models::SandboxPermissions::UseDefault
} else {
@@ -34,7 +34,6 @@ use codex_protocol::exec_output::ExecToolCallOutput;
use codex_protocol::exec_output::StreamOutput;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::SandboxEnforcement;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
@@ -63,6 +62,7 @@ use codex_shell_escalation::ShellCommandExecutor;
use codex_shell_escalation::Stopwatch;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
@@ -206,9 +206,9 @@ pub(super) async fn try_run_zsh_fork(
call_id: ctx.call_id.clone(),
tool_name: GuardianCommandSource::Shell,
approval_policy: ctx.turn.approval_policy.value(),
sandbox_policy: command_executor.sandbox_policy.clone(),
permission_profile: command_executor.permission_profile.clone(),
file_system_sandbox_policy: command_executor.file_system_sandbox_policy.clone(),
network_sandbox_policy: command_executor.network_sandbox_policy,
sandbox_policy_cwd: command_executor.sandbox_policy_cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
approval_sandbox_permissions,
prompt_permissions: req.additional_permissions.clone(),
@@ -268,7 +268,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
network: exec_request.network.clone(),
windows_sandbox_level: exec_request.windows_sandbox_level,
arg0: exec_request.arg0.clone(),
sandbox_policy_cwd: ctx.turn.cwd.clone(),
sandbox_policy_cwd: exec_request.windows_sandbox_policy_cwd.clone(),
codex_linux_sandbox_exe: ctx.turn.codex_linux_sandbox_exe.clone(),
use_legacy_landlock: ctx.turn.features.use_legacy_landlock(),
};
@@ -279,9 +279,9 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
call_id: ctx.call_id.clone(),
tool_name: GuardianCommandSource::UnifiedExec,
approval_policy: ctx.turn.approval_policy.value(),
sandbox_policy: exec_request.sandbox_policy.clone(),
permission_profile: exec_request.permission_profile.clone(),
file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(),
network_sandbox_policy: exec_request.network_sandbox_policy,
sandbox_policy_cwd: exec_request.windows_sandbox_policy_cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
approval_sandbox_permissions: approval_sandbox_permissions(
req.sandbox_permissions,
@@ -314,9 +314,9 @@ struct CoreShellActionProvider {
call_id: String,
tool_name: GuardianCommandSource,
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
permission_profile: PermissionProfile,
file_system_sandbox_policy: FileSystemSandboxPolicy,
network_sandbox_policy: NetworkSandboxPolicy,
sandbox_policy_cwd: AbsolutePathBuf,
sandbox_permissions: SandboxPermissions,
approval_sandbox_permissions: SandboxPermissions,
prompt_permissions: Option<AdditionalPermissionProfile>,
@@ -366,9 +366,7 @@ impl CoreShellActionProvider {
fn shell_request_escalation_execution(
sandbox_permissions: SandboxPermissions,
sandbox_policy: &SandboxPolicy,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
network_sandbox_policy: NetworkSandboxPolicy,
permission_profile: &PermissionProfile,
additional_permissions: Option<&AdditionalPermissionProfile>,
) -> EscalationExecution {
match sandbox_permissions {
@@ -381,15 +379,7 @@ impl CoreShellActionProvider {
EscalationExecution::Permissions(
EscalationPermissions::ResolvedPermissionProfile(
ResolvedPermissionProfile {
permission_profile:
PermissionProfile::from_runtime_permissions_with_enforcement(
SandboxEnforcement::from_legacy_sandbox_policy(
sandbox_policy,
),
file_system_sandbox_policy,
network_sandbox_policy,
),
sandbox_policy: sandbox_policy.clone(),
permission_profile: permission_profile.clone(),
},
),
)
@@ -608,8 +598,9 @@ impl EscalationPolicy for CoreShellActionProvider {
argv,
InterceptedExecPolicyContext {
approval_policy: self.approval_policy,
sandbox_policy: &self.sandbox_policy,
permission_profile: self.permission_profile.clone(),
file_system_sandbox_policy: &self.file_system_sandbox_policy,
sandbox_cwd: self.sandbox_policy_cwd.as_path(),
sandbox_permissions: self.approval_sandbox_permissions,
enable_shell_wrapper_parsing:
ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING,
@@ -632,9 +623,7 @@ impl EscalationPolicy for CoreShellActionProvider {
DecisionSource::PrefixRule => EscalationExecution::Unsandboxed,
DecisionSource::UnmatchedCommandFallback => Self::shell_request_escalation_execution(
self.sandbox_permissions,
&self.sandbox_policy,
&self.file_system_sandbox_policy,
self.network_sandbox_policy,
&self.permission_profile,
self.prompt_permissions.as_ref(),
),
};
@@ -660,8 +649,9 @@ fn evaluate_intercepted_exec_policy(
) -> Evaluation {
let InterceptedExecPolicyContext {
approval_policy,
sandbox_policy,
permission_profile,
file_system_sandbox_policy,
sandbox_cwd,
sandbox_permissions,
enable_shell_wrapper_parsing,
} = context;
@@ -685,8 +675,9 @@ fn evaluate_intercepted_exec_policy(
let fallback = |cmd: &[String]| {
crate::exec_policy::render_decision_for_unmatched_command(
approval_policy,
sandbox_policy,
&permission_profile,
file_system_sandbox_policy,
sandbox_cwd,
cmd,
sandbox_permissions,
used_complex_parsing,
@@ -702,11 +693,12 @@ fn evaluate_intercepted_exec_policy(
)
}
#[derive(Clone, Copy)]
#[derive(Clone)]
struct InterceptedExecPolicyContext<'a> {
approval_policy: AskForApproval,
sandbox_policy: &'a SandboxPolicy,
permission_profile: PermissionProfile,
file_system_sandbox_policy: &'a FileSystemSandboxPolicy,
sandbox_cwd: &'a Path,
sandbox_permissions: SandboxPermissions,
enable_shell_wrapper_parsing: bool,
}
@@ -66,6 +66,14 @@ fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
}])
}
fn permission_profile_from_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile {
PermissionProfile::from_legacy_sandbox_policy(sandbox_policy)
}
fn test_sandbox_cwd() -> AbsolutePathBuf {
AbsolutePathBuf::try_from(host_absolute_path(&["workspace"])).unwrap()
}
#[test]
fn execve_prompt_rejection_keeps_prefix_rules_on_rules_flag() {
assert_eq!(
@@ -266,12 +274,6 @@ fn shell_request_escalation_execution_is_explicit() {
)),
..Default::default()
};
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![AbsolutePathBuf::from_absolute_path("/tmp/original/output").unwrap()],
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
};
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path {
@@ -287,13 +289,15 @@ fn shell_request_escalation_execution_is_explicit() {
},
]);
let network_sandbox_policy = NetworkSandboxPolicy::Restricted;
let permission_profile = PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
network_sandbox_policy,
);
assert_eq!(
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::UseDefault,
&sandbox_policy,
&file_system_sandbox_policy,
network_sandbox_policy,
&permission_profile,
/*additional_permissions*/ None,
),
EscalationExecution::TurnDefault,
@@ -301,9 +305,7 @@ fn shell_request_escalation_execution_is_explicit() {
assert_eq!(
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::RequireEscalated,
&sandbox_policy,
&file_system_sandbox_policy,
network_sandbox_policy,
&permission_profile,
/*additional_permissions*/ None,
),
EscalationExecution::Unsandboxed,
@@ -311,19 +313,11 @@ fn shell_request_escalation_execution_is_explicit() {
assert_eq!(
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::WithAdditionalPermissions,
&sandbox_policy,
&file_system_sandbox_policy,
network_sandbox_policy,
&permission_profile,
Some(&requested_permissions),
),
EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile(
ResolvedPermissionProfile {
permission_profile: PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
network_sandbox_policy,
),
sandbox_policy,
},
ResolvedPermissionProfile { permission_profile },
)),
);
}
@@ -395,8 +389,6 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
&read_only_file_system_sandbox_policy(),
NetworkSandboxPolicy::Restricted,
);
let sandbox_policy = SandboxPolicy::new_read_only_policy();
let workdir = AbsolutePathBuf::try_from(std::env::current_dir()?)?;
let target = std::env::temp_dir().join("execve-hook-short-circuit.txt");
let target_str = target.display().to_string();
@@ -410,9 +402,11 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
call_id: "execve-hook-call".to_string(),
tool_name: GuardianCommandSource::Shell,
approval_policy: AskForApproval::OnRequest,
sandbox_policy,
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
sandbox_policy_cwd: workdir.clone(),
sandbox_permissions: SandboxPermissions::RequireEscalated,
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
prompt_permissions: None,
@@ -464,6 +458,7 @@ fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_pars
parser.parse("test.rules", policy_src).unwrap();
let policy = parser.build();
let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "zsh"])).unwrap();
let sandbox_cwd = test_sandbox_cwd();
let enable_intercepted_exec_policy_shell_wrapper_parsing = false;
let evaluation = evaluate_intercepted_exec_policy(
@@ -476,8 +471,11 @@ fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_pars
],
InterceptedExecPolicyContext {
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: sandbox_cwd.as_path(),
sandbox_permissions: SandboxPermissions::UseDefault,
enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing,
},
@@ -515,6 +513,7 @@ fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled()
parser.parse("test.rules", policy_src).unwrap();
let policy = parser.build();
let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "bash"])).unwrap();
let sandbox_cwd = test_sandbox_cwd();
let enable_intercepted_exec_policy_shell_wrapper_parsing = true;
let evaluation = evaluate_intercepted_exec_policy(
@@ -527,8 +526,11 @@ fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled()
],
InterceptedExecPolicyContext {
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: sandbox_cwd.as_path(),
sandbox_permissions: SandboxPermissions::UseDefault,
enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing,
},
@@ -562,6 +564,7 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
parser.parse("test.rules", &policy_src).unwrap();
let policy = parser.build();
let program = AbsolutePathBuf::try_from(git_path).unwrap();
let sandbox_cwd = test_sandbox_cwd();
let evaluation = evaluate_intercepted_exec_policy(
&policy,
@@ -569,8 +572,11 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
&["git".to_string(), "status".to_string()],
InterceptedExecPolicyContext {
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: sandbox_cwd.as_path(),
sandbox_permissions: SandboxPermissions::UseDefault,
enable_shell_wrapper_parsing: false,
},
@@ -602,6 +608,7 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = SandboxPolicy::new_workspace_write_policy();
let file_system_sandbox_policy = read_only_file_system_sandbox_policy();
let sandbox_cwd = test_sandbox_cwd();
let preapproved = evaluate_intercepted_exec_policy(
&policy,
@@ -609,8 +616,9 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
&argv,
InterceptedExecPolicyContext {
approval_policy,
sandbox_policy: &sandbox_policy,
permission_profile: permission_profile_from_sandbox_policy(&sandbox_policy),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: sandbox_cwd.as_path(),
sandbox_permissions: super::approval_sandbox_permissions(
SandboxPermissions::WithAdditionalPermissions,
/*additional_permissions_preapproved*/ true,
@@ -624,8 +632,9 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
&argv,
InterceptedExecPolicyContext {
approval_policy,
sandbox_policy: &sandbox_policy,
permission_profile: permission_profile_from_sandbox_policy(&sandbox_policy),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: sandbox_cwd.as_path(),
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
enable_shell_wrapper_parsing: false,
},
@@ -650,6 +659,7 @@ host_executable(name = "git", paths = ["{allowed_git_literal}"])
parser.parse("test.rules", &policy_src).unwrap();
let policy = parser.build();
let program = AbsolutePathBuf::try_from(other_git.clone()).unwrap();
let sandbox_cwd = test_sandbox_cwd();
let evaluation = evaluate_intercepted_exec_policy(
&policy,
@@ -657,8 +667,11 @@ host_executable(name = "git", paths = ["{allowed_git_literal}"])
&["git".to_string(), "status".to_string()],
InterceptedExecPolicyContext {
approval_policy: AskForApproval::OnRequest,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
permission_profile: permission_profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
),
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
sandbox_cwd: sandbox_cwd.as_path(),
sandbox_permissions: SandboxPermissions::UseDefault,
enable_shell_wrapper_parsing: false,
},
@@ -789,7 +789,6 @@ impl UnifiedExecProcessManager {
context.turn.tools_config.unified_exec_shell_mode.clone(),
);
let file_system_sandbox_policy = context.turn.file_system_sandbox_policy();
let sandbox_policy = context.turn.sandbox_policy();
let exec_approval_requirement = context
.session
.services
@@ -797,8 +796,9 @@ impl UnifiedExecProcessManager {
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &request.command,
approval_policy: context.turn.approval_policy.value(),
sandbox_policy: &sandbox_policy,
permission_profile: context.turn.permission_profile(),
file_system_sandbox_policy: &file_system_sandbox_policy,
sandbox_cwd: context.turn.cwd.as_path(),
sandbox_permissions: if request.additional_permissions_preapproved {
crate::sandboxing::SandboxPermissions::UseDefault
} else {
-5
View File
@@ -4,7 +4,6 @@ use crate::models::PermissionProfile;
use crate::parse_command::ParsedCommand;
use crate::protocol::FileChange;
use crate::protocol::ReviewDecision;
use crate::protocol::SandboxPolicy;
use crate::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use schemars::JsonSchema;
@@ -16,13 +15,9 @@ use std::path::PathBuf;
use ts_rs::TS;
/// Fully resolved permissions for rerunning an intercepted child process.
///
/// `permission_profile` is the canonical permission model. `sandbox_policy`
/// remains as the legacy adapter for sandbox backends that still require it.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ResolvedPermissionProfile {
pub permission_profile: PermissionProfile,
pub sandbox_policy: SandboxPolicy,
}
#[allow(clippy::large_enum_variant)]
@@ -10,37 +10,12 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::permissions::ReadDenyMatcher;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::canonicalize_preserving_symlinks;
use std::collections::HashSet;
use std::num::NonZeroUsize;
use std::path::Path;
use std::path::PathBuf;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct EffectiveSandboxPermissions {
pub sandbox_policy: SandboxPolicy,
}
impl EffectiveSandboxPermissions {
pub fn new(
sandbox_policy: &SandboxPolicy,
additional_permissions: Option<&AdditionalPermissionProfile>,
) -> Self {
let Some(additional_permissions) = additional_permissions else {
return Self {
sandbox_policy: sandbox_policy.clone(),
};
};
Self {
sandbox_policy: effective_sandbox_policy(sandbox_policy, Some(additional_permissions)),
}
}
}
pub fn normalize_additional_permissions(
additional_permissions: AdditionalPermissionProfile,
) -> Result<AdditionalPermissionProfile, String> {
@@ -446,48 +421,6 @@ fn merge_permission_entries(
merged
}
fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
let mut out = Vec::with_capacity(paths.len());
let mut seen = HashSet::new();
for path in paths {
if seen.insert(path.to_path_buf()) {
out.push(path);
}
}
out
}
fn additional_permission_roots(
additional_permissions: &AdditionalPermissionProfile,
) -> (Vec<AbsolutePathBuf>, Vec<AbsolutePathBuf>) {
(
dedup_absolute_paths(
additional_permissions
.file_system
.as_ref()
.map(|file_system| {
file_system
.explicit_path_entries()
.filter_map(|(path, access)| access.can_read().then_some(path.clone()))
.collect()
})
.unwrap_or_default(),
),
dedup_absolute_paths(
additional_permissions
.file_system
.as_ref()
.map(|file_system| {
file_system
.explicit_path_entries()
.filter_map(|(path, access)| access.can_write().then_some(path.clone()))
.collect()
})
.unwrap_or_default(),
),
)
}
fn merge_file_system_policy_with_additional_permissions(
file_system_policy: &FileSystemSandboxPolicy,
additional_permissions: &FileSystemPermissions,
@@ -578,73 +511,6 @@ pub fn effective_permission_profile(
)
}
fn sandbox_policy_with_additional_permissions(
sandbox_policy: &SandboxPolicy,
additional_permissions: &AdditionalPermissionProfile,
) -> SandboxPolicy {
if additional_permissions.is_empty() {
return sandbox_policy.clone();
}
let (_extra_reads, extra_writes) = additional_permission_roots(additional_permissions);
match sandbox_policy {
SandboxPolicy::DangerFullAccess => SandboxPolicy::DangerFullAccess,
SandboxPolicy::ExternalSandbox { network_access } => SandboxPolicy::ExternalSandbox {
network_access: if merge_network_access(
network_access.is_enabled(),
additional_permissions,
) {
NetworkAccess::Enabled
} else {
NetworkAccess::Restricted
},
},
SandboxPolicy::WorkspaceWrite {
writable_roots,
network_access,
exclude_tmpdir_env_var,
exclude_slash_tmp,
} => {
let mut merged_writes = writable_roots.clone();
merged_writes.extend(extra_writes);
SandboxPolicy::WorkspaceWrite {
writable_roots: dedup_absolute_paths(merged_writes),
network_access: merge_network_access(*network_access, additional_permissions),
exclude_tmpdir_env_var: *exclude_tmpdir_env_var,
exclude_slash_tmp: *exclude_slash_tmp,
}
}
SandboxPolicy::ReadOnly { network_access } => {
if extra_writes.is_empty() {
SandboxPolicy::ReadOnly {
network_access: merge_network_access(*network_access, additional_permissions),
}
} else {
// todo(dylan) - for now, this grants more access than the request. We should restrict this,
// but we should add a new SandboxPolicy variant to handle this. While the feature is still
// UnderDevelopment, it's a useful approximation of the desired behavior.
SandboxPolicy::WorkspaceWrite {
writable_roots: dedup_absolute_paths(extra_writes),
network_access: merge_network_access(*network_access, additional_permissions),
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
}
}
}
}
}
fn effective_sandbox_policy(
sandbox_policy: &SandboxPolicy,
additional_permissions: Option<&AdditionalPermissionProfile>,
) -> SandboxPolicy {
additional_permissions.map_or_else(
|| sandbox_policy.clone(),
|permissions| sandbox_policy_with_additional_permissions(sandbox_policy, permissions),
)
}
pub fn should_require_platform_sandbox(
file_system_policy: &FileSystemSandboxPolicy,
network_policy: NetworkSandboxPolicy,
@@ -2,7 +2,6 @@ use super::effective_file_system_sandbox_policy;
use super::intersect_permission_profiles;
use super::merge_file_system_policy_with_additional_permissions;
use super::normalize_additional_permissions;
use super::sandbox_policy_with_additional_permissions;
use super::should_require_platform_sandbox;
use codex_protocol::models::AdditionalPermissionProfile as PermissionProfile;
use codex_protocol::models::FileSystemPermissions;
@@ -13,8 +12,6 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use dunce::canonicalize;
use pretty_assertions::assert_eq;
@@ -757,66 +754,6 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() {
);
}
#[test]
fn read_only_additional_permissions_can_enable_network_without_writes() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let policy = sandbox_policy_with_additional_permissions(
&SandboxPolicy::ReadOnly {
network_access: false,
},
&PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![path]),
Some(Vec::new()),
)),
},
);
assert_eq!(
policy,
SandboxPolicy::ReadOnly {
network_access: true,
}
);
}
#[test]
fn external_sandbox_additional_permissions_can_enable_network() {
let temp_dir = TempDir::new().expect("create temp dir");
let path = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let policy = sandbox_policy_with_additional_permissions(
&SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
},
&PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![path]),
Some(Vec::new()),
)),
},
);
assert_eq!(
policy,
SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Enabled,
}
);
}
#[test]
fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roots() {
let temp_dir = TempDir::new().expect("create temp dir");