From 231098ec1adf5d89e26e96928a2a204403610ca5 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 18 Mar 2026 23:41:40 +0000 Subject: [PATCH] refactor(fs): split FileSystem trait into core + FileSystemExt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract optional methods (usage, mkfifo, limits) from FileSystem into a new FileSystemExt supertrait. This reduces the implementation burden for custom filesystem backends — they only need an empty `impl FileSystemExt for T {}` to accept sensible defaults. FileSystem: 13 required POSIX methods (core contract) FileSystemExt: 3 optional methods with defaults (resource tracking, FIFOs) Closes #742 --- .../examples/custom_filesystem_impl.rs | 7 +- crates/bashkit/src/fs/memory.rs | 5 +- crates/bashkit/src/fs/mod.rs | 12 +- crates/bashkit/src/fs/mountable.rs | 5 +- crates/bashkit/src/fs/overlay.rs | 5 +- crates/bashkit/src/fs/posix.rs | 5 +- crates/bashkit/src/fs/traits.rs | 111 +++++++++++------- crates/bashkit/src/lib.rs | 4 +- crates/bashkit/tests/custom_fs_tests.rs | 6 +- crates/bashkit/tests/dev_null_tests.rs | 10 +- crates/bashkit/tests/threat_model_tests.rs | 4 +- 11 files changed, 118 insertions(+), 56 deletions(-) diff --git a/crates/bashkit/examples/custom_filesystem_impl.rs b/crates/bashkit/examples/custom_filesystem_impl.rs index c1d3c2f7..9c985b80 100644 --- a/crates/bashkit/examples/custom_filesystem_impl.rs +++ b/crates/bashkit/examples/custom_filesystem_impl.rs @@ -6,7 +6,9 @@ //! //! Run with: cargo run --example custom_filesystem_impl -use bashkit::{Bash, DirEntry, Error, FileSystem, FileType, Metadata, Result, async_trait}; +use bashkit::{ + Bash, DirEntry, Error, FileSystem, FileSystemExt, FileType, Metadata, Result, async_trait, +}; use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; @@ -101,6 +103,9 @@ impl SessionFileSystemAdapter { } // The async_trait macro is re-exported from bashkit for convenience +#[async_trait] +impl FileSystemExt for SessionFileSystemAdapter {} + #[async_trait] impl FileSystem for SessionFileSystemAdapter { async fn read_file(&self, path: &Path) -> Result> { diff --git a/crates/bashkit/src/fs/memory.rs b/crates/bashkit/src/fs/memory.rs index 80285d55..6f667005 100644 --- a/crates/bashkit/src/fs/memory.rs +++ b/crates/bashkit/src/fs/memory.rs @@ -46,7 +46,7 @@ use std::sync::RwLock; use std::time::SystemTime; use super::limits::{FsLimits, FsUsage}; -use super::traits::{DirEntry, FileSystem, FileType, Metadata}; +use super::traits::{DirEntry, FileSystem, FileSystemExt, FileType, Metadata}; use crate::error::Result; #[cfg(feature = "failpoints")] @@ -1330,7 +1330,10 @@ impl FileSystem for InMemoryFs { None => Err(IoError::new(ErrorKind::NotFound, "not found").into()), } } +} +#[async_trait] +impl FileSystemExt for InMemoryFs { async fn mkfifo(&self, path: &Path, mode: u32) -> Result<()> { self.limits .validate_path(path) diff --git a/crates/bashkit/src/fs/mod.rs b/crates/bashkit/src/fs/mod.rs index 04075136..84e1918b 100644 --- a/crates/bashkit/src/fs/mod.rs +++ b/crates/bashkit/src/fs/mod.rs @@ -60,11 +60,14 @@ //! Best for: complex behavior, custom caching, specialized semantics. //! //! ```rust,ignore -//! use bashkit::{async_trait, FileSystem, Bash}; +//! use bashkit::{async_trait, FileSystem, FileSystemExt, Bash}; //! //! struct MyFs { /* ... */ } //! //! #[async_trait] +//! impl FileSystemExt for MyFs {} +//! +//! #[async_trait] //! impl FileSystem for MyFs { //! async fn write_file(&self, path: &Path, content: &[u8]) -> Result<()> { //! // You MUST check: is path a directory? @@ -294,7 +297,7 @@ //! Implement the [`FileSystem`] trait to create custom storage backends: //! //! ```rust -//! use bashkit::{async_trait, FileSystem, DirEntry, Metadata, FileType, Result, Error}; +//! use bashkit::{async_trait, FileSystem, FileSystemExt, DirEntry, Metadata, FileType, Result, Error}; //! use std::path::{Path, PathBuf}; //! use std::collections::HashMap; //! use std::sync::RwLock; @@ -316,6 +319,9 @@ //! } //! //! #[async_trait] +//! impl FileSystemExt for SimpleFs {} +//! +//! #[async_trait] //! impl FileSystem for SimpleFs { //! async fn read_file(&self, path: &Path) -> Result> { //! let files = self.files.read().unwrap(); @@ -408,7 +414,7 @@ pub use posix::PosixFs; #[cfg(feature = "realfs")] pub use realfs::{RealFs, RealFsMode}; #[allow(unused_imports)] -pub use traits::{DirEntry, FileSystem, FileType, Metadata, fs_errors}; +pub use traits::{DirEntry, FileSystem, FileSystemExt, FileType, Metadata, fs_errors}; use crate::error::Result; use std::io::{Error as IoError, ErrorKind}; diff --git a/crates/bashkit/src/fs/mountable.rs b/crates/bashkit/src/fs/mountable.rs index 2f16624a..46f552d4 100644 --- a/crates/bashkit/src/fs/mountable.rs +++ b/crates/bashkit/src/fs/mountable.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; use super::limits::{FsLimits, FsUsage}; -use super::traits::{DirEntry, FileSystem, FileType, Metadata}; +use super::traits::{DirEntry, FileSystem, FileSystemExt, FileType, Metadata}; use crate::error::Result; use std::io::ErrorKind; @@ -467,7 +467,10 @@ impl FileSystem for MountableFs { let (fs, resolved) = self.resolve(path); fs.chmod(&resolved, mode).await } +} +#[async_trait] +impl FileSystemExt for MountableFs { fn usage(&self) -> FsUsage { // Aggregate usage from root and all mounts let mut total = self.root.usage(); diff --git a/crates/bashkit/src/fs/overlay.rs b/crates/bashkit/src/fs/overlay.rs index eced9891..50812702 100644 --- a/crates/bashkit/src/fs/overlay.rs +++ b/crates/bashkit/src/fs/overlay.rs @@ -20,7 +20,7 @@ use std::sync::{Arc, RwLock}; use super::limits::{FsLimits, FsUsage}; use super::memory::InMemoryFs; -use super::traits::{DirEntry, FileSystem, FileType, Metadata}; +use super::traits::{DirEntry, FileSystem, FileSystemExt, FileType, Metadata}; use crate::error::Result; /// Copy-on-write overlay filesystem. @@ -795,7 +795,10 @@ impl FileSystem for OverlayFs { Err(IoError::new(ErrorKind::NotFound, "not found").into()) } +} +#[async_trait] +impl FileSystemExt for OverlayFs { fn usage(&self) -> FsUsage { self.compute_usage() } diff --git a/crates/bashkit/src/fs/posix.rs b/crates/bashkit/src/fs/posix.rs index 0f7f538a..9a751b1f 100644 --- a/crates/bashkit/src/fs/posix.rs +++ b/crates/bashkit/src/fs/posix.rs @@ -53,7 +53,7 @@ use std::sync::Arc; use super::backend::FsBackend; use super::limits::{FsLimits, FsUsage}; -use super::traits::{DirEntry, FileSystem, Metadata, fs_errors}; +use super::traits::{DirEntry, FileSystem, FileSystemExt, Metadata, fs_errors}; use crate::error::Result; /// POSIX-compatible filesystem wrapper. @@ -234,7 +234,10 @@ impl FileSystem for PosixFs { async fn chmod(&self, path: &Path, mode: u32) -> Result<()> { self.backend.chmod(path, mode).await } +} +#[async_trait] +impl FileSystemExt for PosixFs { fn usage(&self) -> FsUsage { self.backend.usage() } diff --git a/crates/bashkit/src/fs/traits.rs b/crates/bashkit/src/fs/traits.rs index a904a60d..023fa324 100644 --- a/crates/bashkit/src/fs/traits.rs +++ b/crates/bashkit/src/fs/traits.rs @@ -101,10 +101,68 @@ pub mod fs_errors { } } +/// Optional filesystem extensions for resource tracking and special file types. +/// +/// This trait provides methods that most custom filesystem implementations do not +/// need to override. All methods have sensible defaults: +/// +/// - [`usage()`](FileSystemExt::usage) returns zero usage +/// - [`limits()`](FileSystemExt::limits) returns unlimited +/// - [`mkfifo()`](FileSystemExt::mkfifo) returns "not supported" +/// +/// Built-in implementations (`InMemoryFs`, `OverlayFs`, `MountableFs`) override +/// these to provide real statistics. Custom backends can opt in by implementing +/// just the methods they need. +/// +/// `FileSystemExt` is a supertrait of [`FileSystem`], so its methods are +/// available on any `dyn FileSystem` trait object. +#[async_trait] +pub trait FileSystemExt: Send + Sync { + /// Get current filesystem usage statistics. + /// + /// Returns total bytes used, file count, and directory count. + /// Used by `du` and `df` builtins. + /// + /// # Default Implementation + /// + /// Returns zeros. Implementations should override for accurate stats. + fn usage(&self) -> FsUsage { + FsUsage::default() + } + + /// Create a named pipe (FIFO) at the given path. + /// + /// FIFOs are simulated as buffered files in the virtual filesystem. + /// Reading from a FIFO returns its buffered content, writing appends to it. + /// + /// # Default Implementation + /// + /// Returns "not supported" error. Override in implementations that support FIFOs. + async fn mkfifo(&self, _path: &Path, _mode: u32) -> Result<()> { + Err(std::io::Error::other("mkfifo not supported").into()) + } + + /// Get filesystem limits. + /// + /// Returns the configured limits for this filesystem. + /// Used by `df` builtin to show available space. + /// + /// # Default Implementation + /// + /// Returns unlimited limits. + fn limits(&self) -> FsLimits { + FsLimits::unlimited() + } +} + /// Async virtual filesystem trait. /// -/// This trait defines the interface for all filesystem implementations in Bashkit. -/// Implement this trait to create custom storage backends. +/// This trait defines the core interface for all filesystem implementations in +/// Bashkit. It contains only the essential POSIX-like operations. Optional +/// methods for resource tracking and special file types live in +/// [`FileSystemExt`], which is a supertrait — so all `FileSystem` implementors +/// must also implement `FileSystemExt` (usually just `impl FileSystemExt for T {}` +/// to accept the defaults). /// /// # Thread Safety /// @@ -114,15 +172,19 @@ pub mod fs_errors { /// /// # Implementing FileSystem /// -/// To create a custom filesystem, implement all methods in this trait. +/// To create a custom filesystem, implement all methods in this trait and +/// add an empty `FileSystemExt` impl (or override specific extension methods). /// See `examples/custom_filesystem_impl.rs` for a complete implementation. /// /// ```rust,ignore -/// use bashkit::{async_trait, FileSystem, Result}; +/// use bashkit::{async_trait, FileSystem, FileSystemExt, Result}; /// /// pub struct MyFileSystem { /* ... */ } /// /// #[async_trait] +/// impl FileSystemExt for MyFileSystem {} +/// +/// #[async_trait] /// impl FileSystem for MyFileSystem { /// async fn read_file(&self, path: &Path) -> Result> { /// // Your implementation @@ -151,7 +213,7 @@ pub mod fs_errors { /// - [`OverlayFs`](crate::OverlayFs) - Copy-on-write layered filesystem /// - [`MountableFs`](crate::MountableFs) - Multiple mount points #[async_trait] -pub trait FileSystem: Send + Sync { +pub trait FileSystem: FileSystemExt { /// Read a file's contents as bytes. /// /// # Errors @@ -287,42 +349,6 @@ pub trait FileSystem: Send + Sync { /// /// Returns an error if the path does not exist. async fn chmod(&self, path: &Path, mode: u32) -> Result<()>; - - /// Get current filesystem usage statistics. - /// - /// Returns total bytes used, file count, and directory count. - /// Used by `du` and `df` builtins. - /// - /// # Default Implementation - /// - /// Returns zeros. Implementations should override for accurate stats. - fn usage(&self) -> FsUsage { - FsUsage::default() - } - - /// Create a named pipe (FIFO) at the given path. - /// - /// FIFOs are simulated as buffered files in the virtual filesystem. - /// Reading from a FIFO returns its buffered content, writing appends to it. - /// - /// # Default Implementation - /// - /// Returns "not supported" error. Override in implementations that support FIFOs. - async fn mkfifo(&self, _path: &Path, _mode: u32) -> Result<()> { - Err(std::io::Error::other("mkfifo not supported").into()) - } - - /// Get filesystem limits. - /// - /// Returns the configured limits for this filesystem. - /// Used by `df` builtin to show available space. - /// - /// # Default Implementation - /// - /// Returns unlimited limits. - fn limits(&self) -> FsLimits { - FsLimits::unlimited() - } } /// File or directory metadata. @@ -652,6 +678,9 @@ mod tests { // Test via a minimal struct that only implements the defaults struct Dummy; + #[async_trait] + impl FileSystemExt for Dummy {} + #[async_trait] impl FileSystem for Dummy { async fn read_file(&self, _: &Path) -> crate::error::Result> { diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index eca63bea..7ad7bb5e 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -419,8 +419,8 @@ pub use async_trait::async_trait; pub use builtins::{Builtin, Context as BuiltinContext}; pub use error::{Error, Result}; pub use fs::{ - DirEntry, FileSystem, FileType, FsBackend, FsLimitExceeded, FsLimits, FsUsage, InMemoryFs, - Metadata, MountableFs, OverlayFs, PosixFs, VfsSnapshot, normalize_path, + DirEntry, FileSystem, FileSystemExt, FileType, FsBackend, FsLimitExceeded, FsLimits, FsUsage, + InMemoryFs, Metadata, MountableFs, OverlayFs, PosixFs, VfsSnapshot, normalize_path, verify_filesystem_requirements, }; #[cfg(feature = "realfs")] diff --git a/crates/bashkit/tests/custom_fs_tests.rs b/crates/bashkit/tests/custom_fs_tests.rs index 69802f83..42029aab 100644 --- a/crates/bashkit/tests/custom_fs_tests.rs +++ b/crates/bashkit/tests/custom_fs_tests.rs @@ -4,7 +4,8 @@ //! are properly exported from the crate's public API. use bashkit::{ - Bash, DirEntry, Error, FileSystem, FileType, InMemoryFs, Metadata, Result, async_trait, + Bash, DirEntry, Error, FileSystem, FileSystemExt, FileType, InMemoryFs, Metadata, Result, + async_trait, }; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -48,6 +49,9 @@ impl MinimalFs { } } +#[async_trait] +impl FileSystemExt for MinimalFs {} + #[async_trait] impl FileSystem for MinimalFs { async fn read_file(&self, path: &Path) -> Result> { diff --git a/crates/bashkit/tests/dev_null_tests.rs b/crates/bashkit/tests/dev_null_tests.rs index 45c70a36..96639728 100644 --- a/crates/bashkit/tests/dev_null_tests.rs +++ b/crates/bashkit/tests/dev_null_tests.rs @@ -6,8 +6,8 @@ //! Security invariant: Redirects to /dev/null NEVER reach the filesystem layer. use bashkit::{ - Bash, DirEntry, Error, FileSystem, FileType, InMemoryFs, Metadata, MountableFs, OverlayFs, - Result, async_trait, + Bash, DirEntry, Error, FileSystem, FileSystemExt, FileType, InMemoryFs, Metadata, MountableFs, + OverlayFs, Result, async_trait, }; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -152,6 +152,9 @@ impl TrackingFs { } } +#[async_trait] +impl FileSystemExt for TrackingFs {} + #[async_trait] impl FileSystem for TrackingFs { async fn read_file(&self, path: &Path) -> Result> { @@ -376,6 +379,9 @@ impl MaliciousDevFs { } } +#[async_trait] +impl FileSystemExt for MaliciousDevFs {} + #[async_trait] impl FileSystem for MaliciousDevFs { async fn read_file(&self, path: &Path) -> Result> { diff --git a/crates/bashkit/tests/threat_model_tests.rs b/crates/bashkit/tests/threat_model_tests.rs index bd9858d0..959636b7 100644 --- a/crates/bashkit/tests/threat_model_tests.rs +++ b/crates/bashkit/tests/threat_model_tests.rs @@ -6,8 +6,8 @@ //! Run with: `cargo test threat_` use bashkit::{ - Bash, ExecutionLimits, FileSystem, FsLimits, InMemoryFs, MemoryLimits, OverlayFs, - SessionLimits, TraceEventDetails, TraceEventKind, TraceMode, + Bash, ExecutionLimits, FileSystem, FileSystemExt, FsLimits, InMemoryFs, MemoryLimits, + OverlayFs, SessionLimits, TraceEventDetails, TraceEventKind, TraceMode, }; use std::path::Path; use std::sync::Arc;