Skip to content

Commit a1ad578

Browse files
committed
core: adopt host_executable() rules in zsh-fork
1 parent b148d98 commit a1ad578

File tree

2 files changed

+301
-20
lines changed

2 files changed

+301
-20
lines changed

codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs

Lines changed: 95 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ use crate::tools::sandboxing::SandboxablePreference;
1616
use crate::tools::sandboxing::ToolCtx;
1717
use crate::tools::sandboxing::ToolError;
1818
use codex_execpolicy::Decision;
19+
use codex_execpolicy::Evaluation;
20+
use codex_execpolicy::MatchOptions;
1921
use codex_execpolicy::Policy;
2022
use codex_execpolicy::RuleMatch;
2123
use codex_protocol::config_types::WindowsSandboxLevel;
@@ -431,6 +433,12 @@ impl CoreShellActionProvider {
431433
}
432434
}
433435

436+
// Shell-wrapper parsing is weaker than direct exec interception because it can
437+
// only see the script text, not the final resolved executable path. Keep it
438+
// disabled by default so path-sensitive rules rely on the later authoritative
439+
// execve interception.
440+
const ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING: bool = false;
441+
434442
#[async_trait::async_trait]
435443
impl EscalationPolicy for CoreShellActionProvider {
436444
async fn determine_action(
@@ -493,29 +501,18 @@ impl EscalationPolicy for CoreShellActionProvider {
493501
.await;
494502
}
495503

496-
let command = join_program_and_argv(program, argv);
497-
let (commands, used_complex_parsing) =
498-
if let Some(commands) = parse_shell_lc_plain_commands(&command) {
499-
(commands, false)
500-
} else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) {
501-
(vec![single_command], true)
502-
} else {
503-
(vec![command.clone()], false)
504-
};
505-
506-
let fallback = |cmd: &[String]| {
507-
crate::exec_policy::render_decision_for_unmatched_command(
504+
let evaluation = {
505+
let policy = self.policy.read().await;
506+
evaluate_intercepted_exec_policy(
507+
&policy,
508+
program,
509+
argv,
508510
self.approval_policy,
509511
&self.sandbox_policy,
510-
cmd,
511512
self.sandbox_permissions,
512-
used_complex_parsing,
513+
ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING,
513514
)
514515
};
515-
let evaluation = {
516-
let policy = self.policy.read().await;
517-
policy.check_multiple(commands.iter(), &fallback)
518-
};
519516
// When true, means the Evaluation was due to *.rules, not the
520517
// fallback function.
521518
let decision_driven_by_policy =
@@ -552,6 +549,86 @@ impl EscalationPolicy for CoreShellActionProvider {
552549
}
553550
}
554551

552+
fn evaluate_intercepted_exec_policy(
553+
policy: &Policy,
554+
program: &AbsolutePathBuf,
555+
argv: &[String],
556+
approval_policy: AskForApproval,
557+
sandbox_policy: &SandboxPolicy,
558+
sandbox_permissions: SandboxPermissions,
559+
enable_intercepted_exec_policy_shell_wrapper_parsing: bool,
560+
) -> Evaluation {
561+
let CandidateCommands {
562+
commands,
563+
used_complex_parsing,
564+
} = if enable_intercepted_exec_policy_shell_wrapper_parsing {
565+
// In this codepath, the first argument in `commands` could be a bare
566+
// name like `find` instead of an absolute path like `/usr/bin/find`.
567+
// It could also be a shell built-in like `echo`.
568+
commands_for_intercepted_exec_policy(program, argv)
569+
} else {
570+
// In this codepath, `commands` has a single entry where the program
571+
// is always an absolute path.
572+
CandidateCommands {
573+
commands: vec![join_program_and_argv(program, argv)],
574+
used_complex_parsing: false,
575+
}
576+
};
577+
578+
let fallback = |cmd: &[String]| {
579+
crate::exec_policy::render_decision_for_unmatched_command(
580+
approval_policy,
581+
sandbox_policy,
582+
cmd,
583+
sandbox_permissions,
584+
used_complex_parsing,
585+
)
586+
};
587+
588+
policy.check_multiple_with_options(
589+
commands.iter(),
590+
&fallback,
591+
&MatchOptions {
592+
resolve_host_executables: true,
593+
},
594+
)
595+
}
596+
597+
struct CandidateCommands {
598+
commands: Vec<Vec<String>>,
599+
used_complex_parsing: bool,
600+
}
601+
602+
fn commands_for_intercepted_exec_policy(
603+
program: &AbsolutePathBuf,
604+
argv: &[String],
605+
) -> CandidateCommands {
606+
if let [_, flag, script] = argv {
607+
let shell_command = [
608+
program.to_string_lossy().to_string(),
609+
flag.clone(),
610+
script.clone(),
611+
];
612+
if let Some(commands) = parse_shell_lc_plain_commands(&shell_command) {
613+
return CandidateCommands {
614+
commands,
615+
used_complex_parsing: false,
616+
};
617+
}
618+
if let Some(single_command) = parse_shell_lc_single_command_prefix(&shell_command) {
619+
return CandidateCommands {
620+
commands: vec![single_command],
621+
used_complex_parsing: true,
622+
};
623+
}
624+
}
625+
626+
CandidateCommands {
627+
commands: vec![join_program_and_argv(program, argv)],
628+
used_complex_parsing: false,
629+
}
630+
}
631+
555632
struct CoreShellCommandExecutor {
556633
command: Vec<String>,
557634
cwd: PathBuf,

codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs

Lines changed: 206 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ use super::CoreShellActionProvider;
22
#[cfg(target_os = "macos")]
33
use super::CoreShellCommandExecutor;
44
use super::ParsedShellCommand;
5+
use super::commands_for_intercepted_exec_policy;
6+
use super::evaluate_intercepted_exec_policy;
57
use super::extract_shell_script;
68
use super::join_program_and_argv;
79
use super::map_exec_result;
@@ -12,14 +14,16 @@ use crate::config::Permissions;
1214
#[cfg(target_os = "macos")]
1315
use crate::config::types::ShellEnvironmentPolicy;
1416
use crate::exec::SandboxType;
15-
#[cfg(target_os = "macos")]
1617
use crate::protocol::AskForApproval;
1718
use crate::protocol::ReadOnlyAccess;
1819
use crate::protocol::SandboxPolicy;
19-
#[cfg(target_os = "macos")]
2020
use crate::sandboxing::SandboxPermissions;
2121
#[cfg(target_os = "macos")]
2222
use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE;
23+
use codex_execpolicy::Decision;
24+
use codex_execpolicy::Evaluation;
25+
use codex_execpolicy::PolicyParser;
26+
use codex_execpolicy::RuleMatch;
2327
#[cfg(target_os = "macos")]
2428
use codex_protocol::config_types::WindowsSandboxLevel;
2529
use codex_protocol::models::FileSystemPermissions;
@@ -36,8 +40,25 @@ use codex_utils_absolute_path::AbsolutePathBuf;
3640
use pretty_assertions::assert_eq;
3741
#[cfg(target_os = "macos")]
3842
use std::collections::HashMap;
43+
use std::path::PathBuf;
3944
use std::time::Duration;
4045

46+
fn host_absolute_path(segments: &[&str]) -> String {
47+
let mut path = if cfg!(windows) {
48+
PathBuf::from(r"C:\")
49+
} else {
50+
PathBuf::from("/")
51+
};
52+
for segment in segments {
53+
path.push(segment);
54+
}
55+
path.to_string_lossy().into_owned()
56+
}
57+
58+
fn starlark_string(value: &str) -> String {
59+
value.replace('\\', "\\\\").replace('"', "\\\"")
60+
}
61+
4162
#[test]
4263
fn extract_shell_script_preserves_login_flag() {
4364
assert_eq!(
@@ -126,6 +147,24 @@ fn join_program_and_argv_replaces_original_argv_zero() {
126147
);
127148
}
128149

150+
#[test]
151+
fn commands_for_intercepted_exec_policy_parses_plain_shell_wrappers() {
152+
let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "bash"])).unwrap();
153+
let candidate_commands = commands_for_intercepted_exec_policy(
154+
&program,
155+
&["not-bash".into(), "-lc".into(), "git status && pwd".into()],
156+
);
157+
158+
assert_eq!(
159+
candidate_commands.commands,
160+
vec![
161+
vec!["git".to_string(), "status".to_string()],
162+
vec!["pwd".to_string()],
163+
]
164+
);
165+
assert!(!candidate_commands.used_complex_parsing);
166+
}
167+
129168
#[test]
130169
fn map_exec_result_preserves_stdout_and_stderr() {
131170
let out = map_exec_result(
@@ -203,6 +242,171 @@ fn shell_request_escalation_execution_is_explicit() {
203242
);
204243
}
205244

245+
#[test]
246+
fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_parsing_disabled() {
247+
let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#;
248+
let mut parser = PolicyParser::new();
249+
parser.parse("test.rules", policy_src).unwrap();
250+
let policy = parser.build();
251+
let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "zsh"])).unwrap();
252+
253+
let enable_intercepted_exec_policy_shell_wrapper_parsing = false;
254+
let evaluation = evaluate_intercepted_exec_policy(
255+
&policy,
256+
&program,
257+
&[
258+
"zsh".to_string(),
259+
"-lc".to_string(),
260+
"npm publish".to_string(),
261+
],
262+
AskForApproval::OnRequest,
263+
&SandboxPolicy::new_read_only_policy(),
264+
SandboxPermissions::UseDefault,
265+
enable_intercepted_exec_policy_shell_wrapper_parsing,
266+
);
267+
268+
assert!(
269+
matches!(
270+
evaluation.matched_rules.as_slice(),
271+
[RuleMatch::HeuristicsRuleMatch { command, decision: Decision::Allow }]
272+
if command == &vec![
273+
program.to_string_lossy().to_string(),
274+
"-lc".to_string(),
275+
"npm publish".to_string(),
276+
]
277+
),
278+
r#"This is allowed because when shell wrapper parsing is disabled,
279+
the policy evaluation does not try to parse the shell command and instead
280+
matches the whole command line with the resolved program path, which in this
281+
case is `/bin/zsh` followed by some arguments.
282+
283+
Because there is no policy rule for `/bin/zsh` or `zsh`, the decision is to
284+
allow the command and let the sandbox be responsible for enforcing any
285+
restrictions.
286+
287+
That said, if /bin/zsh is the zsh-fork, then the execve wrapper should
288+
ultimately intercept the `npm publish` command and apply the policy rules to it.
289+
"#
290+
);
291+
}
292+
293+
#[test]
294+
fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled() {
295+
let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#;
296+
let mut parser = PolicyParser::new();
297+
parser.parse("test.rules", policy_src).unwrap();
298+
let policy = parser.build();
299+
let program = AbsolutePathBuf::try_from(host_absolute_path(&["bin", "bash"])).unwrap();
300+
301+
let enable_intercepted_exec_policy_shell_wrapper_parsing = true;
302+
let evaluation = evaluate_intercepted_exec_policy(
303+
&policy,
304+
&program,
305+
&[
306+
"bash".to_string(),
307+
"-lc".to_string(),
308+
"npm publish".to_string(),
309+
],
310+
AskForApproval::OnRequest,
311+
&SandboxPolicy::new_read_only_policy(),
312+
SandboxPermissions::UseDefault,
313+
enable_intercepted_exec_policy_shell_wrapper_parsing,
314+
);
315+
316+
assert_eq!(
317+
evaluation,
318+
Evaluation {
319+
decision: Decision::Prompt,
320+
matched_rules: vec![RuleMatch::PrefixRuleMatch {
321+
matched_prefix: vec!["npm".to_string(), "publish".to_string()],
322+
decision: Decision::Prompt,
323+
resolved_program: None,
324+
justification: None,
325+
}],
326+
}
327+
);
328+
}
329+
330+
#[test]
331+
fn intercepted_exec_policy_uses_host_executable_mappings() {
332+
let git_path = host_absolute_path(&["usr", "bin", "git"]);
333+
let git_path_literal = starlark_string(&git_path);
334+
let policy_src = format!(
335+
r#"
336+
prefix_rule(pattern = ["git", "status"], decision = "prompt")
337+
host_executable(name = "git", paths = ["{git_path_literal}"])
338+
"#
339+
);
340+
let mut parser = PolicyParser::new();
341+
parser.parse("test.rules", &policy_src).unwrap();
342+
let policy = parser.build();
343+
let program = AbsolutePathBuf::try_from(git_path).unwrap();
344+
345+
let evaluation = evaluate_intercepted_exec_policy(
346+
&policy,
347+
&program,
348+
&["git".to_string(), "status".to_string()],
349+
AskForApproval::OnRequest,
350+
&SandboxPolicy::new_read_only_policy(),
351+
SandboxPermissions::UseDefault,
352+
false,
353+
);
354+
355+
assert_eq!(
356+
evaluation,
357+
Evaluation {
358+
decision: Decision::Prompt,
359+
matched_rules: vec![RuleMatch::PrefixRuleMatch {
360+
matched_prefix: vec!["git".to_string(), "status".to_string()],
361+
decision: Decision::Prompt,
362+
resolved_program: Some(program),
363+
justification: None,
364+
}],
365+
}
366+
);
367+
assert!(CoreShellActionProvider::decision_driven_by_policy(
368+
&evaluation.matched_rules,
369+
evaluation.decision
370+
));
371+
}
372+
373+
#[test]
374+
fn intercepted_exec_policy_rejects_disallowed_host_executable_mapping() {
375+
let allowed_git = host_absolute_path(&["usr", "bin", "git"]);
376+
let other_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]);
377+
let allowed_git_literal = starlark_string(&allowed_git);
378+
let policy_src = format!(
379+
r#"
380+
prefix_rule(pattern = ["git", "status"], decision = "prompt")
381+
host_executable(name = "git", paths = ["{allowed_git_literal}"])
382+
"#
383+
);
384+
let mut parser = PolicyParser::new();
385+
parser.parse("test.rules", &policy_src).unwrap();
386+
let policy = parser.build();
387+
let program = AbsolutePathBuf::try_from(other_git.clone()).unwrap();
388+
389+
let evaluation = evaluate_intercepted_exec_policy(
390+
&policy,
391+
&program,
392+
&["git".to_string(), "status".to_string()],
393+
AskForApproval::OnRequest,
394+
&SandboxPolicy::new_read_only_policy(),
395+
SandboxPermissions::UseDefault,
396+
false,
397+
);
398+
399+
assert!(matches!(
400+
evaluation.matched_rules.as_slice(),
401+
[RuleMatch::HeuristicsRuleMatch { command, .. }]
402+
if command == &vec![other_git, "status".to_string()]
403+
));
404+
assert!(!CoreShellActionProvider::decision_driven_by_policy(
405+
&evaluation.matched_rules,
406+
evaluation.decision
407+
));
408+
}
409+
206410
#[cfg(target_os = "macos")]
207411
#[tokio::test]
208412
async fn prepare_escalated_exec_turn_default_preserves_macos_seatbelt_extensions() {

0 commit comments

Comments
 (0)