fix(security): prevent panics and add internal error handling

- Fix date builtin to validate strftime format strings before formatting,
  preventing chrono panic on invalid specifiers like %Q
- Add Internal error variant for unexpected failures with safe messaging
- Update threat model with TM-INT-xxx category for internal error handling:
  - TM-INT-001: Builtin panic recovery via catch_unwind
  - TM-INT-002: Panic info leak prevention (sanitized errors)
  - TM-INT-003: Date format validation
  - TM-INT-004/005/006: Error message safety
- Add comprehensive tests for date format validation and error handling
- Update interpreter comment to clarify panic catching applies to all builtins

The implementation ensures that:
1. Invalid inputs produce human-readable error messages
2. Panics are caught and converted to safe error responses
3. No internal details (paths, addresses, stack traces) are exposed
4. Scripts continue execution after recoverable errors

https://claude.ai/code/session_01XjPeQtrNSz8tEaRXgFuHnx

Author: claude

crates/bashkit/docs/threat-model.md

@@ -197,6 +197,35 @@ let tenant_b = Bash::builder()
 // tenant_a cannot access tenant_b's files or state
 ```
 
+### Internal Error Handling (TM-INT-*)
+
+BashKit is designed to never crash, even when processing malicious or malformed input.
+All unexpected errors are caught and converted to safe, human-readable messages.
+
+| Threat | Attack Example | Mitigation | Code Reference |
+|--------|---------------|------------|----------------|
+| Builtin panic (TM-INT-001) | Trigger panic in builtin | `catch_unwind` wrapper | [`interpreter/mod.rs`][interp] |
+| Info leak in panic (TM-INT-002) | Panic exposes secrets | Sanitized error messages | [`interpreter/mod.rs`][interp] |
+| Date format crash (TM-INT-003) | Invalid strftime: `+%Q` | Pre-validation | [`builtins/date.rs`][date] |
+
+**Panic Recovery:**
+
+All builtins (both built-in and custom) are wrapped with panic catching:
+
+```rust,ignore
+// If a builtin panics, the script continues with a sanitized error
+// The panic message is NOT exposed (may contain sensitive data)
+// Output: "bash: <command>: builtin failed unexpectedly"
+```
+
+**Error Message Safety:**
+
+Error messages never expose:
+- Stack traces or call stacks
+- Memory addresses
+- Real filesystem paths (only virtual paths)
+- Panic messages that may contain secrets
+
 ## Security Testing
 
 BashKit includes comprehensive security tests:
@@ -223,6 +252,7 @@ All threats use stable IDs in the format `TM-<CATEGORY>-<NUMBER>`:
 | TM-INJ | Injection |
 | TM-NET | Network Security |
 | TM-ISO | Multi-Tenant Isolation |
+| TM-INT | Internal Error Handling |
 
 Full threat analysis: [`specs/006-threat-model.md`][spec]
 
@@ -237,3 +267,5 @@ Full threat analysis: [`specs/006-threat-model.md`][spec]
 [network_tests]: https://github.com/anthropics/bashkit/blob/main/crates/bashkit/tests/network_security_tests.rs
 [fuzz]: https://github.com/anthropics/bashkit/tree/main/crates/bashkit/fuzz
 [spec]: https://github.com/anthropics/bashkit/blob/main/specs/006-threat-model.md
+[interp]: https://github.com/anthropics/bashkit/blob/main/crates/bashkit/src/interpreter/mod.rs
+[date]: https://github.com/anthropics/bashkit/blob/main/crates/bashkit/src/builtins/date.rs

crates/bashkit/src/builtins/date.rs

@@ -1,8 +1,14 @@
 //! Date builtin - display or format date and time
+//!
+//! SECURITY: Format strings are validated before use to prevent panics.
+//! Invalid format specifiers result in an error message, not a crash.
+//! Additionally, runtime format errors (e.g., timezone unavailable) are
+//! caught and return graceful errors.
 
 use std::fmt::Write;
 
 use async_trait::async_trait;
+use chrono::format::{Item, StrftimeItems};
 use chrono::{Local, Utc};
 
 use super::{Builtin, Context};
@@ -38,6 +44,21 @@ use crate::interpreter::ExecResult;
 ///   %%  Literal %
 pub struct Date;
 
+/// Validate a strftime format string.
+/// Returns Ok(()) if valid, or an error message describing the issue.
+///
+/// THREAT[TM-INT-003]: chrono::format() can panic on invalid format specifiers
+/// Mitigation: Pre-validate format string and return human-readable error
+fn validate_format(format: &str) -> std::result::Result<(), String> {
+    // StrftimeItems parses the format string and yields Item::Error for invalid specifiers
+    for item in StrftimeItems::new(format) {
+        if let Item::Error = item {
+            return Err(format!("invalid format string: '{}'", format));
+        }
+    }
+    Ok(())
+}
+
 #[async_trait]
 impl Builtin for Date {
     async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
@@ -49,6 +70,17 @@ impl Builtin for Date {
             None => "%a %b %e %H:%M:%S %Z %Y", // Default format like: "Mon Jan  1 12:00:00 UTC 2024"
         };
 
+        // SECURITY: Validate format string before use to prevent panics
+        // THREAT[TM-INT-003]: Invalid format strings could cause chrono to panic
+        if let Err(e) = validate_format(format) {
+            return Ok(ExecResult {
+                stdout: String::new(),
+                stderr: format!("date: {}\n", e),
+                exit_code: 1,
+                control_flow: crate::interpreter::ControlFlow::None,
+            });
+        }
+
         // Format the date, handling potential errors gracefully.
         // chrono's Display::fmt can return an error (e.g., when %Z timezone name
         // cannot be determined), which would cause to_string() to panic.
@@ -163,6 +195,7 @@ mod tests {
         assert_eq!(parts.len(), 3);
     }
 
+    // Tests from main: timezone handling
     #[tokio::test]
     async fn test_date_timezone_utc() {
         // %Z with UTC should always work and produce "UTC"
@@ -229,4 +262,30 @@ mod tests {
         assert_eq!(result.exit_code, 0);
         assert!(result.stdout.starts_with("Today is "));
     }
+
+    // Tests for invalid format validation (TM-INT-003)
+    #[tokio::test]
+    async fn test_date_invalid_format_specifier() {
+        // Invalid format specifier should return error, not panic
+        let result = run_date(&["+%Q"]).await;
+        assert_eq!(result.exit_code, 1);
+        assert!(result.stderr.contains("invalid format string"));
+        assert!(result.stdout.is_empty());
+    }
+
+    #[tokio::test]
+    async fn test_date_incomplete_format_specifier() {
+        // Incomplete specifier at end should return error, not panic
+        let result = run_date(&["+%Y-%m-%"]).await;
+        assert_eq!(result.exit_code, 1);
+        assert!(result.stderr.contains("invalid format string"));
+    }
+
+    #[tokio::test]
+    async fn test_date_mixed_valid_invalid_format() {
+        // Mix of valid and invalid should still error
+        let result = run_date(&["+%Y-%Q-%d"]).await;
+        assert_eq!(result.exit_code, 1);
+        assert!(result.stderr.contains("invalid format string"));
+    }
 }

crates/bashkit/src/error.rs

@@ -1,4 +1,9 @@
 //! Error types for BashKit
+//!
+//! This module provides error types for the interpreter with the following design goals:
+//! - Human-readable error messages for users
+//! - No leakage of sensitive information (paths, memory addresses, secrets)
+//! - Clear categorization for programmatic handling
 
 use crate::limits::LimitExceeded;
 use thiserror::Error;
@@ -7,6 +12,9 @@ use thiserror::Error;
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// BashKit error types.
+///
+/// All error messages are designed to be safe for display to end users without
+/// exposing internal details or sensitive information.
 #[derive(Error, Debug)]
 pub enum Error {
     /// Parse error occurred while parsing the script.
@@ -32,4 +40,20 @@ pub enum Error {
     /// Network error.
     #[error("network error: {0}")]
     Network(String),
+
+    /// Internal error for unexpected failures.
+    ///
+    /// THREAT[TM-INT-002]: Unexpected internal failures should not crash the interpreter.
+    /// This error type provides a human-readable message without exposing:
+    /// - Stack traces
+    /// - Memory addresses
+    /// - Internal file paths
+    /// - Panic messages that may contain sensitive data
+    ///
+    /// Use this for:
+    /// - Recovered panics that need to abort execution
+    /// - Logic errors that indicate a bug
+    /// - Security-sensitive failures where details should not be exposed
+    #[error("internal error: {0}")]
+    Internal(String),
 }

crates/bashkit/src/interpreter/mod.rs

@@ -1590,7 +1590,8 @@ impl Interpreter {
             };
 
             // Execute builtin with panic catching for security
-            // SECURITY: Custom builtins may panic - we catch this to:
+            // THREAT[TM-INT-001]: Builtins may panic on unexpected input
+            // SECURITY: All builtins (built-in and custom) may panic - we catch this to:
             // 1. Prevent interpreter crash
             // 2. Avoid leaking panic message (may contain sensitive info)
             // 3. Return sanitized error to user

crates/bashkit/src/tool.rs

@@ -441,6 +441,7 @@ fn error_kind(e: &Error) -> String {
         Error::CommandNotFound(_) => "command_not_found".to_string(),
         Error::ResourceLimit(_) => "resource_limit".to_string(),
         Error::Network(_) => "network_error".to_string(),
+        Error::Internal(_) => "internal_error".to_string(),
     }
 }
 

crates/bashkit/tests/builtin_error_security_tests.rs

@@ -966,3 +966,86 @@ async fn builtin_error_leaky_message_bad_pattern() {
         "Leaky builtin exposes internal details (this is BAD)"
     );
 }
+
+// ============================================================================
+// TM-INT-003: Date Format Validation Tests
+// ============================================================================
+
+/// Test that invalid date format specifiers return human-readable errors
+/// THREAT[TM-INT-003]: Invalid strftime formats could cause chrono to panic
+#[tokio::test]
+async fn date_invalid_format_returns_error_not_panic() {
+    let mut bash = Bash::new();
+
+    // %Q is not a valid strftime specifier
+    let result = bash.exec("date '+%Q'").await.unwrap();
+
+    // Should get an error, not a panic
+    assert_eq!(result.exit_code, 1);
+    assert!(
+        result.stderr.contains("invalid format string"),
+        "Should provide human-readable error: {}",
+        result.stderr
+    );
+    assert!(result.stdout.is_empty());
+}
+
+/// Test that incomplete format specifiers are handled gracefully
+#[tokio::test]
+async fn date_incomplete_format_returns_error() {
+    let mut bash = Bash::new();
+
+    // Trailing % is incomplete
+    let result = bash.exec("date '+%Y-%m-%'").await.unwrap();
+
+    assert_eq!(result.exit_code, 1);
+    assert!(
+        result.stderr.contains("invalid format string"),
+        "Should provide error for incomplete format: {}",
+        result.stderr
+    );
+}
+
+/// Test that valid formats still work correctly
+#[tokio::test]
+async fn date_valid_formats_work() {
+    let mut bash = Bash::new();
+
+    // ISO date format
+    let result = bash.exec("date '+%Y-%m-%d'").await.unwrap();
+    assert_eq!(result.exit_code, 0);
+    assert!(result.stdout.len() >= 10); // YYYY-MM-DD
+
+    // Unix timestamp
+    let result = bash.exec("date '+%s'").await.unwrap();
+    assert_eq!(result.exit_code, 0);
+    assert!(result.stdout.trim().parse::<i64>().is_ok());
+}
+
+/// Test that error messages don't expose implementation details
+#[tokio::test]
+async fn date_error_messages_are_safe() {
+    let mut bash = Bash::new();
+
+    let result = bash.exec("date '+%Q'").await.unwrap();
+
+    // Error should be user-friendly
+    assert!(!result.stderr.contains("panic"));
+    assert!(!result.stderr.contains("unwrap"));
+    assert!(!result.stderr.contains("chrono"));
+    assert!(!result.stderr.contains("0x")); // No memory addresses
+}
+
+/// Test date command in pipeline continues on error
+#[tokio::test]
+async fn date_error_allows_script_to_continue() {
+    let mut bash = Bash::new();
+
+    // Script should continue after date error (unless set -e)
+    let result = bash.exec("date '+%Q'; echo 'continued'").await.unwrap();
+
+    assert!(
+        result.stdout.contains("continued"),
+        "Script should continue after date error"
+    );
+}

specs/005-security-testing.md

@@ -148,7 +148,7 @@ fail_point!("module::function", |action| {
 
 - `crates/bashkit/tests/security_failpoint_tests.rs` - Fail-point security tests
 - `crates/bashkit/tests/threat_model_tests.rs` - Threat model tests (51 tests)
-- `crates/bashkit/tests/builtin_error_security_tests.rs` - Custom builtin error security tests (34 tests)
+- `crates/bashkit/tests/builtin_error_security_tests.rs` - Builtin error security tests (39 tests, includes TM-INT-003)
 - `crates/bashkit/src/limits.rs` - Resource limit fail points
 - `crates/bashkit/src/fs/memory.rs` - Filesystem fail points
 - `crates/bashkit/src/interpreter/mod.rs` - Interpreter fail points, panic catching

specs/006-threat-model.md

@@ -25,6 +25,7 @@ All threats use a stable ID format: `TM-<CATEGORY>-<NUMBER>`
 | TM-INJ | Injection | Command injection, path injection |
 | TM-NET | Network Security | DNS manipulation, HTTP attacks, network bypass |
 | TM-ISO | Isolation | Multi-tenant cross-access |
+| TM-INT | Internal Errors | Panic recovery, error message safety, unexpected failures |
 
 ### Adding New Threats
 
@@ -535,6 +536,62 @@ let tenant_b = Bash::builder()
 
 ---
 
+### 7. Internal Error Handling
+
+#### 7.1 Panic Recovery
+
+| ID | Threat | Attack Vector | Mitigation | Status |
+|----|--------|--------------|------------|--------|
+| TM-INT-001 | Builtin panic crash | Invalid input triggers panic in builtin | `catch_unwind` wrapper on all builtins | **MITIGATED** |
+| TM-INT-002 | Panic info leak | Panic message reveals sensitive data | Sanitized error messages (no panic details) | **MITIGATED** |
+| TM-INT-003 | Date format panic | Invalid strftime format causes chrono panic | Pre-validation with `StrftimeItems` | **MITIGATED** |
+
+**Current Risk**: LOW - All builtin panics are caught and converted to sanitized errors
+
+**Implementation**: `interpreter/mod.rs` - Panic catching for all builtins:
+```rust
+// THREAT[TM-INT-001]: Builtins may panic on unexpected input
+let result = AssertUnwindSafe(builtin.execute(ctx)).catch_unwind().await;
+
+match result {
+    Ok(Ok(exec_result)) => exec_result,
+    Ok(Err(e)) => return Err(e),
+    Err(_panic) => {
+        // THREAT[TM-INT-002]: Panic message may contain sensitive info
+        // Return sanitized error - never expose panic details
+        ExecResult::err(format!("bash: {}: builtin failed unexpectedly\n", name), 1)
+    }
+}
+```
+
+**Date Format Validation** (TM-INT-003): `builtins/date.rs`
+```rust
+// THREAT[TM-INT-003]: chrono::format() can panic on invalid format specifiers
+fn validate_format(format: &str) -> Result<(), String> {
+    for item in StrftimeItems::new(format) {
+        if let Item::Error = item {
+            return Err(format!("invalid format string: '{}'", format));
+        }
+    }
+    Ok(())
+}
+```
+
+#### 7.2 Error Message Safety
+
+| ID | Threat | Attack Vector | Mitigation | Status |
+|----|--------|--------------|------------|--------|
+| TM-INT-004 | Path leak in errors | Error shows real filesystem paths | Virtual paths only in messages | **MITIGATED** |
+| TM-INT-005 | Memory addr in errors | Debug output shows addresses | Display impl hides addresses | **MITIGATED** |
+| TM-INT-006 | Stack trace exposure | Panic unwinds show call stack | `catch_unwind` prevents propagation | **MITIGATED** |
+
+**Error Type Design**: `error.rs`
+- All error messages are designed for end-user display
+- `Internal` error variant for unexpected failures (never includes panic details)
+- Error types implement Display without exposing internals
+
+---
+
 ## Vulnerability Summary
 
 This section maps former vulnerability IDs to the new threat ID scheme and tracks status.
@@ -588,7 +645,9 @@ This section maps former vulnerability IDs to the new threat ID scheme and track
 | Network allowlist | TM-INF-010, TM-NET-001 to TM-NET-007 | `network/allowlist.rs` | Yes |
 | Sandboxed eval, no exec | TM-ESC-005 to TM-ESC-008, TM-INJ-003 | `interpreter/mod.rs` | Yes |
 | Fail-point testing | All controls | `security_failpoint_tests.rs` | Yes |
-| Builtin panic catching | Custom builtin safety | `interpreter/mod.rs` | Yes |
+| Builtin panic catching | TM-INT-001, TM-INT-002, TM-INT-006 | `interpreter/mod.rs` | Yes |
+| Date format validation | TM-INT-003 | `builtins/date.rs` | Yes |
+| Error message sanitization | TM-INT-004, TM-INT-005 | `error.rs` | Yes |
 | HTTP response size limit | TM-NET-008, TM-NET-012 | `network/client.rs` | Yes |
 | HTTP connect timeout | TM-NET-009 | `network/client.rs` | Yes |
 | HTTP read timeout | TM-NET-010 | `network/client.rs` | Yes |