Use executor path context for permission preapproval (#41928)

## Why

Permission grants can belong to an executor whose path convention differs from
the local host, such as a Windows remote environment. Matching those grants
against a host-native working directory can reject valid permissions.

## What changed

- Normalize, materialize, and compare additional filesystem permissions using
  the selected executor's URI-based sandbox context.
- Apply the same context-aware preapproval flow to `exec_command`,
  `apply_patch`, and extension tools.
- Fail closed when symbolic temporary-directory or project-root permissions
  cannot be resolved from executor metadata, while accepting opaque working
  directory URIs when requested paths use the executor's convention.

## Testing

- Added coverage for reusing a Windows permission grant from a non-Windows
  host, resolving symbolic temporary-directory grants, and normalizing paths
  with an opaque Windows working directory.

GitOrigin-RevId: aee54350a25430a0443f8319e2bb0bbdc4f44282
This commit is contained in:
iceweasel-oai
2026-08-31 22:58:26 +00:00
committed by copyberry
parent 9127f21890
commit c4350b4ca2
8 changed files with 280 additions and 30 deletions
@@ -314,8 +314,8 @@ async fn effective_patch_permissions(
.collect::<Result<Vec<_>, _>>()?;
let effective_additional_permissions = apply_granted_turn_permissions(
session,
environment_id,
native_cwd.as_path(),
environment,
cwd,
crate::sandboxing::SandboxPermissions::UseDefault,
write_permissions_for_paths(&native_file_paths, &file_system_sandbox_policy, &native_cwd),
)
@@ -184,8 +184,8 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall<'_>
};
let additional_permissions = apply_granted_turn_permissions(
invocation.session.as_ref(),
&environment.selection.environment_id,
native_cwd.as_path(),
environment,
environment.cwd(),
SandboxPermissions::UseDefault,
/*additional_permissions*/ None,
)
+128 -16
View File
@@ -35,15 +35,16 @@ mod view_image;
pub(crate) mod view_image_spec;
mod wait_for_environment;
use codex_sandboxing::policy_transforms::materialize_additional_permissions;
use codex_file_system::FileSystemSandboxContext;
use codex_sandboxing::policy_transforms::materialize_additional_permissions_with_context;
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
use codex_sandboxing::policy_transforms::normalize_additional_permissions_with_context;
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,7 +182,7 @@ pub(crate) fn normalize_and_validate_additional_permissions(
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
permissions_preapproved: bool,
_cwd: &Path,
context: &codex_protocol::permissions::FileSystemSandboxPolicyContext<'_>,
) -> Result<Option<AdditionalPermissionProfile>, String> {
let uses_additional_permissions = matches!(
sandbox_permissions,
@@ -210,7 +211,8 @@ pub(crate) fn normalize_and_validate_additional_permissions(
.to_string(),
);
};
let normalized = normalize_additional_permissions(additional_permissions)?;
let normalized =
normalize_additional_permissions_with_context(additional_permissions, context)?;
if normalized.is_empty() {
return Err(
"`additional_permissions` must include at least one requested permission in `network` or `file_system`"
@@ -236,6 +238,15 @@ pub(super) struct EffectiveAdditionalPermissions {
pub permissions_preapproved: bool,
}
pub(super) fn file_system_sandbox_policy_context_for_cwd<'a>(
sandbox_context: &'a FileSystemSandboxContext,
cwd: &'a PathUri,
) -> Option<codex_protocol::permissions::FileSystemSandboxPolicyContext<'a>> {
let mut context = sandbox_context.policy_context()?;
context.cwd = cwd;
Some(context)
}
pub(super) fn implicit_granted_permissions(
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<&AdditionalPermissionProfile>,
@@ -255,8 +266,8 @@ pub(super) fn implicit_granted_permissions(
pub(super) async fn apply_granted_turn_permissions(
session: &Session,
environment_id: &str,
cwd: &Path,
environment: &TurnEnvironment,
cwd: &PathUri,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
) -> EffectiveAdditionalPermissions {
@@ -268,6 +279,7 @@ pub(super) async fn apply_granted_turn_permissions(
};
}
let environment_id = &environment.selection.environment_id;
let granted_session_permissions = session.granted_session_permissions(environment_id).await;
let granted_turn_permissions = session.granted_turn_permissions(environment_id).await;
let granted_permissions = merge_permission_profiles(
@@ -278,13 +290,15 @@ pub(super) async fn apply_granted_turn_permissions(
additional_permissions.as_ref(),
granted_permissions.as_ref(),
);
let sandbox_context = environment.sandbox_context(/*additional_permissions*/ None);
let context = file_system_sandbox_policy_context_for_cwd(&sandbox_context, cwd);
let preapproved_permissions = granted_permissions.as_ref().and_then(|granted| {
if additional_permissions.is_none() {
Some(granted.clone())
} else {
effective_permissions
.as_ref()
.and_then(|effective| preapproved_permission_profile(effective, granted, cwd))
effective_permissions.as_ref().and_then(|effective| {
preapproved_permission_profile(effective, granted, context.as_ref()?)
})
}
});
let permissions_preapproved = preapproved_permissions.is_some();
@@ -309,11 +323,35 @@ pub(super) async fn apply_granted_turn_permissions(
fn preapproved_permission_profile(
effective_permissions: &AdditionalPermissionProfile,
granted_permissions: &AdditionalPermissionProfile,
cwd: &Path,
context: &codex_protocol::permissions::FileSystemSandboxPolicyContext<'_>,
) -> Option<AdditionalPermissionProfile> {
if effective_permissions
.file_system
.as_ref()
.is_some_and(|permissions| {
permissions.entries.iter().any(|entry| {
(matches!(
&entry.path,
codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir,
}
) && context
.temporary_directories
.is_none_or(<[PathUri]>::is_empty))
|| matches!(
&entry.path,
codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::ProjectRoots { .. },
} if context.workspace_roots.is_empty()
)
})
})
{
return None;
}
let (Ok(effective), Ok(granted)) = (
materialize_additional_permissions(effective_permissions.clone(), cwd),
materialize_additional_permissions(granted_permissions.clone(), cwd),
materialize_additional_permissions_with_context(effective_permissions.clone(), context),
materialize_additional_permissions_with_context(granted_permissions.clone(), context),
) else {
return None;
};
@@ -352,12 +390,14 @@ mod tests {
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
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::merge_permission_profiles;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
@@ -382,10 +422,20 @@ mod tests {
}
}
fn local_context(cwd: &PathUri) -> FileSystemSandboxPolicyContext<'_> {
FileSystemSandboxPolicyContext {
cwd,
workspace_roots: std::slice::from_ref(cwd),
temporary_directories: None,
user_home_dir: None,
}
}
#[test]
fn preapproved_permissions_work_when_request_permissions_tool_is_enabled_without_exec_permission_approvals_feature()
{
let cwd = tempdir().expect("tempdir");
let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
let normalized = normalize_and_validate_additional_permissions(
/*additional_permissions_allowed*/ false,
@@ -399,7 +449,7 @@ mod tests {
SandboxPermissions::WithAdditionalPermissions,
Some(network_permissions()),
/*permissions_preapproved*/ true,
cwd.path(),
&local_context(&cwd),
)
.expect("preapproved permissions should be allowed");
@@ -409,6 +459,7 @@ mod tests {
#[test]
fn fresh_additional_permissions_still_require_exec_permission_approvals_feature() {
let cwd = tempdir().expect("tempdir");
let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
let err = normalize_and_validate_additional_permissions(
/*additional_permissions_allowed*/ false,
@@ -416,7 +467,7 @@ mod tests {
SandboxPermissions::WithAdditionalPermissions,
Some(network_permissions()),
/*permissions_preapproved*/ false,
cwd.path(),
&local_context(&cwd),
)
.expect_err("fresh inline permission requests should remain disabled");
@@ -493,10 +544,71 @@ mod tests {
let effective_permissions =
merge_permission_profiles(Some(&requested_permissions), Some(&stored_grant))
.expect("merged permissions");
let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
assert_eq!(
preapproved_permission_profile(&effective_permissions, &stored_grant, cwd.path()),
preapproved_permission_profile(
&effective_permissions,
&stored_grant,
&local_context(&cwd),
),
Some(stored_grant)
);
}
#[test]
fn symbolic_tmpdir_preapproval_requires_executor_metadata() {
let cwd = PathUri::parse("file:///C:/workspace").expect("Windows cwd");
let temporary_directory = PathUri::parse("file:///C:/Temp").expect("Windows temp dir");
let concrete = FileSystemSandboxEntry::new(
temporary_directory.clone().into(),
FileSystemAccessMode::Write,
);
let symbolic = FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Tmpdir,
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
};
let effective = AdditionalPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![concrete.clone(), symbolic],
glob_scan_max_depth: None,
}),
..Default::default()
};
let granted = AdditionalPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![concrete],
glob_scan_max_depth: None,
}),
..Default::default()
};
assert_eq!(
preapproved_permission_profile(&effective, &granted, &local_context(&cwd)),
None
);
let empty_temporary_directories = [];
let empty_tmpdir_context = FileSystemSandboxPolicyContext {
temporary_directories: Some(&empty_temporary_directories),
..local_context(&cwd)
};
assert_eq!(
preapproved_permission_profile(&effective, &granted, &empty_tmpdir_context),
None
);
let temporary_directories = [temporary_directory];
let tmpdir_context = FileSystemSandboxPolicyContext {
temporary_directories: Some(&temporary_directories),
..local_context(&cwd)
};
assert_eq!(
preapproved_permission_profile(&effective, &granted, &tmpdir_context),
Some(granted)
);
}
}
@@ -5,8 +5,10 @@ use codex_protocol::models::NetworkPermissions;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
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;
@@ -20,6 +22,15 @@ fn file_system_permissions(entries: Vec<FileSystemSandboxEntry>) -> AdditionalPe
}
}
fn local_context(cwd: &PathUri) -> FileSystemSandboxPolicyContext<'_> {
FileSystemSandboxPolicyContext {
cwd,
workspace_roots: std::slice::from_ref(cwd),
temporary_directories: None,
user_home_dir: None,
}
}
#[test]
fn preapproval_accepts_reordered_replay_of_one_accumulated_grant() {
let cwd = tempdir().expect("tempdir");
@@ -35,9 +46,10 @@ fn preapproval_accepts_reordered_replay_of_one_accumulated_grant() {
});
let replay = file_system_permissions(vec![write]);
let effective = merge_permission_profiles(Some(&replay), Some(&granted)).expect("permissions");
let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
assert_eq!(
preapproved_permission_profile(&effective, &granted, cwd.path()),
preapproved_permission_profile(&effective, &granted, &local_context(&cwd)),
Some(granted)
);
}
@@ -50,6 +62,8 @@ fn preapproval_requires_fresh_read_and_write_beneath_a_deny() {
FileSystemSandboxEntry::new(root.clone().into(), FileSystemAccessMode::Write),
FileSystemSandboxEntry::new(root.join("secrets").into(), FileSystemAccessMode::Deny),
]);
let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
let context = local_context(&cwd);
let preapproved = [FileSystemAccessMode::Read, FileSystemAccessMode::Write].map(|access| {
let requested = file_system_permissions(vec![FileSystemSandboxEntry::new(
@@ -59,7 +73,7 @@ fn preapproval_requires_fresh_read_and_write_beneath_a_deny() {
let effective =
merge_permission_profiles(Some(&requested), Some(&granted)).expect("permissions");
preapproved_permission_profile(&effective, &granted, cwd.path())
preapproved_permission_profile(&effective, &granted, &context)
});
assert_eq!(preapproved, [None, None]);
@@ -74,9 +88,10 @@ fn preapproval_fails_closed_when_materialization_rejects_both_profiles() {
},
FileSystemAccessMode::Write,
)]);
let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI");
assert_eq!(
preapproved_permission_profile(&invalid, &invalid, cwd.path()),
preapproved_permission_profile(&invalid, &invalid, &local_context(&cwd)),
None
);
}
@@ -12,6 +12,7 @@ use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::apply_granted_turn_permissions;
use crate::tools::handlers::apply_patch::intercept_apply_patch;
use crate::tools::handlers::file_system_sandbox_policy_context_for_cwd;
use crate::tools::handlers::implicit_granted_permissions;
use crate::tools::handlers::normalize_and_validate_additional_permissions;
use crate::tools::handlers::parse_arguments;
@@ -301,12 +302,20 @@ 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 sandbox_context =
turn_environment.sandbox_context(/*additional_permissions*/ None);
let Some(permission_context) =
file_system_sandbox_policy_context_for_cwd(&sandbox_context, &cwd)
else {
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(
"selected environment sandbox context is missing cwd".to_string(),
));
};
let effective_additional_permissions = apply_granted_turn_permissions(
context.session.as_ref(),
&turn_environment.selection.environment_id,
permission_cwd.as_path(),
turn_environment,
&cwd,
sandbox_permissions,
additional_permissions,
)
@@ -343,7 +352,7 @@ impl ExecCommandHandler {
effective_additional_permissions.sandbox_permissions,
effective_additional_permissions.additional_permissions,
effective_additional_permissions.permissions_preapproved,
permission_cwd,
&permission_context,
)
},
|permissions| Ok(Some(permissions)),
@@ -280,6 +280,99 @@ async fn shell_mode_for_environment_uses_direct_mode_for_remote_environments() -
Ok(())
}
#[tokio::test]
#[cfg(not(windows))]
async fn exec_command_reuses_foreign_windows_grant() {
use crate::session::tests::make_session_and_context_with_auth_and_config_and_rx;
use codex_features::Feature;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::FileSystemPermissions;
use codex_utils_path_uri::PathUri;
let (session, mut turn, _events) = make_session_and_context_with_auth_and_config_and_rx(
codex_login::CodexAuth::from_api_key("Test API Key"),
Vec::new(),
|config| {
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("test setup should allow request permissions");
},
)
.await;
let cwd = PathUri::parse("file:///C:/workspace").expect("valid Windows cwd");
let granted_permissions = AdditionalPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_path_uris(
/*read*/ Some(Vec::new()),
/*write*/
Some(vec![
PathUri::parse("file:///C:/workspace/granted").expect("valid Windows grant"),
]),
)),
..Default::default()
};
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let turn_state = {
let active_turn = session.active_turn.lock().await;
Arc::clone(&active_turn.as_ref().expect("active turn").turn_state)
};
turn_state.lock().await.record_granted_permissions(
codex_exec_server::REMOTE_ENVIRONMENT_ID,
granted_permissions.clone(),
);
{
let turn = Arc::get_mut(&mut turn).expect("turn should be uniquely owned");
let TurnEnvironmentState::Ready(environment) = turn
.environments
.environments
.first_mut()
.expect("primary environment")
else {
panic!("primary environment should be ready");
};
environment.selection.environment_id = codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string();
environment.selection.cwd = cwd.clone();
environment.selection.workspace_roots = vec![cwd.clone()];
environment.config_mut().workspace_roots = vec![cwd];
environment.environment = Arc::new(
Environment::create_for_tests(Some("ws://127.0.0.1:1/remote-exec-server".to_string()))
.expect("remote environment"),
);
}
let response = ExecCommandHandler::default()
.handle(ToolInvocation {
session: Arc::clone(&session),
step_context: StepContext::for_test(Arc::clone(&turn)),
turn,
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "foreign-windows-grant".to_string(),
tool_name: codex_tools::ToolName::plain("exec_command"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: serde_json::json!({
"cmd": "*** Begin Patch\n*** Add File: granted/file.txt\n+text\n*** End Patch",
"workdir": "nested",
"sandbox_permissions": "with_additional_permissions",
"additional_permissions": granted_permissions,
})
.to_string(),
},
})
.await;
let Err(FunctionCallError::RespondToModel(message)) = response else {
panic!("raw patch should stop before remote execution");
};
assert!(
message.contains("apply_patch verification failed"),
"matching foreign grant should reach patch interception: {message}"
);
}
#[tokio::test]
async fn exec_command_pre_tool_use_payload_uses_raw_command() {
let payload = ToolPayload::Function {
@@ -67,7 +67,6 @@ pub fn normalize_additional_permissions_with_context(
if path.infer_path_convention().is_none()
|| path.infer_path_convention() != context.cwd.infer_path_convention()
|| path.join(".").is_err()
|| context.cwd.join(".").is_err()
{
return Err(format!(
"permission path `{path}` does not match executor cwd `{}`",
@@ -18,6 +18,8 @@ use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathConvention;
use codex_utils_path_uri::PathUri;
use dunce::canonicalize;
use pretty_assertions::assert_eq;
@@ -162,9 +164,29 @@ fn normalize_additional_permissions_only_checks_convention_with_context() {
temporary_directories: None,
};
assert!(
normalize_additional_permissions_with_context(permissions, &context).is_err(),
normalize_additional_permissions_with_context(permissions.clone(), &context).is_err(),
"context-aware normalization should reject mismatched conventions"
);
let cwd = LegacyAppPathString::from_string(
r"\\?\Volume{00000000-0000-0000-0000-000000000000}\workspace",
)
.to_path_uri(PathConvention::Windows)
.expect("opaque Windows cwd");
assert!(cwd.is_opaque(), "volume GUID cwd should stay opaque");
let workspace_roots = [cwd.clone()];
let context = FileSystemSandboxPolicyContext {
cwd: &cwd,
workspace_roots: &workspace_roots,
user_home_dir: None,
temporary_directories: None,
};
assert_eq!(
normalize_additional_permissions_with_context(permissions.clone(), &context)
.expect("absolute path with opaque cwd"),
permissions
);
}
#[cfg(unix)]