openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-30 19:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

second pass at tui integration

Browse Source
This commit is contained in:
kevin zhao
2025-11-20 12:25:23 -05:00
parent 54e6e4ac32
commit 4e2112e490
26 changed files with 594 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+63 -3
View File
@@ -845,6 +845,37 @@ impl Session {
.await
}
pub(crate) async fn persist_command_allow_prefix(
&self,
prefix: &[String],
) -> Result<(), crate::exec_policy::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
@@ -858,6 +889,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 +917,7 @@ impl Session {
cwd,
reason,
risk,
allow_prefix,
parsed_cmd,
});
self.send_event(turn_context, event).await;
@@ -1383,8 +1416,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 +1490,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 +1576,29 @@ 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
)
{
if 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;
codex-rs/core/src/codex_delegate.rs
+8 -1
View File
@@ -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.
codex-rs/core/src/default_client.rs
+6 -2
View File
@@ -288,7 +288,7 @@ mod tests {
#[test]
fn test_get_codex_user_agent() {
let user_agent = get_codex_user_agent();
assert!(user_agent.starts_with("codex_cli_rs/"));
assert!(user_agent.starts_with(&format!("{}/", originator().value)));
}
#[tokio::test]
@@ -365,9 +365,13 @@ mod tests {
#[cfg(target_os = "macos")]
fn test_macos() {
use regex_lite::Regex;
let originator = originator().value.clone();
let user_agent = get_codex_user_agent();
let re = Regex::new(
r"^codex_cli_rs/\d+\.\d+\.\d+ \(Mac OS \d+\.\d+\.\d+; (x86_64|arm64)\) (\S+)$",
format!(
r"^{originator}/\d+\.\d+\.\d+ \(Mac OS \d+\.\d+\.\d+; (x86_64|arm64)\) (\S+)$"
)
.as_str(),
)
.unwrap();
assert!(re.is_match(&user_agent));
codex-rs/core/src/exec_policy.rs
+195 -27
View File
@@ -4,10 +4,12 @@ use std::path::PathBuf;
use std::sync::Arc;
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
use codex_execpolicy::AmendError;
use codex_execpolicy::Decision;
use codex_execpolicy::Evaluation;
use codex_execpolicy::Policy;
use codex_execpolicy::PolicyParser;
use codex_execpolicy::append_allow_prefix_rule;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use thiserror::Error;
@@ -23,6 +25,13 @@ const FORBIDDEN_REASON: &str = "execpolicy forbids this command";
const PROMPT_REASON: &str = "execpolicy requires approval for this command";
const POLICY_DIR_NAME: &str = "policy";
const POLICY_EXTENSION: &str = "codexpolicy";
const DEFAULT_POLICY_FILE: &str = "default.codexpolicy";
#[derive(Debug, PartialEq)]
pub(crate) struct CommandApprovalOutcome {
pub requirement: ApprovalRequirement,
pub allow_prefix: Option<Vec<String>>,
}
#[derive(Debug, Error)]
pub enum ExecPolicyError {
@@ -45,6 +54,15 @@ pub enum ExecPolicyError {
},
}
#[derive(Debug, Error)]
pub enum ExecPolicyUpdateError {
#[error("failed to update execpolicy file {path}: {source}")]
AppendRule { path: PathBuf, source: AmendError },
#[error("failed to reload execpolicy after updating policy: {0}")]
Reload(#[from] ExecPolicyError),
}
pub(crate) async fn exec_policy_for(
features: &Features,
codex_home: &Path,
@@ -84,6 +102,105 @@ pub(crate) async fn exec_policy_for(
Ok(policy)
}
pub(crate) fn default_policy_path(codex_home: &Path) -> PathBuf {
codex_home.join(POLICY_DIR_NAME).join(DEFAULT_POLICY_FILE)
}
pub(crate) async fn append_allow_prefix_rule_and_reload(
features: &Features,
codex_home: &Path,
prefix: &[String],
) -> Result<Arc<Policy>, ExecPolicyUpdateError> {
let policy_path = default_policy_path(codex_home);
append_allow_prefix_rule(&policy_path, prefix).map_err(|source| {
ExecPolicyUpdateError::AppendRule {
path: policy_path,
source,
}
})?;
exec_policy_for(features, codex_home)
.await
.map_err(ExecPolicyUpdateError::Reload)
}
fn requirement_from_decision(
decision: Decision,
approval_policy: AskForApproval,
) -> ApprovalRequirement {
match decision {
Decision::Forbidden => ApprovalRequirement::Forbidden {
reason: FORBIDDEN_REASON.to_string(),
},
Decision::Prompt => {
let reason = PROMPT_REASON.to_string();
if matches!(approval_policy, AskForApproval::Never) {
ApprovalRequirement::Forbidden { reason }
} else {
ApprovalRequirement::NeedsApproval {
reason: Some(reason),
}
}
}
Decision::Allow => ApprovalRequirement::Skip,
}
}
fn allow_prefix_if_applicable(
commands: &[Vec<String>],
evaluation: &Evaluation,
requirement: &ApprovalRequirement,
) -> Option<Vec<String>> {
if !matches!(requirement, ApprovalRequirement::NeedsApproval { .. }) {
return None;
}
if !matches!(evaluation, Evaluation::NoMatch) {
return None;
}
let [single_command] = commands else {
return None;
};
Some(single_command.clone())
}
pub(crate) fn command_approval_outcome_for_command(
policy: &Policy,
command: &[String],
approval_policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
sandbox_permissions: SandboxPermissions,
) -> CommandApprovalOutcome {
let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]);
let evaluation = policy.check_multiple(commands.iter());
let requirement = match evaluation {
Evaluation::Match { decision, .. } => requirement_from_decision(decision, approval_policy),
Evaluation::NoMatch => {
if requires_initial_appoval(
approval_policy,
sandbox_policy,
command,
sandbox_permissions,
) {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
}
};
let allow_prefix = allow_prefix_if_applicable(&commands, &evaluation, &requirement);
CommandApprovalOutcome {
requirement,
allow_prefix,
}
}
#[cfg(test)]
fn evaluate_with_policy(
policy: &Policy,
command: &[String],
@@ -93,26 +210,14 @@ fn evaluate_with_policy(
let evaluation = policy.check_multiple(commands.iter());
match evaluation {
Evaluation::Match { decision, .. } => match decision {
Decision::Forbidden => Some(ApprovalRequirement::Forbidden {
reason: FORBIDDEN_REASON.to_string(),
}),
Decision::Prompt => {
let reason = PROMPT_REASON.to_string();
if matches!(approval_policy, AskForApproval::Never) {
Some(ApprovalRequirement::Forbidden { reason })
} else {
Some(ApprovalRequirement::NeedsApproval {
reason: Some(reason),
})
}
}
Decision::Allow => Some(ApprovalRequirement::Skip),
},
Evaluation::Match { decision, .. } => {
Some(requirement_from_decision(decision, approval_policy))
}
Evaluation::NoMatch => None,
}
}
#[cfg(test)]
pub(crate) fn create_approval_requirement_for_command(
policy: &Policy,
command: &[String],
@@ -120,20 +225,14 @@ pub(crate) fn create_approval_requirement_for_command(
sandbox_policy: &SandboxPolicy,
sandbox_permissions: SandboxPermissions,
) -> ApprovalRequirement {
if let Some(requirement) = evaluate_with_policy(policy, command, approval_policy) {
return requirement;
}
if requires_initial_appoval(
command_approval_outcome_for_command(
policy,
command,
approval_policy,
sandbox_policy,
command,
sandbox_permissions,
) {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
)
.requirement
}
async fn collect_policy_files(dir: &Path) -> Result<Vec<PathBuf>, ExecPolicyError> {
@@ -362,4 +461,73 @@ prefix_rule(pattern=["rm"], decision="forbidden")
ApprovalRequirement::NeedsApproval { reason: None }
);
}
#[test]
fn allow_prefix_is_present_for_single_command_without_policy_match() {
let command = vec!["python".to_string()];
let empty_policy = Policy::empty();
let outcome = command_approval_outcome_for_command(
&empty_policy,
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
SandboxPermissions::UseDefault,
);
assert_eq!(
outcome.requirement,
ApprovalRequirement::NeedsApproval { reason: None }
);
assert_eq!(outcome.allow_prefix, Some(command));
}
#[test]
fn allow_prefix_is_omitted_when_policy_prompts() {
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
let mut parser = PolicyParser::new();
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = parser.build();
let command = vec!["rm".to_string()];
let outcome = command_approval_outcome_for_command(
&policy,
&command,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
);
assert_eq!(
outcome.requirement,
ApprovalRequirement::NeedsApproval {
reason: Some(PROMPT_REASON.to_string())
}
);
assert_eq!(outcome.allow_prefix, None);
}
#[test]
fn allow_prefix_is_omitted_for_multi_command_scripts() {
let command = vec![
"bash".to_string(),
"-lc".to_string(),
"python && echo ok".to_string(),
];
let outcome = command_approval_outcome_for_command(
&Policy::empty(),
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
SandboxPermissions::UseDefault,
);
assert_eq!(
outcome.requirement,
ApprovalRequirement::NeedsApproval { reason: None }
);
assert_eq!(outcome.allow_prefix, None);
}
}
codex-rs/core/src/tools/handlers/shell.rs
+11 -8
View File
@@ -9,7 +9,7 @@ use crate::apply_patch::convert_apply_patch_to_protocol;
use crate::codex::TurnContext;
use crate::exec::ExecParams;
use crate::exec_env::create_env;
use crate::exec_policy::create_approval_requirement_for_command;
use crate::exec_policy::command_approval_outcome_for_command;
use crate::function_tool::FunctionCallError;
use crate::is_safe_command::is_known_safe_command;
use crate::protocol::ExecCommandSource;
@@ -297,6 +297,14 @@ 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 approval_outcome = command_approval_outcome_for_command(
exec_policy.as_ref(),
&exec_params.command,
turn.approval_policy,
&turn.sandbox_policy,
SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)),
);
let req = ShellRequest {
command: exec_params.command.clone(),
cwd: exec_params.cwd.clone(),
@@ -304,13 +312,8 @@ impl ShellHandler {
env: exec_params.env.clone(),
with_escalated_permissions: exec_params.with_escalated_permissions,
justification: exec_params.justification.clone(),
approval_requirement: create_approval_requirement_for_command(
&turn.exec_policy,
&exec_params.command,
turn.approval_policy,
&turn.sandbox_policy,
SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)),
),
approval_requirement: approval_outcome.requirement,
approval_allow_prefix: approval_outcome.allow_prefix,
};
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = ShellRuntime::new();
codex-rs/core/src/tools/runtimes/apply_patch.rs
+1
View File
@@ -127,6 +127,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
cwd,
Some(reason),
risk,
None,
)
.await
} else if user_explicitly_approved {
codex-rs/core/src/tools/runtimes/shell.rs
+11 -1
View File
@@ -32,6 +32,7 @@ pub struct ShellRequest {
pub with_escalated_permissions: Option<bool>,
pub justification: Option<String>,
pub approval_requirement: ApprovalRequirement,
pub approval_allow_prefix: Option<Vec<String>>,
}
impl ProvidesSandboxRetryData for ShellRequest {
@@ -100,13 +101,22 @@ impl Approvable<ShellRequest> for ShellRuntime {
.clone()
.or_else(|| req.justification.clone());
let risk = ctx.risk.clone();
let allow_prefix = req.approval_allow_prefix.clone();
let session = ctx.session;
let turn = ctx.turn;
let call_id = ctx.call_id.to_string();
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,
allow_prefix,
)
.await
})
.await
codex-rs/core/src/tools/runtimes/unified_exec.rs
+13 -1
View File
@@ -35,6 +35,7 @@ pub struct UnifiedExecRequest {
pub with_escalated_permissions: Option<bool>,
pub justification: Option<String>,
pub approval_requirement: ApprovalRequirement,
pub approval_allow_prefix: Option<Vec<String>>,
}
impl ProvidesSandboxRetryData for UnifiedExecRequest {
@@ -65,6 +66,7 @@ impl UnifiedExecRequest {
with_escalated_permissions: Option<bool>,
justification: Option<String>,
approval_requirement: ApprovalRequirement,
approval_allow_prefix: Option<Vec<String>>,
) -> Self {
Self {
command,
@@ -73,6 +75,7 @@ impl UnifiedExecRequest {
with_escalated_permissions,
justification,
approval_requirement,
approval_allow_prefix,
}
}
}
@@ -120,10 +123,19 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
.clone()
.or_else(|| req.justification.clone());
let risk = ctx.risk.clone();
let allow_prefix = req.approval_allow_prefix.clone();
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,
allow_prefix,
)
.await
})
.await
codex-rs/core/src/unified_exec/session_manager.rs
+11 -8
View File
@@ -11,7 +11,7 @@ use crate::codex::TurnContext;
use crate::exec::ExecToolCallOutput;
use crate::exec::StreamOutput;
use crate::exec_env::create_env;
use crate::exec_policy::create_approval_requirement_for_command;
use crate::exec_policy::command_approval_outcome_for_command;
use crate::protocol::BackgroundEventEvent;
use crate::protocol::EventMsg;
use crate::protocol::ExecCommandSource;
@@ -445,19 +445,22 @@ 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 approval_outcome = command_approval_outcome_for_command(
exec_policy.as_ref(),
command,
context.turn.approval_policy,
&context.turn.sandbox_policy,
SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)),
);
let req = UnifiedExecToolRequest::new(
command.to_vec(),
cwd,
create_env(&context.turn.shell_environment_policy),
with_escalated_permissions,
justification,
create_approval_requirement_for_command(
&context.turn.exec_policy,
command,
context.turn.approval_policy,
&context.turn.sandbox_policy,
SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)),
),
approval_outcome.requirement,
approval_outcome.allow_prefix,
);
let tool_ctx = ToolCtx {
session: context.session.as_ref(),
codex-rs/core/tests/suite/approvals.rs
+1
View File
@@ -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;
codex-rs/core/tests/suite/codex_delegate.rs
+1
View File
@@ -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");
codex-rs/core/tests/suite/otel.rs
+6
View File
@@ -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();