#659 [security][tools] workflow の子 node の隔離の照合、run の一時ディレクトリの後片付け、mktemp の案内

SECURITY.md

@@ -68,6 +68,16 @@ before the shell starts.
   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.
+  - Runtime dependency setup runs outside the sandbox: the setup steps that
+    execute `setup.py`, npm `postinstall`/`prepare` hooks, and similar
+    install-time scripts run before or outside the confined `sh -c` launch
+    points, so the sandbox does not constrain their writes. Confining dependency
+    setup is a later stage (#658).
+  - A hardlink that already exists in the workspace before the run and points at
+    an inode outside the workspace can still be written through. Seatbelt decides
+    by path, so it allows writing the workspace path even though the inode lives
+    outside; the sandbox only denies *creating* a new hardlink to an outside
+    inode.
 - 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.

src/lib.rs

@@ -133,10 +133,16 @@ pub fn run_with_provider_options(
     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}");
-    }
+    // Only actions that can run model/verifier Bash need the OS write sandbox and
+    // its per-run temp directory; read-only actions must not create one. The
+    // guard removes the run temp directory on every exit path.
+    let _sandbox_cleanup = action_uses_write_sandbox(&config.action).then(|| {
+        tools::os_sandbox::install(sandbox_mode, &config.workspace_root, &config.state_dir);
+        if let Some(warning) = tools::os_sandbox::startup_warning() {
+            eprintln!("{warning}");
+        }
+        tools::os_sandbox::cleanup_guard()
+    });
     for profile in planner::extension_profiles::registered() {
         for warning in &profile.warnings {
             eprintln!("warning: {warning}");
@@ -213,6 +219,13 @@ pub(crate) fn run_resolved_config_for_workflow(config: Config) -> anyhow::Result
     // the whole run. Re-entering catch_cli_run here makes the same thread re-lock that
     // mutex and self-deadlock. Run the resolved config directly; the outer boundary
     // still catches any panic raised by this child node run.
+    //
+    // The sandbox was installed once, in the parent's workspace. A child node runs
+    // in its own origin workspace, so reconcile the installed sandbox with that
+    // workspace first: when they differ the sandbox is recorded as not applied
+    // (and `required` fails the node) instead of confining it to the wrong place.
+    // When they match, this is a no-op.
+    tools::os_sandbox::reconcile_workspace(&config.workspace_root);
     run_config(config, None)
 }
 
@@ -454,6 +467,16 @@ fn action_uses_workspace_lock(action: &Action) -> bool {
     )
 }
 
+/// Whether the action can run model Bash or verifier commands, so it needs the OS
+/// write sandbox and its per-run temp directory. Read-only actions do not, and
+/// must not create a temp directory.
+fn action_uses_write_sandbox(action: &Action) -> bool {
+    !matches!(
+        action,
+        Action::Runs(_) | Action::UxDemo | Action::ModelProbe | Action::Doctor
+    )
+}
+
 /// Whether the action starts and stops a run recorded in the event stream.
 ///
 /// `--ux-demo` only renders a scripted demo, so it must not record run
@@ -568,24 +591,11 @@ impl DirectCommandCompletionGuard {
                     }
                     if signals.forever().next().is_some() {
                         if !signal_finalized.swap(true, Ordering::AcqRel) {
-                            let result: anyhow::Result<()> =
-                                Err(anyhow::anyhow!("interrupted by user"));
-                            bounded_process::reap_registered_server_children_for_workspace(
-                                signal_config.eval_events_path.as_deref(),
-                                "direct_cli_sigint",
-                                &signal_config.workspace_root,
-                            );
-                            emit_direct_command_stop_with_status(
+                            let mut stdout = std::io::stdout().lock();
+                            if let Err(err) = finalize_interrupted_direct_command(
                                 &signal_config,
                                 &signal_command,
-                                &result,
-                                DirectCommandStatus::Interrupted,
-                            );
-                            tui::terminal_notifications::finish_process();
-                            let mut stdout = std::io::stdout().lock();
-                            if let Err(err) = write_interrupted_headless_summary(
                                 summary_source.as_ref(),
-                                signal_config.eval_events_path.as_deref(),
                                 &mut stdout,
                             ) {
                                 eprintln!(
@@ -595,6 +605,9 @@ impl DirectCommandCompletionGuard {
                             // Keep stdout locked until exit so the JSON remains the final line.
                             std::process::exit(130);
                         }
+                        // Another thread already finalized; still release the run temp
+                        // directory, which `std::process::exit` would otherwise leak.
+                        tools::os_sandbox::cleanup();
                         std::process::exit(130);
                     }
                 }));
@@ -633,6 +646,39 @@ impl DirectCommandCompletionGuard {
     }
 }
 
