mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
Merge e7ed3c2884 into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -69,6 +69,37 @@ Codex can access MCP servers. To configure them, refer to the [config docs](./do
|
||||
|
||||
Codex CLI supports a rich set of configuration options, with preferences stored in `~/.codex/config.toml`. For full configuration options, see [Configuration](./docs/config.md).
|
||||
|
||||
### Execpolicy Quickstart
|
||||
|
||||
Codex can enforce your own rules-based execution policy before it runs shell commands.
|
||||
|
||||
1. Create a policy directory: `mkdir -p ~/.codex/policy`.
|
||||
2. Create one or more `.codexpolicy` files in that folder. Codex automatically loads every `.codexpolicy` file in there on startup.
|
||||
3. Write `prefix_rule` entries to describe the commands you want to allow, prompt, or block:
|
||||
|
||||
```starlark
|
||||
prefix_rule(
|
||||
pattern = ["git", ["push", "fetch"]],
|
||||
decision = "prompt", # allow | prompt | forbidden
|
||||
match = [["git", "push", "origin", "main"]], # examples that must match
|
||||
not_match = [["git", "status"]], # examples that must not match
|
||||
)
|
||||
```
|
||||
|
||||
- `pattern` is a list of shell tokens, evaluated from left to right; wrap tokens in a nested list to express alternatives (e.g., match both `push` and `fetch`).
|
||||
- `decision` sets the severity; Codex picks the strictest decision when multiple rules match (forbidden > prompt > allow).
|
||||
- `match` and `not_match` act as (optional) unit tests. Codex validates them when it loads your policy, so you get feedback if an example has unexpected behavior.
|
||||
|
||||
In this example rule, if Codex wants to run commands with the prefix `git push` or `git fetch`, it will first ask for user approval.
|
||||
|
||||
Use [`execpolicy2` CLI](./codex-rs/execpolicy2/README.md) to preview decisions for policy files:
|
||||
|
||||
```shell
|
||||
cargo run -p codex-execpolicy2 -- check --policy ~/.codex/policy/default.codexpolicy git push origin main
|
||||
```
|
||||
|
||||
Pass multiple `--policy` flags to test how several files combine. See the [`codex-rs/execpolicy2` README](./codex-rs/execpolicy2/README.md) for a more detailed walkthrough of the available syntax.
|
||||
|
||||
---
|
||||
|
||||
### Docs & FAQ
|
||||
|
||||
Generated
+16
-15
@@ -1086,6 +1086,7 @@ dependencies = [
|
||||
"codex-apply-patch",
|
||||
"codex-arg0",
|
||||
"codex-async-utils",
|
||||
"codex-execpolicy",
|
||||
"codex-file-search",
|
||||
"codex-git",
|
||||
"codex-keyring-store",
|
||||
@@ -1208,6 +1209,21 @@ dependencies = [
|
||||
[[package]]
|
||||
name = "codex-execpolicy"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"clap",
|
||||
"multimap",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"shlex",
|
||||
"starlark",
|
||||
"thiserror 2.0.17",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "codex-execpolicy-legacy"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"allocative",
|
||||
"anyhow",
|
||||
@@ -1225,21 +1241,6 @@ dependencies = [
|
||||
"tempfile",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "codex-execpolicy2"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"clap",
|
||||
"multimap",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"shlex",
|
||||
"starlark",
|
||||
"thiserror 2.0.17",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "codex-feedback"
|
||||
version = "0.0.0"
|
||||
|
||||
+2
-1
@@ -18,7 +18,7 @@ members = [
|
||||
"exec",
|
||||
"exec-server",
|
||||
"execpolicy",
|
||||
"execpolicy2",
|
||||
"execpolicy-legacy",
|
||||
"keyring-store",
|
||||
"file-search",
|
||||
"linux-sandbox",
|
||||
@@ -67,6 +67,7 @@ codex-chatgpt = { path = "chatgpt" }
|
||||
codex-common = { path = "common" }
|
||||
codex-core = { path = "core" }
|
||||
codex-exec = { path = "exec" }
|
||||
codex-execpolicy = { path = "execpolicy" }
|
||||
codex-feedback = { path = "feedback" }
|
||||
codex-file-search = { path = "file-search" }
|
||||
codex-git = { path = "utils/git" }
|
||||
|
||||
@@ -438,6 +438,13 @@ server_request_definitions! {
|
||||
response: v2::CommandExecutionRequestApprovalResponse,
|
||||
},
|
||||
|
||||
/// Sent when approval is requested for a specific file change.
|
||||
/// This request is used for Turns started via turn/start.
|
||||
FileChangeRequestApproval => "item/fileChange/requestApproval" {
|
||||
params: v2::FileChangeRequestApprovalParams,
|
||||
response: v2::FileChangeRequestApprovalResponse,
|
||||
},
|
||||
|
||||
/// DEPRECATED APIs below
|
||||
/// Request to approve a patch.
|
||||
/// This request is used for Turns started via the legacy APIs (i.e. SendUserTurn, SendUserMessage).
|
||||
|
||||
@@ -794,20 +794,23 @@ pub struct FileUpdateChange {
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[serde(tag = "type", rename_all = "camelCase")]
|
||||
#[ts(tag = "type")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub enum PatchChangeKind {
|
||||
Add,
|
||||
Delete,
|
||||
Update,
|
||||
Update { move_path: Option<PathBuf> },
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub enum PatchApplyStatus {
|
||||
InProgress,
|
||||
Completed,
|
||||
Failed,
|
||||
Declined,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
@@ -982,6 +985,26 @@ pub struct CommandExecutionRequestApprovalResponse {
|
||||
pub accept_settings: Option<CommandExecutionRequestAcceptSettings>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct FileChangeRequestApprovalParams {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
/// Optional explanatory reason (e.g. request for extra write access).
|
||||
pub reason: Option<String>,
|
||||
/// [UNSTABLE] When set, the agent is asking the user to allow writes under this root
|
||||
/// for the remainder of the session (unclear if this is honored today).
|
||||
pub grant_root: Option<PathBuf>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct FileChangeRequestApprovalResponse {
|
||||
pub decision: ApprovalDecision,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
|
||||
@@ -24,6 +24,8 @@ use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::CommandExecutionRequestAcceptSettings;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalParams;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::GetAccountRateLimitsResponse;
|
||||
use codex_app_server_protocol::InitializeParams;
|
||||
use codex_app_server_protocol::InitializeResponse;
|
||||
@@ -677,6 +679,9 @@ impl CodexClient {
|
||||
ServerRequest::CommandExecutionRequestApproval { request_id, params } => {
|
||||
self.handle_command_execution_request_approval(request_id, params)?;
|
||||
}
|
||||
ServerRequest::FileChangeRequestApproval { request_id, params } => {
|
||||
self.approve_file_change_request(request_id, params)?;
|
||||
}
|
||||
other => {
|
||||
bail!("received unsupported server request: {other:?}");
|
||||
}
|
||||
@@ -717,6 +722,37 @@ impl CodexClient {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn approve_file_change_request(
|
||||
&mut self,
|
||||
request_id: RequestId,
|
||||
params: FileChangeRequestApprovalParams,
|
||||
) -> Result<()> {
|
||||
let FileChangeRequestApprovalParams {
|
||||
thread_id,
|
||||
turn_id,
|
||||
item_id,
|
||||
reason,
|
||||
grant_root,
|
||||
} = params;
|
||||
|
||||
println!(
|
||||
"\n< fileChange approval requested for thread {thread_id}, turn {turn_id}, item {item_id}"
|
||||
);
|
||||
if let Some(reason) = reason.as_deref() {
|
||||
println!("< reason: {reason}");
|
||||
}
|
||||
if let Some(grant_root) = grant_root.as_deref() {
|
||||
println!("< grant root: {}", grant_root.display());
|
||||
}
|
||||
|
||||
let response = FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Accept,
|
||||
};
|
||||
self.send_server_request_response(request_id, &response)?;
|
||||
println!("< approved fileChange request for item {item_id}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn send_server_request_response<T>(&mut self, request_id: RequestId, response: &T) -> Result<()>
|
||||
where
|
||||
T: Serialize,
|
||||
|
||||
@@ -15,12 +15,17 @@ use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::CommandExecutionStatus;
|
||||
use codex_app_server_protocol::ExecCommandApprovalParams;
|
||||
use codex_app_server_protocol::ExecCommandApprovalResponse;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::FileUpdateChange;
|
||||
use codex_app_server_protocol::InterruptConversationResponse;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
use codex_app_server_protocol::McpToolCallError;
|
||||
use codex_app_server_protocol::McpToolCallResult;
|
||||
use codex_app_server_protocol::McpToolCallStatus;
|
||||
use codex_app_server_protocol::PatchApplyStatus;
|
||||
use codex_app_server_protocol::PatchChangeKind as V2PatchChangeKind;
|
||||
use codex_app_server_protocol::ReasoningSummaryPartAddedNotification;
|
||||
use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification;
|
||||
use codex_app_server_protocol::ReasoningTextDeltaNotification;
|
||||
@@ -40,6 +45,7 @@ use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::ExecApprovalRequestEvent;
|
||||
use codex_core::protocol::ExecCommandEndEvent;
|
||||
use codex_core::protocol::FileChange as CoreFileChange;
|
||||
use codex_core::protocol::McpToolCallBeginEvent;
|
||||
use codex_core::protocol::McpToolCallEndEvent;
|
||||
use codex_core::protocol::Op;
|
||||
@@ -47,7 +53,9 @@ use codex_core::protocol::ReviewDecision;
|
||||
use codex_core::review_format::format_review_findings_block;
|
||||
use codex_protocol::ConversationId;
|
||||
use codex_protocol::protocol::ReviewOutputEvent;
|
||||
use std::collections::HashMap;
|
||||
use std::convert::TryFrom;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::oneshot;
|
||||
use tracing::error;
|
||||
@@ -70,24 +78,74 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
}
|
||||
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id,
|
||||
turn_id,
|
||||
changes,
|
||||
reason,
|
||||
grant_root,
|
||||
}) => {
|
||||
let params = ApplyPatchApprovalParams {
|
||||
conversation_id,
|
||||
call_id,
|
||||
file_changes: changes,
|
||||
reason,
|
||||
grant_root,
|
||||
};
|
||||
let rx = outgoing
|
||||
.send_request(ServerRequestPayload::ApplyPatchApproval(params))
|
||||
.await;
|
||||
tokio::spawn(async move {
|
||||
on_patch_approval_response(event_id, rx, conversation).await;
|
||||
});
|
||||
}
|
||||
}) => match api_version {
|
||||
ApiVersion::V1 => {
|
||||
let params = ApplyPatchApprovalParams {
|
||||
conversation_id,
|
||||
call_id,
|
||||
file_changes: changes.clone(),
|
||||
reason,
|
||||
grant_root,
|
||||
};
|
||||
let rx = outgoing
|
||||
.send_request(ServerRequestPayload::ApplyPatchApproval(params))
|
||||
.await;
|
||||
tokio::spawn(async move {
|
||||
on_patch_approval_response(event_id, rx, conversation).await;
|
||||
});
|
||||
}
|
||||
ApiVersion::V2 => {
|
||||
// Until we migrate the core to be aware of a first class FileChangeItem
|
||||
// and emit the corresponding EventMsg, we repurpose the call_id as the item_id.
|
||||
let item_id = call_id.clone();
|
||||
let patch_changes = convert_patch_changes(&changes);
|
||||
|
||||
let first_start = {
|
||||
let mut map = turn_summary_store.lock().await;
|
||||
let summary = map.entry(conversation_id).or_default();
|
||||
summary.file_change_started.insert(item_id.clone())
|
||||
};
|
||||
if first_start {
|
||||
let item = ThreadItem::FileChange {
|
||||
id: item_id.clone(),
|
||||
changes: patch_changes.clone(),
|
||||
status: PatchApplyStatus::InProgress,
|
||||
};
|
||||
let notification = ItemStartedNotification { item };
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::ItemStarted(notification))
|
||||
.await;
|
||||
}
|
||||
|
||||
let params = FileChangeRequestApprovalParams {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: turn_id.clone(),
|
||||
item_id: item_id.clone(),
|
||||
reason,
|
||||
grant_root,
|
||||
};
|
||||
let rx = outgoing
|
||||
.send_request(ServerRequestPayload::FileChangeRequestApproval(params))
|
||||
.await;
|
||||
tokio::spawn(async move {
|
||||
on_file_change_request_approval_response(
|
||||
event_id,
|
||||
conversation_id,
|
||||
item_id,
|
||||
patch_changes,
|
||||
rx,
|
||||
conversation,
|
||||
outgoing,
|
||||
turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
});
|
||||
}
|
||||
},
|
||||
EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
|
||||
call_id,
|
||||
turn_id,
|
||||
@@ -244,6 +302,49 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
.send_server_notification(ServerNotification::ItemCompleted(notification))
|
||||
.await;
|
||||
}
|
||||
EventMsg::PatchApplyBegin(patch_begin_event) => {
|
||||
// Until we migrate the core to be aware of a first class FileChangeItem
|
||||
// and emit the corresponding EventMsg, we repurpose the call_id as the item_id.
|
||||
let item_id = patch_begin_event.call_id.clone();
|
||||
|
||||
let first_start = {
|
||||
let mut map = turn_summary_store.lock().await;
|
||||
let summary = map.entry(conversation_id).or_default();
|
||||
summary.file_change_started.insert(item_id.clone())
|
||||
};
|
||||
if first_start {
|
||||
let item = ThreadItem::FileChange {
|
||||
id: item_id.clone(),
|
||||
changes: convert_patch_changes(&patch_begin_event.changes),
|
||||
status: PatchApplyStatus::InProgress,
|
||||
};
|
||||
let notification = ItemStartedNotification { item };
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::ItemStarted(notification))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
EventMsg::PatchApplyEnd(patch_end_event) => {
|
||||
// Until we migrate the core to be aware of a first class FileChangeItem
|
||||
// and emit the corresponding EventMsg, we repurpose the call_id as the item_id.
|
||||
let item_id = patch_end_event.call_id.clone();
|
||||
|
||||
let status = if patch_end_event.success {
|
||||
PatchApplyStatus::Completed
|
||||
} else {
|
||||
PatchApplyStatus::Failed
|
||||
};
|
||||
let changes = convert_patch_changes(&patch_end_event.changes);
|
||||
complete_file_change_item(
|
||||
conversation_id,
|
||||
item_id,
|
||||
changes,
|
||||
status,
|
||||
outgoing.as_ref(),
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
EventMsg::ExecCommandBegin(exec_command_begin_event) => {
|
||||
let item = ThreadItem::CommandExecution {
|
||||
id: exec_command_begin_event.call_id.clone(),
|
||||
@@ -365,6 +466,32 @@ async fn emit_turn_completed_with_status(
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn complete_file_change_item(
|
||||
conversation_id: ConversationId,
|
||||
item_id: String,
|
||||
changes: Vec<FileUpdateChange>,
|
||||
status: PatchApplyStatus,
|
||||
outgoing: &OutgoingMessageSender,
|
||||
turn_summary_store: &TurnSummaryStore,
|
||||
) {
|
||||
{
|
||||
let mut map = turn_summary_store.lock().await;
|
||||
if let Some(summary) = map.get_mut(&conversation_id) {
|
||||
summary.file_change_started.remove(&item_id);
|
||||
}
|
||||
}
|
||||
|
||||
let item = ThreadItem::FileChange {
|
||||
id: item_id,
|
||||
changes,
|
||||
status,
|
||||
};
|
||||
let notification = ItemCompletedNotification { item };
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::ItemCompleted(notification))
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn find_and_remove_turn_summary(
|
||||
conversation_id: ConversationId,
|
||||
turn_summary_store: &TurnSummaryStore,
|
||||
@@ -512,6 +639,110 @@ fn render_review_output_text(output: &ReviewOutputEvent) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
fn convert_patch_changes(changes: &HashMap<PathBuf, CoreFileChange>) -> Vec<FileUpdateChange> {
|
||||
let mut converted: Vec<FileUpdateChange> = changes
|
||||
.iter()
|
||||
.map(|(path, change)| FileUpdateChange {
|
||||
path: path.to_string_lossy().into_owned(),
|
||||
kind: map_patch_change_kind(change),
|
||||
diff: format_file_change_diff(change),
|
||||
})
|
||||
.collect();
|
||||
converted.sort_by(|a, b| a.path.cmp(&b.path));
|
||||
converted
|
||||
}
|
||||
|
||||
fn map_patch_change_kind(change: &CoreFileChange) -> V2PatchChangeKind {
|
||||
match change {
|
||||
CoreFileChange::Add { .. } => V2PatchChangeKind::Add,
|
||||
CoreFileChange::Delete { .. } => V2PatchChangeKind::Delete,
|
||||
CoreFileChange::Update { move_path, .. } => V2PatchChangeKind::Update {
|
||||
move_path: move_path.clone(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn format_file_change_diff(change: &CoreFileChange) -> String {
|
||||
match change {
|
||||
CoreFileChange::Add { content } => content.clone(),
|
||||
CoreFileChange::Delete { content } => content.clone(),
|
||||
CoreFileChange::Update {
|
||||
unified_diff,
|
||||
move_path,
|
||||
} => {
|
||||
if let Some(path) = move_path {
|
||||
format!("{unified_diff}\n\nMoved to: {}", path.display())
|
||||
} else {
|
||||
unified_diff.clone()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn on_file_change_request_approval_response(
|
||||
event_id: String,
|
||||
conversation_id: ConversationId,
|
||||
item_id: String,
|
||||
changes: Vec<FileUpdateChange>,
|
||||
receiver: oneshot::Receiver<JsonValue>,
|
||||
codex: Arc<CodexConversation>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
turn_summary_store: TurnSummaryStore,
|
||||
) {
|
||||
let response = receiver.await;
|
||||
let (decision, completion_status) = match response {
|
||||
Ok(value) => {
|
||||
let response = serde_json::from_value::<FileChangeRequestApprovalResponse>(value)
|
||||
.unwrap_or_else(|err| {
|
||||
error!("failed to deserialize FileChangeRequestApprovalResponse: {err}");
|
||||
FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Decline,
|
||||
}
|
||||
});
|
||||
|
||||
let (decision, completion_status) = match response.decision {
|
||||
ApprovalDecision::Accept => (ReviewDecision::Approved, None),
|
||||
ApprovalDecision::Decline => {
|
||||
(ReviewDecision::Denied, Some(PatchApplyStatus::Declined))
|
||||
}
|
||||
ApprovalDecision::Cancel => {
|
||||
(ReviewDecision::Abort, Some(PatchApplyStatus::Declined))
|
||||
}
|
||||
};
|
||||
// Allow EventMsg::PatchApplyEnd to emit ItemCompleted for accepted patches.
|
||||
// Only short-circuit on declines/cancels/failures.
|
||||
(decision, completion_status)
|
||||
}
|
||||
Err(err) => {
|
||||
error!("request failed: {err:?}");
|
||||
(ReviewDecision::Denied, Some(PatchApplyStatus::Failed))
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(status) = completion_status {
|
||||
complete_file_change_item(
|
||||
conversation_id,
|
||||
item_id,
|
||||
changes,
|
||||
status,
|
||||
outgoing.as_ref(),
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
if let Err(err) = codex
|
||||
.submit(Op::PatchApproval {
|
||||
id: event_id,
|
||||
decision,
|
||||
})
|
||||
.await
|
||||
{
|
||||
error!("failed to submit PatchApproval: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
async fn on_command_execution_request_approval_response(
|
||||
event_id: String,
|
||||
receiver: oneshot::Receiver<JsonValue>,
|
||||
|
||||
@@ -139,6 +139,7 @@ use codex_protocol::protocol::USER_MESSAGE_BEGIN;
|
||||
use codex_protocol::user_input::UserInput as CoreInputItem;
|
||||
use codex_utils_json_to_toml::json_to_toml;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::OsStr;
|
||||
use std::io::Error as IoError;
|
||||
use std::path::Path;
|
||||
@@ -162,6 +163,7 @@ pub(crate) type PendingInterrupts = Arc<Mutex<HashMap<ConversationId, PendingInt
|
||||
#[derive(Default, Clone)]
|
||||
pub(crate) struct TurnSummary {
|
||||
pub(crate) last_error_message: Option<String>,
|
||||
pub(crate) file_change_started: HashSet<String>,
|
||||
}
|
||||
|
||||
pub(crate) type TurnSummaryStore = Arc<Mutex<HashMap<ConversationId, TurnSummary>>>;
|
||||
|
||||
@@ -1,14 +1,20 @@
|
||||
use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::create_apply_patch_sse_response;
|
||||
use app_test_support::create_final_assistant_message_sse_response;
|
||||
use app_test_support::create_mock_chat_completions_server;
|
||||
use app_test_support::create_mock_chat_completions_server_unchecked;
|
||||
use app_test_support::create_shell_sse_response;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::ApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionStatus;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
use codex_app_server_protocol::JSONRPCNotification;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::PatchApplyStatus;
|
||||
use codex_app_server_protocol::PatchChangeKind;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
@@ -471,6 +477,300 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_file_change_approval_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let tmp = TempDir::new()?;
|
||||
let codex_home = tmp.path().join("codex_home");
|
||||
std::fs::create_dir(&codex_home)?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
std::fs::create_dir(&workspace)?;
|
||||
|
||||
let patch = r#"*** Begin Patch
|
||||
*** Add File: README.md
|
||||
+new line
|
||||
*** End Patch
|
||||
"#;
|
||||
let responses = vec![
|
||||
create_apply_patch_sse_response(patch, "patch-call")?,
|
||||
create_final_assistant_message_sse_response("patch applied")?,
|
||||
];
|
||||
let server = create_mock_chat_completions_server(responses).await;
|
||||
create_config_toml(&codex_home, &server.uri(), "untrusted")?;
|
||||
|
||||
let mut mcp = McpProcess::new(&codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let start_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
cwd: Some(workspace.to_string_lossy().into_owned()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let start_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(start_req)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
|
||||
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "apply patch".into(),
|
||||
}],
|
||||
cwd: Some(workspace.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let turn_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
|
||||
)
|
||||
.await??;
|
||||
let TurnStartResponse { turn } = to_response::<TurnStartResponse>(turn_resp)?;
|
||||
|
||||
let started_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let started_notif = mcp
|
||||
.read_stream_until_notification_message("item/started")
|
||||
.await?;
|
||||
let started: ItemStartedNotification =
|
||||
serde_json::from_value(started_notif.params.clone().expect("item/started params"))?;
|
||||
if let ThreadItem::FileChange { .. } = started.item {
|
||||
return Ok::<ThreadItem, anyhow::Error>(started.item);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
let ThreadItem::FileChange {
|
||||
ref id,
|
||||
status,
|
||||
ref changes,
|
||||
} = started_file_change
|
||||
else {
|
||||
unreachable!("loop ensures we break on file change items");
|
||||
};
|
||||
assert_eq!(id, "patch-call");
|
||||
assert_eq!(status, PatchApplyStatus::InProgress);
|
||||
let started_changes = changes.clone();
|
||||
|
||||
let server_req = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_request_message(),
|
||||
)
|
||||
.await??;
|
||||
let ServerRequest::FileChangeRequestApproval { request_id, params } = server_req else {
|
||||
panic!("expected FileChangeRequestApproval request")
|
||||
};
|
||||
assert_eq!(params.item_id, "patch-call");
|
||||
assert_eq!(params.thread_id, thread.id);
|
||||
assert_eq!(params.turn_id, turn.id);
|
||||
let expected_readme_path = workspace.join("README.md");
|
||||
let expected_readme_path = expected_readme_path.to_string_lossy().into_owned();
|
||||
pretty_assertions::assert_eq!(
|
||||
started_changes,
|
||||
vec![codex_app_server_protocol::FileUpdateChange {
|
||||
path: expected_readme_path.clone(),
|
||||
kind: PatchChangeKind::Add,
|
||||
diff: "new line\n".to_string(),
|
||||
}]
|
||||
);
|
||||
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Accept,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let completed_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let completed_notif = mcp
|
||||
.read_stream_until_notification_message("item/completed")
|
||||
.await?;
|
||||
let completed: ItemCompletedNotification = serde_json::from_value(
|
||||
completed_notif
|
||||
.params
|
||||
.clone()
|
||||
.expect("item/completed params"),
|
||||
)?;
|
||||
if let ThreadItem::FileChange { .. } = completed.item {
|
||||
return Ok::<ThreadItem, anyhow::Error>(completed.item);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
let ThreadItem::FileChange { ref id, status, .. } = completed_file_change else {
|
||||
unreachable!("loop ensures we break on file change items");
|
||||
};
|
||||
assert_eq!(id, "patch-call");
|
||||
assert_eq!(status, PatchApplyStatus::Completed);
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let readme_contents = std::fs::read_to_string(expected_readme_path)?;
|
||||
assert_eq!(readme_contents, "new line\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_file_change_approval_decline_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let tmp = TempDir::new()?;
|
||||
let codex_home = tmp.path().join("codex_home");
|
||||
std::fs::create_dir(&codex_home)?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
std::fs::create_dir(&workspace)?;
|
||||
|
||||
let patch = r#"*** Begin Patch
|
||||
*** Add File: README.md
|
||||
+new line
|
||||
*** End Patch
|
||||
"#;
|
||||
let responses = vec![
|
||||
create_apply_patch_sse_response(patch, "patch-call")?,
|
||||
create_final_assistant_message_sse_response("patch declined")?,
|
||||
];
|
||||
let server = create_mock_chat_completions_server(responses).await;
|
||||
create_config_toml(&codex_home, &server.uri(), "untrusted")?;
|
||||
|
||||
let mut mcp = McpProcess::new(&codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let start_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
cwd: Some(workspace.to_string_lossy().into_owned()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let start_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(start_req)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
|
||||
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "apply patch".into(),
|
||||
}],
|
||||
cwd: Some(workspace.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let turn_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
|
||||
)
|
||||
.await??;
|
||||
let TurnStartResponse { turn } = to_response::<TurnStartResponse>(turn_resp)?;
|
||||
|
||||
let started_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let started_notif = mcp
|
||||
.read_stream_until_notification_message("item/started")
|
||||
.await?;
|
||||
let started: ItemStartedNotification =
|
||||
serde_json::from_value(started_notif.params.clone().expect("item/started params"))?;
|
||||
if let ThreadItem::FileChange { .. } = started.item {
|
||||
return Ok::<ThreadItem, anyhow::Error>(started.item);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
let ThreadItem::FileChange {
|
||||
ref id,
|
||||
status,
|
||||
ref changes,
|
||||
} = started_file_change
|
||||
else {
|
||||
unreachable!("loop ensures we break on file change items");
|
||||
};
|
||||
assert_eq!(id, "patch-call");
|
||||
assert_eq!(status, PatchApplyStatus::InProgress);
|
||||
let started_changes = changes.clone();
|
||||
|
||||
let server_req = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_request_message(),
|
||||
)
|
||||
.await??;
|
||||
let ServerRequest::FileChangeRequestApproval { request_id, params } = server_req else {
|
||||
panic!("expected FileChangeRequestApproval request")
|
||||
};
|
||||
assert_eq!(params.item_id, "patch-call");
|
||||
assert_eq!(params.thread_id, thread.id);
|
||||
assert_eq!(params.turn_id, turn.id);
|
||||
let expected_readme_path = workspace.join("README.md");
|
||||
let expected_readme_path_str = expected_readme_path.to_string_lossy().into_owned();
|
||||
pretty_assertions::assert_eq!(
|
||||
started_changes,
|
||||
vec![codex_app_server_protocol::FileUpdateChange {
|
||||
path: expected_readme_path_str.clone(),
|
||||
kind: PatchChangeKind::Add,
|
||||
diff: "new line\n".to_string(),
|
||||
}]
|
||||
);
|
||||
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Decline,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let completed_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let completed_notif = mcp
|
||||
.read_stream_until_notification_message("item/completed")
|
||||
.await?;
|
||||
let completed: ItemCompletedNotification = serde_json::from_value(
|
||||
completed_notif
|
||||
.params
|
||||
.clone()
|
||||
.expect("item/completed params"),
|
||||
)?;
|
||||
if let ThreadItem::FileChange { .. } = completed.item {
|
||||
return Ok::<ThreadItem, anyhow::Error>(completed.item);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
let ThreadItem::FileChange { ref id, status, .. } = completed_file_change else {
|
||||
unreachable!("loop ensures we break on file change items");
|
||||
};
|
||||
assert_eq!(id, "patch-call");
|
||||
assert_eq!(status, PatchApplyStatus::Declined);
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
assert!(
|
||||
!expected_readme_path.exists(),
|
||||
"declined patch should not be applied"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Helper to create a config.toml pointing at the mock model server.
|
||||
fn create_config_toml(
|
||||
codex_home: &Path,
|
||||
|
||||
@@ -22,6 +22,7 @@ chrono = { workspace = true, features = ["serde"] }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-apply-patch = { workspace = true }
|
||||
codex-async-utils = { workspace = true }
|
||||
codex-execpolicy = { workspace = true }
|
||||
codex-file-search = { workspace = true }
|
||||
codex-git = { workspace = true }
|
||||
codex-keyring-store = { workspace = true }
|
||||
|
||||
@@ -66,7 +66,6 @@ use crate::context_manager::ContextManager;
|
||||
use crate::environment_context::EnvironmentContext;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::Result as CodexResult;
|
||||
use crate::error::http_status_code_value;
|
||||
#[cfg(test)]
|
||||
use crate::exec::StreamOutput;
|
||||
use crate::mcp::auth::compute_auth_statuses;
|
||||
@@ -80,6 +79,7 @@ use crate::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::BackgroundEventEvent;
|
||||
use crate::protocol::DeprecationNoticeEvent;
|
||||
use crate::protocol::ErrorEvent;
|
||||
use crate::protocol::Event;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::ExecApprovalRequestEvent;
|
||||
@@ -121,6 +121,7 @@ use crate::user_instructions::UserInstructions;
|
||||
use crate::user_notification::UserNotification;
|
||||
use crate::util::backoff;
|
||||
use codex_async_utils::OrCancelExt;
|
||||
use codex_execpolicy::Policy as ExecPolicy;
|
||||
use codex_otel::otel_event_manager::OtelEventManager;
|
||||
use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
@@ -133,7 +134,6 @@ use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_readiness::Readiness;
|
||||
use codex_utils_readiness::ReadinessFlag;
|
||||
use codex_utils_tokenizer::warm_model_cache;
|
||||
use reqwest::StatusCode;
|
||||
|
||||
/// The high-level interface to the Codex system.
|
||||
/// It operates as a queue pair where you send submissions and receive events.
|
||||
@@ -167,6 +167,10 @@ impl Codex {
|
||||
|
||||
let user_instructions = get_user_instructions(&config).await;
|
||||
|
||||
let exec_policy = crate::exec_policy::exec_policy_for(&config.features, &config.codex_home)
|
||||
.await
|
||||
.map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy: {err}")))?;
|
||||
|
||||
let config = Arc::new(config);
|
||||
|
||||
let session_configuration = SessionConfiguration {
|
||||
@@ -183,6 +187,7 @@ impl Codex {
|
||||
cwd: config.cwd.clone(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
features: config.features.clone(),
|
||||
exec_policy,
|
||||
session_source,
|
||||
};
|
||||
|
||||
@@ -280,6 +285,7 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) final_output_json_schema: Option<Value>,
|
||||
pub(crate) codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
pub(crate) tool_call_gate: Arc<ReadinessFlag>,
|
||||
pub(crate) exec_policy: Arc<ExecPolicy>,
|
||||
pub(crate) truncation_policy: TruncationPolicy,
|
||||
}
|
||||
|
||||
@@ -336,6 +342,8 @@ pub(crate) struct SessionConfiguration {
|
||||
|
||||
/// Set of feature flags for this session
|
||||
features: Features,
|
||||
/// Execpolicy policy, applied only when enabled by feature flag.
|
||||
exec_policy: Arc<ExecPolicy>,
|
||||
|
||||
// TODO(pakrym): Remove config from here
|
||||
original_config_do_not_use: Arc<Config>,
|
||||
@@ -436,6 +444,7 @@ impl Session {
|
||||
final_output_json_schema: None,
|
||||
codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
exec_policy: session_configuration.exec_policy.clone(),
|
||||
truncation_policy: TruncationPolicy::new(&per_turn_config),
|
||||
}
|
||||
}
|
||||
@@ -910,6 +919,7 @@ impl Session {
|
||||
|
||||
let event = EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id,
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
changes,
|
||||
reason,
|
||||
grant_root,
|
||||
@@ -1187,11 +1197,9 @@ impl Session {
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
message: impl Into<String>,
|
||||
http_status_code: Option<StatusCode>,
|
||||
) {
|
||||
let event = EventMsg::StreamError(StreamErrorEvent {
|
||||
message: message.into(),
|
||||
http_status_code: http_status_code_value(http_status_code),
|
||||
});
|
||||
self.send_event(turn_context, event).await;
|
||||
}
|
||||
@@ -1683,7 +1691,6 @@ mod handlers {
|
||||
id: sub_id.clone(),
|
||||
msg: EventMsg::Error(ErrorEvent {
|
||||
message: "Failed to shutdown rollout recorder".to_string(),
|
||||
http_status_code: None,
|
||||
}),
|
||||
};
|
||||
sess.send_event_raw(event).await;
|
||||
@@ -1789,6 +1796,7 @@ async fn spawn_review_thread(
|
||||
final_output_json_schema: None,
|
||||
codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(),
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
exec_policy: parent_turn_context.exec_policy.clone(),
|
||||
truncation_policy: TruncationPolicy::new(&per_turn_config),
|
||||
};
|
||||
|
||||
@@ -1937,8 +1945,10 @@ pub(crate) async fn run_task(
|
||||
}
|
||||
Err(e) => {
|
||||
info!("Turn error: {e:#}");
|
||||
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
|
||||
.await;
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: e.to_string(),
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
// let the user continue the conversation
|
||||
break;
|
||||
}
|
||||
@@ -2062,7 +2072,6 @@ async fn run_turn(
|
||||
sess.notify_stream_error(
|
||||
&turn_context,
|
||||
format!("Reconnecting... {retries}/{max_retries}"),
|
||||
e.http_status_code(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -2608,6 +2617,7 @@ mod tests {
|
||||
cwd: config.cwd.clone(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
features: Features::default(),
|
||||
exec_policy: Arc::new(ExecPolicy::empty()),
|
||||
session_source: SessionSource::Exec,
|
||||
};
|
||||
|
||||
@@ -2685,6 +2695,7 @@ mod tests {
|
||||
cwd: config.cwd.clone(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
features: Features::default(),
|
||||
exec_policy: Arc::new(ExecPolicy::empty()),
|
||||
session_source: SessionSource::Exec,
|
||||
};
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
|
||||
use crate::bash::parse_shell_lc_plain_commands;
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
|
||||
@@ -8,7 +10,7 @@ pub fn requires_initial_appoval(
|
||||
policy: AskForApproval,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
command: &[String],
|
||||
with_escalated_permissions: bool,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> bool {
|
||||
if is_known_safe_command(command) {
|
||||
return false;
|
||||
@@ -24,8 +26,7 @@ pub fn requires_initial_appoval(
|
||||
// In restricted sandboxes (ReadOnly/WorkspaceWrite), do not prompt for
|
||||
// non‑escalated, non‑dangerous commands — let the sandbox enforce
|
||||
// restrictions (e.g., block network/write) without a user prompt.
|
||||
let wants_escalation: bool = with_escalated_permissions;
|
||||
if wants_escalation {
|
||||
if sandbox_permissions.requires_escalated_permissions() {
|
||||
return true;
|
||||
}
|
||||
command_might_be_dangerous(command)
|
||||
|
||||
@@ -10,6 +10,7 @@ use crate::error::Result as CodexResult;
|
||||
use crate::features::Feature;
|
||||
use crate::protocol::AgentMessageEvent;
|
||||
use crate::protocol::CompactedItem;
|
||||
use crate::protocol::ErrorEvent;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::TaskStartedEvent;
|
||||
use crate::protocol::TurnContextItem;
|
||||
@@ -127,8 +128,10 @@ async fn run_compact_task_inner(
|
||||
continue;
|
||||
}
|
||||
sess.set_total_tokens_full(turn_context.as_ref()).await;
|
||||
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
|
||||
.await;
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: e.to_string(),
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
return;
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -138,14 +141,15 @@ async fn run_compact_task_inner(
|
||||
sess.notify_stream_error(
|
||||
turn_context.as_ref(),
|
||||
format!("Reconnecting... {retries}/{max_retries}"),
|
||||
e.http_status_code(),
|
||||
)
|
||||
.await;
|
||||
tokio::time::sleep(delay).await;
|
||||
continue;
|
||||
} else {
|
||||
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
|
||||
.await;
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: e.to_string(),
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::codex::TurnContext;
|
||||
use crate::error::Result as CodexResult;
|
||||
use crate::protocol::AgentMessageEvent;
|
||||
use crate::protocol::CompactedItem;
|
||||
use crate::protocol::ErrorEvent;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::RolloutItem;
|
||||
use crate::protocol::TaskStartedEvent;
|
||||
@@ -29,8 +30,10 @@ pub(crate) async fn run_remote_compact_task(sess: Arc<Session>, turn_context: Ar
|
||||
|
||||
async fn run_remote_compact_task_inner(sess: &Arc<Session>, turn_context: &Arc<TurnContext>) {
|
||||
if let Err(err) = run_remote_compact_task_inner_impl(sess, turn_context).await {
|
||||
let event = err.to_error_event(Some("Error running remote compact task".to_string()));
|
||||
sess.send_event(turn_context, EventMsg::Error(event)).await;
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: format!("Error running remote compact task: {err}"),
|
||||
});
|
||||
sess.send_event(turn_context, event).await;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -379,6 +379,7 @@ pub struct Notice {
|
||||
/// Tracks whether the user has seen the model migration prompt
|
||||
pub hide_gpt5_1_migration_prompt: Option<bool>,
|
||||
/// Tracks whether the user has seen the gpt-5.1-codex-max migration prompt
|
||||
#[serde(rename = "hide_gpt-5.1-codex-max_migration_prompt")]
|
||||
pub hide_gpt_5_1_codex_max_migration_prompt: Option<bool>,
|
||||
}
|
||||
|
||||
|
||||
@@ -10,7 +10,6 @@ use chrono::Local;
|
||||
use chrono::Utc;
|
||||
use codex_async_utils::CancelErr;
|
||||
use codex_protocol::ConversationId;
|
||||
use codex_protocol::protocol::ErrorEvent;
|
||||
use codex_protocol::protocol::RateLimitSnapshot;
|
||||
use reqwest::StatusCode;
|
||||
use serde_json;
|
||||
@@ -431,37 +430,6 @@ impl CodexErr {
|
||||
pub fn downcast_ref<T: std::any::Any>(&self) -> Option<&T> {
|
||||
(self as &dyn std::any::Any).downcast_ref::<T>()
|
||||
}
|
||||
|
||||
pub fn http_status_code(&self) -> Option<StatusCode> {
|
||||
match self {
|
||||
CodexErr::UnexpectedStatus(err) => Some(err.status),
|
||||
CodexErr::RetryLimit(err) => Some(err.status),
|
||||
CodexErr::UsageLimitReached(_) | CodexErr::UsageNotIncluded => {
|
||||
Some(StatusCode::TOO_MANY_REQUESTS)
|
||||
}
|
||||
CodexErr::InternalServerError => Some(StatusCode::INTERNAL_SERVER_ERROR),
|
||||
CodexErr::ResponseStreamFailed(err) => err.source.status(),
|
||||
CodexErr::ConnectionFailed(err) => err.source.status(),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn to_error_event(&self, message_prefix: Option<String>) -> ErrorEvent {
|
||||
let error_message = self.to_string();
|
||||
let message: String = match message_prefix {
|
||||
Some(prefix) => format!("{prefix}: {error_message}"),
|
||||
None => error_message,
|
||||
};
|
||||
|
||||
ErrorEvent {
|
||||
message,
|
||||
http_status_code: http_status_code_value(self.http_status_code()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn http_status_code_value(http_status_code: Option<StatusCode>) -> Option<u16> {
|
||||
http_status_code.as_ref().map(StatusCode::as_u16)
|
||||
}
|
||||
|
||||
pub fn get_error_message_ui(e: &CodexErr) -> String {
|
||||
@@ -807,43 +775,4 @@ mod tests {
|
||||
assert_eq!(err.to_string(), expected);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_event_includes_http_status_code_when_available() {
|
||||
let err = CodexErr::UnexpectedStatus(UnexpectedResponseError {
|
||||
status: StatusCode::BAD_REQUEST,
|
||||
body: "oops".to_string(),
|
||||
request_id: Some("req-1".to_string()),
|
||||
});
|
||||
let event = err.to_error_event(None);
|
||||
|
||||
assert_eq!(
|
||||
event.message,
|
||||
"unexpected status 400 Bad Request: oops, request id: req-1"
|
||||
);
|
||||
assert_eq!(
|
||||
event.http_status_code,
|
||||
Some(StatusCode::BAD_REQUEST.as_u16())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_event_omits_http_status_code_when_unknown() {
|
||||
let event = CodexErr::Fatal("boom".to_string()).to_error_event(None);
|
||||
|
||||
assert_eq!(event.message, "Fatal error: boom");
|
||||
assert_eq!(event.http_status_code, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_event_applies_message_wrapper() {
|
||||
let event = CodexErr::Fatal("boom".to_string())
|
||||
.to_error_event(Some("Error running remote compact task".to_string()));
|
||||
|
||||
assert_eq!(
|
||||
event.message,
|
||||
"Error running remote compact task: Fatal error: boom"
|
||||
);
|
||||
assert_eq!(event.http_status_code, None);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,365 @@
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use thiserror::Error;
|
||||
use tokio::fs;
|
||||
|
||||
use crate::bash::parse_shell_lc_plain_commands;
|
||||
use crate::features::Feature;
|
||||
use crate::features::Features;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::tools::sandboxing::ApprovalRequirement;
|
||||
|
||||
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";
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub enum ExecPolicyError {
|
||||
#[error("failed to read execpolicy files from {dir}: {source}")]
|
||||
ReadDir {
|
||||
dir: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
|
||||
#[error("failed to read execpolicy file {path}: {source}")]
|
||||
ReadFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
|
||||
#[error("failed to parse execpolicy file {path}: {source}")]
|
||||
ParsePolicy {
|
||||
path: String,
|
||||
source: codex_execpolicy::Error,
|
||||
},
|
||||
}
|
||||
|
||||
pub(crate) async fn exec_policy_for(
|
||||
features: &Features,
|
||||
codex_home: &Path,
|
||||
) -> Result<Arc<Policy>, ExecPolicyError> {
|
||||
if !features.enabled(Feature::ExecPolicy) {
|
||||
return Ok(Arc::new(Policy::empty()));
|
||||
}
|
||||
|
||||
let policy_dir = codex_home.join(POLICY_DIR_NAME);
|
||||
let policy_paths = collect_policy_files(&policy_dir).await?;
|
||||
|
||||
let mut parser = PolicyParser::new();
|
||||
for policy_path in &policy_paths {
|
||||
let contents =
|
||||
fs::read_to_string(policy_path)
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadFile {
|
||||
path: policy_path.clone(),
|
||||
source,
|
||||
})?;
|
||||
let identifier = policy_path.to_string_lossy().to_string();
|
||||
parser
|
||||
.parse(&identifier, &contents)
|
||||
.map_err(|source| ExecPolicyError::ParsePolicy {
|
||||
path: identifier,
|
||||
source,
|
||||
})?;
|
||||
}
|
||||
|
||||
let policy = Arc::new(parser.build());
|
||||
tracing::debug!(
|
||||
"loaded execpolicy from {} files in {}",
|
||||
policy_paths.len(),
|
||||
policy_dir.display()
|
||||
);
|
||||
|
||||
Ok(policy)
|
||||
}
|
||||
|
||||
fn evaluate_with_policy(
|
||||
policy: &Policy,
|
||||
command: &[String],
|
||||
approval_policy: AskForApproval,
|
||||
) -> Option<ApprovalRequirement> {
|
||||
let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]);
|
||||
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::NoMatch => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn create_approval_requirement_for_command(
|
||||
policy: &Policy,
|
||||
command: &[String],
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> ApprovalRequirement {
|
||||
if let Some(requirement) = evaluate_with_policy(policy, command, approval_policy) {
|
||||
return requirement;
|
||||
}
|
||||
|
||||
if requires_initial_appoval(
|
||||
approval_policy,
|
||||
sandbox_policy,
|
||||
command,
|
||||
sandbox_permissions,
|
||||
) {
|
||||
ApprovalRequirement::NeedsApproval { reason: None }
|
||||
} else {
|
||||
ApprovalRequirement::Skip
|
||||
}
|
||||
}
|
||||
|
||||
async fn collect_policy_files(dir: &Path) -> Result<Vec<PathBuf>, ExecPolicyError> {
|
||||
let mut read_dir = match fs::read_dir(dir).await {
|
||||
Ok(read_dir) => read_dir,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
|
||||
Err(source) => {
|
||||
return Err(ExecPolicyError::ReadDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
let mut policy_paths = Vec::new();
|
||||
while let Some(entry) =
|
||||
read_dir
|
||||
.next_entry()
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
})?
|
||||
{
|
||||
let path = entry.path();
|
||||
let file_type = entry
|
||||
.file_type()
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
if path
|
||||
.extension()
|
||||
.and_then(|ext| ext.to_str())
|
||||
.is_some_and(|ext| ext == POLICY_EXTENSION)
|
||||
&& file_type.is_file()
|
||||
{
|
||||
policy_paths.push(path);
|
||||
}
|
||||
}
|
||||
|
||||
policy_paths.sort();
|
||||
|
||||
Ok(policy_paths)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::features::Feature;
|
||||
use crate::features::Features;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn returns_empty_policy_when_feature_disabled() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.disable(Feature::ExecPolicy);
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
|
||||
let policy = exec_policy_for(&features, temp_dir.path())
|
||||
.await
|
||||
.expect("policy result");
|
||||
|
||||
let commands = [vec!["rm".to_string()]];
|
||||
assert!(matches!(
|
||||
policy.check_multiple(commands.iter()),
|
||||
Evaluation::NoMatch
|
||||
));
|
||||
assert!(!temp_dir.path().join(POLICY_DIR_NAME).exists());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn collect_policy_files_returns_empty_when_dir_missing() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
|
||||
let policy_dir = temp_dir.path().join(POLICY_DIR_NAME);
|
||||
let files = collect_policy_files(&policy_dir)
|
||||
.await
|
||||
.expect("collect policy files");
|
||||
|
||||
assert!(files.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_policies_from_policy_subdirectory() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
let policy_dir = temp_dir.path().join(POLICY_DIR_NAME);
|
||||
fs::create_dir_all(&policy_dir).expect("create policy dir");
|
||||
fs::write(
|
||||
policy_dir.join("deny.codexpolicy"),
|
||||
r#"prefix_rule(pattern=["rm"], decision="forbidden")"#,
|
||||
)
|
||||
.expect("write policy file");
|
||||
|
||||
let policy = exec_policy_for(&Features::with_defaults(), temp_dir.path())
|
||||
.await
|
||||
.expect("policy result");
|
||||
let command = [vec!["rm".to_string()]];
|
||||
assert!(matches!(
|
||||
policy.check_multiple(command.iter()),
|
||||
Evaluation::Match { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ignores_policies_outside_policy_dir() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
fs::write(
|
||||
temp_dir.path().join("root.codexpolicy"),
|
||||
r#"prefix_rule(pattern=["ls"], decision="prompt")"#,
|
||||
)
|
||||
.expect("write policy file");
|
||||
|
||||
let policy = exec_policy_for(&Features::with_defaults(), temp_dir.path())
|
||||
.await
|
||||
.expect("policy result");
|
||||
let command = [vec!["ls".to_string()]];
|
||||
assert!(matches!(
|
||||
policy.check_multiple(command.iter()),
|
||||
Evaluation::NoMatch
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluates_bash_lc_inner_commands() {
|
||||
let policy_src = r#"
|
||||
prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
let policy = parser.build();
|
||||
|
||||
let forbidden_script = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"rm -rf /tmp".to_string(),
|
||||
];
|
||||
|
||||
let requirement =
|
||||
evaluate_with_policy(&policy, &forbidden_script, AskForApproval::OnRequest)
|
||||
.expect("expected match for forbidden command");
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ApprovalRequirement::Forbidden {
|
||||
reason: FORBIDDEN_REASON.to_string()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_requirement_prefers_execpolicy_match() {
|
||||
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 requirement = create_approval_requirement_for_command(
|
||||
&policy,
|
||||
&command,
|
||||
AskForApproval::OnRequest,
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
SandboxPermissions::UseDefault,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ApprovalRequirement::NeedsApproval {
|
||||
reason: Some(PROMPT_REASON.to_string())
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_requirement_respects_approval_policy() {
|
||||
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 requirement = create_approval_requirement_for_command(
|
||||
&policy,
|
||||
&command,
|
||||
AskForApproval::Never,
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
SandboxPermissions::UseDefault,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ApprovalRequirement::Forbidden {
|
||||
reason: PROMPT_REASON.to_string()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_requirement_falls_back_to_heuristics() {
|
||||
let command = vec!["python".to_string()];
|
||||
|
||||
let empty_policy = Policy::empty();
|
||||
let requirement = create_approval_requirement_for_command(
|
||||
&empty_policy,
|
||||
&command,
|
||||
AskForApproval::UnlessTrusted,
|
||||
&SandboxPolicy::ReadOnly,
|
||||
SandboxPermissions::UseDefault,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ApprovalRequirement::NeedsApproval { reason: None }
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -42,6 +42,8 @@ pub enum Feature {
|
||||
ViewImageTool,
|
||||
/// Allow the model to request web searches.
|
||||
WebSearchRequest,
|
||||
/// Gate the execpolicy enforcement for shell/unified exec.
|
||||
ExecPolicy,
|
||||
/// Enable the model-based risk assessments for sandboxed commands.
|
||||
SandboxCommandAssessment,
|
||||
/// Enable Windows sandbox (restricted token) on Windows.
|
||||
@@ -297,6 +299,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Stable,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ExecPolicy,
|
||||
key: "exec_policy",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::SandboxCommandAssessment,
|
||||
key: "experimental_sandbox_command_assessment",
|
||||
|
||||
@@ -25,6 +25,7 @@ mod environment_context;
|
||||
pub mod error;
|
||||
pub mod exec;
|
||||
pub mod exec_env;
|
||||
mod exec_policy;
|
||||
pub mod features;
|
||||
mod flags;
|
||||
pub mod git_info;
|
||||
|
||||
@@ -26,6 +26,28 @@ use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub enum SandboxPermissions {
|
||||
UseDefault,
|
||||
RequireEscalated,
|
||||
}
|
||||
|
||||
impl SandboxPermissions {
|
||||
pub fn requires_escalated_permissions(self) -> bool {
|
||||
matches!(self, SandboxPermissions::RequireEscalated)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<bool> for SandboxPermissions {
|
||||
fn from(with_escalated_permissions: bool) -> Self {
|
||||
if with_escalated_permissions {
|
||||
SandboxPermissions::RequireEscalated
|
||||
} else {
|
||||
SandboxPermissions::UseDefault
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct CommandSpec {
|
||||
pub program: String,
|
||||
|
||||
@@ -179,15 +179,17 @@ impl ToolEmitter {
|
||||
ctx.turn,
|
||||
EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||
call_id: ctx.call_id.to_string(),
|
||||
turn_id: ctx.turn.sub_id.clone(),
|
||||
auto_approved: *auto_approved,
|
||||
changes: changes.clone(),
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
(Self::ApplyPatch { .. }, ToolEventStage::Success(output)) => {
|
||||
(Self::ApplyPatch { changes, .. }, ToolEventStage::Success(output)) => {
|
||||
emit_patch_end(
|
||||
ctx,
|
||||
changes.clone(),
|
||||
output.stdout.text.clone(),
|
||||
output.stderr.text.clone(),
|
||||
output.exit_code == 0,
|
||||
@@ -195,11 +197,12 @@ impl ToolEmitter {
|
||||
.await;
|
||||
}
|
||||
(
|
||||
Self::ApplyPatch { .. },
|
||||
Self::ApplyPatch { changes, .. },
|
||||
ToolEventStage::Failure(ToolEventFailure::Output(output)),
|
||||
) => {
|
||||
emit_patch_end(
|
||||
ctx,
|
||||
changes.clone(),
|
||||
output.stdout.text.clone(),
|
||||
output.stderr.text.clone(),
|
||||
output.exit_code == 0,
|
||||
@@ -207,10 +210,17 @@ impl ToolEmitter {
|
||||
.await;
|
||||
}
|
||||
(
|
||||
Self::ApplyPatch { .. },
|
||||
Self::ApplyPatch { changes, .. },
|
||||
ToolEventStage::Failure(ToolEventFailure::Message(message)),
|
||||
) => {
|
||||
emit_patch_end(ctx, String::new(), (*message).to_string(), false).await;
|
||||
emit_patch_end(
|
||||
ctx,
|
||||
changes.clone(),
|
||||
String::new(),
|
||||
(*message).to_string(),
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
(
|
||||
Self::UnifiedExec {
|
||||
@@ -409,15 +419,23 @@ async fn emit_exec_end(
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn emit_patch_end(ctx: ToolEventCtx<'_>, stdout: String, stderr: String, success: bool) {
|
||||
async fn emit_patch_end(
|
||||
ctx: ToolEventCtx<'_>,
|
||||
changes: HashMap<PathBuf, FileChange>,
|
||||
stdout: String,
|
||||
stderr: String,
|
||||
success: bool,
|
||||
) {
|
||||
ctx.session
|
||||
.send_event(
|
||||
ctx.turn,
|
||||
EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||
call_id: ctx.call_id.to_string(),
|
||||
turn_id: ctx.turn.sub_id.clone(),
|
||||
stdout,
|
||||
stderr,
|
||||
success,
|
||||
changes,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -9,9 +9,11 @@ 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::function_tool::FunctionCallError;
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
use crate::protocol::ExecCommandSource;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -302,6 +304,13 @@ 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)),
|
||||
),
|
||||
};
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = ShellRuntime::new();
|
||||
|
||||
@@ -11,11 +11,13 @@ use crate::error::get_error_message_ui;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::sandboxing::SandboxManager;
|
||||
use crate::tools::sandboxing::ApprovalCtx;
|
||||
use crate::tools::sandboxing::ApprovalRequirement;
|
||||
use crate::tools::sandboxing::ProvidesSandboxRetryData;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::default_approval_requirement;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
|
||||
@@ -49,40 +51,52 @@ impl ToolOrchestrator {
|
||||
let otel_cfg = codex_otel::otel_event_manager::ToolDecisionSource::Config;
|
||||
|
||||
// 1) Approval
|
||||
let needs_initial_approval =
|
||||
tool.wants_initial_approval(req, approval_policy, &turn_ctx.sandbox_policy);
|
||||
let mut already_approved = false;
|
||||
|
||||
if needs_initial_approval {
|
||||
let mut risk = None;
|
||||
|
||||
if let Some(metadata) = req.sandbox_retry_data() {
|
||||
risk = tool_ctx
|
||||
.session
|
||||
.assess_sandbox_command(turn_ctx, &tool_ctx.call_id, &metadata.command, None)
|
||||
.await;
|
||||
let requirement = tool.approval_requirement(req).unwrap_or_else(|| {
|
||||
default_approval_requirement(approval_policy, &turn_ctx.sandbox_policy)
|
||||
});
|
||||
match requirement {
|
||||
ApprovalRequirement::Skip => {
|
||||
otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg);
|
||||
}
|
||||
ApprovalRequirement::Forbidden { reason } => {
|
||||
return Err(ToolError::Rejected(reason));
|
||||
}
|
||||
ApprovalRequirement::NeedsApproval { reason } => {
|
||||
let mut risk = None;
|
||||
|
||||
let approval_ctx = ApprovalCtx {
|
||||
session: tool_ctx.session,
|
||||
turn: turn_ctx,
|
||||
call_id: &tool_ctx.call_id,
|
||||
retry_reason: None,
|
||||
risk,
|
||||
};
|
||||
let decision = tool.start_approval_async(req, approval_ctx).await;
|
||||
|
||||
otel.tool_decision(otel_tn, otel_ci, decision, otel_user.clone());
|
||||
|
||||
match decision {
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||
return Err(ToolError::Rejected("rejected by user".to_string()));
|
||||
if let Some(metadata) = req.sandbox_retry_data() {
|
||||
risk = tool_ctx
|
||||
.session
|
||||
.assess_sandbox_command(
|
||||
turn_ctx,
|
||||
&tool_ctx.call_id,
|
||||
&metadata.command,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {}
|
||||
|
||||
let approval_ctx = ApprovalCtx {
|
||||
session: tool_ctx.session,
|
||||
turn: turn_ctx,
|
||||
call_id: &tool_ctx.call_id,
|
||||
retry_reason: reason,
|
||||
risk,
|
||||
};
|
||||
let decision = tool.start_approval_async(req, approval_ctx).await;
|
||||
|
||||
otel.tool_decision(otel_tn, otel_ci, decision, otel_user.clone());
|
||||
|
||||
match decision {
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||
return Err(ToolError::Rejected("rejected by user".to_string()));
|
||||
}
|
||||
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {}
|
||||
}
|
||||
already_approved = true;
|
||||
}
|
||||
already_approved = true;
|
||||
} else {
|
||||
otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg);
|
||||
}
|
||||
|
||||
// 2) First attempt under the selected sandbox.
|
||||
|
||||
@@ -4,13 +4,12 @@ Runtime: shell
|
||||
Executes shell requests under the orchestrator: asks for approval when needed,
|
||||
builds a CommandSpec, and runs it under the current SandboxAttempt.
|
||||
*/
|
||||
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::sandboxing::execute_env;
|
||||
use crate::tools::runtimes::build_command_spec;
|
||||
use crate::tools::sandboxing::Approvable;
|
||||
use crate::tools::sandboxing::ApprovalCtx;
|
||||
use crate::tools::sandboxing::ApprovalRequirement;
|
||||
use crate::tools::sandboxing::ProvidesSandboxRetryData;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::SandboxRetryData;
|
||||
@@ -20,7 +19,6 @@ use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use futures::future::BoxFuture;
|
||||
use std::path::PathBuf;
|
||||
@@ -33,6 +31,7 @@ pub struct ShellRequest {
|
||||
pub env: std::collections::HashMap<String, String>,
|
||||
pub with_escalated_permissions: Option<bool>,
|
||||
pub justification: Option<String>,
|
||||
pub approval_requirement: ApprovalRequirement,
|
||||
}
|
||||
|
||||
impl ProvidesSandboxRetryData for ShellRequest {
|
||||
@@ -114,18 +113,8 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
})
|
||||
}
|
||||
|
||||
fn wants_initial_approval(
|
||||
&self,
|
||||
req: &ShellRequest,
|
||||
policy: AskForApproval,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
) -> bool {
|
||||
requires_initial_appoval(
|
||||
policy,
|
||||
sandbox_policy,
|
||||
&req.command,
|
||||
req.with_escalated_permissions.unwrap_or(false),
|
||||
)
|
||||
fn approval_requirement(&self, req: &ShellRequest) -> Option<ApprovalRequirement> {
|
||||
Some(req.approval_requirement.clone())
|
||||
}
|
||||
|
||||
fn wants_escalated_first_attempt(&self, req: &ShellRequest) -> bool {
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
|
||||
/*
|
||||
Runtime: unified exec
|
||||
|
||||
@@ -10,6 +9,7 @@ use crate::error::SandboxErr;
|
||||
use crate::tools::runtimes::build_command_spec;
|
||||
use crate::tools::sandboxing::Approvable;
|
||||
use crate::tools::sandboxing::ApprovalCtx;
|
||||
use crate::tools::sandboxing::ApprovalRequirement;
|
||||
use crate::tools::sandboxing::ProvidesSandboxRetryData;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::SandboxRetryData;
|
||||
@@ -22,9 +22,7 @@ use crate::tools::sandboxing::with_cached_approval;
|
||||
use crate::unified_exec::UnifiedExecError;
|
||||
use crate::unified_exec::UnifiedExecSession;
|
||||
use crate::unified_exec::UnifiedExecSessionManager;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use futures::future::BoxFuture;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
@@ -36,6 +34,7 @@ pub struct UnifiedExecRequest {
|
||||
pub env: HashMap<String, String>,
|
||||
pub with_escalated_permissions: Option<bool>,
|
||||
pub justification: Option<String>,
|
||||
pub approval_requirement: ApprovalRequirement,
|
||||
}
|
||||
|
||||
impl ProvidesSandboxRetryData for UnifiedExecRequest {
|
||||
@@ -65,6 +64,7 @@ impl UnifiedExecRequest {
|
||||
env: HashMap<String, String>,
|
||||
with_escalated_permissions: Option<bool>,
|
||||
justification: Option<String>,
|
||||
approval_requirement: ApprovalRequirement,
|
||||
) -> Self {
|
||||
Self {
|
||||
command,
|
||||
@@ -72,6 +72,7 @@ impl UnifiedExecRequest {
|
||||
env,
|
||||
with_escalated_permissions,
|
||||
justification,
|
||||
approval_requirement,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -129,18 +130,8 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
})
|
||||
}
|
||||
|
||||
fn wants_initial_approval(
|
||||
&self,
|
||||
req: &UnifiedExecRequest,
|
||||
policy: AskForApproval,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
) -> bool {
|
||||
requires_initial_appoval(
|
||||
policy,
|
||||
sandbox_policy,
|
||||
&req.command,
|
||||
req.with_escalated_permissions.unwrap_or(false),
|
||||
)
|
||||
fn approval_requirement(&self, req: &UnifiedExecRequest) -> Option<ApprovalRequirement> {
|
||||
Some(req.approval_requirement.clone())
|
||||
}
|
||||
|
||||
fn wants_escalated_first_attempt(&self, req: &UnifiedExecRequest) -> bool {
|
||||
|
||||
@@ -86,6 +86,37 @@ pub(crate) struct ApprovalCtx<'a> {
|
||||
pub risk: Option<SandboxCommandAssessment>,
|
||||
}
|
||||
|
||||
// Specifies what tool orchestrator should do with a given tool call.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum ApprovalRequirement {
|
||||
/// No approval required for this tool call
|
||||
Skip,
|
||||
/// Approval required for this tool call
|
||||
NeedsApproval { reason: Option<String> },
|
||||
/// Execution forbidden for this tool call
|
||||
Forbidden { reason: String },
|
||||
}
|
||||
|
||||
/// - Never, OnFailure: do not ask
|
||||
/// - OnRequest: ask unless sandbox policy is DangerFullAccess
|
||||
/// - UnlessTrusted: always ask
|
||||
pub(crate) fn default_approval_requirement(
|
||||
policy: AskForApproval,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
) -> ApprovalRequirement {
|
||||
let needs_approval = match policy {
|
||||
AskForApproval::Never | AskForApproval::OnFailure => false,
|
||||
AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess),
|
||||
AskForApproval::UnlessTrusted => true,
|
||||
};
|
||||
|
||||
if needs_approval {
|
||||
ApprovalRequirement::NeedsApproval { reason: None }
|
||||
} else {
|
||||
ApprovalRequirement::Skip
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) trait Approvable<Req> {
|
||||
type ApprovalKey: Hash + Eq + Clone + Debug + Serialize;
|
||||
|
||||
@@ -106,22 +137,11 @@ pub(crate) trait Approvable<Req> {
|
||||
matches!(policy, AskForApproval::Never)
|
||||
}
|
||||
|
||||
/// Decide whether an initial user approval should be requested before the
|
||||
/// first attempt. Defaults to the orchestrator's behavior (pre‑refactor):
|
||||
/// - Never, OnFailure: do not ask
|
||||
/// - OnRequest: ask unless sandbox policy is DangerFullAccess
|
||||
/// - UnlessTrusted: always ask
|
||||
fn wants_initial_approval(
|
||||
&self,
|
||||
_req: &Req,
|
||||
policy: AskForApproval,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
) -> bool {
|
||||
match policy {
|
||||
AskForApproval::Never | AskForApproval::OnFailure => false,
|
||||
AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess),
|
||||
AskForApproval::UnlessTrusted => true,
|
||||
}
|
||||
/// Override the default approval requirement. Return `Some(_)` to specify
|
||||
/// a custom requirement, or `None` to fall back to
|
||||
/// policy-based default.
|
||||
fn approval_requirement(&self, _req: &Req) -> Option<ApprovalRequirement> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Decide we can request an approval for no-sandbox execution.
|
||||
|
||||
@@ -185,6 +185,7 @@ fn truncate_with_byte_estimate(s: &str, policy: TruncationPolicy) -> String {
|
||||
if s.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
|
||||
let total_chars = s.chars().count();
|
||||
let max_bytes = policy.byte_budget();
|
||||
|
||||
@@ -204,24 +205,55 @@ fn truncate_with_byte_estimate(s: &str, policy: TruncationPolicy) -> String {
|
||||
let total_bytes = s.len();
|
||||
|
||||
let (left_budget, right_budget) = split_budget(max_bytes);
|
||||
let prefix_end = pick_prefix_end(s, left_budget);
|
||||
let mut suffix_start = pick_suffix_start(s, right_budget);
|
||||
if suffix_start < prefix_end {
|
||||
suffix_start = prefix_end;
|
||||
}
|
||||
|
||||
let left_chars = s[..prefix_end].chars().count();
|
||||
let right_chars = s[suffix_start..].chars().count();
|
||||
let removed_chars = total_chars
|
||||
.saturating_sub(left_chars)
|
||||
.saturating_sub(right_chars);
|
||||
let (removed_chars, left, right) = split_string(s, left_budget, right_budget);
|
||||
|
||||
let marker = format_truncation_marker(
|
||||
policy,
|
||||
removed_units_for_source(policy, total_bytes.saturating_sub(max_bytes), removed_chars),
|
||||
);
|
||||
|
||||
assemble_truncated_output(&s[..prefix_end], &s[suffix_start..], &marker)
|
||||
assemble_truncated_output(left, right, &marker)
|
||||
}
|
||||
|
||||
fn split_string(s: &str, beginning_bytes: usize, end_bytes: usize) -> (usize, &str, &str) {
|
||||
if s.is_empty() {
|
||||
return (0, "", "");
|
||||
}
|
||||
|
||||
let len = s.len();
|
||||
let tail_start_target = len.saturating_sub(end_bytes);
|
||||
let mut prefix_end = 0usize;
|
||||
let mut suffix_start = len;
|
||||
let mut removed_chars = 0usize;
|
||||
let mut suffix_started = false;
|
||||
|
||||
for (idx, ch) in s.char_indices() {
|
||||
let char_end = idx + ch.len_utf8();
|
||||
if char_end <= beginning_bytes {
|
||||
prefix_end = char_end;
|
||||
continue;
|
||||
}
|
||||
|
||||
if idx >= tail_start_target {
|
||||
if !suffix_started {
|
||||
suffix_start = idx;
|
||||
suffix_started = true;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
removed_chars = removed_chars.saturating_add(1);
|
||||
}
|
||||
|
||||
if suffix_start < prefix_end {
|
||||
suffix_start = prefix_end;
|
||||
}
|
||||
|
||||
let before = &s[..prefix_end];
|
||||
let after = &s[suffix_start..];
|
||||
|
||||
(removed_chars, before, after)
|
||||
}
|
||||
|
||||
fn format_truncation_marker(policy: TruncationPolicy, removed_count: u64) -> String {
|
||||
@@ -270,42 +302,54 @@ fn approx_tokens_from_byte_count(bytes: usize) -> u64 {
|
||||
/ (APPROX_BYTES_PER_TOKEN as u64)
|
||||
}
|
||||
|
||||
fn truncate_on_boundary(input: &str, max_len: usize) -> &str {
|
||||
if input.len() <= max_len {
|
||||
return input;
|
||||
}
|
||||
let mut end = max_len;
|
||||
while end > 0 && !input.is_char_boundary(end) {
|
||||
end -= 1;
|
||||
}
|
||||
&input[..end]
|
||||
}
|
||||
|
||||
fn pick_prefix_end(s: &str, left_budget: usize) -> usize {
|
||||
truncate_on_boundary(s, left_budget).len()
|
||||
}
|
||||
|
||||
fn pick_suffix_start(s: &str, right_budget: usize) -> usize {
|
||||
let start_tail = s.len().saturating_sub(right_budget);
|
||||
let mut idx = start_tail.min(s.len());
|
||||
while idx < s.len() && !s.is_char_boundary(idx) {
|
||||
idx += 1;
|
||||
}
|
||||
idx
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
|
||||
use super::TruncationPolicy;
|
||||
use super::approx_token_count;
|
||||
use super::formatted_truncate_text;
|
||||
use super::split_string;
|
||||
use super::truncate_function_output_items_with_policy;
|
||||
use super::truncate_text;
|
||||
use super::truncate_with_token_budget;
|
||||
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn split_string_works() {
|
||||
assert_eq!(split_string("hello world", 5, 5), (1, "hello", "world"));
|
||||
assert_eq!(split_string("abc", 0, 0), (3, "", ""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_string_handles_empty_string() {
|
||||
assert_eq!(split_string("", 4, 4), (0, "", ""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_string_only_keeps_prefix_when_tail_budget_is_zero() {
|
||||
assert_eq!(split_string("abcdef", 3, 0), (3, "abc", ""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_string_only_keeps_suffix_when_prefix_budget_is_zero() {
|
||||
assert_eq!(split_string("abcdef", 0, 3), (3, "", "def"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_string_handles_overlapping_budgets_without_removal() {
|
||||
assert_eq!(split_string("abcdef", 4, 4), (0, "abcd", "ef"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_string_respects_utf8_boundaries() {
|
||||
assert_eq!(split_string("😀abc😀", 5, 5), (1, "😀a", "c😀"));
|
||||
|
||||
assert_eq!(split_string("😀😀😀😀😀", 1, 1), (5, "", ""));
|
||||
assert_eq!(split_string("😀😀😀😀😀", 7, 7), (3, "😀", "😀"));
|
||||
assert_eq!(split_string("😀😀😀😀😀", 8, 8), (1, "😀😀", "😀😀"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn truncate_bytes_less_than_placeholder_returns_placeholder() {
|
||||
let content = "example output";
|
||||
|
||||
@@ -11,10 +11,12 @@ 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::protocol::BackgroundEventEvent;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::ExecCommandSource;
|
||||
use crate::sandboxing::ExecEnv;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
use crate::tools::events::ToolEventFailure;
|
||||
@@ -449,6 +451,13 @@ impl UnifiedExecSessionManager {
|
||||
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)),
|
||||
),
|
||||
);
|
||||
let tool_ctx = ToolCtx {
|
||||
session: context.session.as_ref(),
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
|
||||
#[tokio::test]
|
||||
async fn execpolicy_blocks_shell_invocation() -> Result<()> {
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
let policy_path = config.codex_home.join("policy").join("policy.codexpolicy");
|
||||
fs::create_dir_all(
|
||||
policy_path
|
||||
.parent()
|
||||
.expect("policy directory must have a parent"),
|
||||
)
|
||||
.expect("create policy directory");
|
||||
fs::write(
|
||||
&policy_path,
|
||||
r#"prefix_rule(pattern=["echo"], decision="forbidden")"#,
|
||||
)
|
||||
.expect("write policy file");
|
||||
});
|
||||
let server = start_mock_server().await;
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "shell-forbidden";
|
||||
let args = json!({
|
||||
"command": ["echo", "blocked"],
|
||||
"timeout_ms": 1_000,
|
||||
});
|
||||
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "shell", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let session_model = test.session_configured.model.clone();
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "run shell command".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: session_model,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let EventMsg::ExecCommandEnd(end) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::ExecCommandEnd(_))
|
||||
})
|
||||
.await
|
||||
else {
|
||||
unreachable!()
|
||||
};
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TaskComplete(_))
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
end.aggregated_output
|
||||
.contains("execpolicy forbids this command"),
|
||||
"unexpected output: {}",
|
||||
end.aggregated_output
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -28,6 +28,7 @@ mod compact_remote;
|
||||
mod compact_resume_fork;
|
||||
mod deprecation_notice;
|
||||
mod exec;
|
||||
mod exec_policy;
|
||||
mod fork_conversation;
|
||||
mod grep_files;
|
||||
mod items;
|
||||
|
||||
@@ -72,7 +72,7 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc
|
||||
- `EventMsg::AgentMessage` – Messages from the `Model`
|
||||
- `EventMsg::ExecApprovalRequest` – Request approval from user to execute a command
|
||||
- `EventMsg::TaskComplete` – A task completed successfully
|
||||
- `EventMsg::Error` – A task stopped with an error (includes an optional `http_status_code` when available)
|
||||
- `EventMsg::Error` – A task stopped with an error
|
||||
- `EventMsg::Warning` – A non-fatal warning that the client should surface to the user
|
||||
- `EventMsg::TurnComplete` – Contains a `response_id` bookmark for last `response_id` executed by the task. This can be used to continue the task at a later point in time, perhaps with additional user input.
|
||||
|
||||
|
||||
@@ -144,7 +144,7 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> {
|
||||
std::process::exit(exit_code);
|
||||
}
|
||||
|
||||
// TODO: replace with execpolicy2
|
||||
// TODO: replace with execpolicy
|
||||
|
||||
fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome {
|
||||
if file.ends_with("rm") {
|
||||
|
||||
@@ -43,12 +43,17 @@ impl McpEscalationPolicy {
|
||||
|
||||
async fn prompt(
|
||||
&self,
|
||||
_file: &Path,
|
||||
file: &Path,
|
||||
argv: &[String],
|
||||
workdir: &Path,
|
||||
context: RequestContext<RoleServer>,
|
||||
) -> Result<CreateElicitationResult, McpError> {
|
||||
let command = shlex::try_join(argv.iter().map(String::as_str)).unwrap_or_default();
|
||||
let args = shlex::try_join(argv.iter().skip(1).map(String::as_str)).unwrap_or_default();
|
||||
let command = if args.is_empty() {
|
||||
file.display().to_string()
|
||||
} else {
|
||||
format!("{} {}", file.display(), args)
|
||||
};
|
||||
context
|
||||
.peer
|
||||
.create_elicitation(CreateElicitationRequestParam {
|
||||
|
||||
@@ -161,7 +161,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
|
||||
fn process_event(&mut self, event: Event) -> CodexStatus {
|
||||
let Event { id: _, msg } = event;
|
||||
match msg {
|
||||
EventMsg::Error(ErrorEvent { message, .. }) => {
|
||||
EventMsg::Error(ErrorEvent { message }) => {
|
||||
let prefix = "ERROR:".style(self.red);
|
||||
ts_msg!(self, "{prefix} {message}");
|
||||
}
|
||||
@@ -221,7 +221,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
|
||||
EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => {
|
||||
ts_msg!(self, "{}", message.style(self.dimmed));
|
||||
}
|
||||
EventMsg::StreamError(StreamErrorEvent { message, .. }) => {
|
||||
EventMsg::StreamError(StreamErrorEvent { message }) => {
|
||||
ts_msg!(self, "{}", message.style(self.dimmed));
|
||||
}
|
||||
EventMsg::TaskStarted(_) => {
|
||||
@@ -346,6 +346,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
|
||||
call_id,
|
||||
auto_approved,
|
||||
changes,
|
||||
..
|
||||
}) => {
|
||||
// Store metadata so we can calculate duration later when we
|
||||
// receive the corresponding PatchApplyEnd event.
|
||||
|
||||
@@ -166,6 +166,7 @@ pub struct FileUpdateChange {
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum PatchApplyStatus {
|
||||
InProgress,
|
||||
Completed,
|
||||
Failed,
|
||||
}
|
||||
|
||||
@@ -539,7 +539,6 @@ fn error_event_produces_error() {
|
||||
"e1",
|
||||
EventMsg::Error(codex_core::protocol::ErrorEvent {
|
||||
message: "boom".to_string(),
|
||||
http_status_code: Some(500),
|
||||
}),
|
||||
));
|
||||
assert_eq!(
|
||||
@@ -579,7 +578,6 @@ fn stream_error_event_produces_error() {
|
||||
"e1",
|
||||
EventMsg::StreamError(codex_core::protocol::StreamErrorEvent {
|
||||
message: "retrying".to_string(),
|
||||
http_status_code: Some(500),
|
||||
}),
|
||||
));
|
||||
assert_eq!(
|
||||
@@ -598,7 +596,6 @@ fn error_followed_by_task_complete_produces_turn_failed() {
|
||||
"e1",
|
||||
EventMsg::Error(ErrorEvent {
|
||||
message: "boom".to_string(),
|
||||
http_status_code: Some(500),
|
||||
}),
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -825,6 +822,7 @@ fn patch_apply_success_produces_item_completed_patchapply() {
|
||||
"p1",
|
||||
EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||
call_id: "call-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
auto_approved: true,
|
||||
changes: changes.clone(),
|
||||
}),
|
||||
@@ -837,9 +835,11 @@ fn patch_apply_success_produces_item_completed_patchapply() {
|
||||
"p2",
|
||||
EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||
call_id: "call-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
stdout: "applied 3 changes".to_string(),
|
||||
stderr: String::new(),
|
||||
success: true,
|
||||
changes: changes.clone(),
|
||||
}),
|
||||
);
|
||||
let out_end = ep.collect_thread_events(&end);
|
||||
@@ -894,6 +894,7 @@ fn patch_apply_failure_produces_item_completed_patchapply_failed() {
|
||||
"p1",
|
||||
EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||
call_id: "call-2".to_string(),
|
||||
turn_id: "turn-2".to_string(),
|
||||
auto_approved: false,
|
||||
changes: changes.clone(),
|
||||
}),
|
||||
@@ -905,9 +906,11 @@ fn patch_apply_failure_produces_item_completed_patchapply_failed() {
|
||||
"p2",
|
||||
EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||
call_id: "call-2".to_string(),
|
||||
turn_id: "turn-2".to_string(),
|
||||
stdout: String::new(),
|
||||
stderr: "failed to apply".to_string(),
|
||||
success: false,
|
||||
changes: changes.clone(),
|
||||
}),
|
||||
);
|
||||
let out_end = ep.collect_thread_events(&end);
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
[package]
|
||||
edition = "2024"
|
||||
name = "codex-execpolicy-legacy"
|
||||
description = "Legacy exec policy engine for validating proposed exec calls."
|
||||
version = { workspace = true }
|
||||
|
||||
[[bin]]
|
||||
name = "codex-execpolicy-legacy"
|
||||
path = "src/main.rs"
|
||||
|
||||
[lib]
|
||||
name = "codex_execpolicy_legacy"
|
||||
path = "src/lib.rs"
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
allocative = { workspace = true }
|
||||
anyhow = { workspace = true }
|
||||
clap = { workspace = true, features = ["derive"] }
|
||||
derive_more = { workspace = true, features = ["display"] }
|
||||
env_logger = { workspace = true }
|
||||
log = { workspace = true }
|
||||
multimap = { workspace = true }
|
||||
path-absolutize = { workspace = true }
|
||||
regex-lite = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_with = { workspace = true, features = ["macros"] }
|
||||
starlark = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
tempfile = { workspace = true }
|
||||
@@ -0,0 +1,183 @@
|
||||
# codex-execpolicy-legacy
|
||||
|
||||
This crate hosts the original execpolicy implementation. The newer prefix-rule
|
||||
engine lives in `codex-execpolicy`.
|
||||
|
||||
The goal of this library is to classify a proposed [`execv(3)`](https://linux.die.net/man/3/execv) command into one of the following states:
|
||||
|
||||
- `safe` The command is safe to run (\*).
|
||||
- `match` The command matched a rule in the policy, but the caller should decide whether it is safe to run based on the files it will write.
|
||||
- `forbidden` The command is not allowed to be run.
|
||||
- `unverified` The safety cannot be determined: make the user decide.
|
||||
|
||||
(\*) Whether an `execv(3)` call should be considered "safe" often requires additional context beyond the arguments to `execv()` itself. For example, if you trust an autonomous software agent to write files in your source tree, then deciding whether `/bin/cp foo bar` is "safe" depends on `getcwd(3)` for the calling process as well as the `realpath` of `foo` and `bar` when resolved against `getcwd()`.
|
||||
To that end, rather than returning a boolean, the validator returns a structured result that the client is expected to use to determine the "safety" of the proposed `execv()` call.
|
||||
|
||||
For example, to check the command `ls -l foo`, the checker would be invoked as follows:
|
||||
|
||||
```shell
|
||||
cargo run -p codex-execpolicy-legacy -- check ls -l foo | jq
|
||||
```
|
||||
|
||||
It will exit with `0` and print the following to stdout:
|
||||
|
||||
```json
|
||||
{
|
||||
"result": "safe",
|
||||
"match": {
|
||||
"program": "ls",
|
||||
"flags": [
|
||||
{
|
||||
"name": "-l"
|
||||
}
|
||||
],
|
||||
"opts": [],
|
||||
"args": [
|
||||
{
|
||||
"index": 1,
|
||||
"type": "ReadableFile",
|
||||
"value": "foo"
|
||||
}
|
||||
],
|
||||
"system_path": ["/bin/ls", "/usr/bin/ls"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Of note:
|
||||
|
||||
- `foo` is tagged as a `ReadableFile`, so the caller should resolve `foo` relative to `getcwd()` and `realpath` it (as it may be a symlink) to determine whether `foo` is safe to read.
|
||||
- While the specified executable is `ls`, `"system_path"` offers `/bin/ls` and `/usr/bin/ls` as viable alternatives to avoid using whatever `ls` happens to appear first on the user's `$PATH`. If either exists on the host, it is recommended to use it as the first argument to `execv(3)` instead of `ls`.
|
||||
|
||||
Further, "safety" in this system is not a guarantee that the command will execute successfully. As an example, `cat /Users/mbolin/code/codex/README.md` may be considered "safe" if the system has decided the agent is allowed to read anything under `/Users/mbolin/code/codex`, but it will fail at runtime if `README.md` does not exist. (Though this is "safe" in that the agent did not read any files that it was not authorized to read.)
|
||||
|
||||
## Policy
|
||||
|
||||
Currently, the default policy is defined in [`default.policy`](./src/default.policy) within the crate.
|
||||
|
||||
The system uses [Starlark](https://bazel.build/rules/language) as the file format because, unlike something like JSON or YAML, it supports "macros" without compromising on safety or reproducibility. (Under the hood, we use [`starlark-rust`](https://github.com/facebook/starlark-rust) as the specific Starlark implementation.)
|
||||
|
||||
This policy contains "rules" such as:
|
||||
|
||||
```python
|
||||
define_program(
|
||||
program="cp",
|
||||
options=[
|
||||
flag("-r"),
|
||||
flag("-R"),
|
||||
flag("--recursive"),
|
||||
],
|
||||
args=[ARG_RFILES, ARG_WFILE],
|
||||
system_path=["/bin/cp", "/usr/bin/cp"],
|
||||
should_match=[
|
||||
["foo", "bar"],
|
||||
],
|
||||
should_not_match=[
|
||||
["foo"],
|
||||
],
|
||||
)
|
||||
```
|
||||
|
||||
This rule means that:
|
||||
|
||||
- `cp` can be used with any of the following flags (where "flag" means "an option that does not take an argument"): `-r`, `-R`, `--recursive`.
|
||||
- The initial `ARG_RFILES` passed to `args` means that it expects one or more arguments that correspond to "readable files"
|
||||
- The final `ARG_WFILE` passed to `args` means that it expects exactly one argument that corresponds to a "writeable file."
|
||||
- As a means of a lightweight way of including a unit test alongside the definition, the `should_match` list is a list of examples of `execv(3)` args that should match the rule and `should_not_match` is a list of examples that should not match. These examples are verified when the `.policy` file is loaded.
|
||||
|
||||
Note that the language of the `.policy` file is still evolving, as we have to continue to expand it so it is sufficiently expressive to accept all commands we want to consider "safe" without allowing unsafe commands to pass through.
|
||||
|
||||
The integrity of `default.policy` is verified [via unit tests](./tests).
|
||||
|
||||
Further, the CLI supports a `--policy` option to specify a custom `.policy` file for ad-hoc testing.
|
||||
|
||||
## Output Type: `match`
|
||||
|
||||
Going back to the `cp` example, because the rule matches an `ARG_WFILE`, it will return `match` instead of `safe`:
|
||||
|
||||
```shell
|
||||
cargo run -p codex-execpolicy-legacy -- check cp src1 src2 dest | jq
|
||||
```
|
||||
|
||||
If the caller wants to consider allowing this command, it should parse the JSON to pick out the `WriteableFile` arguments and decide whether they are safe to write:
|
||||
|
||||
```json
|
||||
{
|
||||
"result": "match",
|
||||
"match": {
|
||||
"program": "cp",
|
||||
"flags": [],
|
||||
"opts": [],
|
||||
"args": [
|
||||
{
|
||||
"index": 0,
|
||||
"type": "ReadableFile",
|
||||
"value": "src1"
|
||||
},
|
||||
{
|
||||
"index": 1,
|
||||
"type": "ReadableFile",
|
||||
"value": "src2"
|
||||
},
|
||||
{
|
||||
"index": 2,
|
||||
"type": "WriteableFile",
|
||||
"value": "dest"
|
||||
}
|
||||
],
|
||||
"system_path": ["/bin/cp", "/usr/bin/cp"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Note the exit code is still `0` for a `match` unless the `--require-safe` flag is specified, in which case the exit code is `12`.
|
||||
|
||||
## Output Type: `forbidden`
|
||||
|
||||
It is also possible to define a rule that, if it matches a command, should flag it as _forbidden_. For example, we do not want agents to be able to run `applied deploy` _ever_, so we define the following rule:
|
||||
|
||||
```python
|
||||
define_program(
|
||||
program="applied",
|
||||
args=["deploy"],
|
||||
forbidden="Infrastructure Risk: command contains 'applied deploy'",
|
||||
should_match=[
|
||||
["deploy"],
|
||||
],
|
||||
should_not_match=[
|
||||
["lint"],
|
||||
],
|
||||
)
|
||||
```
|
||||
|
||||
Note that for a rule to be forbidden, the `forbidden` keyword arg must be specified as the reason the command is forbidden. This will be included in the output:
|
||||
|
||||
```shell
|
||||
cargo run -p codex-execpolicy-legacy -- check applied deploy | jq
|
||||
```
|
||||
|
||||
```json
|
||||
{
|
||||
"result": "forbidden",
|
||||
"reason": "Infrastructure Risk: command contains 'applied deploy'",
|
||||
"cause": {
|
||||
"Exec": {
|
||||
"exec": {
|
||||
"program": "applied",
|
||||
"flags": [],
|
||||
"opts": [],
|
||||
"args": [
|
||||
{
|
||||
"index": 0,
|
||||
"type": {
|
||||
"Literal": "deploy"
|
||||
},
|
||||
"value": "deploy"
|
||||
}
|
||||
],
|
||||
"system_path": []
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,96 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::arg_matcher::ArgMatcher;
|
||||
use crate::arg_resolver::PositionalArg;
|
||||
use serde_with::DisplayFromStr;
|
||||
use serde_with::serde_as;
|
||||
|
||||
pub type Result<T> = std::result::Result<T, Error>;
|
||||
|
||||
#[serde_as]
|
||||
#[derive(Debug, Eq, PartialEq, Serialize)]
|
||||
#[serde(tag = "type")]
|
||||
pub enum Error {
|
||||
NoSpecForProgram {
|
||||
program: String,
|
||||
},
|
||||
OptionMissingValue {
|
||||
program: String,
|
||||
option: String,
|
||||
},
|
||||
OptionFollowedByOptionInsteadOfValue {
|
||||
program: String,
|
||||
option: String,
|
||||
value: String,
|
||||
},
|
||||
UnknownOption {
|
||||
program: String,
|
||||
option: String,
|
||||
},
|
||||
UnexpectedArguments {
|
||||
program: String,
|
||||
args: Vec<PositionalArg>,
|
||||
},
|
||||
DoubleDashNotSupportedYet {
|
||||
program: String,
|
||||
},
|
||||
MultipleVarargPatterns {
|
||||
program: String,
|
||||
first: ArgMatcher,
|
||||
second: ArgMatcher,
|
||||
},
|
||||
RangeStartExceedsEnd {
|
||||
start: usize,
|
||||
end: usize,
|
||||
},
|
||||
RangeEndOutOfBounds {
|
||||
end: usize,
|
||||
len: usize,
|
||||
},
|
||||
PrefixOverlapsSuffix {},
|
||||
NotEnoughArgs {
|
||||
program: String,
|
||||
args: Vec<PositionalArg>,
|
||||
arg_patterns: Vec<ArgMatcher>,
|
||||
},
|
||||
InternalInvariantViolation {
|
||||
message: String,
|
||||
},
|
||||
VarargMatcherDidNotMatchAnything {
|
||||
program: String,
|
||||
matcher: ArgMatcher,
|
||||
},
|
||||
EmptyFileName {},
|
||||
LiteralValueDidNotMatch {
|
||||
expected: String,
|
||||
actual: String,
|
||||
},
|
||||
InvalidPositiveInteger {
|
||||
value: String,
|
||||
},
|
||||
MissingRequiredOptions {
|
||||
program: String,
|
||||
options: Vec<String>,
|
||||
},
|
||||
SedCommandNotProvablySafe {
|
||||
command: String,
|
||||
},
|
||||
ReadablePathNotInReadableFolders {
|
||||
file: PathBuf,
|
||||
folders: Vec<PathBuf>,
|
||||
},
|
||||
WriteablePathNotInWriteableFolders {
|
||||
file: PathBuf,
|
||||
folders: Vec<PathBuf>,
|
||||
},
|
||||
CannotCheckRelativePath {
|
||||
file: PathBuf,
|
||||
},
|
||||
CannotCanonicalizePath {
|
||||
file: String,
|
||||
#[serde_as(as = "DisplayFromStr")]
|
||||
error: std::io::ErrorKind,
|
||||
},
|
||||
}
|
||||
@@ -0,0 +1,45 @@
|
||||
#![allow(clippy::type_complexity)]
|
||||
#![allow(clippy::too_many_arguments)]
|
||||
#[macro_use]
|
||||
extern crate starlark;
|
||||
|
||||
mod arg_matcher;
|
||||
mod arg_resolver;
|
||||
mod arg_type;
|
||||
mod error;
|
||||
mod exec_call;
|
||||
mod execv_checker;
|
||||
mod opt;
|
||||
mod policy;
|
||||
mod policy_parser;
|
||||
mod program;
|
||||
mod sed_command;
|
||||
mod valid_exec;
|
||||
|
||||
pub use arg_matcher::ArgMatcher;
|
||||
pub use arg_resolver::PositionalArg;
|
||||
pub use arg_type::ArgType;
|
||||
pub use error::Error;
|
||||
pub use error::Result;
|
||||
pub use exec_call::ExecCall;
|
||||
pub use execv_checker::ExecvChecker;
|
||||
pub use opt::Opt;
|
||||
pub use policy::Policy;
|
||||
pub use policy_parser::PolicyParser;
|
||||
pub use program::Forbidden;
|
||||
pub use program::MatchedExec;
|
||||
pub use program::NegativeExamplePassedCheck;
|
||||
pub use program::PositiveExampleFailedCheck;
|
||||
pub use program::ProgramSpec;
|
||||
pub use sed_command::parse_sed_command;
|
||||
pub use valid_exec::MatchedArg;
|
||||
pub use valid_exec::MatchedFlag;
|
||||
pub use valid_exec::MatchedOpt;
|
||||
pub use valid_exec::ValidExec;
|
||||
|
||||
const DEFAULT_POLICY: &str = include_str!("default.policy");
|
||||
|
||||
pub fn get_default_policy() -> starlark::Result<Policy> {
|
||||
let parser = PolicyParser::new("#default", DEFAULT_POLICY);
|
||||
parser.parse()
|
||||
}
|
||||
@@ -0,0 +1,169 @@
|
||||
use anyhow::Result;
|
||||
use clap::Parser;
|
||||
use clap::Subcommand;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::Policy;
|
||||
use codex_execpolicy_legacy::PolicyParser;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::de;
|
||||
use starlark::Error as StarlarkError;
|
||||
use std::path::PathBuf;
|
||||
use std::str::FromStr;
|
||||
|
||||
const MATCHED_BUT_WRITES_FILES_EXIT_CODE: i32 = 12;
|
||||
const MIGHT_BE_SAFE_EXIT_CODE: i32 = 13;
|
||||
const FORBIDDEN_EXIT_CODE: i32 = 14;
|
||||
|
||||
#[derive(Parser, Deserialize, Debug)]
|
||||
#[command(version, about, long_about = None)]
|
||||
pub struct Args {
|
||||
/// If the command fails the policy, exit with 13, but print parseable JSON
|
||||
/// to stdout.
|
||||
#[clap(long)]
|
||||
pub require_safe: bool,
|
||||
|
||||
/// Path to the policy file.
|
||||
#[clap(long, short = 'p')]
|
||||
pub policy: Option<PathBuf>,
|
||||
|
||||
#[command(subcommand)]
|
||||
pub command: Command,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize, Subcommand)]
|
||||
pub enum Command {
|
||||
/// Checks the command as if the arguments were the inputs to execv(3).
|
||||
Check {
|
||||
#[arg(trailing_var_arg = true)]
|
||||
command: Vec<String>,
|
||||
},
|
||||
|
||||
/// Checks the command encoded as a JSON object.
|
||||
#[clap(name = "check-json")]
|
||||
CheckJson {
|
||||
/// JSON object with "program" (str) and "args" (list[str]) fields.
|
||||
#[serde(deserialize_with = "deserialize_from_json")]
|
||||
exec: ExecArg,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize)]
|
||||
pub struct ExecArg {
|
||||
pub program: String,
|
||||
|
||||
#[serde(default)]
|
||||
pub args: Vec<String>,
|
||||
}
|
||||
|
||||
fn main() -> Result<()> {
|
||||
env_logger::init();
|
||||
|
||||
let args = Args::parse();
|
||||
let policy = match args.policy {
|
||||
Some(policy) => {
|
||||
let policy_source = policy.to_string_lossy().to_string();
|
||||
let unparsed_policy = std::fs::read_to_string(policy)?;
|
||||
let parser = PolicyParser::new(&policy_source, &unparsed_policy);
|
||||
parser.parse()
|
||||
}
|
||||
None => get_default_policy(),
|
||||
};
|
||||
let policy = policy.map_err(StarlarkError::into_anyhow)?;
|
||||
|
||||
let exec = match args.command {
|
||||
Command::Check { command } => match command.split_first() {
|
||||
Some((first, rest)) => ExecArg {
|
||||
program: first.to_string(),
|
||||
args: rest.to_vec(),
|
||||
},
|
||||
None => {
|
||||
eprintln!("no command provided");
|
||||
std::process::exit(1);
|
||||
}
|
||||
},
|
||||
Command::CheckJson { exec } => exec,
|
||||
};
|
||||
|
||||
let (output, exit_code) = check_command(&policy, exec, args.require_safe);
|
||||
let json = serde_json::to_string(&output)?;
|
||||
println!("{json}");
|
||||
std::process::exit(exit_code);
|
||||
}
|
||||
|
||||
fn check_command(
|
||||
policy: &Policy,
|
||||
ExecArg { program, args }: ExecArg,
|
||||
check: bool,
|
||||
) -> (Output, i32) {
|
||||
let exec_call = ExecCall { program, args };
|
||||
match policy.check(&exec_call) {
|
||||
Ok(MatchedExec::Match { exec }) => {
|
||||
if exec.might_write_files() {
|
||||
let exit_code = if check {
|
||||
MATCHED_BUT_WRITES_FILES_EXIT_CODE
|
||||
} else {
|
||||
0
|
||||
};
|
||||
(Output::Match { r#match: exec }, exit_code)
|
||||
} else {
|
||||
(Output::Safe { r#match: exec }, 0)
|
||||
}
|
||||
}
|
||||
Ok(MatchedExec::Forbidden { reason, cause }) => {
|
||||
let exit_code = if check { FORBIDDEN_EXIT_CODE } else { 0 };
|
||||
(Output::Forbidden { reason, cause }, exit_code)
|
||||
}
|
||||
Err(err) => {
|
||||
let exit_code = if check { MIGHT_BE_SAFE_EXIT_CODE } else { 0 };
|
||||
(Output::Unverified { error: err }, exit_code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
#[serde(tag = "result")]
|
||||
pub enum Output {
|
||||
/// The command is verified as safe.
|
||||
#[serde(rename = "safe")]
|
||||
Safe { r#match: ValidExec },
|
||||
|
||||
/// The command has matched a rule in the policy, but the caller should
|
||||
/// decide whether it is "safe" given the files it wants to write.
|
||||
#[serde(rename = "match")]
|
||||
Match { r#match: ValidExec },
|
||||
|
||||
/// The user is forbidden from running the command.
|
||||
#[serde(rename = "forbidden")]
|
||||
Forbidden {
|
||||
reason: String,
|
||||
cause: codex_execpolicy_legacy::Forbidden,
|
||||
},
|
||||
|
||||
/// The safety of the command could not be verified.
|
||||
#[serde(rename = "unverified")]
|
||||
Unverified {
|
||||
error: codex_execpolicy_legacy::Error,
|
||||
},
|
||||
}
|
||||
|
||||
fn deserialize_from_json<'de, D>(deserializer: D) -> Result<ExecArg, D::Error>
|
||||
where
|
||||
D: de::Deserializer<'de>,
|
||||
{
|
||||
let s = String::deserialize(deserializer)?;
|
||||
let decoded = serde_json::from_str(&s)
|
||||
.map_err(|e| serde::de::Error::custom(format!("JSON parse error: {e}")))?;
|
||||
Ok(decoded)
|
||||
}
|
||||
|
||||
impl FromStr for ExecArg {
|
||||
type Err = anyhow::Error;
|
||||
|
||||
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
||||
serde_json::from_str(s).map_err(Into::into)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,103 @@
|
||||
use multimap::MultiMap;
|
||||
use regex_lite::Error as RegexError;
|
||||
use regex_lite::Regex;
|
||||
|
||||
use crate::ExecCall;
|
||||
use crate::Forbidden;
|
||||
use crate::MatchedExec;
|
||||
use crate::NegativeExamplePassedCheck;
|
||||
use crate::ProgramSpec;
|
||||
use crate::error::Error;
|
||||
use crate::error::Result;
|
||||
use crate::policy_parser::ForbiddenProgramRegex;
|
||||
use crate::program::PositiveExampleFailedCheck;
|
||||
|
||||
pub struct Policy {
|
||||
programs: MultiMap<String, ProgramSpec>,
|
||||
forbidden_program_regexes: Vec<ForbiddenProgramRegex>,
|
||||
forbidden_substrings_pattern: Option<Regex>,
|
||||
}
|
||||
|
||||
impl Policy {
|
||||
pub fn new(
|
||||
programs: MultiMap<String, ProgramSpec>,
|
||||
forbidden_program_regexes: Vec<ForbiddenProgramRegex>,
|
||||
forbidden_substrings: Vec<String>,
|
||||
) -> std::result::Result<Self, RegexError> {
|
||||
let forbidden_substrings_pattern = if forbidden_substrings.is_empty() {
|
||||
None
|
||||
} else {
|
||||
let escaped_substrings = forbidden_substrings
|
||||
.iter()
|
||||
.map(|s| regex_lite::escape(s))
|
||||
.collect::<Vec<_>>()
|
||||
.join("|");
|
||||
Some(Regex::new(&format!("({escaped_substrings})"))?)
|
||||
};
|
||||
Ok(Self {
|
||||
programs,
|
||||
forbidden_program_regexes,
|
||||
forbidden_substrings_pattern,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn check(&self, exec_call: &ExecCall) -> Result<MatchedExec> {
|
||||
let ExecCall { program, args } = &exec_call;
|
||||
for ForbiddenProgramRegex { regex, reason } in &self.forbidden_program_regexes {
|
||||
if regex.is_match(program) {
|
||||
return Ok(MatchedExec::Forbidden {
|
||||
cause: Forbidden::Program {
|
||||
program: program.clone(),
|
||||
exec_call: exec_call.clone(),
|
||||
},
|
||||
reason: reason.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for arg in args {
|
||||
if let Some(regex) = &self.forbidden_substrings_pattern
|
||||
&& regex.is_match(arg)
|
||||
{
|
||||
return Ok(MatchedExec::Forbidden {
|
||||
cause: Forbidden::Arg {
|
||||
arg: arg.clone(),
|
||||
exec_call: exec_call.clone(),
|
||||
},
|
||||
reason: format!("arg `{arg}` contains forbidden substring"),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let mut last_err = Err(Error::NoSpecForProgram {
|
||||
program: program.clone(),
|
||||
});
|
||||
if let Some(spec_list) = self.programs.get_vec(program) {
|
||||
for spec in spec_list {
|
||||
match spec.check(exec_call) {
|
||||
Ok(matched_exec) => return Ok(matched_exec),
|
||||
Err(err) => {
|
||||
last_err = Err(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
last_err
|
||||
}
|
||||
|
||||
pub fn check_each_good_list_individually(&self) -> Vec<PositiveExampleFailedCheck> {
|
||||
let mut violations = Vec::new();
|
||||
for (_program, spec) in self.programs.flat_iter() {
|
||||
violations.extend(spec.verify_should_match_list());
|
||||
}
|
||||
violations
|
||||
}
|
||||
|
||||
pub fn check_each_bad_list_individually(&self) -> Vec<NegativeExamplePassedCheck> {
|
||||
let mut violations = Vec::new();
|
||||
for (_program, spec) in self.programs.flat_iter() {
|
||||
violations.extend(spec.verify_should_not_match_list());
|
||||
}
|
||||
violations
|
||||
}
|
||||
}
|
||||
+2
-2
@@ -1,5 +1,5 @@
|
||||
use codex_execpolicy::NegativeExamplePassedCheck;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::NegativeExamplePassedCheck;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
#[test]
|
||||
fn verify_everything_in_bad_list_is_rejected() {
|
||||
+11
-11
@@ -1,15 +1,15 @@
|
||||
extern crate codex_execpolicy;
|
||||
extern crate codex_execpolicy_legacy;
|
||||
|
||||
use codex_execpolicy::ArgMatcher;
|
||||
use codex_execpolicy::ArgType;
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedArg;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::Result;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::ArgMatcher;
|
||||
use codex_execpolicy_legacy::ArgType;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedArg;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::Policy;
|
||||
use codex_execpolicy_legacy::Result;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn setup() -> Policy {
|
||||
+2
-2
@@ -1,5 +1,5 @@
|
||||
use codex_execpolicy::PositiveExampleFailedCheck;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::PositiveExampleFailedCheck;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
#[test]
|
||||
fn verify_everything_in_good_list_is_allowed() {
|
||||
+12
-12
@@ -1,16 +1,16 @@
|
||||
use codex_execpolicy::ArgMatcher;
|
||||
use codex_execpolicy::ArgType;
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedArg;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::MatchedOpt;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::Result;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::ArgMatcher;
|
||||
use codex_execpolicy_legacy::ArgType;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedArg;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::MatchedOpt;
|
||||
use codex_execpolicy_legacy::Policy;
|
||||
use codex_execpolicy_legacy::Result;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
extern crate codex_execpolicy;
|
||||
extern crate codex_execpolicy_legacy;
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn setup() -> Policy {
|
||||
+9
-9
@@ -1,13 +1,13 @@
|
||||
use codex_execpolicy::ArgType;
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedArg;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
use codex_execpolicy::Result;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy_legacy::ArgType;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedArg;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::PolicyParser;
|
||||
use codex_execpolicy_legacy::Result;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
|
||||
extern crate codex_execpolicy;
|
||||
extern crate codex_execpolicy_legacy;
|
||||
|
||||
#[test]
|
||||
fn test_invalid_subcommand() -> Result<()> {
|
||||
+11
-11
@@ -1,15 +1,15 @@
|
||||
extern crate codex_execpolicy;
|
||||
extern crate codex_execpolicy_legacy;
|
||||
|
||||
use codex_execpolicy::ArgType;
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedArg;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::MatchedFlag;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::Result;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::ArgType;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedArg;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::MatchedFlag;
|
||||
use codex_execpolicy_legacy::Policy;
|
||||
use codex_execpolicy_legacy::Result;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn setup() -> Policy {
|
||||
+2
-2
@@ -1,5 +1,5 @@
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::parse_sed_command;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::parse_sed_command;
|
||||
|
||||
#[test]
|
||||
fn parses_simple_print_command() {
|
||||
+9
-9
@@ -1,15 +1,15 @@
|
||||
extern crate codex_execpolicy;
|
||||
extern crate codex_execpolicy_legacy;
|
||||
|
||||
use std::vec;
|
||||
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::MatchedFlag;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::PositionalArg;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::MatchedFlag;
|
||||
use codex_execpolicy_legacy::Policy;
|
||||
use codex_execpolicy_legacy::PositionalArg;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn setup() -> Policy {
|
||||
+12
-12
@@ -1,16 +1,16 @@
|
||||
extern crate codex_execpolicy;
|
||||
extern crate codex_execpolicy_legacy;
|
||||
|
||||
use codex_execpolicy::ArgType;
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedArg;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::MatchedFlag;
|
||||
use codex_execpolicy::MatchedOpt;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::Result;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use codex_execpolicy_legacy::ArgType;
|
||||
use codex_execpolicy_legacy::Error;
|
||||
use codex_execpolicy_legacy::ExecCall;
|
||||
use codex_execpolicy_legacy::MatchedArg;
|
||||
use codex_execpolicy_legacy::MatchedExec;
|
||||
use codex_execpolicy_legacy::MatchedFlag;
|
||||
use codex_execpolicy_legacy::MatchedOpt;
|
||||
use codex_execpolicy_legacy::Policy;
|
||||
use codex_execpolicy_legacy::Result;
|
||||
use codex_execpolicy_legacy::ValidExec;
|
||||
use codex_execpolicy_legacy::get_default_policy;
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn setup() -> Policy {
|
||||
@@ -1,33 +1,29 @@
|
||||
[package]
|
||||
edition = "2024"
|
||||
name = "codex-execpolicy"
|
||||
version = { workspace = true }
|
||||
|
||||
[[bin]]
|
||||
name = "codex-execpolicy"
|
||||
path = "src/main.rs"
|
||||
edition = "2024"
|
||||
description = "Codex exec policy: prefix-based Starlark rules for command decisions."
|
||||
|
||||
[lib]
|
||||
name = "codex_execpolicy"
|
||||
path = "src/lib.rs"
|
||||
|
||||
[[bin]]
|
||||
name = "codex-execpolicy"
|
||||
path = "src/main.rs"
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
allocative = { workspace = true }
|
||||
anyhow = { workspace = true }
|
||||
clap = { workspace = true, features = ["derive"] }
|
||||
derive_more = { workspace = true, features = ["display"] }
|
||||
env_logger = { workspace = true }
|
||||
log = { workspace = true }
|
||||
multimap = { workspace = true }
|
||||
path-absolutize = { workspace = true }
|
||||
regex-lite = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_with = { workspace = true, features = ["macros"] }
|
||||
shlex = { workspace = true }
|
||||
starlark = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
tempfile = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
+42
-162
@@ -1,180 +1,60 @@
|
||||
# codex_execpolicy
|
||||
# codex-execpolicy
|
||||
|
||||
The goal of this library is to classify a proposed [`execv(3)`](https://linux.die.net/man/3/execv) command into one of the following states:
|
||||
## Overview
|
||||
- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, match?, not_match?)`.
|
||||
- This release covers the prefix-rule subset of the execpolicy language; a richer language will follow.
|
||||
- Tokens are matched in order; any `pattern` element may be a list to denote alternatives. `decision` defaults to `allow`; valid values: `allow`, `prompt`, `forbidden`.
|
||||
- `match` / `not_match` supply example invocations that are validated at load time (think of them as unit tests); examples can be token arrays or strings (strings are tokenized with `shlex`).
|
||||
- The CLI always prints the JSON serialization of the evaluation result.
|
||||
- The legacy rule matcher lives in `codex-execpolicy-legacy`.
|
||||
|
||||
- `safe` The command is safe to run (\*).
|
||||
- `match` The command matched a rule in the policy, but the caller should decide whether it is safe to run based on the files it will write.
|
||||
- `forbidden` The command is not allowed to be run.
|
||||
- `unverified` The safety cannot be determined: make the user decide.
|
||||
|
||||
(\*) Whether an `execv(3)` call should be considered "safe" often requires additional context beyond the arguments to `execv()` itself. For example, if you trust an autonomous software agent to write files in your source tree, then deciding whether `/bin/cp foo bar` is "safe" depends on `getcwd(3)` for the calling process as well as the `realpath` of `foo` and `bar` when resolved against `getcwd()`.
|
||||
To that end, rather than returning a boolean, the validator returns a structured result that the client is expected to use to determine the "safety" of the proposed `execv()` call.
|
||||
|
||||
For example, to check the command `ls -l foo`, the checker would be invoked as follows:
|
||||
|
||||
```shell
|
||||
cargo run -- check ls -l foo | jq
|
||||
```
|
||||
|
||||
It will exit with `0` and print the following to stdout:
|
||||
|
||||
```json
|
||||
{
|
||||
"result": "safe",
|
||||
"match": {
|
||||
"program": "ls",
|
||||
"flags": [
|
||||
{
|
||||
"name": "-l"
|
||||
}
|
||||
],
|
||||
"opts": [],
|
||||
"args": [
|
||||
{
|
||||
"index": 1,
|
||||
"type": "ReadableFile",
|
||||
"value": "foo"
|
||||
}
|
||||
],
|
||||
"system_path": ["/bin/ls", "/usr/bin/ls"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Of note:
|
||||
|
||||
- `foo` is tagged as a `ReadableFile`, so the caller should resolve `foo` relative to `getcwd()` and `realpath` it (as it may be a symlink) to determine whether `foo` is safe to read.
|
||||
- While the specified executable is `ls`, `"system_path"` offers `/bin/ls` and `/usr/bin/ls` as viable alternatives to avoid using whatever `ls` happens to appear first on the user's `$PATH`. If either exists on the host, it is recommended to use it as the first argument to `execv(3)` instead of `ls`.
|
||||
|
||||
Further, "safety" in this system is not a guarantee that the command will execute successfully. As an example, `cat /Users/mbolin/code/codex/README.md` may be considered "safe" if the system has decided the agent is allowed to read anything under `/Users/mbolin/code/codex`, but it will fail at runtime if `README.md` does not exist. (Though this is "safe" in that the agent did not read any files that it was not authorized to read.)
|
||||
|
||||
## Policy
|
||||
|
||||
Currently, the default policy is defined in [`default.policy`](./src/default.policy) within the crate.
|
||||
|
||||
The system uses [Starlark](https://bazel.build/rules/language) as the file format because, unlike something like JSON or YAML, it supports "macros" without compromising on safety or reproducibility. (Under the hood, we use [`starlark-rust`](https://github.com/facebook/starlark-rust) as the specific Starlark implementation.)
|
||||
|
||||
This policy contains "rules" such as:
|
||||
|
||||
```python
|
||||
define_program(
|
||||
program="cp",
|
||||
options=[
|
||||
flag("-r"),
|
||||
flag("-R"),
|
||||
flag("--recursive"),
|
||||
],
|
||||
args=[ARG_RFILES, ARG_WFILE],
|
||||
system_path=["/bin/cp", "/usr/bin/cp"],
|
||||
should_match=[
|
||||
["foo", "bar"],
|
||||
],
|
||||
should_not_match=[
|
||||
["foo"],
|
||||
],
|
||||
## Policy shapes
|
||||
- Prefix rules use Starlark syntax:
|
||||
```starlark
|
||||
prefix_rule(
|
||||
pattern = ["cmd", ["alt1", "alt2"]], # ordered tokens; list entries denote alternatives
|
||||
decision = "prompt", # allow | prompt | forbidden; defaults to allow
|
||||
match = [["cmd", "alt1"], "cmd alt2"], # examples that must match this rule
|
||||
not_match = [["cmd", "oops"], "cmd alt3"], # examples that must not match this rule
|
||||
)
|
||||
```
|
||||
|
||||
This rule means that:
|
||||
|
||||
- `cp` can be used with any of the following flags (where "flag" means "an option that does not take an argument"): `-r`, `-R`, `--recursive`.
|
||||
- The initial `ARG_RFILES` passed to `args` means that it expects one or more arguments that correspond to "readable files"
|
||||
- The final `ARG_WFILE` passed to `args` means that it expects exactly one argument that corresponds to a "writeable file."
|
||||
- As a means of a lightweight way of including a unit test alongside the definition, the `should_match` list is a list of examples of `execv(3)` args that should match the rule and `should_not_match` is a list of examples that should not match. These examples are verified when the `.policy` file is loaded.
|
||||
|
||||
Note that the language of the `.policy` file is still evolving, as we have to continue to expand it so it is sufficiently expressive to accept all commands we want to consider "safe" without allowing unsafe commands to pass through.
|
||||
|
||||
The integrity of `default.policy` is verified [via unit tests](./tests).
|
||||
|
||||
Further, the CLI supports a `--policy` option to specify a custom `.policy` file for ad-hoc testing.
|
||||
|
||||
## Output Type: `match`
|
||||
|
||||
Going back to the `cp` example, because the rule matches an `ARG_WFILE`, it will return `match` instead of `safe`:
|
||||
|
||||
```shell
|
||||
cargo run -- check cp src1 src2 dest | jq
|
||||
## CLI
|
||||
- Provide one or more policy files (for example `src/default.codexpolicy`) to check a command:
|
||||
```bash
|
||||
cargo run -p codex-execpolicy -- check --policy path/to/policy.codexpolicy git status
|
||||
```
|
||||
- Pass multiple `--policy` flags to merge rules, evaluated in the order provided:
|
||||
```bash
|
||||
cargo run -p codex-execpolicy -- check --policy base.codexpolicy --policy overrides.codexpolicy git status
|
||||
```
|
||||
- Output is JSON by default; pass `--pretty` for pretty-printed JSON
|
||||
- Example outcomes:
|
||||
- Match: `{"match": { ... "decision": "allow" ... }}`
|
||||
- No match: `"noMatch"`
|
||||
|
||||
If the caller wants to consider allowing this command, it should parse the JSON to pick out the `WriteableFile` arguments and decide whether they are safe to write:
|
||||
|
||||
## Response shapes
|
||||
- Match:
|
||||
```json
|
||||
{
|
||||
"result": "match",
|
||||
"match": {
|
||||
"program": "cp",
|
||||
"flags": [],
|
||||
"opts": [],
|
||||
"args": [
|
||||
"decision": "allow|prompt|forbidden",
|
||||
"matchedRules": [
|
||||
{
|
||||
"index": 0,
|
||||
"type": "ReadableFile",
|
||||
"value": "src1"
|
||||
},
|
||||
{
|
||||
"index": 1,
|
||||
"type": "ReadableFile",
|
||||
"value": "src2"
|
||||
},
|
||||
{
|
||||
"index": 2,
|
||||
"type": "WriteableFile",
|
||||
"value": "dest"
|
||||
"prefixRuleMatch": {
|
||||
"matchedPrefix": ["<token>", "..."],
|
||||
"decision": "allow|prompt|forbidden"
|
||||
}
|
||||
}
|
||||
],
|
||||
"system_path": ["/bin/cp", "/usr/bin/cp"]
|
||||
]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Note the exit code is still `0` for a `match` unless the `--require-safe` flag is specified, in which case the exit code is `12`.
|
||||
|
||||
## Output Type: `forbidden`
|
||||
|
||||
It is also possible to define a rule that, if it matches a command, should flag it as _forbidden_. For example, we do not want agents to be able to run `applied deploy` _ever_, so we define the following rule:
|
||||
|
||||
```python
|
||||
define_program(
|
||||
program="applied",
|
||||
args=["deploy"],
|
||||
forbidden="Infrastructure Risk: command contains 'applied deploy'",
|
||||
should_match=[
|
||||
["deploy"],
|
||||
],
|
||||
should_not_match=[
|
||||
["lint"],
|
||||
],
|
||||
)
|
||||
```
|
||||
|
||||
Note that for a rule to be forbidden, the `forbidden` keyword arg must be specified as the reason the command is forbidden. This will be included in the output:
|
||||
|
||||
```shell
|
||||
cargo run -- check applied deploy | jq
|
||||
```
|
||||
|
||||
- No match:
|
||||
```json
|
||||
{
|
||||
"result": "forbidden",
|
||||
"reason": "Infrastructure Risk: command contains 'applied deploy'",
|
||||
"cause": {
|
||||
"Exec": {
|
||||
"exec": {
|
||||
"program": "applied",
|
||||
"flags": [],
|
||||
"opts": [],
|
||||
"args": [
|
||||
{
|
||||
"index": 0,
|
||||
"type": {
|
||||
"Literal": "deploy"
|
||||
},
|
||||
"value": "deploy"
|
||||
}
|
||||
],
|
||||
"system_path": []
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
"noMatch"
|
||||
```
|
||||
|
||||
- `matchedRules` lists every rule whose prefix matched the command; `matchedPrefix` is the exact prefix that matched.
|
||||
- The effective `decision` is the strictest severity across all matches (`forbidden` > `prompt` > `allow`).
|
||||
|
||||
@@ -1,96 +1,26 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::arg_matcher::ArgMatcher;
|
||||
use crate::arg_resolver::PositionalArg;
|
||||
use serde_with::DisplayFromStr;
|
||||
use serde_with::serde_as;
|
||||
use starlark::Error as StarlarkError;
|
||||
use thiserror::Error;
|
||||
|
||||
pub type Result<T> = std::result::Result<T, Error>;
|
||||
|
||||
#[serde_as]
|
||||
#[derive(Debug, Eq, PartialEq, Serialize)]
|
||||
#[serde(tag = "type")]
|
||||
#[derive(Debug, Error)]
|
||||
pub enum Error {
|
||||
NoSpecForProgram {
|
||||
program: String,
|
||||
},
|
||||
OptionMissingValue {
|
||||
program: String,
|
||||
option: String,
|
||||
},
|
||||
OptionFollowedByOptionInsteadOfValue {
|
||||
program: String,
|
||||
option: String,
|
||||
value: String,
|
||||
},
|
||||
UnknownOption {
|
||||
program: String,
|
||||
option: String,
|
||||
},
|
||||
UnexpectedArguments {
|
||||
program: String,
|
||||
args: Vec<PositionalArg>,
|
||||
},
|
||||
DoubleDashNotSupportedYet {
|
||||
program: String,
|
||||
},
|
||||
MultipleVarargPatterns {
|
||||
program: String,
|
||||
first: ArgMatcher,
|
||||
second: ArgMatcher,
|
||||
},
|
||||
RangeStartExceedsEnd {
|
||||
start: usize,
|
||||
end: usize,
|
||||
},
|
||||
RangeEndOutOfBounds {
|
||||
end: usize,
|
||||
len: usize,
|
||||
},
|
||||
PrefixOverlapsSuffix {},
|
||||
NotEnoughArgs {
|
||||
program: String,
|
||||
args: Vec<PositionalArg>,
|
||||
arg_patterns: Vec<ArgMatcher>,
|
||||
},
|
||||
InternalInvariantViolation {
|
||||
message: String,
|
||||
},
|
||||
VarargMatcherDidNotMatchAnything {
|
||||
program: String,
|
||||
matcher: ArgMatcher,
|
||||
},
|
||||
EmptyFileName {},
|
||||
LiteralValueDidNotMatch {
|
||||
expected: String,
|
||||
actual: String,
|
||||
},
|
||||
InvalidPositiveInteger {
|
||||
value: String,
|
||||
},
|
||||
MissingRequiredOptions {
|
||||
program: String,
|
||||
options: Vec<String>,
|
||||
},
|
||||
SedCommandNotProvablySafe {
|
||||
command: String,
|
||||
},
|
||||
ReadablePathNotInReadableFolders {
|
||||
file: PathBuf,
|
||||
folders: Vec<PathBuf>,
|
||||
},
|
||||
WriteablePathNotInWriteableFolders {
|
||||
file: PathBuf,
|
||||
folders: Vec<PathBuf>,
|
||||
},
|
||||
CannotCheckRelativePath {
|
||||
file: PathBuf,
|
||||
},
|
||||
CannotCanonicalizePath {
|
||||
file: String,
|
||||
#[serde_as(as = "DisplayFromStr")]
|
||||
error: std::io::ErrorKind,
|
||||
#[error("invalid decision: {0}")]
|
||||
InvalidDecision(String),
|
||||
#[error("invalid pattern element: {0}")]
|
||||
InvalidPattern(String),
|
||||
#[error("invalid example: {0}")]
|
||||
InvalidExample(String),
|
||||
#[error(
|
||||
"expected every example to match at least one rule. rules: {rules:?}; unmatched examples: \
|
||||
{examples:?}"
|
||||
)]
|
||||
ExampleDidNotMatch {
|
||||
rules: Vec<String>,
|
||||
examples: Vec<String>,
|
||||
},
|
||||
#[error("expected example to not match rule `{rule}`: {example}")]
|
||||
ExampleDidMatch { rule: String, example: String },
|
||||
#[error("starlark error: {0}")]
|
||||
Starlark(StarlarkError),
|
||||
}
|
||||
|
||||
@@ -1,45 +1,15 @@
|
||||
#![allow(clippy::type_complexity)]
|
||||
#![allow(clippy::too_many_arguments)]
|
||||
#[macro_use]
|
||||
extern crate starlark;
|
||||
pub mod decision;
|
||||
pub mod error;
|
||||
pub mod parser;
|
||||
pub mod policy;
|
||||
pub mod rule;
|
||||
|
||||
mod arg_matcher;
|
||||
mod arg_resolver;
|
||||
mod arg_type;
|
||||
mod error;
|
||||
mod exec_call;
|
||||
mod execv_checker;
|
||||
mod opt;
|
||||
mod policy;
|
||||
mod policy_parser;
|
||||
mod program;
|
||||
mod sed_command;
|
||||
mod valid_exec;
|
||||
|
||||
pub use arg_matcher::ArgMatcher;
|
||||
pub use arg_resolver::PositionalArg;
|
||||
pub use arg_type::ArgType;
|
||||
pub use decision::Decision;
|
||||
pub use error::Error;
|
||||
pub use error::Result;
|
||||
pub use exec_call::ExecCall;
|
||||
pub use execv_checker::ExecvChecker;
|
||||
pub use opt::Opt;
|
||||
pub use parser::PolicyParser;
|
||||
pub use policy::Evaluation;
|
||||
pub use policy::Policy;
|
||||
pub use policy_parser::PolicyParser;
|
||||
pub use program::Forbidden;
|
||||
pub use program::MatchedExec;
|
||||
pub use program::NegativeExamplePassedCheck;
|
||||
pub use program::PositiveExampleFailedCheck;
|
||||
pub use program::ProgramSpec;
|
||||
pub use sed_command::parse_sed_command;
|
||||
pub use valid_exec::MatchedArg;
|
||||
pub use valid_exec::MatchedFlag;
|
||||
pub use valid_exec::MatchedOpt;
|
||||
pub use valid_exec::ValidExec;
|
||||
|
||||
const DEFAULT_POLICY: &str = include_str!("default.policy");
|
||||
|
||||
pub fn get_default_policy() -> starlark::Result<Policy> {
|
||||
let parser = PolicyParser::new("#default", DEFAULT_POLICY);
|
||||
parser.parse()
|
||||
}
|
||||
pub use rule::Rule;
|
||||
pub use rule::RuleMatch;
|
||||
pub use rule::RuleRef;
|
||||
|
||||
+49
-150
@@ -1,167 +1,66 @@
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use clap::Parser;
|
||||
use clap::Subcommand;
|
||||
use codex_execpolicy::ExecCall;
|
||||
use codex_execpolicy::MatchedExec;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
use codex_execpolicy::ValidExec;
|
||||
use codex_execpolicy::get_default_policy;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::de;
|
||||
use starlark::Error as StarlarkError;
|
||||
use std::path::PathBuf;
|
||||
use std::str::FromStr;
|
||||
|
||||
const MATCHED_BUT_WRITES_FILES_EXIT_CODE: i32 = 12;
|
||||
const MIGHT_BE_SAFE_EXIT_CODE: i32 = 13;
|
||||
const FORBIDDEN_EXIT_CODE: i32 = 14;
|
||||
|
||||
#[derive(Parser, Deserialize, Debug)]
|
||||
#[command(version, about, long_about = None)]
|
||||
pub struct Args {
|
||||
/// If the command fails the policy, exit with 13, but print parseable JSON
|
||||
/// to stdout.
|
||||
#[clap(long)]
|
||||
pub require_safe: bool,
|
||||
|
||||
/// Path to the policy file.
|
||||
#[clap(long, short = 'p')]
|
||||
pub policy: Option<PathBuf>,
|
||||
|
||||
#[command(subcommand)]
|
||||
pub command: Command,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize, Subcommand)]
|
||||
pub enum Command {
|
||||
/// Checks the command as if the arguments were the inputs to execv(3).
|
||||
/// CLI for evaluating exec policies
|
||||
#[derive(Parser)]
|
||||
#[command(name = "codex-execpolicy")]
|
||||
enum Cli {
|
||||
/// Evaluate a command against a policy.
|
||||
Check {
|
||||
#[arg(trailing_var_arg = true)]
|
||||
#[arg(short, long = "policy", value_name = "PATH", required = true)]
|
||||
policies: Vec<PathBuf>,
|
||||
|
||||
/// Pretty-print the JSON output.
|
||||
#[arg(long)]
|
||||
pretty: bool,
|
||||
|
||||
/// Command tokens to check.
|
||||
#[arg(
|
||||
value_name = "COMMAND",
|
||||
required = true,
|
||||
trailing_var_arg = true,
|
||||
allow_hyphen_values = true
|
||||
)]
|
||||
command: Vec<String>,
|
||||
},
|
||||
|
||||
/// Checks the command encoded as a JSON object.
|
||||
#[clap(name = "check-json")]
|
||||
CheckJson {
|
||||
/// JSON object with "program" (str) and "args" (list[str]) fields.
|
||||
#[serde(deserialize_with = "deserialize_from_json")]
|
||||
exec: ExecArg,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize)]
|
||||
pub struct ExecArg {
|
||||
pub program: String,
|
||||
|
||||
#[serde(default)]
|
||||
pub args: Vec<String>,
|
||||
}
|
||||
|
||||
fn main() -> Result<()> {
|
||||
env_logger::init();
|
||||
let cli = Cli::parse();
|
||||
match cli {
|
||||
Cli::Check {
|
||||
policies,
|
||||
command,
|
||||
pretty,
|
||||
} => cmd_check(policies, command, pretty),
|
||||
}
|
||||
}
|
||||
|
||||
let args = Args::parse();
|
||||
let policy = match args.policy {
|
||||
Some(policy) => {
|
||||
let policy_source = policy.to_string_lossy().to_string();
|
||||
let unparsed_policy = std::fs::read_to_string(policy)?;
|
||||
let parser = PolicyParser::new(&policy_source, &unparsed_policy);
|
||||
parser.parse()
|
||||
}
|
||||
None => get_default_policy(),
|
||||
fn cmd_check(policy_paths: Vec<PathBuf>, args: Vec<String>, pretty: bool) -> Result<()> {
|
||||
let policy = load_policies(&policy_paths)?;
|
||||
|
||||
let eval = policy.check(&args);
|
||||
let json = if pretty {
|
||||
serde_json::to_string_pretty(&eval)?
|
||||
} else {
|
||||
serde_json::to_string(&eval)?
|
||||
};
|
||||
let policy = policy.map_err(StarlarkError::into_anyhow)?;
|
||||
|
||||
let exec = match args.command {
|
||||
Command::Check { command } => match command.split_first() {
|
||||
Some((first, rest)) => ExecArg {
|
||||
program: first.to_string(),
|
||||
args: rest.to_vec(),
|
||||
},
|
||||
None => {
|
||||
eprintln!("no command provided");
|
||||
std::process::exit(1);
|
||||
}
|
||||
},
|
||||
Command::CheckJson { exec } => exec,
|
||||
};
|
||||
|
||||
let (output, exit_code) = check_command(&policy, exec, args.require_safe);
|
||||
let json = serde_json::to_string(&output)?;
|
||||
println!("{json}");
|
||||
std::process::exit(exit_code);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn check_command(
|
||||
policy: &Policy,
|
||||
ExecArg { program, args }: ExecArg,
|
||||
check: bool,
|
||||
) -> (Output, i32) {
|
||||
let exec_call = ExecCall { program, args };
|
||||
match policy.check(&exec_call) {
|
||||
Ok(MatchedExec::Match { exec }) => {
|
||||
if exec.might_write_files() {
|
||||
let exit_code = if check {
|
||||
MATCHED_BUT_WRITES_FILES_EXIT_CODE
|
||||
} else {
|
||||
0
|
||||
};
|
||||
(Output::Match { r#match: exec }, exit_code)
|
||||
} else {
|
||||
(Output::Safe { r#match: exec }, 0)
|
||||
}
|
||||
}
|
||||
Ok(MatchedExec::Forbidden { reason, cause }) => {
|
||||
let exit_code = if check { FORBIDDEN_EXIT_CODE } else { 0 };
|
||||
(Output::Forbidden { reason, cause }, exit_code)
|
||||
}
|
||||
Err(err) => {
|
||||
let exit_code = if check { MIGHT_BE_SAFE_EXIT_CODE } else { 0 };
|
||||
(Output::Unverified { error: err }, exit_code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
#[serde(tag = "result")]
|
||||
pub enum Output {
|
||||
/// The command is verified as safe.
|
||||
#[serde(rename = "safe")]
|
||||
Safe { r#match: ValidExec },
|
||||
|
||||
/// The command has matched a rule in the policy, but the caller should
|
||||
/// decide whether it is "safe" given the files it wants to write.
|
||||
#[serde(rename = "match")]
|
||||
Match { r#match: ValidExec },
|
||||
|
||||
/// The user is forbidden from running the command.
|
||||
#[serde(rename = "forbidden")]
|
||||
Forbidden {
|
||||
reason: String,
|
||||
cause: codex_execpolicy::Forbidden,
|
||||
},
|
||||
|
||||
/// The safety of the command could not be verified.
|
||||
#[serde(rename = "unverified")]
|
||||
Unverified { error: codex_execpolicy::Error },
|
||||
}
|
||||
|
||||
fn deserialize_from_json<'de, D>(deserializer: D) -> Result<ExecArg, D::Error>
|
||||
where
|
||||
D: de::Deserializer<'de>,
|
||||
{
|
||||
let s = String::deserialize(deserializer)?;
|
||||
let decoded = serde_json::from_str(&s)
|
||||
.map_err(|e| serde::de::Error::custom(format!("JSON parse error: {e}")))?;
|
||||
Ok(decoded)
|
||||
}
|
||||
|
||||
impl FromStr for ExecArg {
|
||||
type Err = anyhow::Error;
|
||||
|
||||
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
||||
serde_json::from_str(s).map_err(Into::into)
|
||||
fn load_policies(policy_paths: &[PathBuf]) -> Result<codex_execpolicy::Policy> {
|
||||
let mut parser = PolicyParser::new();
|
||||
for policy_path in policy_paths {
|
||||
let policy_file_contents = fs::read_to_string(policy_path)
|
||||
.with_context(|| format!("failed to read policy at {}", policy_path.display()))?;
|
||||
let policy_identifier = policy_path.to_string_lossy().to_string();
|
||||
parser.parse(&policy_identifier, &policy_file_contents)?;
|
||||
}
|
||||
Ok(parser.build())
|
||||
}
|
||||
|
||||
@@ -1,103 +1,84 @@
|
||||
use crate::decision::Decision;
|
||||
use crate::rule::RuleMatch;
|
||||
use crate::rule::RuleRef;
|
||||
use multimap::MultiMap;
|
||||
use regex_lite::Error as RegexError;
|
||||
use regex_lite::Regex;
|
||||
|
||||
use crate::ExecCall;
|
||||
use crate::Forbidden;
|
||||
use crate::MatchedExec;
|
||||
use crate::NegativeExamplePassedCheck;
|
||||
use crate::ProgramSpec;
|
||||
use crate::error::Error;
|
||||
use crate::error::Result;
|
||||
use crate::policy_parser::ForbiddenProgramRegex;
|
||||
use crate::program::PositiveExampleFailedCheck;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct Policy {
|
||||
programs: MultiMap<String, ProgramSpec>,
|
||||
forbidden_program_regexes: Vec<ForbiddenProgramRegex>,
|
||||
forbidden_substrings_pattern: Option<Regex>,
|
||||
rules_by_program: MultiMap<String, RuleRef>,
|
||||
}
|
||||
|
||||
impl Policy {
|
||||
pub fn new(
|
||||
programs: MultiMap<String, ProgramSpec>,
|
||||
forbidden_program_regexes: Vec<ForbiddenProgramRegex>,
|
||||
forbidden_substrings: Vec<String>,
|
||||
) -> std::result::Result<Self, RegexError> {
|
||||
let forbidden_substrings_pattern = if forbidden_substrings.is_empty() {
|
||||
None
|
||||
} else {
|
||||
let escaped_substrings = forbidden_substrings
|
||||
.iter()
|
||||
.map(|s| regex_lite::escape(s))
|
||||
.collect::<Vec<_>>()
|
||||
.join("|");
|
||||
Some(Regex::new(&format!("({escaped_substrings})"))?)
|
||||
pub fn new(rules_by_program: MultiMap<String, RuleRef>) -> Self {
|
||||
Self { rules_by_program }
|
||||
}
|
||||
|
||||
pub fn empty() -> Self {
|
||||
Self::new(MultiMap::new())
|
||||
}
|
||||
|
||||
pub fn rules(&self) -> &MultiMap<String, RuleRef> {
|
||||
&self.rules_by_program
|
||||
}
|
||||
|
||||
pub fn check(&self, cmd: &[String]) -> Evaluation {
|
||||
let rules = match cmd.first() {
|
||||
Some(first) => match self.rules_by_program.get_vec(first) {
|
||||
Some(rules) => rules,
|
||||
None => return Evaluation::NoMatch,
|
||||
},
|
||||
None => return Evaluation::NoMatch,
|
||||
};
|
||||
Ok(Self {
|
||||
programs,
|
||||
forbidden_program_regexes,
|
||||
forbidden_substrings_pattern,
|
||||
})
|
||||
|
||||
let matched_rules: Vec<RuleMatch> =
|
||||
rules.iter().filter_map(|rule| rule.matches(cmd)).collect();
|
||||
match matched_rules.iter().map(RuleMatch::decision).max() {
|
||||
Some(decision) => Evaluation::Match {
|
||||
decision,
|
||||
matched_rules,
|
||||
},
|
||||
None => Evaluation::NoMatch,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn check(&self, exec_call: &ExecCall) -> Result<MatchedExec> {
|
||||
let ExecCall { program, args } = &exec_call;
|
||||
for ForbiddenProgramRegex { regex, reason } in &self.forbidden_program_regexes {
|
||||
if regex.is_match(program) {
|
||||
return Ok(MatchedExec::Forbidden {
|
||||
cause: Forbidden::Program {
|
||||
program: program.clone(),
|
||||
exec_call: exec_call.clone(),
|
||||
},
|
||||
reason: reason.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
pub fn check_multiple<Commands>(&self, commands: Commands) -> Evaluation
|
||||
where
|
||||
Commands: IntoIterator,
|
||||
Commands::Item: AsRef<[String]>,
|
||||
{
|
||||
let matched_rules: Vec<RuleMatch> = commands
|
||||
.into_iter()
|
||||
.flat_map(|command| match self.check(command.as_ref()) {
|
||||
Evaluation::Match { matched_rules, .. } => matched_rules,
|
||||
Evaluation::NoMatch => Vec::new(),
|
||||
})
|
||||
.collect();
|
||||
|
||||
for arg in args {
|
||||
if let Some(regex) = &self.forbidden_substrings_pattern
|
||||
&& regex.is_match(arg)
|
||||
{
|
||||
return Ok(MatchedExec::Forbidden {
|
||||
cause: Forbidden::Arg {
|
||||
arg: arg.clone(),
|
||||
exec_call: exec_call.clone(),
|
||||
},
|
||||
reason: format!("arg `{arg}` contains forbidden substring"),
|
||||
});
|
||||
}
|
||||
match matched_rules.iter().map(RuleMatch::decision).max() {
|
||||
Some(decision) => Evaluation::Match {
|
||||
decision,
|
||||
matched_rules,
|
||||
},
|
||||
None => Evaluation::NoMatch,
|
||||
}
|
||||
|
||||
let mut last_err = Err(Error::NoSpecForProgram {
|
||||
program: program.clone(),
|
||||
});
|
||||
if let Some(spec_list) = self.programs.get_vec(program) {
|
||||
for spec in spec_list {
|
||||
match spec.check(exec_call) {
|
||||
Ok(matched_exec) => return Ok(matched_exec),
|
||||
Err(err) => {
|
||||
last_err = Err(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
last_err
|
||||
}
|
||||
|
||||
pub fn check_each_good_list_individually(&self) -> Vec<PositiveExampleFailedCheck> {
|
||||
let mut violations = Vec::new();
|
||||
for (_program, spec) in self.programs.flat_iter() {
|
||||
violations.extend(spec.verify_should_match_list());
|
||||
}
|
||||
violations
|
||||
}
|
||||
|
||||
pub fn check_each_bad_list_individually(&self) -> Vec<NegativeExamplePassedCheck> {
|
||||
let mut violations = Vec::new();
|
||||
for (_program, spec) in self.programs.flat_iter() {
|
||||
violations.extend(spec.verify_should_not_match_list());
|
||||
}
|
||||
violations
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum Evaluation {
|
||||
NoMatch,
|
||||
Match {
|
||||
decision: Decision,
|
||||
#[serde(rename = "matchedRules")]
|
||||
matched_rules: Vec<RuleMatch>,
|
||||
},
|
||||
}
|
||||
|
||||
impl Evaluation {
|
||||
pub fn is_match(&self) -> bool {
|
||||
matches!(self, Self::Match { .. })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
use std::any::Any;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_execpolicy2::Decision;
|
||||
use codex_execpolicy2::Evaluation;
|
||||
use codex_execpolicy2::PolicyParser;
|
||||
use codex_execpolicy2::RuleMatch;
|
||||
use codex_execpolicy2::RuleRef;
|
||||
use codex_execpolicy2::rule::PatternToken;
|
||||
use codex_execpolicy2::rule::PrefixPattern;
|
||||
use codex_execpolicy2::rule::PrefixRule;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
use codex_execpolicy::RuleMatch;
|
||||
use codex_execpolicy::RuleRef;
|
||||
use codex_execpolicy::rule::PatternToken;
|
||||
use codex_execpolicy::rule::PrefixPattern;
|
||||
use codex_execpolicy::rule::PrefixRule;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn tokens(cmd: &[&str]) -> Vec<String> {
|
||||
@@ -1,29 +0,0 @@
|
||||
[package]
|
||||
name = "codex-execpolicy2"
|
||||
version = { workspace = true }
|
||||
edition = "2024"
|
||||
description = "Codex exec policy v2: prefix-based Starlark rules for command decisions."
|
||||
|
||||
[lib]
|
||||
name = "codex_execpolicy2"
|
||||
path = "src/lib.rs"
|
||||
|
||||
[[bin]]
|
||||
name = "codex-execpolicy2"
|
||||
path = "src/main.rs"
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
anyhow = { workspace = true }
|
||||
clap = { workspace = true, features = ["derive"] }
|
||||
multimap = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
shlex = { workspace = true }
|
||||
starlark = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
@@ -1,59 +0,0 @@
|
||||
# codex-execpolicy2
|
||||
|
||||
## Overview
|
||||
- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, match?, not_match?)`.
|
||||
- This release covers only the prefix-rule subset of the planned execpolicy v2 language; a richer language will follow.
|
||||
- Tokens are matched in order; any `pattern` element may be a list to denote alternatives. `decision` defaults to `allow`; valid values: `allow`, `prompt`, `forbidden`.
|
||||
- `match` / `not_match` supply example invocations that are validated at load time (think of them as unit tests); examples can be token arrays or strings (strings are tokenized with `shlex`).
|
||||
- The CLI always prints the JSON serialization of the evaluation result (whether a match or not).
|
||||
|
||||
## Policy shapes
|
||||
- Prefix rules use Starlark syntax:
|
||||
```starlark
|
||||
prefix_rule(
|
||||
pattern = ["cmd", ["alt1", "alt2"]], # ordered tokens; list entries denote alternatives
|
||||
decision = "prompt", # allow | prompt | forbidden; defaults to allow
|
||||
match = [["cmd", "alt1"], "cmd alt2"], # examples that must match this rule
|
||||
not_match = [["cmd", "oops"], "cmd alt3"], # examples that must not match this rule
|
||||
)
|
||||
```
|
||||
|
||||
## Response shapes
|
||||
- Match:
|
||||
```json
|
||||
{
|
||||
"match": {
|
||||
"decision": "allow|prompt|forbidden",
|
||||
"matchedRules": [
|
||||
{
|
||||
"prefixRuleMatch": {
|
||||
"matchedPrefix": ["<token>", "..."],
|
||||
"decision": "allow|prompt|forbidden"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- No match:
|
||||
```json
|
||||
"noMatch"
|
||||
```
|
||||
|
||||
- `matchedRules` lists every rule whose prefix matched the command; `matchedPrefix` is the exact prefix that matched.
|
||||
- The effective `decision` is the strictest severity across all matches (`forbidden` > `prompt` > `allow`).
|
||||
|
||||
## CLI
|
||||
- Provide one or more policy files (for example `src/default.codexpolicy`) to check a command:
|
||||
```bash
|
||||
cargo run -p codex-execpolicy2 -- check --policy path/to/policy.codexpolicy git status
|
||||
```
|
||||
- Pass multiple `--policy` flags to merge rules, evaluated in the order provided:
|
||||
```bash
|
||||
cargo run -p codex-execpolicy2 -- check --policy base.codexpolicy --policy overrides.codexpolicy git status
|
||||
```
|
||||
- Output is newline-delimited JSON by default; pass `--pretty` for pretty-printed JSON if desired.
|
||||
- Example outcomes:
|
||||
- Match: `{"match": { ... "decision": "allow" ... }}`
|
||||
- No match: `"noMatch"`
|
||||
@@ -1,26 +0,0 @@
|
||||
use starlark::Error as StarlarkError;
|
||||
use thiserror::Error;
|
||||
|
||||
pub type Result<T> = std::result::Result<T, Error>;
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub enum Error {
|
||||
#[error("invalid decision: {0}")]
|
||||
InvalidDecision(String),
|
||||
#[error("invalid pattern element: {0}")]
|
||||
InvalidPattern(String),
|
||||
#[error("invalid example: {0}")]
|
||||
InvalidExample(String),
|
||||
#[error(
|
||||
"expected every example to match at least one rule. rules: {rules:?}; unmatched examples: \
|
||||
{examples:?}"
|
||||
)]
|
||||
ExampleDidNotMatch {
|
||||
rules: Vec<String>,
|
||||
examples: Vec<String>,
|
||||
},
|
||||
#[error("expected example to not match rule `{rule}`: {example}")]
|
||||
ExampleDidMatch { rule: String, example: String },
|
||||
#[error("starlark error: {0}")]
|
||||
Starlark(StarlarkError),
|
||||
}
|
||||
@@ -1,15 +0,0 @@
|
||||
pub mod decision;
|
||||
pub mod error;
|
||||
pub mod parser;
|
||||
pub mod policy;
|
||||
pub mod rule;
|
||||
|
||||
pub use decision::Decision;
|
||||
pub use error::Error;
|
||||
pub use error::Result;
|
||||
pub use parser::PolicyParser;
|
||||
pub use policy::Evaluation;
|
||||
pub use policy::Policy;
|
||||
pub use rule::Rule;
|
||||
pub use rule::RuleMatch;
|
||||
pub use rule::RuleRef;
|
||||
@@ -1,66 +0,0 @@
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use clap::Parser;
|
||||
use codex_execpolicy2::PolicyParser;
|
||||
|
||||
/// CLI for evaluating exec policies
|
||||
#[derive(Parser)]
|
||||
#[command(name = "codex-execpolicy2")]
|
||||
enum Cli {
|
||||
/// Evaluate a command against a policy.
|
||||
Check {
|
||||
#[arg(short, long = "policy", value_name = "PATH", required = true)]
|
||||
policies: Vec<PathBuf>,
|
||||
|
||||
/// Pretty-print the JSON output.
|
||||
#[arg(long)]
|
||||
pretty: bool,
|
||||
|
||||
/// Command tokens to check.
|
||||
#[arg(
|
||||
value_name = "COMMAND",
|
||||
required = true,
|
||||
trailing_var_arg = true,
|
||||
allow_hyphen_values = true
|
||||
)]
|
||||
command: Vec<String>,
|
||||
},
|
||||
}
|
||||
|
||||
fn main() -> Result<()> {
|
||||
let cli = Cli::parse();
|
||||
match cli {
|
||||
Cli::Check {
|
||||
policies,
|
||||
command,
|
||||
pretty,
|
||||
} => cmd_check(policies, command, pretty),
|
||||
}
|
||||
}
|
||||
|
||||
fn cmd_check(policy_paths: Vec<PathBuf>, args: Vec<String>, pretty: bool) -> Result<()> {
|
||||
let policy = load_policies(&policy_paths)?;
|
||||
|
||||
let eval = policy.check(&args);
|
||||
let json = if pretty {
|
||||
serde_json::to_string_pretty(&eval)?
|
||||
} else {
|
||||
serde_json::to_string(&eval)?
|
||||
};
|
||||
println!("{json}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn load_policies(policy_paths: &[PathBuf]) -> Result<codex_execpolicy2::Policy> {
|
||||
let mut parser = PolicyParser::new();
|
||||
for policy_path in policy_paths {
|
||||
let policy_file_contents = fs::read_to_string(policy_path)
|
||||
.with_context(|| format!("failed to read policy at {}", policy_path.display()))?;
|
||||
let policy_identifier = policy_path.to_string_lossy().to_string();
|
||||
parser.parse(&policy_identifier, &policy_file_contents)?;
|
||||
}
|
||||
Ok(parser.build())
|
||||
}
|
||||
@@ -1,80 +0,0 @@
|
||||
use crate::decision::Decision;
|
||||
use crate::rule::RuleMatch;
|
||||
use crate::rule::RuleRef;
|
||||
use multimap::MultiMap;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct Policy {
|
||||
rules_by_program: MultiMap<String, RuleRef>,
|
||||
}
|
||||
|
||||
impl Policy {
|
||||
pub fn new(rules_by_program: MultiMap<String, RuleRef>) -> Self {
|
||||
Self { rules_by_program }
|
||||
}
|
||||
|
||||
pub fn rules(&self) -> &MultiMap<String, RuleRef> {
|
||||
&self.rules_by_program
|
||||
}
|
||||
|
||||
pub fn check(&self, cmd: &[String]) -> Evaluation {
|
||||
let rules = match cmd.first() {
|
||||
Some(first) => match self.rules_by_program.get_vec(first) {
|
||||
Some(rules) => rules,
|
||||
None => return Evaluation::NoMatch,
|
||||
},
|
||||
None => return Evaluation::NoMatch,
|
||||
};
|
||||
|
||||
let matched_rules: Vec<RuleMatch> =
|
||||
rules.iter().filter_map(|rule| rule.matches(cmd)).collect();
|
||||
match matched_rules.iter().map(RuleMatch::decision).max() {
|
||||
Some(decision) => Evaluation::Match {
|
||||
decision,
|
||||
matched_rules,
|
||||
},
|
||||
None => Evaluation::NoMatch,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn check_multiple<Commands>(&self, commands: Commands) -> Evaluation
|
||||
where
|
||||
Commands: IntoIterator,
|
||||
Commands::Item: AsRef<[String]>,
|
||||
{
|
||||
let matched_rules: Vec<RuleMatch> = commands
|
||||
.into_iter()
|
||||
.flat_map(|command| match self.check(command.as_ref()) {
|
||||
Evaluation::Match { matched_rules, .. } => matched_rules,
|
||||
Evaluation::NoMatch => Vec::new(),
|
||||
})
|
||||
.collect();
|
||||
|
||||
match matched_rules.iter().map(RuleMatch::decision).max() {
|
||||
Some(decision) => Evaluation::Match {
|
||||
decision,
|
||||
matched_rules,
|
||||
},
|
||||
None => Evaluation::NoMatch,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum Evaluation {
|
||||
NoMatch,
|
||||
Match {
|
||||
decision: Decision,
|
||||
#[serde(rename = "matchedRules")]
|
||||
matched_rules: Vec<RuleMatch>,
|
||||
},
|
||||
}
|
||||
|
||||
impl Evaluation {
|
||||
pub fn is_match(&self) -> bool {
|
||||
matches!(self, Self::Match { .. })
|
||||
}
|
||||
}
|
||||
@@ -210,6 +210,7 @@ async fn run_codex_tool_session_inner(
|
||||
}
|
||||
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id,
|
||||
turn_id: _,
|
||||
reason,
|
||||
grant_root,
|
||||
changes,
|
||||
|
||||
@@ -57,6 +57,10 @@ pub struct ExecApprovalRequestEvent {
|
||||
pub struct ApplyPatchApprovalRequestEvent {
|
||||
/// Responses API call id for the associated patch apply call, if available.
|
||||
pub call_id: String,
|
||||
/// Turn ID that this patch belongs to.
|
||||
/// Uses `#[serde(default)]` for backwards compatibility with older senders.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
pub changes: HashMap<PathBuf, FileChange>,
|
||||
/// Optional explanatory reason (e.g. request for extra write access).
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -686,8 +686,6 @@ pub struct ExitedReviewModeEvent {
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct ErrorEvent {
|
||||
pub message: String,
|
||||
#[serde(default)]
|
||||
pub http_status_code: Option<u16>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
@@ -1365,8 +1363,6 @@ pub struct UndoCompletedEvent {
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct StreamErrorEvent {
|
||||
pub message: String,
|
||||
#[serde(default)]
|
||||
pub http_status_code: Option<u16>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
@@ -1378,6 +1374,10 @@ pub struct StreamInfoEvent {
|
||||
pub struct PatchApplyBeginEvent {
|
||||
/// Identifier so this can be paired with the PatchApplyEnd event.
|
||||
pub call_id: String,
|
||||
/// Turn ID that this patch belongs to.
|
||||
/// Uses `#[serde(default)]` for backwards compatibility.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
/// If true, there was no ApplyPatchApprovalRequest for this patch.
|
||||
pub auto_approved: bool,
|
||||
/// The changes to be applied.
|
||||
@@ -1388,12 +1388,19 @@ pub struct PatchApplyBeginEvent {
|
||||
pub struct PatchApplyEndEvent {
|
||||
/// Identifier for the PatchApplyBegin that finished.
|
||||
pub call_id: String,
|
||||
/// Turn ID that this patch belongs to.
|
||||
/// Uses `#[serde(default)]` for backwards compatibility.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
/// Captured stdout (summary printed by apply_patch).
|
||||
pub stdout: String,
|
||||
/// Captured stderr (parser errors, IO failures, etc.).
|
||||
pub stderr: String,
|
||||
/// Whether the patch was applied successfully.
|
||||
pub success: bool,
|
||||
/// The changes that were applied (mirrors PatchApplyBeginEvent::changes).
|
||||
#[serde(default)]
|
||||
pub changes: HashMap<PathBuf, FileChange>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
|
||||
@@ -1464,6 +1464,7 @@ impl ChatWidget {
|
||||
// }),
|
||||
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
changes: HashMap::from([
|
||||
(
|
||||
PathBuf::from("/tmp/test.txt"),
|
||||
@@ -1654,7 +1655,7 @@ impl ChatWidget {
|
||||
self.on_rate_limit_snapshot(ev.rate_limits);
|
||||
}
|
||||
EventMsg::Warning(WarningEvent { message }) => self.on_warning(message),
|
||||
EventMsg::Error(ErrorEvent { message, .. }) => self.on_error(message),
|
||||
EventMsg::Error(ErrorEvent { message }) => self.on_error(message),
|
||||
EventMsg::McpStartupUpdate(ev) => self.on_mcp_startup_update(ev),
|
||||
EventMsg::McpStartupComplete(ev) => self.on_mcp_startup_complete(ev),
|
||||
EventMsg::TurnAborted(ev) => match ev.reason {
|
||||
@@ -1697,9 +1698,7 @@ impl ChatWidget {
|
||||
}
|
||||
EventMsg::UndoStarted(ev) => self.on_undo_started(ev),
|
||||
EventMsg::UndoCompleted(ev) => self.on_undo_completed(ev),
|
||||
EventMsg::StreamError(StreamErrorEvent { message, .. }) => {
|
||||
self.on_stream_error(message)
|
||||
}
|
||||
EventMsg::StreamError(StreamErrorEvent { message }) => self.on_stream_error(message),
|
||||
EventMsg::UserMessage(ev) => {
|
||||
if from_replay {
|
||||
self.on_user_message_event(ev);
|
||||
|
||||
@@ -4,6 +4,9 @@ use codex_core::CodexConversation;
|
||||
use codex_core::ConversationManager;
|
||||
use codex_core::NewConversation;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::protocol::ErrorEvent;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use tokio::sync::mpsc::UnboundedSender;
|
||||
use tokio::sync::mpsc::unbounded_channel;
|
||||
@@ -28,9 +31,16 @@ pub(crate) fn spawn_agent(
|
||||
session_configured,
|
||||
} = match server.new_conversation(config).await {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
// TODO: surface this error to the user.
|
||||
tracing::error!("failed to initialize codex: {e}");
|
||||
#[allow(clippy::print_stderr)]
|
||||
Err(err) => {
|
||||
let message = err.to_string();
|
||||
eprintln!("{message}");
|
||||
app_event_tx_clone.send(AppEvent::CodexEvent(Event {
|
||||
id: "".to_string(),
|
||||
msg: EventMsg::Error(ErrorEvent { message }),
|
||||
}));
|
||||
app_event_tx_clone.send(AppEvent::ExitRequest);
|
||||
tracing::error!("failed to initialize codex: {err}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1912,6 +1912,7 @@ fn approval_modal_patch_snapshot() {
|
||||
);
|
||||
let ev = ApplyPatchApprovalRequestEvent {
|
||||
call_id: "call-approve-patch".into(),
|
||||
turn_id: "turn-approve-patch".into(),
|
||||
changes,
|
||||
reason: Some("The model wants to apply changes".into()),
|
||||
grant_root: Some(PathBuf::from("/tmp")),
|
||||
@@ -2164,6 +2165,7 @@ fn apply_patch_events_emit_history_cells() {
|
||||
);
|
||||
let ev = ApplyPatchApprovalRequestEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
changes,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
@@ -2204,6 +2206,7 @@ fn apply_patch_events_emit_history_cells() {
|
||||
);
|
||||
let begin = PatchApplyBeginEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
auto_approved: true,
|
||||
changes: changes2,
|
||||
};
|
||||
@@ -2220,11 +2223,20 @@ fn apply_patch_events_emit_history_cells() {
|
||||
);
|
||||
|
||||
// 3) End apply success -> success cell
|
||||
let mut end_changes = HashMap::new();
|
||||
end_changes.insert(
|
||||
PathBuf::from("foo.txt"),
|
||||
FileChange::Add {
|
||||
content: "hello\n".to_string(),
|
||||
},
|
||||
);
|
||||
let end = PatchApplyEndEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
stdout: "ok\n".into(),
|
||||
stderr: String::new(),
|
||||
success: true,
|
||||
changes: end_changes,
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
id: "s1".into(),
|
||||
@@ -2252,6 +2264,7 @@ fn apply_patch_manual_approval_adjusts_header() {
|
||||
id: "s1".into(),
|
||||
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
changes: proposed_changes,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
@@ -2270,6 +2283,7 @@ fn apply_patch_manual_approval_adjusts_header() {
|
||||
id: "s1".into(),
|
||||
msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
auto_approved: false,
|
||||
changes: apply_changes,
|
||||
}),
|
||||
@@ -2299,6 +2313,7 @@ fn apply_patch_manual_flow_snapshot() {
|
||||
id: "s1".into(),
|
||||
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
changes: proposed_changes,
|
||||
reason: Some("Manual review required".into()),
|
||||
grant_root: None,
|
||||
@@ -2321,6 +2336,7 @@ fn apply_patch_manual_flow_snapshot() {
|
||||
id: "s1".into(),
|
||||
msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||
call_id: "c1".into(),
|
||||
turn_id: "turn-c1".into(),
|
||||
auto_approved: false,
|
||||
changes: apply_changes,
|
||||
}),
|
||||
@@ -2348,6 +2364,7 @@ fn apply_patch_approval_sends_op_with_submission_id() {
|
||||
);
|
||||
let ev = ApplyPatchApprovalRequestEvent {
|
||||
call_id: "call-999".into(),
|
||||
turn_id: "turn-999".into(),
|
||||
changes,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
@@ -2387,6 +2404,7 @@ fn apply_patch_full_flow_integration_like() {
|
||||
id: "sub-xyz".into(),
|
||||
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "call-1".into(),
|
||||
turn_id: "turn-call-1".into(),
|
||||
changes,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
@@ -2427,17 +2445,25 @@ fn apply_patch_full_flow_integration_like() {
|
||||
id: "sub-xyz".into(),
|
||||
msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||
call_id: "call-1".into(),
|
||||
turn_id: "turn-call-1".into(),
|
||||
auto_approved: false,
|
||||
changes: changes2,
|
||||
}),
|
||||
});
|
||||
let mut end_changes = HashMap::new();
|
||||
end_changes.insert(
|
||||
PathBuf::from("pkg.rs"),
|
||||
FileChange::Add { content: "".into() },
|
||||
);
|
||||
chat.handle_codex_event(Event {
|
||||
id: "sub-xyz".into(),
|
||||
msg: EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||
call_id: "call-1".into(),
|
||||
turn_id: "turn-call-1".into(),
|
||||
stdout: String::from("ok"),
|
||||
stderr: String::new(),
|
||||
success: true,
|
||||
changes: end_changes,
|
||||
}),
|
||||
});
|
||||
}
|
||||
@@ -2458,6 +2484,7 @@ fn apply_patch_untrusted_shows_approval_modal() {
|
||||
id: "sub-1".into(),
|
||||
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "call-1".into(),
|
||||
turn_id: "turn-call-1".into(),
|
||||
changes,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
@@ -2506,6 +2533,7 @@ fn apply_patch_request_shows_diff_summary() {
|
||||
id: "sub-apply".into(),
|
||||
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "call-apply".into(),
|
||||
turn_id: "turn-apply".into(),
|
||||
changes,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
@@ -2596,7 +2624,6 @@ fn stream_error_updates_status_indicator() {
|
||||
id: "sub-1".into(),
|
||||
msg: EventMsg::StreamError(StreamErrorEvent {
|
||||
message: msg.to_string(),
|
||||
http_status_code: None,
|
||||
}),
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user