Skip to content

fix(bash-guard): deny git verbs hidden in command substitutions#226

Merged
rennf93 merged 1 commit into
masterfrom
fix/bash-guard-command-substitution
Jun 19, 2026
Merged

fix(bash-guard): deny git verbs hidden in command substitutions#226
rennf93 merged 1 commit into
masterfrom
fix/bash-guard-command-substitution

Conversation

@rennf93

@rennf93 rennf93 commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Closes the command-substitution bypass that antfleet-ops flagged in #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 supersedes #223, whose submitted fix could not land as-is:

  • it targeted the correct source file, but ran the new deny unconditionally — on Grok agents (hook runs with ROBOCO_GUARD_SKIP_GIT=1 for graceful --deny) it would exit 2 and hard-cancel the run;
  • it scanned the raw command, so a single-quoted / heredoc literal $(git …) was a false positive;
  • it failed open on a parse error.

This rework:

  • runs inside the ROBOCO_GUARD_SKIP_GIT guard, so on Grok it stays the native --deny's job (never a run-cancelling deny);
  • 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).

Once this merges, #223 should be closed with a link here.

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).
@github-actions github-actions Bot added tests Test suite changes build Makefile / Docker / Docker Compose / packaging labels Jun 19, 2026
@github-actions

Copy link
Copy Markdown

Thanks for opening your first pull request on RoboCo!

Quick checklist before review (most of these are enforced by CI, but worth a glance):

  • make quality — ruff format check, ruff check, mypy, pytest (≥80% coverage), and the rest of the gate
  • Panel changes pass pnpm lint and pnpm exec tsc --noEmit (run from panel/)
  • No # noqa / # type: ignore shortcuts; pre-existing violations in touched files are fixed
  • Added an entry under ## [Unreleased] in CHANGELOG.md
  • Signed the CLA (the bot will prompt you on this PR)
  • Signed your commits — master requires verified signatures (SSH signing setup)
  • Updated any affected docs under docs/

See CONTRIBUTING.md for the full workflow and the Code of Conduct for the community standards we follow.

Welcome aboard — a maintainer will review shortly.

@rennf93 rennf93 merged commit 50e3127 into master Jun 19, 2026
6 checks passed
@rennf93 rennf93 deleted the fix/bash-guard-command-substitution branch June 19, 2026 18:51
@github-project-automation github-project-automation Bot moved this from Backlog to Done in RoboCo Kanban Jun 19, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

build Makefile / Docker / Docker Compose / packaging tests Test suite changes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant