mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-08-10 08:21:54 +00:00
fix(agents): close two heredoc bypasses in the guard
A body piped into a shell is executable, not data, so it is no longer stripped: bash <<'EOF' containing a blocked command is matched again. The opener's own line is now preserved, so a redirect written after the delimiter is still seen. Only the body between delimiters is replaced. Both found by review on the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ASfWmW3RGy4qAQm6s8U9jH
This commit is contained in:
@@ -94,21 +94,38 @@ def deny(reason: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
def strip_heredocs(command: str) -> str:
|
||||
"""Replace heredoc bodies with a placeholder.
|
||||
#: A heredoc opener, its same-line remainder, and the body up to the delimiter.
|
||||
HEREDOC = re.compile(
|
||||
r"(<<-?\s*[\"']?(\w+)[\"']?)([^\n]*)\n.*?^[ \t]*\2\b",
|
||||
re.DOTALL | re.MULTILINE,
|
||||
)
|
||||
|
||||
A heredoc body is data — a commit message, a file being written, a PR body —
|
||||
not a command. Matching rules against it produces false denials, such as
|
||||
blocking a commit whose message merely discusses `rm -rf docs/images`. The
|
||||
redirection and the delimiter stay on the command line, so a heredoc that
|
||||
genuinely writes into a protected path is still caught.
|
||||
#: Commands that execute their heredoc body instead of consuming it as data.
|
||||
SHELL_INTERPRETER = re.compile(r"\b(?:bash|sh|zsh|dash|ksh|eval)\b")
|
||||
|
||||
|
||||
def strip_heredocs(command: str) -> str:
|
||||
"""Replace data heredoc bodies with a placeholder.
|
||||
|
||||
A heredoc body is usually data — a commit message, a file being written, a
|
||||
PR body — not a command, and matching rules against it produces false
|
||||
denials, such as blocking a commit whose message merely discusses a
|
||||
protected path.
|
||||
|
||||
Two things keep that from becoming a bypass. A body piped into a shell
|
||||
(`bash <<'EOF'`) is executable, so it is left intact and still matched. And
|
||||
the opener's own line is preserved in full, so a redirection written after
|
||||
the delimiter (`cat <<'EOF' > docs/v1/x.mdx`) is still seen.
|
||||
"""
|
||||
return re.sub(
|
||||
r"(<<-?\s*[\"']?(\w+)[\"']?).*?^\s*\2\b",
|
||||
r"\1 HEREDOC_BODY",
|
||||
command,
|
||||
flags=re.DOTALL | re.MULTILINE,
|
||||
)
|
||||
|
||||
def replace(match: re.Match[str]) -> str:
|
||||
line_start = command.rfind("\n", 0, match.start()) + 1
|
||||
opener_line = command[line_start : match.start()] + match.group(1)
|
||||
if SHELL_INTERPRETER.search(opener_line):
|
||||
return match.group(0)
|
||||
return f"{match.group(1)}{match.group(3)}\nHEREDOC_BODY\n{match.group(2)}"
|
||||
|
||||
return HEREDOC.sub(replace, command)
|
||||
|
||||
|
||||
def bash_violation(command: str) -> str | None:
|
||||
|
||||
@@ -136,6 +136,31 @@ def test_a_heredoc_writing_into_a_frozen_snapshot_is_still_blocked() -> None:
|
||||
assert "frozen release snapshots" in reason
|
||||
|
||||
|
||||
def test_a_redirect_after_the_heredoc_delimiter_is_still_seen() -> None:
|
||||
"""The opener's own line must survive stripping, redirect included."""
|
||||
command = "cat <<'EOF' > docs/v1.15.0/index.mdx\nsome new content\nEOF\n"
|
||||
reason = guard.bash_violation(command)
|
||||
assert reason is not None
|
||||
assert "frozen release snapshots" in reason
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("command", "expected"),
|
||||
[
|
||||
("bash <<'EOF'\nrm -rf docs/images/a.png\nEOF\n", "docs/images/"),
|
||||
("sh <<'EOF'\npip install ruff\nEOF\n", "do not use pip directly"),
|
||||
("bash -s <<'EOF'\ngit commit --no-verify -m x\nEOF\n", "--no-verify"),
|
||||
],
|
||||
)
|
||||
def test_a_heredoc_piped_into_a_shell_is_executable_not_data(
|
||||
command: str, expected: str
|
||||
) -> None:
|
||||
"""A body a shell will run must still be matched, not stripped as data."""
|
||||
reason = guard.bash_violation(command)
|
||||
assert reason is not None, f"expected {command!r} to be blocked"
|
||||
assert expected in reason
|
||||
|
||||
|
||||
def test_a_command_after_a_heredoc_is_still_inspected() -> None:
|
||||
command = "cat <<'EOF' > notes.txt\njust notes\nEOF\npip install ruff\n"
|
||||
reason = guard.bash_violation(command)
|
||||
|
||||
Reference in New Issue
Block a user