guard against substitution

This commit is contained in:
Dylan Hurd
2026-02-01 08:41:53 -08:00
parent 825ab4712a
commit 4f77bdc423
2 changed files with 163 additions and 2 deletions

View File

@@ -372,7 +372,15 @@ fn parse_shell_lc_commands_for_execpolicy(command: &[String]) -> Option<Vec<Vec<
}
let (_, script) = extract_bash_command(command)?;
let tokens = shlex_split(script)?;
let tokens = if let Some(tokens) = shlex_split(script) {
tokens
} else {
let first_line = script.lines().next().unwrap_or_default();
if first_line.is_empty() || contains_shell_substitution(first_line) {
return None;
}
shlex_split(first_line)?
};
let mut commands = Vec::new();
let mut current = Vec::new();
@@ -407,7 +415,13 @@ fn finalize_shell_tokens(tokens: &mut Vec<String>) -> Option<Vec<String>> {
index += 1;
}
let command_tokens = tokens[index..].to_vec();
let mut command_tokens = Vec::new();
for token in &tokens[index..] {
if is_redirection_token(token) {
break;
}
command_tokens.push(token.clone());
}
tokens.clear();
if command_tokens.is_empty() {
None
@@ -434,6 +448,14 @@ fn is_env_assignment(token: &str) -> bool {
chars.all(|c| matches!(c, 'A'..='Z' | 'a'..='z' | '0'..='9' | '_'))
}
fn is_redirection_token(token: &str) -> bool {
token.contains('<') || token.contains('>')
}
fn contains_shell_substitution(line: &str) -> bool {
line.chars().any(|c| matches!(c, '$' | '`' | '(' | ')'))
}
/// 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.
@@ -902,6 +924,17 @@ prefix_rule(pattern=["rm"], decision="forbidden")
);
}
#[test]
fn parse_shell_lc_fallback_rejects_substitution_chars() {
let command = vec![
"bash".to_string(),
"-lc".to_string(),
"python3 $(echo hi) <<'PY'\nprint('hi)\nPY".to_string(),
];
assert!(parse_shell_lc_commands_for_execpolicy(&command).is_none());
}
#[tokio::test]
async fn justification_is_included_in_forbidden_exec_approval_requirement() {
let policy_src = r#"

View File

@@ -1850,6 +1850,134 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn heredoc_execpolicy_amendment_skips_future_approvals() -> 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 command = r#"python3 <<'PY'
print("hello")
PY"#;
let call_id_first = "heredoc-allow-first";
let args_first = json!({
"command": command,
"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-heredoc-1"),
first_event,
ev_completed("resp-heredoc-1"),
]),
)
.await;
let _first_results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-heredoc-1", "done"),
ev_completed("resp-heredoc-2"),
]),
)
.await;
submit_turn(
&test,
"heredoc allow first",
approval_policy,
sandbox_policy.clone(),
)
.await?;
let approval = expect_exec_approval(&test, command).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 = "heredoc-allow-second";
let args_second = json!({
"command": command,
"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-heredoc-3"),
second_event,
ev_completed("resp-heredoc-3"),
]),
)
.await;
let second_results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-heredoc-2", "done"),
ev_completed("resp-heredoc-4"),
]),
)
.await;
submit_turn(
&test,
"heredoc allow 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(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn approving_execpolicy_prefix_applies_to_env_prefixed_commands() -> Result<()> {