Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 133 additions & 2 deletions codex-rs/core/src/exec_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use codex_execpolicy::AmendError;
use codex_execpolicy::Decision;
use codex_execpolicy::Error as ExecPolicyRuleError;
use codex_execpolicy::Evaluation;
use codex_execpolicy::MatchOptions;
use codex_execpolicy::NetworkRuleProtocol;
use codex_execpolicy::Policy;
use codex_execpolicy::PolicyParser;
Expand Down Expand Up @@ -221,14 +222,22 @@ impl ExecPolicyManager {
used_complex_parsing,
)
};
let evaluation = exec_policy.check_multiple(commands.iter(), &exec_policy_fallback);
let match_options = MatchOptions {
resolve_host_executables: true,
};
let evaluation = exec_policy.check_multiple_with_options(
commands.iter(),
&exec_policy_fallback,
&match_options,
);

let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
prefix_rule.as_ref(),
&evaluation.matched_rules,
exec_policy.as_ref(),
&commands,
&exec_policy_fallback,
&match_options,
);

match evaluation.decision {
Expand Down Expand Up @@ -630,6 +639,7 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule(
exec_policy: &Policy,
commands: &[Vec<String>],
exec_policy_fallback: &impl Fn(&[String]) -> Decision,
match_options: &MatchOptions,
) -> Option<ExecPolicyAmendment> {
let prefix_rule = prefix_rule?;
if prefix_rule.is_empty() {
Expand All @@ -656,6 +666,7 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule(
&amendment.command,
commands,
exec_policy_fallback,
match_options,
) {
Some(amendment)
} else {
Expand All @@ -668,6 +679,7 @@ fn prefix_rule_would_approve_all_commands(
prefix_rule: &[String],
commands: &[Vec<String>],
exec_policy_fallback: &impl Fn(&[String]) -> Decision,
match_options: &MatchOptions,
) -> bool {
let mut policy_with_prefix_rule = exec_policy.clone();
if policy_with_prefix_rule
Expand All @@ -679,7 +691,7 @@ fn prefix_rule_would_approve_all_commands(

commands.iter().all(|command| {
policy_with_prefix_rule
.check(command, exec_policy_fallback)
.check_with_options(command, exec_policy_fallback, match_options)
.decision
== Decision::Allow
})
Expand Down Expand Up @@ -849,6 +861,15 @@ mod tests {
path.to_string_lossy().into_owned()
}

fn host_program_path(name: &str) -> String {
let executable_name = if cfg!(windows) {
format!("{name}.exe")
} else {
name.to_string()
};
host_absolute_path(&["usr", "bin", &executable_name])
}

fn starlark_string(value: &str) -> String {
value.replace('\\', "\\\\").replace('"', "\\\"")
}
Expand Down Expand Up @@ -1398,6 +1419,115 @@ prefix_rule(
);
}

#[tokio::test]
async fn absolute_path_exec_approval_requirement_matches_host_executable_rules() {
let git_path = host_program_path("git");
let git_path_literal = starlark_string(&git_path);
let policy_src = format!(
r#"
host_executable(name = "git", paths = ["{git_path_literal}"])
prefix_rule(pattern=["git"], decision="allow")
"#
);
let mut parser = PolicyParser::new();
parser
.parse("test.rules", &policy_src)
.expect("parse policy");
let manager = ExecPolicyManager::new(Arc::new(parser.build()));
let command = vec![git_path, "status".to_string()];

let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
.await;

assert_eq!(
requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: true,
proposed_execpolicy_amendment: None,
}
);
}

#[tokio::test]
async fn absolute_path_exec_approval_requirement_ignores_disallowed_host_executable_paths() {
let allowed_git_path = host_program_path("git");
let disallowed_git_path = host_absolute_path(&[
"opt",
"homebrew",
"bin",
if cfg!(windows) { "git.exe" } else { "git" },
]);
let allowed_git_path_literal = starlark_string(&allowed_git_path);
let policy_src = format!(
r#"
host_executable(name = "git", paths = ["{allowed_git_path_literal}"])
prefix_rule(pattern=["git"], decision="prompt")
"#
);
let mut parser = PolicyParser::new();
parser
.parse("test.rules", &policy_src)
.expect("parse policy");
let manager = ExecPolicyManager::new(Arc::new(parser.build()));
let command = vec![disallowed_git_path, "status".to_string()];

let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
.await;

assert_eq!(
requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
}
);
}

#[tokio::test]
async fn requested_prefix_rule_can_approve_absolute_path_commands() {
let command = vec![
host_program_path("cargo"),
"install".to_string(),
"cargo-insta".to_string(),
];
let manager = ExecPolicyManager::default();

let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
})
.await;

assert_eq!(
requirement,
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
"cargo".to_string(),
"install".to_string(),
])),
}
);
}

