From f2a01683878e0701453b89aadb93ec47d353b4dc Mon Sep 17 00:00:00 2001 From: Henry Park Date: Sun, 26 Jul 2026 14:51:17 -0700 Subject: [PATCH 1/5] fix(process_sandbox): retire dead DockerProcessSandboxBackend DockerProcessSandboxBackend, its ProcessSandboxBackend trait, and the broker/approval helpers around it (docker.rs, broker.rs, approval.rs, backend.rs) had zero consumers outside this crate and were never constructed in production: `HostProcessExecutor::process_sandbox_executor` defaults to `None` (services.rs), and `with_process_sandbox_executor` is only ever called from test fixtures. Dispatching `system.process_sandbox.run` already returned `missing_process_sandbox_executor` before this change; it still does after, since production wiring in production.rs/process_executor.rs/services.rs is untouched. The dynamic process-compatibility lane is superseded by the persistent per-user sandbox and was already non-functional under the non-root/zero-cap posture. Kept: `PROCESS_SANDBOX_CAPABILITY_ID` (still routed by ID), the `SandboxProcessPlan`/`ValidatedSandboxProcessPlan` types and their validation (plan.rs, validation.rs), and `DEFAULT_PROCESS_SANDBOX_IMAGE` plus tests/docker_security.rs, which exercises the built sandbox image directly and has no dependency on the removed backend. Dropped the async-trait/ironclaw_processes/secrecy/serde_json/tokio/tempfile deps that only the deleted code used. Refs #6686 Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 6 - crates/ironclaw_process_sandbox/Cargo.toml | 9 - .../ironclaw_process_sandbox/src/approval.rs | 120 ---- .../ironclaw_process_sandbox/src/backend.rs | 159 ----- crates/ironclaw_process_sandbox/src/broker.rs | 146 ----- crates/ironclaw_process_sandbox/src/docker.rs | 604 ------------------ crates/ironclaw_process_sandbox/src/lib.rs | 33 +- crates/ironclaw_process_sandbox/src/tests.rs | 577 +---------------- 8 files changed, 11 insertions(+), 1643 deletions(-) delete mode 100644 crates/ironclaw_process_sandbox/src/approval.rs delete mode 100644 crates/ironclaw_process_sandbox/src/backend.rs delete mode 100644 crates/ironclaw_process_sandbox/src/broker.rs delete mode 100644 crates/ironclaw_process_sandbox/src/docker.rs diff --git a/Cargo.lock b/Cargo.lock index a969bcc8f1e..98e9463b031 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4115,15 +4115,9 @@ dependencies = [ name = "ironclaw_process_sandbox" version = "0.1.0" dependencies = [ - "async-trait", "ironclaw_host_api", - "ironclaw_processes", - "secrecy", "serde", - "serde_json", - "tempfile", "thiserror 2.0.18", - "tokio", ] [[package]] diff --git a/crates/ironclaw_process_sandbox/Cargo.toml b/crates/ironclaw_process_sandbox/Cargo.toml index 534013aa444..321187f0597 100644 --- a/crates/ironclaw_process_sandbox/Cargo.toml +++ b/crates/ironclaw_process_sandbox/Cargo.toml @@ -8,15 +8,6 @@ publish = false layer = "runtimes" [dependencies] -async-trait = "0.1" ironclaw_host_api = { path = "../ironclaw_host_api" } -ironclaw_processes = { path = "../ironclaw_processes" } -secrecy = "0.10" serde = { version = "1", features = ["derive"] } -serde_json = "1" thiserror = "2" -tokio = { version = "1", features = ["io-util", "macros", "process", "time"] } - -[dev-dependencies] -tempfile = "3" -tokio = { version = "1", features = ["macros", "rt", "time"] } diff --git a/crates/ironclaw_process_sandbox/src/approval.rs b/crates/ironclaw_process_sandbox/src/approval.rs deleted file mode 100644 index f1ca4f86b39..00000000000 --- a/crates/ironclaw_process_sandbox/src/approval.rs +++ /dev/null @@ -1,120 +0,0 @@ -use serde::{Deserialize, Serialize}; - -use crate::{ - ProcessSandboxPlanError, SandboxCommandPlan, SandboxCredentialBinding, SandboxMount, - SandboxProcessPlan, -}; -use ironclaw_host_api::{RuntimeCredentialTarget, SecretHandle}; - -/// Sanitized authority summary shown before a sandbox process is approved. -/// -/// This structure exposes commands, mount destinations, requested network -/// hosts, and credential aliases/placeholders. It never includes host paths or -/// raw secret material. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct SandboxProcessApprovalSummary { - pub install_command: Option>, - pub run_command: Vec, - pub mounts: Vec, - pub install_allowed_hosts: Vec, - pub allowed_network_hosts: Vec, - pub credentials: Vec, - pub direct_egress_lockdown: bool, -} - -impl SandboxProcessApprovalSummary { - /// Builds a sanitized approval summary from a validated plan. - pub fn from_plan(plan: &SandboxProcessPlan) -> Result { - plan.validate()?; - Ok(Self { - install_command: plan - .install - .as_ref() - .map(|install| command_line(&install.command)), - run_command: command_line(&plan.run), - mounts: vec![ - SandboxApprovalMount::from_mount("workspace", &plan.mounts.workspace), - SandboxApprovalMount::from_mount("tools", &plan.mounts.tools), - SandboxApprovalMount::from_mount("cache", &plan.mounts.cache), - ], - install_allowed_hosts: plan - .install - .as_ref() - .map(|install| install.allowed_hosts.clone()) - .unwrap_or_default(), - allowed_network_hosts: plan.network.runtime_hosts.clone(), - credentials: plan - .credentials - .iter() - .map(SandboxApprovalCredential::from_binding) - .collect(), - direct_egress_lockdown: plan.network.direct_egress_lockdown, - }) - } -} - -/// Approval-facing mount description with the logical mount name. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct SandboxApprovalMount { - pub name: String, - pub container_path: String, - pub writable: bool, -} - -impl SandboxApprovalMount { - fn from_mount(name: &str, mount: &SandboxMount) -> Self { - Self { - name: name.to_string(), - container_path: mount.container_path.clone(), - writable: mount.writable, - } - } -} - -/// Approval-facing credential description with redacted secret data. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct SandboxApprovalCredential { - pub secret_alias: SecretHandle, - pub approved_host: String, - pub placeholder_env: Option, - pub placeholder_value: String, - pub target: String, - pub required: bool, -} - -impl SandboxApprovalCredential { - fn from_binding(binding: &SandboxCredentialBinding) -> Self { - Self { - secret_alias: binding.handle.clone(), - approved_host: binding.approved_host.clone(), - placeholder_env: binding.placeholder_env.clone(), - placeholder_value: binding.placeholder_value.clone(), - target: credential_target_summary(&binding.target), - required: binding.required, - } - } -} - -fn command_line(command: &SandboxCommandPlan) -> Vec { - let mut line = vec![command.command.clone()]; - line.extend(command.args.clone()); - line -} - -fn credential_target_summary(target: &RuntimeCredentialTarget) -> String { - match target { - RuntimeCredentialTarget::Header { name, prefix } => { - format!( - "header:{name}={}", - prefix.as_deref().unwrap_or_default() - ) - } - RuntimeCredentialTarget::QueryParam { name } => format!("query:{name}="), - RuntimeCredentialTarget::PathPlaceholder { placeholder } => { - format!("path:{placeholder}=") - } - RuntimeCredentialTarget::BodyJsonPointer { pointer } => { - format!("body:{pointer}=") - } - } -} diff --git a/crates/ironclaw_process_sandbox/src/backend.rs b/crates/ironclaw_process_sandbox/src/backend.rs deleted file mode 100644 index fc350af8b56..00000000000 --- a/crates/ironclaw_process_sandbox/src/backend.rs +++ /dev/null @@ -1,159 +0,0 @@ -use std::sync::Arc; - -use async_trait::async_trait; -use ironclaw_host_api::{ProcessId, ResourceScope}; -use ironclaw_processes::{ - ProcessCancellationToken, ProcessExecutionError, ProcessExecutionRequest, - ProcessExecutionResult, ProcessExecutor, -}; -use serde::Serialize; -use serde_json::json; -use thiserror::Error; - -use crate::{ - ProcessSandboxPlanError, SandboxProcessPhase, SandboxProcessPlan, ValidatedSandboxProcessPlan, -}; - -/// Backend execution request for a validated sandbox process. -/// -/// The request carries host-owned identity and cancellation state alongside a -/// validated plan. Backends should use `scope` for audit and resource ownership, -/// not as a source of extra authority. -#[derive(Debug, Clone)] -pub struct SandboxProcessRequest { - pub process_id: ProcessId, - pub scope: ResourceScope, - pub plan: ValidatedSandboxProcessPlan, - pub cancellation: ProcessCancellationToken, -} - -/// Completed sandbox process result returned by a backend. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SandboxProcessResult { - pub output: SandboxProcessOutput, -} - -/// Ordered output from each executed sandbox phase. -#[derive(Debug, Clone, PartialEq, Eq, Default)] -pub struct SandboxProcessOutput { - pub phases: Vec, -} - -/// Serializable output for one install or run phase. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub struct SandboxPhaseOutput { - pub phase: SandboxProcessPhase, - pub exit_code: i32, - pub stdout: String, - pub stderr: String, - pub stdout_truncated: bool, - pub stderr_truncated: bool, - pub wall_clock_ms: u64, -} - -/// Stable process sandbox failure kinds exposed through `ProcessExecutor`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ProcessSandboxErrorKind { - InvalidProcessSandboxPlan, - DockerSpawnFailed, - DockerIoFailed, - Cancelled, - Timeout, -} - -impl ProcessSandboxErrorKind { - /// Returns the stable machine-readable error kind string. - pub const fn as_str(self) -> &'static str { - match self { - Self::InvalidProcessSandboxPlan => "invalid_process_sandbox_plan", - Self::DockerSpawnFailed => "docker_spawn_failed", - Self::DockerIoFailed => "docker_io_failed", - Self::Cancelled => "cancelled", - Self::Timeout => "timeout", - } - } -} - -impl std::fmt::Display for ProcessSandboxErrorKind { - fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - formatter.write_str(self.as_str()) - } -} - -/// Backend-level sandbox execution error. -#[derive(Debug, Clone, PartialEq, Eq, Error)] -#[error("process sandbox execution failed: {kind}")] -pub struct ProcessSandboxError { - pub kind: ProcessSandboxErrorKind, -} - -impl ProcessSandboxError { - /// Constructs a sandbox execution error from a stable kind. - pub fn new(kind: ProcessSandboxErrorKind) -> Self { - Self { kind } - } -} - -impl From for ProcessSandboxError { - fn from(_: ProcessSandboxPlanError) -> Self { - Self::new(ProcessSandboxErrorKind::InvalidProcessSandboxPlan) - } -} - -/// Backend contract for executing validated process sandbox requests. -/// -/// Implementations own the physical isolation mechanism. They must not accept -/// raw Docker flags, raw host paths, or raw secret material from plan JSON. -#[async_trait] -pub trait ProcessSandboxBackend: Send + Sync { - /// Executes the validated sandbox request. - async fn execute( - &self, - request: SandboxProcessRequest, - ) -> Result; -} - -/// `ProcessExecutor` adapter for the process sandbox backend. -/// -/// This adapter owns JSON deserialization and validation so generic process -/// callers receive the same stable error kind for malformed or invalid plans. -#[derive(Clone)] -pub struct ProcessSandboxExecutor { - backend: Arc, -} - -impl ProcessSandboxExecutor { - /// Constructs a process executor over a sandbox backend. - pub fn new(backend: Arc) -> Self { - Self { backend } - } -} - -#[async_trait] -impl ProcessExecutor for ProcessSandboxExecutor { - async fn execute( - &self, - request: ProcessExecutionRequest, - ) -> Result { - let plan = serde_json::from_value::(request.input) - .map_err(|_| ProcessExecutionError::new("invalid_process_sandbox_plan"))?; - let plan = ValidatedSandboxProcessPlan::new(plan) - .map_err(|_| ProcessExecutionError::new("invalid_process_sandbox_plan"))?; - let result = self - .backend - .execute(SandboxProcessRequest { - process_id: request.process_id, - scope: request.scope, - plan, - cancellation: request.cancellation, - }) - .await - .map_err(|error| ProcessExecutionError::new(error.kind.as_str()))?; - Ok(ProcessExecutionResult { - output: json!({ - "kind": "process_sandbox_result", - "phases": result.output.phases, - }), - }) - } -} diff --git a/crates/ironclaw_process_sandbox/src/broker.rs b/crates/ironclaw_process_sandbox/src/broker.rs deleted file mode 100644 index 131395a8673..00000000000 --- a/crates/ironclaw_process_sandbox/src/broker.rs +++ /dev/null @@ -1,146 +0,0 @@ -use std::collections::HashMap; - -use ironclaw_host_api::{RuntimeCredentialTarget, SecretHandle}; -use secrecy::{ExposeSecret, SecretString}; -use thiserror::Error; - -use crate::{ - ProcessSandboxPlanError, SandboxCredentialBinding, plan::validate_unique_credential_targets, -}; - -/// Header rewrite performed by the credential broker. -/// -/// `old_value` is the placeholder-bearing value from the sandbox request. The -/// replacement secret is retained as `SecretString` so debug output does not -/// expose raw credential material. -#[derive(Debug, Clone)] -pub struct BrokerHeaderRewrite { - pub name: String, - pub old_value: String, - pub new_value: SecretString, - pub secret_alias: SecretHandle, -} - -/// Broker rewrite result containing forwarded headers and audit metadata. -#[derive(Debug, Clone)] -pub struct BrokerRewriteResult { - pub headers: Vec<(String, String)>, - pub rewrites: Vec, -} - -/// Failure returned when a required broker rewrite cannot be completed. -#[derive(Debug, Clone, PartialEq, Eq, Error)] -pub enum BrokerRewriteError { - #[error("required secret {secret_alias} is missing")] - MissingRequiredSecret { secret_alias: SecretHandle }, -} - -/// Credential rewrite policy for brokered sandbox egress. -/// -/// The policy matches approved hosts and header placeholders, rewrites them to -/// leased secrets, and provides best-effort redaction for broker-visible error -/// paths. -#[derive(Debug, Clone)] -pub struct SandboxBrokerPolicy { - bindings: Vec, -} - -impl SandboxBrokerPolicy { - /// Validates and constructs a broker rewrite policy. - pub fn new(bindings: Vec) -> Result { - let policy = Self { bindings }; - for binding in &policy.bindings { - binding.validate()?; - } - validate_unique_credential_targets(&policy.bindings)?; - Ok(policy) - } - - /// Rewrites approved placeholder headers for the request host. - pub fn rewrite_headers( - &self, - host: &str, - headers: Vec<(String, String)>, - secrets: &HashMap, - ) -> Result { - let mut rewrites = Vec::new(); - let mut rewritten_headers = Vec::with_capacity(headers.len()); - for (name, value) in headers { - let Some(binding) = self.matching_header_binding(host, &name, &value) else { - rewritten_headers.push((name, value)); - continue; - }; - let Some(secret) = secrets.get(&binding.handle) else { - if binding.required { - return Err(BrokerRewriteError::MissingRequiredSecret { - secret_alias: binding.handle.clone(), - }); - } - rewritten_headers.push((name, value)); - continue; - }; - let RuntimeCredentialTarget::Header { prefix, .. } = &binding.target else { - rewritten_headers.push((name, value)); - continue; - }; - let prefix = prefix.as_deref().unwrap_or_default(); - let new_plain = format!("{prefix}{}", secret.expose_secret()); - rewrites.push(BrokerHeaderRewrite { - name: name.clone(), - old_value: value, - new_value: SecretString::from(new_plain.clone()), - secret_alias: binding.handle.clone(), - }); - rewritten_headers.push((name, new_plain)); - } - Ok(BrokerRewriteResult { - headers: rewritten_headers, - rewrites, - }) - } - - /// Redacts leased secret values from text before returning it to callers. - pub fn sanitize_text( - &self, - text: &str, - secrets: &HashMap, - ) -> String { - let mut values = secrets - .values() - .map(|secret| secret.expose_secret()) - .filter(|value| !value.is_empty()) - .collect::>(); - values.sort_unstable_by_key(|value| std::cmp::Reverse(value.len())); - values.into_iter().fold(text.to_string(), |acc, value| { - acc.replace(value, "[REDACTED]") - }) - } - - fn matching_header_binding( - &self, - host: &str, - header_name: &str, - header_value: &str, - ) -> Option<&SandboxCredentialBinding> { - let host_without_port = host.split(':').next().unwrap_or(host); - self.bindings.iter().find(|binding| { - binding - .approved_host - .eq_ignore_ascii_case(host_without_port) - && match &binding.target { - RuntimeCredentialTarget::Header { name, prefix } => { - name.eq_ignore_ascii_case(header_name) - && header_value - == format!( - "{}{}", - prefix.as_deref().unwrap_or_default(), - binding.placeholder_value - ) - } - RuntimeCredentialTarget::QueryParam { .. } - | RuntimeCredentialTarget::PathPlaceholder { .. } - | RuntimeCredentialTarget::BodyJsonPointer { .. } => false, - } - }) - } -} diff --git a/crates/ironclaw_process_sandbox/src/docker.rs b/crates/ironclaw_process_sandbox/src/docker.rs deleted file mode 100644 index cfff89de1f2..00000000000 --- a/crates/ironclaw_process_sandbox/src/docker.rs +++ /dev/null @@ -1,604 +0,0 @@ -use std::{ - net::Ipv4Addr, - path::{Path, PathBuf}, - process::Stdio, - sync::{ - Arc, - atomic::{AtomicU64, Ordering}, - }, - time::{Duration, Instant}, -}; - -use async_trait::async_trait; -use serde::{Deserialize, Serialize}; -use thiserror::Error; -use tokio::{ - io::{AsyncRead, AsyncReadExt}, - process::{Child, Command}, - time, -}; - -use crate::{ - DEFAULT_PROCESS_SANDBOX_IMAGE, DEFAULT_STDERR_LIMIT, DEFAULT_STDOUT_LIMIT, DEFAULT_TIMEOUT_MS, - ProcessSandboxBackend, ProcessSandboxError, ProcessSandboxErrorKind, ProcessSandboxPlanError, - SandboxCommandPlan, SandboxPhaseOutput, SandboxProcessOutput, SandboxProcessRequest, - SandboxProcessResult, ValidatedSandboxProcessPlan, - validation::{validate_env_has_no_raw_sensitive_values, validate_env_name}, -}; -use ironclaw_processes::ProcessCancellationToken; - -static CONTAINER_SEQUENCE: AtomicU64 = AtomicU64::new(1); - -const DOCKER_MEMORY_LIMIT: &str = "512m"; -const DOCKER_PIDS_LIMIT: &str = "256"; -const DOCKER_CPU_LIMIT: &str = "2"; - -/// Trusted Docker backend configuration for the process sandbox. -/// -/// Host paths and the image name come from host configuration, not from the -/// runtime-supplied process plan. The backend translates validated logical -/// plans into a restricted `docker run` invocation. -#[derive(Debug, Clone)] -pub struct DockerProcessSandboxConfig { - pub docker_bin: String, - pub image: String, - pub workspace_host_path: PathBuf, - pub tools_host_path: PathBuf, - pub cache_host_path: PathBuf, - pub broker: Option, -} - -impl DockerProcessSandboxConfig { - /// Builds a Docker config with the default binary and sandbox image. - pub fn new( - workspace_host_path: impl Into, - tools_host_path: impl Into, - cache_host_path: impl Into, - ) -> Self { - Self { - docker_bin: "docker".to_string(), - image: DEFAULT_PROCESS_SANDBOX_IMAGE.to_string(), - workspace_host_path: workspace_host_path.into(), - tools_host_path: tools_host_path.into(), - cache_host_path: cache_host_path.into(), - broker: None, - } - } -} - -/// Host-side broker configuration used by credentialed sandbox runs. -/// -/// The proxy URL and CA certificate mount are injected by trusted composition -/// so container traffic can be pinned to the broker and sanitized before it -/// leaves the sandbox. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct DockerBrokerConfig { - pub proxy_url: String, - pub ca_cert_host_path: PathBuf, - pub ca_cert_container_path: String, -} - -/// Sandbox execution phase represented in Docker invocations and output. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum SandboxProcessPhase { - Install, - Run, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct DockerInvocation { - pub docker_bin: String, - pub phase: SandboxProcessPhase, - pub container_name: String, - pub args: Vec, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct DockerRunOutput { - pub exit_code: i32, - pub stdout: Vec, - pub stderr: Vec, - pub wall_clock_ms: u64, - pub stdout_truncated: bool, - pub stderr_truncated: bool, -} - -#[derive(Debug, Clone, PartialEq, Eq, Error)] -pub(crate) enum DockerRunError { - #[error("Docker process failed to start")] - Spawn, - #[error("Docker process I/O failed")] - Io, - #[error("Docker process was cancelled")] - Cancelled, - #[error("Docker process timed out")] - Timeout, -} - -#[async_trait] -pub(crate) trait DockerRunner: Send + Sync { - async fn run( - &self, - invocation: DockerInvocation, - command: &SandboxCommandPlan, - cancellation: ProcessCancellationToken, - ) -> Result; -} - -#[derive(Debug, Clone, Copy, Default)] -pub(crate) struct SystemDockerRunner; - -#[async_trait] -impl DockerRunner for SystemDockerRunner { - async fn run( - &self, - invocation: DockerInvocation, - command_plan: &SandboxCommandPlan, - cancellation: ProcessCancellationToken, - ) -> Result { - let started = Instant::now(); - let mut command = Command::new(&invocation.docker_bin); - command - .args(&invocation.args) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()); - let mut child = command.spawn().map_err(|_| DockerRunError::Spawn)?; - let stdout = child.stdout.take().ok_or(DockerRunError::Io)?; - let stderr = child.stderr.take().ok_or(DockerRunError::Io)?; - let stdout_limit = command_plan - .max_stdout_bytes - .unwrap_or(DEFAULT_STDOUT_LIMIT); - let stderr_limit = command_plan - .max_stderr_bytes - .unwrap_or(DEFAULT_STDERR_LIMIT); - let stdout_reader = tokio::spawn(read_bounded_async(stdout, stdout_limit)); - let stderr_reader = tokio::spawn(read_bounded_async(stderr, stderr_limit)); - let timeout = Duration::from_millis(command_plan.timeout_ms.unwrap_or(DEFAULT_TIMEOUT_MS)); - - let status = tokio::select! { - status = child.wait() => status.map_err(|_| DockerRunError::Io)?, - _ = cancellation.cancelled() => { - abort_docker_child(&invocation.docker_bin, &invocation.container_name, &mut child).await; - return Err(DockerRunError::Cancelled); - } - _ = time::sleep(timeout) => { - abort_docker_child(&invocation.docker_bin, &invocation.container_name, &mut child).await; - return Err(DockerRunError::Timeout); - } - }; - - let (stdout, stdout_truncated) = stdout_reader - .await - .map_err(|_| DockerRunError::Io)? - .map_err(|_| DockerRunError::Io)?; - let (stderr, stderr_truncated) = stderr_reader - .await - .map_err(|_| DockerRunError::Io)? - .map_err(|_| DockerRunError::Io)?; - Ok(DockerRunOutput { - exit_code: status.code().unwrap_or(-1), - stdout, - stderr, - wall_clock_ms: started.elapsed().as_millis().min(u128::from(u64::MAX)) as u64, - stdout_truncated, - stderr_truncated, - }) - } -} - -async fn abort_docker_child(docker_bin: &str, container_name: &str, child: &mut Child) { - let _ = child.start_kill(); - let _ = child.wait().await; - cleanup_container(docker_bin, container_name).await; -} - -async fn cleanup_container(docker_bin: &str, container_name: &str) { - let _ = Command::new(docker_bin) - .args(["rm", "-f", container_name]) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .await; -} - -async fn read_bounded_async(mut reader: R, limit: u64) -> Result<(Vec, bool), std::io::Error> -where - R: AsyncRead + Unpin, -{ - let limit = usize::try_from(limit).unwrap_or(usize::MAX); - let mut output = Vec::new(); - let mut buffer = [0_u8; 8192]; - let mut truncated = false; - loop { - let read = reader.read(&mut buffer).await?; - if read == 0 { - return Ok((output, truncated)); - } - let remaining = limit.saturating_sub(output.len()); - if remaining == 0 { - truncated = true; - continue; - } - let take = read.min(remaining); - output.extend_from_slice(&buffer[..take]); - if take < read { - truncated = true; - } - } -} - -/// Docker-backed implementation of the process sandbox backend. -/// -/// The backend enforces the Docker-specific security contract: host-owned -/// mount roots, configured image only, no host environment inheritance, -/// resource limits, dropped capabilities, no-new-privileges, and broker-only -/// egress for credentialed runtime phases. -#[derive(Clone)] -pub struct DockerProcessSandboxBackend { - config: DockerProcessSandboxConfig, - runner: Arc, -} - -impl DockerProcessSandboxBackend { - /// Constructs a Docker sandbox backend using the system Docker runner. - pub fn new(config: DockerProcessSandboxConfig) -> Self { - Self { - config, - runner: Arc::new(SystemDockerRunner), - } - } - - #[cfg(test)] - pub(crate) fn with_runner( - config: DockerProcessSandboxConfig, - runner: Arc, - ) -> Self { - Self { config, runner } - } -} - -#[async_trait] -impl ProcessSandboxBackend for DockerProcessSandboxBackend { - async fn execute( - &self, - request: SandboxProcessRequest, - ) -> Result { - let mut phases = Vec::new(); - if let Some(install) = &request.plan.install { - let invocation = docker_invocation_for_phase( - &self.config, - &request.plan, - SandboxProcessPhase::Install, - &install.command, - )?; - let output = self - .runner - .run(invocation, &install.command, request.cancellation.clone()) - .await - .map_err(sandbox_process_error)?; - let exit_code = output.exit_code; - phases.push(phase_output(SandboxProcessPhase::Install, output)); - if exit_code != 0 { - return Ok(SandboxProcessResult { - output: SandboxProcessOutput { phases }, - }); - } - } - - let invocation = docker_invocation_for_phase( - &self.config, - &request.plan, - SandboxProcessPhase::Run, - &request.plan.run, - )?; - let output = self - .runner - .run(invocation, &request.plan.run, request.cancellation) - .await - .map_err(sandbox_process_error)?; - phases.push(phase_output(SandboxProcessPhase::Run, output)); - - Ok(SandboxProcessResult { - output: SandboxProcessOutput { phases }, - }) - } -} - -fn sandbox_process_error(error: DockerRunError) -> ProcessSandboxError { - ProcessSandboxError::new(match error { - DockerRunError::Spawn => ProcessSandboxErrorKind::DockerSpawnFailed, - DockerRunError::Io => ProcessSandboxErrorKind::DockerIoFailed, - DockerRunError::Cancelled => ProcessSandboxErrorKind::Cancelled, - DockerRunError::Timeout => ProcessSandboxErrorKind::Timeout, - }) -} - -fn phase_output(phase: SandboxProcessPhase, output: DockerRunOutput) -> SandboxPhaseOutput { - SandboxPhaseOutput { - phase, - exit_code: output.exit_code, - stdout: String::from_utf8_lossy(&output.stdout).into_owned(), - stderr: String::from_utf8_lossy(&output.stderr).into_owned(), - stdout_truncated: output.stdout_truncated, - stderr_truncated: output.stderr_truncated, - wall_clock_ms: output.wall_clock_ms, - } -} - -pub(crate) fn docker_invocation_for_phase( - config: &DockerProcessSandboxConfig, - plan: &ValidatedSandboxProcessPlan, - phase: SandboxProcessPhase, - command: &SandboxCommandPlan, -) -> Result { - let spec = DockerPhaseSpec::new(config, plan, phase, command)?; - let container_name = next_container_name(phase); - let mut args = Vec::with_capacity(48 + command.args.len() + command.env.len()); - args.extend([ - "run".to_string(), - "--name".to_string(), - container_name.clone(), - "--rm".to_string(), - "--init".to_string(), - "--memory".to_string(), - DOCKER_MEMORY_LIMIT.to_string(), - "--memory-swap".to_string(), - DOCKER_MEMORY_LIMIT.to_string(), - "--pids-limit".to_string(), - DOCKER_PIDS_LIMIT.to_string(), - "--cpus".to_string(), - DOCKER_CPU_LIMIT.to_string(), - "--security-opt".to_string(), - "no-new-privileges".to_string(), - "--cap-drop".to_string(), - "ALL".to_string(), - "--cap-add".to_string(), - "SETPCAP".to_string(), - "--cap-add".to_string(), - "SETUID".to_string(), - "--cap-add".to_string(), - "SETGID".to_string(), - ]); - if spec.needs_net_admin() { - args.push("--cap-add".to_string()); - args.push("NET_ADMIN".to_string()); - } - - args.extend(spec.network_args(config)); - args.extend(spec.mount_args(config, plan)?); - args.extend(spec.env_args(config, plan)?); - if let Some(working_dir) = &command.working_dir { - args.push("--workdir".to_string()); - args.push(working_dir.clone()); - } - args.push(config.image.clone()); - args.push(command.command.clone()); - args.extend(command.args.clone()); - Ok(DockerInvocation { - docker_bin: config.docker_bin.clone(), - phase, - container_name, - args, - }) -} - -fn next_container_name(phase: SandboxProcessPhase) -> String { - let phase = match phase { - SandboxProcessPhase::Install => "install", - SandboxProcessPhase::Run => "run", - }; - let sequence = CONTAINER_SEQUENCE.fetch_add(1, Ordering::Relaxed); - format!("ironclaw-sandbox-{phase}-{}-{sequence}", std::process::id()) -} - -struct DockerPhaseSpec<'a> { - command: &'a SandboxCommandPlan, - phase: SandboxProcessPhase, - has_credentials: bool, - network_mode: &'static str, -} - -impl<'a> DockerPhaseSpec<'a> { - fn new( - config: &DockerProcessSandboxConfig, - plan: &ValidatedSandboxProcessPlan, - phase: SandboxProcessPhase, - command: &'a SandboxCommandPlan, - ) -> Result { - let has_credentials = !plan.credentials.is_empty(); - if phase == SandboxProcessPhase::Run && has_credentials && config.broker.is_none() { - return Err(ProcessSandboxPlanError::CredentialedRunWithoutBroker); - } - if phase == SandboxProcessPhase::Install && install_needs_network(plan) { - return Err(ProcessSandboxPlanError::UnenforcedNetworkHosts { phase: "install" }); - } - if phase == SandboxProcessPhase::Run - && !has_credentials - && !plan.network.runtime_hosts.is_empty() - { - return Err(ProcessSandboxPlanError::UnenforcedNetworkHosts { phase: "run" }); - } - let network_mode = match phase { - SandboxProcessPhase::Install => "none", - SandboxProcessPhase::Run if has_credentials => "bridge", - SandboxProcessPhase::Run => "none", - }; - Ok(Self { - command, - phase, - has_credentials, - network_mode, - }) - } - - fn brokered_run(&self) -> bool { - self.phase == SandboxProcessPhase::Run && self.has_credentials - } - - fn needs_net_admin(&self) -> bool { - self.brokered_run() - } - - fn include_broker_ca(&self) -> bool { - self.brokered_run() - } - - fn tools_readonly(&self, plan: &ValidatedSandboxProcessPlan) -> bool { - self.phase == SandboxProcessPhase::Install || !plan.mounts.tools.writable - } - - fn cache_readonly(&self, plan: &ValidatedSandboxProcessPlan) -> bool { - self.phase == SandboxProcessPhase::Install || !plan.mounts.cache.writable - } - - fn network_args(&self, config: &DockerProcessSandboxConfig) -> Vec { - let mut args = vec!["--network".to_string(), self.network_mode.to_string()]; - if let Some(host) = self.broker_add_host(config) { - args.extend(["--add-host".to_string(), format!("{host}:host-gateway")]); - } - if self.brokered_run() { - args.extend([ - "--env".to_string(), - "IRONCLAW_EGRESS_LOCKDOWN=broker-only".to_string(), - ]); - } - args - } - - fn broker_add_host(&self, config: &DockerProcessSandboxConfig) -> Option { - self.brokered_run().then_some(())?; - config - .broker - .as_ref() - .and_then(|broker| broker_host_for_add_host(&broker.proxy_url)) - } - - fn mount_args( - &self, - config: &DockerProcessSandboxConfig, - plan: &ValidatedSandboxProcessPlan, - ) -> Result, ProcessSandboxPlanError> { - let mut args = Vec::new(); - args.extend(bind_mount_arg( - &config.workspace_host_path, - &plan.mounts.workspace.container_path, - !plan.mounts.workspace.writable, - )?); - args.extend(bind_mount_arg( - &config.tools_host_path, - &plan.mounts.tools.container_path, - self.tools_readonly(plan), - )?); - args.extend(bind_mount_arg( - &config.cache_host_path, - &plan.mounts.cache.container_path, - self.cache_readonly(plan), - )?); - if self.include_broker_ca() - && let Some(broker) = &config.broker - { - args.extend(bind_mount_arg( - &broker.ca_cert_host_path, - &broker.ca_cert_container_path, - true, - )?); - } - Ok(args) - } - - fn env_args( - &self, - config: &DockerProcessSandboxConfig, - plan: &ValidatedSandboxProcessPlan, - ) -> Result, ProcessSandboxPlanError> { - let mut env = self.command.env.clone(); - if self.include_broker_ca() - && let Some(broker) = &config.broker - { - for name in ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"] { - env.insert(name.to_string(), broker.proxy_url.clone()); - } - for name in [ - "SSL_CERT_FILE", - "REQUESTS_CA_BUNDLE", - "NODE_EXTRA_CA_CERTS", - "GIT_SSL_CAINFO", - "CURL_CA_BUNDLE", - ] { - env.insert(name.to_string(), broker.ca_cert_container_path.clone()); - } - env.insert( - "IRONCLAW_BROKER_PROXY".to_string(), - broker.proxy_url.clone(), - ); - } - let placeholders = plan - .credentials - .iter() - .map(|binding| binding.placeholder_value.as_str()) - .collect::>(); - validate_env_has_no_raw_sensitive_values(&env, &placeholders)?; - let mut args = Vec::new(); - for (name, value) in env { - if !is_broker_proxy_env_name(&name) { - validate_env_name(&name)?; - } - args.push("--env".to_string()); - args.push(format!("{name}={value}")); - } - Ok(args) - } -} - -fn install_needs_network(plan: &ValidatedSandboxProcessPlan) -> bool { - plan.install - .as_ref() - .is_some_and(|install| !install.allowed_hosts.is_empty()) -} - -fn is_broker_proxy_env_name(name: &str) -> bool { - matches!( - name, - "HTTP_PROXY" | "HTTPS_PROXY" | "http_proxy" | "https_proxy" - ) -} - -fn bind_mount_arg( - host_path: &Path, - container_path: &str, - readonly: bool, -) -> Result, ProcessSandboxPlanError> { - if container_path.contains(',') { - return Err(ProcessSandboxPlanError::InvalidContainerPath { - path: container_path.to_string(), - }); - } - let host_path = host_path.display().to_string(); - if host_path.contains(',') { - return Err(ProcessSandboxPlanError::InvalidHostPath { path: host_path }); - } - let mut spec = format!("type=bind,src={},dst={}", host_path, container_path); - if readonly { - spec.push_str(",readonly"); - } - Ok(vec!["--mount".to_string(), spec]) -} - -fn broker_host_for_add_host(proxy_url: &str) -> Option { - let host = broker_host(proxy_url)?; - if host.parse::().is_ok() { - None - } else { - Some(host.to_string()) - } -} - -pub(crate) fn broker_host(proxy_url: &str) -> Option<&str> { - let (_, rest) = proxy_url.split_once("://")?; - let host_port_path = rest.split('/').next().unwrap_or(rest); - let host = host_port_path.split(':').next().unwrap_or(host_port_path); - (!host.is_empty()).then_some(host) -} diff --git a/crates/ironclaw_process_sandbox/src/lib.rs b/crates/ironclaw_process_sandbox/src/lib.rs index 9fe4d71e139..5ab6157e131 100644 --- a/crates/ironclaw_process_sandbox/src/lib.rs +++ b/crates/ironclaw_process_sandbox/src/lib.rs @@ -1,31 +1,15 @@ -//! Docker process sandbox process executor for IronClaw Reborn. +//! Process sandbox plan types for IronClaw Reborn. //! -//! This crate owns the dynamic process compatibility lane: a trusted host can -//! execute a typed [`SandboxProcessPlan`] through [`ProcessExecutor`] while -//! keeping host paths in executor configuration and secret material behind -//! broker policy. +//! This crate owns the typed [`SandboxProcessPlan`] contract: the runtime +//! validates model-supplied plans through [`ValidatedSandboxProcessPlan`] +//! before the host dispatches them under +//! [`PROCESS_SANDBOX_CAPABILITY_ID`]. There is no production backend wired +//! for that capability today (see host_runtime's `process_executor`); this +//! crate only owns plan validation. -mod approval; -mod backend; -mod broker; -mod docker; mod plan; mod validation; -pub use approval::{ - SandboxApprovalCredential, SandboxApprovalMount, SandboxProcessApprovalSummary, -}; -pub use backend::{ - ProcessSandboxBackend, ProcessSandboxError, ProcessSandboxErrorKind, ProcessSandboxExecutor, - SandboxPhaseOutput, SandboxProcessOutput, SandboxProcessRequest, SandboxProcessResult, -}; -pub use broker::{ - BrokerHeaderRewrite, BrokerRewriteError, BrokerRewriteResult, SandboxBrokerPolicy, -}; -pub use docker::{ - DockerBrokerConfig, DockerProcessSandboxBackend, DockerProcessSandboxConfig, - SandboxProcessPhase, -}; pub use plan::{ ProcessSandboxPlanError, SandboxCommandPlan, SandboxCredentialBinding, SandboxInstallPlan, SandboxMount, SandboxMounts, SandboxNetworkPlan, SandboxProcessPlan, @@ -41,8 +25,5 @@ pub const DEFAULT_WORKSPACE_MOUNT: &str = "/workspace"; pub const DEFAULT_TOOLS_MOUNT: &str = "/ironclaw/state/tools"; pub const DEFAULT_CACHE_MOUNT: &str = "/ironclaw/state/cache"; -pub(crate) const DEFAULT_STDOUT_LIMIT: u64 = 1024 * 1024; -pub(crate) const DEFAULT_STDERR_LIMIT: u64 = 256 * 1024; -pub(crate) const DEFAULT_TIMEOUT_MS: u64 = 30_000; pub(crate) const MAX_OUTPUT_LIMIT: u64 = 10 * 1024 * 1024; pub(crate) const MAX_TIMEOUT_MS: u64 = 300_000; diff --git a/crates/ironclaw_process_sandbox/src/tests.rs b/crates/ironclaw_process_sandbox/src/tests.rs index d5db0d62ab3..159ee5925d9 100644 --- a/crates/ironclaw_process_sandbox/src/tests.rs +++ b/crates/ironclaw_process_sandbox/src/tests.rs @@ -1,34 +1,10 @@ -use std::{ - collections::HashMap, - path::Path, - sync::{ - Arc, Mutex, - atomic::{AtomicUsize, Ordering}, - }, -}; +use std::collections::HashMap; -use async_trait::async_trait; -use ironclaw_host_api::{ - AgentId, CapabilityId, ExtensionId, InvocationId, MountView, ProcessId, ProjectId, - ResourceEstimate, ResourceScope, RuntimeCredentialTarget, RuntimeKind, SecretHandle, TenantId, - ThreadId, UserId, -}; -use ironclaw_processes::{ProcessCancellationToken, ProcessExecutionRequest, ProcessExecutor}; -use secrecy::SecretString; -use serde_json::Value; +use ironclaw_host_api::{RuntimeCredentialTarget, SecretHandle}; use crate::{ - BrokerRewriteError, DEFAULT_PROCESS_SANDBOX_IMAGE, DockerBrokerConfig, - DockerProcessSandboxBackend, DockerProcessSandboxConfig, ProcessSandboxBackend, - ProcessSandboxError, ProcessSandboxExecutor, ProcessSandboxPlanError as SandboxPlanError, - SandboxBrokerPolicy, SandboxCommandPlan, SandboxCredentialBinding, SandboxInstallPlan, - SandboxMounts, SandboxNetworkPlan, SandboxProcessApprovalSummary, SandboxProcessOutput, - SandboxProcessPhase, SandboxProcessPlan, SandboxProcessRequest, SandboxProcessResult, - ValidatedSandboxProcessPlan, - docker::{ - DockerInvocation, DockerRunError, DockerRunOutput, DockerRunner, broker_host, - docker_invocation_for_phase, - }, + ProcessSandboxPlanError as SandboxPlanError, SandboxCommandPlan, SandboxCredentialBinding, + SandboxInstallPlan, SandboxMounts, SandboxNetworkPlan, SandboxProcessPlan, validation::{is_container_absolute_path, validate_header_name, validate_host}, }; @@ -80,25 +56,6 @@ fn sample_plan() -> SandboxProcessPlan { } } -fn sample_config(root: &Path) -> DockerProcessSandboxConfig { - DockerProcessSandboxConfig { - docker_bin: "docker".to_string(), - image: DEFAULT_PROCESS_SANDBOX_IMAGE.to_string(), - workspace_host_path: root.join("workspace"), - tools_host_path: root.join("tools"), - cache_host_path: root.join("cache"), - broker: Some(DockerBrokerConfig { - proxy_url: "http://host.docker.internal:4489".to_string(), - ca_cert_host_path: root.join("broker-ca.pem"), - ca_cert_container_path: "/ironclaw/broker/ca.pem".to_string(), - }), - } -} - -fn validated_sample_plan() -> ValidatedSandboxProcessPlan { - ValidatedSandboxProcessPlan::new(sample_plan()).unwrap() -} - #[test] fn plan_validation_rejects_raw_secret_env_values() { let mut plan = sample_plan(); @@ -433,529 +390,3 @@ fn plan_validation_rejects_missing_or_mismatched_placeholder_env() { } ); } - -#[test] -fn docker_args_never_include_secret_material() { - let temp = tempfile::tempdir().unwrap(); - let plan = validated_sample_plan(); - let config = sample_config(temp.path()); - - let invocation = - docker_invocation_for_phase(&config, &plan, SandboxProcessPhase::Run, &plan.run).unwrap(); - let joined = invocation.args.join("\n"); - - assert!(!joined.contains("real-notion-secret")); - assert!(joined.contains("NOTION_API_KEY=NOTION_API_KEY")); - assert!(joined.contains("IRONCLAW_EGRESS_LOCKDOWN=broker-only")); - assert!(joined.contains("HTTP_PROXY=http://host.docker.internal:4489")); - assert!(joined.contains("HTTPS_PROXY=http://host.docker.internal:4489")); - assert!(joined.contains("http_proxy=http://host.docker.internal:4489")); - assert!(joined.contains("https_proxy=http://host.docker.internal:4489")); - assert!(joined.contains("--add-host\nhost.docker.internal:host-gateway")); - assert!(joined.contains("--memory\n512m")); - assert!(joined.contains("--pids-limit\n256")); - assert!(joined.contains("--cap-add\nSETUID")); - assert!(joined.contains(DEFAULT_PROCESS_SANDBOX_IMAGE)); - assert!( - invocation - .container_name - .starts_with("ironclaw-sandbox-run-") - ); -} - -#[test] -fn docker_broker_host_parses_proxy_url_hosts() { - let host_gateway = broker_host("http://host.docker.internal:4489"); - let path_host = broker_host("https://broker.local/path"); - - assert_eq!(host_gateway, Some("host.docker.internal")); - assert_eq!(path_host, Some("broker.local")); - assert_eq!(broker_host("broker.local:4489"), None); -} - -#[test] -fn docker_builder_rejects_credentialed_run_without_broker() { - let temp = tempfile::tempdir().unwrap(); - let plan = validated_sample_plan(); - let mut config = sample_config(temp.path()); - config.broker = None; - - let error = docker_invocation_for_phase(&config, &plan, SandboxProcessPhase::Run, &plan.run) - .unwrap_err(); - - assert_eq!(error, SandboxPlanError::CredentialedRunWithoutBroker); -} - -#[test] -fn install_and_run_phases_have_different_mount_and_network_policies() { - let temp = tempfile::tempdir().unwrap(); - let plan = validated_sample_plan(); - let config = sample_config(temp.path()); - let install = docker_invocation_for_phase( - &config, - &plan, - SandboxProcessPhase::Install, - &plan.install.as_ref().unwrap().command, - ) - .unwrap(); - let run = - docker_invocation_for_phase(&config, &plan, SandboxProcessPhase::Run, &plan.run).unwrap(); - let install_args = install.args.join("\n"); - let run_args = run.args.join("\n"); - - assert!(install_args.contains("--network\nnone")); - assert!(!install_args.contains("IRONCLAW_EGRESS_LOCKDOWN=broker-only")); - assert!(install_args.contains("dst=/ironclaw/state/tools,readonly")); - assert!(install_args.contains("dst=/ironclaw/state/cache,readonly")); - assert!(run_args.contains("IRONCLAW_EGRESS_LOCKDOWN=broker-only")); - assert!(run_args.contains("dst=/ironclaw/state/tools,readonly")); - assert!(run_args.contains("dst=/ironclaw/state/cache,readonly")); -} - -#[test] -fn docker_invocation_rejects_unenforced_network_hosts() { - let temp = tempfile::tempdir().unwrap(); - let mut plan = sample_plan(); - plan.run.env.clear(); - plan.network.direct_egress_lockdown = false; - plan.credentials.clear(); - let plan = ValidatedSandboxProcessPlan::new(plan).unwrap(); - let config = sample_config(temp.path()); - - let error = docker_invocation_for_phase(&config, &plan, SandboxProcessPhase::Run, &plan.run) - .unwrap_err(); - - assert_eq!( - error, - SandboxPlanError::UnenforcedNetworkHosts { phase: "run" } - ); -} - -#[test] -fn docker_invocation_rejects_unenforced_install_allowed_hosts() { - let temp = tempfile::tempdir().unwrap(); - let mut plan = sample_plan(); - plan.install.as_mut().unwrap().allowed_hosts = vec!["registry.npmjs.org".to_string()]; - let plan = ValidatedSandboxProcessPlan::new(plan).unwrap(); - let config = sample_config(temp.path()); - - let error = docker_invocation_for_phase( - &config, - &plan, - SandboxProcessPhase::Install, - &plan.install.as_ref().unwrap().command, - ) - .unwrap_err(); - - assert_eq!( - error, - SandboxPlanError::UnenforcedNetworkHosts { phase: "install" } - ); -} - -#[test] -fn docker_invocation_rejects_host_paths_that_break_mount_specs() { - let temp = tempfile::tempdir().unwrap(); - let plan = validated_sample_plan(); - let mut config = sample_config(temp.path()); - config.workspace_host_path = temp.path().join("workspace,with-comma"); - - let error = docker_invocation_for_phase(&config, &plan, SandboxProcessPhase::Run, &plan.run) - .unwrap_err(); - - assert!(matches!(error, SandboxPlanError::InvalidHostPath { .. })); -} - -#[test] -fn broker_policy_rewrites_only_approved_host_and_header() { - let plan = sample_plan(); - let policy = SandboxBrokerPolicy::new(plan.credentials).unwrap(); - let mut secrets = HashMap::new(); - secrets.insert( - SecretHandle::new("notion_token").unwrap(), - SecretString::from("real-notion-secret"), - ); - - let approved = policy - .rewrite_headers( - "api.notion.com:443", - vec![( - "Authorization".to_string(), - "Bearer NOTION_API_KEY".to_string(), - )], - &secrets, - ) - .unwrap(); - let denied = policy - .rewrite_headers( - "example.com", - vec![( - "Authorization".to_string(), - "Bearer NOTION_API_KEY".to_string(), - )], - &secrets, - ) - .unwrap(); - - assert_eq!( - approved.headers, - vec![( - "Authorization".to_string(), - "Bearer real-notion-secret".to_string() - )] - ); - assert_eq!(approved.rewrites.len(), 1); - assert_eq!( - denied.headers, - vec![( - "Authorization".to_string(), - "Bearer NOTION_API_KEY".to_string() - )] - ); - assert!(denied.rewrites.is_empty()); -} - -#[test] -fn broker_policy_rejects_missing_required_secret() { - let plan = sample_plan(); - let policy = SandboxBrokerPolicy::new(plan.credentials).unwrap(); - let error = policy - .rewrite_headers( - "api.notion.com", - vec![( - "Authorization".to_string(), - "Bearer NOTION_API_KEY".to_string(), - )], - &HashMap::new(), - ) - .unwrap_err(); - - assert_eq!( - error, - BrokerRewriteError::MissingRequiredSecret { - secret_alias: SecretHandle::new("notion_token").unwrap() - } - ); -} - -#[test] -fn broker_policy_rejects_duplicate_credential_targets() { - let mut plan = sample_plan(); - let mut duplicate = plan.credentials[0].clone(); - duplicate.approved_host = "API.NOTION.COM".to_string(); - duplicate.target = RuntimeCredentialTarget::Header { - name: "authorization".to_string(), - prefix: Some("Bearer ".to_string()), - }; - plan.credentials.push(duplicate); - - let error = SandboxBrokerPolicy::new(plan.credentials).unwrap_err(); - - assert!(matches!( - error, - SandboxPlanError::DuplicateCredentialTarget { .. } - )); -} - -#[test] -fn broker_redacts_secret_values_from_error_paths() { - let plan = sample_plan(); - let policy = SandboxBrokerPolicy::new(plan.credentials).unwrap(); - let mut secrets = HashMap::new(); - secrets.insert( - SecretHandle::new("notion_token").unwrap(), - SecretString::from("real-notion-secret"), - ); - - let sanitized = policy.sanitize_text("upstream echoed real-notion-secret", &secrets); - - assert_eq!(sanitized, "upstream echoed [REDACTED]"); -} - -#[test] -fn broker_redacts_longer_secret_before_embedded_substring() { - let policy = SandboxBrokerPolicy::new(Vec::new()).unwrap(); - let mut secrets = HashMap::new(); - secrets.insert( - SecretHandle::new("short").unwrap(), - SecretString::from("token"), - ); - secrets.insert( - SecretHandle::new("long").unwrap(), - SecretString::from("token-extended"), - ); - - let sanitized = policy.sanitize_text("upstream echoed token-extended", &secrets); - - assert_eq!(sanitized, "upstream echoed [REDACTED]"); -} - -#[test] -fn broker_policy_allows_empty_bindings() { - let policy = SandboxBrokerPolicy::new(Vec::new()).unwrap(); - let result = policy - .rewrite_headers( - "api.notion.com", - vec![( - "Authorization".to_string(), - "Bearer placeholder".to_string(), - )], - &HashMap::new(), - ) - .unwrap(); - - let expected_headers = vec![( - "Authorization".to_string(), - "Bearer placeholder".to_string(), - )]; - assert_eq!(result.headers, expected_headers); - assert!(result.rewrites.is_empty()); -} - -#[test] -fn approval_summary_contains_only_sanitized_authority_details() { - let mut plan = sample_plan(); - plan.install.as_mut().unwrap().allowed_hosts = vec!["registry.npmjs.org".to_string()]; - - let summary = SandboxProcessApprovalSummary::from_plan(&plan).unwrap(); - let serialized = serde_json::to_string(&summary).unwrap(); - - assert_eq!( - summary.install_command, - Some(vec![ - "npm".to_string(), - "install".to_string(), - "-g".to_string(), - "notion-cli".to_string() - ]) - ); - assert_eq!( - summary.run_command, - vec!["notion".to_string(), "list".to_string()] - ); - assert_eq!(summary.install_allowed_hosts, vec!["registry.npmjs.org"]); - assert_eq!(summary.allowed_network_hosts, vec!["api.notion.com"]); - assert!(summary.direct_egress_lockdown); - assert_eq!(summary.credentials[0].secret_alias.as_str(), "notion_token"); - assert_eq!( - summary.credentials[0].target, - "header:Authorization=Bearer " - ); - assert!(serialized.contains("NOTION_API_KEY")); - assert!(!serialized.contains("real-notion-secret")); -} - -#[derive(Default)] -struct RecordingRunner { - invocations: Mutex>, -} - -#[async_trait] -impl DockerRunner for RecordingRunner { - async fn run( - &self, - invocation: DockerInvocation, - _command: &SandboxCommandPlan, - _cancellation: ProcessCancellationToken, - ) -> Result { - self.invocations.lock().unwrap().push(invocation); - Ok(DockerRunOutput { - exit_code: 0, - stdout: b"{\"ok\":true}".to_vec(), - stderr: Vec::new(), - wall_clock_ms: 12, - stdout_truncated: false, - stderr_truncated: false, - }) - } -} - -#[derive(Default)] -struct InstallFailsRunner { - calls: AtomicUsize, -} - -#[async_trait] -impl DockerRunner for InstallFailsRunner { - async fn run( - &self, - invocation: DockerInvocation, - _command: &SandboxCommandPlan, - _cancellation: ProcessCancellationToken, - ) -> Result { - let call = self.calls.fetch_add(1, Ordering::SeqCst); - Ok(DockerRunOutput { - exit_code: if invocation.phase == SandboxProcessPhase::Install && call == 0 { - 42 - } else { - 0 - }, - stdout: b"install failed".to_vec(), - stderr: Vec::new(), - wall_clock_ms: 12, - stdout_truncated: false, - stderr_truncated: false, - }) - } -} - -#[tokio::test] -async fn executor_returns_sanitized_phase_output_json() { - let temp = tempfile::tempdir().unwrap(); - let runner = Arc::new(RecordingRunner::default()); - let backend = DockerProcessSandboxBackend::with_runner( - sample_config(temp.path()), - runner.clone() as Arc, - ); - let executor = ProcessSandboxExecutor::new(Arc::new(backend)); - - let result = executor - .execute(sample_request(serde_json::to_value(sample_plan()).unwrap())) - .await - .unwrap(); - - assert_eq!(result.output["kind"], "process_sandbox_result"); - assert_eq!(result.output["phases"].as_array().unwrap().len(), 2); - assert_eq!(runner.invocations.lock().unwrap().len(), 2); -} - -#[tokio::test] -async fn docker_backend_stops_after_failed_install_phase() { - let temp = tempfile::tempdir().unwrap(); - let runner = Arc::new(InstallFailsRunner::default()); - let backend = DockerProcessSandboxBackend::with_runner( - sample_config(temp.path()), - runner.clone() as Arc, - ); - - let result = backend - .execute(SandboxProcessRequest { - process_id: ProcessId::new(), - scope: sample_request(serde_json::json!({})).scope, - plan: validated_sample_plan(), - cancellation: ProcessCancellationToken::new(), - }) - .await - .unwrap(); - - assert_eq!(runner.calls.load(Ordering::SeqCst), 1); - assert_eq!(result.output.phases.len(), 1); - assert_eq!(result.output.phases[0].phase, SandboxProcessPhase::Install); - assert_eq!(result.output.phases[0].exit_code, 42); -} - -#[derive(Clone)] -struct FailingRunner { - error: DockerRunError, -} - -#[async_trait] -impl DockerRunner for FailingRunner { - async fn run( - &self, - _invocation: DockerInvocation, - _command: &SandboxCommandPlan, - cancellation: ProcessCancellationToken, - ) -> Result { - if self.error == DockerRunError::Cancelled { - cancellation.cancel(); - } - Err(self.error.clone()) - } -} - -#[tokio::test] -async fn executor_maps_docker_runner_errors_to_stable_kinds() { - for (runner_error, expected_kind) in [ - (DockerRunError::Spawn, "docker_spawn_failed"), - (DockerRunError::Io, "docker_io_failed"), - (DockerRunError::Cancelled, "cancelled"), - (DockerRunError::Timeout, "timeout"), - ] { - let temp = tempfile::tempdir().unwrap(); - let backend = DockerProcessSandboxBackend::with_runner( - sample_config(temp.path()), - Arc::new(FailingRunner { - error: runner_error, - }), - ); - let executor = ProcessSandboxExecutor::new(Arc::new(backend)); - - let error = executor - .execute(sample_request(serde_json::to_value(sample_plan()).unwrap())) - .await - .unwrap_err(); - - assert_eq!(error.kind, expected_kind); - } -} - -#[derive(Default)] -struct CountingBackend { - calls: AtomicUsize, -} - -#[async_trait] -impl ProcessSandboxBackend for CountingBackend { - async fn execute( - &self, - _request: SandboxProcessRequest, - ) -> Result { - self.calls.fetch_add(1, Ordering::SeqCst); - Ok(SandboxProcessResult { - output: SandboxProcessOutput::default(), - }) - } -} - -#[tokio::test] -async fn executor_rejects_invalid_process_sandbox_plan_without_backend_execution() { - let backend = Arc::new(CountingBackend::default()); - let executor = ProcessSandboxExecutor::new(backend.clone()); - - let malformed = executor - .execute(sample_request(serde_json::json!({ "run": 1 }))) - .await - .unwrap_err(); - let invalid = executor - .execute(sample_request( - serde_json::to_value({ - let mut plan = sample_plan(); - plan.run.command.clear(); - plan - }) - .unwrap(), - )) - .await - .unwrap_err(); - - assert_eq!(malformed.kind, "invalid_process_sandbox_plan"); - assert_eq!(invalid.kind, "invalid_process_sandbox_plan"); - assert_eq!(backend.calls.load(Ordering::SeqCst), 0); -} - -fn sample_request(input: Value) -> ProcessExecutionRequest { - ProcessExecutionRequest { - process_id: ProcessId::new(), - invocation_id: InvocationId::new(), - scope: ResourceScope { - tenant_id: TenantId::new("tenant").unwrap(), - user_id: UserId::new("user").unwrap(), - agent_id: Some(AgentId::new("agent").unwrap()), - project_id: Some(ProjectId::new("project").unwrap()), - mission_id: None, - thread_id: Some(ThreadId::new("thread").unwrap()), - invocation_id: InvocationId::new(), - }, - authenticated_actor_user_id: None, - extension_id: ExtensionId::new("system.process_sandbox").unwrap(), - capability_id: CapabilityId::new("system.process_sandbox.run").unwrap(), - runtime: RuntimeKind::System, - estimate: ResourceEstimate::default(), - mounts: MountView::default(), - resource_reservation: None, - authorized_continuation: None, - input, - cancellation: ProcessCancellationToken::new(), - } -} From 0e9a00781152e080451eb7486a3d116c8e0a2e3f Mon Sep 17 00:00:00 2001 From: Henry Park Date: Sun, 26 Jul 2026 21:53:08 -0700 Subject: [PATCH 2/5] fix(architecture): drop stale docker.rs ratchet entry The frozen-path ratchet is shrink-only by design: it pins per-path test-support counts so they can only go down, never back up. The `docker.rs` entry tracked a method in the process-sandbox Docker backend that PR #6693 deletes outright, so the file no longer exists and the baseline entry is dead weight, not a real regression risk. Removing the entry is the fix, and the ratchet test itself is the regression test: `cargo test -p ironclaw_architecture` now passes because there is no stale path left to check for. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/reborn_struct_test_support_ratchet.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rs b/crates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rs index d2d885fa2bc..fcfda358b86 100644 --- a/crates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rs +++ b/crates/ironclaw_architecture/tests/reborn_struct_test_support_ratchet.rs @@ -304,12 +304,6 @@ const FROZEN_PATH_COUNTS: &[FrozenPathCount] = &[ path: "crates/ironclaw_operator/src/operator_service_lifecycle.rs", count: 5, }, - FrozenPathCount { - category: "test-support", - item_kind: "method", - path: "crates/ironclaw_process_sandbox/src/docker.rs", - count: 1, - }, FrozenPathCount { category: "test-support", item_kind: "method", From f3fbc8e2a87d723ad4a39e6fc5ad8f11118e6543 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Sun, 26 Jul 2026 21:54:18 -0700 Subject: [PATCH 3/5] docs(process-sandbox): describe crate as plan-validation-only PR #6693 deleted the Docker process-sandbox backend, but four docs still described it as present: the crate's own CLAUDE.md guardrails, the per-crate row in crates/AGENTS.md, FEATURE_PARITY.md's Docker sandbox row, and the Reborn parity doc's per-project-sandbox row (plus its accompanying note). Rewrite all four to match the crate's own lib.rs doc comment: ironclaw_process_sandbox owns only the typed SandboxProcessPlan contract and validation, no production backend is wired for the capability today, and the parity docs now honestly mark the capability as not covered rather than silently dropping the row. Co-Authored-By: Claude Opus 5 (1M context) --- FEATURE_PARITY.md | 2 +- crates/AGENTS.md | 2 +- crates/ironclaw_process_sandbox/CLAUDE.md | 10 +++++----- docs/reborn/engine-v2-to-reborn-parity.md | 13 ++++++------- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/FEATURE_PARITY.md b/FEATURE_PARITY.md index 0dcb188c0d7..04c3eacefcc 100644 --- a/FEATURE_PARITY.md +++ b/FEATURE_PARITY.md @@ -747,7 +747,7 @@ CLAUDE.md for the full mapping + gap catalog. | SSRF IPv6 transition bypass block | ✅ | ❌ | Block IPv4-mapped IPv6 bypasses | | Cron webhook SSRF guard | ✅ | ❌ | SSRF checks on webhook delivery | | Loopback-first | ✅ | 🚧 | HTTP binds 0.0.0.0 | -| Docker sandbox | ✅ | ✅ | Orchestrator/worker containers; opt-in `sandbox.docker.gpus` passthrough; Reborn process sandbox MVP adds typed `SandboxProcessPlan`, backend-neutral `ProcessSandboxBackend`, hardened Docker command construction, fail-closed unenforced network hosts, explicit timeout/cancel cleanup, loop-to-host `SandboxProcessPlan` validation/spawn dispatch, and a host-runtime approval/lease spawn path for `system.process_sandbox.run`; production MITM broker/product wiring still partial | +| Docker sandbox | ✅ | ❌ | Orchestrator/worker containers; opt-in `sandbox.docker.gpus` passthrough; Reborn defines a typed `SandboxProcessPlan` contract (`ironclaw_process_sandbox`) with plan validation only — no production execution backend is wired for it yet | | Podman support | ✅ | ❌ | `--container` accepts both Docker + Podman | | WASM sandbox | ❌ | ✅ | IronClaw innovation | | Sandbox env sanitization | ✅ | 🚧 | Shell tool scrubs env vars (secret detection); Reborn process sandbox rejects sensitive raw env values in plans and uses placeholders for brokered credentials, but production secure-capture and MITM transport wiring remain partial | diff --git a/crates/AGENTS.md b/crates/AGENTS.md index 5686b133075..da6aca0f6d7 100644 --- a/crates/AGENTS.md +++ b/crates/AGENTS.md @@ -114,7 +114,7 @@ Boundary rule: if you need an upstream crate in a low-level crate, stop and chec | `ironclaw_wasm` | `ironclaw_wasm/AGENTS.md`, `ironclaw_wasm/CLAUDE.md`, `docs/reborn/contracts/wasm.md`, `wit/tool.wit` | WASM runtime lane, component/WIT bindings, folded `wasm_sandbox_core` primitives, store, host adapters, runtime config. | Privileged host effects outside mediated APIs; copied secrets/network/resource logic; product/runtime-specific dependencies inside `wasm_sandbox_core`. | | `ironclaw_wasm_limiter` | `Cargo.toml`, `src/lib.rs` | Shared `wasmtime::ResourceLimiter` for WASM tool and hook runtimes. | Product adapter workflow, policy decisions, or runtime-specific side effects beyond limiter accounting. | | `ironclaw_extensions` | `ironclaw_extensions/AGENTS.md`, `ironclaw_extensions/CLAUDE.md` | Declarative extension manifests (v2), capability descriptors, side-effect-free in-memory registry, installation records. | Execution of any kind (WASM/MCP/process), secrets, trust decisions. | -| `ironclaw_process_sandbox` | `ironclaw_process_sandbox/CLAUDE.md` | Docker process-sandbox backend behind `ironclaw_processes::ProcessExecutor`: typed sandbox plans, install/credentialed-run phase separation, mount roots. | Process lifecycle/stores (`ironclaw_processes`); raw Docker flags for extensions. | +| `ironclaw_process_sandbox` | `ironclaw_process_sandbox/CLAUDE.md` | Typed `SandboxProcessPlan` contract and validation only: install/credentialed-run phase separation in plan types. No production execution backend is wired for this capability today. | Process lifecycle/stores (`ironclaw_processes`); raw Docker flags for extensions; adding an execution backend here. | ### Turns, threads, loops, engine diff --git a/crates/ironclaw_process_sandbox/CLAUDE.md b/crates/ironclaw_process_sandbox/CLAUDE.md index 08e0a9990e7..a12995b7201 100644 --- a/crates/ironclaw_process_sandbox/CLAUDE.md +++ b/crates/ironclaw_process_sandbox/CLAUDE.md @@ -1,8 +1,8 @@ # ironclaw_process_sandbox guardrails -- Own the Reborn process sandbox compatibility lane for arbitrary commands, generated code, repo-local code, and user-installed CLIs. +- Own the typed `SandboxProcessPlan` contract only: plan types and validation (`ValidatedSandboxProcessPlan`) for arbitrary commands, generated code, repo-local code, and user-installed CLIs. - Accept only typed `SandboxProcessPlan` input. Do not accept raw Docker flags, raw host paths, host environment inheritance, or raw secret material from plan JSON. -- Keep physical Docker mount roots in trusted executor configuration, never in `ProcessExecutionRequest.input`. -- Treat install and credentialed run phases separately: install may write scoped tool/cache state with no secrets; credentialed run uses brokered secrets and read-only tool/cache state. -- Secret values must stay inside broker/lease seams and redaction helpers. Docker args, process output, errors, and debug data must not contain secret material. -- Do not stretch `ironclaw_scripts`; this crate is for dynamic sandbox process execution behind `ProcessExecutor`. +- There is no production backend wired for this capability today — see `ironclaw_host_runtime`'s `process_executor` for the dispatch seam this crate's plans feed into. Do not add Docker mount-root or executor configuration here; that lives with whatever crate eventually wires a real backend. +- Treat install and credentialed run phases separately in the plan types: install may declare scoped tool/cache state with no secrets; credentialed run declares brokered secrets and read-only tool/cache state. +- Secret values must stay inside broker/lease seams and redaction helpers. Plan JSON, validation errors, and debug data must not contain secret material. +- Do not stretch `ironclaw_scripts`; this crate is plan-validation-only and does not itself execute anything. diff --git a/docs/reborn/engine-v2-to-reborn-parity.md b/docs/reborn/engine-v2-to-reborn-parity.md index c94b2fd2ffa..89a898d0080 100644 --- a/docs/reborn/engine-v2-to-reborn-parity.md +++ b/docs/reborn/engine-v2-to-reborn-parity.md @@ -60,7 +60,7 @@ evidence; **Partial** = an equivalent exists but with a scoped delta noted; | **Gates / Approvals** — `gate/` (`ExecutionGate`, `GatePipeline`, `LeaseGate`, `GateResolution`, `ResumeKind`), auth/approval resume | Durable approval requests resolved into bounded scoped leases; typed gate/resume with exact invocation identity; deny-continue flow | `contracts/approvals.md`, `contracts/capability-access.md`, `contracts/run-state.md`; plan `docs/plans/2026-06-15-reborn-approval-deny-continue.md` | `ironclaw_approvals`, `ironclaw_run_state` | `tests/reborn_approval_traces_parity.rs`, `tests/integration/auth/auth_failure.rs`, `crates/ironclaw_reborn_composition/tests/budget_approval_e2e.rs`, `crates/ironclaw_reborn_composition/src/factory/local_dev_host_tests/approval_gates.rs` | Covered (note 3) | | **CodeAct / Tier 1** — embedded Python via Monty (RLM): context-as-variables, `llm_query()` recursive subagent, compact output metadata; `executor/scripting.rs` | Two parts: (a) native script/software execution lane (`RuntimeKind::Script`); (b) CodeAct is an *allowed* pluggable parent loop family, and recursive subagents exist as `spawn_subagent` | `contracts/scripts.md`, `contracts/agent-loop-protocol.md` (CodeAct as parent protocol) | `ironclaw_scripts`, `ironclaw_agent_loop`, `ironclaw_process_sandbox` | `tests/integration/process_port.rs`, `tests/reborn_subagent_spawn_e2e.rs`, `crates/ironclaw_reborn_composition/tests/subagent_runtime_wiring.rs` | Partial (note 4) | | **Self-modify** — prompt overlays / orchestrator patches applied by the self-improvement mission; skill versioning/rollback (`memory/skill_tracker.rs::SkillTracker`) | Versioned skill evolution (distill/refine with validation); overlay/orchestrator self-patching intentionally not carried over (Reborn has no Monty orchestrator to patch) | plan `docs/plans/2026-06-16-reborn-skill-evolution.md`; `contracts/skills-extension.md` | `ironclaw_skills::learning`, `ironclaw_skills` | `crates/ironclaw_skills/src/learning.rs` (refine/distill tests) | Partial (note 4) | -| **Per-project Docker sandbox** — filesystem/shell tools routed through a per-project container (`SANDBOX_ENABLED`; `crates/Dockerfile.sandbox`) | Typed process plans, backend-neutral sandbox backends, hardened Docker command construction, fail-closed network-host validation, timeout/cancel cleanup | `contracts/processes.md`, `contracts/scripts.md` | `ironclaw_process_sandbox`, `ironclaw_processes`, `ironclaw_wasm` | `tests/integration/process_port.rs`, `crates/ironclaw_reborn_composition/tests/production_runtime_automations.rs` | Covered (note 5) | +| **Per-project Docker sandbox** — filesystem/shell tools routed through a per-project container (`SANDBOX_ENABLED`; `crates/Dockerfile.sandbox`) | Typed `SandboxProcessPlan` contract and validation only (`ironclaw_process_sandbox`); no production execution backend is wired for this capability today | `contracts/processes.md`, `contracts/scripts.md` | `ironclaw_process_sandbox`, `ironclaw_processes`, `ironclaw_wasm` | `tests/integration/process_port.rs` | Not covered (note 5) | | **OpenAI-compatible Responses API** — engine v2's OpenAI-compatible ingress | Contract-first, ProductSurface-backed Chat Completions + Responses (create/retrieve/cancel), idempotency/opaque-ref, projection-backed SSE streaming | `contracts/openai-compatible-api.md` | `ironclaw_reborn_openai_compat` | `crates/ironclaw_reborn_openai_compat/tests/responses_workflow_handlers_contract.rs`, `.../chat_workflow_handlers_contract.rs`, `.../streaming_handlers_contract.rs`, `.../error_contract.rs`, `.../ref_store_contract.rs` | Covered (note 6) | | **Skills** — trusted/installed skills, activation criteria, `skill_*` tools | First-party in-process skills extension: portable `SKILL.md` bundles; kernel owns trust/visibility/leases/context injection; catalog-first model-selected activation | `contracts/skills-extension.md` | `ironclaw_skills` | `tests/integration/group_extensions/` (suite), `crates/ironclaw_reborn_cli/tests/smoke.rs` | Covered | | **Hooks** — lifecycle hooks (6 points); `Hook` trait | Host-mediated hook execution with multi-backend persistence and an adversarial parity oracle | (hooks are exercised through `contracts/extensions.md` + capability dispatch) | `ironclaw_hooks`, `ironclaw_hooks_libsql`, `ironclaw_hooks_postgres`, `ironclaw_hooks_parity` | `crates/ironclaw_runner/tests/hooks_integration.rs`, `crates/ironclaw_hooks_parity/tests/parity_matrix.rs`, `crates/ironclaw_hooks_parity/tests/multi_host_adversarial.rs`, `crates/ironclaw_reborn_composition/tests/third_party_hook_projection.rs` | Covered | @@ -128,12 +128,11 @@ evidence; **Partial** = an equivalent exists but with a scoped delta noted; record that even its side-effect gating was only a soft nudge, never a hard gate. -5. **Sandbox (Covered).** The per-project Docker sandbox is reproduced as typed - process plans over backend-neutral sandbox backends with hardened command - construction and fail-closed network-host validation. The comparison doc - marks production MITM/product wiring as still partial, but the engine-v2 - capability (route filesystem/shell effects through a per-project container) - is present and tested. +5. **Sandbox (Not covered).** Reborn defines a typed `SandboxProcessPlan` + contract with plan validation only (`ironclaw_process_sandbox`); no + production execution backend is wired for it. The engine-v2 capability + (route filesystem/shell effects through a per-project Docker container) has + no Reborn equivalent running today. 6. **OpenAI-compatible API (Covered for the engine-v2 surface).** Reborn ships contract-first, ProductSurface-backed Chat Completions and the Responses From 672eff0c5469bc5f47d00cf7130d58b9ee34ce42 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Sun, 26 Jul 2026 22:03:23 -0700 Subject: [PATCH 4/5] docs(reborn): reconcile sandbox parity row with Known Gaps/conclusion CodeRabbit flagged that the sandbox row said Not covered but the Known Gaps list and Conclusion still claimed every runtime concern (incl. sandbox) maps cleanly to Reborn, and cited ironclaw_processes/ ironclaw_wasm/process_port.rs as parity evidence for a backend that doesn't exist. Label those as non-backend/adjacent, point evidence at the crate's own plan-validation tests, and add sandbox to Known Gaps and the Conclusion. Co-Authored-By: Claude Opus 5 (1M context) --- docs/reborn/engine-v2-to-reborn-parity.md | 34 ++++++++++++++--------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/docs/reborn/engine-v2-to-reborn-parity.md b/docs/reborn/engine-v2-to-reborn-parity.md index 89a898d0080..66cdec5e60c 100644 --- a/docs/reborn/engine-v2-to-reborn-parity.md +++ b/docs/reborn/engine-v2-to-reborn-parity.md @@ -60,7 +60,7 @@ evidence; **Partial** = an equivalent exists but with a scoped delta noted; | **Gates / Approvals** — `gate/` (`ExecutionGate`, `GatePipeline`, `LeaseGate`, `GateResolution`, `ResumeKind`), auth/approval resume | Durable approval requests resolved into bounded scoped leases; typed gate/resume with exact invocation identity; deny-continue flow | `contracts/approvals.md`, `contracts/capability-access.md`, `contracts/run-state.md`; plan `docs/plans/2026-06-15-reborn-approval-deny-continue.md` | `ironclaw_approvals`, `ironclaw_run_state` | `tests/reborn_approval_traces_parity.rs`, `tests/integration/auth/auth_failure.rs`, `crates/ironclaw_reborn_composition/tests/budget_approval_e2e.rs`, `crates/ironclaw_reborn_composition/src/factory/local_dev_host_tests/approval_gates.rs` | Covered (note 3) | | **CodeAct / Tier 1** — embedded Python via Monty (RLM): context-as-variables, `llm_query()` recursive subagent, compact output metadata; `executor/scripting.rs` | Two parts: (a) native script/software execution lane (`RuntimeKind::Script`); (b) CodeAct is an *allowed* pluggable parent loop family, and recursive subagents exist as `spawn_subagent` | `contracts/scripts.md`, `contracts/agent-loop-protocol.md` (CodeAct as parent protocol) | `ironclaw_scripts`, `ironclaw_agent_loop`, `ironclaw_process_sandbox` | `tests/integration/process_port.rs`, `tests/reborn_subagent_spawn_e2e.rs`, `crates/ironclaw_reborn_composition/tests/subagent_runtime_wiring.rs` | Partial (note 4) | | **Self-modify** — prompt overlays / orchestrator patches applied by the self-improvement mission; skill versioning/rollback (`memory/skill_tracker.rs::SkillTracker`) | Versioned skill evolution (distill/refine with validation); overlay/orchestrator self-patching intentionally not carried over (Reborn has no Monty orchestrator to patch) | plan `docs/plans/2026-06-16-reborn-skill-evolution.md`; `contracts/skills-extension.md` | `ironclaw_skills::learning`, `ironclaw_skills` | `crates/ironclaw_skills/src/learning.rs` (refine/distill tests) | Partial (note 4) | -| **Per-project Docker sandbox** — filesystem/shell tools routed through a per-project container (`SANDBOX_ENABLED`; `crates/Dockerfile.sandbox`) | Typed `SandboxProcessPlan` contract and validation only (`ironclaw_process_sandbox`); no production execution backend is wired for this capability today | `contracts/processes.md`, `contracts/scripts.md` | `ironclaw_process_sandbox`, `ironclaw_processes`, `ironclaw_wasm` | `tests/integration/process_port.rs` | Not covered (note 5) | +| **Per-project Docker sandbox** — filesystem/shell tools routed through a per-project container (`SANDBOX_ENABLED`; `crates/Dockerfile.sandbox`) | Typed `SandboxProcessPlan` contract and validation only (`ironclaw_process_sandbox`); no production execution backend is wired for this capability today | `contracts/processes.md`, `contracts/scripts.md` | `ironclaw_process_sandbox` (plan validation only); `ironclaw_processes`, `ironclaw_wasm` are unrelated general process/WASM runtimes, not the sandbox backend | `crates/ironclaw_process_sandbox/src/tests.rs` (plan validation); `tests/integration/process_port.rs` shows the *inert* `RecordingProcessPort` — it proves no real OS process is spawned, i.e. the absence of a backend, not evidence of one | Not covered (note 5) | | **OpenAI-compatible Responses API** — engine v2's OpenAI-compatible ingress | Contract-first, ProductSurface-backed Chat Completions + Responses (create/retrieve/cancel), idempotency/opaque-ref, projection-backed SSE streaming | `contracts/openai-compatible-api.md` | `ironclaw_reborn_openai_compat` | `crates/ironclaw_reborn_openai_compat/tests/responses_workflow_handlers_contract.rs`, `.../chat_workflow_handlers_contract.rs`, `.../streaming_handlers_contract.rs`, `.../error_contract.rs`, `.../ref_store_contract.rs` | Covered (note 6) | | **Skills** — trusted/installed skills, activation criteria, `skill_*` tools | First-party in-process skills extension: portable `SKILL.md` bundles; kernel owns trust/visibility/leases/context injection; catalog-first model-selected activation | `contracts/skills-extension.md` | `ironclaw_skills` | `tests/integration/group_extensions/` (suite), `crates/ironclaw_reborn_cli/tests/smoke.rs` | Covered | | **Hooks** — lifecycle hooks (6 points); `Hook` trait | Host-mediated hook execution with multi-backend persistence and an adversarial parity oracle | (hooks are exercised through `contracts/extensions.md` + capability dispatch) | `ironclaw_hooks`, `ironclaw_hooks_libsql`, `ironclaw_hooks_postgres`, `ironclaw_hooks_parity` | `crates/ironclaw_runner/tests/hooks_integration.rs`, `crates/ironclaw_hooks_parity/tests/parity_matrix.rs`, `crates/ironclaw_hooks_parity/tests/multi_host_adversarial.rs`, `crates/ironclaw_reborn_composition/tests/third_party_hook_projection.rs` | Covered | @@ -166,6 +166,10 @@ The only items that are not **Covered** are: - **Per-action ReliabilityTracker (EMA)** — Gap (note 7). Internal heuristic with no wire/contract surface; safe to drop or re-introduce as a new observability slice. +- **Per-project Docker sandbox** — Not covered (note 5). `ironclaw_process_sandbox` + owns a typed plan contract and validation only; no production execution + backend is wired, so effects that engine v2 routed through a per-project + Docker container currently have no Reborn execution path at all. These correspond to entries in the "Notable Gaps Before Reborn Can Replace Legacy" section of @@ -180,19 +184,23 @@ and no shipping path depends on it. ## Conclusion Every one of engine v2's five primitives (Thread, Step, Capability, MemoryDoc, -Project) and every runtime concern it layered on top (missions, -gates/approvals, sandbox, OpenAI-compatible Responses API, skills/hooks/ -extensions, effect dispatch, events/projections, the `LlmBackend`/`Store`/ -`EffectExecutor`/`WorkspaceReader` trait boundaries) maps to a named Reborn -contract, one or more dedicated crates, and passing test evidence. Two -capabilities are honestly **Partial** and one is a **Gap** — all three are -internal/experimental engine-v2 features (in-loop Monty CodeAct, unified -learning-mission firing, EMA reliability tracking) with no durable contract, -wire format, or shipping code path that engine-v2 removal would strand. +Project) and every runtime concern it layered on top — missions, +gates/approvals, OpenAI-compatible Responses API, skills/hooks/extensions, +effect dispatch, events/projections, the `LlmBackend`/`Store`/ +`EffectExecutor`/`WorkspaceReader` trait boundaries — maps to a named Reborn +contract, one or more dedicated crates, and passing test evidence, with one +notable exception: the **per-project Docker sandbox** is **Not covered** +(note 5) — Reborn validates typed sandbox plans but wires no production +execution backend for them today. Two further capabilities are honestly +**Partial** and one is a **Gap** — these three are internal/experimental +engine-v2 features (in-loop Monty CodeAct, unified learning-mission firing, +EMA reliability tracking) with no durable contract, wire format, or shipping +code path that engine-v2 removal would strand. Because engine v2 is default-off (`ENGINE_V2` false) and Reborn is fully independent of `ironclaw_engine`, deleting engine v2 removes only dead-by-default interim code. **Engine v2 can be safely removed.** The recommended follow-ups -(CodeAct loop family productionization, unified learning-mission wiring, and an -optional reliability/estimation observability slice) are net-new Reborn work, -not regressions caused by the removal. +(CodeAct loop family productionization, unified learning-mission wiring, an +optional reliability/estimation observability slice, and — separately from +this removal — wiring a production sandbox execution backend) are net-new +Reborn work, not regressions caused by the removal. From 08d0ccda2e5e3e0f3e59a0c73e3955bc431db877 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Sun, 26 Jul 2026 22:12:53 -0700 Subject: [PATCH 5/5] docs(reborn): fix parity doc legend, stale mount claim, gap cross-refs Address CodeRabbit review 4783753432 on PR #6693: define the new "Not covered" legend status, correct the WorkspaceReader row's stale claim that per-project mounts are realized by the process-sandbox bind-mount path (they're realized by the RootFilesystem mount catalog instead), and narrow the "These correspond to" cross-reference so it doesn't misattribute the sandbox gap to unrelated tracked epics. Co-Authored-By: Claude Opus 5 (1M context) --- docs/reborn/engine-v2-to-reborn-parity.md | 27 ++++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/docs/reborn/engine-v2-to-reborn-parity.md b/docs/reborn/engine-v2-to-reborn-parity.md index 66cdec5e60c..f082e36e681 100644 --- a/docs/reborn/engine-v2-to-reborn-parity.md +++ b/docs/reborn/engine-v2-to-reborn-parity.md @@ -43,7 +43,9 @@ engine v2 and are intentionally reshaped or deferred in Reborn. Legend: **Covered** = Reborn has an equivalent contract, crate, and test evidence; **Partial** = an equivalent exists but with a scoped delta noted; -**Gap** = no direct equivalent. +**Not covered** = a typed contract/plan exists but no production execution +backend is wired for it; **Gap** = no direct equivalent, and none is needed +(safe to drop or reintroduce later as a new slice). | Engine v2 capability | Reborn equivalent | Contract doc | Crate(s) | Test evidence | Status | | --- | --- | --- | --- | --- | --- | @@ -69,7 +71,7 @@ evidence; **Partial** = an equivalent exists but with a scoped delta noted; | **Events + Projections** — `ThreadEvent`, `EventKind` (18 variants), event sourcing from day one; `types/event.rs` | Explicit boundary between realtime delivery, durable audit/history, transcript milestones, and derived projections; durable event store | `contracts/events.md`, `contracts/events-projections.md` | `ironclaw_events`, `ironclaw_event_projections`, `ironclaw_event_streams`, `ironclaw_reborn_event_store` | `crates/ironclaw_reborn_event_store/tests/durable_event_store_contract.rs`, `.../filesystem_event_log_contract.rs`, `.../coalescing_sink_contract.rs`, `crates/ironclaw_runner/tests/loop_milestone_event_projection.rs` | Covered | | **`LlmBackend` trait** — `complete(messages, actions, config)`; host wraps `LlmProvider` | Host-managed model requests / tool-capable model gateway; provider-safe tool projection (dotted `CapabilityId` ↔ provider names) | `contracts/host-api.md`, `contracts/runtime-workflows.md` | `ironclaw_llm`, `ironclaw_runner` (model routes/gateway) | `crates/ironclaw_runner/tests/model_routes.rs`, `crates/ironclaw_runner/tests/llm_gateway.rs` | Covered | | **`Store` trait** — 20-method Thread/Step/Event/Project/Doc/Lease/Mission CRUD; host wraps `Database` (PG + libSQL) | Durable stores over the scoped-filesystem substrate + PostgreSQL/libSQL, with backend-parity readiness diagnostics | `contracts/storage-placement.md`, `contracts/turn-persistence.md`, `contracts/run-state.md` | `ironclaw_run_state`, `ironclaw_reborn_event_store`, `ironclaw_memory_native`, `ironclaw_conversations` | `crates/ironclaw_reborn_composition/tests/postgres_substrate.rs`, `.../libsql_substrate.rs`, `crates/ironclaw_conversations/tests/conversation_state_store_contract.rs` | Covered | -| **`WorkspaceReader` trait** — read-side workspace access; `traits/workspace.rs`, `workspace/` mounts (`MountBackend`, `ProjectMounts`) | Scoped filesystem substrate + memory read side; per-project mounts realized by the process sandbox bind-mount path | `contracts/filesystem.md`, `contracts/memory.md` | `ironclaw_filesystem`, `ironclaw_memory` | `crates/ironclaw_reborn_composition/tests/libsql_substrate.rs`, `tests/reborn_trace_file_tools_parity.rs` | Covered | +| **`WorkspaceReader` trait** — read-side workspace access; `traits/workspace.rs`, `workspace/` mounts (`MountBackend`, `ProjectMounts`) | Scoped filesystem substrate + memory read side; per-project scoping realized by the `RootFilesystem` mount catalog (`CompositeRootFilesystem`/`ScopedFilesystem`), independent of the (Not covered) process-sandbox execution backend | `contracts/filesystem.md`, `contracts/memory.md` | `ironclaw_filesystem`, `ironclaw_memory` | `crates/ironclaw_reborn_composition/tests/libsql_substrate.rs`, `tests/reborn_trace_file_tools_parity.rs` | Covered | | **ConversationManager / ConversationSurface** — routes UI messages to threads; `runtime/conversation.rs` | Conversation binding: inbound routing to the correct run/thread with scope isolation | `contracts/conversation-binding.md`, `contracts/communication-delivery-resolution.md` | `ironclaw_conversations`, `ironclaw_outbound` | `tests/reborn_direct_chat_user_scope_isolation_parity.rs`, `tests/reborn_outbound_reply_target_scope_isolation_parity.rs`, `crates/ironclaw_conversations/tests/inbound_contract.rs` | Covered | | **Context builder + compaction** — `executor/context.rs`, `executor/compaction.rs` (context assembly, compaction near model limit) | Loop context strategies and prompt envelope assembly | `contracts/turns-agent-loop.md` | `ironclaw_agent_loop` (`strategies/context.rs`), `ironclaw_prompt_envelope` | `crates/ironclaw_agent_loop/src/executor/tests.rs`, `crates/ironclaw_reborn_composition/src/runtime/tests/default_system_prompt.rs` | Covered | | **ThreadTree / sub-agents** — parent-child relationships; `runtime/tree.rs` | Subagent spawn through capability authorization/dispatch, wired into the runtime | `contracts/agent-loop-protocol.md` (`spawn_subagent`) | `ironclaw_agent_loop`, `ironclaw_reborn_composition` | `tests/reborn_subagent_spawn_e2e.rs`, `crates/ironclaw_reborn_composition/tests/subagent_runtime_wiring.rs` | Covered | @@ -171,15 +173,18 @@ The only items that are not **Covered** are: backend is wired, so effects that engine v2 routed through a per-project Docker container currently have no Reborn execution path at all. -These correspond to entries in the "Notable Gaps Before Reborn Can Replace -Legacy" section of -`docs/internal/2026-06-26-legacy-vs-reborn-feature-comparison.md` (Automation -production readiness; Model/provider feature parity) and to the residual epics -noted in `docs/reborn/production-cutover-readiness-closeout.md` (#4539 -approvals parity, #3029 migration). Critically, **none of these gaps are -engine-v2-specific**: they are Reborn-vs-legacy-product deltas that exist -independently of whether engine v2 is present, because engine v2 is gated off -and no shipping path depends on it. +The unified learning-mission automation gap corresponds to the "Automation +production readiness" and "Model/provider feature parity" entries in the +"Notable Gaps Before Reborn Can Replace Legacy" section of +`docs/internal/2026-06-26-legacy-vs-reborn-feature-comparison.md`, and to the +residual epics noted in `docs/reborn/production-cutover-readiness-closeout.md` +(#4539 approvals parity, #3029 migration). The per-project Docker sandbox gap +has no separate tracked follow-up ticket today; `FEATURE_PARITY.md` carries +the same "plan validation only, no execution backend" status as the row +above. Critically, **none of these gaps are engine-v2-specific**: they are +Reborn-vs-legacy-product deltas that exist independently of whether engine v2 +is present, because engine v2 is gated off and no shipping path depends on +it. ## Conclusion