feat(agent-sdk): block destructive Bash at runtime via PreToolUse hook#241
Conversation
The agent runs bypassPermissions with Bash allowed, so the bans on force push, git reset --hard, gh pr merge, and the GraphQL merge mutations were prompt-only. Add a Claude Agent SDK PreToolUse hook that denies matching Bash commands at runtime (permissionDecision deny), emitting agent.hook.denied (rule label only, never the raw command). Lift the canonical FORBIDDEN set into src/utils/forbidden-bash.ts as the single source of truth shared by the runtime hook, the static check:no-destructive guard, and the regression test. Defense-in-depth backing branch protection, not a sandbox (see #240 for adjacent vectors). Closes #222 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015v2bspPF1ZTTG9ZZrbBgA1
|
Warning Review limit reached
More reviews will be available in 3 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a runtime enforcement layer to prevent destructive git/GitHub operations from being executed via the Claude Agent SDK’s Bash tool when the agent is running under bypassPermissions. It introduces a PreToolUse hook that denies matching commands before execution, and consolidates the forbidden-pattern set into a single shared module to keep runtime enforcement, CI static scanning, and regression tests in sync.
Changes:
- Introduces a shared
FORBIDDENrule set + matcher helper (findForbiddenBash) as the single source of truth. - Wires a
PreToolUsehook into agent execution to deny forbiddenBashcommands at runtime and emit a structuredagent.hook.deniedlog. - Refactors the existing CI guard + ship-workflow regression test to import the shared rule set; adds focused unit tests for the hook.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/utils/forbidden-bash.ts |
New shared forbidden-pattern module used by both runtime and static enforcement. |
src/core/hooks/forbidden-bash.ts |
New PreToolUse hook that blocks forbidden Bash commands and logs denials without logging raw commands. |
src/core/executor.ts |
Wires the PreToolUse hook into Agent SDK queryOptions. |
scripts/check-no-destructive-actions.ts |
Removes inline forbidden patterns; imports from the shared module for CI static scanning. |
test/workflows/ship/destructive-actions.test.ts |
Removes inline forbidden patterns; imports from the shared module for regression coverage. |
src/core/hooks/forbidden-bash.test.ts |
New unit tests validating deny/allow behavior and ensuring logs don’t include raw commands. |
docs/operate/observability.md |
Documents the agent.hook.denied event and its fields/interpretation. |
docs/build/architecture.md |
Documents the runtime destructive-Bash gating layer and its relationship to the CI guard. |
CLAUDE.md |
Adds/updates security invariant #5 to codify runtime destructive-Bash gating and shared pattern source. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… exclusive Address review: a --force-with-lease command now reports its specific rule and is not double-counted by the static scanner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015v2bspPF1ZTTG9ZZrbBgA1
Summary
The agent subprocess runs under
bypassPermissionswith the Bash tool allowed. Before this change, the bans on force-push,git reset --hard,gh pr merge, and GraphQL merge mutations (mergePullRequest,mergeBranch) were enforced only by prompt instructions — a prompt-injected command bypassed them entirely. This PR wires a Claude Agent SDKPreToolUsehook that physically denies any matching Bash command before the tool executes, making the bans enforceable at runtime. The pattern set is extracted intosrc/utils/forbidden-bash.tsas the single source of truth, shared by the new hook, the existing static CI guard, and the regression test (all three inline copies removed) so they cannot drift. This is security invariant #5 in CLAUDE.md.Threat model. Defense-in-depth backing remote branch protection and least-privilege tokens, not a sandbox. A regex denylist on free-form shell cannot defeat obfuscation (variable indirection,
base64 | sh, command substitution); the durable controls remain branch-protection rules + a scoped token. Follow-up #240 tracks adjacent vectors such as.git/writes and future MCP merge tools.Flow
Changes
src/utils/forbidden-bash.ts(new): exportsFORBIDDEN+findForbiddenBash(command)— the single source of truth replacing three inline copies. Two pattern fixes during review: removed a dead(?!-with-lease-if)lookahead (guarded a non-existent flag); broadened+refspecdetection to\s\+\wsogit push origin +HEAD:mainis caught without false-positiving on JS string concatenation in source.src/core/hooks/forbidden-bash.ts(new):createForbiddenBashHook(log)returns aHookCallback; guards onhook_event_name === "PreToolUse"andtool_name === "Bash"before testing the command, denies withpermissionDecision: "deny", emitsagent.hook.denied(fieldstool,ruleonly — the raw command is never logged, token-leak risk).src/core/executor.ts: wires the hook intoqueryOptions.hooks.PreToolUsewith a"Bash"matcher.scripts/check-no-destructive-actions.ts+test/workflows/ship/destructive-actions.test.ts: drop their inlineFORBIDDENcopies, import the shared module.src/core/hooks/forbidden-bash.test.ts(new): 29 cases — table-driven deny across all 14 rules (incl. the+refspecform), allow cases, malformed-input + non-PreToolUseshort-circuits, and an exact-log-keys assertion proving the raw command never enters the warn payload.CLAUDE.md(5th invariant) +docs/build/architecture.md+docs/operate/observability.md(agent.hook.denied).Related issues
Closes #222. Follow-up: #240.
Test plan
src/core/hooks/forbidden-bash.test.ts— 29 cases, all 14 FORBIDDEN rules covered by deny sweep;+refspecbypass closed and pinnedFORBIDDENcopies removed from the script + the ship test; both import the shared module; static guard (check:no-destructive) and spawned-script guard test still greendocs:buildcleanbun testhas only pre-existing DB/Valkey-absent failures (stash-baseline identical); this change adds passing tests with 0 new failures. The isolated CI runner is authoritative.