From f1b88a267b3bb8c7fc96f58863bcca7d37c119da Mon Sep 17 00:00:00 2001 From: Alan Szmyt Date: Mon, 31 Aug 2026 15:56:01 -0400 Subject: [PATCH] feat(pipeline): centralize bounded process execution Add one process execution port for wrapped tools and migrate command transforms, aggregation, dependency probes, doctor/plugin checks, and PDF probing onto it. Bound runtime and captured output, control child environments, classify explicit shell use, redact secret-bearing diagnostics, validate declared outputs, and terminate process trees on timeout or cancellation where supported. Document platform behavior and keep toolchain fingerprints scoped to #359. Closes #356 --- .../renderflow-core/src/adapters/command.rs | 100 +- crates/renderflow-core/src/commands/plugin.rs | 46 +- crates/renderflow-core/src/commands/system.rs | 32 +- crates/renderflow-core/src/deps.rs | 16 +- crates/renderflow-core/src/lib.rs | 1 + crates/renderflow-core/src/process.rs | 1592 +++++++++++++++++ crates/renderflow-core/src/strategies/pdf.rs | 91 +- .../src/transforms/aggregation.rs | 415 +---- .../renderflow-core/src/transforms/command.rs | 215 +-- docs/process-execution.md | 179 ++ 10 files changed, 2044 insertions(+), 643 deletions(-) create mode 100644 crates/renderflow-core/src/process.rs create mode 100644 docs/process-execution.md diff --git a/crates/renderflow-core/src/adapters/command.rs b/crates/renderflow-core/src/adapters/command.rs index e194e01..0fb2917 100644 --- a/crates/renderflow-core/src/adapters/command.rs +++ b/crates/renderflow-core/src/adapters/command.rs @@ -1,65 +1,56 @@ -use anyhow::{bail, Result}; -use std::io::ErrorKind; -use std::process::Command; +use anyhow::Result; use tracing::{error, info}; -pub fn run_command(program: &str, args: &[&str]) -> Result<()> { - info!(program = program, args = ?args, "Running command"); - - let output = Command::new(program).args(args).output().map_err(|e| { - if e.kind() == ErrorKind::NotFound { - anyhow::anyhow!( - "`{}` was not found. Make sure it is installed and available in your PATH.", - program - ) - } else { - anyhow::anyhow!("Failed to launch `{}`: {}", program, e) - } - })?; - - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); +use crate::process::{ + is_explicit_shell_invocation, ProcessExecutor, ProcessRequest, DEFAULT_CAPTURE_LIMIT_BYTES, + DEFAULT_PROCESS_TIMEOUT, +}; - if !stdout.is_empty() { - info!(stdout = %stdout.trim_end(), "Command stdout"); +/// Run one external command through Renderflow's canonical bounded process executor. +/// +/// `program` is executed directly with the provided argv. If the caller +/// explicitly names a shell program and supplies a shell-evaluation flag such as +/// `-c`, the request is classified as an explicit shell invocation so the wider +/// trust boundary remains visible in process diagnostics. +pub fn run_command(program: &str, args: &[&str]) -> Result<()> { + let owned_args: Vec = args.iter().map(|value| (*value).to_string()).collect(); + let request = if is_explicit_shell_invocation(program, &owned_args) { + ProcessRequest::shell(program) + } else { + ProcessRequest::direct(program) } + .args(owned_args) + .timeout(DEFAULT_PROCESS_TIMEOUT) + .capture_limit(DEFAULT_CAPTURE_LIMIT_BYTES); - if !stderr.is_empty() { - if output.status.success() { - info!(stderr = %stderr.trim_end(), "Command stderr"); - } else { - error!(stderr = %stderr.trim_end(), "Command stderr"); - } + let result = ProcessExecutor::new().execute(request)?; + + if !result.stdout().redacted_text().is_empty() { + info!( + stdout = %result.stdout().redacted_text().trim_end(), + truncated = result.stdout().truncated(), + "Command stdout" + ); } - if !output.status.success() { - let stderr_hint = if stderr.trim().is_empty() { - String::new() + if !result.stderr().redacted_text().is_empty() { + if result.is_success() { + info!( + stderr = %result.stderr().redacted_text().trim_end(), + truncated = result.stderr().truncated(), + "Command stderr" + ); } else { - format!("\nStderr: {}", stderr.trim_end()) - }; - match output.status.code() { - Some(code) => { - error!(program = program, exit_code = code, "Command failed"); - bail!( - "Command `{}` failed with exit code {}{}", - program, - code, - stderr_hint - ); - } - None => { - error!(program = program, "Command terminated by signal"); - bail!( - "Command `{}` was terminated by a signal{}", - program, - stderr_hint - ); - } + error!( + stderr = %result.stderr().redacted_text().trim_end(), + truncated = result.stderr().truncated(), + "Command stderr" + ); } } - info!(program = program, "Command completed successfully"); + result.ensure_success()?; + info!(program = program, duration_ms = result.duration_ms(), "Command completed successfully"); Ok(()) } @@ -79,13 +70,14 @@ mod tests { assert!(result.is_ok(), "echo with multiple args should succeed"); } + #[cfg(unix)] #[test] fn test_failure() { let result = run_command("false", &[]); assert!(result.is_err(), "false should fail"); let err = result.unwrap_err().to_string(); assert!( - err.contains("failed with exit code"), + err.contains("exited with code"), "error message should mention exit code" ); } @@ -105,10 +97,10 @@ mod tests { ); } + #[cfg(unix)] #[test] fn test_failure_error_includes_stderr() { - // `sh -c` lets us write to stderr and exit non-zero in a portable way. - let result = run_command("sh", &["-c", "echo 'some error output' >&2; exit 1"]); + let result = run_command("sh", &["-c", "printf '%s' 'some error output' >&2; exit 1"]); assert!(result.is_err(), "command should fail"); let err = result.unwrap_err().to_string(); assert!( diff --git a/crates/renderflow-core/src/commands/plugin.rs b/crates/renderflow-core/src/commands/plugin.rs index 4027d67..8cfc979 100644 --- a/crates/renderflow-core/src/commands/plugin.rs +++ b/crates/renderflow-core/src/commands/plugin.rs @@ -1,18 +1,13 @@ use anyhow::Result; -use std::process::Command; +use crate::process::ProcessExecutor; use crate::transforms::plugin::{PluginCapabilities, PluginMetadata, PluginRegistry}; + // ── list ────────────────────────────────────────────────────────────────────── /// Run `renderflow plugin list`. /// -/// Prints a summary table of every plugin in `registry`: -/// -/// ```text -/// Registered plugins (2): -/// upper 1.0.0 A test plugin -/// lower 0.5.0 Converts text to lowercase -/// ``` +/// Prints a summary table of every plugin in `registry`. pub fn run_list(registry: &PluginRegistry) -> Result<()> { let mut names: Vec<&str> = registry.plugin_names(); names.sort(); @@ -40,9 +35,6 @@ pub fn run_list(registry: &PluginRegistry) -> Result<()> { // ── info ────────────────────────────────────────────────────────────────────── /// Run `renderflow plugin info `. -/// -/// Prints the full [`PluginMetadata`] for the named plugin, or returns an -/// error when the plugin is not registered. pub fn run_info(registry: &PluginRegistry, name: &str) -> Result<()> { let info = registry .plugin_info(name) @@ -109,9 +101,6 @@ fn print_capabilities(caps: &PluginCapabilities) { // ── validate ───────────────────────────────────────────────────────────────── /// Run `renderflow plugin validate`. -/// -/// Validates all plugin metadata in `registry` and prints a report. -/// Returns an error when any validation issue is found. pub fn run_validate(registry: &PluginRegistry) -> Result<()> { let issues = registry.validate_all(); @@ -132,12 +121,8 @@ pub fn run_validate(registry: &PluginRegistry) -> Result<()> { /// Run `renderflow plugin doctor`. /// -/// Runs diagnostics on every registered plugin: -/// * Validates metadata. -/// * Checks that required external tools are present on `PATH`. -/// -/// Prints a report and returns `Ok(())` even when issues are found (so the -/// caller can decide whether to treat the output as advisory or fatal). +/// Validates metadata and probes required external tools through Renderflow's +/// canonical bounded process executor. pub fn run_doctor(registry: &PluginRegistry) -> Result<()> { let mut names: Vec<&str> = registry.plugin_names(); names.sort(); @@ -154,13 +139,11 @@ pub fn run_doctor(registry: &PluginRegistry) -> Result<()> { for name in &names { let mut issues: Vec = Vec::new(); - // 1. Metadata validation. if let Some(meta) = registry.metadata(name) { if let Err(e) = meta.validate() { issues.push(format!("invalid metadata: {}", e)); } - // 2. Required tools check. for tool in &meta.required_tools { if !tool_is_available(tool) { issues.push(format!("required tool '{}' was not found on PATH", tool)); @@ -188,13 +171,9 @@ pub fn run_doctor(registry: &PluginRegistry) -> Result<()> { Ok(()) } -/// Return `true` when `tool` can be found on the system `PATH`. +/// Return `true` when `tool` can be found and successfully version-probed. fn tool_is_available(tool: &str) -> bool { - Command::new(tool) - .arg("--version") - .output() - .map(|o| o.status.success()) - .unwrap_or(false) + ProcessExecutor::new().probe_version(tool).is_available() } #[cfg(test)] @@ -220,8 +199,6 @@ mod tests { .with_author("Tester") } - // ── run_list ────────────────────────────────────────────────────────────── - #[test] fn test_list_empty_registry_succeeds() { let registry = PluginRegistry::new(); @@ -244,8 +221,6 @@ mod tests { assert!(run_list(®istry).is_ok()); } - // ── run_info ────────────────────────────────────────────────────────────── - #[test] fn test_info_known_plugin_succeeds() { let mut registry = PluginRegistry::new(); @@ -270,8 +245,6 @@ mod tests { assert!(run_info(®istry, "bare").is_ok()); } - // ── run_validate ────────────────────────────────────────────────────────── - #[test] fn test_validate_empty_registry_succeeds() { let registry = PluginRegistry::new(); @@ -287,8 +260,6 @@ mod tests { assert!(run_validate(®istry).is_ok()); } - // ── run_doctor ──────────────────────────────────────────────────────────── - #[test] fn test_doctor_empty_registry_succeeds() { let registry = PluginRegistry::new(); @@ -312,12 +283,9 @@ mod tests { registry .register_with_metadata(Arc::new(DummyPlugin("needs-tool")), meta) .unwrap(); - // doctor returns Ok even when tools are missing (advisory output only) assert!(run_doctor(®istry).is_ok()); } - // ── tool_is_available ───────────────────────────────────────────────────── - #[test] fn test_tool_is_available_returns_false_for_nonexistent() { assert!(!tool_is_available("__renderflow_nonexistent_xyz__")); diff --git a/crates/renderflow-core/src/commands/system.rs b/crates/renderflow-core/src/commands/system.rs index bda59e1..9ce4c87 100644 --- a/crates/renderflow-core/src/commands/system.rs +++ b/crates/renderflow-core/src/commands/system.rs @@ -1,5 +1,7 @@ use anyhow::{bail, Result}; -use std::{env, path::PathBuf, process::Command}; +use std::{env, path::PathBuf}; + +use crate::process::{ProcessExecutor, ToolProbeStatus}; struct ToolCheck { name: &'static str, @@ -24,23 +26,19 @@ const TOOL_CHECKS: [ToolCheck; 3] = [ ]; fn probe_tool_version(name: &str) -> Result { - Command::new(name) - .arg("--version") - .output() - .map_err(|_| format!("missing ({name} not found in PATH)")) - .and_then(|output| { - if output.status.success() { - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - let line = stdout.lines().next().or_else(|| stderr.lines().next()); - Ok(line.unwrap_or("available").trim().to_string()) - } else { - Err(format!( - "installed but failed to execute ({name} --version)" - )) - } - }) + let probe = ProcessExecutor::new().probe_version(name); + match probe.status { + ToolProbeStatus::Available => Ok(probe + .version_line + .unwrap_or_else(|| "available".to_string())), + ToolProbeStatus::Missing => Err(format!("missing ({name} not found in PATH)")), + ToolProbeStatus::TimedOut => Err(format!("installed but version probe timed out ({name} --version)")), + ToolProbeStatus::Failed => Err(probe + .diagnostic + .unwrap_or_else(|| format!("installed but failed to execute ({name} --version)"))), + } } + pub fn run_version() { println!("renderflow {}", env!("CARGO_PKG_VERSION")); } diff --git a/crates/renderflow-core/src/deps.rs b/crates/renderflow-core/src/deps.rs index 81db7ba..ba95f64 100644 --- a/crates/renderflow-core/src/deps.rs +++ b/crates/renderflow-core/src/deps.rs @@ -1,15 +1,12 @@ use anyhow::Result; -use std::process::Command; use crate::error::RenderError; +use crate::process::ProcessExecutor; -/// Check whether a tool is available in the system PATH. +/// Check whether a tool is available in the system PATH using the canonical +/// bounded version-probe path. fn tool_available(name: &str) -> bool { - Command::new(name) - .arg("--version") - .output() - .map(|o| o.status.success()) - .unwrap_or(false) + ProcessExecutor::new().probe_version(name).is_available() } /// Verify that `pandoc` is installed and available in PATH. @@ -113,7 +110,7 @@ mod tests { fn test_check_pandoc_error_contains_install_hint() { let result = check_pandoc(); if pandoc_available() { - return; // nothing to assert + return; } let msg = result.unwrap_err().to_string(); assert!( @@ -158,8 +155,6 @@ mod tests { #[test] fn test_validate_dependencies_without_pdf_only_checks_pandoc() { - // When pdf_requested is false, tectonic is not checked. - // We can only verify the outcome matches what pandoc availability predicts. let result = validate_dependencies(false); if pandoc_available() { assert!( @@ -194,7 +189,6 @@ mod tests { #[test] fn test_tool_available_with_known_tool() { - // `cargo` is always available in a Rust build environment and supports `--version`. assert!( tool_available("cargo"), "cargo should always be available in a Rust build environment" diff --git a/crates/renderflow-core/src/lib.rs b/crates/renderflow-core/src/lib.rs index 7389499..1228140 100644 --- a/crates/renderflow-core/src/lib.rs +++ b/crates/renderflow-core/src/lib.rs @@ -24,6 +24,7 @@ mod incremental; mod input_format; pub mod optimization; mod pipeline; +pub mod process; mod sdk; pub mod strategies; mod template; diff --git a/crates/renderflow-core/src/process.rs b/crates/renderflow-core/src/process.rs new file mode 100644 index 0000000..74ba329 --- /dev/null +++ b/crates/renderflow-core/src/process.rs @@ -0,0 +1,1592 @@ +//! Bounded, policy-aware external process execution. +//! +//! All production subprocesses in Renderflow should flow through this module so +//! adapters do not independently reinvent environment handling, output capture, +//! timeout/cancellation behavior, process-tree termination, diagnostics, or +//! expected-output validation. + +use std::collections::{BTreeMap, BTreeSet}; +use std::env; +use std::fmt; +use std::fs; +use std::io::{self, Read, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, ExitStatus, Stdio}; +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, +}; +use std::thread; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +#[cfg(unix)] +use std::os::unix::process::CommandExt; + +use tracing::debug; + +/// Default wall-clock timeout for ordinary wrapped tools. +pub const DEFAULT_PROCESS_TIMEOUT: Duration = Duration::from_secs(30 * 60); +/// Default maximum bytes retained independently for stdout and stderr. +pub const DEFAULT_CAPTURE_LIMIT_BYTES: usize = 256 * 1024; +const PROBE_TIMEOUT: Duration = Duration::from_secs(5); +const PROBE_CAPTURE_LIMIT_BYTES: usize = 16 * 1024; +const POLL_INTERVAL: Duration = Duration::from_millis(20); +const TERMINATION_GRACE: Duration = Duration::from_millis(300); + +/// Whether the caller requested direct argv execution or an explicit shell. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProcessInvocationKind { + Direct, + Shell, +} + +/// Declarative network intent for future sandbox/policy hooks. +/// +/// This value is evidence/policy input; it is not itself a network sandbox. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProcessNetworkPolicy { + Unspecified, + Allow, + Deny, +} + +/// Process-tree termination support available on the current platform. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProcessTreeTermination { + UnixProcessGroup, + WindowsTaskkill, + DirectChild, +} + +/// Platform evidence attached to every process outcome. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ProcessPlatform { + pub os: &'static str, + pub arch: &'static str, + pub tree_termination: ProcessTreeTermination, +} + +impl ProcessPlatform { + fn current(tree_mode: ProcessTreeMode) -> Self { + let tree_termination = if tree_mode == ProcessTreeMode::ChildOnly { + ProcessTreeTermination::DirectChild + } else if cfg!(unix) { + ProcessTreeTermination::UnixProcessGroup + } else if cfg!(windows) { + ProcessTreeTermination::WindowsTaskkill + } else { + ProcessTreeTermination::DirectChild + }; + Self { + os: env::consts::OS, + arch: env::consts::ARCH, + tree_termination, + } + } +} + +/// Whether cancellation/timeout should target a process tree or only the child. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProcessTreeMode { + ProcessTree, + ChildOnly, +} + +/// Clonable cancellation signal for synchronous process execution. +#[derive(Debug, Clone, Default)] +pub struct ProcessCancellationToken { + cancelled: Arc, +} + +impl ProcessCancellationToken { + pub fn new() -> Self { + Self::default() + } + + pub fn cancel(&self) { + self.cancelled.store(true, Ordering::SeqCst); + } + + pub fn is_cancelled(&self) -> bool { + self.cancelled.load(Ordering::SeqCst) + } +} + +/// How stdin is connected to the child. +pub enum ProcessInput { + Null, + Inherit, + Bytes(Vec), +} + +impl fmt::Debug for ProcessInput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Null => f.write_str("Null"), + Self::Inherit => f.write_str("Inherit"), + Self::Bytes(bytes) => f + .debug_struct("Bytes") + .field("len", &bytes.len()) + .finish(), + } + } +} + +/// How stdout/stderr are connected to the child. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProcessOutputMode { + Null, + Inherit, + Capture { max_bytes: usize }, +} + +impl ProcessOutputMode { + pub fn capture(max_bytes: usize) -> Self { + Self::Capture { max_bytes } + } +} + +/// One process argument with an explicit sensitivity marker. +#[derive(Clone, PartialEq, Eq)] +pub struct ProcessArgument { + value: String, + sensitive: bool, +} + +impl ProcessArgument { + pub fn plain(value: impl Into) -> Self { + Self { + value: value.into(), + sensitive: false, + } + } + + pub fn sensitive(value: impl Into) -> Self { + Self { + value: value.into(), + sensitive: true, + } + } + + fn value(&self) -> &str { + &self.value + } +} + +impl fmt::Debug for ProcessArgument { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.sensitive { + f.write_str("[REDACTED]") + } else { + f.debug_tuple("arg").field(&self.value).finish() + } + } +} + +#[derive(Clone)] +struct EnvironmentValue { + value: String, + sensitive: bool, +} + +impl fmt::Debug for EnvironmentValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.sensitive { + f.write_str("[REDACTED]") + } else { + f.debug_tuple("value").field(&self.value).finish() + } + } +} + +/// Controlled child-environment policy. +/// +/// The default inherits ordinary parent variables but strips names that look +/// credential-bearing. A caller must explicitly allow or set sensitive values. +#[derive(Debug, Clone)] +pub struct ProcessEnvironment { + inherit_filtered: bool, + allow_sensitive: BTreeSet, + deny: BTreeSet, + overrides: BTreeMap, +} + +impl Default for ProcessEnvironment { + fn default() -> Self { + Self::filtered_inherit() + } +} + +impl ProcessEnvironment { + pub fn filtered_inherit() -> Self { + Self { + inherit_filtered: true, + allow_sensitive: BTreeSet::new(), + deny: BTreeSet::new(), + overrides: BTreeMap::new(), + } + } + + pub fn clear() -> Self { + Self { + inherit_filtered: false, + allow_sensitive: BTreeSet::new(), + deny: BTreeSet::new(), + overrides: BTreeMap::new(), + } + } + + pub fn allow_sensitive(mut self, name: impl Into) -> Self { + self.allow_sensitive.insert(normalize_env_name(&name.into())); + self + } + + pub fn deny(mut self, name: impl Into) -> Self { + self.deny.insert(normalize_env_name(&name.into())); + self + } + + pub fn set(mut self, name: impl Into, value: impl Into) -> Self { + let name = name.into(); + let sensitive = is_sensitive_name(&name); + self.overrides.insert( + normalize_env_name(&name), + EnvironmentValue { + value: value.into(), + sensitive, + }, + ); + self + } + + pub fn set_sensitive( + mut self, + name: impl Into, + value: impl Into, + ) -> Self { + let name = name.into(); + self.overrides.insert( + normalize_env_name(&name), + EnvironmentValue { + value: value.into(), + sensitive: true, + }, + ); + self + } + + fn apply(&self, command: &mut Command) -> Vec { + command.env_clear(); + let mut redactions = Vec::new(); + + if self.inherit_filtered { + for (name, value) in env::vars_os() { + let normalized = normalize_env_name(&name.to_string_lossy()); + if self.deny.contains(&normalized) { + continue; + } + let sensitive = is_sensitive_name(&normalized); + if sensitive && !self.allow_sensitive.contains(&normalized) { + continue; + } + if sensitive { + redactions.push(value.to_string_lossy().into_owned()); + } + command.env(&name, &value); + } + } + + for (name, value) in &self.overrides { + command.env(name, &value.value); + if value.sensitive || is_sensitive_name(name) { + redactions.push(value.value.clone()); + } + } + + redactions + } +} + +/// Expected filesystem output produced by a subprocess. +#[derive(Debug, Clone)] +pub struct ProcessExpectedOutput { + path: PathBuf, + kind: ExpectedOutputKind, + require_non_empty: bool, + require_change: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ExpectedOutputKind { + Any, + File, + Directory, +} + +impl ProcessExpectedOutput { + pub fn any(path: impl Into) -> Self { + Self { + path: path.into(), + kind: ExpectedOutputKind::Any, + require_non_empty: false, + require_change: false, + } + } + + pub fn file(path: impl Into) -> Self { + Self { + path: path.into(), + kind: ExpectedOutputKind::File, + require_non_empty: false, + require_change: false, + } + } + + pub fn directory(path: impl Into) -> Self { + Self { + path: path.into(), + kind: ExpectedOutputKind::Directory, + require_non_empty: false, + require_change: false, + } + } + + pub fn require_non_empty(mut self) -> Self { + self.require_non_empty = true; + self + } + + pub fn require_change(mut self) -> Self { + self.require_change = true; + self + } + + pub fn path(&self) -> &Path { + &self.path + } +} + +/// Request supplied to the canonical process executor. +pub struct ProcessRequest { + executable: String, + args: Vec, + invocation_kind: ProcessInvocationKind, + working_directory: Option, + stdin: ProcessInput, + stdout: ProcessOutputMode, + stderr: ProcessOutputMode, + environment: ProcessEnvironment, + timeout: Option, + cancellation: Option, + tree_mode: ProcessTreeMode, + expected_outputs: Vec, + redacted_values: Vec, + network_policy: ProcessNetworkPolicy, + sandbox_profile: Option, +} + +impl fmt::Debug for ProcessRequest { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProcessRequest") + .field("executable", &self.executable) + .field("args", &safe_argument_display(&self.args, &Redactor::default())) + .field("invocation_kind", &self.invocation_kind) + .field("working_directory", &self.working_directory) + .field("stdin", &self.stdin) + .field("stdout", &self.stdout) + .field("stderr", &self.stderr) + .field("environment", &"") + .field("timeout", &self.timeout) + .field("tree_mode", &self.tree_mode) + .field("expected_outputs", &self.expected_outputs) + .field("network_policy", &self.network_policy) + .field("sandbox_profile", &self.sandbox_profile) + .finish() + } +} + +impl ProcessRequest { + pub fn direct(executable: impl Into) -> Self { + Self::new(executable.into(), ProcessInvocationKind::Direct) + } + + pub fn shell(executable: impl Into) -> Self { + Self::new(executable.into(), ProcessInvocationKind::Shell) + } + + fn new(executable: String, invocation_kind: ProcessInvocationKind) -> Self { + Self { + executable, + args: Vec::new(), + invocation_kind, + working_directory: None, + stdin: ProcessInput::Null, + stdout: ProcessOutputMode::capture(DEFAULT_CAPTURE_LIMIT_BYTES), + stderr: ProcessOutputMode::capture(DEFAULT_CAPTURE_LIMIT_BYTES), + environment: ProcessEnvironment::default(), + timeout: Some(DEFAULT_PROCESS_TIMEOUT), + cancellation: None, + tree_mode: ProcessTreeMode::ProcessTree, + expected_outputs: Vec::new(), + redacted_values: Vec::new(), + network_policy: ProcessNetworkPolicy::Unspecified, + sandbox_profile: None, + } + } + + pub fn arg(mut self, value: impl Into) -> Self { + self.args.push(ProcessArgument::plain(value)); + self + } + + pub fn sensitive_arg(mut self, value: impl Into) -> Self { + let value = value.into(); + self.redacted_values.push(value.clone()); + self.args.push(ProcessArgument::sensitive(value)); + self + } + + pub fn args(mut self, values: I) -> Self + where + I: IntoIterator, + S: Into, + { + self.args + .extend(values.into_iter().map(|value| ProcessArgument::plain(value))); + self + } + + pub fn working_directory(mut self, path: impl Into) -> Self { + self.working_directory = Some(path.into()); + self + } + + pub fn stdin(mut self, input: ProcessInput) -> Self { + self.stdin = input; + self + } + + pub fn stdout(mut self, mode: ProcessOutputMode) -> Self { + self.stdout = mode; + self + } + + pub fn stderr(mut self, mode: ProcessOutputMode) -> Self { + self.stderr = mode; + self + } + + pub fn capture_limit(mut self, max_bytes: usize) -> Self { + self.stdout = ProcessOutputMode::capture(max_bytes); + self.stderr = ProcessOutputMode::capture(max_bytes); + self + } + + pub fn environment(mut self, environment: ProcessEnvironment) -> Self { + self.environment = environment; + self + } + + pub fn timeout(mut self, timeout: Duration) -> Self { + self.timeout = Some(timeout); + self + } + + pub fn without_timeout(mut self) -> Self { + self.timeout = None; + self + } + + pub fn cancellation(mut self, cancellation: ProcessCancellationToken) -> Self { + self.cancellation = Some(cancellation); + self + } + + pub fn child_only(mut self) -> Self { + self.tree_mode = ProcessTreeMode::ChildOnly; + self + } + + pub fn expect_output(mut self, expected: ProcessExpectedOutput) -> Self { + self.expected_outputs.push(expected); + self + } + + pub fn redact_value(mut self, value: impl Into) -> Self { + self.redacted_values.push(value.into()); + self + } + + pub fn network_policy(mut self, policy: ProcessNetworkPolicy) -> Self { + self.network_policy = policy; + self + } + + pub fn sandbox_profile(mut self, profile: impl Into) -> Self { + self.sandbox_profile = Some(profile.into()); + self + } + + pub fn executable(&self) -> &str { + &self.executable + } + + pub fn invocation_kind(&self) -> ProcessInvocationKind { + self.invocation_kind + } +} + +/// Hook for environment-specific policy enforcement such as network or sandbox +/// restrictions. The core request fields are declarative until a configured +/// hook enforces them. +pub trait ProcessPolicyHook: Send + Sync { + fn validate(&self, request: &ProcessRequest) -> Result<(), String>; +} + +/// Canonical subprocess execution service. +#[derive(Clone, Default)] +pub struct ProcessExecutor { + hooks: Vec>, +} + +impl ProcessExecutor { + pub fn new() -> Self { + Self::default() + } + + pub fn with_policy_hook(mut self, hook: Arc) -> Self { + self.hooks.push(hook); + self + } + + pub fn execute(&self, request: ProcessRequest) -> Result { + if request.executable.trim().is_empty() { + return Err(ProcessError::PolicyRejected( + "process executable must not be empty".to_string(), + )); + } + if request.invocation_kind == ProcessInvocationKind::Direct + && is_explicit_shell_invocation( + &request.executable, + &request + .args + .iter() + .map(|arg| arg.value.clone()) + .collect::>(), + ) + { + return Err(ProcessError::ShellRequiresOptIn { + executable: request.executable.clone(), + }); + } + + let mut redactions = request.redacted_values.clone(); + for argument in &request.args { + if argument.sensitive { + redactions.push(argument.value.clone()); + } + } + let mut redactor = Redactor::new(redactions); + + for hook in &self.hooks { + if let Err(message) = hook.validate(&request) { + return Err(ProcessError::PolicyRejected(redactor.redact(&message))); + } + } + + let before_outputs: Vec = request + .expected_outputs + .iter() + .map(|expected| OutputSnapshot::capture(expected.path())) + .collect(); + + let mut command = Command::new(&request.executable); + command.args(request.args.iter().map(ProcessArgument::value)); + if let Some(path) = &request.working_directory { + command.current_dir(path); + } + redactor.extend(request.environment.apply(&mut command)); + + command.stdin(match &request.stdin { + ProcessInput::Null => Stdio::null(), + ProcessInput::Inherit => Stdio::inherit(), + ProcessInput::Bytes(_) => Stdio::piped(), + }); + command.stdout(stdio_for_output(request.stdout)); + command.stderr(stdio_for_output(request.stderr)); + + #[cfg(unix)] + if request.tree_mode == ProcessTreeMode::ProcessTree { + command.process_group(0); + } + + let safe_args = safe_argument_display(&request.args, &redactor); + let command_display = safe_command_display(&request.executable, &safe_args); + debug!( + executable = %request.executable, + args = ?safe_args, + shell = request.invocation_kind == ProcessInvocationKind::Shell, + timeout_ms = request.timeout.map(|value| value.as_millis() as u64), + network_policy = ?request.network_policy, + sandbox_profile = ?request.sandbox_profile, + "Starting external process" + ); + + let started_at_epoch_ms = epoch_millis(); + let started = Instant::now(); + let mut child = command.spawn().map_err(|error| { + if error.kind() == io::ErrorKind::NotFound { + ProcessError::MissingExecutable { + executable: request.executable.clone(), + } + } else { + ProcessError::Launch { + executable: request.executable.clone(), + message: redactor.redact(&error.to_string()), + } + } + })?; + + let stdout_reader = match request.stdout { + ProcessOutputMode::Capture { max_bytes } => child + .stdout + .take() + .map(|reader| spawn_bounded_reader(reader, max_bytes)), + _ => None, + }; + let stderr_reader = match request.stderr { + ProcessOutputMode::Capture { max_bytes } => child + .stderr + .take() + .map(|reader| spawn_bounded_reader(reader, max_bytes)), + _ => None, + }; + + let stdin_writer = match request.stdin { + ProcessInput::Bytes(bytes) => child.stdin.take().map(|mut writer| { + thread::spawn(move || -> io::Result<()> { + match writer.write_all(&bytes) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::BrokenPipe => Ok(()), + Err(error) => Err(error), + } + }) + }), + _ => None, + }; + + let termination = loop { + if request + .cancellation + .as_ref() + .is_some_and(ProcessCancellationToken::is_cancelled) + { + terminate_process_tree(&mut child, request.tree_mode).map_err(|error| { + ProcessError::Io(redactor.redact(&format!( + "failed to terminate cancelled process: {error}" + ))) + })?; + break ProcessTermination::Cancelled; + } + + if request.timeout.is_some_and(|timeout| started.elapsed() >= timeout) { + terminate_process_tree(&mut child, request.tree_mode).map_err(|error| { + ProcessError::Io(redactor.redact(&format!( + "failed to terminate timed-out process: {error}" + ))) + })?; + break ProcessTermination::TimedOut; + } + + match child.try_wait().map_err(|error| { + ProcessError::Io(redactor.redact(&format!( + "failed while waiting for process: {error}" + ))) + })? { + Some(status) => break termination_from_status(status), + None => thread::sleep(POLL_INTERVAL), + } + }; + + if let Some(writer) = stdin_writer { + match writer.join() { + Ok(Ok(())) => {} + Ok(Err(error)) if !termination.is_success() => { + debug!(error = %redactor.redact(&error.to_string()), "stdin writer ended after process termination"); + } + Ok(Err(error)) => { + return Err(ProcessError::Io(redactor.redact(&format!( + "failed to write process stdin: {error}" + )))); + } + Err(_) if !termination.is_success() => {} + Err(_) => { + return Err(ProcessError::Io( + "process stdin writer thread panicked".to_string(), + )); + } + } + } + + let stdout_bytes = join_bounded_reader(stdout_reader, "stdout")?; + let stderr_bytes = join_bounded_reader(stderr_reader, "stderr")?; + let stdout = CapturedOutput::new(stdout_bytes, &redactor); + let stderr = CapturedOutput::new(stderr_bytes, &redactor); + + let output_failures = if termination.is_success() { + request + .expected_outputs + .iter() + .zip(before_outputs.iter()) + .filter_map(|(expected, before)| expected.validate(before)) + .collect() + } else { + Vec::new() + }; + + let result = ProcessResult { + command_display, + termination, + stdout, + stderr, + started_at_epoch_ms, + duration_ms: started.elapsed().as_millis() as u64, + platform: ProcessPlatform::current(request.tree_mode), + output_failures, + }; + + debug!( + termination = ?result.termination, + duration_ms = result.duration_ms, + stdout_bytes = result.stdout.total_bytes, + stderr_bytes = result.stderr.total_bytes, + stdout_truncated = result.stdout.truncated, + stderr_truncated = result.stderr.truncated, + output_failures = result.output_failures.len(), + "External process completed" + ); + + Ok(result) + } + + pub fn execute_checked(&self, request: ProcessRequest) -> Result { + let result = self.execute(request)?; + result.ensure_success()?; + Ok(result) + } + + /// Probe ` --version` using the same bounded process policy. + pub fn probe_version(&self, executable: &str) -> ToolProbeEvidence { + let request = ProcessRequest::direct(executable) + .arg("--version") + .timeout(PROBE_TIMEOUT) + .capture_limit(PROBE_CAPTURE_LIMIT_BYTES); + + match self.execute(request) { + Ok(result) => { + let version_line = first_non_empty_line(result.stdout.redacted_text()) + .or_else(|| first_non_empty_line(result.stderr.redacted_text())) + .map(str::to_string); + let status = match result.termination { + ProcessTermination::Exited { code: 0 } => ToolProbeStatus::Available, + ProcessTermination::TimedOut => ToolProbeStatus::TimedOut, + _ => ToolProbeStatus::Failed, + }; + ToolProbeEvidence { + executable: executable.to_string(), + status, + version_line, + duration_ms: result.duration_ms, + platform: result.platform, + diagnostic: if status == ToolProbeStatus::Available { + None + } else { + Some(result.failure_message()) + }, + } + } + Err(ProcessError::MissingExecutable { .. }) => ToolProbeEvidence { + executable: executable.to_string(), + status: ToolProbeStatus::Missing, + version_line: None, + duration_ms: 0, + platform: ProcessPlatform::current(ProcessTreeMode::ProcessTree), + diagnostic: Some(format!("{executable} not found in PATH")), + }, + Err(error) => ToolProbeEvidence { + executable: executable.to_string(), + status: ToolProbeStatus::Failed, + version_line: None, + duration_ms: 0, + platform: ProcessPlatform::current(ProcessTreeMode::ProcessTree), + diagnostic: Some(error.to_string()), + }, + } + } +} + +/// Completed process termination classification. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProcessTermination { + Exited { code: i32 }, + Signaled, + TimedOut, + Cancelled, +} + +impl ProcessTermination { + pub fn is_success(self) -> bool { + matches!(self, Self::Exited { code: 0 }) + } +} + +/// Bounded captured stream. Raw bytes remain private from `Debug`; callers can +/// explicitly consume them for binary-safe pipelines while diagnostics should +/// use [`redacted_text`](Self::redacted_text). +pub struct CapturedOutput { + bytes: Vec, + redacted_text: String, + total_bytes: u64, + truncated: bool, +} + +impl CapturedOutput { + fn new(bytes: BoundedBytes, redactor: &Redactor) -> Self { + let redacted_text = redactor.redact(&String::from_utf8_lossy(&bytes.bytes)); + Self { + bytes: bytes.bytes, + redacted_text, + total_bytes: bytes.total_bytes, + truncated: bytes.truncated, + } + } + + pub fn bytes(&self) -> &[u8] { + &self.bytes + } + + pub fn redacted_text(&self) -> &str { + &self.redacted_text + } + + pub fn total_bytes(&self) -> u64 { + self.total_bytes + } + + pub fn truncated(&self) -> bool { + self.truncated + } + + fn diagnostic_text(&self) -> String { + let text = self.redacted_text.trim_end(); + if self.truncated { + format!( + "{text}\n[capture truncated: retained {} of {} bytes]", + self.bytes.len(), self.total_bytes + ) + } else { + text.to_string() + } + } +} + +impl fmt::Debug for CapturedOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("CapturedOutput") + .field("retained_bytes", &self.bytes.len()) + .field("total_bytes", &self.total_bytes) + .field("truncated", &self.truncated) + .field("redacted_text", &self.redacted_text) + .finish() + } +} + +/// Structured process outcome suitable for later execution-evidence projection. +pub struct ProcessResult { + command_display: String, + termination: ProcessTermination, + stdout: CapturedOutput, + stderr: CapturedOutput, + started_at_epoch_ms: u64, + duration_ms: u64, + platform: ProcessPlatform, + output_failures: Vec, +} + +impl ProcessResult { + pub fn termination(&self) -> ProcessTermination { + self.termination + } + + pub fn stdout(&self) -> &CapturedOutput { + &self.stdout + } + + pub fn stderr(&self) -> &CapturedOutput { + &self.stderr + } + + pub fn started_at_epoch_ms(&self) -> u64 { + self.started_at_epoch_ms + } + + pub fn duration_ms(&self) -> u64 { + self.duration_ms + } + + pub fn platform(&self) -> &ProcessPlatform { + &self.platform + } + + pub fn output_failures(&self) -> &[String] { + &self.output_failures + } + + pub fn is_success(&self) -> bool { + self.termination.is_success() && self.output_failures.is_empty() + } + + pub fn ensure_success(&self) -> Result<(), ProcessError> { + if self.is_success() { + Ok(()) + } else { + Err(ProcessError::Unsuccessful(self.failure_message())) + } + } + + pub fn failure_message(&self) -> String { + let mut message = match self.termination { + ProcessTermination::Exited { code } => { + format!("Command `{}` exited with code {code}", self.command_display) + } + ProcessTermination::Signaled => { + format!("Command `{}` was terminated by a signal", self.command_display) + } + ProcessTermination::TimedOut => { + format!("Command `{}` timed out", self.command_display) + } + ProcessTermination::Cancelled => { + format!("Command `{}` was cancelled", self.command_display) + } + }; + + let stderr = self.stderr.diagnostic_text(); + if !stderr.is_empty() { + message.push_str("\nStderr: "); + message.push_str(&stderr); + } + if !self.output_failures.is_empty() { + message.push_str("\nOutput validation: "); + message.push_str(&self.output_failures.join("; ")); + } + message + } +} + +impl fmt::Debug for ProcessResult { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProcessResult") + .field("command", &self.command_display) + .field("termination", &self.termination) + .field("stdout", &self.stdout) + .field("stderr", &self.stderr) + .field("started_at_epoch_ms", &self.started_at_epoch_ms) + .field("duration_ms", &self.duration_ms) + .field("platform", &self.platform) + .field("output_failures", &self.output_failures) + .finish() + } +} + +/// Stable tool-probe classification. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolProbeStatus { + Available, + Missing, + Failed, + TimedOut, +} + +/// Version-probe evidence for tool discovery/provenance consumers. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ToolProbeEvidence { + pub executable: String, + pub status: ToolProbeStatus, + pub version_line: Option, + pub duration_ms: u64, + pub platform: ProcessPlatform, + pub diagnostic: Option, +} + +impl ToolProbeEvidence { + pub fn is_available(&self) -> bool { + self.status == ToolProbeStatus::Available + } +} + +/// Errors raised by process policy, launch, I/O, or checked execution. +#[derive(Debug)] +pub enum ProcessError { + MissingExecutable { executable: String }, + Launch { executable: String, message: String }, + ShellRequiresOptIn { executable: String }, + PolicyRejected(String), + Io(String), + Unsuccessful(String), +} + +impl fmt::Display for ProcessError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::MissingExecutable { executable } => write!( + f, + "`{executable}` was not found. Make sure it is installed and available in PATH." + ), + Self::Launch { + executable, + message, + } => write!(f, "Failed to launch `{executable}`: {message}"), + Self::ShellRequiresOptIn { executable } => write!( + f, + "Shell executable `{executable}` requires explicit ProcessRequest::shell(...) opt-in" + ), + Self::PolicyRejected(message) => write!(f, "Process policy rejected request: {message}"), + Self::Io(message) => f.write_str(message), + Self::Unsuccessful(message) => f.write_str(message), + } + } +} + +impl std::error::Error for ProcessError {} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct OutputSnapshot { + exists: bool, + is_file: bool, + is_directory: bool, + len: Option, + modified: Option, +} + +impl OutputSnapshot { + fn capture(path: &Path) -> Self { + match fs::metadata(path) { + Ok(metadata) => Self { + exists: true, + is_file: metadata.is_file(), + is_directory: metadata.is_dir(), + len: metadata.is_file().then_some(metadata.len()), + modified: metadata.modified().ok(), + }, + Err(_) => Self { + exists: false, + is_file: false, + is_directory: false, + len: None, + modified: None, + }, + } + } +} + +impl ProcessExpectedOutput { + fn validate(&self, before: &OutputSnapshot) -> Option { + let after = OutputSnapshot::capture(&self.path); + if !after.exists { + return Some(format!("expected output '{}' was not produced", self.path.display())); + } + match self.kind { + ExpectedOutputKind::Any => {} + ExpectedOutputKind::File if !after.is_file => { + return Some(format!("expected output '{}' is not a file", self.path.display())); + } + ExpectedOutputKind::Directory if !after.is_directory => { + return Some(format!( + "expected output '{}' is not a directory", + self.path.display() + )); + } + _ => {} + } + if self.require_non_empty && after.is_file && after.len == Some(0) { + return Some(format!("expected output '{}' is empty", self.path.display())); + } + if self.require_change && &after == before { + return Some(format!( + "expected output '{}' was not changed by the process", + self.path.display() + )); + } + None + } +} + +#[derive(Default)] +struct Redactor { + secrets: Vec, +} + +impl Redactor { + fn new(values: Vec) -> Self { + let mut redactor = Self::default(); + redactor.extend(values); + redactor + } + + fn extend(&mut self, values: I) + where + I: IntoIterator, + { + self.secrets + .extend(values.into_iter().filter(|value| !value.is_empty())); + self.secrets.sort_by_key(|value| std::cmp::Reverse(value.len())); + self.secrets.dedup(); + } + + fn redact(&self, text: &str) -> String { + let mut redacted = redact_url_credentials(text); + for secret in &self.secrets { + redacted = redacted.replace(secret, "[REDACTED]"); + } + redact_bearer_tokens(&redacted) + } +} + +#[derive(Debug)] +struct BoundedBytes { + bytes: Vec, + total_bytes: u64, + truncated: bool, +} + +fn spawn_bounded_reader(reader: R, max_bytes: usize) -> thread::JoinHandle> +where + R: Read + Send + 'static, +{ + thread::spawn(move || drain_bounded(reader, max_bytes)) +} + +fn drain_bounded(mut reader: R, max_bytes: usize) -> io::Result { + let mut bytes = Vec::with_capacity(max_bytes.min(8 * 1024)); + let mut total_bytes = 0_u64; + let mut buffer = [0_u8; 8 * 1024]; + loop { + let read = reader.read(&mut buffer)?; + if read == 0 { + break; + } + total_bytes = total_bytes.saturating_add(read as u64); + if bytes.len() < max_bytes { + let remaining = max_bytes - bytes.len(); + bytes.extend_from_slice(&buffer[..read.min(remaining)]); + } + } + Ok(BoundedBytes { + truncated: total_bytes > bytes.len() as u64, + bytes, + total_bytes, + }) +} + +fn join_bounded_reader( + handle: Option>>, + stream: &str, +) -> Result { + match handle { + Some(handle) => match handle.join() { + Ok(Ok(bytes)) => Ok(bytes), + Ok(Err(error)) => Err(ProcessError::Io(format!( + "failed to capture process {stream}: {error}" + ))), + Err(_) => Err(ProcessError::Io(format!( + "process {stream} reader thread panicked" + ))), + }, + None => Ok(BoundedBytes { + bytes: Vec::new(), + total_bytes: 0, + truncated: false, + }), + } +} + +fn stdio_for_output(mode: ProcessOutputMode) -> Stdio { + match mode { + ProcessOutputMode::Null => Stdio::null(), + ProcessOutputMode::Inherit => Stdio::inherit(), + ProcessOutputMode::Capture { .. } => Stdio::piped(), + } +} + +fn termination_from_status(status: ExitStatus) -> ProcessTermination { + match status.code() { + Some(code) => ProcessTermination::Exited { code }, + None => ProcessTermination::Signaled, + } +} + +fn terminate_process_tree(child: &mut Child, mode: ProcessTreeMode) -> io::Result<()> { + if child.try_wait()?.is_some() { + return Ok(()); + } + + if mode == ProcessTreeMode::ChildOnly { + let _ = child.kill(); + let _ = child.wait(); + return Ok(()); + } + + #[cfg(unix)] + { + let pid = child.id() as i32; + unsafe extern "C" { + fn kill(pid: i32, signal: i32) -> i32; + } + const SIGTERM: i32 = 15; + const SIGKILL: i32 = 9; + + let term_result = unsafe { kill(-pid, SIGTERM) }; + if term_result != 0 { + let _ = child.kill(); + } + let deadline = Instant::now() + TERMINATION_GRACE; + while Instant::now() < deadline { + if child.try_wait()?.is_some() { + return Ok(()); + } + thread::sleep(POLL_INTERVAL); + } + let _ = unsafe { kill(-pid, SIGKILL) }; + let _ = child.kill(); + let _ = child.wait(); + Ok(()) + } + + #[cfg(windows)] + { + let pid = child.id().to_string(); + let taskkill_succeeded = Command::new("taskkill") + .args(["/PID", &pid, "/T", "/F"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_ok_and(|status| status.success()); + if !taskkill_succeeded { + let _ = child.kill(); + } + let _ = child.wait(); + return Ok(()); + } + + #[cfg(not(any(unix, windows)))] + { + let _ = child.kill(); + let _ = child.wait(); + Ok(()) + } +} + +fn safe_argument_display(args: &[ProcessArgument], redactor: &Redactor) -> Vec { + let mut safe = Vec::with_capacity(args.len()); + let mut redact_next = false; + for argument in args { + let raw = argument.value(); + let rendered = if argument.sensitive || redact_next { + "[REDACTED]".to_string() + } else if let Some((name, _value)) = raw.split_once('=') { + if is_sensitive_name(name) { + format!("{name}=[REDACTED]") + } else { + redactor.redact(raw) + } + } else if raw.to_ascii_lowercase().starts_with("authorization:") { + "Authorization: [REDACTED]".to_string() + } else { + redactor.redact(raw) + }; + redact_next = !argument.sensitive && !raw.contains('=') && is_sensitive_name(raw); + safe.push(rendered); + } + safe +} + +fn safe_command_display(executable: &str, args: &[String]) -> String { + if args.is_empty() { + executable.to_string() + } else { + format!("{} {}", executable, args.join(" ")) + } +} + +fn normalize_env_name(name: &str) -> String { + name.to_ascii_uppercase() +} + +fn is_sensitive_name(name: &str) -> bool { + let normalized = name + .trim_start_matches('-') + .replace('-', "_") + .to_ascii_uppercase(); + normalized.contains("TOKEN") + || normalized.contains("SECRET") + || normalized.contains("PASSWORD") + || normalized.contains("PASSWD") + || normalized.contains("API_KEY") + || normalized.contains("APIKEY") + || normalized.contains("CREDENTIAL") + || normalized.contains("PRIVATE_KEY") + || normalized == "AUTHORIZATION" + || normalized.ends_with("_AUTH") +} + +/// Return `true` when an explicitly named shell is being asked to interpret a +/// command string. Wrappers use this to opt in visibly instead of smuggling a +/// shell through the direct-argv path. +pub(crate) fn is_explicit_shell_invocation(executable: &str, args: &[String]) -> bool { + let basename = Path::new(executable) + .file_name() + .and_then(|value| value.to_str()) + .unwrap_or(executable) + .to_ascii_lowercase(); + let is_shell = matches!( + basename.as_str(), + "sh" + | "bash" + | "zsh" + | "dash" + | "ksh" + | "fish" + | "cmd" + | "cmd.exe" + | "powershell" + | "powershell.exe" + | "pwsh" + | "pwsh.exe" + ); + if !is_shell { + return false; + } + args.iter().any(|argument| { + argument == "-c" + || argument == "-Command" + || argument == "-command" + || argument == "/C" + || argument == "/c" + }) +} + +fn redact_url_credentials(text: &str) -> String { + let Some(scheme_index) = text.find("://") else { + return text.to_string(); + }; + let authority_start = scheme_index + 3; + let authority_end = text[authority_start..] + .find(['/', ' ', '\n', '\r', '\t']) + .map(|offset| authority_start + offset) + .unwrap_or(text.len()); + let authority = &text[authority_start..authority_end]; + let Some(at_index) = authority.rfind('@') else { + return text.to_string(); + }; + if !authority[..at_index].contains(':') { + return text.to_string(); + } + format!( + "{}[REDACTED]@{}{}", + &text[..authority_start], + &authority[at_index + 1..], + &text[authority_end..] + ) +} + +fn redact_bearer_tokens(text: &str) -> String { + let mut result = String::with_capacity(text.len()); + let mut remainder = text; + loop { + let lower = remainder.to_ascii_lowercase(); + let Some(index) = lower.find("bearer ") else { + result.push_str(remainder); + break; + }; + result.push_str(&remainder[..index]); + let token_start = index + "bearer ".len(); + result.push_str(&remainder[index..token_start]); + result.push_str("[REDACTED]"); + let token_end = remainder[token_start..] + .find(char::is_whitespace) + .map(|offset| token_start + offset) + .unwrap_or(remainder.len()); + remainder = &remainder[token_end..]; + } + result +} + +fn first_non_empty_line(text: &str) -> Option<&str> { + text.lines().map(str::trim).find(|line| !line.is_empty()) +} + +fn epoch_millis() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_millis() as u64) + .unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_executable_is_classified() { + let error = ProcessExecutor::new() + .execute(ProcessRequest::direct( + "__renderflow_process_executor_missing_tool__", + )) + .unwrap_err(); + assert!(matches!(error, ProcessError::MissingExecutable { .. })); + } + + #[cfg(unix)] + #[test] + fn non_zero_exit_is_structured() { + let result = ProcessExecutor::new() + .execute(ProcessRequest::direct("false")) + .unwrap(); + assert_eq!(result.termination(), ProcessTermination::Exited { code: 1 }); + assert!(!result.is_success()); + } + + #[cfg(unix)] + #[test] + fn timeout_terminates_process() { + let started = Instant::now(); + let result = ProcessExecutor::new() + .execute( + ProcessRequest::shell("sh") + .args(["-c", "sleep 5"]) + .timeout(Duration::from_millis(75)), + ) + .unwrap(); + assert_eq!(result.termination(), ProcessTermination::TimedOut); + assert!(started.elapsed() < Duration::from_secs(2)); + } + + #[cfg(unix)] + #[test] + fn cancellation_terminates_process_tree() { + let directory = tempfile::tempdir().unwrap(); + let marker = directory.path().join("descendant-finished"); + let script = format!("(sleep 1; touch '{}') & wait", marker.display()); + let token = ProcessCancellationToken::new(); + let cancel = token.clone(); + thread::spawn(move || { + thread::sleep(Duration::from_millis(75)); + cancel.cancel(); + }); + + let result = ProcessExecutor::new() + .execute( + ProcessRequest::shell("sh") + .args(["-c", script.as_str()]) + .cancellation(token) + .timeout(Duration::from_secs(3)), + ) + .unwrap(); + assert_eq!(result.termination(), ProcessTermination::Cancelled); + thread::sleep(Duration::from_millis(1100)); + assert!( + !marker.exists(), + "a descendant survived process-tree cancellation" + ); + } + + #[cfg(unix)] + #[test] + fn capture_is_bounded_and_reports_truncation() { + let result = ProcessExecutor::new() + .execute_checked( + ProcessRequest::shell("sh") + .args(["-c", "printf '0123456789abcdef'"]) + .capture_limit(8), + ) + .unwrap(); + assert_eq!(result.stdout().bytes(), b"01234567"); + assert_eq!(result.stdout().total_bytes(), 16); + assert!(result.stdout().truncated()); + } + + #[cfg(unix)] + #[test] + fn secret_values_are_redacted_from_debug_and_errors() { + let secret = "super-secret-value-123"; + let result = ProcessExecutor::new() + .execute( + ProcessRequest::shell("sh") + .args(["-c", "printf '%s' \"$RF_SECRET\" >&2; exit 7"]) + .environment(ProcessEnvironment::clear().set_sensitive("RF_SECRET", secret)) + .redact_value(secret), + ) + .unwrap(); + let error = result.ensure_success().unwrap_err().to_string(); + let debug = format!("{result:?}"); + assert!(!error.contains(secret)); + assert!(!debug.contains(secret)); + assert!(error.contains("[REDACTED]")); + } + + #[cfg(unix)] + #[test] + fn invalid_expected_output_fails_checked_execution() { + let directory = tempfile::tempdir().unwrap(); + let output = directory.path().join("missing.out"); + let error = ProcessExecutor::new() + .execute_checked( + ProcessRequest::direct("true") + .expect_output(ProcessExpectedOutput::file(&output)), + ) + .unwrap_err(); + assert!(error.to_string().contains("was not produced")); + } + + #[cfg(unix)] + #[test] + fn direct_shell_requires_explicit_opt_in() { + let error = ProcessExecutor::new() + .execute(ProcessRequest::direct("sh").args(["-c", "true"])) + .unwrap_err(); + assert!(matches!(error, ProcessError::ShellRequiresOptIn { .. })); + } + + #[test] + fn filtered_environment_detects_secret_names() { + assert!(is_sensitive_name("GITHUB_TOKEN")); + assert!(is_sensitive_name("OPENAI_API_KEY")); + assert!(!is_sensitive_name("PATH")); + assert!(!is_sensitive_name("HOME")); + } + + #[test] + fn bearer_tokens_and_url_credentials_are_redacted() { + let redactor = Redactor::default(); + assert_eq!( + redactor.redact("Authorization: Bearer abc123"), + "Authorization: Bearer [REDACTED]" + ); + assert_eq!( + redactor.redact("https://user:password@example.com/path"), + "https://[REDACTED]@example.com/path" + ); + } + + #[cfg(unix)] + #[test] + fn version_probe_captures_tool_evidence() { + let probe = ProcessExecutor::new().probe_version("rustc"); + assert!(probe.is_available()); + assert!(probe + .version_line + .as_deref() + .is_some_and(|line| line.contains("rustc"))); + } +} diff --git a/crates/renderflow-core/src/strategies/pdf.rs b/crates/renderflow-core/src/strategies/pdf.rs index 1111703..4807e9e 100644 --- a/crates/renderflow-core/src/strategies/pdf.rs +++ b/crates/renderflow-core/src/strategies/pdf.rs @@ -1,9 +1,9 @@ use anyhow::{Context, Result}; -use std::io::ErrorKind; use std::path::Path; use tracing::info; use crate::adapters::command::run_command; +use crate::process::{ProcessExecutor, ToolProbeStatus}; use crate::strategies::{OutputStrategy, PandocArgs, RenderContext}; /// Renders a document to PDF format using pandoc with the tectonic PDF engine. @@ -20,21 +20,28 @@ impl PdfStrategy { } } - /// Returns an error if the tectonic PDF engine is not installed. + /// Returns an error if the tectonic PDF engine is not installed or cannot + /// be version-probed through the canonical process executor. fn check_tectonic() -> Result<()> { - match std::process::Command::new("tectonic") - .arg("--version") - .output() - { - Err(e) if e.kind() == ErrorKind::NotFound => { - anyhow::bail!( - "PDF rendering failed: `tectonic` is not installed.\n\n\ - Fix:\n\ - - Install tectonic: https://tectonic-typesetting.github.io/en-US/\n\ - - Or configure a different PDF engine" - ); - } - _ => Ok(()), + let probe = ProcessExecutor::new().probe_version("tectonic"); + match probe.status { + ToolProbeStatus::Available => Ok(()), + ToolProbeStatus::Missing => anyhow::bail!( + "PDF rendering failed: `tectonic` is not installed.\n\n\ + Fix:\n\ + - Install tectonic: https://tectonic-typesetting.github.io/en-US/\n\ + - Or configure a different PDF engine" + ), + ToolProbeStatus::TimedOut => anyhow::bail!( + "PDF rendering failed: `tectonic --version` timed out. \ + Verify the tectonic installation before retrying." + ), + ToolProbeStatus::Failed => anyhow::bail!( + "PDF rendering failed: tectonic is installed but its version probe failed: {}", + probe + .diagnostic + .unwrap_or_else(|| "unknown process failure".to_string()) + ), } } } @@ -49,13 +56,11 @@ impl OutputStrategy for PdfStrategy { Self::check_tectonic()?; - // Resolve the optional template to a file path within the template directory. let template_path = if let Some(ref name) = self.template { let path = Path::new(&self.template_dir).join(name); if !path.exists() { anyhow::bail!( - "Template file not found: '{}'. \ - Ensure the template exists in the configured template directory.", + "Template file not found: '{}'. Ensure the template exists in the configured template directory.", path.display() ); } @@ -86,12 +91,11 @@ impl OutputStrategy for PdfStrategy { let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); run_command("pandoc", &args_refs) - .with_context(|| format!( - "Failed to render PDF output '{}'. \ - Check that pandoc and tectonic are installed (`pandoc --version`, `tectonic --version`) \ - and that the input file '{}' is valid Markdown.", - ctx.output_path, ctx.input_path - ))?; + .with_context(|| format!( + "Failed to render PDF output '{}'. Check that pandoc and tectonic are installed \ + (`pandoc --version`, `tectonic --version`) and that the input file '{}' is valid Markdown.", + ctx.output_path, ctx.input_path + ))?; info!(output = %ctx.output_path, "PDF rendering completed successfully"); Ok(()) } @@ -117,13 +121,8 @@ mod tests { } } - /// Returns `true` if the `tectonic` binary is available in PATH. fn tectonic_available() -> bool { - std::process::Command::new("tectonic") - .arg("--version") - .output() - .map(|o| o.status.success()) - .unwrap_or(false) + ProcessExecutor::new().probe_version("tectonic").is_available() } #[test] @@ -134,7 +133,6 @@ mod tests { let result = strategy.render(&ctx); assert!(result.is_err()); let msg = format!("{:#}", result.unwrap_err()); - // The error is either a missing-tectonic error or a pandoc render error. assert!( msg.contains("tectonic") || msg.contains("Failed to render PDF output"), "error should describe what failed: {}", @@ -145,15 +143,14 @@ mod tests { #[test] fn test_check_tectonic_returns_clear_error_when_missing() { if tectonic_available() { - // Nothing to test when tectonic is present. return; } let result = PdfStrategy::check_tectonic(); assert!(result.is_err()); let msg = result.unwrap_err().to_string(); assert!( - msg.contains("tectonic") && msg.contains("not installed"), - "error should explain that tectonic is not installed: {}", + msg.contains("tectonic"), + "error should explain the tectonic problem: {}", msg ); } @@ -166,8 +163,6 @@ mod tests { #[test] fn test_pdf_strategy_no_template_does_not_check_template_dir() { - // When no template is configured the template_dir is never accessed, - // so a non-existent directory must not cause an error at construction time. let strategy = PdfStrategy::new(None, "/nonexistent/dir".to_string()); assert!(strategy.template.is_none()); } @@ -185,14 +180,10 @@ mod tests { assert_eq!(ctx.input_format, InputFormat::Rst); } - /// Verifies that `dry_run = true` causes the strategy to skip pandoc and - /// tectonic checks entirely and return `Ok(())`. #[test] fn test_pdf_strategy_dry_run_skips_execution() { let vars = HashMap::new(); let strategy = PdfStrategy::new(None, "templates".to_string()); - // Use a non-existent input path; real pandoc/tectonic calls would fail, - // but dry-run must succeed without invoking any external tools. let ctx = RenderContext { input_path: "/nonexistent/input.md", input_format: InputFormat::Markdown, @@ -200,15 +191,9 @@ mod tests { variables: &vars, dry_run: true, }; - let result = strategy.render(&ctx); - assert!( - result.is_ok(), - "dry-run should succeed without invoking pandoc or tectonic: {:?}", - result - ); + assert!(strategy.render(&ctx).is_ok()); } - /// Verifies that `dry_run = true` skips even template validation. #[test] fn test_pdf_strategy_dry_run_with_missing_template_skips_execution() { let vars = HashMap::new(); @@ -223,12 +208,7 @@ mod tests { variables: &vars, dry_run: true, }; - let result = strategy.render(&ctx); - assert!( - result.is_ok(), - "dry-run should succeed even with a missing template: {:?}", - result - ); + assert!(strategy.render(&ctx).is_ok()); } #[test] @@ -239,10 +219,8 @@ mod tests { let mut input = NamedTempFile::new().unwrap(); writeln!(input, "# Hello\n\nThis is a test.").unwrap(); - let output = NamedTempFile::new().unwrap(); let output_path = output.path().with_extension("pdf"); - let vars = HashMap::new(); let strategy = PdfStrategy::new(None, "templates".to_string()); let ctx = RenderContext { @@ -252,8 +230,7 @@ mod tests { variables: &vars, dry_run: false, }; - let result = strategy.render(&ctx); - assert!(result.is_ok()); + assert!(strategy.render(&ctx).is_ok()); assert!(output_path.exists()); } } diff --git a/crates/renderflow-core/src/transforms/aggregation.rs b/crates/renderflow-core/src/transforms/aggregation.rs index 91193e2..cc6096e 100644 --- a/crates/renderflow-core/src/transforms/aggregation.rs +++ b/crates/renderflow-core/src/transforms/aggregation.rs @@ -3,114 +3,48 @@ // available for callers embedding renderflow as a library. use std::collections::HashMap; -use std::process::Stdio; use anyhow::{Context, Result}; use tracing::{debug, info}; +use crate::process::{ + is_explicit_shell_invocation, ProcessExpectedOutput, ProcessExecutor, ProcessInput, + ProcessOutputMode, ProcessRequest, DEFAULT_CAPTURE_LIMIT_BYTES, DEFAULT_PROCESS_TIMEOUT, +}; + /// A transform that consumes an ordered collection of inputs and produces a /// single aggregated output artifact. -/// -/// Unlike [`Transform`](super::Transform), which operates on a single input -/// string, an `AggregationTransform` is designed for edition-level workflows -/// such as combining multiple page images into a single CBZ archive or PDF -/// document. -/// -/// # Example -/// -/// ```rust -/// use renderflow::transforms::aggregation::{AggregationRegistry, AggregationTransform}; -/// use anyhow::Result; -/// -/// struct JoinLines; -/// impl AggregationTransform for JoinLines { -/// fn name(&self) -> &str { "join-lines" } -/// fn aggregate(&self, inputs: &[&str], output_path: &str) -> Result<()> { -/// std::fs::write(output_path, inputs.join("\n"))?; -/// Ok(()) -/// } -/// } -/// -/// let mut registry = AggregationRegistry::new(); -/// registry.register(Box::new(JoinLines)); -/// ``` pub trait AggregationTransform: Send + Sync { - /// Human-readable name for this transform, used in log messages and - /// error context. fn name(&self) -> &str { "AggregationTransform" } - /// Aggregate the ordered `inputs` and write the result to `output_path`. - /// - /// `inputs` is an ordered slice of strings. For file-based aggregation - /// (e.g. images to CBZ) the strings are file paths; for text-based - /// aggregation they are document content strings. - /// - /// The transform is responsible for writing its output to `output_path`. - /// - /// # Errors - /// - /// Returns an error when the aggregation fails, the external command - /// cannot be started, or the output cannot be written. + /// Aggregate ordered input paths and write the result to `output_path`. fn aggregate(&self, inputs: &[&str], output_path: &str) -> Result<()>; } -/// A registry of named [`AggregationTransform`] implementations. -/// -/// Transforms are stored by name and can be looked up and applied by name. -/// This registry is used to select the correct aggregation strategy for a -/// given collection-based DAG edge (identified by its -/// [`label`](crate::graph::TransformDefinition::label)). -/// -/// # Example -/// -/// ```rust -/// use renderflow::transforms::aggregation::{AggregationRegistry, CommandAggregationTransform}; -/// -/// let mut registry = AggregationRegistry::new(); -/// registry.register(Box::new(CommandAggregationTransform::cbz("pages-to-cbz"))); -/// registry.register(Box::new(CommandAggregationTransform::images_to_pdf("images-to-pdf"))); -/// ``` +/// Registry of named collection transforms. pub struct AggregationRegistry { transforms: HashMap>, } impl AggregationRegistry { - /// Create an empty registry. pub fn new() -> Self { Self { transforms: HashMap::new(), } } - /// Register a named aggregation transform. - /// - /// If a transform with the same name is already registered, it is - /// replaced by the new one. pub fn register(&mut self, transform: Box) -> &mut Self { let name = transform.name().to_string(); self.transforms.insert(name, transform); self } - /// Look up a transform by name. - /// - /// Returns `None` when no transform with the given `name` has been - /// registered. pub fn get(&self, name: &str) -> Option<&dyn AggregationTransform> { - self.transforms.get(name).map(|t| t.as_ref()) + self.transforms.get(name).map(|transform| transform.as_ref()) } - /// Apply the named aggregation transform to the ordered `inputs`, - /// writing the result to `output_path`. - /// - /// # Errors - /// - /// Returns an error when: - /// * no transform with `name` is registered, or - /// * the transform's [`aggregate`](AggregationTransform::aggregate) call - /// fails. pub fn apply(&self, name: &str, inputs: &[&str], output_path: &str) -> Result<()> { let transform = self.get(name).ok_or_else(|| { anyhow::anyhow!("Aggregation transform '{}' not found in registry", name) @@ -133,35 +67,12 @@ impl Default for AggregationRegistry { } } -/// An aggregation transform backed by an external command. -/// -/// The command is invoked with a processed argument list derived from `args`. -/// Two placeholders are supported: +/// Collection transform backed by an external executable. /// -/// | Placeholder | Expansion | -/// |--------------|------------------------------------------------------------------| -/// | `{inputs}` | Standalone: one argument per input path. Embedded in a larger | -/// | | string: all paths joined by a single space. | -/// | `{output}` | Replaced with the `output_path` supplied to [`aggregate`]. | -/// -/// When neither placeholder appears in `args`, all inputs are joined with -/// newlines and piped to the command's `stdin`; `stdout` is ignored and the -/// command is expected to write its output directly (e.g. via shell -/// redirection captured in a wrapper arg). -/// -/// # Example -/// -/// ```rust -/// use renderflow::transforms::aggregation::{AggregationTransform, CommandAggregationTransform}; -/// -/// // Equivalent to: zip -j output.cbz page1.jpg page2.jpg -/// let t = CommandAggregationTransform::new( -/// "images-to-cbz", -/// "zip", -/// vec!["-j".to_string(), "{output}".to_string(), "{inputs}".to_string()], -/// ); -/// assert_eq!(t.name(), "images-to-cbz"); -/// ``` +/// `{inputs}` expands to one argv entry per input when it is a standalone +/// argument, or a space-joined string when embedded. `{output}` expands to the +/// declared output path. Explicit shell programs remain visibly classified as +/// shell invocations by the canonical process executor. pub struct CommandAggregationTransform { name: String, program: String, @@ -169,11 +80,6 @@ pub struct CommandAggregationTransform { } impl CommandAggregationTransform { - /// Create a new `CommandAggregationTransform`. - /// - /// * `name` – human-readable identifier used in logs and registry lookups. - /// * `program` – executable to invoke (looked up on `PATH`). - /// * `args` – argument list; may include `{inputs}` and `{output}` placeholders. pub fn new(name: impl Into, program: impl Into, args: Vec) -> Self { Self { name: name.into(), @@ -182,13 +88,6 @@ impl CommandAggregationTransform { } } - /// Build a **CBZ** aggregation transform. - /// - /// Uses the system `zip` command to package ordered image files into a - /// Comic Book ZIP (`.cbz`) archive. The `-j` flag strips directory - /// components so that all images appear at the archive root. - /// - /// Generated command: `zip -j {output} {inputs}` pub fn cbz(name: impl Into) -> Self { Self::new( name, @@ -201,12 +100,6 @@ impl CommandAggregationTransform { ) } - /// Build an **images-to-PDF** aggregation transform. - /// - /// Uses `img2pdf` to losslessly combine ordered image files into a PDF - /// document, preserving the original image data without re-encoding. - /// - /// Generated command: `img2pdf --output {output} {inputs}` pub fn images_to_pdf(name: impl Into) -> Self { Self::new( name, @@ -219,17 +112,6 @@ impl CommandAggregationTransform { ) } - /// Build a **TIFF-to-press-PDF** aggregation transform. - /// - /// Uses Ghostscript (`gs`) to combine TIFF source files into a - /// press-quality PDF document (PDF settings: `/press`), optimised for - /// high-resolution print output. - /// - /// Generated command: - /// ```text - /// gs -dBATCH -dNOPAUSE -sDEVICE=pdfwrite -dPDFSETTINGS=/press - /// -sOutputFile={output} {inputs} - /// ``` pub fn tiff_to_press_pdf(name: impl Into) -> Self { Self::new( name, @@ -267,16 +149,14 @@ impl AggregationTransform for CommandAggregationTransform { "Running aggregation transform" ); - // Expand placeholders in each argument. + let has_inputs_placeholder = self.args.iter().any(|arg| arg.contains("{inputs}")); let mut processed_args: Vec = Vec::new(); for arg in &self.args { if arg == "{inputs}" { - // Standalone placeholder: one argument per input path. - processed_args.extend(inputs.iter().map(|s| s.to_string())); + processed_args.extend(inputs.iter().map(|value| (*value).to_string())); } else { let mut processed = arg.clone(); if processed.contains("{inputs}") { - // Embedded placeholder: join all paths with a single space. processed = processed.replace("{inputs}", &inputs.join(" ")); } if processed.contains("{output}") { @@ -286,27 +166,25 @@ impl AggregationTransform for CommandAggregationTransform { } } - let cmd_output = std::process::Command::new(&self.program) - .args(&processed_args) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .with_context(|| format!("Failed to start aggregation command '{}'", self.program))? - .wait_with_output() - .with_context(|| { - format!("Failed to wait for aggregation command '{}'", self.program) - })?; - - if !cmd_output.status.success() { - let stderr = String::from_utf8_lossy(&cmd_output.stderr); - anyhow::bail!( - "Aggregation command '{}' exited with status {}: {}", - self.program, - cmd_output.status, - stderr.trim() - ); + let request = if is_explicit_shell_invocation(&self.program, &processed_args) { + ProcessRequest::shell(&self.program) + } else { + ProcessRequest::direct(&self.program) } + .args(processed_args) + .stdin(if has_inputs_placeholder { + ProcessInput::Null + } else { + ProcessInput::Bytes(inputs.join("\n").into_bytes()) + }) + .stdout(ProcessOutputMode::capture(DEFAULT_CAPTURE_LIMIT_BYTES)) + .stderr(ProcessOutputMode::capture(DEFAULT_CAPTURE_LIMIT_BYTES)) + .timeout(DEFAULT_PROCESS_TIMEOUT) + .expect_output(ProcessExpectedOutput::file(output_path).require_change()); + + ProcessExecutor::new() + .execute_checked(request) + .with_context(|| format!("Aggregation command '{}' failed", self.program))?; info!( transform = %self.name, @@ -321,14 +199,12 @@ impl AggregationTransform for CommandAggregationTransform { mod tests { use super::*; - // ── helpers ─────────────────────────────────────────────────────────────── - - /// A simple aggregation transform that joins inputs with newlines. struct JoinTransform; impl AggregationTransform for JoinTransform { fn name(&self) -> &str { "join" } + fn aggregate(&self, inputs: &[&str], output_path: &str) -> Result<()> { std::fs::write(output_path, inputs.join("\n")) .context("JoinTransform: failed to write output")?; @@ -341,13 +217,12 @@ mod tests { fn name(&self) -> &str { "always-fails" } + fn aggregate(&self, _inputs: &[&str], _output_path: &str) -> Result<()> { anyhow::bail!("intentional failure") } } - // ── AggregationRegistry ─────────────────────────────────────────────────── - #[test] fn test_registry_empty_get_returns_none() { let registry = AggregationRegistry::new(); @@ -377,55 +252,9 @@ mod tests { registry.register(Box::new(AlwaysFails)); let dir = tempfile::tempdir().unwrap(); let out = dir.path().join("out.txt"); - let result = registry.apply("always-fails", &["a"], out.to_str().unwrap()); - assert!(result.is_err()); - } - - #[test] - fn test_registry_apply_succeeds_and_writes_output() { - let mut registry = AggregationRegistry::new(); - registry.register(Box::new(JoinTransform)); - let dir = tempfile::tempdir().unwrap(); - let out = dir.path().join("out.txt"); - registry - .apply("join", &["first", "second", "third"], out.to_str().unwrap()) - .unwrap(); - let content = std::fs::read_to_string(&out).unwrap(); - assert_eq!(content, "first\nsecond\nthird"); - } - - #[test] - fn test_registry_register_replaces_existing() { - struct WriteA; - impl AggregationTransform for WriteA { - fn name(&self) -> &str { - "writer" - } - fn aggregate(&self, _: &[&str], p: &str) -> Result<()> { - std::fs::write(p, "a")?; - Ok(()) - } - } - struct WriteB; - impl AggregationTransform for WriteB { - fn name(&self) -> &str { - "writer" - } - fn aggregate(&self, _: &[&str], p: &str) -> Result<()> { - std::fs::write(p, "b")?; - Ok(()) - } - } - - let mut registry = AggregationRegistry::new(); - registry.register(Box::new(WriteA)); - registry.register(Box::new(WriteB)); - let dir = tempfile::tempdir().unwrap(); - let out = dir.path().join("out.txt"); - registry - .apply("writer", &["ignored"], out.to_str().unwrap()) - .unwrap(); - assert_eq!(std::fs::read_to_string(&out).unwrap(), "b"); + assert!(registry + .apply("always-fails", &["a"], out.to_str().unwrap()) + .is_err()); } #[test] @@ -437,28 +266,21 @@ mod tests { registry .apply("join", &["page1", "page2", "page3"], out.to_str().unwrap()) .unwrap(); - let content = std::fs::read_to_string(&out).unwrap(); - let pos1 = content.find("page1").expect("page1 missing"); - let pos2 = content.find("page2").expect("page2 missing"); - let pos3 = content.find("page3").expect("page3 missing"); - assert!(pos1 < pos2, "page1 must come before page2"); - assert!(pos2 < pos3, "page2 must come before page3"); + assert_eq!(std::fs::read_to_string(&out).unwrap(), "page1\npage2\npage3"); } - // ── CommandAggregationTransform ─────────────────────────────────────────── - #[test] fn test_command_aggregation_name_stored() { - let t = CommandAggregationTransform::new("my-agg", "echo", vec![]); - assert_eq!(t.name(), "my-agg"); + let transform = CommandAggregationTransform::new("my-agg", "echo", vec![]); + assert_eq!(transform.name(), "my-agg"); } #[test] fn test_command_aggregation_empty_inputs_returns_error() { - let t = CommandAggregationTransform::new("test", "echo", vec![]); + let transform = CommandAggregationTransform::new("test", "echo", vec![]); let dir = tempfile::tempdir().unwrap(); let out = dir.path().join("out.txt"); - let result = t.aggregate(&[], out.to_str().unwrap()); + let result = transform.aggregate(&[], out.to_str().unwrap()); assert!(result.is_err()); assert!(result .unwrap_err() @@ -468,47 +290,47 @@ mod tests { #[test] fn test_command_aggregation_invalid_program_returns_error() { - let t = CommandAggregationTransform::new( + let transform = CommandAggregationTransform::new( "bad-program", "__nonexistent_program__", vec!["{inputs}".to_string()], ); let dir = tempfile::tempdir().unwrap(); let out = dir.path().join("out.txt"); - let result = t.aggregate(&["a"], out.to_str().unwrap()); - assert!(result.is_err()); + assert!(transform + .aggregate(&["a"], out.to_str().unwrap()) + .is_err()); } + #[cfg(unix)] #[test] fn test_command_aggregation_inputs_embedded_placeholder() { let dir = tempfile::tempdir().unwrap(); let out = dir.path().join("out.txt"); - let in1 = dir.path().join("a.txt"); let in2 = dir.path().join("b.txt"); std::fs::write(&in1, "aaa").unwrap(); std::fs::write(&in2, "bbb").unwrap(); - // {inputs} embedded in a shell -c argument is space-joined. - let t = CommandAggregationTransform::new( + let transform = CommandAggregationTransform::new( "cat-agg", "sh", vec!["-c".to_string(), "cat {inputs} > {output}".to_string()], ); - t.aggregate( - &[in1.to_str().unwrap(), in2.to_str().unwrap()], - out.to_str().unwrap(), - ) - .unwrap(); - let content = std::fs::read_to_string(&out).unwrap(); - assert_eq!(content, "aaabbb"); + transform + .aggregate( + &[in1.to_str().unwrap(), in2.to_str().unwrap()], + out.to_str().unwrap(), + ) + .unwrap(); + assert_eq!(std::fs::read_to_string(&out).unwrap(), "aaabbb"); } + #[cfg(unix)] #[test] fn test_command_aggregation_ordering_preserved() { let dir = tempfile::tempdir().unwrap(); let out = dir.path().join("out.txt"); - let in1 = dir.path().join("p1.txt"); let in2 = dir.path().join("p2.txt"); let in3 = dir.path().join("p3.txt"); @@ -516,107 +338,40 @@ mod tests { std::fs::write(&in2, "page2").unwrap(); std::fs::write(&in3, "page3").unwrap(); - let t = CommandAggregationTransform::new( + let transform = CommandAggregationTransform::new( "ordered-cat", "sh", vec!["-c".to_string(), "cat {inputs} > {output}".to_string()], ); - t.aggregate( - &[ - in1.to_str().unwrap(), - in2.to_str().unwrap(), - in3.to_str().unwrap(), - ], - out.to_str().unwrap(), - ) - .unwrap(); - let content = std::fs::read_to_string(&out).unwrap(); - let pos1 = content.find("page1").expect("page1 missing"); - let pos2 = content.find("page2").expect("page2 missing"); - let pos3 = content.find("page3").expect("page3 missing"); - assert!(pos1 < pos2, "page1 must come before page2"); - assert!(pos2 < pos3, "page2 must come before page3"); - } - - // ── factory methods ─────────────────────────────────────────────────────── - - #[test] - fn test_cbz_factory_name_and_program() { - let t = CommandAggregationTransform::cbz("pages-to-cbz"); - assert_eq!(t.name(), "pages-to-cbz"); - assert_eq!(t.program, "zip"); - } - - #[test] - fn test_cbz_factory_args_contain_output_and_inputs() { - let t = CommandAggregationTransform::cbz("cbz"); - assert!( - t.args.iter().any(|a| a.contains("{output}")), - "args must contain {{output}}" - ); - assert!( - t.args.iter().any(|a| a.contains("{inputs}")), - "args must contain {{inputs}}" - ); - } - - #[test] - fn test_cbz_factory_uses_dash_j_flag() { - let t = CommandAggregationTransform::cbz("cbz"); - assert!( - t.args.contains(&"-j".to_string()), - "CBZ args must include -j to strip paths" - ); - } - - #[test] - fn test_images_to_pdf_factory_name_and_program() { - let t = CommandAggregationTransform::images_to_pdf("images-pdf"); - assert_eq!(t.name(), "images-pdf"); - assert_eq!(t.program, "img2pdf"); - } - - #[test] - fn test_images_to_pdf_factory_args_contain_output_and_inputs() { - let t = CommandAggregationTransform::images_to_pdf("images-pdf"); - assert!( - t.args.iter().any(|a| a.contains("{output}")), - "args must contain {{output}}" - ); - assert!( - t.args.iter().any(|a| a.contains("{inputs}")), - "args must contain {{inputs}}" - ); - } - - #[test] - fn test_tiff_to_press_pdf_factory_name_and_program() { - let t = CommandAggregationTransform::tiff_to_press_pdf("press-pdf"); - assert_eq!(t.name(), "press-pdf"); - assert_eq!(t.program, "gs"); - } - - #[test] - fn test_tiff_to_press_pdf_factory_uses_press_settings() { - let t = CommandAggregationTransform::tiff_to_press_pdf("press-pdf"); - assert!( - t.args.iter().any(|a| a.contains("/press")), - "TIFF-to-press-PDF must use -dPDFSETTINGS=/press" - ); + transform + .aggregate( + &[ + in1.to_str().unwrap(), + in2.to_str().unwrap(), + in3.to_str().unwrap(), + ], + out.to_str().unwrap(), + ) + .unwrap(); + assert_eq!(std::fs::read_to_string(&out).unwrap(), "page1page2page3"); } #[test] - fn test_tiff_to_press_pdf_factory_args_contain_output_and_inputs() { - let t = CommandAggregationTransform::tiff_to_press_pdf("press-pdf"); - assert!( - t.args - .iter() - .any(|a| a.contains("{output}") || a.contains("OutputFile")), - "args must contain output placeholder" - ); - assert!( - t.args.iter().any(|a| a.contains("{inputs}")), - "args must contain {{inputs}}" - ); + fn test_factory_shapes() { + let cbz = CommandAggregationTransform::cbz("pages-to-cbz"); + assert_eq!(cbz.program, "zip"); + assert!(cbz.args.iter().any(|arg| arg.contains("{output}"))); + assert!(cbz.args.iter().any(|arg| arg.contains("{inputs}"))); + assert!(cbz.args.contains(&"-j".to_string())); + + let pdf = CommandAggregationTransform::images_to_pdf("images-pdf"); + assert_eq!(pdf.program, "img2pdf"); + assert!(pdf.args.iter().any(|arg| arg.contains("{output}"))); + assert!(pdf.args.iter().any(|arg| arg.contains("{inputs}"))); + + let press = CommandAggregationTransform::tiff_to_press_pdf("press-pdf"); + assert_eq!(press.program, "gs"); + assert!(press.args.iter().any(|arg| arg.contains("/press"))); + assert!(press.args.iter().any(|arg| arg.contains("{inputs}"))); } } diff --git a/crates/renderflow-core/src/transforms/command.rs b/crates/renderflow-core/src/transforms/command.rs index a43fb1c..2c68f51 100644 --- a/crates/renderflow-core/src/transforms/command.rs +++ b/crates/renderflow-core/src/transforms/command.rs @@ -1,50 +1,25 @@ use std::io::Write; -use std::process::Stdio; use anyhow::{Context, Result}; use super::Transform; +use crate::process::{ + is_explicit_shell_invocation, ProcessExpectedOutput, ProcessExecutor, ProcessInput, + ProcessOutputMode, ProcessRequest, DEFAULT_CAPTURE_LIMIT_BYTES, DEFAULT_PROCESS_TIMEOUT, +}; /// A [`Transform`] that executes an external command to process document content. /// -/// `CommandTransform` supports two modes of operation depending on which -/// placeholders appear in `args`: -/// -/// * **File-based** – if `{input}` appears in any argument it is replaced with -/// the path of a temporary file that contains the input string. If `{output}` -/// appears it is replaced with the path of a (initially empty) temporary -/// output file; the file's content is read after the command exits and -/// returned as the transform result. -/// -/// * **Pipe-based** – when neither `{input}` nor `{output}` is present the -/// input string is written to the command's `stdin` and the transform result -/// is read from `stdout`. -/// -/// The two modes may be mixed: `{input}` with no `{output}` reads the result -/// from `stdout`; `{output}` with no `{input}` still pipes input via `stdin`. -/// -/// The command must exit with status 0; a non-zero exit code causes -/// [`Transform::apply`] to return an error containing the program name, exit -/// status, and any output written to `stderr`. +/// All subprocess policy is delegated to Renderflow's canonical process +/// executor. The legacy transform still returns UTF-8 text; binary-native +/// command transforms belong on the artifact API introduced separately. pub struct CommandTransform { - /// Human-readable name used in log messages and error context. name: String, - /// External binary to invoke (looked up on `PATH`). program: String, - /// Arguments passed to the program; may contain `{input}` and `{output}` - /// placeholder strings. args: Vec, } impl CommandTransform { - /// Create a new `CommandTransform`. - /// - /// # Parameters - /// - /// * `name` – human-readable identifier for log messages and errors. - /// * `program` – external binary to invoke (resolved via `PATH`). - /// * `args` – command-line arguments; may contain `{input}` and - /// `{output}` placeholders that are replaced with temporary file paths. pub fn new(name: impl Into, program: impl Into, args: Vec) -> Self { Self { name: name.into(), @@ -60,99 +35,79 @@ impl Transform for CommandTransform { } fn apply(&self, input: String) -> Result { - let has_input_placeholder = self.args.iter().any(|a| a.contains("{input}")); - let has_output_placeholder = self.args.iter().any(|a| a.contains("{output}")); + let has_input_placeholder = self.args.iter().any(|arg| arg.contains("{input}")); + let has_output_placeholder = self.args.iter().any(|arg| arg.contains("{output}")); - // Write input to a temp file when the {input} placeholder is used. let input_file = if has_input_placeholder { - let mut f = + let mut file = tempfile::NamedTempFile::new().context("Failed to create input temp file")?; - f.write_all(input.as_bytes()) + file.write_all(input.as_bytes()) .context("Failed to write to input temp file")?; - Some(f) + Some(file) } else { None }; - // Create an (empty) temp file when the {output} placeholder is used. let output_file = if has_output_placeholder { Some(tempfile::NamedTempFile::new().context("Failed to create output temp file")?) } else { None }; - // Replace placeholders in each argument. let processed_args: Vec = self .args .iter() .map(|arg| { - let mut a = arg.clone(); - if let Some(ref f) = input_file { - a = a.replace("{input}", &f.path().to_string_lossy()); + let mut processed = arg.clone(); + if let Some(ref file) = input_file { + processed = processed.replace("{input}", &file.path().to_string_lossy()); } - if let Some(ref f) = output_file { - a = a.replace("{output}", &f.path().to_string_lossy()); + if let Some(ref file) = output_file { + processed = processed.replace("{output}", &file.path().to_string_lossy()); } - a + processed }) .collect(); - // When reading from a file the command doesn't need stdin. - let stdin_mode = if has_input_placeholder { - Stdio::null() + let request = if is_explicit_shell_invocation(&self.program, &processed_args) { + ProcessRequest::shell(&self.program) } else { - Stdio::piped() - }; - // When writing to a file the command's stdout is irrelevant. - let stdout_mode = if has_output_placeholder { - Stdio::null() - } else { - Stdio::piped() - }; - - let mut child = std::process::Command::new(&self.program) - .args(&processed_args) - .stdin(stdin_mode) - .stdout(stdout_mode) - .stderr(Stdio::piped()) - .spawn() - .with_context(|| format!("Failed to start program '{}'", self.program))?; - - // Pipe input via stdin when the {input} placeholder is not used. - if !has_input_placeholder { - if let Some(mut stdin_handle) = child.stdin.take() { - // Ignore broken-pipe errors: the command may have already exited - // (e.g. `echo -n hello`) without consuming stdin, which is fine. - match stdin_handle.write_all(input.as_bytes()) { - Ok(()) => {} - Err(e) if e.kind() == std::io::ErrorKind::BrokenPipe => {} - Err(e) => return Err(e).context("Failed to write to command stdin"), - } - // Drop `stdin_handle` here to close stdin before `wait_with_output`. - } + ProcessRequest::direct(&self.program) } + .args(processed_args) + .stdin(if has_input_placeholder { + ProcessInput::Null + } else { + ProcessInput::Bytes(input.into_bytes()) + }) + .stdout(if has_output_placeholder { + ProcessOutputMode::Null + } else { + ProcessOutputMode::capture(DEFAULT_CAPTURE_LIMIT_BYTES) + }) + .stderr(ProcessOutputMode::capture(DEFAULT_CAPTURE_LIMIT_BYTES)) + .timeout(DEFAULT_PROCESS_TIMEOUT); - let cmd_output = child - .wait_with_output() - .with_context(|| format!("Failed to wait for program '{}'", self.program))?; + let request = if let Some(ref file) = output_file { + request.expect_output(ProcessExpectedOutput::file(file.path()).require_change()) + } else { + request + }; - if !cmd_output.status.success() { - let stderr = String::from_utf8_lossy(&cmd_output.stderr); - anyhow::bail!( - "Command '{}' exited with status {}: {}", - self.program, - cmd_output.status, - stderr.trim() - ); - } + let result = ProcessExecutor::new() + .execute_checked(request) + .with_context(|| format!("Command transform '{}' failed", self.name))?; - // Read result from the output file when the {output} placeholder was used, - // otherwise parse stdout as UTF-8. - if let Some(ref f) = output_file { - std::fs::read_to_string(f.path()) - .with_context(|| format!("Failed to read output file '{}'", f.path().display())) + if let Some(ref file) = output_file { + std::fs::read_to_string(file.path()).with_context(|| { + format!( + "Failed to read UTF-8 transform output file '{}'", + file.path().display() + ) + }) } else { - String::from_utf8(cmd_output.stdout).context("Command stdout is not valid UTF-8") + String::from_utf8(result.stdout().bytes().to_vec()) + .context("Command stdout is not valid UTF-8") } } } @@ -161,100 +116,90 @@ impl Transform for CommandTransform { mod tests { use super::*; - // ── pipe-based (stdin / stdout) ─────────────────────────────────────────── - #[test] fn test_command_transform_name() { - let t = CommandTransform::new("my-transform", "cat", vec![]); - assert_eq!(t.name(), "my-transform"); + let transform = CommandTransform::new("my-transform", "cat", vec![]); + assert_eq!(transform.name(), "my-transform"); } + #[cfg(unix)] #[test] fn test_pipe_based_passthrough_via_cat() { - // `cat` with no arguments echoes stdin to stdout. - let t = CommandTransform::new("cat-pass", "cat", vec![]); - let result = t.apply("hello world".to_string()).unwrap(); + let transform = CommandTransform::new("cat-pass", "cat", vec![]); + let result = transform.apply("hello world".to_string()).unwrap(); assert_eq!(result, "hello world"); } + #[cfg(unix)] #[test] fn test_pipe_based_multiline_input() { - let t = CommandTransform::new("cat-multi", "cat", vec![]); + let transform = CommandTransform::new("cat-multi", "cat", vec![]); let input = "line one\nline two\nline three".to_string(); - let result = t.apply(input.clone()).unwrap(); + let result = transform.apply(input.clone()).unwrap(); assert_eq!(result, input); } + #[cfg(unix)] #[test] fn test_pipe_based_empty_input() { - let t = CommandTransform::new("cat-empty", "cat", vec![]); - let result = t.apply(String::new()).unwrap(); + let transform = CommandTransform::new("cat-empty", "cat", vec![]); + let result = transform.apply(String::new()).unwrap(); assert_eq!(result, ""); } - // ── file-based ({input} placeholder) ───────────────────────────────────── - + #[cfg(unix)] #[test] fn test_file_based_input_placeholder() { - // `cat {input}` reads the input from a temp file. - let t = CommandTransform::new("cat-file", "cat", vec!["{input}".to_string()]); - let result = t.apply("file content".to_string()).unwrap(); + let transform = CommandTransform::new("cat-file", "cat", vec!["{input}".to_string()]); + let result = transform.apply("file content".to_string()).unwrap(); assert_eq!(result, "file content"); } - // ── file-based ({output} placeholder) ──────────────────────────────────── - #[cfg(unix)] #[test] fn test_file_based_output_placeholder() { - // `sh -c "echo hello > {output}"` writes to the output temp file. - let t = CommandTransform::new( + let transform = CommandTransform::new( "echo-to-file", "sh", vec!["-c".to_string(), "printf '%s' hello > {output}".to_string()], ); - let result = t.apply(String::new()).unwrap(); + let result = transform.apply(String::new()).unwrap(); assert_eq!(result, "hello"); } - // ── both placeholders ───────────────────────────────────────────────────── - #[cfg(unix)] #[test] fn test_both_placeholders_copy_input_to_output() { - // `cp {input} {output}` copies the input temp file to the output temp file. - let t = CommandTransform::new( + let transform = CommandTransform::new( "cp-transform", "cp", vec!["{input}".to_string(), "{output}".to_string()], ); - let result = t.apply("copied content".to_string()).unwrap(); + let result = transform.apply("copied content".to_string()).unwrap(); assert_eq!(result, "copied content"); } - // ── error handling ──────────────────────────────────────────────────────── - #[test] fn test_nonexistent_program_returns_error() { - let t = CommandTransform::new("bad-program", "__nonexistent_program_renderflow__", vec![]); - let err = t.apply("input".to_string()).unwrap_err(); - let msg = err.to_string(); + let transform = + CommandTransform::new("bad-program", "__nonexistent_program_renderflow__", vec![]); + let error = transform.apply("input".to_string()).unwrap_err(); + let message = format!("{error:#}"); assert!( - msg.contains("Failed to start program"), - "expected 'Failed to start program' in: {msg}" + message.contains("was not found") || message.contains("Command transform"), + "unexpected error: {message}" ); } #[cfg(unix)] #[test] fn test_nonzero_exit_code_returns_error() { - // `false` always exits with a non-zero status. - let t = CommandTransform::new("false-cmd", "false", vec![]); - let err = t.apply("input".to_string()).unwrap_err(); - let msg = err.to_string(); + let transform = CommandTransform::new("false-cmd", "false", vec![]); + let error = transform.apply("input".to_string()).unwrap_err(); + let message = format!("{error:#}"); assert!( - msg.contains("exited with status"), - "expected 'exited with status' in: {msg}" + message.contains("exited with code") || message.contains("Command transform"), + "unexpected error: {message}" ); } } diff --git a/docs/process-execution.md b/docs/process-execution.md new file mode 100644 index 0000000..9e28ff9 --- /dev/null +++ b/docs/process-execution.md @@ -0,0 +1,179 @@ +# External Process Execution + +Renderflow wraps external tools, so subprocess behavior is part of the engine's +security, reproducibility, and reliability boundary. Production tool execution +should use `renderflow::process::ProcessExecutor` rather than invoking +`std::process::Command` independently inside adapters. + +## Execution contract + +`ProcessRequest` separates executable identity from arguments and defaults to +**direct argv execution**. No shell parsing occurs unless the caller explicitly +constructs a shell request. + +```rust +use renderflow::process::{ProcessExecutor, ProcessRequest}; + +let result = ProcessExecutor::new().execute_checked( + ProcessRequest::direct("pandoc") + .args(["input.md", "--output", "output.html"]), +)?; +``` + +Explicit shell execution is a wider trust boundary and must remain visible: + +```rust +let request = ProcessRequest::shell("sh") + .args(["-c", "printf '%s' hello"]); +``` + +A direct request that names a known shell and supplies a command-evaluation flag +such as `-c`, `/C`, or `-Command` is rejected. Existing legacy transform YAML +that intentionally names a shell is classified as shell execution by its +compatibility adapter instead of being treated as ordinary direct argv. + +## Bounded lifetime + +Ordinary requests default to a 30-minute wall-clock timeout. Callers may choose +a shorter timeout or explicitly disable it when a capability has a reviewed +reason to run without a deadline. + +The executor polls the child synchronously; Renderflow does not require an async +runtime merely to gain cancellation. `ProcessCancellationToken` is clonable and +may be triggered from another thread or, later, bridged to higher-level engine +cancellation. + +### Process-tree termination + +Cancellation and timeout attempt to terminate spawned child work rather than +only abandoning the caller: + +- Linux/macOS/other Unix targets start the child in its own process group, send + `SIGTERM` to the group, wait a bounded grace period, then escalate to + `SIGKILL`. +- Windows attempts `taskkill /T /F` for tree termination and falls back to the + direct child kill API if that facility is unavailable. +- Other unsupported platforms fall back to terminating the direct child. + +The chosen capability is recorded in `ProcessPlatform` evidence. Callers can +explicitly request child-only termination for a capability that cannot safely +be grouped. + +## Bounded stdout and stderr + +Captured stdout and stderr are drained concurrently so a noisy child cannot +block on a full pipe. Only a bounded prefix is retained in memory; the executor +continues draining the remainder and records: + +- retained bytes; +- total bytes observed; and +- whether truncation occurred. + +The default capture limit is 256 KiB **per stream**. Binary callers can access +the retained raw bytes explicitly. Raw bytes are private from `Debug` output; +user-facing diagnostics should use the redacted text projection. + +Transforms that produce large binary artifacts should write declared files into +the artifact workflow rather than using stdout as an unbounded payload channel. + +## Environment policy + +The default child environment is a filtered inheritance of the parent process. +Variables whose names look credential-bearing are removed unless the caller +explicitly allows or sets them. The filter includes names containing patterns +such as: + +- `TOKEN`; +- `SECRET`; +- `PASSWORD` / `PASSWD`; +- `API_KEY` / `APIKEY`; +- `CREDENTIAL`; +- `PRIVATE_KEY`; and +- authorization-style names. + +`ProcessEnvironment::clear()` provides a clean environment, while explicit +allow/deny/override methods support reviewed adapter requirements. + +A sensitive environment value that is intentionally passed to a child is also +registered with the diagnostic redactor. + +## Secret redaction + +Process diagnostics never intentionally log raw sensitive arguments or +sensitive environment values. The executor redacts: + +- arguments explicitly marked sensitive; +- values following credential-looking flags; +- credential-looking `NAME=value` arguments; +- registered secret values; +- bearer-token values; and +- credentials embedded in URL authorities. + +This is defense in depth, not a license to put secrets into command arguments. +Prefer environment/OIDC/provider-native secret mechanisms whenever possible. + +## Expected outputs + +A request can declare expected files or directories. Checked execution only +succeeds when both the process exit state and output validation succeed. +Expected outputs can require: + +- a path to exist; +- the expected file/directory kind; +- a non-empty file; or +- a change relative to the pre-execution snapshot. + +This prevents a subprocess that exits `0` without producing its promised output +from being treated as a successful transform. Artifact import/materialization +remains owned by the artifact kernel; process execution only validates the +filesystem contract it was given. + +## Tool version probes + +`ProcessExecutor::probe_version()` executes ` --version` with a short +bounded timeout and small capture budget. It produces structured +`ToolProbeEvidence` containing: + +- available/missing/failed/timed-out state; +- first version line when available; +- duration; and +- platform evidence. + +This is the execution primitive for dependency checks and doctor/plugin +inspection. The broader capability registry and reproducible toolchain +fingerprint are intentionally owned by Renderflow #359. + +## Network and sandbox policy hooks + +`ProcessRequest` carries network intent and an optional sandbox-profile ID, and +`ProcessExecutor` accepts `ProcessPolicyHook` implementations. These fields are +**not** claims that the core executor automatically provides network isolation or +OS sandboxing. A deployment/profile that requires those guarantees must attach a +policy hook backed by an actual enforcement mechanism. + +## Temporary state + +Temporary input/output paths used by compatibility transforms remain +caller-owned RAII state. The executor does not recursively delete arbitrary +filesystem paths and never removes a verified final artifact as cleanup. + +## Migration boundary + +The following production paths use the canonical executor after #356: + +- built-in Pandoc/FFmpeg rendering through the command adapter; +- legacy YAML `CommandTransform` execution; +- command-backed collection/aggregation transforms; +- dependency availability probes; +- PDF/Tectonic probing; +- `renderflow doctor` probes; and +- plugin required-tool probes. + +The historical repository-audit command still shells out only to gather its own +`date`/`git` report metadata. It is not a wrapped transform/tool adapter and is +outside the reusable execution-provider boundary defined here. + +Future adapters, including HandBrake work in #345, must use this process port. +#355 will project process outcomes into richer execution evidence, #358 will +bridge orchestration cancellation/resume semantics, and #359 will add stable tool +capability IDs and complete version/toolchain fingerprints.