mirror of
https://github.com/openai/codex.git
synced 2026-10-21 16:38:24 +00:00
codex: fix CI failure on PR #32000
This commit is contained in:
@@ -233,17 +233,17 @@ pub enum GuardianApprovalRequestSource {
|
||||
pub enum GuardianReviewedAction {
|
||||
Shell {
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<String>>,
|
||||
},
|
||||
UnifiedExec {
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<String>>,
|
||||
tty: bool,
|
||||
},
|
||||
Execve {
|
||||
source: GuardianCommandSource,
|
||||
program: String,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<String>>,
|
||||
},
|
||||
ApplyPatch {},
|
||||
NetworkAccess {
|
||||
|
||||
@@ -25,6 +25,7 @@ use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use serde_json::Value;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -590,66 +591,97 @@ async fn handle_patch_approval(
|
||||
} = event;
|
||||
let approval_id = call_id.clone();
|
||||
let guardian_decision = if routes_approval_to_guardian(parent_ctx) {
|
||||
let files = changes
|
||||
.keys()
|
||||
.map(|path| {
|
||||
#[allow(deprecated)]
|
||||
parent_ctx.cwd.join(path)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let review_cancel = cancel_token.child_token();
|
||||
let patch = changes
|
||||
.iter()
|
||||
.map(|(path, change)| match change {
|
||||
codex_protocol::protocol::FileChange::Add { content } => {
|
||||
format!("*** Add File: {}\n{}", path.display(), content)
|
||||
}
|
||||
codex_protocol::protocol::FileChange::Delete { content } => {
|
||||
format!("*** Delete File: {}\n{}", path.display(), content)
|
||||
}
|
||||
codex_protocol::protocol::FileChange::Update {
|
||||
unified_diff,
|
||||
move_path,
|
||||
} => {
|
||||
if let Some(move_path) = move_path {
|
||||
format!(
|
||||
"*** Update File: {}\n*** Move to: {}\n{}",
|
||||
path.display(),
|
||||
move_path.display(),
|
||||
unified_diff
|
||||
match parent_ctx.environments.primary() {
|
||||
Some(environment) => {
|
||||
let cwd = environment.cwd().clone();
|
||||
let files = changes
|
||||
.iter()
|
||||
.flat_map(|(path, change)| {
|
||||
std::iter::once(path).chain(match change {
|
||||
codex_protocol::protocol::FileChange::Update {
|
||||
move_path: Some(move_path),
|
||||
..
|
||||
} => Some(move_path),
|
||||
codex_protocol::protocol::FileChange::Add { .. }
|
||||
| codex_protocol::protocol::FileChange::Delete { .. }
|
||||
| codex_protocol::protocol::FileChange::Update {
|
||||
move_path: None,
|
||||
..
|
||||
} => None,
|
||||
})
|
||||
})
|
||||
.map(|path| cwd.join(path.to_string_lossy().as_ref()))
|
||||
.collect::<Result<Vec<_>, _>>();
|
||||
match files {
|
||||
Ok(files) => {
|
||||
let review_cancel = cancel_token.child_token();
|
||||
let patch = changes
|
||||
.iter()
|
||||
.map(|(path, change)| match change {
|
||||
codex_protocol::protocol::FileChange::Add { content } => {
|
||||
format!("*** Add File: {}\n{}", path.display(), content)
|
||||
}
|
||||
codex_protocol::protocol::FileChange::Delete { content } => {
|
||||
format!("*** Delete File: {}\n{}", path.display(), content)
|
||||
}
|
||||
codex_protocol::protocol::FileChange::Update {
|
||||
unified_diff,
|
||||
move_path,
|
||||
} => {
|
||||
if let Some(move_path) = move_path {
|
||||
format!(
|
||||
"*** Update File: {}\n*** Move to: {}\n{}",
|
||||
path.display(),
|
||||
move_path.display(),
|
||||
unified_diff
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"*** Update File: {}\n{}",
|
||||
path.display(),
|
||||
unified_diff
|
||||
)
|
||||
}
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
let review_rx = spawn_approval_request_review(
|
||||
Arc::clone(parent_session),
|
||||
Arc::clone(parent_ctx),
|
||||
new_guardian_review_id(),
|
||||
GuardianApprovalRequest::ApplyPatch {
|
||||
id: approval_id.clone(),
|
||||
cwd,
|
||||
files,
|
||||
patch,
|
||||
},
|
||||
reason.clone(),
|
||||
GuardianApprovalRequestSource::DelegatedSubagent,
|
||||
review_cancel.clone(),
|
||||
);
|
||||
Some(
|
||||
await_approval_with_cancel(
|
||||
async move { review_rx.await.unwrap_or_default() },
|
||||
parent_session,
|
||||
&approval_id,
|
||||
cancel_token,
|
||||
Some(&review_cancel),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
} else {
|
||||
format!("*** Update File: {}\n{}", path.display(), unified_diff)
|
||||
}
|
||||
Err(error) => {
|
||||
tracing::warn!(%error, %cwd, "failed to resolve delegated patch path");
|
||||
Some(ReviewDecision::Abort)
|
||||
}
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
let review_rx = spawn_approval_request_review(
|
||||
Arc::clone(parent_session),
|
||||
Arc::clone(parent_ctx),
|
||||
new_guardian_review_id(),
|
||||
GuardianApprovalRequest::ApplyPatch {
|
||||
id: approval_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: parent_ctx.cwd.clone(),
|
||||
files,
|
||||
patch,
|
||||
},
|
||||
reason.clone(),
|
||||
GuardianApprovalRequestSource::DelegatedSubagent,
|
||||
review_cancel.clone(),
|
||||
);
|
||||
Some(
|
||||
await_approval_with_cancel(
|
||||
async move { review_rx.await.unwrap_or_default() },
|
||||
parent_session,
|
||||
&approval_id,
|
||||
cancel_token,
|
||||
Some(&review_cancel),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
}
|
||||
None => {
|
||||
tracing::warn!("delegated patch approval has no primary environment");
|
||||
Some(ReviewDecision::Abort)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -806,10 +838,18 @@ async fn handle_request_permissions(
|
||||
reason: event.reason,
|
||||
permissions: event.permissions,
|
||||
};
|
||||
let cwd = event.cwd.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
parent_ctx.cwd.clone()
|
||||
});
|
||||
let cwd = event
|
||||
.cwd
|
||||
.or_else(|| {
|
||||
parent_ctx
|
||||
.environments
|
||||
.primary()
|
||||
.map(|environment| environment.cwd().clone())
|
||||
})
|
||||
.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
PathUri::from_abs_path(&parent_ctx.cwd)
|
||||
});
|
||||
let response_fut = parent_session.request_permissions_for_cwd(
|
||||
parent_ctx,
|
||||
call_id.clone(),
|
||||
@@ -859,9 +899,9 @@ async fn await_request_permissions_with_cancel<F>(
|
||||
parent_session: &Session,
|
||||
call_id: &str,
|
||||
cancel_token: &CancellationToken,
|
||||
) -> RequestPermissionsResponse
|
||||
) -> RequestPermissionsResponse<PathUri>
|
||||
where
|
||||
F: core::future::Future<Output = Option<RequestPermissionsResponse>>,
|
||||
F: core::future::Future<Output = Option<RequestPermissionsResponse<PathUri>>>,
|
||||
{
|
||||
tokio::select! {
|
||||
biased;
|
||||
|
||||
@@ -48,6 +48,7 @@ use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use crate::context_manager::is_user_turn_boundary;
|
||||
use codex_protocol::dynamic_tools::DynamicToolResponse;
|
||||
@@ -423,7 +424,7 @@ pub async fn request_user_input_response(
|
||||
pub async fn request_permissions_response(
|
||||
sess: &Arc<Session>,
|
||||
id: String,
|
||||
response: RequestPermissionsResponse,
|
||||
response: RequestPermissionsResponse<PathUri>,
|
||||
) {
|
||||
sess.notify_request_permissions_response(&id, response)
|
||||
.await;
|
||||
|
||||
@@ -2615,10 +2615,9 @@ impl Session {
|
||||
}
|
||||
PermissionGrantScope::Session => {
|
||||
let mut state = self.state.lock().await;
|
||||
state.record_granted_permissions(
|
||||
environment_id,
|
||||
response.permissions.clone().into(),
|
||||
);
|
||||
let permissions: AdditionalPermissionProfile<PathUri> =
|
||||
response.permissions.clone().into();
|
||||
state.record_granted_permissions(environment_id, permissions);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,8 +12,7 @@ use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::protocol::MultiAgentVersion;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::effective_network_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile_for_uri;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::FutureExt;
|
||||
use futures::future::BoxFuture;
|
||||
@@ -298,33 +297,31 @@ impl TurnContext {
|
||||
|
||||
pub(crate) fn file_system_sandbox_context(
|
||||
&self,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
cwd: &PathUri,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
environment: &TurnEnvironment,
|
||||
) -> FileSystemSandboxContext {
|
||||
let (base_file_system_sandbox_policy, base_network_sandbox_policy) =
|
||||
self.permission_profile.to_runtime_permissions();
|
||||
let file_system_sandbox_policy = effective_file_system_sandbox_policy(
|
||||
&base_file_system_sandbox_policy,
|
||||
let environment_is_remote = environment.environment.is_remote();
|
||||
let permission_profile = if environment_is_remote {
|
||||
self.config.permissions.permission_profile()
|
||||
} else {
|
||||
&self.permission_profile
|
||||
};
|
||||
let permissions = effective_permission_profile_for_uri(
|
||||
permission_profile,
|
||||
additional_permissions.as_ref(),
|
||||
);
|
||||
let network_sandbox_policy = effective_network_sandbox_policy(
|
||||
base_network_sandbox_policy,
|
||||
additional_permissions.as_ref(),
|
||||
);
|
||||
let permissions = PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
self.permission_profile.enforcement(),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
FileSystemSandboxContext {
|
||||
permissions: permissions.into(),
|
||||
cwd: Some(cwd.clone()),
|
||||
workspace_roots: self
|
||||
.config
|
||||
.effective_workspace_roots()
|
||||
.iter()
|
||||
.map(PathUri::from_abs_path)
|
||||
.collect(),
|
||||
permissions,
|
||||
cwd: Some(environment.cwd().clone()),
|
||||
workspace_roots: if environment_is_remote {
|
||||
Vec::new()
|
||||
} else {
|
||||
self.config
|
||||
.effective_workspace_roots()
|
||||
.iter()
|
||||
.map(PathUri::from_abs_path)
|
||||
.collect()
|
||||
},
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: self
|
||||
.config
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::collections::VecDeque;
|
||||
@@ -41,7 +42,7 @@ pub(crate) struct SessionState {
|
||||
pub(crate) current_time_reminder: CurrentTimeReminderState,
|
||||
pub(crate) active_connector_selection: HashSet<String>,
|
||||
pub(crate) pending_session_start_sources: VecDeque<codex_hooks::SessionStartSource>,
|
||||
granted_permissions_by_environment_id: HashMap<String, AdditionalPermissionProfile>,
|
||||
granted_permissions_by_environment_id: HashMap<String, AdditionalPermissionProfile<PathUri>>,
|
||||
next_turn_is_first: bool,
|
||||
}
|
||||
|
||||
@@ -288,7 +289,7 @@ impl SessionState {
|
||||
pub(crate) fn record_granted_permissions(
|
||||
&mut self,
|
||||
environment_id: &str,
|
||||
permissions: AdditionalPermissionProfile,
|
||||
permissions: AdditionalPermissionProfile<PathUri>,
|
||||
) {
|
||||
let granted_permissions = merge_permission_profiles(
|
||||
self.granted_permissions_by_environment_id
|
||||
@@ -304,7 +305,7 @@ impl SessionState {
|
||||
pub(crate) fn granted_permissions(
|
||||
&self,
|
||||
environment_id: &str,
|
||||
) -> Option<AdditionalPermissionProfile> {
|
||||
) -> Option<AdditionalPermissionProfile<PathUri>> {
|
||||
self.granted_permissions_by_environment_id
|
||||
.get(environment_id)
|
||||
.cloned()
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use std::collections::BTreeSet;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -46,12 +45,10 @@ use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::PatchApplyUpdatedEvent;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions_for_uri;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathConvention;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500);
|
||||
@@ -219,25 +216,37 @@ fn file_paths_for_action(action: &ApplyPatchAction) -> Vec<PathUri> {
|
||||
}
|
||||
|
||||
fn write_permissions_for_paths(
|
||||
file_paths: &[AbsolutePathBuf],
|
||||
file_paths: &[PathUri],
|
||||
file_system_sandbox_policy: &codex_protocol::permissions::FileSystemSandboxPolicy,
|
||||
cwd: &AbsolutePathBuf,
|
||||
) -> Option<AdditionalPermissionProfile> {
|
||||
let write_paths = file_paths
|
||||
.iter()
|
||||
.map(|path| {
|
||||
path.parent()
|
||||
.unwrap_or_else(|| path.clone())
|
||||
.into_path_buf()
|
||||
})
|
||||
.filter(|path| {
|
||||
!file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd.as_path())
|
||||
})
|
||||
.collect::<BTreeSet<_>>()
|
||||
.into_iter()
|
||||
.map(AbsolutePathBuf::from_absolute_path)
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.ok()?;
|
||||
cwd: &PathUri,
|
||||
) -> Option<AdditionalPermissionProfile<PathUri>> {
|
||||
let native_cwd = if cwd.infer_path_convention() == Some(PathConvention::native()) {
|
||||
cwd.to_abs_path().ok()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let mut write_paths = Vec::new();
|
||||
for file_path in file_paths {
|
||||
let already_writable = native_cwd.as_ref().is_some_and(|native_cwd| {
|
||||
file_path.infer_path_convention() == Some(PathConvention::native())
|
||||
&& file_path
|
||||
.to_abs_path()
|
||||
.ok()
|
||||
.is_some_and(|native_file_path| {
|
||||
file_system_sandbox_policy.can_write_path_with_cwd(
|
||||
native_file_path.as_path(),
|
||||
native_cwd.as_path(),
|
||||
)
|
||||
})
|
||||
});
|
||||
if already_writable {
|
||||
continue;
|
||||
}
|
||||
let write_path = file_path.parent().unwrap_or_else(|| file_path.clone());
|
||||
if !write_paths.contains(&write_path) {
|
||||
write_paths.push(write_path);
|
||||
}
|
||||
}
|
||||
|
||||
let permissions = (!write_paths.is_empty()).then_some(AdditionalPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
@@ -247,7 +256,7 @@ fn write_permissions_for_paths(
|
||||
..Default::default()
|
||||
})?;
|
||||
|
||||
normalize_additional_permissions(permissions).ok()
|
||||
normalize_additional_permissions_for_uri(permissions).ok()
|
||||
}
|
||||
|
||||
/// Extracts the raw patch text used as the command-shaped hook input for apply_patch.
|
||||
@@ -264,65 +273,26 @@ async fn effective_patch_permissions(
|
||||
environment_id: &str,
|
||||
action: &ApplyPatchAction,
|
||||
cwd: &PathUri,
|
||||
) -> std::io::Result<(
|
||||
Vec<PathUri>,
|
||||
crate::tools::handlers::EffectiveAdditionalPermissions,
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy,
|
||||
)> {
|
||||
let file_paths = file_paths_for_action(action);
|
||||
let native_cwd = cwd.to_abs_path()?;
|
||||
let granted_permissions = merge_permission_profiles(
|
||||
session
|
||||
.granted_session_permissions(environment_id)
|
||||
.await
|
||||
.as_ref(),
|
||||
session
|
||||
.granted_turn_permissions(environment_id)
|
||||
.await
|
||||
.as_ref(),
|
||||
);
|
||||
let base_file_system_sandbox_policy = turn.file_system_sandbox_policy();
|
||||
let file_system_sandbox_policy = effective_file_system_sandbox_policy(
|
||||
&base_file_system_sandbox_policy,
|
||||
granted_permissions.as_ref(),
|
||||
);
|
||||
let native_file_paths = file_paths
|
||||
.iter()
|
||||
.map(PathUri::to_abs_path)
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
session,
|
||||
environment_id,
|
||||
native_cwd.as_path(),
|
||||
crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
write_permissions_for_paths(&native_file_paths, &file_system_sandbox_policy, &native_cwd),
|
||||
)
|
||||
.await;
|
||||
|
||||
Ok((
|
||||
file_paths,
|
||||
effective_additional_permissions,
|
||||
file_system_sandbox_policy,
|
||||
))
|
||||
}
|
||||
|
||||
fn patch_permissions_without_path_matching(
|
||||
action: &ApplyPatchAction,
|
||||
) -> (
|
||||
Vec<PathUri>,
|
||||
crate::tools::handlers::EffectiveAdditionalPermissions,
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy,
|
||||
) {
|
||||
// TODO(anp): Make permission matching operate on PathUri. Until then, foreign paths skip
|
||||
// permission matching; a managed turn still fails closed at the platform sandbox boundary.
|
||||
let file_paths = file_paths_for_action(action);
|
||||
let file_system_sandbox_policy = turn.file_system_sandbox_policy();
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
session,
|
||||
environment_id,
|
||||
cwd,
|
||||
crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, cwd),
|
||||
)
|
||||
.await;
|
||||
|
||||
(
|
||||
file_paths_for_action(action),
|
||||
crate::tools::handlers::EffectiveAdditionalPermissions {
|
||||
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
additional_permissions: None,
|
||||
permissions_preapproved: false,
|
||||
},
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted(),
|
||||
file_paths,
|
||||
effective_additional_permissions,
|
||||
file_system_sandbox_policy,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -383,10 +353,8 @@ impl ApplyPatchHandler {
|
||||
));
|
||||
};
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
let sandbox = turn.file_system_sandbox_context(
|
||||
/*additional_permissions*/ None,
|
||||
turn_environment.cwd(),
|
||||
);
|
||||
let sandbox = turn
|
||||
.file_system_sandbox_context(/*additional_permissions*/ None, turn_environment);
|
||||
match codex_apply_patch::verify_apply_patch_args(
|
||||
args,
|
||||
turn_environment.cwd(),
|
||||
@@ -404,8 +372,7 @@ impl ApplyPatchHandler {
|
||||
&changes,
|
||||
turn_environment.cwd(),
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|_| patch_permissions_without_path_matching(&changes));
|
||||
.await;
|
||||
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
|
||||
.await
|
||||
{
|
||||
@@ -554,7 +521,8 @@ pub(crate) async fn intercept_apply_patch(
|
||||
call_id: &str,
|
||||
tool_name: &str,
|
||||
) -> Result<Option<FunctionToolOutput>, FunctionCallError> {
|
||||
let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, cwd);
|
||||
let sandbox =
|
||||
turn.file_system_sandbox_context(/*additional_permissions*/ None, &turn_environment);
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, Some(&sandbox))
|
||||
.await
|
||||
{
|
||||
@@ -567,8 +535,7 @@ pub(crate) async fn intercept_apply_patch(
|
||||
&changes,
|
||||
cwd,
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|_| patch_permissions_without_path_matching(&changes));
|
||||
.await;
|
||||
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -117,15 +117,13 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
let mut environments =
|
||||
Vec::with_capacity(invocation.step_context.environments.turn_environments.len());
|
||||
for environment in &invocation.step_context.environments.turn_environments {
|
||||
// TODO(anp): Migrate extension ToolEnvironment and granted-permission lookup to PathUri
|
||||
// so extensions can receive foreign environment cwd values.
|
||||
let Ok(native_cwd) = environment.cwd().to_abs_path() else {
|
||||
continue;
|
||||
};
|
||||
let additional_permissions = apply_granted_turn_permissions(
|
||||
invocation.session.as_ref(),
|
||||
&environment.environment_id,
|
||||
native_cwd.as_path(),
|
||||
environment.cwd(),
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
)
|
||||
@@ -133,7 +131,7 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
.additional_permissions;
|
||||
let file_system_sandbox_context = invocation
|
||||
.turn
|
||||
.file_system_sandbox_context(additional_permissions, environment.cwd());
|
||||
.file_system_sandbox_context(additional_permissions, environment);
|
||||
environments.push(ToolEnvironment {
|
||||
environment_id: environment.environment_id.clone(),
|
||||
cwd: native_cwd,
|
||||
|
||||
@@ -37,15 +37,15 @@ mod view_image;
|
||||
pub(crate) mod view_image_spec;
|
||||
mod wait_for_environment;
|
||||
|
||||
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
|
||||
use codex_sandboxing::policy_transforms::intersect_permission_profiles_for_uri;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions_for_uri;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Map;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
@@ -181,10 +181,10 @@ pub(crate) fn normalize_and_validate_additional_permissions(
|
||||
additional_permissions_allowed: bool,
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
permissions_preapproved: bool,
|
||||
_cwd: &Path,
|
||||
) -> Result<Option<AdditionalPermissionProfile>, String> {
|
||||
_cwd: &PathUri,
|
||||
) -> Result<Option<AdditionalPermissionProfile<PathUri>>, String> {
|
||||
let uses_additional_permissions = matches!(
|
||||
sandbox_permissions,
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
@@ -212,7 +212,7 @@ pub(crate) fn normalize_and_validate_additional_permissions(
|
||||
.to_string(),
|
||||
);
|
||||
};
|
||||
let normalized = normalize_additional_permissions(additional_permissions)?;
|
||||
let normalized = normalize_additional_permissions_for_uri(additional_permissions)?;
|
||||
if normalized.is_empty() {
|
||||
return Err(
|
||||
"`additional_permissions` must include at least one requested permission in `network` or `file_system`"
|
||||
@@ -234,15 +234,15 @@ pub(crate) fn normalize_and_validate_additional_permissions(
|
||||
|
||||
pub(super) struct EffectiveAdditionalPermissions {
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
pub permissions_preapproved: bool,
|
||||
}
|
||||
|
||||
pub(super) fn implicit_granted_permissions(
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<&AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<&AdditionalPermissionProfile<PathUri>>,
|
||||
effective_additional_permissions: &EffectiveAdditionalPermissions,
|
||||
) -> Option<AdditionalPermissionProfile> {
|
||||
) -> Option<AdditionalPermissionProfile<PathUri>> {
|
||||
if !sandbox_permissions.uses_additional_permissions()
|
||||
&& !matches!(sandbox_permissions, SandboxPermissions::RequireEscalated)
|
||||
&& additional_permissions.is_none()
|
||||
@@ -258,9 +258,9 @@ pub(super) fn implicit_granted_permissions(
|
||||
pub(super) async fn apply_granted_turn_permissions(
|
||||
session: &Session,
|
||||
environment_id: &str,
|
||||
cwd: &Path,
|
||||
cwd: &PathUri,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
) -> EffectiveAdditionalPermissions {
|
||||
if matches!(sandbox_permissions, SandboxPermissions::RequireEscalated) {
|
||||
return EffectiveAdditionalPermissions {
|
||||
@@ -302,16 +302,16 @@ pub(super) async fn apply_granted_turn_permissions(
|
||||
}
|
||||
|
||||
fn permissions_are_preapproved(
|
||||
effective_permissions: &AdditionalPermissionProfile,
|
||||
granted_permissions: AdditionalPermissionProfile,
|
||||
cwd: &Path,
|
||||
effective_permissions: &AdditionalPermissionProfile<PathUri>,
|
||||
granted_permissions: AdditionalPermissionProfile<PathUri>,
|
||||
cwd: &PathUri,
|
||||
) -> bool {
|
||||
let materialized_effective_permissions = intersect_permission_profiles(
|
||||
let materialized_effective_permissions = intersect_permission_profiles_for_uri(
|
||||
effective_permissions.clone(),
|
||||
effective_permissions.clone(),
|
||||
cwd,
|
||||
);
|
||||
intersect_permission_profiles(effective_permissions.clone(), granted_permissions, cwd)
|
||||
intersect_permission_profiles_for_uri(effective_permissions.clone(), granted_permissions, cwd)
|
||||
== materialized_effective_permissions
|
||||
}
|
||||
|
||||
@@ -331,13 +331,13 @@ mod tests {
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
|
||||
use codex_sandboxing::policy_transforms::intersect_permission_profiles_for_uri;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn network_permissions() -> AdditionalPermissionProfile {
|
||||
fn network_permissions() -> AdditionalPermissionProfile<PathUri> {
|
||||
AdditionalPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
@@ -346,12 +346,12 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn file_system_permissions(path: &std::path::Path) -> AdditionalPermissionProfile {
|
||||
fn file_system_permissions(path: &std::path::Path) -> AdditionalPermissionProfile<PathUri> {
|
||||
AdditionalPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![
|
||||
AbsolutePathBuf::from_absolute_path(path).expect("absolute path"),
|
||||
PathUri::from_host_native_path(path).expect("absolute path URI"),
|
||||
]),
|
||||
)),
|
||||
..Default::default()
|
||||
@@ -362,6 +362,7 @@ mod tests {
|
||||
fn preapproved_permissions_work_when_request_permissions_tool_is_enabled_without_exec_permission_approvals_feature()
|
||||
{
|
||||
let cwd = tempdir().expect("tempdir");
|
||||
let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
|
||||
|
||||
let normalized = normalize_and_validate_additional_permissions(
|
||||
/*additional_permissions_allowed*/ false,
|
||||
@@ -375,7 +376,7 @@ mod tests {
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
Some(network_permissions()),
|
||||
/*permissions_preapproved*/ true,
|
||||
cwd.path(),
|
||||
&cwd_uri,
|
||||
)
|
||||
.expect("preapproved permissions should be allowed");
|
||||
|
||||
@@ -385,6 +386,7 @@ mod tests {
|
||||
#[test]
|
||||
fn fresh_additional_permissions_still_require_exec_permission_approvals_feature() {
|
||||
let cwd = tempdir().expect("tempdir");
|
||||
let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
|
||||
|
||||
let err = normalize_and_validate_additional_permissions(
|
||||
/*additional_permissions_allowed*/ false,
|
||||
@@ -392,7 +394,7 @@ mod tests {
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
Some(network_permissions()),
|
||||
/*permissions_preapproved*/ false,
|
||||
cwd.path(),
|
||||
&cwd_uri,
|
||||
)
|
||||
.expect_err("fresh inline permission requests should remain disabled");
|
||||
|
||||
@@ -439,6 +441,7 @@ mod tests {
|
||||
#[test]
|
||||
fn relative_deny_glob_grants_remain_preapproved_after_materialization() {
|
||||
let cwd = tempdir().expect("tempdir");
|
||||
let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
|
||||
let requested_permissions = AdditionalPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
entries: vec![
|
||||
@@ -459,10 +462,10 @@ mod tests {
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let stored_grant = intersect_permission_profiles(
|
||||
let stored_grant = intersect_permission_profiles_for_uri(
|
||||
requested_permissions.clone(),
|
||||
requested_permissions.clone(),
|
||||
cwd.path(),
|
||||
&cwd_uri,
|
||||
);
|
||||
let effective_permissions =
|
||||
merge_permission_profiles(Some(&requested_permissions), Some(&stored_grant))
|
||||
@@ -471,7 +474,7 @@ mod tests {
|
||||
assert!(permissions_are_preapproved(
|
||||
&effective_permissions,
|
||||
stored_grant,
|
||||
cwd.path(),
|
||||
&cwd_uri,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,13 +87,14 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
.clone();
|
||||
let exec_permission_approvals_enabled =
|
||||
session.features().enabled(Feature::ExecPermissionApprovals);
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
let requested_additional_permissions = additional_permissions.clone().map(Into::into);
|
||||
let permission_cwd = PathUri::from_abs_path(&exec_params.cwd);
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
session.as_ref(),
|
||||
&turn_environment.environment_id,
|
||||
exec_params.cwd.as_path(),
|
||||
&permission_cwd,
|
||||
exec_params.sandbox_permissions,
|
||||
additional_permissions,
|
||||
additional_permissions.map(Into::into),
|
||||
)
|
||||
.await;
|
||||
let additional_permissions_allowed = exec_permission_approvals_enabled
|
||||
@@ -112,7 +113,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
effective_additional_permissions.sandbox_permissions,
|
||||
effective_additional_permissions.additional_permissions,
|
||||
effective_additional_permissions.permissions_preapproved,
|
||||
&exec_params.cwd,
|
||||
&permission_cwd,
|
||||
)
|
||||
},
|
||||
|permissions| Ok(Some(permissions)),
|
||||
@@ -138,10 +139,9 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
}
|
||||
|
||||
// Intercept apply_patch if present.
|
||||
let apply_patch_cwd = PathUri::from_abs_path(&exec_params.cwd);
|
||||
if let Some(output) = intercept_apply_patch(
|
||||
&exec_params.command,
|
||||
&apply_patch_cwd,
|
||||
&permission_cwd,
|
||||
fs.as_ref(),
|
||||
turn_environment.clone(),
|
||||
session.clone(),
|
||||
|
||||
@@ -40,7 +40,7 @@ pub(crate) struct ExecCommandArgs {
|
||||
#[serde(default)]
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
#[serde(default)]
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<String>>,
|
||||
#[serde(default)]
|
||||
justification: Option<String>,
|
||||
#[serde(default)]
|
||||
|
||||
@@ -12,7 +12,6 @@ use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
use crate::tools::handlers::implicit_granted_permissions;
|
||||
use crate::tools::handlers::normalize_and_validate_additional_permissions;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::handlers::parse_arguments_with_base_path;
|
||||
use crate::tools::handlers::resolve_tool_environment;
|
||||
use crate::tools::handlers::rewrite_function_string_argument;
|
||||
use crate::tools::handlers::updated_hook_command;
|
||||
@@ -29,9 +28,7 @@ use crate::unified_exec::generate_chunk_id;
|
||||
use codex_features::Feature;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::policy_transforms::resolve_additional_permission_paths;
|
||||
use codex_shell_command::shell_detect::detect_shell_type;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
@@ -151,52 +148,23 @@ impl ExecCommandHandler {
|
||||
let environment = Arc::clone(&turn_environment.environment);
|
||||
let fs = environment.get_filesystem();
|
||||
|
||||
// A foreign cwd cannot seed the AbsolutePathBufGuard used to resolve relative paths in the
|
||||
// permissions config below. Consult the configured platform-sandbox requirement before
|
||||
// deciding whether parsing may continue without that base path.
|
||||
let sandbox = SandboxManager::new().select_initial(
|
||||
&turn.file_system_sandbox_policy(),
|
||||
turn.network_sandbox_policy(),
|
||||
SandboxablePreference::Auto,
|
||||
turn.windows_sandbox_level,
|
||||
turn.network.is_some(),
|
||||
);
|
||||
// `to_abs_path()` alone cannot identify foreign drive paths: `file:///C:/repo` is
|
||||
// representable as `/C:/repo` on POSIX. Require the inferred convention to match too.
|
||||
let cwd_uses_native_convention =
|
||||
cwd.infer_path_convention() == Some(PathConvention::native());
|
||||
// TODO(anp): Remove this parsing split once sandboxing supports foreign paths.
|
||||
let native_cwd = match cwd.to_abs_path() {
|
||||
Ok(cwd) if cwd_uses_native_convention => Some(cwd),
|
||||
_ if sandbox == SandboxType::None => None,
|
||||
Err(err) => return Err(FunctionCallError::RespondToModel(err.to_string())),
|
||||
Ok(_) => {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"path URI `{cwd}` does not use the host's native {} path convention",
|
||||
PathConvention::native()
|
||||
)));
|
||||
}
|
||||
};
|
||||
let mut args: ExecCommandArgs = match native_cwd.as_ref() {
|
||||
Some(native_cwd) => {
|
||||
// The base path only resolves paths nested in the permissions config types.
|
||||
parse_arguments_with_base_path(&arguments, native_cwd)?
|
||||
}
|
||||
None => {
|
||||
// Parsing without a base only skips relative-path resolution inside the
|
||||
// permissions config. That is safe only for a truly unsandboxed attempt;
|
||||
// sandboxed attempts fall through and return the conversion error below.
|
||||
parse_arguments(&arguments)?
|
||||
}
|
||||
};
|
||||
let mut args: ExecCommandArgs = parse_arguments(&arguments)?;
|
||||
let additional_permissions = args
|
||||
.additional_permissions
|
||||
.take()
|
||||
.map(|permissions| resolve_additional_permission_paths(permissions, &cwd))
|
||||
.transpose()
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
let hook_command = args.cmd.clone();
|
||||
// TODO(anp) wire PathUri through implicit skills instead of skipping on foreign paths
|
||||
if let Some(native_cwd) = native_cwd.as_ref() {
|
||||
// TODO(anp): Wire PathUri through implicit skills instead of skipping foreign paths.
|
||||
if cwd.infer_path_convention() == Some(PathConvention::native())
|
||||
&& let Ok(native_cwd) = cwd.to_abs_path()
|
||||
{
|
||||
maybe_emit_implicit_skill_invocation(
|
||||
session.as_ref(),
|
||||
context.turn.as_ref(),
|
||||
&hook_command,
|
||||
native_cwd,
|
||||
&native_cwd,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -245,7 +213,6 @@ impl ExecCommandHandler {
|
||||
yield_time_ms,
|
||||
max_output_tokens,
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
justification,
|
||||
prefix_rule,
|
||||
..
|
||||
@@ -254,12 +221,10 @@ impl ExecCommandHandler {
|
||||
let exec_permission_approvals_enabled =
|
||||
session.features().enabled(Feature::ExecPermissionApprovals);
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
// TODO(anp): Make permission matching operate on PathUri for remote environments.
|
||||
let permission_cwd = native_cwd.as_ref().unwrap_or(&turn.config.cwd);
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
context.session.as_ref(),
|
||||
&turn_environment.environment_id,
|
||||
permission_cwd.as_path(),
|
||||
&cwd,
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
)
|
||||
@@ -299,7 +264,7 @@ impl ExecCommandHandler {
|
||||
effective_additional_permissions.sandbox_permissions,
|
||||
effective_additional_permissions.additional_permissions,
|
||||
effective_additional_permissions.permissions_preapproved,
|
||||
permission_cwd,
|
||||
&cwd,
|
||||
)
|
||||
},
|
||||
|permissions| Ok(Some(permissions)),
|
||||
|
||||
@@ -147,10 +147,8 @@ impl ViewImageHandler {
|
||||
))
|
||||
})?;
|
||||
let model_visible_path = path_uri.inferred_native_path_string();
|
||||
let sandbox = turn.file_system_sandbox_context(
|
||||
/*additional_permissions*/ None,
|
||||
turn_environment.cwd(),
|
||||
);
|
||||
let sandbox = turn
|
||||
.file_system_sandbox_context(/*additional_permissions*/ None, turn_environment);
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
|
||||
let metadata = fs
|
||||
|
||||
@@ -28,6 +28,7 @@ use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use indexmap::IndexMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
@@ -652,19 +653,23 @@ impl NetworkApprovalService {
|
||||
.await
|
||||
} else {
|
||||
let available_decisions = None;
|
||||
let cwd = if let Some(owner_call) = owner_call.as_ref() {
|
||||
owner_call.trigger.cwd.clone()
|
||||
let cwd_uri = if let Some(owner_call) = owner_call.as_ref() {
|
||||
Some(owner_call.trigger.cwd.clone())
|
||||
} else {
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
turn_context.cwd.clone()
|
||||
})
|
||||
.map(|environment| environment.cwd().clone())
|
||||
};
|
||||
let cwd = match cwd_uri {
|
||||
Some(cwd) => cwd,
|
||||
None =>
|
||||
{
|
||||
#[allow(deprecated)]
|
||||
PathUri::from_abs_path(&turn_context.cwd)
|
||||
}
|
||||
};
|
||||
session
|
||||
.request_command_approval(
|
||||
|
||||
@@ -30,7 +30,7 @@ use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile_for_uri;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::future::BoxFuture;
|
||||
use std::path::PathBuf;
|
||||
@@ -49,7 +49,7 @@ pub struct ApplyPatchRequest {
|
||||
pub file_paths: Vec<PathUri>,
|
||||
pub changes: std::collections::HashMap<PathBuf, FileChange>,
|
||||
pub exec_approval_requirement: ExecApprovalRequirement,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
pub permissions_preapproved: bool,
|
||||
}
|
||||
|
||||
@@ -77,17 +77,10 @@ impl ApplyPatchRuntime {
|
||||
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 {
|
||||
id: call_id.to_string(),
|
||||
cwd,
|
||||
files,
|
||||
cwd: req.action.cwd.clone(),
|
||||
files: req.file_paths.clone(),
|
||||
patch: req.action.patch.clone(),
|
||||
})
|
||||
}
|
||||
@@ -100,16 +93,28 @@ impl ApplyPatchRuntime {
|
||||
return None;
|
||||
}
|
||||
|
||||
let permissions =
|
||||
effective_permission_profile(attempt.permissions, req.additional_permissions.as_ref());
|
||||
let environment_is_remote = req.turn_environment.environment.is_remote();
|
||||
let permission_profile = if environment_is_remote {
|
||||
attempt.exec_server_permissions
|
||||
} else {
|
||||
attempt.permissions
|
||||
};
|
||||
let permissions = effective_permission_profile_for_uri(
|
||||
permission_profile,
|
||||
req.additional_permissions.as_ref(),
|
||||
);
|
||||
Some(FileSystemSandboxContext {
|
||||
permissions: permissions.into(),
|
||||
permissions,
|
||||
cwd: Some(attempt.sandbox_cwd.clone()),
|
||||
workspace_roots: attempt
|
||||
.workspace_roots
|
||||
.iter()
|
||||
.map(PathUri::from_abs_path)
|
||||
.collect(),
|
||||
workspace_roots: if environment_is_remote {
|
||||
Vec::new()
|
||||
} else {
|
||||
attempt
|
||||
.workspace_roots
|
||||
.iter()
|
||||
.map(PathUri::from_abs_path)
|
||||
.collect()
|
||||
},
|
||||
windows_sandbox_level: attempt.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: attempt.windows_sandbox_private_desktop,
|
||||
use_legacy_landlock: attempt.use_legacy_landlock,
|
||||
|
||||
@@ -40,7 +40,7 @@ pub(crate) fn build_sandbox_command(
|
||||
command: &[String],
|
||||
cwd: &AbsolutePathBuf,
|
||||
env: &HashMap<String, String>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
) -> Result<SandboxCommand, ToolError> {
|
||||
let (program, args) = command
|
||||
.split_first()
|
||||
|
||||
@@ -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;
|
||||
@@ -63,7 +64,7 @@ pub struct ShellRequest {
|
||||
pub explicit_env_overrides: HashMap<String, String>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
#[cfg(unix)]
|
||||
pub additional_permissions_preapproved: bool,
|
||||
pub justification: Option<String>,
|
||||
@@ -96,7 +97,7 @@ pub(crate) struct ApprovalKey {
|
||||
command: Vec<String>,
|
||||
cwd: AbsolutePathBuf,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
}
|
||||
|
||||
impl ShellRuntime {
|
||||
@@ -142,7 +143,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
) -> BoxFuture<'a, ReviewDecision> {
|
||||
let keys = self.approval_keys(req);
|
||||
let command = req.command.clone();
|
||||
let cwd = req.cwd.clone();
|
||||
let cwd = PathUri::from_abs_path(&req.cwd);
|
||||
let environment_id = Some(req.turn_environment.environment_id.clone());
|
||||
let reason = ctx
|
||||
.retry_reason
|
||||
@@ -184,7 +185,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
Ok(ApprovalAction::Shell {
|
||||
id: ctx.call_id.to_string(),
|
||||
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(),
|
||||
@@ -227,7 +228,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
call_id: ctx.call_id.clone(),
|
||||
tool_name: flat_tool_name(&ctx.tool_name).into_owned(),
|
||||
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(),
|
||||
|
||||
@@ -49,6 +49,7 @@ use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::localize_additional_permissions_for_current_host;
|
||||
use codex_shell_command::bash::parse_shell_lc_plain_commands;
|
||||
use codex_shell_command::bash::parse_shell_lc_single_command_prefix;
|
||||
use codex_shell_escalation::EscalateServer;
|
||||
@@ -103,6 +104,19 @@ fn approval_sandbox_permissions(
|
||||
}
|
||||
}
|
||||
|
||||
fn localize_additional_permissions_for_zsh_fork(
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
) -> Result<Option<AdditionalPermissionProfile>, ToolError> {
|
||||
additional_permissions
|
||||
.map(localize_additional_permissions_for_current_host)
|
||||
.transpose()
|
||||
.map_err(|err| {
|
||||
ToolError::Rejected(format!(
|
||||
"zsh-fork cannot localize additional permission URIs: {err}"
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) async fn try_run_zsh_fork(
|
||||
req: &ShellRequest,
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
@@ -232,6 +246,8 @@ pub(super) async fn try_run_zsh_fork(
|
||||
req.sandbox_permissions,
|
||||
req.additional_permissions_preapproved,
|
||||
);
|
||||
let prompt_permissions =
|
||||
localize_additional_permissions_for_zsh_fork(req.additional_permissions.clone())?;
|
||||
let escalation_policy = CoreShellActionProvider {
|
||||
policy: Arc::clone(&exec_policy),
|
||||
session: Arc::clone(&ctx.session),
|
||||
@@ -244,7 +260,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
file_system_sandbox_policy: command_executor.file_system_sandbox_policy.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
approval_sandbox_permissions,
|
||||
prompt_permissions: req.additional_permissions.clone(),
|
||||
prompt_permissions,
|
||||
stopwatch: stopwatch.clone(),
|
||||
};
|
||||
|
||||
@@ -316,6 +332,8 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
codex_linux_sandbox_exe: ctx.turn.config.codex_linux_sandbox_exe.clone(),
|
||||
use_legacy_landlock: ctx.turn.config.features.use_legacy_landlock(),
|
||||
};
|
||||
let prompt_permissions =
|
||||
localize_additional_permissions_for_zsh_fork(req.additional_permissions.clone())?;
|
||||
let escalation_policy = CoreShellActionProvider {
|
||||
policy: Arc::clone(&exec_policy),
|
||||
session: Arc::clone(&ctx.session),
|
||||
@@ -331,7 +349,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
req.sandbox_permissions,
|
||||
req.additional_permissions_preapproved,
|
||||
),
|
||||
prompt_permissions: req.additional_permissions.clone(),
|
||||
prompt_permissions,
|
||||
stopwatch: Stopwatch::unlimited(),
|
||||
};
|
||||
|
||||
@@ -500,8 +518,8 @@ impl CoreShellActionProvider {
|
||||
source,
|
||||
program: program.to_string_lossy().into_owned(),
|
||||
argv: argv.to_vec(),
|
||||
cwd: workdir.clone(),
|
||||
additional_permissions,
|
||||
cwd: PathUri::from_abs_path(&workdir),
|
||||
additional_permissions: additional_permissions.clone().map(Into::into),
|
||||
},
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
@@ -521,11 +539,11 @@ impl CoreShellActionProvider {
|
||||
approval_id,
|
||||
environment_id,
|
||||
command,
|
||||
workdir.clone(),
|
||||
PathUri::from_abs_path(&workdir),
|
||||
/*reason*/ None,
|
||||
/*network_approval_context*/ None,
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
additional_permissions,
|
||||
additional_permissions.map(Into::into),
|
||||
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
|
||||
)
|
||||
.await;
|
||||
@@ -1013,7 +1031,7 @@ impl CoreShellCommandExecutor {
|
||||
cwd,
|
||||
env,
|
||||
managed_network: None,
|
||||
additional_permissions,
|
||||
additional_permissions: additional_permissions.map(Into::into),
|
||||
};
|
||||
let options = ExecOptions {
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
|
||||
@@ -55,7 +55,6 @@ use futures::future::BoxFuture;
|
||||
use std::collections::HashMap;
|
||||
use std::io;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::error;
|
||||
|
||||
/// Request payload used by the unified-exec runtime after approvals and
|
||||
/// sandbox preferences have been resolved for the current turn.
|
||||
@@ -74,7 +73,7 @@ pub struct UnifiedExecRequest {
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub tty: bool,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
#[cfg(unix)]
|
||||
pub additional_permissions_preapproved: bool,
|
||||
pub justification: Option<String>,
|
||||
@@ -90,7 +89,7 @@ pub struct UnifiedExecApprovalKey {
|
||||
pub cwd: PathUri,
|
||||
pub tty: bool,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
}
|
||||
|
||||
/// Runtime adapter that keeps policy and sandbox orchestration on the
|
||||
@@ -118,7 +117,7 @@ fn build_unified_exec_sandbox_command(
|
||||
cwd: &PathUri,
|
||||
env: &HashMap<String, String>,
|
||||
managed_network: Option<ManagedNetworkSandboxContext>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
) -> Result<SandboxCommand, ToolError> {
|
||||
let (program, args) = command
|
||||
.split_first()
|
||||
@@ -183,15 +182,6 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
.clone()
|
||||
.or_else(|| req.justification.clone());
|
||||
Box::pin(async move {
|
||||
let native_cwd = match req.cwd.to_abs_path() {
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
// TODO(anp) make sandboxing work for foreign OSes, in the meantime this should
|
||||
// be impossible for single-OS app-servers
|
||||
error!(cwd = %req.cwd, ?e, "got non-native path in start_approval_async");
|
||||
return ReviewDecision::Abort;
|
||||
}
|
||||
};
|
||||
with_cached_approval(&session.services, "unified_exec", keys, || async move {
|
||||
let available_decisions = None;
|
||||
session
|
||||
@@ -201,7 +191,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
/*approval_id*/ None,
|
||||
environment_id,
|
||||
command,
|
||||
native_cwd,
|
||||
req.cwd.clone(),
|
||||
reason,
|
||||
ctx.network_approval_context.clone(),
|
||||
req.exec_approval_requirement
|
||||
@@ -224,7 +214,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
Ok(ApprovalAction::ExecCommand {
|
||||
id: ctx.call_id.to_string(),
|
||||
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(),
|
||||
@@ -278,7 +268,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
call_id: ctx.call_id.clone(),
|
||||
tool_name: flat_tool_name(&ctx.tool_name).into_owned(),
|
||||
command: req.command.clone(),
|
||||
cwd: req.cwd.to_abs_path().ok()?,
|
||||
cwd: req.cwd.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
additional_permissions: req.additional_permissions.clone(),
|
||||
justification: req.justification.clone(),
|
||||
|
||||
@@ -25,7 +25,7 @@ use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile_for_uri;
|
||||
use codex_tools::ToolName;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -480,7 +480,7 @@ impl<'a> SandboxAttempt<'a> {
|
||||
) -> Result<crate::sandboxing::ExecRequest, CodexErr> {
|
||||
let network = self.network_proxy(network);
|
||||
let managed_network = command.managed_network.clone();
|
||||
let exec_server_permissions = effective_permission_profile(
|
||||
let exec_server_permissions = effective_permission_profile_for_uri(
|
||||
self.exec_server_permissions,
|
||||
command.additional_permissions.as_ref(),
|
||||
);
|
||||
@@ -509,7 +509,7 @@ impl<'a> SandboxAttempt<'a> {
|
||||
exec_request.exec_server_managed_network = managed_network;
|
||||
if self.sandbox_requested {
|
||||
exec_request.exec_server_sandbox = Some(FileSystemSandboxContext {
|
||||
permissions: exec_server_permissions.into(),
|
||||
permissions: exec_server_permissions,
|
||||
cwd: Some(exec_request.windows_sandbox_policy_cwd.clone()),
|
||||
workspace_roots: Vec::new(),
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
|
||||
@@ -103,7 +103,7 @@ pub(crate) struct ExecCommandRequest {
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub tty: bool,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
pub additional_permissions_preapproved: bool,
|
||||
pub justification: Option<String>,
|
||||
pub prefix_rule: Option<Vec<String>>,
|
||||
|
||||
@@ -618,7 +618,7 @@ pub enum Op {
|
||||
/// Call id for the in-flight request.
|
||||
id: String,
|
||||
/// User-granted permissions.
|
||||
response: RequestPermissionsResponse,
|
||||
response: RequestPermissionsResponse<PathUri>,
|
||||
},
|
||||
|
||||
/// Resolve a dynamic tool call request.
|
||||
|
||||
@@ -24,6 +24,7 @@ pub use manager::SandboxType;
|
||||
pub use manager::SandboxablePreference;
|
||||
pub use manager::compatibility_sandbox_policy_for_permission_profile;
|
||||
pub use manager::get_platform_sandbox;
|
||||
pub use manager::localize_additional_permissions_for_current_host;
|
||||
pub use manager::with_managed_mitm_ca_readable_root;
|
||||
pub use windows::WindowsSandboxFilesystemOverrides;
|
||||
pub use windows::permission_profile_supports_windows_restricted_token_sandbox;
|
||||
@@ -45,7 +46,8 @@ impl From<SandboxTransformError> for CodexErr {
|
||||
fn from(err: SandboxTransformError) -> Self {
|
||||
match err {
|
||||
error @ SandboxTransformError::InvalidCommandCwd { .. }
|
||||
| error @ SandboxTransformError::InvalidSandboxPolicyCwd { .. } => {
|
||||
| error @ SandboxTransformError::InvalidSandboxPolicyCwd { .. }
|
||||
| error @ SandboxTransformError::InvalidAdditionalPermissions { .. } => {
|
||||
CodexErr::InvalidRequest(error.to_string())
|
||||
}
|
||||
SandboxTransformError::MissingLinuxSandboxExecutable => {
|
||||
|
||||
@@ -22,6 +22,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathConvention;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::collections::HashMap;
|
||||
use std::ffi::OsString;
|
||||
@@ -101,7 +102,26 @@ pub struct SandboxCommand {
|
||||
pub cwd: PathUri,
|
||||
pub env: HashMap<String, String>,
|
||||
pub managed_network: Option<ManagedNetworkSandboxContext>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
|
||||
}
|
||||
|
||||
pub fn localize_additional_permissions_for_current_host(
|
||||
additional_permissions: AdditionalPermissionProfile<PathUri>,
|
||||
) -> io::Result<AdditionalPermissionProfile> {
|
||||
if let Some(file_system) = additional_permissions.file_system.as_ref()
|
||||
&& let Some((path, _)) = file_system
|
||||
.explicit_path_entries()
|
||||
.find(|(path, _)| path.infer_path_convention() != Some(PathConvention::native()))
|
||||
{
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
format!(
|
||||
"permission path URI `{path}` is not native to the current host's {} path convention",
|
||||
PathConvention::native()
|
||||
),
|
||||
));
|
||||
}
|
||||
additional_permissions.try_into()
|
||||
}
|
||||
|
||||
/// A host-native launch request produced after [`SandboxManager::transform`] validates URI inputs.
|
||||
@@ -211,6 +231,9 @@ pub enum SandboxTransformError {
|
||||
cwd: PathUri,
|
||||
source: io::Error,
|
||||
},
|
||||
InvalidAdditionalPermissions {
|
||||
source: io::Error,
|
||||
},
|
||||
MissingLinuxSandboxExecutable,
|
||||
EnvironmentNetworkProxy(String),
|
||||
#[cfg(target_os = "linux")]
|
||||
@@ -234,6 +257,10 @@ impl std::fmt::Display for SandboxTransformError {
|
||||
f,
|
||||
"sandbox policy cwd URI `{cwd}` is not valid on this host: {source}"
|
||||
),
|
||||
Self::InvalidAdditionalPermissions { source } => write!(
|
||||
f,
|
||||
"additional permission URIs are not valid on this host: {source}"
|
||||
),
|
||||
Self::MissingLinuxSandboxExecutable => {
|
||||
write!(f, "missing codex-linux-sandbox executable path")
|
||||
}
|
||||
@@ -256,7 +283,8 @@ impl std::error::Error for SandboxTransformError {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
match self {
|
||||
Self::InvalidCommandCwd { source, .. }
|
||||
| Self::InvalidSandboxPolicyCwd { source, .. } => Some(source),
|
||||
| Self::InvalidSandboxPolicyCwd { source, .. }
|
||||
| Self::InvalidAdditionalPermissions { source } => Some(source),
|
||||
Self::MissingLinuxSandboxExecutable => None,
|
||||
Self::EnvironmentNetworkProxy(_) => None,
|
||||
#[cfg(target_os = "linux")]
|
||||
@@ -337,7 +365,18 @@ impl SandboxManager {
|
||||
} = request;
|
||||
#[cfg(target_os = "macos")]
|
||||
let managed_network = command.managed_network.as_ref();
|
||||
let additional_permissions = command.additional_permissions.take();
|
||||
let additional_permissions = match command
|
||||
.additional_permissions
|
||||
.take()
|
||||
.map(localize_additional_permissions_for_current_host)
|
||||
.transpose()
|
||||
{
|
||||
Ok(additional_permissions) => additional_permissions,
|
||||
Err(_) if sandbox == SandboxType::None => None,
|
||||
Err(source) => {
|
||||
return Err(SandboxTransformError::InvalidAdditionalPermissions { source });
|
||||
}
|
||||
};
|
||||
let managed_mitm_ca_trust_bundle_path =
|
||||
network.and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path);
|
||||
let base_effective_permission_profile =
|
||||
|
||||
Reference in New Issue
Block a user