mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Allow approved filesystem escalation while preserving denied reads (#49353)
## Why Denied-read policies previously kept explicitly approved commands within their original filesystem restrictions, preventing operations that need broader write access, such as updating Git metadata. ## What changed - Apply broader filesystem permissions after approval of explicit escalation while retaining the sandbox, denied reads, and network policy. Command allow rules alone do not widen filesystem access. - Reject escalation when the required local sandbox is unavailable or a remote exec server lacks support for preserving the restrictions. - Track escalated terminal access so subsequent `write_stdin` calls still require review when stdin approval is enabled. ## Testing Add coverage for approved and denied escalation, allow rules, remote server compatibility, and stdin approval before input delivery. Add a Windows integration test verifying that an approved `git pull` updates Git metadata while exact-path and glob-based read denials remain enforced. Serialize Windows sandbox integration tests across processes to protect shared test account credentials. GitOrigin-RevId: 5dce6d41487ee4d39bd795c8d086398ab9794d5b
This commit is contained in:
@@ -29,6 +29,9 @@ use codex_otel::ToolDecisionSource;
|
||||
use codex_protocol::error::CodexErrorDetails;
|
||||
use codex_protocol::error::SandboxErr;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES;
|
||||
use codex_protocol::permissions::file_system_root;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
@@ -222,15 +225,71 @@ impl ToolOrchestrator {
|
||||
|
||||
// 2) First attempt under the selected sandbox.
|
||||
let unsandboxed_allowed = unsandboxed_execution_allowed(&file_system_sandbox_policy);
|
||||
let sandbox_override = if unsandboxed_allowed {
|
||||
sandbox_override_for_first_attempt(
|
||||
tool.sandbox_permissions(req),
|
||||
&requirement,
|
||||
&file_system_sandbox_policy,
|
||||
)
|
||||
let sandbox_override = sandbox_override_for_first_attempt(
|
||||
tool.sandbox_permissions(req),
|
||||
&requirement,
|
||||
&file_system_sandbox_policy,
|
||||
already_approved,
|
||||
);
|
||||
let (escalated_profile, writable_root_metadata) = if sandbox_override
|
||||
== SandboxOverride::EscalatedSandboxWithRestrictions
|
||||
{
|
||||
let context = environment.sandbox_context(/*additional_permissions*/ None);
|
||||
let policy_context = context.policy_context();
|
||||
let baseline = permission_profile.file_system_sandbox_policy();
|
||||
let file_system = baseline.for_approved_command(&policy_context);
|
||||
let writable_root_metadata = file_system_root(&policy_context).is_some_and(|root| {
|
||||
file_system.can_write_path(&root, &policy_context)
|
||||
&& PROTECTED_METADATA_PATH_NAMES.iter().any(|name| {
|
||||
root.join_descendant(name)
|
||||
.is_ok_and(|path| file_system.can_write_path(&path, &policy_context))
|
||||
})
|
||||
});
|
||||
let escalated_profile = (file_system != baseline
|
||||
&& file_system
|
||||
!= baseline.materialize_project_roots_with_path_uris(workspace_roots))
|
||||
.then(|| {
|
||||
PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
permission_profile.enforcement(),
|
||||
&file_system,
|
||||
permission_profile.network_sandbox_policy(),
|
||||
)
|
||||
});
|
||||
(escalated_profile, writable_root_metadata)
|
||||
} else {
|
||||
SandboxOverride::NoOverride
|
||||
(None, false)
|
||||
};
|
||||
if (escalated_profile.is_some() || writable_root_metadata)
|
||||
&& environment.environment.is_remote()
|
||||
{
|
||||
let info = environment.environment.info().await.map_err(|err| {
|
||||
ToolError::Rejected(format!(
|
||||
"could not verify exec-server support for escalation with denied reads: {err}"
|
||||
))
|
||||
})?;
|
||||
if !matches!(info.platform_os.as_deref(), Some("windows" | "macos"))
|
||||
&& !info
|
||||
.capabilities
|
||||
.linux_approved_root_write_preserves_restrictions
|
||||
{
|
||||
return Err(ToolError::Rejected(
|
||||
"this exec-server cannot safely escalate with denied reads; upgrade the exec-server and retry"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
let escalated_local_profile = escalated_profile
|
||||
.as_ref()
|
||||
.filter(|_| !executor_managed_process_sandbox)
|
||||
.map(|profile| {
|
||||
profile
|
||||
.clone()
|
||||
.materialize_project_roots_with_path_uris(workspace_roots)
|
||||
});
|
||||
let initial_permissions = escalated_local_profile
|
||||
.as_ref()
|
||||
.or(escalated_profile.as_ref())
|
||||
.unwrap_or(&permissions);
|
||||
let network_approval_spec = tool.network_approval_spec(req, tool_ctx);
|
||||
// An explicit owner proxy requirement permits filtered egress, like an enabled
|
||||
// controller proxy. Traffic-only owner policies stay offline unless command
|
||||
@@ -265,11 +324,13 @@ impl ToolOrchestrator {
|
||||
let sandbox_preference = tool.sandbox_preference();
|
||||
let sandbox_requested = match sandbox_override {
|
||||
SandboxOverride::BypassSandboxFirstAttempt => false,
|
||||
SandboxOverride::NoOverride => sandbox_manager.should_sandbox(
|
||||
&permissions,
|
||||
sandbox_preference,
|
||||
managed_network_active,
|
||||
),
|
||||
SandboxOverride::NoOverride | SandboxOverride::EscalatedSandboxWithRestrictions => {
|
||||
sandbox_manager.should_sandbox(
|
||||
initial_permissions,
|
||||
sandbox_preference,
|
||||
managed_network_active,
|
||||
)
|
||||
}
|
||||
};
|
||||
let windows_sandbox_type = codex_protocol::sandbox::effective_windows_sandbox_type(
|
||||
sandbox_config.windows_sandbox_type,
|
||||
@@ -277,7 +338,7 @@ impl ToolOrchestrator {
|
||||
);
|
||||
let initial_sandbox = if sandbox_requested && !executor_managed_process_sandbox {
|
||||
sandbox_manager.select_initial(
|
||||
&permissions,
|
||||
initial_permissions,
|
||||
sandbox_preference,
|
||||
windows_sandbox_type,
|
||||
managed_network_active,
|
||||
@@ -285,6 +346,9 @@ impl ToolOrchestrator {
|
||||
} else {
|
||||
SandboxType::None
|
||||
};
|
||||
if !executor_managed_process_sandbox {
|
||||
sandbox_override.ensure_native_sandbox(initial_sandbox)?;
|
||||
}
|
||||
|
||||
let sandbox_policy_cwd = tool
|
||||
.sandbox_cwd(req)
|
||||
@@ -298,8 +362,8 @@ impl ToolOrchestrator {
|
||||
let initial_attempt = SandboxAttempt {
|
||||
sandbox: initial_sandbox,
|
||||
sandbox_requested,
|
||||
permissions: &permissions,
|
||||
exec_server_permissions: permission_profile,
|
||||
permissions: initial_permissions,
|
||||
exec_server_permissions: escalated_profile.as_ref().unwrap_or(permission_profile),
|
||||
enforce_managed_network: managed_network_active,
|
||||
manager: &sandbox_manager,
|
||||
sandbox_cwd: &sandbox_policy_cwd,
|
||||
@@ -449,14 +513,14 @@ impl ToolOrchestrator {
|
||||
|
||||
let retry_sandbox_requested = !unsandboxed_allowed
|
||||
&& sandbox_manager.should_sandbox(
|
||||
&permissions,
|
||||
initial_permissions,
|
||||
sandbox_preference,
|
||||
managed_network_active,
|
||||
);
|
||||
let retry_sandbox = if retry_sandbox_requested && !executor_managed_process_sandbox
|
||||
{
|
||||
sandbox_manager.select_initial(
|
||||
&permissions,
|
||||
initial_permissions,
|
||||
sandbox_preference,
|
||||
windows_sandbox_type,
|
||||
managed_network_active,
|
||||
@@ -472,8 +536,10 @@ impl ToolOrchestrator {
|
||||
let retry_attempt = SandboxAttempt {
|
||||
sandbox: retry_sandbox,
|
||||
sandbox_requested: retry_sandbox_requested,
|
||||
permissions: &permissions,
|
||||
exec_server_permissions: permission_profile,
|
||||
permissions: initial_permissions,
|
||||
exec_server_permissions: escalated_profile
|
||||
.as_ref()
|
||||
.unwrap_or(permission_profile),
|
||||
enforce_managed_network: managed_network_active,
|
||||
manager: &sandbox_manager,
|
||||
sandbox_cwd: &sandbox_policy_cwd,
|
||||
|
||||
@@ -629,6 +629,17 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecAttempt> for UnifiedExecRunt
|
||||
req.additional_permissions.as_ref(),
|
||||
internal_permissions.as_ref(),
|
||||
);
|
||||
let baseline_file_system = req
|
||||
.turn_environment
|
||||
.permission_profile()
|
||||
.file_system_sandbox_policy();
|
||||
let permissions = if baseline_file_system.has_denied_read_restrictions()
|
||||
&& attempt.exec_server_permissions.file_system_sandbox_policy() != baseline_file_system
|
||||
{
|
||||
permissions.with_filesystem_escalation()
|
||||
} else {
|
||||
permissions
|
||||
};
|
||||
|
||||
if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode {
|
||||
let command = build_unified_exec_sandbox_command(
|
||||
|
||||
@@ -233,19 +233,36 @@ pub(crate) fn default_exec_approval_requirement(
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum SandboxOverride {
|
||||
NoOverride,
|
||||
EscalatedSandboxWithRestrictions,
|
||||
BypassSandboxFirstAttempt,
|
||||
}
|
||||
|
||||
impl SandboxOverride {
|
||||
pub(crate) fn ensure_native_sandbox(self, sandbox: SandboxType) -> Result<(), ToolError> {
|
||||
if self == Self::EscalatedSandboxWithRestrictions && sandbox == SandboxType::None {
|
||||
return Err(ToolError::Rejected(
|
||||
"command escalation with denied reads requires an available filesystem sandbox"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn sandbox_override_for_first_attempt(
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
exec_approval_requirement: &ExecApprovalRequirement,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
already_approved: bool,
|
||||
) -> 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.
|
||||
// Only actual approval of an explicit escalation may widen the filesystem;
|
||||
// a command allow rule does not authorize removing filesystem restrictions.
|
||||
if !unsandboxed_execution_allowed(file_system_sandbox_policy) {
|
||||
return SandboxOverride::NoOverride;
|
||||
return if sandbox_permissions.requires_escalated_permissions() && already_approved {
|
||||
SandboxOverride::EscalatedSandboxWithRestrictions
|
||||
} else {
|
||||
SandboxOverride::NoOverride
|
||||
};
|
||||
}
|
||||
|
||||
// ExecPolicy `Allow` can intentionally imply full trust (Skip + bypass_sandbox=true),
|
||||
|
||||
@@ -163,6 +163,7 @@ fn additional_permissions_allow_bypass_sandbox_first_attempt_when_execpolicy_ski
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
&FileSystemSandboxPolicy::default(),
|
||||
/*already_approved*/ false,
|
||||
),
|
||||
SandboxOverride::BypassSandboxFirstAttempt
|
||||
);
|
||||
@@ -178,13 +179,14 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
&FileSystemSandboxPolicy::default(),
|
||||
/*already_approved*/ true,
|
||||
),
|
||||
SandboxOverride::BypassSandboxFirstAttempt
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deny_read_blocks_explicit_escalation_and_policy_bypass() {
|
||||
fn deny_read_preserves_the_sandbox_for_explicit_escalation_and_blocks_policy_bypass() {
|
||||
let file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
@@ -201,11 +203,16 @@ fn deny_read_blocks_explicit_escalation_and_policy_bypass() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
&file_system_policy,
|
||||
/*already_approved*/ true,
|
||||
),
|
||||
SandboxOverride::NoOverride,
|
||||
"explicit escalation would drop deny-read filesystem policy, so keep the first attempt sandboxed",
|
||||
SandboxOverride::EscalatedSandboxWithRestrictions,
|
||||
"explicit escalation must widen the first attempt without dropping the sandbox",
|
||||
);
|
||||
assert!(!unsandboxed_execution_allowed(&file_system_policy));
|
||||
assert!(matches!(
|
||||
SandboxOverride::EscalatedSandboxWithRestrictions.ensure_native_sandbox(SandboxType::None),
|
||||
Err(ToolError::Rejected(reason)) if reason.contains("requires an available filesystem sandbox"),
|
||||
));
|
||||
assert_eq!(
|
||||
sandbox_permissions_preserving_denied_reads(
|
||||
SandboxPermissions::RequireEscalated,
|
||||
@@ -235,6 +242,7 @@ fn deny_read_blocks_explicit_escalation_and_policy_bypass() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
&file_system_policy,
|
||||
/*already_approved*/ true,
|
||||
),
|
||||
SandboxOverride::NoOverride,
|
||||
"exec-policy allow rules would drop deny-read filesystem policy, so keep the first attempt sandboxed",
|
||||
|
||||
@@ -33,6 +33,7 @@ pub(crate) struct TerminalPermissions {
|
||||
policy: TerminalPolicy,
|
||||
sandbox_source: TerminalSandboxSource,
|
||||
launch_permissions: SandboxPermissions,
|
||||
filesystem_escalated: bool,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
internal_permissions: Option<AdditionalPermissionProfile>,
|
||||
}
|
||||
@@ -100,14 +101,19 @@ impl TerminalPermissions {
|
||||
merge_permission_profiles(additional_permissions, internal_permissions),
|
||||
),
|
||||
sandbox_source,
|
||||
// A bypass is a property of the successful attempt, not a difference
|
||||
// between settings: a full-access environment can still bypass a proxy.
|
||||
// Retain the successful attempt's authority independently from its baseline.
|
||||
launch_permissions,
|
||||
filesystem_escalated: false,
|
||||
additional_permissions: additional_permissions.cloned(),
|
||||
internal_permissions: internal_permissions.cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_filesystem_escalation(mut self) -> Self {
|
||||
self.filesystem_escalated = true;
|
||||
self
|
||||
}
|
||||
|
||||
/// Compares launch policy with current policy including retained grants,
|
||||
/// without reading turn state or touching a process.
|
||||
fn review_requirement(
|
||||
@@ -129,15 +135,17 @@ impl TerminalPermissions {
|
||||
}
|
||||
// Runtime-internal grants are part of an ordinary launch, so only permissions
|
||||
// beyond the baseline plus those grants need a fresh stdin approval.
|
||||
Ok(if bypassed || &self.policy != current {
|
||||
SandboxPermissions::RequireEscalated
|
||||
} else if self.policy.sandbox.permissions
|
||||
== effective_permission_profile(baseline, self.internal_permissions.as_ref())
|
||||
{
|
||||
SandboxPermissions::UseDefault
|
||||
} else {
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
})
|
||||
Ok(
|
||||
if bypassed || self.filesystem_escalated || &self.policy != current {
|
||||
SandboxPermissions::RequireEscalated
|
||||
} else if self.policy.sandbox.permissions
|
||||
== effective_permission_profile(baseline, self.internal_permissions.as_ref())
|
||||
{
|
||||
SandboxPermissions::UseDefault
|
||||
} else {
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
fn approval_reason(
|
||||
@@ -146,6 +154,8 @@ impl TerminalPermissions {
|
||||
) -> Result<String, serde_json::Error> {
|
||||
let authority = if self.launch_permissions.requires_escalated_permissions() {
|
||||
"This terminal was launched outside the sandbox, bypassing any managed network proxy."
|
||||
} else if self.filesystem_escalated {
|
||||
"This terminal has approved broader filesystem access; denied reads are still enforced."
|
||||
} else if self.policy.sandbox.permissions == PermissionProfile::Disabled {
|
||||
"This terminal runs without a filesystem sandbox."
|
||||
} else {
|
||||
|
||||
@@ -29,6 +29,7 @@ fn terminal_permissions(profile: &PermissionProfile) -> TerminalPermissions {
|
||||
},
|
||||
sandbox_source: TerminalSandboxSource::Native,
|
||||
launch_permissions: SandboxPermissions::UseDefault,
|
||||
filesystem_escalated: false,
|
||||
additional_permissions: None,
|
||||
internal_permissions: None,
|
||||
}
|
||||
|
||||
@@ -3802,6 +3802,102 @@ touch {outside_path:?}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test_case(Some(ReviewDecision::Approved); "approved")]
|
||||
#[test_case(Some(ReviewDecision::denied("rejected by user")); "denied")]
|
||||
#[test_case(None; "allow_rule_skips_approval_without_widening")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn explicit_escalation_with_denied_reads(decision: Option<ReviewDecision>) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = start_mock_server().await;
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
if decision.is_none() {
|
||||
let rules = home.path().join("rules");
|
||||
fs::create_dir(&rules)?;
|
||||
fs::write(
|
||||
rules.join("default.rules"),
|
||||
r#"prefix_rule(pattern=["/bin/sh", "escalation.sh"], decision="allow")"#,
|
||||
)?;
|
||||
}
|
||||
let outside_marker = home.path().join("escalated-marker");
|
||||
let quoted_marker = shlex::try_join([outside_marker.to_string_lossy().as_ref()])?;
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let mut builder = test_codex()
|
||||
.with_home(home)
|
||||
.with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.features.enable(Feature::UnifiedExec).unwrap();
|
||||
config.features.disable(Feature::ShellZshFork).unwrap();
|
||||
let mut file_system =
|
||||
restrictive_workspace_write_profile().file_system_sandbox_policy();
|
||||
file_system.entries.push(FileSystemSandboxEntry::new(
|
||||
config.cwd.join("secret.txt").into(),
|
||||
FileSystemAccessMode::Deny,
|
||||
));
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::from_runtime_permissions(
|
||||
&file_system,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
))
|
||||
.unwrap();
|
||||
})
|
||||
.with_workspace_setup(move |cwd, _fs| async move {
|
||||
fs::write(cwd.join("secret.txt"), "secret")?;
|
||||
fs::write(
|
||||
cwd.join("escalation.sh"),
|
||||
format!(
|
||||
"printf ran > ran || exit 10; (printf outside > {quoted_marker}) 2> errors; \
|
||||
if cat secret.txt > errors 2>&1; then exit 11; fi; \
|
||||
grep -q secret.txt errors || exit 12; printf 'protected\\n'"
|
||||
),
|
||||
)?;
|
||||
Ok(())
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
let workspace_marker = test.cwd.path().join("ran");
|
||||
let command = "/bin/sh escalation.sh";
|
||||
let call_id = "explicit-deny-read-escalation";
|
||||
let event = exec_command_event(
|
||||
call_id,
|
||||
command,
|
||||
Some(10_000),
|
||||
SandboxPermissions::RequireEscalated,
|
||||
/*justification*/ None,
|
||||
)?;
|
||||
mount_sse_once(&server, sse(vec![event, ev_completed("run")])).await;
|
||||
let response = mount_sse_once(&server, sse(vec![ev_completed("done")])).await;
|
||||
submit_turn_preserving_active_permission_profile(&test, "run the command", approval_policy)
|
||||
.await?;
|
||||
let approved = decision == Some(ReviewDecision::Approved);
|
||||
let rejected = decision.is_some() && !approved;
|
||||
if let Some(decision) = decision {
|
||||
let request = expect_exec_approval(&test, command).await;
|
||||
assert!(!outside_marker.exists() && !workspace_marker.exists());
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: request.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
wait_for_completion_without_approval(&test).await;
|
||||
let result = parse_result(&response.single_request().function_call_output(call_id));
|
||||
if rejected {
|
||||
assert!(result.stdout.contains("rejected by user"), "{result:?}");
|
||||
assert!(!workspace_marker.exists());
|
||||
} else {
|
||||
assert_eq!(
|
||||
(result.exit_code, result.stdout.as_str()),
|
||||
(Some(0), "protected\n")
|
||||
);
|
||||
assert_eq!(fs::read_to_string(workspace_marker)?, "ran");
|
||||
}
|
||||
assert_eq!(outside_marker.exists(), approved);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Verifies that zsh-fork applies an inner script's allow rule even when the
|
||||
/// model invokes an outer wrapper, and that the escalated script retains the
|
||||
/// named profile needed to reconstruct the original sandbox remotely without
|
||||
|
||||
@@ -4,6 +4,7 @@ use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_exec_server::ExecParams;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
@@ -13,8 +14,11 @@ use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::permissions::ReadDenyMatcher;
|
||||
use codex_protocol::permissions::project_roots_glob_pattern;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -44,6 +48,9 @@ use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
use test_case::test_case;
|
||||
use tokio::net::TcpListener;
|
||||
@@ -65,6 +72,11 @@ const REPLAY_RETAINED_OUTPUT_SEQ: u64 = 800;
|
||||
enum PushedExecScenario {
|
||||
Complete,
|
||||
DirectDenied,
|
||||
DenyWin,
|
||||
DenyWinUnchanged,
|
||||
DenyOldLinux,
|
||||
DenyOldLinuxWithRoot(FileSystemAccessMode),
|
||||
DenyLinux,
|
||||
ElevatedPowerShell,
|
||||
RejectedLongWindowsDangerousCommand,
|
||||
SandboxedInterceptedPatch,
|
||||
@@ -156,18 +168,34 @@ async fn respond_environment_info(
|
||||
) {
|
||||
let shell = if matches!(
|
||||
scenario,
|
||||
PushedExecScenario::ElevatedPowerShell
|
||||
PushedExecScenario::DenyWin
|
||||
| PushedExecScenario::DenyWinUnchanged
|
||||
| PushedExecScenario::ElevatedPowerShell
|
||||
| PushedExecScenario::RejectedLongWindowsDangerousCommand
|
||||
) {
|
||||
json!({ "name": "powershell", "path": "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" })
|
||||
} else {
|
||||
json!({ "name": "zsh", "path": "/bin/zsh" })
|
||||
};
|
||||
let platform_os = matches!(
|
||||
let platform_os = match scenario {
|
||||
PushedExecScenario::DenyWin
|
||||
| PushedExecScenario::DenyWinUnchanged
|
||||
| PushedExecScenario::RejectedLongWindowsDangerousCommand => Some("windows"),
|
||||
PushedExecScenario::DenyOldLinux
|
||||
| PushedExecScenario::DenyOldLinuxWithRoot(_)
|
||||
| PushedExecScenario::DenyLinux => Some("linux"),
|
||||
_ => None,
|
||||
};
|
||||
let mut capabilities = json!({ "networkProxyLaunch": true });
|
||||
if matches!(
|
||||
scenario,
|
||||
PushedExecScenario::RejectedLongWindowsDangerousCommand
|
||||
)
|
||||
.then_some("windows");
|
||||
PushedExecScenario::DenyOldLinuxWithRoot(_) | PushedExecScenario::DenyLinux
|
||||
) {
|
||||
capabilities["linuxRootWritePreservesDevices"] = json!(true);
|
||||
}
|
||||
if matches!(scenario, PushedExecScenario::DenyLinux) {
|
||||
capabilities["linuxApprovedRootWritePreservesRestrictions"] = json!(true);
|
||||
}
|
||||
send_exec_server_json(
|
||||
websocket,
|
||||
json!({
|
||||
@@ -175,7 +203,7 @@ async fn respond_environment_info(
|
||||
"result": {
|
||||
"shell": shell,
|
||||
"platformOs": platform_os,
|
||||
"capabilities": { "networkProxyLaunch": true }
|
||||
"capabilities": capabilities,
|
||||
}
|
||||
}),
|
||||
)
|
||||
@@ -185,6 +213,7 @@ async fn respond_environment_info(
|
||||
async fn serve_exec_with_pushed_events(
|
||||
listener: TcpListener,
|
||||
scenario: PushedExecScenario,
|
||||
process_started: Arc<AtomicBool>,
|
||||
) -> PushedExecServerResult {
|
||||
let unrestricted_patch = matches!(
|
||||
scenario,
|
||||
@@ -198,7 +227,10 @@ async fn serve_exec_with_pushed_events(
|
||||
// The runtime may still be finishing local setup before its first tool call.
|
||||
let request = read_exec_server_json(&mut websocket, STARTUP_TIMEOUT).await;
|
||||
match request["method"].as_str() {
|
||||
Some("process/start") => break request,
|
||||
Some("process/start") => {
|
||||
process_started.store(true, Ordering::SeqCst);
|
||||
break request;
|
||||
}
|
||||
Some("environment/info") => {
|
||||
respond_environment_info(&mut websocket, &request["id"], scenario).await;
|
||||
}
|
||||
@@ -372,7 +404,13 @@ async fn serve_exec_with_pushed_events(
|
||||
.await;
|
||||
|
||||
match scenario {
|
||||
PushedExecScenario::Complete | PushedExecScenario::ElevatedPowerShell => {
|
||||
PushedExecScenario::Complete
|
||||
| PushedExecScenario::DenyWin
|
||||
| PushedExecScenario::DenyWinUnchanged
|
||||
| PushedExecScenario::DenyOldLinux
|
||||
| PushedExecScenario::DenyOldLinuxWithRoot(_)
|
||||
| PushedExecScenario::DenyLinux
|
||||
| PushedExecScenario::ElevatedPowerShell => {
|
||||
let encoded_output = BASE64_STANDARD.encode(COMPLETE_OUTPUT);
|
||||
for message in [
|
||||
json!({
|
||||
@@ -475,8 +513,13 @@ async fn serve_exec_with_pushed_events(
|
||||
"failure": null,
|
||||
"sandboxDenied": true,
|
||||
}),
|
||||
PushedExecScenario::ElevatedPowerShell => {
|
||||
panic!("elevated remote PowerShell must not read a remote process")
|
||||
PushedExecScenario::DenyWin
|
||||
| PushedExecScenario::DenyWinUnchanged
|
||||
| PushedExecScenario::DenyOldLinux
|
||||
| PushedExecScenario::DenyOldLinuxWithRoot(_)
|
||||
| PushedExecScenario::DenyLinux
|
||||
| PushedExecScenario::ElevatedPowerShell => {
|
||||
panic!("completed remote PowerShell must not read a remote process")
|
||||
}
|
||||
PushedExecScenario::RejectedLongWindowsDangerousCommand => {
|
||||
panic!("dangerous command must not read a remote process")
|
||||
@@ -571,6 +614,12 @@ async fn serve_exec_with_pushed_events(
|
||||
#[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Disabled, false ; "disabled_managed_network_omits_executor_proxy_launch")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: true }, true ; "foreign_windows_managed_network_preserves_approval_registration"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, ManagedNetworkScenario::None, true ; "foreign_windows_workspace_sandbox"))]
|
||||
#[test_case(PushedExecScenario::DenyWin, ManagedNetworkScenario::None, true ; "old_windows_deny_read_allows_approved_escalation")]
|
||||
#[test_case(PushedExecScenario::DenyWinUnchanged, ManagedNetworkScenario::None, true ; "old_windows_deny_read_keeps_unmaterializable_policy_unchanged")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::DenyOldLinux, ManagedNetworkScenario::None, false ; "old_linux_deny_read_rejects_approved_escalation"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::DenyOldLinuxWithRoot(FileSystemAccessMode::Deny), ManagedNetworkScenario::None, false ; "old_linux_deny_read_keeps_root_deny_unchanged"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::DenyOldLinuxWithRoot(FileSystemAccessMode::Write), ManagedNetworkScenario::None, false ; "device_only_linux_deny_read_rejects_unchanged_writable_root_metadata"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::DenyLinux, ManagedNetworkScenario::None, false ; "linux_deny_read_allows_approved_escalation"))]
|
||||
#[test_case(PushedExecScenario::ElevatedPowerShell, ManagedNetworkScenario::None, true ; "windows_elevated_powershell_disables_profile")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::RejectedLongWindowsDangerousCommand, ManagedNetworkScenario::None, true ; "remote_windows_dangerous_command_rejection_is_bounded"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedInterceptedPatch, ManagedNetworkScenario::None, true ; "foreign_windows_intercepted_patch_is_sandboxed"))]
|
||||
@@ -593,9 +642,27 @@ async fn exec_command_consumes_pushed_remote_process_events(
|
||||
policy_callbacks: true
|
||||
}
|
||||
);
|
||||
let deny_read = matches!(
|
||||
scenario,
|
||||
PushedExecScenario::DenyWin
|
||||
| PushedExecScenario::DenyWinUnchanged
|
||||
| PushedExecScenario::DenyOldLinux
|
||||
| PushedExecScenario::DenyOldLinuxWithRoot(_)
|
||||
| PushedExecScenario::DenyLinux
|
||||
);
|
||||
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let server = start_mock_server().await;
|
||||
let tool_call = match scenario {
|
||||
_ if deny_read => ev_function_call(
|
||||
CALL_ID,
|
||||
"exec_command",
|
||||
&json!({
|
||||
"cmd": "codex-deny-read-escalation-test",
|
||||
"sandbox_permissions": "require_escalated",
|
||||
"justification": "Can I run outside the workspace?",
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
PushedExecScenario::SandboxedDirectPatch
|
||||
| PushedExecScenario::SandboxedDirectPatchDenied
|
||||
| PushedExecScenario::SandboxedDirectPatchRetry => ev_apply_patch_custom_tool_call(
|
||||
@@ -642,7 +709,12 @@ async fn exec_command_consumes_pushed_remote_process_events(
|
||||
)
|
||||
.await;
|
||||
let exec_server_url = format!("ws://{}", listener.local_addr()?);
|
||||
let exec_server = tokio::spawn(serve_exec_with_pushed_events(listener, scenario));
|
||||
let process_started = Arc::new(AtomicBool::new(false));
|
||||
let exec_server = tokio::spawn(serve_exec_with_pushed_events(
|
||||
listener,
|
||||
scenario,
|
||||
Arc::clone(&process_started),
|
||||
));
|
||||
let mut builder = test_codex().with_exec_server_url(exec_server_url);
|
||||
if managed_network_configured {
|
||||
let cloud_config_bundle = match managed_network {
|
||||
@@ -705,7 +777,8 @@ timeout = 900
|
||||
}
|
||||
let mut builder = builder.with_config(move |config| {
|
||||
config.project_doc_max_bytes = 0;
|
||||
if matches!(scenario, PushedExecScenario::ElevatedPowerShell) {
|
||||
if matches!(scenario, PushedExecScenario::ElevatedPowerShell) || (foreign_cwd && deny_read)
|
||||
{
|
||||
config.set_windows_elevated_sandbox_enabled(/*value*/ true);
|
||||
}
|
||||
if managed_network_configured {
|
||||
@@ -719,6 +792,45 @@ timeout = 900
|
||||
|
||||
let turn_permission_profile = if managed_network_configured {
|
||||
test.session_configured.permission_profile.clone()
|
||||
} else if deny_read {
|
||||
let mut filesystem = FileSystemSandboxPolicy::read_only();
|
||||
if let PushedExecScenario::DenyOldLinuxWithRoot(access) = scenario {
|
||||
filesystem.entries[0].access = access;
|
||||
}
|
||||
filesystem.entries.extend(
|
||||
[
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(Some("secret.txt".into())),
|
||||
},
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: project_roots_glob_pattern("*.token".as_ref()),
|
||||
},
|
||||
]
|
||||
.map(|path| FileSystemSandboxEntry::new(path, FileSystemAccessMode::Deny)),
|
||||
);
|
||||
if matches!(scenario, PushedExecScenario::DenyWinUnchanged) {
|
||||
filesystem.entries.push(FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Tmpdir,
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
));
|
||||
}
|
||||
if matches!(
|
||||
scenario,
|
||||
PushedExecScenario::DenyOldLinuxWithRoot(FileSystemAccessMode::Write)
|
||||
) {
|
||||
let cwd = PathUri::parse("file:///workspace")?;
|
||||
let context = FileSystemSandboxPolicyContext {
|
||||
cwd: &cwd,
|
||||
workspace_roots: std::slice::from_ref(&cwd),
|
||||
user_home_dir: None,
|
||||
temporary_directories: None,
|
||||
};
|
||||
filesystem = filesystem.for_approved_command(&context);
|
||||
assert_eq!(filesystem.for_approved_command(&context), filesystem);
|
||||
}
|
||||
PermissionProfile::from_runtime_permissions(&filesystem, NetworkSandboxPolicy::Enabled)
|
||||
} else if matches!(scenario, PushedExecScenario::FullDiskInterceptedPatch) {
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
|
||||
@@ -757,7 +869,7 @@ timeout = 900
|
||||
PermissionProfile::Disabled
|
||||
};
|
||||
let (sandbox_policy, permission_profile) =
|
||||
turn_permission_fields(turn_permission_profile, test.config.cwd.as_path());
|
||||
turn_permission_fields(turn_permission_profile.clone(), test.config.cwd.as_path());
|
||||
test.codex
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
@@ -765,18 +877,26 @@ timeout = 900
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
environments: foreign_cwd.then(|| {
|
||||
let cwd = PathUri::parse("file:///C:/workspace").expect("valid Windows cwd");
|
||||
environments: (foreign_cwd || deny_read).then(|| {
|
||||
let cwd = PathUri::parse(if foreign_cwd {
|
||||
"file:///C:/workspace"
|
||||
} else {
|
||||
"file:///workspace"
|
||||
})
|
||||
.expect("valid executor cwd");
|
||||
let mut workspace_roots = vec![cwd.clone()];
|
||||
if foreign_cwd {
|
||||
workspace_roots.push(
|
||||
PathUri::parse("file:///D:/other-workspace")
|
||||
.expect("valid Windows workspace root"),
|
||||
);
|
||||
}
|
||||
TurnEnvironmentSelections::new(
|
||||
test.config.cwd.clone(),
|
||||
vec![TurnEnvironmentSelection {
|
||||
environment_id: codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
cwd: cwd.clone(),
|
||||
workspace_roots: vec![
|
||||
cwd,
|
||||
PathUri::parse("file:///D:/other-workspace")
|
||||
.expect("valid Windows workspace root"),
|
||||
],
|
||||
cwd,
|
||||
workspace_roots,
|
||||
config: EnvironmentConfigState::FromThread,
|
||||
}],
|
||||
)
|
||||
@@ -788,6 +908,7 @@ timeout = 900
|
||||
PushedExecScenario::SandboxedDirectPatchDenied
|
||||
| PushedExecScenario::SandboxedDirectPatchRetry
|
||||
)
|
||||
|| deny_read
|
||||
{
|
||||
AskForApproval::OnRequest
|
||||
} else {
|
||||
@@ -820,6 +941,16 @@ timeout = 900
|
||||
EventMsg::ExecCommandBegin(event) if event.call_id == CALL_ID => {
|
||||
saw_exec_command_begin = true;
|
||||
}
|
||||
EventMsg::ExecApprovalRequest(approval) if deny_read => {
|
||||
assert_eq!(approval.call_id, CALL_ID);
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
EventMsg::ApplyPatchApprovalRequest(approval)
|
||||
if matches!(
|
||||
scenario,
|
||||
@@ -849,6 +980,24 @@ timeout = 900
|
||||
}
|
||||
}
|
||||
}
|
||||
if matches!(
|
||||
scenario,
|
||||
PushedExecScenario::DenyOldLinux
|
||||
| PushedExecScenario::DenyOldLinuxWithRoot(FileSystemAccessMode::Write)
|
||||
) {
|
||||
let request = response_mock.last_request().context("upgrade response")?;
|
||||
let (output, _) = request
|
||||
.function_call_output_content_and_success(CALL_ID)
|
||||
.context("upgrade output")?;
|
||||
assert!(
|
||||
output
|
||||
.expect("upgrade rejection should include output")
|
||||
.contains("upgrade the exec-server")
|
||||
);
|
||||
assert!(!process_started.load(Ordering::SeqCst));
|
||||
exec_server.abort();
|
||||
return Ok(());
|
||||
}
|
||||
if matches!(
|
||||
scenario,
|
||||
PushedExecScenario::RejectedLongWindowsDangerousCommand
|
||||
@@ -998,7 +1147,7 @@ timeout = 900
|
||||
"toolCallId": CALL_ID,
|
||||
}),
|
||||
);
|
||||
if foreign_cwd && !managed_network_configured {
|
||||
if foreign_cwd && !managed_network_configured && !deny_read {
|
||||
let permissions = response_mock.requests()[0]
|
||||
.message_input_texts("developer")
|
||||
.join("\n")
|
||||
@@ -1036,10 +1185,40 @@ timeout = 900
|
||||
.is_some_and(|argv| argv.iter().any(|arg| arg == "-NoProfile")),
|
||||
"elevated remote PowerShell must not load a user profile"
|
||||
);
|
||||
} else if deny_read {
|
||||
assert_eq!(params["sandbox"]["windowsSandboxLevel"], "elevated");
|
||||
} else {
|
||||
assert_eq!(params["sandbox"]["windowsSandboxLevel"], "restricted-token");
|
||||
}
|
||||
}
|
||||
if deny_read {
|
||||
let params = &exec_server_result.process_start["params"];
|
||||
let params: ExecParams = serde_json::from_value(params.clone())?;
|
||||
let sandbox = params.sandbox.context("sandbox with denied reads")?;
|
||||
let context = sandbox.policy_context();
|
||||
let policy = sandbox.permissions.file_system_sandbox_policy();
|
||||
let policy = policy.materialize_project_roots_with_path_uris(context.workspace_roots);
|
||||
if matches!(
|
||||
scenario,
|
||||
PushedExecScenario::DenyWinUnchanged | PushedExecScenario::DenyOldLinuxWithRoot(_)
|
||||
) {
|
||||
let baseline = turn_permission_profile.file_system_sandbox_policy();
|
||||
assert_eq!(
|
||||
policy,
|
||||
baseline.materialize_project_roots_with_path_uris(context.workspace_roots)
|
||||
);
|
||||
assert!(!policy.can_write_path(&context.cwd.join("../outside")?, &context));
|
||||
} else {
|
||||
let matcher =
|
||||
ReadDenyMatcher::from_context(&policy, &context).context("denied reads")?;
|
||||
assert!(policy.can_write_path(&context.cwd.join("../outside")?, &context));
|
||||
for root in context.workspace_roots {
|
||||
for path in ["secret.txt", "private.token"] {
|
||||
assert!(matcher.is_read_denied_uri(&root.join(path)?, &context));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if managed_network_enabled {
|
||||
let params = &exec_server_result.process_start["params"];
|
||||
assert_eq!(params["enforceManagedNetwork"], true);
|
||||
@@ -1078,7 +1257,13 @@ timeout = 900
|
||||
let output = output.context("exec_command output should contain text")?;
|
||||
let process_read_requests = exec_server_result.process_read_requests;
|
||||
match scenario {
|
||||
PushedExecScenario::Complete | PushedExecScenario::ElevatedPowerShell => {
|
||||
PushedExecScenario::Complete
|
||||
| PushedExecScenario::DenyWin
|
||||
| PushedExecScenario::DenyWinUnchanged
|
||||
| PushedExecScenario::DenyOldLinux
|
||||
| PushedExecScenario::DenyOldLinuxWithRoot(_)
|
||||
| PushedExecScenario::DenyLinux
|
||||
| PushedExecScenario::ElevatedPowerShell => {
|
||||
assert_ne!(success, Some(false));
|
||||
assert!(saw_exec_command_begin);
|
||||
assert!(output.contains("Process exited with code 0"));
|
||||
|
||||
@@ -9,6 +9,10 @@ use codex_features::Feature;
|
||||
use codex_protocol::approvals::ExecApprovalKind;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
@@ -192,6 +196,123 @@ async fn stdin_reviews_retained_grants_after_turn_permissions_expire() -> Result
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn stdin_reviews_escalated_terminal_with_denied_reads_before_delivering_input() -> Result<()>
|
||||
{
|
||||
skip_if_target_windows!(Ok(()), "uses a POSIX interactive shell");
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
let harness = TestCodexHarness::with_auto_env_builder(test_codex().with_config(|config| {
|
||||
config.features.enable(Feature::WriteStdinApproval).unwrap();
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ExecPermissionApprovals)
|
||||
.unwrap();
|
||||
config.features.enable(Feature::UnifiedExec).unwrap();
|
||||
config.features.disable(Feature::ShellZshFork).unwrap();
|
||||
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
let mut file_system = PermissionProfile::read_only().file_system_sandbox_policy();
|
||||
file_system.entries.push(FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: "**/secret.env".into(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
));
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::from_runtime_permissions(
|
||||
&file_system,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
))
|
||||
.unwrap();
|
||||
}))
|
||||
.await?;
|
||||
let codex = &harness.test().codex;
|
||||
harness.write_file("secret.env", "secret").await?;
|
||||
let _open = mount_function_call_agent_response(
|
||||
harness.server(),
|
||||
"open",
|
||||
&json!({"cmd":"/bin/bash --noprofile --norc", "tty":true, "yield_time_ms":200, "sandbox_permissions":"require_escalated", "justification":"open an escalated terminal"}).to_string(),
|
||||
"exec_command",
|
||||
).await;
|
||||
codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "open terminal".into(),
|
||||
text_elements: Vec::new(),
|
||||
}]))
|
||||
.await?;
|
||||
let launch = wait_for_event_match(codex, |event| match event {
|
||||
EventMsg::ExecApprovalRequest(request) => Some(request.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
assert_eq!(launch.kind, ExecApprovalKind::Command);
|
||||
codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: launch.effective_approval_id(),
|
||||
turn_id: Some(launch.turn_id),
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let _writes = mount_sse_sequence(harness.server(), vec![
|
||||
tool_response("denied", "write_stdin", json!({"session_id":1000,"chars":"printf denied > denied-marker\n"})),
|
||||
tool_response("allowed", "write_stdin", json!({"session_id":1000,"chars":"if test -e denied-marker || cat secret.env; then exit 12; fi; printf approved > approved-marker; exit\n"})),
|
||||
sse(vec![ev_completed("done")]),
|
||||
]).await;
|
||||
codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "send input".into(),
|
||||
text_elements: Vec::new(),
|
||||
}]))
|
||||
.await?;
|
||||
for (id, decision) in [
|
||||
("denied", ReviewDecision::denied("blocked input")),
|
||||
("allowed", ReviewDecision::Approved),
|
||||
] {
|
||||
let request = wait_for_event_match(codex, |event| match event {
|
||||
EventMsg::ExecApprovalRequest(request) => Some(request.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
assert_eq!(
|
||||
(
|
||||
request.kind,
|
||||
request.call_id.as_str(),
|
||||
request.effective_approval_id().as_str()
|
||||
),
|
||||
(ExecApprovalKind::WriteStdin, "open", id)
|
||||
);
|
||||
assert!(!harness.path_exists("denied-marker").await?);
|
||||
assert!(!harness.path_exists("approved-marker").await?);
|
||||
codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: request.effective_approval_id(),
|
||||
turn_id: Some(request.turn_id),
|
||||
decision,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
let mut process_end = None;
|
||||
let mut turn_completed = false;
|
||||
while process_end.is_none() || !turn_completed {
|
||||
match wait_for_event(codex, |_| true).await {
|
||||
EventMsg::ExecCommandEnd(event) if event.call_id == "open" => process_end = Some(event),
|
||||
EventMsg::TurnComplete(_) => turn_completed = true,
|
||||
EventMsg::ExecApprovalRequest(request) => {
|
||||
anyhow::bail!("unexpected additional stdin approval: {request:?}")
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
let process_end = process_end.expect("retained terminal should finish");
|
||||
assert_eq!(process_end.exit_code, 0);
|
||||
assert!(!harness.path_exists("denied-marker").await?);
|
||||
assert_eq!(harness.read_file_text("approved-marker").await?, "approved");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn strict_stdin_review_reaches_guardian_with_sandbox_prompts_disabled() -> Result<()> {
|
||||
skip_if_target_windows!(Ok(()), "uses a POSIX interactive shell");
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
use anyhow::Context;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec::process_exec_tool_call;
|
||||
@@ -17,6 +20,11 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
@@ -26,14 +34,18 @@ use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::test_codex::TestCodexHarness;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_with_timeout;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use serial_test::serial;
|
||||
use std::collections::HashMap;
|
||||
use std::ffi::OsString;
|
||||
use std::fs::File;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
use tempfile::TempDir;
|
||||
|
||||
struct EnvVarGuard {
|
||||
@@ -63,20 +75,32 @@ impl Drop for EnvVarGuard {
|
||||
}
|
||||
|
||||
enum TestCodexHome {
|
||||
Persistent(PathBuf),
|
||||
Temporary(TempDir),
|
||||
Persistent(PathBuf, File),
|
||||
Temporary(TempDir, File),
|
||||
}
|
||||
|
||||
impl TestCodexHome {
|
||||
fn path(&self) -> &Path {
|
||||
match self {
|
||||
Self::Persistent(path) => path.as_path(),
|
||||
Self::Temporary(temp_dir) => temp_dir.path(),
|
||||
Self::Persistent(path, _account_lock) => path.as_path(),
|
||||
Self::Temporary(temp_dir, _account_lock) => temp_dir.path(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn codex_home_for_windows_sandbox_test(name: &str) -> anyhow::Result<TestCodexHome> {
|
||||
// Bazel shards and nextest processes share Windows accounts. Hold this lock
|
||||
// through the test so they cannot rotate each other's credentials mid-launch.
|
||||
let lock_path = dirs::data_local_dir()
|
||||
.context("resolve local app data for the Windows sandbox test lock")?
|
||||
.join("codex-windows-sandbox-integration-test.lock");
|
||||
let lock = File::options()
|
||||
.read(true)
|
||||
.append(true)
|
||||
.create(true)
|
||||
.open(lock_path)?;
|
||||
lock.lock().context("lock Windows sandbox test accounts")?;
|
||||
|
||||
if let Some(test_tmpdir) = std::env::var_os("TEST_TMPDIR") {
|
||||
// The elevated backend provisions machine-local sandbox users. Bazel
|
||||
// retries run in the same Windows VM, so keep CODEX_HOME stable within
|
||||
@@ -84,10 +108,10 @@ fn codex_home_for_windows_sandbox_test(name: &str) -> anyhow::Result<TestCodexHo
|
||||
let codex_home = PathBuf::from(test_tmpdir).join(name);
|
||||
std::fs::create_dir_all(&codex_home)
|
||||
.with_context(|| format!("create stable test CODEX_HOME {}", codex_home.display()))?;
|
||||
return Ok(TestCodexHome::Persistent(codex_home));
|
||||
return Ok(TestCodexHome::Persistent(codex_home, lock));
|
||||
}
|
||||
|
||||
Ok(TestCodexHome::Temporary(TempDir::new()?))
|
||||
Ok(TestCodexHome::Temporary(TempDir::new()?, lock))
|
||||
}
|
||||
|
||||
fn stage_windows_sandbox_helpers() -> anyhow::Result<()> {
|
||||
@@ -825,3 +849,154 @@ async fn windows_elevated_unified_exec_enforces_large_recursive_deny_reads() ->
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial(codex_home)]
|
||||
async fn windows_elevated_approved_git_pull_preserves_deny_read() -> anyhow::Result<()> {
|
||||
use EventMsg::ExecApprovalRequest;
|
||||
use EventMsg::ExecCommandEnd;
|
||||
use EventMsg::TurnComplete;
|
||||
|
||||
let codex_home = codex_home_for_windows_sandbox_test("windows-elevated-git-pull-deny-read")?;
|
||||
let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", codex_home.path().as_os_str());
|
||||
stage_windows_sandbox_helpers()?;
|
||||
let home = dunce::canonicalize(codex_home.path())?.abs();
|
||||
let builder = test_codex()
|
||||
.with_windows_cmd_shell()
|
||||
.with_config(move |config| {
|
||||
config.codex_home = home;
|
||||
config.set_windows_elevated_sandbox_enabled(true);
|
||||
config.prefer_mxc = false;
|
||||
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
config.features.enable(Feature::UnifiedExec).unwrap();
|
||||
let mut filesystem = FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
);
|
||||
filesystem.entries.extend(
|
||||
[
|
||||
config.cwd.join("exact-secret.txt").into(),
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".into(),
|
||||
},
|
||||
]
|
||||
.map(|path| FileSystemSandboxEntry::new(path, FileSystemAccessMode::Deny)),
|
||||
);
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::from_runtime_permissions(
|
||||
&filesystem,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
))
|
||||
.unwrap();
|
||||
})
|
||||
.with_workspace_setup(|cwd, _fs| async move {
|
||||
std::fs::write(cwd.join("exact-secret.txt"), "exact secret")?;
|
||||
std::fs::write(cwd.join("secret.env"), "glob secret")?;
|
||||
Ok(())
|
||||
});
|
||||
let harness = TestCodexHarness::with_builder(builder).await?;
|
||||
let git = |cwd: &Path, args: &[&str]| -> anyhow::Result<String> {
|
||||
let output = Command::new("git")
|
||||
.args(["-c", "commit.gpgsign=false"])
|
||||
.current_dir(cwd)
|
||||
.args(args)
|
||||
.output()?;
|
||||
anyhow::ensure!(
|
||||
output.status.success(),
|
||||
"git {args:?}: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
Ok(String::from_utf8(output.stdout)?.trim().to_string())
|
||||
};
|
||||
let upstream = TempDir::new()?;
|
||||
let upstream = upstream.path();
|
||||
let upstream_path = upstream.to_str().context("UTF-8 upstream path")?;
|
||||
let cwd = harness.cwd();
|
||||
let read = |name| std::fs::read_to_string(cwd.join(name));
|
||||
let init = ["init", "--initial-branch=main"];
|
||||
let commit = ["commit", "-am", "fixture"];
|
||||
git(upstream, &init)?;
|
||||
git(upstream, &["config", "user.name", "Codex Test"])?;
|
||||
git(upstream, &["config", "user.email", "codex@test.com"])?;
|
||||
std::fs::write(upstream.join("tracked.txt"), "before\n")?;
|
||||
git(upstream, &["add", "tracked.txt"])?;
|
||||
git(upstream, &commit)?;
|
||||
git(cwd, &init)?;
|
||||
git(cwd, &["pull", "-q", "--ff-only", upstream_path, "main"])?;
|
||||
let fetch_head = cwd.join(".git/FETCH_HEAD");
|
||||
std::fs::remove_file(&fetch_head)?;
|
||||
std::fs::write(upstream.join("tracked.txt"), "after\n")?;
|
||||
git(upstream, &commit)?;
|
||||
let expected = git(upstream, &["rev-parse", "HEAD"])?;
|
||||
let bundle = cwd.join("upstream.bundle");
|
||||
let bundle_path = bundle.to_str().context("UTF-8 bundle path")?;
|
||||
git(upstream, &["bundle", "create", bundle_path, "--all"])?;
|
||||
|
||||
let args = json!({"cmd": concat!(
|
||||
"git pull --quiet --ff-only .\\upstream.bundle main || exit /b 10 & ",
|
||||
"type tracked.txt >NUL || exit /b 11 & ",
|
||||
"type exact-secret.txt >NUL 2>NUL && exit /b 12 & ",
|
||||
"type secret.env >NUL 2>NUL && exit /b 13 & ",
|
||||
"echo git-and-denials-ok>git-and-denials-ok.txt || exit /b 14 & exit /b 0"
|
||||
), "sandbox_permissions": "require_escalated",
|
||||
"justification": "Can I pull the fixture repository?",
|
||||
"yield_time_ms": 30_000, "tty": false, "login": false,
|
||||
});
|
||||
let call_id = "git-pull";
|
||||
core_test_support::responses::mount_function_call_agent_response(
|
||||
harness.server(),
|
||||
call_id,
|
||||
&args.to_string(),
|
||||
"exec_command",
|
||||
)
|
||||
.await;
|
||||
let codex = &harness.test().codex;
|
||||
codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "pull the fixture repository".into(),
|
||||
text_elements: Vec::new(),
|
||||
}]))
|
||||
.await?;
|
||||
let approval = wait_for_event(codex, |event| {
|
||||
matches!(event, ExecApprovalRequest(_) | TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
let ExecApprovalRequest(approval) = approval else {
|
||||
anyhow::bail!("expected approval before git ran: {approval:?}");
|
||||
};
|
||||
assert_eq!(approval.call_id, call_id);
|
||||
assert!(!fetch_head.exists());
|
||||
assert!(!cwd.join("git-and-denials-ok.txt").exists());
|
||||
assert_eq!(read("tracked.txt")?.trim(), "before");
|
||||
codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
let mut process_end = None;
|
||||
let mut turn_completed = false;
|
||||
while process_end.is_none() || !turn_completed {
|
||||
match wait_for_event_with_timeout(codex, |_| true, Duration::from_secs(120)).await {
|
||||
ExecCommandEnd(event) if event.call_id == call_id => process_end = Some(event),
|
||||
TurnComplete(_) => turn_completed = true,
|
||||
ExecApprovalRequest(request) => {
|
||||
anyhow::bail!("unexpected additional Git approval: {request:?}")
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
let result = process_end.expect("approved Git process should finish");
|
||||
let output = &result.aggregated_output;
|
||||
assert_eq!(result.exit_code, 0, "sandbox output: {output}");
|
||||
assert_eq!(read("git-and-denials-ok.txt")?.trim(), "git-and-denials-ok");
|
||||
assert!(read(".git/FETCH_HEAD")?.starts_with(&expected));
|
||||
assert_eq!(read("tracked.txt")?.trim(), "after");
|
||||
assert_eq!(read("exact-secret.txt")?, "exact secret");
|
||||
assert_eq!(read("secret.env")?, "glob secret");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user