diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 25e0463f..12a01d5b 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -853,7 +853,7 @@ an embedder-supplied JavaScript object. | Partial filesystem mutation (TM-FS-014) | Failed write/copy or cross-mount move leaves corruption, duplication, or retained quota | Failure-atomic `FileSystem` contract; RealFs sibling staging; MountableFs destination rollback; NamespaceFs cross-device rejection; shared conformance + failpoint tests | MITIGATED | | Partial tar extraction (TM-FS-015) | A late unsafe or malformed entry leaves earlier files behind | Validate the complete archive and file limits before the first VFS mutation | MITIGATED | | yq in-place partial update (TM-FS-016) | A failed transform or write truncates the source file | Evaluate and serialize before writing; random sibling temporary file, mode preservation, and rename-on-success | MITIGATED | -| JS host filesystem widens the sandbox (TM-FS-017) | `new Bash({ fs })` gives a script whatever the embedder's object exposes | Paths are normalized by `PosixFs` before any host call, so traversal cannot select a path the embedder did not scope. The host object *is* the boundary and is the embedder's to scope; its bytes also live outside the VFS quotas, so the embedder owns the storage limit | ACCEPTED (opt-in, embedder-scoped) | +| JS host filesystem widens the sandbox (TM-FS-017) | `new Bash({ fs })` gives a script whatever the embedder's object exposes | Paths are normalized by `PosixFs` before any host call, and reads of host-reported symlinks are rejected before reaching a potentially symlink-following host method. The host object *is* the boundary and is the embedder's to scope; its bytes also live outside the VFS quotas, so the embedder owns the storage limit | ACCEPTED (opt-in, embedder-scoped) | ### Unicode Security (TM-UNI-*) diff --git a/crates/bashkit/src/fs/posix.rs b/crates/bashkit/src/fs/posix.rs index 503e273f..2ceb97cf 100644 --- a/crates/bashkit/src/fs/posix.rs +++ b/crates/bashkit/src/fs/posix.rs @@ -9,6 +9,7 @@ //! //! | Check | Description | //! |-------|-------------| +//! | Inert symlinks | `read_file` never delegates symlink paths to the backend | //! | Type-safe writes | `write_file` fails with "is a directory" if path is a directory | //! | Type-safe mkdir | `mkdir` fails with "file exists" if path is a file | //! | Parent directory | Write operations require parent directory to exist | @@ -66,6 +67,7 @@ use crate::error::Result; /// /// | Operation | Check | /// |-----------|-------| +/// | `read_file` | Fails if path is a directory or symlink | /// | `write_file` | Fails if path is a directory | /// | `append_file` | Fails if path is a directory | /// | `mkdir` | Fails if path exists as file (always) or dir (unless recursive) | @@ -124,11 +126,15 @@ impl PosixFs { impl FileSystem for PosixFs { async fn read_file(&self, path: &Path) -> Result> { let path = Self::normalize(path); - // Check if it's a directory - if let Ok(meta) = self.backend.stat(&path).await - && meta.file_type.is_dir() - { - return Err(fs_errors::is_a_directory()); + if let Ok(meta) = self.backend.stat(&path).await { + if meta.file_type.is_dir() { + return Err(fs_errors::is_a_directory()); + } + // THREAT[TM-ESC-002]: Raw backends may follow symlinks when reading. + // Mitigation: keep symlinks inert by rejecting them before delegation. + if meta.file_type.is_symlink() { + return Err(IoError::new(std::io::ErrorKind::NotFound, "file not found").into()); + } } self.backend.read(&path).await } @@ -295,9 +301,12 @@ mod tests { use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::sync::Mutex; + use std::sync::atomic::{AtomicUsize, Ordering}; struct AppendCreatesFileBackend { files: Mutex>, + symlinks: Mutex>, + reads: AtomicUsize, } impl AppendCreatesFileBackend { @@ -307,6 +316,8 @@ mod tests { files.insert(PathBuf::from("/tmp")); Self { files: Mutex::new(files), + symlinks: Mutex::new(HashSet::new()), + reads: AtomicUsize::new(0), } } } @@ -314,6 +325,7 @@ mod tests { #[async_trait] impl FsBackend for AppendCreatesFileBackend { async fn read(&self, _path: &Path) -> Result> { + self.reads.fetch_add(1, Ordering::Relaxed); Ok(Vec::new()) } @@ -342,6 +354,17 @@ mod tests { } async fn stat(&self, path: &Path) -> Result { + if self + .symlinks + .lock() + .expect("backend lock poisoned") + .contains(path) + { + return Ok(Metadata { + file_type: FileType::Symlink, + ..Metadata::default() + }); + } if self .files .lock() @@ -377,7 +400,11 @@ mod tests { Ok(()) } - async fn symlink(&self, _target: &Path, _link: &Path) -> Result<()> { + async fn symlink(&self, _target: &Path, link: &Path) -> Result<()> { + self.symlinks + .lock() + .expect("backend lock poisoned") + .insert(link.to_path_buf()); Ok(()) } @@ -482,4 +509,21 @@ mod tests { "append should fail when parent doesn't exist" ); } + + #[tokio::test] + async fn test_posix_read_does_not_delegate_symlink_to_backend() { + let fs = PosixFs::new(AppendCreatesFileBackend::new()); + fs.symlink(Path::new("/outside-secret"), Path::new("/tmp/link")) + .await + .expect("symlink should succeed"); + + let result = fs.read_file(Path::new("/tmp/link")).await; + + assert!(result.is_err(), "symlink reads must fail"); + assert_eq!( + fs.backend().reads.load(Ordering::Relaxed), + 0, + "raw backends may follow symlinks, so PosixFs must not delegate the read" + ); + } } diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index a210272b..f0470df7 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -321,7 +321,7 @@ panicked. Resolved with `wrapping_*` ops, masked shift amounts, clamped exponent | TM-FS-014 | Partial filesystem mutation | Failed write/copy or copy-delete move corrupts/replaces a destination, duplicates a source, or consumes retained quota | `FileSystem` failure-atomicity contract; locked in-memory rename; MountableFs restores cross-mount destinations while NamespaceFs rejects cross-mount rename; RealFs stages and flushes sibling files before rename; failpoint and conformance regressions | **MITIGATED** | | TM-FS-015 | Partial archive extraction | A late traversal, malformed header, or size failure leaves earlier attacker-controlled files behind | Tar validates the complete archive and per-file limits before its first VFS mutation; conformance regression uses a valid entry followed by traversal | **MITIGATED** | | TM-FS-016 | yq in-place partial or destructive update | Parse, evaluation, serialization, or write failure truncates the source; predictable temporary names permit collisions | Complete evaluation and bounded serialization first; write a random sibling temporary file, preserve mode, and rename only after success; failpoint regressions cover allocation, all backend-write classes, chmod, rename, original-byte retention, and temporary cleanup | **MITIGATED** | -| TM-FS-017 | JS host filesystem widens the sandbox to embedder storage | `new Bash({ fs })` in the wasm bindings routes every VFS operation into embedder-supplied JS, so a script reaches whatever that object exposes | Paths are normalized by `PosixFs` before any host call, so traversal cannot select a path the embedder did not scope; the host object is the security boundary and is the embedder's to scope (mount root, allowlist, read-only). Reads and writes bypass the in-memory quotas — see the FS-quota scope note in §1 | **ACCEPTED** (embedder-scoped, opt-in) | +| TM-FS-017 | JS host filesystem widens the sandbox to embedder storage | `new Bash({ fs })` in the wasm bindings routes every VFS operation into embedder-supplied JS, so a script reaches whatever that object exposes | Paths are normalized by `PosixFs` before any host call, and `read_file` rejects host-reported symlinks before delegating to a potentially symlink-following host read. The host object remains the security boundary and is the embedder's to scope (mount root, allowlist, read-only). Reads and writes bypass the in-memory quotas — see the FS-quota scope note in §1 | **ACCEPTED** (embedder-scoped, opt-in) | **Current Risk**: MEDIUM - Two open escape vectors (TM-ESC-012, TM-ESC-013) need remediation