mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Make permission transforms aware of executor path context (#41909)
## What changed - Add context-aware normalization, materialization, and intersection helpers for additional filesystem permissions. - Resolve project roots, home-relative deny globs, temporary directories, and filesystem roots from the executor's `FileSystemSandboxPolicyContext` while preserving URI path conventions and symlinked paths. - Keep deny constraints when intersecting grants across POSIX, Windows, and UNC paths, and reject incompatible path conventions when executor context is available. ## Testing - Cover cross-convention validation, empty temporary-directory contexts, home-relative deny globs, and rooted deny globs for POSIX, Windows, and UNC paths. GitOrigin-RevId: 14fd866a17a0ac4a595aa4f0e2fb82bfae1a8838
This commit is contained in:
1 parent
2f0a5d5516
commit
b51b07785b
6 files changed
+534
-246
No files matched your search
@@ -287,16 +287,6 @@ fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfil
|
||||
}
|
||||
}
|
||||
|
||||
fn normalized_directory_write_permissions(path: &Path) -> Result<RequestPermissionProfile> {
|
||||
Ok(RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
})
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn with_additional_permissions_requires_approval_under_on_request() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -508,7 +498,7 @@ async fn request_permissions_auto_review_applies_guardian_decision(outcome: &str
|
||||
let requested_dir = test.workspace_path("guardian-requested-permissions");
|
||||
fs::create_dir_all(&requested_dir)?;
|
||||
let requested_permissions = requested_directory_write_permissions(&requested_dir);
|
||||
let normalized_permissions = normalized_directory_write_permissions(&requested_dir)?;
|
||||
let normalized_permissions = requested_directory_write_permissions(&requested_dir);
|
||||
let call_id = "guardian-request-permissions";
|
||||
let reason = "Guardian should review access to the requested directory";
|
||||
let responses = mount_sse_sequence(
|
||||
@@ -576,9 +566,7 @@ async fn request_permissions_auto_review_applies_guardian_decision(outcome: &str
|
||||
})
|
||||
.context("expected Guardian review request")?;
|
||||
assert!(guardian_request.body_contains_text(reason));
|
||||
assert!(
|
||||
guardian_request.body_contains_text(&requested_dir.canonicalize()?.display().to_string())
|
||||
);
|
||||
assert!(guardian_request.body_contains_text(&requested_dir.display().to_string()));
|
||||
|
||||
let output = requests
|
||||
.iter()
|
||||
@@ -765,7 +753,6 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul
|
||||
|
||||
let nested_dir = test.workspace_path("nested");
|
||||
fs::create_dir_all(&nested_dir)?;
|
||||
let nested_dir_canonical = nested_dir.canonicalize()?;
|
||||
let requested_write = nested_dir.join("relative-write.txt");
|
||||
let _ = fs::remove_file(&requested_write);
|
||||
|
||||
@@ -780,7 +767,7 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul
|
||||
let expected_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![absolute_path(&nested_dir_canonical)]),
|
||||
Some(vec![absolute_path(&nested_dir)]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -1096,15 +1083,7 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let normalized_requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(
|
||||
outside_dir.path().canonicalize()?,
|
||||
)?]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let normalized_requested_permissions = requested_permissions.clone();
|
||||
let event =
|
||||
exec_command_event_with_request_permissions(call_id, &command, &requested_permissions)?;
|
||||
|
||||
@@ -1202,15 +1181,7 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let normalized_requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(
|
||||
outside_dir.path().canonicalize()?,
|
||||
)?]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let normalized_requested_permissions = requested_permissions.clone();
|
||||
let event =
|
||||
exec_command_event_with_request_permissions(call_id, &command, &requested_permissions)?;
|
||||
|
||||
@@ -1308,15 +1279,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let normalized_requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(
|
||||
outside_dir.path().canonicalize()?,
|
||||
)?]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let normalized_requested_permissions = requested_permissions.clone();
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
@@ -1430,7 +1393,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req
|
||||
);
|
||||
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(outside_dir.path())?;
|
||||
requested_directory_write_permissions(outside_dir.path());
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
@@ -1550,7 +1513,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls_without_in
|
||||
);
|
||||
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(outside_dir.path())?;
|
||||
requested_directory_write_permissions(outside_dir.path());
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
@@ -1677,28 +1640,10 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let normalized_requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![
|
||||
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
|
||||
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
|
||||
]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let granted_permissions = normalized_directory_write_permissions(first_dir.path())?;
|
||||
let normalized_requested_permissions = requested_permissions.clone();
|
||||
let granted_permissions = requested_directory_write_permissions(first_dir.path());
|
||||
let second_dir_permissions = requested_directory_write_permissions(second_dir.path());
|
||||
let merged_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![
|
||||
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
|
||||
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
|
||||
]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let merged_permissions = requested_permissions.clone();
|
||||
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
@@ -1835,7 +1780,7 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> {
|
||||
let outside_dir = tempfile::tempdir()?;
|
||||
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(outside_dir.path())?;
|
||||
requested_directory_write_permissions(outside_dir.path());
|
||||
|
||||
let _first_turn = mount_sse_sequence(
|
||||
&server,
|
||||
@@ -1952,7 +1897,7 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> {
|
||||
let outside_write = outside_dir.path().join("session-sticky-write.txt");
|
||||
let requested_permissions = requested_directory_write_permissions(outside_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(outside_dir.path())?;
|
||||
requested_directory_write_permissions(outside_dir.path());
|
||||
let command = format!(
|
||||
"printf {:?} > {:?} && cat {:?}",
|
||||
"session-sticky-ok", outside_write, outside_write
|
||||
|
||||
@@ -98,16 +98,6 @@ fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfil
|
||||
}
|
||||
}
|
||||
|
||||
fn normalized_directory_write_permissions(path: &Path) -> Result<RequestPermissionProfile> {
|
||||
Ok(RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
})
|
||||
}
|
||||
|
||||
fn parse_result(item: &Value) -> (Option<i64>, String) {
|
||||
let output_str = item
|
||||
.get("output")
|
||||
@@ -241,7 +231,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s
|
||||
);
|
||||
let requested_permissions = requested_directory_write_permissions(requested_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(requested_dir.path())?;
|
||||
requested_directory_write_permissions(requested_dir.path());
|
||||
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
@@ -378,13 +368,10 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
} else {
|
||||
"patched-via-request-permissions"
|
||||
};
|
||||
let requested_file = requested_dir
|
||||
.path()
|
||||
.canonicalize()?
|
||||
.join(requested_file_name);
|
||||
let requested_file = requested_dir.path().join(requested_file_name);
|
||||
let requested_permissions = requested_directory_write_permissions(requested_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(requested_dir.path())?;
|
||||
requested_directory_write_permissions(requested_dir.path());
|
||||
let patch = build_add_file_patch(&requested_file, patch_content);
|
||||
|
||||
let response_prefix = if strict_auto_review {
|
||||
|
||||
Reference in new issue
Block a user