Add PreToolUse:Bash hook for OPC script directory guard#160
Add PreToolUse:Bash hook for OPC script directory guard#160yurukusa wants to merge 1 commit intoparcadei:mainfrom
Conversation
Prevents running scripts/(mcp|core)/ from wrong directory by requiring cd $CLAUDE_OPC_DIR && prefix. Without this, uv run misses pyproject.toml and causes ModuleNotFoundError. Fixes parcadei#148 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
PR author is not in the allowed authors list. |
📝 WalkthroughWalkthroughA new Bash pre-tool hook is introduced that prevents unintended execution of scripts in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/hooks/src/mcp-directory-guard.ts:
- Around line 33-44: The regex in buildCdPrefixPattern currently doesn't allow
cd targets wrapped in quotes and will fail to recognize commands like cd
"$CLAUDE_OPC_DIR" && ...; update the pattern to accept optional single or double
quotes around the variants (both '\\$CLAUDE_OPC_DIR', '\\$\\{CLAUDE_OPC_DIR\\}'
and escapedDir) so quoted forms match, and wherever the code constructs/suggests
a corrected cd target (the place that uses escapedDir to emit a replacement),
ensure the resolved OPC path is properly escaped and wrapped in quotes so paths
with spaces produce a valid cd "path" && ... suggestion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a3243b87-50d4-4533-bc12-20eb22d73065
⛔ Files ignored due to path filters (1)
.claude/hooks/dist/mcp-directory-guard.mjsis excluded by!**/dist/**,!**/dist/**,!**/.claude/hooks/dist/**
📒 Files selected for processing (2)
.claude/hooks/src/mcp-directory-guard.ts.claude/settings.json
| function buildCdPrefixPattern(opcDir: string | null): RegExp { | ||
| const escapedDir = opcDir ? opcDir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') : ''; | ||
| // Match: cd <opc-dir-variant> && (with flexible whitespace) | ||
| const variants = [ | ||
| '\\$CLAUDE_OPC_DIR', | ||
| '\\$\\{CLAUDE_OPC_DIR\\}', | ||
| ]; | ||
| if (escapedDir) { | ||
| variants.push(escapedDir); | ||
| } | ||
| return new RegExp(`^\\s*cd\\s+(${variants.join('|')})\\s*&&`); | ||
| } |
There was a problem hiding this comment.
Allow quoted cd prefixes and quote corrected cd target.
Line 43 currently rejects valid forms like cd "$CLAUDE_OPC_DIR" && ..., and Line 87 can emit a broken command when the resolved OPC path contains spaces. This can deny correct commands and suggest unusable retries.
🔧 Proposed fix
function buildCdPrefixPattern(opcDir: string | null): RegExp {
const escapedDir = opcDir ? opcDir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') : '';
// Match: cd <opc-dir-variant> && (with flexible whitespace)
const variants = [
'\\$CLAUDE_OPC_DIR',
'\\$\\{CLAUDE_OPC_DIR\\}',
];
if (escapedDir) {
variants.push(escapedDir);
}
- return new RegExp(`^\\s*cd\\s+(${variants.join('|')})\\s*&&`);
+ const target = `(?:${variants.join('|')})`;
+ // allow both: cd $CLAUDE_OPC_DIR && ... and cd "$CLAUDE_OPC_DIR" && ...
+ return new RegExp(`^\\s*cd\\s+(?:(["'])${target}\\1|${target})\\s*&&`);
}
+
+function quoteShellArg(value: string): string {
+ return "'" + value.replace(/'/g, "'\\''") + "'";
+}
@@
- const dirRef = opcDir || '$CLAUDE_OPC_DIR';
+ const dirRef = opcDir ? quoteShellArg(opcDir) : '"$CLAUDE_OPC_DIR"';
const corrected = `cd ${dirRef} && ${command.trimStart()}`;Also applies to: 86-87
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 42-42: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^\\s*cd\\s+(${variants.join('|')})\\s*&&)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/src/mcp-directory-guard.ts around lines 33 - 44, The regex in
buildCdPrefixPattern currently doesn't allow cd targets wrapped in quotes and
will fail to recognize commands like cd "$CLAUDE_OPC_DIR" && ...; update the
pattern to accept optional single or double quotes around the variants (both
'\\$CLAUDE_OPC_DIR', '\\$\\{CLAUDE_OPC_DIR\\}' and escapedDir) so quoted forms
match, and wherever the code constructs/suggests a corrected cd target (the
place that uses escapedDir to emit a replacement), ensure the resolved OPC path
is properly escaped and wrapped in quotes so paths with spaces produce a valid
cd "path" && ... suggestion.
1 participant
Summary
Fixes #148
Adds a
PreToolUse:Bashhook that preventsscripts/(mcp|core)/from running outside the OPC directory. Without this guard,uv runmissesopc/pyproject.tomland its dependencies, causingModuleNotFoundError.scripts/(mcp|core)/pathscd $CLAUDE_OPC_DIR &&(or the resolved path)Example: blocked
uv run python scripts/mcp/research.py --topic "foo"Deny reason includes corrected command:
Example: allowed
Files changed
.claude/hooks/src/mcp-directory-guard.ts— hook implementation using sharedgetOpcDir()andPreToolUseHookOutputtypes.claude/hooks/dist/mcp-directory-guard.mjs— esbuild bundle.claude/settings.json— registered underPreToolUsewith"matcher": "Bash"Test plan
scripts/mcp/path without cd prefix — should be denied with corrected suggestioncd $CLAUDE_OPC_DIR &&prefix — should pass throughscripts/(mcp|core)/— should pass through (no-op)Summary by CodeRabbit