mirror of
https://github.com/openai/codex.git
synced 2026-05-29 15:30:22 +00:00
fix: preserve deny-read sandboxing for safe commands (#23943)
## Why Permission profiles can mark filesystem entries as unreadable with `deny` rules, including glob patterns. Several shell execution paths treated known-safe commands or execpolicy `allow` rules as sufficient to run outside the filesystem sandbox. That is not valid for read-capable commands: for example, `cat` or `ls` may be reasonable to allow generally, but dropping the sandbox would also drop deny-read constraints such as `**/*.env`. ## What changed - Added a shared check that treats active deny-read restrictions as incompatible with unsandboxed execution. - Kept first-attempt execution sandboxed for explicit escalation and execpolicy allow bypasses when deny-read entries are present. - Prevented no-sandbox retry after a sandbox denial when the active filesystem policy contains deny-read entries. - Updated the zsh-fork execve path so prefix-rule `allow` decisions continue inside the current sandbox when deny-read restrictions are active. ## Verification - `cargo test -p codex-core tools::sandboxing::tests` - `cargo test -p codex-core tools::runtimes::shell::unix_escalation::tests` - `cargo test -p codex-core shell_command_enforces_glob_deny_read_policy`
This commit is contained in:
@@ -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;
|
||||
@@ -294,6 +295,8 @@ impl ToolOrchestrator {
|
||||
network_policy_decision,
|
||||
})));
|
||||
}
|
||||
let unsandboxed_allowed =
|
||||
unsandboxed_execution_allowed(&file_system_sandbox_policy);
|
||||
// Under `Never` or `OnRequest`, do not retry without sandbox;
|
||||
// surface a concise sandbox denial that preserves the
|
||||
// original output.
|
||||
@@ -315,6 +318,12 @@ impl ToolOrchestrator {
|
||||
})));
|
||||
}
|
||||
}
|
||||
if !unsandboxed_allowed && network_approval_context.is_none() {
|
||||
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
|
||||
output,
|
||||
network_policy_decision,
|
||||
})));
|
||||
}
|
||||
let retry_reason =
|
||||
if let Some(network_approval_context) = network_approval_context.as_ref() {
|
||||
format!(
|
||||
@@ -357,14 +366,30 @@ impl ToolOrchestrator {
|
||||
.await?;
|
||||
}
|
||||
|
||||
let escalated_attempt = SandboxAttempt {
|
||||
sandbox: SandboxType::None,
|
||||
let retry_sandbox = if unsandboxed_allowed {
|
||||
SandboxType::None
|
||||
} else {
|
||||
self.sandbox.select_initial(
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
tool.sandbox_preference(),
|
||||
turn_ctx.windows_sandbox_level,
|
||||
managed_network_active,
|
||||
)
|
||||
};
|
||||
let retry_codex_linux_sandbox_exe = if unsandboxed_allowed {
|
||||
None
|
||||
} else {
|
||||
turn_ctx.codex_linux_sandbox_exe.as_ref()
|
||||
};
|
||||
let retry_attempt = SandboxAttempt {
|
||||
sandbox: retry_sandbox,
|
||||
permissions: &turn_ctx.permission_profile,
|
||||
enforce_managed_network: managed_network_active,
|
||||
manager: &self.sandbox,
|
||||
sandbox_cwd,
|
||||
workspace_roots: workspace_roots.as_slice(),
|
||||
codex_linux_sandbox_exe: None,
|
||||
codex_linux_sandbox_exe: retry_codex_linux_sandbox_exe,
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level: turn_ctx.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_ctx
|
||||
@@ -375,14 +400,9 @@ impl ToolOrchestrator {
|
||||
};
|
||||
|
||||
// Second attempt.
|
||||
let (retry_result, retry_deferred_network_approval) = Self::run_attempt(
|
||||
tool,
|
||||
req,
|
||||
tool_ctx,
|
||||
&escalated_attempt,
|
||||
managed_network_active,
|
||||
)
|
||||
.await;
|
||||
let (retry_result, retry_deferred_network_approval) =
|
||||
Self::run_attempt(tool, req, tool_ctx, &retry_attempt, managed_network_active)
|
||||
.await;
|
||||
retry_result.map(|output| OrchestratorRunResult {
|
||||
output,
|
||||
deferred_network_approval: retry_deferred_network_approval,
|
||||
|
||||
@@ -36,6 +36,7 @@ use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
use crate::tools::sandboxing::sandbox_permissions_preserving_denied_reads;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
@@ -207,8 +208,13 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
req: &ShellRequest,
|
||||
ctx: &ToolCtx,
|
||||
) -> Option<NetworkApprovalSpec> {
|
||||
let file_system_sandbox_policy = ctx.turn.file_system_sandbox_policy();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&file_system_sandbox_policy,
|
||||
);
|
||||
let network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions)?;
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions)?;
|
||||
Some(NetworkApprovalSpec {
|
||||
network: Some(network.clone()),
|
||||
mode: NetworkApprovalMode::Immediate,
|
||||
@@ -233,9 +239,14 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
ctx: &ToolCtx,
|
||||
) -> Result<ExecToolCallOutput, ToolError> {
|
||||
let session_shell = ctx.session.user_shell();
|
||||
let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&file_system_sandbox_policy,
|
||||
);
|
||||
let managed_network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
|
||||
let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions);
|
||||
let env = exec_env_for_sandbox_permissions(&req.env, sandbox_permissions);
|
||||
let explicit_env_overrides = req.explicit_env_overrides.clone();
|
||||
#[cfg(unix)]
|
||||
let (env, explicit_env_overrides) = {
|
||||
|
||||
@@ -22,6 +22,8 @@ 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::sandbox_permissions_preserving_denied_reads;
|
||||
use crate::tools::sandboxing::unsandboxed_execution_allowed;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::MatchOptions;
|
||||
@@ -117,6 +119,15 @@ pub(super) async fn try_run_zsh_fork(
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let (attempt_file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&attempt_file_system_sandbox_policy,
|
||||
);
|
||||
let req = &ShellRequest {
|
||||
sandbox_permissions,
|
||||
..req.clone()
|
||||
};
|
||||
let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
prepend_zsh_fork_bin_to_path(&mut env, shell_zsh_path);
|
||||
let command =
|
||||
@@ -372,11 +383,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
|
||||
@@ -616,8 +634,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
|
||||
@@ -625,10 +645,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(),
|
||||
),
|
||||
};
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
@@ -289,11 +306,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 +321,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 +644,102 @@ 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(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn denied_reads_keep_granular_sandbox_rejection_for_escalation() -> anyhow::Result<()> {
|
||||
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(PolicyParser::new().build())),
|
||||
session: Arc::new(session),
|
||||
turn: Arc::new(turn_context),
|
||||
call_id: "deny-read-granular-sandbox-reject".to_string(),
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_policy_cwd: workdir.clone(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
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(host_absolute_path(&["usr", "bin", "printf"])).unwrap(),
|
||||
&["printf".to_string(), "hello".to_string()],
|
||||
&workdir,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
action,
|
||||
codex_shell_escalation::EscalationDecision::Deny {
|
||||
reason: Some("Execution forbidden by policy".to_string())
|
||||
}
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default() {
|
||||
let policy = PolicyParser::new().build();
|
||||
|
||||
@@ -34,6 +34,7 @@ use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
use crate::tools::sandboxing::sandbox_permissions_preserving_denied_reads;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use crate::unified_exec::NoopSpawnLifecycle;
|
||||
use crate::unified_exec::UnifiedExecError;
|
||||
@@ -230,8 +231,13 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
req: &UnifiedExecRequest,
|
||||
ctx: &ToolCtx,
|
||||
) -> Option<NetworkApprovalSpec> {
|
||||
let file_system_sandbox_policy = ctx.turn.file_system_sandbox_policy();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&file_system_sandbox_policy,
|
||||
);
|
||||
let network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions)?;
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions)?;
|
||||
Some(NetworkApprovalSpec {
|
||||
network: Some(network.clone()),
|
||||
mode: NetworkApprovalMode::Deferred,
|
||||
@@ -257,6 +263,15 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
) -> Result<UnifiedExecProcess, ToolError> {
|
||||
let base_command = &req.command;
|
||||
let session_shell = ctx.session.user_shell();
|
||||
let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions();
|
||||
let sandbox_permissions = sandbox_permissions_preserving_denied_reads(
|
||||
req.sandbox_permissions,
|
||||
&file_system_sandbox_policy,
|
||||
);
|
||||
let req = &UnifiedExecRequest {
|
||||
sandbox_permissions,
|
||||
..req.clone()
|
||||
};
|
||||
let managed_network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
|
||||
let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
|
||||
@@ -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,34 @@ pub(crate) fn sandbox_override_for_first_attempt(
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns true when the active filesystem policy can be represented by
|
||||
/// running without a filesystem sandbox.
|
||||
///
|
||||
/// Denied reads only exist inside the sandbox. If a policy contains any
|
||||
/// denied-read paths, bypassing the sandbox would silently grant those reads,
|
||||
/// so escalation must keep the command sandboxed with the denied reads intact.
|
||||
pub(crate) fn unsandboxed_execution_allowed(
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
) -> bool {
|
||||
!file_system_sandbox_policy.has_denied_read_restrictions()
|
||||
}
|
||||
|
||||
pub(crate) fn sandbox_permissions_preserving_denied_reads(
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
) -> SandboxPermissions {
|
||||
if sandbox_permissions.requires_escalated_permissions()
|
||||
&& !unsandboxed_execution_allowed(file_system_sandbox_policy)
|
||||
{
|
||||
// `RequireEscalated` normally asks the executor to bypass the sandbox.
|
||||
// When denied reads are active, that would drop the only mechanism that
|
||||
// enforces them, so fall back to the default sandboxed attempt instead.
|
||||
SandboxPermissions::UseDefault
|
||||
} else {
|
||||
sandbox_permissions
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn managed_network_for_sandbox_permissions(
|
||||
network: Option<&NetworkProxy>,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
|
||||
@@ -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,28 @@ 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_permissions_preserving_denied_reads(
|
||||
SandboxPermissions::RequireEscalated,
|
||||
&file_system_policy,
|
||||
),
|
||||
SandboxPermissions::UseDefault,
|
||||
);
|
||||
assert_eq!(
|
||||
sandbox_permissions_preserving_denied_reads(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
&file_system_policy,
|
||||
),
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
);
|
||||
assert_eq!(
|
||||
sandbox_permissions_preserving_denied_reads(
|
||||
SandboxPermissions::RequireEscalated,
|
||||
&FileSystemSandboxPolicy::default(),
|
||||
),
|
||||
SandboxPermissions::RequireEscalated,
|
||||
);
|
||||
assert_eq!(
|
||||
sandbox_override_for_first_attempt(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
@@ -167,7 +189,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",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -10,6 +10,12 @@ use codex_features::Feature;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol;
|
||||
use codex_protocol::approvals::NetworkPolicyAmendment;
|
||||
use codex_protocol::approvals::NetworkPolicyRuleAction;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -3244,6 +3250,212 @@ allow_local_binding = true
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn network_approval_retry_keeps_deny_read_sandbox_for_escalated_command() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
fs::write(
|
||||
home.path().join("config.toml"),
|
||||
r#"default_permissions = "workspace"
|
||||
|
||||
[permissions.workspace.filesystem]
|
||||
":minimal" = "read"
|
||||
|
||||
[permissions.workspace.network]
|
||||
enabled = true
|
||||
mode = "limited"
|
||||
allow_local_binding = true
|
||||
"#,
|
||||
)?;
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: true,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
let mut builder = test_codex()
|
||||
.with_home(home)
|
||||
.with_cloud_requirements(managed_network_requirements_loader())
|
||||
.with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
assert!(
|
||||
test.config.managed_network_requirements_enabled(),
|
||||
"expected managed network requirements to be enabled"
|
||||
);
|
||||
assert!(
|
||||
test.config.permissions.network.is_some(),
|
||||
"expected managed network proxy config to be present"
|
||||
);
|
||||
test.session_configured
|
||||
.network_proxy
|
||||
.as_ref()
|
||||
.expect("expected runtime managed network proxy addresses");
|
||||
|
||||
let mut file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
&sandbox_policy,
|
||||
test.config.cwd.as_path(),
|
||||
);
|
||||
file_system_sandbox_policy
|
||||
.entries
|
||||
.push(FileSystemSandboxEntry {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: format!("{}/**/*.env", test.config.cwd.as_path().display()),
|
||||
},
|
||||
access: FileSystemAccessMode::Deny,
|
||||
});
|
||||
assert!(
|
||||
file_system_sandbox_policy.has_denied_read_restrictions(),
|
||||
"test must exercise a permission profile with denied reads"
|
||||
);
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_sandbox_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
|
||||
let call_id = "deny-read-network-retry";
|
||||
let fetch_command = r#"python3 -c "import urllib.request; opener = urllib.request.build_opener(urllib.request.ProxyHandler()); print('OK:' + opener.open('http://codex-network-test.invalid', timeout=30).read().decode(errors='replace'))""#
|
||||
.to_string();
|
||||
let event = shell_event(
|
||||
call_id,
|
||||
&fetch_command,
|
||||
/*timeout_ms*/ 30_000,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
)?;
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-deny-read-network-1"),
|
||||
event,
|
||||
ev_completed("resp-deny-read-network-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-deny-read-network-1", "done"),
|
||||
ev_completed("resp-deny-read-network-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let (turn_sandbox_policy, turn_permission_profile) =
|
||||
turn_permission_fields(permission_profile, test.config.cwd.as_path());
|
||||
let session_model = test.session_configured.model.clone();
|
||||
test.codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "deny-read network retry".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
environments: None,
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
cwd: Some(test.config.cwd.to_path_buf()),
|
||||
approval_policy: Some(approval_policy),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
sandbox_policy: Some(turn_sandbox_policy),
|
||||
permission_profile: turn_permission_profile,
|
||||
collaboration_mode: Some(codex_protocol::config_types::CollaborationMode {
|
||||
mode: codex_protocol::config_types::ModeKind::Default,
|
||||
settings: codex_protocol::config_types::Settings {
|
||||
model: session_model,
|
||||
reasoning_effort: None,
|
||||
developer_instructions: None,
|
||||
},
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
|
||||
let mut command_approval_count = 0;
|
||||
let approval = loop {
|
||||
let remaining = deadline
|
||||
.checked_duration_since(std::time::Instant::now())
|
||||
.expect("timed out waiting for network approval request");
|
||||
let event = wait_for_event_with_timeout(
|
||||
&test.codex,
|
||||
|event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
},
|
||||
remaining,
|
||||
)
|
||||
.await;
|
||||
match event {
|
||||
EventMsg::ExecApprovalRequest(approval) => {
|
||||
if approval.command.first().map(std::string::String::as_str)
|
||||
== Some("network-access")
|
||||
{
|
||||
break approval;
|
||||
}
|
||||
command_approval_count += 1;
|
||||
assert_eq!(
|
||||
command_approval_count, 1,
|
||||
"expected only the outer explicit escalation approval"
|
||||
);
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => {
|
||||
panic!("expected network approval request before completion");
|
||||
}
|
||||
other => panic!("unexpected event: {other:?}"),
|
||||
}
|
||||
};
|
||||
assert_eq!(command_approval_count, 1);
|
||||
let network_context = approval
|
||||
.network_approval_context
|
||||
.clone()
|
||||
.expect("expected network approval context");
|
||||
assert_eq!(network_context.protocol, NetworkApprovalProtocol::Http);
|
||||
let allow_network_amendment = approval
|
||||
.proposed_network_policy_amendments
|
||||
.clone()
|
||||
.expect("expected proposed network policy amendments")
|
||||
.into_iter()
|
||||
.find(|amendment| amendment.action == NetworkPolicyRuleAction::Allow)
|
||||
.expect("expected allow network policy amendment");
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: allow_network_amendment,
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
let output = parse_result(&results.single_request().function_call_output(call_id));
|
||||
assert!(
|
||||
output.exit_code.is_some(),
|
||||
"expected the approved retry to report command output"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn network_approval_flow_survives_danger_full_access_session_start() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user