mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
protocol: derive effective file access from filesystem policies
This commit is contained in:
Michael Bolin
@@ -1,4 +1,5 @@
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::OsStr;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -8,11 +9,13 @@ use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use strum_macros::Display;
|
||||
use tracing::error;
|
||||
use ts_rs::TS;
|
||||
|
||||
use crate::protocol::NetworkAccess;
|
||||
use crate::protocol::ReadOnlyAccess;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::protocol::WritableRoot;
|
||||
|
||||
#[derive(
|
||||
Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS,
|
||||
@@ -141,6 +144,114 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns true when filesystem reads are unrestricted.
|
||||
pub fn has_full_disk_read_access(&self) -> bool {
|
||||
match self.kind {
|
||||
FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
|
||||
FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| {
|
||||
matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Special { value }
|
||||
if matches!(value, FileSystemSpecialPath::Root) && entry.access.can_read()
|
||||
)
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns true when filesystem writes are unrestricted.
|
||||
pub fn has_full_disk_write_access(&self) -> bool {
|
||||
match self.kind {
|
||||
FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
|
||||
FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| {
|
||||
matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Special { value }
|
||||
if matches!(value, FileSystemSpecialPath::Root)
|
||||
&& entry.access.can_write()
|
||||
)
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns true when platform-default readable roots should be included.
|
||||
pub fn include_platform_defaults(&self) -> bool {
|
||||
!self.has_full_disk_read_access()
|
||||
&& matches!(self.kind, FileSystemSandboxKind::Restricted)
|
||||
&& self.entries.iter().any(|entry| {
|
||||
matches!(
|
||||
&entry.path,
|
||||
FileSystemPath::Special { value }
|
||||
if matches!(value, FileSystemSpecialPath::Minimal)
|
||||
&& entry.access.can_read()
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns the explicit readable roots resolved against the provided cwd.
|
||||
pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
|
||||
if self.has_full_disk_read_access() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
dedup_absolute_paths(
|
||||
self.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.access.can_read())
|
||||
.filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns the writable roots together with read-only carveouts resolved
|
||||
/// against the provided cwd.
|
||||
pub fn get_writable_roots_with_cwd(&self, cwd: &Path) -> Vec<WritableRoot> {
|
||||
if self.has_full_disk_write_access() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
let unreadable_roots = self.get_unreadable_roots_with_cwd(cwd);
|
||||
dedup_absolute_paths(
|
||||
self.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.access.can_write())
|
||||
.filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
|
||||
.collect(),
|
||||
)
|
||||
.into_iter()
|
||||
.map(|root| {
|
||||
let mut read_only_subpaths = default_read_only_subpaths_for_writable_root(&root);
|
||||
read_only_subpaths.extend(
|
||||
unreadable_roots
|
||||
.iter()
|
||||
.filter(|path| path.as_path().starts_with(root.as_path()))
|
||||
.cloned(),
|
||||
);
|
||||
WritableRoot {
|
||||
root,
|
||||
read_only_subpaths: dedup_absolute_paths(read_only_subpaths),
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Returns explicit unreadable roots resolved against the provided cwd.
|
||||
pub fn get_unreadable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
|
||||
if !matches!(self.kind, FileSystemSandboxKind::Restricted) {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
dedup_absolute_paths(
|
||||
self.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.access == FileSystemAccessMode::None)
|
||||
.filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn to_legacy_sandbox_policy(
|
||||
&self,
|
||||
network_policy: NetworkSandboxPolicy,
|
||||
@@ -422,6 +533,16 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy {
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_file_system_path(
|
||||
path: &FileSystemPath,
|
||||
cwd: Option<&AbsolutePathBuf>,
|
||||
) -> Option<AbsolutePathBuf> {
|
||||
match path {
|
||||
FileSystemPath::Path { path } => Some(path.clone()),
|
||||
FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd),
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_file_system_special_path(
|
||||
value: &FileSystemSpecialPath,
|
||||
cwd: Option<&AbsolutePathBuf>,
|
||||
@@ -471,3 +592,104 @@ fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
|
||||
}
|
||||
deduped
|
||||
}
|
||||
|
||||
fn default_read_only_subpaths_for_writable_root(
|
||||
writable_root: &AbsolutePathBuf,
|
||||
) -> Vec<AbsolutePathBuf> {
|
||||
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_git = writable_root
|
||||
.join(".git")
|
||||
.expect(".git is a valid relative path");
|
||||
// This applies to typical repos (directory .git), worktrees/submodules
|
||||
// (file .git with gitdir pointer), and bare repos when the gitdir is the
|
||||
// writable root itself.
|
||||
let top_level_git_is_file = top_level_git.as_path().is_file();
|
||||
let top_level_git_is_dir = top_level_git.as_path().is_dir();
|
||||
if top_level_git_is_dir || top_level_git_is_file {
|
||||
if top_level_git_is_file
|
||||
&& is_git_pointer_file(&top_level_git)
|
||||
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
|
||||
{
|
||||
subpaths.push(gitdir);
|
||||
}
|
||||
subpaths.push(top_level_git);
|
||||
}
|
||||
|
||||
// Make .agents/skills and .codex/config.toml and related files read-only
|
||||
// to the agent, by default.
|
||||
for subdir in &[".agents", ".codex"] {
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex = writable_root.join(subdir).expect("valid relative path");
|
||||
if top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
}
|
||||
|
||||
dedup_absolute_paths(subpaths)
|
||||
}
|
||||
|
||||
fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool {
|
||||
path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git"))
|
||||
}
|
||||
|
||||
fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option<AbsolutePathBuf> {
|
||||
let contents = match std::fs::read_to_string(dot_git.as_path()) {
|
||||
Ok(contents) => contents,
|
||||
Err(err) => {
|
||||
error!(
|
||||
"Failed to read {path} for gitdir pointer: {err}",
|
||||
path = dot_git.as_path().display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
|
||||
let trimmed = contents.trim();
|
||||
let (_, gitdir_raw) = match trimmed.split_once(':') {
|
||||
Some(parts) => parts,
|
||||
None => {
|
||||
error!(
|
||||
"Expected {path} to contain a gitdir pointer, but it did not match `gitdir: <path>`.",
|
||||
path = dot_git.as_path().display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let gitdir_raw = gitdir_raw.trim();
|
||||
if gitdir_raw.is_empty() {
|
||||
error!(
|
||||
"Expected {path} to contain a gitdir pointer, but it was empty.",
|
||||
path = dot_git.as_path().display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
let base = match dot_git.as_path().parent() {
|
||||
Some(base) => base,
|
||||
None => {
|
||||
error!(
|
||||
"Unable to resolve parent directory for {path}.",
|
||||
path = dot_git.as_path().display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let gitdir_path = match AbsolutePathBuf::resolve_path_against_base(gitdir_raw, base) {
|
||||
Ok(path) => path,
|
||||
Err(err) => {
|
||||
error!(
|
||||
"Failed to resolve gitdir path {gitdir_raw} from {path}: {err}",
|
||||
path = dot_git.as_path().display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
if !gitdir_path.as_path().exists() {
|
||||
error!(
|
||||
"Resolved gitdir path {path} does not exist.",
|
||||
path = gitdir_path.as_path().display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
Some(gitdir_path)
|
||||
}
|
||||
|
||||
@@ -61,6 +61,13 @@ pub use crate::approvals::NetworkApprovalContext;
|
||||
pub use crate::approvals::NetworkApprovalProtocol;
|
||||
pub use crate::approvals::NetworkPolicyAmendment;
|
||||
pub use crate::approvals::NetworkPolicyRuleAction;
|
||||
pub use crate::permissions::FileSystemAccessMode;
|
||||
pub use crate::permissions::FileSystemPath;
|
||||
pub use crate::permissions::FileSystemSandboxEntry;
|
||||
pub use crate::permissions::FileSystemSandboxKind;
|
||||
pub use crate::permissions::FileSystemSandboxPolicy;
|
||||
pub use crate::permissions::FileSystemSpecialPath;
|
||||
pub use crate::permissions::NetworkSandboxPolicy;
|
||||
pub use crate::request_user_input::RequestUserInputEvent;
|
||||
|
||||
/// Open/close tags for special user-input blocks. Used across crates to avoid
|
||||
@@ -542,7 +549,6 @@ impl NetworkAccess {
|
||||
matches!(self, NetworkAccess::Enabled)
|
||||
}
|
||||
}
|
||||
|
||||
fn default_include_platform_defaults() -> bool {
|
||||
true
|
||||
}
|
||||
@@ -883,45 +889,11 @@ impl SandboxPolicy {
|
||||
// For each root, compute subpaths that should remain read-only.
|
||||
roots
|
||||
.into_iter()
|
||||
.map(|writable_root| {
|
||||
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_git = writable_root
|
||||
.join(".git")
|
||||
.expect(".git is a valid relative path");
|
||||
// This applies to typical repos (directory .git), worktrees/submodules
|
||||
// (file .git with gitdir pointer), and bare repos when the gitdir is the
|
||||
// writable root itself.
|
||||
let top_level_git_is_file = top_level_git.as_path().is_file();
|
||||
let top_level_git_is_dir = top_level_git.as_path().is_dir();
|
||||
if top_level_git_is_dir || top_level_git_is_file {
|
||||
if top_level_git_is_file
|
||||
&& is_git_pointer_file(&top_level_git)
|
||||
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
|
||||
&& !subpaths
|
||||
.iter()
|
||||
.any(|subpath| subpath.as_path() == gitdir.as_path())
|
||||
{
|
||||
subpaths.push(gitdir);
|
||||
}
|
||||
subpaths.push(top_level_git);
|
||||
}
|
||||
|
||||
// Make .agents/skills and .codex/config.toml and
|
||||
// related files read-only to the agent, by default.
|
||||
for subdir in &[".agents", ".codex"] {
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex =
|
||||
writable_root.join(subdir).expect("valid relative path");
|
||||
if top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
}
|
||||
|
||||
WritableRoot {
|
||||
root: writable_root,
|
||||
read_only_subpaths: subpaths,
|
||||
}
|
||||
.map(|writable_root| WritableRoot {
|
||||
read_only_subpaths: default_read_only_subpaths_for_writable_root(
|
||||
&writable_root,
|
||||
),
|
||||
root: writable_root,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -929,6 +901,49 @@ impl SandboxPolicy {
|
||||
}
|
||||
}
|
||||
|
||||
fn default_read_only_subpaths_for_writable_root(
|
||||
writable_root: &AbsolutePathBuf,
|
||||
) -> Vec<AbsolutePathBuf> {
|
||||
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_git = writable_root
|
||||
.join(".git")
|
||||
.expect(".git is a valid relative path");
|
||||
// This applies to typical repos (directory .git), worktrees/submodules
|
||||
// (file .git with gitdir pointer), and bare repos when the gitdir is the
|
||||
// writable root itself.
|
||||
let top_level_git_is_file = top_level_git.as_path().is_file();
|
||||
let top_level_git_is_dir = top_level_git.as_path().is_dir();
|
||||
if top_level_git_is_dir || top_level_git_is_file {
|
||||
if top_level_git_is_file
|
||||
&& is_git_pointer_file(&top_level_git)
|
||||
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
|
||||
{
|
||||
subpaths.push(gitdir);
|
||||
}
|
||||
subpaths.push(top_level_git);
|
||||
}
|
||||
|
||||
// Make .agents/skills and .codex/config.toml and related files read-only
|
||||
// to the agent, by default.
|
||||
for subdir in &[".agents", ".codex"] {
|
||||
#[allow(clippy::expect_used)]
|
||||
let top_level_codex = writable_root.join(subdir).expect("valid relative path");
|
||||
if top_level_codex.as_path().is_dir() {
|
||||
subpaths.push(top_level_codex);
|
||||
}
|
||||
}
|
||||
|
||||
let mut deduped = Vec::with_capacity(subpaths.len());
|
||||
let mut seen = HashSet::new();
|
||||
for path in subpaths {
|
||||
if seen.insert(path.to_path_buf()) {
|
||||
deduped.push(path);
|
||||
}
|
||||
}
|
||||
deduped
|
||||
}
|
||||
|
||||
fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool {
|
||||
path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git"))
|
||||
}
|
||||
@@ -3156,11 +3171,13 @@ mod tests {
|
||||
use crate::permissions::FileSystemPath;
|
||||
use crate::permissions::FileSystemSandboxEntry;
|
||||
use crate::permissions::FileSystemSandboxPolicy;
|
||||
use crate::permissions::FileSystemSpecialPath;
|
||||
use crate::permissions::NetworkSandboxPolicy;
|
||||
use anyhow::Result;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use tempfile::NamedTempFile;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn external_sandbox_reports_full_access_flags() {
|
||||
@@ -3241,6 +3258,97 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restricted_file_system_policy_reports_full_access_from_root_entries() {
|
||||
let read_only = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}]);
|
||||
assert!(read_only.has_full_disk_read_access());
|
||||
assert!(!read_only.has_full_disk_write_access());
|
||||
assert!(!read_only.include_platform_defaults());
|
||||
|
||||
let writable = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
}]);
|
||||
assert!(writable.has_full_disk_read_access());
|
||||
assert!(writable.has_full_disk_write_access());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restricted_file_system_policy_derives_effective_paths() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
std::fs::create_dir_all(cwd.path().join(".agents")).expect("create .agents");
|
||||
std::fs::create_dir_all(cwd.path().join(".codex")).expect("create .codex");
|
||||
let cwd_absolute =
|
||||
AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir");
|
||||
let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path())
|
||||
.expect("resolve unreadable path");
|
||||
let agents = AbsolutePathBuf::resolve_path_against_base(".agents", cwd.path())
|
||||
.expect("resolve .agents");
|
||||
let codex = AbsolutePathBuf::resolve_path_against_base(".codex", cwd.path())
|
||||
.expect("resolve .codex");
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Minimal,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: secret.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
},
|
||||
]);
|
||||
|
||||
assert!(!policy.has_full_disk_read_access());
|
||||
assert!(!policy.has_full_disk_write_access());
|
||||
assert!(policy.include_platform_defaults());
|
||||
assert_eq!(
|
||||
policy.get_readable_roots_with_cwd(cwd.path()),
|
||||
vec![cwd_absolute]
|
||||
);
|
||||
assert_eq!(
|
||||
policy.get_unreadable_roots_with_cwd(cwd.path()),
|
||||
vec![secret.clone()]
|
||||
);
|
||||
|
||||
let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
|
||||
assert_eq!(writable_roots.len(), 1);
|
||||
assert_eq!(writable_roots[0].root.as_path(), cwd.path());
|
||||
assert!(
|
||||
writable_roots[0]
|
||||
.read_only_subpaths
|
||||
.iter()
|
||||
.any(|path| path.as_path() == secret.as_path())
|
||||
);
|
||||
assert!(
|
||||
writable_roots[0]
|
||||
.read_only_subpaths
|
||||
.iter()
|
||||
.any(|path| path.as_path() == agents.as_path())
|
||||
);
|
||||
assert!(
|
||||
writable_roots[0]
|
||||
.read_only_subpaths
|
||||
.iter()
|
||||
.any(|path| path.as_path() == codex.as_path())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_system_policy_rejects_legacy_bridge_for_non_workspace_writes() {
|
||||
let cwd = if cfg!(windows) {
|
||||
|
||||
Reference in New Issue
Block a user