From ebb6cc4509c8bb3f325dbe48c79341c96d6d6f4c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 2 Apr 2026 22:22:38 +0000 Subject: [PATCH] fix(snapshot): add integrity verification and preserve execution limits - Add SHA-256 digest to snapshot serialization to detect tampering - Tampered or truncated snapshots are rejected with clear error - Fix JS bindings from_snapshot() to accept BashOptions for limits - from_snapshot() now builds a properly configured instance via build_bash() then restores snapshot state, preserving limits Closes #977 Closes #978 --- crates/bashkit-js/src/lib.rs | 42 +++++++++++---- crates/bashkit/src/snapshot.rs | 50 ++++++++++++++++-- crates/bashkit/tests/snapshot_tests.rs | 73 ++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 14 deletions(-) diff --git a/crates/bashkit-js/src/lib.rs b/crates/bashkit-js/src/lib.rs index 39b04911..6add996b 100644 --- a/crates/bashkit-js/src/lib.rs +++ b/crates/bashkit-js/src/lib.rs @@ -395,10 +395,32 @@ impl Bash { } /// Create a new Bash instance from a snapshot. + /// + /// Accepts optional `BashOptions` to re-apply execution limits. + /// Without options, safe defaults are used (not unlimited). #[napi(factory)] - pub fn from_snapshot(data: napi::bindgen_prelude::Buffer) -> napi::Result { - let bash = - RustBash::from_snapshot(&data).map_err(|e| napi::Error::from_reason(e.to_string()))?; + pub fn from_snapshot( + data: napi::bindgen_prelude::Buffer, + options: Option, + ) -> napi::Result { + let opts = options.unwrap_or_else(default_opts); + + // Build a configured Bash instance with proper limits, then restore snapshot state + let mut bash = build_bash( + opts.username.as_deref(), + opts.hostname.as_deref(), + opts.max_commands, + opts.max_loop_iterations, + opts.files.as_ref(), + opts.python.unwrap_or(false), + &opts.external_functions.clone().unwrap_or_default(), + None, + ); + // restore_snapshot preserves the instance's limits while restoring shell state + bash.restore_snapshot(&data) + .map_err(|e| napi::Error::from_reason(e.to_string()))?; + + let cancelled = bash.cancellation_token(); let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -407,13 +429,13 @@ impl Bash { state: Arc::new(SharedState { inner: Mutex::new(bash), rt: tokio::sync::Mutex::new(rt), - cancelled: Arc::new(AtomicBool::new(false)), - username: None, - hostname: None, - max_commands: None, - max_loop_iterations: None, - python: false, - external_functions: Vec::new(), + cancelled, + username: opts.username, + hostname: opts.hostname, + max_commands: opts.max_commands, + max_loop_iterations: opts.max_loop_iterations, + python: opts.python.unwrap_or(false), + external_functions: opts.external_functions.unwrap_or_default(), external_handler: None, }), }) diff --git a/crates/bashkit/src/snapshot.rs b/crates/bashkit/src/snapshot.rs index 10caec62..34a2d041 100644 --- a/crates/bashkit/src/snapshot.rs +++ b/crates/bashkit/src/snapshot.rs @@ -50,12 +50,21 @@ //! - File descriptors, pipes, background jobs (ephemeral) //! - Execution limits configuration (caller should configure on restore) +use sha2::{Digest, Sha256}; + use crate::fs::VfsSnapshot; use crate::interpreter::ShellState; /// Schema version for snapshot format compatibility. const SNAPSHOT_VERSION: u32 = 1; +/// HMAC-like keyed hash prefix used to detect snapshot tampering. +/// The key is combined with the JSON payload to produce a SHA-256 digest +/// that is prepended to the serialized bytes. +const INTEGRITY_TAG: &[u8; 8] = b"BKSNAP01"; +/// Length of the SHA-256 digest prepended to snapshot bytes. +const DIGEST_LEN: usize = 32; + /// A serializable snapshot of a Bash interpreter's state. /// /// Combines shell state (variables, env, cwd, etc.) with VFS contents @@ -75,15 +84,37 @@ pub struct Snapshot { } impl Snapshot { - /// Serialize this snapshot to JSON bytes. + /// Serialize this snapshot to integrity-protected bytes. + /// + /// Format: `[32-byte SHA-256 digest][JSON payload]` + /// The digest covers `INTEGRITY_TAG || JSON` to detect tampering. pub fn to_bytes(&self) -> crate::Result> { - serde_json::to_vec(self).map_err(|e| crate::Error::Internal(e.to_string())) + let json = serde_json::to_vec(self).map_err(|e| crate::Error::Internal(e.to_string()))?; + let digest = Self::compute_digest(&json); + let mut out = Vec::with_capacity(DIGEST_LEN + json.len()); + out.extend_from_slice(&digest); + out.extend_from_slice(&json); + Ok(out) } - /// Deserialize a snapshot from JSON bytes. + /// Deserialize a snapshot from integrity-protected bytes. + /// + /// Verifies the SHA-256 digest before deserializing. Rejects tampered snapshots. pub fn from_bytes(data: &[u8]) -> crate::Result { + if data.len() < DIGEST_LEN { + return Err(crate::Error::Internal( + "snapshot too short: missing integrity digest".to_string(), + )); + } + let (stored_digest, json) = data.split_at(DIGEST_LEN); + let expected = Self::compute_digest(json); + if stored_digest != expected.as_slice() { + return Err(crate::Error::Internal( + "snapshot integrity check failed: data may have been tampered with".to_string(), + )); + } let snap: Self = - serde_json::from_slice(data).map_err(|e| crate::Error::Internal(e.to_string()))?; + serde_json::from_slice(json).map_err(|e| crate::Error::Internal(e.to_string()))?; if snap.version != SNAPSHOT_VERSION { return Err(crate::Error::Internal(format!( "unsupported snapshot version {} (expected {})", @@ -92,6 +123,17 @@ impl Snapshot { } Ok(snap) } + + /// Compute SHA-256 digest over `INTEGRITY_TAG || payload`. + fn compute_digest(payload: &[u8]) -> [u8; DIGEST_LEN] { + let mut hasher = Sha256::new(); + hasher.update(INTEGRITY_TAG); + hasher.update(payload); + let result = hasher.finalize(); + let mut out = [0u8; DIGEST_LEN]; + out.copy_from_slice(&result); + out + } } impl crate::Bash { diff --git a/crates/bashkit/tests/snapshot_tests.rs b/crates/bashkit/tests/snapshot_tests.rs index ae0aeb51..b9e43458 100644 --- a/crates/bashkit/tests/snapshot_tests.rs +++ b/crates/bashkit/tests/snapshot_tests.rs @@ -387,3 +387,76 @@ async fn snapshot_session_counters_transferred() { assert!(snap.session_commands > 0); assert!(snap.session_exec_calls > 0); } + +// ==================== Integrity verification (Issue #977) ==================== + +#[tokio::test] +async fn snapshot_tampered_bytes_rejected() { + let mut bash = Bash::new(); + bash.exec("x=42").await.unwrap(); + + let mut bytes = bash.snapshot().unwrap(); + + // Tamper with a byte in the JSON payload (after the 32-byte digest) + if bytes.len() > 40 { + bytes[40] ^= 0xFF; + } + + let result = Bash::from_snapshot(&bytes); + assert!(result.is_err()); + let err_msg = result.err().expect("should be error").to_string(); + assert!( + err_msg.contains("integrity"), + "Error should mention integrity: {}", + err_msg + ); +} + +#[tokio::test] +async fn snapshot_truncated_rejected() { + let result = Bash::from_snapshot(&[0u8; 10]); + assert!(result.is_err()); +} + +#[tokio::test] +async fn snapshot_modified_digest_rejected() { + let mut bash = Bash::new(); + bash.exec("x=42").await.unwrap(); + + let mut bytes = bash.snapshot().unwrap(); + + // Modify the digest (first 32 bytes) + bytes[0] ^= 0xFF; + + let result = Bash::from_snapshot(&bytes); + assert!(result.is_err()); +} + +// ==================== Limits preserved after restore (Issue #978) ==================== + +#[tokio::test] +async fn restore_snapshot_preserves_limits() { + use bashkit::ExecutionLimits; + + let limits = ExecutionLimits::new().max_commands(5); + + // Create a bash instance with strict command limit + let mut bash = Bash::builder().limits(limits.clone()).build(); + bash.exec("x=42").await.unwrap(); + let bytes = bash.snapshot().unwrap(); + + // Create a new instance with same limits, then restore snapshot state + let mut restored = Bash::builder().limits(limits).build(); + restored.restore_snapshot(&bytes).unwrap(); + + // Verify state was restored (simple command within limit) + let r = restored.exec("echo $x").await.unwrap(); + assert_eq!(r.stdout.trim(), "42"); + + // Verify limits are still enforced — many commands should hit the limit + let r = restored + .exec("echo 1; echo 2; echo 3; echo 4; echo 5; echo 6; echo 7; echo 8; echo 9; echo 10") + .await; + // Should hit the command limit and return an error + assert!(r.is_err(), "Should hit max_commands limit after restore"); +}