mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(security): bash-guard git-ops check inspects commands, not file content (#165)
The git network/auth deny rule matched its regex against the whole command string, so heredoc bodies and echo/printf arguments that merely documented git verbs (a README, a notes file) were treated as git invocations and denied. This wedged smoke-13's dev: after wiping the README via an Edit/Write fallback it could not restore it because every `cat > README.md << EOF ... git commit ... EOF` was blocked. The git-ops check now runs against a skeleton of the command with heredoc bodies and echo/printf literal args stripped (both are data the shell writes, never executed). Quoted args to a shell interpreter (`bash -c "... && git fetch"`) ARE executed, are not echo/printf/heredoc bodies, and so survive untouched — the hook's core purpose is preserved. A sentinel prefix distinguishes a legitimately-empty skeleton from a python failure (fail closed on failure). All other rules, including the #164 import-bypass rule, still inspect the full command.
This commit is contained in:
@@ -36,8 +36,53 @@ except Exception:
|
||||
|
||||
low=$(printf '%s' "$cmd" | tr "[:upper:]" "[:lower:]")
|
||||
|
||||
# Skeletonize the command for the git-ops check ONLY (#165): strip heredoc
|
||||
# bodies and echo/printf literal arguments. Those are data the shell writes
|
||||
# to a file, never commands the shell executes — so a README/heredoc that
|
||||
# merely documents `git commit` must not be mistaken for invoking git.
|
||||
# Quoted args to a shell interpreter (`bash -c "... && git fetch"`) ARE
|
||||
# executed, are not echo/printf/heredoc bodies, and so survive untouched.
|
||||
# Every other rule below still inspects the full command ($low).
|
||||
git_skel=$(printf '%s' "$cmd" | python3 -c '
|
||||
import sys, re
|
||||
src = sys.stdin.read()
|
||||
lines = src.split("\n")
|
||||
opener = re.compile(r"<<-?\s*[^\sA-Za-z_]*([A-Za-z_]\w*)")
|
||||
kept = []
|
||||
i = 0
|
||||
n = len(lines)
|
||||
while i < n:
|
||||
line = lines[i]
|
||||
kept.append(line)
|
||||
m = opener.search(line)
|
||||
if m:
|
||||
delim = m.group(1)
|
||||
dash = "<<-" in line
|
||||
i += 1
|
||||
while i < n:
|
||||
body = lines[i]
|
||||
cand = body.strip() if dash else body
|
||||
if cand == delim:
|
||||
kept.append(body)
|
||||
break
|
||||
i += 1
|
||||
i += 1
|
||||
skel = "\n".join(kept)
|
||||
skel = re.sub(r"(^|[\n;&|]|&&|\|\|)\s*(echo|printf)\b[^\n;&|]*", r"\1", skel)
|
||||
sys.stdout.write("__SKEL_OK__" + skel)
|
||||
' 2>/dev/null)
|
||||
# A successful run is prefixed with the sentinel even when the skeleton is
|
||||
# legitimately empty (whole command was echo/heredoc). No sentinel means
|
||||
# python failed — fail closed by inspecting the full command.
|
||||
if [[ "$git_skel" == __SKEL_OK__* ]]; then
|
||||
git_skel="${git_skel#__SKEL_OK__}"
|
||||
else
|
||||
git_skel="$cmd"
|
||||
fi
|
||||
git_skel_low=$(printf '%s' "$git_skel" | tr "[:upper:]" "[:lower:]")
|
||||
|
||||
# --- git network / auth ops ---------------------------------------------------
|
||||
if echo "$low" | grep -qE '(^|[[:space:];&|])git[[:space:]]+(fetch|pull|push|clone|remote|ls-remote|checkout|commit|merge|rebase|reset|cherry-pick|revert|tag[[:space:]]+-d|update-ref|reflog[[:space:]]+delete)'; then
|
||||
if echo "$git_skel_low" | grep -qE '(^|[[:space:];&|])git[[:space:]]+(fetch|pull|push|clone|remote|ls-remote|checkout|commit|merge|rebase|reset|cherry-pick|revert|tag[[:space:]]+-d|update-ref|reflog[[:space:]]+delete)'; then
|
||||
echo "Denied: shell git for network / auth / branch-mutating ops is blocked." >&2
|
||||
echo "Use the verb listed in your role's State→Verb table (e.g. commit, complete, i_am_done)." >&2
|
||||
exit 2
|
||||
|
||||
@@ -171,3 +171,82 @@ def test_allows_reading_roboco_source_with_cat() -> None:
|
||||
|
||||
def test_allows_grep_for_roboco_symbol() -> None:
|
||||
assert _run("grep -rn 'import roboco' tests/") == _ALLOWED
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Task #165: git-ops check must inspect commands, not file CONTENT.
|
||||
# A file whose body documents git verbs (README, notes, a heredoc) is data
|
||||
# the shell writes — not a git invocation. It must NOT be denied. But a real
|
||||
# git command (including inside `bash -c "..."`, which IS executed) must
|
||||
# still be denied — that is the hook's entire reason to exist.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_allows_heredoc_readme_documenting_git_verbs() -> None:
|
||||
"""The exact smoke-13 wedge: restoring a README via heredoc whose body
|
||||
explains `git commit` / `git push`. The body is data, not commands."""
|
||||
cmd = (
|
||||
"cat > README.md << 'EOF'\n"
|
||||
"# Project\n"
|
||||
"Run `git commit -m msg` to save your work.\n"
|
||||
"Then `git push` to publish.\n"
|
||||
"EOF"
|
||||
)
|
||||
assert _run(cmd) == _ALLOWED
|
||||
|
||||
|
||||
def test_allows_unquoted_heredoc_documenting_git() -> None:
|
||||
cmd = (
|
||||
"cat > docs/setup.md <<EOF\n"
|
||||
"git clone the repo, then git checkout -b feature.\n"
|
||||
"EOF"
|
||||
)
|
||||
assert _run(cmd) == _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"
|
||||
assert _run(cmd) == _ALLOWED
|
||||
|
||||
|
||||
def test_allows_echo_writing_git_instructions_to_file() -> None:
|
||||
assert _run('echo "remember to git commit and git push" >> notes.md') == _ALLOWED
|
||||
|
||||
|
||||
def test_allows_printf_writing_git_instructions() -> None:
|
||||
assert _run("printf 'git merge then git reset --hard\\n' > steps.txt") == _ALLOWED
|
||||
|
||||
|
||||
def test_allows_python_writing_file_content_mentioning_git() -> None:
|
||||
"""Non-roboco python that writes a string containing git verbs to a
|
||||
file. Not a roboco import (so #164 is irrelevant) and not a git call."""
|
||||
assert _run("python3 -c \"open('r.md','w').write('git push to ship')\"") == _ALLOWED
|
||||
|
||||
|
||||
def test_still_denies_real_git_push() -> None:
|
||||
"""Regression guard: the actual command must still be blocked."""
|
||||
assert _run("git push origin feature/backend/ABC12345") == _DENIED
|
||||
|
||||
|
||||
def test_still_denies_git_in_bash_c_string() -> None:
|
||||
"""The hook's core purpose (per its header): a compound command whose
|
||||
first token is `cd` but which executes `git fetch`. The quoted string
|
||||
is EXECUTED — not a heredoc/echo body — so it must NOT be skeletonized
|
||||
away."""
|
||||
assert _run('bash -c "cd /workspace && git fetch origin"') == _DENIED
|
||||
|
||||
|
||||
def test_still_denies_git_commit_after_cd() -> None:
|
||||
assert _run("cd /workspace && git commit -m 'x'") == _DENIED
|
||||
|
||||
|
||||
def test_still_denies_git_after_echo_separator() -> None:
|
||||
"""echo's args are stripped, but the `&&` boundary is preserved so the
|
||||
real `git push` after it is still seen."""
|
||||
assert _run('echo "starting" && git push') == _DENIED
|
||||
|
||||
|
||||
def test_still_denies_printf_piped_into_git_apply_path() -> None:
|
||||
"""printf body stripped, but `| git checkout` survives the separator."""
|
||||
assert _run("printf 'patch' | git checkout -- .") == _DENIED
|
||||
|
||||
Reference in New Issue
Block a user