Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion crates/nono-cli/src/tool-sandbox/platform/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ struct ResolvedToolSandboxPlan {
deny_only: BTreeMap<String, ResolvedDenyOnlyCommand>,
allowed_direct_bypasses: Vec<PathBuf>,
allowed_direct_bypass_ids: HashSet<FileId>,
/// Writable dirs (cwd, etc.) get a subtree exec grant instead of the
/// per-file allow-list, since files can appear there after setup.
outer_exec_writable_dirs: Vec<PathBuf>,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -233,6 +236,7 @@ impl ResolvedToolSandboxPlan {
let allowed_direct_bypasses =
resolve_allowed_direct_bypasses(config, &resolved, &deny_only, &governance_denies)?;
let allowed_direct_bypass_ids = resolve_file_ids(&allowed_direct_bypasses)?;
let outer_exec_writable_dirs = collect_outer_exec_writable_dirs(outer_caps, &search_dirs);
Ok(Self {
config: config.clone(),
resolved,
Expand All @@ -241,6 +245,7 @@ impl ResolvedToolSandboxPlan {
deny_only,
allowed_direct_bypasses,
allowed_direct_bypass_ids,
outer_exec_writable_dirs,
})
}
}
Expand Down Expand Up @@ -471,6 +476,7 @@ impl PreparedToolSandboxRuntime {
pub(crate) fn apply_outer_exec_gate(&self) -> Result<()> {
apply_outer_exec_gate(
&self.inner.allowed_outer_exec_files,
&self.inner.plan.outer_exec_writable_dirs,
self.inner.landlock_abi,
)
}
Expand Down Expand Up @@ -2645,6 +2651,25 @@ fn reject_group_or_world_writable_path(
Ok(())
}

/// Dirs the outer capability set grants write/readwrite to (e.g. cwd). Kept
/// separate from `build_outer_exec_files`'s per-file enumeration since a
/// writable dir's contents can change after setup.
fn collect_outer_exec_writable_dirs(
caps: &CapabilitySet,
executable_dirs: &[PathBuf],
) -> Vec<PathBuf> {
let mut dirs: Vec<PathBuf> = caps
.fs_capabilities()
.iter()
.filter(|cap| !cap.is_file && cap.access.contains(AccessMode::Write))
.map(|cap| cap.resolved.clone())
.filter(|dir| !executable_dirs.contains(dir))
.collect();
dirs.sort();
dirs.dedup();
dirs
}

fn outer_caps_grant_write(caps: &CapabilitySet, path: &Path) -> bool {
caps.fs_capabilities().iter().any(|cap| {
cap.access.contains(AccessMode::Write)
Expand Down Expand Up @@ -2983,7 +3008,11 @@ fn add_outer_exec_file_with_deps(
Ok(())
}

fn apply_outer_exec_gate(paths: &[PathBuf], abi: nono::DetectedAbi) -> Result<()> {
fn apply_outer_exec_gate(
paths: &[PathBuf],
writable_dirs: &[PathBuf],
abi: nono::DetectedAbi,
) -> Result<()> {
if !abi.has_execute() {
return Err(NonoError::SandboxInit(format!(
"tool-sandbox outer exec gate requires Landlock ABI V3+; detected {}",
Expand Down Expand Up @@ -3024,6 +3053,24 @@ fn apply_outer_exec_gate(paths: &[PathBuf], abi: nono::DetectedAbi) -> Result<()
})?;
}

// Subtree grant: files written into these dirs after setup must still run.
for dir in writable_dirs {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

🔒 [HIGH · security] Granting subtree AccessFs::Execute to writable directories (via outer_exec_writable_dirs) in apply_outer_exec_gate introduces a severe security bypass of the command_policies execution gate on Linux. A malicious sandboxed child can read any controlled binary (such as git or kubectl), copy it into a writable directory (like the current working directory . or /tmp), and execute that copy directly. Because Landlock is strictly allow-list oriented and cannot express deny-within-allow, granting execution rights to the writable directory allows execution of any file inside it. This bypasses the shim and supervisor entirely, circumventing all argument filtering, policy rules, credential isolation, and auditing. To maintain the security boundary, this change must be reverted. Scripts should instead be executed by running their respective interpreters (e.g., bash script.sh) directly, which does not require direct script execution privileges.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the accepted residual risk documented on #1390. A copied policed binary gains no elevation: credential promotion and proxy network bind to the verified supervisor handshake, not to whether a file is executable — so an unshimmed copy runs under the same fs/network limits as the agent, which can already write-and-run arbitrary code in a writable grant-dir when no command_policies exist. This brings command_policies in line with that default. On Linux, Landlock is a strict allow-list with no deny-within-allow, and a copy has a fresh inode, so the exec gate cannot both permit new files in cwd to run and block a copied policed binary — the two are mutually exclusive. The original policed binary stays blocked from direct exec via the inode-identity gate.

let fd = PathFd::new(dir).map_err(|err| {
NonoError::SandboxInit(format!(
"tool-sandbox outer exec gate cannot open {}: {err}",
dir.display()
))
})?;
ruleset = ruleset
.add_rule(PathBeneath::new(fd, AccessFs::Execute))
.map_err(|err| {
NonoError::SandboxInit(format!(
"tool-sandbox outer exec gate add_rule for {}: {err}",
dir.display()
))
})?;
}

let status = ruleset.restrict_self().map_err(|err| {
NonoError::SandboxInit(format!(
"tool-sandbox outer exec gate restrict_self failed: {err}"
Expand Down Expand Up @@ -5365,6 +5412,7 @@ mod tests {
deny_only: BTreeMap::new(),
allowed_direct_bypasses: Vec::new(),
allowed_direct_bypass_ids: HashSet::new(),
outer_exec_writable_dirs: Vec::new(),
},
shims_by_command: BTreeMap::from([("git".to_string(), shim.clone())]),
credential_handles: BTreeMap::new(),
Expand Down
148 changes: 148 additions & 0 deletions crates/nono-cli/tests/execution_strategy_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

use std::fs;
use std::net::TcpListener;
#[cfg(target_os = "linux")]
use std::os::unix::fs::PermissionsExt;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};

Expand Down Expand Up @@ -64,6 +66,15 @@ fn python3_available() -> bool {
.unwrap_or(false)
}

#[cfg(target_os = "linux")]
fn cc_available() -> bool {
Command::new("cc")
.arg("--version")
.output()
.map(|o| o.status.success())
.unwrap_or(false)
}

#[test]
#[cfg(target_os = "linux")]
fn direct_denies_path_outside_grant() {
Expand Down Expand Up @@ -423,3 +434,140 @@ fn env_credentials_with_command_policies_non_shim_entry_succeeds() {
"env_credentials + command_policies with a non-shim entry must succeed\nstdout: {stdout}\nstderr: {stderr}",
);
}

/// A script written into a writable grant-dir (cwd) must still execute under
/// an active `command_policies` outer exec gate.
#[test]
#[cfg(target_os = "linux")]
fn command_policies_allows_script_exec_in_writable_grant_dir() {
Comment thread
kipz marked this conversation as resolved.
let (_tmp, home, workspace) = setup_isolated_home("cmd-policies-script-exec");

let script_path = workspace.join("s.sh");
fs::write(&script_path, "#!/usr/bin/env bash\necho ok\n").expect("write script");
fs::set_permissions(&script_path, fs::Permissions::from_mode(0o755))
.expect("chmod script executable");

let profile_path = write_profile(
&home,
"cmd-policies-script-exec",
&format!(
r#"{{
"meta": {{ "name": "cmd-policies-script-exec-test" }},
"filesystem": {{ "allow": ["{workspace}"] }},
"network": {{ "block": true }},
"command_policies": {{
"commands": {{
"cat": {{
"executable": "/bin/cat"
}}
}}
}}
}}"#,
workspace = workspace.display()
),
);

let output = run_nono(
&[
"run",
"--profile",
profile_path.to_str().expect("profile path"),
"--no-rollback",
"--",
"bash",
"-c",
"./s.sh",
],
&home,
&workspace,
);

let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);

assert!(
output.status.success(),
"a script in a writable grant-dir must still execute under command_policies\nstdout: {stdout}\nstderr: {stderr}",
);
assert!(
stdout.contains("ok"),
"expected script output in stdout\nstdout: {stdout}\nstderr: {stderr}",
);
}

/// Same as above but for a binary compiled at runtime, so it wasn't on disk
/// when the outer exec gate was set up.
#[test]
#[cfg(target_os = "linux")]
fn command_policies_allows_compiled_binary_exec_in_writable_grant_dir() {
if !cc_available() {
eprintln!("skipping: cc not available");
return;
}

let (_tmp, home, workspace) = setup_isolated_home("cmd-policies-bin-exec");

let source_path = workspace.join("b.c");
fs::write(
&source_path,
"#include <stdio.h>\nint main(void) { printf(\"ok\\n\"); return 0; }\n",
)
.expect("write source");
let binary_path = workspace.join("b");
let compile_status = Command::new("cc")
.args([
"-o",
binary_path.to_str().expect("binary path"),
source_path.to_str().expect("source path"),
])
.status()
.expect("run cc");
assert!(compile_status.success(), "cc must compile the test binary");

let profile_path = write_profile(
&home,
"cmd-policies-bin-exec",
&format!(
r#"{{
"meta": {{ "name": "cmd-policies-bin-exec-test" }},
"filesystem": {{ "allow": ["{workspace}"] }},
"network": {{ "block": true }},
"command_policies": {{
"commands": {{
"cat": {{
"executable": "/bin/cat"
}}
}}
}}
}}"#,
workspace = workspace.display()
),
);

let output = run_nono(
&[
"run",
"--profile",
profile_path.to_str().expect("profile path"),
"--no-rollback",
"--",
"bash",
"-c",
"./b",
],
&home,
&workspace,
);

let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);

assert!(
output.status.success(),
"a freshly compiled binary in a writable grant-dir must still execute under command_policies\nstdout: {stdout}\nstderr: {stderr}",
);
assert!(
stdout.contains("ok"),
"expected binary output in stdout\nstdout: {stdout}\nstderr: {stderr}",
);
}
Loading