Merge remote-tracking branch 'origin/main' into aibrahim/spawn-agent-model-overrides

# Conflicts:
#	codex-rs/core/src/tools/spec.rs
This commit is contained in:
Ahmed Ibrahim
2026-03-09 19:37:45 -07:00
188 changed files with 12386 additions and 2063 deletions
+11
View File
@@ -410,6 +410,9 @@
"request_permissions": {
"type": "boolean"
},
"request_permissions_tool": {
"type": "boolean"
},
"request_rule": {
"type": "boolean"
},
@@ -1313,6 +1316,10 @@
"description": "Reject MCP elicitation prompts.",
"type": "boolean"
},
"request_permissions": {
"description": "Reject approval prompts related to built-in permission requests.",
"type": "boolean"
},
"rules": {
"description": "Reject prompts triggered by execpolicy `prompt` rules.",
"type": "boolean"
@@ -1324,6 +1331,7 @@
},
"required": [
"mcp_elicitations",
"request_permissions",
"rules",
"sandbox_approval"
],
@@ -1882,6 +1890,9 @@
"request_permissions": {
"type": "boolean"
},
"request_permissions_tool": {
"type": "boolean"
},
"request_rule": {
"type": "boolean"
},
+36 -63
View File
@@ -155,12 +155,11 @@ fn strip_total_output_header(output: &str) -> Option<(&str, u32)> {
pub(crate) mod tools {
use crate::tools::spec::JsonSchema;
use codex_protocol::config_types::WebSearchContextSize;
use codex_protocol::config_types::WebSearchFilters;
use codex_protocol::config_types::WebSearchUserLocation;
use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters;
use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLocation;
use codex_protocol::config_types::WebSearchUserLocationType;
use serde::Deserialize;
use serde::Serialize;
use serde::Serializer;
/// When serialized as JSON, this produces a valid "Tool" in the OpenAI
/// Responses API.
@@ -181,16 +180,10 @@ pub(crate) mod tools {
WebSearch {
#[serde(skip_serializing_if = "Option::is_none")]
external_web_access: Option<bool>,
#[serde(
skip_serializing_if = "Option::is_none",
serialize_with = "serialize_web_search_filters"
)]
filters: Option<WebSearchFilters>,
#[serde(
skip_serializing_if = "Option::is_none",
serialize_with = "serialize_web_search_user_location"
)]
user_location: Option<WebSearchUserLocation>,
#[serde(skip_serializing_if = "Option::is_none")]
filters: Option<ResponsesApiWebSearchFilters>,
#[serde(skip_serializing_if = "Option::is_none")]
user_location: Option<ResponsesApiWebSearchUserLocation>,
#[serde(skip_serializing_if = "Option::is_none")]
search_context_size: Option<WebSearchContextSize>,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -212,63 +205,43 @@ pub(crate) mod tools {
}
}
fn serialize_web_search_filters<S>(
filters: &Option<WebSearchFilters>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
match filters {
Some(filters) => {
#[derive(Serialize)]
struct SerializableWebSearchFilters<'a> {
#[serde(skip_serializing_if = "Option::is_none")]
allowed_domains: Option<&'a Vec<String>>,
}
#[derive(Debug, Clone, Serialize, PartialEq)]
pub(crate) struct ResponsesApiWebSearchFilters {
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) allowed_domains: Option<Vec<String>>,
}
SerializableWebSearchFilters {
allowed_domains: filters.allowed_domains.as_ref(),
}
.serialize(serializer)
impl From<ConfigWebSearchFilters> for ResponsesApiWebSearchFilters {
fn from(filters: ConfigWebSearchFilters) -> Self {
Self {
allowed_domains: filters.allowed_domains,
}
None => serializer.serialize_none(),
}
}
fn serialize_web_search_user_location<S>(
user_location: &Option<WebSearchUserLocation>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
match user_location {
Some(user_location) => {
#[derive(Serialize)]
struct SerializableWebSearchUserLocation<'a> {
#[serde(rename = "type")]
r#type: WebSearchUserLocationType,
#[serde(skip_serializing_if = "Option::is_none")]
country: Option<&'a String>,
#[serde(skip_serializing_if = "Option::is_none")]
region: Option<&'a String>,
#[serde(skip_serializing_if = "Option::is_none")]
city: Option<&'a String>,
#[serde(skip_serializing_if = "Option::is_none")]
timezone: Option<&'a String>,
}
#[derive(Debug, Clone, Serialize, PartialEq)]
pub(crate) struct ResponsesApiWebSearchUserLocation {
#[serde(rename = "type")]
pub(crate) r#type: WebSearchUserLocationType,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) country: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) region: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) city: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) timezone: Option<String>,
}
SerializableWebSearchUserLocation {
r#type: user_location.r#type,
country: user_location.country.as_ref(),
region: user_location.region.as_ref(),
city: user_location.city.as_ref(),
timezone: user_location.timezone.as_ref(),
}
.serialize(serializer)
impl From<ConfigWebSearchUserLocation> for ResponsesApiWebSearchUserLocation {
fn from(user_location: ConfigWebSearchUserLocation) -> Self {
Self {
r#type: user_location.r#type,
country: user_location.country,
region: user_location.region,
city: user_location.city,
timezone: user_location.timezone,
}
None => serializer.serialize_none(),
}
}
+147 -4
View File
@@ -71,6 +71,7 @@ use codex_otel::current_span_w3c_trace_context;
use codex_otel::set_parent_from_w3c_trace_context;
use codex_protocol::ThreadId;
use codex_protocol::approvals::ElicitationRequestEvent;
use codex_protocol::approvals::ExecApprovalRequestSkillMetadata;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::approvals::NetworkPolicyAmendment;
use codex_protocol::approvals::NetworkPolicyRuleAction;
@@ -102,6 +103,10 @@ use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnContextItem;
use codex_protocol::protocol::TurnContextNetworkItem;
use codex_protocol::protocol::TurnStartedEvent;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionsArgs;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::ElicitationResponse;
@@ -301,6 +306,7 @@ use crate::windows_sandbox::WindowsSandboxLevelExt;
use codex_async_utils::OrCancelExt;
use codex_otel::SessionTelemetry;
use codex_otel::TelemetryAuthMode;
use codex_otel::metrics::names::THREAD_STARTED_METRIC;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
@@ -945,6 +951,11 @@ impl SessionConfiguration {
pub(crate) fn apply(&self, updates: &SessionSettingsUpdate) -> ConstraintResult<Self> {
let mut next_configuration = self.clone();
let file_system_policy_matches_legacy = self.file_system_sandbox_policy
== FileSystemSandboxPolicy::from_legacy_sandbox_policy(
self.sandbox_policy.get(),
&self.cwd,
);
if let Some(collaboration_mode) = updates.collaboration_mode.clone() {
next_configuration.collaboration_mode = collaboration_mode;
}
@@ -960,18 +971,29 @@ impl SessionConfiguration {
if let Some(approval_policy) = updates.approval_policy {
next_configuration.approval_policy.set(approval_policy)?;
}
let mut sandbox_policy_changed = false;
if let Some(sandbox_policy) = updates.sandbox_policy.clone() {
next_configuration.sandbox_policy.set(sandbox_policy)?;
next_configuration.file_system_sandbox_policy =
FileSystemSandboxPolicy::from(next_configuration.sandbox_policy.get());
next_configuration.network_sandbox_policy =
NetworkSandboxPolicy::from(next_configuration.sandbox_policy.get());
sandbox_policy_changed = true;
}
if let Some(windows_sandbox_level) = updates.windows_sandbox_level {
next_configuration.windows_sandbox_level = windows_sandbox_level;
}
let mut cwd_changed = false;
if let Some(cwd) = updates.cwd.clone() {
next_configuration.cwd = cwd;
cwd_changed = true;
}
if sandbox_policy_changed || (cwd_changed && file_system_policy_matches_legacy) {
// Preserve richer split policies across cwd-only updates; only
// rederive when the session is already using the legacy bridge.
next_configuration.file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(
next_configuration.sandbox_policy.get(),
&next_configuration.cwd,
);
}
if let Some(app_server_client_name) = updates.app_server_client_name.clone() {
next_configuration.app_server_client_name = Some(app_server_client_name);
@@ -1408,7 +1430,7 @@ impl Session {
};
config.features.emit_metrics(&session_telemetry);
session_telemetry.counter(
"codex.thread.started",
THREAD_STARTED_METRIC,
1,
&[(
"is_git",
@@ -2721,6 +2743,7 @@ impl Session {
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
additional_permissions: Option<PermissionProfile>,
skill_metadata: Option<ExecApprovalRequestSkillMetadata>,
available_decisions: Option<Vec<ReviewDecision>>,
) -> ReviewDecision {
// command-level approvals use `call_id`.
@@ -2774,6 +2797,7 @@ impl Session {
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
additional_permissions,
skill_metadata,
available_decisions: Some(available_decisions),
parsed_cmd,
});
@@ -2817,6 +2841,58 @@ impl Session {
rx_approve
}
pub async fn request_permissions(
&self,
turn_context: &TurnContext,
call_id: String,
args: RequestPermissionsArgs,
) -> Option<RequestPermissionsResponse> {
match turn_context.approval_policy.value() {
AskForApproval::Never => {
return Some(RequestPermissionsResponse {
permissions: PermissionProfile::default(),
scope: PermissionGrantScope::Turn,
});
}
AskForApproval::Reject(reject_config)
if reject_config.rejects_request_permissions() =>
{
return Some(RequestPermissionsResponse {
permissions: PermissionProfile::default(),
scope: PermissionGrantScope::Turn,
});
}
AskForApproval::OnFailure
| AskForApproval::OnRequest
| AskForApproval::UnlessTrusted
| AskForApproval::Reject(_) => {}
}
let (tx_response, rx_response) = oneshot::channel();
let prev_entry = {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
ts.insert_pending_request_permissions(call_id.clone(), tx_response)
}
None => None,
}
};
if prev_entry.is_some() {
warn!("Overwriting existing pending request_permissions for call_id: {call_id}");
}
let event = EventMsg::RequestPermissions(RequestPermissionsEvent {
call_id,
turn_id: turn_context.sub_id.clone(),
reason: args.reason,
permissions: args.permissions,
});
self.send_event(turn_context, event).await;
rx_response.await.ok()
}
pub async fn request_user_input(
&self,
turn_context: &TurnContext,
@@ -2953,6 +3029,59 @@ impl Session {
}
}
pub async fn notify_request_permissions_response(
&self,
call_id: &str,
response: RequestPermissionsResponse,
) {
let mut granted_for_session = None;
let entry = {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
let entry = ts.remove_pending_request_permissions(call_id);
if entry.is_some() && !response.permissions.is_empty() {
match response.scope {
PermissionGrantScope::Turn => {
ts.record_granted_permissions(response.permissions.clone());
}
PermissionGrantScope::Session => {
granted_for_session = Some(response.permissions.clone());
}
}
}
entry
}
None => None,
}
};
if let Some(permissions) = granted_for_session {
let mut state = self.state.lock().await;
state.record_granted_permissions(permissions);
}
match entry {
Some(tx_response) => {
tx_response.send(response).ok();
}
None => {
warn!("No pending request_permissions found for call_id: {call_id}");
}
}
}
pub(crate) async fn granted_turn_permissions(&self) -> Option<PermissionProfile> {
let active = self.active_turn.lock().await;
let active = active.as_ref()?;
let ts = active.turn_state.lock().await;
ts.granted_permissions()
}
pub(crate) async fn granted_session_permissions(&self) -> Option<PermissionProfile> {
let state = self.state.lock().await;
state.granted_permissions()
}
pub async fn notify_dynamic_tool_response(&self, call_id: &str, response: DynamicToolResponse) {
let entry = {
let mut active = self.active_turn.lock().await;
@@ -3192,7 +3321,6 @@ impl Session {
DeveloperInstructions::from_policy(
turn_context.sandbox_policy.get(),
turn_context.approval_policy.value(),
turn_context.features.enabled(Feature::GuardianApproval),
self.services.exec_policy.current().as_ref(),
&turn_context.cwd,
turn_context.features.enabled(Feature::RequestPermissions),
@@ -3921,6 +4049,10 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
handlers::request_user_input_response(&sess, id, response).await;
false
}
Op::RequestPermissionsResponse { id, response } => {
handlers::request_permissions_response(&sess, id, response).await;
false
}
Op::DynamicToolResponse { id, response } => {
handlers::dynamic_tool_response(&sess, id, response).await;
false
@@ -4103,6 +4235,7 @@ mod handlers {
use codex_protocol::protocol::ThreadRolledBackEvent;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::WarningEvent;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputResponse;
use crate::context_manager::is_user_turn_boundary;
@@ -4345,6 +4478,15 @@ mod handlers {
sess.notify_user_input_response(&id, response).await;
}
pub async fn request_permissions_response(
sess: &Arc<Session>,
id: String,
response: RequestPermissionsResponse,
) {
sess.notify_request_permissions_response(&id, response)
.await;
}
pub async fn dynamic_tool_response(
sess: &Arc<Session>,
id: String,
@@ -6221,6 +6363,7 @@ fn realtime_text_for_event(msg: &EventMsg) -> Option<String> {
| EventMsg::ImageGenerationBegin(_)
| EventMsg::ImageGenerationEnd(_)
| EventMsg::ExecApprovalRequest(_)
| EventMsg::RequestPermissions(_)
| EventMsg::RequestUserInput(_)
| EventMsg::DynamicToolCallRequest(_)
| EventMsg::DynamicToolCallResponse(_)
+163
View File
@@ -13,6 +13,10 @@ use codex_protocol::protocol::RequestUserInputEvent;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::Submission;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionsArgs;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::user_input::UserInput;
@@ -251,6 +255,19 @@ async fn forward_events(
)
.await;
}
Event {
msg: EventMsg::RequestPermissions(event),
..
} => {
handle_request_permissions(
&codex,
&parent_session,
&parent_ctx,
event,
&cancel_token,
)
.await;
}
Event {
id,
msg: EventMsg::RequestUserInput(event),
@@ -332,6 +349,7 @@ async fn handle_exec_approval(
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
skill_metadata,
available_decisions,
..
} = event;
@@ -346,6 +364,7 @@ async fn handle_exec_approval(
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
skill_metadata,
available_decisions,
);
let decision = await_approval_with_cancel(
@@ -423,6 +442,30 @@ async fn handle_request_user_input(
let _ = codex.submit(Op::UserInputAnswer { id, response }).await;
}
async fn handle_request_permissions(
codex: &Codex,
parent_session: &Session,
parent_ctx: &TurnContext,
event: RequestPermissionsEvent,
cancel_token: &CancellationToken,
) {
let call_id = event.call_id;
let args = RequestPermissionsArgs {
reason: event.reason,
permissions: event.permissions,
};
let response_fut = parent_session.request_permissions(parent_ctx, call_id.clone(), args);
let response =
await_request_permissions_with_cancel(response_fut, parent_session, &call_id, cancel_token)
.await;
let _ = codex
.submit(Op::RequestPermissionsResponse {
id: call_id,
response,
})
.await;
}
async fn await_user_input_with_cancel<F>(
fut: F,
parent_session: &Session,
@@ -449,6 +492,34 @@ where
}
}
async fn await_request_permissions_with_cancel<F>(
fut: F,
parent_session: &Session,
call_id: &str,
cancel_token: &CancellationToken,
) -> RequestPermissionsResponse
where
F: core::future::Future<Output = Option<RequestPermissionsResponse>>,
{
tokio::select! {
biased;
_ = cancel_token.cancelled() => {
let empty = RequestPermissionsResponse {
permissions: Default::default(),
scope: PermissionGrantScope::Turn,
};
parent_session
.notify_request_permissions_response(call_id, empty.clone())
.await;
empty
}
response = fut => response.unwrap_or_else(|| RequestPermissionsResponse {
permissions: Default::default(),
scope: PermissionGrantScope::Turn,
}),
}
}
/// Await an approval decision, aborting on cancellation.
async fn await_approval_with_cancel<F>(
fut: F,
@@ -477,11 +548,16 @@ where
mod tests {
use super::*;
use async_channel::bounded;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::RawResponseItemEvent;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnAbortedEvent;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use pretty_assertions::assert_eq;
use tokio::sync::watch;
@@ -600,4 +676,91 @@ mod tests {
.expect("forward_ops did not exit")
.expect("forward_ops join error");
}
#[tokio::test]
async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
let (parent_session, parent_ctx, rx_events) =
crate::codex::make_session_and_context_with_rx().await;
*parent_session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let codex = Arc::new(Codex {
tx_sub,
rx_event: rx_events_child,
agent_status,
session: Arc::clone(&parent_session),
});
let call_id = "tool-call-1".to_string();
let expected_response = RequestPermissionsResponse {
permissions: PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..PermissionProfile::default()
},
scope: PermissionGrantScope::Turn,
};
let cancel_token = CancellationToken::new();
let request_call_id = call_id.clone();
let handle = tokio::spawn({
let codex = Arc::clone(&codex);
let parent_session = Arc::clone(&parent_session);
let parent_ctx = Arc::clone(&parent_ctx);
let cancel_token = cancel_token.clone();
async move {
handle_request_permissions(
codex.as_ref(),
parent_session.as_ref(),
parent_ctx.as_ref(),
RequestPermissionsEvent {
call_id: request_call_id,
turn_id: "child-turn-1".to_string(),
reason: Some("need access".to_string()),
permissions: PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..PermissionProfile::default()
},
},
&cancel_token,
)
.await;
}
});
let request_event = timeout(Duration::from_secs(1), rx_events.recv())
.await
.expect("request_permissions event timed out")
.expect("request_permissions event missing");
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected RequestPermissions event");
};
assert_eq!(request.call_id, call_id.clone());
parent_session
.notify_request_permissions_response(&call_id, expected_response.clone())
.await;
timeout(Duration::from_secs(1), handle)
.await
.expect("handle_request_permissions hung")
.expect("handle_request_permissions join error");
let submission = timeout(Duration::from_secs(1), rx_sub.recv())
.await
.expect("request_permissions response timed out")
.expect("request_permissions response missing");
assert_eq!(
submission.op,
Op::RequestPermissionsResponse {
id: call_id,
response: expected_response,
}
);
}
}
+249
View File
@@ -17,6 +17,14 @@ use crate::tools::format_exec_output_str;
use codex_protocol::ThreadId;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::request_permissions::PermissionGrantScope;
use tracing::Span;
use crate::protocol::CompactedItem;
@@ -1885,6 +1893,100 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
}
}
#[tokio::test]
async fn session_configuration_apply_preserves_split_file_system_policy_on_cwd_only_update() {
let mut session_configuration = make_session_configuration_for_tests().await;
let workspace = tempfile::tempdir().expect("create temp dir");
let project_root = workspace.path().join("project");
let original_cwd = project_root.join("subdir");
let docs_dir = original_cwd.join("docs");
std::fs::create_dir_all(&docs_dir).expect("create docs dir");
let docs_dir =
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs_dir).expect("docs");
session_configuration.cwd = original_cwd;
session_configuration.sandbox_policy =
codex_config::Constrained::allow_any(SandboxPolicy::WorkspaceWrite {
writable_roots: Vec::new(),
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs_dir.clone()],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
});
session_configuration.file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs_dir },
access: FileSystemAccessMode::Read,
},
]);
let updated = session_configuration
.apply(&SessionSettingsUpdate {
cwd: Some(project_root),
..Default::default()
})
.expect("cwd-only update should succeed");
assert_eq!(
updated.file_system_sandbox_policy,
session_configuration.file_system_sandbox_policy
);
}
#[tokio::test]
async fn session_configuration_apply_rederives_legacy_file_system_policy_on_cwd_update() {
let mut session_configuration = make_session_configuration_for_tests().await;
let workspace = tempfile::tempdir().expect("create temp dir");
let project_root = workspace.path().join("project");
let original_cwd = project_root.join("subdir");
let docs_dir = original_cwd.join("docs");
std::fs::create_dir_all(&docs_dir).expect("create docs dir");
let docs_dir =
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs_dir).expect("docs");
session_configuration.cwd = original_cwd;
session_configuration.sandbox_policy =
codex_config::Constrained::allow_any(SandboxPolicy::WorkspaceWrite {
writable_roots: Vec::new(),
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs_dir],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
});
session_configuration.file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(
session_configuration.sandbox_policy.get(),
&session_configuration.cwd,
);
let updated = session_configuration
.apply(&SessionSettingsUpdate {
cwd: Some(project_root.clone()),
..Default::default()
})
.expect("cwd-only update should succeed");
assert_eq!(
updated.file_system_sandbox_policy,
FileSystemSandboxPolicy::from_legacy_sandbox_policy(
updated.sandbox_policy.get(),
&project_root,
)
);
}
#[tokio::test]
async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
let codex_home = tempfile::tempdir().expect("create temp dir");
@@ -2141,6 +2243,153 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
(session, turn_context)
}
#[tokio::test]
async fn notify_request_permissions_response_ignores_unmatched_call_id() {
let (session, _turn_context) = make_session_and_context().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
session
.notify_request_permissions_response(
"missing",
codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: codex_protocol::models::PermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
scope: PermissionGrantScope::Turn,
},
)
.await;
assert_eq!(session.granted_turn_permissions().await, None);
}
#[tokio::test]
async fn request_permissions_emits_event_when_reject_policy_allows_requests() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
Arc::get_mut(&mut turn_context)
.expect("single turn context ref")
.approval_policy
.set(crate::protocol::AskForApproval::Reject(
crate::protocol::RejectConfig {
sandbox_approval: true,
rules: true,
request_permissions: false,
mcp_elicitations: true,
},
))
.expect("test setup should allow updating approval policy");
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let call_id = "call-1".to_string();
let expected_response = codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: codex_protocol::models::PermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
scope: PermissionGrantScope::Turn,
};
let handle = tokio::spawn({
let session = Arc::clone(&session);
let turn_context = Arc::clone(&turn_context);
let call_id = call_id.clone();
async move {
session
.request_permissions(
turn_context.as_ref(),
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
reason: Some("need network".to_string()),
permissions: codex_protocol::models::PermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
},
)
.await
}
});
let request_event = tokio::time::timeout(StdDuration::from_secs(1), rx.recv())
.await
.expect("request_permissions event timed out")
.expect("request_permissions event missing");
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected request_permissions event");
};
assert_eq!(request.call_id, call_id);
session
.notify_request_permissions_response(&request.call_id, expected_response.clone())
.await;
let response = tokio::time::timeout(StdDuration::from_secs(1), handle)
.await
.expect("request_permissions future timed out")
.expect("request_permissions join error");
assert_eq!(response, Some(expected_response));
}
#[tokio::test]
async fn request_permissions_returns_empty_grant_when_reject_policy_blocks_requests() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
Arc::get_mut(&mut turn_context)
.expect("single turn context ref")
.approval_policy
.set(crate::protocol::AskForApproval::Reject(
crate::protocol::RejectConfig {
sandbox_approval: false,
rules: false,
request_permissions: true,
mcp_elicitations: false,
},
))
.expect("test setup should allow updating approval policy");
let response = session
.request_permissions(
&turn_context,
"call-1".to_string(),
codex_protocol::request_permissions::RequestPermissionsArgs {
reason: Some("need network".to_string()),
permissions: codex_protocol::models::PermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
},
)
.await;
assert_eq!(
response,
Some(
codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: codex_protocol::models::PermissionProfile::default(),
scope: PermissionGrantScope::Turn,
}
)
);
assert!(
tokio::time::timeout(StdDuration::from_millis(50), rx.recv())
.await
.is_err(),
"unexpected request_permissions event emitted",
);
}
#[tokio::test]
async fn submit_with_id_captures_current_span_trace_context() {
let (session, _turn_context) = make_session_and_context().await;
+12 -5
View File
@@ -16,6 +16,8 @@ use codex_execpolicy::RuleMatch;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::codex_linux_sandbox_exe_or_skip;
use core_test_support::responses::ev_assistant_message;
@@ -70,15 +72,17 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
.features
.enable(Feature::RequestPermissions)
.expect("test setup should allow enabling request permissions");
turn_context_raw
.sandbox_policy
.set(SandboxPolicy::DangerFullAccess)
.expect("test setup should allow updating sandbox policy");
// This test is about request-permissions validation, not managed sandbox
// policy enforcement. Widen the derived sandbox policies directly so the
// command runs without depending on a platform sandbox binary.
turn_context_raw.file_system_sandbox_policy =
codex_protocol::permissions::FileSystemSandboxPolicy::from(
&SandboxPolicy::DangerFullAccess,
);
FileSystemSandboxPolicy::from(turn_context_raw.sandbox_policy.get());
turn_context_raw.network_sandbox_policy =
codex_protocol::permissions::NetworkSandboxPolicy::from(&SandboxPolicy::DangerFullAccess);
NetworkSandboxPolicy::from(turn_context_raw.sandbox_policy.get());
let mut config = (*turn_context_raw.config).clone();
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
let config = Arc::new(config);
@@ -92,11 +96,14 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
turn_context_raw.provider = config.model_provider.clone();
let session = Arc::new(session);
let turn_context = Arc::new(turn_context_raw);
let expiration_ms: u64 = if cfg!(windows) { 2_500 } else { 1_000 };
let params = ExecParams {
command: if cfg!(windows) {
vec![
"cmd.exe".to_string(),
"/Q".to_string(),
"/D".to_string(),
"/C".to_string(),
"echo hi".to_string(),
]
@@ -108,7 +115,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
]
},
cwd: turn_context.cwd.clone(),
expiration: 1000.into(),
expiration: expiration_ms.into(),
env: HashMap::new(),
network: None,
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
+150
View File
@@ -574,6 +574,156 @@ fn permissions_profiles_reject_nested_entries_for_non_project_roots() -> std::io
Ok(())
}
fn load_workspace_permission_profile(profile: PermissionProfileToml) -> std::io::Result<Config> {
let codex_home = TempDir::new()?;
let cwd = TempDir::new()?;
std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?;
Config::load_from_base_config_with_overrides(
ConfigToml {
default_permissions: Some("workspace".to_string()),
permissions: Some(PermissionsToml {
entries: BTreeMap::from([("workspace".to_string(), profile)]),
}),
..Default::default()
},
ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
},
codex_home.path().to_path_buf(),
)
}
#[test]
fn permissions_profiles_allow_unknown_special_paths() -> std::io::Result<()> {
let config = load_workspace_permission_profile(PermissionProfileToml {
filesystem: Some(FilesystemPermissionsToml {
entries: BTreeMap::from([(
":future_special_path".to_string(),
FilesystemPermissionToml::Access(FileSystemAccessMode::Read),
)]),
}),
network: None,
})?;
assert_eq!(
config.permissions.file_system_sandbox_policy,
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::unknown(":future_special_path", None),
},
access: FileSystemAccessMode::Read,
}]),
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: Vec::new(),
},
network_access: false,
}
);
assert!(
config.startup_warnings.iter().any(|warning| warning.contains(
"Configured filesystem path `:future_special_path` is not recognized by this version of Codex and will be ignored."
)),
"{:?}",
config.startup_warnings
);
Ok(())
}
#[test]
fn permissions_profiles_allow_unknown_special_paths_with_nested_entries() -> std::io::Result<()> {
let config = load_workspace_permission_profile(PermissionProfileToml {
filesystem: Some(FilesystemPermissionsToml {
entries: BTreeMap::from([(
":future_special_path".to_string(),
FilesystemPermissionToml::Scoped(BTreeMap::from([(
"docs".to_string(),
FileSystemAccessMode::Read,
)])),
)]),
}),
network: None,
})?;
assert_eq!(
config.permissions.file_system_sandbox_policy,
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::unknown(":future_special_path", Some("docs".into())),
},
access: FileSystemAccessMode::Read,
}]),
);
assert!(
config.startup_warnings.iter().any(|warning| warning.contains(
"Configured filesystem path `:future_special_path` with nested entry `docs` is not recognized by this version of Codex and will be ignored."
)),
"{:?}",
config.startup_warnings
);
Ok(())
}
#[test]
fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io::Result<()> {
let config = load_workspace_permission_profile(PermissionProfileToml {
filesystem: None,
network: None,
})?;
assert_eq!(
config.permissions.file_system_sandbox_policy,
FileSystemSandboxPolicy::restricted(Vec::new())
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: Vec::new(),
},
network_access: false,
}
);
assert!(
config.startup_warnings.iter().any(|warning| warning.contains(
"Permissions profile `workspace` does not define any recognized filesystem entries for this version of Codex."
)),
"{:?}",
config.startup_warnings
);
Ok(())
}
#[test]
fn permissions_profiles_allow_empty_filesystem_with_warning() -> std::io::Result<()> {
let config = load_workspace_permission_profile(PermissionProfileToml {
filesystem: Some(FilesystemPermissionsToml {
entries: BTreeMap::new(),
}),
network: None,
})?;
assert_eq!(
config.permissions.file_system_sandbox_policy,
FileSystemSandboxPolicy::restricted(Vec::new())
);
assert!(
config.startup_warnings.iter().any(|warning| warning.contains(
"Permissions profile `workspace` does not define any recognized filesystem entries for this version of Codex."
)),
"{:?}",
config.startup_warnings
);
Ok(())
}
#[test]
fn permissions_profiles_reject_project_root_parent_traversal() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
+11 -3
View File
@@ -1971,7 +1971,11 @@ impl Config {
let configured_network_proxy_config =
network_proxy_config_from_profile_network(profile.network.as_ref());
let (mut file_system_sandbox_policy, network_sandbox_policy) =
compile_permission_profile(permissions, default_permissions)?;
compile_permission_profile(
permissions,
default_permissions,
&mut startup_warnings,
)?;
let mut sandbox_policy = file_system_sandbox_policy
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?;
if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) {
@@ -2004,7 +2008,8 @@ impl Config {
}
}
}
let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy);
let file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, &resolved_cwd);
let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy);
(
configured_network_proxy_config,
@@ -2330,7 +2335,10 @@ impl Config {
if effective_sandbox_policy == original_sandbox_policy {
file_system_sandbox_policy
} else {
FileSystemSandboxPolicy::from(&effective_sandbox_policy)
FileSystemSandboxPolicy::from_legacy_sandbox_policy(
&effective_sandbox_policy,
&resolved_cwd,
)
};
let effective_network_sandbox_policy =
if effective_sandbox_policy == original_sandbox_policy {
+164 -47
View File
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::io;
use std::path::Component;
@@ -158,30 +159,27 @@ pub(crate) fn resolve_permission_profile<'a>(
pub(crate) fn compile_permission_profile(
permissions: &PermissionsToml,
profile_name: &str,
startup_warnings: &mut Vec<String>,
) -> io::Result<(FileSystemSandboxPolicy, NetworkSandboxPolicy)> {
let profile = resolve_permission_profile(permissions, profile_name)?;
let filesystem = profile.filesystem.as_ref().ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"permissions profile `{profile_name}` must define a `[permissions.{profile_name}.filesystem]` table"
),
)
})?;
if filesystem.is_empty() {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"permissions profile `{profile_name}` must define at least one filesystem entry"
),
));
}
let mut entries = Vec::new();
for (path, permission) in &filesystem.entries {
compile_filesystem_permission(path, permission, &mut entries)?;
if let Some(filesystem) = profile.filesystem.as_ref() {
if filesystem.is_empty() {
push_warning(
startup_warnings,
missing_filesystem_entries_warning(profile_name),
);
} else {
for (path, permission) in &filesystem.entries {
compile_filesystem_permission(path, permission, &mut entries, startup_warnings)?;
}
}
} else {
push_warning(
startup_warnings,
missing_filesystem_entries_warning(profile_name),
);
}
let network_sandbox_policy = compile_network_sandbox_policy(profile.network.as_ref());
@@ -207,16 +205,17 @@ fn compile_filesystem_permission(
path: &str,
permission: &FilesystemPermissionToml,
entries: &mut Vec<FileSystemSandboxEntry>,
startup_warnings: &mut Vec<String>,
) -> io::Result<()> {
match permission {
FilesystemPermissionToml::Access(access) => entries.push(FileSystemSandboxEntry {
path: compile_filesystem_path(path)?,
path: compile_filesystem_path(path, startup_warnings)?,
access: *access,
}),
FilesystemPermissionToml::Scoped(scoped_entries) => {
for (subpath, access) in scoped_entries {
entries.push(FileSystemSandboxEntry {
path: compile_scoped_filesystem_path(path, subpath)?,
path: compile_scoped_filesystem_path(path, subpath, startup_warnings)?,
access: *access,
});
}
@@ -225,8 +224,12 @@ fn compile_filesystem_permission(
Ok(())
}
fn compile_filesystem_path(path: &str) -> io::Result<FileSystemPath> {
if let Some(special) = parse_special_path(path)? {
fn compile_filesystem_path(
path: &str,
startup_warnings: &mut Vec<String>,
) -> io::Result<FileSystemPath> {
if let Some(special) = parse_special_path(path) {
maybe_push_unknown_special_path_warning(&special, startup_warnings);
return Ok(FileSystemPath::Special { value: special });
}
@@ -234,21 +237,33 @@ fn compile_filesystem_path(path: &str) -> io::Result<FileSystemPath> {
Ok(FileSystemPath::Path { path })
}
fn compile_scoped_filesystem_path(path: &str, subpath: &str) -> io::Result<FileSystemPath> {
fn compile_scoped_filesystem_path(
path: &str,
subpath: &str,
startup_warnings: &mut Vec<String>,
) -> io::Result<FileSystemPath> {
if subpath == "." {
return compile_filesystem_path(path);
return compile_filesystem_path(path, startup_warnings);
}
if let Some(special) = parse_special_path(path)? {
if !matches!(special, FileSystemSpecialPath::ProjectRoots { .. }) {
return Err(io::Error::new(
if let Some(special) = parse_special_path(path) {
let subpath = parse_relative_subpath(subpath)?;
let special = match special {
FileSystemSpecialPath::ProjectRoots { .. } => Ok(FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(subpath)),
}),
FileSystemSpecialPath::Unknown { path, .. } => Ok(FileSystemPath::Special {
value: FileSystemSpecialPath::unknown(path, Some(subpath)),
}),
_ => Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("filesystem path `{path}` does not support nested entries"),
));
)),
}?;
if let FileSystemPath::Special { value } = &special {
maybe_push_unknown_special_path_warning(value, startup_warnings);
}
return Ok(FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(parse_relative_subpath(subpath)?)),
});
return Ok(special);
}
let subpath = parse_relative_subpath(subpath)?;
@@ -257,33 +272,90 @@ fn compile_scoped_filesystem_path(path: &str, subpath: &str) -> io::Result<FileS
Ok(FileSystemPath::Path { path })
}
fn parse_special_path(path: &str) -> io::Result<Option<FileSystemSpecialPath>> {
let special = match path {
// WARNING: keep this parser forward-compatible.
// Adding a new `:special_path` must not make older Codex versions reject the
// config. Unknown values intentionally round-trip through
// `FileSystemSpecialPath::Unknown` so they can be surfaced as warnings and
// ignored, rather than aborting config load.
fn parse_special_path(path: &str) -> Option<FileSystemSpecialPath> {
match path {
":root" => Some(FileSystemSpecialPath::Root),
":minimal" => Some(FileSystemSpecialPath::Minimal),
":project_roots" => Some(FileSystemSpecialPath::project_roots(None)),
":tmpdir" => Some(FileSystemSpecialPath::Tmpdir),
_ if path.starts_with(':') => {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("unknown filesystem special path `{path}`"),
));
}
_ if path.starts_with(':') => Some(FileSystemSpecialPath::unknown(path, None)),
_ => None,
};
Ok(special)
}
}
fn parse_absolute_path(path: &str) -> io::Result<AbsolutePathBuf> {
let path_ref = Path::new(path);
if !path_ref.is_absolute() && path != "~" && !path.starts_with("~/") {
parse_absolute_path_for_platform(path, cfg!(windows))
}
fn parse_absolute_path_for_platform(path: &str, is_windows: bool) -> io::Result<AbsolutePathBuf> {
let path_ref = normalize_absolute_path_for_platform(path, is_windows);
if !is_absolute_path_for_platform(path, path_ref.as_ref(), is_windows)
&& path != "~"
&& !path.starts_with("~/")
{
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("filesystem path `{path}` must be absolute, use `~/...`, or start with `:`"),
));
}
AbsolutePathBuf::from_absolute_path(path_ref)
AbsolutePathBuf::from_absolute_path(path_ref.as_ref())
}
fn is_absolute_path_for_platform(path: &str, normalized_path: &Path, is_windows: bool) -> bool {
if is_windows {
is_windows_absolute_path(path)
|| is_windows_absolute_path(&normalized_path.to_string_lossy())
} else {
normalized_path.is_absolute()
}
}
fn normalize_absolute_path_for_platform(path: &str, is_windows: bool) -> Cow<'_, Path> {
if !is_windows {
return Cow::Borrowed(Path::new(path));
}
match normalize_windows_device_path(path) {
Some(normalized) => Cow::Owned(PathBuf::from(normalized)),
None => Cow::Borrowed(Path::new(path)),
}
}
fn normalize_windows_device_path(path: &str) -> Option<String> {
if let Some(unc) = path.strip_prefix(r"\\?\UNC\") {
return Some(format!(r"\\{unc}"));
}
if let Some(unc) = path.strip_prefix(r"\\.\UNC\") {
return Some(format!(r"\\{unc}"));
}
if let Some(path) = path.strip_prefix(r"\\?\")
&& is_windows_drive_absolute_path(path)
{
return Some(path.to_string());
}
if let Some(path) = path.strip_prefix(r"\\.\")
&& is_windows_drive_absolute_path(path)
{
return Some(path.to_string());
}
None
}
fn is_windows_absolute_path(path: &str) -> bool {
is_windows_drive_absolute_path(path) || path.starts_with(r"\\")
}
fn is_windows_drive_absolute_path(path: &str) -> bool {
let bytes = path.as_bytes();
bytes.len() >= 3
&& bytes[0].is_ascii_alphabetic()
&& bytes[1] == b':'
&& matches!(bytes[2], b'\\' | b'/')
}
fn parse_relative_subpath(subpath: &str) -> io::Result<PathBuf> {
@@ -304,3 +376,48 @@ fn parse_relative_subpath(subpath: &str) -> io::Result<PathBuf> {
),
))
}
fn push_warning(startup_warnings: &mut Vec<String>, message: String) {
tracing::warn!("{message}");
startup_warnings.push(message);
}
fn missing_filesystem_entries_warning(profile_name: &str) -> String {
format!(
"Permissions profile `{profile_name}` does not define any recognized filesystem entries for this version of Codex. Filesystem access will remain restricted. Upgrade Codex if this profile expects filesystem permissions."
)
}
fn maybe_push_unknown_special_path_warning(
special: &FileSystemSpecialPath,
startup_warnings: &mut Vec<String>,
) {
let FileSystemSpecialPath::Unknown { path, subpath } = special else {
return;
};
push_warning(
startup_warnings,
match subpath.as_deref() {
Some(subpath) => format!(
"Configured filesystem path `{path}` with nested entry `{}` is not recognized by this version of Codex and will be ignored. Upgrade Codex if this path is required.",
subpath.display()
),
None => format!(
"Configured filesystem path `{path}` is not recognized by this version of Codex and will be ignored. Upgrade Codex if this path is required."
),
},
);
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn normalize_absolute_path_for_platform_simplifies_windows_verbatim_paths() {
let parsed =
normalize_absolute_path_for_platform(r"\\?\D:\c\x\worktrees\2508\swift-base", true);
assert_eq!(parsed, PathBuf::from(r"D:\c\x\worktrees\2508\swift-base"));
}
}
@@ -421,9 +421,23 @@ fn for_prompt_rewrites_image_generation_calls_when_images_are_supported() {
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputImage {
image_url: "data:image/png;base64,Zm9v".to_string(),
}],
content: vec![
ContentItem::InputText {
text: "Image Generation Call".to_string(),
},
ContentItem::InputText {
text: "Image ID: ig_123".to_string(),
},
ContentItem::InputText {
text: "Prompt: lobster".to_string(),
},
ContentItem::InputImage {
image_url: "data:image/png;base64,Zm9v".to_string(),
},
ContentItem::InputText {
text: "Saved to: CWD".to_string(),
},
],
end_turn: None,
phase: None,
},
@@ -475,10 +489,24 @@ fn for_prompt_rewrites_image_generation_calls_when_images_are_unsupported() {
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: "image content omitted because you do not support image input"
.to_string(),
}],
content: vec![
ContentItem::InputText {
text: "Image Generation Call".to_string(),
},
ContentItem::InputText {
text: "Image ID: ig_123".to_string(),
},
ContentItem::InputText {
text: "Prompt: lobster".to_string(),
},
ContentItem::InputText {
text: "image content omitted because you do not support image input"
.to_string(),
},
ContentItem::InputText {
text: "Saved to: CWD".to_string(),
},
],
end_turn: None,
phase: None,
},
+22 -2
View File
@@ -215,17 +215,37 @@ pub(crate) fn rewrite_image_generation_calls_for_stateless_input(items: &mut Vec
*items = original_items
.into_iter()
.map(|item| match item {
ResponseItem::ImageGenerationCall { result, .. } => {
ResponseItem::ImageGenerationCall {
id,
revised_prompt,
result,
..
} => {
let image_url = if result.starts_with("data:") {
result
} else {
format!("data:image/png;base64,{result}")
};
let revised_prompt = revised_prompt.unwrap_or_default();
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputImage { image_url }],
content: vec![
ContentItem::InputText {
text: "Image Generation Call".to_string(),
},
ContentItem::InputText {
text: format!("Image ID: {id}"),
},
ContentItem::InputText {
text: format!("Prompt: {revised_prompt}"),
},
ContentItem::InputImage { image_url },
ContentItem::InputText {
text: "Saved to: CWD".to_string(),
},
],
end_turn: None,
phase: None,
}
@@ -43,7 +43,6 @@ fn build_permissions_update_item(
Some(DeveloperInstructions::from_policy(
next.sandbox_policy.get(),
next.approval_policy.value(),
next.features.enabled(Feature::GuardianApproval),
exec_policy,
&next.cwd,
next.features.enabled(Feature::RequestPermissions),
+4
View File
@@ -1569,6 +1569,7 @@ prefix_rule(pattern=["git"], decision="prompt")
AskForApproval::Reject(RejectConfig {
sandbox_approval: false,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}),
&SandboxPolicy::new_read_only_policy(),
@@ -1590,6 +1591,7 @@ prefix_rule(pattern=["git"], decision="prompt")
approval_policy: AskForApproval::Reject(RejectConfig {
sandbox_approval: true,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}),
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
@@ -1626,6 +1628,7 @@ prefix_rule(pattern=["git"], decision="prompt")
approval_policy: AskForApproval::Reject(RejectConfig {
sandbox_approval: true,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}),
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
@@ -1660,6 +1663,7 @@ prefix_rule(pattern=["git"], decision="prompt")
approval_policy: AskForApproval::Reject(RejectConfig {
sandbox_approval: false,
rules: true,
request_permissions: false,
mcp_elicitations: false,
}),
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
+31 -5
View File
@@ -93,6 +93,8 @@ pub enum Feature {
ApplyPatchFreeform,
/// Allow requesting additional filesystem permissions while staying sandboxed.
RequestPermissions,
/// Expose the built-in request_permissions tool.
RequestPermissionsTool,
/// Allow the model to request web searches that fetch live content.
WebSearchRequest,
/// Allow the model to request web searches that fetch cached content.
@@ -147,7 +149,7 @@ pub enum Feature {
Steer,
/// Allow request_user_input in Default collaboration mode.
DefaultModeRequestUserInput,
/// Enable guardian subagent approvals.
/// Enable automatic review for approval prompts.
GuardianApproval,
/// Enable collaboration modes (Plan, Default).
/// Kept for config backward compatibility; behavior is always collaboration-modes-enabled.
@@ -581,6 +583,12 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::RequestPermissionsTool,
key: "request_permissions_tool",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::UseLinuxSandboxBwrap,
key: "use_linux_sandbox_bwrap",
@@ -702,8 +710,8 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::GuardianApproval,
key: "guardian_approval",
stage: Stage::Experimental {
name: "Guardian approvals",
menu_description: "Let a guardian subagent review `on-request` approval prompts instead of showing them to you, including sandbox escapes and blocked network access.",
name: "Automatic approval review",
menu_description: "Dispatch `on-request` approval prompts (for e.g. sandbox escapes or blocked network access) to a carefully-prompted security reviewer subagent rather than blocking the agent on your input.",
announcement: "",
},
default_enabled: false,
@@ -909,17 +917,35 @@ mod tests {
let stage = spec.stage;
assert!(matches!(stage, Stage::Experimental { .. }));
assert_eq!(stage.experimental_menu_name(), Some("Guardian approvals"));
assert_eq!(
stage.experimental_menu_name(),
Some("Automatic approval review")
);
assert_eq!(
stage.experimental_menu_description().map(str::to_owned),
Some(
"Let a guardian subagent review `on-request` approval prompts instead of showing them to you, including sandbox escapes and blocked network access.".to_string()
"Dispatch `on-request` approval prompts (for e.g. sandbox escapes or blocked network access) to a carefully-prompted security reviewer subagent rather than blocking the agent on your input.".to_string()
)
);
assert_eq!(stage.experimental_announcement(), None);
assert_eq!(Feature::GuardianApproval.default_enabled(), false);
}
#[test]
fn request_permissions_is_under_development() {
assert_eq!(Feature::RequestPermissions.stage(), Stage::UnderDevelopment);
assert_eq!(Feature::RequestPermissions.default_enabled(), false);
}
#[test]
fn request_permissions_tool_is_under_development() {
assert_eq!(
Feature::RequestPermissionsTool.stage(),
Stage::UnderDevelopment
);
assert_eq!(Feature::RequestPermissionsTool.default_enabled(), false);
}
#[test]
fn image_generation_is_under_development() {
assert_eq!(Feature::ImageGeneration.stage(), Stage::UnderDevelopment);
+270 -77
View File
@@ -12,15 +12,19 @@
//! 4. Approve only low- and medium-risk actions (`risk_score < 80`).
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::WarningEvent;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use serde::Serialize;
use serde_json::Value;
@@ -66,7 +70,7 @@ const GUARDIAN_RECENT_ENTRY_LIMIT: usize = 40;
const GUARDIAN_TRUNCATION_TAG: &str = "guardian_truncated";
pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!(
"Guardian rejected this action due to unacceptable risk. ",
"This action was rejected due to unacceptable risk. ",
"The agent must not attempt to achieve the same outcome via workaround, ",
"indirect execution, or policy circumvention. ",
"Proceed only with a materially safer alternative, or stop and request user input.",
@@ -89,12 +93,6 @@ pub(crate) fn is_guardian_subagent_source(
)
}
/// Canonical description of the action the guardian is being asked to review.
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct GuardianReviewRequest {
pub(crate) action: Value,
}
/// Coarse risk label paired with the numeric `risk_score`.
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq)]
#[serde(rename_all = "lowercase")]
@@ -120,6 +118,66 @@ pub(crate) struct GuardianAssessment {
evidence: Vec<GuardianEvidence>,
}
#[derive(Debug, Clone, PartialEq)]
pub(crate) enum GuardianApprovalRequest {
Shell {
command: Vec<String>,
cwd: PathBuf,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<PermissionProfile>,
justification: Option<String>,
},
ExecCommand {
command: Vec<String>,
cwd: PathBuf,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<PermissionProfile>,
justification: Option<String>,
tty: bool,
},
#[cfg(unix)]
Execve {
tool_name: String,
program: String,
argv: Vec<String>,
cwd: PathBuf,
additional_permissions: Option<PermissionProfile>,
},
ApplyPatch {
cwd: PathBuf,
files: Vec<AbsolutePathBuf>,
change_count: usize,
patch: String,
},
NetworkAccess {
target: String,
host: String,
protocol: NetworkApprovalProtocol,
port: u16,
},
McpToolCall {
server: String,
tool_name: String,
arguments: Option<Value>,
connector_id: Option<String>,
connector_name: Option<String>,
connector_description: Option<String>,
tool_title: Option<String>,
tool_description: Option<String>,
annotations: Option<GuardianMcpAnnotations>,
},
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub(crate) struct GuardianMcpAnnotations {
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) destructive_hint: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) open_world_hint: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) read_only_hint: Option<bool>,
}
/// Transcript entry retained for guardian review after filtering.
#[derive(Debug, PartialEq, Eq)]
struct GuardianTranscriptEntry {
@@ -164,14 +222,11 @@ impl GuardianTranscriptEntryKind {
async fn run_guardian_review(
session: Arc<Session>,
turn: Arc<TurnContext>,
request: GuardianReviewRequest,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
) -> ReviewDecision {
session
.notify_background_event(
turn.as_ref(),
"Guardian assessing approval request...".to_string(),
)
.notify_background_event(turn.as_ref(), "Reviewing approval request...".to_string())
.await;
let prompt_items = build_guardian_prompt_items(session.as_ref(), retry_reason, request).await;
@@ -199,14 +254,15 @@ async fn run_guardian_review(
Some(Err(err)) => GuardianAssessment {
risk_level: GuardianRiskLevel::High,
risk_score: 100,
rationale: format!("Guardian review failed: {err}"),
rationale: format!("Automatic approval review failed: {err}"),
evidence: vec![],
},
None => GuardianAssessment {
risk_level: GuardianRiskLevel::High,
risk_score: 100,
rationale: "Guardian review timed out while evaluating the requested approval."
.to_string(),
rationale:
"Automatic approval review timed out while evaluating the requested approval."
.to_string(),
evidence: vec![],
},
};
@@ -216,8 +272,7 @@ async fn run_guardian_review(
// Emit a concise warning so the parent turn has an auditable summary of the
// guardian decision without needing the full subagent transcript.
let warning = format!(
"Guardian {verdict} approval request ({}/100, {}): {}",
assessment.risk_score,
"Automatic approval review {verdict} (risk: {}): {}",
assessment.risk_level.as_str(),
assessment.rationale
);
@@ -239,7 +294,7 @@ async fn run_guardian_review(
pub(crate) async fn review_approval_request(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
request: GuardianReviewRequest,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
) -> ReviewDecision {
run_guardian_review(Arc::clone(session), Arc::clone(turn), request, retry_reason).await
@@ -256,11 +311,11 @@ pub(crate) async fn review_approval_request(
async fn build_guardian_prompt_items(
session: &Session,
retry_reason: Option<String>,
request: GuardianReviewRequest,
request: GuardianApprovalRequest,
) -> Vec<UserInput> {
let history = session.clone_history().await;
let transcript_entries = collect_guardian_transcript_entries(history.raw_items());
let planned_action_json = format_guardian_action_pretty(&request.action);
let planned_action_json = format_guardian_action_pretty(&request);
let (transcript_entries, omission_note) =
render_guardian_transcript_entries(transcript_entries.as_slice());
@@ -400,6 +455,13 @@ fn render_guardian_transcript_entries(
fn collect_guardian_transcript_entries(items: &[ResponseItem]) -> Vec<GuardianTranscriptEntry> {
let mut entries = Vec::new();
let mut tool_names_by_call_id = HashMap::new();
let non_empty_entry = |kind, text: String| {
(!text.trim().is_empty()).then_some(GuardianTranscriptEntry { kind, text })
};
let content_entry =
|kind, content| content_items_to_text(content).and_then(|text| non_empty_entry(kind, text));
let serialized_entry =
|kind, serialized: Option<String>| serialized.and_then(|text| non_empty_entry(kind, text));
for item in items {
let entry = match item {
@@ -407,25 +469,16 @@ fn collect_guardian_transcript_entries(items: &[ResponseItem]) -> Vec<GuardianTr
if is_contextual_user_message_content(content) {
None
} else {
content_items_to_text(content).map(|text| GuardianTranscriptEntry {
kind: GuardianTranscriptEntryKind::User,
text,
})
content_entry(GuardianTranscriptEntryKind::User, content)
}
}
ResponseItem::Message { role, content, .. } if role == "assistant" => {
content_items_to_text(content).map(|text| GuardianTranscriptEntry {
kind: GuardianTranscriptEntryKind::Assistant,
text,
})
content_entry(GuardianTranscriptEntryKind::Assistant, content)
}
ResponseItem::LocalShellCall { action, .. } => serde_json::to_string(action)
.ok()
.filter(|text| !text.trim().is_empty())
.map(|text| GuardianTranscriptEntry {
kind: GuardianTranscriptEntryKind::Tool("tool shell call".to_string()),
text,
}),
ResponseItem::LocalShellCall { action, .. } => serialized_entry(
GuardianTranscriptEntryKind::Tool("tool shell call".to_string()),
serde_json::to_string(action).ok(),
),
ResponseItem::FunctionCall {
call_id,
name,
@@ -450,28 +503,26 @@ fn collect_guardian_transcript_entries(items: &[ResponseItem]) -> Vec<GuardianTr
text: input.clone(),
})
}
ResponseItem::WebSearchCall { action, .. } => action
.as_ref()
.and_then(|action| serde_json::to_string(action).ok())
.filter(|text| !text.trim().is_empty())
.map(|text| GuardianTranscriptEntry {
kind: GuardianTranscriptEntryKind::Tool("tool web_search call".to_string()),
text,
}),
ResponseItem::WebSearchCall { action, .. } => action.as_ref().and_then(|action| {
serialized_entry(
GuardianTranscriptEntryKind::Tool("tool web_search call".to_string()),
serde_json::to_string(action).ok(),
)
}),
ResponseItem::FunctionCallOutput { call_id, output }
| ResponseItem::CustomToolCallOutput { call_id, output } => output
.body
.to_text()
.filter(|text| !text.trim().is_empty())
.map(|text| GuardianTranscriptEntry {
kind: GuardianTranscriptEntryKind::Tool(
tool_names_by_call_id.get(call_id).map_or_else(
|| "tool result".to_string(),
|name| format!("tool {name} result"),
| ResponseItem::CustomToolCallOutput { call_id, output } => {
output.body.to_text().and_then(|text| {
non_empty_entry(
GuardianTranscriptEntryKind::Tool(
tool_names_by_call_id.get(call_id).map_or_else(
|| "tool result".to_string(),
|name| format!("tool {name} result"),
),
),
),
text,
}),
text,
)
})
}
_ => None,
};
@@ -506,6 +557,13 @@ async fn run_guardian_subagent(
.models_manager
.list_models(crate::models_manager::manager::RefreshStrategy::Offline)
.await;
let preferred_reasoning_effort = |supports_low: bool, fallback| {
if supports_low {
Some(codex_protocol::openai_models::ReasoningEffort::Low)
} else {
fallback
}
};
// Prefer `GUARDIAN_PREFERRED_MODEL` when the active provider exposes it,
// but fall back to the parent turn's active model so guardian does not
// become a blanket deny on providers or test environments that do not
@@ -514,28 +572,23 @@ async fn run_guardian_subagent(
.iter()
.find(|preset| preset.model == GUARDIAN_PREFERRED_MODEL);
let (guardian_model, guardian_reasoning_effort) = if let Some(preset) = preferred_model {
let reasoning_effort = if preset
.supported_reasoning_efforts
.iter()
.any(|effort| effort.effort == codex_protocol::openai_models::ReasoningEffort::Low)
{
Some(codex_protocol::openai_models::ReasoningEffort::Low)
} else {
Some(preset.default_reasoning_effort)
};
let reasoning_effort = preferred_reasoning_effort(
preset
.supported_reasoning_efforts
.iter()
.any(|effort| effort.effort == codex_protocol::openai_models::ReasoningEffort::Low),
Some(preset.default_reasoning_effort),
);
(GUARDIAN_PREFERRED_MODEL.to_string(), reasoning_effort)
} else {
let reasoning_effort = if turn
.model_info
.supported_reasoning_levels
.iter()
.any(|preset| preset.effort == codex_protocol::openai_models::ReasoningEffort::Low)
{
Some(codex_protocol::openai_models::ReasoningEffort::Low)
} else {
let reasoning_effort = preferred_reasoning_effort(
turn.model_info
.supported_reasoning_levels
.iter()
.any(|preset| preset.effort == codex_protocol::openai_models::ReasoningEffort::Low),
turn.reasoning_effort
.or(turn.model_info.default_reasoning_level)
};
.or(turn.model_info.default_reasoning_level),
);
(turn.model_info.slug.clone(), reasoning_effort)
};
let guardian_config = build_guardian_subagent_config(
@@ -678,9 +731,149 @@ fn truncate_guardian_action_value(value: Value) -> Value {
}
}
fn format_guardian_action_pretty(action: &Value) -> String {
serde_json::to_string_pretty(&truncate_guardian_action_value(action.clone()))
.unwrap_or_else(|_| "null".to_string())
fn format_guardian_action_pretty(action: &GuardianApprovalRequest) -> String {
let mut value = match action {
GuardianApprovalRequest::Shell {
command,
cwd,
sandbox_permissions,
additional_permissions,
justification,
} => {
let mut action = serde_json::json!({
"tool": "shell",
"command": command,
"cwd": cwd,
"sandbox_permissions": sandbox_permissions,
"additional_permissions": additional_permissions,
"justification": justification,
});
if let Some(action) = action.as_object_mut() {
if additional_permissions.is_none() {
action.remove("additional_permissions");
}
if justification.is_none() {
action.remove("justification");
}
}
action
}
GuardianApprovalRequest::ExecCommand {
command,
cwd,
sandbox_permissions,
additional_permissions,
justification,
tty,
} => {
let mut action = serde_json::json!({
"tool": "exec_command",
"command": command,
"cwd": cwd,
"sandbox_permissions": sandbox_permissions,
"additional_permissions": additional_permissions,
"justification": justification,
"tty": tty,
});
if let Some(action) = action.as_object_mut() {
if additional_permissions.is_none() {
action.remove("additional_permissions");
}
if justification.is_none() {
action.remove("justification");
}
}
action
}
#[cfg(unix)]
GuardianApprovalRequest::Execve {
tool_name,
program,
argv,
cwd,
additional_permissions,
} => {
let mut action = serde_json::json!({
"tool": tool_name,
"program": program,
"argv": argv,
"cwd": cwd,
"additional_permissions": additional_permissions,
});
if let Some(action) = action.as_object_mut()
&& additional_permissions.is_none()
{
action.remove("additional_permissions");
}
action
}
GuardianApprovalRequest::ApplyPatch {
cwd,
files,
change_count,
patch,
} => serde_json::json!({
"tool": "apply_patch",
"cwd": cwd,
"files": files,
"change_count": change_count,
"patch": patch,
}),
GuardianApprovalRequest::NetworkAccess {
target,
host,
protocol,
port,
} => serde_json::json!({
"tool": "network_access",
"target": target,
"host": host,
"protocol": protocol,
"port": port,
}),
GuardianApprovalRequest::McpToolCall {
server,
tool_name,
arguments,
connector_id,
connector_name,
connector_description,
tool_title,
tool_description,
annotations,
} => {
let mut action = serde_json::json!({
"tool": "mcp_tool_call",
"server": server,
"tool_name": tool_name,
"arguments": arguments,
"connector_id": connector_id,
"connector_name": connector_name,
"connector_description": connector_description,
"tool_title": tool_title,
"tool_description": tool_description,
"annotations": annotations,
});
if let Some(action) = action.as_object_mut() {
for key in [
("arguments", arguments.is_none()),
("connector_id", connector_id.is_none()),
("connector_name", connector_name.is_none()),
("connector_description", connector_description.is_none()),
("tool_title", tool_title.is_none()),
("tool_description", tool_description.is_none()),
("annotations", annotations.is_none()),
] {
if key.1 {
action.remove(key.0);
}
}
}
action
}
};
value = truncate_guardian_action_value(value);
serde_json::to_string_pretty(&value).unwrap_or_else(|_| "null".to_string())
}
fn guardian_truncate_text(content: &str, token_cap: usize) -> String {
+25 -25
View File
@@ -6,6 +6,7 @@ use crate::config_loader::FeatureRequirementsToml;
use crate::config_loader::NetworkConstraints;
use crate::config_loader::RequirementSource;
use crate::config_loader::Sourced;
use crate::test_support;
use codex_network_proxy::NetworkProxyConfig;
use codex_protocol::models::ContentItem;
use core_test_support::context_snapshot;
@@ -22,6 +23,8 @@ use insta::assert_snapshot;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::path::PathBuf;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
#[test]
fn build_guardian_transcript_keeps_original_numbering() {
@@ -154,21 +157,18 @@ fn guardian_truncate_text_keeps_prefix_suffix_and_xml_marker() {
#[test]
fn format_guardian_action_pretty_truncates_large_string_fields() {
let action = serde_json::json!({
"tool": "apply_patch",
"cwd": PathBuf::from("/tmp"),
"files": Vec::<String>::new(),
"change_count": 1usize,
"patch": "line\n".repeat(10_000),
});
let patch = "line\n".repeat(10_000);
let action = GuardianApprovalRequest::ApplyPatch {
cwd: PathBuf::from("/tmp"),
files: Vec::new(),
change_count: 1usize,
patch: patch.clone(),
};
let rendered = format_guardian_action_pretty(&action);
let original_patch = action["patch"]
.as_str()
.expect("test patch should serialize as a string");
assert!(rendered.contains("\"tool\": \"apply_patch\""));
assert!(rendered.len() < original_patch.len());
assert!(rendered.len() < patch.len());
}
#[test]
@@ -253,7 +253,7 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
let mut config = (*turn.config).clone();
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
let config = Arc::new(config);
let models_manager = Arc::new(crate::test_support::models_manager_with_provider(
let models_manager = Arc::new(test_support::models_manager_with_provider(
config.codex_home.clone(),
Arc::clone(&session.services.auth_manager),
config.model_provider.clone(),
@@ -307,19 +307,19 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
let prompt = build_guardian_prompt_items(
session.as_ref(),
Some("Sandbox denied outbound git push to github.com.".to_string()),
GuardianReviewRequest {
action: serde_json::json!({
"tool": "shell",
"command": [
"git",
"push",
"origin",
"guardian-approval-mvp"
],
"cwd": "/repo/codex-rs/core",
"sandbox_permissions": crate::sandboxing::SandboxPermissions::UseDefault,
"justification": "Need to push the reviewed docs fix to the repo remote.",
}),
GuardianApprovalRequest::Shell {
command: vec![
"git".to_string(),
"push".to_string(),
"origin".to_string(),
"guardian-approval-mvp".to_string(),
],
cwd: PathBuf::from("/repo/codex-rs/core"),
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
additional_permissions: None,
justification: Some(
"Need to push the reviewed docs fix to the repo remote.".to_string(),
),
},
)
.await;
+2 -1
View File
@@ -33,7 +33,8 @@ pub async fn spawn_command_under_linux_sandbox<P>(
where
P: AsRef<Path>,
{
let file_system_sandbox_policy = FileSystemSandboxPolicy::from(sandbox_policy);
let file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_policy_cwd);
let network_sandbox_policy = NetworkSandboxPolicy::from(sandbox_policy);
let args = create_linux_sandbox_command_args_for_policies(
command,
@@ -1739,6 +1739,7 @@ mod tests {
RejectConfig {
sandbox_approval: false,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}
)));
@@ -1751,6 +1752,7 @@ mod tests {
RejectConfig {
sandbox_approval: false,
rules: false,
request_permissions: false,
mcp_elicitations: true,
}
)));
+44 -107
View File
@@ -18,7 +18,8 @@ use crate::config::edit::ConfigEditsBuilder;
use crate::config::types::AppToolApproval;
use crate::connectors;
use crate::features::Feature;
use crate::guardian::GuardianReviewRequest;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianMcpAnnotations;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
@@ -574,88 +575,23 @@ fn persistent_mcp_tool_approval_key(
fn build_guardian_mcp_tool_review_request(
invocation: &McpInvocation,
metadata: Option<&McpToolApprovalMetadata>,
) -> GuardianReviewRequest {
let mut action = serde_json::Map::from_iter([
(
"tool".to_string(),
serde_json::Value::String("mcp_tool_call".to_string()),
),
(
"server".to_string(),
serde_json::Value::String(invocation.server.clone()),
),
(
"tool_name".to_string(),
serde_json::Value::String(invocation.tool.clone()),
),
]);
if let Some(arguments) = invocation.arguments.clone() {
action.insert("arguments".to_string(), arguments);
}
if let Some(metadata) = metadata {
if let Some(connector_id) = metadata.connector_id.as_ref() {
action.insert(
"connector_id".to_string(),
serde_json::Value::String(connector_id.clone()),
);
}
if let Some(connector_name) = metadata.connector_name.as_ref() {
action.insert(
"connector_name".to_string(),
serde_json::Value::String(connector_name.clone()),
);
}
if let Some(connector_description) = metadata.connector_description.as_ref() {
action.insert(
"connector_description".to_string(),
serde_json::Value::String(connector_description.clone()),
);
}
if let Some(tool_title) = metadata.tool_title.as_ref() {
action.insert(
"tool_title".to_string(),
serde_json::Value::String(tool_title.clone()),
);
}
if let Some(tool_description) = metadata.tool_description.as_ref() {
action.insert(
"tool_description".to_string(),
serde_json::Value::String(tool_description.clone()),
);
}
if let Some(annotations) = metadata.annotations.as_ref() {
let mut annotation_map = serde_json::Map::new();
if let Some(destructive_hint) = annotations.destructive_hint {
annotation_map.insert(
"destructive_hint".to_string(),
serde_json::Value::Bool(destructive_hint),
);
}
if let Some(open_world_hint) = annotations.open_world_hint {
annotation_map.insert(
"open_world_hint".to_string(),
serde_json::Value::Bool(open_world_hint),
);
}
if let Some(read_only_hint) = annotations.read_only_hint {
annotation_map.insert(
"read_only_hint".to_string(),
serde_json::Value::Bool(read_only_hint),
);
}
if !annotation_map.is_empty() {
action.insert(
"annotations".to_string(),
serde_json::Value::Object(annotation_map),
);
}
}
}
GuardianReviewRequest {
action: serde_json::Value::Object(action),
) -> GuardianApprovalRequest {
GuardianApprovalRequest::McpToolCall {
server: invocation.server.clone(),
tool_name: invocation.tool.clone(),
arguments: invocation.arguments.clone(),
connector_id: metadata.and_then(|metadata| metadata.connector_id.clone()),
connector_name: metadata.and_then(|metadata| metadata.connector_name.clone()),
connector_description: metadata.and_then(|metadata| metadata.connector_description.clone()),
tool_title: metadata.and_then(|metadata| metadata.tool_title.clone()),
tool_description: metadata.and_then(|metadata| metadata.tool_description.clone()),
annotations: metadata
.and_then(|metadata| metadata.annotations.as_ref())
.map(|annotations| GuardianMcpAnnotations {
destructive_hint: annotations.destructive_hint,
open_world_hint: annotations.open_world_hint,
read_only_hint: annotations.read_only_hint,
}),
}
}
@@ -1599,20 +1535,18 @@ mod tests {
assert_eq!(
request,
GuardianReviewRequest {
action: serde_json::json!({
"tool": "mcp_tool_call",
"server": CODEX_APPS_MCP_SERVER_NAME,
"tool_name": "browser_navigate",
"arguments": {
"url": "https://example.com",
},
"connector_id": "playwright",
"connector_name": "Playwright",
"connector_description": "Browser automation",
"tool_title": "Navigate",
"tool_description": "Open a page",
}),
GuardianApprovalRequest::McpToolCall {
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
tool_name: "browser_navigate".to_string(),
arguments: Some(serde_json::json!({
"url": "https://example.com",
})),
connector_id: Some("playwright".to_string()),
connector_name: Some("Playwright".to_string()),
connector_description: Some("Browser automation".to_string()),
tool_title: Some("Navigate".to_string()),
tool_description: Some("Open a page".to_string()),
annotations: None,
}
);
}
@@ -1637,16 +1571,19 @@ mod tests {
assert_eq!(
request,
GuardianReviewRequest {
action: serde_json::json!({
"tool": "mcp_tool_call",
"server": "custom_server",
"tool_name": "dangerous_tool",
"annotations": {
"destructive_hint": true,
"open_world_hint": true,
"read_only_hint": false,
},
GuardianApprovalRequest::McpToolCall {
server: "custom_server".to_string(),
tool_name: "dangerous_tool".to_string(),
arguments: None,
connector_id: None,
connector_name: None,
connector_description: None,
tool_title: None,
tool_description: None,
annotations: Some(GuardianMcpAnnotations {
destructive_hint: Some(true),
open_world_hint: Some(true),
read_only_hint: Some(false),
}),
}
);
+83
View File
@@ -10,6 +10,7 @@ use super::plugin_manifest_name;
use super::plugin_manifest_paths;
use super::store::DEFAULT_PLUGIN_VERSION;
use super::store::PluginId;
use super::store::PluginIdError;
use super::store::PluginInstallResult;
use super::store::PluginStore;
use super::store::PluginStoreError;
@@ -18,6 +19,8 @@ use crate::config::Config;
use crate::config::ConfigService;
use crate::config::ConfigServiceError;
use crate::config::ConfigToml;
use crate::config::edit::ConfigEdit;
use crate::config::edit::ConfigEditsBuilder;
use crate::config::profile::ConfigProfile;
use crate::config::types::McpServerConfig;
use crate::config::types::PluginConfig;
@@ -290,6 +293,24 @@ impl PluginsManager {
Ok(result)
}
pub async fn uninstall_plugin(&self, plugin_id: String) -> Result<(), PluginUninstallError> {
let plugin_id = PluginId::parse(&plugin_id)?;
let store = self.store.clone();
let plugin_id_for_store = plugin_id.clone();
tokio::task::spawn_blocking(move || store.uninstall(&plugin_id_for_store))
.await
.map_err(PluginUninstallError::join)??;
ConfigEditsBuilder::new(&self.codex_home)
.with_edits([ConfigEdit::ClearPath {
segments: vec!["plugins".to_string(), plugin_id.as_key()],
}])
.apply()
.await?;
Ok(())
}
pub fn list_marketplaces_for_config(
&self,
config: &Config,
@@ -428,6 +449,31 @@ impl PluginInstallError {
}
}
#[derive(Debug, thiserror::Error)]
pub enum PluginUninstallError {
#[error("{0}")]
InvalidPluginId(#[from] PluginIdError),
#[error("{0}")]
Store(#[from] PluginStoreError),
#[error("{0}")]
Config(#[from] anyhow::Error),
#[error("failed to join plugin uninstall task: {0}")]
Join(#[from] tokio::task::JoinError),
}
impl PluginUninstallError {
fn join(source: tokio::task::JoinError) -> Self {
Self::Join(source)
}
pub fn is_invalid_request(&self) -> bool {
matches!(self, Self::InvalidPluginId(_))
}
}
fn plugins_feature_enabled_from_stack(config_layer_stack: &ConfigLayerStack) -> bool {
// Plugins are intentionally opt-in from the persisted user config only. Project config
// layers should not be able to enable plugin loading for a checkout.
@@ -1553,6 +1599,43 @@ mod tests {
assert!(config.contains("enabled = true"));
}
#[tokio::test]
async fn uninstall_plugin_removes_cache_and_config_entry() {
let tmp = tempfile::tempdir().unwrap();
write_plugin(
&tmp.path().join("plugins/cache/debug"),
"sample-plugin/local",
"sample-plugin",
);
write_file(
&tmp.path().join(CONFIG_TOML_FILE),
r#"[features]
plugins = true
[plugins."sample-plugin@debug"]
enabled = true
"#,
);
let manager = PluginsManager::new(tmp.path().to_path_buf());
manager
.uninstall_plugin("sample-plugin@debug".to_string())
.await
.unwrap();
manager
.uninstall_plugin("sample-plugin@debug".to_string())
.await
.unwrap();
assert!(
!tmp.path()
.join("plugins/cache/debug/sample-plugin")
.exists()
);
let config = fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap();
assert!(!config.contains(r#"[plugins."sample-plugin@debug"]"#));
}
#[tokio::test]
async fn list_marketplaces_includes_enabled_state() {
let tmp = tempfile::tempdir().unwrap();
+1
View File
@@ -17,6 +17,7 @@ pub use manager::PluginCapabilitySummary;
pub use manager::PluginInstallError;
pub use manager::PluginInstallRequest;
pub use manager::PluginLoadOutcome;
pub use manager::PluginUninstallError;
pub use manager::PluginsManager;
pub use manager::load_plugin_apps;
pub(crate) use manager::plugin_namespace_for_skill_path;
+9
View File
@@ -135,6 +135,15 @@ impl PluginStore {
installed_path,
})
}
pub fn uninstall(&self, plugin_id: &PluginId) -> Result<(), PluginStoreError> {
let plugin_path = self
.root
.as_path()
.join(&plugin_id.marketplace_name)
.join(&plugin_id.plugin_name);
remove_existing_target(&plugin_path)
}
}
#[derive(Debug, thiserror::Error)]
+1
View File
@@ -144,6 +144,7 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option<EventPersistenceMode> {
| EventMsg::TerminalInteraction(_)
| EventMsg::ExecCommandOutputDelta(_)
| EventMsg::ExecApprovalRequest(_)
| EventMsg::RequestPermissions(_)
| EventMsg::RequestUserInput(_)
| EventMsg::ElicitationRequest(_)
| EventMsg::ApplyPatchApprovalRequest(_)
+46
View File
@@ -204,6 +204,7 @@ mod tests {
use codex_protocol::protocol::FileSystemSpecialPath;
use codex_protocol::protocol::RejectConfig;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
#[test]
@@ -316,6 +317,7 @@ mod tests {
AskForApproval::Reject(RejectConfig {
sandbox_approval: false,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}),
&policy_workspace_only,
@@ -348,6 +350,7 @@ mod tests {
AskForApproval::Reject(RejectConfig {
sandbox_approval: true,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}),
&policy_workspace_only,
@@ -403,4 +406,47 @@ mod tests {
SafetyCheck::AskUser,
);
}
#[test]
fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() {
let tmp = TempDir::new().unwrap();
let cwd = tmp.path().to_path_buf();
let blocked_path = cwd.join("docs").join("blocked.txt");
let docs_absolute = AbsolutePathBuf::resolve_path_against_base("docs", &cwd).unwrap();
let action = ApplyPatchAction::new_add_for_test(&blocked_path, "".to_string());
let sandbox_policy = SandboxPolicy::ExternalSandbox {
network_access: codex_protocol::protocol::NetworkAccess::Restricted,
};
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: docs_absolute,
},
access: FileSystemAccessMode::Read,
},
]);
assert!(!is_write_patch_constrained_to_writable_paths(
&action,
&file_system_sandbox_policy,
&cwd,
));
assert_eq!(
assess_patch_safety(
&action,
AskForApproval::OnRequest,
&sandbox_policy,
&file_system_sandbox_policy,
&cwd,
WindowsSandboxLevel::Disabled,
),
SafetyCheck::AskUser,
);
}
}
@@ -31,6 +31,28 @@ pub(crate) fn merge_macos_seatbelt_profile_extensions(
}
}
pub(crate) fn intersect_macos_seatbelt_profile_extensions(
requested: Option<MacOsSeatbeltProfileExtensions>,
granted: Option<MacOsSeatbeltProfileExtensions>,
) -> Option<MacOsSeatbeltProfileExtensions> {
match (requested, granted) {
(Some(requested), Some(granted)) => {
let macos_automation = intersect_macos_automation_permission(
&requested.macos_automation,
&granted.macos_automation,
);
Some(MacOsSeatbeltProfileExtensions {
macos_preferences: requested.macos_preferences.min(granted.macos_preferences),
macos_automation,
macos_accessibility: requested.macos_accessibility && granted.macos_accessibility,
macos_calendar: requested.macos_calendar && granted.macos_calendar,
})
}
_ => None,
}
}
/// Unions two preferences permissions by keeping the more permissive one.
///
/// The larger rank wins: `None < ReadOnly < ReadWrite`. When both sides have
@@ -75,8 +97,40 @@ fn union_macos_automation_permission(
}
}
fn intersect_macos_automation_permission(
requested: &MacOsAutomationPermission,
granted: &MacOsAutomationPermission,
) -> MacOsAutomationPermission {
match (requested, granted) {
(_, MacOsAutomationPermission::None) | (MacOsAutomationPermission::None, _) => {
MacOsAutomationPermission::None
}
(MacOsAutomationPermission::All, granted) => granted.clone(),
(MacOsAutomationPermission::BundleIds(requested), MacOsAutomationPermission::All) => {
MacOsAutomationPermission::BundleIds(requested.clone())
}
(
MacOsAutomationPermission::BundleIds(requested),
MacOsAutomationPermission::BundleIds(granted),
) => {
let bundle_ids = requested
.iter()
.filter(|bundle_id| granted.contains(bundle_id))
.cloned()
.collect::<Vec<String>>();
if bundle_ids.is_empty() {
MacOsAutomationPermission::None
} else {
MacOsAutomationPermission::BundleIds(bundle_ids)
}
}
}
}
#[cfg(all(test, target_os = "macos"))]
mod tests {
use super::intersect_macos_automation_permission;
use super::intersect_macos_seatbelt_profile_extensions;
use super::merge_macos_seatbelt_profile_extensions;
use super::union_macos_automation_permission;
use super::union_macos_preferences_permission;
@@ -141,4 +195,38 @@ mod tests {
assert_eq!(merged, MacOsAutomationPermission::All);
}
#[test]
fn intersect_macos_automation_permission_keeps_common_bundle_ids() {
let requested = MacOsAutomationPermission::BundleIds(vec![
"com.apple.Notes".to_string(),
"com.apple.Calendar".to_string(),
]);
let granted = MacOsAutomationPermission::BundleIds(vec!["com.apple.Notes".to_string()]);
let intersected = intersect_macos_automation_permission(&requested, &granted);
assert_eq!(
intersected,
MacOsAutomationPermission::BundleIds(vec!["com.apple.Notes".to_string()])
);
}
#[test]
fn intersect_macos_seatbelt_profile_extensions_preserves_default_grant() {
let requested = MacOsSeatbeltProfileExtensions {
macos_preferences: MacOsPreferencesPermission::ReadWrite,
macos_automation: MacOsAutomationPermission::BundleIds(vec![
"com.apple.Notes".to_string(),
]),
macos_accessibility: true,
macos_calendar: true,
};
let granted = MacOsSeatbeltProfileExtensions::default();
let intersected =
intersect_macos_seatbelt_profile_extensions(Some(requested), Some(granted));
assert_eq!(intersected, Some(MacOsSeatbeltProfileExtensions::default()));
}
}
+225 -12
View File
@@ -28,6 +28,7 @@ use codex_network_proxy::NetworkProxy;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::MacOsSeatbeltProfileExtensions;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
pub use codex_protocol::models::SandboxPermissions;
use codex_protocol::permissions::FileSystemAccessMode;
@@ -40,6 +41,7 @@ use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_utils_absolute_path::AbsolutePathBuf;
use dunce::canonicalize;
use macos_permissions::intersect_macos_seatbelt_profile_extensions;
use macos_permissions::merge_macos_seatbelt_profile_extensions;
use std::collections::HashMap;
use std::collections::HashSet;
@@ -146,22 +148,137 @@ impl EffectiveSandboxPermissions {
pub(crate) fn normalize_additional_permissions(
additional_permissions: PermissionProfile,
) -> Result<PermissionProfile, String> {
let network = additional_permissions
.network
.filter(|network| !network.is_empty());
let file_system = additional_permissions
.file_system
.map(|file_system| {
let read = file_system
.read
.map(|paths| normalize_permission_paths(paths, "file_system.read"));
let write = file_system
.write
.map(|paths| normalize_permission_paths(paths, "file_system.write"));
FileSystemPermissions { read, write }
})
.filter(|file_system| !file_system.is_empty());
let macos = additional_permissions.macos;
Ok(PermissionProfile {
network: additional_permissions.network,
file_system: additional_permissions
.file_system
.map(|file_system| FileSystemPermissions {
read: file_system
.read
.map(|paths| normalize_permission_paths(paths, "file_system.read")),
write: file_system
.write
.map(|paths| normalize_permission_paths(paths, "file_system.write")),
}),
macos: additional_permissions.macos,
network,
file_system,
macos,
})
}
pub(crate) fn merge_permission_profiles(
base: Option<&PermissionProfile>,
permissions: Option<&PermissionProfile>,
) -> Option<PermissionProfile> {
let Some(permissions) = permissions else {
return base.cloned();
};
match base {
Some(base) => {
let network = match (base.network.as_ref(), permissions.network.as_ref()) {
(
Some(NetworkPermissions {
enabled: Some(true),
}),
_,
)
| (
_,
Some(NetworkPermissions {
enabled: Some(true),
}),
) => Some(NetworkPermissions {
enabled: Some(true),
}),
_ => None,
};
let file_system = match (base.file_system.as_ref(), permissions.file_system.as_ref()) {
(Some(base), Some(permissions)) => Some(FileSystemPermissions {
read: merge_permission_paths(base.read.as_ref(), permissions.read.as_ref()),
write: merge_permission_paths(base.write.as_ref(), permissions.write.as_ref()),
})
.filter(|file_system| !file_system.is_empty()),
(Some(base), None) => Some(base.clone()),
(None, Some(permissions)) => Some(permissions.clone()),
(None, None) => None,
};
let macos = merge_macos_seatbelt_profile_extensions(
base.macos.as_ref(),
permissions.macos.as_ref(),
);
Some(PermissionProfile {
network,
file_system,
macos,
})
.filter(|permissions| !permissions.is_empty())
}
None => Some(permissions.clone()).filter(|permissions| !permissions.is_empty()),
}
}
pub fn intersect_permission_profiles(
requested: PermissionProfile,
granted: PermissionProfile,
) -> PermissionProfile {
let file_system = requested
.file_system
.map(|requested_file_system| {
let granted_file_system = granted.file_system.unwrap_or_default();
let read = requested_file_system
.read
.map(|requested_read| {
let granted_read = granted_file_system.read.unwrap_or_default();
requested_read
.into_iter()
.filter(|path| granted_read.contains(path))
.collect()
})
.filter(|paths: &Vec<_>| !paths.is_empty());
let write = requested_file_system
.write
.map(|requested_write| {
let granted_write = granted_file_system.write.unwrap_or_default();
requested_write
.into_iter()
.filter(|path| granted_write.contains(path))
.collect()
})
.filter(|paths: &Vec<_>| !paths.is_empty());
FileSystemPermissions { read, write }
})
.filter(|file_system| !file_system.is_empty());
let network = match (requested.network, granted.network) {
(
Some(NetworkPermissions {
enabled: Some(true),
}),
Some(NetworkPermissions {
enabled: Some(true),
}),
) => Some(NetworkPermissions {
enabled: Some(true),
}),
_ => None,
};
let macos = intersect_macos_seatbelt_profile_extensions(requested.macos, granted.macos);
PermissionProfile {
network,
file_system,
macos,
}
}
fn normalize_permission_paths(
paths: Vec<AbsolutePathBuf>,
_permission_kind: &str,
@@ -182,6 +299,29 @@ fn normalize_permission_paths(
out
}
fn merge_permission_paths(
base: Option<&Vec<AbsolutePathBuf>>,
permissions: Option<&Vec<AbsolutePathBuf>>,
) -> Option<Vec<AbsolutePathBuf>> {
match (base, permissions) {
(Some(base), Some(permissions)) => {
let mut merged = Vec::with_capacity(base.len() + permissions.len());
let mut seen = HashSet::with_capacity(base.len() + permissions.len());
for path in base.iter().chain(permissions.iter()) {
if seen.insert(path.clone()) {
merged.push(path.clone());
}
}
Some(merged).filter(|paths| !paths.is_empty())
}
(Some(base), None) => Some(base.clone()),
(None, Some(permissions)) => Some(permissions.clone()),
(None, None) => None,
}
}
fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
let mut out = Vec::with_capacity(paths.len());
let mut seen = HashSet::new();
@@ -581,6 +721,8 @@ mod tests {
#[cfg(target_os = "macos")]
use super::EffectiveSandboxPermissions;
use super::SandboxManager;
#[cfg(target_os = "macos")]
use super::intersect_permission_profiles;
use super::merge_file_system_policy_with_additional_permissions;
use super::normalize_additional_permissions;
use super::sandbox_policy_with_additional_permissions;
@@ -791,6 +933,77 @@ mod tests {
);
}
#[test]
fn normalize_additional_permissions_drops_empty_nested_profiles() {
let permissions = normalize_additional_permissions(PermissionProfile {
network: Some(NetworkPermissions { enabled: None }),
file_system: Some(FileSystemPermissions {
read: None,
write: None,
}),
macos: None,
})
.expect("permissions");
assert_eq!(permissions, PermissionProfile::default());
}
#[cfg(target_os = "macos")]
#[test]
fn normalize_additional_permissions_preserves_default_macos_preferences_permission() {
let permissions = normalize_additional_permissions(PermissionProfile {
macos: Some(MacOsSeatbeltProfileExtensions::default()),
..Default::default()
})
.expect("permissions");
assert_eq!(
permissions,
PermissionProfile {
macos: Some(MacOsSeatbeltProfileExtensions::default()),
..Default::default()
}
);
}
#[cfg(target_os = "macos")]
#[test]
fn intersect_permission_profiles_preserves_default_macos_grants() {
let requested = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(Vec::from(["/tmp/requested"
.try_into()
.expect("absolute path")])),
write: None,
}),
macos: Some(MacOsSeatbeltProfileExtensions {
macos_preferences: MacOsPreferencesPermission::ReadWrite,
macos_automation: MacOsAutomationPermission::BundleIds(vec![
"com.apple.Notes".to_string(),
]),
macos_accessibility: true,
macos_calendar: true,
}),
..Default::default()
};
let granted = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(Vec::new()),
write: None,
}),
macos: Some(MacOsSeatbeltProfileExtensions::default()),
..Default::default()
};
assert_eq!(
intersect_permission_profiles(requested, granted),
PermissionProfile {
macos: Some(MacOsSeatbeltProfileExtensions::default()),
..Default::default()
}
);
}
#[cfg(target_os = "macos")]
#[test]
fn normalize_additional_permissions_preserves_macos_permissions() {
+38 -4
View File
@@ -331,10 +331,9 @@ pub(crate) fn create_seatbelt_command_args(
enforce_managed_network: bool,
network: Option<&NetworkProxy>,
) -> Vec<String> {
create_seatbelt_command_args_for_policies_with_extensions(
create_seatbelt_command_args_with_extensions(
command,
&FileSystemSandboxPolicy::from(sandbox_policy),
NetworkSandboxPolicy::from(sandbox_policy),
sandbox_policy,
sandbox_policy_cwd,
enforce_managed_network,
network,
@@ -410,7 +409,7 @@ pub(crate) fn create_seatbelt_command_args_with_extensions(
) -> Vec<String> {
create_seatbelt_command_args_for_policies_with_extensions(
command,
&FileSystemSandboxPolicy::from(sandbox_policy),
&FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_policy_cwd),
NetworkSandboxPolicy::from(sandbox_policy),
sandbox_policy_cwd,
enforce_managed_network,
@@ -596,6 +595,7 @@ mod tests {
use super::normalize_path_for_sandbox;
use super::unix_socket_dir_params;
use super::unix_socket_policy;
use crate::protocol::ReadOnlyAccess;
use crate::protocol::SandboxPolicy;
use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE;
use crate::seatbelt_permissions::MacOsAutomationPermission;
@@ -865,6 +865,40 @@ sys.exit(0 if allowed else 13)
assert!(!policy.contains("(allow user-preference-write)"));
}
#[test]
fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() {
let tmp = TempDir::new().expect("tempdir");
let cwd = tmp.path().join("workspace");
fs::create_dir_all(cwd.join("docs")).expect("create docs");
let docs = AbsolutePathBuf::from_absolute_path(cwd.join("docs")).expect("absolute docs");
let args = create_seatbelt_command_args(
vec!["/bin/true".to_string()],
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs.clone()],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
cwd.as_path(),
false,
None,
);
let docs_param = format!("-DWRITABLE_ROOT_0_RO_0={}", docs.as_path().display());
assert!(
!seatbelt_policy_arg(&args).contains("WRITABLE_ROOT_0_RO_0"),
"legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}"
);
assert!(
!args.iter().any(|arg| arg == &docs_param),
"unexpected seatbelt carveout parameter for redundant legacy readable root: {args:#?}"
);
}
#[test]
fn seatbelt_args_default_extension_profile_keeps_preferences_read_access() {
let cwd = std::env::temp_dir();
+18 -5
View File
@@ -740,7 +740,13 @@ mod tests {
let dir = tempdir()?;
let home = dir.path();
fs::write(home.join(".bashrc"), "read -r ignored\n").await?;
let read_status_path = home.join("stdin-read-status");
let read_status_display = read_status_path.display();
// Persist the startup `read` exit status so the test can assert whether
// bash saw EOF on stdin after the snapshot process exits.
let bashrc =
format!("read -t 1 -r ignored\nprintf '%s' \"$?\" > \"{read_status_display}\"\n");
fs::write(home.join(".bashrc"), bashrc).await?;
let shell = Shell {
shell_type: ShellType::Bash,
@@ -753,10 +759,17 @@ mod tests {
"HOME=\"{home_display}\"; export HOME; {}",
bash_snapshot_script()
);
let output =
run_script_with_timeout(&shell, &script, Duration::from_millis(500), true, home)
.await
.context("run snapshot command")?;
let output = run_script_with_timeout(&shell, &script, Duration::from_secs(2), true, home)
.await
.context("run snapshot command")?;
let read_status = fs::read_to_string(&read_status_path)
.await
.context("read stdin probe status")?;
assert_eq!(
read_status, "1",
"expected shell startup read to see EOF on stdin; status={read_status:?}"
);
assert!(
output.contains("# Snapshot file"),
+14
View File
@@ -1,5 +1,6 @@
//! Session-wide mutable state.
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseItem;
use std::collections::HashMap;
use std::collections::HashSet;
@@ -32,6 +33,7 @@ pub(crate) struct SessionState {
pub(crate) startup_regular_task: Option<JoinHandle<CodexResult<RegularTask>>>,
pub(crate) active_mcp_tool_selection: Option<Vec<String>>,
pub(crate) active_connector_selection: HashSet<String>,
granted_permissions: Option<PermissionProfile>,
}
impl SessionState {
@@ -49,6 +51,7 @@ impl SessionState {
startup_regular_task: None,
active_mcp_tool_selection: None,
active_connector_selection: HashSet::new(),
granted_permissions: None,
}
}
@@ -218,6 +221,17 @@ impl SessionState {
self.active_mcp_tool_selection = None;
}
pub(crate) fn record_granted_permissions(&mut self, permissions: PermissionProfile) {
self.granted_permissions = crate::sandboxing::merge_permission_profiles(
self.granted_permissions.as_ref(),
Some(&permissions),
);
}
pub(crate) fn granted_permissions(&self) -> Option<PermissionProfile> {
self.granted_permissions.clone()
}
// Adds connector IDs to the active set and returns the merged selection.
pub(crate) fn merge_connector_selection<I>(&mut self, connector_ids: I) -> HashSet<String>
where
+30
View File
@@ -10,6 +10,7 @@ use tokio_util::task::AbortOnDropHandle;
use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::ElicitationResponse;
use rmcp::model::RequestId;
@@ -18,7 +19,9 @@ use tokio::sync::oneshot;
use crate::codex::TurnContext;
use crate::protocol::ReviewDecision;
use crate::protocol::TokenUsage;
use crate::sandboxing::merge_permission_profiles;
use crate::tasks::SessionTask;
use codex_protocol::models::PermissionProfile;
/// Metadata about the currently running turn.
pub(crate) struct ActiveTurn {
@@ -73,10 +76,12 @@ impl ActiveTurn {
#[derive(Default)]
pub(crate) struct TurnState {
pending_approvals: HashMap<String, oneshot::Sender<ReviewDecision>>,
pending_request_permissions: HashMap<String, oneshot::Sender<RequestPermissionsResponse>>,
pending_user_input: HashMap<String, oneshot::Sender<RequestUserInputResponse>>,
pending_elicitations: HashMap<(String, RequestId), oneshot::Sender<ElicitationResponse>>,
pending_dynamic_tools: HashMap<String, oneshot::Sender<DynamicToolResponse>>,
pending_input: Vec<ResponseInputItem>,
granted_permissions: Option<PermissionProfile>,
pub(crate) tool_calls: u64,
pub(crate) token_usage_at_turn_start: TokenUsage,
}
@@ -99,12 +104,28 @@ impl TurnState {
pub(crate) fn clear_pending(&mut self) {
self.pending_approvals.clear();
self.pending_request_permissions.clear();
self.pending_user_input.clear();
self.pending_elicitations.clear();
self.pending_dynamic_tools.clear();
self.pending_input.clear();
}
pub(crate) fn insert_pending_request_permissions(
&mut self,
key: String,
tx: oneshot::Sender<RequestPermissionsResponse>,
) -> Option<oneshot::Sender<RequestPermissionsResponse>> {
self.pending_request_permissions.insert(key, tx)
}
pub(crate) fn remove_pending_request_permissions(
&mut self,
key: &str,
) -> Option<oneshot::Sender<RequestPermissionsResponse>> {
self.pending_request_permissions.remove(key)
}
pub(crate) fn insert_pending_user_input(
&mut self,
key: String,
@@ -171,6 +192,15 @@ impl TurnState {
pub(crate) fn has_pending_input(&self) -> bool {
!self.pending_input.is_empty()
}
pub(crate) fn record_granted_permissions(&mut self, permissions: PermissionProfile) {
self.granted_permissions =
merge_permission_profiles(self.granted_permissions.as_ref(), Some(&permissions));
}
pub(crate) fn granted_permissions(&self) -> Option<PermissionProfile> {
self.granted_permissions.clone()
}
}
impl ActiveTurn {
+19 -18
View File
@@ -33,6 +33,9 @@ use crate::protocol::TurnCompleteEvent;
use crate::state::ActiveTurn;
use crate::state::RunningTask;
use crate::state::TaskKind;
use codex_otel::metrics::names::TURN_E2E_DURATION_METRIC;
use codex_otel::metrics::names::TURN_TOKEN_USAGE_METRIC;
use codex_otel::metrics::names::TURN_TOOL_CALL_METRIC;
use codex_protocol::items::TurnItem;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
@@ -145,7 +148,7 @@ impl Session {
let timer = turn_context
.session_telemetry
.start_timer("codex.turn.e2e_duration_ms", &[])
.start_timer(TURN_E2E_DURATION_METRIC, &[])
.ok();
let done_clone = Arc::clone(&done);
@@ -201,8 +204,13 @@ impl Session {
}
pub async fn abort_all_tasks(self: &Arc<Self>, reason: TurnAbortReason) {
for task in self.take_all_running_tasks().await {
self.handle_task_abort(task, reason.clone()).await;
if let Some(mut active_turn) = self.take_active_turn().await {
for task in active_turn.drain_tasks() {
self.handle_task_abort(task, reason.clone()).await;
}
// Let interrupted tasks observe cancellation before dropping pending approvals, or an
// in-flight approval wait can surface as a model-visible rejection before TurnAborted.
active_turn.clear_pending().await;
}
if reason == TurnAbortReason::Interrupted {
self.close_unified_exec_processes().await;
@@ -273,7 +281,7 @@ impl Session {
},
);
self.services.session_telemetry.histogram(
"codex.turn.tool.call",
TURN_TOOL_CALL_METRIC,
i64::try_from(turn_tool_calls).unwrap_or(i64::MAX),
&[tmp_mem],
);
@@ -296,27 +304,27 @@ impl Session {
.max(0),
};
self.services.session_telemetry.histogram(
"codex.turn.token_usage",
TURN_TOKEN_USAGE_METRIC,
turn_token_usage.total_tokens,
&[("token_type", "total"), tmp_mem],
);
self.services.session_telemetry.histogram(
"codex.turn.token_usage",
TURN_TOKEN_USAGE_METRIC,
turn_token_usage.input_tokens,
&[("token_type", "input"), tmp_mem],
);
self.services.session_telemetry.histogram(
"codex.turn.token_usage",
TURN_TOKEN_USAGE_METRIC,
turn_token_usage.cached_input(),
&[("token_type", "cached_input"), tmp_mem],
);
self.services.session_telemetry.histogram(
"codex.turn.token_usage",
TURN_TOKEN_USAGE_METRIC,
turn_token_usage.output_tokens,
&[("token_type", "output"), tmp_mem],
);
self.services.session_telemetry.histogram(
"codex.turn.token_usage",
TURN_TOKEN_USAGE_METRIC,
turn_token_usage.reasoning_output_tokens,
&[("token_type", "reasoning_output"), tmp_mem],
);
@@ -342,16 +350,9 @@ impl Session {
*active = Some(turn);
}
async fn take_all_running_tasks(&self) -> Vec<RunningTask> {
async fn take_active_turn(&self) -> Option<ActiveTurn> {
let mut active = self.active_turn.lock().await;
match active.take() {
Some(mut at) => {
at.clear_pending().await;
at.drain_tasks()
}
None => Vec::new(),
}
active.take()
}
pub(crate) async fn close_unified_exec_processes(&self) {
@@ -18,6 +18,7 @@ use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::events::ToolEmitter;
use crate::tools::events::ToolEventCtx;
use crate::tools::handlers::apply_granted_turn_permissions;
use crate::tools::handlers::parse_arguments;
use crate::tools::orchestrator::ToolOrchestrator;
use crate::tools::registry::ToolHandler;
@@ -30,7 +31,10 @@ use crate::tools::spec::JsonSchema;
use async_trait::async_trait;
use codex_apply_patch::ApplyPatchAction;
use codex_apply_patch::ApplyPatchFileChange;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::BTreeSet;
use std::sync::Arc;
pub struct ApplyPatchHandler;
@@ -61,6 +65,31 @@ fn to_abs_path(cwd: &Path, path: &Path) -> Option<AbsolutePathBuf> {
AbsolutePathBuf::resolve_path_against_base(path, cwd).ok()
}
fn write_permissions_for_paths(file_paths: &[AbsolutePathBuf]) -> Option<PermissionProfile> {
let write_paths = file_paths
.iter()
.map(|path| {
path.parent()
.unwrap_or_else(|| path.clone())
.into_path_buf()
})
.collect::<BTreeSet<_>>()
.into_iter()
.map(AbsolutePathBuf::from_absolute_path)
.collect::<Result<Vec<_>, _>>()
.ok()?;
let permissions = (!write_paths.is_empty()).then_some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(write_paths),
}),
..Default::default()
})?;
crate::sandboxing::normalize_additional_permissions(permissions).ok()
}
#[async_trait]
impl ToolHandler for ApplyPatchHandler {
fn kind(&self) -> ToolKind {
@@ -119,6 +148,12 @@ impl ToolHandler for ApplyPatchHandler {
InternalApplyPatchInvocation::DelegateToExec(apply) => {
let changes = convert_apply_patch_to_protocol(&apply.action);
let file_paths = file_paths_for_action(&apply.action);
let effective_additional_permissions = apply_granted_turn_permissions(
session.as_ref(),
crate::sandboxing::SandboxPermissions::UseDefault,
write_permissions_for_paths(&file_paths),
)
.await;
let emitter =
ToolEmitter::apply_patch(changes.clone(), apply.auto_approved);
let event_ctx = ToolEventCtx::new(
@@ -134,6 +169,12 @@ impl ToolHandler for ApplyPatchHandler {
file_paths,
changes,
exec_approval_requirement: apply.exec_approval_requirement,
sandbox_permissions: effective_additional_permissions
.sandbox_permissions,
additional_permissions: effective_additional_permissions
.additional_permissions,
permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
timeout_ms: None,
codex_exe: turn.codex_linux_sandbox_exe.clone(),
};
@@ -222,6 +263,12 @@ pub(crate) async fn intercept_apply_patch(
InternalApplyPatchInvocation::DelegateToExec(apply) => {
let changes = convert_apply_patch_to_protocol(&apply.action);
let approval_keys = file_paths_for_action(&apply.action);
let effective_additional_permissions = apply_granted_turn_permissions(
session.as_ref(),
crate::sandboxing::SandboxPermissions::UseDefault,
write_permissions_for_paths(&approval_keys),
)
.await;
let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved);
let event_ctx = ToolEventCtx::new(
session.as_ref(),
@@ -236,6 +283,11 @@ pub(crate) async fn intercept_apply_patch(
file_paths: approval_keys,
changes,
exec_approval_requirement: apply.exec_approval_requirement,
sandbox_permissions: effective_additional_permissions.sandbox_permissions,
additional_permissions: effective_additional_permissions
.additional_permissions,
permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
timeout_ms,
codex_exe: turn.codex_linux_sandbox_exe.clone(),
};
+61 -2
View File
@@ -10,6 +10,7 @@ mod mcp_resource;
pub(crate) mod multi_agents;
mod plan;
mod read_file;
mod request_permissions;
mod request_user_input;
mod search_tool_bm25;
mod shell;
@@ -24,8 +25,10 @@ use serde_json::Value;
use std::path::Path;
use std::path::PathBuf;
use crate::codex::Session;
use crate::function_tool::FunctionCallError;
use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::merge_permission_profiles;
use crate::sandboxing::normalize_additional_permissions;
pub use apply_patch::ApplyPatchHandler;
pub use artifacts::ArtifactsHandler;
@@ -41,6 +44,8 @@ pub use mcp_resource::McpResourceHandler;
pub use multi_agents::MultiAgentHandler;
pub use plan::PlanHandler;
pub use read_file::ReadFileHandler;
pub use request_permissions::RequestPermissionsHandler;
pub(crate) use request_permissions::request_permissions_tool_description;
pub use request_user_input::RequestUserInputHandler;
pub(crate) use request_user_input::request_user_input_tool_description;
pub(crate) use search_tool_bm25::DEFAULT_LIMIT as SEARCH_TOOL_BM25_DEFAULT_LIMIT;
@@ -90,11 +95,12 @@ fn resolve_workdir_base_path(
/// Validates feature/policy constraints for `with_additional_permissions` and
/// normalizes any path-based permissions. Errors if the request is invalid.
pub(super) fn normalize_and_validate_additional_permissions(
pub(crate) fn normalize_and_validate_additional_permissions(
request_permission_enabled: bool,
approval_policy: AskForApproval,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<PermissionProfile>,
permissions_preapproved: bool,
_cwd: &Path,
) -> Result<Option<PermissionProfile>, String> {
let uses_additional_permissions = matches!(
@@ -112,7 +118,7 @@ pub(super) fn normalize_and_validate_additional_permissions(
}
if uses_additional_permissions {
if !matches!(approval_policy, AskForApproval::OnRequest) {
if !permissions_preapproved && !matches!(approval_policy, AskForApproval::OnRequest) {
return Err(format!(
"approval policy is {approval_policy:?}; reject command — you cannot request additional permissions unless the approval policy is OnRequest"
));
@@ -146,3 +152,56 @@ pub(super) fn normalize_and_validate_additional_permissions(
Ok(None)
}
}
pub(super) struct EffectiveAdditionalPermissions {
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
pub permissions_preapproved: bool,
}
pub(super) async fn apply_granted_turn_permissions(
session: &Session,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<PermissionProfile>,
) -> EffectiveAdditionalPermissions {
if matches!(sandbox_permissions, SandboxPermissions::RequireEscalated) {
return EffectiveAdditionalPermissions {
sandbox_permissions,
additional_permissions,
permissions_preapproved: false,
};
}
let granted_session_permissions = session.granted_session_permissions().await;
let granted_turn_permissions = session.granted_turn_permissions().await;
let granted_permissions = merge_permission_profiles(
granted_session_permissions.as_ref(),
granted_turn_permissions.as_ref(),
);
let effective_permissions = merge_permission_profiles(
additional_permissions.as_ref(),
granted_permissions.as_ref(),
);
let permissions_preapproved = match (effective_permissions.as_ref(), granted_permissions) {
(Some(effective_permissions), Some(granted_permissions)) => {
crate::sandboxing::intersect_permission_profiles(
effective_permissions.clone(),
granted_permissions,
) == *effective_permissions
}
_ => false,
};
let sandbox_permissions =
if effective_permissions.is_some() && !sandbox_permissions.uses_additional_permissions() {
SandboxPermissions::WithAdditionalPermissions
} else {
sandbox_permissions
};
EffectiveAdditionalPermissions {
sandbox_permissions,
additional_permissions: effective_permissions,
permissions_preapproved,
}
}
@@ -0,0 +1,75 @@
use async_trait::async_trait;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::request_permissions::RequestPermissionsArgs;
use crate::function_tool::FunctionCallError;
use crate::sandboxing::normalize_additional_permissions;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
pub(crate) fn request_permissions_tool_description() -> String {
"Request additional permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
.to_string()
}
pub struct RequestPermissionsHandler;
#[async_trait]
impl ToolHandler for RequestPermissionsHandler {
fn kind(&self) -> ToolKind {
ToolKind::Function
}
async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
let ToolInvocation {
session,
turn,
call_id,
payload,
..
} = invocation;
let arguments = match payload {
ToolPayload::Function { arguments } => arguments,
_ => {
return Err(FunctionCallError::RespondToModel(
"request_permissions handler received unsupported payload".to_string(),
));
}
};
let mut args: RequestPermissionsArgs =
parse_arguments_with_base_path(&arguments, turn.cwd.as_path())?;
args.permissions = normalize_additional_permissions(args.permissions)
.map_err(FunctionCallError::RespondToModel)?;
if args.permissions.is_empty() {
return Err(FunctionCallError::RespondToModel(
"request_permissions requires at least one permission".to_string(),
));
}
let response = session
.request_permissions(turn.as_ref(), call_id, args)
.await
.ok_or_else(|| {
FunctionCallError::RespondToModel(
"request_permissions was cancelled before receiving a response".to_string(),
)
})?;
let content = serde_json::to_string(&response).map_err(|err| {
FunctionCallError::Fatal(format!(
"failed to serialize request_permissions response: {err}"
))
})?;
Ok(ToolOutput::Function {
body: FunctionCallOutputBody::Text(content),
success: Some(true),
})
}
}
+22 -5
View File
@@ -20,6 +20,7 @@ use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::events::ToolEmitter;
use crate::tools::events::ToolEventCtx;
use crate::tools::handlers::apply_granted_turn_permissions;
use crate::tools::handlers::apply_patch::intercept_apply_patch;
use crate::tools::handlers::normalize_and_validate_additional_permissions;
use crate::tools::handlers::parse_arguments_with_base_path;
@@ -332,17 +333,29 @@ impl ShellHandler {
}
let request_permission_enabled = session.features().enabled(Feature::RequestPermissions);
let effective_additional_permissions = apply_granted_turn_permissions(
session.as_ref(),
exec_params.sandbox_permissions,
additional_permissions,
)
.await;
let normalized_additional_permissions = normalize_and_validate_additional_permissions(
request_permission_enabled,
turn.approval_policy.value(),
exec_params.sandbox_permissions,
additional_permissions,
effective_additional_permissions.sandbox_permissions,
effective_additional_permissions.additional_permissions,
effective_additional_permissions.permissions_preapproved,
&exec_params.cwd,
)
.map_err(FunctionCallError::RespondToModel)?;
// Approval policy guard for explicit escalation in non-OnRequest modes.
if exec_params.sandbox_permissions.requests_sandbox_override()
// Sticky turn permissions have already been approved, so they should
// continue through the normal exec approval flow for the command.
if effective_additional_permissions
.sandbox_permissions
.requests_sandbox_override()
&& !effective_additional_permissions.permissions_preapproved
&& !matches!(
turn.approval_policy.value(),
codex_protocol::protocol::AskForApproval::OnRequest
@@ -387,7 +400,11 @@ impl ShellHandler {
command: &exec_params.command,
approval_policy: turn.approval_policy.value(),
sandbox_policy: turn.sandbox_policy.get(),
sandbox_permissions: exec_params.sandbox_permissions,
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
codex_protocol::models::SandboxPermissions::UseDefault
} else {
effective_additional_permissions.sandbox_permissions
},
prefix_rule,
})
.await;
@@ -399,7 +416,7 @@ impl ShellHandler {
env: exec_params.env.clone(),
explicit_env_overrides,
network: exec_params.network.clone(),
sandbox_permissions: exec_params.sandbox_permissions,
sandbox_permissions: effective_additional_permissions.sandbox_permissions,
additional_permissions: normalized_additional_permissions,
justification: exec_params.justification.clone(),
exec_approval_requirement,
@@ -10,6 +10,7 @@ use crate::skills::maybe_emit_implicit_skill_invocation;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::handlers::apply_granted_turn_permissions;
use crate::tools::handlers::apply_patch::intercept_apply_patch;
use crate::tools::handlers::normalize_and_validate_additional_permissions;
use crate::tools::handlers::parse_arguments;
@@ -170,8 +171,19 @@ impl ToolHandler for UnifiedExecHandler {
let request_permission_enabled =
session.features().enabled(Feature::RequestPermissions);
let effective_additional_permissions = apply_granted_turn_permissions(
context.session.as_ref(),
sandbox_permissions,
additional_permissions,
)
.await;
if sandbox_permissions.requests_sandbox_override()
// Sticky turn permissions have already been approved, so they should
// continue through the normal exec approval flow for the command.
if effective_additional_permissions
.sandbox_permissions
.requests_sandbox_override()
&& !effective_additional_permissions.permissions_preapproved
&& !matches!(
context.turn.approval_policy.value(),
codex_protocol::protocol::AskForApproval::OnRequest
@@ -192,8 +204,9 @@ impl ToolHandler for UnifiedExecHandler {
match normalize_and_validate_additional_permissions(
request_permission_enabled,
context.turn.approval_policy.value(),
sandbox_permissions,
additional_permissions,
effective_additional_permissions.sandbox_permissions,
effective_additional_permissions.additional_permissions,
effective_additional_permissions.permissions_preapproved,
&cwd,
) {
Ok(normalized) => normalized,
@@ -229,8 +242,11 @@ impl ToolHandler for UnifiedExecHandler {
workdir,
network: context.turn.network.clone(),
tty,
sandbox_permissions,
sandbox_permissions: effective_additional_permissions
.sandbox_permissions,
additional_permissions: normalized_additional_permissions,
additional_permissions_preapproved: effective_additional_permissions
.permissions_preapproved,
justification,
prefix_rule,
},
+7 -10
View File
@@ -1,6 +1,6 @@
use crate::codex::Session;
use crate::guardian::GUARDIAN_REJECTION_MESSAGE;
use crate::guardian::GuardianReviewRequest;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::network_policy_decision::denied_network_policy_message;
@@ -21,7 +21,6 @@ use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::WarningEvent;
use indexmap::IndexMap;
use serde_json::json;
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
@@ -344,14 +343,11 @@ impl NetworkApprovalService {
review_approval_request(
&session,
&turn_context,
GuardianReviewRequest {
action: json!({
"tool": "network_access",
"target": target,
"host": request.host,
"protocol": key.protocol,
"port": key.port,
}),
GuardianApprovalRequest::NetworkAccess {
target,
host: request.host,
protocol,
port: key.port,
},
Some(policy_denial_message.clone()),
)
@@ -371,6 +367,7 @@ impl NetworkApprovalService {
Some(network_approval_context.clone()),
None,
None,
None,
available_decisions,
)
.await
+28 -23
View File
@@ -5,7 +5,7 @@
//! `codex --codex-run-as-apply-patch`, and runs under the current
//! `SandboxAttempt` with a minimal environment.
use crate::exec::ExecToolCallOutput;
use crate::guardian::GuardianReviewRequest;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::sandboxing::CommandSpec;
@@ -23,12 +23,12 @@ use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::with_cached_approval;
use codex_apply_patch::ApplyPatchAction;
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::ReviewDecision;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
use serde_json::json;
use std::collections::HashMap;
use std::path::PathBuf;
@@ -38,6 +38,9 @@ pub struct ApplyPatchRequest {
pub file_paths: Vec<AbsolutePathBuf>,
pub changes: std::collections::HashMap<PathBuf, FileChange>,
pub exec_approval_requirement: ExecApprovalRequirement,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
pub permissions_preapproved: bool,
pub timeout_ms: Option<u64>,
pub codex_exe: Option<PathBuf>,
}
@@ -50,15 +53,12 @@ impl ApplyPatchRuntime {
Self
}
fn build_guardian_review_request(req: &ApplyPatchRequest) -> GuardianReviewRequest {
GuardianReviewRequest {
action: json!({
"tool": "apply_patch",
"cwd": req.action.cwd,
"files": req.file_paths,
"change_count": req.changes.len(),
"patch": req.action.patch,
}),
fn build_guardian_review_request(req: &ApplyPatchRequest) -> GuardianApprovalRequest {
GuardianApprovalRequest::ApplyPatch {
cwd: req.action.cwd.clone(),
files: req.file_paths.clone(),
change_count: req.changes.len(),
patch: req.action.patch.clone(),
}
}
@@ -91,8 +91,8 @@ impl ApplyPatchRuntime {
expiration: req.timeout_ms.into(),
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
env: HashMap::new(),
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: None,
})
}
@@ -135,8 +135,11 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
let changes = req.changes.clone();
Box::pin(async move {
if routes_approval_to_guardian(turn) {
let request = ApplyPatchRuntime::build_guardian_review_request(req);
return review_approval_request(session, turn, request, retry_reason).await;
let action = ApplyPatchRuntime::build_guardian_review_request(req);
return review_approval_request(session, turn, action, retry_reason).await;
}
if req.permissions_preapproved && retry_reason.is_none() {
return ReviewDecision::Approved;
}
if let Some(reason) = retry_reason {
let rx_approve = session
@@ -215,6 +218,7 @@ mod tests {
!runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig {
sandbox_approval: true,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}))
);
@@ -222,6 +226,7 @@ mod tests {
runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig {
sandbox_approval: false,
rules: false,
request_permissions: false,
mcp_elicitations: false,
}))
);
@@ -248,6 +253,9 @@ mod tests {
reason: None,
proposed_execpolicy_amendment: None,
},
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
permissions_preapproved: false,
timeout_ms: None,
codex_exe: None,
};
@@ -256,14 +264,11 @@ mod tests {
assert_eq!(
guardian_request,
GuardianReviewRequest {
action: json!({
"tool": "apply_patch",
"cwd": expected_cwd,
"files": request.file_paths,
"change_count": 1usize,
"patch": expected_patch,
}),
GuardianApprovalRequest::ApplyPatch {
cwd: expected_cwd,
files: request.file_paths,
change_count: 1usize,
patch: expected_patch,
}
);
}
+17 -24
View File
@@ -11,7 +11,7 @@ pub(crate) mod zsh_fork_backend;
use crate::command_canonicalization::canonicalize_command_for_approval;
use crate::exec::ExecToolCallOutput;
use crate::features::Feature;
use crate::guardian::GuardianReviewRequest;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::powershell::prefix_powershell_script_with_utf8;
@@ -38,7 +38,6 @@ use codex_network_proxy::NetworkProxy;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ReviewDecision;
use futures::future::BoxFuture;
use serde_json::json;
use std::collections::HashMap;
use std::path::PathBuf;
@@ -145,33 +144,26 @@ impl Approvable<ShellRequest> for ShellRuntime {
let keys = self.approval_keys(req);
let command = req.command.clone();
let cwd = req.cwd.clone();
let reason = ctx
.retry_reason
.clone()
.or_else(|| req.justification.clone());
let retry_reason = ctx.retry_reason.clone();
let reason = retry_reason.clone().or_else(|| req.justification.clone());
let session = ctx.session;
let turn = ctx.turn;
let call_id = ctx.call_id.to_string();
Box::pin(async move {
if routes_approval_to_guardian(turn) {
let mut action = json!({
"tool": "shell",
"command": command,
"cwd": cwd,
"sandbox_permissions": req.sandbox_permissions,
"additional_permissions": req.additional_permissions,
"justification": reason,
});
if let Some(action) = action.as_object_mut() {
if req.additional_permissions.is_none() {
action.remove("additional_permissions");
}
if reason.is_none() {
action.remove("justification");
}
}
let request = GuardianReviewRequest { action };
return review_approval_request(session, turn, request, None).await;
return review_approval_request(
session,
turn,
GuardianApprovalRequest::Shell {
command,
cwd,
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
},
retry_reason,
)
.await;
}
with_cached_approval(&session.services, "shell", keys, move || async move {
let available_decisions = None;
@@ -188,6 +180,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
.proposed_execpolicy_amendment()
.cloned(),
req.additional_permissions.clone(),
None,
available_decisions,
)
.await
@@ -7,7 +7,7 @@ use crate::exec::SandboxType;
use crate::exec::is_likely_sandbox_denied;
use crate::exec_policy::prompt_is_rejected_by_policy;
use crate::features::Feature;
use crate::guardian::GuardianReviewRequest;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::sandboxing::ExecRequest;
@@ -31,6 +31,7 @@ use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ExecApprovalRequestSkillMetadata;
use codex_protocol::protocol::NetworkPolicyRuleAction;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
@@ -49,7 +50,6 @@ use codex_shell_escalation::PreparedExec;
use codex_shell_escalation::ShellCommandExecutor;
use codex_shell_escalation::Stopwatch;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde_json::json;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
@@ -385,16 +385,19 @@ impl CoreShellActionProvider {
Ok(stopwatch
.pause_for(async move {
if routes_approval_to_guardian(&turn) {
let request = GuardianReviewRequest {
action: json!({
"tool": tool_name,
"program": program,
"argv": argv,
"cwd": workdir,
"additional_permissions": additional_permissions,
}),
};
return review_approval_request(&session, &turn, request, None).await;
return review_approval_request(
&session,
&turn,
GuardianApprovalRequest::Execve {
tool_name: tool_name.to_string(),
program: program.to_string_lossy().into_owned(),
argv: argv.to_vec(),
cwd: workdir,
additional_permissions,
},
None,
)
.await;
}
let available_decisions = vec![
Some(ReviewDecision::Approved),
@@ -410,6 +413,14 @@ impl CoreShellActionProvider {
.into_iter()
.flatten()
.collect();
let skill_metadata = match decision_source {
DecisionSource::SkillScript { skill } => {
Some(ExecApprovalRequestSkillMetadata {
path_to_skills_md: skill.path_to_skills_md.clone(),
})
}
DecisionSource::PrefixRule | DecisionSource::UnmatchedCommandFallback => None,
};
session
.request_command_approval(
&turn,
@@ -421,6 +432,7 @@ impl CoreShellActionProvider {
None,
None,
additional_permissions,
skill_metadata,
Some(available_decisions),
)
.await
@@ -9,7 +9,7 @@ use crate::error::CodexErr;
use crate::error::SandboxErr;
use crate::exec::ExecExpiration;
use crate::features::Feature;
use crate::guardian::GuardianReviewRequest;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::powershell::prefix_powershell_script_with_utf8;
@@ -41,7 +41,6 @@ use codex_network_proxy::NetworkProxy;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ReviewDecision;
use futures::future::BoxFuture;
use serde_json::json;
use std::collections::HashMap;
use std::path::PathBuf;
@@ -113,31 +112,24 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
let call_id = ctx.call_id.to_string();
let command = req.command.clone();
let cwd = req.cwd.clone();
let reason = ctx
.retry_reason
.clone()
.or_else(|| req.justification.clone());
let retry_reason = ctx.retry_reason.clone();
let reason = retry_reason.clone().or_else(|| req.justification.clone());
Box::pin(async move {
if routes_approval_to_guardian(turn) {
let mut action = json!({
"tool": "exec_command",
"command": command,
"cwd": cwd,
"sandbox_permissions": req.sandbox_permissions,
"additional_permissions": req.additional_permissions,
"justification": reason,
"tty": req.tty,
});
if let Some(action) = action.as_object_mut() {
if req.additional_permissions.is_none() {
action.remove("additional_permissions");
}
if reason.is_none() {
action.remove("justification");
}
}
let request = GuardianReviewRequest { action };
return review_approval_request(session, turn, request, None).await;
return review_approval_request(
session,
turn,
GuardianApprovalRequest::ExecCommand {
command,
cwd,
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
tty: req.tty,
},
retry_reason,
)
.await;
}
with_cached_approval(&session.services, "unified_exec", keys, || async move {
let available_decisions = None;
@@ -154,6 +146,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
.proposed_execpolicy_amendment()
.cloned(),
req.additional_permissions.clone(),
None,
available_decisions,
)
.await
+2
View File
@@ -398,6 +398,7 @@ mod tests {
let policy = AskForApproval::Reject(RejectConfig {
sandbox_approval: true,
rules: false,
request_permissions: false,
mcp_elicitations: false,
});
@@ -417,6 +418,7 @@ mod tests {
let policy = AskForApproval::Reject(RejectConfig {
sandbox_approval: false,
rules: true,
request_permissions: false,
mcp_elicitations: true,
});
+247 -101
View File
@@ -16,6 +16,7 @@ use crate::tools::handlers::apply_patch::create_apply_patch_json_tool;
use crate::tools::handlers::multi_agents::DEFAULT_WAIT_TIMEOUT_MS;
use crate::tools::handlers::multi_agents::MAX_WAIT_TIMEOUT_MS;
use crate::tools::handlers::multi_agents::MIN_WAIT_TIMEOUT_MS;
use crate::tools::handlers::request_permissions_tool_description;
use crate::tools::handlers::request_user_input_tool_description;
use crate::tools::registry::ToolRegistryBuilder;
use codex_protocol::config_types::WebSearchConfig;
@@ -66,6 +67,7 @@ pub(crate) struct ToolsConfig {
pub agent_roles: BTreeMap<String, AgentRoleConfig>,
pub search_tool: bool,
pub request_permission_enabled: bool,
pub request_permissions_tool_enabled: bool,
pub js_repl_enabled: bool,
pub js_repl_tools_only: bool,
pub collab_tools: bool,
@@ -107,6 +109,7 @@ impl ToolsConfig {
features.enabled(Feature::ImageGeneration) && supports_image_generation(model_info);
let include_agent_jobs = include_collab_tools;
let request_permission_enabled = features.enabled(Feature::RequestPermissions);
let request_permissions_tool_enabled = features.enabled(Feature::RequestPermissionsTool);
let shell_command_backend =
if features.enabled(Feature::ShellTool) && features.enabled(Feature::ShellZshFork) {
ShellCommandBackendConfig::ZshFork
@@ -167,6 +170,7 @@ impl ToolsConfig {
agent_roles: BTreeMap::new(),
search_tool: include_search_tool,
request_permission_enabled,
request_permissions_tool_enabled,
js_repl_enabled: include_js_repl,
js_repl_tools_only: include_js_repl_tools_only,
collab_tools: include_collab_tools,
@@ -255,6 +259,94 @@ impl From<JsonSchema> for AdditionalProperties {
}
}
fn create_network_permissions_schema() -> JsonSchema {
JsonSchema::Object {
properties: BTreeMap::from([(
"enabled".to_string(),
JsonSchema::Boolean {
description: Some("Set to true to request network access.".to_string()),
},
)]),
required: None,
additional_properties: Some(false.into()),
}
}
fn create_file_system_permissions_schema() -> JsonSchema {
JsonSchema::Object {
properties: BTreeMap::from([
(
"read".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some("Absolute paths to grant read access to.".to_string()),
},
),
(
"write".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some("Absolute paths to grant write access to.".to_string()),
},
),
]),
required: None,
additional_properties: Some(false.into()),
}
}
fn create_macos_permissions_schema() -> JsonSchema {
JsonSchema::Object {
properties: BTreeMap::from([
(
"preferences".to_string(),
JsonSchema::String {
description: Some(
"macOS preferences access. Supported values: `none`, `read_only`, or `read_write`."
.to_string(),
),
},
),
(
"automations".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some("macOS automation access as app bundle identifiers.".to_string()),
},
),
(
"accessibility".to_string(),
JsonSchema::Boolean {
description: Some("Whether to request macOS accessibility access.".to_string()),
},
),
(
"calendar".to_string(),
JsonSchema::Boolean {
description: Some("Whether to request macOS calendar access.".to_string()),
},
),
]),
required: None,
additional_properties: Some(false.into()),
}
}
fn create_permissions_schema() -> JsonSchema {
JsonSchema::Object {
properties: BTreeMap::from([
("network".to_string(), create_network_permissions_schema()),
(
"file_system".to_string(),
create_file_system_permissions_schema(),
),
("macos".to_string(), create_macos_permissions_schema()),
]),
required: None,
additional_properties: Some(false.into()),
}
}
fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap<String, JsonSchema> {
let mut properties = BTreeMap::from([
(
@@ -299,103 +391,7 @@ fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap<Stri
if request_permission_enabled {
properties.insert(
"additional_permissions".to_string(),
JsonSchema::Object {
properties: BTreeMap::from([
(
"network".to_string(),
JsonSchema::Object {
properties: BTreeMap::from([(
"enabled".to_string(),
JsonSchema::Boolean {
description: Some(
"Set to true to enable network access for this command."
.to_string(),
),
},
)]),
required: None,
additional_properties: Some(false.into()),
},
),
(
"file_system".to_string(),
JsonSchema::Object {
properties: BTreeMap::from([
(
"read".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
"Additional filesystem paths to grant read access for this command."
.to_string(),
),
},
),
(
"write".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
"Additional filesystem paths to grant write access for this command."
.to_string(),
),
},
),
]),
required: None,
additional_properties: Some(false.into()),
},
),
(
"macos".to_string(),
JsonSchema::Object {
properties: BTreeMap::from([
(
"preferences".to_string(),
JsonSchema::String {
description: Some(
"Additional macOS preferences access for this command. Supported values: \"readonly\" or \"readwrite\"."
.to_string(),
),
},
),
(
"automations".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
"Additional macOS automation targets for this command as bundle IDs, or use true in clients that support boolean union payloads."
.to_string(),
),
},
),
(
"accessibility".to_string(),
JsonSchema::Boolean {
description: Some(
"Set to true to allow macOS accessibility APIs for this command."
.to_string(),
),
},
),
(
"calendar".to_string(),
JsonSchema::Boolean {
description: Some(
"Set to true to allow macOS Calendar access for this command."
.to_string(),
),
},
),
]),
required: None,
additional_properties: Some(false.into()),
},
),
]),
required: None,
additional_properties: Some(false.into()),
},
create_permissions_schema(),
);
}
@@ -1163,6 +1159,30 @@ fn create_request_user_input_tool(
})
}
fn create_request_permissions_tool() -> ToolSpec {
let mut properties = BTreeMap::new();
properties.insert(
"reason".to_string(),
JsonSchema::String {
description: Some(
"Optional short explanation for why additional permissions are needed.".to_string(),
),
},
);
properties.insert("permissions".to_string(), create_permissions_schema());
ToolSpec::Function(ResponsesApiTool {
name: "request_permissions".to_string(),
description: request_permissions_tool_description(),
strict: false,
parameters: JsonSchema::Object {
properties,
required: Some(vec!["permissions".to_string()]),
additional_properties: Some(false.into()),
},
})
}
fn create_close_agent_tool() -> ToolSpec {
let mut properties = BTreeMap::new();
properties.insert(
@@ -1892,6 +1912,7 @@ pub(crate) fn build_specs_with_available_models(
use crate::tools::handlers::MultiAgentHandler;
use crate::tools::handlers::PlanHandler;
use crate::tools::handlers::ReadFileHandler;
use crate::tools::handlers::RequestPermissionsHandler;
use crate::tools::handlers::RequestUserInputHandler;
use crate::tools::handlers::SearchToolBm25Handler;
use crate::tools::handlers::ShellCommandHandler;
@@ -1912,6 +1933,7 @@ pub(crate) fn build_specs_with_available_models(
let mcp_handler = Arc::new(McpHandler);
let mcp_resource_handler = Arc::new(McpResourceHandler);
let shell_command_handler = Arc::new(ShellCommandHandler::from(config.shell_command_backend));
let request_permissions_handler = Arc::new(RequestPermissionsHandler);
let request_user_input_handler = Arc::new(RequestUserInputHandler {
default_mode_request_user_input: config.default_mode_request_user_input,
});
@@ -1985,6 +2007,11 @@ pub(crate) fn build_specs_with_available_models(
builder.register_handler("request_user_input", request_user_input_handler);
}
if config.request_permissions_tool_enabled {
builder.push_spec(create_request_permissions_tool());
builder.register_handler("request_permissions", request_permissions_handler);
}
if config.search_tool
&& let Some(app_tools) = app_tools
{
@@ -2063,11 +2090,11 @@ pub(crate) fn build_specs_with_available_models(
filters: config
.web_search_config
.as_ref()
.and_then(|cfg| cfg.filters.clone()),
.and_then(|cfg| cfg.filters.clone().map(Into::into)),
user_location: config
.web_search_config
.as_ref()
.and_then(|cfg| cfg.user_location.clone()),
.and_then(|cfg| cfg.user_location.clone().map(Into::into)),
search_context_size: config
.web_search_config
.as_ref()
@@ -2370,6 +2397,11 @@ mod tests {
expected.insert(tool_name(&spec).to_string(), spec);
}
if config.request_permission_enabled {
let spec = create_request_permissions_tool();
expected.insert(tool_name(&spec).to_string(), spec);
}
// Exact name set match — this is the only test allowed to fail when tools change.
let actual_names: HashSet<_> = actual.keys().cloned().collect();
let expected_names: HashSet<_> = expected.keys().cloned().collect();
@@ -2498,6 +2530,55 @@ mod tests {
);
}
#[test]
fn request_permissions_requires_feature_flag() {
let config = test_config();
let model_info =
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
let features = Features::with_defaults();
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_info: &model_info,
features: &features,
web_search_mode: Some(WebSearchMode::Cached),
session_source: SessionSource::Cli,
});
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
assert_lacks_tool_name(&tools, "request_permissions");
let mut features = Features::with_defaults();
features.enable(Feature::RequestPermissionsTool);
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_info: &model_info,
features: &features,
web_search_mode: Some(WebSearchMode::Cached),
session_source: SessionSource::Cli,
});
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
let request_permissions_tool = find_tool(&tools, "request_permissions");
assert_eq!(
request_permissions_tool.spec,
create_request_permissions_tool()
);
}
#[test]
fn request_permissions_tool_is_independent_from_additional_permissions() {
let config = test_config();
let model_info =
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
let mut features = Features::with_defaults();
features.enable(Feature::RequestPermissions);
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_info: &model_info,
features: &features,
web_search_mode: Some(WebSearchMode::Cached),
session_source: SessionSource::Cli,
});
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
assert_lacks_tool_name(&tools, "request_permissions");
}
#[test]
fn spawn_agent_tool_schema_includes_model_overrides() {
let config = test_config();
@@ -2824,8 +2905,12 @@ mod tests {
tool.spec,
ToolSpec::WebSearch {
external_web_access: Some(true),
filters: web_search_config.filters,
user_location: web_search_config.user_location,
filters: web_search_config
.filters
.map(crate::client_common::tools::ResponsesApiWebSearchFilters::from),
user_location: web_search_config
.user_location
.map(crate::client_common::tools::ResponsesApiWebSearchUserLocation::from),
search_context_size: web_search_config.search_context_size,
search_content_types: None,
}
@@ -3667,6 +3752,67 @@ Examples of valid command strings:
assert!(additional_properties.contains_key("macos"));
}
#[test]
fn request_permissions_tool_includes_full_permission_schema() {
let tool = super::create_request_permissions_tool();
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else {
panic!("expected function tool");
};
let JsonSchema::Object { properties, .. } = parameters else {
panic!("expected object parameters");
};
let Some(JsonSchema::Object {
properties: permission_properties,
additional_properties,
..
}) = properties.get("permissions")
else {
panic!("expected permissions object");
};
assert_eq!(additional_properties, &Some(false.into()));
assert!(permission_properties.contains_key("network"));
assert!(permission_properties.contains_key("file_system"));
assert!(permission_properties.contains_key("macos"));
let Some(JsonSchema::Object {
properties: network_properties,
additional_properties,
..
}) = permission_properties.get("network")
else {
panic!("expected network object");
};
assert_eq!(additional_properties, &Some(false.into()));
assert!(network_properties.contains_key("enabled"));
let Some(JsonSchema::Object {
properties: file_system_properties,
additional_properties,
..
}) = permission_properties.get("file_system")
else {
panic!("expected file_system object");
};
assert_eq!(additional_properties, &Some(false.into()));
assert!(file_system_properties.contains_key("read"));
assert!(file_system_properties.contains_key("write"));
let Some(JsonSchema::Object {
properties: macos_properties,
additional_properties,
..
}) = permission_properties.get("macos")
else {
panic!("expected macos object");
};
assert_eq!(additional_properties, &Some(false.into()));
assert!(macos_properties.contains_key("preferences"));
assert!(macos_properties.contains_key("automations"));
assert!(macos_properties.contains_key("accessibility"));
assert!(macos_properties.contains_key("calendar"));
}
#[test]
fn test_shell_command_tool() {
let tool = super::create_shell_command_tool(true, false);
+2 -3
View File
@@ -1,6 +1,8 @@
use std::time::Duration;
use std::time::Instant;
use codex_otel::metrics::names::TURN_TTFM_DURATION_METRIC;
use codex_otel::metrics::names::TURN_TTFT_DURATION_METRIC;
use codex_protocol::items::TurnItem;
use codex_protocol::models::ResponseItem;
use tokio::sync::Mutex;
@@ -9,9 +11,6 @@ use crate::ResponseEvent;
use crate::codex::TurnContext;
use crate::stream_events_utils::raw_assistant_output_text_from_item;
const TURN_TTFT_DURATION_METRIC: &str = "codex.turn.ttft.duration_ms";
const TURN_TTFM_DURATION_METRIC: &str = "codex.turn.ttfm.duration_ms";
pub(crate) async fn record_turn_ttft_metric(turn_context: &TurnContext, event: &ResponseEvent) {
let Some(duration) = turn_context
.turn_timing_state
+2
View File
@@ -95,6 +95,7 @@ pub(crate) struct ExecCommandRequest {
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
pub additional_permissions_preapproved: bool,
pub justification: Option<String>,
pub prefix_rule: Option<Vec<String>>,
}
@@ -240,6 +241,7 @@ mod tests {
tty: true,
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
additional_permissions_preapproved: false,
justification: None,
prefix_rule: None,
},
@@ -583,7 +583,11 @@ impl UnifiedExecProcessManager {
command: &request.command,
approval_policy: context.turn.approval_policy.value(),
sandbox_policy: context.turn.sandbox_policy.get(),
sandbox_permissions: request.sandbox_permissions,
sandbox_permissions: if request.additional_permissions_preapproved {
crate::sandboxing::SandboxPermissions::UseDefault
} else {
request.sandbox_permissions
},
prefix_rule: request.prefix_rule.clone(),
})
.await;
+2
View File
@@ -104,6 +104,8 @@ mod remote_models;
mod request_compression;
#[cfg(not(target_os = "windows"))]
mod request_permissions;
#[cfg(not(target_os = "windows"))]
mod request_permissions_tool;
mod request_user_input;
mod resume;
mod resume_warning;
+19 -3
View File
@@ -14,6 +14,7 @@ use core_test_support::responses::ev_local_shell_call;
use core_test_support::responses::ev_message_item_added;
use core_test_support::responses::ev_output_text_delta;
use core_test_support::responses::ev_reasoning_item;
use core_test_support::responses::ev_reasoning_item_added;
use core_test_support::responses::ev_reasoning_summary_text_delta;
use core_test_support::responses::ev_reasoning_text_delta;
use core_test_support::responses::ev_response_created;
@@ -629,20 +630,35 @@ async fn record_responses_sets_span_fields_for_response_events() {
let sse_body = sse(vec![
ev_response_created("resp-1"),
ev_function_call("call-1", "fn", "{\"value\":1}"),
ev_custom_tool_call("custom-1", "custom_tool", "{\"key\":\"value\"}"),
serde_json::json!({
"type": "response.output_item.added",
"item": {
"type": "function_call",
"call_id": "call-1",
"name": "fn",
"arguments": "{\"value\":1}"
}
}),
ev_message_item_added("msg-added", "hi there"),
ev_reasoning_item_added("reasoning-1", &["summary"]),
ev_output_text_delta("delta"),
ev_reasoning_summary_text_delta("summary-delta"),
ev_reasoning_text_delta("raw-delta"),
ev_function_call("call-1", "fn", "{\"key\":\"value\"}"),
ev_custom_tool_call("custom-1", "custom_tool", "{\"key\":\"value\"}"),
ev_assistant_message("msg-1", "agent"),
ev_reasoning_item("reasoning-1", &["summary"], &[]),
ev_completed("resp-1"),
]);
mount_response_once(&server, sse_response(sse_body)).await;
mount_response_once(
&server,
sse_response(sse(vec![
ev_assistant_message("msg-2", "follow-up complete"),
ev_completed("resp-2"),
])),
)
.await;
let TestCodex { codex, .. } = test_codex()
.with_config(|config| {
@@ -490,7 +490,6 @@ async fn permissions_message_includes_writable_roots() -> Result<()> {
let expected = DeveloperInstructions::from_policy(
&sandbox_policy,
AskForApproval::OnRequest,
false,
&Policy::empty(),
test.config.cwd.as_path(),
false,
+150 -116
View File
@@ -29,6 +29,7 @@ use core_test_support::wait_for_event_match;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use serial_test::serial;
use std::ffi::OsString;
use std::fs;
use std::time::Duration;
@@ -37,7 +38,6 @@ use tokio::sync::oneshot;
const STARTUP_CONTEXT_HEADER: &str = "Startup context from Codex.";
const MEMORY_PROMPT_PHRASE: &str =
"You have access to a memory folder with guidance from prior runs.";
fn websocket_request_text(
request: &core_test_support::responses::WebSocketRequest,
) -> Option<String> {
@@ -54,6 +54,33 @@ fn websocket_request_instructions(
.map(str::to_owned)
}
async fn wait_for_matching_websocket_request<F>(
server: &core_test_support::responses::WebSocketTestServer,
description: &str,
predicate: F,
) -> core_test_support::responses::WebSocketRequest
where
F: Fn(&core_test_support::responses::WebSocketRequest) -> bool,
{
let deadline = tokio::time::Instant::now() + Duration::from_secs(10);
loop {
if let Some(request) = server
.connections()
.iter()
.flat_map(|connection| connection.iter())
.find(|request| predicate(request))
.cloned()
{
return request;
}
assert!(
tokio::time::Instant::now() < deadline,
"timed out waiting for {description}"
);
tokio::time::sleep(Duration::from_millis(10)).await;
}
}
async fn seed_recent_thread(
test: &TestCodex,
title: &str,
@@ -230,6 +257,7 @@ async fn conversation_start_audio_text_close_round_trip() -> Result<()> {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[serial(openai_api_key_env)]
async fn conversation_start_uses_openai_env_key_fallback_with_chatgpt_auth() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -632,21 +660,23 @@ async fn conversation_uses_experimental_realtime_ws_backend_prompt_override() ->
async fn conversation_uses_experimental_realtime_ws_startup_context_override() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_websocket_server(vec![
vec![],
vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_custom_context", "instructions": "prompt from config" }
})]],
])
let startup_server = start_websocket_server(vec![vec![]]).await;
let realtime_server = start_websocket_server(vec![vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_custom_context", "instructions": "prompt from config" }
})]]])
.await;
let mut builder = test_codex().with_config(|config| {
config.experimental_realtime_ws_backend_prompt = Some("prompt from config".to_string());
config.experimental_realtime_ws_startup_context =
Some("custom startup context".to_string());
let mut builder = test_codex().with_config({
let realtime_base_url = realtime_server.uri().to_string();
move |config| {
config.experimental_realtime_ws_base_url = Some(realtime_base_url);
config.experimental_realtime_ws_backend_prompt = Some("prompt from config".to_string());
config.experimental_realtime_ws_startup_context =
Some("custom startup context".to_string());
}
});
let test = builder.build_with_websocket_server(&server).await?;
let test = builder.build_with_websocket_server(&startup_server).await?;
seed_recent_thread(
&test,
"Recent work: cleaned up startup flows and reviewed websocket routing.",
@@ -656,7 +686,11 @@ async fn conversation_uses_experimental_realtime_ws_startup_context_override() -
.await?;
fs::create_dir_all(test.workspace_path("docs"))?;
fs::write(test.workspace_path("README.md"), "workspace marker")?;
assert!(server.wait_for_handshakes(1, Duration::from_secs(2)).await);
assert!(
startup_server
.wait_for_handshakes(1, Duration::from_secs(2))
.await
);
test.codex
.submit(Op::RealtimeConversationStart(ConversationStartParams {
@@ -665,17 +699,12 @@ async fn conversation_uses_experimental_realtime_ws_startup_context_override() -
}))
.await?;
wait_for_event_match(&test.codex, |msg| match msg {
EventMsg::RealtimeConversationRealtime(RealtimeConversationRealtimeEvent {
payload: RealtimeEvent::SessionUpdated { session_id, .. },
}) if session_id == "sess_custom_context" => Some(Ok(())),
EventMsg::Error(err) => Some(Err(err.clone())),
_ => None,
})
.await
.unwrap_or_else(|err: ErrorEvent| panic!("conversation start failed: {err:?}"));
let startup_context_request = server.wait_for_request(1, 0).await;
let startup_context_request = wait_for_matching_websocket_request(
&realtime_server,
"startup context request with instructions",
|request| websocket_request_instructions(request).is_some(),
)
.await;
let instructions = websocket_request_instructions(&startup_context_request)
.expect("custom startup context request should contain instructions");
@@ -683,7 +712,8 @@ async fn conversation_uses_experimental_realtime_ws_startup_context_override() -
assert!(!instructions.contains(STARTUP_CONTEXT_HEADER));
assert!(!instructions.contains("## Machine / Workspace Map"));
server.shutdown().await;
startup_server.shutdown().await;
realtime_server.shutdown().await;
Ok(())
}
@@ -691,20 +721,22 @@ async fn conversation_uses_experimental_realtime_ws_startup_context_override() -
async fn conversation_disables_realtime_startup_context_with_empty_override() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_websocket_server(vec![
vec![],
vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_no_context", "instructions": "prompt from config" }
})]],
])
let startup_server = start_websocket_server(vec![vec![]]).await;
let realtime_server = start_websocket_server(vec![vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_no_context", "instructions": "prompt from config" }
})]]])
.await;
let mut builder = test_codex().with_config(|config| {
config.experimental_realtime_ws_backend_prompt = Some("prompt from config".to_string());
config.experimental_realtime_ws_startup_context = Some(String::new());
let mut builder = test_codex().with_config({
let realtime_base_url = realtime_server.uri().to_string();
move |config| {
config.experimental_realtime_ws_base_url = Some(realtime_base_url);
config.experimental_realtime_ws_backend_prompt = Some("prompt from config".to_string());
config.experimental_realtime_ws_startup_context = Some(String::new());
}
});
let test = builder.build_with_websocket_server(&server).await?;
let test = builder.build_with_websocket_server(&startup_server).await?;
seed_recent_thread(
&test,
"Recent work: cleaned up startup flows and reviewed websocket routing.",
@@ -714,7 +746,11 @@ async fn conversation_disables_realtime_startup_context_with_empty_override() ->
.await?;
fs::create_dir_all(test.workspace_path("docs"))?;
fs::write(test.workspace_path("README.md"), "workspace marker")?;
assert!(server.wait_for_handshakes(1, Duration::from_secs(2)).await);
assert!(
startup_server
.wait_for_handshakes(1, Duration::from_secs(2))
.await
);
test.codex
.submit(Op::RealtimeConversationStart(ConversationStartParams {
@@ -723,17 +759,12 @@ async fn conversation_disables_realtime_startup_context_with_empty_override() ->
}))
.await?;
wait_for_event_match(&test.codex, |msg| match msg {
EventMsg::RealtimeConversationRealtime(RealtimeConversationRealtimeEvent {
payload: RealtimeEvent::SessionUpdated { session_id, .. },
}) if session_id == "sess_no_context" => Some(Ok(())),
EventMsg::Error(err) => Some(Err(err.clone())),
_ => None,
})
.await
.unwrap_or_else(|err: ErrorEvent| panic!("conversation start failed: {err:?}"));
let startup_context_request = server.wait_for_request(1, 0).await;
let startup_context_request = wait_for_matching_websocket_request(
&realtime_server,
"startup context disable request with instructions",
|request| websocket_request_instructions(request).is_some(),
)
.await;
let instructions = websocket_request_instructions(&startup_context_request)
.expect("startup context disable request should contain instructions");
@@ -741,7 +772,8 @@ async fn conversation_disables_realtime_startup_context_with_empty_override() ->
assert!(!instructions.contains(STARTUP_CONTEXT_HEADER));
assert!(!instructions.contains("## Machine / Workspace Map"));
server.shutdown().await;
startup_server.shutdown().await;
realtime_server.shutdown().await;
Ok(())
}
@@ -749,17 +781,20 @@ async fn conversation_disables_realtime_startup_context_with_empty_override() ->
async fn conversation_start_injects_startup_context_from_thread_history() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_websocket_server(vec![
vec![],
vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_context", "instructions": "backend prompt" }
})]],
])
let startup_server = start_websocket_server(vec![vec![]]).await;
let realtime_server = start_websocket_server(vec![vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_context", "instructions": "backend prompt" }
})]]])
.await;
let mut builder = test_codex();
let test = builder.build_with_websocket_server(&server).await?;
let mut builder = test_codex().with_config({
let realtime_base_url = realtime_server.uri().to_string();
move |config| {
config.experimental_realtime_ws_base_url = Some(realtime_base_url);
}
});
let test = builder.build_with_websocket_server(&startup_server).await?;
seed_recent_thread(
&test,
"Recent work: cleaned up startup flows and reviewed websocket routing.",
@@ -777,17 +812,12 @@ async fn conversation_start_injects_startup_context_from_thread_history() -> Res
}))
.await?;
wait_for_event_match(&test.codex, |msg| match msg {
EventMsg::RealtimeConversationRealtime(RealtimeConversationRealtimeEvent {
payload: RealtimeEvent::SessionUpdated { session_id, .. },
}) if session_id == "sess_context" => Some(Ok(())),
EventMsg::Error(err) => Some(Err(err.clone())),
_ => None,
})
.await
.unwrap_or_else(|err: ErrorEvent| panic!("conversation start failed: {err:?}"));
let startup_context_request = server.wait_for_request(1, 0).await;
let startup_context_request = wait_for_matching_websocket_request(
&realtime_server,
"startup context request with instructions",
|request| websocket_request_instructions(request).is_some(),
)
.await;
let startup_context = websocket_request_instructions(&startup_context_request)
.expect("startup context request should contain instructions");
@@ -802,7 +832,8 @@ async fn conversation_start_injects_startup_context_from_thread_history() -> Res
assert!(startup_context.contains("README.md"));
assert!(!startup_context.contains(MEMORY_PROMPT_PHRASE));
server.shutdown().await;
startup_server.shutdown().await;
realtime_server.shutdown().await;
Ok(())
}
@@ -810,17 +841,20 @@ async fn conversation_start_injects_startup_context_from_thread_history() -> Res
async fn conversation_startup_context_falls_back_to_workspace_map() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_websocket_server(vec![
vec![],
vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_workspace", "instructions": "backend prompt" }
})]],
])
let startup_server = start_websocket_server(vec![vec![]]).await;
let realtime_server = start_websocket_server(vec![vec![vec![json!({
"type": "session.updated",
"session": { "id": "sess_workspace", "instructions": "backend prompt" }
})]]])
.await;
let mut builder = test_codex();
let test = builder.build_with_websocket_server(&server).await?;
let mut builder = test_codex().with_config({
let realtime_base_url = realtime_server.uri().to_string();
move |config| {
config.experimental_realtime_ws_base_url = Some(realtime_base_url);
}
});
let test = builder.build_with_websocket_server(&startup_server).await?;
fs::create_dir_all(test.workspace_path("codex-rs/core"))?;
fs::write(test.workspace_path("notes.txt"), "workspace marker")?;
@@ -831,17 +865,12 @@ async fn conversation_startup_context_falls_back_to_workspace_map() -> Result<()
}))
.await?;
wait_for_event_match(&test.codex, |msg| match msg {
EventMsg::RealtimeConversationRealtime(RealtimeConversationRealtimeEvent {
payload: RealtimeEvent::SessionUpdated { session_id, .. },
}) if session_id == "sess_workspace" => Some(Ok(())),
EventMsg::Error(err) => Some(Err(err.clone())),
_ => None,
})
.await
.unwrap_or_else(|err: ErrorEvent| panic!("conversation start failed: {err:?}"));
let startup_context_request = server.wait_for_request(1, 0).await;
let startup_context_request = wait_for_matching_websocket_request(
&realtime_server,
"workspace-map startup context request with instructions",
|request| websocket_request_instructions(request).is_some(),
)
.await;
let startup_context = websocket_request_instructions(&startup_context_request)
.expect("startup context request should contain instructions");
@@ -850,7 +879,8 @@ async fn conversation_startup_context_falls_back_to_workspace_map() -> Result<()
assert!(startup_context.contains("notes.txt"));
assert!(startup_context.contains("codex-rs/"));
server.shutdown().await;
startup_server.shutdown().await;
realtime_server.shutdown().await;
Ok(())
}
@@ -858,21 +888,24 @@ async fn conversation_startup_context_falls_back_to_workspace_map() -> Result<()
async fn conversation_startup_context_is_truncated_and_sent_once_per_start() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_websocket_server(vec![
let startup_server = start_websocket_server(vec![vec![]]).await;
let realtime_server = start_websocket_server(vec![vec![
vec![json!({
"type": "session.updated",
"session": { "id": "sess_truncated", "instructions": "backend prompt" }
})],
vec![],
vec![
vec![json!({
"type": "session.updated",
"session": { "id": "sess_truncated", "instructions": "backend prompt" }
})],
vec![],
],
])
]])
.await;
let oversized_summary = "recent work ".repeat(3_500);
let mut builder = test_codex();
let test = builder.build_with_websocket_server(&server).await?;
let mut builder = test_codex().with_config({
let realtime_base_url = realtime_server.uri().to_string();
move |config| {
config.experimental_realtime_ws_base_url = Some(realtime_base_url);
}
});
let test = builder.build_with_websocket_server(&startup_server).await?;
seed_recent_thread(&test, &oversized_summary, "summary", "oversized").await?;
fs::write(test.workspace_path("marker.txt"), "marker")?;
@@ -883,17 +916,12 @@ async fn conversation_startup_context_is_truncated_and_sent_once_per_start() ->
}))
.await?;
wait_for_event_match(&test.codex, |msg| match msg {
EventMsg::RealtimeConversationRealtime(RealtimeConversationRealtimeEvent {
payload: RealtimeEvent::SessionUpdated { session_id, .. },
}) if session_id == "sess_truncated" => Some(Ok(())),
EventMsg::Error(err) => Some(Err(err.clone())),
_ => None,
})
.await
.unwrap_or_else(|err: ErrorEvent| panic!("conversation start failed: {err:?}"));
let startup_context_request = server.wait_for_request(1, 0).await;
let startup_context_request = wait_for_matching_websocket_request(
&realtime_server,
"truncated startup context request with instructions",
|request| websocket_request_instructions(request).is_some(),
)
.await;
let startup_context = websocket_request_instructions(&startup_context_request)
.expect("startup context request should contain instructions");
assert!(startup_context.contains(STARTUP_CONTEXT_HEADER));
@@ -905,13 +933,19 @@ async fn conversation_startup_context_is_truncated_and_sent_once_per_start() ->
}))
.await?;
let explicit_text_request = server.wait_for_request(1, 1).await;
let explicit_text_request = wait_for_matching_websocket_request(
&realtime_server,
"explicit realtime text request",
|request| websocket_request_text(request).as_deref() == Some("hello"),
)
.await;
assert_eq!(
websocket_request_text(&explicit_text_request),
Some("hello".to_string())
);
server.shutdown().await;
startup_server.shutdown().await;
realtime_server.shutdown().await;
Ok(())
}
@@ -12,6 +12,8 @@ use codex_protocol::protocol::ExecApprovalRequestEvent;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::responses::ev_assistant_message;
@@ -19,10 +21,10 @@ 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::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
#[cfg(target_os = "macos")]
use core_test_support::skip_if_sandbox;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
@@ -97,7 +99,65 @@ fn shell_event_with_request_permissions(
Ok(ev_function_call(call_id, "shell_command", &args_str))
}
#[cfg(target_os = "macos")]
fn request_permissions_tool_event(
call_id: &str,
reason: &str,
permissions: &PermissionProfile,
) -> Result<Value> {
let args = json!({
"reason": reason,
"permissions": permissions,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "request_permissions", &args_str))
}
fn shell_command_event(call_id: &str, command: &str) -> Result<Value> {
let args = json!({
"command": command,
"timeout_ms": 1_000_u64,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "shell_command", &args_str))
}
fn exec_command_event(call_id: &str, command: &str) -> Result<Value> {
let args = json!({
"cmd": command,
"yield_time_ms": 1_000_u64,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "exec_command", &args_str))
}
fn exec_command_event_with_request_permissions(
call_id: &str,
command: &str,
additional_permissions: &PermissionProfile,
) -> Result<Value> {
let args = json!({
"cmd": command,
"yield_time_ms": 1_000_u64,
"sandbox_permissions": SandboxPermissions::WithAdditionalPermissions,
"additional_permissions": additional_permissions,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "exec_command", &args_str))
}
fn exec_command_event_with_missing_additional_permissions(
call_id: &str,
command: &str,
) -> Result<Value> {
let args = json!({
"cmd": command,
"yield_time_ms": 1_000_u64,
"sandbox_permissions": SandboxPermissions::WithAdditionalPermissions,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "exec_command", &args_str))
}
fn shell_event_with_raw_request_permissions(
call_id: &str,
command: &str,
@@ -177,6 +237,46 @@ async fn expect_exec_approval(
}
}
async fn wait_for_exec_approval_or_completion(
test: &TestCodex,
) -> Option<ExecApprovalRequestEvent> {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::ExecApprovalRequest(approval) => Some(approval),
EventMsg::TurnComplete(_) => None,
other => panic!("unexpected event: {other:?}"),
}
}
async fn expect_request_permissions_event(
test: &TestCodex,
expected_call_id: &str,
) -> PermissionProfile {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::RequestPermissions(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::RequestPermissions(request) => {
assert_eq!(request.call_id, expected_call_id);
request.permissions
}
EventMsg::TurnComplete(_) => panic!("expected request_permissions before completion"),
other => panic!("unexpected event: {other:?}"),
}
}
fn workspace_write_excluding_tmp() -> SandboxPolicy {
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
@@ -187,8 +287,27 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy {
}
}
fn requested_directory_write_permissions(path: &Path) -> PermissionProfile {
PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![absolute_path(path)]),
}),
..Default::default()
}
}
fn normalized_directory_write_permissions(path: &Path) -> Result<PermissionProfile> {
Ok(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
}),
..Default::default()
})
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn with_additional_permissions_requires_approval_under_on_request() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
@@ -205,6 +324,10 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
@@ -273,7 +396,6 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn relative_additional_permissions_resolve_against_tool_workdir() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
@@ -290,6 +412,10 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
@@ -387,6 +513,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
@@ -483,6 +613,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
@@ -562,7 +696,6 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
@@ -579,6 +712,10 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
@@ -665,7 +802,6 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn with_additional_permissions_denied_approval_blocks_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -680,6 +816,10 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
@@ -763,3 +903,759 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
let _ = fs::remove_file(outside_write);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let outside_write = outside_dir.path().join("sticky-write.txt");
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"sticky-grant-ok", outside_write, outside_write
);
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![absolute_path(outside_dir.path())]),
}),
..Default::default()
};
let normalized_requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![AbsolutePathBuf::try_from(
outside_dir.path().canonicalize()?,
)?]),
}),
..Default::default()
};
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-sticky-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-sticky-1"),
]),
sse(vec![
ev_response_created("resp-sticky-2"),
exec_command_event("exec-call", &command)?,
ev_completed("resp-sticky-2"),
]),
sse(vec![
ev_response_created("resp-sticky-3"),
ev_assistant_message("msg-sticky-1", "done"),
ev_completed("resp-sticky-3"),
]),
],
)
.await;
submit_turn(
&test,
"write outside the workspace",
approval_policy,
sandbox_policy,
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
},
})
.await?;
if let Some(approval) = wait_for_exec_approval_or_completion(&test).await {
assert_eq!(
approval.additional_permissions,
Some(normalized_requested_permissions.clone())
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
}
let exec_output = responses
.function_call_output_text("exec-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected exec-call output"));
let result = parse_result(&exec_output);
assert_eq!(result.exit_code, Some(0));
assert_eq!(result.stdout.trim(), "sticky-grant-ok");
assert_eq!(fs::read_to_string(&outside_write)?, "sticky-grant-ok");
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_request() -> Result<()>
{
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let outside_write = outside_dir.path().join("sticky-explicit-write.txt");
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"sticky-explicit-grant-ok", outside_write, outside_write
);
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
let normalized_requested_permissions =
normalized_directory_write_permissions(outside_dir.path())?;
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-sticky-explicit-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-sticky-explicit-1"),
]),
sse(vec![
ev_response_created("resp-sticky-explicit-2"),
exec_command_event_with_request_permissions(
"exec-call",
&command,
&requested_permissions,
)?,
ev_completed("resp-sticky-explicit-2"),
]),
sse(vec![
ev_response_created("resp-sticky-explicit-3"),
ev_assistant_message("msg-sticky-explicit-1", "done"),
ev_completed("resp-sticky-explicit-3"),
]),
],
)
.await;
submit_turn(
&test,
"write outside the workspace",
approval_policy,
sandbox_policy,
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
})
.await?;
if let Some(approval) = wait_for_exec_approval_or_completion(&test).await {
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
}
let exec_output = responses
.function_call_output_text("exec-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected exec-call output"));
let result = parse_result(&exec_output);
assert!(
result.exit_code.is_none_or(|exit_code| exit_code == 0),
"expected success output, got exit_code={:?}, stdout={:?}",
result.exit_code,
result.stdout
);
assert_eq!(result.stdout.trim(), "sticky-explicit-grant-ok");
assert_eq!(
fs::read_to_string(&outside_write)?,
"sticky-explicit-grant-ok"
);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let outside_write = outside_dir.path().join("sticky-shell-write.txt");
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"sticky-shell-grant-ok", outside_write, outside_write
);
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
let normalized_requested_permissions =
normalized_directory_write_permissions(outside_dir.path())?;
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-sticky-shell-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-sticky-shell-1"),
]),
sse(vec![
ev_response_created("resp-sticky-shell-2"),
shell_command_event("shell-call", &command)?,
ev_completed("resp-sticky-shell-2"),
]),
sse(vec![
ev_response_created("resp-sticky-shell-3"),
ev_assistant_message("msg-sticky-shell-1", "done"),
ev_completed("resp-sticky-shell-3"),
]),
],
)
.await;
submit_turn(
&test,
"write outside the workspace",
approval_policy,
sandbox_policy,
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
},
})
.await?;
if let Some(approval) = wait_for_exec_approval_or_completion(&test).await {
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
}
let shell_output = responses
.function_call_output_text("shell-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected shell-call output"));
let result = parse_result(&shell_output);
assert!(
result.exit_code.is_none_or(|exit_code| exit_code == 0),
"expected success output, got exit_code={:?}, stdout={:?}",
result.exit_code,
result.stdout
);
assert_eq!(result.stdout.trim(), "sticky-shell-grant-ok");
assert_eq!(fs::read_to_string(&outside_write)?, "sticky-shell-grant-ok");
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let first_dir = tempfile::tempdir()?;
let second_dir = tempfile::tempdir()?;
let second_write = second_dir.path().join("partial-grant-write.txt");
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"partial-grant-ok", second_write, second_write
);
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![
absolute_path(first_dir.path()),
absolute_path(second_dir.path()),
]),
}),
..Default::default()
};
let normalized_requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
]),
}),
..Default::default()
};
let granted_permissions = normalized_directory_write_permissions(first_dir.path())?;
let second_dir_permissions = requested_directory_write_permissions(second_dir.path());
let merged_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
]),
}),
..Default::default()
};
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-partial-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-partial-1"),
]),
sse(vec![
ev_response_created("resp-partial-2"),
exec_command_event_with_request_permissions(
"exec-call",
&command,
&second_dir_permissions,
)?,
ev_completed("resp-partial-2"),
]),
sse(vec![
ev_response_created("resp-partial-3"),
ev_assistant_message("msg-partial-1", "done"),
ev_completed("resp-partial-3"),
]),
],
)
.await;
submit_turn(
&test,
"write outside the workspace",
approval_policy,
sandbox_policy,
)
.await?;
let initial_request = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(initial_request, normalized_requested_permissions);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: granted_permissions.clone(),
scope: PermissionGrantScope::Turn,
},
})
.await?;
let approval = expect_exec_approval(&test, &command).await;
let approval_permissions = approval
.additional_permissions
.clone()
.unwrap_or_else(|| panic!("expected merged additional permissions"));
assert_eq!(approval_permissions.network, None);
assert_eq!(approval_permissions.macos, None);
let approval_file_system = approval_permissions
.file_system
.unwrap_or_else(|| panic!("expected filesystem permissions"));
assert!(approval_file_system.read.as_ref().is_none_or(Vec::is_empty));
let mut approval_writes = approval_file_system.write.unwrap_or_default();
approval_writes.sort_by_key(|path| path.display().to_string());
let mut expected_writes = merged_permissions
.file_system
.unwrap_or_else(|| panic!("expected merged filesystem permissions"))
.write
.unwrap_or_default();
expected_writes.sort_by_key(|path| path.display().to_string());
assert_eq!(approval_writes, expected_writes);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
let exec_output = responses
.function_call_output_text("exec-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected exec-call output"));
let result = parse_result(&exec_output);
assert_eq!(result.exit_code, Some(0));
assert_eq!(result.stdout.trim(), "partial-grant-ok");
assert_eq!(fs::read_to_string(&second_write)?, "partial-grant-ok");
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
let normalized_requested_permissions =
normalized_directory_write_permissions(outside_dir.path())?;
let _first_turn = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-turn-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-turn-1"),
]),
sse(vec![
ev_response_created("resp-turn-2"),
ev_assistant_message("msg-turn-1", "done"),
ev_completed("resp-turn-2"),
]),
],
)
.await;
submit_turn(
&test,
"request permissions for later use",
approval_policy,
sandbox_policy.clone(),
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
})
.await?;
wait_for_completion(&test).await;
let second_turn = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-turn-3"),
exec_command_event_with_missing_additional_permissions(
"exec-call",
"printf 'should not run'",
)?,
ev_completed("resp-turn-3"),
]),
sse(vec![
ev_response_created("resp-turn-4"),
ev_assistant_message("msg-turn-2", "done"),
ev_completed("resp-turn-4"),
]),
],
)
.await;
submit_turn(
&test,
"try to reuse permissions in a later turn",
approval_policy,
sandbox_policy,
)
.await?;
wait_for_completion(&test).await;
let output = second_turn
.function_call_output_text("exec-call")
.unwrap_or_else(|| panic!("expected exec-call output"));
assert!(output.contains("missing `additional_permissions`"));
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn request_permissions_session_grants_carry_across_turns() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let outside_dir = tempfile::tempdir()?;
let outside_write = outside_dir.path().join("session-sticky-write.txt");
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
let normalized_requested_permissions =
normalized_directory_write_permissions(outside_dir.path())?;
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"session-sticky-ok", outside_write, outside_write
);
let _first_turn = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-session-turn-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-session-turn-1"),
]),
sse(vec![
ev_response_created("resp-session-turn-2"),
ev_assistant_message("msg-session-turn-1", "done"),
ev_completed("resp-session-turn-2"),
]),
],
)
.await;
submit_turn(
&test,
"request session permissions for later use",
approval_policy,
sandbox_policy.clone(),
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Session,
},
})
.await?;
wait_for_completion(&test).await;
let second_turn = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-session-turn-3"),
exec_command_event("exec-call", &command)?,
ev_completed("resp-session-turn-3"),
]),
sse(vec![
ev_response_created("resp-session-turn-4"),
ev_assistant_message("msg-session-turn-2", "done"),
ev_completed("resp-session-turn-4"),
]),
],
)
.await;
submit_turn(
&test,
"reuse session permissions in a later turn",
approval_policy,
sandbox_policy,
)
.await?;
let completion_event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
if let EventMsg::ExecApprovalRequest(approval) = completion_event {
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_completion(&test).await;
}
let exec_output = second_turn
.function_call_output_text("exec-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected exec-call output"));
let result = parse_result(&exec_output);
assert_eq!(result.exit_code, Some(0));
assert_eq!(result.stdout.trim(), "session-sticky-ok");
assert_eq!(fs::read_to_string(&outside_write)?, "session-sticky-ok");
Ok(())
}
@@ -0,0 +1,420 @@
#![allow(clippy::unwrap_used, clippy::expect_used)]
#![cfg(target_os = "macos")]
use anyhow::Result;
use codex_core::config::Constrained;
use codex_core::features::Feature;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::responses::ev_apply_patch_function_call;
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_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::skip_if_sandbox;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use regex_lite::Regex;
use serde_json::Value;
use serde_json::json;
use std::fs;
use std::path::Path;
fn absolute_path(path: &Path) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(path).expect("absolute path")
}
fn request_permissions_tool_event(
call_id: &str,
reason: &str,
permissions: &PermissionProfile,
) -> Result<Value> {
let args = json!({
"reason": reason,
"permissions": permissions,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "request_permissions", &args_str))
}
fn exec_command_event(call_id: &str, command: &str) -> Result<Value> {
let args = json!({
"cmd": command,
"yield_time_ms": 1_000_u64,
});
let args_str = serde_json::to_string(&args)?;
Ok(ev_function_call(call_id, "exec_command", &args_str))
}
fn build_add_file_patch(patch_path: &Path, content: &str) -> String {
format!(
"*** Begin Patch\n*** Add File: {}\n+{}\n*** End Patch\n",
patch_path.display(),
content
)
}
fn workspace_write_excluding_tmp() -> SandboxPolicy {
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: Default::default(),
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
}
}
fn requested_directory_write_permissions(path: &Path) -> PermissionProfile {
PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![absolute_path(path)]),
}),
..Default::default()
}
}
fn normalized_directory_write_permissions(path: &Path) -> Result<PermissionProfile> {
Ok(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
}),
..Default::default()
})
}
fn parse_result(item: &Value) -> (Option<i64>, String) {
let output_str = item
.get("output")
.and_then(Value::as_str)
.expect("shell output payload");
match serde_json::from_str::<Value>(output_str) {
Ok(parsed) => {
let exit_code = parsed["metadata"]["exit_code"].as_i64();
let stdout = parsed["output"].as_str().unwrap_or_default().to_string();
(exit_code, stdout)
}
Err(_) => {
let structured = Regex::new(r"(?s)^Exit code:\s*(-?\d+).*?Output:\n(.*)$").unwrap();
let regex =
Regex::new(r"(?s)^.*?Process exited with code (\d+)\n.*?Output:\n(.*)$").unwrap();
if let Some(captures) = structured.captures(output_str) {
let exit_code = captures.get(1).unwrap().as_str().parse::<i64>().unwrap();
let output = captures.get(2).unwrap().as_str();
(Some(exit_code), output.to_string())
} else if let Some(captures) = regex.captures(output_str) {
let exit_code = captures.get(1).unwrap().as_str().parse::<i64>().unwrap();
let output = captures.get(2).unwrap().as_str();
(Some(exit_code), output.to_string())
} else {
(None, output_str.to_string())
}
}
}
}
async fn submit_turn(
test: &TestCodex,
prompt: &str,
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
) -> Result<()> {
let session_model = test.session_configured.model.clone();
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: prompt.into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd.path().to_path_buf(),
approval_policy,
sandbox_policy,
model: session_model,
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
})
.await?;
Ok(())
}
async fn expect_request_permissions_event(
test: &TestCodex,
expected_call_id: &str,
) -> PermissionProfile {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::RequestPermissions(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::RequestPermissions(request) => {
assert_eq!(request.call_id, expected_call_id);
request.permissions
}
EventMsg::TurnComplete(_) => panic!("expected request_permissions before completion"),
other => panic!("unexpected event: {other:?}"),
}
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn approved_folder_write_request_permissions_unblocks_later_exec_without_sandbox_args()
-> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let requested_dir = tempfile::tempdir()?;
let requested_file = requested_dir.path().join("allowed-write.txt");
let command = format!(
"printf {:?} > {:?} && cat {:?}",
"folder-grant-ok", requested_file, requested_file
);
let requested_permissions = requested_directory_write_permissions(requested_dir.path());
let normalized_requested_permissions =
normalized_directory_write_permissions(requested_dir.path())?;
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-request-permissions-1"),
request_permissions_tool_event(
"permissions-call",
"Allow writing outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-request-permissions-1"),
]),
sse(vec![
ev_response_created("resp-request-permissions-2"),
exec_command_event("exec-call", &command)?,
ev_completed("resp-request-permissions-2"),
]),
sse(vec![
ev_response_created("resp-request-permissions-3"),
ev_assistant_message("msg-request-permissions-1", "done"),
ev_completed("resp-request-permissions-3"),
]),
],
)
.await;
submit_turn(
&test,
"write outside the workspace",
approval_policy,
sandbox_policy,
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
})
.await?;
let completion_event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
if let EventMsg::ExecApprovalRequest(approval) = completion_event {
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
})
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
}
let exec_output = responses
.function_call_output_text("exec-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected exec-call output"));
let (exit_code, stdout) = parse_result(&exec_output);
assert!(exit_code.is_none() || exit_code == Some(0));
assert!(stdout.contains("folder-grant-ok"));
assert!(
requested_file.exists(),
"touch command should create the file"
);
assert_eq!(fs::read_to_string(&requested_file)?, "folder-grant-ok");
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(target_os = "macos")]
async fn approved_folder_write_request_permissions_unblocks_later_apply_patch_without_prompt()
-> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = workspace_write_excluding_tmp();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
config
.features
.enable(Feature::RequestPermissions)
.expect("test config should allow feature update");
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let requested_dir = tempfile::tempdir()?;
let requested_file = requested_dir.path().join("allowed-patch.txt");
let requested_permissions = requested_directory_write_permissions(requested_dir.path());
let normalized_requested_permissions =
normalized_directory_write_permissions(requested_dir.path())?;
let patch = build_add_file_patch(&requested_file, "patched-via-request-permissions");
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-request-permissions-patch-1"),
request_permissions_tool_event(
"permissions-call",
"Allow patching outside the workspace",
&requested_permissions,
)?,
ev_completed("resp-request-permissions-patch-1"),
]),
sse(vec![
ev_response_created("resp-request-permissions-patch-2"),
ev_apply_patch_function_call("apply-patch-call", &patch),
ev_completed("resp-request-permissions-patch-2"),
]),
sse(vec![
ev_response_created("resp-request-permissions-patch-3"),
ev_assistant_message("msg-request-permissions-patch-1", "done"),
ev_completed("resp-request-permissions-patch-3"),
]),
],
)
.await;
submit_turn(
&test,
"patch outside the workspace",
approval_policy,
sandbox_policy,
)
.await?;
let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await;
assert_eq!(
granted_permissions,
normalized_requested_permissions.clone()
);
test.codex
.submit(Op::RequestPermissionsResponse {
id: "permissions-call".to_string(),
response: RequestPermissionsResponse {
permissions: normalized_requested_permissions,
scope: PermissionGrantScope::Turn,
},
})
.await?;
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ApplyPatchApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::TurnComplete(_) => {}
EventMsg::ApplyPatchApprovalRequest(approval) => {
panic!(
"unexpected apply_patch approval request after granted permissions: {:?}",
approval.call_id
)
}
other => panic!("unexpected event: {other:?}"),
}
let patch_output = responses
.function_call_output_text("apply-patch-call")
.map(|output| json!({ "output": output }))
.unwrap_or_else(|| panic!("expected apply-patch-call output"));
let (exit_code, stdout) = parse_result(&patch_output);
assert!(exit_code.is_none() || exit_code == Some(0));
assert!(
stdout.contains("Success."),
"unexpected patch output: {stdout}"
);
assert_eq!(
fs::read_to_string(&requested_file)?,
"patched-via-request-permissions\n"
);
Ok(())
}
+81 -2
View File
@@ -13,10 +13,49 @@ use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::TestCodexBuilder;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use tempfile::TempDir;
use wiremock::MockServer;
async fn resume_until_initial_messages(
builder: &mut TestCodexBuilder,
server: &MockServer,
home: Arc<TempDir>,
rollout_path: PathBuf,
predicate: impl Fn(&[EventMsg]) -> bool,
) -> Result<TestCodex> {
let deadline = tokio::time::Instant::now() + Duration::from_secs(2);
let poll_interval = Duration::from_millis(10);
let mut last_initial_messages = "<missing initial messages>".to_string();
loop {
let resumed = builder
.resume(server, Arc::clone(&home), rollout_path.clone())
.await?;
if let Some(initial_messages) = resumed.session_configured.initial_messages.as_ref() {
if predicate(initial_messages) {
return Ok(resumed);
}
last_initial_messages = format!("{initial_messages:#?}");
}
if tokio::time::Instant::now() >= deadline {
panic!(
"timed out waiting for rollout resume messages to stabilize: {last_initial_messages}"
);
}
drop(resumed);
tokio::time::sleep(poll_interval).await;
}
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> {
@@ -57,7 +96,26 @@ async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> {
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
let resumed = builder.resume(&server, home, rollout_path).await?;
let resumed = resume_until_initial_messages(
&mut builder,
&server,
home,
rollout_path,
|initial_messages| {
matches!(
initial_messages,
[
EventMsg::TurnStarted(_),
EventMsg::UserMessage(_),
EventMsg::TokenCount(_),
EventMsg::AgentMessage(_),
EventMsg::TokenCount(_),
EventMsg::TurnComplete(_),
]
)
},
)
.await?;
let initial_messages = resumed
.session_configured
.initial_messages
@@ -123,7 +181,28 @@ async fn resume_includes_initial_messages_from_reasoning_events() -> Result<()>
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
let resumed = builder.resume(&server, home, rollout_path).await?;
let resumed = resume_until_initial_messages(
&mut builder,
&server,
home,
rollout_path,
|initial_messages| {
matches!(
initial_messages,
[
EventMsg::TurnStarted(_),
EventMsg::UserMessage(_),
EventMsg::TokenCount(_),
EventMsg::AgentReasoning(_),
EventMsg::AgentReasoningRawContent(_),
EventMsg::AgentMessage(_),
EventMsg::TokenCount(_),
EventMsg::TurnComplete(_),
]
)
},
)
.await?;
let initial_messages = resumed
.session_configured
.initial_messages
+51 -14
View File
@@ -4,6 +4,7 @@ use std::ffi::OsString;
use std::fs;
use std::net::TcpListener;
use std::path::Path;
use std::sync::Arc;
use std::time::Duration;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
@@ -36,11 +37,13 @@ use core_test_support::skip_if_no_network;
use core_test_support::stdio_server_bin;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use core_test_support::wait_for_event_with_timeout;
use reqwest::Client;
use reqwest::StatusCode;
use serde_json::Value;
use serde_json::json;
use serial_test::serial;
use tempfile::tempdir;
use tokio::net::TcpStream;
use tokio::process::Child;
use tokio::process::Command;
use tokio::time::Instant;
@@ -263,7 +266,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = core_test_support::wait_for_event_with_timeout(
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
@@ -853,8 +856,8 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
/// This test writes to a fallback credentials file in CODEX_HOME.
/// Ideally, we wouldn't need to serialize the test but it's much more cumbersome to wire CODEX_HOME through the code.
#[serial(codex_home)]
#[test]
#[serial(codex_home)]
fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> {
const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024;
@@ -936,8 +939,8 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
wait_for_streamable_http_server(&mut http_server_child, &bind_addr, Duration::from_secs(5))
.await?;
let temp_home = tempdir()?;
let _guard = EnvVarGuard::set("CODEX_HOME", temp_home.path().as_os_str());
let temp_home = Arc::new(tempdir()?);
let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", temp_home.path().as_os_str());
write_fallback_oauth_tokens(
temp_home.path(),
server_name,
@@ -948,10 +951,10 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
)?;
let fixture = test_codex()
.with_home(temp_home.clone())
.with_config(move |config| {
// This test seeds OAuth tokens in CODEX_HOME/.credentials.json and
// validates file-backed OAuth loading. Force file mode so Linux
// keyring backend quirks do not affect this test.
// Keep OAuth credentials isolated to this test home because Bazel
// runs the full core suite in one process.
config.mcp_oauth_credentials_store_mode = serde_json::from_value(json!("file"))
.expect("`file` should deserialize as OAuthCredentialsStoreMode");
let mut servers = config.mcp_servers.get().clone();
@@ -984,6 +987,31 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
.await?;
let session_model = fixture.session_configured.model.clone();
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
if tool_list.tools.contains_key(&tool_name) {
break;
}
let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
if Instant::now() >= tools_ready_deadline {
panic!(
"timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}"
);
}
sleep(Duration::from_millis(200)).await;
}
fixture
.codex
.submit(Op::UserTurn {
@@ -1078,7 +1106,8 @@ async fn wait_for_streamable_http_server(
timeout: Duration,
) -> anyhow::Result<()> {
let deadline = Instant::now() + timeout;
let metadata_url = format!("http://{address}/.well-known/oauth-authorization-server/mcp");
let client = Client::builder().no_proxy().build()?;
loop {
if let Some(status) = server_child.try_wait()? {
return Err(anyhow::anyhow!(
@@ -1090,22 +1119,30 @@ async fn wait_for_streamable_http_server(
if remaining.is_zero() {
return Err(anyhow::anyhow!(
"timed out waiting for streamable HTTP server at {address}: deadline reached"
"timed out waiting for streamable HTTP server metadata at {metadata_url}: deadline reached"
));
}
match tokio::time::timeout(remaining, TcpStream::connect(address)).await {
Ok(Ok(_)) => return Ok(()),
match tokio::time::timeout(remaining, client.get(&metadata_url).send()).await {
Ok(Ok(response)) if response.status() == StatusCode::OK => return Ok(()),
Ok(Ok(response)) => {
if Instant::now() >= deadline {
return Err(anyhow::anyhow!(
"timed out waiting for streamable HTTP server metadata at {metadata_url}: HTTP {}",
response.status()
));
}
}
Ok(Err(error)) => {
if Instant::now() >= deadline {
return Err(anyhow::anyhow!(
"timed out waiting for streamable HTTP server at {address}: {error}"
"timed out waiting for streamable HTTP server metadata at {metadata_url}: {error}"
));
}
}
Err(_) => {
return Err(anyhow::anyhow!(
"timed out waiting for streamable HTTP server at {address}: connect call timed out"
"timed out waiting for streamable HTTP server metadata at {metadata_url}: request timed out"
));
}
}
+9 -8
View File
@@ -262,15 +262,16 @@ async fn unicode_output(login: bool) -> anyhow::Result<()> {
})
.await?;
// We use a child process on windows instead of a direct builtin like 'echo' to ensure that Powershell
// config is actually being set correctly.
let call_id = "unicode_output";
mount_shell_responses_with_timeout(
&harness,
call_id,
"git -c alias.say='!printf \"%s\" \"naïve_café\"' say",
Some(login),
MEDIUM_TIMEOUT,
)
.await;
let command = if cfg!(windows) {
"cmd /c echo naïve_café"
} else {
"echo \"naïve_café\""
};
mount_shell_responses_with_timeout(&harness, call_id, command, Some(login), MEDIUM_TIMEOUT)
.await;
harness.submit("run the command without login").await?;
let output = harness.function_call_stdout(call_id).await;
@@ -349,7 +349,7 @@ async fn shell_output_for_freeform_tool_records_duration(
let test = builder.build(&server).await?;
let call_id = "shell-structured";
let responses = shell_responses(call_id, vec!["/bin/sh", "-c", "sleep 1"], output_type)?;
let responses = shell_responses(call_id, vec!["/bin/sh", "-c", "sleep 0.2"], output_type)?;
let mock = mount_sse_sequence(&server, responses).await;
test.submit_turn_with_policy(
@@ -381,7 +381,7 @@ $"#;
.and_then(|value| value.as_str().parse::<f32>().ok())
.expect("expected structured shell output to contain wall time seconds");
assert!(
wall_time_seconds > 0.5,
wall_time_seconds > 0.1,
"expected wall time to be greater than zero seconds, got {wall_time_seconds}"
);
@@ -740,6 +740,7 @@ async fn shell_command_output_is_freeform() -> Result<()> {
let call_id = "shell-command";
let args = json!({
"command": "echo shell command",
"login": false,
"timeout_ms": 1_000,
});
let responses = vec![
@@ -791,6 +792,7 @@ async fn shell_command_output_is_not_truncated_under_10k_bytes() -> Result<()> {
let call_id = "shell-command";
let args = json!({
"command": "perl -e 'print \"1\" x 10000'",
"login": false,
"timeout_ms": 1000,
});
let responses = vec![
@@ -841,6 +843,7 @@ async fn shell_command_output_is_not_truncated_over_10k_bytes() -> Result<()> {
let call_id = "shell-command";
let args = json!({
"command": "perl -e 'print \"1\" x 10001'",
"login": false,
"timeout_ms": 1000,
});
let responses = vec![
@@ -7,6 +7,7 @@ use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
use codex_protocol::protocol::ExecApprovalRequestSkillMetadata;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RejectConfig;
use codex_protocol::protocol::ReviewDecision;
@@ -242,6 +243,14 @@ permissions:
..Default::default()
})
);
assert_eq!(
approval.skill_metadata,
Some(ExecApprovalRequestSkillMetadata {
path_to_skills_md: test
.codex_home_path()
.join("skills/mbolin-test-skill/agents/openai.yaml"),
})
);
test.codex
.submit(Op::ExecApproval {
@@ -279,6 +288,7 @@ async fn shell_zsh_fork_skill_script_reject_policy_with_sandbox_approval_false_s
let approval_policy = AskForApproval::Reject(RejectConfig {
sandbox_approval: false,
rules: true,
request_permissions: false,
mcp_elicitations: false,
});
let server = start_mock_server().await;
@@ -371,6 +381,7 @@ async fn shell_zsh_fork_skill_script_reject_policy_with_sandbox_approval_true_sk
let approval_policy = AskForApproval::Reject(RejectConfig {
sandbox_approval: true,
rules: false,
request_permissions: false,
mcp_elicitations: false,
});
let server = start_mock_server().await;