From 00a8fe96f861b035a61d42fccf04b8a80dec6519 Mon Sep 17 00:00:00 2001 From: Quinn O'Donnell <90010678+Quinnod345@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:29:32 -0400 Subject: [PATCH] fix(hooks): skip stdin listeners on require and match exact tsc paths in post-edit-typecheck config-protection.js and post-edit-typecheck.js attached process.stdin listeners at module scope. tests/hooks/hook-input.test.js requires config-protection.js, so that test never exits while stdin stays open. Attach the listeners only when the script runs directly (require.main === module), as stop-format-typecheck.js and quality-gate.js already do. post-edit-typecheck.js picked tsc diagnostics with line.includes(candidate), so editing a root index.ts reported the errors of src/index.ts, and a Windows relative path (src\index.ts) never matched tsc's forward-slash output. Compare the file at the start of each diagnostic instead, the same check diagnosticMatchesFile() does in stop-format-typecheck.js. Refs #3821 --- scripts/hooks/config-protection.js | 50 +++---- scripts/hooks/post-edit-typecheck.js | 167 +++++++++++++----------- tests/hooks/config-protection.test.js | 25 ++++ tests/hooks/post-edit-typecheck.test.js | 126 ++++++++++++++++++ 4 files changed, 268 insertions(+), 100 deletions(-) create mode 100644 tests/hooks/post-edit-typecheck.test.js diff --git a/scripts/hooks/config-protection.js b/scripts/hooks/config-protection.js index 6adbdfbae..dfc102473 100644 --- a/scripts/hooks/config-protection.js +++ b/scripts/hooks/config-protection.js @@ -232,31 +232,33 @@ function checkTarget(filePath) { module.exports = { run }; // Stdin fallback for spawnSync execution -let truncated = /^(1|true|yes)$/i.test(String(process.env.ECC_HOOK_INPUT_TRUNCATED || '')); -process.stdin.setEncoding('utf8'); -process.stdin.on('data', chunk => { - if (raw.length < MAX_STDIN) { - const remaining = MAX_STDIN - raw.length; - raw += chunk.substring(0, remaining); - if (chunk.length > remaining) truncated = true; - } else { - truncated = true; - } -}); - -process.stdin.on('end', () => { - const result = run(raw, { - truncated, - maxStdin: Number(process.env.ECC_HOOK_INPUT_MAX_BYTES) || MAX_STDIN +if (require.main === module) { + let truncated = /^(1|true|yes)$/i.test(String(process.env.ECC_HOOK_INPUT_TRUNCATED || '')); + process.stdin.setEncoding('utf8'); + process.stdin.on('data', chunk => { + if (raw.length < MAX_STDIN) { + const remaining = MAX_STDIN - raw.length; + raw += chunk.substring(0, remaining); + if (chunk.length > remaining) truncated = true; + } else { + truncated = true; + } }); - if (result.stderr) { - process.stderr.write(result.stderr + '\n'); - } + process.stdin.on('end', () => { + const result = run(raw, { + truncated, + maxStdin: Number(process.env.ECC_HOOK_INPUT_MAX_BYTES) || MAX_STDIN + }); - if (result.exitCode === 2) { - process.exit(2); - } + if (result.stderr) { + process.stderr.write(result.stderr + '\n'); + } - process.stdout.write(raw); -}); + if (result.exitCode === 2) { + process.exit(2); + } + + process.stdout.write(raw); + }); +} diff --git a/scripts/hooks/post-edit-typecheck.js b/scripts/hooks/post-edit-typecheck.js index 28640c0ac..a0c55d2c7 100644 --- a/scripts/hooks/post-edit-typecheck.js +++ b/scripts/hooks/post-edit-typecheck.js @@ -14,92 +14,107 @@ const fs = require("fs"); const path = require("path"); const MAX_DIRECT_STDIN_BYTES = 16 * 1024 * 1024; -let data = ""; -let stdinBytes = 0; -let oversized = false; -process.stdin.setEncoding("utf8"); -process.stdin.on("data", (chunk) => { - if (oversized) return; - stdinBytes += Buffer.byteLength(chunk, "utf8"); - if (stdinBytes > MAX_DIRECT_STDIN_BYTES) { - data = ""; - oversized = true; - return; +// tsc --pretty false starts each diagnostic with the file it is about: +// `src/a.ts(1,14): error TS2322: ...`. Compare that path instead of looking +// for the edited path anywhere in the line, which let a root `index.ts` claim +// the errors of `src/index.ts`. Same check as diagnosticMatchesFile() in +// stop-format-typecheck.js. +function diagnosticMatchesFile(line, paths) { + const location = /^(.+?)\(\d+,\d+\): /.exec(line); + if (!location) return false; + const file = location[1].replace(/\\/g, "/"); + for (const candidate of paths) { + if (file === candidate.replace(/\\/g, "/")) return true; } - data += chunk; -}); + return false; +} -process.stdin.on("end", () => { - if (oversized) { - process.exit(0); - return; - } - try { - const input = JSON.parse(data); - const filePath = input.tool_input?.file_path; +if (require.main === module) { + let data = ""; + let stdinBytes = 0; + let oversized = false; + process.stdin.setEncoding("utf8"); - if (filePath && /\.(ts|tsx)$/.test(filePath)) { - const resolvedPath = path.resolve(filePath); - if (!fs.existsSync(resolvedPath)) { - process.stdout.write(data, () => process.exit(0)); - return; - } - // Find nearest tsconfig.json by walking up (max 20 levels to prevent infinite loop) - let dir = path.dirname(resolvedPath); - const root = path.parse(dir).root; - let depth = 0; + process.stdin.on("data", (chunk) => { + if (oversized) return; + stdinBytes += Buffer.byteLength(chunk, "utf8"); + if (stdinBytes > MAX_DIRECT_STDIN_BYTES) { + data = ""; + oversized = true; + return; + } + data += chunk; + }); - while (dir !== root && depth < 20) { - if (fs.existsSync(path.join(dir, "tsconfig.json"))) { - break; + process.stdin.on("end", () => { + if (oversized) { + process.exit(0); + return; + } + try { + const input = JSON.parse(data); + const filePath = input.tool_input?.file_path; + + if (filePath && /\.(ts|tsx)$/.test(filePath)) { + const resolvedPath = path.resolve(filePath); + if (!fs.existsSync(resolvedPath)) { + process.stdout.write(data, () => process.exit(0)); + return; } - dir = path.dirname(dir); - depth++; - } + // Find nearest tsconfig.json by walking up (max 20 levels to prevent infinite loop) + let dir = path.dirname(resolvedPath); + const root = path.parse(dir).root; + let depth = 0; - if (fs.existsSync(path.join(dir, "tsconfig.json"))) { - try { - // Use npx.cmd on Windows to avoid shell: true which enables command injection - const npxBin = process.platform === "win32" ? "npx.cmd" : "npx"; - execFileSync(npxBin, ["tsc", "--noEmit", "--pretty", "false"], { - cwd: dir, - encoding: "utf8", - stdio: ["pipe", "pipe", "pipe"], - timeout: 30000, - }); - } catch (err) { - // tsc exits non-zero when there are errors — filter to edited file - const output = (err.stdout || "") + (err.stderr || ""); - // Compute paths that uniquely identify the edited file. - // tsc output uses paths relative to its cwd (the tsconfig dir), - // so check for the relative path, absolute path, and original path. - // Avoid bare basename matching — it causes false positives when - // multiple files share the same name (e.g., src/utils.ts vs tests/utils.ts). - const relPath = path.relative(dir, resolvedPath); - const candidates = new Set([filePath, resolvedPath, relPath]); - const relevantLines = output - .split("\n") - .filter((line) => { - for (const candidate of candidates) { - if (line.includes(candidate)) return true; - } - return false; - }) - .slice(0, 10); + while (dir !== root && depth < 20) { + if (fs.existsSync(path.join(dir, "tsconfig.json"))) { + break; + } + dir = path.dirname(dir); + depth++; + } - if (relevantLines.length > 0) { - console.error( - "[Hook] TypeScript errors in " + path.basename(filePath) + ":", - ); - relevantLines.forEach((line) => console.error(line)); + if (fs.existsSync(path.join(dir, "tsconfig.json"))) { + try { + // Use npx.cmd on Windows to avoid shell: true which enables command injection + const npxBin = process.platform === "win32" ? "npx.cmd" : "npx"; + execFileSync(npxBin, ["tsc", "--noEmit", "--pretty", "false"], { + cwd: dir, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + timeout: 30000, + }); + } catch (err) { + // tsc exits non-zero when there are errors — filter to edited file + const output = (err.stdout || "") + (err.stderr || ""); + // Compute paths that uniquely identify the edited file. + // tsc output uses paths relative to its cwd (the tsconfig dir), + // so check for the relative path, absolute path, and original path. + // Avoid bare basename matching — it causes false positives when + // multiple files share the same name (e.g., src/utils.ts vs tests/utils.ts). + const relPath = path.relative(dir, resolvedPath); + const candidates = new Set([filePath, resolvedPath, relPath]); + const relevantLines = output + .split("\n") + .filter((line) => diagnosticMatchesFile(line, candidates)) + .slice(0, 10); + + if (relevantLines.length > 0) { + console.error( + "[Hook] TypeScript errors in " + path.basename(filePath) + ":", + ); + relevantLines.forEach((line) => console.error(line)); + } } } } + } catch { + // Invalid input — pass through } - } catch { - // Invalid input — pass through - } - process.stdout.write(data, () => process.exit(0)); -}); + process.stdout.write(data, () => process.exit(0)); + }); +} + +module.exports = { diagnosticMatchesFile }; diff --git a/tests/hooks/config-protection.test.js b/tests/hooks/config-protection.test.js index 6d78bedac..0e74a2855 100644 --- a/tests/hooks/config-protection.test.js +++ b/tests/hooks/config-protection.test.js @@ -587,6 +587,31 @@ function runTests() { }) ); + results.push( + test('requiring config-protection in-process does not attach stdin listeners or echo stdin', () => { + return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-require-')), tmpDir => { + const protectedPath = path.join(tmpDir, '.eslintrc.js'); + fs.writeFileSync(protectedPath, 'module.exports = {};'); + const configProtectionPath = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'config-protection.js'); + const script = [ + 'const beforeData = process.stdin.listenerCount("data");', + 'const beforeEnd = process.stdin.listenerCount("end");', + `const mod = require(${JSON.stringify(configProtectionPath)});`, + 'if (typeof mod.run !== "function") process.exit(3);', + 'if (process.stdin.listenerCount("data") !== beforeData || process.stdin.listenerCount("end") !== beforeEnd) process.exit(4);' + ].join(''); + const result = spawnSync(process.execPath, ['-e', script], { + input: JSON.stringify({ tool_name: 'Write', tool_input: { file_path: protectedPath } }), + encoding: 'utf8', + timeout: 5000 + }); + assert.strictEqual(result.status, 0, `require() should not attach stdin listeners or run the CLI handler (exit ${result.status}): ${result.stderr}`); + assert.strictEqual(result.stdout, '', 'require() must not echo stdin'); + assert.strictEqual(result.stderr, '', 'require() must not execute the stdin fallback block'); + }); + }) + ); + const passed = results.filter(result => result === 'passed').length; const failed = results.filter(result => result === 'failed').length; const skipped = results.filter(result => result === 'skipped').length; diff --git a/tests/hooks/post-edit-typecheck.test.js b/tests/hooks/post-edit-typecheck.test.js new file mode 100644 index 000000000..ba6a95a36 --- /dev/null +++ b/tests/hooks/post-edit-typecheck.test.js @@ -0,0 +1,126 @@ +/** + * Tests for scripts/hooks/post-edit-typecheck.js + * + * Run with: node tests/hooks/post-edit-typecheck.test.js + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const { diagnosticMatchesFile } = require('../../scripts/hooks/post-edit-typecheck'); + +const SCRIPT = path.resolve(__dirname, '../../scripts/hooks/post-edit-typecheck.js'); +const SKIPPED = Symbol('skipped'); + +function test(name, fn) { + try { + if (fn() === SKIPPED) { + console.log(` SKIP ${name}`); + return 'skipped'; + } + console.log(` PASS ${name}`); + return 'passed'; + } catch (error) { + console.log(` FAIL ${name}`); + console.log(` Error: ${error && error.message ? error.message : error}`); + return 'failed'; + } +} + +const results = []; + +results.push( + test('matches forward-slash tsc paths against Windows edited paths', () => { + const candidates = new Set(['C:\\project\\src\\index.ts', 'src\\index.ts']); + assert.strictEqual(diagnosticMatchesFile('src/index.ts(1,14): error TS2322: Type', candidates), true); + assert.strictEqual(diagnosticMatchesFile('src/other.ts(1,14): error TS2322: Type', candidates), false); + }) +); + +results.push( + test('matches a diagnostic by the file it names, not by a substring of the line', () => { + const rootIndex = new Set(['/project/index.ts', 'index.ts']); + assert.strictEqual(diagnosticMatchesFile('index.ts(1,14): error TS2322: Type', rootIndex), true); + assert.strictEqual(diagnosticMatchesFile('src/index.ts(1,14): error TS2322: Type', rootIndex), false); + assert.strictEqual(diagnosticMatchesFile('other-index.ts(1,14): error TS2322: Type', rootIndex), false); + const utils = new Set(['/project/src/utils.ts', 'src/utils.ts']); + assert.strictEqual(diagnosticMatchesFile("app.ts(3,1): error TS2307: Cannot find module './src/utils.ts'", utils), false); + assert.strictEqual(diagnosticMatchesFile(" Type 'string' is not assignable to type 'number'.", utils), false); + assert.strictEqual(diagnosticMatchesFile('src/utils.ts(1,14): error TS2345: call(1,2): bad', utils), true); + assert.strictEqual(diagnosticMatchesFile('app/(auth)/[...slug]/page.tsx(3,5): error TS2322', new Set(['app/(auth)/[...slug]/page.tsx'])), true); + }) +); + +results.push( + test('requiring the hook does not attach stdin listeners or echo stdin', () => { + const script = [ + 'const beforeData = process.stdin.listenerCount("data");', + 'const beforeEnd = process.stdin.listenerCount("end");', + `const mod = require(${JSON.stringify(SCRIPT)});`, + 'if (typeof mod.diagnosticMatchesFile !== "function") process.exit(3);', + 'if (process.stdin.listenerCount("data") !== beforeData || process.stdin.listenerCount("end") !== beforeEnd) process.exit(4);' + ].join(''); + const result = spawnSync(process.execPath, ['-e', script], { + input: JSON.stringify({ tool_input: { file_path: path.join(os.tmpdir(), 'missing.ts') } }), + encoding: 'utf8', + timeout: 5000 + }); + assert.strictEqual(result.status, 0, `require() should not attach stdin listeners (exit ${result.status}): ${result.stderr}`); + assert.strictEqual(result.stdout, '', 'require() must not echo stdin'); + assert.strictEqual(result.stderr, '', 'require() must not run the stdin handler'); + }) +); + +results.push( + test('hook reports a tsc diagnostic only under the file it names', () => { + // On Windows the hook runs npx.cmd through execFileSync without a shell, + // and Node cannot launch a .cmd file that way, so a stubbed npx never + // runs there. The matcher cases above cover Windows paths. + if (process.platform === 'win32') return SKIPPED; + + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-post-edit-typecheck-')); + try { + const binDir = path.join(projectDir, 'bin'); + fs.mkdirSync(path.join(projectDir, 'src')); + fs.mkdirSync(binDir); + fs.writeFileSync(path.join(projectDir, 'tsconfig.json'), '{}'); + fs.writeFileSync(path.join(projectDir, 'index.ts'), 'export const ok = 1;\n'); + fs.writeFileSync(path.join(projectDir, 'src', 'index.ts'), 'export const bad: number = "x";\n'); + const diagnostic = "src/index.ts(1,14): error TS2322: Type 'string' is not assignable to type 'number'."; + const fakeTsc = path.join(binDir, 'fake-tsc.js'); + fs.writeFileSync(fakeTsc, `process.stdout.write(${JSON.stringify(`${diagnostic}\n`)});\nprocess.exitCode = 2;\n`); + fs.writeFileSync(path.join(binDir, 'npx'), `#!/usr/bin/env node\nrequire(${JSON.stringify(fakeTsc)});\n`); + fs.chmodSync(path.join(binDir, 'npx'), 0o755); + + const runHook = editedFile => + spawnSync(process.execPath, [SCRIPT], { + input: JSON.stringify({ tool_input: { file_path: editedFile } }), + env: { ...process.env, PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}` }, + encoding: 'utf8', + timeout: 10000 + }); + + const rootResult = runHook(path.join(projectDir, 'index.ts')); + assert.strictEqual(rootResult.status, 0, rootResult.stderr); + assert.ok(!rootResult.stderr.includes('TypeScript errors'), `root index.ts must not get src/index.ts errors:\n${rootResult.stderr}`); + + const nestedResult = runHook(path.join(projectDir, 'src', 'index.ts')); + assert.strictEqual(nestedResult.status, 0, nestedResult.stderr); + assert.ok(nestedResult.stderr.includes(`TypeScript errors in index.ts:\n${diagnostic}`), nestedResult.stderr); + } finally { + fs.rmSync(projectDir, { recursive: true, force: true }); + } + return undefined; + }) +); + +const passed = results.filter(result => result === 'passed').length; +const failed = results.filter(result => result === 'failed').length; +const skipped = results.filter(result => result === 'skipped').length; +console.log(`\nResults: Passed: ${passed}, Failed: ${failed}, Skipped: ${skipped}`); +process.exitCode = failed > 0 ? 1 : 0;