chore: introduce SandboxPolicy::WorkspaceWrite::include_default_writable_roots (#1785)
Without this change, it is challenging to create integration tests to verify that the folders not included in `writable_roots` in `SandboxPolicy::WorkspaceWrite` are read-only because, by default, `get_writable_roots_with_cwd()` includes `TMPDIR`, which is where most integrationt tests do their work. This introduces a `use_exact_writable_roots` option to disable the default includes returned by `get_writable_roots_with_cwd()`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1785). * #1765 * __->__ #1785
This commit is contained in:
@@ -7,6 +7,7 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String {
|
|||||||
SandboxPolicy::WorkspaceWrite {
|
SandboxPolicy::WorkspaceWrite {
|
||||||
writable_roots,
|
writable_roots,
|
||||||
network_access,
|
network_access,
|
||||||
|
include_default_writable_roots,
|
||||||
} => {
|
} => {
|
||||||
let mut summary = "workspace-write".to_string();
|
let mut summary = "workspace-write".to_string();
|
||||||
if !writable_roots.is_empty() {
|
if !writable_roots.is_empty() {
|
||||||
@@ -19,6 +20,9 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String {
|
|||||||
.join(", ")
|
.join(", ")
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
if !*include_default_writable_roots {
|
||||||
|
summary.push_str(" (exact writable roots)");
|
||||||
|
}
|
||||||
if *network_access {
|
if *network_access {
|
||||||
summary.push_str(" (network access enabled)");
|
summary.push_str(" (network access enabled)");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -356,6 +356,7 @@ impl ConfigToml {
|
|||||||
Some(s) => SandboxPolicy::WorkspaceWrite {
|
Some(s) => SandboxPolicy::WorkspaceWrite {
|
||||||
writable_roots: s.writable_roots.clone(),
|
writable_roots: s.writable_roots.clone(),
|
||||||
network_access: s.network_access,
|
network_access: s.network_access,
|
||||||
|
include_default_writable_roots: true,
|
||||||
},
|
},
|
||||||
None => SandboxPolicy::new_workspace_write_policy(),
|
None => SandboxPolicy::new_workspace_write_policy(),
|
||||||
},
|
},
|
||||||
@@ -727,6 +728,7 @@ writable_roots = [
|
|||||||
SandboxPolicy::WorkspaceWrite {
|
SandboxPolicy::WorkspaceWrite {
|
||||||
writable_roots: vec![PathBuf::from("/tmp")],
|
writable_roots: vec![PathBuf::from("/tmp")],
|
||||||
network_access: false,
|
network_access: false,
|
||||||
|
include_default_writable_roots: true,
|
||||||
},
|
},
|
||||||
sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override)
|
sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override)
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -180,9 +180,19 @@ pub enum SandboxPolicy {
|
|||||||
/// default.
|
/// default.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
network_access: bool,
|
network_access: bool,
|
||||||
|
|
||||||
|
/// When set to `true`, will include defaults like the current working
|
||||||
|
/// directory and TMPDIR (on macOS). When `false`, only `writable_roots`
|
||||||
|
/// are used. (Mainly used for testing.)
|
||||||
|
#[serde(default = "default_true")]
|
||||||
|
include_default_writable_roots: bool,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn default_true() -> bool {
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
impl FromStr for SandboxPolicy {
|
impl FromStr for SandboxPolicy {
|
||||||
type Err = serde_json::Error;
|
type Err = serde_json::Error;
|
||||||
|
|
||||||
@@ -204,6 +214,7 @@ impl SandboxPolicy {
|
|||||||
SandboxPolicy::WorkspaceWrite {
|
SandboxPolicy::WorkspaceWrite {
|
||||||
writable_roots: vec![],
|
writable_roots: vec![],
|
||||||
network_access: false,
|
network_access: false,
|
||||||
|
include_default_writable_roots: true,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -235,7 +246,15 @@ impl SandboxPolicy {
|
|||||||
match self {
|
match self {
|
||||||
SandboxPolicy::DangerFullAccess => Vec::new(),
|
SandboxPolicy::DangerFullAccess => Vec::new(),
|
||||||
SandboxPolicy::ReadOnly => Vec::new(),
|
SandboxPolicy::ReadOnly => Vec::new(),
|
||||||
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
|
SandboxPolicy::WorkspaceWrite {
|
||||||
|
writable_roots,
|
||||||
|
include_default_writable_roots,
|
||||||
|
..
|
||||||
|
} => {
|
||||||
|
if !*include_default_writable_roots {
|
||||||
|
return writable_roots.clone();
|
||||||
|
}
|
||||||
|
|
||||||
let mut roots = writable_roots.clone();
|
let mut roots = writable_roots.clone();
|
||||||
roots.push(cwd.to_path_buf());
|
roots.push(cwd.to_path_buf());
|
||||||
|
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
|||||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||||
writable_roots: writable_roots.to_vec(),
|
writable_roots: writable_roots.to_vec(),
|
||||||
network_access: false,
|
network_access: false,
|
||||||
|
include_default_writable_roots: true,
|
||||||
};
|
};
|
||||||
let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox");
|
let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox");
|
||||||
let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program));
|
let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program));
|
||||||
|
|||||||
Reference in New Issue
Block a user