mirror of
https://github.com/openai/codex.git
synced 2026-09-28 08:43:01 +08:00
Preserve unparsed shell wrappers in exec policy (#39588)
## Why Reducing a heredoc shell script to its inner executable lets a prefix rule for that executable apply to the entire wrapper, even though the full script was not parsed as a plain command. ## What changed - Fall back to evaluating the complete shell wrapper when plain-command parsing fails, including for heredoc scripts. - Keep these commands sandboxed when only the inner executable is allowed. - Propose the full wrapper as the exec policy amendment when approval is needed. ## Testing Added exec policy, Unix escalation, and approval scenario coverage for unparsed and heredoc shell wrappers. GitOrigin-RevId: 8f65133acb6b7c638263917e1d9137e45990772c
This commit is contained in:
@@ -37,7 +37,6 @@ use crate::config::Config;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use codex_shell_command::bash::parse_shell_lc_plain_commands;
|
||||
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;
|
||||
|
||||
@@ -171,14 +170,12 @@ pub(crate) struct UnmatchedCommandContext<'a> {
|
||||
pub(crate) permission_profile: &'a PermissionProfile,
|
||||
pub(crate) windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub(crate) sandbox_permissions: SandboxPermissions,
|
||||
pub(crate) used_complex_parsing: bool,
|
||||
pub(crate) command_origin: ExecPolicyCommandOrigin,
|
||||
}
|
||||
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
struct ExecPolicyCommands {
|
||||
commands: Vec<Vec<String>>,
|
||||
used_complex_parsing: bool,
|
||||
command_origin: ExecPolicyCommandOrigin,
|
||||
}
|
||||
|
||||
@@ -326,7 +323,6 @@ impl ExecPolicyManager {
|
||||
req: ExecApprovalRequest<'_>,
|
||||
ExecPolicyCommands {
|
||||
commands,
|
||||
used_complex_parsing,
|
||||
command_origin,
|
||||
}: ExecPolicyCommands,
|
||||
) -> ExecApprovalRequirement {
|
||||
@@ -341,11 +337,8 @@ impl ExecPolicyManager {
|
||||
allow_prefix_rules,
|
||||
} = req;
|
||||
let exec_policy = self.current_for_environment(environment_policy, allow_prefix_rules);
|
||||
// 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.
|
||||
let auto_amendment_allowed =
|
||||
!used_complex_parsing && allow_prefix_rules == AllowPrefixRules::Honor;
|
||||
// Avoid reusable approvals when this model does not honor prefix rules.
|
||||
let auto_amendment_allowed = allow_prefix_rules == AllowPrefixRules::Honor;
|
||||
let exec_policy_fallback = |cmd: &[String]| {
|
||||
render_decision_for_unmatched_command(
|
||||
cmd,
|
||||
@@ -354,7 +347,6 @@ impl ExecPolicyManager {
|
||||
permission_profile: &permission_profile,
|
||||
windows_sandbox_level,
|
||||
sandbox_permissions,
|
||||
used_complex_parsing,
|
||||
command_origin,
|
||||
},
|
||||
)
|
||||
@@ -748,7 +740,6 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
permission_profile,
|
||||
windows_sandbox_level,
|
||||
sandbox_permissions,
|
||||
used_complex_parsing,
|
||||
command_origin,
|
||||
} = context;
|
||||
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
|
||||
@@ -769,7 +760,6 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
&& profile_has_managed_filesystem_restrictions(permission_profile);
|
||||
|
||||
if is_known_safe
|
||||
&& !used_complex_parsing
|
||||
&& (approval_policy == AskForApproval::UnlessTrusted
|
||||
|| windows_managed_fs_restrictions_without_sandbox_backend)
|
||||
{
|
||||
@@ -860,7 +850,6 @@ fn commands_for_exec_policy(command: &[String]) -> ExecPolicyCommands {
|
||||
{
|
||||
return ExecPolicyCommands {
|
||||
commands,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
};
|
||||
}
|
||||
@@ -873,23 +862,13 @@ fn commands_for_exec_policy(command: &[String]) -> ExecPolicyCommands {
|
||||
{
|
||||
return ExecPolicyCommands {
|
||||
commands,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::PowerShell,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(single_command) = parse_shell_lc_single_command_prefix(command) {
|
||||
return ExecPolicyCommands {
|
||||
commands: vec![single_command],
|
||||
used_complex_parsing: true,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
};
|
||||
}
|
||||
|
||||
ExecPolicyCommands {
|
||||
commands: vec![command.to_vec()],
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,7 +34,6 @@ impl ExecPolicyManager {
|
||||
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),
|
||||
|
||||
@@ -687,7 +687,6 @@ fn commands_for_exec_policy_falls_back_for_empty_shell_script() {
|
||||
commands_for_exec_policy(&command),
|
||||
ExecPolicyCommands {
|
||||
commands: vec![command],
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
}
|
||||
);
|
||||
@@ -705,7 +704,6 @@ fn commands_for_exec_policy_falls_back_for_whitespace_shell_script() {
|
||||
commands_for_exec_policy(&command),
|
||||
ExecPolicyCommands {
|
||||
commands: vec![command],
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
}
|
||||
);
|
||||
@@ -751,7 +749,7 @@ async fn ignore_user_config_keeps_user_policy_files() -> std::io::Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn evaluates_heredoc_script_against_prefix_rules() {
|
||||
async fn heredoc_script_stays_in_sandbox_despite_inner_allow_rule() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
@@ -768,15 +766,19 @@ async fn evaluates_heredoc_script_against_prefix_rules() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY'\nprint('hello')\nPY".to_string(),
|
||||
])),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn omits_auto_amendment_for_heredoc_fallback_prompts() {
|
||||
async fn proposes_full_command_amendment_for_heredoc_prompts() {
|
||||
assert_exec_approval_requirement_for_command(
|
||||
ExecApprovalRequirementScenario {
|
||||
policy_src: None,
|
||||
@@ -792,14 +794,18 @@ async fn omits_auto_amendment_for_heredoc_fallback_prompts() {
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY'\nprint('hello')\nPY".to_string(),
|
||||
])),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_match() {
|
||||
async fn heredoc_prompt_replaces_unrelated_requested_prefix_with_full_command() {
|
||||
assert_exec_approval_requirement_for_command(
|
||||
ExecApprovalRequirementScenario {
|
||||
policy_src: None,
|
||||
@@ -819,14 +825,18 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_mat
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY'\nprint('hello')\nPY".to_string(),
|
||||
])),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_matches() {
|
||||
async fn heredoc_prompt_replaces_inner_requested_prefix_with_full_command() {
|
||||
assert_exec_approval_requirement_for_command(
|
||||
ExecApprovalRequirementScenario {
|
||||
policy_src: None,
|
||||
@@ -842,7 +852,11 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_matches(
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY'\nprint('hello')\nPY".to_string(),
|
||||
])),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -1164,7 +1178,6 @@ fn unmatched_granular_policy_still_prompts_for_restricted_sandbox_escalation() {
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
@@ -1184,7 +1197,6 @@ fn unmatched_on_request_uses_permission_profile_file_system_policy_for_escalatio
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
@@ -1204,7 +1216,6 @@ fn known_safe_on_request_still_prompts_for_restricted_sandbox_escalation() {
|
||||
permission_profile: &PermissionProfile::workspace_write(),
|
||||
windows_sandbox_level: WindowsSandboxLevel::RestrictedToken,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
|
||||
@@ -61,7 +61,6 @@ fn commands_for_exec_policy_parses_powershell_shell_wrapper() {
|
||||
commands_for_exec_policy(&command),
|
||||
ExecPolicyCommands {
|
||||
commands: vec![vec!["echo".to_string(), "blocked".to_string()]],
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::PowerShell,
|
||||
}
|
||||
);
|
||||
@@ -80,7 +79,6 @@ fn unmatched_safe_powershell_words_are_allowed() {
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::PowerShell,
|
||||
},
|
||||
)
|
||||
@@ -104,7 +102,6 @@ fn read_only_windows_sandbox_runs_unmatched_commands_under_sandbox() {
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
windows_sandbox_level,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
@@ -125,7 +122,6 @@ fn read_only_windows_policy_without_sandbox_backend_still_requires_approval() {
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
),
|
||||
@@ -166,7 +162,6 @@ fn writable_windows_policy_without_sandbox_backend_still_requires_approval() {
|
||||
permission_profile: &permission_profile,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
used_complex_parsing: false,
|
||||
command_origin: ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
|
||||
@@ -45,7 +45,6 @@ use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_sandboxing::record_filesystem_sandbox_violation;
|
||||
use codex_shell_command::bash::parse_shell_lc_plain_commands;
|
||||
use codex_shell_command::bash::parse_shell_lc_single_command_prefix;
|
||||
use codex_shell_escalation::EscalateServer;
|
||||
use codex_shell_escalation::EscalationDecision;
|
||||
use codex_shell_escalation::EscalationExecution;
|
||||
@@ -681,10 +680,7 @@ fn evaluate_intercepted_exec_policy(
|
||||
sandbox_permissions,
|
||||
enable_shell_wrapper_parsing,
|
||||
} = context;
|
||||
let CandidateCommands {
|
||||
commands,
|
||||
used_complex_parsing,
|
||||
} = if enable_shell_wrapper_parsing {
|
||||
let commands = if enable_shell_wrapper_parsing {
|
||||
// In this codepath, the first argument in `commands` could be a bare
|
||||
// name like `find` instead of an absolute path like `/usr/bin/find`.
|
||||
// It could also be a shell built-in like `echo`.
|
||||
@@ -692,10 +688,7 @@ fn evaluate_intercepted_exec_policy(
|
||||
} else {
|
||||
// In this codepath, `commands` has a single entry where the program
|
||||
// is always an absolute path.
|
||||
CandidateCommands {
|
||||
commands: vec![join_program_and_argv(program, argv)],
|
||||
used_complex_parsing: false,
|
||||
}
|
||||
vec![join_program_and_argv(program, argv)]
|
||||
};
|
||||
|
||||
let fallback = |cmd: &[String]| {
|
||||
@@ -706,7 +699,6 @@ fn evaluate_intercepted_exec_policy(
|
||||
permission_profile: &permission_profile,
|
||||
windows_sandbox_level,
|
||||
sandbox_permissions,
|
||||
used_complex_parsing,
|
||||
command_origin: crate::exec_policy::ExecPolicyCommandOrigin::Generic,
|
||||
},
|
||||
)
|
||||
@@ -730,39 +722,24 @@ struct InterceptedExecPolicyContext {
|
||||
enable_shell_wrapper_parsing: bool,
|
||||
}
|
||||
|
||||
struct CandidateCommands {
|
||||
commands: Vec<Vec<String>>,
|
||||
used_complex_parsing: bool,
|
||||
}
|
||||
|
||||
fn commands_for_intercepted_exec_policy(
|
||||
program: &AbsolutePathBuf,
|
||||
argv: &[String],
|
||||
) -> CandidateCommands {
|
||||
) -> Vec<Vec<String>> {
|
||||
if let [_, flag, script] = argv {
|
||||
let shell_command = [
|
||||
program.to_string_lossy().to_string(),
|
||||
flag.clone(),
|
||||
script.clone(),
|
||||
];
|
||||
if let Some(commands) = parse_shell_lc_plain_commands(&shell_command) {
|
||||
return CandidateCommands {
|
||||
commands,
|
||||
used_complex_parsing: false,
|
||||
};
|
||||
}
|
||||
if let Some(single_command) = parse_shell_lc_single_command_prefix(&shell_command) {
|
||||
return CandidateCommands {
|
||||
commands: vec![single_command],
|
||||
used_complex_parsing: true,
|
||||
};
|
||||
if let Some(commands) = parse_shell_lc_plain_commands(&shell_command)
|
||||
&& !commands.is_empty()
|
||||
{
|
||||
return commands;
|
||||
}
|
||||
}
|
||||
|
||||
CandidateCommands {
|
||||
commands: vec![join_program_and_argv(program, argv)],
|
||||
used_complex_parsing: false,
|
||||
}
|
||||
vec![join_program_and_argv(program, argv)]
|
||||
}
|
||||
|
||||
struct CoreShellCommandExecutor {
|
||||
|
||||
@@ -257,13 +257,24 @@ fn commands_for_intercepted_exec_policy_parses_plain_shell_wrappers() {
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
candidate_commands.commands,
|
||||
candidate_commands,
|
||||
vec![
|
||||
vec!["git".to_string(), "status".to_string()],
|
||||
vec!["pwd".to_string()],
|
||||
]
|
||||
);
|
||||
assert!(!candidate_commands.used_complex_parsing);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn commands_for_intercepted_exec_policy_preserves_unparsed_shell_wrappers() {
|
||||
let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "bash"])).unwrap();
|
||||
for script in ["", " \n\t", "cat <<'EOF'\nhello\nEOF"] {
|
||||
let argv = ["not-bash".into(), "-lc".into(), script.into()];
|
||||
assert_eq!(
|
||||
commands_for_intercepted_exec_policy(&program, &argv),
|
||||
vec![join_program_and_argv(&program, &argv)]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -608,6 +608,12 @@ impl Expectation {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
enum ExpectedExecPolicyAmendment {
|
||||
Prefix(&'static [&'static str]),
|
||||
FullCommand,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
enum Outcome {
|
||||
Auto,
|
||||
@@ -618,7 +624,7 @@ enum Outcome {
|
||||
ExecApprovalWithAmendment {
|
||||
decision: ReviewDecision,
|
||||
expected_reason: Option<&'static str>,
|
||||
expected_execpolicy_amendment: Option<&'static [&'static str]>,
|
||||
expected_execpolicy_amendment: Option<ExpectedExecPolicyAmendment>,
|
||||
},
|
||||
PatchApproval {
|
||||
decision: ReviewDecision,
|
||||
@@ -1052,7 +1058,10 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
outcome: Outcome::ExecApprovalWithAmendment {
|
||||
decision: ReviewDecision::denied("rejected by user"),
|
||||
expected_reason: None,
|
||||
expected_execpolicy_amendment: Some(&["echo", "known-safe-escalation"]),
|
||||
expected_execpolicy_amendment: Some(ExpectedExecPolicyAmendment::Prefix(&[
|
||||
"echo",
|
||||
"known-safe-escalation",
|
||||
])),
|
||||
},
|
||||
expectation: Expectation::CommandFailure {
|
||||
output_contains: "rejected by user",
|
||||
@@ -1079,6 +1088,26 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
output_contains: "you should not ask for escalated permissions",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "cat_heredoc_inner_allow_rule_requires_escalation_approval",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::RunCommandWithPolicy {
|
||||
command: "cat <<'EOF'\nhello\nEOF",
|
||||
policy_src: r#"prefix_rule(pattern=["cat"], decision="allow")"#,
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
features: vec![],
|
||||
model_override: Some("gpt-5.2"),
|
||||
outcome: Outcome::ExecApprovalWithAmendment {
|
||||
decision: ReviewDecision::denied("rejected by user"),
|
||||
expected_reason: None,
|
||||
expected_execpolicy_amendment: Some(ExpectedExecPolicyAmendment::FullCommand),
|
||||
},
|
||||
expectation: Expectation::CommandFailure {
|
||||
output_contains: "rejected by user",
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "cat_heredoc_file_redirect_prefix_rule_requires_escalation_approval",
|
||||
approval_policy: OnRequest,
|
||||
@@ -1122,7 +1151,7 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
},
|
||||
},
|
||||
ScenarioSpec {
|
||||
name: "python_heredoc_requested_prefix_rule_omits_amendment",
|
||||
name: "python_heredoc_requested_prefix_rule_proposes_full_command",
|
||||
approval_policy: OnRequest,
|
||||
sandbox_policy: workspace_write(false),
|
||||
action: ActionKind::RunCommandWithPrefixRule {
|
||||
@@ -1137,7 +1166,7 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
outcome: Outcome::ExecApprovalWithAmendment {
|
||||
decision: ReviewDecision::denied("rejected by user"),
|
||||
expected_reason: None,
|
||||
expected_execpolicy_amendment: None,
|
||||
expected_execpolicy_amendment: Some(ExpectedExecPolicyAmendment::FullCommand),
|
||||
},
|
||||
expectation: Expectation::CommandFailure {
|
||||
output_contains: "rejected by user",
|
||||
@@ -2130,9 +2159,15 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
|
||||
scenario.name
|
||||
);
|
||||
}
|
||||
let expected_execpolicy_amendment = expected_execpolicy_amendment.map(|command| {
|
||||
ExecPolicyAmendment::new(command.iter().map(|part| (*part).to_string()).collect())
|
||||
});
|
||||
let expected_execpolicy_amendment =
|
||||
expected_execpolicy_amendment.as_ref().map(|expected| {
|
||||
ExecPolicyAmendment::new(match expected {
|
||||
ExpectedExecPolicyAmendment::Prefix(command) => {
|
||||
command.iter().map(|part| (*part).to_string()).collect()
|
||||
}
|
||||
ExpectedExecPolicyAmendment::FullCommand => approval.command.clone(),
|
||||
})
|
||||
});
|
||||
assert_eq!(
|
||||
approval.proposed_execpolicy_amendment, expected_execpolicy_amendment,
|
||||
"unexpected execpolicy amendment for {}",
|
||||
|
||||
@@ -159,26 +159,6 @@ pub(crate) fn parse_shell_lc_literal_commands(command: &[String]) -> Option<Vec<
|
||||
Some(commands)
|
||||
}
|
||||
|
||||
/// Returns the parsed argv for a single shell command in a here-doc style
|
||||
/// script (`<<`), as long as the script contains exactly one command node.
|
||||
pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option<Vec<String>> {
|
||||
let (_, script) = extract_bash_command(command)?;
|
||||
let tree = try_parse_shell(script)?;
|
||||
let root = tree.root_node();
|
||||
if root.has_error() {
|
||||
return None;
|
||||
}
|
||||
if !has_named_descendant_kind(root, "heredoc_redirect") {
|
||||
return None;
|
||||
}
|
||||
if has_named_descendant_kind(root, "file_redirect") {
|
||||
return None;
|
||||
}
|
||||
|
||||
let command_node = find_single_command_node(root)?;
|
||||
parse_heredoc_command_words(command_node, script)
|
||||
}
|
||||
|
||||
fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Vec<String>> {
|
||||
if cmd.kind() != "command" {
|
||||
return None;
|
||||
@@ -277,43 +257,6 @@ fn parse_literal_shell_word(node: Node<'_>, src: &str) -> Option<String> {
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_heredoc_command_words(cmd: Node<'_>, src: &str) -> Option<Vec<String>> {
|
||||
if cmd.kind() != "command" {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut words = Vec::new();
|
||||
let mut cursor = cmd.walk();
|
||||
for child in cmd.named_children(&mut cursor) {
|
||||
match child.kind() {
|
||||
"command_name" => {
|
||||
let word_node = child.named_child(0)?;
|
||||
if !matches!(word_node.kind(), "word" | "number")
|
||||
|| !is_literal_word_or_number(word_node, src)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
words.push(word_node.utf8_text(src.as_bytes()).ok()?.to_owned());
|
||||
}
|
||||
"word" | "number" => {
|
||||
if !is_literal_word_or_number(child, src) {
|
||||
return None;
|
||||
}
|
||||
words.push(child.utf8_text(src.as_bytes()).ok()?.to_owned());
|
||||
}
|
||||
// Allow heredoc constructs that attach stdin to a single command
|
||||
// without changing argv matching semantics for the executable
|
||||
// prefix. Other file redirects may write outside the sandbox and
|
||||
// must not be collapsed to the executable prefix for execpolicy.
|
||||
"comment" => {}
|
||||
kind if is_allowed_heredoc_attachment_kind(kind) => {}
|
||||
_ => return None,
|
||||
}
|
||||
}
|
||||
|
||||
if words.is_empty() { None } else { Some(words) }
|
||||
}
|
||||
|
||||
fn is_literal_word_or_number(node: Node<'_>, src: &str) -> bool {
|
||||
if !matches!(node.kind(), "word" | "number") {
|
||||
return false;
|
||||
@@ -330,50 +273,6 @@ fn is_literal_word_or_number(node: Node<'_>, src: &str) -> bool {
|
||||
})
|
||||
}
|
||||
|
||||
fn is_allowed_heredoc_attachment_kind(kind: &str) -> bool {
|
||||
matches!(
|
||||
kind,
|
||||
"heredoc_body"
|
||||
| "simple_heredoc_body"
|
||||
| "heredoc_redirect"
|
||||
| "herestring_redirect"
|
||||
| "redirected_statement"
|
||||
)
|
||||
}
|
||||
|
||||
fn find_single_command_node(root: Node<'_>) -> Option<Node<'_>> {
|
||||
let mut stack = vec![root];
|
||||
let mut single_command = None;
|
||||
while let Some(node) = stack.pop() {
|
||||
if node.kind() == "command" {
|
||||
if single_command.is_some() {
|
||||
return None;
|
||||
}
|
||||
single_command = Some(node);
|
||||
}
|
||||
|
||||
let mut cursor = node.walk();
|
||||
for child in node.named_children(&mut cursor) {
|
||||
stack.push(child);
|
||||
}
|
||||
}
|
||||
single_command
|
||||
}
|
||||
|
||||
fn has_named_descendant_kind(node: Node<'_>, kind: &str) -> bool {
|
||||
let mut stack = vec![node];
|
||||
while let Some(current) = stack.pop() {
|
||||
if current.kind() == kind {
|
||||
return true;
|
||||
}
|
||||
let mut cursor = current.walk();
|
||||
for child in current.named_children(&mut cursor) {
|
||||
stack.push(child);
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn parse_double_quoted_string(node: Node, src: &str) -> Option<String> {
|
||||
if node.kind() != "string" {
|
||||
return None;
|
||||
@@ -591,30 +490,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn heredoc_prefix_does_not_restore_dynamic_words() {
|
||||
for script in [
|
||||
"find . -{delete,print}",
|
||||
"find . -del*",
|
||||
r"find . -de\lete",
|
||||
"cat ~",
|
||||
"cat ~HOME",
|
||||
"cat =sh",
|
||||
"c*",
|
||||
] {
|
||||
let command = [
|
||||
"bash".to_owned(),
|
||||
"-lc".to_owned(),
|
||||
format!("{script} <<'EOF'\nEOF"),
|
||||
];
|
||||
assert_eq!(
|
||||
parse_shell_lc_single_command_prefix(&command),
|
||||
None,
|
||||
"{script:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_variable_assignment_prefix() {
|
||||
assert!(parse_seq("FOO=bar ls").is_none());
|
||||
@@ -689,103 +564,4 @@ mod tests {
|
||||
assert!(parse_seq("rg -g\"$(pwd)\" pattern").is_none());
|
||||
assert!(parse_seq("rg -g\"$(echo '*.py')\" pattern").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_supports_heredoc() {
|
||||
let command = vec![
|
||||
"zsh".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY'\nprint('hello')\nPY".to_string(),
|
||||
];
|
||||
let parsed = parse_shell_lc_single_command_prefix(&command);
|
||||
assert_eq!(parsed, Some(vec!["python3".to_string()]));
|
||||
|
||||
let command_unquoted = vec![
|
||||
"zsh".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 << PY\nprint('hello')\nPY".to_string(),
|
||||
];
|
||||
let parsed_unquoted = parse_shell_lc_single_command_prefix(&command_unquoted);
|
||||
assert_eq!(parsed_unquoted, Some(vec!["python3".to_string()]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_multi_command_scripts() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY'\nprint('hello')\nPY\necho done".to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_non_heredoc_redirects() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"echo hello > /tmp/out.txt".to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_heredoc_with_extra_file_redirect() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 <<'PY' > /tmp/out.txt\nprint('hello')\nPY".to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_heredoc_with_variable_assignment() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"PATH=/tmp/evil:$PATH cat <<'EOF'\nhello\nEOF".to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_herestring_with_chaining() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
r#"echo hello > /tmp/out.txt && cat /tmp/out.txt"#.to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_herestring_with_substitution() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
r#"python3 <<< "$(rm -rf /)""#.to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_arithmetic_shift_non_heredoc_script() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"echo $((1<<2))".to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_shell_lc_single_command_prefix_rejects_heredoc_command_with_word_expansion() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python3 $((1<<2)) <<'PY'\nprint('hello')\nPY".to_string(),
|
||||
];
|
||||
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user