feat: add gatekeeper plugin for auto-approve safe commands#30
Conversation
Two-layer security plugin for Claude Code: - Layer 1 (PreToolUse): pattern matching for instant allow/deny - Layer 2 (PermissionRequest): AI agent review for unknown commands
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The pre-tool-use.js build output is needed at runtime by the PreToolUse hook, so it must be committed.
- Anchor all DENY rules with ^ to prevent matching in echo/comments - rm -rf / now only matches exact root (not /var, /tmp, /home/...) - rm -rf ~ now only matches current user home (not ~ubuntu, ~admin) - Add regression tests for subdirectory paths and embedded patterns
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the gatekeeper plugin, a valuable security addition for auto-approving safe shell commands. The core logic is sound, but I've identified a critical security vulnerability related to command chaining that needs to be addressed. I've also suggested several other security and usability improvements to the pattern matching rules. My review focuses on making the pattern-matching layer more robust before passing commands to the AI agent.
- Fix crash on valid JSON non-object input (e.g. null, 42, []) - Report invalid JSON to stderr with exit code 1 instead of silent exit 0 - Add .catch() handler to main() for unhandled promise rejections - Document empty stdin passthrough intent with comment
There was a problem hiding this comment.
4 issues found across 16 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="plugins/gatekeeper/src/pre-tool-use.ts">
<violation number="1" location="plugins/gatekeeper/src/pre-tool-use.ts:51">
P0: Security bypass: Command chaining allows dangerous commands to be auto-approved. A command like `ls; rm -rf /` starts with `ls` (matches ALLOW_RULES) but contains destructive operations that won't be caught by DENY_RULES. Consider checking for command separators (`;`, `&&`, `||`, `|`, `$()`, backticks) before auto-allowing, or only allowing commands that don't contain any separators.</violation>
</file>
<file name="plugins/gatekeeper/.gitignore">
<violation number="1" location="plugins/gatekeeper/.gitignore:1">
P2: `!dist/` re-includes the directory but still leaves its contents ignored due to the root `dist` ignore. If you intend to commit built artifacts under `plugins/gatekeeper/dist`, you also need to unignore the files themselves.</violation>
</file>
<file name="plugins/gatekeeper/hooks/hooks.json">
<violation number="1" location="plugins/gatekeeper/hooks/hooks.json:22">
P2: Security policy mismatch: The AI agent prompt approves force pushes to non-main branches, but the README documents git push as "non-force only". This inconsistency could lead to unexpected force pushes being approved. Consider aligning the AI agent policy with the documented behavior, or update the README to reflect that force push to non-main branches is allowed.</violation>
</file>
<file name="plugins/gatekeeper/src/pre-tool-use.test.ts">
<violation number="1" location="plugins/gatekeeper/src/pre-tool-use.test.ts:382">
P1: Missing security test: This test documents that chained commands are allowed if the first command is safe, but the test suite doesn't verify behavior for `npm test && rm -rf /` where a safe command is chained with a dangerous one. Based on the implementation's `^`-anchored patterns, this would be auto-allowed (a security gap). Consider adding a test that explicitly documents this limitation, e.g.:
```typescript
test('SECURITY NOTE: dangerous commands chained after safe commands are auto-allowed', () => {
// This documents a known limitation - only the first command is checked
expectAllow(bash('npm test && rm -rf /'))
})
```</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Add command chaining/substitution detection before DENY/ALLOW checks - Remove exec/x from package manager allowlist (arbitrary code execution risk) - Improve isGitPushNonForce to detect combined short flags (e.g., -vf) - Add rm -rf /* to DENY_RULES for root wildcard deletion - Fix .gitignore to also unignore dist/** contents - Align AI agent prompt with documented non-force-push policy - Update tests for all security changes
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="plugins/gatekeeper/src/pre-tool-use.ts">
<violation number="1" location="plugins/gatekeeper/src/pre-tool-use.ts:63">
P2: The regex `\s-[^\s]*f` matches any flag containing `f`, not just short `-f`. This causes false positives for safe flags like `--follow-tags` and `--no-verify`, preventing `git push --follow-tags` from being auto-approved. Use `\s-(?!-)[^\s]*f` to restrict matching to single-dash flags only.</violation>
<violation number="2" location="plugins/gatekeeper/src/pre-tool-use.ts:95">
P0: Security bypass: the command chaining check (step 0) runs before the DENY check (step 1), so appending `;`, `&&`, `|`, etc. to a destructive command like `rm -rf /` downgrades it from "blocked" to "passthrough." Move the DENY check before the chaining check so known-destructive patterns are always blocked.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="plugins/gatekeeper/hooks/hooks.json">
<violation number="1" location="plugins/gatekeeper/hooks/hooks.json:23">
P1: Downgrading the security review agent from `opus` to `haiku` significantly weakens Layer 2 protection. This agent is the last line of defense for unrecognized commands and is explicitly tasked with detecting obfuscated commands. Haiku's weaker reasoning makes it more susceptible to adversarial inputs (e.g., chained commands hiding destructive operations via `;`, `&&`, `$(...)`) and less reliable at catching nuanced security risks. Consider keeping `opus` (or at least `sonnet`) for this security-critical role, or adding explicit examples of obfuscation patterns to the prompt to compensate.
(Based on your team's feedback about checking for command separators and subshell bypasses before auto-approving.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Replace permissive auto-approve prompt with attack-pattern-focused prompt - Add command chaining analysis guidance (pattern #7) - Add project-scope vs system-scope distinction - Upgrade PermissionRequest agent model to opus per Claude Code team recommendation
- Move DENY check before command chaining detection so destructive commands like `rm -rf / ; echo` are still blocked - Fix isGitPushNonForce regex to use negative lookahead (?!-) to exclude long flags like --follow-tags and --no-verify from false positive force detection - Add tests for deny-before-chaining priority and git push edge cases
Summary
Adds the gatekeeper plugin, a two-layer security system for Claude Code that automatically approves safe commands and blocks dangerous operations while delegating unknown commands to an AI security agent for review.
Architecture
Layer 1: Pattern Matching (PreToolUse Hook)
Layer 2: AI Security Review (PermissionRequest Hook)
Key Features
Changes
plugins/gatekeeper/directory with complete plugin implementationPlugin Structure
Test Plan
Related Documentation
plugins/gatekeeper/README.mdfor usage guide