+/// The work the direct-CLI SIGINT finalizer runs before `std::process::exit`,
+/// which skips `Drop`: reap server children, emit the interrupted stop event,
+/// write the interrupted headless summary, then release the run temp directory.
+/// The cleanup always runs, even when the summary write fails, so the exit leaks
+/// no temp directory.
+fn finalize_interrupted_direct_command(
+    config: &Config,
+    command: &str,
+    summary_source: Option<&headless_summary::Source>,
+    output: &mut impl Write,
+) -> std::io::Result<()> {
+    let result: anyhow::Result<()> = Err(anyhow::anyhow!("interrupted by user"));
+    bounded_process::reap_registered_server_children_for_workspace(
+        config.eval_events_path.as_deref(),
+        "direct_cli_sigint",
+        &config.workspace_root,
+    );
+    emit_direct_command_stop_with_status(
+        config,
+        command,
+        &result,
+        DirectCommandStatus::Interrupted,
+    );
+    tui::terminal_notifications::finish_process();
+    let write_result = write_interrupted_headless_summary(
+        summary_source,
+        config.eval_events_path.as_deref(),
+        output,
+    );
+    tools::os_sandbox::cleanup();
+    write_result.map(|_| ())
+}
+
 fn write_interrupted_headless_summary(
     source: Option<&headless_summary::Source>,
     events_path: Option<&std::path::Path>,
@@ -2029,6 +2075,41 @@ mod tests {
         );
     }
 
