mirror of
https://github.com/openai/codex.git
synced 2026-04-29 02:41:12 +03:00
feat: make sandbox read access configurable with ReadOnlyAccess (#11387)
`SandboxPolicy::ReadOnly` previously implied broad read access and could
not express a narrower read surface.
This change introduces an explicit read-access model so we can support
user-configurable read restrictions in follow-up work, while preserving
current behavior today.
It also ensures unsupported backends fail closed for restricted-read
policies instead of silently granting broader access than intended.
## What
- Added `ReadOnlyAccess` in protocol with:
- `Restricted { include_platform_defaults, readable_roots }`
- `FullAccess`
- Updated `SandboxPolicy` to carry read-access configuration:
- `ReadOnly { access: ReadOnlyAccess }`
- `WorkspaceWrite { ..., read_only_access: ReadOnlyAccess }`
- Preserved existing behavior by defaulting current construction paths
to `ReadOnlyAccess::FullAccess`.
- Threaded the new fields through sandbox policy consumers and call
sites across `core`, `tui`, `linux-sandbox`, `windows-sandbox`, and
related tests.
- Updated Seatbelt policy generation to honor restricted read roots by
emitting scoped read rules when full read access is not granted.
- Added fail-closed behavior on Linux and Windows backends when
restricted read access is requested but not yet implemented there
(`UnsupportedOperation`).
- Regenerated app-server protocol schema and TypeScript artifacts,
including `ReadOnlyAccess`.
## Compatibility / rollout
- Runtime behavior remains unchanged by default (`FullAccess`).
- API/schema changes are in place so future config wiring can enable
restricted read access without another policy-shape migration.
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
//! between user and agent.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::OsStr;
|
||||
use std::fmt;
|
||||
use std::path::Path;
|
||||
@@ -382,6 +383,107 @@ impl NetworkAccess {
|
||||
}
|
||||
}
|
||||
|
||||
fn default_include_platform_defaults() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
/// Determines how read-only file access is granted inside a restricted
|
||||
/// sandbox.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS)]
|
||||
#[strum(serialize_all = "kebab-case")]
|
||||
#[serde(tag = "type", rename_all = "kebab-case")]
|
||||
#[ts(tag = "type")]
|
||||
pub enum ReadOnlyAccess {
|
||||
/// Restrict reads to an explicit set of roots.
|
||||
///
|
||||
/// When `include_platform_defaults` is `true`, platform defaults required
|
||||
/// for basic execution are included in addition to `readable_roots`.
|
||||
Restricted {
|
||||
/// Include built-in platform read roots required for basic process
|
||||
/// execution.
|
||||
#[serde(default = "default_include_platform_defaults")]
|
||||
include_platform_defaults: bool,
|
||||
/// Additional absolute roots that should be readable.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
readable_roots: Vec<AbsolutePathBuf>,
|
||||
},
|
||||
|
||||
/// Allow unrestricted file reads.
|
||||
#[default]
|
||||
FullAccess,
|
||||
}
|
||||
|
||||
impl ReadOnlyAccess {
|
||||
pub fn has_full_disk_read_access(&self) -> bool {
|
||||
matches!(self, ReadOnlyAccess::FullAccess)
|
||||
}
|
||||
|
||||
/// Returns the readable roots for restricted read access.
|
||||
///
|
||||
/// For [`ReadOnlyAccess::FullAccess`], returns an empty list because
|
||||
/// callers should grant blanket read access instead.
|
||||
pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
|
||||
let mut roots: Vec<AbsolutePathBuf> = match self {
|
||||
ReadOnlyAccess::FullAccess => return Vec::new(),
|
||||
ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults,
|
||||
readable_roots,
|
||||
} => {
|
||||
let mut roots = readable_roots.clone();
|
||||
if *include_platform_defaults {
|
||||
#[cfg(target_os = "macos")]
|
||||
for platform_path in [
|
||||
"/bin", "/dev", "/etc", "/Library", "/private", "/sbin", "/System", "/tmp",
|
||||
"/usr",
|
||||
] {
|
||||
#[allow(clippy::expect_used)]
|
||||
roots.push(
|
||||
AbsolutePathBuf::from_absolute_path(platform_path)
|
||||
.expect("platform defaults should be absolute"),
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
for platform_path in ["/bin", "/dev", "/etc", "/lib", "/lib64", "/tmp", "/usr"]
|
||||
{
|
||||
#[allow(clippy::expect_used)]
|
||||
roots.push(
|
||||
AbsolutePathBuf::from_absolute_path(platform_path)
|
||||
.expect("platform defaults should be absolute"),
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
for platform_path in [
|
||||
r"C:\Windows",
|
||||
r"C:\Program Files",
|
||||
r"C:\Program Files (x86)",
|
||||
r"C:\ProgramData",
|
||||
] {
|
||||
#[allow(clippy::expect_used)]
|
||||
roots.push(
|
||||
AbsolutePathBuf::from_absolute_path(platform_path)
|
||||
.expect("platform defaults should be absolute"),
|
||||
);
|
||||
}
|
||||
|
||||
match AbsolutePathBuf::from_absolute_path(cwd) {
|
||||
Ok(cwd_root) => roots.push(cwd_root),
|
||||
Err(err) => {
|
||||
error!("Ignoring invalid cwd {cwd:?} for sandbox readable root: {err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
roots
|
||||
}
|
||||
};
|
||||
|
||||
let mut seen = HashSet::new();
|
||||
roots.retain(|root| seen.insert(root.to_path_buf()));
|
||||
roots
|
||||
}
|
||||
}
|
||||
|
||||
/// Determines execution restrictions for model shell commands.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)]
|
||||
#[strum(serialize_all = "kebab-case")]
|
||||
@@ -391,9 +493,16 @@ pub enum SandboxPolicy {
|
||||
#[serde(rename = "danger-full-access")]
|
||||
DangerFullAccess,
|
||||
|
||||
/// Read-only access to the entire file-system.
|
||||
/// Read-only access configuration.
|
||||
#[serde(rename = "read-only")]
|
||||
ReadOnly,
|
||||
ReadOnly {
|
||||
/// Read access granted while running under this policy.
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access"
|
||||
)]
|
||||
access: ReadOnlyAccess,
|
||||
},
|
||||
|
||||
/// Indicates the process is already in an external sandbox. Allows full
|
||||
/// disk access while honoring the provided network setting.
|
||||
@@ -413,6 +522,13 @@ pub enum SandboxPolicy {
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
writable_roots: Vec<AbsolutePathBuf>,
|
||||
|
||||
/// Read access granted while running under this policy.
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access"
|
||||
)]
|
||||
read_only_access: ReadOnlyAccess,
|
||||
|
||||
/// When set to `true`, outbound network access is allowed. `false` by
|
||||
/// default.
|
||||
#[serde(default)]
|
||||
@@ -473,7 +589,9 @@ impl FromStr for SandboxPolicy {
|
||||
impl SandboxPolicy {
|
||||
/// Returns a policy with read-only disk access and no network.
|
||||
pub fn new_read_only_policy() -> Self {
|
||||
SandboxPolicy::ReadOnly
|
||||
SandboxPolicy::ReadOnly {
|
||||
access: ReadOnlyAccess::FullAccess,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a policy that can read the entire disk, but can only write to
|
||||
@@ -482,22 +600,29 @@ impl SandboxPolicy {
|
||||
pub fn new_workspace_write_policy() -> Self {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: ReadOnlyAccess::FullAccess,
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Always returns `true`; restricting read access is not supported.
|
||||
pub fn has_full_disk_read_access(&self) -> bool {
|
||||
true
|
||||
match self {
|
||||
SandboxPolicy::DangerFullAccess => true,
|
||||
SandboxPolicy::ExternalSandbox { .. } => true,
|
||||
SandboxPolicy::ReadOnly { access } => access.has_full_disk_read_access(),
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
read_only_access, ..
|
||||
} => read_only_access.has_full_disk_read_access(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn has_full_disk_write_access(&self) -> bool {
|
||||
match self {
|
||||
SandboxPolicy::DangerFullAccess => true,
|
||||
SandboxPolicy::ExternalSandbox { .. } => true,
|
||||
SandboxPolicy::ReadOnly => false,
|
||||
SandboxPolicy::ReadOnly { .. } => false,
|
||||
SandboxPolicy::WorkspaceWrite { .. } => false,
|
||||
}
|
||||
}
|
||||
@@ -506,11 +631,37 @@ impl SandboxPolicy {
|
||||
match self {
|
||||
SandboxPolicy::DangerFullAccess => true,
|
||||
SandboxPolicy::ExternalSandbox { network_access } => network_access.is_enabled(),
|
||||
SandboxPolicy::ReadOnly => false,
|
||||
SandboxPolicy::ReadOnly { .. } => false,
|
||||
SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the list of readable roots (tailored to the current working
|
||||
/// directory) when read access is restricted.
|
||||
///
|
||||
/// For policies with full read access, this returns an empty list because
|
||||
/// callers should grant blanket reads.
|
||||
pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
|
||||
let mut roots = match self {
|
||||
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => Vec::new(),
|
||||
SandboxPolicy::ReadOnly { access } => access.get_readable_roots_with_cwd(cwd),
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
read_only_access, ..
|
||||
} => {
|
||||
let mut roots = read_only_access.get_readable_roots_with_cwd(cwd);
|
||||
roots.extend(
|
||||
self.get_writable_roots_with_cwd(cwd)
|
||||
.into_iter()
|
||||
.map(|root| root.root),
|
||||
);
|
||||
roots
|
||||
}
|
||||
};
|
||||
let mut seen = HashSet::new();
|
||||
roots.retain(|root| seen.insert(root.to_path_buf()));
|
||||
roots
|
||||
}
|
||||
|
||||
/// Returns the list of writable roots (tailored to the current working
|
||||
/// directory) together with subpaths that should remain read‑only under
|
||||
/// each writable root.
|
||||
@@ -518,9 +669,10 @@ impl SandboxPolicy {
|
||||
match self {
|
||||
SandboxPolicy::DangerFullAccess => Vec::new(),
|
||||
SandboxPolicy::ExternalSandbox { .. } => Vec::new(),
|
||||
SandboxPolicy::ReadOnly => Vec::new(),
|
||||
SandboxPolicy::ReadOnly { .. } => Vec::new(),
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
read_only_access: _,
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
network_access: _,
|
||||
@@ -2565,6 +2717,38 @@ mod tests {
|
||||
assert!(enabled.has_full_network_access());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workspace_write_restricted_read_access_includes_effective_writable_roots() {
|
||||
let cwd = if cfg!(windows) {
|
||||
Path::new(r"C:\workspace")
|
||||
} else {
|
||||
Path::new("/tmp/workspace")
|
||||
};
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: false,
|
||||
readable_roots: vec![],
|
||||
},
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: false,
|
||||
};
|
||||
|
||||
let readable_roots = policy.get_readable_roots_with_cwd(cwd);
|
||||
let writable_roots = policy.get_writable_roots_with_cwd(cwd);
|
||||
|
||||
for writable_root in writable_roots {
|
||||
assert!(
|
||||
readable_roots
|
||||
.iter()
|
||||
.any(|root| root.as_path() == writable_root.root.as_path()),
|
||||
"expected writable root {} to also be readable",
|
||||
writable_root.root.as_path().display()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn item_started_event_from_web_search_emits_begin_event() {
|
||||
let event = ItemStartedEvent {
|
||||
@@ -2730,7 +2914,7 @@ mod tests {
|
||||
model: "codex-mini-latest".to_string(),
|
||||
model_provider_id: "openai".to_string(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
cwd: PathBuf::from("/home/user/project"),
|
||||
reasoning_effort: Some(ReasoningEffortConfig::default()),
|
||||
history_log_id: 0,
|
||||
|
||||
Reference in New Issue
Block a user