From d9331cb17f784bd9221a77830a7dd17306d77671 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 13 Feb 2026 02:22:55 -0800 Subject: [PATCH] fix: eliminate command injection in hooks, fix pass-through newline corruption, add 8 tests Replace shell: true with npx.cmd on Windows in post-edit-format.js and post-edit-typecheck.js to prevent command injection via crafted file paths. Replace console.log(data) with process.stdout.write(data) in check-console-log.js to avoid appending extra newlines to pass-through data. --- scripts/hooks/check-console-log.js | 4 +- scripts/hooks/post-edit-format.js | 7 +-- scripts/hooks/post-edit-typecheck.js | 5 +- tests/hooks/hooks.test.js | 73 ++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/scripts/hooks/check-console-log.js b/scripts/hooks/check-console-log.js index 62d10242..6a08326d 100755 --- a/scripts/hooks/check-console-log.js +++ b/scripts/hooks/check-console-log.js @@ -39,7 +39,7 @@ process.stdin.on('data', chunk => { process.stdin.on('end', () => { try { if (!isGitRepo()) { - console.log(data); + process.stdout.write(data); process.exit(0); } @@ -65,5 +65,5 @@ process.stdin.on('end', () => { } // Always output the original data - console.log(data); + process.stdout.write(data); }); diff --git a/scripts/hooks/post-edit-format.js b/scripts/hooks/post-edit-format.js index c57ef948..827fbd27 100644 --- a/scripts/hooks/post-edit-format.js +++ b/scripts/hooks/post-edit-format.js @@ -27,10 +27,11 @@ process.stdin.on('end', () => { if (filePath && /\.(ts|tsx|js|jsx)$/.test(filePath)) { try { - execFileSync('npx', ['prettier', '--write', filePath], { + // Use npx.cmd on Windows to avoid shell: true which enables command injection + const npxBin = process.platform === 'win32' ? 'npx.cmd' : 'npx'; + execFileSync(npxBin, ['prettier', '--write', filePath], { stdio: ['pipe', 'pipe', 'pipe'], - timeout: 15000, - shell: process.platform === 'win32' + timeout: 15000 }); } catch { // Prettier not installed, file missing, or failed — non-blocking diff --git a/scripts/hooks/post-edit-typecheck.js b/scripts/hooks/post-edit-typecheck.js index b5cfb6eb..31f6c065 100644 --- a/scripts/hooks/post-edit-typecheck.js +++ b/scripts/hooks/post-edit-typecheck.js @@ -49,12 +49,13 @@ process.stdin.on("end", () => { if (fs.existsSync(path.join(dir, "tsconfig.json"))) { try { - execFileSync("npx", ["tsc", "--noEmit", "--pretty", "false"], { + // 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, - shell: process.platform === "win32", }); } catch (err) { // tsc exits non-zero when there are errors — filter to edited file diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 7e184631..e071194c 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -1447,6 +1447,79 @@ async function runTests() { } })) passed++; else failed++; + // ─── Round 20 bug fix tests ─── + console.log('\ncheck-console-log.js (exact pass-through):'); + + if (await asyncTest('stdout is exact byte match of stdin (no trailing newline)', async () => { + // Before the fix, console.log(data) added a trailing \n. + // process.stdout.write(data) should preserve exact bytes. + const stdinData = '{"tool":"test","value":42}'; + const result = await runScript(path.join(scriptsDir, 'check-console-log.js'), stdinData); + assert.strictEqual(result.code, 0); + // stdout should be exactly the input — no extra newline appended + assert.strictEqual(result.stdout, stdinData, 'Should not append extra newline to output'); + })) passed++; else failed++; + + if (await asyncTest('preserves empty string stdin without adding newline', async () => { + const result = await runScript(path.join(scriptsDir, 'check-console-log.js'), ''); + assert.strictEqual(result.code, 0); + assert.strictEqual(result.stdout, '', 'Empty input should produce empty output'); + })) passed++; else failed++; + + if (await asyncTest('preserves data with embedded newlines exactly', async () => { + const stdinData = 'line1\nline2\nline3'; + const result = await runScript(path.join(scriptsDir, 'check-console-log.js'), stdinData); + assert.strictEqual(result.code, 0); + assert.strictEqual(result.stdout, stdinData, 'Should preserve embedded newlines without adding extra'); + })) passed++; else failed++; + + console.log('\npost-edit-format.js (security & extension tests):'); + + if (await asyncTest('source code does not pass shell option to execFileSync (security)', async () => { + const formatSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-format.js'), 'utf8'); + // Strip comments to avoid matching "shell: true" in comment text + const codeOnly = formatSource.replace(/\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, ''); + assert.ok(!codeOnly.includes('shell:'), 'post-edit-format.js should not pass shell option in code'); + assert.ok(formatSource.includes('npx.cmd'), 'Should use npx.cmd for Windows cross-platform safety'); + })) passed++; else failed++; + + if (await asyncTest('matches .tsx extension for formatting', async () => { + const stdinJson = JSON.stringify({ tool_input: { file_path: '/nonexistent/component.tsx' } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-format.js'), stdinJson); + assert.strictEqual(result.code, 0); + // Should attempt to format (will fail silently since file doesn't exist, but should pass through) + assert.ok(result.stdout.includes('component.tsx'), 'Should pass through data for .tsx files'); + })) passed++; else failed++; + + if (await asyncTest('matches .jsx extension for formatting', async () => { + const stdinJson = JSON.stringify({ tool_input: { file_path: '/nonexistent/component.jsx' } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-format.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.ok(result.stdout.includes('component.jsx'), 'Should pass through data for .jsx files'); + })) passed++; else failed++; + + console.log('\npost-edit-typecheck.js (security & extension tests):'); + + if (await asyncTest('source code does not pass shell option to execFileSync (security)', async () => { + const typecheckSource = fs.readFileSync(path.join(scriptsDir, 'post-edit-typecheck.js'), 'utf8'); + // Strip comments to avoid matching "shell: true" in comment text + const codeOnly = typecheckSource.replace(/\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, ''); + assert.ok(!codeOnly.includes('shell:'), 'post-edit-typecheck.js should not pass shell option in code'); + assert.ok(typecheckSource.includes('npx.cmd'), 'Should use npx.cmd for Windows cross-platform safety'); + })) passed++; else failed++; + + if (await asyncTest('matches .tsx extension for type checking', async () => { + const testDir = createTestDir(); + const testFile = path.join(testDir, 'component.tsx'); + fs.writeFileSync(testFile, 'const x: number = 1;'); + + const stdinJson = JSON.stringify({ tool_input: { file_path: testFile } }); + const result = await runScript(path.join(scriptsDir, 'post-edit-typecheck.js'), stdinJson); + assert.strictEqual(result.code, 0); + assert.ok(result.stdout.includes('tool_input'), 'Should pass through data for .tsx files'); + cleanupTestDir(testDir); + })) passed++; else failed++; + // Summary console.log('\n=== Test Results ==='); console.log(`Passed: ${passed}`);