mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
Gate enhanced execpolicy suggestions behind feature config
In codex-rs/core/src/features.rs and codex-rs/core/config.schema.json, add the enhanced_exec_policy_suggestions feature flag as a disabled-by-default under-development config. In codex-rs/core/src/exec_policy.rs, codex-rs/core/src/tools/handlers/shell.rs, and codex-rs/core/src/unified_exec/process_manager.rs, route auto-generated execpolicy suggestion behavior through the new feature gate while preserving legacy suggestions when it is off. In codex-rs/core/src/exec_policy_tests.rs, cover both the legacy default-off behavior and the enhanced suggestion path explicitly.
This commit is contained in:
@@ -380,6 +380,9 @@
|
||||
"enable_request_compression": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"enhanced_exec_policy_suggestions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"exec_permission_approvals": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -1986,6 +1989,9 @@
|
||||
"enable_request_compression": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"enhanced_exec_policy_suggestions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"exec_permission_approvals": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -223,9 +223,33 @@ impl ExecPolicyManager {
|
||||
self.policy.load_full()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) async fn create_exec_approval_requirement_for_command(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
) -> ExecApprovalRequirement {
|
||||
self.create_exec_approval_requirement_for_command_impl(
|
||||
req, /*enhanced_exec_policy_suggestions*/ false,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn create_exec_approval_requirement_for_command_with_enhanced_suggestions(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
enhanced_exec_policy_suggestions: bool,
|
||||
) -> ExecApprovalRequirement {
|
||||
self.create_exec_approval_requirement_for_command_impl(
|
||||
req,
|
||||
enhanced_exec_policy_suggestions,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn create_exec_approval_requirement_for_command_impl(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
enhanced_exec_policy_suggestions: bool,
|
||||
) -> ExecApprovalRequirement {
|
||||
let ExecApprovalRequest {
|
||||
command,
|
||||
@@ -288,6 +312,7 @@ impl ExecPolicyManager {
|
||||
try_derive_execpolicy_amendment_for_prompt_rules(
|
||||
&evaluation.matched_rules,
|
||||
&commands,
|
||||
enhanced_exec_policy_suggestions,
|
||||
)
|
||||
} else {
|
||||
None
|
||||
@@ -305,6 +330,7 @@ impl ExecPolicyManager {
|
||||
try_derive_execpolicy_amendment_for_allow_rules(
|
||||
&evaluation.matched_rules,
|
||||
&commands,
|
||||
enhanced_exec_policy_suggestions,
|
||||
)
|
||||
} else {
|
||||
None
|
||||
@@ -644,13 +670,15 @@ fn commands_for_exec_policy(command: &[String]) -> (Vec<Vec<String>>, bool) {
|
||||
/// - Examples:
|
||||
/// - execpolicy: empty. Command: `["python"]`. Heuristics prompt -> `Some(vec!["python"])`.
|
||||
/// - 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`. For multi-command scripts,
|
||||
/// we derive the suggestion from the first parsed segment, so this returns `Some(vec!["cd", "/some/folder"])`.
|
||||
/// Parsed commands include `cd /some/folder`, `prog1 --option1 arg1`, and `prog2 --option2 arg2`. In enhanced mode for
|
||||
/// multi-command scripts, 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,
|
||||
/// 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(
|
||||
matched_rules: &[RuleMatch],
|
||||
commands: &[Vec<String>],
|
||||
enhanced_exec_policy_suggestions: bool,
|
||||
) -> Option<ExecPolicyAmendment> {
|
||||
if matched_rules
|
||||
.iter()
|
||||
@@ -659,8 +687,11 @@ fn try_derive_execpolicy_amendment_for_prompt_rules(
|
||||
return None;
|
||||
}
|
||||
|
||||
if commands.len() > 1 {
|
||||
return auto_derived_execpolicy_amendment(&commands[0]);
|
||||
if enhanced_exec_policy_suggestions && commands.len() > 1 {
|
||||
return auto_derived_execpolicy_amendment_for_mode(
|
||||
&commands[0],
|
||||
/*enhanced_exec_policy_suggestions*/ true,
|
||||
);
|
||||
}
|
||||
|
||||
matched_rules
|
||||
@@ -669,7 +700,10 @@ fn try_derive_execpolicy_amendment_for_prompt_rules(
|
||||
RuleMatch::HeuristicsRuleMatch {
|
||||
command,
|
||||
decision: Decision::Prompt,
|
||||
} => auto_derived_execpolicy_amendment(command),
|
||||
} => auto_derived_execpolicy_amendment_for_mode(
|
||||
command,
|
||||
enhanced_exec_policy_suggestions,
|
||||
),
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
@@ -680,13 +714,17 @@ fn try_derive_execpolicy_amendment_for_prompt_rules(
|
||||
fn try_derive_execpolicy_amendment_for_allow_rules(
|
||||
matched_rules: &[RuleMatch],
|
||||
commands: &[Vec<String>],
|
||||
enhanced_exec_policy_suggestions: bool,
|
||||
) -> Option<ExecPolicyAmendment> {
|
||||
if matched_rules.iter().any(is_policy_match) {
|
||||
return None;
|
||||
}
|
||||
|
||||
if commands.len() > 1 {
|
||||
return auto_derived_execpolicy_amendment(&commands[0]);
|
||||
if enhanced_exec_policy_suggestions && commands.len() > 1 {
|
||||
return auto_derived_execpolicy_amendment_for_mode(
|
||||
&commands[0],
|
||||
/*enhanced_exec_policy_suggestions*/ true,
|
||||
);
|
||||
}
|
||||
|
||||
matched_rules
|
||||
@@ -695,18 +733,32 @@ fn try_derive_execpolicy_amendment_for_allow_rules(
|
||||
RuleMatch::HeuristicsRuleMatch {
|
||||
command,
|
||||
decision: Decision::Allow,
|
||||
} => auto_derived_execpolicy_amendment(command),
|
||||
} => auto_derived_execpolicy_amendment_for_mode(
|
||||
command,
|
||||
enhanced_exec_policy_suggestions,
|
||||
),
|
||||
_ => 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.
|
||||
/// In enhanced mode, 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> {
|
||||
fn auto_derived_execpolicy_amendment_for_mode(
|
||||
command: &[String],
|
||||
enhanced_exec_policy_suggestions: bool,
|
||||
) -> Option<ExecPolicyAmendment> {
|
||||
if command.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
if !enhanced_exec_policy_suggestions {
|
||||
return Some(ExecPolicyAmendment::from(command.to_vec()));
|
||||
}
|
||||
|
||||
let prefix: Vec<String> = command
|
||||
.iter()
|
||||
.take_while(|token| !token.starts_with('-'))
|
||||
@@ -717,11 +769,7 @@ fn auto_derived_execpolicy_amendment(command: &[String]) -> Option<ExecPolicyAme
|
||||
return Some(ExecPolicyAmendment::from(prefix));
|
||||
}
|
||||
|
||||
if !command.is_empty() {
|
||||
return Some(ExecPolicyAmendment::from(command.to_vec()));
|
||||
}
|
||||
|
||||
None
|
||||
Some(ExecPolicyAmendment::from(command.to_vec()))
|
||||
}
|
||||
|
||||
fn derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
|
||||
@@ -81,6 +81,17 @@ fn unrestricted_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
FileSystemSandboxPolicy::unrestricted()
|
||||
}
|
||||
|
||||
async fn create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
manager: &ExecPolicyManager,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
) -> ExecApprovalRequirement {
|
||||
manager
|
||||
.create_exec_approval_requirement_for_command_with_enhanced_suggestions(
|
||||
req, /*enhanced_exec_policy_suggestions*/ true,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn test_config() -> (TempDir, Config) {
|
||||
let home = TempDir::new().expect("create temp dir");
|
||||
let config = ConfigBuilder::default()
|
||||
@@ -1127,16 +1138,18 @@ async fn request_rule_falls_back_to_first_segment_for_multi_command_scripts() {
|
||||
];
|
||||
let manager = ExecPolicyManager::default();
|
||||
|
||||
let requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
let requirement = create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
&manager,
|
||||
ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
})
|
||||
.await;
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
@@ -1166,16 +1179,18 @@ async fn heuristics_apply_when_other_commands_match_policy() {
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
ExecPolicyManager::new(policy)
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
&ExecPolicyManager::new(policy),
|
||||
ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
})
|
||||
.await,
|
||||
},
|
||||
)
|
||||
.await,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
@@ -1270,6 +1285,41 @@ async fn proposed_execpolicy_amendment_stops_before_first_flag_for_generated_sug
|
||||
"codex-core".to_string(),
|
||||
];
|
||||
|
||||
let manager = ExecPolicyManager::default();
|
||||
let requirement = create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
&manager,
|
||||
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_preserves_full_command_when_enhanced_suggestions_disabled() {
|
||||
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 {
|
||||
@@ -1286,10 +1336,7 @@ async fn proposed_execpolicy_amendment_stops_before_first_flag_for_generated_sug
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
"test".to_string(),
|
||||
]))
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command))
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -1299,16 +1346,18 @@ async fn proposed_execpolicy_amendment_falls_back_to_whole_command_if_flag_start
|
||||
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 {
|
||||
let requirement = create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
&manager,
|
||||
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;
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
@@ -1358,16 +1407,18 @@ async fn proposed_execpolicy_amendment_is_based_on_first_segment_for_multi_comma
|
||||
"cargo build && echo ok".to_string(),
|
||||
];
|
||||
let manager = ExecPolicyManager::default();
|
||||
let requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
let requirement = create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
&manager,
|
||||
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;
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
@@ -1398,16 +1449,18 @@ async fn proposed_execpolicy_amendment_uses_whole_one_token_first_segment_for_mu
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
ExecPolicyManager::new(policy)
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
create_exec_approval_requirement_with_enhanced_suggestions(
|
||||
&ExecPolicyManager::new(policy),
|
||||
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,
|
||||
},
|
||||
)
|
||||
.await,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["cat".to_string()])),
|
||||
|
||||
@@ -118,6 +118,8 @@ pub enum Feature {
|
||||
UseLegacyLandlock,
|
||||
/// Allow the model to request approval and propose exec rules.
|
||||
RequestRule,
|
||||
/// Use tighter generated execpolicy prefix suggestions for approval prompts.
|
||||
EnhancedExecPolicySuggestions,
|
||||
/// Enable Windows sandbox (restricted token) on Windows.
|
||||
WindowsSandbox,
|
||||
/// Use the elevated Windows sandbox pipeline (setup + runner).
|
||||
@@ -677,6 +679,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Removed,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::EnhancedExecPolicySuggestions,
|
||||
key: "enhanced_exec_policy_suggestions",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::WindowsSandbox,
|
||||
key: "experimental_windows_sandbox",
|
||||
|
||||
@@ -428,18 +428,24 @@ impl ShellHandler {
|
||||
let exec_approval_requirement = session
|
||||
.services
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
sandbox_policy: turn.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &turn.file_system_sandbox_policy,
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
codex_protocol::models::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
effective_additional_permissions.sandbox_permissions
|
||||
.create_exec_approval_requirement_for_command_with_enhanced_suggestions(
|
||||
ExecApprovalRequest {
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
sandbox_policy: turn.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &turn.file_system_sandbox_policy,
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved
|
||||
{
|
||||
codex_protocol::models::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
effective_additional_permissions.sandbox_permissions
|
||||
},
|
||||
prefix_rule,
|
||||
},
|
||||
prefix_rule,
|
||||
})
|
||||
session
|
||||
.features()
|
||||
.enabled(Feature::EnhancedExecPolicySuggestions),
|
||||
)
|
||||
.await;
|
||||
|
||||
let req = ShellRequest {
|
||||
|
||||
@@ -15,6 +15,7 @@ use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::exec_env::create_env;
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
use crate::features::Feature;
|
||||
use crate::protocol::ExecCommandSource;
|
||||
use crate::sandboxing::ExecRequest;
|
||||
use crate::tools::context::ExecCommandToolOutput;
|
||||
@@ -596,18 +597,24 @@ impl UnifiedExecProcessManager {
|
||||
.session
|
||||
.services
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
sandbox_policy: context.turn.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &context.turn.file_system_sandbox_policy,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
request.sandbox_permissions
|
||||
.create_exec_approval_requirement_for_command_with_enhanced_suggestions(
|
||||
ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
sandbox_policy: context.turn.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &context.turn.file_system_sandbox_policy,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
request.sandbox_permissions
|
||||
},
|
||||
prefix_rule: request.prefix_rule.clone(),
|
||||
},
|
||||
prefix_rule: request.prefix_rule.clone(),
|
||||
})
|
||||
context
|
||||
.session
|
||||
.features()
|
||||
.enabled(Feature::EnhancedExecPolicySuggestions),
|
||||
)
|
||||
.await;
|
||||
let req = UnifiedExecToolRequest {
|
||||
command: request.command.clone(),
|
||||
|
||||
Reference in New Issue
Block a user