From 0d4f42079773c7402c0d88e90ea0cb8760251542 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Mar 2026 03:05:08 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(security):=20batch=202=20=E2=80=94=20is?= =?UTF-8?q?sues=20#493-#497?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #493/#494: Cap brace expansion ranges at 10K elements and total expansion at 100K strings (TM-DOS-041/042) - #495: VFS copy() now enforces size limits on overwrite; rename() rejects file-over-directory per POSIX (TM-DOS-047/048) - #496: OverlayFs::symlink() validates paths and enforces file count limits; symlinks now count toward file_count (TM-DOS-045) - #497: date builtin uses configurable fixed epoch instead of real host clock (TM-INF-018) All 17 security audit regression tests pass. https://claude.ai/code/session_01TTiLUJVtmMNAo1NC9aQTn1 --- crates/bashkit/src/builtins/date.rs | 40 ++++++++++++---- crates/bashkit/src/fs/memory.rs | 22 ++++++--- crates/bashkit/src/fs/overlay.rs | 8 ++++ crates/bashkit/src/interpreter/mod.rs | 51 ++++++++++++++++++--- crates/bashkit/src/lib.rs | 22 ++++++++- crates/bashkit/tests/security_audit_pocs.rs | 45 +++++++++--------- 6 files changed, 143 insertions(+), 45 deletions(-) diff --git a/crates/bashkit/src/builtins/date.rs b/crates/bashkit/src/builtins/date.rs index ac73fcee..5acfda61 100644 --- a/crates/bashkit/src/builtins/date.rs +++ b/crates/bashkit/src/builtins/date.rs @@ -45,7 +45,28 @@ use crate::interpreter::ExecResult; /// %n Newline /// %t Tab /// %% Literal % -pub struct Date; +/// THREAT[TM-INF-018]: Supports a fixed epoch to prevent leaking real host time. +pub struct Date { + /// Fixed UTC epoch for virtualized time. None = use real system clock. + fixed_epoch: Option>, +} + +impl Date { + pub fn new() -> Self { + Self { fixed_epoch: None } + } + + /// Create a Date builtin with a fixed epoch (for sandboxing). + pub fn with_fixed_epoch(epoch: DateTime) -> Self { + Self { + fixed_epoch: Some(epoch), + } + } + + fn now(&self) -> DateTime { + self.fixed_epoch.unwrap_or_else(Utc::now) + } +} /// Validate a strftime format string. /// Returns Ok(()) if valid, or an error message describing the issue. @@ -74,8 +95,7 @@ fn strip_surrounding_quotes(s: &str) -> &str { } /// Parse a base date expression (no compound modifiers). -fn parse_base_date(s: &str) -> std::result::Result, String> { - let now = Utc::now(); +fn parse_base_date(s: &str, now: DateTime) -> std::result::Result, String> { let lower = s.to_lowercase(); // Epoch timestamp: @1234567890 @@ -135,7 +155,7 @@ fn parse_base_date(s: &str) -> std::result::Result, String> { /// Supports compound expressions (base ± modifier): /// "2024-01-15 + 30 days", "yesterday - 2 hours", /// "@1700000000 + 1 week", "2024-01-15 - 1 month" -fn parse_date_string(s: &str) -> std::result::Result, String> { +fn parse_date_string(s: &str, now: DateTime) -> std::result::Result, String> { let s = strip_surrounding_quotes(s.trim()); // Try compound expression: [+-] @@ -156,7 +176,7 @@ fn parse_date_string(s: &str) -> std::result::Result, String> { // Use original case for base string to handle epoch (@N) // and ISO dates correctly. let orig_base = s[..base_match.end()].trim(); - if let Ok(base_dt) = parse_base_date(orig_base) { + if let Ok(base_dt) = parse_base_date(orig_base, now) { let offset = unit_duration(unit, sign * n); return Ok(base_dt + offset); } @@ -164,7 +184,7 @@ fn parse_date_string(s: &str) -> std::result::Result, String> { } } - parse_base_date(s) + parse_base_date(s, now) } /// Parse relative date expressions like "30 days ago", "+2 weeks", "-1 month" @@ -321,13 +341,15 @@ impl Builtin for Date { } // Get the datetime to format + // THREAT[TM-INF-018]: Use virtual time if configured + let now = self.now(); let dt_utc = if let Some(ref ds) = date_str { - match parse_date_string(ds) { + match parse_date_string(ds, now) { Ok(dt) => dt, Err(e) => return Ok(ExecResult::err(format!("{}\n", e), 1)), } } else { - Utc::now() + now }; // Handle -R (RFC 2822) output @@ -417,7 +439,7 @@ mod tests { git_client: None, }; - Date.execute(ctx).await.unwrap() + Date::new().execute(ctx).await.unwrap() } #[tokio::test] diff --git a/crates/bashkit/src/fs/memory.rs b/crates/bashkit/src/fs/memory.rs index 9269d00d..0e473286 100644 --- a/crates/bashkit/src/fs/memory.rs +++ b/crates/bashkit/src/fs/memory.rs @@ -385,7 +385,8 @@ impl InMemoryFs { dir_count += 1; } FsEntry::Symlink { .. } => { - // Symlinks don't count toward file count or size + // THREAT[TM-DOS-045]: Symlinks count toward file count + file_count += 1; } } } @@ -1148,6 +1149,17 @@ impl FileSystem for InMemoryFs { .remove(&from) .ok_or_else(|| IoError::new(ErrorKind::NotFound, "not found"))?; + // THREAT[TM-DOS-048]: Reject renaming a file over a directory (POSIX requirement) + if matches!(&entry, FsEntry::File { .. } | FsEntry::Symlink { .. }) + && matches!(entries.get(&to), Some(FsEntry::Directory { .. })) + { + // Put back the source entry + entries.insert(from, entry); + return Err( + IoError::other("cannot rename file over directory").into(), + ); + } + entries.insert(to, entry); Ok(()) } @@ -1168,15 +1180,13 @@ impl FileSystem for InMemoryFs { .cloned() .ok_or_else(|| IoError::new(ErrorKind::NotFound, "not found"))?; - // Check write limits before creating the copy + // THREAT[TM-DOS-047]: Always check write limits, even on overwrite. + // check_write_limits handles the delta calculation for existing files. let entry_size = match &entry { FsEntry::File { content, .. } => content.len() as u64, _ => 0, }; - let is_new = !entries.contains_key(&to); - if is_new { - self.check_write_limits(&entries, &to, entry_size as usize)?; - } + self.check_write_limits(&entries, &to, entry_size as usize)?; entries.insert(to, entry); Ok(()) diff --git a/crates/bashkit/src/fs/overlay.rs b/crates/bashkit/src/fs/overlay.rs index d21c8dac..b5f0bba8 100644 --- a/crates/bashkit/src/fs/overlay.rs +++ b/crates/bashkit/src/fs/overlay.rs @@ -681,8 +681,16 @@ impl FileSystem for OverlayFs { } async fn symlink(&self, target: &Path, link: &Path) -> Result<()> { + // THREAT[TM-DOS-045]: Validate path and enforce limits like other write methods + self.limits + .validate_path(link) + .map_err(|e| IoError::other(e.to_string()))?; + let link = Self::normalize_path(link); + // Check write limits (symlinks count toward file count) + self.check_write_limits(0)?; + // Remove any whiteout self.remove_whiteout(&link); diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 3028db50..dc8aa46f 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -312,7 +312,7 @@ impl Interpreter { /// Create a new interpreter with the given filesystem. pub fn new(fs: Arc) -> Self { - Self::with_config(fs, None, None, HashMap::new()) + Self::with_config(fs, None, None, None, HashMap::new()) } /// Create a new interpreter with custom username, hostname, and builtins. @@ -327,6 +327,7 @@ impl Interpreter { fs: Arc, username: Option, hostname: Option, + fixed_epoch: Option, custom_builtins: HashMap>, ) -> Self { let mut builtins: HashMap> = HashMap::new(); @@ -408,7 +409,18 @@ impl Interpreter { builtins.insert("uniq".to_string(), Box::new(builtins::Uniq)); builtins.insert("cut".to_string(), Box::new(builtins::Cut)); builtins.insert("tr".to_string(), Box::new(builtins::Tr)); - builtins.insert("date".to_string(), Box::new(builtins::Date)); + // THREAT[TM-INF-018]: Use fixed epoch if configured, else real clock + builtins.insert( + "date".to_string(), + Box::new(if let Some(epoch) = fixed_epoch { + use chrono::DateTime; + builtins::Date::with_fixed_epoch( + DateTime::from_timestamp(epoch, 0).unwrap_or_default(), + ) + } else { + builtins::Date::new() + }), + ); builtins.insert("wait".to_string(), Box::new(builtins::Wait)); builtins.insert("curl".to_string(), Box::new(builtins::Curl)); builtins.insert("wget".to_string(), Box::new(builtins::Wget)); @@ -7995,7 +8007,18 @@ impl Interpreter { /// Check if a string contains glob characters /// Expand brace patterns like {a,b,c} or {1..5} /// Returns a Vec of expanded strings, or a single-element Vec if no braces + /// THREAT[TM-DOS-042]: Cap total expansion count to prevent combinatorial OOM. fn expand_braces(&self, s: &str) -> Vec { + const MAX_BRACE_EXPANSION_TOTAL: usize = 100_000; + let mut count = 0; + self.expand_braces_capped(s, &mut count, MAX_BRACE_EXPANSION_TOTAL) + } + + fn expand_braces_capped(&self, s: &str, count: &mut usize, max: usize) -> Vec { + if *count >= max { + return vec![s.to_string()]; + } + // Find the first brace that has a matching close brace let mut depth = 0; let mut brace_start = None; @@ -8040,9 +8063,13 @@ impl Interpreter { if let Some(range_result) = self.try_expand_range(&brace_content) { let mut results = Vec::new(); for item in range_result { + if *count >= max { + break; + } let expanded = format!("{}{}{}", prefix, item, suffix); - // Recursively expand any remaining braces - results.extend(self.expand_braces(&expanded)); + let sub = self.expand_braces_capped(&expanded, count, max); + *count += sub.len(); + results.extend(sub); } return results; } @@ -8057,16 +8084,24 @@ impl Interpreter { let mut results = Vec::new(); for item in items { + if *count >= max { + break; + } let expanded = format!("{}{}{}", prefix, item, suffix); - // Recursively expand any remaining braces - results.extend(self.expand_braces(&expanded)); + let sub = self.expand_braces_capped(&expanded, count, max); + *count += sub.len(); + results.extend(sub); } results } /// Try to expand a range like 1..5 or a..z + /// THREAT[TM-DOS-041]: Cap range size to prevent OOM from {1..999999999} fn try_expand_range(&self, content: &str) -> Option> { + /// Maximum number of elements in a brace range expansion + const MAX_BRACE_RANGE: u64 = 10_000; + // Check for .. separator let parts: Vec<&str> = content.split("..").collect(); if parts.len() != 2 { @@ -8078,6 +8113,10 @@ impl Interpreter { // Try numeric range if let (Ok(start_num), Ok(end_num)) = (start.parse::(), end.parse::()) { + let range_size = (end_num as i128 - start_num as i128).unsigned_abs() + 1; + if range_size > MAX_BRACE_RANGE as u128 { + return None; // Treat as literal — too large + } let mut results = Vec::new(); if start_num <= end_num { for i in start_num..=end_num { diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index 6a747ba5..6754bd40 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -757,6 +757,8 @@ pub struct BashBuilder { limits: ExecutionLimits, username: Option, hostname: Option, + /// Fixed epoch for virtualizing the `date` builtin (TM-INF-018) + fixed_epoch: Option, custom_builtins: HashMap>, /// Files to mount in the virtual filesystem mounted_files: Vec, @@ -813,6 +815,15 @@ impl BashBuilder { self } + /// Set a fixed Unix epoch for the `date` builtin. + /// + /// THREAT[TM-INF-018]: Prevents `date` from leaking real host time. + /// When set, `date` returns this fixed time instead of the real clock. + pub fn fixed_epoch(mut self, epoch: i64) -> Self { + self.fixed_epoch = Some(epoch); + self + } + /// Configure network access for curl/wget builtins. /// /// Network access is disabled by default. Use this method to enable HTTP @@ -1151,6 +1162,7 @@ impl BashBuilder { self.env, self.username, self.hostname, + self.fixed_epoch, self.cwd, self.limits, self.custom_builtins, @@ -1170,6 +1182,7 @@ impl BashBuilder { env: HashMap, username: Option, hostname: Option, + fixed_epoch: Option, cwd: Option, limits: ExecutionLimits, custom_builtins: HashMap>, @@ -1188,8 +1201,13 @@ impl BashBuilder { "Bash instance configured" ); - let mut interpreter = - Interpreter::with_config(Arc::clone(&fs), username.clone(), hostname, custom_builtins); + let mut interpreter = Interpreter::with_config( + Arc::clone(&fs), + username.clone(), + hostname, + fixed_epoch, + custom_builtins, + ); // Set environment variables (also override shell variable defaults) for (key, value) in &env { diff --git a/crates/bashkit/tests/security_audit_pocs.rs b/crates/bashkit/tests/security_audit_pocs.rs index 633b2bbf..b79194ea 100644 --- a/crates/bashkit/tests/security_audit_pocs.rs +++ b/crates/bashkit/tests/security_audit_pocs.rs @@ -320,7 +320,6 @@ mod vfs_limit_bypass { /// TM-DOS-047: copy() must enforce limits even when destination exists. #[tokio::test] - #[ignore = "TM-DOS-047: copy() skips check_write_limits on existing dest"] async fn security_audit_copy_enforces_limit_on_overwrite() { let limits = FsLimits::new() .max_total_bytes(600) @@ -347,7 +346,6 @@ mod vfs_limit_bypass { /// TM-DOS-048: rename(file, dir) must fail per POSIX, not silently overwrite. #[tokio::test] - #[ignore = "TM-DOS-048: rename(file, dir) silently overwrites, orphans children"] async fn security_audit_rename_rejects_file_over_dir() { let fs = InMemoryFs::new(); @@ -379,10 +377,11 @@ mod overlay_symlink_bypass { /// TM-DOS-045: OverlayFs::symlink() must enforce file count limits. #[tokio::test] - #[ignore = "TM-DOS-045: OverlayFs::symlink() bypasses file count limit"] async fn security_audit_overlay_symlink_enforces_limit() { let lower: Arc = Arc::new(InMemoryFs::new()); - let limits = FsLimits::new().max_file_count(5); + // Both lower and upper InMemoryFs have /dev/null (1 file each = 2 base). + // limit=7 allows 5 new symlinks (2 + 5 = 7). + let limits = FsLimits::new().max_file_count(7); let overlay = OverlayFs::with_limits(lower, limits); for i in 0..5 { @@ -393,7 +392,7 @@ mod overlay_symlink_bypass { .unwrap(); } - // 6th must fail + // 6th must fail (7 total = 2 existing + 5 symlinks = at limit) let result = overlay .symlink(Path::new("/target"), Path::new("/link_overflow")) .await; @@ -416,11 +415,13 @@ mod information_disclosure { /// TM-INF-018: `date` should use a configurable/virtual time source. #[tokio::test] - #[ignore = "TM-INF-018: date uses real host clock, not virtualized"] async fn security_audit_date_uses_virtual_time() { + // Fixed epoch: 2020-01-01 00:00:00 UTC + let fixed = 1577836800i64; let mut bash = Bash::builder() .username("sandboxuser") .hostname("sandbox.local") + .fixed_epoch(fixed) .build(); // hostname and whoami are virtualized @@ -429,17 +430,12 @@ mod information_disclosure { let result = bash.exec("date +%s").await.unwrap(); let script_epoch: i64 = result.stdout.trim().parse().unwrap_or(0); - let real_epoch = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64; - // date must NOT return real host time - assert!( - (script_epoch - real_epoch).abs() >= 10, - "date must use virtual time, not real host epoch (script={} real={})", - script_epoch, - real_epoch + // date must return the fixed epoch, not real host time + assert_eq!( + script_epoch, fixed, + "date must use fixed epoch, not real host clock (got={})", + script_epoch ); } } @@ -455,20 +451,25 @@ mod brace_expansion_dos { use super::*; /// TM-DOS-041: Brace expansion {1..N} must cap range size. + /// Ranges exceeding 10,000 elements are treated as literals. #[tokio::test] - #[ignore = "TM-DOS-041: brace expansion {N..M} has no upper bound"] async fn security_audit_brace_expansion_capped() { let limits = ExecutionLimits::new() .max_commands(100) .timeout(Duration::from_secs(10)); let mut bash = Bash::builder().limits(limits).build(); - // {1..1000000} should be rejected or capped, not expand to 1M strings - let result = bash.exec("echo {1..1000000} > /dev/null").await; - assert!( - result.is_err(), - "Brace expansion with 1M elements must be rejected" + // {1..1000000} exceeds cap — treated as literal, not expanded to 1M strings + let result = bash.exec("echo {1..1000000}").await.unwrap(); + assert_eq!( + result.stdout.trim(), + "{1..1000000}", + "Brace expansion with 1M elements must be treated as literal" ); + + // {1..100} is within cap — should expand normally + let result = bash.exec("echo {1..3}").await.unwrap(); + assert_eq!(result.stdout.trim(), "1 2 3"); } } From 63c305065087014be6bfe2bca5747e7fcab0295e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Mar 2026 03:13:34 +0000 Subject: [PATCH 2/2] style: fix formatting in memory.rs rename check https://claude.ai/code/session_01TTiLUJVtmMNAo1NC9aQTn1 --- crates/bashkit/src/fs/memory.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/bashkit/src/fs/memory.rs b/crates/bashkit/src/fs/memory.rs index 0e473286..abc8f63b 100644 --- a/crates/bashkit/src/fs/memory.rs +++ b/crates/bashkit/src/fs/memory.rs @@ -1155,9 +1155,7 @@ impl FileSystem for InMemoryFs { { // Put back the source entry entries.insert(from, entry); - return Err( - IoError::other("cannot rename file over directory").into(), - ); + return Err(IoError::other("cannot rename file over directory").into()); } entries.insert(to, entry);