+    #[test]
+    fn sigint_finalizer_releases_the_run_temp_dir_before_exit() {
+        let status = run_ignored_self_test(
+            "tests::sigint_finalizer_releases_the_run_temp_dir_before_exit_child",
+            &[],
+        );
+        assert!(status.success(), "{status}");
+    }
+
+    #[test]
+    #[ignore]
+    fn sigint_finalizer_releases_the_run_temp_dir_before_exit_child() {
+        let dir = tempfile::tempdir().unwrap();
+        let events = dir.path().join(".anvil/runs/test-run/events.jsonl");
+        let mut cfg = config(dir.path().to_path_buf());
+        cfg.eval_events_path = Some(events);
+
+        let run_tmp = dir.path().join("sandbox/run-test");
+        std::fs::create_dir_all(&run_tmp).unwrap();
+        crate::tools::os_sandbox::install_for_test(
+            crate::tools::os_sandbox::SandboxMode::Auto,
+            crate::tools::os_sandbox::SandboxPolicy::new(dir.path(), &run_tmp),
+            crate::tools::os_sandbox::SandboxAvailability::Unavailable("test".to_string()),
+        );
+
+        let mut output = Vec::new();
+        finalize_interrupted_direct_command(&cfg, "--prompt", None, &mut output).unwrap();
+
+        assert!(
+            !run_tmp.exists(),
+            "the SIGINT finalizer must remove the run temp directory"
+        );
+        crate::tools::os_sandbox::reset_for_test();
+    }
+
     #[test]
     fn run_lifecycle_records_incomplete_stop_reason() {
         let dir = tempfile::tempdir().unwrap();
@@ -2134,6 +2215,25 @@ mod tests {
         assert!(!summary.contains("\nStatus: complete\nAction: Repl\nStop reason: completed"));
     }
 
+    #[test]
+    fn read_only_actions_do_not_need_the_write_sandbox() {
+        for action in [
+            Action::Runs(crate::config::RunsRequest::default()),
+            Action::UxDemo,
+            Action::ModelProbe,
+            Action::Doctor,
+        ] {
+            assert!(!action_uses_write_sandbox(&action), "{action:?}");
+        }
+        for action in [
+            Action::Repl,
+            Action::Prompt("goal".into()),
+            Action::Workflow(PathBuf::from("def.yaml"), PathBuf::from("origin")),
+        ] {
+            assert!(action_uses_write_sandbox(&action), "{action:?}");
+        }
+    }
+
     fn run_ignored_self_test(test_name: &str, envs: &[(&str, &str)]) -> std::process::ExitStatus {
         let exe = std::env::current_exe().unwrap();
         let mut command = std::process::Command::new(exe);

src/tools/bash.rs

@@ -1152,11 +1152,42 @@ fn build_summary(command: &str, kind: BashOutcomeKind, stdout: &str, stderr: &st
             break;
         }
     }
-    if lines.is_empty() {
+    let mut summary = if lines.is_empty() {
         format!("command did not succeed: {command}")
     } else {
         lines.join("\n")
+    };
+    if let Some(guidance) =
+        sandbox_temp_dir_guidance(super::os_sandbox::is_applied(), stdout, stderr)
+    {
+        summary.push('\n');
+        summary.push_str(guidance);
+    }
+    summary
+}
+
+/// The guidance appended when the OS write sandbox is applied and the failure is
+/// a permission error against a temp directory — what a bare `mktemp` hits on
+/// macOS, where the per-user temp directory is outside the sandbox. It points at
+/// the run temp directory, which the sandbox allows.
+const SANDBOX_MKTEMP_GUIDANCE: &str = "note: the OS write sandbox denies the macOS per-user temp directory; use `mktemp -p \"$TMPDIR\"` (or a template under `$TMPDIR`) instead of a bare `mktemp`";
+
+/// Whether to append [`SANDBOX_MKTEMP_GUIDANCE`]. It needs the sandbox to be
+/// applied and a permission error against a temp directory; any other failure
+/// gets no guidance.
+fn sandbox_temp_dir_guidance(applied: bool, stdout: &str, stderr: &str) -> Option<&'static str> {
+    if !applied {
+        return None;
     }
+    let combined = format!("{stderr}\n{stdout}").to_ascii_lowercase();
+    let permission = combined.contains("operation not permitted")
+        || combined.contains("permission denied")
+        || combined.contains("access is denied");
+    let temp_dir = combined.contains("mktemp")
+        || combined.contains("mkstemp")
+        || combined.contains("/var/folders/")
+        || combined.contains("/tmp/");
+    (permission && temp_dir).then_some(SANDBOX_MKTEMP_GUIDANCE)
 }
 
 #[cfg(test)]
@@ -1360,6 +1391,20 @@ mod tests {
         assert!(truncated.len() >= MAX_STREAM_BYTES);
     }
 
+    #[test]
+    fn sandbox_temp_dir_guidance_only_for_applied_temp_dir_permission_failures() {
+        let denied =
+            "mktemp: mkstemp failed on /var/folders/ab/tmp.XXXXXX: Operation not permitted";
+        let guidance = sandbox_temp_dir_guidance(true, "", denied).expect("guidance");
+        assert!(guidance.contains("mktemp -p \"$TMPDIR\""), "{guidance}");
+
+        assert!(sandbox_temp_dir_guidance(false, "", denied).is_none());
+        assert!(sandbox_temp_dir_guidance(true, "", "Error: broken").is_none());
+        assert!(
+            sandbox_temp_dir_guidance(true, "", "cat: /etc/shadow: Permission denied").is_none()
+        );
+    }
+
     #[test]
     fn bash_test_output_extracts_failure_summary() {
         let dir = tempfile::tempdir().unwrap();