Skip to content

fix: avoid dev-hook false positives in non-dev commands #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tsubasakong wants to merge 3 commits into
base: main
Choose a base branch
from
+179 −5
Open

fix: avoid dev-hook false positives in non-dev commands #356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c39bbab
fix: avoid dev-hook false positives in non-dev commands
tsubasakong Mar 7, 2026
d84f965
Merge upstream/main into fix/352-dev-server-hook-heredoc-false-positive
tsubasakong Mar 8, 2026
4eb8db7
test: exercise dev blocker directly
tsubasakong Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 85 additions & 1 deletion scripts/hooks/pre-bash-dev-server-block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@
'use strict';

const MAX_STDIN = 1024 * 1024;
const DEV_COMMAND_WORDS = new Set([
'npm',
'pnpm',
'yarn',
'bun',
'npx',
'bash',
'sh',
'zsh',
'fish',
'tmux'
Comment on lines +13 to +17
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Wrapper commands can still false-positive on quoted text.

Because bash, sh, zsh, fish, and tmux are in DEV_COMMAND_WORDS, Line 165 still runs devPattern on the full raw segment after the wrapper is identified. Cases like bash -lc 'echo "npm run dev"' or tmux display-message "npm run dev" will still be blocked even though they never start a dev server, so the quoted/heredoc false-positive path remains for wrapper commands.

Also applies to: 160-166

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/hooks/pre-bash-dev-server-block.js` around lines 13 - 17, When a
wrapper command from DEV_COMMAND_WORDS is detected, we should not run devPattern
against the entire raw segment because quoted arguments (e.g., bash -lc 'echo
"npm run dev"' or tmux display-message "npm run dev") cause false positives;
instead, extract the inner command string passed to the wrapper (strip wrapper
name and common flags like -c, -lc, --, and tmux/display-message argument
wrappers) or skip quoted/heredoc sections, then run devPattern only on that
extracted/unquoted command; update the logic that currently calls devPattern on
the full raw segment so it first detects wrapper commands in DEV_COMMAND_WORDS,
extracts the actual child command, unquotes or removes heredoc content, and only
then applies devPattern to avoid wrapper-induced false positives.

]);
const SKIPPABLE_PREFIX_WORDS = new Set(['env', 'command', 'builtin', 'exec', 'noglob', 'sudo']);

function splitShellSegments(command) {
const segments = [];
Expand Down Expand Up @@ -37,6 +50,71 @@ function splitShellSegments(command) {
return segments;
}

function readToken(input, startIndex) {
let index = startIndex;
while (index < input.length && /\s/.test(input[index])) index += 1;
if (index >= input.length) return null;

let token = '';
let quote = null;

while (index < input.length) {
const ch = input[index];
if (quote) {
if (ch === quote) {
quote = null;
index += 1;
continue;
}
if (ch === '\\' && quote === '"' && index + 1 < input.length) {
token += input[index + 1];
index += 2;
continue;
}
token += ch;
index += 1;
continue;
}

if (ch === '"' || ch === "'") {
quote = ch;
index += 1;
continue;
}

if (/\s/.test(ch)) break;

if (ch === '\\' && index + 1 < input.length) {
token += input[index + 1];
index += 2;
continue;
}

token += ch;
index += 1;
}

return { token, end: index };
}

function getLeadingCommandWord(segment) {
let index = 0;

while (index < segment.length) {
const parsed = readToken(segment, index);
if (!parsed) return null;
index = parsed.end;

const token = parsed.token;
if (!token) continue;
if (/^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token)) continue;
if (SKIPPABLE_PREFIX_WORDS.has(token)) continue;
return token;
}

return null;
}

let raw = '';
process.stdin.setEncoding('utf8');
process.stdin.on('data', chunk => {
Expand All @@ -56,7 +134,13 @@ process.stdin.on('end', () => {
const tmuxLauncher = /^\s*tmux\s+(new|new-session|new-window|split-window)\b/;
const devPattern = /\b(npm\s+run\s+dev|pnpm(?:\s+run)?\s+dev|yarn\s+dev|bun\s+run\s+dev)\b/;

const hasBlockedDev = segments.some(segment => devPattern.test(segment) && !tmuxLauncher.test(segment));
const hasBlockedDev = segments.some(segment => {
const commandWord = getLeadingCommandWord(segment);
if (!commandWord || !DEV_COMMAND_WORDS.has(commandWord)) {
return false;
}
return devPattern.test(segment) && !tmuxLauncher.test(segment);
});

if (hasBlockedDev) {
console.error('[Hook] BLOCKED: Dev server must run in tmux for log access');
Expand Down
19 changes: 19 additions & 0 deletions tests/integration/hooks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,25 @@ async function runTests() {
}
})) passed++; else failed++;

if (await asyncTest('dev server blocker ignores heredoc prose in non-dev commands', async () => {
const blockingCommand = hooks.hooks.PreToolUse[0].hooks[0].command;
const heredocCommand = [
'gh pr create --title "test" --body "$(cat <<\'EOF\'',
'## Test plan',
'',
'- [ ] Run npm run dev to verify the site starts',
'',
'EOF',
')"'
].join('\n');
const result = await runHookCommand(blockingCommand, {
tool_input: { command: heredocCommand }
});

assert.strictEqual(result.code, 0, 'Non-dev commands with heredoc prose should pass');
assert.ok(!result.stderr.includes('BLOCKED'), 'Should not emit a BLOCKED message');
})) passed++; else failed++;

if (await asyncTest('hooks handle missing files gracefully', async () => {
const testDir = createTestDir();
const transcriptPath = path.join(testDir, 'nonexistent.jsonl');
Expand Down