mirror of
https://github.com/openai/codex.git
synced 2026-05-06 06:12:59 +03:00
Tighten generated execpolicy prefix suggestions
In codex-rs/core/src/exec_policy.rs, narrow auto-generated execpolicy amendments by truncating a command at the first flag-like token, while falling back to the whole command segment when that would leave fewer than two tokens. For parseable multi-command shell scripts, derive the suggestion from the first parsed segment. In codex-rs/core/src/exec_policy_tests.rs, add and update coverage for single-command flag truncation, early-flag fallback, and multi-command first-segment suggestion behavior.
This commit is contained in:
@@ -287,6 +287,7 @@ impl ExecPolicyManager {
|
|||||||
if auto_amendment_allowed {
|
if auto_amendment_allowed {
|
||||||
try_derive_execpolicy_amendment_for_prompt_rules(
|
try_derive_execpolicy_amendment_for_prompt_rules(
|
||||||
&evaluation.matched_rules,
|
&evaluation.matched_rules,
|
||||||
|
&commands,
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
@@ -301,7 +302,10 @@ impl ExecPolicyManager {
|
|||||||
is_policy_match(rule_match) && rule_match.decision() == Decision::Allow
|
is_policy_match(rule_match) && rule_match.decision() == Decision::Allow
|
||||||
}),
|
}),
|
||||||
proposed_execpolicy_amendment: if auto_amendment_allowed {
|
proposed_execpolicy_amendment: if auto_amendment_allowed {
|
||||||
try_derive_execpolicy_amendment_for_allow_rules(&evaluation.matched_rules)
|
try_derive_execpolicy_amendment_for_allow_rules(
|
||||||
|
&evaluation.matched_rules,
|
||||||
|
&commands,
|
||||||
|
)
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
},
|
},
|
||||||
@@ -640,12 +644,13 @@ fn commands_for_exec_policy(command: &[String]) -> (Vec<Vec<String>>, bool) {
|
|||||||
/// - Examples:
|
/// - Examples:
|
||||||
/// - execpolicy: empty. Command: `["python"]`. Heuristics prompt -> `Some(vec!["python"])`.
|
/// - execpolicy: empty. Command: `["python"]`. Heuristics prompt -> `Some(vec!["python"])`.
|
||||||
/// - execpolicy: empty. Command: `["bash", "-c", "cd /some/folder && prog1 --option1 arg1 && prog2 --option2 arg2"]`.
|
/// - execpolicy: empty. Command: `["bash", "-c", "cd /some/folder && prog1 --option1 arg1 && prog2 --option2 arg2"]`.
|
||||||
/// Parsed commands include `cd /some/folder`, `prog1 --option1 arg1`, and `prog2 --option2 arg2`. If heuristics allow `cd` but prompt
|
/// Parsed commands include `cd /some/folder`, `prog1 --option1 arg1`, and `prog2 --option2 arg2`. For multi-command scripts,
|
||||||
/// on `prog1`, we return `Some(vec!["prog1", "--option1", "arg1"])`.
|
/// we derive the suggestion from the first parsed segment, so this returns `Some(vec!["cd", "/some/folder"])`.
|
||||||
/// - execpolicy: contains a `prompt for prefix ["prog2"]` rule. For the same command as above,
|
/// - execpolicy: contains a `prompt for prefix ["prog2"]` rule. For the same command as above,
|
||||||
/// we return `None` because an execpolicy prompt still applies even if we amend execpolicy to allow ["prog1", "--option1", "arg1"].
|
/// we return `None` because an execpolicy prompt still applies even if we amend execpolicy to allow ["cd", "/some/folder"].
|
||||||
fn try_derive_execpolicy_amendment_for_prompt_rules(
|
fn try_derive_execpolicy_amendment_for_prompt_rules(
|
||||||
matched_rules: &[RuleMatch],
|
matched_rules: &[RuleMatch],
|
||||||
|
commands: &[Vec<String>],
|
||||||
) -> Option<ExecPolicyAmendment> {
|
) -> Option<ExecPolicyAmendment> {
|
||||||
if matched_rules
|
if matched_rules
|
||||||
.iter()
|
.iter()
|
||||||
@@ -654,13 +659,17 @@ fn try_derive_execpolicy_amendment_for_prompt_rules(
|
|||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if commands.len() > 1 {
|
||||||
|
return auto_derived_execpolicy_amendment(&commands[0]);
|
||||||
|
}
|
||||||
|
|
||||||
matched_rules
|
matched_rules
|
||||||
.iter()
|
.iter()
|
||||||
.find_map(|rule_match| match rule_match {
|
.find_map(|rule_match| match rule_match {
|
||||||
RuleMatch::HeuristicsRuleMatch {
|
RuleMatch::HeuristicsRuleMatch {
|
||||||
command,
|
command,
|
||||||
decision: Decision::Prompt,
|
decision: Decision::Prompt,
|
||||||
} => Some(ExecPolicyAmendment::from(command.clone())),
|
} => auto_derived_execpolicy_amendment(command),
|
||||||
_ => None,
|
_ => None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -670,22 +679,51 @@ fn try_derive_execpolicy_amendment_for_prompt_rules(
|
|||||||
/// - If any execpolicy rule matches, return None, because we would already be running command outside the sandbox
|
/// - If any execpolicy rule matches, return None, because we would already be running command outside the sandbox
|
||||||
fn try_derive_execpolicy_amendment_for_allow_rules(
|
fn try_derive_execpolicy_amendment_for_allow_rules(
|
||||||
matched_rules: &[RuleMatch],
|
matched_rules: &[RuleMatch],
|
||||||
|
commands: &[Vec<String>],
|
||||||
) -> Option<ExecPolicyAmendment> {
|
) -> Option<ExecPolicyAmendment> {
|
||||||
if matched_rules.iter().any(is_policy_match) {
|
if matched_rules.iter().any(is_policy_match) {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if commands.len() > 1 {
|
||||||
|
return auto_derived_execpolicy_amendment(&commands[0]);
|
||||||
|
}
|
||||||
|
|
||||||
matched_rules
|
matched_rules
|
||||||
.iter()
|
.iter()
|
||||||
.find_map(|rule_match| match rule_match {
|
.find_map(|rule_match| match rule_match {
|
||||||
RuleMatch::HeuristicsRuleMatch {
|
RuleMatch::HeuristicsRuleMatch {
|
||||||
command,
|
command,
|
||||||
decision: Decision::Allow,
|
decision: Decision::Allow,
|
||||||
} => Some(ExecPolicyAmendment::from(command.clone())),
|
} => auto_derived_execpolicy_amendment(command),
|
||||||
_ => None,
|
_ => None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Keep generated execpolicy suggestions broad enough to cover similar
|
||||||
|
/// invocations, but stop before the first flag so we do not bake incidental
|
||||||
|
/// option values into a persisted allow rule.
|
||||||
|
///
|
||||||
|
/// If truncating before the first flag would leave fewer than two tokens, fall
|
||||||
|
/// back to the whole command segment.
|
||||||
|
fn auto_derived_execpolicy_amendment(command: &[String]) -> Option<ExecPolicyAmendment> {
|
||||||
|
let prefix: Vec<String> = command
|
||||||
|
.iter()
|
||||||
|
.take_while(|token| !token.starts_with('-'))
|
||||||
|
.cloned()
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
if prefix.len() >= 2 {
|
||||||
|
return Some(ExecPolicyAmendment::from(prefix));
|
||||||
|
}
|
||||||
|
|
||||||
|
if !command.is_empty() {
|
||||||
|
return Some(ExecPolicyAmendment::from(command.to_vec()));
|
||||||
|
}
|
||||||
|
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
fn derive_requested_execpolicy_amendment_from_prefix_rule(
|
fn derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||||
prefix_rule: Option<&Vec<String>>,
|
prefix_rule: Option<&Vec<String>>,
|
||||||
matched_rules: &[RuleMatch],
|
matched_rules: &[RuleMatch],
|
||||||
|
|||||||
@@ -1032,7 +1032,7 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn empty_bash_lc_script_falls_back_to_original_command() {
|
async fn empty_bash_lc_script_falls_back_to_whole_command_when_truncated_prefix_is_too_short() {
|
||||||
let command = vec!["bash".to_string(), "-lc".to_string(), "".to_string()];
|
let command = vec!["bash".to_string(), "-lc".to_string(), "".to_string()];
|
||||||
|
|
||||||
let manager = ExecPolicyManager::default();
|
let manager = ExecPolicyManager::default();
|
||||||
@@ -1057,7 +1057,8 @@ async fn empty_bash_lc_script_falls_back_to_original_command() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn whitespace_bash_lc_script_falls_back_to_original_command() {
|
async fn whitespace_bash_lc_script_falls_back_to_whole_command_when_truncated_prefix_is_too_short()
|
||||||
|
{
|
||||||
let command = vec![
|
let command = vec![
|
||||||
"bash".to_string(),
|
"bash".to_string(),
|
||||||
"-lc".to_string(),
|
"-lc".to_string(),
|
||||||
@@ -1118,7 +1119,7 @@ async fn request_rule_uses_prefix_rule() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands() {
|
async fn request_rule_falls_back_to_first_segment_for_multi_command_scripts() {
|
||||||
let command = vec![
|
let command = vec![
|
||||||
"bash".to_string(),
|
"bash".to_string(),
|
||||||
"-lc".to_string(),
|
"-lc".to_string(),
|
||||||
@@ -1142,9 +1143,9 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands(
|
|||||||
ExecApprovalRequirement::NeedsApproval {
|
ExecApprovalRequirement::NeedsApproval {
|
||||||
reason: None,
|
reason: None,
|
||||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||||
"rm".to_string(),
|
"cargo".to_string(),
|
||||||
"-rf".to_string(),
|
"install".to_string(),
|
||||||
"/tmp/codex".to_string(),
|
"cargo-insta".to_string(),
|
||||||
])),
|
])),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
@@ -1178,8 +1179,8 @@ async fn heuristics_apply_when_other_commands_match_policy() {
|
|||||||
ExecApprovalRequirement::NeedsApproval {
|
ExecApprovalRequirement::NeedsApproval {
|
||||||
reason: None,
|
reason: None,
|
||||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||||
"orange".to_string()
|
"apple".to_string()
|
||||||
]))
|
])),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -1260,6 +1261,64 @@ async fn proposed_execpolicy_amendment_is_present_for_single_command_without_pol
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn proposed_execpolicy_amendment_stops_before_first_flag_for_generated_suggestions() {
|
||||||
|
let command = vec![
|
||||||
|
"cargo".to_string(),
|
||||||
|
"test".to_string(),
|
||||||
|
"--package".to_string(),
|
||||||
|
"codex-core".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(),
|
||||||
|
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||||
|
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||||
|
prefix_rule: None,
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
requirement,
|
||||||
|
ExecApprovalRequirement::NeedsApproval {
|
||||||
|
reason: None,
|
||||||
|
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||||
|
"cargo".to_string(),
|
||||||
|
"test".to_string(),
|
||||||
|
]))
|
||||||
|
}
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn proposed_execpolicy_amendment_falls_back_to_whole_command_if_flag_starts_too_early() {
|
||||||
|
let command = vec!["curl".to_string(), "-k".to_string(), "xyz.com".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(),
|
||||||
|
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||||
|
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||||
|
prefix_rule: None,
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
requirement,
|
||||||
|
ExecApprovalRequirement::NeedsApproval {
|
||||||
|
reason: None,
|
||||||
|
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command))
|
||||||
|
}
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
|
async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
|
||||||
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
|
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
|
||||||
@@ -1292,7 +1351,7 @@ async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() {
|
async fn proposed_execpolicy_amendment_is_based_on_first_segment_for_multi_command_scripts() {
|
||||||
let command = vec![
|
let command = vec![
|
||||||
"bash".to_string(),
|
"bash".to_string(),
|
||||||
"-lc".to_string(),
|
"-lc".to_string(),
|
||||||
@@ -1323,7 +1382,8 @@ async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scripts() {
|
async fn proposed_execpolicy_amendment_uses_whole_one_token_first_segment_for_multi_command_scripts()
|
||||||
|
{
|
||||||
let policy_src = r#"prefix_rule(pattern=["cat"], decision="allow")"#;
|
let policy_src = r#"prefix_rule(pattern=["cat"], decision="allow")"#;
|
||||||
let mut parser = PolicyParser::new();
|
let mut parser = PolicyParser::new();
|
||||||
parser
|
parser
|
||||||
@@ -1350,9 +1410,7 @@ async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scri
|
|||||||
.await,
|
.await,
|
||||||
ExecApprovalRequirement::NeedsApproval {
|
ExecApprovalRequirement::NeedsApproval {
|
||||||
reason: None,
|
reason: None,
|
||||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["cat".to_string()])),
|
||||||
"apple".to_string()
|
|
||||||
])),
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user