mirror of
https://github.com/openai/codex.git
synced 2026-02-01 14:44:17 +00:00
fix env var execpolicy parsing
This commit is contained in:
@@ -24,11 +24,13 @@ use thiserror::Error;
|
||||
use tokio::fs;
|
||||
use tokio::task::spawn_blocking;
|
||||
|
||||
use crate::bash::extract_bash_command;
|
||||
use crate::bash::parse_shell_lc_plain_commands;
|
||||
use crate::features::Feature;
|
||||
use crate::features::Features;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use shlex::split as shlex_split;
|
||||
use shlex::try_join as shlex_try_join;
|
||||
|
||||
const PROMPT_CONFLICT_REASON: &str =
|
||||
@@ -132,8 +134,8 @@ impl ExecPolicyManager {
|
||||
prefix_rule,
|
||||
} = req;
|
||||
let exec_policy = self.current();
|
||||
let commands =
|
||||
parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]);
|
||||
let commands = parse_shell_lc_commands_for_execpolicy(command)
|
||||
.unwrap_or_else(|| vec![command.to_vec()]);
|
||||
let exec_policy_fallback = |cmd: &[String]| {
|
||||
render_decision_for_unmatched_command(
|
||||
approval_policy,
|
||||
@@ -362,6 +364,76 @@ fn default_policy_path(codex_home: &Path) -> PathBuf {
|
||||
codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE)
|
||||
}
|
||||
|
||||
fn parse_shell_lc_commands_for_execpolicy(command: &[String]) -> Option<Vec<Vec<String>>> {
|
||||
if let Some(commands) = parse_shell_lc_plain_commands(command) {
|
||||
if !commands.is_empty() {
|
||||
return Some(commands);
|
||||
}
|
||||
}
|
||||
|
||||
let (_, script) = extract_bash_command(command)?;
|
||||
let tokens = shlex_split(script)?;
|
||||
let mut commands = Vec::new();
|
||||
let mut current = Vec::new();
|
||||
|
||||
for token in tokens {
|
||||
if is_shell_separator(&token) {
|
||||
if let Some(command_tokens) = finalize_shell_tokens(&mut current) {
|
||||
commands.push(command_tokens);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
current.push(token);
|
||||
}
|
||||
|
||||
if let Some(command_tokens) = finalize_shell_tokens(&mut current) {
|
||||
commands.push(command_tokens);
|
||||
}
|
||||
|
||||
if commands.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(commands)
|
||||
}
|
||||
}
|
||||
|
||||
fn finalize_shell_tokens(tokens: &mut Vec<String>) -> Option<Vec<String>> {
|
||||
if tokens.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut index = 0;
|
||||
while index < tokens.len() && is_env_assignment(&tokens[index]) {
|
||||
index += 1;
|
||||
}
|
||||
|
||||
let command_tokens = tokens[index..].to_vec();
|
||||
tokens.clear();
|
||||
if command_tokens.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(command_tokens)
|
||||
}
|
||||
}
|
||||
|
||||
fn is_shell_separator(token: &str) -> bool {
|
||||
matches!(token, "&&" | "||" | ";" | "|")
|
||||
}
|
||||
|
||||
fn is_env_assignment(token: &str) -> bool {
|
||||
let Some((name, _value)) = token.split_once('=') else {
|
||||
return false;
|
||||
};
|
||||
let mut chars = name.chars();
|
||||
let Some(first) = chars.next() else {
|
||||
return false;
|
||||
};
|
||||
if !matches!(first, 'A'..='Z' | 'a'..='z' | '_') {
|
||||
return false;
|
||||
}
|
||||
chars.all(|c| matches!(c, 'A'..='Z' | 'a'..='z' | '0'..='9' | '_'))
|
||||
}
|
||||
|
||||
/// Derive a proposed execpolicy amendment when a command requires user approval
|
||||
/// - If any execpolicy rule prompts, return None, because an amendment would not skip that policy requirement.
|
||||
/// - Otherwise return the first heuristics Prompt.
|
||||
|
||||
@@ -1849,3 +1849,129 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approving_execpolicy_prefix_applies_to_env_prefixed_commands() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let approval_policy = AskForApproval::UnlessTrusted;
|
||||
let sandbox_policy = SandboxPolicy::ReadOnly;
|
||||
let sandbox_policy_for_config = sandbox_policy.clone();
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id_first = "allow-prefix-env-first";
|
||||
let command_first = "FOO=bar python3 -c 'print(\"first\")'";
|
||||
let args_first = json!({
|
||||
"command": command_first,
|
||||
"timeout_ms": 1_000,
|
||||
"prefix_rule": ["python3"],
|
||||
});
|
||||
let first_event = ev_function_call(
|
||||
call_id_first,
|
||||
"shell_command",
|
||||
&serde_json::to_string(&args_first)?,
|
||||
);
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-prefix-env-1"),
|
||||
first_event,
|
||||
ev_completed("resp-prefix-env-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let _first_results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-prefix-env-1", "done"),
|
||||
ev_completed("resp-prefix-env-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"allow prefix env first",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let approval = expect_exec_approval(&test, command_first).await;
|
||||
let expected_amendment = ExecPolicyAmendment::new(vec!["python3".to_string()]);
|
||||
assert_eq!(
|
||||
approval.proposed_execpolicy_amendment,
|
||||
Some(expected_amendment.clone())
|
||||
);
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: expected_amendment,
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
let policy_path = test.home.path().join("rules").join("default.rules");
|
||||
let policy_contents = fs::read_to_string(&policy_path)?;
|
||||
assert!(
|
||||
policy_contents.contains(r#"prefix_rule(pattern=["python3"], decision="allow")"#),
|
||||
"unexpected policy contents: {policy_contents}"
|
||||
);
|
||||
|
||||
let call_id_second = "allow-prefix-env-second";
|
||||
let command_second = "FOO=baz python3 -c 'print(\"second\")'";
|
||||
let args_second = json!({
|
||||
"command": command_second,
|
||||
"timeout_ms": 1_000,
|
||||
});
|
||||
let second_event = ev_function_call(
|
||||
call_id_second,
|
||||
"shell_command",
|
||||
&serde_json::to_string(&args_second)?,
|
||||
);
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-prefix-env-3"),
|
||||
second_event,
|
||||
ev_completed("resp-prefix-env-3"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let second_results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-prefix-env-2", "done"),
|
||||
ev_completed("resp-prefix-env-4"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"allow prefix env second",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
wait_for_completion_without_approval(&test).await;
|
||||
|
||||
let second_output = parse_result(
|
||||
&second_results
|
||||
.single_request()
|
||||
.function_call_output(call_id_second),
|
||||
);
|
||||
assert_eq!(second_output.exit_code.unwrap_or(0), 0);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user