fix(hooks): do not echo raw input from plugin-hook-bootstrap.js

Rebased onto origin/main (49128b576). Fixtures moved from
scripts/hooks/ to /tmp/ecc-pr2380-fixtures/ per reviewer feedback.

(Original commit b0e49036 was based on cc6724ee; main has since
refactored spawnShell to use a shellArgs variable and added
PowerShell .sh fallback paths. This rebase adapts the
const result = spawnSync(...) + __rawInput tagging pattern to
all three spawnSync call sites in spawnShell.)
This commit is contained in:
chs0813
2026-08-14 22:32:30 +08:00
committed by Your Name
parent c9de8f5b2b
commit a224617abb
3 changed files with 326 additions and 25 deletions
+46 -12
View File
@@ -22,15 +22,40 @@ function writeStderr(stderr) {
}
}
function passthrough(raw, result) {
function passthrough(result) {
const stdout = typeof result?.stdout === 'string' ? result.stdout : '';
if (stdout) {
// Most ECC hook scripts follow a `run(rawInput) -> rawInput` passthrough
// pattern: they do their work, then return the original input so the hook
// chain's tool result is preserved. The harness then writes the verbatim
// raw input (tool_input + tool_response, often 1-275 KB) into the session
// transcript as a hook_success attachment -- ~89% of every ECC session's
// transcript is this bloat. Detect the passthrough and emit empty stdout
// instead; the harness falls back to the tool_use's original result, the
// same path #2240 established for bash-hook-dispatcher.
//
// IMPORTANT: a strict `stdout === raw` check misses the common case where
// child processes' synchronous `process.stdout.write()` writes hit the
// ~64 KB Node.js pipe buffer and get truncated -- stdout is then exactly
// 65536 bytes and a strict prefix of raw. So we also detect that
// truncation sentinel.
const raw = typeof result?.__rawInput === 'string' ? result.__rawInput : '';
const STDOUT_PIPE_CAP = 64 * 1024;
const looksLikePassthrough =
(stdout.length === STDOUT_PIPE_CAP && raw.startsWith(stdout)) ||
(raw.length > 0 && stdout === raw);
if (looksLikePassthrough) {
writeStderr(
'[Hook] bootstrap: hook returned raw input as stdout; emitting empty to avoid transcript bloat\n'
);
return;
}
process.stdout.write(stdout);
return;
}
if (!Number.isInteger(result?.status) || result.status === 0) {
process.stdout.write(raw);
writeStderr('[Hook] bootstrap: hook produced no output; emitting empty stdout\n');
}
}
@@ -146,7 +171,7 @@ function spawnNode(rootDir, relPath, raw, args) {
CLAUDE_PLUGIN_ROOT: rootDir,
ECC_PLUGIN_ROOT: rootDir,
};
return spawnSync(process.execPath, [resolveTarget(rootDir, relPath), ...args], {
const result = spawnSync(process.execPath, [resolveTarget(rootDir, relPath), ...args], {
input: raw,
encoding: 'utf8',
env: hookEnv,
@@ -154,6 +179,11 @@ function spawnNode(rootDir, relPath, raw, args) {
timeout: 30000,
windowsHide: true,
});
// Tag result with the raw input so passthrough() can detect the
// "hook returned raw input as stdout" pattern and suppress it
// (the dominant source of session-transcript bloat).
result.__rawInput = raw;
return result;
}
// spawnShell is not used by any hook in the shipped hooks.json configuration
@@ -190,7 +220,7 @@ function spawnShell(rootDir, relPath, raw, args) {
stderr: '[Hook] .sh script requested but no bash binary found on Windows; skipping\n',
};
}
return spawnSync(bash, [scriptPath, ...args], {
const bashResult = spawnSync(bash, [scriptPath, ...args], {
input: raw,
encoding: 'utf8',
env: hookEnv,
@@ -198,6 +228,8 @@ function spawnShell(rootDir, relPath, raw, args) {
timeout: 30000,
windowsHide: true,
});
bashResult.__rawInput = raw;
return bashResult;
}
const shellArgs = isPs
@@ -206,7 +238,7 @@ function spawnShell(rootDir, relPath, raw, args) {
? ['-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-File', scriptPath, ...args]
: [scriptPath, ...args];
return spawnSync(shell, shellArgs, {
const result = spawnSync(shell, shellArgs, {
input: raw,
encoding: 'utf8',
env: hookEnv,
@@ -214,6 +246,8 @@ function spawnShell(rootDir, relPath, raw, args) {
timeout: 30000,
windowsHide: true,
});
result.__rawInput = raw;
return result;
}
function main() {
@@ -224,7 +258,9 @@ function main() {
);
if (!mode || !relPath || !rootDir) {
process.stdout.write(raw);
writeStderr(
'[Hook] bootstrap: missing required args (mode/relPath/rootDir); emitting empty stdout\n'
);
process.exit(0);
}
@@ -235,17 +271,15 @@ function main() {
} else if (mode === 'shell') {
result = spawnShell(rootDir, relPath, raw, args);
} else {
writeStderr(`[Hook] unknown bootstrap mode: ${mode}\n`);
process.stdout.write(raw);
writeStderr(`[Hook] unknown bootstrap mode: ${mode}; emitting empty stdout\n`);
process.exit(0);
}
} catch (error) {
writeStderr(`[Hook] bootstrap resolution failed: ${error.message}\n`);
process.stdout.write(raw);
writeStderr(`[Hook] bootstrap resolution failed: ${error.message}; emitting empty stdout\n`);
process.exit(0);
}
passthrough(raw, result);
passthrough(result);
writeStderr(result.stderr);
if (result.error || result.signal || result.status === null) {
@@ -275,4 +309,4 @@ if (require.main === module || require.main === undefined) {
module.exports = {
main,
normalizePluginRootForPlatform,
};
};
@@ -0,0 +1,256 @@
/**
* Regression tests for plugin-hook-bootstrap.js raw-echo bloat.
*
* Before the fix, every fallthrough path in plugin-hook-bootstrap.js
* (the actual entry point used by ECC plugin hooks, NOT run-with-flags.js)
* echoed the full raw hook input JSON to stdout. For a typical
* PostToolUse:Edit payload this is 10-130 KB of tool_input + tool_response
* per tool call. The harness then wrote that stdout into the session
* transcript as a hook_success attachment, ballooning 51 transcripts
* to a combined 1.06 GB (89% of which was raw-echo bloat).
*
* The fix removes the 4 echo-raw sites in plugin-hook-bootstrap.js:
* - line 137: missing mode/relPath/rootDir
* - line 149: unknown mode
* - line 154: catch on spawn failure
* - line 31: passthrough() default when hook outputs nothing
*
* For each, we emit empty stdout and a stderr explanation. The harness
* then falls back to the tool_use's original result, mirroring the
* pattern already shipped in #2240 (bash-hook-dispatcher.js) and #2227
* (run-with-flags.js truncation path).
*
* Related:
* - #2222 / #2227 — fixed the *truncated* path of run-with-flags.js
* - #2239 / #2240 — fixed the same bug in bash-hook-dispatcher.js
* - #1575 — "token limit so fast" (symptom caused in part by this)
*
* Fixtures live under `/tmp/ecc-pr2380-fixtures/` (per reviewer feedback
* on #2380 — keep temp fixture files out of the live scripts/hooks/ tree).
*/
'use strict';
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { spawnSync } = require('child_process');
const repoRoot = path.join(__dirname, '..', '..');
const bootstrap = path.join(repoRoot, 'scripts', 'hooks', 'plugin-hook-bootstrap.js');
const FIXTURE_DIR = '/tmp/ecc-pr2380-fixtures';
function ensureFixtureDir() {
fs.mkdirSync(FIXTURE_DIR, { recursive: true });
}
function test(name, fn) {
try {
fn();
console.log(`${name}`);
return true;
} catch (error) {
console.log(`${name}`);
console.log(` Error: ${error.message}`);
return false;
}
}
function runBootstrap(args, input, env) {
return spawnSync('node', [bootstrap, ...args], {
input,
encoding: 'utf8',
cwd: repoRoot,
env: { ...process.env, ...(env || {}) },
timeout: 30000,
maxBuffer: 16 * 1024 * 1024,
stdio: ['pipe', 'pipe', 'pipe']
});
}
function realisticPostToolUseEditPayload() {
return JSON.stringify({
session_id: 'test-session',
transcript_path: '/tmp/test.jsonl',
cwd: '/tmp',
permission_mode: 'auto',
hook_event_name: 'PostToolUse',
tool_name: 'Edit',
tool_input: {
file_path: '/tmp/example.ts',
old_string: 'a'.repeat(200),
new_string: 'b'.repeat(200)
},
tool_response: { filePath: '/tmp/example.ts', diff: 'c'.repeat(100 * 1024) },
tool_use_id: 'call_test_1'
});
}
console.log('\nplugin-hook-bootstrap raw-echo (no bloat) tests:');
ensureFixtureDir();
let passed = 0;
let failed = 0;
// --- Bug site #1: line 137 (missing args) ---
if (
test('fallthrough 1: missing mode emits empty stdout (no raw echo)', () => {
const payload = realisticPostToolUseEditPayload();
const result = runBootstrap([], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, '', 'missing-args path must NOT echo raw input (was ' + result.stdout.length + ' bytes)');
})
)
passed++;
else failed++;
// --- Bug site #2: line 149 (unknown mode) ---
if (
test('fallthrough 2: unknown mode emits empty stdout (no raw echo)', () => {
const payload = realisticPostToolUseEditPayload();
const result = runBootstrap(['bogus-mode', path.join(FIXTURE_DIR, 'noop-hook-fixture.js')], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, '', 'unknown-mode path must NOT echo raw input (was ' + result.stdout.length + ' bytes)');
assert.match(result.stderr, /unknown bootstrap mode/);
})
)
passed++;
else failed++;
// --- Bug site #3: line 31 (passthrough default) — THE CORE BUG ---
// This is what fires on EVERY successful hook call where the hook script
// itself didn't write to stdout. The default `passthrough` behavior is
// to echo raw input — which is the bulk of the bloat.
if (
test('fallthrough 3: silent hook does NOT echo raw input (the core bug)', () => {
const payload = realisticPostToolUseEditPayload();
// A no-op node hook that reads stdin and exits silently. Lives in
// /tmp/ecc-pr2380-fixtures/ — NOT in the live scripts/hooks/ tree.
const noopHookPath = path.join(FIXTURE_DIR, 'noop-hook-fixture.js');
fs.writeFileSync(noopHookPath, "process.stdin.resume(); process.stdin.on('end', () => process.exit(0));");
try {
const result = runBootstrap(['node', noopHookPath], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, '', 'silent hook must NOT echo raw input (was ' + result.stdout.length + ' bytes)');
} finally {
fs.unlinkSync(noopHookPath);
}
})
)
passed++;
else failed++;
// --- Bug site #4: tool_response leak guard (the user-visible symptom) ---
if (
test('fallthrough 4: tool_response contents never leak into stdout', () => {
const marker = 'PAYLOAD_MARKER_DO_NOT_LEAK_X9Z42';
const payload = JSON.stringify({
session_id: 'test',
hook_event_name: 'PostToolUse',
tool_name: 'Edit',
tool_input: { file_path: '/tmp/x', old_string: 'A', new_string: 'B' },
tool_response: { filePath: '/tmp/x', leaked: marker, diff: 'x'.repeat(50 * 1024) }
});
const result = runBootstrap([], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
assert.ok(!result.stdout.includes(marker), 'tool_response contents must not appear in stdout');
})
)
passed++;
else failed++;
// --- GREEN-side: behavior we want preserved ---
if (
test('GREEN: hook that outputs JSON is passed through unchanged', () => {
// When the hook legitimately produces output (e.g., PreToolUse
// additionalContext), we must preserve that output verbatim.
const fixturePath = path.join(FIXTURE_DIR, 'echo-fixture.js');
const expectedOutput = '{"hookSpecificOutput":{"permissionDecision":"allow"}}\n';
fs.writeFileSync(
fixturePath,
"process.stdin.resume(); process.stdin.on('end', () => { process.stdout.write('" + expectedOutput.replace(/\n/g, '\\n') + "'); process.exit(0); });"
);
try {
const payload = realisticPostToolUseEditPayload();
const result = runBootstrap(['node', fixturePath], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
assert.ok(result.stdout.length > 0, 'hook that produced output should have non-empty stdout');
// Must not contain the raw input — only the hook's own output
assert.ok(!result.stdout.includes('tool_response'), 'when hook outputs its own stdout, raw input must not also be echoed');
} finally {
fs.unlinkSync(fixturePath);
}
})
)
passed++;
else failed++;
// --- THE CORE ECC PATTERN: most ECC hooks do `process.stdout.write(run(data))`
// where run(data) returns the raw input unchanged. Bootstrap must detect
// this and emit empty stdout instead of writing raw back. ---
if (
test('CORE ECC PATTERN: hook returning raw input as stdout is suppressed', () => {
// Simulate the post-edit-accumulator pattern: read stdin, return it
// unchanged via process.stdout.write. This is THE dominant source of
// transcript bloat — 12+ ECC hook scripts use this exact pattern.
const fixturePath = path.join(FIXTURE_DIR, 'passthrough-fixture.js');
fs.writeFileSync(
fixturePath,
"let d=''; process.stdin.setEncoding('utf8'); process.stdin.on('data', c => d += c); process.stdin.on('end', () => { process.stdout.write(d); process.exit(0); });"
);
try {
const payload = realisticPostToolUseEditPayload();
const result = runBootstrap(['node', fixturePath], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, '', 'hook that returned raw input as stdout must be suppressed (was ' + result.stdout.length + ' bytes)');
assert.match(result.stderr, /returned raw input as stdout/, 'stderr should explain the suppression');
} finally {
fs.unlinkSync(fixturePath);
}
})
)
passed++;
else failed++;
// --- Regression guard: hook with its OWN non-raw output (not equal to raw)
// must still pass through unchanged. ---
if (
test('hook with its own non-raw output passes through unchanged', () => {
const fixturePath = path.join(FIXTURE_DIR, 'own-output-fixture.js');
const ownOutput = '{"hookSpecificOutput":{"additionalContext":"hello"}}\n';
fs.writeFileSync(
fixturePath,
"process.stdin.resume(); process.stdin.on('end', () => { process.stdout.write('" + ownOutput.replace(/\n/g, '\\n').replace(/"/g, '\\"') + "'); process.exit(0); });"
);
try {
const payload = realisticPostToolUseEditPayload();
const result = runBootstrap(['node', fixturePath], payload, {
CLAUDE_PLUGIN_ROOT: repoRoot
});
assert.strictEqual(result.status, 0);
// Should contain the hook's own output, not the raw input
assert.ok(result.stdout.includes('additionalContext'), 'hook own output must be preserved');
assert.ok(!result.stdout.includes('tool_response'), 'raw input must NOT be echoed when hook has its own output');
} finally {
fs.unlinkSync(fixturePath);
}
})
)
passed++;
else failed++;
console.log('\n ' + passed + ' passed, ' + failed + ' failed\n');
process.exit(failed > 0 ? 1 : 0);
+24 -13
View File
@@ -61,12 +61,14 @@ function runTests() {
let passed = 0;
let failed = 0;
if (test('passes stdin through when required bootstrap inputs are missing', () => {
if (test('emits empty stdout and stderr warning when required bootstrap inputs are missing', () => {
const result = run([], { input: '{"ok":true}' });
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, '{"ok":true}');
assert.strictEqual(result.stderr, '');
// Empty stdout (not the raw input) so the harness falls back to the
// tool_use's original result -- prevents session-transcript bloat.
assert.strictEqual(result.stdout, '');
assert.ok(result.stderr.includes('missing required args'));
})) passed++; else failed++;
if (test('normalizes Windows Git Bash POSIX drive roots', () => {
@@ -143,7 +145,7 @@ process.stdout.write(JSON.stringify({
}
})) passed++; else failed++;
if (test('node mode passes original stdin when child exits cleanly without stdout', () => {
if (test('node mode emits empty stdout when child exits cleanly without stdout', () => {
const root = createTempDir();
try {
writeFile(root, path.join('scripts', 'silent.js'), 'process.exit(0);\n');
@@ -154,7 +156,10 @@ process.stdout.write(JSON.stringify({
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, 'raw-input');
// Empty stdout (not the raw input) -- the dominant source of
// session-transcript bloat pre-fix.
assert.strictEqual(result.stdout, '');
assert.ok(result.stderr.includes('emitting empty stdout'));
} finally {
cleanup(root);
}
@@ -225,7 +230,7 @@ process.exit(7);
}
})) passed++; else failed++;
if (test('shell mode fails open when no shell runtime is available', () => {
if (test('shell mode fails open with empty stdout when no shell runtime is available', () => {
const root = createTempDir();
try {
writeFile(root, path.join('scripts', 'hook.sh'), 'printf unreachable\n');
@@ -237,14 +242,16 @@ process.exit(7);
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, 'raw-input');
// Empty stdout (not the raw input) so the harness falls back to the
// tool_use's original result.
assert.strictEqual(result.stdout, '');
assert.ok(result.stderr.includes('shell runtime unavailable'));
} finally {
cleanup(root);
}
})) passed++; else failed++;
if (test('rejects target paths that escape the plugin root', () => {
if (test('rejects target paths that escape the plugin root with empty stdout', () => {
const root = createTempDir();
try {
const result = run(['node', path.join('..', 'outside.js')], {
@@ -253,14 +260,16 @@ process.exit(7);
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, 'raw-input');
// Empty stdout (not the raw input) -- the resolver throws, fallthrough
// path emits empty + stderr explanation.
assert.strictEqual(result.stdout, '');
assert.ok(result.stderr.includes('Path traversal rejected'));
} finally {
cleanup(root);
}
})) passed++; else failed++;
if (test('unknown mode fails open with stderr warning', () => {
if (test('unknown mode fails open with empty stdout and stderr warning', () => {
const root = createTempDir();
try {
const result = run(['python', 'hook.py'], {
@@ -269,7 +278,9 @@ process.exit(7);
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, 'raw-input');
// Empty stdout (not the raw input) -- unknown mode fallthrough path
// emits empty + stderr explanation.
assert.strictEqual(result.stdout, '');
assert.ok(result.stderr.includes('unknown bootstrap mode: python'));
} finally {
cleanup(root);
@@ -375,7 +386,7 @@ process.exit(7);
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, 'raw-input');
assert.strictEqual(result.stdout, '');
assert.ok(
result.stderr.includes('no bash binary found') ||
result.stderr.includes('shell runtime unavailable'),
@@ -391,4 +402,4 @@ process.exit(7);
process.exit(failed > 0 ? 1 : 0);
}
runTests();
runTests();