From 5bf22c92846cffe10508b4493d958b49194dfbe7 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 10:15:42 +0000 Subject: [PATCH] fix(interpreter): process substitution expands to /dev/fd/63 `<(cmd)` / `>(cmd)` now expand to bash's `/dev/fd/63`, `/dev/fd/62`, ... per command, skipping fds still open and reusing 63 once the command (or the compound command that opened it) finishes. The data lives in a per-interpreter fd namespace: `ProcSubFs` wraps each shell's VFS view and answers `/dev/fd/N` (N >= 3) itself, never forwarding such a path to the shared filesystem, so tenants on one VFS each get their own `/dev/fd/63` (TM-ISO-028). Replaces the `/dev/fd/proc_sub_` VFS files and the global counter. `set -C` no longer refuses `> >(cmd)`. Bash-oracle floor raised to 103 101. --- crates/bashkit/docs/threat-model.md | 1 + crates/bashkit/src/fs/mod.rs | 4 + crates/bashkit/src/fs/proc_sub_fds.rs | 359 ++++++++++++++++++ crates/bashkit/src/fs/std_streams.rs | 3 +- crates/bashkit/src/interpreter/mod.rs | 249 ++++++------ crates/bashkit/src/interpreter/redirection.rs | 40 +- crates/bashkit/src/lib.rs | 7 +- crates/bashkit/tests/integration/main.rs | 1 + .../process_substitution_fd_tests.rs | 265 +++++++++++++ .../bash/bashbox-process-substitution.test.sh | 6 +- knowledge/log.md | 1 + knowledge/operations/limitations.md | 2 +- knowledge/security/threat-model.md | 10 +- scripts/bash-oracle/floor.txt | 2 +- 14 files changed, 815 insertions(+), 135 deletions(-) create mode 100644 crates/bashkit/src/fs/proc_sub_fds.rs create mode 100644 crates/bashkit/tests/integration/process_substitution_fd_tests.rs diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 468c65301..1802c3e5c 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -543,6 +543,7 @@ echo $user_input | Wrapper rebuild drops constructor capabilities (TM-ISO-025) | A binding reset loses limits, policy files, callbacks, or network policy | Canonical capability matrix with executable evidence; rebuilds retain constructor config | **MITIGATED** | | Shared ToolRegistry request context (TM-ISO-026) | Concurrent shell/Python/TypeScript calls leak tenant identity or traces | Per-request `ExecutionExtensions`, task-local runtime routing, and callback-owned context | **MITIGATED** | | Stale request authority and retained host-extension handles (TM-ISO-027) | A late runtime/transport/callback result crosses completion, or a builtin/tool keeps VFS or request context past completion/cancellation | Shared request budget plus one revocable capability lease, cancellation-aware awaits, deterministic late-use failure, RAII closure/release, and explicit `insert_trusted` escape hatch | **MITIGATED** | +| Process substitution shared through the VFS (TM-ISO-028) | Tenants on one filesystem read or clobber each other's `<(cmd)` data at `/dev/fd/63` | `/dev/fd/N` (N >= 3) resolves in each interpreter's own fd namespace; nothing reaches the shared filesystem | **MITIGATED** | Each [`Bash`] instance is fully isolated. For multi-tenant environments, create separate instances per tenant: diff --git a/crates/bashkit/src/fs/mod.rs b/crates/bashkit/src/fs/mod.rs index e0405f064..5879a71eb 100644 --- a/crates/bashkit/src/fs/mod.rs +++ b/crates/bashkit/src/fs/mod.rs @@ -408,6 +408,7 @@ mod mountable; mod namespace; mod overlay; mod posix; +mod proc_sub_fds; mod readonly; #[cfg(feature = "realfs")] mod realfs; @@ -425,6 +426,9 @@ pub use mountable::MountableFs; pub use namespace::{NamespaceAccess, NamespaceFs, NamespaceFsBuilder}; pub use overlay::OverlayFs; pub use posix::PosixFs; +pub(crate) use proc_sub_fds::{ + PROC_SUB_FIRST_FD, ProcSubBuffer, ProcSubData, ProcSubFs, proc_sub_fd, +}; pub use readonly::ReadOnlyFs; #[cfg(feature = "realfs")] pub use realfs::{RealFs, RealFsMode}; diff --git a/crates/bashkit/src/fs/proc_sub_fds.rs b/crates/bashkit/src/fs/proc_sub_fds.rs new file mode 100644 index 000000000..6647c834d --- /dev/null +++ b/crates/bashkit/src/fs/proc_sub_fds.rs @@ -0,0 +1,359 @@ +//! Per-shell `/dev/fd/N` namespace for process substitution. +//! +//! Decision: `<(cmd)` / `>(cmd)` expand to bash's `/dev/fd/63`, `/dev/fd/62`, +//! ... but the data never lives in the VFS. Several interpreters (tenants) can +//! share one filesystem; a shared `/dev/fd/63` file would let one read or +//! clobber another's substitution (TM-ISO-028). Each interpreter wraps its VFS +//! in [`ProcSubFs`], which answers `/dev/fd/N` (N >= 3) from this shell's own +//! fd table and never forwards such a path to the shared filesystem: an fd not +//! open in this shell is "No such file or directory", as on Linux. +//! +//! Fds 0-2 pass through (builtins see them via `std_streams`, redirects via +//! `dev_fd_alias`). The table is a stack: each simple command closes the +//! substitutions it opened (`ProcSubMark` in the interpreter). Background jobs +//! get a copy (fds are inherited at fork). Paths without `fd/` in them skip +//! the table without normalizing, so other VFS calls pay one byte scan. + +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex, MutexGuard}; + +use async_trait::async_trait; + +use super::{ + DirEntry, FileSystem, FileSystemExt, FileType, FsLimits, FsUsage, Metadata, VfsSnapshot, + fs_errors, +}; +use crate::error::Result; +use crate::time_compat::{SystemTime, UNIX_EPOCH}; + +/// First fd bash hands out for a process substitution; later ones count down. +pub(crate) const PROC_SUB_FIRST_FD: i32 = 63; + +/// What a command writes to a `>(cmd)` fd, fed to `cmd` afterwards. +pub(crate) type ProcSubBuffer = Arc>>; + +/// One open process substitution. +#[derive(Clone)] +pub(crate) enum ProcSubData { + /// `<(cmd)`: the list's output, read by the command. + Input(Arc<[u8]>), + /// `>(cmd)`: what the command writes, fed to the list afterwards. + Output(ProcSubBuffer), +} + +pub(crate) struct ProcSubFs { + inner: Arc, + /// Open substitutions in allocation order. + fds: Mutex>, +} + +/// What a path names in this shell's fd namespace. +enum Slot { + /// Not `/dev/fd/N` with N >= 3: the VFS handles it. + Vfs, + Open(i32, ProcSubData), + Closed, +} + +/// `/dev/fd/N` with N >= 3 (after `.`/`..` normalization). +pub(crate) fn proc_sub_fd(path: &Path) -> Option { + let raw = path.as_os_str().as_encoded_bytes(); + if !raw.windows(3).any(|w| w == b"fd/") { + return None; + } + let normalized = super::normalize_path(path); + let n = normalized.to_str()?.strip_prefix("/dev/fd/")?; + if n.is_empty() || n.len() > 9 || !n.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + let fd: i32 = n.parse().ok()?; + (fd >= 3).then_some(fd) +} + +fn closed_error() -> crate::Error { + fs_errors::not_found("No such file or directory") +} + +fn not_permitted() -> crate::Error { + std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "Operation not permitted", + ) + .into() +} + +impl ProcSubFs { + pub(crate) fn new(inner: Arc) -> Arc { + Arc::new(Self { + inner, + fds: Mutex::new(Vec::new()), + }) + } + + /// Same filesystem, a copy of the open fds (a forked job). + pub(crate) fn fork(&self) -> Arc { + Arc::new(Self { + inner: Arc::clone(&self.inner), + fds: Mutex::new(self.table().clone()), + }) + } + + fn table(&self) -> MutexGuard<'_, Vec<(i32, ProcSubData)>> { + self.fds.lock().unwrap_or_else(|e| e.into_inner()) + } + + pub(crate) fn open_count(&self) -> usize { + self.table().len() + } + + pub(crate) fn is_open(&self, fd: i32) -> bool { + self.table().iter().any(|(n, _)| *n == fd) + } + + pub(crate) fn open(&self, fd: i32, data: ProcSubData) { + self.table().push((fd, data)); + } + + /// Close every substitution opened after the first `len`. + pub(crate) fn close_from(&self, len: usize) { + let mut table = self.table(); + if table.len() > len { + table.truncate(len); + } + } + + fn slot(&self, path: &Path) -> Slot { + let Some(fd) = proc_sub_fd(path) else { + return Slot::Vfs; + }; + match self.table().iter().rev().find(|(n, _)| *n == fd) { + Some((_, data)) => Slot::Open(fd, data.clone()), + None => Slot::Closed, + } + } + + /// Error for an operation that a pipe end does not support. + fn refuse(&self, path: &Path) -> Option { + match self.slot(path) { + Slot::Vfs => None, + Slot::Open(..) => Some(not_permitted()), + Slot::Closed => Some(closed_error()), + } + } + + fn pipe_metadata() -> Metadata { + Metadata { + file_type: FileType::Fifo, + size: 0, + mode: 0o600, + modified: UNIX_EPOCH, + created: UNIX_EPOCH, + } + } + + fn write_pipe(&self, path: &Path, content: &[u8]) -> Option> { + match self.slot(path) { + Slot::Vfs => None, + Slot::Open(_, ProcSubData::Output(buf)) => { + buf.lock() + .unwrap_or_else(|e| e.into_inner()) + .extend_from_slice(content); + Some(Ok(())) + } + Slot::Open(_, ProcSubData::Input(_)) => Some(Err(not_permitted())), + Slot::Closed => Some(Err(closed_error())), + } + } +} + +#[async_trait] +impl FileSystemExt for ProcSubFs { + fn usage(&self) -> FsUsage { + self.inner.usage() + } + async fn mkfifo(&self, path: &Path, mode: u32) -> Result<()> { + if let Some(e) = self.refuse(path) { + return Err(e); + } + self.inner.mkfifo(path, mode).await + } + fn limits(&self) -> FsLimits { + self.inner.limits() + } + fn vfs_snapshot(&self) -> Option { + self.inner.vfs_snapshot() + } + fn vfs_restore(&self, snapshot: &VfsSnapshot) -> Result<()> { + self.inner.vfs_restore(snapshot) + } + fn backend_kind(&self) -> &'static str { + self.inner.backend_kind() + } +} + +#[async_trait] +impl FileSystem for ProcSubFs { + async fn read_file(&self, path: &Path) -> Result> { + match self.slot(path) { + Slot::Vfs => self.inner.read_file(path).await, + Slot::Open(_, ProcSubData::Input(data)) => Ok(data.to_vec()), + // Reading the write end of `>(cmd)` sees no data. + Slot::Open(_, ProcSubData::Output(_)) => Ok(Vec::new()), + Slot::Closed => Err(closed_error()), + } + } + async fn write_file(&self, path: &Path, content: &[u8]) -> Result<()> { + match self.write_pipe(path, content) { + Some(r) => r, + None => self.inner.write_file(path, content).await, + } + } + async fn append_file(&self, path: &Path, content: &[u8]) -> Result<()> { + match self.write_pipe(path, content) { + Some(r) => r, + None => self.inner.append_file(path, content).await, + } + } + async fn mkdir(&self, path: &Path, recursive: bool) -> Result<()> { + if let Some(e) = self.refuse(path) { + return Err(e); + } + self.inner.mkdir(path, recursive).await + } + async fn remove(&self, path: &Path, recursive: bool) -> Result<()> { + if let Some(e) = self.refuse(path) { + return Err(e); + } + self.inner.remove(path, recursive).await + } + async fn stat(&self, path: &Path) -> Result { + match self.slot(path) { + Slot::Vfs => self.inner.stat(path).await, + Slot::Open(..) => Ok(Self::pipe_metadata()), + Slot::Closed => Err(closed_error()), + } + } + async fn lstat(&self, path: &Path) -> Result { + match self.slot(path) { + Slot::Vfs => self.inner.lstat(path).await, + // Linux: `/dev/fd/N` is a symlink to `pipe:[inode]`. + Slot::Open(..) => Ok(Metadata { + file_type: FileType::Symlink, + mode: 0o700, + ..Self::pipe_metadata() + }), + Slot::Closed => Err(closed_error()), + } + } + async fn read_dir(&self, path: &Path) -> Result> { + self.inner.read_dir(path).await + } + async fn exists(&self, path: &Path) -> Result { + match self.slot(path) { + Slot::Vfs => self.inner.exists(path).await, + Slot::Open(..) => Ok(true), + Slot::Closed => Ok(false), + } + } + async fn rename(&self, from: &Path, to: &Path) -> Result<()> { + if let Some(e) = self.refuse(from).or_else(|| self.refuse(to)) { + return Err(e); + } + self.inner.rename(from, to).await + } + async fn copy(&self, from: &Path, to: &Path) -> Result<()> { + if proc_sub_fd(from).is_none() && proc_sub_fd(to).is_none() { + return self.inner.copy(from, to).await; + } + let data = self.read_file(from).await?; + self.write_file(to, &data).await + } + async fn symlink(&self, target: &Path, link: &Path) -> Result<()> { + if let Some(e) = self.refuse(link) { + return Err(e); + } + self.inner.symlink(target, link).await + } + async fn read_link(&self, path: &Path) -> Result { + match self.slot(path) { + Slot::Vfs => self.inner.read_link(path).await, + // Linux names the pipe's inode; there is none here. + Slot::Open(fd, _) => Ok(PathBuf::from(format!("pipe:[{fd}]"))), + Slot::Closed => Err(closed_error()), + } + } + async fn chmod(&self, path: &Path, mode: u32) -> Result<()> { + if let Some(e) = self.refuse(path) { + return Err(e); + } + self.inner.chmod(path, mode).await + } + async fn set_modified_time(&self, path: &Path, time: SystemTime) -> Result<()> { + if let Some(e) = self.refuse(path) { + return Err(e); + } + self.inner.set_modified_time(path, time).await + } + fn as_search_capable(&self) -> Option<&dyn super::SearchCapable> { + // Indexed search would read the VFS, not the open substitutions. + if self.open_count() == 0 { + self.inner.as_search_capable() + } else { + None + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::fs::InMemoryFs; + + #[test] + fn recognizes_dev_fd_numbers() { + assert_eq!(proc_sub_fd(Path::new("/dev/fd/63")), Some(63)); + assert_eq!(proc_sub_fd(Path::new("/dev/../dev/fd/./62")), Some(62)); + assert_eq!(proc_sub_fd(Path::new("/dev/fd/1")), None); + assert_eq!(proc_sub_fd(Path::new("/dev/fd/x")), None); + assert_eq!(proc_sub_fd(Path::new("/tmp/fd/63")), None); + assert_eq!(proc_sub_fd(Path::new("/dev/fd")), None); + } + + #[tokio::test] + async fn open_fds_are_private_to_the_wrapper() { + let shared: Arc = Arc::new(InMemoryFs::new()); + let a = ProcSubFs::new(Arc::clone(&shared)); + let b = ProcSubFs::new(Arc::clone(&shared)); + let p = Path::new("/dev/fd/63"); + a.open(63, ProcSubData::Input(Arc::from(&b"a-data"[..]))); + assert_eq!(a.read_file(p).await.unwrap(), b"a-data"); + assert!(a.exists(p).await.unwrap()); + assert_eq!(a.stat(p).await.unwrap().file_type, FileType::Fifo); + // Other shell, same VFS: not open there, and never written through. + assert!(!b.exists(p).await.unwrap()); + assert!(b.read_file(p).await.is_err()); + assert!(b.write_file(p, b"x").await.is_err()); + assert!(!shared.exists(p).await.unwrap()); + a.close_from(0); + assert!(a.read_file(p).await.is_err()); + } + + #[tokio::test] + async fn output_fd_collects_writes() { + let fs = ProcSubFs::new(Arc::new(InMemoryFs::new())); + let buf = Arc::new(Mutex::new(Vec::new())); + fs.open(62, ProcSubData::Output(Arc::clone(&buf))); + let p = Path::new("/dev/fd/62"); + fs.write_file(p, b"a").await.unwrap(); + fs.append_file(p, b"b").await.unwrap(); + fs.copy(Path::new("/dev/fd/62"), Path::new("/dev/fd/62")) + .await + .unwrap(); + assert_eq!(buf.lock().unwrap().as_slice(), b"ab"); + let forked = fs.fork(); + assert!(forked.is_open(62)); + fs.close_from(0); + assert!(forked.is_open(62)); + assert!(!fs.is_open(62)); + } +} diff --git a/crates/bashkit/src/fs/std_streams.rs b/crates/bashkit/src/fs/std_streams.rs index 715406a15..4e61f73a7 100644 --- a/crates/bashkit/src/fs/std_streams.rs +++ b/crates/bashkit/src/fs/std_streams.rs @@ -7,7 +7,8 @@ //! builtin's VFS view in [`StdStreamsFs`] only when an argument names one of //! these paths: reads of fd 0 return the command's stdin, writes to fd 1/2 //! are captured and appended to the builtin's stdout/stderr afterwards. -//! Only fds 0-2: `/dev/fd/63` etc. stay real VFS files (process substitution). +//! Only fds 0-2: `/dev/fd/63` etc. are the shell's process substitutions, +//! answered by `ProcSubFs` underneath. use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 95b357863..13079c2ce 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -53,8 +53,6 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; use std::sync::{Arc, Mutex as StdMutex}; -/// Monotonic counter for unique process substitution file paths -static PROC_SUB_COUNTER: AtomicU64 = AtomicU64::new(0); static TIME_REPORT_COUNTER: AtomicU64 = AtomicU64::new(0); // Important decision: report a bash-compatible version surface instead of the @@ -1394,6 +1392,14 @@ struct NoforkScope { outer: Option>, } +/// Where a simple command's process substitutions start: the deferred `>(...)` +/// lists and the `/dev/fd/N` fds it opens are released from here when it ends. +#[derive(Clone, Copy)] +struct ProcSubMark { + deferred: usize, + fds: usize, +} + /// Interpreter state. pub struct Interpreter { fs: Arc, @@ -1626,12 +1632,12 @@ pub struct Interpreter { /// final-command errexit behavior. condition_sequence_depth: usize, /// Deferred output process substitutions: after a command writes to the - /// virtual file path, run these commands with the file content as stdin. - /// Each entry is (virtual_path, commands_to_run). - deferred_proc_subs: Vec<(String, Vec)>, - /// Process substitution paths created by this interpreter instance. - /// Used to avoid deleting paths owned by other sessions sharing the same VFS. - proc_sub_paths: HashSet, + /// substitution's fd, run these commands with what it wrote as stdin. + /// Each entry is (write buffer, commands_to_run). + deferred_proc_subs: Vec<(crate::fs::ProcSubBuffer, Vec)>, + /// This shell's `/dev/fd/N` namespace for process substitutions; `fs` + /// is this same wrapper (see `fs/proc_sub_fds.rs`, TM-ISO-028). + proc_subs: Arc, /// PRNG state for $RANDOM (LCG seeded per-instance from OS entropy). /// NOT cryptographically secure — matches real bash behavior. /// Uses `AtomicU32` for interior mutability so $RANDOM can advance state @@ -1729,6 +1735,10 @@ impl Interpreter { builtin_filter: Option, hardened_timing: bool, ) -> Self { + // Every VFS access of this shell (redirects, builtins, `source`, + // tests) resolves `/dev/fd/N` in its own fd namespace first. + let proc_subs = crate::fs::ProcSubFs::new(fs); + let fs: Arc = Arc::clone(&proc_subs) as Arc; // Macro to reduce boilerplate for simple zero-arg builtin registration. // Custom-construction builtins (date, source, hostname, etc.) are registered below. macro_rules! register_builtins { @@ -2169,7 +2179,7 @@ impl Interpreter { err_trap_skip_stage: false, condition_sequence_depth: 0, deferred_proc_subs: Vec::new(), - proc_sub_paths: HashSet::new(), + proc_subs, random_state: AtomicU32::new(random_seed), shell_features, hardened_timing, @@ -2270,8 +2280,10 @@ impl Interpreter { .load(Ordering::Relaxed) .wrapping_mul(1_103_515_245) .wrapping_add(12_345); + // The job inherits the open substitution fds, then owns its copy. + let job_proc_subs = self.proc_subs.fork(); Interpreter { - fs: Arc::clone(&self.fs), + fs: Arc::clone(&job_proc_subs) as Arc, env: self.env.clone(), scoped: self.scoped.clone(), source_depth: self.source_depth, @@ -2357,7 +2369,7 @@ impl Interpreter { err_trap_skip_stage: false, condition_sequence_depth: self.condition_sequence_depth, deferred_proc_subs: Vec::new(), - proc_sub_paths: HashSet::new(), + proc_subs: job_proc_subs, random_state: AtomicU32::new(random_seed), shell_features: self.shell_features, hardened_timing: self.hardened_timing, @@ -2627,7 +2639,7 @@ impl Interpreter { self.condition_sequence_depth = 0; self.loop_depth = 0; self.return_depth = 0; - self.deferred_proc_subs.clear(); + self.close_proc_sub_fds(); self.clear_pending_fd_redirect_state(); // Top-level timeouts drop the interpreter future at await points, so // BASH_SOURCE cleanup after script execution may not run. Reset both @@ -3407,14 +3419,12 @@ impl Interpreter { result } - /// Clean up process substitution temp files (`/dev/fd/proc_sub_*`). - /// Called from Bash::exec() after execute() returns, outside the - /// recursive async call chain to avoid increasing stack frame size. - pub(crate) async fn cleanup_proc_sub_files(&mut self) { - let paths = std::mem::take(&mut self.proc_sub_paths); - for path in paths { - let _ = self.fs.remove(Path::new(&path), false).await; - } + /// Close every process substitution fd still open (one opened by a + /// compound command's words, or left by an aborted command). + /// Called from Bash::exec() after execute() returns. + pub(crate) fn close_proc_sub_fds(&mut self) { + self.proc_subs.close_from(0); + self.deferred_proc_subs.clear(); } /// Inner script execution — runs commands without resetting counters. @@ -3837,11 +3847,15 @@ impl Interpreter { Command::Pipeline(pipeline) => self.execute_pipeline(pipeline).await, Command::List(list) => self.execute_list(list).await, Command::Compound(compound, redirects) => { + // Substitutions in its words or redirects (`for x in + // <(a)`, `done < <(b)`, `[[ -e <(c) ]]`) close with it. + let proc_sub_fds = self.proc_subs.open_count(); // Own frame: keeps this arm's temporaries off the stack // of every `$(...)`/function nesting level. let result = self .execute_compound_with_redirects(compound, redirects) .await; + self.proc_subs.close_from(proc_sub_fds); // `( )`, `[[ ]]` and `(( ))` set PIPESTATUS to their own // status like a simple command; other compounds leave // the last inner command's (bash). @@ -7463,9 +7477,19 @@ impl Interpreter { Some(result) } - /// Discard deferred output process substitutions queued by the current simple command. - fn discard_deferred_proc_subs_from(&mut self, start: usize) { - self.deferred_proc_subs.truncate(start); + /// Where the current simple command's process substitutions start. + fn proc_sub_mark(&self) -> ProcSubMark { + ProcSubMark { + deferred: self.deferred_proc_subs.len(), + fds: self.proc_subs.open_count(), + } + } + + /// Discard deferred output process substitutions queued by the current + /// simple command and close the fds it opened. + fn discard_deferred_proc_subs_from(&mut self, start: ProcSubMark) { + self.deferred_proc_subs.truncate(start.deferred); + self.proc_subs.close_from(start.fds); } /// Execute deferred output process substitutions (`>(cmd)`) queued by the @@ -7473,24 +7497,18 @@ impl Interpreter { /// and must not be drained by nested command substitutions. async fn run_deferred_proc_subs_from( &mut self, - start: usize, + start: ProcSubMark, result: &mut Result, ) -> Result<()> { - if self.deferred_proc_subs.len() <= start { + // The command is done: its substitution fds close (bash reuses 63). + self.proc_subs.close_from(start.fds); + if self.deferred_proc_subs.len() <= start.deferred { return Ok(()); } - let deferred = self.deferred_proc_subs.split_off(start); - for (path_str, commands) in deferred { - let path = Path::new(&path_str); - let stdin_data = if let Ok(bytes) = self.fs.read_file(path).await { - if bytes.is_empty() { - None - } else { - Some(bytes.into()) - } - } else { - None - }; + let deferred = self.deferred_proc_subs.split_off(start.deferred); + for (buf, commands) in deferred { + let bytes = std::mem::take(&mut *buf.lock().unwrap_or_else(|e| e.into_inner())); + let stdin_data: Option = (!bytes.is_empty()).then(|| bytes.into()); // `>(cmd)` runs in a subshell too (see expand_process_substitution). // Boxed: keeps this future (in every simple command's await chain) // small so deep `$(...)` nesting stays within the stack. @@ -7647,7 +7665,7 @@ impl Interpreter { } else { command }; - let deferred_proc_sub_start = self.deferred_proc_subs.len(); + let deferred_proc_sub_start = self.proc_sub_mark(); let (_debug_stdout, _debug_stderr) = self.run_debug_trap().await; // The command word undergoes the same field splitting as args: @@ -11059,12 +11077,6 @@ impl Interpreter { )); } - let path_str = format!( - "/dev/fd/proc_sub_{}", - PROC_SUB_COUNTER.fetch_add(1, Ordering::Relaxed) - ); - let path = Path::new(&path_str); - if is_input { let mut stdout = String::new(); // The substituted list runs in a subshell: nothing it changes @@ -11098,19 +11110,45 @@ impl Interpreter { if let Some(e) = failed { return Err(e); } - if self.fs.write_file(path, stdout.as_bytes()).await.is_err() { - Ok(stdout) - } else { - self.proc_sub_paths.insert(path_str.clone()); - Ok(path_str) - } + // Allocated after the list ran: in bash the child closes its own + // end, so a substitution nested inside sees the number free. + let fd = self.allocate_proc_sub_fd()?; + self.proc_subs.open( + fd, + crate::fs::ProcSubData::Input(Arc::from(stdout.into_bytes())), + ); + Ok(format!("/dev/fd/{fd}")) } else { - let _ = self.fs.write_file(path, b"").await; - self.proc_sub_paths.insert(path_str.clone()); - self.deferred_proc_subs - .push((path_str.clone(), commands.to_vec())); - Ok(path_str) + let fd = self.allocate_proc_sub_fd()?; + let buf = Arc::new(StdMutex::new(Vec::new())); + self.proc_subs + .open(fd, crate::fs::ProcSubData::Output(Arc::clone(&buf))); + self.deferred_proc_subs.push((buf, commands.to_vec())); + Ok(format!("/dev/fd/{fd}")) + } + } + + /// Fd for the next process substitution, as bash picks it: the highest + /// free one counting down from 63, then upwards from 64. + fn allocate_proc_sub_fd(&self) -> Result { + let open = self.proc_subs.open_count(); + if open >= self.limits.max_file_descriptors { + return Err(crate::limits::LimitExceeded::MaxFileDescriptors( + self.limits.max_file_descriptors, + ) + .into()); } + let first = crate::fs::PROC_SUB_FIRST_FD; + // At most `open` numbers are taken by substitutions, so the upward + // scan ends within `open + 64` steps. + (3..=first) + .rev() + .chain(first + 1..=first + 1 + open as i32 + 64) + .find(|&fd| !self.proc_subs.is_open(fd) && !self.fd_is_open(fd)) + .ok_or_else(|| { + crate::limits::LimitExceeded::MaxFileDescriptors(self.limits.max_file_descriptors) + .into() + }) } // THREAT[TM-DOS-089]: Command substitution body extracted into a Box::pin-ed @@ -15746,7 +15784,19 @@ cat /tmp/test_fd_exec_public.txt"#, assert_eq!(lines, vec!["count:2", "arg1:p", "arg2:q"]); } - /// Issue #1184: input process substitution temp files must be cleaned up + /// Fds left open (and VFS entries) after the commands ran. + async fn proc_sub_leftovers(interp: &Interpreter, fs: &Arc) -> Vec { + let mut left: Vec = Vec::new(); + if interp.proc_subs.open_count() != 0 { + left.push(format!("{} open fds", interp.proc_subs.open_count())); + } + if let Ok(entries) = fs.read_dir(Path::new("/dev/fd")).await { + left.extend(entries.into_iter().map(|e| e.name)); + } + left + } + + /// Issue #1184: input process substitutions close with their command. #[tokio::test] async fn test_proc_sub_input_cleanup() { let fs: Arc = Arc::new(InMemoryFs::new()); @@ -15756,22 +15806,10 @@ cat /tmp/test_fd_exec_public.txt"#, let ast = parser.parse().unwrap(); let result = interp.execute(&ast).await.unwrap(); assert_eq!(result.exit_code, 0); - interp.cleanup_proc_sub_files().await; - - if let Ok(entries) = fs.read_dir(Path::new("/dev/fd")).await { - let leaked: Vec<_> = entries - .iter() - .filter(|e| e.name.starts_with("proc_sub_")) - .collect(); - assert!( - leaked.is_empty(), - "proc_sub files leaked in /dev/fd: {:?}", - leaked.iter().map(|e| &e.name).collect::>() - ); - } + assert_eq!(proc_sub_leftovers(&interp, &fs).await, Vec::::new()); } - /// Issue #1184: output process substitution temp files must be cleaned up + /// Issue #1184: output process substitutions close with their command. #[tokio::test] async fn test_proc_sub_output_cleanup() { let fs: Arc = Arc::new(InMemoryFs::new()); @@ -15781,19 +15819,8 @@ cat /tmp/test_fd_exec_public.txt"#, let ast = parser.parse().unwrap(); let result = interp.execute(&ast).await.unwrap(); assert_eq!(result.exit_code, 0); - interp.cleanup_proc_sub_files().await; - - if let Ok(entries) = fs.read_dir(Path::new("/dev/fd")).await { - let leaked: Vec<_> = entries - .iter() - .filter(|e| e.name.starts_with("proc_sub_")) - .collect(); - assert!( - leaked.is_empty(), - "proc_sub files leaked in /dev/fd: {:?}", - leaked.iter().map(|e| &e.name).collect::>() - ); - } + assert_eq!(result.stdout, "data 1\ndata 2\ndata 3\n"); + assert_eq!(proc_sub_leftovers(&interp, &fs).await, Vec::::new()); } /// Issue #1184: cleanup happens even when command fails @@ -15802,45 +15829,47 @@ cat /tmp/test_fd_exec_public.txt"#, let fs: Arc = Arc::new(InMemoryFs::new()); let mut interp = Interpreter::new(Arc::clone(&fs)); - let parser = Parser::new(r#"cat <(echo "data") && false; true"#); + let parser = Parser::new(r#"cat <(echo "data") && false; true; cat <(echo x) /nope"#); let ast = parser.parse().unwrap(); let _result = interp.execute(&ast).await.unwrap(); - interp.cleanup_proc_sub_files().await; - - if let Ok(entries) = fs.read_dir(Path::new("/dev/fd")).await { - let leaked: Vec<_> = entries - .iter() - .filter(|e| e.name.starts_with("proc_sub_")) - .collect(); - assert!( - leaked.is_empty(), - "proc_sub files leaked after failed command: {:?}", - leaked.iter().map(|e| &e.name).collect::>() - ); - } + assert_eq!(proc_sub_leftovers(&interp, &fs).await, Vec::::new()); + // Words of a compound command stay open until the exec ends. + let parser = Parser::new(r#"case <(true) in *) : ;; esac"#); + let ast = parser.parse().unwrap(); + let _result = interp.execute(&ast).await.unwrap(); + interp.close_proc_sub_fds(); + assert_eq!(proc_sub_leftovers(&interp, &fs).await, Vec::::new()); } - /// Regression: cleanup must not remove process substitution paths owned by other sessions. + /// TM-ISO-028: a substitution is an fd of its own shell, never a file in + /// the shared VFS another interpreter could read. #[tokio::test] - async fn test_proc_sub_cleanup_does_not_delete_other_session_files() { + async fn test_proc_sub_fd_is_private_to_its_interpreter() { let fs: Arc = Arc::new(InMemoryFs::new()); let mut owner = Interpreter::new(Arc::clone(&fs)); - let mut other = Interpreter::new(Arc::clone(&fs)); + let other = Interpreter::new(Arc::clone(&fs)); - let parser = Parser::new(r#"echo <(echo "data")"#); + let parser = Parser::new(r#"f() { echo "$1"; cat "$1"; }; f <(echo data)"#); let ast = parser.parse().unwrap(); let result = owner.execute(&ast).await.unwrap(); - let proc_sub_path = result.stdout.trim().to_string(); - assert!(proc_sub_path.starts_with("/dev/fd/proc_sub_")); - assert!(fs.read_file(Path::new(&proc_sub_path)).await.is_ok()); - - other.cleanup_proc_sub_files().await; - + assert_eq!(result.stdout, "/dev/fd/63\ndata\n"); + // While open in `owner`, the fd exists only there. + owner + .proc_subs + .open(63, crate::fs::ProcSubData::Input(Arc::from(&b"secret"[..]))); + assert_eq!( + owner.fs.read_file(Path::new("/dev/fd/63")).await.unwrap(), + b"secret" + ); + assert!(other.fs.read_file(Path::new("/dev/fd/63")).await.is_err()); assert!( - fs.read_file(Path::new(&proc_sub_path)).await.is_ok(), - "cleanup from another interpreter removed {}", - proc_sub_path + other + .fs + .write_file(Path::new("/dev/fd/63"), b"x") + .await + .is_err() ); + assert!(!fs.exists(Path::new("/dev/fd/63")).await.unwrap()); } /// Regression: all known internal prefixes must be caught by is_internal_variable(). diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 1ec24e522..2f256bc0f 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -98,7 +98,7 @@ impl Interpreter { let target_path = self.expand_word(&redirect.target).await?; let path = self.resolve_path(&target_path); // Handle /dev/null at interpreter level - cannot be bypassed - if dev_fd_alias(&path) == Some(0) { + if self.fd_alias(&path) == Some(0) { // `< /dev/stdin` re-reads the current stdin: no-op. } else if is_dev_null(&path) { stdin = Some(crate::StreamData::new()); // EOF @@ -283,7 +283,7 @@ impl Interpreter { return Ok(result); } let path = self.resolve_path(&target_path); - if let Some(target_fd) = dev_fd_alias(&path) { + if let Some(target_fd) = self.fd_alias(&path) { self.dup_output_fast(&mut result, redirect.fd.unwrap_or(1), target_fd) .await?; } else if is_dev_null(&path) { @@ -294,7 +294,7 @@ impl Interpreter { } else { if redirect.kind == RedirectKind::Output && self.scoped.variables.get("SHOPT_C").map(|v| v.as_str()) == Some("1") - && self.fs.stat(&path).await.is_ok() + && self.fs.stat(&path).await.is_ok_and(clobber_protected) { result.stdout = crate::StreamData::new(); result.stderr = self @@ -341,7 +341,7 @@ impl Interpreter { return Ok(result); } let path = self.resolve_path(&target_path); - if let Some(target_fd) = dev_fd_alias(&path) { + if let Some(target_fd) = self.fd_alias(&path) { self.dup_output_fast(&mut result, redirect.fd.unwrap_or(1), target_fd) .await?; } else if is_dev_null(&path) { @@ -425,7 +425,7 @@ impl Interpreter { target_path: &str, ) -> Result { let path = self.resolve_path(target_path); - if let Some(target_fd) = dev_fd_alias(&path) { + if let Some(target_fd) = self.fd_alias(&path) { // `&> /dev/fd/N` == `>&N 2>&N`, resolved against the // original descriptors. if target_fd == 2 { @@ -606,7 +606,7 @@ impl Interpreter { return Ok(result); } let path = self.resolve_path(&target_path); - if let Some(target_fd) = dev_fd_alias(&path) { + if let Some(target_fd) = self.fd_alias(&path) { let src_fd = redirect.fd.unwrap_or(1); self.dup_output_fd_table(src_fd, target_fd, &mut fd1, &mut fd2); continue; @@ -615,7 +615,7 @@ impl Interpreter { if redirect.kind == RedirectKind::Output && self.scoped.variables.get("SHOPT_C").map(|v| v.as_str()) == Some("1") && !is_dev_null(&path) - && self.fs.stat(&path).await.is_ok() + && self.fs.stat(&path).await.is_ok_and(clobber_protected) { result.stdout = crate::StreamData::new(); result.stderr = self @@ -644,7 +644,7 @@ impl Interpreter { return Ok(result); } let path = self.resolve_path(&target_path); - if let Some(target_fd) = dev_fd_alias(&path) { + if let Some(target_fd) = self.fd_alias(&path) { let src_fd = redirect.fd.unwrap_or(1); self.dup_output_fd_table(src_fd, target_fd, &mut fd1, &mut fd2); continue; @@ -663,7 +663,7 @@ impl Interpreter { RedirectKind::OutputBoth => { let target_path = self.expand_word(&redirect.target).await?; let path = self.resolve_path(&target_path); - if let Some(target_fd) = dev_fd_alias(&path) { + if let Some(target_fd) = self.fd_alias(&path) { self.dup_output_fd_table(1, target_fd, &mut fd1, &mut fd2); self.dup_output_fd_table(2, 1, &mut fd1, &mut fd2); continue; @@ -903,12 +903,25 @@ fn buffer_text(lines: &[String]) -> crate::StreamData { text.into() } +/// `set -C`: `>` may not truncate an existing file. A pipe (FIFO, process +/// substitution) is not truncated, so it stays writable. +fn clobber_protected(meta: crate::fs::Metadata) -> bool { + meta.file_type != crate::fs::FileType::Fifo +} + /// Lines of `text`, reversed so `pop()` yields the first. fn reversed_lines(text: &str) -> Vec { text.lines().rev().map(str::to_string).collect() } impl Interpreter { + /// Descriptor a `/dev/...` redirect target aliases (`dev_fd_alias`), + /// except an open process substitution's `/dev/fd/N`: that is a pipe + /// end, opened through the fs like a file (`ProcSubFs`). + pub(super) fn fd_alias(&self, path: &Path) -> Option { + dev_fd_alias(path).filter(|&fd| fd < 3 || !self.proc_subs.is_open(fd)) + } + /// Whether `fd` is open in this shell (or provided by an enclosing /// command's redirections). pub(super) fn fd_is_open(&self, fd: i32) -> bool { @@ -1170,10 +1183,17 @@ impl Interpreter { let target_path = self.expand_word(&redirect.target).await?; let path = self.resolve_path(&target_path); self.ensure_persistent_fd_capacity(n)?; - let entry = if let Some(target_fd) = dev_fd_alias(&path) { + let entry = if let Some(target_fd) = self.fd_alias(&path) { self.exec_fd_alias_target(target_fd) } else if is_dev_null(&path) { FdTarget::DevNull + } else if crate::fs::proc_sub_fd(&path).is_some_and(|fd| self.proc_subs.is_open(fd)) + { + // WTF: `exec 3> >(cmd)` would need `cmd` to outlive the + // `exec` and read fd 3 as it is written; it runs right + // after the `exec` instead (no input), so later writes to + // fd 3 are dropped rather than failing on the closed fd. + FdTarget::DevNull } else { // Truncate (or create) the file on open, like real // `exec >file`; a failed open leaves the fd as it was. diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index c09e16992..d5b63e301 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -1257,10 +1257,9 @@ impl Bash { if installed_invocation { self.interpreter.truncate_call_stack(call_stack_baseline); } - // Issue #1184: clean up process substitution temp files after execution. - // Done here (outside Interpreter::execute) to avoid increasing the - // recursive async state machine size which causes stack overflow. - self.interpreter.cleanup_proc_sub_files().await; + // Issue #1184: close process substitution fds left open (a compound + // command's words, an aborted command) so the next exec starts at 63. + self.interpreter.close_proc_sub_fds(); let duration_ms = exec_start.elapsed().as_millis() as u64; // Record history entry for each line of the script diff --git a/crates/bashkit/tests/integration/main.rs b/crates/bashkit/tests/integration/main.rs index 4d2268bdf..388dce90d 100644 --- a/crates/bashkit/tests/integration/main.rs +++ b/crates/bashkit/tests/integration/main.rs @@ -114,6 +114,7 @@ pub mod output_truncation_tests; pub mod parallel_sessions_tests; pub mod partial_parse_tests; pub mod pipeline_tests; +pub mod process_substitution_fd_tests; pub mod proptest_differential; pub mod python_integration_tests; pub mod python_security_tests; diff --git a/crates/bashkit/tests/integration/process_substitution_fd_tests.rs b/crates/bashkit/tests/integration/process_substitution_fd_tests.rs new file mode 100644 index 000000000..dc4d5ff10 --- /dev/null +++ b/crates/bashkit/tests/integration/process_substitution_fd_tests.rs @@ -0,0 +1,265 @@ +//! Process substitution paths: `<(cmd)` / `>(cmd)` expand to `/dev/fd/N` +//! with bash's numbering (63, 62, ... per command), resolved in a per-shell +//! fd namespace, never as files in the (possibly shared) VFS. +//! +//! Expected output verified against GNU bash 5.2. + +use bashkit::{ + Bash, DirEntry, FileSystem, FileSystemExt, InMemoryFs, Metadata, Result, async_trait, +}; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; +use std::time::SystemTime; + +async fn run(script: &str) -> bashkit::ExecResult { + let mut bash = Bash::builder().build(); + bash.exec(script).await.unwrap() +} + +#[tokio::test] +async fn single_substitution_is_fd_63() { + let r = run("echo <(true)").await; + assert_eq!(r.stdout, "/dev/fd/63\n"); +} + +#[tokio::test] +async fn several_in_one_command_count_down() { + let r = run("echo <(true) <(true) >(true) <(true)").await; + assert_eq!(r.stdout, "/dev/fd/63 /dev/fd/62 /dev/fd/61 /dev/fd/60\n"); +} + +#[tokio::test] +async fn numbers_are_reused_by_the_next_command() { + let r = + run("echo <(true); echo <(true)\nfor i in 1 2; do echo <(true); done\n{ echo <(true); }") + .await; + assert_eq!(r.stdout, "/dev/fd/63\n".repeat(5)); +} + +#[tokio::test] +async fn nested_and_command_substitution_start_at_63() { + let r = run( + "echo $(echo <(true))\necho <(echo <(true))\ncat <(echo <(true) <(true))\n( echo <(true) )", + ) + .await; + assert_eq!( + r.stdout, + "/dev/fd/63\n/dev/fd/63\n/dev/fd/63 /dev/fd/62\n/dev/fd/63\n" + ); +} + +#[tokio::test] +async fn live_caller_fds_are_skipped() { + // The function's argument keeps fd 63 open while its body runs. + let r = run("f() { echo <(true); cat \"$1\"; }; f <(echo outer)\n\ + g() { echo \"in g: $1\"; echo <(true) <(true); }; g <(true) <(true)\n\ + cat <(true) <(echo $(echo <(true)))") + .await; + assert_eq!( + r.stdout, + "/dev/fd/62\nouter\nin g: /dev/fd/63\n/dev/fd/61 /dev/fd/60\n/dev/fd/62\n" + ); +} + +#[tokio::test] +async fn exec_opened_fd_is_skipped() { + let r = run("exec 63>/dev/null; echo <(true); exec 63>&-; echo <(true)").await; + assert_eq!(r.stdout, "/dev/fd/62\n/dev/fd/63\n"); +} + +#[tokio::test] +async fn consumers_read_the_substitution() { + let r = run("cat <(echo a) <(echo b)\n\ + diff <(echo a) <(echo b); echo rc=$?\n\ + while read l; do echo \"got $l\"; done < <(printf '1\\n2\\n')\n\ + paste <(printf '1\\n2\\n') <(printf 'a\\nb\\n')\n\ + source <(echo 'echo sourced')\n\ + bash <(echo 'echo ran')\n\ + exec 3< <(echo three); cat <&3\n\ + [ -e <(true) ] && echo exists\n\ + test -r <(true) && echo readable\n\ + test -p <(true) && echo pipe\n\ + for x in 1; do cat <(echo loop); done < <(echo ignored); echo <(true)") + .await; + assert_eq!( + r.stdout, + "a\nb\n1c1\n< a\n---\n> b\nrc=1\ngot 1\ngot 2\n1\ta\n2\tb\nsourced\nran\nthree\nexists\nreadable\npipe\nloop\n/dev/fd/63\n" + ); +} + +#[tokio::test] +async fn writers_feed_the_output_substitution() { + let r = run("echo hi > >(tr a-z A-Z)\n\ + echo abc | tee >(rev) >/dev/null\n\ + cat > >(cat) <<< heredoc\n\ + set -C; echo noclobber > >(cat)") + .await; + assert_eq!(r.stdout, "HI\ncba\nheredoc\nnoclobber\n"); +} + +#[tokio::test] +async fn closed_substitution_is_gone() { + // The fd closes when the command that created it finishes. + let r = run("x=$(echo <(echo hi)); cat $x; echo rc=$?; cat /dev/fd/63; echo rc=$?").await; + assert_eq!(r.stdout, "rc=1\nrc=1\n"); + assert!(r.stderr.contains("/dev/fd/63: No such file or directory")); +} + +/// Records every path the shell hands to the shared filesystem. +struct RecordingFs { + inner: Arc, + touched: Mutex>, +} + +impl RecordingFs { + fn note(&self, path: &Path) { + self.touched.lock().unwrap().push(path.to_path_buf()); + } + fn touched_dev_fd(&self) -> Vec { + self.touched + .lock() + .unwrap() + .iter() + .filter(|p| p.starts_with("/dev/fd")) + .cloned() + .collect() + } +} + +#[async_trait] +impl FileSystemExt for RecordingFs {} + +#[async_trait] +impl FileSystem for RecordingFs { + async fn read_file(&self, path: &Path) -> Result> { + self.note(path); + self.inner.read_file(path).await + } + async fn write_file(&self, path: &Path, content: &[u8]) -> Result<()> { + self.note(path); + self.inner.write_file(path, content).await + } + async fn append_file(&self, path: &Path, content: &[u8]) -> Result<()> { + self.note(path); + self.inner.append_file(path, content).await + } + async fn mkdir(&self, path: &Path, recursive: bool) -> Result<()> { + self.note(path); + self.inner.mkdir(path, recursive).await + } + async fn remove(&self, path: &Path, recursive: bool) -> Result<()> { + self.note(path); + self.inner.remove(path, recursive).await + } + async fn stat(&self, path: &Path) -> Result { + self.note(path); + self.inner.stat(path).await + } + async fn read_dir(&self, path: &Path) -> Result> { + self.inner.read_dir(path).await + } + async fn exists(&self, path: &Path) -> Result { + self.note(path); + self.inner.exists(path).await + } + async fn rename(&self, from: &Path, to: &Path) -> Result<()> { + self.note(from); + self.note(to); + self.inner.rename(from, to).await + } + async fn copy(&self, from: &Path, to: &Path) -> Result<()> { + self.note(from); + self.note(to); + self.inner.copy(from, to).await + } + async fn symlink(&self, target: &Path, link: &Path) -> Result<()> { + self.note(link); + self.inner.symlink(target, link).await + } + async fn read_link(&self, path: &Path) -> Result { + self.note(path); + self.inner.read_link(path).await + } + async fn chmod(&self, path: &Path, mode: u32) -> Result<()> { + self.note(path); + self.inner.chmod(path, mode).await + } + async fn set_modified_time(&self, path: &Path, time: SystemTime) -> Result<()> { + self.note(path); + self.inner.set_modified_time(path, time).await + } +} + +// THREAT[TM-ISO-028]: process substitution data lives in the shell's own fd +// namespace. Nothing reaches the shared filesystem, so a second interpreter +// on the same VFS can neither read nor clobber it. +#[tokio::test] +async fn substitution_never_reaches_shared_filesystem() { + let shared = Arc::new(RecordingFs { + inner: Arc::new(InMemoryFs::new()), + touched: Mutex::new(Vec::new()), + }); + let mut bash = Bash::builder() + .fs(Arc::clone(&shared) as Arc) + .build(); + let r = bash + .exec( + "cat <(echo secret); echo hi > >(cat); diff <(echo a) <(echo a) && echo same\n\ + f() { cat \"$1\"; [ -e \"$1\" ] && echo live; }; f <(echo arg)", + ) + .await + .unwrap(); + assert_eq!(r.stdout, "secret\nhi\nsame\narg\nlive\n"); + assert_eq!(shared.touched_dev_fd(), Vec::::new()); +} + +#[tokio::test] +async fn second_interpreter_does_not_see_first_ones_fd() { + let fs: Arc = Arc::new(InMemoryFs::new()); + let mut a = Bash::builder().fs(Arc::clone(&fs)).build(); + let mut b = Bash::builder().fs(Arc::clone(&fs)).build(); + // `a` keeps /dev/fd/63 open across its function call; `b` runs while + // `a`'s script is suspended between commands of the same exec. + let ra = a + .exec("f() { cat \"$1\"; }; f <(echo tenant-a)") + .await + .unwrap(); + assert_eq!(ra.stdout, "tenant-a\n"); + let rb = b + .exec("cat /dev/fd/63; echo rc=$?; [ -e /dev/fd/63 ] || echo absent; echo x > /dev/fd/63; cat <(echo tenant-b)") + .await + .unwrap(); + assert_eq!(rb.stdout, "rc=1\nabsent\ntenant-b\n"); + assert!(!fs.exists(Path::new("/dev/fd/63")).await.unwrap()); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn concurrent_interpreters_keep_their_own_fd_63() { + let fs: Arc = Arc::new(InMemoryFs::new()); + let mut tasks = Vec::new(); + for t in 0..4 { + let fs = Arc::clone(&fs); + tasks.push(tokio::spawn(async move { + let mut bash = Bash::builder().fs(fs).build(); + let script = format!( + "for i in $(seq 1 40); do f() {{ sleep 0; cat \"$1\"; }}; f <(echo t{t}-$i); done" + ); + let r = bash.exec(&script).await.unwrap(); + let expected: String = (1..=40).map(|i| format!("t{t}-{i}\n")).collect(); + assert_eq!(r.stdout, expected); + })); + } + for task in tasks { + task.await.unwrap(); + } + assert!(!fs.exists(Path::new("/dev/fd/63")).await.unwrap()); +} + +#[tokio::test] +async fn exec_onto_output_substitution_does_not_abort() { + // bash prints "kept" via cat; Bashkit drops it (limitations.md) but must + // not fail the script on the closed fd. + let r = run("exec 3> >(cat); echo kept >&3; exec 3>&-; echo after").await; + assert!(r.stdout.ends_with("after\n"), "{}", r.stdout); + assert_eq!(r.exit_code, 0); +} diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-process-substitution.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-process-substitution.test.sh index 5009d343c..43586b0f3 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-process-substitution.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-process-substitution.test.sh @@ -11,7 +11,6 @@ hi ### bashbox_process_substitution_two_substitutions_are_numbered_down_from_63 # two substitutions are numbered down from 63 -### skip: process substitution paths are /dev/fd/proc_sub_, not /dev/fd/63 (limitations.md, Process substitution row: tenant-safe naming on a shared VFS) echo <(true) <(true) >(true); cat <(echo a) <(echo b) ### expect /dev/fd/63 /dev/fd/62 /dev/fd/61 @@ -21,7 +20,6 @@ b ### bashbox_process_substitution_the_number_is_reused_once_the_command_is_done # the number is reused once the command is done -### skip: process substitution paths are /dev/fd/proc_sub_, not /dev/fd/63 (limitations.md, Process substitution row: tenant-safe naming on a shared VFS) echo <(true); f(){ echo <(true); }; f ### expect /dev/fd/63 @@ -30,7 +28,7 @@ echo <(true); f(){ echo <(true); }; f ### bashbox_process_substitution_part_of_a_word # part of a word -### skip: process substitution is its own word, not joined to adjacent text (x<(true) is two words), and paths are /dev/fd/proc_sub_ (limitations.md, Process substitution row) +### skip: process substitution is its own word, not joined to adjacent text (x<(true) is two words) (limitations.md, Process substitution row) echo x<(true) ### expect x/dev/fd/63 @@ -77,7 +75,7 @@ x=1; cat <(x=2; echo $x); echo $x ### bashbox_process_substitution_assigned_to_a_variable # assigned to a variable -### skip: x=<(true) parses as an assignment then a separate process-substitution word, and paths are /dev/fd/proc_sub_ (limitations.md, Process substitution row) +### skip: x=<(true) parses as an assignment then a separate process-substitution word (limitations.md, Process substitution row) x=<(true); echo $x ### expect /dev/fd/63 diff --git a/knowledge/log.md b/knowledge/log.md index 36e1ebbab..53c861c84 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -2,6 +2,7 @@ ## 2026-10-08 +* **Fix**: process substitution expands to bash's `/dev/fd/63`, counting down per command (`echo <(true) <(true)` is `/dev/fd/63 /dev/fd/62`), skipping fds still open (a caller's substitution, `exec 63>f`), and reusing 63 once the command, or the compound command whose words or redirects opened it, finishes. The fds live in a per-interpreter namespace (`ProcSubFs` wraps each shell's VFS view, TM-ISO-028) instead of `/dev/fd/proc_sub_` files in the shared VFS, so tenants on one filesystem each get their own `/dev/fd/63`; a closed one is "No such file or directory". `set -C` no longer refuses `> >(cmd)` (a FIFO is not clobbered). Remaining gaps in [Limitations](operations/limitations.md). bash-oracle match 103 to 104, strict 101 to 102. * **Fix**: coprocesses run concurrently. `coproc NAME cmd` is a background job wired to the shell through two bounded in-process pipes: writes to `${NAME[1]}` feed its stdin, `exec {NAME[1]}>&-` delivers EOF, `read <&${NAME[0]}` / `read -u` take its output line by line, so `coproc UPPER { tr a-z A-Z; }` and request/response loops (`echo q >&${C[1]}; read r <&${C[0]}`) work; the body used to run once at `coproc` with empty input. fds follow bash (63/60, then 62/58), `$NAME_PID`/`$!`/`wait $NAME_PID` give the job, a second coproc while one runs prints bash's `still exists` warning, NAME and NAME_PID are unset and the fds closed once `wait` reaps it, and `exec()` closes the shell's ends before waiting for jobs. `<&$empty` is now an error instead of reading the shell's stdin. See foundations/parallel-execution "Coprocesses", L-PROC-005. bash-oracle match 102 to 103, strict 100 to 101. * **Fix**: `$SHLVL` follows bash's fork suppression. bash execs some simple commands in place: the last one of a `bash -c` string or `$( )` body (also the right side of a trailing `&&`/`||`/`;`; no redirections, no EXIT/ERR or signal trap set), the last one of a `( )`/`<( )` body (a sole one even with redirections), and any simple command run with `&`; a shell started there lowers `$SHLVL` first and so keeps the level. `exec bash` lowers it too, except directly inside `( )`; a substitution inside a pipeline stage does not. `bash -c 'bash -c "echo \$SHLVL"'` printed 3 in a script started at 1, now 2. A `bash`/`sh` child no longer inherits trap handlers (an ignored signal stays ignored), so `bash -c 'trap "echo bye" EXIT; bash -c true'` prints `bye` once. L-PROC-003 narrowed. bash-oracle match 101 to 102, strict 99 to 100. diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index ed7ee538f..3694e36c3 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -108,7 +108,7 @@ Target: IEEE 1003.1-2024 Shell Command Language. | `timeout` | Basic usage | `-k` kill timeout | | `bash`/`sh` | `-c`, `-n`, `-e`, `-x`, `-u`, `-f`, `-o option`, script files, stdin, `--version`, `--help` | Login shell | | Expansion errors | `set -u` unbound variables stop the shell | `$((1/0))` evaluates to 0 and `${x!}` expands empty; bash reports them and drops the rest of the line the failing command ends on (`bashbox_line_number_an_expansion_error_drops_the_rest_of_the_line_it_ends_on`, skipped) | -| Process substitution | `<(cmd)` and `>(cmd)` as filenames, nesting, redirections | The path is `/dev/fd/proc_sub_`, not bash's `/dev/fd/63`. Several interpreters can share one filesystem, so a per-shell descending number would let one tenant's substitution overwrite another's. A script that parses the number sees a different one (`scripts/bash-oracle/cases/process-substitution-dev-fd.sh`). A substitution is always its own word: bash joins it to adjacent text (`x<(true)` is `x/dev/fd/63`, `x=<(true)` assigns the path), Bashkit splits it off (`bashbox-process-substitution.test.sh`, skipped). The substituted list runs in a subshell, as in bash | +| Process substitution | `<(cmd)` and `>(cmd)` as filenames, nesting, redirections | Paths are bash's `/dev/fd/63`, `/dev/fd/62`, ... per command, resolved in the shell's own fd namespace, never the shared VFS (TM-ISO-028; `process_substitution_fd_tests.rs`). The data is buffered, not a pipe: `<(cmd)` runs to completion before the command, `>(cmd)` runs after it. `readlink` prints `pipe:[N]` with the fd as N, not an inode; `ls /dev/fd` lists none of them; `exec 3> >(cmd)` runs `cmd` right after the `exec` with no input; later writes to fd 3 are dropped. `[[ -e <(true) ]]` and `for x in <(a)` do not parse a substitution there. A substitution is always its own word: bash joins it to adjacent text (`x<(true)` is `x/dev/fd/63`, `x=<(true)` assigns the path), Bashkit splits it off (`bashbox-process-substitution.test.sh`, skipped). The substituted list runs in a subshell, as in bash | | Aliases | `alias`/`unalias`, `shopt -s expand_aliases`, trailing-space chaining, recursion guard | Expanded when the simple command runs, not when its line is read: bash parses a whole line (and a function body at definition) before an `alias`/`unalias` on it takes effect; Bashkit applies them to the very next command | | Named fds / `<>` | `{var}>file`, `{var}file`, `{var}>>file`, `{var}<<&N`/`<&N` allocate the lowest free fd >= 10 on any command (persisting, as in bash); `{var}>&-` closes; `>&N` with N closed is `Bad file descriptor`; `N<>file`, `N<<file` has no shared read/write offset (writes append); `3&1`, `( ... ) 3>&1`) is dropped; a `Bad file descriptor` message ignores redirect order (`2>&1 >&7`); output to a stdout closed by the command's own `>&-` reports `write error: Bad file descriptor` without naming the command, since the redirect is applied after it ran | | Locale | UTF-8 text everywhere; `${#v}` counts bytes when `LC_ALL`/`LC_CTYPE`/`LANG` (first non-empty) is `C` or `POSIX` | With no locale variable set bashkit is UTF-8, where bash falls back to C. Under C, `${s:off:len}`, `${s^^}`, `${s#?}` and glob `?` still work on characters, not bytes: shell words are Rust strings and cannot hold a split UTF-8 sequence (L-STREAM-001) | diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index ae13d6503..082fdae97 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -965,6 +965,7 @@ Only exact domain matches are allowed (TM-NET-017). | TM-ISO-025 | Wrapper rebuild silently drops or restores revoked capabilities | A binding's `reset()` or fresh-execution path rebuilds `Bash` without limits, policy files, callbacks, network policy, or other host configuration, or replays an explicitly revoked mount, changing the sandbox contract after the first call | The canonical capability manifest requires executable evidence for every supported surface cell. NAPI `SharedState` retains constructor `files`, and `build_bash_from_state` is the single rebuild path used by construction and reset. Host mutations applied *after* construction are recorded on the same path: NAPI `runtime_env`/`runtime_mounts` and the Python equivalents replay `set_env()` and runtime mounts on every rebuild. Runtime mount replay keys use the VFS-normalized mount point, so `unmount()` retracts the record even when mount and unmount callers use equivalent paths containing `.` or `..`; a rebuild cannot resurrect the removed mount. Script-set env is not recorded, so `reset()` still discards it. Regressions: `Bash: reset restores configured files`, `Bash: reset preserves setEnv values`, `Bash: reset preserves a runtime FileSystem mount`, `Bash: unmount retracts the replay, so reset does not resurrect it`, `Bash: unmount retracts an equivalent normalized mount path`, `test_reset_preserves_set_env`, `test_unmount_retracts_replay`, `test_unmount_retracts_equivalent_normalized_mount_path` | **MITIGATED** | | TM-ISO-026 | Shared ToolRegistry leaks tenant identity or traces across requests/runtimes | One immutable registry and callback set may serve concurrent tenants through shell, Python, and TypeScript; registry-global mutable request state would cross-contaminate authorization and telemetry | `ToolCallRequest` carries tenant + a fresh bounded trace through `ExecutionExtensions`; runtime bridges expose it through the core `RuntimeCallContext` task-local only for the current run; callbacks receive owned `ToolArgs` context; no tenant value is stored in the registry (`bashkit-scripted-tool/src/tool_registry.rs`) | **MITIGATED** | | TM-ISO-027 | Request-owned execution authority or retained facility handles survive completion | A retained runtime, transport, callback, budget, extension, or custom-builtin VFS handle emits late output, charges a reused request, or accesses facilities after completion/cancellation | One `ExecutionBudget` and one capability lease are created before initialization and closed/revoked on every exit. The budget gates work and async results; `ExecutionCapability`, scoped VFS wrappers, host-call brokers, and `ToolArgs` context gate retained facilities. Cleanup is bounded/idempotent and reports only sanitized counts. `BuiltinRegistry::insert_trusted` explicitly models the intentional unscoped host escape hatch. Shared lifecycle evidence is defined in [Request Execution Lifecycle](request-lifecycle.md); Rust and Python capability regressions cover late/cross-request/cancellation use and cleanup failure. | **MITIGATED** | +| TM-ISO-028 | Process substitution data shared through the VFS | Tenants sharing one filesystem read or clobber each other's `<(cmd)` output, or plant `/dev/fd/63` for a victim to consume | `/dev/fd/N` (N >= 3) is a per-interpreter fd namespace (`ProcSubFs` wraps each shell's VFS view): open substitutions are served from memory, closed ones are "No such file or directory", and no such path reaches the shared filesystem. Regressions: `process_substitution_fd_tests` (`substitution_never_reaches_shared_filesystem`, `second_interpreter_does_not_see_first_ones_fd`, `concurrent_interpreters_keep_their_own_fd_63`), `fs::proc_sub_fds::tests` | **MITIGATED** | **TM-ISO-004**: Fixed, see table; env wiring in `builtins/jq/compat.rs` and `builtins/jq/mod.rs`. @@ -974,10 +975,11 @@ unlimited aggregate resources. Fixed by `SessionLimits` (see table); issue #655. **TM-ISO-006**: unbounded `HashMap` growth (`variables`, `arrays`, `assoc_arrays`, `functions`) could OOM the process. Fixed by `MemoryLimits` (see table); issue #656. -**Note**: `PROC_SUB_COUNTER` (`AtomicU64`) is a global monotonic counter for process substitution -paths (`/dev/fd/proc_sub_N`). This is a minor timing side-channel (reveals approximate execution -ordering across concurrent sessions) but does not leak data since paths are resolved within each -session's isolated VFS. +**TM-ISO-028**: process substitution used to write `<(cmd)` output to `/dev/fd/proc_sub_N` in the +VFS, a path any interpreter mounted on the same filesystem could read or overwrite (and a global +counter leaked cross-session ordering). Now `/dev/fd/N` (N >= 3) is answered per interpreter by +`ProcSubFs` (`fs/proc_sub_fds.rs`), which wraps that shell's VFS view and never forwards such a path +to the shared filesystem. **Current Risk**: MEDIUM - cumulative resource bypass (TM-ISO-005) and memory exhaustion (TM-ISO-006) diff --git a/scripts/bash-oracle/floor.txt b/scripts/bash-oracle/floor.txt index 021b3f448..5c4f5ebba 100644 --- a/scripts/bash-oracle/floor.txt +++ b/scripts/bash-oracle/floor.txt @@ -1 +1 @@ -103 101 +104 102