Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion codex-rs/core/src/session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1060,9 +1060,10 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
) -> std::io::Result<crate::tools::sandboxing::ApprovalAction> {
Ok(crate::tools::sandboxing::ApprovalAction::Shell {
id: ctx.call_id.to_string(),
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
command: Vec::new(),
#[allow(deprecated)]
cwd: ctx.turn.cwd.clone(),
cwd: PathUri::from_abs_path(&ctx.turn.cwd),
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
Expand Down
121 changes: 119 additions & 2 deletions codex-rs/core/src/tools/approvals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::guardian::new_guardian_review_id;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian_with_reviewer;
use crate::hook_runtime::run_permission_request_hooks;
use crate::sandboxing::SandboxPermissions;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::tools::flat_tool_name;
Expand All @@ -18,10 +19,119 @@ use crate::tools::sandboxing::ToolRuntime;
use codex_config::types::ApprovalsReviewer;
use codex_hooks::PermissionRequestDecision;
use codex_otel::ToolDecisionSource;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::protocol::NetworkPolicyRuleAction;
use codex_protocol::protocol::ReviewDecision;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;

pub(super) type ApprovalAction = crate::guardian::GuardianApprovalRequest;
#[derive(Clone, Debug, PartialEq)]
pub(crate) enum ApprovalAction {
Shell {
id: String,
environment_id: String,
command: Vec<String>,
cwd: PathUri,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
justification: Option<String>,
},
ExecCommand {
id: String,
environment_id: String,
command: Vec<String>,
cwd: PathUri,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
justification: Option<String>,
tty: bool,
},
ApplyPatch {
id: String,
environment_id: String,
cwd: PathUri,
files: Vec<PathUri>,
patch: String,
},
}

impl ApprovalAction {
fn into_guardian_request(self) -> std::io::Result<crate::guardian::GuardianApprovalRequest> {
Ok(match self {
Self::Shell {
id,
environment_id,
command,
cwd,
sandbox_permissions,
additional_permissions,
justification,
} => crate::guardian::GuardianApprovalRequest::Shell {
id,
command,
cwd: guardian_cwd(&environment_id, cwd)?,
sandbox_permissions,
additional_permissions,
justification,
},
Self::ExecCommand {
id,
environment_id,
command,
cwd,
sandbox_permissions,
additional_permissions,
justification,
tty,
} => crate::guardian::GuardianApprovalRequest::ExecCommand {
id,
command,
cwd: guardian_cwd(&environment_id, cwd)?,
sandbox_permissions,
additional_permissions,
justification,
tty,
},
Self::ApplyPatch {
id,
environment_id,
cwd,
files,
patch,
} => crate::guardian::GuardianApprovalRequest::ApplyPatch {
id,
cwd: guardian_cwd(&environment_id, cwd)?,
files: files
.into_iter()
.map(|path| path.to_abs_path())
.collect::<std::io::Result<Vec<_>>>()?,
patch,
},
})
}
}

fn guardian_cwd(environment_id: &str, cwd: PathUri) -> std::io::Result<AbsolutePathBuf> {
match cwd.to_abs_path() {
Ok(cwd) => Ok(cwd),
Err(err) if environment_id != codex_exec_server::LOCAL_ENVIRONMENT_ID => Err(err),
Err(_) => {
let cwd_display = cwd.to_string();
let path = cwd.to_url().to_file_path().map_err(|()| {
std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!("local cwd URI `{cwd_display}` is not a host-native path"),
)
})?;
AbsolutePathBuf::from_absolute_path_checked(path).map_err(|err| {
std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!("local cwd URI `{cwd_display}` is not absolute: {err}"),
)
})
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(super) enum ApprovalReviewer {
Expand Down Expand Up @@ -113,7 +223,10 @@ where
let resolution = match reviewer {
ApprovalReviewer::Guardian => {
let review_id = new_guardian_review_id();
let action = match tool.approval_action(req, &ctx) {
let action = match tool
.approval_action(req, &ctx)
.and_then(ApprovalAction::into_guardian_request)
{
Ok(action) => action,
Err(err) => {
tracing::error!(%err, "failed to build automatic approval action");
Expand Down Expand Up @@ -211,3 +324,7 @@ fn record_resolution(
source,
);
}

#[cfg(all(test, unix))]
#[path = "approvals_tests.rs"]
mod tests;
22 changes: 22 additions & 0 deletions codex-rs/core/src/tools/approvals_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use super::*;
use pretty_assertions::assert_eq;

#[test]
fn guardian_cwd_preserves_drive_shaped_local_posix_path() {
let native_cwd = AbsolutePathBuf::try_from(std::path::PathBuf::from("/C:/workspace"))
.expect("drive-shaped POSIX path should be absolute");
let cwd = PathUri::from_abs_path(&native_cwd);

assert_eq!(
guardian_cwd(codex_exec_server::LOCAL_ENVIRONMENT_ID, cwd)
.expect("local cwd should retain the host path convention"),
native_cwd
);
}

#[test]
fn guardian_cwd_rejects_foreign_remote_path() {
let cwd = PathUri::parse("file:///C:/workspace").expect("valid Windows path URI");

assert!(guardian_cwd(codex_exec_server::REMOTE_ENVIRONMENT_ID, cwd).is_err());
}
23 changes: 7 additions & 16 deletions codex-rs/core/src/tools/runtimes/apply_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,14 @@ impl ApplyPatchRuntime {
&self.committed_delta
}

fn build_guardian_review_request(
req: &ApplyPatchRequest,
call_id: &str,
) -> std::io::Result<ApprovalAction> {
// TODO(anp): Remove this conversion once the guardian API supports PathUri.
let cwd = req.action.cwd.to_abs_path()?;
let files = req
.file_paths
.iter()
.map(PathUri::to_abs_path)
.collect::<std::io::Result<Vec<_>>>()?;
Ok(ApprovalAction::ApplyPatch {
fn build_approval_action(req: &ApplyPatchRequest, call_id: &str) -> ApprovalAction {
ApprovalAction::ApplyPatch {
id: call_id.to_string(),
cwd,
files,
environment_id: req.turn_environment.environment_id.clone(),
cwd: req.action.cwd.clone(),
files: req.file_paths.clone(),
patch: req.action.patch.clone(),
})
}
}

fn file_system_sandbox_context_for_attempt(
Expand Down Expand Up @@ -188,7 +179,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
req: &ApplyPatchRequest,
ctx: &ApprovalCtx<'_>,
) -> std::io::Result<ApprovalAction> {
ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id)
Ok(ApplyPatchRuntime::build_approval_action(req, ctx.call_id))
}

fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool {
Expand Down
27 changes: 10 additions & 17 deletions codex-rs/core/src/tools/runtimes/apply_patch_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,17 @@ fn wants_no_sandbox_approval_granular_respects_sandbox_flag() {
}

#[tokio::test]
async fn guardian_review_request_includes_patch_context() {
let path = std::env::temp_dir()
.join("guardian-apply-patch-test.txt")
.abs();
let action =
ApplyPatchAction::new_add_for_test(&PathUri::from_abs_path(&path), "hello".to_string());
let expected_cwd = action.cwd.to_abs_path().expect("native patch cwd");
async fn approval_action_preserves_patch_path_uris() {
let path = PathUri::parse("file:///C:/workspace/guardian-apply-patch-test.txt")
.expect("valid foreign path URI");
let action = ApplyPatchAction::new_add_for_test(&path, "hello".to_string());
let expected_cwd = action.cwd.clone();
let expected_patch = action.patch.clone();
let request = ApplyPatchRequest {
turn_environment: test_turn_environment(codex_exec_server::LOCAL_ENVIRONMENT_ID),
action,
file_paths: vec![PathUri::from_abs_path(&path)],
changes: HashMap::from([(
path.to_path_buf(),
FileChange::Add {
content: "hello".to_string(),
},
)]),
file_paths: vec![path.clone()],
changes: HashMap::new(),
exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
Expand All @@ -75,13 +68,13 @@ async fn guardian_review_request_includes_patch_context() {
permissions_preapproved: false,
};

let guardian_request = ApplyPatchRuntime::build_guardian_review_request(&request, "call-1")
.expect("native guardian request cwd");
let approval_action = ApplyPatchRuntime::build_approval_action(&request, "call-1");

assert_eq!(
guardian_request,
approval_action,
ApprovalAction::ApplyPatch {
id: "call-1".to_string(),
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
cwd: expected_cwd,
files: vec![path],
patch: expected_patch,
Expand Down
4 changes: 3 additions & 1 deletion codex-rs/core/src/tools/runtimes/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use codex_protocol::protocol::ReviewDecision;
use codex_sandboxing::SandboxablePreference;
use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use futures::future::BoxFuture;
use std::collections::HashMap;
use tokio_util::sync::CancellationToken;
Expand Down Expand Up @@ -183,8 +184,9 @@ impl Approvable<ShellRequest> for ShellRuntime {
) -> std::io::Result<ApprovalAction> {
Ok(ApprovalAction::Shell {
id: ctx.call_id.to_string(),
environment_id: req.turn_environment.environment_id.clone(),
command: req.command.clone(),
cwd: req.cwd.clone(),
cwd: PathUri::from_abs_path(&req.cwd),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
Expand Down
3 changes: 2 additions & 1 deletion codex-rs/core/src/tools/runtimes/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
) -> std::io::Result<ApprovalAction> {
Ok(ApprovalAction::ExecCommand {
id: ctx.call_id.to_string(),
environment_id: req.turn_environment.environment_id.clone(),
command: req.command.clone(),
cwd: req.cwd.to_abs_path()?,
cwd: req.cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/tools/sandboxing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub(crate) struct ApprovalCtx<'a> {
pub network_approval_context: Option<NetworkApprovalContext>,
}

pub(crate) type ApprovalAction = super::approvals::ApprovalAction;
pub(crate) use super::approvals::ApprovalAction;

#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct PermissionRequestPayload {
Expand Down
Loading