feat: add /tmp by default (#1919)
Replaces the `include_default_writable_roots` option on `sandbox_workspace_write` (that defaulted to `true`, which was slightly weird/annoying) with `exclude_tmpdir_env_var`, which defaults to `false`. Though perhaps more importantly `/tmp` is now enabled by default as part of `sandbox_mode = "workspace-write"`, though `exclude_slash_tmp = false` can be used to disable this.
This commit is contained in:
@@ -7,22 +7,26 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
network_access,
|
||||
include_default_writable_roots,
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
} => {
|
||||
let mut summary = "workspace-write".to_string();
|
||||
if !writable_roots.is_empty() {
|
||||
summary.push_str(&format!(
|
||||
" [{}]",
|
||||
writable_roots
|
||||
.iter()
|
||||
.map(|p| p.to_string_lossy())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
));
|
||||
|
||||
let mut writable_entries = Vec::<String>::new();
|
||||
writable_entries.push("workdir".to_string());
|
||||
if !*exclude_slash_tmp {
|
||||
writable_entries.push("/tmp".to_string());
|
||||
}
|
||||
if !*include_default_writable_roots {
|
||||
summary.push_str(" (exact writable roots)");
|
||||
if !*exclude_tmpdir_env_var {
|
||||
writable_entries.push("$TMPDIR".to_string());
|
||||
}
|
||||
writable_entries.extend(
|
||||
writable_roots
|
||||
.iter()
|
||||
.map(|p| p.to_string_lossy().to_string()),
|
||||
);
|
||||
|
||||
summary.push_str(&format!(" [{}]", writable_entries.join(", ")));
|
||||
if *network_access {
|
||||
summary.push_str(" (network access enabled)");
|
||||
}
|
||||
|
||||
@@ -275,9 +275,12 @@ sandbox_mode = "workspace-write"
|
||||
|
||||
# Extra settings that only apply when `sandbox = "workspace-write"`.
|
||||
[sandbox_workspace_write]
|
||||
# By default, only the cwd for the Codex session will be writable (and $TMPDIR
|
||||
# on macOS), but you can specify additional writable folders in this array.
|
||||
writable_roots = ["/tmp"]
|
||||
# By default, the cwd for the Codex session will be writable as well as $TMPDIR
|
||||
# if set) and /tmp (if it exists). Setting the respective options to `true`
|
||||
# will override those defaults.
|
||||
exclude_tmpdir_env_var = false
|
||||
exclude_slash_tmp = false
|
||||
|
||||
# Allow the command being run inside the sandbox to make outbound network
|
||||
# requests. Disabled by default.
|
||||
network_access = false
|
||||
|
||||
@@ -361,10 +361,16 @@ impl ConfigToml {
|
||||
match resolved_sandbox_mode {
|
||||
SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(),
|
||||
SandboxMode::WorkspaceWrite => match self.sandbox_workspace_write.as_ref() {
|
||||
Some(s) => SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: s.writable_roots.clone(),
|
||||
network_access: s.network_access,
|
||||
include_default_writable_roots: true,
|
||||
Some(SandboxWorkplaceWrite {
|
||||
writable_roots,
|
||||
network_access,
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
}) => SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: writable_roots.clone(),
|
||||
network_access: *network_access,
|
||||
exclude_tmpdir_env_var: *exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp: *exclude_slash_tmp,
|
||||
},
|
||||
None => SandboxPolicy::new_workspace_write_policy(),
|
||||
},
|
||||
@@ -745,8 +751,10 @@ sandbox_mode = "workspace-write"
|
||||
|
||||
[sandbox_workspace_write]
|
||||
writable_roots = [
|
||||
"/tmp",
|
||||
"/my/workspace",
|
||||
]
|
||||
exclude_tmpdir_env_var = true
|
||||
exclude_slash_tmp = true
|
||||
"#;
|
||||
|
||||
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(sandbox_workspace_write)
|
||||
@@ -754,9 +762,10 @@ writable_roots = [
|
||||
let sandbox_mode_override = None;
|
||||
assert_eq!(
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![PathBuf::from("/tmp")],
|
||||
writable_roots: vec![PathBuf::from("/my/workspace")],
|
||||
network_access: false,
|
||||
include_default_writable_roots: true,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
},
|
||||
sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override)
|
||||
);
|
||||
|
||||
@@ -98,6 +98,10 @@ pub struct SandboxWorkplaceWrite {
|
||||
pub writable_roots: Vec<PathBuf>,
|
||||
#[serde(default)]
|
||||
pub network_access: bool,
|
||||
#[serde(default)]
|
||||
pub exclude_tmpdir_env_var: bool,
|
||||
#[serde(default)]
|
||||
pub exclude_slash_tmp: bool,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, PartialEq, Default)]
|
||||
|
||||
@@ -185,11 +185,16 @@ pub enum SandboxPolicy {
|
||||
#[serde(default)]
|
||||
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,
|
||||
/// When set to `true`, will NOT include the per-user `TMPDIR`
|
||||
/// environment variable among the default writable roots. Defaults to
|
||||
/// `false`.
|
||||
#[serde(default)]
|
||||
exclude_tmpdir_env_var: bool,
|
||||
|
||||
/// When set to `true`, will NOT include the `/tmp` among the default
|
||||
/// writable roots on UNIX. Defaults to `false`.
|
||||
#[serde(default)]
|
||||
exclude_slash_tmp: bool,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -203,10 +208,6 @@ pub struct WritableRoot {
|
||||
pub read_only_subpaths: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
fn default_true() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
impl FromStr for SandboxPolicy {
|
||||
type Err = serde_json::Error;
|
||||
|
||||
@@ -228,7 +229,8 @@ impl SandboxPolicy {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
include_default_writable_roots: true,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -263,27 +265,40 @@ impl SandboxPolicy {
|
||||
SandboxPolicy::ReadOnly => Vec::new(),
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
include_default_writable_roots,
|
||||
..
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
network_access: _,
|
||||
} => {
|
||||
// Start from explicitly configured writable roots.
|
||||
let mut roots: Vec<PathBuf> = writable_roots.clone();
|
||||
|
||||
// Optionally include defaults (cwd and TMPDIR on macOS).
|
||||
if *include_default_writable_roots {
|
||||
roots.push(cwd.to_path_buf());
|
||||
// Always include defaults: cwd, /tmp (if present on Unix), and
|
||||
// on macOS, the per-user TMPDIR unless explicitly excluded.
|
||||
roots.push(cwd.to_path_buf());
|
||||
|
||||
// Also include the per-user tmp dir on macOS.
|
||||
// Note this is added dynamically rather than storing it in
|
||||
// `writable_roots` because `writable_roots` contains only static
|
||||
// values deserialized from the config file.
|
||||
if cfg!(target_os = "macos") {
|
||||
if let Some(tmpdir) = std::env::var_os("TMPDIR") {
|
||||
roots.push(PathBuf::from(tmpdir));
|
||||
}
|
||||
// Include /tmp on Unix unless explicitly excluded.
|
||||
if cfg!(unix) && !exclude_slash_tmp {
|
||||
let slash_tmp = PathBuf::from("/tmp");
|
||||
if slash_tmp.is_dir() {
|
||||
roots.push(slash_tmp);
|
||||
}
|
||||
}
|
||||
|
||||
// Include $TMPDIR unless explicitly excluded. On macOS, TMPDIR
|
||||
// is per-user, so writes to TMPDIR should not be readable by
|
||||
// other users on the system.
|
||||
//
|
||||
// By comparison, TMPDIR is not guaranteed to be defined on
|
||||
// Linux or Windows, but supporting it here gives users a way to
|
||||
// provide the model with their own temporary directory without
|
||||
// having to hardcode it in the config.
|
||||
if !exclude_tmpdir_env_var
|
||||
&& let Some(tmpdir) = std::env::var_os("TMPDIR")
|
||||
&& !tmpdir.is_empty()
|
||||
{
|
||||
roots.push(PathBuf::from(tmpdir));
|
||||
}
|
||||
|
||||
// For each root, compute subpaths that should remain read-only.
|
||||
roots
|
||||
.into_iter()
|
||||
|
||||
@@ -134,6 +134,11 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn create_seatbelt_args_with_read_only_git_subpath() {
|
||||
if cfg!(target_os = "windows") {
|
||||
// /tmp does not exist on Windows, so skip this test.
|
||||
return;
|
||||
}
|
||||
|
||||
// Create a temporary workspace with two writable roots: one containing
|
||||
// a top-level .git directory and one without it.
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
@@ -144,19 +149,21 @@ mod tests {
|
||||
root_with_git_git_canon,
|
||||
root_without_git_canon,
|
||||
} = populate_tmpdir(tmp.path());
|
||||
let cwd = tmp.path().join("cwd");
|
||||
|
||||
// Build a policy that only includes the two test roots as writable and
|
||||
// does not automatically include defaults like cwd or TMPDIR.
|
||||
// does not automatically include defaults TMPDIR or /tmp.
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![root_with_git.clone(), root_without_git.clone()],
|
||||
network_access: false,
|
||||
include_default_writable_roots: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
let args = create_seatbelt_command_args(
|
||||
vec!["/bin/echo".to_string(), "hello".to_string()],
|
||||
&policy,
|
||||
tmp.path(),
|
||||
&cwd,
|
||||
);
|
||||
|
||||
// Build the expected policy text using a raw string for readability.
|
||||
@@ -169,12 +176,12 @@ mod tests {
|
||||
; allow read-only file operations
|
||||
(allow file-read*)
|
||||
(allow file-write*
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1"))
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")) (subpath (param "WRITABLE_ROOT_2"))
|
||||
)
|
||||
"#,
|
||||
);
|
||||
|
||||
let expected_args = vec![
|
||||
let mut expected_args = vec![
|
||||
"-p".to_string(),
|
||||
expected_policy,
|
||||
format!(
|
||||
@@ -189,16 +196,25 @@ mod tests {
|
||||
"-DWRITABLE_ROOT_1={}",
|
||||
root_without_git_canon.to_string_lossy()
|
||||
),
|
||||
format!("-DWRITABLE_ROOT_2={}", cwd.to_string_lossy()),
|
||||
];
|
||||
|
||||
expected_args.extend(vec![
|
||||
"--".to_string(),
|
||||
"/bin/echo".to_string(),
|
||||
"hello".to_string(),
|
||||
];
|
||||
]);
|
||||
|
||||
assert_eq!(args, expected_args);
|
||||
assert_eq!(expected_args, args);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
if cfg!(target_os = "windows") {
|
||||
// /tmp does not exist on Windows, so skip this test.
|
||||
return;
|
||||
}
|
||||
|
||||
// Create a temporary workspace with two writable roots: one containing
|
||||
// a top-level .git directory and one without it.
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
@@ -215,7 +231,8 @@ mod tests {
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
include_default_writable_roots: true,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
};
|
||||
|
||||
let args = create_seatbelt_command_args(
|
||||
@@ -224,17 +241,14 @@ mod tests {
|
||||
root_with_git.as_path(),
|
||||
);
|
||||
|
||||
let tmpdir_env_var = if cfg!(target_os = "macos") {
|
||||
std::env::var("TMPDIR")
|
||||
.ok()
|
||||
.map(PathBuf::from)
|
||||
.and_then(|p| p.canonicalize().ok())
|
||||
.map(|p| p.to_string_lossy().to_string())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let tmpdir_env_var = std::env::var("TMPDIR")
|
||||
.ok()
|
||||
.map(PathBuf::from)
|
||||
.and_then(|p| p.canonicalize().ok())
|
||||
.map(|p| p.to_string_lossy().to_string());
|
||||
|
||||
let tempdir_policy_entry = if tmpdir_env_var.is_some() {
|
||||
" (subpath (param \"WRITABLE_ROOT_1\"))"
|
||||
r#" (subpath (param "WRITABLE_ROOT_2"))"#
|
||||
} else {
|
||||
""
|
||||
};
|
||||
@@ -249,7 +263,7 @@ mod tests {
|
||||
; allow read-only file operations
|
||||
(allow file-read*)
|
||||
(allow file-write*
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ){tempdir_policy_entry}
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
|
||||
)
|
||||
"#,
|
||||
);
|
||||
@@ -265,10 +279,17 @@ mod tests {
|
||||
"-DWRITABLE_ROOT_0_RO_0={}",
|
||||
root_with_git_git_canon.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1={}",
|
||||
PathBuf::from("/tmp")
|
||||
.canonicalize()
|
||||
.expect("canonicalize /tmp")
|
||||
.to_string_lossy()
|
||||
),
|
||||
];
|
||||
|
||||
if let Some(p) = tmpdir_env_var {
|
||||
expected_args.push(format!("-DWRITABLE_ROOT_1={p}"));
|
||||
expected_args.push(format!("-DWRITABLE_ROOT_2={p}"));
|
||||
}
|
||||
|
||||
expected_args.extend(vec![
|
||||
@@ -277,7 +298,7 @@ mod tests {
|
||||
"hello".to_string(),
|
||||
]);
|
||||
|
||||
assert_eq!(args, expected_args);
|
||||
assert_eq!(expected_args, args);
|
||||
}
|
||||
|
||||
struct PopulatedTmp {
|
||||
|
||||
@@ -76,7 +76,8 @@ async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() {
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![test_scenario.repo_parent.clone()],
|
||||
network_access: false,
|
||||
include_default_writable_roots: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
test_scenario
|
||||
@@ -101,7 +102,8 @@ async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() {
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![test_scenario.repo_root.clone()],
|
||||
network_access: false,
|
||||
include_default_writable_roots: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
test_scenario
|
||||
|
||||
@@ -51,7 +51,11 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: writable_roots.to_vec(),
|
||||
network_access: false,
|
||||
include_default_writable_roots: true,
|
||||
// Exclude tmp-related folders from writable roots because we need a
|
||||
// folder that is writable by tests but that we intentionally disallow
|
||||
// writing to in the sandbox.
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox");
|
||||
let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program));
|
||||
|
||||
Reference in New Issue
Block a user