mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
changes
This commit is contained in:
@@ -161,6 +161,35 @@ impl ExecPolicyManager {
|
||||
pub(crate) async fn create_exec_approval_requirement_for_command(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
) -> ExecApprovalRequirement {
|
||||
let exec_policy = self.current();
|
||||
Self::create_exec_approval_requirement_for_command_with_policy(exec_policy.as_ref(), req)
|
||||
}
|
||||
|
||||
pub(crate) async fn create_exec_approval_requirement_for_command_with_overlay(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
overlay_prompt_prefixes: &[Vec<String>],
|
||||
) -> ExecApprovalRequirement {
|
||||
if overlay_prompt_prefixes.is_empty() {
|
||||
return self.create_exec_approval_requirement_for_command(req).await;
|
||||
}
|
||||
|
||||
let mut exec_policy = self.current().as_ref().clone();
|
||||
for prefix in overlay_prompt_prefixes {
|
||||
if let Err(err) = exec_policy.add_prefix_rule(prefix, Decision::Prompt) {
|
||||
tracing::warn!(
|
||||
"failed to add in-memory execpolicy overlay prompt prefix {prefix:?}: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Self::create_exec_approval_requirement_for_command_with_policy(&exec_policy, req)
|
||||
}
|
||||
|
||||
fn create_exec_approval_requirement_for_command_with_policy(
|
||||
exec_policy: &Policy,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
) -> ExecApprovalRequirement {
|
||||
let ExecApprovalRequest {
|
||||
command,
|
||||
@@ -169,7 +198,6 @@ impl ExecPolicyManager {
|
||||
sandbox_permissions,
|
||||
prefix_rule,
|
||||
} = req;
|
||||
let exec_policy = self.current();
|
||||
let (commands, used_complex_parsing) = commands_for_exec_policy(command);
|
||||
// Keep heredoc prefix parsing for rule evaluation so existing
|
||||
// allow/prompt/forbidden rules still apply, but avoid auto-derived
|
||||
@@ -1221,6 +1249,64 @@ prefix_rule(
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_overlay_merges_with_base_policy_without_mutating_manager() {
|
||||
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.rules", policy_src)
|
||||
.expect("parse policy");
|
||||
let manager = ExecPolicyManager::new(Arc::new(parser.build()));
|
||||
let overlay_prefix = vec!["/tmp/skill-script.sh".to_string()];
|
||||
|
||||
let overlay_prompted = manager
|
||||
.create_exec_approval_requirement_for_command_with_overlay(
|
||||
ExecApprovalRequest {
|
||||
command: &overlay_prefix,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
&[overlay_prefix.clone()],
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
overlay_prompted,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: Some("`/tmp/skill-script.sh` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
|
||||
let rm_command = vec!["rm".to_string(), "-f".to_string(), "tmp.txt".to_string()];
|
||||
let base_rule_still_applies = manager
|
||||
.create_exec_approval_requirement_for_command_with_overlay(
|
||||
ExecApprovalRequest {
|
||||
command: &rm_command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
&[overlay_prefix],
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
base_rule_still_applies,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: Some("`rm -f tmp.txt` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
manager.current().get_allowed_prefixes(),
|
||||
Vec::<Vec<String>>::new(),
|
||||
"overlay prefixes must not persist in session execpolicy state"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_bash_lc_script_falls_back_to_original_command() {
|
||||
let command = vec!["bash".to_string(), "-lc".to_string(), "".to_string()];
|
||||
|
||||
@@ -9,6 +9,8 @@ use crate::error::CodexErr;
|
||||
#[cfg(unix)]
|
||||
use crate::error::SandboxErr;
|
||||
#[cfg(unix)]
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
#[cfg(unix)]
|
||||
use crate::protocol::EventMsg;
|
||||
#[cfg(unix)]
|
||||
use crate::protocol::ExecCommandOutputDeltaEvent;
|
||||
@@ -17,9 +19,11 @@ use crate::protocol::ExecOutputStream;
|
||||
#[cfg(unix)]
|
||||
use crate::protocol::ReviewDecision;
|
||||
#[cfg(unix)]
|
||||
use anyhow::Context as _;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::approvals::ExecPolicyAmendment;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
#[cfg(unix)]
|
||||
use anyhow::Context as _;
|
||||
#[cfg(unix)]
|
||||
use codex_utils_pty::process_group::kill_child_process_group;
|
||||
#[cfg(unix)]
|
||||
@@ -349,6 +353,67 @@ impl ZshExecBridge {
|
||||
argv.clone()
|
||||
};
|
||||
|
||||
let mut command_for_execpolicy = command_for_approval.clone();
|
||||
if let Some(program) = command_for_execpolicy.first_mut() {
|
||||
*program = file.clone();
|
||||
}
|
||||
|
||||
let overlay_prompt_prefixes = {
|
||||
let registry = turn
|
||||
.skill_prefix_permissions
|
||||
.read()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
registry.keys().cloned().collect::<Vec<_>>()
|
||||
};
|
||||
let exec_approval_requirement = session
|
||||
.services
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command_with_overlay(
|
||||
ExecApprovalRequest {
|
||||
command: &command_for_execpolicy,
|
||||
approval_policy: turn.approval_policy,
|
||||
sandbox_policy: &turn.sandbox_policy,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
&overlay_prompt_prefixes,
|
||||
)
|
||||
.await;
|
||||
|
||||
let (reason, proposed_execpolicy_amendment) = match exec_approval_requirement {
|
||||
ExecApprovalRequirement::Skip { .. } => {
|
||||
write_json_line(
|
||||
&mut stream,
|
||||
&WrapperIpcResponse::ExecResponse {
|
||||
request_id,
|
||||
action: WrapperExecAction::Run,
|
||||
reason: None,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
return Ok(false);
|
||||
}
|
||||
ExecApprovalRequirement::Forbidden { reason } => {
|
||||
write_json_line(
|
||||
&mut stream,
|
||||
&WrapperIpcResponse::ExecResponse {
|
||||
request_id,
|
||||
action: WrapperExecAction::Deny,
|
||||
reason: Some(reason),
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
return Ok(true);
|
||||
}
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason,
|
||||
proposed_execpolicy_amendment,
|
||||
} => (
|
||||
reason.or(approval_reason.clone()),
|
||||
proposed_execpolicy_amendment,
|
||||
),
|
||||
};
|
||||
|
||||
let approval_id = Uuid::new_v4().to_string();
|
||||
let decision = session
|
||||
.request_command_approval(
|
||||
@@ -357,9 +422,9 @@ impl ZshExecBridge {
|
||||
Some(approval_id),
|
||||
command_for_approval,
|
||||
PathBuf::from(cwd),
|
||||
approval_reason,
|
||||
reason,
|
||||
None,
|
||||
None::<ExecPolicyAmendment>,
|
||||
proposed_execpolicy_amendment,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user