mirror of
https://github.com/openai/codex.git
synced 2026-05-04 19:36:45 +00:00
protocol: preserve glob scan depth in permission profiles (#18713)
## Why #18274 made `PermissionProfile` the canonical file-system permissions shape, but the round-trip from `FileSystemSandboxPolicy` to `PermissionProfile` still dropped one piece of policy metadata: `glob_scan_max_depth`. That field is security-relevant for deny-read globs such as `**/*.env`. On Linux, bubblewrap sandbox construction uses it to bound unreadable glob expansion. If a profile copied from active runtime permissions loses this value and is submitted back as an override, the resulting `FileSystemSandboxPolicy` can behave differently even though the visible permission entries look equivalent. ## What changed - Add `glob_scan_max_depth` to protocol `FileSystemPermissions` and preserve it when converting to/from `FileSystemSandboxPolicy`. - Keep legacy `read`/`write` JSON for simple path-only permissions, but force canonical JSON when glob scan depth is present so the metadata is not silently dropped. - Carry `globScanMaxDepth` through app-server `AdditionalFileSystemPermissions`, generated JSON/TypeScript schemas, and app-server/TUI conversion call sites. - Preserve the metadata through sandboxing permission normalization, merging, and intersection. - Carry the merged scan depth into the effective `FileSystemSandboxPolicy` used for command execution, so bounded deny-read globs reach Linux bubblewrap materialization. ## Verification - `cargo test -p codex-sandboxing glob_scan -- --nocapture` - `cargo test -p codex-sandboxing policy_transforms -- --nocapture` - `just fix -p codex-sandboxing` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18713). * #18288 * #18287 * #18286 * #18285 * #18284 * #18283 * #18282 * #18281 * #18280 * #18279 * #18278 * #18277 * #18276 * #18275 * __->__ #18713
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::num::NonZeroUsize;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::LazyLock;
|
||||
@@ -146,6 +147,7 @@ impl SandboxPermissions {
|
||||
#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)]
|
||||
pub struct FileSystemPermissions {
|
||||
pub entries: Vec<FileSystemSandboxEntry>,
|
||||
pub glob_scan_max_depth: Option<NonZeroUsize>,
|
||||
}
|
||||
|
||||
pub type LegacyReadWriteRoots = (Option<Vec<AbsolutePathBuf>>, Option<Vec<AbsolutePathBuf>>);
|
||||
@@ -172,7 +174,10 @@ impl FileSystemPermissions {
|
||||
access: FileSystemAccessMode::Write,
|
||||
}));
|
||||
}
|
||||
Self { entries }
|
||||
Self {
|
||||
entries,
|
||||
glob_scan_max_depth: None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn explicit_path_entries(
|
||||
@@ -190,6 +195,10 @@ impl FileSystemPermissions {
|
||||
}
|
||||
|
||||
fn as_legacy_permissions(&self) -> Option<LegacyFileSystemPermissions> {
|
||||
if self.glob_scan_max_depth.is_some() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut read = Vec::new();
|
||||
let mut write = Vec::new();
|
||||
|
||||
@@ -225,6 +234,8 @@ struct LegacyFileSystemPermissions {
|
||||
struct CanonicalFileSystemPermissions {
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
entries: Vec<FileSystemSandboxEntry>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
glob_scan_max_depth: Option<NonZeroUsize>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
@@ -244,6 +255,7 @@ impl Serialize for FileSystemPermissions {
|
||||
} else {
|
||||
CanonicalFileSystemPermissions {
|
||||
entries: self.entries.clone(),
|
||||
glob_scan_max_depth: self.glob_scan_max_depth,
|
||||
}
|
||||
.serialize(serializer)
|
||||
}
|
||||
@@ -256,9 +268,13 @@ impl<'de> Deserialize<'de> for FileSystemPermissions {
|
||||
D: Deserializer<'de>,
|
||||
{
|
||||
match FileSystemPermissionsDe::deserialize(deserializer)? {
|
||||
FileSystemPermissionsDe::Canonical(CanonicalFileSystemPermissions { entries }) => {
|
||||
Ok(Self { entries })
|
||||
}
|
||||
FileSystemPermissionsDe::Canonical(CanonicalFileSystemPermissions {
|
||||
entries,
|
||||
glob_scan_max_depth,
|
||||
}) => Ok(Self {
|
||||
entries,
|
||||
glob_scan_max_depth,
|
||||
}),
|
||||
FileSystemPermissionsDe::Legacy(LegacyFileSystemPermissions { read, write }) => {
|
||||
Ok(Self::from_read_write_roots(read, write))
|
||||
}
|
||||
@@ -352,13 +368,18 @@ impl From<&FileSystemSandboxPolicy> for FileSystemPermissions {
|
||||
}]
|
||||
}
|
||||
};
|
||||
Self { entries }
|
||||
Self {
|
||||
entries,
|
||||
glob_scan_max_depth: value.glob_scan_max_depth.and_then(NonZeroUsize::new),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<&FileSystemPermissions> for FileSystemSandboxPolicy {
|
||||
fn from(value: &FileSystemPermissions) -> Self {
|
||||
FileSystemSandboxPolicy::restricted(value.entries.clone())
|
||||
let mut policy = FileSystemSandboxPolicy::restricted(value.entries.clone());
|
||||
policy.glob_scan_max_depth = value.glob_scan_max_depth.map(usize::from);
|
||||
policy
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1828,6 +1849,69 @@ mod tests {
|
||||
assert_eq!(permission_profile.is_empty(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_profile_round_trip_preserves_glob_scan_max_depth() {
|
||||
let mut file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
}]);
|
||||
file_system_sandbox_policy.glob_scan_max_depth = Some(2);
|
||||
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_sandbox_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
permission_profile.file_system_sandbox_policy(),
|
||||
file_system_sandbox_policy
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_system_permissions_with_glob_scan_depth_uses_canonical_json() -> Result<()> {
|
||||
let path = AbsolutePathBuf::try_from(PathBuf::from(if cfg!(windows) {
|
||||
r"C:\tmp\allowed"
|
||||
} else {
|
||||
"/tmp/allowed"
|
||||
}))
|
||||
.expect("absolute path");
|
||||
let file_system_permissions = FileSystemPermissions {
|
||||
entries: vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path },
|
||||
access: FileSystemAccessMode::Read,
|
||||
}],
|
||||
glob_scan_max_depth: NonZeroUsize::new(2),
|
||||
};
|
||||
|
||||
let serialized = serde_json::to_value(&file_system_permissions)?;
|
||||
|
||||
assert_eq!(serialized.get("read"), None);
|
||||
assert_eq!(serialized.get("write"), None);
|
||||
assert_eq!(
|
||||
serialized.get("glob_scan_max_depth"),
|
||||
Some(&serde_json::json!(2))
|
||||
);
|
||||
assert!(serialized.get("entries").is_some());
|
||||
assert_eq!(
|
||||
serde_json::from_value::<FileSystemPermissions>(serialized)?,
|
||||
file_system_permissions
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_system_permissions_rejects_zero_glob_scan_depth() {
|
||||
serde_json::from_value::<FileSystemPermissions>(serde_json::json!({
|
||||
"entries": [],
|
||||
"glob_scan_max_depth": 0,
|
||||
}))
|
||||
.expect_err("zero glob scan depth should fail deserialization");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn convert_mcp_content_to_items_builds_data_urls_when_missing_prefix() {
|
||||
let contents = vec![serde_json::json!({
|
||||
|
||||
Reference in New Issue
Block a user