mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-06-30 19:00:57 +08:00
fix(hooks): guard doc-file-warning stdin listeners behind require.main (#2358)
* fix(hooks): guard doc-file-warning stdin listeners behind require.main doc-file-warning.js registered process.stdin data/end listeners at module scope while also exporting run(). run-with-flags.js require()s any hook that exports run() for its in-process fast path, so importing this hook attached stray stdin listeners to the dispatcher process, corrupting the PreToolUse stdout JSON contract. This is the exact failure run-with-flags' own SAFETY comment warns about, and 24 sibling hooks already guard against it. - Move the stdin entrypoint into main() and gate it behind require.main === module - pre-write-doc-warn.js now calls main() explicitly instead of relying on the import side effect - Add regression tests: require() attaches no stdin listeners, run()/main() stay exported, and the pre-write-doc-warn shim still warns * docs(hooks): add JSDoc for doc-file-warning main() entrypoint Satisfies the docstring-coverage pre-merge check; documents the stdin entrypoint and why it must not run on require().
This commit is contained in:
parent
b1d5d6366d
commit
88f6894267
@ -17,7 +17,6 @@ const path = require('path');
|
|||||||
const { buildPreToolUseAdditionalContext } = require('./pretooluse-visible-output');
|
const { buildPreToolUseAdditionalContext } = require('./pretooluse-visible-output');
|
||||||
|
|
||||||
const MAX_STDIN = 1024 * 1024;
|
const MAX_STDIN = 1024 * 1024;
|
||||||
let data = '';
|
|
||||||
|
|
||||||
// Known ad-hoc filenames that indicate impulse/scratch files (case-sensitive, uppercase only)
|
// Known ad-hoc filenames that indicate impulse/scratch files (case-sensitive, uppercase only)
|
||||||
const ADHOC_FILENAMES = /^(NOTES|TODO|SCRATCH|TEMP|DRAFT|BRAINSTORM|SPIKE|DEBUG|WIP)\.(md|txt)$/;
|
const ADHOC_FILENAMES = /^(NOTES|TODO|SCRATCH|TEMP|DRAFT|BRAINSTORM|SPIKE|DEBUG|WIP)\.(md|txt)$/;
|
||||||
@ -70,27 +69,40 @@ function run(inputOrRaw, _options = {}) {
|
|||||||
return { exitCode: 0 };
|
return { exitCode: 0 };
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports = { run };
|
/**
|
||||||
|
* Stdin entrypoint for direct/spawnSync execution: reads the hook payload from
|
||||||
|
* stdin (capped at MAX_STDIN), runs the policy, and writes the PreToolUse result
|
||||||
|
* to stdout. Must only run when invoked directly, never on require(), so the
|
||||||
|
* stdin listeners are not leaked into a parent that loads this hook in-process.
|
||||||
|
*/
|
||||||
|
function main() {
|
||||||
|
let data = '';
|
||||||
|
process.stdin.setEncoding('utf8');
|
||||||
|
process.stdin.on('data', c => {
|
||||||
|
if (data.length < MAX_STDIN) {
|
||||||
|
const remaining = MAX_STDIN - data.length;
|
||||||
|
data += c.substring(0, remaining);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Stdin fallback for spawnSync execution
|
process.stdin.on('end', () => {
|
||||||
process.stdin.setEncoding('utf8');
|
const result = run(data);
|
||||||
process.stdin.on('data', c => {
|
|
||||||
if (data.length < MAX_STDIN) {
|
|
||||||
const remaining = MAX_STDIN - data.length;
|
|
||||||
data += c.substring(0, remaining);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
process.stdin.on('end', () => {
|
if (result.stderr) {
|
||||||
const result = run(data);
|
process.stderr.write(result.stderr + '\n');
|
||||||
|
}
|
||||||
|
|
||||||
if (result.stderr) {
|
if (Object.prototype.hasOwnProperty.call(result, 'additionalContext')) {
|
||||||
process.stderr.write(result.stderr + '\n');
|
process.stdout.write(buildPreToolUseAdditionalContext(result.additionalContext));
|
||||||
}
|
} else {
|
||||||
|
process.stdout.write(data);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
if (Object.prototype.hasOwnProperty.call(result, 'additionalContext')) {
|
module.exports = { run, main };
|
||||||
process.stdout.write(buildPreToolUseAdditionalContext(result.additionalContext));
|
|
||||||
} else {
|
// Stdin fallback for spawnSync execution — only when invoked directly, not via require()
|
||||||
process.stdout.write(data);
|
if (require.main === module) {
|
||||||
}
|
main();
|
||||||
});
|
}
|
||||||
|
|||||||
@ -6,4 +6,5 @@
|
|||||||
|
|
||||||
'use strict';
|
'use strict';
|
||||||
|
|
||||||
require('./doc-file-warning.js');
|
// doc-file-warning.js guards its stdin entrypoint behind require.main; call main() explicitly.
|
||||||
|
require('./doc-file-warning.js').main();
|
||||||
|
|||||||
@ -218,6 +218,47 @@ function runTests() {
|
|||||||
assert.strictEqual(stdout, JSON.stringify(input));
|
assert.strictEqual(stdout, JSON.stringify(input));
|
||||||
}) ? passed++ : failed++);
|
}) ? passed++ : failed++);
|
||||||
|
|
||||||
|
// 11. Regression: requiring the hook in-process (run-with-flags fast path) must not
|
||||||
|
// attach module-scope stdin listeners to the dispatcher process.
|
||||||
|
(test('require() does not attach stdin listeners (in-process safe)', () => {
|
||||||
|
const resolved = require.resolve(script);
|
||||||
|
delete require.cache[resolved];
|
||||||
|
const endBefore = process.stdin.listenerCount('end');
|
||||||
|
const dataBefore = process.stdin.listenerCount('data');
|
||||||
|
const mod = require(resolved);
|
||||||
|
assert.strictEqual(process.stdin.listenerCount('end'), endBefore,
|
||||||
|
'require() must not attach a stdin "end" listener');
|
||||||
|
assert.strictEqual(process.stdin.listenerCount('data'), dataBefore,
|
||||||
|
'require() must not attach a stdin "data" listener');
|
||||||
|
assert.strictEqual(typeof mod.run, 'function', 'run() must remain exported');
|
||||||
|
assert.strictEqual(typeof mod.main, 'function', 'main() must be exported for entrypoints');
|
||||||
|
}) ? passed++ : failed++);
|
||||||
|
|
||||||
|
// 12. Regression: exported run() still classifies correctly in-process, no stdin needed.
|
||||||
|
(test('exported run() works in-process for warned and allowed files', () => {
|
||||||
|
delete require.cache[require.resolve(script)];
|
||||||
|
const { run } = require(script);
|
||||||
|
const warned = run(JSON.stringify({ tool_input: { file_path: 'TODO.md' } }));
|
||||||
|
assert.ok(Array.isArray(warned.additionalContext)
|
||||||
|
&& warned.additionalContext.join('\n').includes('WARNING'),
|
||||||
|
'warned file should return additionalContext with WARNING');
|
||||||
|
const allowed = run(JSON.stringify({ tool_input: { file_path: 'README.md' } }));
|
||||||
|
assert.ok(!('additionalContext' in allowed), 'allowed file should not return additionalContext');
|
||||||
|
}) ? passed++ : failed++);
|
||||||
|
|
||||||
|
// 13. Regression: pre-write-doc-warn.js backward-compat entrypoint still emits the warning.
|
||||||
|
(test('pre-write-doc-warn.js shim still warns via main()', () => {
|
||||||
|
const shim = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'pre-write-doc-warn.js');
|
||||||
|
const r = spawnSync('node', [shim], {
|
||||||
|
encoding: 'utf8',
|
||||||
|
input: JSON.stringify({ tool_input: { file_path: 'TODO.md' } }),
|
||||||
|
timeout: 10000,
|
||||||
|
});
|
||||||
|
assert.strictEqual(r.status || 0, 0, 'shim should exit 0');
|
||||||
|
assert.ok(JSON.parse(r.stdout).hookSpecificOutput.additionalContext.includes('TODO.md'),
|
||||||
|
'shim should still emit the ad-hoc filename warning');
|
||||||
|
}) ? passed++ : failed++);
|
||||||
|
|
||||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
||||||
process.exit(failed > 0 ? 1 : 0);
|
process.exit(failed > 0 ? 1 : 0);
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user