openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-30 19:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: use AbsolutePathBuf for permission profile file roots

Browse Source
This commit is contained in:
Michael Bolin
2026-02-26 17:27:01 -08:00
parent a11da86b37
commit 554fd38785
20 changed files with 173 additions and 184 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json
+6 -2
View File
@@ -1,11 +1,15 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"AbsolutePathBuf": {
"description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.",
"type": "string"
},
"AdditionalFileSystemPermissions": {
"properties": {
"read": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
@@ -14,7 +18,7 @@
},
"write": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
codex-rs/app-server-protocol/schema/json/EventMsg.json
+2 -2
View File
@@ -3411,7 +3411,7 @@
"properties": {
"read": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
@@ -3420,7 +3420,7 @@
},
"write": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
codex-rs/app-server-protocol/schema/json/ServerRequest.json
+6 -2
View File
@@ -1,11 +1,15 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"AbsolutePathBuf": {
"description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.",
"type": "string"
},
"AdditionalFileSystemPermissions": {
"properties": {
"read": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
@@ -14,7 +18,7 @@
},
"write": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
+4 -4
View File
@@ -5,7 +5,7 @@
"properties": {
"read": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
@@ -14,7 +14,7 @@
},
"write": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
@@ -4722,7 +4722,7 @@
"properties": {
"read": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
@@ -4731,7 +4731,7 @@
},
"write": {
"items": {
"type": "string"
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": [
"array",
codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts
+2 -1
View File
@@ -1,5 +1,6 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "./AbsolutePathBuf";
export type FileSystemPermissions = { read: Array<string> | null, write: Array<string> | null, };
export type FileSystemPermissions = { read: Array<AbsolutePathBuf> | null, write: Array<AbsolutePathBuf> | null, };
codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts
+2 -1
View File
@@ -1,5 +1,6 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
export type AdditionalFileSystemPermissions = { read: Array<string> | null, write: Array<string> | null, };
export type AdditionalFileSystemPermissions = { read: Array<AbsolutePathBuf> | null, write: Array<AbsolutePathBuf> | null, };
codex-rs/app-server-protocol/src/protocol/common.rs
+6 -1
View File
@@ -893,10 +893,15 @@ mod tests {
use codex_protocol::account::PlanType;
use codex_protocol::parse_command::ParsedCommand;
use codex_protocol::protocol::AskForApproval;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::path::PathBuf;
fn absolute_path(path: &str) -> AbsolutePathBuf {
AbsolutePathBuf::from_absolute_path(path).expect("absolute path")
}
#[test]
fn serialize_new_conversation() -> Result<()> {
let request = ClientRequest::NewConversation {
@@ -1533,7 +1538,7 @@ mod tests {
additional_permissions: Some(v2::AdditionalPermissionProfile {
network: None,
file_system: Some(v2::AdditionalFileSystemPermissions {
read: Some(vec![std::path::PathBuf::from("/tmp/allowed")]),
read: Some(vec![absolute_path("/tmp/allowed")]),
write: None,
}),
macos: None,
codex-rs/app-server-protocol/src/protocol/v2.rs
+33 -2
View File
@@ -811,8 +811,8 @@ impl From<CoreNetworkApprovalContext> for NetworkApprovalContext {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct AdditionalFileSystemPermissions {
pub read: Option<Vec<PathBuf>>,
pub write: Option<Vec<PathBuf>>,
pub read: Option<Vec<AbsolutePathBuf>>,
pub write: Option<Vec<AbsolutePathBuf>>,
}
impl From<CoreFileSystemPermissions> for AdditionalFileSystemPermissions {
@@ -4142,6 +4142,37 @@ mod tests {
AbsolutePathBuf::from_absolute_path(path).expect("path must be absolute")
}
#[test]
fn command_execution_request_approval_rejects_relative_additional_permission_paths() {
let err = serde_json::from_value::<CommandExecutionRequestApprovalParams>(json!({
"threadId": "thr_123",
"turnId": "turn_123",
"itemId": "call_123",
"command": "cat file",
"cwd": "/tmp",
"commandActions": null,
"reason": null,
"networkApprovalContext": null,
"additionalPermissions": {
"network": null,
"fileSystem": {
"read": ["relative/path"],
"write": null
},
"macos": null
},
"proposedExecpolicyAmendment": null,
"proposedNetworkPolicyAmendments": null,
"availableDecisions": null
}))
.expect_err("relative additional permission paths should fail");
assert!(
err.to_string()
.contains("AbsolutePathBuf deserialized without a base path"),
"unexpected error: {err}"
);
}
#[test]
fn sandbox_policy_round_trips_external_sandbox_network_access() {
let v2_policy = SandboxPolicy::ExternalSandbox {
codex-rs/app-server/README.md
+1 -1
View File
@@ -710,7 +710,7 @@ Certain actions (shell commands or modifying files) may require explicit user ap
Order of messages:
1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action.
2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted.
2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access; any filesystem paths in that payload are absolute on the wire. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted.
3. Client response — for example `{ "decision": "accept" }`, `{ "decision": "acceptForSession" }`, `{ "decision": { "acceptWithExecpolicyAmendment": { "execpolicy_amendment": [...] } } }`, `{ "decision": { "applyNetworkPolicyAmendment": { "network_policy_amendment": { "host": "example.com", "action": "allow" } } } }`, `{ "decision": "decline" }`, or `{ "decision": "cancel" }`.
4. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result.
codex-rs/app-server/src/transport.rs
+7 -2
View File
@@ -671,12 +671,17 @@ pub(crate) async fn route_outgoing_envelope(
mod tests {
use super::*;
use crate::error_code::OVERLOADED_ERROR_CODE;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::path::PathBuf;
use tokio::time::Duration;
use tokio::time::timeout;
fn absolute_path(path: &str) -> AbsolutePathBuf {
AbsolutePathBuf::from_absolute_path(path).expect("absolute path")
}
#[test]
fn app_server_transport_parses_stdio_listen_url() {
let transport = AppServerTransport::from_listen_url(AppServerTransport::DEFAULT_LISTEN_URL)
@@ -977,7 +982,7 @@ mod tests {
network: None,
file_system: Some(
codex_app_server_protocol::AdditionalFileSystemPermissions {
read: Some(vec![PathBuf::from("/tmp/allowed")]),
read: Some(vec![absolute_path("/tmp/allowed")]),
write: None,
},
),
@@ -1039,7 +1044,7 @@ mod tests {
network: None,
file_system: Some(
codex_app_server_protocol::AdditionalFileSystemPermissions {
read: Some(vec![PathBuf::from("/tmp/allowed")]),
read: Some(vec![absolute_path("/tmp/allowed")]),
write: None,
},
),
codex-rs/core/src/exec.rs
-3
View File
@@ -521,9 +521,6 @@ pub(crate) mod errors {
SandboxTransformError::SeatbeltUnavailable => CodexErr::UnsupportedOperation(
"seatbelt sandbox is only available on macOS".to_string(),
),
SandboxTransformError::InvalidAdditionalPermissionsPath(path) => {
CodexErr::InvalidRequest(format!("invalid additional_permissions path: {path}"))
}
}
}
}
codex-rs/core/src/sandboxing/mod.rs
+18 -60
View File
@@ -92,8 +92,6 @@ pub enum SandboxPreference {
pub(crate) enum SandboxTransformError {
#[error("missing codex-linux-sandbox executable path")]
MissingLinuxSandboxExecutable,
#[error("invalid additional permissions path: {0}")]
InvalidAdditionalPermissionsPath(String),
#[cfg(not(target_os = "macos"))]
#[error("seatbelt sandbox is only available on macOS")]
SeatbeltUnavailable,
@@ -101,19 +99,16 @@ pub(crate) enum SandboxTransformError {
pub(crate) fn normalize_additional_permissions(
additional_permissions: PermissionProfile,
command_cwd: &Path,
) -> Result<PermissionProfile, String> {
let Some(file_system) = additional_permissions.file_system else {
return Ok(PermissionProfile::default());
};
let read = file_system
.read
.map(|paths| normalize_permission_paths(paths, command_cwd, "file_system.read"))
.transpose()?;
.map(|paths| normalize_permission_paths(paths, "file_system.read"));
let write = file_system
.write
.map(|paths| normalize_permission_paths(paths, command_cwd, "file_system.write"))
.transpose()?;
.map(|paths| normalize_permission_paths(paths, "file_system.write"));
Ok(PermissionProfile {
file_system: Some(FileSystemPermissions { read, write }),
..Default::default()
@@ -121,48 +116,25 @@ pub(crate) fn normalize_additional_permissions(
}
fn normalize_permission_paths(
paths: Vec<PathBuf>,
command_cwd: &Path,
permission_kind: &str,
) -> Result<Vec<PathBuf>, String> {
paths: Vec<AbsolutePathBuf>,
_permission_kind: &str,
) -> Vec<AbsolutePathBuf> {
let mut out = Vec::with_capacity(paths.len());
let mut seen = HashSet::new();
for path in paths {
if path.as_os_str().is_empty() {
return Err(format!("{permission_kind} contains an empty path"));
}
let resolved = if path.is_absolute() {
AbsolutePathBuf::from_absolute_path(path.clone()).map_err(|err| {
format!(
"{permission_kind} path `{}` is invalid: {err}",
path.display()
)
})?
} else {
AbsolutePathBuf::resolve_path_against_base(&path, command_cwd).map_err(|err| {
format!(
"{permission_kind} path `{}` cannot be resolved against cwd `{}`: {err}",
path.display(),
command_cwd.display()
)
})?
};
let canonicalized = resolved
let canonicalized = path
.as_path()
.canonicalize()
.ok()
.and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
.unwrap_or(resolved);
let canonicalized = canonicalized.to_path_buf();
.unwrap_or(path);
if seen.insert(canonicalized.clone()) {
out.push(canonicalized);
}
}
Ok(out)
out
}
fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
@@ -178,37 +150,23 @@ fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
fn additional_permission_roots(
additional_permissions: &PermissionProfile,
) -> Result<(Vec<AbsolutePathBuf>, Vec<AbsolutePathBuf>), SandboxTransformError> {
let to_abs = |paths: &[PathBuf]| {
let mut out = Vec::with_capacity(paths.len());
for path in paths {
let absolute = AbsolutePathBuf::from_absolute_path(path.clone()).map_err(|err| {
SandboxTransformError::InvalidAdditionalPermissionsPath(format!(
"`{}`: {err}",
path.display()
))
})?;
out.push(absolute);
}
Ok(dedup_absolute_paths(out))
};
Ok((
to_abs(
) -> (Vec<AbsolutePathBuf>, Vec<AbsolutePathBuf>) {
(
dedup_absolute_paths(
additional_permissions
.file_system
.as_ref()
.and_then(|file_system| file_system.read.as_deref())
.and_then(|file_system| file_system.read.clone())
.unwrap_or_default(),
)?,
to_abs(
),
dedup_absolute_paths(
additional_permissions
.file_system
.as_ref()
.and_then(|file_system| file_system.write.as_deref())
.and_then(|file_system| file_system.write.clone())
.unwrap_or_default(),
)?,
))
),
)
}
fn merge_read_only_access_with_additional_reads(
@@ -239,7 +197,7 @@ fn sandbox_policy_with_additional_permissions(
return Ok(sandbox_policy.clone());
}
let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions)?;
let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions);
let policy = match sandbox_policy {
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
codex-rs/core/src/skills/loader.rs
+21 -11
View File
@@ -17,6 +17,7 @@ use crate::skills::system::system_cache_root_dir;
use codex_app_server_protocol::ConfigLayerSource;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use dirs::home_dir;
use dunce::canonicalize as canonicalize_path;
use serde::Deserialize;
@@ -573,15 +574,18 @@ fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata {
}
};
let parsed: SkillMetadataFile = match serde_yaml::from_str(&contents) {
Ok(parsed) => parsed,
Err(error) => {
tracing::warn!(
"ignoring {path}: invalid {label}: {error}",
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
let parsed: SkillMetadataFile = {
let _guard = AbsolutePathBufGuard::new(skill_dir);
match serde_yaml::from_str(&contents) {
Ok(parsed) => parsed,
Err(error) => {
tracing::warn!(
"ignoring {path}: invalid {label}: {error}",
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
}
}
};
@@ -1376,8 +1380,14 @@ permissions:
Some(PermissionProfile {
network: Some(true),
file_system: Some(FileSystemPermissions {
read: Some(vec![PathBuf::from("./data")]),
write: Some(vec![PathBuf::from("./output")]),
read: Some(vec![
AbsolutePathBuf::try_from(normalized(skill_dir.join("data").as_path()))
.expect("absolute data path"),
]),
write: Some(vec![
AbsolutePathBuf::try_from(normalized(skill_dir.join("output").as_path()))
.expect("absolute output path"),
]),
}),
macos: None,
})
codex-rs/core/src/skills/permissions.rs
+15 -69
View File
@@ -1,7 +1,5 @@
use std::collections::HashSet;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
#[cfg(target_os = "macos")]
use codex_protocol::models::MacOsAutomationValue;
@@ -11,7 +9,6 @@ use codex_protocol::models::MacOsPreferencesValue;
use codex_protocol::models::MacOsSeatbeltProfileExtensions;
use codex_protocol::models::PermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use dirs::home_dir;
use dunce::canonicalize as canonicalize_path;
use tracing::warn;
@@ -23,7 +20,7 @@ use crate::protocol::ReadOnlyAccess;
use crate::protocol::SandboxPolicy;
pub(crate) fn compile_permission_profile(
skill_dir: &Path,
_skill_dir: &Path,
permissions: Option<PermissionProfile>,
) -> Option<Permissions> {
let PermissionProfile {
@@ -33,12 +30,10 @@ pub(crate) fn compile_permission_profile(
} = permissions?;
let file_system = file_system.unwrap_or_default();
let fs_read = normalize_permission_paths(
skill_dir,
file_system.read.as_deref().unwrap_or_default(),
"permissions.file_system.read",
);
let fs_write = normalize_permission_paths(
skill_dir,
file_system.write.as_deref().unwrap_or_default(),
"permissions.file_system.write",
);
@@ -83,16 +78,12 @@ pub(crate) fn compile_permission_profile(
})
}
fn normalize_permission_paths(
skill_dir: &Path,
values: &[PathBuf],
field: &str,
) -> Vec<AbsolutePathBuf> {
fn normalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec<AbsolutePathBuf> {
let mut paths = Vec::new();
let mut seen = HashSet::new();
for value in values {
let Some(path) = normalize_permission_path(skill_dir, value, field) else {
let Some(path) = normalize_permission_path(value, field) else {
continue;
};
if seen.insert(path.clone()) {
@@ -103,26 +94,8 @@ fn normalize_permission_paths(
paths
}
fn normalize_permission_path(
skill_dir: &Path,
value: &Path,
field: &str,
) -> Option<AbsolutePathBuf> {
let value = value.to_string_lossy();
let trimmed = value.trim();
if trimmed.is_empty() {
warn!("ignoring {field}: value is empty");
return None;
}
let expanded = expand_home(trimmed);
let absolute = if expanded.is_absolute() {
expanded
} else {
skill_dir.join(expanded)
};
let normalized = normalize_lexically(&absolute);
let canonicalized = canonicalize_path(&normalized).unwrap_or(normalized);
fn normalize_permission_path(value: &AbsolutePathBuf, field: &str) -> Option<AbsolutePathBuf> {
let canonicalized = canonicalize_path(value.as_path()).unwrap_or_else(|_| value.to_path_buf());
match AbsolutePathBuf::from_absolute_path(&canonicalized) {
Ok(path) => Some(path),
Err(error) => {
@@ -132,21 +105,6 @@ fn normalize_permission_path(
}
}
fn expand_home(path: &str) -> PathBuf {
if path == "~" {
if let Some(home) = home_dir() {
return home;
}
return PathBuf::from(path);
}
if let Some(rest) = path.strip_prefix("~/")
&& let Some(home) = home_dir()
{
return home.join(rest);
}
PathBuf::from(path)
}
#[cfg(target_os = "macos")]
fn build_macos_seatbelt_profile_extensions(
permissions: &MacOsPermissions,
@@ -233,22 +191,6 @@ fn build_macos_seatbelt_profile_extensions(
None
}
fn normalize_lexically(path: &Path) -> PathBuf {
let mut normalized = PathBuf::new();
for component in path.components() {
match component {
Component::CurDir => {}
Component::ParentDir => {
normalized.pop();
}
Component::RootDir | Component::Prefix(_) | Component::Normal(_) => {
normalized.push(component.as_os_str());
}
}
}
normalized
}
#[cfg(test)]
mod tests {
use super::compile_permission_profile;
@@ -269,7 +211,11 @@ mod tests {
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::fs;
use std::path::PathBuf;
use std::path::Path;
fn absolute_path(path: &Path) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(path).expect("absolute path")
}
#[test]
fn compile_permission_profile_normalizes_paths() {
@@ -285,11 +231,11 @@ mod tests {
network: Some(true),
file_system: Some(FileSystemPermissions {
read: Some(vec![
PathBuf::from("./data"),
PathBuf::from("./data"),
PathBuf::from("scripts/../data"),
absolute_path(&skill_dir.join("data")),
absolute_path(&skill_dir.join("data")),
absolute_path(&skill_dir.join("scripts/../data")),
]),
write: Some(vec![PathBuf::from("./output")]),
write: Some(vec![absolute_path(&skill_dir.join("output"))]),
}),
..Default::default()
}),
@@ -389,7 +335,7 @@ mod tests {
Some(PermissionProfile {
network: Some(true),
file_system: Some(FileSystemPermissions {
read: Some(vec![PathBuf::from("./data")]),
read: Some(vec![absolute_path(&skill_dir.join("data"))]),
write: Some(Vec::new()),
}),
..Default::default()
codex-rs/core/src/tools/handlers/mod.rs
+2 -2
View File
@@ -63,7 +63,7 @@ pub(super) fn normalize_and_validate_additional_permissions(
approval_policy: AskForApproval,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<PermissionProfile>,
cwd: &Path,
_cwd: &Path,
) -> Result<Option<PermissionProfile>, String> {
let uses_additional_permissions = matches!(
sandbox_permissions,
@@ -91,7 +91,7 @@ pub(super) fn normalize_and_validate_additional_permissions(
.to_string(),
);
};
let normalized = normalize_additional_permissions(additional_permissions, cwd)?;
let normalized = normalize_additional_permissions(additional_permissions)?;
if normalized.is_empty() {
return Err(
"`additional_permissions` must include at least one path in `file_system.read` or `file_system.write`"
codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs
+3 -2
View File
@@ -36,7 +36,6 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
#[cfg(target_os = "macos")]
use std::collections::HashMap;
use std::path::PathBuf;
use std::time::Duration;
#[test]
@@ -152,7 +151,9 @@ fn shell_request_escalation_execution_is_explicit() {
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: None,
write: Some(vec![PathBuf::from("./output")]),
write: Some(vec![
AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(),
]),
}),
..Default::default()
};
codex-rs/core/tests/suite/request_permissions.rs
+17 -7
View File
@@ -13,6 +13,7 @@ use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
@@ -31,6 +32,11 @@ use regex_lite::Regex;
use serde_json::Value;
use serde_json::json;
use std::fs;
use std::path::Path;
fn absolute_path(path: &Path) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(path).expect("absolute path")
}
struct CommandResult {
exit_code: Option<i64>,
@@ -187,7 +193,7 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![requested_write.clone()]),
write: Some(vec![absolute_path(&requested_write)]),
}),
..Default::default()
};
@@ -272,7 +278,7 @@ async fn read_only_with_additional_permissions_widens_to_unrequested_cwd_write()
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![requested_write.clone()]),
write: Some(vec![absolute_path(&requested_write)]),
}),
..Default::default()
};
@@ -363,7 +369,7 @@ async fn read_only_with_additional_permissions_widens_to_unrequested_tmp_write()
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![requested_write.clone()]),
write: Some(vec![absolute_path(&requested_write)]),
}),
..Default::default()
};
@@ -454,14 +460,16 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![outside_dir.path().to_path_buf()]),
write: Some(vec![absolute_path(outside_dir.path())]),
}),
..Default::default()
};
let normalized_requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![outside_dir.path().canonicalize()?]),
write: Some(vec![AbsolutePathBuf::try_from(
outside_dir.path().canonicalize()?,
)?]),
}),
..Default::default()
};
@@ -548,14 +556,16 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![outside_dir.path().to_path_buf()]),
write: Some(vec![absolute_path(outside_dir.path())]),
}),
..Default::default()
};
let normalized_requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(vec![outside_dir.path().canonicalize()?]),
write: Some(vec![AbsolutePathBuf::try_from(
outside_dir.path().canonicalize()?,
)?]),
}),
..Default::default()
};
codex-rs/core/tests/suite/skill_approval.rs
+14 -3
View File
@@ -13,6 +13,7 @@ use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::responses::mount_function_call_agent_response;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
@@ -26,6 +27,10 @@ use std::fs;
use std::path::Path;
use std::path::PathBuf;
fn absolute_path(path: &Path) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(path).expect("absolute path")
}
fn write_skill_metadata(home: &Path, name: &str, contents: &str) -> Result<()> {
let metadata_dir = home.join("skills").join(name).join("agents");
fs::create_dir_all(&metadata_dir)?;
@@ -330,8 +335,14 @@ permissions:
approval.additional_permissions,
Some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![PathBuf::from("./data")]),
write: Some(vec![PathBuf::from("./output")]),
read: Some(vec![absolute_path(
&test.codex_home_path().join("skills/mbolin-test-skill/data"),
)]),
write: Some(vec![absolute_path(
&test
.codex_home_path()
.join("skills/mbolin-test-skill/output"),
)]),
}),
..Default::default()
})
@@ -590,7 +601,7 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R
Some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: None,
write: Some(vec![allowed_dir.clone()]),
write: Some(vec![absolute_path(&allowed_dir)]),
}),
..Default::default()
})
codex-rs/protocol/src/models.rs
+3 -3
View File
@@ -1,6 +1,5 @@
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
use codex_utils_image::load_and_resize_to_fit;
use serde::Deserialize;
@@ -20,6 +19,7 @@ use crate::protocol::WritableRoot;
use crate::user_input::UserInput;
use codex_execpolicy::Policy;
use codex_git::GhostCommit;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_image::error::ImageProcessingError;
use schemars::JsonSchema;
@@ -54,8 +54,8 @@ impl SandboxPermissions {
#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
pub struct FileSystemPermissions {
pub read: Option<Vec<PathBuf>>,
pub write: Option<Vec<PathBuf>>,
pub read: Option<Vec<AbsolutePathBuf>>,
pub write: Option<Vec<AbsolutePathBuf>>,
}
impl FileSystemPermissions {
codex-rs/tui/src/bottom_pane/approval_overlay.rs
+11 -6
View File
@@ -614,10 +614,15 @@ mod tests {
use codex_protocol::protocol::ExecPolicyAmendment;
use codex_protocol::protocol::NetworkApprovalProtocol;
use codex_protocol::protocol::NetworkPolicyAmendment;
use codex_utils_absolute_path::AbsolutePathBuf;
use insta::assert_snapshot;
use pretty_assertions::assert_eq;
use tokio::sync::mpsc::unbounded_channel;
fn absolute_path(path: &str) -> AbsolutePathBuf {
AbsolutePathBuf::from_absolute_path(path).expect("absolute path")
}
fn render_overlay_lines(view: &ApprovalOverlay, width: u16) -> String {
let height = view.desired_height(width);
let mut buf = Buffer::empty(Rect::new(0, 0, width, height));
@@ -851,8 +856,8 @@ mod tests {
fn additional_permissions_exec_options_hide_execpolicy_amendment() {
let additional_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![PathBuf::from("/tmp/readme.txt")]),
write: Some(vec![PathBuf::from("/tmp/out.txt")]),
read: Some(vec![absolute_path("/tmp/readme.txt")]),
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
..Default::default()
};
@@ -884,8 +889,8 @@ mod tests {
network_approval_context: None,
additional_permissions: Some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![PathBuf::from("/tmp/readme.txt")]),
write: Some(vec![PathBuf::from("/tmp/out.txt")]),
read: Some(vec![absolute_path("/tmp/readme.txt")]),
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
..Default::default()
}),
@@ -923,8 +928,8 @@ mod tests {
network_approval_context: None,
additional_permissions: Some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![PathBuf::from("/tmp/readme.txt")]),
write: Some(vec![PathBuf::from("/tmp/out.txt")]),
read: Some(vec![absolute_path("/tmp/readme.txt")]),
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
..Default::default()
}),