From 872df03e1fc35686c3f6168aa8defc791b7a6474 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 10 Oct 2026 19:13:24 -0400 Subject: [PATCH] fix(hooks): enforce literal formatter invocation at spawn boundaries --- scripts/hooks/pre-bash-commit-quality.js | 2 +- scripts/hooks/quality-gate.js | 1 + tests/hooks/cost-tracker.test.js | 27 ++++++++++++++++++------ tests/hooks/quality-gate.test.js | 21 +++++++++++++++++- 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/scripts/hooks/pre-bash-commit-quality.js b/scripts/hooks/pre-bash-commit-quality.js index 6504a1b56..b4ec15368 100644 --- a/scripts/hooks/pre-bash-commit-quality.js +++ b/scripts/hooks/pre-bash-commit-quality.js @@ -322,7 +322,7 @@ function getLinterInvocation(command, args, platform = process.platform) { function runLinterCommand(command, args) { try { const invocation = getLinterInvocation(command, args); - return spawnSync(invocation.command, invocation.args, invocation.options); + return spawnSync(invocation.command, invocation.args, { ...invocation.options, shell: false }); } catch (error) { return { status: null, stdout: '', stderr: '', error }; } diff --git a/scripts/hooks/quality-gate.js b/scripts/hooks/quality-gate.js index dd5f5a0fb..733bffbea 100755 --- a/scripts/hooks/quality-gate.js +++ b/scripts/hooks/quality-gate.js @@ -36,6 +36,7 @@ function exec(command, args, cwd = process.cwd()) { const invocation = getLinterInvocation(command, args); return spawnSync(invocation.command, invocation.args, { ...invocation.options, + shell: false, cwd, env: invocation.options.env || process.env, timeout: 15000 diff --git a/tests/hooks/cost-tracker.test.js b/tests/hooks/cost-tracker.test.js index b64815aee..ef6e2463f 100644 --- a/tests/hooks/cost-tracker.test.js +++ b/tests/hooks/cost-tracker.test.js @@ -13,6 +13,12 @@ const { getCostSnapshotPath } = require('../../scripts/lib/session-cost-snapshot const script = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'cost-tracker.js'); +// Harness caches belong to a real private root shared by parent and hook child. +const cacheRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-cost-cache-'))); +const savedTempEnvironment = Object.fromEntries(['TMPDIR', 'TMP', 'TEMP'] + .map(name => [name, process.env[name]])); +for (const name of Object.keys(savedTempEnvironment)) process.env[name] = cacheRoot; + function test(name, fn) { try { fn(); @@ -50,13 +56,13 @@ function runScript(input, envOverrides = {}) { encoding: 'utf8', input: inputStr, timeout: 10000, - env: { ...process.env, ...envOverrides }, + env: { ...process.env, ...envOverrides, TMPDIR: cacheRoot, TMP: cacheRoot, TEMP: cacheRoot }, }); return { code: result.status || 0, stdout: result.stdout || '', stderr: result.stderr || '' }; } function removeHarnessCostCache(sessionId) { - const cachePath = path.join(os.tmpdir(), `harness-cost-${sessionId}.json`); + const cachePath = path.join(cacheRoot, `harness-cost-${sessionId}.json`); try { fs.unlinkSync(cachePath); } catch (err) { @@ -405,7 +411,7 @@ function runTests() { }, }, ]); - const harnessCachePath = path.join(os.tmpdir(), `harness-cost-${sessionId}.json`); + const harnessCachePath = path.join(cacheRoot, `harness-cost-${sessionId}.json`); const nowEpoch = Math.floor(Date.now() / 1000); fs.writeFileSync( harnessCachePath, @@ -449,7 +455,7 @@ function runTests() { ]); fs.writeFileSync( - path.join(os.tmpdir(), `harness-cost-${sessionId}.json`), + path.join(cacheRoot, `harness-cost-${sessionId}.json`), JSON.stringify({ ts: Math.floor(Date.now() / 1000), cost_usd: 999 }), 'utf8' ); @@ -750,7 +756,7 @@ function runTests() { }, }, ]); - const harnessCachePath = path.join(os.tmpdir(), `harness-cost-${sessionId}.json`); + const harnessCachePath = path.join(cacheRoot, `harness-cost-${sessionId}.json`); const staleEpoch = Math.floor(Date.now() / 1000) - 3600; fs.writeFileSync( harnessCachePath, @@ -778,7 +784,14 @@ function runTests() { }) ? passed++ : failed++); console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); - process.exit(failed > 0 ? 1 : 0); + process.exitCode = failed > 0 ? 1 : 0; } -runTests(); +try { + runTests(); +} finally { + for (const [name, value] of Object.entries(savedTempEnvironment)) { + if (value === undefined) delete process.env[name]; else process.env[name] = value; + } + fs.rmSync(cacheRoot, { recursive: true, force: true }); +} diff --git a/tests/hooks/quality-gate.test.js b/tests/hooks/quality-gate.test.js index dfc75f1b4..f9c67d3b0 100644 --- a/tests/hooks/quality-gate.test.js +++ b/tests/hooks/quality-gate.test.js @@ -200,7 +200,7 @@ function withFormatterFixture(formatter, fallback, fn) { ECC_TEST_EXIT: '0' }); const input = JSON.stringify({ tool_input: { file_path: filePath } }); - const runHook = overrides => spawnSync(process.execPath, [hookPath], { + const runHook = (overrides, nodeArguments = []) => spawnSync(process.execPath, [...nodeArguments, hookPath], { cwd: srcDir, input, encoding: 'utf8', timeout: 20000, env: { ...env, ...overrides } }); @@ -248,6 +248,25 @@ for (const formatter of ['prettier', 'biome']) { })) passed++; else failed++; } +if (test('preserves literal formatter arguments even if invocation options drift to shell:true', () => { + withFormatterFixture('prettier', false, fixture => { + const preload = path.join(fixture.root, 'invocation-drift.cjs'); + const helperPath = path.resolve(__dirname, '../../scripts/hooks/pre-bash-commit-quality.js'); + fs.writeFileSync(preload, [ + `const helper = require(${JSON.stringify(helperPath)});`, + 'const original = helper.getLinterInvocation;', + 'helper.getLinterInvocation = (...args) => {', + ' const invocation = original(...args);', + ' invocation.options.shell = true;', + ' return invocation;', + '};' + ].join('\n')); + const result = fixture.runHook({}, ['--require', preload]); + assertInvocation(fixture, result, ['--check', fixture.filePath]); + assert.strictEqual(result.stderr, ''); + }); +})) passed++; else failed++; + if (test('reports a real formatter failure in strict mode while preserving hook input', () => { withFormatterFixture('prettier', false, fixture => { const result = fixture.runHook({ ECC_TEST_EXIT: '2' });