#[tokio::test]
async fn exec_approval_requirement_respects_approval_policy() {
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
Expand Down Expand Up @@ -1952,6 +2082,7 @@ prefix_rule(
&Policy::empty(),
&commands,
&|_: &[String]| Decision::Allow,
&MatchOptions::default(),
)
}

Expand Down
76 changes: 58 additions & 18 deletions codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use crate::tools::sandboxing::SandboxablePreference;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use codex_execpolicy::Decision;
use codex_execpolicy::Evaluation;
use codex_execpolicy::MatchOptions;
use codex_execpolicy::Policy;
use codex_execpolicy::RuleMatch;
use codex_protocol::config_types::WindowsSandboxLevel;
Expand Down Expand Up @@ -493,29 +495,17 @@ impl EscalationPolicy for CoreShellActionProvider {
.await;
}

let command = join_program_and_argv(program, argv);
let (commands, used_complex_parsing) =
if let Some(commands) = parse_shell_lc_plain_commands(&command) {
(commands, false)
} else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) {
(vec![single_command], true)
} else {
(vec![command.clone()], false)
};

let fallback = |cmd: &[String]| {
crate::exec_policy::render_decision_for_unmatched_command(
let evaluation = {
let policy = self.policy.read().await;
evaluate_intercepted_exec_policy(
&policy,
program,
argv,
self.approval_policy,
&self.sandbox_policy,
cmd,
self.sandbox_permissions,
used_complex_parsing,
)
};
let evaluation = {
let policy = self.policy.read().await;
policy.check_multiple(commands.iter(), &fallback)
};
// When true, means the Evaluation was due to *.rules, not the
// fallback function.
let decision_driven_by_policy =
Expand Down Expand Up @@ -552,6 +542,56 @@ impl EscalationPolicy for CoreShellActionProvider {
}
}

fn evaluate_intercepted_exec_policy(
policy: &Policy,
program: &AbsolutePathBuf,
argv: &[String],
approval_policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
sandbox_permissions: SandboxPermissions,
) -> Evaluation {
let (commands, used_complex_parsing) = commands_for_intercepted_exec_policy(program, argv);

let fallback = |cmd: &[String]| {
crate::exec_policy::render_decision_for_unmatched_command(
approval_policy,
sandbox_policy,
cmd,
sandbox_permissions,
used_complex_parsing,
)
};

policy.check_multiple_with_options(
commands.iter(),
&fallback,
&MatchOptions {
resolve_host_executables: true,
},
)
}

fn commands_for_intercepted_exec_policy(
program: &AbsolutePathBuf,
argv: &[String],
) -> (Vec<Vec<String>>, bool) {
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 (commands, false);
}
if let Some(single_command) = parse_shell_lc_single_command_prefix(&shell_command) {
return (vec![single_command], true);
}
}

(vec![join_program_and_argv(program, argv)], false)
}

struct CoreShellCommandExecutor {
command: Vec<String>,
cwd: PathBuf,
Expand Down
Loading
Loading