Skip to content

feat(agent-sdk): block destructive Bash at runtime via PreToolUse hook#241

Merged
chrisleekr merged 2 commits into
mainfrom
fix/issue-222
Jun 20, 2026
Merged

feat(agent-sdk): block destructive Bash at runtime via PreToolUse hook#241
chrisleekr merged 2 commits into
mainfrom
fix/issue-222

Conversation

@chrisleekr

Copy link
Copy Markdown
Owner

Summary

The agent subprocess runs under bypassPermissions with 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 SDK PreToolUse hook that physically denies any matching Bash command before the tool executes, making the bans enforceable at runtime. The pattern set is extracted into src/utils/forbidden-bash.ts as 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

flowchart TD
  Trig["Webhook trigger<br/>prompt-only bans"]:::neutral
  Agent["Agent subprocess<br/>bypassPermissions and Bash allowed"]:::neutral
  Hook["PreToolUse hook<br/>src/core/hooks/forbidden-bash.ts"]:::new
  Deny["permissionDecision deny<br/>agent.hook.denied logged, rule label only"]:::new
  Allow["Safe Bash commands pass through"]:::new
  Trig --> Agent --> Hook
  Hook -->|"matches FORBIDDEN"| Deny
  Hook -->|"no match"| Allow
  Shared["src/utils/forbidden-bash.ts<br/>FORBIDDEN plus findForbiddenBash<br/>single source of truth"]:::new
  Guard["scripts/check-no-destructive-actions.ts<br/>build-time CI guard"]:::neutral
  Test["destructive-actions.test.ts"]:::neutral
  Shared --> Hook
  Shared --> Guard
  Shared --> Test
  classDef new fill:#196f3d,color:#ffffff
  classDef neutral fill:#2c3e50,color:#ffffff
Loading

Changes

  • src/utils/forbidden-bash.ts (new): exports FORBIDDEN + 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 +refspec detection to \s\+\w so git push origin +HEAD:main is caught without false-positiving on JS string concatenation in source.
  • src/core/hooks/forbidden-bash.ts (new): createForbiddenBashHook(log) returns a HookCallback; guards on hook_event_name === "PreToolUse" and tool_name === "Bash" before testing the command, denies with permissionDecision: "deny", emits agent.hook.denied (fields tool, rule only — the raw command is never logged, token-leak risk).
  • src/core/executor.ts: wires the hook into queryOptions.hooks.PreToolUse with a "Bash" matcher.
  • scripts/check-no-destructive-actions.ts + test/workflows/ship/destructive-actions.test.ts: drop their inline FORBIDDEN copies, import the shared module.
  • src/core/hooks/forbidden-bash.test.ts (new): 29 cases — table-driven deny across all 14 rules (incl. the +refspec form), allow cases, malformed-input + non-PreToolUse short-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; +refspec bypass closed and pinned
  • Inline FORBIDDEN copies removed from the script + the ship test; both import the shared module; static guard (check:no-destructive) and spawned-script guard test still green
  • typecheck / lint / format / docs:build clean
  • Local full bun test has 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.

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
Copilot AI review requested due to automatic review settings June 20, 2026 13:10
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@chrisleekr, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 54ee537d-cbb1-4759-a66a-caaaefd0e82b

📥 Commits

Reviewing files that changed from the base of the PR and between 56fa714 and d5271f1.

📒 Files selected for processing (9)
  • CLAUDE.md
  • docs/build/architecture.md
  • docs/operate/observability.md
  • scripts/check-no-destructive-actions.ts
  • src/core/executor.ts
  • src/core/hooks/forbidden-bash.test.ts
  • src/core/hooks/forbidden-bash.ts
  • src/utils/forbidden-bash.ts
  • test/workflows/ship/destructive-actions.test.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 FORBIDDEN rule set + matcher helper (findForbiddenBash) as the single source of truth.
  • Wires a PreToolUse hook into agent execution to deny forbidden Bash commands at runtime and emit a structured agent.hook.denied log.
  • 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.

Comment thread src/utils/forbidden-bash.ts
… 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
@chrisleekr chrisleekr merged commit f3132f2 into main Jun 20, 2026
11 checks passed
@chrisleekr chrisleekr deleted the fix/issue-222 branch June 20, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security(agent-sdk): wire PreToolUse hook to runtime-block destructive Bash (force push, gh pr merge, reset --hard) the prompt only forbids

2 participants