clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-23 12:34:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: preserve deny-read sandboxing for safe commands

Browse Source
This commit is contained in:
Michael Bolin
2026-05-21 13:46:43 -07:00
parent 24faf49b2a
commit 101e4b3c61
5 changed files with 154 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
codex-rs/core/src/tools/orchestrator.rs
View File

@@ -28,6 +28,7 @@ use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::default_exec_approval_requirement;
use crate::tools::sandboxing::sandbox_override_for_first_attempt;
use crate::tools::sandboxing::unsandboxed_execution_allowed;
use codex_hooks::PermissionRequestDecision;
use codex_otel::ToolDecisionSource;
use codex_protocol::error::CodexErr;
@@ -291,6 +292,12 @@ impl ToolOrchestrator {
network_policy_decision,
})));
}
if !unsandboxed_execution_allowed(&file_system_sandbox_policy) {
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
network_policy_decision,
})));
}
// Under `Never` or `OnRequest`, do not retry without sandbox;
// surface a concise sandbox denial that preserves the
// original output.

38
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
View File

@@ -21,6 +21,7 @@ use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
use crate::tools::sandboxing::unsandboxed_execution_allowed;
use codex_execpolicy::Decision;
use codex_execpolicy::Evaluation;
use codex_execpolicy::MatchOptions;
@@ -97,6 +98,18 @@ fn approval_sandbox_permissions(
}
}
fn approval_sandbox_permissions_for_policy(
sandbox_permissions: SandboxPermissions,
additional_permissions_preapproved: bool,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> SandboxPermissions {
if unsandboxed_execution_allowed(file_system_sandbox_policy) {
approval_sandbox_permissions(sandbox_permissions, additional_permissions_preapproved)
} else {
SandboxPermissions::UseDefault
}
}
pub(super) async fn try_run_zsh_fork(
req: &ShellRequest,
attempt: &SandboxAttempt<'_>,
@@ -196,9 +209,10 @@ pub(super) async fn try_run_zsh_fork(
if let Some(cancellation) = attempt.network_denial_cancellation_token.clone() {
cancel_token = cancel_when_either(cancel_token, cancellation);
}
let approval_sandbox_permissions = approval_sandbox_permissions(
let approval_sandbox_permissions = approval_sandbox_permissions_for_policy(
req.sandbox_permissions,
req.additional_permissions_preapproved,
&command_executor.file_system_sandbox_policy,
);
let escalation_policy = CoreShellActionProvider {
policy: Arc::clone(&exec_policy),
@@ -283,9 +297,10 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(),
sandbox_policy_cwd: exec_request.windows_sandbox_policy_cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
approval_sandbox_permissions: approval_sandbox_permissions(
approval_sandbox_permissions: approval_sandbox_permissions_for_policy(
req.sandbox_permissions,
req.additional_permissions_preapproved,
&exec_request.file_system_sandbox_policy,
),
prompt_permissions: req.additional_permissions.clone(),
stopwatch: Stopwatch::unlimited(),
@@ -367,11 +382,18 @@ impl CoreShellActionProvider {
fn shell_request_escalation_execution(
sandbox_permissions: SandboxPermissions,
permission_profile: &PermissionProfile,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
additional_permissions: Option<&AdditionalPermissionProfile>,
) -> EscalationExecution {
match sandbox_permissions {
SandboxPermissions::UseDefault => EscalationExecution::TurnDefault,
SandboxPermissions::RequireEscalated => EscalationExecution::Unsandboxed,
SandboxPermissions::RequireEscalated => {
if unsandboxed_execution_allowed(file_system_sandbox_policy) {
EscalationExecution::Unsandboxed
} else {
EscalationExecution::TurnDefault
}
}
SandboxPermissions::WithAdditionalPermissions => additional_permissions
.map(|_| {
// Shell request additional permissions were already normalized and
@@ -611,8 +633,10 @@ impl EscalationPolicy for CoreShellActionProvider {
// fallback function.
let decision_driven_by_policy =
Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision);
let needs_escalation =
self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy;
let unsandboxed_allowed = unsandboxed_execution_allowed(&self.file_system_sandbox_policy);
let needs_escalation = unsandboxed_allowed
&& (self.sandbox_permissions.requires_escalated_permissions()
|| decision_driven_by_policy);
let decision_source = if decision_driven_by_policy {
DecisionSource::PrefixRule
@@ -620,10 +644,12 @@ impl EscalationPolicy for CoreShellActionProvider {
DecisionSource::UnmatchedCommandFallback
};
let escalation_execution = match decision_source {
DecisionSource::PrefixRule => EscalationExecution::Unsandboxed,
DecisionSource::PrefixRule if unsandboxed_allowed => EscalationExecution::Unsandboxed,
DecisionSource::PrefixRule => EscalationExecution::TurnDefault,
DecisionSource::UnmatchedCommandFallback => Self::shell_request_escalation_execution(
self.sandbox_permissions,
&self.permission_profile,
&self.file_system_sandbox_policy,
self.prompt_permissions.as_ref(),
),
};

98
codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs
View File

@@ -66,6 +66,23 @@ fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
}])
}
fn denied_read_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/*.env".to_string(),
},
access: FileSystemAccessMode::Deny,
},
])
}
fn test_sandbox_cwd() -> AbsolutePathBuf {
AbsolutePathBuf::try_from(host_absolute_path(&["workspace"])).unwrap()
}
@@ -129,6 +146,26 @@ fn approval_sandbox_permissions_only_downgrades_preapproved_additional_permissio
);
}
#[test]
fn approval_sandbox_permissions_for_policy_suppresses_no_sandbox_with_deny_reads() {
let file_system_sandbox_policy =
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/*.env".to_string(),
},
access: FileSystemAccessMode::Deny,
}]);
assert_eq!(
super::approval_sandbox_permissions_for_policy(
SandboxPermissions::RequireEscalated,
/*additional_permissions_preapproved*/ false,
&file_system_sandbox_policy,
),
SandboxPermissions::UseDefault,
);
}
#[test]
fn extract_shell_script_preserves_login_flag() {
assert_eq!(
@@ -289,11 +326,13 @@ fn shell_request_escalation_execution_is_explicit() {
&file_system_sandbox_policy,
network_sandbox_policy,
);
let read_only_file_system_policy = read_only_file_system_sandbox_policy();
assert_eq!(
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::UseDefault,
&permission_profile,
&file_system_sandbox_policy,
/*additional_permissions*/ None,
),
EscalationExecution::TurnDefault,
@@ -302,14 +341,25 @@ fn shell_request_escalation_execution_is_explicit() {
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::RequireEscalated,
&permission_profile,
&read_only_file_system_policy,
/*additional_permissions*/ None,
),
EscalationExecution::Unsandboxed,
);
assert_eq!(
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::RequireEscalated,
&permission_profile,
&file_system_sandbox_policy,
/*additional_permissions*/ None,
),
EscalationExecution::TurnDefault,
);
assert_eq!(
CoreShellActionProvider::shell_request_escalation_execution(
crate::sandboxing::SandboxPermissions::WithAdditionalPermissions,
&permission_profile,
&file_system_sandbox_policy,
Some(&requested_permissions),
),
EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile(
@@ -614,6 +664,54 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
));
}
#[tokio::test(flavor = "current_thread")]
async fn denied_reads_keep_prefix_rule_allow_inside_sandbox() -> anyhow::Result<()> {
let cat_path = host_absolute_path(&["usr", "bin", "cat"]);
let cat_path_literal = starlark_string(&cat_path);
let policy_src = format!(
r#"
prefix_rule(pattern = ["{cat_path_literal}"], decision = "allow")
"#
);
let mut parser = PolicyParser::new();
parser.parse("test.rules", &policy_src).unwrap();
let policy = parser.build();
let (session, turn_context) = make_session_and_context().await;
let file_system_sandbox_policy = denied_read_file_system_sandbox_policy();
let permission_profile = PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
NetworkSandboxPolicy::Restricted,
);
let workdir = test_sandbox_cwd();
let provider = CoreShellActionProvider {
policy: Arc::new(RwLock::new(policy)),
session: Arc::new(session),
turn: Arc::new(turn_context),
call_id: "deny-read-prefix-allow".to_string(),
tool_name: GuardianCommandSource::Shell,
approval_policy: AskForApproval::OnRequest,
permission_profile,
file_system_sandbox_policy,
sandbox_policy_cwd: workdir.clone(),
sandbox_permissions: SandboxPermissions::UseDefault,
approval_sandbox_permissions: SandboxPermissions::UseDefault,
prompt_permissions: None,
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
};
let action = codex_shell_escalation::EscalationPolicy::determine_action(
&provider,
&AbsolutePathBuf::try_from(cat_path).unwrap(),
&["cat".to_string(), "/tmp/visible.txt".to_string()],
&workdir,
)
.await?;
assert_eq!(action, codex_shell_escalation::EscalationDecision::Run);
Ok(())
}
#[test]
fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default() {
let policy = PolicyParser::new().build();

19
codex-rs/core/src/tools/sandboxing.rs
View File

@@ -248,6 +248,13 @@ pub(crate) fn sandbox_override_for_first_attempt(
exec_approval_requirement: &ExecApprovalRequirement,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> SandboxOverride {
// Deny-read restrictions are part of the active permission policy. Running
// without a filesystem sandbox would discard them, even if the command was
// otherwise approved by rules or explicit escalation.
if !unsandboxed_execution_allowed(file_system_sandbox_policy) {
return SandboxOverride::NoOverride;
}
// ExecPolicy `Allow` can intentionally imply full trust (Skip + bypass_sandbox=true),
// which supersedes `with_additional_permissions` sandboxed execution hints.
if matches!(
@@ -260,12 +267,6 @@ pub(crate) fn sandbox_override_for_first_attempt(
return SandboxOverride::BypassSandboxFirstAttempt;
}
// Deny-read restrictions suppress explicit escalation because that path
// would otherwise discard the filesystem policy entirely.
if file_system_sandbox_policy.has_denied_read_restrictions() {
return SandboxOverride::NoOverride;
}
if sandbox_permissions.requires_escalated_permissions() {
SandboxOverride::BypassSandboxFirstAttempt
} else {
@@ -273,6 +274,12 @@ pub(crate) fn sandbox_override_for_first_attempt(
}
}
pub(crate) fn unsandboxed_execution_allowed(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> bool {
!file_system_sandbox_policy.has_denied_read_restrictions()
}
pub(crate) fn managed_network_for_sandbox_permissions(
network: Option<&NetworkProxy>,
sandbox_permissions: SandboxPermissions,

7
codex-rs/core/src/tools/sandboxing_tests.rs
View File

@@ -138,7 +138,7 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() {
}
#[test]
fn deny_read_blocks_explicit_escalation_but_preserves_policy_bypass() {
fn deny_read_blocks_explicit_escalation_and_policy_bypass() {
let file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/*.env".to_string(),
@@ -158,6 +158,7 @@ fn deny_read_blocks_explicit_escalation_but_preserves_policy_bypass() {
SandboxOverride::NoOverride,
"explicit escalation would drop deny-read filesystem policy, so keep the first attempt sandboxed",
);
assert!(!unsandboxed_execution_allowed(&file_system_policy));
assert_eq!(
sandbox_override_for_first_attempt(
SandboxPermissions::WithAdditionalPermissions,
@@ -167,7 +168,7 @@ fn deny_read_blocks_explicit_escalation_but_preserves_policy_bypass() {
},
&file_system_policy,
),
SandboxOverride::BypassSandboxFirstAttempt,
"exec-policy allow rules intentionally bypass sandbox even when deny-read entries exist",
SandboxOverride::NoOverride,
"exec-policy allow rules would drop deny-read filesystem policy, so keep the first attempt sandboxed",
);
}