From 07f7b8cd5114150c7a995b4f7c87a1c69a5dd993 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 6 Aug 2026 23:59:20 -0500 Subject: [PATCH] fix(snapshot): enforce fs limits before chunk expansion --- crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/snapshot/graph.rs | 73 +++++++++++++++++----------- crates/bashkit/src/snapshot/mod.rs | 13 +++-- knowledge/security/threat-model.md | 2 +- 4 files changed, 55 insertions(+), 35 deletions(-) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 25e0463f1..35a5aefaf 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -825,7 +825,7 @@ serialization API both handle key material and integrity tags. | Snapshot forgery (TM-SNAP-001) | Forge a valid digest using the public `BKSNAP01` tag | Keyed HMAC API (`to_bytes_keyed`/`from_bytes_keyed`) for tamper-evident snapshots | MITIGATED | | Object store poisoning (TM-SNAP-002) | Substitute a different blob under a referenced object ID in the host's store | Every object is verified against its content hash on load; the graph is a Merkle tree, so a keyed commit authenticates everything it reaches | MITIGATED | | Hash agility (TM-SNAP-003) | A snapshot claims a hash algorithm the reader does not implement | Algorithm ID in the container header, rejected when unknown | MITIGATED | -| Chunk or decompression bomb (TM-SNAP-004) | A small snapshot expands into an unbounded allocation during checkout | Per-object decompression capped; filesystem limits validated before any mutation | MITIGATED | +| Chunk or decompression bomb (TM-SNAP-004) | A small snapshot expands into an unbounded allocation during checkout | Per-object decompression capped; live filesystem file-size and total-byte limits enforced before chunk materialization | MITIGATED | | Malformed object graph (TM-SNAP-005) | Cyclic parents, absurd declared entry counts, or a chunk served where a tree is expected | Kind tags checked against context, declared counts bounded before allocation, ancestry walks track visited commits | MITIGATED | | Capability mismatch on restore (TM-SNAP-006) | State captured with tools or features the restoring instance lacks is restored into it silently | Per-commit capability fingerprint with a `Superset` default policy, plus state-evidence checks that fire under every policy | MITIGATED | diff --git a/crates/bashkit/src/snapshot/graph.rs b/crates/bashkit/src/snapshot/graph.rs index 7ee58c95e..8c7358a0d 100644 --- a/crates/bashkit/src/snapshot/graph.rs +++ b/crates/bashkit/src/snapshot/graph.rs @@ -21,39 +21,41 @@ use super::objects::{ KIND_FILE, KIND_SHELL, KIND_TREE, ObjectId, TreeEntry, TreeEntryKind, }; use super::{SNAPSHOT_VERSION, Snapshot, SnapshotOptions}; +use crate::FsLimits; use crate::interpreter::{ShellState, ShellStateOptions}; /// Hard ceiling on commits walked by [`SnapshotGraph::ancestry`], so a cyclic /// or adversarial parent chain cannot spin forever (TM-SNAP-005). const MAX_ANCESTRY: usize = 1_000_000; -/// Ceiling on total file bytes materialized by one checkout (TM-SNAP-004). -/// -/// Filesystem limits are the real policy, but they are only checked once a -/// complete `VfsSnapshot` exists. Assembly happens before that, so without a -/// budget here a manifest declaring an enormous size — or a tree of many such -/// files — could exhaust memory before any limit had a chance to reject it. -/// Deliberately generous: this is a backstop against absurd input, not a -/// substitute for `FsLimits`. +/// Absolute ceiling on checkout materialization, including unlimited backends. const MAX_CHECKOUT_BYTES: u64 = 4 * 1024 * 1024 * 1024; /// Running total of file bytes materialized during one checkout. struct CheckoutBudget { remaining: u64, + max_file_size: u64, } impl CheckoutBudget { - fn new() -> Self { + fn new(limits: &FsLimits) -> Self { Self { - remaining: MAX_CHECKOUT_BYTES, + remaining: limits.max_total_bytes.min(MAX_CHECKOUT_BYTES), + max_file_size: limits.max_file_size.min(MAX_CHECKOUT_BYTES), } } - fn spend(&mut self, bytes: u64) -> crate::Result<()> { + fn spend_file(&mut self, bytes: u64) -> crate::Result<()> { + if bytes > self.max_file_size { + return Err(crate::Error::Internal(format!( + "snapshot file size {bytes} exceeds the {} byte filesystem limit", + self.max_file_size + ))); + } self.remaining = self.remaining.checked_sub(bytes).ok_or_else(|| { - crate::Error::Internal(format!( - "snapshot checkout exceeds the {MAX_CHECKOUT_BYTES} byte materialization limit" - )) + crate::Error::Internal( + "snapshot checkout exceeds the filesystem materialization limit".to_string(), + ) })?; Ok(()) } @@ -378,6 +380,7 @@ impl SnapshotGraph { pub(crate) fn materialize( root: CommitId, source: &impl ObjectSource, + limits: &FsLimits, ) -> crate::Result<(Snapshot, CapabilityFingerprint)> { let commit = Self::read_commit(root, source)?; @@ -393,7 +396,7 @@ impl SnapshotGraph { let entries = objects::decode_tree(&load(tree_id, KIND_TREE, source)?)?; // One budget for the whole tree: many small files must not add // up to more than a single huge one is allowed to be. - let mut budget = CheckoutBudget::new(); + let mut budget = CheckoutBudget::new(limits); Some(objects::tree_to_vfs(&entries, |file_id| { resolve_file(file_id, source, &mut budget) })?) @@ -439,13 +442,13 @@ fn resolve_file( ) -> crate::Result> { match objects::decode_file(&load(file_id, KIND_FILE, source)?)? { FileContent::Inline(bytes) => { - budget.spend(bytes.len() as u64)?; + budget.spend_file(bytes.len() as u64)?; Ok(bytes) } FileContent::Chunked { size, chunks } => { // Charge the *declared* size to the budget before allocating, so a // manifest cannot commit us to work the budget would refuse. - budget.spend(size)?; + budget.spend_file(size)?; let mut out = Vec::with_capacity(usize::try_from(size).unwrap_or_default().min(1 << 24)); @@ -669,7 +672,7 @@ impl crate::Bash { source: &impl ObjectSource, policy: super::CheckoutPolicy, ) -> crate::Result<()> { - let (snapshot, caps) = SnapshotGraph::materialize(root, source)?; + let (snapshot, caps) = SnapshotGraph::materialize(root, source, &self.fs.limits())?; self.apply_checked(&snapshot, &caps, policy) } @@ -730,7 +733,7 @@ mod tests { // the file is only 1 KiB. Assembly must stop at the declared size. let file_id = chunk_bomb(&mut store, 64, 1024); - let mut budget = CheckoutBudget::new(); + let mut budget = CheckoutBudget::new(&FsLimits::unlimited()); let err = resolve_file(file_id, &store, &mut budget).unwrap_err(); assert!( err.to_string().contains("declared"), @@ -746,22 +749,36 @@ mod tests { // allocating first is the failure mode. let file_id = chunk_bomb(&mut store, 4, u64::MAX / 2); - let mut budget = CheckoutBudget::new(); + let mut budget = CheckoutBudget::new(&FsLimits::unlimited()); let err = resolve_file(file_id, &store, &mut budget).unwrap_err(); assert!( - err.to_string().contains("materialization limit"), + err.to_string().contains("filesystem limit"), "expected a budget rejection, got: {err}" ); } + #[test] + fn configured_file_limit_is_refused_before_materializing_chunks() { + let mut store = HashMap::new(); + let file_id = chunk_bomb(&mut store, 2, (chunker::MAX_CHUNK * 2) as u64); + let limits = crate::FsLimits::new().max_file_size(chunker::MAX_CHUNK as u64); + + let mut budget = CheckoutBudget::new(&limits); + let err = resolve_file(file_id, &store, &mut budget).unwrap_err(); + assert!( + err.to_string().contains("file size"), + "expected a configured file-size rejection, got: {err}" + ); + } + #[test] fn the_budget_is_shared_across_every_file_in_a_tree() { // Many individually-legal files must not add up past the ceiling. - let mut budget = CheckoutBudget::new(); - assert!(budget.spend(MAX_CHECKOUT_BYTES / 2).is_ok()); - assert!(budget.spend(MAX_CHECKOUT_BYTES / 2).is_ok()); + let mut budget = CheckoutBudget::new(&FsLimits::unlimited()); + assert!(budget.spend_file(MAX_CHECKOUT_BYTES / 2).is_ok()); + assert!(budget.spend_file(MAX_CHECKOUT_BYTES / 2).is_ok()); assert!( - budget.spend(1).is_err(), + budget.spend_file(1).is_err(), "the budget must be cumulative, not per-file" ); } @@ -779,7 +796,7 @@ mod tests { }); let file_id = store_object(&mut store, &manifest); - let mut budget = CheckoutBudget::new(); + let mut budget = CheckoutBudget::new(&FsLimits::unlimited()); let err = resolve_file(file_id, &store, &mut budget).unwrap_err(); assert!( err.to_string().contains("maximum"), @@ -801,7 +818,7 @@ mod tests { }); let file_id = store_object(&mut store, &manifest); - let mut budget = CheckoutBudget::new(); + let mut budget = CheckoutBudget::new(&FsLimits::unlimited()); let out = resolve_file(file_id, &store, &mut budget).unwrap(); assert_eq!(out.len(), 1500); assert_eq!(&out[..1000], &vec![b'x'; 1000][..]); @@ -831,7 +848,7 @@ mod tests { let tree = encode_tree(&entries); let entries = objects::decode_tree(&tree.payload).unwrap(); - let mut budget = CheckoutBudget::new(); + let mut budget = CheckoutBudget::new(&FsLimits::unlimited()); let result = objects::tree_to_vfs(&entries, |file_id| { resolve_file(file_id, &store, &mut budget) }); diff --git a/crates/bashkit/src/snapshot/mod.rs b/crates/bashkit/src/snapshot/mod.rs index 527e55c7d..4f8ab8b7c 100644 --- a/crates/bashkit/src/snapshot/mod.rs +++ b/crates/bashkit/src/snapshot/mod.rs @@ -182,7 +182,7 @@ impl Snapshot { /// Accepts both the current object-graph format and legacy v1 JSON /// payloads. Verifies integrity before decoding, and rejects tampering. pub fn from_bytes(data: &[u8]) -> crate::Result { - Ok(decode_sealed(data, None)?.0) + Ok(decode_sealed(data, None, &crate::FsLimits::default())?.0) } /// Serialize with a caller-provided secret key for tamper-proof integrity. @@ -202,7 +202,7 @@ impl Snapshot { /// /// Rejects snapshots where the HMAC does not match, preventing forgery. pub fn from_bytes_keyed(data: &[u8], key: &[u8]) -> crate::Result { - Ok(decode_sealed(data, Some(key))?.0) + Ok(decode_sealed(data, Some(key), &crate::FsLimits::default())?.0) } /// Compute SHA-256 digest over `INTEGRITY_TAG || payload`. @@ -283,12 +283,15 @@ fn unseal<'a>(data: &'a [u8], key: Option<&[u8]>) -> crate::Result<&'a [u8]> { fn decode_sealed( data: &[u8], key: Option<&[u8]>, + limits: &crate::FsLimits, ) -> crate::Result<(Snapshot, Option)> { let body = unseal(data, key)?; if container::is_v2(body) { let parsed = container::decode(body)?; - let (snapshot, caps) = SnapshotGraph::materialize(parsed.root, &parsed.objects)?; + // THREAT[TM-SNAP-004]: reject declared file sizes and totals against + // live VFS limits before expanding content-addressed chunks. + let (snapshot, caps) = SnapshotGraph::materialize(parsed.root, &parsed.objects, limits)?; return Ok((snapshot, Some(caps))); } @@ -439,7 +442,7 @@ impl crate::Bash { data: &[u8], policy: CheckoutPolicy, ) -> crate::Result<()> { - let (snap, caps) = decode_sealed(data, None)?; + let (snap, caps) = decode_sealed(data, None, &self.fs.limits())?; self.apply_restore(&snap, caps.as_ref(), policy) } @@ -517,7 +520,7 @@ impl crate::Bash { key: &[u8], policy: CheckoutPolicy, ) -> crate::Result<()> { - let (snap, caps) = decode_sealed(data, Some(key))?; + let (snap, caps) = decode_sealed(data, Some(key), &self.fs.limits())?; self.apply_restore(&snap, caps.as_ref(), policy) } } diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index a210272be..acef64657 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -991,7 +991,7 @@ patterns via `.redact_env("MY_CUSTOM_SECRET")`. | TM-SNAP-001 | Snapshot forgery via public tag | Attacker computes valid SHA-256 digest using public `BKSNAP01` tag | Document limitation; add keyed HMAC API (`to_bytes_keyed`/`from_bytes_keyed`) | **MITIGATED** | | TM-SNAP-002 | Object store poisoning | Attacker controlling the host's object store substitutes a different blob under a referenced object ID | Every object is verified against its content hash on load (`Encoded::from_storage`); the graph is a Merkle tree, so a keyed commit authenticates everything it reaches | **MITIGATED** | | TM-SNAP-003 | Hash algorithm agility | SHA-256 weakens, or a snapshot claims an algorithm the reader does not implement | Algorithm ID recorded in the container header and rejected when unknown; ID changes do not require a framing break | **MITIGATED** | -| TM-SNAP-004 | Chunk or decompression bomb | A small snapshot expands into an unbounded allocation during checkout | Per-object decompression capped before materialization; filesystem limits validated before any mutation, leaving the instance untouched on refusal | **MITIGATED** | +| TM-SNAP-004 | Chunk or decompression bomb | A small snapshot expands into an unbounded allocation during checkout | Per-object decompression is capped; live filesystem file-size and total-byte limits are enforced before chunk materialization or mutation | **MITIGATED** | | TM-SNAP-005 | Malformed object graph | Cyclic or self-referencing parents, absurd declared entry counts, or a chunk served where a tree is expected | Kind tags checked against the kind expected from context; declared counts bounded before allocation; ancestry walks track visited commits and cap iterations | **MITIGATED** | | TM-SNAP-006 | Capability mismatch on restore | State captured with tools, features, or a filesystem backend the restoring instance lacks is restored into it silently | Capability fingerprint recorded per commit; `CheckoutPolicy::Superset` by default, so a restore into an environment missing any recorded capability fails; state-evidence checks fire regardless of policy | **MITIGATED** |