Skip to content

Commit fa636e2

Browse files
chaliyclaude
andauthored
fix(python): preserve security config across Bash.reset() (#463)
## Summary - `reset()` was creating bare `Bash::builder()`, discarding all config - Now rebuilds with stored username, hostname, max_commands, max_loop_iterations - Prevents silent removal of DoS protections after reset ## Test plan - [x] `test_reset_preserves_config` — verifies username, hostname, and limits survive reset - [x] Rust builds clean, Python lint/format pass Closes #424 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1e42124 commit fa636e2

File tree

2 files changed

+43
-2
lines changed

2 files changed

+43
-2
lines changed

crates/bashkit-python/src/lib.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,34 @@ impl PyBash {
267267
})
268268
}
269269

270-
/// Reset interpreter to fresh state.
270+
/// Reset interpreter to fresh state, preserving security configuration.
271271
/// Releases GIL before blocking on tokio to prevent deadlock.
272272
fn reset(&self, py: Python<'_>) -> PyResult<()> {
273273
let inner = self.inner.clone();
274+
// THREAT[TM-PY-026]: Rebuild with same config to preserve DoS protections.
275+
let username = self.username.clone();
276+
let hostname = self.hostname.clone();
277+
let max_commands = self.max_commands;
278+
let max_loop_iterations = self.max_loop_iterations;
274279

275280
py.detach(|| {
276281
self.rt.block_on(async move {
277282
let mut bash = inner.lock().await;
278-
let builder = Bash::builder();
283+
let mut builder = Bash::builder();
284+
if let Some(ref u) = username {
285+
builder = builder.username(u);
286+
}
287+
if let Some(ref h) = hostname {
288+
builder = builder.hostname(h);
289+
}
290+
let mut limits = ExecutionLimits::new();
291+
if let Some(mc) = max_commands {
292+
limits = limits.max_commands(mc as usize);
293+
}
294+
if let Some(mli) = max_loop_iterations {
295+
limits = limits.max_loop_iterations(mli as usize);
296+
}
297+
builder = builder.limits(limits);
279298
*bash = builder.build();
280299
Ok(())
281300
})

crates/bashkit-python/tests/test_bashkit.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,28 @@ def test_reset():
249249
assert r.stdout.strip() == "empty"
250250

251251

252+
# Issue #424: reset() should preserve security configuration
253+
def test_reset_preserves_config():
254+
bash = Bash(max_commands=5, username="testuser", hostname="testhost")
255+
# Verify config works before reset
256+
r = bash.execute_sync("whoami")
257+
assert r.stdout.strip() == "testuser"
258+
259+
bash.reset()
260+
261+
# Config should survive reset
262+
r = bash.execute_sync("whoami")
263+
assert r.stdout.strip() == "testuser", "username lost after reset"
264+
265+
r = bash.execute_sync("hostname")
266+
assert r.stdout.strip() == "testhost", "hostname lost after reset"
267+
268+
# Max commands limit should still be enforced
269+
# Run enough commands to hit the limit
270+
r = bash.execute_sync("echo 1; echo 2; echo 3; echo 4; echo 5; echo 6")
271+
assert r.exit_code != 0 or "limit" in r.stderr.lower() or r.stdout.count("\n") <= 5
272+
273+
252274
# -- BashTool: LLM metadata ------------------------------------------------
253275

254276

0 commit comments

Comments
 (0)