diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 849dcf6e2..f3d9bb61a 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -598,16 +598,17 @@ class ExecTool(Tool): """Best-effort safety guard for potentially destructive commands.""" cmd = command.strip() lower = cmd.lower() + match_text = re.sub(r"(^|[^\\])#.*$", r"\1", lower).strip() # allow_patterns take priority over deny_patterns so that users can # exempt specific commands (e.g. "rm -rf" inside a build directory) # from the hardcoded deny list via configuration. explicitly_allowed = bool(self.allow_patterns) and any( - re.search(p, lower) for p in self.allow_patterns + re.fullmatch(p, match_text) for p in self.allow_patterns ) if not explicitly_allowed: for pattern in self.deny_patterns: - if re.search(pattern, lower): + if re.search(pattern, match_text): return "Error: Command blocked by deny pattern filter" if self.allow_patterns: diff --git a/tests/tools/test_exec_allow_patterns.py b/tests/tools/test_exec_allow_patterns.py index 1a2a90521..473f4ef66 100644 --- a/tests/tools/test_exec_allow_patterns.py +++ b/tests/tools/test_exec_allow_patterns.py @@ -15,7 +15,7 @@ def test_deny_patterns_block_rm_rf(): def test_allow_patterns_bypass_deny(): """allow_patterns take priority: matching command skips deny check.""" - tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/tmp/"]) + tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/tmp/.*"]) result = tool._guard_command("rm -rf /tmp/build", "/tmp") assert result is None @@ -49,10 +49,32 @@ def test_allow_patterns_bypass_extra_deny(): def test_allow_patterns_is_whitelist_only(): """When allow_patterns is set, non-matching non-denied commands are blocked.""" - tool = ExecTool(allow_patterns=[r"\becho\b"]) + tool = ExecTool(allow_patterns=[r"echo\s+hello"]) # echo matches allow → ok assert tool._guard_command("echo hello", "/tmp") is None # ls does not match allow and is not in deny → blocked by allowlist result = tool._guard_command("ls /tmp", "/tmp") assert result is not None assert "allowlist" in result.lower() + + +def test_allow_patterns_do_not_allow_chained_command_bypass(): + """A partial allowlist match must not bypass deny patterns in chained commands.""" + tool = ExecTool(allow_patterns=[r"\becho\b"]) + result = tool._guard_command("echo hello; rm -rf /", "/tmp") + assert result is not None + assert "deny pattern filter" in result.lower() + + +def test_allow_patterns_strip_shell_comments_before_matching(): + """Comments are stripped before allow and deny pattern checks.""" + tool = ExecTool(allow_patterns=[r"echo\s+hello"]) + result = tool._guard_command("echo hello # comment with rm -rf /", "/tmp") + assert result is None + + +def test_allow_patterns_fullmatch_allows_exact_command(): + """A full-command allow pattern can still exempt an exact denied command.""" + tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/tmp/build"]) + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is None