mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
app-server: include filesystem entries in permission requests (#19086)
## Why `item/permissions/requestApproval` sends a requested permission profile to app-server clients. The core profile already stores filesystem permissions as `entries`, but the v2 compatibility conversion used the legacy `read`/`write` projection whenever possible and left `entries` unset. That made the request ambiguous for clients that consume the canonical v2 shape: `permissions.fileSystem.entries` was missing even though filesystem access was being requested. A client that rendered or echoed grants from `entries` could treat the request as having no filesystem permission entries, then return an empty or incomplete grant. The app-server intersects responses with the original request, so omitted filesystem permissions are denied. ## What Changed - Populate `AdditionalFileSystemPermissions.entries` when converting legacy read/write roots for request permission payloads, while preserving `read` and `write` for compatibility. - Mark `read` and `write` as transitional schema fields in the generated app-server schema. - Add regression coverage for the v2 conversion, the app-server `item/permissions/requestApproval` round trip, and TUI app-server approval conversion expectations. - Refresh generated JSON and TypeScript schema fixtures. ## Verification - `just fmt` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-app-server request_permissions_round_trip` - `cargo test -p codex-tui converts_request_permissions_into_granted_permissions` - `cargo test -p codex-tui resolves_permissions_and_user_input_through_app_server_request_id`
This commit is contained in:
committed by
GitHub
Unverified
parent
993e3f407e
commit
8bc667b07b
@@ -1273,7 +1273,9 @@ impl From<CoreNetworkApprovalContext> for NetworkApprovalContext {
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct AdditionalFileSystemPermissions {
|
||||
/// This will be removed in favor of `entries`.
|
||||
pub read: Option<Vec<AbsolutePathBuf>>,
|
||||
/// This will be removed in favor of `entries`.
|
||||
pub write: Option<Vec<AbsolutePathBuf>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
@@ -1286,11 +1288,26 @@ pub struct AdditionalFileSystemPermissions {
|
||||
impl From<CoreFileSystemPermissions> for AdditionalFileSystemPermissions {
|
||||
fn from(value: CoreFileSystemPermissions) -> Self {
|
||||
if let Some((read, write)) = value.legacy_read_write_roots() {
|
||||
let mut entries = Vec::with_capacity(
|
||||
read.as_ref().map_or(0, Vec::len) + write.as_ref().map_or(0, Vec::len),
|
||||
);
|
||||
if let Some(paths) = read.as_ref() {
|
||||
entries.extend(paths.iter().map(|path| FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path: path.clone() },
|
||||
access: FileSystemAccessMode::Read,
|
||||
}));
|
||||
}
|
||||
if let Some(paths) = write.as_ref() {
|
||||
entries.extend(paths.iter().map(|path| FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path: path.clone() },
|
||||
access: FileSystemAccessMode::Write,
|
||||
}));
|
||||
}
|
||||
Self {
|
||||
read,
|
||||
write,
|
||||
glob_scan_max_depth: None,
|
||||
entries: None,
|
||||
entries: Some(entries),
|
||||
}
|
||||
} else {
|
||||
Self {
|
||||
@@ -7764,6 +7781,45 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn additional_file_system_permissions_populates_entries_for_legacy_roots() {
|
||||
let read_only_path = absolute_path("read-only");
|
||||
let read_write_path = absolute_path("read-write");
|
||||
let core_permissions = CoreFileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![read_only_path.clone()]),
|
||||
Some(vec![read_write_path.clone()]),
|
||||
);
|
||||
|
||||
let permissions = AdditionalFileSystemPermissions::from(core_permissions.clone());
|
||||
|
||||
assert_eq!(
|
||||
permissions,
|
||||
AdditionalFileSystemPermissions {
|
||||
read: Some(vec![read_only_path.clone()]),
|
||||
write: Some(vec![read_write_path.clone()]),
|
||||
glob_scan_max_depth: None,
|
||||
entries: Some(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: read_only_path,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: read_write_path,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
]),
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
CoreFileSystemPermissions::from(permissions),
|
||||
core_permissions
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn additional_file_system_permissions_rejects_zero_glob_scan_depth() {
|
||||
serde_json::from_value::<AdditionalFileSystemPermissions>(json!({
|
||||
|
||||
Reference in New Issue
Block a user