fix(exec): remove ad-hoc shell comment stripping from _guard_command
- Removes match_text regex that stripped # comments before pattern matching (broke on quoted # inside strings) - allow_patterns now run re.fullmatch against the full lowercased command - deny_patterns search the original lowercased command - Replaces comment-stripping test with comment-tail bypass regression (touch canary # echo allowlisted must be blocked) - Adds Re-bin regression for quoted hash + blocked command (echo "#" followed by blocked command must be caught) - All 10 tests pass Signed-off-by: axelray-dev <110029405+axelray-dev@users.noreply.github.com>
This commit is contained in:
@@ -598,17 +598,16 @@ 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.fullmatch(p, match_text) for p in self.allow_patterns
|
||||
re.fullmatch(p, lower) for p in self.allow_patterns
|
||||
)
|
||||
if not explicitly_allowed:
|
||||
for pattern in self.deny_patterns:
|
||||
if re.search(pattern, match_text):
|
||||
if re.search(pattern, lower):
|
||||
return "Error: Command blocked by deny pattern filter"
|
||||
|
||||
if self.allow_patterns:
|
||||
|
||||
@@ -66,11 +66,20 @@ def test_allow_patterns_do_not_allow_chained_command_bypass():
|
||||
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_do_not_allow_comment_tail_bypass():
|
||||
"""Comment tails must not make a non-allowlisted command match."""
|
||||
tool = ExecTool(allow_patterns=[r"echo allowlisted"])
|
||||
result = tool._guard_command("touch canary # echo allowlisted", "/tmp")
|
||||
assert result is not None
|
||||
assert "allowlist" in result.lower()
|
||||
|
||||
|
||||
def test_deny_patterns_search_original_command_with_quoted_hash():
|
||||
"""Deny checks must still inspect text after a quoted hash."""
|
||||
tool = ExecTool(deny_patterns=[r"\brm\s+-rf\s+/"])
|
||||
result = tool._guard_command('echo "#"; rm -rf /', "/tmp")
|
||||
assert result is not None
|
||||
assert "deny pattern filter" in result.lower()
|
||||
|
||||
|
||||
def test_allow_patterns_fullmatch_allows_exact_command():
|
||||
|
||||
Reference in New Issue
Block a user