diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 20eeff0a..fd12a2bc 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1235,6 +1235,8 @@ impl Interpreter { &s[..end] } + // Decision: restored `$!` must match Bashkit-produced virtual numeric job ids. + const MAX_RESTORED_LAST_BG_PID_LEN: usize = 20; const MAX_GLOB_DEPTH: usize = 50; /// Create a new interpreter with the given filesystem. @@ -2205,6 +2207,14 @@ impl Interpreter { func_bytes, Self::is_internal_variable, ); + // Keep live budget consistent with validate_shell_state_restore_limits, + // which counts the restored `$!` toward variable bytes. + if let Some(last_bg_pid) = &self.last_bg_pid { + self.memory_budget.variable_bytes = self + .memory_budget + .variable_bytes + .saturating_add(last_bg_pid.len()); + } } fn migrate_legacy_attr_markers( @@ -2292,7 +2302,18 @@ impl Interpreter { } } - let budget = crate::limits::MemoryBudget::recompute_from_state( + if let Some(last_bg_pid) = &state.last_bg_pid + && (last_bg_pid.is_empty() + || last_bg_pid.len() > Self::MAX_RESTORED_LAST_BG_PID_LEN + || !last_bg_pid.bytes().all(|b| b.is_ascii_digit())) + { + return Err(crate::limits::LimitExceeded::Memory( + "invalid restored last background pid".to_string(), + ) + .into()); + } + + let mut budget = crate::limits::MemoryBudget::recompute_from_state( &state.variables, &state.arrays, &state.assoc_arrays, @@ -2300,6 +2321,9 @@ impl Interpreter { 0, Self::is_internal_variable, ); + if let Some(last_bg_pid) = &state.last_bg_pid { + budget.variable_bytes = budget.variable_bytes.saturating_add(last_bg_pid.len()); + } if budget.variable_count > self.memory_limits.max_variable_count { return Err(crate::limits::LimitExceeded::Memory(format!( diff --git a/crates/bashkit/tests/integration/snapshot_tests.rs b/crates/bashkit/tests/integration/snapshot_tests.rs index 445803d7..ea99df79 100644 --- a/crates/bashkit/tests/integration/snapshot_tests.rs +++ b/crates/bashkit/tests/integration/snapshot_tests.rs @@ -715,6 +715,32 @@ async fn snapshot_restore_rejects_tampered_dir_stack_above_pushd_limit() { ); } +#[tokio::test] +async fn snapshot_restore_rejects_invalid_last_bg_pid() { + let mut src = Bash::new(); + src.exec("true &").await.unwrap(); + let bytes = src.snapshot().unwrap(); + + for invalid_pid in ["9".repeat(64), "not-a-pid".to_string()] { + let mut tampered_json: serde_json::Value = serde_json::from_slice(&bytes[32..]).unwrap(); + tampered_json["shell"]["last_bg_pid"] = invalid_pid.into(); + + let tampered_snapshot: Snapshot = serde_json::from_value(tampered_json).unwrap(); + let tampered_bytes = tampered_snapshot.to_bytes().unwrap(); + + // Use permissive limits so the rejection is attributable to last_bg_pid + // validation, not incidental variable-byte pressure from the snapshot. + let mut restored = Bash::new(); + let err = restored + .restore_snapshot(&tampered_bytes) + .expect_err("restore must reject invalid snapshot-controlled last_bg_pid"); + assert!( + err.to_string().contains("last background pid"), + "expected last_bg_pid validation error, got: {err}" + ); + } +} + // ==================== Atomic restore (Issue #1576) ==================== /// A malformed VFS snapshot (path validation failure) must cause