mirror of
https://github.com/openai/codex.git
synced 2026-02-03 15:33:41 +00:00
Compare commits
2 Commits
remove/doc
...
pr7037
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4317deb85a | ||
|
|
1273f94f18 |
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -1218,6 +1218,7 @@ dependencies = [
|
||||
"serde_json",
|
||||
"shlex",
|
||||
"starlark",
|
||||
"tempfile",
|
||||
"thiserror 2.0.17",
|
||||
]
|
||||
|
||||
|
||||
@@ -153,6 +153,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
allow_prefix: _allow_prefix,
|
||||
parsed_cmd,
|
||||
}) => match api_version {
|
||||
ApiVersion::V1 => {
|
||||
@@ -610,6 +611,7 @@ async fn on_exec_approval_response(
|
||||
.submit(Op::ExecApproval {
|
||||
id: event_id,
|
||||
decision: response.decision,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
{
|
||||
@@ -783,6 +785,7 @@ async fn on_command_execution_request_approval_response(
|
||||
.submit(Op::ExecApproval {
|
||||
id: event_id,
|
||||
decision,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -68,6 +68,7 @@ use crate::error::CodexErr;
|
||||
use crate::error::Result as CodexResult;
|
||||
#[cfg(test)]
|
||||
use crate::exec::StreamOutput;
|
||||
use crate::exec_policy::ExecPolicyUpdateError;
|
||||
use crate::mcp::auth::compute_auth_statuses;
|
||||
use crate::mcp_connection_manager::McpConnectionManager;
|
||||
use crate::model_family::find_family_for_model;
|
||||
@@ -845,11 +846,43 @@ impl Session {
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn persist_command_allow_prefix(
|
||||
&self,
|
||||
prefix: &[String],
|
||||
) -> Result<(), ExecPolicyUpdateError> {
|
||||
let (features, codex_home) = {
|
||||
let state = self.state.lock().await;
|
||||
(
|
||||
state.session_configuration.features.clone(),
|
||||
state
|
||||
.session_configuration
|
||||
.original_config_do_not_use
|
||||
.codex_home
|
||||
.clone(),
|
||||
)
|
||||
};
|
||||
|
||||
let policy =
|
||||
crate::exec_policy::append_allow_prefix_rule_and_reload(&features, &codex_home, prefix)
|
||||
.await?;
|
||||
|
||||
let mut state = self.state.lock().await;
|
||||
state.session_configuration.exec_policy = policy;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn current_exec_policy(&self) -> Arc<ExecPolicy> {
|
||||
let state = self.state.lock().await;
|
||||
state.session_configuration.exec_policy.clone()
|
||||
}
|
||||
|
||||
/// Emit an exec approval request event and await the user's decision.
|
||||
///
|
||||
/// The request is keyed by `sub_id`/`call_id` so matching responses are delivered
|
||||
/// to the correct in-flight turn. If the task is aborted, this returns the
|
||||
/// default `ReviewDecision` (`Denied`).
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn request_command_approval(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
@@ -858,6 +891,7 @@ impl Session {
|
||||
cwd: PathBuf,
|
||||
reason: Option<String>,
|
||||
risk: Option<SandboxCommandAssessment>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
) -> ReviewDecision {
|
||||
let sub_id = turn_context.sub_id.clone();
|
||||
// Add the tx_approve callback to the map before sending the request.
|
||||
@@ -885,6 +919,7 @@ impl Session {
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
allow_prefix,
|
||||
parsed_cmd,
|
||||
});
|
||||
self.send_event(turn_context, event).await;
|
||||
@@ -1383,8 +1418,12 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
|
||||
handlers::user_input_or_turn(&sess, sub.id.clone(), sub.op, &mut previous_context)
|
||||
.await;
|
||||
}
|
||||
Op::ExecApproval { id, decision } => {
|
||||
handlers::exec_approval(&sess, id, decision).await;
|
||||
Op::ExecApproval {
|
||||
id,
|
||||
decision,
|
||||
allow_prefix,
|
||||
} => {
|
||||
handlers::exec_approval(&sess, id, decision, allow_prefix).await;
|
||||
}
|
||||
Op::PatchApproval { id, decision } => {
|
||||
handlers::patch_approval(&sess, id, decision).await;
|
||||
@@ -1453,6 +1492,7 @@ mod handlers {
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::ReviewRequest;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use std::sync::Arc;
|
||||
@@ -1538,7 +1578,28 @@ mod handlers {
|
||||
*previous_context = Some(turn_context);
|
||||
}
|
||||
|
||||
pub async fn exec_approval(sess: &Arc<Session>, id: String, decision: ReviewDecision) {
|
||||
pub async fn exec_approval(
|
||||
sess: &Arc<Session>,
|
||||
id: String,
|
||||
decision: ReviewDecision,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
) {
|
||||
if let Some(prefix) = allow_prefix
|
||||
&& matches!(
|
||||
decision,
|
||||
ReviewDecision::Approved | ReviewDecision::ApprovedForSession
|
||||
)
|
||||
&& let Err(err) = sess.persist_command_allow_prefix(&prefix).await
|
||||
{
|
||||
let message = format!("Failed to update execpolicy allow list: {err}");
|
||||
tracing::warn!("{message}");
|
||||
let warning = EventMsg::Warning(WarningEvent { message });
|
||||
sess.send_event_raw(Event {
|
||||
id: id.clone(),
|
||||
msg: warning,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
match decision {
|
||||
ReviewDecision::Abort => {
|
||||
sess.interrupt_task().await;
|
||||
|
||||
@@ -235,6 +235,7 @@ async fn handle_exec_approval(
|
||||
event.cwd,
|
||||
event.reason,
|
||||
event.risk,
|
||||
event.allow_prefix,
|
||||
);
|
||||
let decision = await_approval_with_cancel(
|
||||
approval_fut,
|
||||
@@ -244,7 +245,13 @@ async fn handle_exec_approval(
|
||||
)
|
||||
.await;
|
||||
|
||||
let _ = codex.submit(Op::ExecApproval { id, decision }).await;
|
||||
let _ = codex
|
||||
.submit(Op::ExecApproval {
|
||||
id,
|
||||
decision,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
/// Handle an ApplyPatchApprovalRequest by consulting the parent session and replying.
|
||||
|
||||
@@ -297,6 +297,7 @@ impl ShellHandler {
|
||||
let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None);
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let exec_policy = session.current_exec_policy().await;
|
||||
let req = ShellRequest {
|
||||
command: exec_params.command.clone(),
|
||||
cwd: exec_params.cwd.clone(),
|
||||
@@ -305,7 +306,7 @@ impl ShellHandler {
|
||||
with_escalated_permissions: exec_params.with_escalated_permissions,
|
||||
justification: exec_params.justification.clone(),
|
||||
approval_requirement: create_approval_requirement_for_command(
|
||||
&turn.exec_policy,
|
||||
exec_policy.as_ref(),
|
||||
&exec_params.command,
|
||||
turn.approval_policy,
|
||||
&turn.sandbox_policy,
|
||||
|
||||
@@ -63,7 +63,7 @@ impl ToolOrchestrator {
|
||||
ApprovalRequirement::Forbidden { reason } => {
|
||||
return Err(ToolError::Rejected(reason));
|
||||
}
|
||||
ApprovalRequirement::NeedsApproval { reason } => {
|
||||
ApprovalRequirement::NeedsApproval { reason, .. } => {
|
||||
let mut risk = None;
|
||||
|
||||
if let Some(metadata) = req.sandbox_retry_data() {
|
||||
|
||||
@@ -127,6 +127,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
cwd,
|
||||
Some(reason),
|
||||
risk,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
} else if user_explicitly_approved {
|
||||
|
||||
@@ -106,7 +106,15 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, key, move || async move {
|
||||
session
|
||||
.request_command_approval(turn, call_id, command, cwd, reason, risk)
|
||||
.request_command_approval(
|
||||
turn,
|
||||
call_id,
|
||||
command,
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
req.approval_requirement.allow_prefix().cloned(),
|
||||
)
|
||||
.await
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -123,7 +123,15 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, key, || async move {
|
||||
session
|
||||
.request_command_approval(turn, call_id, command, cwd, reason, risk)
|
||||
.request_command_approval(
|
||||
turn,
|
||||
call_id,
|
||||
command,
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
req.approval_requirement.allow_prefix().cloned(),
|
||||
)
|
||||
.await
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -92,11 +92,26 @@ pub(crate) enum ApprovalRequirement {
|
||||
/// No approval required for this tool call
|
||||
Skip,
|
||||
/// Approval required for this tool call
|
||||
NeedsApproval { reason: Option<String> },
|
||||
NeedsApproval {
|
||||
reason: Option<String>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
},
|
||||
/// Execution forbidden for this tool call
|
||||
Forbidden { reason: String },
|
||||
}
|
||||
|
||||
impl ApprovalRequirement {
|
||||
pub fn allow_prefix(&self) -> Option<&Vec<String>> {
|
||||
match self {
|
||||
Self::NeedsApproval {
|
||||
allow_prefix: Some(prefix),
|
||||
..
|
||||
} => Some(prefix),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// - Never, OnFailure: do not ask
|
||||
/// - OnRequest: ask unless sandbox policy is DangerFullAccess
|
||||
/// - UnlessTrusted: always ask
|
||||
@@ -111,7 +126,10 @@ pub(crate) fn default_approval_requirement(
|
||||
};
|
||||
|
||||
if needs_approval {
|
||||
ApprovalRequirement::NeedsApproval { reason: None }
|
||||
ApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: None,
|
||||
}
|
||||
} else {
|
||||
ApprovalRequirement::Skip
|
||||
}
|
||||
|
||||
@@ -445,6 +445,7 @@ impl UnifiedExecSessionManager {
|
||||
) -> Result<UnifiedExecSession, UnifiedExecError> {
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(self);
|
||||
let exec_policy = context.session.current_exec_policy().await;
|
||||
let req = UnifiedExecToolRequest::new(
|
||||
command.to_vec(),
|
||||
cwd,
|
||||
@@ -452,7 +453,7 @@ impl UnifiedExecSessionManager {
|
||||
with_escalated_permissions,
|
||||
justification,
|
||||
create_approval_requirement_for_command(
|
||||
&context.turn.exec_policy,
|
||||
exec_policy.as_ref(),
|
||||
command,
|
||||
context.turn.approval_policy,
|
||||
&context.turn.sandbox_policy,
|
||||
|
||||
@@ -1524,6 +1524,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: *decision,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
@@ -93,6 +93,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() {
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::Approved,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.expect("submit exec approval");
|
||||
|
||||
@@ -843,6 +843,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() {
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::Approved,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -901,6 +902,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision()
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -959,6 +961,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() {
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::Approved,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -1017,6 +1020,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() {
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::Denied,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -1075,6 +1079,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision()
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -1134,6 +1139,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() {
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::Denied,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -27,3 +27,4 @@ thiserror = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
143
codex-rs/execpolicy/src/amend.rs
Normal file
143
codex-rs/execpolicy/src/amend.rs
Normal file
@@ -0,0 +1,143 @@
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use serde_json;
|
||||
use thiserror::Error;
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub enum AmendError {
|
||||
#[error("prefix rule requires at least one token")]
|
||||
EmptyPrefix,
|
||||
#[error("policy path has no parent: {path}")]
|
||||
MissingParent { path: PathBuf },
|
||||
#[error("failed to create policy directory {dir}: {source}")]
|
||||
CreatePolicyDir {
|
||||
dir: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to format prefix token {token}: {source}")]
|
||||
SerializeToken {
|
||||
token: String,
|
||||
source: serde_json::Error,
|
||||
},
|
||||
#[error("failed to open policy file {path}: {source}")]
|
||||
OpenPolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to write to policy file {path}: {source}")]
|
||||
WritePolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to read metadata for policy file {path}: {source}")]
|
||||
PolicyMetadata {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
}
|
||||
|
||||
pub fn append_allow_prefix_rule(policy_path: &Path, prefix: &[String]) -> Result<(), AmendError> {
|
||||
if prefix.is_empty() {
|
||||
return Err(AmendError::EmptyPrefix);
|
||||
}
|
||||
|
||||
let tokens: Vec<String> = prefix
|
||||
.iter()
|
||||
.map(|token| {
|
||||
serde_json::to_string(token).map_err(|source| AmendError::SerializeToken {
|
||||
token: token.clone(),
|
||||
source,
|
||||
})
|
||||
})
|
||||
.collect::<Result<_, _>>()?;
|
||||
let pattern = tokens.join(", ");
|
||||
let rule = format!("prefix_rule(pattern=[{pattern}], decision=\"allow\")\n");
|
||||
|
||||
let dir = policy_path
|
||||
.parent()
|
||||
.ok_or_else(|| AmendError::MissingParent {
|
||||
path: policy_path.to_path_buf(),
|
||||
})?;
|
||||
match std::fs::create_dir(dir) {
|
||||
Ok(()) => {}
|
||||
Err(ref source) if source.kind() == std::io::ErrorKind::AlreadyExists => {}
|
||||
Err(source) => {
|
||||
return Err(AmendError::CreatePolicyDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
});
|
||||
}
|
||||
}
|
||||
let mut file = OpenOptions::new()
|
||||
.create(true)
|
||||
.append(true)
|
||||
.open(policy_path)
|
||||
.map_err(|source| AmendError::OpenPolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
let needs_newline = file
|
||||
.metadata()
|
||||
.map(|metadata| metadata.len() > 0)
|
||||
.map_err(|source| AmendError::PolicyMetadata {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
let final_rule = if needs_newline {
|
||||
format!("\n{rule}")
|
||||
} else {
|
||||
rule
|
||||
};
|
||||
|
||||
file.write_all(final_rule.as_bytes())
|
||||
.map_err(|source| AmendError::WritePolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn appends_rule_and_creates_directories() {
|
||||
let tmp = tempdir().expect("create temp dir");
|
||||
let policy_path = tmp.path().join("policy").join("default.codexpolicy");
|
||||
|
||||
append_allow_prefix_rule(&policy_path, &[String::from("echo"), String::from("Hello, world!")])
|
||||
.expect("append rule");
|
||||
|
||||
let contents =
|
||||
std::fs::read_to_string(&policy_path).expect("default.codexpolicy should exist");
|
||||
assert_eq!(
|
||||
contents,
|
||||
"prefix_rule(pattern=[\"echo\", \"Hello, world!\"], decision=\"allow\")\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn separates_rules_with_newlines_when_appending() {
|
||||
let tmp = tempdir().expect("create temp dir");
|
||||
let policy_path = tmp.path().join("policy").join("default.codexpolicy");
|
||||
std::fs::create_dir_all(policy_path.parent().unwrap()).expect("create policy dir");
|
||||
std::fs::write(
|
||||
&policy_path,
|
||||
"prefix_rule(pattern=[\"ls\"], decision=\"allow\")\n",
|
||||
)
|
||||
.expect("write seed rule");
|
||||
|
||||
append_allow_prefix_rule(&policy_path, &[String::from("echo"), String::from("Hello, world!")]).expect("append rule");
|
||||
|
||||
let contents = std::fs::read_to_string(&policy_path).expect("read policy");
|
||||
assert_eq!(
|
||||
contents,
|
||||
"prefix_rule(pattern=[\"ls\"], decision=\"allow\")\n\nprefix_rule(pattern=[\"echo\", \"Hello, world!\"], decision=\"allow\")\n"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1,9 +1,12 @@
|
||||
pub mod amend;
|
||||
pub mod decision;
|
||||
pub mod error;
|
||||
pub mod parser;
|
||||
pub mod policy;
|
||||
pub mod rule;
|
||||
|
||||
pub use amend::AmendError;
|
||||
pub use amend::append_allow_prefix_rule;
|
||||
pub use decision::Decision;
|
||||
pub use error::Error;
|
||||
pub use error::Result;
|
||||
|
||||
@@ -180,6 +180,7 @@ async fn run_codex_tool_session_inner(
|
||||
call_id,
|
||||
reason: _,
|
||||
risk,
|
||||
allow_prefix: _,
|
||||
parsed_cmd,
|
||||
}) => {
|
||||
handle_exec_approval_request(
|
||||
|
||||
@@ -150,6 +150,7 @@ async fn on_exec_approval_response(
|
||||
.submit(Op::ExecApproval {
|
||||
id: event_id,
|
||||
decision: response.decision,
|
||||
allow_prefix: None,
|
||||
})
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -50,6 +50,10 @@ pub struct ExecApprovalRequestEvent {
|
||||
/// Optional model-provided risk assessment describing the blocked command.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub risk: Option<SandboxCommandAssessment>,
|
||||
/// Prefix rule that can be added to the user's execpolicy to allow future runs.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional, type = "Array<string>")]
|
||||
pub allow_prefix: Option<Vec<String>>,
|
||||
pub parsed_cmd: Vec<ParsedCommand>,
|
||||
}
|
||||
|
||||
|
||||
@@ -143,6 +143,9 @@ pub enum Op {
|
||||
id: String,
|
||||
/// The user's decision in response to the request.
|
||||
decision: ReviewDecision,
|
||||
/// When set, persist this prefix to the execpolicy allow list.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
},
|
||||
|
||||
/// Approve a code patch
|
||||
|
||||
@@ -41,6 +41,7 @@ pub(crate) enum ApprovalRequest {
|
||||
command: Vec<String>,
|
||||
reason: Option<String>,
|
||||
risk: Option<SandboxCommandAssessment>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
},
|
||||
ApplyPatch {
|
||||
id: String,
|
||||
@@ -97,8 +98,8 @@ impl ApprovalOverlay {
|
||||
header: Box<dyn Renderable>,
|
||||
) -> (Vec<ApprovalOption>, SelectionViewParams) {
|
||||
let (options, title) = match &variant {
|
||||
ApprovalVariant::Exec { .. } => (
|
||||
exec_options(),
|
||||
ApprovalVariant::Exec { allow_prefix, .. } => (
|
||||
exec_options(allow_prefix.clone()),
|
||||
"Would you like to run the following command?".to_string(),
|
||||
),
|
||||
ApprovalVariant::ApplyPatch { .. } => (
|
||||
@@ -150,8 +151,8 @@ impl ApprovalOverlay {
|
||||
};
|
||||
if let Some(variant) = self.current_variant.as_ref() {
|
||||
match (&variant, option.decision) {
|
||||
(ApprovalVariant::Exec { id, command }, decision) => {
|
||||
self.handle_exec_decision(id, command, decision);
|
||||
(ApprovalVariant::Exec { id, command, .. }, decision) => {
|
||||
self.handle_exec_decision(id, command, decision, option.allow_prefix.clone());
|
||||
}
|
||||
(ApprovalVariant::ApplyPatch { id, .. }, decision) => {
|
||||
self.handle_patch_decision(id, decision);
|
||||
@@ -163,12 +164,19 @@ impl ApprovalOverlay {
|
||||
self.advance_queue();
|
||||
}
|
||||
|
||||
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
|
||||
fn handle_exec_decision(
|
||||
&self,
|
||||
id: &str,
|
||||
command: &[String],
|
||||
decision: ReviewDecision,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
) {
|
||||
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision);
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
|
||||
self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval {
|
||||
id: id.to_string(),
|
||||
decision,
|
||||
allow_prefix,
|
||||
}));
|
||||
}
|
||||
|
||||
@@ -238,8 +246,8 @@ impl BottomPaneView for ApprovalOverlay {
|
||||
&& let Some(variant) = self.current_variant.as_ref()
|
||||
{
|
||||
match &variant {
|
||||
ApprovalVariant::Exec { id, command } => {
|
||||
self.handle_exec_decision(id, command, ReviewDecision::Abort);
|
||||
ApprovalVariant::Exec { id, command, .. } => {
|
||||
self.handle_exec_decision(id, command, ReviewDecision::Abort, None);
|
||||
}
|
||||
ApprovalVariant::ApplyPatch { id, .. } => {
|
||||
self.handle_patch_decision(id, ReviewDecision::Abort);
|
||||
@@ -291,6 +299,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
|
||||
command,
|
||||
reason,
|
||||
risk,
|
||||
allow_prefix,
|
||||
} => {
|
||||
let reason = reason.filter(|item| !item.is_empty());
|
||||
let has_reason = reason.is_some();
|
||||
@@ -310,7 +319,11 @@ impl From<ApprovalRequest> for ApprovalRequestState {
|
||||
}
|
||||
header.extend(full_cmd_lines);
|
||||
Self {
|
||||
variant: ApprovalVariant::Exec { id, command },
|
||||
variant: ApprovalVariant::Exec {
|
||||
id,
|
||||
command,
|
||||
allow_prefix,
|
||||
},
|
||||
header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })),
|
||||
}
|
||||
}
|
||||
@@ -364,8 +377,14 @@ fn render_risk_lines(risk: &SandboxCommandAssessment) -> Vec<Line<'static>> {
|
||||
|
||||
#[derive(Clone)]
|
||||
enum ApprovalVariant {
|
||||
Exec { id: String, command: Vec<String> },
|
||||
ApplyPatch { id: String },
|
||||
Exec {
|
||||
id: String,
|
||||
command: Vec<String>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
},
|
||||
ApplyPatch {
|
||||
id: String,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -374,6 +393,7 @@ struct ApprovalOption {
|
||||
decision: ReviewDecision,
|
||||
display_shortcut: Option<KeyBinding>,
|
||||
additional_shortcuts: Vec<KeyBinding>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
impl ApprovalOption {
|
||||
@@ -384,27 +404,39 @@ impl ApprovalOption {
|
||||
}
|
||||
}
|
||||
|
||||
fn exec_options() -> Vec<ApprovalOption> {
|
||||
fn exec_options(allow_prefix: Option<Vec<String>>) -> Vec<ApprovalOption> {
|
||||
vec![
|
||||
ApprovalOption {
|
||||
label: "Yes, proceed".to_string(),
|
||||
decision: ReviewDecision::Approved,
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
|
||||
allow_prefix: None,
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Yes, and don't ask again for this command".to_string(),
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "No, and tell Codex what to do differently".to_string(),
|
||||
decision: ReviewDecision::Abort,
|
||||
display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
|
||||
allow_prefix: None,
|
||||
},
|
||||
]
|
||||
.into_iter()
|
||||
.chain(allow_prefix.map(|prefix| ApprovalOption {
|
||||
label: "Yes, and don't ask again for commands with this prefix".to_string(),
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
|
||||
allow_prefix: Some(prefix),
|
||||
}))
|
||||
.chain([ApprovalOption {
|
||||
label: "No, and tell Codex what to do differently".to_string(),
|
||||
decision: ReviewDecision::Abort,
|
||||
display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
|
||||
allow_prefix: None,
|
||||
}])
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn patch_options() -> Vec<ApprovalOption> {
|
||||
@@ -414,12 +446,14 @@ fn patch_options() -> Vec<ApprovalOption> {
|
||||
decision: ReviewDecision::Approved,
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
|
||||
allow_prefix: None,
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "No, and tell Codex what to do differently".to_string(),
|
||||
decision: ReviewDecision::Abort,
|
||||
display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
|
||||
allow_prefix: None,
|
||||
},
|
||||
]
|
||||
}
|
||||
@@ -437,6 +471,7 @@ mod tests {
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
reason: Some("reason".to_string()),
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -469,6 +504,41 @@ mod tests {
|
||||
assert!(saw_op, "expected approval decision to emit an op");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_prefix_option_emits_allow_prefix() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = ApprovalOverlay::new(
|
||||
ApprovalRequest::Exec {
|
||||
id: "test".to_string(),
|
||||
command: vec!["echo".to_string()],
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: Some(vec!["echo".to_string()]),
|
||||
},
|
||||
tx,
|
||||
);
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE));
|
||||
let mut saw_op = false;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if let AppEvent::CodexOp(Op::ExecApproval {
|
||||
allow_prefix,
|
||||
decision,
|
||||
..
|
||||
}) = ev
|
||||
{
|
||||
assert_eq!(decision, ReviewDecision::ApprovedForSession);
|
||||
assert_eq!(allow_prefix, Some(vec!["echo".to_string()]));
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
saw_op,
|
||||
"expected approval decision to emit an op with allow prefix"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn header_includes_command_snippet() {
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
@@ -479,6 +549,7 @@ mod tests {
|
||||
command,
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
};
|
||||
|
||||
let view = ApprovalOverlay::new(exec_request, tx);
|
||||
|
||||
@@ -540,6 +540,7 @@ mod tests {
|
||||
command: vec!["echo".into(), "ok".into()],
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1012,6 +1012,7 @@ impl ChatWidget {
|
||||
command: ev.command,
|
||||
reason: ev.reason,
|
||||
risk: ev.risk,
|
||||
allow_prefix: ev.allow_prefix,
|
||||
};
|
||||
self.bottom_pane.push_approval_request(request);
|
||||
self.request_redraw();
|
||||
|
||||
@@ -587,6 +587,7 @@ fn exec_approval_emits_proposed_command_and_decision_history() {
|
||||
"this is a test reason such as one that would be produced by the model".into(),
|
||||
),
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -631,6 +632,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
"this is a test reason such as one that would be produced by the model".into(),
|
||||
),
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -681,6 +683,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -1830,6 +1833,7 @@ fn approval_modal_exec_snapshot() {
|
||||
"this is a test reason such as one that would be produced by the model".into(),
|
||||
),
|
||||
risk: None,
|
||||
allow_prefix: Some(vec!["echo".into(), "hello".into(), "world".into()]),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -1876,6 +1880,7 @@ fn approval_modal_exec_without_reason_snapshot() {
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: Some(vec!["echo".into(), "hello".into(), "world".into()]),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -2089,6 +2094,7 @@ fn status_widget_and_approval_modal_snapshot() {
|
||||
"this is a test reason such as one that would be produced by the model".into(),
|
||||
),
|
||||
risk: None,
|
||||
allow_prefix: Some(vec!["echo".into(), "hello world".into()]),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
|
||||
Reference in New Issue
Block a user