From af2dba2328a010837b306740847bfd8118901625 Mon Sep 17 00:00:00 2001 From: forfrossen <43250388+forfrossen@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:41:58 +0200 Subject: [PATCH] fix(hooks): detect Claude Code via stdin session_id (closes #2520) (#2521) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- hooks/gsd-read-guard.js | 23 +++- ...ug-2344-read-guard-claudecode-env.test.cjs | 3 + ...20-read-guard-hook-subprocess-env.test.cjs | 129 ++++++++++++++++++ tests/read-guard.test.cjs | 13 +- 4 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 tests/bug-2520-read-guard-hook-subprocess-env.test.cjs diff --git a/hooks/gsd-read-guard.js b/hooks/gsd-read-guard.js index 1ec61cd01..4a428e01e 100644 --- a/hooks/gsd-read-guard.js +++ b/hooks/gsd-read-guard.js @@ -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); } diff --git a/tests/bug-2344-read-guard-claudecode-env.test.cjs b/tests/bug-2344-read-guard-claudecode-env.test.cjs index c38596933..0f52425da 100644 --- a/tests/bug-2344-read-guard-claudecode-env.test.cjs +++ b/tests/bug-2344-read-guard-claudecode-env.test.cjs @@ -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 { diff --git a/tests/bug-2520-read-guard-hook-subprocess-env.test.cjs b/tests/bug-2520-read-guard-hook-subprocess-env.test.cjs new file mode 100644 index 000000000..7b16cd2c1 --- /dev/null +++ b/tests/bug-2520-read-guard-hook-subprocess-env.test.cjs @@ -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')); + }); +}); diff --git a/tests/read-guard.test.cjs b/tests/read-guard.test.cjs index 5fa545787..69a8c094c 100644 --- a/tests/read-guard.test.cjs +++ b/tests/read-guard.test.cjs @@ -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,