Skip to content

Commit 90269d5

Browse files
committed
feat(tools): add max_snapshot_bytes limit to transactional ShellExecutor
Add `max_snapshot_bytes: u64` to `ShellConfig` (default 0 = unlimited). `TransactionSnapshot::capture()` now tracks cumulative bytes and aborts with `ToolError::SnapshotFailed` if the configured limit is exceeded, preventing disk exhaustion from unbounded snapshot copies. Closes #2474
1 parent 41cbd7d commit 90269d5

File tree

5 files changed

+64
-6
lines changed

5 files changed

+64
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
88

99
### Added
1010

11+
- feat(tools): `[tools.shell] max_snapshot_bytes` config option to limit transaction snapshot size — returns `SnapshotFailed` when cumulative copied bytes exceed the limit; `0` means unlimited (default)
1112
- feat(tools): transactional `ShellExecutor` — opt-in snapshot+rollback for shell commands; file-level snapshot is captured before write commands (detected via `WRITE_INDICATORS` heuristic + redirection target extraction); rollback restores originals on configurable exit codes; new `ShellConfig` fields: `transactional`, `transaction_scope` (glob-filtered paths), `auto_rollback`, `auto_rollback_exit_codes`, `snapshot_required`; new `ToolError::SnapshotFailed`, `AuditResult::Rollback`, `ToolEvent::Rollback` variants; backed by `tempfile::TempDir` for automatic cleanup on success (closes #2414)
1213
- feat(core): `/new` slash command — resets conversation context (messages, compaction state, tool caches, focus/sidequest, pending plans) while preserving memory, MCP connections, providers, and skills; creates a new `ConversationId` in SQLite for audit trail; generates a session digest for the outgoing conversation fire-and-forget unless `--no-digest` is passed; active sub-agents and background compression tasks are cancelled; `--keep-plan` preserves a pending plan graph; available in all channels (CLI, TUI, Telegram) via the unified `handle_builtin_command` path (closes #2451)
1314
- feat(memory): Kumiho AGM-inspired belief revision for graph edges — new `BeliefRevisionConfig` with `similarity_threshold`; `find_superseded_edges()` uses contradiction heuristic (same relation domain + high cosine similarity = supersession); `superseded_by` column added to `graph_edges` for audit trail; `invalidate_edge_with_supersession()` in `GraphStore`; `resolve_edge_typed` accepts optional `BeliefRevisionConfig`; controlled by `[memory.graph.belief_revision] enabled = false` (migration 056, closes #2441)

crates/zeph-tools/src/config.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,9 @@ pub struct ShellConfig {
520520
/// When false (default), snapshot failure emits a warning and execution proceeds.
521521
#[serde(default)]
522522
pub snapshot_required: bool,
523+
/// Maximum cumulative bytes for transaction snapshots. 0 = unlimited.
524+
#[serde(default)]
525+
pub max_snapshot_bytes: u64,
523526
}
524527

525528
impl ShellConfig {
@@ -588,6 +591,7 @@ impl Default for ShellConfig {
588591
auto_rollback: false,
589592
auto_rollback_exit_codes: Vec::new(),
590593
snapshot_required: false,
594+
max_snapshot_bytes: 0,
591595
}
592596
}
593597
}

crates/zeph-tools/src/shell/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ pub struct ShellExecutor {
112112
auto_rollback: bool,
113113
auto_rollback_exit_codes: Vec<i32>,
114114
snapshot_required: bool,
115+
max_snapshot_bytes: u64,
115116
transaction_scope_matchers: Vec<globset::GlobMatcher>,
116117
}
117118

@@ -165,6 +166,7 @@ impl ShellExecutor {
165166
auto_rollback: config.auto_rollback,
166167
auto_rollback_exit_codes: config.auto_rollback_exit_codes.clone(),
167168
snapshot_required: config.snapshot_required,
169+
max_snapshot_bytes: config.max_snapshot_bytes,
168170
transaction_scope_matchers: build_scope_matchers(&config.transaction_scope),
169171
}
170172
}
@@ -285,7 +287,7 @@ impl ShellExecutor {
285287
if paths.is_empty() {
286288
None
287289
} else {
288-
match TransactionSnapshot::capture(&paths) {
290+
match TransactionSnapshot::capture(&paths, self.max_snapshot_bytes) {
289291
Ok(snap) => {
290292
tracing::debug!(
291293
files = snap.file_count(),

crates/zeph-tools/src/shell/tests.rs

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ fn default_config() -> ShellConfig {
1717
auto_rollback: false,
1818
auto_rollback_exit_codes: Vec::new(),
1919
snapshot_required: false,
20+
max_snapshot_bytes: 0,
2021
}
2122
}
2223

@@ -1710,7 +1711,7 @@ fn transaction_snapshot_capture_and_rollback() {
17101711
std::fs::write(&file, b"original").unwrap();
17111712

17121713
let snap =
1713-
super::transaction::TransactionSnapshot::capture(std::slice::from_ref(&file)).unwrap();
1714+
super::transaction::TransactionSnapshot::capture(std::slice::from_ref(&file), 0).unwrap();
17141715
assert_eq!(snap.file_count(), 1);
17151716

17161717
std::fs::write(&file, b"modified").unwrap();
@@ -1726,7 +1727,7 @@ fn transaction_snapshot_new_file_rollback() {
17261727
let file = dir.path().join("new.txt");
17271728

17281729
let snap =
1729-
super::transaction::TransactionSnapshot::capture(std::slice::from_ref(&file)).unwrap();
1730+
super::transaction::TransactionSnapshot::capture(std::slice::from_ref(&file), 0).unwrap();
17301731
assert_eq!(snap.file_count(), 1);
17311732

17321733
std::fs::write(&file, b"created").unwrap();
@@ -1738,7 +1739,7 @@ fn transaction_snapshot_new_file_rollback() {
17381739

17391740
#[test]
17401741
fn transaction_snapshot_empty_paths() {
1741-
let snap = super::transaction::TransactionSnapshot::capture(&[]).unwrap();
1742+
let snap = super::transaction::TransactionSnapshot::capture(&[], 0).unwrap();
17421743
assert_eq!(snap.file_count(), 0);
17431744
assert_eq!(snap.total_bytes(), 0);
17441745
let report = snap.rollback().unwrap();
@@ -2187,3 +2188,42 @@ async fn custom_rollback_exit_codes() {
21872188
let content2 = std::fs::read(&file).unwrap();
21882189
assert_eq!(content2, b"original", "exit 42 should trigger rollback");
21892190
}
2191+
2192+
// --- snapshot size limit tests ---
2193+
2194+
#[test]
2195+
fn transaction_snapshot_size_limit_exceeded() {
2196+
use crate::shell::transaction::TransactionSnapshot;
2197+
let dir = tempfile::TempDir::new().unwrap();
2198+
let file = dir.path().join("big.txt");
2199+
// Write 100 bytes
2200+
std::fs::write(&file, vec![b'x'; 100]).unwrap();
2201+
2202+
let result = TransactionSnapshot::capture(&[file], 50);
2203+
assert!(result.is_err());
2204+
let msg = result.unwrap_err().to_string();
2205+
assert!(msg.contains("exceeds limit"), "unexpected error: {msg}");
2206+
}
2207+
2208+
#[test]
2209+
fn transaction_snapshot_size_limit_zero_unlimited() {
2210+
use crate::shell::transaction::TransactionSnapshot;
2211+
let dir = tempfile::TempDir::new().unwrap();
2212+
let file = dir.path().join("big.txt");
2213+
std::fs::write(&file, vec![b'x'; 1_000_000]).unwrap();
2214+
2215+
// max_bytes = 0 means unlimited — must succeed
2216+
let result = TransactionSnapshot::capture(&[file], 0);
2217+
assert!(result.is_ok());
2218+
}
2219+
2220+
#[test]
2221+
fn transaction_snapshot_size_limit_within_budget() {
2222+
use crate::shell::transaction::TransactionSnapshot;
2223+
let dir = tempfile::TempDir::new().unwrap();
2224+
let file = dir.path().join("small.txt");
2225+
std::fs::write(&file, b"hello").unwrap();
2226+
2227+
let result = TransactionSnapshot::capture(&[file], 1024);
2228+
assert!(result.is_ok());
2229+
}

crates/zeph-tools/src/shell/transaction.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,17 @@ impl TransactionSnapshot {
5151
/// Capture files at `paths`.
5252
///
5353
/// Non-existent paths are recorded as "new" (rollback will delete them if created).
54+
/// If `max_bytes > 0` and the cumulative size of copied files exceeds `max_bytes`,
55+
/// an error is returned immediately.
5456
///
5557
/// # Errors
5658
///
57-
/// Returns `std::io::Error` if the temp directory or any file copy fails.
58-
pub(crate) fn capture(paths: &[PathBuf]) -> Result<Self, std::io::Error> {
59+
/// Returns `std::io::Error` if the temp directory, any file copy fails, or the
60+
/// snapshot size limit is exceeded.
61+
pub(crate) fn capture(paths: &[PathBuf], max_bytes: u64) -> Result<Self, std::io::Error> {
5962
let backup_dir = tempfile::TempDir::new()?;
6063
let mut entries = Vec::with_capacity(paths.len());
64+
let mut cumulative_bytes: u64 = 0;
6165

6266
for (i, original) in paths.iter().enumerate() {
6367
// Use symlink_metadata to avoid following symlinks — snapshot only regular files.
@@ -91,6 +95,13 @@ impl TransactionSnapshot {
9195
let meta = std::fs::metadata(original)?;
9296
std::fs::set_permissions(&backup_path, meta.permissions())?;
9397

98+
cumulative_bytes += meta.len();
99+
if max_bytes > 0 && cumulative_bytes > max_bytes {
100+
return Err(std::io::Error::other(format!(
101+
"snapshot size {cumulative_bytes} exceeds limit {max_bytes}"
102+
)));
103+
}
104+
94105
entries.push(SnapshotEntry {
95106
original: original.clone(),
96107
kind: EntryKind::Existing { backup_path },

0 commit comments

Comments
 (0)