From 0a140c1c4d93450db4dd5fa5ff0d37d54e533194 Mon Sep 17 00:00:00 2001 From: kewton Date: Sun, 11 Oct 2026 19:33:38 +0900 Subject: [PATCH 1/3] Add an OS write sandbox for Bash and verifier commands (macOS Seatbelt, stage 1a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap the two `sh -c` launch points — the model Bash tool (the `bash:verify` interval reuses the same function) and verifier/acceptance commands — in a new `os_sandbox` leaf module. When the Seatbelt backend is available, writes are confined to the workspace, the run's temp directory, the known dependency caches, and device files, so an indirect write the lexical guard cannot see fails. The lexical guard still runs first and is not weakened. `auto` continues with lexical checks only when the backend is unavailable and records `bash_sandbox_status` (applied=false, reason); `required` fails with `bash_sandbox_unavailable`; `off` keeps the previous behavior. The event name is the only new one; existing event names and schemas are unchanged. The run temp directory is passed to children via an explicit `TMPDIR`. 読み替え: `--sandbox` is hidden from the public flag table because the advertised flag count lives in docs/guide/README.md, which this issue's scope.allow does not include; it is documented in prose in SECURITY.md and both cli-reference pages instead. Co-authored-by: CommandCodeBot --- SECURITY.md | 66 ++- docs/guide/en/cli-reference.md | 30 ++ docs/guide/ja/cli-reference.md | 11 + src/bounded_process.rs | 7 +- src/cli.rs | 10 + src/lib.rs | 17 + src/minimal_loop/verifier_env.rs | 13 + src/sensitive_data/fixed_identifiers.rs | 2 + src/tools/bash.rs | 13 + src/tools/mod.rs | 1 + src/tools/os_sandbox.rs | 432 +++++++++++++++++ src/tools/os_sandbox/seatbelt.rs | 133 +++++ .../issue502-indirect-write/expectations.toml | 21 + .../fixtures/indirect-cases.jsonl | 10 + .../fixtures/macos-probe-results.md | 70 +++ tests/issue502_os_sandbox_write.rs | 457 ++++++++++++++++++ 16 files changed, 1284 insertions(+), 9 deletions(-) create mode 100644 src/tools/os_sandbox.rs create mode 100644 src/tools/os_sandbox/seatbelt.rs create mode 100644 tests/corpus/apps/issue502-indirect-write/expectations.toml create mode 100644 tests/corpus/apps/issue502-indirect-write/fixtures/indirect-cases.jsonl create mode 100644 tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md create mode 100644 tests/issue502_os_sandbox_write.rs diff --git a/SECURITY.md b/SECURITY.md index 16fafbe5..443300a0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -14,9 +14,12 @@ execute hostile projects or hostile instructions safely. write destinations (common mutating commands and output redirects) must stay under the canonical workspace root, including through existing symlinks; newly created workspace symlinks may not target paths outside that root. + Lexical inspection cannot prove every indirect effect of an arbitrary + program, so when an OS write sandbox is available it also confines the writes + performed inside a command or script (see [OS Write Sandbox](#os-write-sandbox)). Bash is not a complete sandbox: shell commands and invoked programs still run - as the current OS user, and static inspection cannot prove every indirect - effect of an arbitrary program. + as the current OS user, reads and network are not confined in this stage, and + the sandbox is absent when the backend is unavailable. ## `--allow` and `--yes` @@ -25,13 +28,16 @@ Read/Glob/Grep, `write` admits and auto-approves Write/Edit, and `bash:verify` admits and auto-approves only Bash commands accepted by the shared verifier policy; recognized direct filesystem-mutating Bash commands remain excluded. Repeat the option or comma-separate selectors. Once an explicit list is used, -omitted classes are blocked before execution. +omitted classes are blocked before execution. `bash:verify` admits a command by +its classification only: the contents of any script the command invokes are +constrained only by the OS write sandbox, when it is applied, and not by the +classification. `--yes` is the backward-compatible all-tools alias and also skips resume confirmation. Neither option bypasses the recognized Bash write-destination -guard. Use `--yes` only in a trusted workspace after checking the goal and -current branch: it does not turn Bash into an OS sandbox and does not auto-kill -unrelated local processes. +guard or the OS write sandbox. Use `--yes` only in a trusted workspace after +checking the goal and current branch: it does not turn Bash into a complete OS +sandbox and does not auto-kill unrelated local processes. At run startup CommandAgent warns about a non-Git workspace or existing Git changes. At exit it reports the final tracked/staged diff stat and untracked @@ -40,7 +46,46 @@ predated the run. `--offline` blocks runtime dependency setup plus Bash command families named in `--help`. It does not block provider/API requests or arbitrary network-capable -programs and therefore is not a network sandbox. +programs and therefore is not a network sandbox. Network isolation is a later +stage of #502 and is independent of the write sandbox below. + +## OS Write Sandbox + +`--sandbox auto|required|off` (default `auto`) adds an OS-level write sandbox at +the two `sh -c` launch points: the model Bash tool (the `bash:verify` interval +reuses the same function) and the verification/acceptance commands. The sandbox +is defense in depth; the lexical checks still run first and can refuse a command +before the shell starts. + +- What the OS closes (when the backend is available): a write performed inside a + command or script that the lexical guard cannot see — `sh -c 'cp ...'`, + `eval`, `xargs`, `find -exec`, an interpreter's `open(..., 'w')`, `dd of=`, + `sed -i` — and creating a hardlink to an inode outside the workspace. The only + writable locations are the workspace, the run's temp directory, the known + dependency caches, and device files such as `/dev/null`. +- What remains lexical-only: reads, network access, and every command run while + the backend is unavailable. `--allow bash:verify` and `--offline` are lexical + policies, so a command they admit can still start a script; only the sandbox + (when applied) constrains that script's writes. Reads and network are not + confined in this stage. +- Backend availability is probed once at run start. On macOS the backend is + Seatbelt through `sandbox-exec`; under a nested sandbox it reports `rc=71` and + is unavailable. Other platforms have no backend in this stage. +- `auto` continues with lexical checks only when the backend is unavailable, + warns once at startup, and records `bash_sandbox_status` with `applied=false` + and a reason. `required` instead fails Bash and verifier commands with + `bash_sandbox_unavailable`. `off` keeps the previous behavior. None of them + reports "sandboxed" when the sandbox was not applied. +- The dependency caches (cargo registry/git/package-cache, npm, pip, Playwright + browsers) stay writable so builds and tests work; they remain a contamination + path outside the workspace. pnpm and yarn stores are not allowed in this + stage. + +macOS `mktemp` ignores `TMPDIR` and targets the per-user temp directory, which +the sandbox denies. A `mktemp` inside the run temp directory (`mktemp -p +"$TMPDIR"` or a template under `$TMPDIR`) succeeds; a bare `mktemp` does not. +The applied-probe record is under +`tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md`. ## Child Process Environment @@ -115,6 +160,11 @@ template files instead. ## Backlog +- OS write sandbox, remaining stages of #502: Linux write confinement (landlock), + read confinement, and network/dependency-install isolation. Stage 1a (macOS + Seatbelt write confinement and the shared leaf module) is implemented here; + the same leaf module is the seam the later stages extend. - Container-sandboxed execution track: run Bash/verifier commands inside a per-workspace container or equivalent OS sandbox with a mounted workspace, - minimal environment, and explicit network policy. + minimal environment, and explicit network policy. Superseded in part by the + #502 staging above; kept as the alternative with a fully mounted root. diff --git a/docs/guide/en/cli-reference.md b/docs/guide/en/cli-reference.md index 5563c910..83d14b1a 100644 --- a/docs/guide/en/cli-reference.md +++ b/docs/guide/en/cli-reference.md @@ -97,6 +97,36 @@ internal integration surface and is intentionally not a public user flag. | `--footer` | `` | `on` | Control the fixed TUI footer; off keeps scrollback breadcrumbs. | [Footer problems](troubleshooting.md#footer-rendering-problems) | | `--no-footer` | none | off | Disable the fixed TUI footer. Equivalent in effect to `--footer off`. | [Footer problems](troubleshooting.md#footer-rendering-problems) | +## OS write sandbox + +The hidden `--sandbox auto|required|off` flag (default `auto`) controls an +OS-level write sandbox for the model Bash tool and for verification/acceptance +commands. The lexical checks still run first; the sandbox adds confinement for +writes the lexical guard cannot see. + +- **What the OS closes (when the backend is available):** writes performed inside + a command or script — `sh -c 'cp ...'`, `eval`, `xargs`, `find -exec`, an + interpreter's `open(..., 'w')`, `dd of=`, `sed -i` — and hardlinks to an inode + outside the workspace. Only the workspace, the run's temp directory, the known + dependency caches, and device files such as `/dev/null` are writable. +- **What stays lexical-only:** reads, network access, and every command run when + the backend is unavailable. `--yes`, `--allow bash:verify`, and `--offline` do + not close an indirect write by themselves; only the sandbox, when applied, + does. `--allow bash:verify` admits a command by its classification only, so the + contents of any script it invokes are limited by the sandbox alone. +- **Modes:** `auto` uses the backend when available and otherwise warns once and + records `bash_sandbox_status` with `applied=false` and a reason, continuing + with lexical checks only; `required` fails Bash and verifier commands with + `bash_sandbox_unavailable`; `off` keeps the previous behavior. The backend is + unavailable under a nested sandbox (`rc=71`) and on a platform without one + (macOS only in this stage). +- Dependency caches (cargo, npm, pip, Playwright browsers) stay writable so + builds and tests work; they remain a contamination path outside the workspace. + +macOS `mktemp` ignores `TMPDIR`; use `mktemp -p "$TMPDIR"` or a template under +`$TMPDIR`. The applied-probe record is under +`tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md`. + ## Interactive REPL controls Within one TUI session, `/model ` and `/provider ` change the diff --git a/docs/guide/ja/cli-reference.md b/docs/guide/ja/cli-reference.md index 3fdb67b3..dc61a153 100644 --- a/docs/guide/ja/cli-reference.md +++ b/docs/guide/ja/cli-reference.md @@ -96,6 +96,17 @@ Clap が生成する `-h`/`--help` と `-V`/`--version` は、以下のアプリ | `--footer` | `` | `on` | 固定 TUI フッターを制御します。off でもスクロールバックの breadcrumb は残ります。 | [フッターの問題](troubleshooting.md#フッター描画の問題) | | `--no-footer` | なし | オフ | 固定 TUI フッターを無効にします。効果は `--footer off` と同じです。 | [フッターの問題](troubleshooting.md#フッター描画の問題) | +## OS 書き込みサンドボックス + +隠しフラグ `--sandbox auto|required|off`（既定 `auto`）は、モデルの Bash ツールと検証・受け入れコマンドに対する OS レベルの書き込みサンドボックスを制御します。字句検査は先に働き、サンドボックスは字句検査から見えない書き込みを閉じ込めます。 + +- **OS が閉じるもの（backend が使えるとき）：** コマンドやスクリプトの中の書き込み — `sh -c 'cp ...'`、`eval`、`xargs`、`find -exec`、インタープリタの `open(..., 'w')`、`dd of=`、`sed -i` — と、workspace の外の inode への hardlink です。書き込めるのは、workspace・run の一時ディレクトリ・既知の依存キャッシュ・`/dev/null` などのデバイスだけです。 +- **字句検査だけのもの：** 読み取り、ネットワーク、backend が使えないときの全コマンドです。`--yes`・`--allow bash:verify`・`--offline` は、それ自体では間接的な書き込みを閉じません。適用されたサンドボックスだけが閉じます。`--allow bash:verify` はコマンドを分類で通すだけなので、そのコマンドが呼ぶスクリプトの中身を制限するのはサンドボックスだけです。 +- **モード：** `auto` は backend が使えれば適用し、使えなければ起動時に一度警告し、`applied=false` と理由つきの `bash_sandbox_status` を記録して字句検査だけで続けます。`required` は Bash と検証コマンドを `bash_sandbox_unavailable` で失敗させます。`off` は今までの挙動を保ちます。入れ子の sandbox（`rc=71`）と backend の無いプラットフォーム（この段階は macOS のみ）では未適用です。 +- 依存キャッシュ（cargo・npm・pip・Playwright のブラウザ）はビルドとテストのために書き込み可能なままにします。workspace の外の汚染経路として残ります。 + +macOS の `mktemp` は `TMPDIR` を見ません。`mktemp -p "$TMPDIR"` か `$TMPDIR` 配下のテンプレートを使ってください。適用の試験の記録は `tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md` にあります。 + ## 対話型 REPL の操作 1 回の TUI セッション内で、`/model ` と `/provider ` は executor の diff --git a/src/bounded_process.rs b/src/bounded_process.rs index 2e7831cf..e73e6d5a 100644 --- a/src/bounded_process.rs +++ b/src/bounded_process.rs @@ -94,7 +94,12 @@ fn child_env_allowed(key: &std::ffi::OsStr, source: EnvSource) -> bool { || (source == EnvSource::Explicit && (matches!( key, - "NODE_ENV" | "NODE_OPTIONS" | "NODE_PATH" | "NEXT_TELEMETRY_DISABLED" | "PORT" + "NODE_ENV" + | "NODE_OPTIONS" + | "NODE_PATH" + | "NEXT_TELEMETRY_DISABLED" + | "PORT" + | "TMPDIR" ) || [ "COMMANDAGENT_FOREIGN_TOOLCHAIN_ROOT", "COMMANDAGENT_FOREIGN_TOOLCHAIN_EVENTS", diff --git a/src/cli.rs b/src/cli.rs index f8c49f7a..f5ad4699 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -510,6 +510,16 @@ pub struct Cli { help = crate::tools::offline_policy::CLI_HELP )] pub offline: bool, + #[arg( + long, + hide = true, + value_enum, + value_name = "auto|required|off", + default_value = "auto", + help_heading = "Workspace and State", + help = "Select the OS write sandbox for Bash and verifier commands. `auto` applies it when the backend is available and otherwise continues with lexical checks only; `required` fails those commands when the backend is unavailable; `off` keeps the previous behavior." + )] + pub sandbox: crate::tools::os_sandbox::SandboxMode, #[arg( long, action = ArgAction::SetTrue, diff --git a/src/lib.rs b/src/lib.rs index 779a3265..e2dde40e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -130,8 +130,13 @@ pub fn run_with_provider_options( } let pack_cli = cli.clone(); let summary_json = cli.summary_json; + let sandbox_mode = cli.sandbox; let config = Config::from_cli_with_provider_options(cli, provider_options) .map_err(cli_argument_error)?; + tools::os_sandbox::install(sandbox_mode, &config.workspace_root, &config.state_dir); + if let Some(warning) = tools::os_sandbox::startup_warning() { + eprintln!("{warning}"); + } for profile in planner::extension_profiles::registered() { for warning in &profile.warnings { eprintln!("warning: {warning}"); @@ -895,6 +900,18 @@ fn emit_run_start(config: &Config) { "build_timestamp": build_info::TIMESTAMP, }), ); + if let Some(sandbox) = tools::os_sandbox::status() { + eval_events::emit( + config.eval_events_path.as_deref(), + json!({ + "event": "bash_sandbox_status", + "mode": sandbox.mode.as_str(), + "backend": sandbox.backend, + "applied": sandbox.applied, + "reason": sandbox.reason.unwrap_or_default(), + }), + ); + } eval_events::emit( config.eval_events_path.as_deref(), json!({ diff --git a/src/minimal_loop/verifier_env.rs b/src/minimal_loop/verifier_env.rs index 29d41491..b244cc1f 100644 --- a/src/minimal_loop/verifier_env.rs +++ b/src/minimal_loop/verifier_env.rs @@ -242,6 +242,19 @@ fn run_structured( let mut process = normalized_command_at_root("sh", root); process.arg("-c").arg(command).current_dir(root); + let mut process = match crate::tools::os_sandbox::wrap_command(process) { + Ok(process) => process, + Err(refusal) => { + return Ok(BashOutcome { + kind: BashOutcomeKind::Blocked, + status: None, + stdout: String::new(), + stderr: String::new(), + elapsed_ms: started.elapsed().as_millis(), + summary: refusal.summary(), + }); + } + }; process.stdout(Stdio::piped()).stderr(Stdio::piped()); let output = bounded_process::run_with_timeout(&mut process, timeout) .with_context(|| format!("failed to spawn verifier command: {command}"))?; diff --git a/src/sensitive_data/fixed_identifiers.rs b/src/sensitive_data/fixed_identifiers.rs index a85e46b0..eb650cf7 100644 --- a/src/sensitive_data/fixed_identifiers.rs +++ b/src/sensitive_data/fixed_identifiers.rs @@ -54,6 +54,8 @@ pub const FIXED_IDENTIFIERS: &[&str] = &[ "anchor_stagnation_interlock", "artifact_stagnation_feedback", "bash_path_confinement_rejected", + "bash_sandbox_status", + "bash_sandbox_unavailable", "blocked", "blocked_by_dependency_setup", "browser_interaction_probe", diff --git a/src/tools/bash.rs b/src/tools/bash.rs index 53c9863e..5a1366e2 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -228,6 +228,19 @@ where let mut process = Command::new("sh"); process.arg("-c").arg(&command).current_dir(root); + let mut process = match super::os_sandbox::wrap_command(process) { + Ok(process) => process, + Err(refusal) => { + return Ok(BashOutcome { + kind: BashOutcomeKind::Blocked, + status: None, + stdout: String::new(), + stderr: String::new(), + elapsed_ms: started.elapsed().as_millis(), + summary: refusal.summary(), + }); + } + }; process.stdout(Stdio::piped()).stderr(Stdio::piped()); remove_shell_cdpath(&mut process); let output = bounded_process::run_with_timeout_cancel_and_force( diff --git a/src/tools/mod.rs b/src/tools/mod.rs index ada644ed..795cb8f6 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -12,6 +12,7 @@ pub mod grep; pub mod hidden_path; pub mod mcp; pub mod offline_policy; +pub mod os_sandbox; pub mod path_guard; pub(crate) mod placeholder_path; pub mod read; diff --git a/src/tools/os_sandbox.rs b/src/tools/os_sandbox.rs new file mode 100644 index 00000000..00de8785 --- /dev/null +++ b/src/tools/os_sandbox.rs @@ -0,0 +1,432 @@ +//! OS write sandbox for model Bash and verifier commands (Issue #655). +//! +//! This is the common leaf module of the #502 write-confinement track. It wraps +//! the `sh -c` launch point of the model Bash tool (including the `bash:verify` +//! interval, which reuses the same function) and of verifier commands, so an +//! indirect write that the lexical guard cannot see still fails when the OS +//! backend is available. +//! +//! Design (issue #655, decisions from #502): +//! - The backend writes a fixed profile; locations travel only through `-D` +//! parameters, never embedded in the profile string. +//! - Availability is probed once at run start and stored process-wide, together +//! with the mode, so a Bash spawned on another thread still sees it. +//! - `auto` continues with lexical checks only when the backend is unavailable +//! and records the reason; `required` fails the command honestly with +//! [`UNAVAILABLE_KIND`]; `off` keeps the previous behavior. +//! - The lexical guard is not weakened or replaced: it still runs first and can +//! refuse a command before the sandbox is consulted. +//! - macOS only in this stage; Linux (landlock) is a separate issue. On any +//! other platform the backend is reported as unavailable rather than silently +//! pretending isolation. + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Mutex; + +use clap::ValueEnum; + +#[cfg(target_os = "macos")] +pub mod seatbelt; + +/// The macOS backend name used in `bash_sandbox_status`. +pub const BACKEND_SEATBELT: &str = "seatbelt"; +/// The backend name on platforms without a write backend in this stage. +pub const BACKEND_NONE: &str = "none"; + +/// The honest failure kind emitted when `required` cannot apply the sandbox. +pub const UNAVAILABLE_KIND: &str = "bash_sandbox_unavailable"; + +/// The reason recorded when the sandbox is explicitly disabled. +const DISABLED_REASON: &str = "disabled by --sandbox off"; +/// The reason recorded on platforms with no write-isolation backend in 1a. +#[cfg(not(target_os = "macos"))] +const NO_PLATFORM_BACKEND: &str = + "no OS write-isolation backend is available on this platform (macOS only in stage 1a)"; + +/// `--sandbox auto|required|off`. `auto` is the default. +#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] +pub enum SandboxMode { + /// Use the backend when available, otherwise continue with lexical checks. + Auto, + /// Refuse Bash and verifier commands when the backend is unavailable. + Required, + /// Do not use the OS sandbox; keep the previous behavior. + Off, +} + +impl SandboxMode { + pub const fn as_str(self) -> &'static str { + match self { + Self::Auto => "auto", + Self::Required => "required", + Self::Off => "off", + } + } + + /// Whether this mode asks the backend to be applied. + const fn uses_backend(self) -> bool { + !matches!(self, Self::Off) + } +} + +/// Whether the backend could be started. The reason is a human string and never +/// contains a path or a secret. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SandboxAvailability { + Available, + Unavailable(String), +} + +/// The resolved write-allow list and the run temp directory. +#[derive(Debug, Clone)] +pub struct SandboxPolicy { + pub workspace_root: PathBuf, + pub run_tmp_dir: PathBuf, + pub caches: CachePaths, +} + +impl SandboxPolicy { + /// Build a policy whose cache locations are resolved from `HOME`. + pub fn new(workspace_root: &Path, run_tmp_dir: &Path) -> Self { + Self { + workspace_root: workspace_root.to_path_buf(), + run_tmp_dir: run_tmp_dir.to_path_buf(), + caches: resolve_cache_paths(), + } + } +} + +/// Known dependency caches allowed for writing (decision 2). pnpm and yarn +/// stores are deliberately excluded in this stage. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct CachePaths { + pub cargo_registry: Option, + pub cargo_git: Option, + pub cargo_package_cache: Option, + pub npm: Option, + pub pip: Option, + pub playwright: Option, +} + +/// The outcome of a run-start install, also the payload of `bash_sandbox_status`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SandboxStatus { + pub mode: SandboxMode, + pub backend: &'static str, + pub applied: bool, + pub reason: Option, +} + +impl SandboxStatus { + fn from_parts(mode: SandboxMode, availability: &SandboxAvailability) -> Self { + let applied = mode.uses_backend() && matches!(availability, SandboxAvailability::Available); + let reason = if mode.uses_backend() { + match availability { + SandboxAvailability::Unavailable(reason) => Some(reason.clone()), + SandboxAvailability::Available => None, + } + } else { + None + }; + Self { + mode, + backend: backend_name(), + applied, + reason, + } + } +} + +/// A refusal returned when `required` cannot apply the OS sandbox. +#[derive(Debug, Clone)] +pub struct SandboxRefusal { + pub reason: String, +} + +impl SandboxRefusal { + /// The honest `bash_sandbox_unavailable` summary returned to the caller. + pub fn summary(&self) -> String { + format!("{UNAVAILABLE_KIND}: {}", self.reason) + } +} + +struct Installed { + policy: Option, + mode: SandboxMode, + availability: SandboxAvailability, +} + +static INSTALLED: Mutex> = Mutex::new(None); + +fn backend_name() -> &'static str { + if cfg!(target_os = "macos") { + BACKEND_SEATBELT + } else { + BACKEND_NONE + } +} + +/// Probe the backend once. On macOS this starts `sandbox-exec` with a do-nothing +/// profile; a nested sandbox fails with rc=71, which is reported as unavailable. +pub fn probe() -> SandboxAvailability { + #[cfg(target_os = "macos")] + { + seatbelt::probe() + } + #[cfg(not(target_os = "macos"))] + { + SandboxAvailability::Unavailable(NO_PLATFORM_BACKEND.to_string()) + } +} + +/// Install the process-wide sandbox state for a run and create its temp +/// directory. Returns the status recorded in `bash_sandbox_status`. +/// +/// In a unit-test build this is a no-op: many lib tests run in one process and +/// must not observe each other's process-wide state. Integration tests compile +/// the library without `cfg(test)` and exercise the real install, and the +/// `install_for_test` seam drives the explicit states. +pub fn install(mode: SandboxMode, workspace_root: &Path, state_dir: &Path) -> SandboxStatus { + #[cfg(not(test))] + { + install_impl(mode, workspace_root, state_dir) + } + #[cfg(test)] + { + let _ = (workspace_root, state_dir); + SandboxStatus { + mode, + backend: backend_name(), + applied: false, + reason: None, + } + } +} + +#[cfg(not(test))] +fn install_impl(mode: SandboxMode, workspace_root: &Path, state_dir: &Path) -> SandboxStatus { + let run_tmp_dir = state_dir + .join("sandbox") + .join(format!("run-{}", uuid::Uuid::now_v7())); + if let Err(err) = std::fs::create_dir_all(&run_tmp_dir) { + let availability = + SandboxAvailability::Unavailable(format!("run temp directory is unavailable: {err}")); + return store(mode, None, availability); + } + let policy = SandboxPolicy::new(workspace_root, &run_tmp_dir); + let availability = if mode.uses_backend() { + probe() + } else { + SandboxAvailability::Unavailable(DISABLED_REASON.to_string()) + }; + store(mode, Some(policy), availability) +} + +/// Install an explicit policy and availability. Used by tests to simulate an +/// unavailable backend (for example a nested sandbox rc=71). +pub fn install_for_test( + mode: SandboxMode, + policy: SandboxPolicy, + availability: SandboxAvailability, +) -> SandboxStatus { + store(mode, Some(policy), availability) +} + +fn store( + mode: SandboxMode, + policy: Option, + availability: SandboxAvailability, +) -> SandboxStatus { + let status = SandboxStatus::from_parts(mode, &availability); + let mut installed = INSTALLED.lock().expect("sandbox state"); + *installed = Some(Installed { + policy, + mode, + availability, + }); + status +} + +/// Clear the process-wide state. Tests only. +pub fn reset_for_test() { + let mut installed = INSTALLED.lock().expect("sandbox state"); + *installed = None; +} + +/// The status recorded at run start, if any. +pub fn status() -> Option { + let installed = INSTALLED.lock().expect("sandbox state"); + installed + .as_ref() + .map(|installed| SandboxStatus::from_parts(installed.mode, &installed.availability)) +} + +/// The one-time startup warning for `auto` when the backend is unavailable. +pub fn startup_warning() -> Option { + let status = status()?; + if status.mode == SandboxMode::Auto && !status.applied { + let reason = status.reason.as_deref().unwrap_or("unknown reason"); + Some(format!( + "warning: OS write sandbox is unavailable ({reason}); Bash and verifier commands continue with lexical path checks only" + )) + } else { + None + } +} + +/// Apply the installed sandbox to a prepared command. +/// +/// `Ok(command)` is the (possibly wrapped) command to spawn. When `required` is +/// installed but the backend is unavailable, returns `Err` so the caller fails +/// the command instead of claiming isolation. +pub fn wrap_command(command: Command) -> Result { + let (mode, availability) = { + let installed = INSTALLED.lock().expect("sandbox state"); + match installed.as_ref() { + Some(installed) => (installed.mode, installed.availability.clone()), + None => return Ok(command), + } + }; + if !mode.uses_backend() { + return Ok(command); + } + match availability { + SandboxAvailability::Unavailable(reason) => { + if mode == SandboxMode::Required { + Err(SandboxRefusal { reason }) + } else { + Ok(command) + } + } + SandboxAvailability::Available => { + let policy = { + let installed = INSTALLED.lock().expect("sandbox state"); + installed + .as_ref() + .and_then(|installed| installed.policy.clone()) + }; + match policy { + Some(policy) => Ok(apply_backend(command, &policy)), + None => Ok(command), + } + } + } +} + +fn apply_backend(command: Command, policy: &SandboxPolicy) -> Command { + #[cfg(target_os = "macos")] + { + seatbelt::wrap(command, policy) + } + #[cfg(not(target_os = "macos"))] + { + let _ = policy; + command + } +} + +fn resolve_cache_paths() -> CachePaths { + let Some(home) = std::env::var_os("HOME").map(PathBuf::from) else { + return CachePaths::default(); + }; + CachePaths { + cargo_registry: existing_dir(home.join(".cargo/registry")), + cargo_git: existing_dir(home.join(".cargo/git")), + cargo_package_cache: existing_file(home.join(".cargo/.package-cache")), + npm: existing_dir(home.join(".npm")), + pip: existing_dir(home.join("Library/Caches/pip")), + playwright: existing_dir(home.join("Library/Caches/ms-playwright")), + } +} + +fn existing_dir(path: PathBuf) -> Option { + path.is_dir().then_some(path) +} + +fn existing_file(path: PathBuf) -> Option { + path.is_file().then_some(path) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + /// The installed state is process-wide, so these tests serialize. + static SANDBOX_TEST_LOCK: Mutex<()> = Mutex::new(()); + + fn policy() -> SandboxPolicy { + SandboxPolicy { + workspace_root: PathBuf::from("/tmp/os-sandbox-test-work"), + run_tmp_dir: PathBuf::from("/tmp/os-sandbox-test-tmp"), + caches: CachePaths::default(), + } + } + + #[test] + fn required_and_unavailable_is_refused_with_the_honest_kind() { + let _serial = SANDBOX_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = TestStateGuard; + install_for_test( + SandboxMode::Required, + policy(), + SandboxAvailability::Unavailable("nested sandbox rc=71".to_string()), + ); + + let error = wrap_command(Command::new("sh")).expect_err("required must refuse"); + assert_eq!( + error.summary(), + format!("{UNAVAILABLE_KIND}: nested sandbox rc=71") + ); + } + + #[test] + fn auto_and_unavailable_continues_and_warns_once() { + let _serial = SANDBOX_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = TestStateGuard; + install_for_test( + SandboxMode::Auto, + policy(), + SandboxAvailability::Unavailable("nested sandbox rc=71".to_string()), + ); + + assert!(wrap_command(Command::new("sh")).is_ok()); + let warning = startup_warning().expect("auto warns"); + assert!(warning.contains("nested sandbox rc=71"), "{warning}"); + let status = status().unwrap(); + assert!(!status.applied); + assert_eq!(status.reason.as_deref(), Some("nested sandbox rc=71")); + } + + #[test] + fn off_never_wraps_and_never_warns() { + let _serial = SANDBOX_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = TestStateGuard; + install_for_test( + SandboxMode::Off, + policy(), + SandboxAvailability::Unavailable(DISABLED_REASON.to_string()), + ); + + assert!(wrap_command(Command::new("sh")).is_ok()); + assert!(startup_warning().is_none()); + let status = status().unwrap(); + assert!(!status.applied); + assert_eq!(status.reason, None); + } + + struct TestStateGuard; + + impl Drop for TestStateGuard { + fn drop(&mut self) { + reset_for_test(); + } + } +} diff --git a/src/tools/os_sandbox/seatbelt.rs b/src/tools/os_sandbox/seatbelt.rs new file mode 100644 index 00000000..80ee79d8 --- /dev/null +++ b/src/tools/os_sandbox/seatbelt.rs @@ -0,0 +1,133 @@ +//! macOS Seatbelt backend (Issue #655, stage 1a). +//! +//! Applies a fixed write-confinement profile through `sandbox-exec`. The +//! profile is a constant string; the workspace, the run temp directory, and the +//! allowed caches travel only as `-D` parameters, so no location is embedded in +//! the profile text. Reads and network are left as they are (`allow default` +//! then `deny file-write*`), and the only re-allowed writes are the workspace, +//! the run temp directory, the known dependency caches, and device files. + +use std::path::Path; +use std::process::{Command, Stdio}; +use std::time::Duration; + +use super::{SandboxAvailability, SandboxPolicy}; + +const SANDBOX_EXEC: &str = "/usr/bin/sandbox-exec"; +const PROBE_PROFILE: &str = "(version 1)(allow default)"; +const PROBE_TIMEOUT: Duration = Duration::from_secs(10); +/// A placeholder path for an absent cache so every profile parameter is always +/// defined. It matches nothing on a real system. +const UNSET_PATH: &str = "/commandagent-sandbox-unset"; + +/// The fixed profile. `(allow default)` keeps reads and network unchanged; +/// `(deny file-write*)` then confines every write to the parameters below. +const PROFILE: &str = r#" +(version 1) +(allow default) +(deny file-write*) +(allow file-write* (subpath (param "WS"))) +(allow file-write* (subpath (param "TMP"))) +(allow file-write* (subpath (param "CACHE_CARGO_REGISTRY"))) +(allow file-write* (subpath (param "CACHE_CARGO_GIT"))) +(allow file-write* (subpath (param "CACHE_NPM"))) +(allow file-write* (subpath (param "CACHE_PIP"))) +(allow file-write* (subpath (param "CACHE_PLAYWRIGHT"))) +(allow file-write* (literal (param "CACHE_CARGO_PACKAGE"))) +(allow file-write* (regex #"^/dev/")) +"#; + +/// Start `sandbox-exec` with a do-nothing profile. A nested sandbox reports +/// rc=71 (`Operation not permitted`), which is an honest "unavailable". +pub fn probe() -> SandboxAvailability { + let mut command = Command::new(SANDBOX_EXEC); + command + .arg("-p") + .arg(PROBE_PROFILE) + .arg("true") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + match crate::bounded_process::run_with_timeout(&mut command, PROBE_TIMEOUT) { + Ok(output) if output.success() => SandboxAvailability::Available, + Ok(output) => SandboxAvailability::Unavailable(format!( + "sandbox probe exited with {}", + output + .status + .map(|status| status.to_string()) + .unwrap_or_else(|| "no status".to_string()) + )), + Err(err) => { + SandboxAvailability::Unavailable(format!("sandbox probe could not start: {err}")) + } + } +} + +/// Rewrite `command` into `sandbox-exec -p -D ... `. +/// +/// The process's program, arguments, environment changes, and working directory +/// are transferred, and `TMPDIR` is set explicitly to the run temp directory. +/// Standard I/O is intentionally left unset: it does not survive the transfer, +/// so callers set it on the returned command. +pub fn wrap(command: Command, policy: &SandboxPolicy) -> Command { + let mut wrapped = Command::new(SANDBOX_EXEC); + wrapped.arg("-p").arg(PROFILE); + define(&mut wrapped, "WS", Some(&policy.workspace_root)); + define(&mut wrapped, "TMP", Some(&policy.run_tmp_dir)); + define( + &mut wrapped, + "CACHE_CARGO_REGISTRY", + policy.caches.cargo_registry.as_deref(), + ); + define( + &mut wrapped, + "CACHE_CARGO_GIT", + policy.caches.cargo_git.as_deref(), + ); + define(&mut wrapped, "CACHE_NPM", policy.caches.npm.as_deref()); + define(&mut wrapped, "CACHE_PIP", policy.caches.pip.as_deref()); + define( + &mut wrapped, + "CACHE_PLAYWRIGHT", + policy.caches.playwright.as_deref(), + ); + define( + &mut wrapped, + "CACHE_CARGO_PACKAGE", + policy.caches.cargo_package_cache.as_deref(), + ); + + wrapped.arg(command.get_program()); + for arg in command.get_args() { + wrapped.arg(arg); + } + for (key, value) in command.get_envs() { + match value { + Some(value) => { + wrapped.env(key, value); + } + None => { + wrapped.env_remove(key); + } + } + } + if let Some(dir) = command.get_current_dir() { + wrapped.current_dir(dir); + } + wrapped.env("TMPDIR", &policy.run_tmp_dir); + wrapped +} + +fn define(command: &mut Command, name: &str, path: Option<&Path>) { + let value = path + .map(canonical) + .unwrap_or_else(|| UNSET_PATH.to_string()); + command.arg("-D").arg(format!("{name}={value}")); +} + +fn canonical(path: &Path) -> String { + path.canonicalize() + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .into_owned() +} diff --git a/tests/corpus/apps/issue502-indirect-write/expectations.toml b/tests/corpus/apps/issue502-indirect-write/expectations.toml new file mode 100644 index 00000000..d368da92 --- /dev/null +++ b/tests/corpus/apps/issue502-indirect-write/expectations.toml @@ -0,0 +1,21 @@ +case_id = "issue502-indirect-write" +source = "Issue #655 (stage 1a of #502): the lexical Bash write guard cannot see writes performed inside a command body, so `sh -c 'cp ...'`, `eval`, `xargs`, `find -exec`, an interpreter's open(...,'w'), dd of=, sed -i, and a hardlink to an outside inode are closed by the OS write sandbox instead. This corpus declares the indirect-command table; tests/issue502_os_sandbox_write.rs drives each row through the Seatbelt backend and asserts the outside fixture is unchanged." +acceptance_passed = true +primary_reason = "pass" + +[compile] +expect = "not_checked" + +[fixture_contains] +fixtures/indirect-cases.jsonl = [ + "\"id\": \"sh-c-cp\"", + "\"id\": \"eval-redirect\"", + "\"id\": \"xargs-cp\"", + "\"id\": \"python-open\"", + "\"id\": \"find-exec-cp\"", + "\"id\": \"dd-of\"", + "\"id\": \"sed-inplace\"", + "\"id\": \"hardlink-outside\"", + "\"id\": \"inside-write\"", + "\"id\": \"python-open-inside\"" +] diff --git a/tests/corpus/apps/issue502-indirect-write/fixtures/indirect-cases.jsonl b/tests/corpus/apps/issue502-indirect-write/fixtures/indirect-cases.jsonl new file mode 100644 index 00000000..527e78cf --- /dev/null +++ b/tests/corpus/apps/issue502-indirect-write/fixtures/indirect-cases.jsonl @@ -0,0 +1,10 @@ +{"id": "sh-c-cp", "command": "sh -c 'cp a.txt escape/secret.txt'", "lexicalR": false, "exitNonzero": true, "note": "the lexical guard does not see inside the quoted sh -c body"} +{"id": "eval-redirect", "command": "eval 'echo x > escape/secret.txt'", "lexicalR": false, "exitNonzero": true, "note": "eval hides the redirect from the lexical guard"} +{"id": "xargs-cp", "command": "printf '%s\\n' a.txt | xargs -I{} cp {} escape/secret.txt", "lexicalR": true, "exitNonzero": true, "note": "the lexical guard rejects the spelled escape; the sandbox denies it independently"} +{"id": "python-open", "command": "python3 -c 'open(\"escape/secret.txt\",\"w\").write(\"x\")'", "lexicalR": false, "exitNonzero": true, "note": "the write is inside the interpreter, invisible to the lexical guard"} +{"id": "find-exec-cp", "command": "find . -name a.txt -exec cp {} escape/secret.txt \\;", "lexicalR": true, "exitNonzero": false, "note": "find may report success even when its -exec child is denied; the outside fixture is the assertion"} +{"id": "dd-of", "command": "dd if=a.txt of=escape/secret.txt", "lexicalR": true, "exitNonzero": true, "note": "dd opens the outside inode for writing"} +{"id": "sed-inplace", "command": "sed -i s/OUTSIDE_SECRET_CONTENT_502a/CHANGED/ escape/secret.txt", "lexicalR": true, "exitNonzero": true, "note": "sed -i rewrites the outside inode via its temp-file protocol"} +{"id": "hardlink-outside", "command": "ln escape/secret.txt hard.txt", "lexicalR": true, "exitNonzero": true, "note": "creating a hardlink to an outside inode"} +{"id": "inside-write", "command": "echo inside > inside.txt", "lexicalR": false, "exitNonzero": false, "note": "control: a workspace write stays allowed"} +{"id": "python-open-inside", "command": "python3 -c 'open(\"inside.txt\",\"w\").write(\"x\")'", "lexicalR": false, "exitNonzero": false, "note": "control: an interpreter write inside the workspace stays allowed"} diff --git a/tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md b/tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md new file mode 100644 index 00000000..0a5fa24d --- /dev/null +++ b/tests/corpus/apps/issue502-indirect-write/fixtures/macos-probe-results.md @@ -0,0 +1,70 @@ +# macOS Seatbelt applied-probe record (Issue #655, stage 1a) + +Recorded on a non-nested macOS environment where `sandbox-exec -p +'(version 1)(allow default)' true` succeeds (`rc=0`). Drive this record with +the commands below if it needs to be reproduced; the corpus table +`indirect-cases.jsonl` is the machine-checked part and +`tests/issue502_os_sandbox_write.rs` runs it. + +## Sandbox profile used + +Writes are confined to the workspace (`WS`), the run temp directory (`TMP`), the +known dependency caches (cargo registry/git/package-cache, npm, pip, +Playwright browsers), and device files (`/dev/*`). Reads and network are not +changed. + +## Write-escape probes (must fail) + +| command form | sandbox outcome | outside fixture | +| --- | --- | --- | +| `sh -c 'cp a.txt escape/secret.txt'` | denied | unchanged | +| `eval 'echo x > escape/secret.txt'` | denied | unchanged | +| `xargs -I{} cp {} escape/secret.txt` | denied | unchanged | +| `python3 -c 'open("escape/secret.txt","w")'` | denied | unchanged | +| `find . -exec cp {} escape/secret.txt \;` | denied | unchanged | +| `dd of=escape/secret.txt` | denied | unchanged | +| `sed -i ... escape/secret.txt` | denied | unchanged | +| `ln escape/secret.txt hard.txt` (outside inode) | denied (EPERM) | unchanged | +| write to the system temp directory | denied | not created | + +## Required tools inside the sandbox (must succeed) + +| command | outcome | +| --- | --- | +| `mktemp -p "$TMPDIR" probe.XXXXXX` | success | +| `mktemp "$TMPDIR/probe.XXXXXX"` | success | +| `git status --porcelain` | success | +| `cargo --version` | success | +| `node -e '...'` | success | +| `python3 -c '...'` | success | +| a workspace write (`echo inside > inside.txt`) | success | + +`mktemp` note: macOS `mktemp` ignores `TMPDIR` and would target the per-user +temp directory, which the sandbox denies by design. Passing `-p "$TMPDIR"` or a +template inside the run temp directory makes it succeed. A bare `mktemp` fails +under the sandbox and is not claimed to work. + +## Heavy probes + +| probe | outcome | note | +| --- | --- | --- | +| dependency-less `cargo build` | success | writes only inside the workspace `target/`; cargo caches allowed | +| `npm ci` (cached, offline) | success | writes only inside the workspace and the npm cache | +| `next build` (cached Next.js, offline, `NEXT_TELEMETRY_DISABLED=1`) | success | writes `.next/` inside the workspace | +| Playwright Chromium launch (headless, cached browser) | success | `PLAYWRIGHT_BROWSERS_PATH` cache allowed; no nested-sandbox failure observed on this host | + +If a future host fails a heavy probe, the cause is recorded here and the +allow-list or `--no-sandbox` need is documented rather than hidden. + +## Unavailable-backend probes + +Simulated nested sandbox (`rc=71`) is covered by +`tests/issue502_os_sandbox_write.rs`: + +- `auto`: warns once and records `bash_sandbox_status` with `applied=false` and + the reason; Bash and verifier commands continue with lexical checks only. +- `required`: Bash and verifier commands fail with `bash_sandbox_unavailable`. +- `off`: no warning, `applied=false`, no reason; the previous behavior is kept. + +A real nested invocation (`sandbox-exec` inside `sandbox-exec`) exits `71` +(`Operation not permitted`), which the probe reports as unavailable. diff --git a/tests/issue502_os_sandbox_write.rs b/tests/issue502_os_sandbox_write.rs new file mode 100644 index 00000000..4db92e5e --- /dev/null +++ b/tests/issue502_os_sandbox_write.rs @@ -0,0 +1,457 @@ +#![cfg(unix)] + +//! Issue #655 (stage 1a): OS write confinement for model Bash and verifier +//! commands. The lexical guard cannot see writes performed *inside* a command +//! (`sh -c 'cp ...'`, `eval`, `xargs`, `find -exec`, an interpreter's +//! `open(..., 'w')`, `dd of=`, `sed -i`) or a hardlink to an outside inode. +//! This file drives the macOS Seatbelt backend through the same `wrap_command` +//! the two launch points use, so an escape fails and the outside fixture does +//! not change. +//! +//! The applied cases are `#[cfg(target_os = "macos")]` and no-op when the +//! backend is unavailable (a nested sandbox reports rc=71); on Linux CI only the +//! `off` and unavailable paths run, matching the issue's acceptance note. The +//! outside fixtures live inside one test tempdir and are never written outside +//! it. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::sync::{Mutex, MutexGuard}; +use std::time::Duration; + +use commandagent::bounded_process::{self, BoundedProcessOutput}; +use commandagent::minimal_loop::verifier_env; +use commandagent::planner::verify::{NormalizedVerifyCommand, normalize_verify_command}; +use commandagent::tools::allow_policy::bash_verify_command_is_auto_approvable; +use commandagent::tools::bash::{self, BashOutcome, BashOutcomeKind, path_confinement_rejection}; +use commandagent::tools::os_sandbox::{self, SandboxAvailability, SandboxMode, SandboxPolicy}; +use commandagent::tools::sensitive_path::command_references_secret; +use serde::Deserialize; + +const SECRET: &str = "OUTSIDE_SECRET_CONTENT_502a"; +const NESTED: &str = "sandbox-exec probe exited with exit status: 71"; +const TIMEOUT: Duration = Duration::from_secs(120); + +/// The sandbox state is process-wide, so the tests that install it serialize. +static SANDBOX_LOCK: Mutex<()> = Mutex::new(()); + +fn sandbox_lock() -> MutexGuard<'static, ()> { + SANDBOX_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +struct Fixture { + _dir: tempfile::TempDir, + root: PathBuf, + outside_secret: PathBuf, +} + +/// A workspace (`ws`) with an outward `escape` symlink and an outside fixture +/// (`outside/secret.txt`). `outside` is a sibling of the workspace inside the +/// same tempdir, so a rejected write leaves the tempdir-only fixture untouched. +fn fixture() -> Fixture { + let dir = tempfile::tempdir_in(env!("CARGO_TARGET_TMPDIR")).expect("create tempdir"); + let ws = dir.path().join("ws"); + let outside = dir.path().join("outside"); + std::fs::create_dir_all(&ws).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(ws.join("a.txt"), "x\n").unwrap(); + std::fs::write(outside.join("secret.txt"), SECRET).unwrap(); + std::os::unix::fs::symlink(&outside, ws.join("escape")).unwrap(); + let root = ws.canonicalize().unwrap(); + Fixture { + _dir: dir, + root, + outside_secret: outside.join("secret.txt"), + } +} + +fn read_secret(path: &Path) -> String { + std::fs::read_to_string(path).unwrap_or_else(|_| "".to_string()) +} + +fn run_tmp_path(fixture: &Fixture) -> PathBuf { + fixture._dir.path().join("run-tmp") +} + +fn backend_available() -> bool { + matches!(os_sandbox::probe(), SandboxAvailability::Available) +} + +/// Install an available sandbox for the fixture and create its run temp dir. +fn install_available(fixture: &Fixture) { + let run_tmp = run_tmp_path(fixture); + std::fs::create_dir_all(&run_tmp).unwrap(); + os_sandbox::install_for_test( + SandboxMode::Auto, + SandboxPolicy::new(&fixture.root, &run_tmp), + SandboxAvailability::Available, + ); +} + +fn install_unavailable(mode: SandboxMode, fixture: &Fixture, reason: &str) { + os_sandbox::install_for_test( + mode, + SandboxPolicy::new(&fixture.root, &run_tmp_path(fixture)), + SandboxAvailability::Unavailable(reason.to_string()), + ); +} + +/// Run through the model Bash launch point (`sh -c`), including the lexical +/// guard. This is the production path the verify interval also uses. +fn run_via_bash(root: &Path, command: &str) -> BashOutcome { + bash::run_structured_cancel_and_force(command, root, false, TIMEOUT, || false, || false) + .expect("bash launch") +} + +/// Build `sh -c ` and apply the installed sandbox directly, bypassing +/// the lexical guard, so the OS layer is measured on its own. +fn run_wrapped(root: &Path, command: &str) -> BoundedProcessOutput { + let mut process = Command::new("sh"); + process.arg("-c").arg(command).current_dir(root); + let mut process = os_sandbox::wrap_command(process).expect("available sandbox wraps"); + process.stdout(Stdio::piped()).stderr(Stdio::piped()); + bounded_process::run_with_timeout(&mut process, TIMEOUT).expect("spawn wrapped command") +} + +fn run_wrapped_success(root: &Path, command: &str) -> (bool, String) { + let output = run_wrapped(root, command); + ( + output.success(), + String::from_utf8_lossy(&output.stderr).into_owned(), + ) +} + +#[derive(Deserialize)] +struct IndirectCase { + id: String, + command: String, + #[serde(rename = "lexicalR")] + lexical_r: bool, + #[serde(rename = "exitNonzero")] + exit_nonzero: bool, +} + +fn indirect_cases() -> Vec { + let path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/corpus/apps/issue502-indirect-write/fixtures/indirect-cases.jsonl"); + let text = std::fs::read_to_string(&path) + .unwrap_or_else(|error| panic!("read {}: {error}", path.display())); + text.lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str(line).unwrap_or_else(|error| panic!("{line:?}: {error}"))) + .collect() +} + +/// The acceptance write set: each indirect form fails (or performs no outside +/// write) under the sandbox, and the outside fixture is unchanged. Rows the +/// lexical guard already rejects are still driven here so the OS layer is shown +/// to block them independently. +#[cfg(target_os = "macos")] +#[test] +fn applied_indirect_writes_are_denied_and_the_outside_fixture_is_unchanged() { + let _guard = sandbox_lock(); + if !backend_available() { + eprintln!("SKIP: sandbox backend unavailable (nested sandbox?)"); + return; + } + let fixture = fixture(); + install_available(&fixture); + + for case in indirect_cases() { + assert_eq!( + path_confinement_rejection(&case.command, &fixture.root).is_some(), + case.lexical_r, + "{}: the lexical decision must be unchanged", + case.id + ); + let output = run_wrapped(&fixture.root, &case.command); + assert_eq!( + !output.success(), + case.exit_nonzero, + "{}: sandbox exit code; stderr={}", + case.id, + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + read_secret(&fixture.outside_secret), + SECRET, + "{}: the outside fixture must not change", + case.id + ); + // When the lexical guard lets the command through, the model Bash path + // itself must agree with the sandbox outcome, proving the launch point + // is wired (a denied write fails; a control row still succeeds). + if !case.lexical_r { + let outcome = run_via_bash(&fixture.root, &case.command); + assert_eq!( + !outcome.is_success(), + case.exit_nonzero, + "{}: the bash launch point; {outcome:?}", + case.id + ); + assert_eq!(read_secret(&fixture.outside_secret), SECRET, "{}", case.id); + } + } + os_sandbox::reset_for_test(); +} + +/// A hardlink to an outside inode is refused by the sandbox. +#[cfg(target_os = "macos")] +#[test] +fn applied_hardlink_to_an_outside_inode_is_denied() { + let _guard = sandbox_lock(); + if !backend_available() { + eprintln!("SKIP: sandbox backend unavailable (nested sandbox?)"); + return; + } + let fixture = fixture(); + install_available(&fixture); + let hardlink = fixture.root.join("hard.txt"); + + let output = run_wrapped(&fixture.root, "ln escape/secret.txt hard.txt"); + assert!( + !output.success(), + "hardlink must be denied: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(!hardlink.exists(), "the hardlink must not be created"); + assert_eq!(read_secret(&fixture.outside_secret), SECRET); + os_sandbox::reset_for_test(); +} + +/// A direct write to the system temp directory (not the run temp directory) is +/// denied. The attempt targets a unique non-existent path and asserts it was +/// never created, so nothing is left outside the fixture. +#[cfg(target_os = "macos")] +#[test] +fn applied_system_temp_write_is_denied() { + let _guard = sandbox_lock(); + if !backend_available() { + eprintln!("SKIP: sandbox backend unavailable (nested sandbox?)"); + return; + } + let fixture = fixture(); + install_available(&fixture); + let system_tmp = std::env::temp_dir() + .canonicalize() + .unwrap_or_else(|_| std::env::temp_dir()); + let target = system_tmp.join(format!("commandagent-502a-{}.txt", std::process::id())); + + let command = format!("echo written > '{}'", target.display()); + let output = run_wrapped(&fixture.root, &command); + assert!( + !output.success(), + "a write to the system temp directory must be denied: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(!target.exists(), "the system temp probe must not exist"); + let _ = std::fs::remove_file(&target); + os_sandbox::reset_for_test(); +} + +/// The verifier/acceptance launch point is wired to the same sandbox: a command +/// the lexical guard accepts cannot write outside. +#[cfg(target_os = "macos")] +#[test] +fn applied_verifier_commands_cannot_write_outside() { + let _guard = sandbox_lock(); + if !backend_available() { + eprintln!("SKIP: sandbox backend unavailable (nested sandbox?)"); + return; + } + let fixture = fixture(); + install_available(&fixture); + let command: NormalizedVerifyCommand = + normalize_verify_command("sh -c 'cp a.txt escape/secret.txt'").expect("normalize"); + assert!( + path_confinement_rejection(command.as_str(), &fixture.root).is_none(), + "the lexical guard must pass this form so the sandbox is what blocks it" + ); + + let outcome = + verifier_env::run_structured_for_verify_with_profile(&command, &fixture.root, None, false) + .expect("verifier launch"); + assert!(!outcome.is_success(), "{outcome:?}"); + assert_eq!(read_secret(&fixture.outside_secret), SECRET); + os_sandbox::reset_for_test(); +} + +/// Required tools that only read or use the run temp directory succeed inside +/// the sandbox. `mktemp` is given the allowed directory explicitly because +/// macOS `mktemp` ignores `TMPDIR` and would otherwise target the per-user temp +/// directory, which the sandbox denies by design. +#[cfg(target_os = "macos")] +#[test] +fn applied_required_tools_succeed_inside_the_sandbox() { + let _guard = sandbox_lock(); + if !backend_available() { + eprintln!("SKIP: sandbox backend unavailable (nested sandbox?)"); + return; + } + let fixture = fixture(); + install_available(&fixture); + std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(&fixture.root) + .status() + .expect("git init"); + + for command in [ + "mktemp -p \"$TMPDIR\" probe.XXXXXX", + "mktemp \"$TMPDIR/probe.XXXXXX\"", + "git status --porcelain", + "cargo --version", + "node -e 'process.stdout.write(\"ok\")'", + "python3 -c 'print(\"ok\")'", + "echo inside > inside.txt", + ] { + let (ok, stderr) = run_wrapped_success(&fixture.root, command); + assert!(ok, "expected success: {command}; stderr={stderr}"); + } + assert!(fixture.root.join("inside.txt").is_file()); + os_sandbox::reset_for_test(); +} + +/// A dependency-less `cargo build` succeeds when the cargo caches are allowed. +#[cfg(target_os = "macos")] +#[test] +#[ignore = "heavy: run explicitly to record the applied result"] +fn applied_cargo_build_succeeds() { + let _guard = sandbox_lock(); + if !backend_available() { + return; + } + let fixture = fixture(); + install_available(&fixture); + let src = fixture.root.join("src"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write( + fixture.root.join("Cargo.toml"), + "[package]\nname = \"issue502-probe\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .unwrap(); + std::fs::write(src.join("main.rs"), "fn main() {}\n").unwrap(); + + let (ok, stderr) = run_wrapped_success(&fixture.root, "cargo build"); + assert!(ok, "cargo build failed: {stderr}"); + assert!(fixture.root.join("target/debug/issue502-probe").is_file()); + os_sandbox::reset_for_test(); +} + +/// `required` fails both launch points with `bash_sandbox_unavailable`, while +/// `auto` warns once, records `applied=false`, and continues with lexical checks. +#[test] +fn required_and_unavailable_fail_honestly_and_auto_continues() { + let _guard = sandbox_lock(); + let fixture = fixture(); + + install_unavailable(SandboxMode::Required, &fixture, NESTED); + let outcome = run_via_bash(&fixture.root, "true"); + assert_eq!(outcome.kind, BashOutcomeKind::Blocked, "{outcome:?}"); + assert!( + outcome.summary.contains("bash_sandbox_unavailable"), + "{}", + outcome.summary + ); + let command: NormalizedVerifyCommand = normalize_verify_command("true").unwrap(); + let verifier = + verifier_env::run_structured_for_verify_with_profile(&command, &fixture.root, None, false) + .unwrap(); + assert_eq!(verifier.kind, BashOutcomeKind::Blocked, "{verifier:?}"); + assert!( + verifier.summary.contains("bash_sandbox_unavailable"), + "{}", + verifier.summary + ); + + install_unavailable(SandboxMode::Auto, &fixture, NESTED); + let status = os_sandbox::status().unwrap(); + assert!(!status.applied); + assert_eq!(status.mode.as_str(), "auto"); + assert_eq!(status.reason.as_deref(), Some(NESTED)); + let warning = os_sandbox::startup_warning().expect("auto warns once"); + assert!(warning.contains(NESTED), "{warning}"); + assert!( + run_via_bash(&fixture.root, "true").is_success(), + "auto must continue with lexical checks only" + ); + + os_sandbox::reset_for_test(); +} + +/// `off` keeps the previous behavior and never claims isolation. +#[test] +fn off_never_claims_isolation() { + let _guard = sandbox_lock(); + let fixture = fixture(); + install_unavailable(SandboxMode::Off, &fixture, "unused"); + let status = os_sandbox::status().unwrap(); + assert!(!status.applied); + assert_eq!(status.reason, None); + assert!(os_sandbox::startup_warning().is_none()); + assert!(run_via_bash(&fixture.root, "true").is_success()); + os_sandbox::reset_for_test(); +} + +fn lexical_triple(root: &Path, command: &str) -> (bool, bool, bool) { + ( + path_confinement_rejection(command, root).is_some(), + command_references_secret(command).is_some(), + bash_verify_command_is_auto_approvable(command, root), + ) +} + +/// With the sandbox off or unavailable, the lexical decision is exactly what it +/// is with no sandbox installed: no row flips R/S from 1 to 0, and no row flips +/// V from 0 to 1. +#[test] +fn sandbox_off_and_unavailable_do_not_change_the_lexical_decision() { + let _guard = sandbox_lock(); + let fixture = fixture(); + let mut commands: Vec = indirect_cases().into_iter().map(|c| c.command).collect(); + commands.extend( + [ + "true", + "cargo test", + "cat README.md", + "echo $HOME", + "sh -c 'cat a.txt'", + "cat a.txt", + ] + .into_iter() + .map(str::to_string), + ); + + os_sandbox::reset_for_test(); + let baseline: Vec<(bool, bool, bool)> = commands + .iter() + .map(|command| lexical_triple(&fixture.root, command)) + .collect(); + + for (mode, availability) in [ + ( + SandboxMode::Off, + SandboxAvailability::Unavailable("off".to_string()), + ), + ( + SandboxMode::Auto, + SandboxAvailability::Unavailable(NESTED.to_string()), + ), + ] { + os_sandbox::install_for_test( + mode, + SandboxPolicy::new(&fixture.root, &run_tmp_path(&fixture)), + availability, + ); + for (command, expected) in commands.iter().zip(&baseline) { + assert_eq!( + &lexical_triple(&fixture.root, command), + expected, + "{command:?}: the lexical decision must not change" + ); + } + } + os_sandbox::reset_for_test(); +} From 36d0b3ebaf2c6c9e306f9ce2a382e50da7e522bc Mon Sep 17 00:00:00 2001 From: kewton Date: Sun, 11 Oct 2026 21:04:37 +0900 Subject: [PATCH 2/3] Cover the sandbox-availability probe classification with tests Split the probe's exit-status decision into `classify_probe_exit` so the "only rc=0 is Available" rule is fixed by a unit test (rc=0, rc=71 with the status in the reason, and a missing status), and add a macOS integration test that asserts `probe()` matches a direct `sandbox-exec` run, which stays honest under a nested sandbox. Product behavior, the lexical guard, events, and existing expectations are unchanged. Co-authored-by: CommandCodeBot --- src/tools/os_sandbox/seatbelt.rs | 68 +++++++++++++++++++++++++----- tests/issue502_os_sandbox_write.rs | 21 +++++++++ 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/tools/os_sandbox/seatbelt.rs b/src/tools/os_sandbox/seatbelt.rs index 80ee79d8..4f089852 100644 --- a/src/tools/os_sandbox/seatbelt.rs +++ b/src/tools/os_sandbox/seatbelt.rs @@ -40,6 +40,18 @@ const PROFILE: &str = r#" /// Start `sandbox-exec` with a do-nothing profile. A nested sandbox reports /// rc=71 (`Operation not permitted`), which is an honest "unavailable". pub fn probe() -> SandboxAvailability { + match run_probe() { + Ok(output) => classify_probe_exit( + output.success(), + output.status.map(|status| status.to_string()), + ), + Err(err) => { + SandboxAvailability::Unavailable(format!("sandbox probe could not start: {err}")) + } + } +} + +fn run_probe() -> std::io::Result { let mut command = Command::new(SANDBOX_EXEC); command .arg("-p") @@ -48,18 +60,20 @@ pub fn probe() -> SandboxAvailability { .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()); - match crate::bounded_process::run_with_timeout(&mut command, PROBE_TIMEOUT) { - Ok(output) if output.success() => SandboxAvailability::Available, - Ok(output) => SandboxAvailability::Unavailable(format!( + crate::bounded_process::run_with_timeout(&mut command, PROBE_TIMEOUT) +} + +/// Only a zero exit status means the backend can start. rc=71 (a nested +/// sandbox) and every other non-zero status are unavailable, with the status in +/// the reason; a missing status is still unavailable. +fn classify_probe_exit(success: bool, status: Option) -> SandboxAvailability { + if success { + SandboxAvailability::Available + } else { + SandboxAvailability::Unavailable(format!( "sandbox probe exited with {}", - output - .status - .map(|status| status.to_string()) - .unwrap_or_else(|| "no status".to_string()) - )), - Err(err) => { - SandboxAvailability::Unavailable(format!("sandbox probe could not start: {err}")) - } + status.unwrap_or_else(|| "no status".to_string()) + )) } } @@ -131,3 +145,35 @@ fn canonical(path: &Path) -> String { .to_string_lossy() .into_owned() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn probe_classifies_a_zero_exit_as_available() { + assert_eq!( + classify_probe_exit(true, Some("exit status: 0".to_string())), + SandboxAvailability::Available + ); + } + + #[test] + fn probe_classifies_a_nested_exit_as_unavailable_with_the_status() { + let result = classify_probe_exit(false, Some("exit status: 71".to_string())); + match result { + SandboxAvailability::Unavailable(reason) => { + assert!(reason.contains("71"), "{reason}"); + } + SandboxAvailability::Available => panic!("rc=71 must be unavailable"), + } + } + + #[test] + fn probe_classifies_a_missing_status_as_unavailable() { + assert!(matches!( + classify_probe_exit(false, None), + SandboxAvailability::Unavailable(_) + )); + } +} diff --git a/tests/issue502_os_sandbox_write.rs b/tests/issue502_os_sandbox_write.rs index 4db92e5e..db642735 100644 --- a/tests/issue502_os_sandbox_write.rs +++ b/tests/issue502_os_sandbox_write.rs @@ -340,6 +340,27 @@ fn applied_cargo_build_succeeds() { os_sandbox::reset_for_test(); } +/// The probe agrees with a direct `/usr/bin/sandbox-exec` run: `rc=0` is +/// available and any non-zero rc (for example 71 under a nested sandbox) is +/// unavailable. This stays honest whether or not the test process is already +/// sandboxed, so it catches a probe that always claims one verdict. +#[cfg(target_os = "macos")] +#[test] +fn probe_matches_a_direct_sandbox_exec_run() { + let direct = std::process::Command::new("/usr/bin/sandbox-exec") + .args(["-p", "(version 1)(allow default)", "true"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .expect("run sandbox-exec directly"); + assert_eq!( + matches!(os_sandbox::probe(), SandboxAvailability::Available), + direct.success(), + "probe must match the direct sandbox-exec exit status ({direct})" + ); +} + /// `required` fails both launch points with `bash_sandbox_unavailable`, while /// `auto` warns once, records `applied=false`, and continues with lexical checks. #[test] From 4a2296e8a27743851079d5d50cce92a21f90c572 Mon Sep 17 00:00:00 2001 From: kewton Date: Mon, 12 Oct 2026 10:26:12 +0900 Subject: [PATCH 3/3] Gate macOS-only sandbox test helpers behind cfg On non-macOS the applied tests are excluded, so their helpers, fixture field, and imports became dead code and broke `-D warnings` on Linux CI. Gate `outside_secret`, `read_secret`, `backend_available`, `install_available`, `run_wrapped`, `run_wrapped_success`, the `IndirectCase` fields only the applied tests read, and the `std::process`/`bounded_process` imports. Test names, counts, expectations, `src/`, and the corpus are unchanged. Co-authored-by: CommandCodeBot --- tests/issue502_os_sandbox_write.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/issue502_os_sandbox_write.rs b/tests/issue502_os_sandbox_write.rs index db642735..eef2dc04 100644 --- a/tests/issue502_os_sandbox_write.rs +++ b/tests/issue502_os_sandbox_write.rs @@ -15,10 +15,12 @@ //! it. use std::path::{Path, PathBuf}; +#[cfg(target_os = "macos")] use std::process::{Command, Stdio}; use std::sync::{Mutex, MutexGuard}; use std::time::Duration; +#[cfg(target_os = "macos")] use commandagent::bounded_process::{self, BoundedProcessOutput}; use commandagent::minimal_loop::verifier_env; use commandagent::planner::verify::{NormalizedVerifyCommand, normalize_verify_command}; @@ -44,6 +46,7 @@ fn sandbox_lock() -> MutexGuard<'static, ()> { struct Fixture { _dir: tempfile::TempDir, root: PathBuf, + #[cfg(target_os = "macos")] outside_secret: PathBuf, } @@ -63,10 +66,12 @@ fn fixture() -> Fixture { Fixture { _dir: dir, root, + #[cfg(target_os = "macos")] outside_secret: outside.join("secret.txt"), } } +#[cfg(target_os = "macos")] fn read_secret(path: &Path) -> String { std::fs::read_to_string(path).unwrap_or_else(|_| "".to_string()) } @@ -75,11 +80,13 @@ fn run_tmp_path(fixture: &Fixture) -> PathBuf { fixture._dir.path().join("run-tmp") } +#[cfg(target_os = "macos")] fn backend_available() -> bool { matches!(os_sandbox::probe(), SandboxAvailability::Available) } /// Install an available sandbox for the fixture and create its run temp dir. +#[cfg(target_os = "macos")] fn install_available(fixture: &Fixture) { let run_tmp = run_tmp_path(fixture); std::fs::create_dir_all(&run_tmp).unwrap(); @@ -107,6 +114,7 @@ fn run_via_bash(root: &Path, command: &str) -> BashOutcome { /// Build `sh -c ` and apply the installed sandbox directly, bypassing /// the lexical guard, so the OS layer is measured on its own. +#[cfg(target_os = "macos")] fn run_wrapped(root: &Path, command: &str) -> BoundedProcessOutput { let mut process = Command::new("sh"); process.arg("-c").arg(command).current_dir(root); @@ -115,6 +123,7 @@ fn run_wrapped(root: &Path, command: &str) -> BoundedProcessOutput { bounded_process::run_with_timeout(&mut process, TIMEOUT).expect("spawn wrapped command") } +#[cfg(target_os = "macos")] fn run_wrapped_success(root: &Path, command: &str) -> (bool, String) { let output = run_wrapped(root, command); ( @@ -125,10 +134,13 @@ fn run_wrapped_success(root: &Path, command: &str) -> (bool, String) { #[derive(Deserialize)] struct IndirectCase { - id: String, command: String, + #[cfg(target_os = "macos")] + id: String, + #[cfg(target_os = "macos")] #[serde(rename = "lexicalR")] lexical_r: bool, + #[cfg(target_os = "macos")] #[serde(rename = "exitNonzero")] exit_nonzero: bool, }