mirror of
https://github.com/openai/codex.git
synced 2026-09-28 08:43:01 +08:00
Bind unified exec approvals to shell executables (#39311)
## Why An unfamiliar executable can ignore its arguments, so trust in an apparent inner command must not implicitly trust the executable that runs it. ## What changed - Evaluate unfamiliar shell executables alongside their parsed commands when determining unified exec approval requirements. Inner commands can add restrictions, but cannot grant trust to the executable. - Include the executable in reusable approval keys so approval for one custom shell does not apply to another. - Parse literal PowerShell scripts without launching the requested executable, preserving command policy checks before approval. ## Testing Add cross-platform coverage for spoofed shell paths, allowed and forbidden inner commands, explicit custom-shell approval, and session approval isolation. GitOrigin-RevId: 0dd2c7e9a2ac30965ef5fe5de1a8d2968bb5f9d0
This commit is contained in:
@@ -41,6 +41,7 @@ use codex_shell_command::bash::parse_shell_lc_single_command_prefix;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use shlex::try_join as shlex_try_join;
|
||||
|
||||
mod executable_identity;
|
||||
mod model_policy;
|
||||
|
||||
pub(crate) use model_policy::AllowPrefixRules;
|
||||
@@ -314,6 +315,20 @@ impl ExecPolicyManager {
|
||||
pub(crate) async fn create_exec_approval_requirement_for_command(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
) -> ExecApprovalRequirement {
|
||||
let commands = commands_for_exec_policy(req.command);
|
||||
self.create_exec_approval_requirement_for_parsed_commands(req, commands)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn create_exec_approval_requirement_for_parsed_commands(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
ExecPolicyCommands {
|
||||
commands,
|
||||
used_complex_parsing,
|
||||
command_origin,
|
||||
}: ExecPolicyCommands,
|
||||
) -> ExecApprovalRequirement {
|
||||
let ExecApprovalRequest {
|
||||
command,
|
||||
@@ -326,11 +341,6 @@ impl ExecPolicyManager {
|
||||
allow_prefix_rules,
|
||||
} = req;
|
||||
let exec_policy = self.current_for_environment(environment_policy, allow_prefix_rules);
|
||||
let ExecPolicyCommands {
|
||||
commands,
|
||||
used_complex_parsing,
|
||||
command_origin,
|
||||
} = commands_for_exec_policy(command);
|
||||
// Keep heredoc prefix parsing for the rules that apply to this model,
|
||||
// but avoid reusable approvals for cyber models or when only the
|
||||
// heredoc fallback parser matched.
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
use super::ExecApprovalRequest;
|
||||
#[cfg(windows)]
|
||||
use super::ExecPolicyCommandOrigin;
|
||||
#[cfg(windows)]
|
||||
use super::ExecPolicyCommands;
|
||||
use super::ExecPolicyManager;
|
||||
use super::commands_for_exec_policy;
|
||||
use crate::shell::Shell;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
#[cfg(windows)]
|
||||
use codex_shell_command::powershell::extract_powershell_command;
|
||||
#[cfg(windows)]
|
||||
use codex_shell_command::powershell::parse_powershell_script_into_plain_commands;
|
||||
use codex_tools::UnifiedExecShellMode;
|
||||
use std::path::Path;
|
||||
|
||||
impl ExecPolicyManager {
|
||||
pub(crate) async fn create_exec_approval_requirement_for_shell(
|
||||
&self,
|
||||
mut request: ExecApprovalRequest<'_>,
|
||||
configured_shell: &Shell,
|
||||
shell_mode: &UnifiedExecShellMode,
|
||||
) -> ExecApprovalRequirement {
|
||||
let command = request.command;
|
||||
let executable = shell_approval_command(command, configured_shell, shell_mode);
|
||||
if executable.len() == command.len() {
|
||||
return self
|
||||
.create_exec_approval_requirement_for_command(request)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
let mut policy_commands = match extract_powershell_command(command) {
|
||||
Some((_, script)) => ExecPolicyCommands {
|
||||
commands: parse_powershell_script_into_plain_commands(script)
|
||||
.unwrap_or_else(|| vec![command.to_vec()]),
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::PowerShell,
|
||||
},
|
||||
None => commands_for_exec_policy(command),
|
||||
};
|
||||
|
||||
#[cfg(not(windows))]
|
||||
let mut policy_commands = commands_for_exec_policy(command);
|
||||
|
||||
// Evaluate the executable alongside its apparent commands. Inner
|
||||
// commands can add restrictions, but cannot grant the executable trust.
|
||||
policy_commands.commands.insert(0, executable.to_vec());
|
||||
request.command = executable;
|
||||
self.create_exec_approval_requirement_for_parsed_commands(request, policy_commands)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
fn shell_approval_command<'a>(
|
||||
command: &'a [String],
|
||||
configured_shell: &Shell,
|
||||
shell_mode: &UnifiedExecShellMode,
|
||||
) -> &'a [String] {
|
||||
let Some(executable) = command.first() else {
|
||||
return command;
|
||||
};
|
||||
let executable_path = Path::new(executable);
|
||||
|
||||
#[cfg(windows)]
|
||||
let is_system_shell = std::env::var_os("SystemRoot").is_some_and(|system_root| {
|
||||
let system_directory = Path::new(&system_root).join("System32");
|
||||
let powershell_directory = system_directory.join("WindowsPowerShell").join("v1.0");
|
||||
executable_path.parent().is_some_and(|parent| {
|
||||
parent
|
||||
.as_os_str()
|
||||
.eq_ignore_ascii_case(system_directory.as_os_str())
|
||||
|| parent
|
||||
.as_os_str()
|
||||
.eq_ignore_ascii_case(powershell_directory.as_os_str())
|
||||
})
|
||||
});
|
||||
|
||||
#[cfg(not(windows))]
|
||||
let is_system_shell = executable_path
|
||||
.parent()
|
||||
.is_some_and(|parent| parent == Path::new("/bin") || parent == Path::new("/usr/bin"));
|
||||
|
||||
#[cfg(windows)]
|
||||
let is_configured_shell = executable_path
|
||||
.as_os_str()
|
||||
.eq_ignore_ascii_case(configured_shell.shell_path.as_os_str());
|
||||
|
||||
#[cfg(not(windows))]
|
||||
let is_configured_shell = executable_path == configured_shell.shell_path.as_path();
|
||||
|
||||
if is_configured_shell
|
||||
|| is_system_shell
|
||||
|| matches!(shell_mode, UnifiedExecShellMode::ZshFork(_))
|
||||
{
|
||||
command
|
||||
} else {
|
||||
// An unfamiliar executable can ignore its arguments, so evaluate the
|
||||
// executable separately from any restrictions on its apparent command.
|
||||
std::slice::from_ref(executable)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "executable_identity_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,72 @@
|
||||
use super::shell_approval_command;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell::ShellType;
|
||||
use codex_tools::UnifiedExecShellMode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[test]
|
||||
fn parent_directory_traversal_is_not_a_trusted_system_shell() {
|
||||
let (configured_executable, unfamiliar_executable) = if cfg!(windows) {
|
||||
let system_root = std::env::var_os("SystemRoot").expect("Windows SystemRoot");
|
||||
let system_directory = PathBuf::from(system_root).join("System32");
|
||||
(
|
||||
system_directory.join("cmd.exe"),
|
||||
system_directory
|
||||
.join("..")
|
||||
.join("workspace")
|
||||
.join("powershell.exe"),
|
||||
)
|
||||
} else {
|
||||
(
|
||||
PathBuf::from("/bin/sh"),
|
||||
PathBuf::from("/bin/../workspace/bash"),
|
||||
)
|
||||
};
|
||||
let shell = Shell {
|
||||
shell_type: if cfg!(windows) {
|
||||
ShellType::Cmd
|
||||
} else {
|
||||
ShellType::Sh
|
||||
},
|
||||
shell_path: configured_executable,
|
||||
};
|
||||
let command = vec![
|
||||
unfamiliar_executable.to_string_lossy().into_owned(),
|
||||
"-c".to_string(),
|
||||
"ls".to_string(),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
shell_approval_command(&command, &shell, &UnifiedExecShellMode::Direct),
|
||||
&command[..1],
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_shell_identity_is_case_insensitive() {
|
||||
let configured_executable = PathBuf::from(r"C:\Custom\pwsh.exe");
|
||||
let configured_shell = Shell {
|
||||
shell_type: ShellType::PowerShell,
|
||||
shell_path: configured_executable.clone(),
|
||||
};
|
||||
let system_root = std::env::var_os("SystemRoot").expect("Windows SystemRoot");
|
||||
let system_executable = PathBuf::from(system_root.to_string_lossy().to_ascii_uppercase())
|
||||
.join("SYSTEM32")
|
||||
.join("WINDOWSPOWERSHELL")
|
||||
.join("V1.0")
|
||||
.join("POWERSHELL.EXE");
|
||||
|
||||
for executable in [
|
||||
configured_executable.to_string_lossy().to_ascii_uppercase(),
|
||||
system_executable.to_string_lossy().into_owned(),
|
||||
] {
|
||||
let command = vec![executable, "-Command".to_string(), "echo safe".to_string()];
|
||||
|
||||
assert_eq!(
|
||||
shell_approval_command(&command, &configured_shell, &UnifiedExecShellMode::Direct),
|
||||
command.as_slice(),
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -237,6 +237,7 @@ impl ApprovalAction {
|
||||
..
|
||||
} => vec![ApprovalCacheKey::ExecCommand(UnifiedExecApprovalKey {
|
||||
environment_id: environment_id.clone(),
|
||||
executable: command.first().cloned(),
|
||||
command: canonicalize_command_for_approval(command),
|
||||
cwd: cwd.clone(),
|
||||
tty: *tty,
|
||||
|
||||
@@ -88,6 +88,7 @@ pub struct UnifiedExecRequest {
|
||||
#[derive(serde::Serialize, Clone, Debug, Eq, PartialEq, Hash)]
|
||||
pub struct UnifiedExecApprovalKey {
|
||||
pub environment_id: String,
|
||||
pub executable: Option<String>,
|
||||
pub command: Vec<String>,
|
||||
pub cwd: PathUri,
|
||||
pub tty: bool,
|
||||
|
||||
@@ -1238,24 +1238,34 @@ impl UnifiedExecProcessManager {
|
||||
};
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(self, request.shell_mode.clone());
|
||||
let session_shell = context.session.user_shell();
|
||||
let configured_shell = request
|
||||
.turn_environment
|
||||
.shell
|
||||
.as_ref()
|
||||
.unwrap_or(session_shell.as_ref());
|
||||
let exec_approval_requirement = context
|
||||
.session
|
||||
.services
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: turn.approval_policy(),
|
||||
permission_profile: request.turn_environment.permission_profile().clone(),
|
||||
environment_policy: request.turn_environment.config().exec_policy.as_ref(),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
request.sandbox_permissions
|
||||
.create_exec_approval_requirement_for_shell(
|
||||
ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: turn.approval_policy(),
|
||||
permission_profile: request.turn_environment.permission_profile().clone(),
|
||||
environment_policy: request.turn_environment.config().exec_policy.as_ref(),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
request.sandbox_permissions
|
||||
},
|
||||
prefix_rule: request.prefix_rule.clone(),
|
||||
allow_prefix_rules: context.step_context.turn.allow_prefix_rules(),
|
||||
},
|
||||
prefix_rule: request.prefix_rule.clone(),
|
||||
allow_prefix_rules: context.step_context.turn.allow_prefix_rules(),
|
||||
})
|
||||
configured_shell,
|
||||
&request.shell_mode,
|
||||
)
|
||||
.await;
|
||||
let req = UnifiedExecToolRequest {
|
||||
command: request.command.clone(),
|
||||
|
||||
@@ -0,0 +1,349 @@
|
||||
use anyhow::Result;
|
||||
use codex_config::Constrained;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_remote;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_codex::turn_permission_fields;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use test_case::test_case;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum ShellAttack {
|
||||
ExactShellName,
|
||||
ExactShellNameWithAllowedInnerCommand,
|
||||
ExactShellNameWithForbiddenInnerCommand,
|
||||
DangerousCommandOnRequest,
|
||||
DangerousCommandNever,
|
||||
ApprovedCustomShell,
|
||||
SessionApprovalDoesNotTrustDifferentShell,
|
||||
SpoofedShellExtension,
|
||||
}
|
||||
|
||||
#[test_case(ShellAttack::ExactShellName; "workspace shell requires approval")]
|
||||
#[test_case(ShellAttack::ExactShellNameWithAllowedInnerCommand; "inner allow does not trust workspace shell")]
|
||||
#[test_case(ShellAttack::ExactShellNameWithForbiddenInnerCommand; "inner forbidden rule still rejects workspace shell")]
|
||||
#[test_case(ShellAttack::DangerousCommandOnRequest; "dangerous inner command requires approval")]
|
||||
#[test_case(ShellAttack::DangerousCommandNever; "dangerous inner command is forbidden without approval")]
|
||||
#[test_case(ShellAttack::ApprovedCustomShell; "approved custom shell still runs")]
|
||||
#[test_case(ShellAttack::SessionApprovalDoesNotTrustDifferentShell; "session approval does not trust a different shell")]
|
||||
#[test_case(ShellAttack::SpoofedShellExtension; "workspace shell with an extra extension requires approval")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_provided_shell_cannot_inherit_inner_command_trust(
|
||||
attack: ShellAttack,
|
||||
) -> Result<()> {
|
||||
skip_if_remote!(
|
||||
Ok(()),
|
||||
"remote executors already replace requested shell paths with their reported shell"
|
||||
);
|
||||
|
||||
let approval_policy = match attack {
|
||||
ShellAttack::DangerousCommandOnRequest => AskForApproval::OnRequest,
|
||||
ShellAttack::DangerousCommandNever => AskForApproval::Never,
|
||||
ShellAttack::ExactShellName
|
||||
| ShellAttack::ExactShellNameWithAllowedInnerCommand
|
||||
| ShellAttack::ExactShellNameWithForbiddenInnerCommand
|
||||
| ShellAttack::ApprovedCustomShell
|
||||
| ShellAttack::SessionApprovalDoesNotTrustDifferentShell
|
||||
| ShellAttack::SpoofedShellExtension => AskForApproval::UnlessTrusted,
|
||||
};
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.use_experimental_unified_exec_tool = true;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnifiedExec)
|
||||
.expect("enable unified exec");
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
let inner_command_rule = match attack {
|
||||
ShellAttack::ExactShellNameWithAllowedInnerCommand => {
|
||||
Some("prefix_rule(pattern=[\"echo\"], decision=\"allow\")\n")
|
||||
}
|
||||
ShellAttack::ExactShellNameWithForbiddenInnerCommand => {
|
||||
if cfg!(windows) {
|
||||
Some(
|
||||
r#"prefix_rule(pattern=["Remove-Item", "C:\\important"], decision="forbidden")"#,
|
||||
)
|
||||
} else {
|
||||
Some("prefix_rule(pattern=[\"rm\"], decision=\"forbidden\")\n")
|
||||
}
|
||||
}
|
||||
ShellAttack::ExactShellName
|
||||
| ShellAttack::DangerousCommandOnRequest
|
||||
| ShellAttack::DangerousCommandNever
|
||||
| ShellAttack::ApprovedCustomShell
|
||||
| ShellAttack::SessionApprovalDoesNotTrustDifferentShell
|
||||
| ShellAttack::SpoofedShellExtension => None,
|
||||
};
|
||||
if let Some(inner_command_rule) = inner_command_rule {
|
||||
let policy_path = config.codex_home.join("rules/default.rules");
|
||||
fs::create_dir_all(policy_path.parent().expect("rules directory"))
|
||||
.expect("create rules directory");
|
||||
fs::write(policy_path, inner_command_rule).expect("write execution policy rule");
|
||||
}
|
||||
});
|
||||
#[cfg(windows)]
|
||||
if matches!(
|
||||
attack,
|
||||
ShellAttack::ExactShellNameWithForbiddenInnerCommand | ShellAttack::ApprovedCustomShell
|
||||
) {
|
||||
let system_root = std::env::var_os("SystemRoot").expect("Windows SystemRoot");
|
||||
let configured_shell = std::path::Path::new(&system_root)
|
||||
.join("System32")
|
||||
.join("WindowsPowerShell")
|
||||
.join("v1.0")
|
||||
.join("powershell.exe");
|
||||
builder = builder.with_user_shell(codex_core::shell::get_shell_by_model_provided_path(
|
||||
&configured_shell,
|
||||
));
|
||||
}
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
let shell_name = match attack {
|
||||
ShellAttack::ExactShellNameWithForbiddenInnerCommand | ShellAttack::ApprovedCustomShell
|
||||
if cfg!(windows) =>
|
||||
{
|
||||
"pwsh.exe"
|
||||
}
|
||||
ShellAttack::ExactShellName
|
||||
| ShellAttack::ExactShellNameWithAllowedInnerCommand
|
||||
| ShellAttack::ExactShellNameWithForbiddenInnerCommand
|
||||
| ShellAttack::DangerousCommandOnRequest
|
||||
| ShellAttack::DangerousCommandNever
|
||||
| ShellAttack::ApprovedCustomShell
|
||||
| ShellAttack::SessionApprovalDoesNotTrustDifferentShell => {
|
||||
if cfg!(windows) {
|
||||
"powershell.exe"
|
||||
} else {
|
||||
"bash"
|
||||
}
|
||||
}
|
||||
ShellAttack::SpoofedShellExtension => {
|
||||
if cfg!(windows) {
|
||||
"powershell.evil"
|
||||
} else {
|
||||
"bash.evil"
|
||||
}
|
||||
}
|
||||
};
|
||||
let shell = test.workspace_path(shell_name);
|
||||
let marker = test.workspace_path("attacker-executed");
|
||||
#[cfg(unix)]
|
||||
{
|
||||
fs::write(&shell, "#!/bin/sh\nprintf ran > attacker-executed\n")?;
|
||||
fs::set_permissions(&shell, fs::Permissions::from_mode(0o755))?;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
let test_executable = std::env::current_exe()?;
|
||||
fs::hard_link(&test_executable, &shell)
|
||||
.or_else(|_| fs::copy(&test_executable, &shell).map(|_| ()))?;
|
||||
fs::write(
|
||||
shell.with_file_name(".codex-executable-identity-fixture"),
|
||||
b"fake shell",
|
||||
)?;
|
||||
}
|
||||
let other_shell = if matches!(
|
||||
attack,
|
||||
ShellAttack::SessionApprovalDoesNotTrustDifferentShell
|
||||
) {
|
||||
let other_shell = test.workspace_path("another").join(shell_name);
|
||||
fs::create_dir_all(other_shell.parent().expect("alternate shell directory"))?;
|
||||
fs::copy(&shell, &other_shell)?;
|
||||
#[cfg(windows)]
|
||||
fs::write(
|
||||
other_shell.with_file_name(".codex-executable-identity-fixture"),
|
||||
b"fake shell",
|
||||
)?;
|
||||
Some(other_shell)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let call_id = "untrusted-shell-path";
|
||||
let other_call_id = "different-untrusted-shell-path";
|
||||
let command = match attack {
|
||||
ShellAttack::ExactShellName if cfg!(windows) => "Write-Output $env:USERNAME",
|
||||
ShellAttack::DangerousCommandOnRequest | ShellAttack::DangerousCommandNever => {
|
||||
if cfg!(windows) {
|
||||
"Remove-Item important -Force"
|
||||
} else {
|
||||
"rm -rf important"
|
||||
}
|
||||
}
|
||||
ShellAttack::ExactShellNameWithForbiddenInnerCommand => {
|
||||
if cfg!(windows) {
|
||||
r"echo shell-safe && Remove-Item C:\important"
|
||||
} else {
|
||||
"echo shell-safe; rm important"
|
||||
}
|
||||
}
|
||||
ShellAttack::ExactShellName
|
||||
| ShellAttack::ExactShellNameWithAllowedInnerCommand
|
||||
| ShellAttack::ApprovedCustomShell
|
||||
| ShellAttack::SessionApprovalDoesNotTrustDifferentShell
|
||||
| ShellAttack::SpoofedShellExtension => "echo shell-safe",
|
||||
};
|
||||
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-untrusted-shell-1"),
|
||||
ev_function_call(
|
||||
call_id,
|
||||
"exec_command",
|
||||
&json!({ "cmd": command, "shell": shell }).to_string(),
|
||||
),
|
||||
ev_completed("resp-untrusted-shell-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
if let Some(other_shell) = other_shell.as_ref() {
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-different-untrusted-shell"),
|
||||
ev_function_call(
|
||||
other_call_id,
|
||||
"exec_command",
|
||||
&json!({ "cmd": command, "shell": other_shell }).to_string(),
|
||||
),
|
||||
ev_completed("resp-different-untrusted-shell"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let completed = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-untrusted-shell", "done"),
|
||||
ev_completed("resp-untrusted-shell-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let (sandbox_policy, permission_profile) =
|
||||
turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path());
|
||||
test.codex
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "inspect the repository".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
approval_policy: Some(approval_policy),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
sandbox_policy: Some(sandbox_policy),
|
||||
permission_profile,
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let event = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
if matches!(
|
||||
attack,
|
||||
ShellAttack::ExactShellNameWithForbiddenInnerCommand | ShellAttack::DangerousCommandNever
|
||||
) {
|
||||
assert!(matches!(event, EventMsg::TurnComplete(_)));
|
||||
let output = completed
|
||||
.single_request()
|
||||
.function_call_output_text(call_id)
|
||||
.expect("forbidden command output");
|
||||
assert!(
|
||||
output.contains("rejected"),
|
||||
"the forbidden command should be rejected: {output}"
|
||||
);
|
||||
#[cfg(windows)]
|
||||
if matches!(attack, ShellAttack::ExactShellNameWithForbiddenInnerCommand) {
|
||||
assert!(
|
||||
output.contains("Remove-Item"),
|
||||
"the forbidden PowerShell command should remain visible to policy: {output}"
|
||||
);
|
||||
}
|
||||
} else {
|
||||
let EventMsg::ExecApprovalRequest(approval) = event else {
|
||||
panic!("workspace shell bypassed approval");
|
||||
};
|
||||
assert_eq!(approval.call_id, call_id);
|
||||
assert!(!marker.exists(), "the shell ran before approval");
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: match attack {
|
||||
ShellAttack::ApprovedCustomShell => ReviewDecision::Approved,
|
||||
ShellAttack::SessionApprovalDoesNotTrustDifferentShell => {
|
||||
ReviewDecision::ApprovedForSession
|
||||
}
|
||||
_ => ReviewDecision::denied("untrusted shell"),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
if other_shell.is_some() {
|
||||
let event = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
let EventMsg::ExecApprovalRequest(approval) = event else {
|
||||
panic!("a different workspace shell reused the first shell's session approval");
|
||||
};
|
||||
assert_eq!(approval.call_id, other_call_id);
|
||||
assert!(
|
||||
marker.exists(),
|
||||
"the session-approved shell should have run"
|
||||
);
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::denied("different untrusted shell"),
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
marker.exists(),
|
||||
matches!(
|
||||
attack,
|
||||
ShellAttack::ApprovedCustomShell
|
||||
| ShellAttack::SessionApprovalDoesNotTrustDifferentShell
|
||||
),
|
||||
"only an explicitly approved custom shell should run"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -16,6 +16,21 @@ use ctor::ctor;
|
||||
#[ctor]
|
||||
pub static CODEX_ALIASES_TEMP_DIR: Option<TestBinaryDispatchGuard> = {
|
||||
configure_test_binary_dispatch("codex-core-tests", |exe_name, argv1| {
|
||||
#[cfg(windows)]
|
||||
if exe_name.eq_ignore_ascii_case("powershell.exe")
|
||||
|| exe_name.eq_ignore_ascii_case("powershell.evil")
|
||||
|| exe_name.eq_ignore_ascii_case("pwsh.exe")
|
||||
{
|
||||
let executable = std::env::current_exe().expect("locate fake PowerShell executable");
|
||||
if executable
|
||||
.with_file_name(".codex-executable-identity-fixture")
|
||||
.is_file()
|
||||
{
|
||||
let marker = executable.with_file_name("attacker-executed");
|
||||
std::fs::write(marker, b"ran").expect("record fake PowerShell execution");
|
||||
std::process::exit(0);
|
||||
}
|
||||
}
|
||||
if argv1 == Some(CODEX_CORE_APPLY_PATCH_ARG1) {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
@@ -62,6 +77,7 @@ mod cyber_exec_policy;
|
||||
mod deprecation_notice;
|
||||
mod exec;
|
||||
mod exec_policy;
|
||||
mod executable_identity;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
mod extension_sandbox;
|
||||
mod external_auth;
|
||||
|
||||
@@ -1,6 +1,4 @@
|
||||
mod powershell_parser;
|
||||
// Production safety and exec-policy callers migrate to this lowerer in a follow-up.
|
||||
#[allow(dead_code)]
|
||||
mod powershell_tree_sitter;
|
||||
|
||||
pub mod is_dangerous_command;
|
||||
@@ -8,3 +6,4 @@ pub mod is_safe_command;
|
||||
#[cfg(windows)]
|
||||
pub(crate) mod windows_safe_commands;
|
||||
pub(crate) use powershell_parser::try_parse_powershell_ast_commands;
|
||||
pub(crate) use powershell_tree_sitter::try_parse_powershell_commands;
|
||||
|
||||
@@ -10,7 +10,7 @@ use tree_sitter::Parser;
|
||||
/// Unknown syntax, parse recovery, and dynamic expressions fail closed instead of being guessed
|
||||
/// at. The accepted CST shapes intentionally cover only common literal command forms; rare
|
||||
/// PowerShell syntax and value-conversion cases stay opaque.
|
||||
pub(super) fn try_parse_powershell_commands(script: &str) -> Option<Vec<Vec<String>>> {
|
||||
pub(crate) fn try_parse_powershell_commands(script: &str) -> Option<Vec<Vec<String>>> {
|
||||
lower_with_tree_sitter(script).ok()
|
||||
}
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::path::PathBuf;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
use crate::command_safety::try_parse_powershell_ast_commands;
|
||||
use crate::command_safety::try_parse_powershell_commands;
|
||||
use crate::shell_detect::ShellType;
|
||||
use crate::shell_detect::detect_shell_type;
|
||||
|
||||
@@ -82,6 +83,14 @@ pub fn parse_powershell_command_into_plain_commands(
|
||||
try_parse_powershell_ast_commands(executable, script)
|
||||
}
|
||||
|
||||
/// Parse literal PowerShell commands without starting a PowerShell executable.
|
||||
///
|
||||
/// Unknown or dynamic syntax stays opaque so unfamiliar executables can be
|
||||
/// evaluated without running them before approval.
|
||||
pub fn parse_powershell_script_into_plain_commands(script: &str) -> Option<Vec<Vec<String>>> {
|
||||
try_parse_powershell_commands(script)
|
||||
}
|
||||
|
||||
/// This function attempts to find a powershell.exe executable on the system.
|
||||
pub fn try_find_powershell_executable_blocking() -> Option<AbsolutePathBuf> {
|
||||
try_find_powershellish_executable_in_path(&["powershell.exe"])
|
||||
@@ -156,6 +165,7 @@ mod tests {
|
||||
use super::extract_powershell_command;
|
||||
#[cfg(windows)]
|
||||
use super::parse_powershell_command_into_plain_commands;
|
||||
use super::parse_powershell_script_into_plain_commands;
|
||||
use super::prefix_powershell_script_with_utf8;
|
||||
|
||||
#[test]
|
||||
@@ -205,6 +215,17 @@ mod tests {
|
||||
assert_eq!(script, "Get-ChildItem | Select-String foo");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_powershell_command_chains_and_preserves_windows_paths() {
|
||||
assert_eq!(
|
||||
parse_powershell_script_into_plain_commands(r"echo safe && Remove-Item C:\important"),
|
||||
Some(vec![
|
||||
vec!["echo".to_string(), "safe".to_string()],
|
||||
vec!["Remove-Item".to_string(), r"C:\important".to_string()],
|
||||
]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prefixes_powershell_command_with_best_effort_utf8() {
|
||||
let cmd = vec![
|
||||
|
||||
Reference in New Issue
Block a user