mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
* fix(hooks): detect Claude Code via stdin session_id, not filtered env (#2520) The #2344 fix assumed `CLAUDECODE` would propagate to hook subprocesses. On Claude Code v2.1.116 it doesn't — Claude Code applies a separate env filter to PreToolUse hook commands that drops bare CLAUDECODE and CLAUDE_SESSION_ID, keeping only CLAUDE_CODE_*-prefixed vars plus CLAUDE_PROJECT_DIR. As a result every Edit/Write on an existing file produced a redundant READ-BEFORE-EDIT advisory inside Claude Code. Use `data.session_id` from the hook's stdin JSON as the primary Claude Code signal (it's part of Claude Code's documented PreToolUse hook-input schema). Keep CLAUDE_CODE_ENTRYPOINT / CLAUDE_CODE_SSE_PORT env checks as propagation-verified fallbacks, and keep the legacy CLAUDE_SESSION_ID / CLAUDECODE checks for back-compat and future-proofing. Add tests/bug-2520-read-guard-hook-subprocess-env.test.cjs, which spawns the hook with an env mirroring the actual Claude Code hook-subprocess filter. Extend the legacy test harnesses to also strip the propagation-verified CLAUDE_CODE_* vars so positive-path tests keep passing when the suite itself runs inside a Claude Code session (same class of leak as #2370 / PR #2375, now covering the new detection signals). Non-Claude-host behavior (OpenCode / MiniMax) is unchanged: with no `session_id` on stdin and no CLAUDE_CODE_* env var, the advisory still fires. Closes #2520 * test(2520): isolate session_id signal from env fallbacks in regression test Per reviewer feedback (Copilot + CodeRabbit on #2521): the session_id isolation test used the helper's default CLAUDE_CODE_ENTRYPOINT / CLAUDE_CODE_SSE_PORT values, so the env fallback would rescue the skip even if the primary `data.session_id` check regressed. Pass an explicit env override that clears those fallbacks, so only the stdin `session_id` signal can trigger the skip. Other cases (env-only fallback, negative / non-Claude host) already override env appropriately. --------- Co-authored-by: forfrossen <forfrossensvart@gmail.com>
This commit is contained in:
@@ -36,8 +36,27 @@ process.stdin.on('end', () => {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Claude Code natively enforces read-before-edit — skip the advisory (#1984, #2344)
|
||||
if (process.env.CLAUDE_SESSION_ID || process.env.CLAUDECODE) {
|
||||
// Claude Code natively enforces read-before-edit — skip the advisory (#1984, #2344, #2520).
|
||||
//
|
||||
// Detection signals, in priority order:
|
||||
// 1. `data.session_id` on the hook's stdin payload — part of Claude
|
||||
// Code's documented PreToolUse hook-input schema, always present.
|
||||
// Reliable across Claude Code versions because it's schema, not env.
|
||||
// 2. `CLAUDE_CODE_ENTRYPOINT` / `CLAUDE_CODE_SSE_PORT` — env vars that
|
||||
// Claude Code does propagate to hook subprocesses (verified on
|
||||
// Claude Code CLI 2.1.116).
|
||||
// 3. `CLAUDE_SESSION_ID` / `CLAUDECODE` — kept for back-compat and in
|
||||
// case future Claude Code versions propagate them to hook
|
||||
// subprocesses. On 2.1.116 they reach Bash tool subprocesses but
|
||||
// not hook subprocesses, which is why checking them alone is
|
||||
// insufficient (regression of #2344 fixed here as #2520).
|
||||
const isClaudeCode =
|
||||
(typeof data.session_id === 'string' && data.session_id.length > 0) ||
|
||||
process.env.CLAUDE_CODE_ENTRYPOINT ||
|
||||
process.env.CLAUDE_CODE_SSE_PORT ||
|
||||
process.env.CLAUDE_SESSION_ID ||
|
||||
process.env.CLAUDECODE;
|
||||
if (isClaudeCode) {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,9 @@ function runHook(payload, envOverrides = {}) {
|
||||
...process.env,
|
||||
CLAUDE_SESSION_ID: '',
|
||||
CLAUDECODE: '',
|
||||
CLAUDE_CODE_ENTRYPOINT: '',
|
||||
CLAUDE_CODE_SSE_PORT: '',
|
||||
CLAUDE_PROJECT_DIR: '',
|
||||
...envOverrides,
|
||||
};
|
||||
try {
|
||||
|
||||
129
tests/bug-2520-read-guard-hook-subprocess-env.test.cjs
Normal file
129
tests/bug-2520-read-guard-hook-subprocess-env.test.cjs
Normal file
@@ -0,0 +1,129 @@
|
||||
/**
|
||||
* Regression test for bug #2520
|
||||
*
|
||||
* The fix for #2344 added `|| process.env.CLAUDECODE` to the Claude Code
|
||||
* skip check. That works in principle — CLAUDECODE=1 is propagated to Bash
|
||||
* tool subprocesses — but it does NOT reach hook subprocesses on Claude Code
|
||||
* v2.1.116. Claude Code applies a separate env filter when spawning
|
||||
* PreToolUse hook commands; that filter drops bare CLAUDECODE and
|
||||
* CLAUDE_SESSION_ID and keeps only CLAUDE_CODE_*-prefixed vars plus
|
||||
* CLAUDE_PROJECT_DIR. `data.session_id` is, however, reliably delivered via
|
||||
* the hook's stdin JSON payload (documented part of Claude Code's hook
|
||||
* input schema).
|
||||
*
|
||||
* Fix: use `data.session_id` as the primary Claude Code signal, with
|
||||
* CLAUDE_CODE_ENTRYPOINT / CLAUDE_CODE_SSE_PORT as env-var fallbacks, and
|
||||
* keep legacy CLAUDECODE / CLAUDE_SESSION_ID for back-compat and
|
||||
* future-proofing.
|
||||
*/
|
||||
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
const { test, describe, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
|
||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
||||
|
||||
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-guard.js');
|
||||
|
||||
/**
|
||||
* Spawn the hook with an env that mirrors the actual Claude Code hook
|
||||
* subprocess env: CLAUDECODE and CLAUDE_SESSION_ID are stripped, only
|
||||
* CLAUDE_CODE_*-prefixed vars (plus CLAUDE_PROJECT_DIR) remain. Extra env
|
||||
* overrides can be supplied via `envOverrides`.
|
||||
*/
|
||||
function runHookInClaudeCodeSubprocess(payload, envOverrides = {}) {
|
||||
const input = JSON.stringify(payload);
|
||||
const baseEnv = { ...process.env };
|
||||
// Strip env vars Claude Code does NOT propagate to hook subprocesses.
|
||||
delete baseEnv.CLAUDECODE;
|
||||
delete baseEnv.CLAUDE_SESSION_ID;
|
||||
const env = {
|
||||
...baseEnv,
|
||||
// Env vars Claude Code DOES propagate to hook subprocesses (observed on
|
||||
// Claude Code CLI 2.1.116).
|
||||
CLAUDE_CODE_ENTRYPOINT: 'cli',
|
||||
CLAUDE_CODE_SSE_PORT: '51291',
|
||||
CLAUDE_PROJECT_DIR: process.cwd(),
|
||||
...envOverrides,
|
||||
};
|
||||
try {
|
||||
const stdout = execFileSync(process.execPath, [HOOK_PATH], {
|
||||
input,
|
||||
encoding: 'utf-8',
|
||||
timeout: 5000,
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env,
|
||||
});
|
||||
return { exitCode: 0, stdout: stdout.trim(), stderr: '' };
|
||||
} catch (err) {
|
||||
return {
|
||||
exitCode: err.status ?? 1,
|
||||
stdout: (err.stdout || '').toString().trim(),
|
||||
stderr: (err.stderr || '').toString().trim(),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
describe('bug #2520: read guard detects Claude Code without relying on CLAUDECODE env', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => { tmpDir = createTempDir('gsd-read-guard-2520-'); });
|
||||
afterEach(() => { cleanup(tmpDir); });
|
||||
|
||||
test('skips advisory when stdin payload includes session_id (Claude Code hook-subprocess env)', () => {
|
||||
const filePath = path.join(tmpDir, 'existing.js');
|
||||
fs.writeFileSync(filePath, 'const x = 1;\n');
|
||||
|
||||
// Isolate the stdin `session_id` signal by clearing the CLAUDE_CODE_*
|
||||
// env fallbacks the helper normally provides. Without this the env
|
||||
// fallback would rescue the skip even if session_id detection broke,
|
||||
// hiding a regression of the primary signal.
|
||||
const result = runHookInClaudeCodeSubprocess(
|
||||
{
|
||||
session_id: 'e7123e54-0977-45dd-848a-b9c8a45a5cd3',
|
||||
tool_name: 'Edit',
|
||||
tool_input: { file_path: filePath, old_string: 'const x = 1;', new_string: 'const x = 2;' },
|
||||
},
|
||||
{ CLAUDE_CODE_ENTRYPOINT: '', CLAUDE_CODE_SSE_PORT: '', CLAUDE_PROJECT_DIR: '' },
|
||||
);
|
||||
|
||||
assert.equal(result.exitCode, 0);
|
||||
assert.equal(
|
||||
result.stdout,
|
||||
'',
|
||||
'advisory must not fire when session_id is present on stdin (real Claude Code hook env)',
|
||||
);
|
||||
});
|
||||
|
||||
test('skips advisory when CLAUDE_CODE_ENTRYPOINT is set (env-var fallback, no session_id on stdin)', () => {
|
||||
const filePath = path.join(tmpDir, 'existing.js');
|
||||
fs.writeFileSync(filePath, 'const x = 1;\n');
|
||||
|
||||
const result = runHookInClaudeCodeSubprocess(
|
||||
{ tool_name: 'Edit', tool_input: { file_path: filePath, old_string: 'const x = 1;', new_string: 'const x = 2;' } },
|
||||
{ CLAUDE_CODE_ENTRYPOINT: 'cli', CLAUDE_CODE_SSE_PORT: '' },
|
||||
);
|
||||
|
||||
assert.equal(result.exitCode, 0);
|
||||
assert.equal(result.stdout, '', 'advisory must not fire when CLAUDE_CODE_ENTRYPOINT is set');
|
||||
});
|
||||
|
||||
test('still injects advisory when no Claude Code signal is present (non-Claude host)', () => {
|
||||
const filePath = path.join(tmpDir, 'existing.js');
|
||||
fs.writeFileSync(filePath, 'const x = 1;\n');
|
||||
|
||||
const result = runHookInClaudeCodeSubprocess(
|
||||
{ tool_name: 'Edit', tool_input: { file_path: filePath, old_string: 'const x = 1;', new_string: 'const x = 2;' } },
|
||||
{ CLAUDE_CODE_ENTRYPOINT: '', CLAUDE_CODE_SSE_PORT: '', CLAUDE_PROJECT_DIR: '' },
|
||||
);
|
||||
|
||||
assert.equal(result.exitCode, 0);
|
||||
assert.ok(result.stdout.length > 0, 'advisory should fire on non-Claude-Code hosts');
|
||||
const output = JSON.parse(result.stdout);
|
||||
assert.ok(output.hookSpecificOutput?.additionalContext?.includes('Read'));
|
||||
});
|
||||
});
|
||||
@@ -28,8 +28,17 @@ const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-guard.js');
|
||||
*/
|
||||
function runHook(payload, envOverrides = {}) {
|
||||
const input = JSON.stringify(payload);
|
||||
// Sanitize CLAUDE_SESSION_ID so positive-path tests work inside Claude Code sessions
|
||||
const env = { ...process.env, CLAUDE_SESSION_ID: '', CLAUDECODE: '', ...envOverrides };
|
||||
// Sanitize all Claude Code detection signals so positive-path tests work
|
||||
// when the test runner itself is running inside Claude Code (#2344, #2520).
|
||||
const env = {
|
||||
...process.env,
|
||||
CLAUDE_SESSION_ID: '',
|
||||
CLAUDECODE: '',
|
||||
CLAUDE_CODE_ENTRYPOINT: '',
|
||||
CLAUDE_CODE_SSE_PORT: '',
|
||||
CLAUDE_PROJECT_DIR: '',
|
||||
...envOverrides,
|
||||
};
|
||||
try {
|
||||
const stdout = execFileSync(process.execPath, [HOOK_PATH], {
|
||||
input,
|
||||
|
||||
Reference in New Issue
Block a user