mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Merge fcad39156d into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -298,17 +298,6 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"CommandExecutionRequestApprovalSkillMetadata": {
|
||||
"properties": {
|
||||
"pathToSkillsMd": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"pathToSkillsMd"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"MacOsAutomationPermission": {
|
||||
"oneOf": [
|
||||
{
|
||||
|
||||
@@ -452,17 +452,6 @@
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"CommandExecutionRequestApprovalSkillMetadata": {
|
||||
"properties": {
|
||||
"pathToSkillsMd": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"pathToSkillsMd"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"DynamicToolCallParams": {
|
||||
"properties": {
|
||||
"arguments": true,
|
||||
|
||||
@@ -1864,17 +1864,6 @@
|
||||
"title": "CommandExecutionRequestApprovalResponse",
|
||||
"type": "object"
|
||||
},
|
||||
"CommandExecutionRequestApprovalSkillMetadata": {
|
||||
"properties": {
|
||||
"pathToSkillsMd": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"pathToSkillsMd"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"DynamicToolCallParams": {
|
||||
"$schema": "http://json-schema.org/draft-07/schema#",
|
||||
"properties": {
|
||||
|
||||
-5
@@ -4,7 +4,6 @@
|
||||
import type { AdditionalPermissionProfile } from "./AdditionalPermissionProfile";
|
||||
import type { CommandAction } from "./CommandAction";
|
||||
import type { CommandExecutionApprovalDecision } from "./CommandExecutionApprovalDecision";
|
||||
import type { CommandExecutionRequestApprovalSkillMetadata } from "./CommandExecutionRequestApprovalSkillMetadata";
|
||||
import type { ExecPolicyAmendment } from "./ExecPolicyAmendment";
|
||||
import type { NetworkApprovalContext } from "./NetworkApprovalContext";
|
||||
import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
|
||||
@@ -44,10 +43,6 @@ commandActions?: Array<CommandAction> | null,
|
||||
* Optional additional permissions requested for this command.
|
||||
*/
|
||||
additionalPermissions?: AdditionalPermissionProfile | null,
|
||||
/**
|
||||
* Optional skill metadata when the approval was triggered by a skill script.
|
||||
*/
|
||||
skillMetadata?: CommandExecutionRequestApprovalSkillMetadata | null,
|
||||
/**
|
||||
* Optional proposed execpolicy amendment to allow similar commands without prompting.
|
||||
*/
|
||||
|
||||
-5
@@ -1,5 +0,0 @@
|
||||
// GENERATED CODE! DO NOT MODIFY BY HAND!
|
||||
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
|
||||
export type CommandExecutionRequestApprovalSkillMetadata = { pathToSkillsMd: string, };
|
||||
@@ -54,7 +54,6 @@ export type { CommandExecutionApprovalDecision } from "./CommandExecutionApprova
|
||||
export type { CommandExecutionOutputDeltaNotification } from "./CommandExecutionOutputDeltaNotification";
|
||||
export type { CommandExecutionRequestApprovalParams } from "./CommandExecutionRequestApprovalParams";
|
||||
export type { CommandExecutionRequestApprovalResponse } from "./CommandExecutionRequestApprovalResponse";
|
||||
export type { CommandExecutionRequestApprovalSkillMetadata } from "./CommandExecutionRequestApprovalSkillMetadata";
|
||||
export type { CommandExecutionSource } from "./CommandExecutionSource";
|
||||
export type { CommandExecutionStatus } from "./CommandExecutionStatus";
|
||||
export type { Config } from "./Config";
|
||||
|
||||
@@ -2298,10 +2298,6 @@ mod tests {
|
||||
command_execution_request_approval_ts.contains("additionalPermissions"),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
command_execution_request_approval_ts.contains("skillMetadata"),
|
||||
true
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -2705,10 +2701,6 @@ export type Config = { stableField: Keep, unstableField: string | null } & ({ [k
|
||||
command_execution_request_approval_json.contains("additionalPermissions"),
|
||||
false
|
||||
);
|
||||
assert_eq!(
|
||||
command_execution_request_approval_json.contains("skillMetadata"),
|
||||
false
|
||||
);
|
||||
|
||||
let client_request_json = fs::read_to_string(output_dir.join("ClientRequest.json"))?;
|
||||
assert_eq!(
|
||||
@@ -2721,7 +2713,6 @@ export type Config = { stableField: Keep, unstableField: string | null } & ({ [k
|
||||
fs::read_to_string(output_dir.join("codex_app_server_protocol.schemas.json"))?;
|
||||
assert_eq!(bundle_json.contains("mockExperimentalField"), false);
|
||||
assert_eq!(bundle_json.contains("additionalPermissions"), false);
|
||||
assert_eq!(bundle_json.contains("skillMetadata"), false);
|
||||
assert_eq!(bundle_json.contains("MockExperimentalMethodParams"), false);
|
||||
assert_eq!(
|
||||
bundle_json.contains("MockExperimentalMethodResponse"),
|
||||
@@ -2731,7 +2722,6 @@ export type Config = { stableField: Keep, unstableField: string | null } & ({ [k
|
||||
fs::read_to_string(output_dir.join("codex_app_server_protocol.v2.schemas.json"))?;
|
||||
assert_eq!(flat_v2_bundle_json.contains("mockExperimentalField"), false);
|
||||
assert_eq!(flat_v2_bundle_json.contains("additionalPermissions"), false);
|
||||
assert_eq!(flat_v2_bundle_json.contains("skillMetadata"), false);
|
||||
assert_eq!(
|
||||
flat_v2_bundle_json.contains("MockExperimentalMethodParams"),
|
||||
false
|
||||
|
||||
@@ -1705,7 +1705,6 @@ mod tests {
|
||||
}),
|
||||
macos: None,
|
||||
}),
|
||||
skill_metadata: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
@@ -1716,31 +1715,4 @@ mod tests {
|
||||
Some("item/commandExecution/requestApproval.additionalPermissions")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_execution_request_approval_skill_metadata_is_marked_experimental() {
|
||||
let params = v2::CommandExecutionRequestApprovalParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
approval_id: None,
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
command: Some("cat file".to_string()),
|
||||
cwd: None,
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: Some(v2::CommandExecutionRequestApprovalSkillMetadata {
|
||||
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
|
||||
}),
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
};
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms);
|
||||
assert_eq!(
|
||||
reason,
|
||||
Some("item/commandExecution/requestApproval.skillMetadata")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,7 +7,6 @@ use crate::protocol::common::AuthMode;
|
||||
use codex_experimental_api_macros::ExperimentalApi;
|
||||
use codex_protocol::account::PlanType;
|
||||
use codex_protocol::approvals::ElicitationRequest as CoreElicitationRequest;
|
||||
use codex_protocol::approvals::ExecApprovalRequestSkillMetadata as CoreExecApprovalRequestSkillMetadata;
|
||||
use codex_protocol::approvals::ExecPolicyAmendment as CoreExecPolicyAmendment;
|
||||
use codex_protocol::approvals::NetworkApprovalContext as CoreNetworkApprovalContext;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol as CoreNetworkApprovalProtocol;
|
||||
@@ -3519,14 +3518,6 @@ impl From<CoreSkillMetadata> for SkillMetadata {
|
||||
}
|
||||
}
|
||||
|
||||
impl From<CoreExecApprovalRequestSkillMetadata> for CommandExecutionRequestApprovalSkillMetadata {
|
||||
fn from(value: CoreExecApprovalRequestSkillMetadata) -> Self {
|
||||
Self {
|
||||
path_to_skills_md: value.path_to_skills_md,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<CoreSkillInterface> for SkillInterface {
|
||||
fn from(value: CoreSkillInterface) -> Self {
|
||||
Self {
|
||||
@@ -5240,11 +5231,6 @@ pub struct CommandExecutionRequestApprovalParams {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
/// Optional skill metadata when the approval was triggered by a skill script.
|
||||
#[experimental("item/commandExecution/requestApproval.skillMetadata")]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub skill_metadata: Option<CommandExecutionRequestApprovalSkillMetadata>,
|
||||
/// Optional proposed execpolicy amendment to allow similar commands without prompting.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
@@ -5266,17 +5252,9 @@ impl CommandExecutionRequestApprovalParams {
|
||||
// We need a generic outbound compatibility design for stripping or
|
||||
// otherwise handling experimental server->client payloads.
|
||||
self.additional_permissions = None;
|
||||
self.skill_metadata = None;
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecutionRequestApprovalSkillMetadata {
|
||||
pub path_to_skills_md: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -6100,7 +6078,6 @@ mod tests {
|
||||
},
|
||||
"macos": null
|
||||
},
|
||||
"skillMetadata": null,
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
@@ -6139,7 +6116,6 @@ mod tests {
|
||||
"contacts": "read_only"
|
||||
}
|
||||
},
|
||||
"skillMetadata": null,
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
@@ -6183,7 +6159,6 @@ mod tests {
|
||||
"contacts": "none"
|
||||
}
|
||||
},
|
||||
"skillMetadata": null,
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
@@ -6199,35 +6174,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_execution_request_approval_accepts_skill_metadata() {
|
||||
let params = serde_json::from_value::<CommandExecutionRequestApprovalParams>(json!({
|
||||
"threadId": "thr_123",
|
||||
"turnId": "turn_123",
|
||||
"itemId": "call_123",
|
||||
"command": "cat file",
|
||||
"cwd": "/tmp",
|
||||
"commandActions": null,
|
||||
"reason": null,
|
||||
"networkApprovalContext": null,
|
||||
"additionalPermissions": null,
|
||||
"skillMetadata": {
|
||||
"pathToSkillsMd": "/tmp/SKILLS.md"
|
||||
},
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
}))
|
||||
.expect("skill metadata should deserialize");
|
||||
|
||||
assert_eq!(
|
||||
params.skill_metadata,
|
||||
Some(CommandExecutionRequestApprovalSkillMetadata {
|
||||
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_request_approval_uses_request_permission_profile() {
|
||||
let read_only_path = if cfg!(windows) {
|
||||
|
||||
@@ -1917,7 +1917,6 @@ impl CodexClient {
|
||||
cwd,
|
||||
command_actions,
|
||||
additional_permissions,
|
||||
skill_metadata,
|
||||
proposed_execpolicy_amendment,
|
||||
proposed_network_policy_amendments,
|
||||
available_decisions,
|
||||
@@ -1952,9 +1951,6 @@ impl CodexClient {
|
||||
if let Some(additional_permissions) = additional_permissions.as_ref() {
|
||||
println!("< additional permissions: {additional_permissions:?}");
|
||||
}
|
||||
if let Some(skill_metadata) = skill_metadata.as_ref() {
|
||||
println!("< skill metadata: {skill_metadata:?}");
|
||||
}
|
||||
if let Some(execpolicy_amendment) = proposed_execpolicy_amendment.as_ref() {
|
||||
println!("< proposed execpolicy amendment: {execpolicy_amendment:?}");
|
||||
}
|
||||
|
||||
@@ -26,7 +26,6 @@ use codex_app_server_protocol::CommandExecutionApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionOutputDeltaNotification;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalParams;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalSkillMetadata;
|
||||
use codex_app_server_protocol::CommandExecutionSource;
|
||||
use codex_app_server_protocol::CommandExecutionStatus;
|
||||
use codex_app_server_protocol::ContextCompactedNotification;
|
||||
@@ -608,7 +607,6 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
proposed_execpolicy_amendment,
|
||||
proposed_network_policy_amendments,
|
||||
additional_permissions,
|
||||
skill_metadata,
|
||||
parsed_cmd,
|
||||
..
|
||||
} = ev;
|
||||
@@ -680,8 +678,6 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
});
|
||||
let additional_permissions =
|
||||
additional_permissions.map(V2AdditionalPermissionProfile::from);
|
||||
let skill_metadata =
|
||||
skill_metadata.map(CommandExecutionRequestApprovalSkillMetadata::from);
|
||||
|
||||
let params = CommandExecutionRequestApprovalParams {
|
||||
thread_id: conversation_id.to_string(),
|
||||
@@ -694,7 +690,6 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
cwd,
|
||||
command_actions,
|
||||
additional_permissions,
|
||||
skill_metadata,
|
||||
proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2,
|
||||
proposed_network_policy_amendments: proposed_network_policy_amendments_v2,
|
||||
available_decisions: Some(available_decisions),
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
pub(crate) mod auth;
|
||||
|
||||
use self::auth::WebsocketAuthPolicy;
|
||||
use self::auth::authorize_upgrade;
|
||||
use self::auth::should_warn_about_unauthenticated_non_loopback_listener;
|
||||
use crate::error_code::OVERLOADED_ERROR_CODE;
|
||||
use crate::message_processor::ConnectionSessionState;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
@@ -10,53 +7,20 @@ use crate::outgoing_message::OutgoingEnvelope;
|
||||
use crate::outgoing_message::OutgoingError;
|
||||
use crate::outgoing_message::OutgoingMessage;
|
||||
use crate::outgoing_message::QueuedOutgoingMessage;
|
||||
use axum::Router;
|
||||
use axum::body::Body;
|
||||
use axum::extract::ConnectInfo;
|
||||
use axum::extract::State;
|
||||
use axum::extract::ws::Message as WebSocketMessage;
|
||||
use axum::extract::ws::WebSocket;
|
||||
use axum::extract::ws::WebSocketUpgrade;
|
||||
use axum::http::HeaderMap;
|
||||
use axum::http::Request;
|
||||
use axum::http::StatusCode;
|
||||
use axum::http::header::ORIGIN;
|
||||
use axum::middleware;
|
||||
use axum::middleware::Next;
|
||||
use axum::response::IntoResponse;
|
||||
use axum::response::Response;
|
||||
use axum::routing::any;
|
||||
use axum::routing::get;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use futures::SinkExt;
|
||||
use futures::StreamExt;
|
||||
use owo_colors::OwoColorize;
|
||||
use owo_colors::Stream;
|
||||
use owo_colors::Style;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::io::ErrorKind;
|
||||
use std::io::Result as IoResult;
|
||||
use std::net::SocketAddr;
|
||||
use std::str::FromStr;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicU64;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
use tokio::io::BufReader;
|
||||
use tokio::io::{self};
|
||||
use tokio::net::TcpListener;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::task::JoinHandle;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::debug;
|
||||
use tracing::error;
|
||||
use tracing::info;
|
||||
use tracing::warn;
|
||||
|
||||
/// Size of the bounded channels used to communicate between tasks. The value
|
||||
@@ -64,85 +28,11 @@ use tracing::warn;
|
||||
/// plenty for an interactive CLI.
|
||||
pub(crate) const CHANNEL_CAPACITY: usize = 128;
|
||||
|
||||
fn colorize(text: &str, style: Style) -> String {
|
||||
text.if_supports_color(Stream::Stderr, |value| value.style(style))
|
||||
.to_string()
|
||||
}
|
||||
mod stdio;
|
||||
mod websocket;
|
||||
|
||||
#[allow(clippy::print_stderr)]
|
||||
fn print_websocket_startup_banner(addr: SocketAddr) {
|
||||
let title = colorize("codex app-server (WebSockets)", Style::new().bold().cyan());
|
||||
let listening_label = colorize("listening on:", Style::new().dimmed());
|
||||
let listen_url = colorize(&format!("ws://{addr}"), Style::new().green());
|
||||
let ready_label = colorize("readyz:", Style::new().dimmed());
|
||||
let ready_url = colorize(&format!("http://{addr}/readyz"), Style::new().green());
|
||||
let health_label = colorize("healthz:", Style::new().dimmed());
|
||||
let health_url = colorize(&format!("http://{addr}/healthz"), Style::new().green());
|
||||
let note_label = colorize("note:", Style::new().dimmed());
|
||||
eprintln!("{title}");
|
||||
eprintln!(" {listening_label} {listen_url}");
|
||||
eprintln!(" {ready_label} {ready_url}");
|
||||
eprintln!(" {health_label} {health_url}");
|
||||
if addr.ip().is_loopback() {
|
||||
eprintln!(
|
||||
" {note_label} binds localhost only (use SSH port-forwarding for remote access)"
|
||||
);
|
||||
} else {
|
||||
eprintln!(
|
||||
" {note_label} websocket auth is opt-in in this build; configure `--ws-auth ...` before real remote use"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct WebSocketListenerState {
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
connection_counter: Arc<AtomicU64>,
|
||||
auth_policy: Arc<WebsocketAuthPolicy>,
|
||||
}
|
||||
|
||||
async fn health_check_handler() -> StatusCode {
|
||||
StatusCode::OK
|
||||
}
|
||||
|
||||
async fn reject_requests_with_origin_header(
|
||||
request: Request<Body>,
|
||||
next: Next,
|
||||
) -> Result<Response, StatusCode> {
|
||||
if request.headers().contains_key(ORIGIN) {
|
||||
warn!(
|
||||
method = %request.method(),
|
||||
uri = %request.uri(),
|
||||
"rejecting websocket listener request with Origin header"
|
||||
);
|
||||
Err(StatusCode::FORBIDDEN)
|
||||
} else {
|
||||
Ok(next.run(request).await)
|
||||
}
|
||||
}
|
||||
|
||||
async fn websocket_upgrade_handler(
|
||||
websocket: WebSocketUpgrade,
|
||||
ConnectInfo(peer_addr): ConnectInfo<SocketAddr>,
|
||||
State(state): State<WebSocketListenerState>,
|
||||
headers: HeaderMap,
|
||||
) -> impl IntoResponse {
|
||||
if let Err(err) = authorize_upgrade(&headers, state.auth_policy.as_ref()) {
|
||||
warn!(
|
||||
%peer_addr,
|
||||
message = err.message(),
|
||||
"rejecting websocket client during upgrade"
|
||||
);
|
||||
return (err.status_code(), err.message()).into_response();
|
||||
}
|
||||
let connection_id = ConnectionId(state.connection_counter.fetch_add(1, Ordering::Relaxed));
|
||||
info!(%peer_addr, "websocket client connected");
|
||||
websocket
|
||||
.on_upgrade(move |stream| async move {
|
||||
run_websocket_connection(connection_id, stream, state.transport_event_tx).await;
|
||||
})
|
||||
.into_response()
|
||||
}
|
||||
pub(crate) use stdio::start_stdio_connection;
|
||||
pub(crate) use websocket::start_websocket_acceptor;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub enum AppServerTransport {
|
||||
@@ -276,262 +166,6 @@ impl OutboundConnectionState {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn start_stdio_connection(
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
stdio_handles: &mut Vec<JoinHandle<()>>,
|
||||
) -> IoResult<()> {
|
||||
let connection_id = ConnectionId(0);
|
||||
let (writer_tx, mut writer_rx) = mpsc::channel::<QueuedOutgoingMessage>(CHANNEL_CAPACITY);
|
||||
let writer_tx_for_reader = writer_tx.clone();
|
||||
transport_event_tx
|
||||
.send(TransportEvent::ConnectionOpened {
|
||||
connection_id,
|
||||
writer: writer_tx,
|
||||
disconnect_sender: None,
|
||||
})
|
||||
.await
|
||||
.map_err(|_| std::io::Error::new(ErrorKind::BrokenPipe, "processor unavailable"))?;
|
||||
|
||||
let transport_event_tx_for_reader = transport_event_tx.clone();
|
||||
stdio_handles.push(tokio::spawn(async move {
|
||||
let stdin = io::stdin();
|
||||
let reader = BufReader::new(stdin);
|
||||
let mut lines = reader.lines();
|
||||
|
||||
loop {
|
||||
match lines.next_line().await {
|
||||
Ok(Some(line)) => {
|
||||
if !forward_incoming_message(
|
||||
&transport_event_tx_for_reader,
|
||||
&writer_tx_for_reader,
|
||||
connection_id,
|
||||
&line,
|
||||
)
|
||||
.await
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
Ok(None) => break,
|
||||
Err(err) => {
|
||||
error!("Failed reading stdin: {err}");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let _ = transport_event_tx_for_reader
|
||||
.send(TransportEvent::ConnectionClosed { connection_id })
|
||||
.await;
|
||||
debug!("stdin reader finished (EOF)");
|
||||
}));
|
||||
|
||||
stdio_handles.push(tokio::spawn(async move {
|
||||
let mut stdout = io::stdout();
|
||||
while let Some(queued_message) = writer_rx.recv().await {
|
||||
let Some(mut json) = serialize_outgoing_message(queued_message.message) else {
|
||||
continue;
|
||||
};
|
||||
json.push('\n');
|
||||
if let Err(err) = stdout.write_all(json.as_bytes()).await {
|
||||
error!("Failed to write to stdout: {err}");
|
||||
break;
|
||||
}
|
||||
if let Some(write_complete_tx) = queued_message.write_complete_tx {
|
||||
let _ = write_complete_tx.send(());
|
||||
}
|
||||
}
|
||||
info!("stdout writer exited (channel closed)");
|
||||
}));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn start_websocket_acceptor(
|
||||
bind_address: SocketAddr,
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
shutdown_token: CancellationToken,
|
||||
auth_policy: WebsocketAuthPolicy,
|
||||
) -> IoResult<JoinHandle<()>> {
|
||||
if should_warn_about_unauthenticated_non_loopback_listener(bind_address, &auth_policy) {
|
||||
warn!(
|
||||
%bind_address,
|
||||
"starting non-loopback websocket listener without auth; websocket auth is opt-in for now and will become the default in a future release"
|
||||
);
|
||||
}
|
||||
let listener = TcpListener::bind(bind_address).await?;
|
||||
let local_addr = listener.local_addr()?;
|
||||
print_websocket_startup_banner(local_addr);
|
||||
info!("app-server websocket listening on ws://{local_addr}");
|
||||
|
||||
let router = Router::new()
|
||||
.route("/readyz", get(health_check_handler))
|
||||
.route("/healthz", get(health_check_handler))
|
||||
.fallback(any(websocket_upgrade_handler))
|
||||
.layer(middleware::from_fn(reject_requests_with_origin_header))
|
||||
.with_state(WebSocketListenerState {
|
||||
transport_event_tx,
|
||||
connection_counter: Arc::new(AtomicU64::new(1)),
|
||||
auth_policy: Arc::new(auth_policy),
|
||||
});
|
||||
let server = axum::serve(
|
||||
listener,
|
||||
router.into_make_service_with_connect_info::<SocketAddr>(),
|
||||
)
|
||||
.with_graceful_shutdown(async move {
|
||||
shutdown_token.cancelled().await;
|
||||
});
|
||||
Ok(tokio::spawn(async move {
|
||||
if let Err(err) = server.await {
|
||||
error!("websocket acceptor failed: {err}");
|
||||
}
|
||||
info!("websocket acceptor shutting down");
|
||||
}))
|
||||
}
|
||||
|
||||
async fn run_websocket_connection(
|
||||
connection_id: ConnectionId,
|
||||
websocket_stream: WebSocket,
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
) {
|
||||
let (writer_tx, writer_rx) = mpsc::channel::<QueuedOutgoingMessage>(CHANNEL_CAPACITY);
|
||||
let writer_tx_for_reader = writer_tx.clone();
|
||||
let disconnect_token = CancellationToken::new();
|
||||
if transport_event_tx
|
||||
.send(TransportEvent::ConnectionOpened {
|
||||
connection_id,
|
||||
writer: writer_tx,
|
||||
disconnect_sender: Some(disconnect_token.clone()),
|
||||
})
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let (websocket_writer, websocket_reader) = websocket_stream.split();
|
||||
let (writer_control_tx, writer_control_rx) =
|
||||
mpsc::channel::<WebSocketMessage>(CHANNEL_CAPACITY);
|
||||
let mut outbound_task = tokio::spawn(run_websocket_outbound_loop(
|
||||
websocket_writer,
|
||||
writer_rx,
|
||||
writer_control_rx,
|
||||
disconnect_token.clone(),
|
||||
));
|
||||
let mut inbound_task = tokio::spawn(run_websocket_inbound_loop(
|
||||
websocket_reader,
|
||||
transport_event_tx.clone(),
|
||||
writer_tx_for_reader,
|
||||
writer_control_tx,
|
||||
connection_id,
|
||||
disconnect_token.clone(),
|
||||
));
|
||||
|
||||
tokio::select! {
|
||||
_ = &mut outbound_task => {
|
||||
disconnect_token.cancel();
|
||||
inbound_task.abort();
|
||||
}
|
||||
_ = &mut inbound_task => {
|
||||
disconnect_token.cancel();
|
||||
outbound_task.abort();
|
||||
}
|
||||
}
|
||||
|
||||
let _ = transport_event_tx
|
||||
.send(TransportEvent::ConnectionClosed { connection_id })
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn run_websocket_outbound_loop(
|
||||
mut websocket_writer: futures::stream::SplitSink<WebSocket, WebSocketMessage>,
|
||||
mut writer_rx: mpsc::Receiver<QueuedOutgoingMessage>,
|
||||
mut writer_control_rx: mpsc::Receiver<WebSocketMessage>,
|
||||
disconnect_token: CancellationToken,
|
||||
) {
|
||||
loop {
|
||||
tokio::select! {
|
||||
_ = disconnect_token.cancelled() => {
|
||||
break;
|
||||
}
|
||||
message = writer_control_rx.recv() => {
|
||||
let Some(message) = message else {
|
||||
break;
|
||||
};
|
||||
if websocket_writer.send(message).await.is_err() {
|
||||
break;
|
||||
}
|
||||
}
|
||||
queued_message = writer_rx.recv() => {
|
||||
let Some(queued_message) = queued_message else {
|
||||
break;
|
||||
};
|
||||
let Some(json) = serialize_outgoing_message(queued_message.message) else {
|
||||
continue;
|
||||
};
|
||||
if websocket_writer.send(WebSocketMessage::Text(json.into())).await.is_err() {
|
||||
break;
|
||||
}
|
||||
if let Some(write_complete_tx) = queued_message.write_complete_tx {
|
||||
let _ = write_complete_tx.send(());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_websocket_inbound_loop(
|
||||
mut websocket_reader: futures::stream::SplitStream<WebSocket>,
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
writer_tx_for_reader: mpsc::Sender<QueuedOutgoingMessage>,
|
||||
writer_control_tx: mpsc::Sender<WebSocketMessage>,
|
||||
connection_id: ConnectionId,
|
||||
disconnect_token: CancellationToken,
|
||||
) {
|
||||
loop {
|
||||
tokio::select! {
|
||||
_ = disconnect_token.cancelled() => {
|
||||
break;
|
||||
}
|
||||
incoming_message = websocket_reader.next() => {
|
||||
match incoming_message {
|
||||
Some(Ok(WebSocketMessage::Text(text))) => {
|
||||
if !forward_incoming_message(
|
||||
&transport_event_tx,
|
||||
&writer_tx_for_reader,
|
||||
connection_id,
|
||||
text.as_ref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
Some(Ok(WebSocketMessage::Ping(payload))) => {
|
||||
match writer_control_tx.try_send(WebSocketMessage::Pong(payload)) {
|
||||
Ok(()) => {}
|
||||
Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => break,
|
||||
Err(tokio::sync::mpsc::error::TrySendError::Full(_)) => {
|
||||
warn!("websocket control queue full while replying to ping; closing connection");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
Some(Ok(WebSocketMessage::Pong(_))) => {}
|
||||
Some(Ok(WebSocketMessage::Close(_))) | None => break,
|
||||
Some(Ok(WebSocketMessage::Binary(_))) => {
|
||||
warn!("dropping unsupported binary websocket message");
|
||||
}
|
||||
Some(Err(err)) => {
|
||||
warn!("websocket receive error: {err}");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn forward_incoming_message(
|
||||
transport_event_tx: &mpsc::Sender<TransportEvent>,
|
||||
writer: &mpsc::Sender<QueuedOutgoingMessage>,
|
||||
@@ -745,7 +379,6 @@ pub(crate) async fn route_outgoing_envelope(
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::error_code::OVERLOADED_ERROR_CODE;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalSkillMetadata;
|
||||
use codex_app_server_protocol::ConfigWarningNotification;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -1127,7 +760,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_execution_request_approval_strips_experimental_fields_without_capability() {
|
||||
async fn command_execution_request_approval_strips_additional_permissions_without_capability() {
|
||||
let connection_id = ConnectionId(8);
|
||||
let (writer_tx, mut writer_rx) = mpsc::channel(1);
|
||||
|
||||
@@ -1171,9 +804,6 @@ mod tests {
|
||||
macos: None,
|
||||
},
|
||||
),
|
||||
skill_metadata: Some(CommandExecutionRequestApprovalSkillMetadata {
|
||||
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
|
||||
}),
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
@@ -1190,11 +820,10 @@ mod tests {
|
||||
.expect("request should be delivered to the connection");
|
||||
let json = serde_json::to_value(message.message).expect("request should serialize");
|
||||
assert_eq!(json["params"].get("additionalPermissions"), None);
|
||||
assert_eq!(json["params"].get("skillMetadata"), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_execution_request_approval_keeps_experimental_fields_with_capability() {
|
||||
async fn command_execution_request_approval_keeps_additional_permissions_with_capability() {
|
||||
let connection_id = ConnectionId(9);
|
||||
let (writer_tx, mut writer_rx) = mpsc::channel(1);
|
||||
|
||||
@@ -1238,9 +867,6 @@ mod tests {
|
||||
macos: None,
|
||||
},
|
||||
),
|
||||
skill_metadata: Some(CommandExecutionRequestApprovalSkillMetadata {
|
||||
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
|
||||
}),
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
@@ -1263,17 +889,11 @@ mod tests {
|
||||
"network": null,
|
||||
"fileSystem": {
|
||||
"read": [allowed_path],
|
||||
"write": null,
|
||||
"write": null,
|
||||
},
|
||||
"macos": null,
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
json["params"]["skillMetadata"],
|
||||
json!({
|
||||
"pathToSkillsMd": "/tmp/SKILLS.md",
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -0,0 +1,88 @@
|
||||
use super::CHANNEL_CAPACITY;
|
||||
use super::TransportEvent;
|
||||
use super::forward_incoming_message;
|
||||
use super::serialize_outgoing_message;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::QueuedOutgoingMessage;
|
||||
use std::io::ErrorKind;
|
||||
use std::io::Result as IoResult;
|
||||
use tokio::io;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
use tokio::io::BufReader;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::task::JoinHandle;
|
||||
use tracing::debug;
|
||||
use tracing::error;
|
||||
use tracing::info;
|
||||
|
||||
pub(crate) async fn start_stdio_connection(
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
stdio_handles: &mut Vec<JoinHandle<()>>,
|
||||
) -> IoResult<()> {
|
||||
let connection_id = ConnectionId(0);
|
||||
let (writer_tx, mut writer_rx) = mpsc::channel::<QueuedOutgoingMessage>(CHANNEL_CAPACITY);
|
||||
let writer_tx_for_reader = writer_tx.clone();
|
||||
transport_event_tx
|
||||
.send(TransportEvent::ConnectionOpened {
|
||||
connection_id,
|
||||
writer: writer_tx,
|
||||
disconnect_sender: None,
|
||||
})
|
||||
.await
|
||||
.map_err(|_| std::io::Error::new(ErrorKind::BrokenPipe, "processor unavailable"))?;
|
||||
|
||||
let transport_event_tx_for_reader = transport_event_tx.clone();
|
||||
stdio_handles.push(tokio::spawn(async move {
|
||||
let stdin = io::stdin();
|
||||
let reader = BufReader::new(stdin);
|
||||
let mut lines = reader.lines();
|
||||
|
||||
loop {
|
||||
match lines.next_line().await {
|
||||
Ok(Some(line)) => {
|
||||
if !forward_incoming_message(
|
||||
&transport_event_tx_for_reader,
|
||||
&writer_tx_for_reader,
|
||||
connection_id,
|
||||
&line,
|
||||
)
|
||||
.await
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
Ok(None) => break,
|
||||
Err(err) => {
|
||||
error!("Failed reading stdin: {err}");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let _ = transport_event_tx_for_reader
|
||||
.send(TransportEvent::ConnectionClosed { connection_id })
|
||||
.await;
|
||||
debug!("stdin reader finished (EOF)");
|
||||
}));
|
||||
|
||||
stdio_handles.push(tokio::spawn(async move {
|
||||
let mut stdout = io::stdout();
|
||||
while let Some(queued_message) = writer_rx.recv().await {
|
||||
let Some(mut json) = serialize_outgoing_message(queued_message.message) else {
|
||||
continue;
|
||||
};
|
||||
json.push('\n');
|
||||
if let Err(err) = stdout.write_all(json.as_bytes()).await {
|
||||
error!("Failed to write to stdout: {err}");
|
||||
break;
|
||||
}
|
||||
if let Some(write_complete_tx) = queued_message.write_complete_tx {
|
||||
let _ = write_complete_tx.send(());
|
||||
}
|
||||
}
|
||||
info!("stdout writer exited (channel closed)");
|
||||
}));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -0,0 +1,308 @@
|
||||
use super::CHANNEL_CAPACITY;
|
||||
use super::TransportEvent;
|
||||
use super::auth::WebsocketAuthPolicy;
|
||||
use super::auth::authorize_upgrade;
|
||||
use super::auth::should_warn_about_unauthenticated_non_loopback_listener;
|
||||
use super::forward_incoming_message;
|
||||
use super::serialize_outgoing_message;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::QueuedOutgoingMessage;
|
||||
use axum::Router;
|
||||
use axum::body::Body;
|
||||
use axum::extract::ConnectInfo;
|
||||
use axum::extract::State;
|
||||
use axum::extract::ws::Message as WebSocketMessage;
|
||||
use axum::extract::ws::WebSocket;
|
||||
use axum::extract::ws::WebSocketUpgrade;
|
||||
use axum::http::HeaderMap;
|
||||
use axum::http::Request;
|
||||
use axum::http::StatusCode;
|
||||
use axum::http::header::ORIGIN;
|
||||
use axum::middleware;
|
||||
use axum::middleware::Next;
|
||||
use axum::response::IntoResponse;
|
||||
use axum::response::Response;
|
||||
use axum::routing::any;
|
||||
use axum::routing::get;
|
||||
use futures::SinkExt;
|
||||
use futures::StreamExt;
|
||||
use owo_colors::OwoColorize;
|
||||
use owo_colors::Stream;
|
||||
use owo_colors::Style;
|
||||
use std::io::Result as IoResult;
|
||||
use std::net::SocketAddr;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicU64;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tokio::net::TcpListener;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::task::JoinHandle;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::error;
|
||||
use tracing::info;
|
||||
use tracing::warn;
|
||||
|
||||
fn colorize(text: &str, style: Style) -> String {
|
||||
text.if_supports_color(Stream::Stderr, |value| value.style(style))
|
||||
.to_string()
|
||||
}
|
||||
|
||||
#[allow(clippy::print_stderr)]
|
||||
fn print_websocket_startup_banner(addr: SocketAddr) {
|
||||
let title = colorize("codex app-server (WebSockets)", Style::new().bold().cyan());
|
||||
let listening_label = colorize("listening on:", Style::new().dimmed());
|
||||
let listen_url = colorize(&format!("ws://{addr}"), Style::new().green());
|
||||
let ready_label = colorize("readyz:", Style::new().dimmed());
|
||||
let ready_url = colorize(&format!("http://{addr}/readyz"), Style::new().green());
|
||||
let health_label = colorize("healthz:", Style::new().dimmed());
|
||||
let health_url = colorize(&format!("http://{addr}/healthz"), Style::new().green());
|
||||
let note_label = colorize("note:", Style::new().dimmed());
|
||||
eprintln!("{title}");
|
||||
eprintln!(" {listening_label} {listen_url}");
|
||||
eprintln!(" {ready_label} {ready_url}");
|
||||
eprintln!(" {health_label} {health_url}");
|
||||
if addr.ip().is_loopback() {
|
||||
eprintln!(
|
||||
" {note_label} binds localhost only (use SSH port-forwarding for remote access)"
|
||||
);
|
||||
} else {
|
||||
eprintln!(
|
||||
" {note_label} websocket auth is opt-in in this build; configure `--ws-auth ...` before real remote use"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct WebSocketListenerState {
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
connection_counter: Arc<AtomicU64>,
|
||||
auth_policy: Arc<WebsocketAuthPolicy>,
|
||||
}
|
||||
|
||||
async fn health_check_handler() -> StatusCode {
|
||||
StatusCode::OK
|
||||
}
|
||||
|
||||
async fn reject_requests_with_origin_header(
|
||||
request: Request<Body>,
|
||||
next: Next,
|
||||
) -> Result<Response, StatusCode> {
|
||||
if request.headers().contains_key(ORIGIN) {
|
||||
warn!(
|
||||
method = %request.method(),
|
||||
uri = %request.uri(),
|
||||
"rejecting websocket listener request with Origin header"
|
||||
);
|
||||
Err(StatusCode::FORBIDDEN)
|
||||
} else {
|
||||
Ok(next.run(request).await)
|
||||
}
|
||||
}
|
||||
|
||||
async fn websocket_upgrade_handler(
|
||||
websocket: WebSocketUpgrade,
|
||||
ConnectInfo(peer_addr): ConnectInfo<SocketAddr>,
|
||||
State(state): State<WebSocketListenerState>,
|
||||
headers: HeaderMap,
|
||||
) -> impl IntoResponse {
|
||||
if let Err(err) = authorize_upgrade(&headers, state.auth_policy.as_ref()) {
|
||||
warn!(
|
||||
%peer_addr,
|
||||
message = err.message(),
|
||||
"rejecting websocket client during upgrade"
|
||||
);
|
||||
return (err.status_code(), err.message()).into_response();
|
||||
}
|
||||
let connection_id = ConnectionId(state.connection_counter.fetch_add(1, Ordering::Relaxed));
|
||||
info!(%peer_addr, "websocket client connected");
|
||||
websocket
|
||||
.on_upgrade(move |stream| async move {
|
||||
run_websocket_connection(connection_id, stream, state.transport_event_tx).await;
|
||||
})
|
||||
.into_response()
|
||||
}
|
||||
|
||||
pub(crate) async fn start_websocket_acceptor(
|
||||
bind_address: SocketAddr,
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
shutdown_token: CancellationToken,
|
||||
auth_policy: WebsocketAuthPolicy,
|
||||
) -> IoResult<JoinHandle<()>> {
|
||||
if should_warn_about_unauthenticated_non_loopback_listener(bind_address, &auth_policy) {
|
||||
warn!(
|
||||
%bind_address,
|
||||
"starting non-loopback websocket listener without auth; websocket auth is opt-in for now and will become the default in a future release"
|
||||
);
|
||||
}
|
||||
let listener = TcpListener::bind(bind_address).await?;
|
||||
let local_addr = listener.local_addr()?;
|
||||
print_websocket_startup_banner(local_addr);
|
||||
info!("app-server websocket listening on ws://{local_addr}");
|
||||
|
||||
let router = Router::new()
|
||||
.route("/readyz", get(health_check_handler))
|
||||
.route("/healthz", get(health_check_handler))
|
||||
.fallback(any(websocket_upgrade_handler))
|
||||
.layer(middleware::from_fn(reject_requests_with_origin_header))
|
||||
.with_state(WebSocketListenerState {
|
||||
transport_event_tx,
|
||||
connection_counter: Arc::new(AtomicU64::new(1)),
|
||||
auth_policy: Arc::new(auth_policy),
|
||||
});
|
||||
let server = axum::serve(
|
||||
listener,
|
||||
router.into_make_service_with_connect_info::<SocketAddr>(),
|
||||
)
|
||||
.with_graceful_shutdown(async move {
|
||||
shutdown_token.cancelled().await;
|
||||
});
|
||||
Ok(tokio::spawn(async move {
|
||||
if let Err(err) = server.await {
|
||||
error!("websocket acceptor failed: {err}");
|
||||
}
|
||||
info!("websocket acceptor shutting down");
|
||||
}))
|
||||
}
|
||||
|
||||
async fn run_websocket_connection(
|
||||
connection_id: ConnectionId,
|
||||
websocket_stream: WebSocket,
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
) {
|
||||
let (writer_tx, writer_rx) = mpsc::channel::<QueuedOutgoingMessage>(CHANNEL_CAPACITY);
|
||||
let writer_tx_for_reader = writer_tx.clone();
|
||||
let disconnect_token = CancellationToken::new();
|
||||
if transport_event_tx
|
||||
.send(TransportEvent::ConnectionOpened {
|
||||
connection_id,
|
||||
writer: writer_tx,
|
||||
disconnect_sender: Some(disconnect_token.clone()),
|
||||
})
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let (websocket_writer, websocket_reader) = websocket_stream.split();
|
||||
let (writer_control_tx, writer_control_rx) =
|
||||
mpsc::channel::<WebSocketMessage>(CHANNEL_CAPACITY);
|
||||
let mut outbound_task = tokio::spawn(run_websocket_outbound_loop(
|
||||
websocket_writer,
|
||||
writer_rx,
|
||||
writer_control_rx,
|
||||
disconnect_token.clone(),
|
||||
));
|
||||
let mut inbound_task = tokio::spawn(run_websocket_inbound_loop(
|
||||
websocket_reader,
|
||||
transport_event_tx.clone(),
|
||||
writer_tx_for_reader,
|
||||
writer_control_tx,
|
||||
connection_id,
|
||||
disconnect_token.clone(),
|
||||
));
|
||||
|
||||
tokio::select! {
|
||||
_ = &mut outbound_task => {
|
||||
disconnect_token.cancel();
|
||||
inbound_task.abort();
|
||||
}
|
||||
_ = &mut inbound_task => {
|
||||
disconnect_token.cancel();
|
||||
outbound_task.abort();
|
||||
}
|
||||
}
|
||||
|
||||
let _ = transport_event_tx
|
||||
.send(TransportEvent::ConnectionClosed { connection_id })
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn run_websocket_outbound_loop(
|
||||
mut websocket_writer: futures::stream::SplitSink<WebSocket, WebSocketMessage>,
|
||||
mut writer_rx: mpsc::Receiver<QueuedOutgoingMessage>,
|
||||
mut writer_control_rx: mpsc::Receiver<WebSocketMessage>,
|
||||
disconnect_token: CancellationToken,
|
||||
) {
|
||||
loop {
|
||||
tokio::select! {
|
||||
_ = disconnect_token.cancelled() => {
|
||||
break;
|
||||
}
|
||||
message = writer_control_rx.recv() => {
|
||||
let Some(message) = message else {
|
||||
break;
|
||||
};
|
||||
if websocket_writer.send(message).await.is_err() {
|
||||
break;
|
||||
}
|
||||
}
|
||||
queued_message = writer_rx.recv() => {
|
||||
let Some(queued_message) = queued_message else {
|
||||
break;
|
||||
};
|
||||
let Some(json) = serialize_outgoing_message(queued_message.message) else {
|
||||
continue;
|
||||
};
|
||||
if websocket_writer.send(WebSocketMessage::Text(json.into())).await.is_err() {
|
||||
break;
|
||||
}
|
||||
if let Some(write_complete_tx) = queued_message.write_complete_tx {
|
||||
let _ = write_complete_tx.send(());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_websocket_inbound_loop(
|
||||
mut websocket_reader: futures::stream::SplitStream<WebSocket>,
|
||||
transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
writer_tx_for_reader: mpsc::Sender<QueuedOutgoingMessage>,
|
||||
writer_control_tx: mpsc::Sender<WebSocketMessage>,
|
||||
connection_id: ConnectionId,
|
||||
disconnect_token: CancellationToken,
|
||||
) {
|
||||
loop {
|
||||
tokio::select! {
|
||||
_ = disconnect_token.cancelled() => {
|
||||
break;
|
||||
}
|
||||
incoming_message = websocket_reader.next() => {
|
||||
match incoming_message {
|
||||
Some(Ok(WebSocketMessage::Text(text))) => {
|
||||
if !forward_incoming_message(
|
||||
&transport_event_tx,
|
||||
&writer_tx_for_reader,
|
||||
connection_id,
|
||||
text.as_ref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
Some(Ok(WebSocketMessage::Ping(payload))) => {
|
||||
match writer_control_tx.try_send(WebSocketMessage::Pong(payload)) {
|
||||
Ok(()) => {}
|
||||
Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => break,
|
||||
Err(tokio::sync::mpsc::error::TrySendError::Full(_)) => {
|
||||
warn!("websocket control queue full while replying to ping; closing connection");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
Some(Ok(WebSocketMessage::Pong(_))) => {}
|
||||
Some(Ok(WebSocketMessage::Close(_))) | None => break,
|
||||
Some(Ok(WebSocketMessage::Binary(_))) => {
|
||||
warn!("dropping unsupported binary websocket message");
|
||||
}
|
||||
Some(Err(err)) => {
|
||||
warn!("websocket receive error: {err}");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -59,13 +59,13 @@ only when the split filesystem policy round-trips through the legacy
|
||||
cases like `/repo = write`, `/repo/a = none`, `/repo/a/b = write`, where the
|
||||
more specific writable child must reopen under a denied parent.
|
||||
|
||||
The Linux sandbox helper prefers `/usr/bin/bwrap` whenever it is available. If
|
||||
`/usr/bin/bwrap` is present but too old to support `--argv0`, the helper keeps
|
||||
using system bubblewrap and switches to a no-`--argv0` compatibility path for
|
||||
the inner re-exec. If `/usr/bin/bwrap` is missing, it falls back to the
|
||||
vendored bubblewrap path compiled into the binary and Codex surfaces a startup
|
||||
warning through its normal notification path instead of printing directly from
|
||||
the sandbox helper.
|
||||
The Linux sandbox helper prefers the first `bwrap` found on `PATH` outside the
|
||||
current working directory whenever it is available. If `bwrap` is present but
|
||||
too old to support `--argv0`, the helper keeps using system bubblewrap and
|
||||
switches to a no-`--argv0` compatibility path for the inner re-exec. If
|
||||
`bwrap` is missing, it falls back to the vendored bubblewrap path compiled into
|
||||
the binary and Codex surfaces a startup warning through its normal notification
|
||||
path instead of printing directly from the sandbox helper.
|
||||
|
||||
### Windows
|
||||
|
||||
|
||||
@@ -75,7 +75,6 @@ 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;
|
||||
@@ -2897,7 +2896,6 @@ 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`.
|
||||
@@ -2951,7 +2949,6 @@ impl Session {
|
||||
proposed_execpolicy_amendment,
|
||||
proposed_network_policy_amendments,
|
||||
additional_permissions,
|
||||
skill_metadata,
|
||||
available_decisions: Some(available_decisions),
|
||||
parsed_cmd,
|
||||
});
|
||||
|
||||
@@ -436,7 +436,6 @@ async fn handle_exec_approval(
|
||||
network_approval_context,
|
||||
proposed_execpolicy_amendment,
|
||||
additional_permissions,
|
||||
skill_metadata,
|
||||
available_decisions,
|
||||
..
|
||||
} = event;
|
||||
@@ -480,7 +479,6 @@ async fn handle_exec_approval(
|
||||
network_approval_context,
|
||||
proposed_execpolicy_amendment,
|
||||
additional_permissions,
|
||||
skill_metadata,
|
||||
available_decisions,
|
||||
),
|
||||
parent_session,
|
||||
|
||||
@@ -286,7 +286,6 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: Some(vec![
|
||||
ReviewDecision::Approved,
|
||||
ReviewDecision::Abort,
|
||||
|
||||
@@ -1162,35 +1162,10 @@ async fn fork_startup_context_then_first_turn_diff_snapshot() -> anyhow::Result<
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&initial.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
// The parent rollout writer drains asynchronously after turn completion.
|
||||
// Wait until the persisted JSONL includes the source user turn before forking from it.
|
||||
let mut source_history_persisted = false;
|
||||
for _ in 0..100 {
|
||||
let history = RolloutRecorder::get_rollout_history(&rollout_path).await;
|
||||
source_history_persisted = history.ok().is_some_and(|history| {
|
||||
history.get_rollout_items().into_iter().any(|item| {
|
||||
matches!(
|
||||
item,
|
||||
RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. })
|
||||
if role == "user"
|
||||
&& content.iter().any(|content_item| {
|
||||
matches!(
|
||||
content_item,
|
||||
ContentItem::InputText { text } if text == "fork seed"
|
||||
)
|
||||
})
|
||||
)
|
||||
})
|
||||
});
|
||||
if source_history_persisted {
|
||||
break;
|
||||
}
|
||||
sleep(StdDuration::from_millis(10)).await;
|
||||
}
|
||||
assert!(
|
||||
source_history_persisted,
|
||||
"source rollout should contain the completed pre-fork user turn before forking"
|
||||
);
|
||||
// Forking reads the persisted rollout JSONL, so force the completed source turn to disk
|
||||
// before snapshotting from it.
|
||||
initial.codex.ensure_rollout_materialized().await;
|
||||
initial.codex.flush_rollout().await;
|
||||
|
||||
let mut fork_config = initial.config.clone();
|
||||
fork_config.permissions.approval_policy =
|
||||
|
||||
@@ -74,6 +74,16 @@ impl CodexThread {
|
||||
self.codex.shutdown_and_wait().await
|
||||
}
|
||||
|
||||
#[doc(hidden)]
|
||||
pub async fn ensure_rollout_materialized(&self) {
|
||||
self.codex.session.ensure_rollout_materialized().await;
|
||||
}
|
||||
|
||||
#[doc(hidden)]
|
||||
pub async fn flush_rollout(&self) {
|
||||
self.codex.session.flush_rollout().await;
|
||||
}
|
||||
|
||||
pub async fn submit_with_trace(
|
||||
&self,
|
||||
op: Op,
|
||||
|
||||
@@ -5695,8 +5695,8 @@ shell_tool = true
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn system_bwrap_warning_reports_missing_system_bwrap() {
|
||||
let warning = system_bwrap_warning_for_path(Path::new("/definitely/not/a/bwrap"))
|
||||
.expect("missing system bwrap should emit a warning");
|
||||
let warning =
|
||||
system_bwrap_warning_for_lookup(None).expect("missing system bwrap should emit a warning");
|
||||
|
||||
assert!(warning.contains("could not find system bubblewrap"));
|
||||
}
|
||||
@@ -5716,12 +5716,48 @@ exit 1
|
||||
let fake_bwrap_path: &Path = fake_bwrap.as_ref();
|
||||
|
||||
assert_eq!(
|
||||
system_bwrap_warning_for_path(fake_bwrap_path),
|
||||
system_bwrap_warning_for_lookup(Some(fake_bwrap_path.to_path_buf())),
|
||||
None,
|
||||
"Do not warn even if bwrap does not support `--argv0`",
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn finds_first_executable_bwrap_in_search_paths() {
|
||||
let temp_dir = tempdir().expect("temp dir");
|
||||
let cwd = temp_dir.path().join("cwd");
|
||||
let first_dir = temp_dir.path().join("first");
|
||||
let second_dir = temp_dir.path().join("second");
|
||||
std::fs::create_dir_all(&cwd).expect("create cwd");
|
||||
std::fs::create_dir_all(&first_dir).expect("create first dir");
|
||||
std::fs::create_dir_all(&second_dir).expect("create second dir");
|
||||
std::fs::write(first_dir.join("bwrap"), "not executable").expect("write non-executable bwrap");
|
||||
let expected_bwrap = write_named_fake_bwrap_in(&second_dir);
|
||||
|
||||
assert_eq!(
|
||||
find_system_bwrap_in_search_paths(vec![first_dir, second_dir], &cwd),
|
||||
Some(expected_bwrap)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn skips_workspace_local_bwrap_in_search_paths() {
|
||||
let temp_dir = tempdir().expect("temp dir");
|
||||
let cwd = temp_dir.path().join("cwd");
|
||||
let trusted_dir = temp_dir.path().join("trusted");
|
||||
std::fs::create_dir_all(&cwd).expect("create cwd");
|
||||
std::fs::create_dir_all(&trusted_dir).expect("create trusted dir");
|
||||
let _workspace_bwrap = write_named_fake_bwrap_in(&cwd);
|
||||
let expected_bwrap = write_named_fake_bwrap_in(&trusted_dir);
|
||||
|
||||
assert_eq!(
|
||||
find_system_bwrap_in_search_paths(vec![cwd.clone(), trusted_dir], &cwd),
|
||||
Some(expected_bwrap)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
#[test]
|
||||
fn system_bwrap_warning_is_disabled_off_linux() {
|
||||
@@ -5730,6 +5766,14 @@ fn system_bwrap_warning_is_disabled_off_linux() {
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn write_fake_bwrap(contents: &str) -> tempfile::TempPath {
|
||||
write_fake_bwrap_in(
|
||||
&std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
contents,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn write_fake_bwrap_in(dir: &Path, contents: &str) -> tempfile::TempPath {
|
||||
use std::fs;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use tempfile::NamedTempFile;
|
||||
@@ -5737,9 +5781,8 @@ fn write_fake_bwrap(contents: &str) -> tempfile::TempPath {
|
||||
// Bazel can mount the OS temp directory `noexec`, so prefer the current
|
||||
// working directory for fake executables and fall back to the default temp
|
||||
// dir outside that environment.
|
||||
let temp_file = std::env::current_dir()
|
||||
let temp_file = NamedTempFile::new_in(dir)
|
||||
.ok()
|
||||
.and_then(|dir| NamedTempFile::new_in(dir).ok())
|
||||
.unwrap_or_else(|| NamedTempFile::new().expect("temp file"));
|
||||
// Linux rejects exec-ing a file that is still open for writing.
|
||||
let path = temp_file.into_temp_path();
|
||||
@@ -5749,6 +5792,18 @@ fn write_fake_bwrap(contents: &str) -> tempfile::TempPath {
|
||||
path
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn write_named_fake_bwrap_in(dir: &Path) -> PathBuf {
|
||||
use std::fs;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let path = dir.join("bwrap");
|
||||
fs::write(&path, "#!/bin/sh\n").expect("write fake bwrap");
|
||||
let permissions = fs::Permissions::from_mode(0o755);
|
||||
fs::set_permissions(&path, permissions).expect("chmod fake bwrap");
|
||||
path
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn approvals_reviewer_defaults_to_manual_only_without_guardian_feature() -> std::io::Result<()>
|
||||
{
|
||||
|
||||
@@ -145,7 +145,7 @@ pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option<u64> = None;
|
||||
pub const CONFIG_TOML_FILE: &str = "config.toml";
|
||||
const OPENAI_BASE_URL_ENV_VAR: &str = "OPENAI_BASE_URL";
|
||||
#[cfg(target_os = "linux")]
|
||||
const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";
|
||||
const SYSTEM_BWRAP_PROGRAM: &str = "bwrap";
|
||||
const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [
|
||||
OPENAI_PROVIDER_ID,
|
||||
OLLAMA_OSS_PROVIDER_ID,
|
||||
@@ -154,7 +154,7 @@ const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
pub fn system_bwrap_warning() -> Option<String> {
|
||||
system_bwrap_warning_for_path(Path::new(SYSTEM_BWRAP_PATH))
|
||||
system_bwrap_warning_for_lookup(find_system_bwrap_in_path())
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
@@ -163,15 +163,40 @@ pub fn system_bwrap_warning() -> Option<String> {
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn system_bwrap_warning_for_path(system_bwrap_path: &Path) -> Option<String> {
|
||||
if !system_bwrap_path.is_file() {
|
||||
return Some(format!(
|
||||
"Codex could not find system bubblewrap at {}. Please install bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime.",
|
||||
system_bwrap_path.display()
|
||||
));
|
||||
fn system_bwrap_warning_for_lookup(system_bwrap_path: Option<PathBuf>) -> Option<String> {
|
||||
match system_bwrap_path {
|
||||
Some(_) => None,
|
||||
None => Some(
|
||||
"Codex could not find system bubblewrap on PATH. Please install bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime."
|
||||
.to_string(),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
#[cfg(target_os = "linux")]
|
||||
pub fn find_system_bwrap_in_path() -> Option<PathBuf> {
|
||||
let search_path = std::env::var_os("PATH")?;
|
||||
let cwd = std::env::current_dir().ok()?;
|
||||
find_system_bwrap_in_search_paths(std::iter::once(PathBuf::from(search_path)), &cwd)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn find_system_bwrap_in_search_paths(
|
||||
search_paths: impl IntoIterator<Item = PathBuf>,
|
||||
cwd: &Path,
|
||||
) -> Option<PathBuf> {
|
||||
let search_path = std::env::join_paths(search_paths).ok()?;
|
||||
let cwd = std::fs::canonicalize(cwd).unwrap_or_else(|_| cwd.to_path_buf());
|
||||
which::which_in_all(SYSTEM_BWRAP_PROGRAM, Some(search_path), &cwd)
|
||||
.ok()?
|
||||
.find_map(|path| {
|
||||
let path = std::fs::canonicalize(path).ok()?;
|
||||
if path.starts_with(&cwd) {
|
||||
None
|
||||
} else {
|
||||
Some(path)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option<PathBuf> {
|
||||
|
||||
@@ -595,6 +595,19 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() {
|
||||
},
|
||||
]);
|
||||
|
||||
#[cfg(windows)]
|
||||
let expected_deny_write_paths = vec![
|
||||
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(cwd.join(".codex"))
|
||||
.expect("absolute .codex"),
|
||||
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs)
|
||||
.expect("absolute docs"),
|
||||
];
|
||||
#[cfg(not(windows))]
|
||||
let expected_deny_write_paths = vec![
|
||||
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs)
|
||||
.expect("absolute docs"),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
resolve_windows_restricted_token_filesystem_overlay(
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
@@ -605,10 +618,7 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() {
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
),
|
||||
Ok(Some(WindowsRestrictedTokenFilesystemOverlay {
|
||||
additional_deny_write_paths: vec![
|
||||
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs)
|
||||
.expect("absolute docs"),
|
||||
],
|
||||
additional_deny_write_paths: expected_deny_write_paths,
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -252,3 +252,37 @@ fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() {
|
||||
SafetyCheck::AskUser,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_project_dot_codex_config_requires_approval() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let action =
|
||||
ApplyPatchAction::new_add_for_test(&cwd.join(".codex").join("config.toml"), "".to_string());
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
let file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, &cwd);
|
||||
|
||||
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,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -384,7 +384,6 @@ impl NetworkApprovalService {
|
||||
Some(network_approval_context.clone()),
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
/*additional_permissions*/ None,
|
||||
/*skill_metadata*/ None,
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -184,7 +184,6 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
.proposed_execpolicy_amendment()
|
||||
.cloned(),
|
||||
req.additional_permissions.clone(),
|
||||
/*skill_metadata*/ None,
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -426,7 +426,6 @@ impl CoreShellActionProvider {
|
||||
/*network_approval_context*/ None,
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
additional_permissions,
|
||||
/*skill_metadata*/ None,
|
||||
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -161,7 +161,6 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
.proposed_execpolicy_amendment()
|
||||
.cloned(),
|
||||
req.additional_permissions.clone(),
|
||||
/*skill_metadata*/ None,
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -2090,12 +2090,12 @@ fn tool_suggest_can_be_registered_without_search_tool() {
|
||||
let ToolSpec::Function(ResponsesApiTool { description, .. }) = &tool_suggest.spec else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request"
|
||||
)
|
||||
);
|
||||
assert!(description.contains("This includes `tool_search` (if available) and other means."));
|
||||
assert!(description.contains(
|
||||
"Suggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin"
|
||||
));
|
||||
assert!(description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2343,6 +2343,9 @@ fn tool_suggest_description_lists_discoverable_tools() {
|
||||
else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert!(description.contains(
|
||||
"Suggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin"
|
||||
));
|
||||
assert!(description.contains("Google Calendar"));
|
||||
assert!(description.contains("Gmail"));
|
||||
assert!(description.contains("Sample Plugin"));
|
||||
@@ -2355,10 +2358,21 @@ fn tool_suggest_description_lists_discoverable_tools() {
|
||||
);
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request"
|
||||
"You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means."
|
||||
)
|
||||
);
|
||||
assert!(description.contains("This includes `tool_search` (if available) and other means."));
|
||||
assert!(description.contains(
|
||||
"For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"Do not suggest a plugin just because one of its connectors or capabilities seems relevant."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard."
|
||||
));
|
||||
assert!(description.contains("DO NOT explore or recommend tools that are not on this list."));
|
||||
assert!(!description.contains("tool_search fails to find a good match"));
|
||||
let JsonSchema::Object { required, .. } = parameters else {
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
# Tool suggestion discovery
|
||||
|
||||
Suggests a discoverable connector or plugin when the user clearly wants a capability that is not currently available in the active `tools` list.
|
||||
Suggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.
|
||||
|
||||
Use this ONLY when:
|
||||
- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.
|
||||
- AND the user's request strongly matches one of the discoverable tools listed below.
|
||||
- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.
|
||||
- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.
|
||||
|
||||
Tool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.
|
||||
|
||||
@@ -14,7 +15,7 @@ Discoverable tools:
|
||||
Workflow:
|
||||
|
||||
1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.
|
||||
2. Match the user's request against the discoverable tools list above.
|
||||
2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.
|
||||
3. If one tool clearly fits, call `tool_suggest` with:
|
||||
- `tool_type`: `connector` or `plugin`
|
||||
- `action_type`: `install` or `enable`
|
||||
|
||||
@@ -118,6 +118,7 @@ async fn python_multiprocessing_lock_works_under_sandbox() {
|
||||
#[cfg(target_os = "linux")]
|
||||
let sandbox_env = match linux_sandbox_test_env().await {
|
||||
Some(env) => env,
|
||||
// Skip on Linux hosts where Landlock cannot actually be enforced.
|
||||
None => return,
|
||||
};
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
@@ -312,6 +313,78 @@ async fn sandbox_distinguishes_command_and_policy_cwds() {
|
||||
assert!(allowed_exists, "allowed path should exist");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sandbox_blocks_first_time_dot_codex_creation() {
|
||||
core_test_support::skip_if_sandbox!();
|
||||
#[cfg(target_os = "linux")]
|
||||
let sandbox_env = match linux_sandbox_test_env().await {
|
||||
Some(env) => env,
|
||||
None => return,
|
||||
};
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
let sandbox_env = HashMap::new();
|
||||
|
||||
let temp = tempfile::tempdir().expect("should be able to create temp dir");
|
||||
let repo_root = temp.path().join("repo");
|
||||
create_dir_all(&repo_root).await.expect("mkdir repo");
|
||||
let dot_codex = repo_root.join(".codex");
|
||||
let config_toml = dot_codex.join("config.toml");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
let mut child = spawn_command_under_sandbox(
|
||||
vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"mkdir -p .codex && echo 'sandbox_mode = \"danger-full-access\"' > .codex/config.toml"
|
||||
.to_string(),
|
||||
],
|
||||
repo_root.clone(),
|
||||
&policy,
|
||||
repo_root.as_path(),
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
sandbox_env,
|
||||
)
|
||||
.await
|
||||
.expect("should spawn command creating .codex");
|
||||
|
||||
let status = child.wait().await.expect("should wait for .codex command");
|
||||
assert!(
|
||||
!status.success(),
|
||||
"sandbox unexpectedly allowed first-time .codex creation: {status:?}"
|
||||
);
|
||||
let dot_codex_metadata = tokio::fs::symlink_metadata(&dot_codex).await;
|
||||
if let Ok(metadata) = dot_codex_metadata {
|
||||
assert!(
|
||||
!metadata.is_dir(),
|
||||
"{} should not be creatable as a directory",
|
||||
dot_codex.display()
|
||||
);
|
||||
} else if let Err(err) = &dot_codex_metadata {
|
||||
assert_eq!(
|
||||
err.kind(),
|
||||
io::ErrorKind::NotFound,
|
||||
"unexpected metadata error for {}: {err}",
|
||||
dot_codex.display()
|
||||
);
|
||||
}
|
||||
let config_toml_exists = match tokio::fs::try_exists(&config_toml).await {
|
||||
Ok(exists) => exists,
|
||||
Err(err) if err.kind() == io::ErrorKind::NotADirectory => false,
|
||||
Err(err) => panic!("try_exists {} failed: {err}", config_toml.display()),
|
||||
};
|
||||
assert!(
|
||||
!config_toml_exists,
|
||||
"{} should not have been created",
|
||||
config_toml.display()
|
||||
);
|
||||
}
|
||||
|
||||
fn unix_sock_body() {
|
||||
unsafe {
|
||||
let mut fds = [0i32; 2];
|
||||
|
||||
@@ -7,25 +7,27 @@ This crate is responsible for producing:
|
||||
- the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox`
|
||||
- this should also be true of the `codex` multitool CLI
|
||||
|
||||
On Linux, the bubblewrap pipeline prefers the system `/usr/bin/bwrap` whenever
|
||||
it is available. If `/usr/bin/bwrap` is present but too old to support
|
||||
On Linux, the bubblewrap pipeline prefers the first `bwrap` found on `PATH`
|
||||
outside the current working directory whenever it is available. If `bwrap` is
|
||||
present but too old to support
|
||||
`--argv0`, the helper keeps using system bubblewrap and switches to a
|
||||
no-`--argv0` compatibility path for the inner re-exec. If `/usr/bin/bwrap` is
|
||||
missing, the helper falls back to the vendored bubblewrap path compiled into
|
||||
this binary.
|
||||
Codex also surfaces a startup warning when `/usr/bin/bwrap` is missing so users
|
||||
know it is falling back to the vendored helper.
|
||||
no-`--argv0` compatibility path for the inner re-exec. If `bwrap` is missing,
|
||||
the helper falls back to the vendored bubblewrap path compiled into this
|
||||
binary.
|
||||
Codex also surfaces a startup warning when `bwrap` is missing so users know it
|
||||
is falling back to the vendored helper.
|
||||
|
||||
**Current Behavior**
|
||||
- Legacy `SandboxPolicy` / `sandbox_mode` configs remain supported.
|
||||
- Bubblewrap is the default filesystem sandbox pipeline.
|
||||
- If `/usr/bin/bwrap` is present, the helper uses it.
|
||||
- If `/usr/bin/bwrap` is present but too old to support `--argv0`, the helper
|
||||
uses a no-`--argv0` compatibility path for the inner re-exec.
|
||||
- If `/usr/bin/bwrap` is missing, the helper falls back to the vendored
|
||||
bubblewrap path.
|
||||
- If `/usr/bin/bwrap` is missing, Codex also surfaces a startup warning instead
|
||||
of printing directly from the sandbox helper.
|
||||
- If `bwrap` is present on `PATH` outside the current working directory, the
|
||||
helper uses it.
|
||||
- If `bwrap` is present but too old to support `--argv0`, the helper uses a
|
||||
no-`--argv0` compatibility path for the inner re-exec.
|
||||
- If `bwrap` is missing, the helper falls back to the vendored bubblewrap
|
||||
path.
|
||||
- If `bwrap` is missing, Codex also surfaces a startup warning instead of
|
||||
printing directly from the sandbox helper.
|
||||
- Legacy Landlock + mount protections remain available as an explicit legacy
|
||||
fallback path.
|
||||
- Set `features.use_legacy_landlock = true` (or CLI `-c use_legacy_landlock=true`)
|
||||
|
||||
@@ -770,14 +770,25 @@ mod tests {
|
||||
assert_eq!(
|
||||
args.args,
|
||||
vec![
|
||||
// Start from a read-only view of the full filesystem.
|
||||
"--ro-bind".to_string(),
|
||||
"/".to_string(),
|
||||
"/".to_string(),
|
||||
// Recreate a writable /dev inside the sandbox.
|
||||
"--dev".to_string(),
|
||||
"/dev".to_string(),
|
||||
// Make the writable root itself writable again.
|
||||
"--bind".to_string(),
|
||||
"/".to_string(),
|
||||
"/".to_string(),
|
||||
// Mask the default protected .codex subpath under that writable
|
||||
// root. Because the root is `/` in this test, the carveout path
|
||||
// appears as `/.codex`.
|
||||
"--ro-bind".to_string(),
|
||||
"/dev/null".to_string(),
|
||||
"/.codex".to_string(),
|
||||
// Rebind /dev after the root bind so device nodes remain
|
||||
// writable/usable inside the writable root.
|
||||
"--bind".to_string(),
|
||||
"/dev".to_string(),
|
||||
"/dev".to_string(),
|
||||
|
||||
@@ -10,8 +10,6 @@ use std::sync::OnceLock;
|
||||
use crate::vendored_bwrap::exec_vendored_bwrap;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
enum BubblewrapLauncher {
|
||||
System(SystemBwrapLauncher),
|
||||
@@ -36,7 +34,10 @@ pub(crate) fn exec_bwrap(argv: Vec<String>, preserved_files: Vec<File>) -> ! {
|
||||
fn preferred_bwrap_launcher() -> BubblewrapLauncher {
|
||||
static LAUNCHER: OnceLock<BubblewrapLauncher> = OnceLock::new();
|
||||
LAUNCHER
|
||||
.get_or_init(|| preferred_bwrap_launcher_for_path(Path::new(SYSTEM_BWRAP_PATH)))
|
||||
.get_or_init(|| match codex_core::config::find_system_bwrap_in_path() {
|
||||
Some(path) => preferred_bwrap_launcher_for_path(&path),
|
||||
None => BubblewrapLauncher::Vendored,
|
||||
})
|
||||
.clone()
|
||||
}
|
||||
|
||||
|
||||
@@ -228,7 +228,6 @@ async fn run_codex_tool_session_inner(
|
||||
parsed_cmd,
|
||||
network_approval_context: _,
|
||||
additional_permissions: _,
|
||||
skill_metadata: _,
|
||||
available_decisions: _,
|
||||
} = ev;
|
||||
handle_exec_approval_request(
|
||||
|
||||
@@ -1,6 +1,3 @@
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::mcp::RequestId;
|
||||
use crate::models::MacOsSeatbeltProfileExtensions;
|
||||
use crate::models::PermissionProfile;
|
||||
@@ -14,6 +11,8 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use ts_rs::TS;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -107,13 +106,6 @@ pub struct NetworkPolicyAmendment {
|
||||
pub action: NetworkPolicyRuleAction,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[ts(rename_all = "snake_case")]
|
||||
pub struct ExecApprovalRequestSkillMetadata {
|
||||
pub path_to_skills_md: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)]
|
||||
pub struct GuardianAssessmentEvent {
|
||||
/// Stable identifier for this guardian review lifecycle.
|
||||
@@ -181,10 +173,6 @@ pub struct ExecApprovalRequestEvent {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub additional_permissions: Option<PermissionProfile>,
|
||||
/// Optional skill metadata when the approval was triggered by a skill script.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub skill_metadata: Option<ExecApprovalRequestSkillMetadata>,
|
||||
/// Ordered list of decisions the client may present for this prompt.
|
||||
///
|
||||
/// When absent, clients should derive the legacy default set from the
|
||||
|
||||
@@ -264,7 +264,7 @@ impl FileSystemSandboxPolicy {
|
||||
/// into split filesystem policy.
|
||||
pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self {
|
||||
let mut file_system_policy = Self::from(sandbox_policy);
|
||||
if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) {
|
||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox_policy {
|
||||
let legacy_writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd);
|
||||
file_system_policy.entries.retain(|entry| {
|
||||
if entry.access != FileSystemAccessMode::Read {
|
||||
@@ -278,6 +278,28 @@ impl FileSystemSandboxPolicy {
|
||||
FileSystemPath::Special { .. } => true,
|
||||
}
|
||||
});
|
||||
|
||||
if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) {
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
&cwd_root, /*protect_missing_dot_codex*/ true,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(
|
||||
&mut file_system_policy.entries,
|
||||
protected_path,
|
||||
);
|
||||
}
|
||||
}
|
||||
for writable_root in writable_roots {
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
writable_root,
|
||||
/*protect_missing_dot_codex*/ false,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(
|
||||
&mut file_system_policy.entries,
|
||||
protected_path,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
file_system_policy
|
||||
@@ -454,7 +476,14 @@ impl FileSystemSandboxPolicy {
|
||||
.iter()
|
||||
.filter(|path| normalize_effective_absolute_path((*path).clone()) == root)
|
||||
.collect();
|
||||
let mut read_only_subpaths = default_read_only_subpaths_for_writable_root(&root);
|
||||
let protect_missing_dot_codex = AbsolutePathBuf::from_absolute_path(cwd)
|
||||
.ok()
|
||||
.is_some_and(|cwd| normalize_effective_absolute_path(cwd) == root);
|
||||
let mut read_only_subpaths: Vec<AbsolutePathBuf> =
|
||||
default_read_only_subpaths_for_writable_root(&root, protect_missing_dot_codex)
|
||||
.into_iter()
|
||||
.filter(|path| !has_explicit_resolved_path_entry(&resolved_entries, path))
|
||||
.collect();
|
||||
// Narrower explicit non-write entries carve out broader writable roots.
|
||||
// More specific write entries still remain writable because they appear
|
||||
// as separate WritableRoot values and are checked independently.
|
||||
@@ -1068,6 +1097,7 @@ fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf {
|
||||
|
||||
fn default_read_only_subpaths_for_writable_root(
|
||||
writable_root: &AbsolutePathBuf,
|
||||
protect_missing_dot_codex: bool,
|
||||
) -> Vec<AbsolutePathBuf> {
|
||||
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
|
||||
#[allow(clippy::expect_used)]
|
||||
@@ -1089,19 +1119,69 @@ fn default_read_only_subpaths_for_writable_root(
|
||||
subpaths.push(top_level_git);
|
||||
}
|
||||
|
||||
// Make .agents/skills and .codex/config.toml and related files read-only
|
||||
// to the agent, by default.
|
||||
for subdir in &[".agents", ".codex"] {
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex = writable_root.join(subdir).expect("valid relative path");
|
||||
if top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_agents = writable_root.join(".agents").expect("valid relative path");
|
||||
if top_level_agents.as_path().is_dir() {
|
||||
subpaths.push(top_level_agents);
|
||||
}
|
||||
|
||||
// Keep top-level project metadata under .codex read-only to the agent by
|
||||
// default. For the workspace root itself, protect it even before the
|
||||
// directory exists so first-time creation still goes through the
|
||||
// protected-path approval flow.
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex = writable_root.join(".codex").expect("valid relative path");
|
||||
if protect_missing_dot_codex || top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
|
||||
dedup_absolute_paths(subpaths, /*normalize_effective_paths*/ false)
|
||||
}
|
||||
|
||||
fn append_path_entry_if_missing(
|
||||
entries: &mut Vec<FileSystemSandboxEntry>,
|
||||
path: AbsolutePathBuf,
|
||||
access: FileSystemAccessMode,
|
||||
) {
|
||||
if entries.iter().any(|entry| {
|
||||
entry.access == access
|
||||
&& matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Path { path: existing } if existing == &path
|
||||
)
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
|
||||
entries.push(FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path },
|
||||
access,
|
||||
});
|
||||
}
|
||||
|
||||
fn append_default_read_only_path_if_no_explicit_rule(
|
||||
entries: &mut Vec<FileSystemSandboxEntry>,
|
||||
path: AbsolutePathBuf,
|
||||
) {
|
||||
if entries.iter().any(|entry| {
|
||||
matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Path { path: existing } if existing == &path
|
||||
)
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
|
||||
append_path_entry_if_missing(entries, path, FileSystemAccessMode::Read);
|
||||
}
|
||||
|
||||
fn has_explicit_resolved_path_entry(
|
||||
entries: &[ResolvedFileSystemEntry],
|
||||
path: &AbsolutePathBuf,
|
||||
) -> bool {
|
||||
entries.iter().any(|entry| &entry.path == path)
|
||||
}
|
||||
|
||||
fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool {
|
||||
path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git"))
|
||||
}
|
||||
@@ -1211,6 +1291,148 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn writable_roots_proactively_protect_missing_dot_codex() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
let expected_root = AbsolutePathBuf::from_absolute_path(
|
||||
cwd.path().canonicalize().expect("canonicalize cwd"),
|
||||
)
|
||||
.expect("absolute canonical root");
|
||||
let expected_dot_codex = expected_root.join(".codex").expect("expected .codex path");
|
||||
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
}]);
|
||||
|
||||
let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
|
||||
assert_eq!(writable_roots.len(), 1);
|
||||
assert_eq!(writable_roots[0].root, expected_root);
|
||||
assert!(
|
||||
writable_roots[0]
|
||||
.read_only_subpaths
|
||||
.contains(&expected_dot_codex)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn writable_roots_skip_default_dot_codex_when_explicit_user_rule_exists() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
let expected_root = AbsolutePathBuf::from_absolute_path(
|
||||
cwd.path().canonicalize().expect("canonicalize cwd"),
|
||||
)
|
||||
.expect("absolute canonical root");
|
||||
let explicit_dot_codex = expected_root.join(".codex").expect("expected .codex path");
|
||||
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: explicit_dot_codex.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
]);
|
||||
|
||||
let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
|
||||
let workspace_root = writable_roots
|
||||
.iter()
|
||||
.find(|root| root.root == expected_root)
|
||||
.expect("workspace writable root");
|
||||
assert!(
|
||||
!workspace_root
|
||||
.read_only_subpaths
|
||||
.contains(&explicit_dot_codex),
|
||||
"explicit .codex rule should win over the default protected carveout"
|
||||
);
|
||||
assert!(
|
||||
policy.can_write_path_with_cwd(
|
||||
explicit_dot_codex
|
||||
.join("config.toml")
|
||||
.expect("config.toml")
|
||||
.as_path(),
|
||||
cwd.path()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_workspace_write_projection_blocks_missing_dot_codex_writes() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
let dot_codex_config = cwd.path().join(".codex").join("config.toml");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: false,
|
||||
readable_roots: vec![],
|
||||
},
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
let file_system_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path());
|
||||
|
||||
assert!(!file_system_policy.can_write_path_with_cwd(&dot_codex_config, cwd.path()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_workspace_write_projection_accepts_relative_cwd() {
|
||||
let relative_cwd = Path::new("workspace");
|
||||
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(
|
||||
std::env::current_dir()
|
||||
.expect("current dir")
|
||||
.join(relative_cwd)
|
||||
.join(".codex"),
|
||||
)
|
||||
.expect("absolute dot codex");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: false,
|
||||
readable_roots: vec![],
|
||||
},
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
let file_system_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, relative_cwd);
|
||||
|
||||
assert_eq!(
|
||||
file_system_policy,
|
||||
FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: expected_dot_codex,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
])
|
||||
);
|
||||
assert!(
|
||||
!file_system_policy
|
||||
.can_write_path_with_cwd(Path::new(".codex/config.toml"), relative_cwd,)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn effective_runtime_roots_canonicalize_symlinked_paths() {
|
||||
@@ -1695,8 +1917,10 @@ mod tests {
|
||||
policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),)
|
||||
);
|
||||
|
||||
let legacy_workspace_write =
|
||||
FileSystemSandboxPolicy::from(&SandboxPolicy::new_workspace_write_policy());
|
||||
let legacy_workspace_write = FileSystemSandboxPolicy::from_legacy_sandbox_policy(
|
||||
&SandboxPolicy::new_workspace_write_policy(),
|
||||
cwd.path(),
|
||||
);
|
||||
assert!(
|
||||
!legacy_workspace_write
|
||||
.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),)
|
||||
|
||||
@@ -64,7 +64,6 @@ use ts_rs::TS;
|
||||
pub use crate::approvals::ApplyPatchApprovalRequestEvent;
|
||||
pub use crate::approvals::ElicitationAction;
|
||||
pub use crate::approvals::ExecApprovalRequestEvent;
|
||||
pub use crate::approvals::ExecApprovalRequestSkillMetadata;
|
||||
pub use crate::approvals::ExecPolicyAmendment;
|
||||
pub use crate::approvals::GuardianAssessmentEvent;
|
||||
pub use crate::approvals::GuardianAssessmentStatus;
|
||||
@@ -1064,13 +1063,20 @@ impl SandboxPolicy {
|
||||
}
|
||||
|
||||
// For each root, compute subpaths that should remain read-only.
|
||||
let cwd_root = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
roots
|
||||
.into_iter()
|
||||
.map(|writable_root| WritableRoot {
|
||||
read_only_subpaths: default_read_only_subpaths_for_writable_root(
|
||||
&writable_root,
|
||||
),
|
||||
root: writable_root,
|
||||
.map(|writable_root| {
|
||||
let protect_missing_dot_codex = cwd_root
|
||||
.as_ref()
|
||||
.is_some_and(|cwd_root| cwd_root == &writable_root);
|
||||
WritableRoot {
|
||||
read_only_subpaths: default_read_only_subpaths_for_writable_root(
|
||||
&writable_root,
|
||||
protect_missing_dot_codex,
|
||||
),
|
||||
root: writable_root,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -1080,6 +1086,7 @@ impl SandboxPolicy {
|
||||
|
||||
fn default_read_only_subpaths_for_writable_root(
|
||||
writable_root: &AbsolutePathBuf,
|
||||
protect_missing_dot_codex: bool,
|
||||
) -> Vec<AbsolutePathBuf> {
|
||||
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
|
||||
#[allow(clippy::expect_used)]
|
||||
@@ -1101,14 +1108,20 @@ fn default_read_only_subpaths_for_writable_root(
|
||||
subpaths.push(top_level_git);
|
||||
}
|
||||
|
||||
// Make .agents/skills and .codex/config.toml and related files read-only
|
||||
// to the agent, by default.
|
||||
for subdir in &[".agents", ".codex"] {
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex = writable_root.join(subdir).expect("valid relative path");
|
||||
if top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_agents = writable_root.join(".agents").expect("valid relative path");
|
||||
if top_level_agents.as_path().is_dir() {
|
||||
subpaths.push(top_level_agents);
|
||||
}
|
||||
|
||||
// Keep top-level project metadata under .codex read-only to the agent by
|
||||
// default. For the workspace root itself, protect it even before the
|
||||
// directory exists so first-time creation still goes through the
|
||||
// protected-path approval flow.
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex = writable_root.join(".codex").expect("valid relative path");
|
||||
if protect_missing_dot_codex || top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
|
||||
let mut deduped = Vec::with_capacity(subpaths.len());
|
||||
@@ -4095,6 +4108,8 @@ mod tests {
|
||||
let expected_docs_public =
|
||||
AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public"))
|
||||
.expect("canonical docs/public");
|
||||
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
|
||||
.expect("canonical .codex");
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
@@ -4116,7 +4131,13 @@ mod tests {
|
||||
assert_eq!(
|
||||
sorted_writable_roots(policy.get_writable_roots_with_cwd(cwd.path())),
|
||||
vec![
|
||||
(canonical_cwd, vec![expected_docs.to_path_buf()]),
|
||||
(
|
||||
canonical_cwd,
|
||||
vec![
|
||||
expected_dot_codex.to_path_buf(),
|
||||
expected_docs.to_path_buf()
|
||||
],
|
||||
),
|
||||
(expected_docs_public.to_path_buf(), Vec::new()),
|
||||
]
|
||||
);
|
||||
@@ -4128,6 +4149,8 @@ mod tests {
|
||||
let docs =
|
||||
AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs");
|
||||
let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd");
|
||||
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
|
||||
.expect("canonical .codex");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: ReadOnlyAccess::Restricted {
|
||||
@@ -4144,7 +4167,7 @@ mod tests {
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path())
|
||||
.get_writable_roots_with_cwd(cwd.path())
|
||||
),
|
||||
vec![(canonical_cwd, Vec::new())]
|
||||
vec![(canonical_cwd, vec![expected_dot_codex.to_path_buf()])]
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -330,8 +330,14 @@ fn build_seatbelt_access_policy(
|
||||
{
|
||||
let excluded_subpath =
|
||||
normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath);
|
||||
let excluded_param = format!("{param_prefix}_{index}_RO_{excluded_index}");
|
||||
let excluded_param = format!("{param_prefix}_{index}_EXCLUDED_{excluded_index}");
|
||||
params.push((excluded_param.clone(), excluded_subpath.into_path_buf()));
|
||||
// Exclude both the exact protected path and anything beneath it.
|
||||
// `subpath` alone leaves a gap for first-time creation of the
|
||||
// protected directory itself, such as `mkdir .codex`.
|
||||
require_parts.push(format!(
|
||||
"(require-not (literal (param \"{excluded_param}\")))"
|
||||
));
|
||||
require_parts.push(format!(
|
||||
"(require-not (subpath (param \"{excluded_param}\")))"
|
||||
));
|
||||
|
||||
@@ -129,22 +129,40 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access()
|
||||
let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/"));
|
||||
let unreadable_root = unreadable_roots.first().expect("expected unreadable root");
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"),
|
||||
policy.contains("(require-not (literal (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected exact read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_RO_0\")))"),
|
||||
policy.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected exact write carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected write carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
args.iter()
|
||||
.any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())),
|
||||
args.iter().any(
|
||||
|arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display())
|
||||
),
|
||||
"expected read carveout parameter in args: {args:#?}"
|
||||
);
|
||||
assert!(
|
||||
args.iter()
|
||||
.any(|arg| arg == &format!("-DWRITABLE_ROOT_0_RO_0={}", unreadable_root.display())),
|
||||
"expected write carveout parameter in args: {args:#?}"
|
||||
let writable_definitions: Vec<String> = args
|
||||
.iter()
|
||||
.filter(|arg| arg.starts_with("-DWRITABLE_ROOT_"))
|
||||
.cloned()
|
||||
.collect();
|
||||
assert_eq!(
|
||||
writable_definitions,
|
||||
vec![
|
||||
"-DWRITABLE_ROOT_0=/".to_string(),
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_0=/.codex".to_string(),
|
||||
format!("-DWRITABLE_ROOT_0_EXCLUDED_1={}", unreadable_root.display()),
|
||||
],
|
||||
"unexpected write carveout parameters in args: {args:#?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -179,7 +197,11 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() {
|
||||
let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/"));
|
||||
let unreadable_root = unreadable_roots.first().expect("expected unreadable root");
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"),
|
||||
policy.contains("(require-not (literal (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected exact read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
@@ -188,8 +210,9 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() {
|
||||
"expected readable root parameter in args: {args:#?}"
|
||||
);
|
||||
assert!(
|
||||
args.iter()
|
||||
.any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())),
|
||||
args.iter().any(
|
||||
|arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display())
|
||||
),
|
||||
"expected read carveout parameter in args: {args:#?}"
|
||||
);
|
||||
}
|
||||
@@ -314,14 +337,17 @@ fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() {
|
||||
/*extensions*/ 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:#?}"
|
||||
!args
|
||||
.iter()
|
||||
.any(|arg| arg.ends_with(&format!("={}", docs.as_path().display()))),
|
||||
"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:#?}"
|
||||
args.iter()
|
||||
.any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_")
|
||||
&& arg.ends_with("/workspace/.codex")),
|
||||
"expected proactive .codex carveout for cwd root: {args:#?}",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -647,30 +673,24 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
/*extensions*/ None,
|
||||
);
|
||||
|
||||
// Build the expected policy text using a raw string for readability.
|
||||
// Note that the policy includes:
|
||||
// - the base policy,
|
||||
// - read-only access to the filesystem,
|
||||
// - write access to WRITABLE_ROOT_0 (but not its .git or .codex), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2.
|
||||
let expected_policy = format!(
|
||||
r#"{MACOS_SEATBELT_BASE_POLICY}
|
||||
; allow read-only file operations
|
||||
(allow file-read*)
|
||||
(allow file-write*
|
||||
(subpath (param "WRITABLE_ROOT_0")) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (subpath (param "WRITABLE_ROOT_1_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_RO_1"))) ) (subpath (param "WRITABLE_ROOT_2"))
|
||||
)
|
||||
|
||||
; macOS permission profile extensions
|
||||
(allow ipc-posix-shm-read* (ipc-posix-name-prefix "apple.cfprefs."))
|
||||
(allow mach-lookup
|
||||
(global-name "com.apple.cfprefsd.daemon")
|
||||
(global-name "com.apple.cfprefsd.agent")
|
||||
(local-name "com.apple.cfprefsd.agent"))
|
||||
(allow user-preference-read)
|
||||
"#,
|
||||
let policy_text = seatbelt_policy_arg(&args);
|
||||
assert!(
|
||||
policy_text.contains("(require-all (subpath (param \"WRITABLE_ROOT_0\"))"),
|
||||
"expected cwd writable root to carry protected carveouts:\n{policy_text}",
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_0"),
|
||||
"expected cwd .codex carveout in policy:\n{policy_text}",
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_0")
|
||||
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1"),
|
||||
"expected explicit writable root .git/.codex carveouts in policy:\n{policy_text}",
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("(subpath (param \"WRITABLE_ROOT_2\"))"),
|
||||
"expected second explicit writable root grant in policy:\n{policy_text}",
|
||||
);
|
||||
|
||||
assert_eq!(seatbelt_policy_arg(&args), expected_policy);
|
||||
|
||||
let expected_definitions = [
|
||||
format!(
|
||||
@@ -679,16 +699,23 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
.expect("canonicalize cwd")
|
||||
.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
|
||||
cwd.canonicalize()
|
||||
.expect("canonicalize cwd")
|
||||
.join(".codex")
|
||||
.display()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1={}",
|
||||
vulnerable_root_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1_RO_0={}",
|
||||
"-DWRITABLE_ROOT_1_EXCLUDED_0={}",
|
||||
dot_git_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1_RO_1={}",
|
||||
"-DWRITABLE_ROOT_1_EXCLUDED_1={}",
|
||||
dot_codex_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
@@ -696,12 +723,15 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
empty_root_canonical.to_string_lossy()
|
||||
),
|
||||
];
|
||||
for expected_definition in expected_definitions {
|
||||
assert!(
|
||||
args.contains(&expected_definition),
|
||||
"expected definition arg `{expected_definition}` in {args:#?}"
|
||||
);
|
||||
}
|
||||
let writable_definitions: Vec<String> = args
|
||||
.iter()
|
||||
.filter(|arg| arg.starts_with("-DWRITABLE_ROOT_"))
|
||||
.cloned()
|
||||
.collect();
|
||||
assert_eq!(
|
||||
writable_definitions, expected_definitions,
|
||||
"unexpected writable-root parameter definitions in {args:#?}"
|
||||
);
|
||||
for (key, value) in macos_dir_params() {
|
||||
let expected_definition = format!("-D{key}={}", value.to_string_lossy());
|
||||
assert!(
|
||||
@@ -818,6 +848,60 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_descendant_carveouts() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(&repo_root).expect("create repo root");
|
||||
|
||||
Command::new("git")
|
||||
.arg("init")
|
||||
.arg(".")
|
||||
.current_dir(&repo_root)
|
||||
.output()
|
||||
.expect("git init .");
|
||||
|
||||
let dot_codex = repo_root.join(".codex");
|
||||
let config_toml = dot_codex.join("config.toml");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![repo_root.as_path().try_into().expect("absolute repo root")],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
let shell_command: Vec<String> = [
|
||||
"bash",
|
||||
"-c",
|
||||
"mkdir -p \"$1\" && echo 'sandbox_mode = \"danger-full-access\"' > \"$2\"",
|
||||
"bash",
|
||||
dot_codex.to_string_lossy().as_ref(),
|
||||
config_toml.to_string_lossy().as_ref(),
|
||||
]
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect();
|
||||
let args = create_seatbelt_command_args_with_extensions(
|
||||
shell_command,
|
||||
&policy,
|
||||
repo_root.as_path(),
|
||||
/*enforce_managed_network*/ false,
|
||||
/*network*/ None,
|
||||
/*extensions*/ None,
|
||||
);
|
||||
|
||||
let policy_text = seatbelt_policy_arg(&args);
|
||||
assert!(
|
||||
policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
|
||||
"expected exact .codex carveout in policy:\n{policy_text}"
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
|
||||
"expected descendant .codex carveout in policy:\n{policy_text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_seatbelt_args_with_read_only_git_pointer_file() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
@@ -971,7 +1055,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
.map(|p| p.to_string_lossy().to_string());
|
||||
|
||||
let tempdir_policy_entry = if tmpdir_env_var.is_some() {
|
||||
r#" (subpath (param "WRITABLE_ROOT_2"))"#
|
||||
r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) )"#
|
||||
} else {
|
||||
""
|
||||
};
|
||||
@@ -986,7 +1070,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
; allow read-only file operations
|
||||
(allow file-read*)
|
||||
(allow file-write*
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_RO_1"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
|
||||
)
|
||||
|
||||
; macOS permission profile extensions
|
||||
@@ -1007,11 +1091,11 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
vulnerable_root_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_0_RO_0={}",
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
|
||||
dot_git_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_0_RO_1={}",
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
|
||||
dot_codex_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
@@ -1025,6 +1109,14 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
|
||||
if let Some(p) = tmpdir_env_var {
|
||||
expected_args.push(format!("-DWRITABLE_ROOT_2={p}"));
|
||||
expected_args.push(format!(
|
||||
"-DWRITABLE_ROOT_2_EXCLUDED_0={}",
|
||||
dot_git_canonical.to_string_lossy()
|
||||
));
|
||||
expected_args.push(format!(
|
||||
"-DWRITABLE_ROOT_2_EXCLUDED_1={}",
|
||||
dot_codex_canonical.to_string_lossy()
|
||||
));
|
||||
}
|
||||
|
||||
expected_args.extend(
|
||||
|
||||
@@ -4979,7 +4979,6 @@ mod tests {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: Vec::new(),
|
||||
},
|
||||
@@ -6626,7 +6625,6 @@ guardian_approval = true
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: Vec::new(),
|
||||
},
|
||||
@@ -6715,7 +6713,6 @@ guardian_approval = true
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: Vec::new(),
|
||||
},
|
||||
|
||||
@@ -453,7 +453,6 @@ mod tests {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: Vec::new(),
|
||||
},
|
||||
@@ -597,7 +596,6 @@ mod tests {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: Vec::new(),
|
||||
},
|
||||
@@ -687,7 +685,6 @@ mod tests {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: Vec::new(),
|
||||
},
|
||||
|
||||
@@ -3456,7 +3456,6 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -3507,7 +3506,6 @@ async fn exec_approval_uses_approval_id_when_present() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
}),
|
||||
@@ -3549,7 +3547,6 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -3605,7 +3602,6 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -9966,7 +9962,6 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
|
||||
])),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -10028,7 +10023,6 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
|
||||
])),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -10077,7 +10071,6 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -10445,7 +10438,6 @@ async fn status_widget_and_approval_modal_snapshot() {
|
||||
])),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
|
||||
@@ -8371,7 +8371,6 @@ guardian_approval = true
|
||||
cwd: Some(PathBuf::from("/tmp/project")),
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
|
||||
@@ -318,7 +318,6 @@ mod tests {
|
||||
cwd: None,
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
|
||||
@@ -627,7 +627,6 @@ mod tests {
|
||||
cwd: Some(PathBuf::from("/tmp")),
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
|
||||
@@ -1317,11 +1317,6 @@ fn exec_approval_request_from_params(
|
||||
.collect()
|
||||
},
|
||||
),
|
||||
skill_metadata: params.skill_metadata.map(|metadata| {
|
||||
codex_protocol::approvals::ExecApprovalRequestSkillMetadata {
|
||||
path_to_skills_md: metadata.path_to_skills_md,
|
||||
}
|
||||
}),
|
||||
available_decisions: params.available_decisions.map(|decisions| {
|
||||
decisions
|
||||
.into_iter()
|
||||
|
||||
@@ -3479,7 +3479,6 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -3529,7 +3528,6 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() {
|
||||
cwd: Some(PathBuf::from("/tmp")),
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
@@ -3564,7 +3562,6 @@ async fn exec_approval_uses_approval_id_when_present() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
}),
|
||||
@@ -3606,7 +3603,6 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -3662,7 +3658,6 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -10680,7 +10675,6 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
|
||||
])),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -10742,7 +10736,6 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
|
||||
])),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -10791,7 +10784,6 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
@@ -11175,7 +11167,6 @@ async fn status_widget_and_approval_modal_snapshot() {
|
||||
])),
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: None,
|
||||
available_decisions: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user