From 2295de90ad1eb568bceaac1eda3c86f4219b9e06 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Mar 2026 00:57:10 +0000 Subject: [PATCH] fix(python): preserve security config across Bash.reset() reset() was creating a bare Bash::builder(), discarding max_commands, max_loop_iterations, username, and hostname. Now rebuilds with the original configuration stored at construction time. Closes #424 https://claude.ai/code/session_01WZjYqxm5xMPAEe7FSHJkDy --- crates/bashkit-python/src/lib.rs | 23 +++++++++++++++++++-- crates/bashkit-python/tests/test_bashkit.py | 22 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/bashkit-python/src/lib.rs b/crates/bashkit-python/src/lib.rs index b17d164e..1d139d56 100644 --- a/crates/bashkit-python/src/lib.rs +++ b/crates/bashkit-python/src/lib.rs @@ -267,15 +267,34 @@ impl PyBash { }) } - /// Reset interpreter to fresh state. + /// Reset interpreter to fresh state, preserving security configuration. /// Releases GIL before blocking on tokio to prevent deadlock. fn reset(&self, py: Python<'_>) -> PyResult<()> { let inner = self.inner.clone(); + // THREAT[TM-PY-026]: Rebuild with same config to preserve DoS protections. + let username = self.username.clone(); + let hostname = self.hostname.clone(); + let max_commands = self.max_commands; + let max_loop_iterations = self.max_loop_iterations; py.detach(|| { self.rt.block_on(async move { let mut bash = inner.lock().await; - let builder = Bash::builder(); + let mut builder = Bash::builder(); + if let Some(ref u) = username { + builder = builder.username(u); + } + if let Some(ref h) = hostname { + builder = builder.hostname(h); + } + let mut limits = ExecutionLimits::new(); + if let Some(mc) = max_commands { + limits = limits.max_commands(mc as usize); + } + if let Some(mli) = max_loop_iterations { + limits = limits.max_loop_iterations(mli as usize); + } + builder = builder.limits(limits); *bash = builder.build(); Ok(()) }) diff --git a/crates/bashkit-python/tests/test_bashkit.py b/crates/bashkit-python/tests/test_bashkit.py index 731476e1..496b4ab1 100644 --- a/crates/bashkit-python/tests/test_bashkit.py +++ b/crates/bashkit-python/tests/test_bashkit.py @@ -249,6 +249,28 @@ def test_reset(): assert r.stdout.strip() == "empty" +# Issue #424: reset() should preserve security configuration +def test_reset_preserves_config(): + bash = Bash(max_commands=5, username="testuser", hostname="testhost") + # Verify config works before reset + r = bash.execute_sync("whoami") + assert r.stdout.strip() == "testuser" + + bash.reset() + + # Config should survive reset + r = bash.execute_sync("whoami") + assert r.stdout.strip() == "testuser", "username lost after reset" + + r = bash.execute_sync("hostname") + assert r.stdout.strip() == "testhost", "hostname lost after reset" + + # Max commands limit should still be enforced + # Run enough commands to hit the limit + r = bash.execute_sync("echo 1; echo 2; echo 3; echo 4; echo 5; echo 6") + assert r.exit_code != 0 or "limit" in r.stderr.lower() or r.stdout.count("\n") <= 5 + + # -- BashTool: LLM metadata ------------------------------------------------