mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(bash-guard): deny git verbs hidden in command substitutions (#226)
Closes the real bypass antfleet flagged in PR #223 (credit to them for the finding): a denied git verb inside $(...) or backticks is expanded by the shell before the wrapping echo/printf runs, so the skeletonizer's strip hid it from the git check. This reworks it correctly where #223's fix could not land: - targets the live source (docker/scripts/bash-guard-hook.sh, COPY'd to /app/scripts/), not a path that doesn't exist; - runs INSIDE the ROBOCO_GUARD_SKIP_GIT guard, so on grok it stays the native --deny's job and never hard-cancels the run (#223 ran it unconditionally); - excludes single-quoted strings and heredoc bodies (literal / data, matching the skeletonizer), so a README documenting git verbs isn't a false positive; - fails closed (a non-sentinel / python failure denies). 44 bash-guard tests pass (5 new: dollar/backtick/double-quoted substitution deny, single-quoted literal allow, grok-skip allow). Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -8,6 +8,7 @@ code 2 to deny, 0 to allow. Tests wrap each command in that envelope.
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
@@ -204,6 +205,46 @@ def test_allows_unquoted_heredoc_documenting_git() -> None:
|
||||
assert _run(cmd) == _ALLOWED
|
||||
|
||||
|
||||
def _run_skip_git(cmd: str) -> int:
|
||||
"""Run the hook the way grok does — ROBOCO_GUARD_SKIP_GIT=1."""
|
||||
payload = json.dumps({"tool_name": "Bash", "tool_input": {"command": cmd}})
|
||||
result = subprocess.run(
|
||||
[str(GUARD)],
|
||||
input=payload,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
env={**os.environ, "ROBOCO_GUARD_SKIP_GIT": "1"},
|
||||
)
|
||||
return result.returncode
|
||||
|
||||
|
||||
# Command-substitution bypass: a git verb inside $(...) / `...` is expanded by
|
||||
# the shell before the wrapping echo/printf runs, so the skeletonizer's strip
|
||||
# would otherwise hide it from the git check.
|
||||
def test_denies_git_verb_in_dollar_substitution() -> None:
|
||||
assert _run("echo $(git fetch origin)") == _DENIED
|
||||
|
||||
|
||||
def test_denies_git_verb_in_double_quoted_substitution() -> None:
|
||||
assert _run('printf "%s" "$(git push)"') == _DENIED
|
||||
|
||||
|
||||
def test_denies_git_verb_in_backtick_substitution() -> None:
|
||||
assert _run("echo `git push origin main`") == _DENIED
|
||||
|
||||
|
||||
def test_allows_single_quoted_literal_substitution() -> None:
|
||||
# Single-quoted: the shell does NOT expand it — a literal, not a run.
|
||||
assert _run("echo '$(git push)'") == _ALLOWED
|
||||
|
||||
|
||||
def test_substitution_check_skipped_on_grok() -> None:
|
||||
# On grok (SKIP_GIT=1) git is the native --deny's job; the hook must NOT
|
||||
# hard-cancel the run on a substitution.
|
||||
assert _run_skip_git("echo $(git push)") == _ALLOWED
|
||||
|
||||
|
||||
def test_allows_dash_heredoc_documenting_git() -> None:
|
||||
"""`<<-DELIM` indents the closing delimiter; body still stripped."""
|
||||
cmd = "cat > n.md <<-EOF\n\tgit rebase main then git push --force\n\tEOF"
|
||||
|
||||
Reference in New Issue
Block a user