mirror of
https://github.com/affaan-m/ECC.git
synced 2026-11-04 15:15:49 +00:00
fix(hooks): stop hook stdin reads when the caller never sends EOF
readStdinRaw() only settled on end, error or close. If the parent kept stdin open without writing EOF, every hook that reads through it waited until the harness killed it (an 8-10 s stall per call). Add an idle deadline (default 3000 ms, ECC_HOOK_INPUT_IDLE_TIMEOUT_MS, 0 disables). It is reset on each chunk, unref()ed, and on expiry detaches from and pauses the stream so a held-open pipe cannot keep the hook alive. Input that already parses as a complete JSON object is accepted; anything else is marked truncated so fail-closed safety hooks still block. bash-hook-dispatcher.js now reads through the shared reader instead of its own end/error-only loop. Fixes #3665 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
90606c8315
commit
0ca9eeb3ce
4 files changed
+256
-20
No files matched your search
@@ -131,6 +131,12 @@ export ECC_DISABLED_HOOKS="pre:bash:tmux-reminder,post:edit:typecheck"
|
||||
# bypass the Bash dispatcher or config-protection checks.
|
||||
export ECC_HOOK_INPUT_MAX_BYTES=524288
|
||||
|
||||
# Stop waiting for hook input after this many idle milliseconds when the
|
||||
# caller never closes stdin (default: 3000; 0 disables the deadline).
|
||||
# Input that is not yet a complete JSON object is treated as incomplete, so
|
||||
# fail-closed safety hooks block instead of hanging until the harness kills them.
|
||||
export ECC_HOOK_INPUT_IDLE_TIMEOUT_MS=5000
|
||||
|
||||
# Disable only GateGuard during setup or recovery
|
||||
export ECC_GATEGUARD=off
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
'use strict';
|
||||
|
||||
const { isHookEnabled } = require('../lib/hook-flags');
|
||||
const { readStdinRaw } = require('./hook-input');
|
||||
const {
|
||||
buildPreToolUseAdditionalContext,
|
||||
combineAdditionalContext,
|
||||
@@ -72,21 +73,6 @@ const POST_BASH_HOOKS = [
|
||||
},
|
||||
];
|
||||
|
||||
function readStdinRaw() {
|
||||
return new Promise(resolve => {
|
||||
let raw = '';
|
||||
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);
|
||||
}
|
||||
});
|
||||
process.stdin.on('end', () => resolve(raw));
|
||||
process.stdin.on('error', () => resolve(raw));
|
||||
});
|
||||
}
|
||||
|
||||
function normalizeHookResult(previousRaw, output) {
|
||||
if (typeof output === 'string' || Buffer.isBuffer(output)) {
|
||||
return {
|
||||
@@ -183,7 +169,9 @@ function runPostBash(rawInput) {
|
||||
|
||||
async function main() {
|
||||
const mode = process.argv[2];
|
||||
const raw = await readStdinRaw();
|
||||
// The shared reader also settles on close and on an idle deadline, so a
|
||||
// parent that never closes stdin cannot hang the dispatcher (#3665).
|
||||
const { raw } = await readStdinRaw(process.stdin, { maxStdin: MAX_STDIN });
|
||||
|
||||
const result = mode === 'post'
|
||||
? runPostBash(raw)
|
||||
|
||||
@@ -3,6 +3,9 @@
|
||||
const { StringDecoder } = require('string_decoder');
|
||||
|
||||
const DEFAULT_MAX_STDIN = 1024 * 1024;
|
||||
// Stay below the shortest per-hook timeout in hooks/hooks.json (5 s) so a
|
||||
// stalled stdin is settled by the reader, not by the harness killing the hook.
|
||||
const DEFAULT_STDIN_IDLE_TIMEOUT_MS = 3000;
|
||||
|
||||
function resolveMaxStdin(value, options = {}) {
|
||||
const writeDiagnostic = options.writeDiagnostic || (() => {});
|
||||
@@ -24,8 +27,38 @@ function resolveMaxStdin(value, options = {}) {
|
||||
return parsed;
|
||||
}
|
||||
|
||||
function resolveStdinIdleTimeout(value, options = {}) {
|
||||
const writeDiagnostic = options.writeDiagnostic || (() => {});
|
||||
if (value === undefined || value === '') return DEFAULT_STDIN_IDLE_TIMEOUT_MS;
|
||||
|
||||
const parsed = Number(value);
|
||||
if (!Number.isSafeInteger(parsed) || parsed < 0) {
|
||||
writeDiagnostic(
|
||||
'[Hook] ECC_HOOK_INPUT_IDLE_TIMEOUT_MS must be a non-negative safe integer; using the 3000 ms default\n'
|
||||
);
|
||||
return DEFAULT_STDIN_IDLE_TIMEOUT_MS;
|
||||
}
|
||||
return parsed;
|
||||
}
|
||||
|
||||
// A JSON object cannot be the prefix of a longer valid document, so a payload
|
||||
// that already parses as one is complete even though EOF never arrived.
|
||||
function isCompleteJsonObject(text) {
|
||||
try {
|
||||
const parsed = JSON.parse(text);
|
||||
return parsed !== null && typeof parsed === 'object';
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function readStdinRaw(stream = process.stdin, options = {}) {
|
||||
const maxStdin = options.maxStdin || DEFAULT_MAX_STDIN;
|
||||
const idleTimeoutMs = options.idleTimeoutMs === undefined
|
||||
? resolveStdinIdleTimeout(process.env.ECC_HOOK_INPUT_IDLE_TIMEOUT_MS)
|
||||
: options.idleTimeoutMs;
|
||||
const writeDiagnostic = options.writeDiagnostic
|
||||
|| (stream === process.stdin ? message => process.stderr.write(message) : () => {});
|
||||
const decoder = new StringDecoder('utf8');
|
||||
let raw = '';
|
||||
let acceptedBytes = 0;
|
||||
@@ -33,7 +66,20 @@ function readStdinRaw(stream = process.stdin, options = {}) {
|
||||
|
||||
return new Promise(resolve => {
|
||||
let settled = false;
|
||||
stream.on('data', chunk => {
|
||||
let idleTimer = null;
|
||||
const clearIdleTimer = () => {
|
||||
if (idleTimer) clearTimeout(idleTimer);
|
||||
idleTimer = null;
|
||||
};
|
||||
const armIdleTimer = () => {
|
||||
if (!(idleTimeoutMs > 0)) return;
|
||||
clearIdleTimer();
|
||||
idleTimer = setTimeout(onIdleTimeout, idleTimeoutMs);
|
||||
// Never keep the process alive just to wait for this deadline.
|
||||
if (typeof idleTimer.unref === 'function') idleTimer.unref();
|
||||
};
|
||||
const onData = chunk => {
|
||||
armIdleTimer();
|
||||
const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
|
||||
if (typeof options.onChunk === 'function') options.onChunk(buffer);
|
||||
const remaining = Math.max(0, maxStdin - acceptedBytes);
|
||||
@@ -43,10 +89,11 @@ function readStdinRaw(stream = process.stdin, options = {}) {
|
||||
acceptedBytes += accepted.length;
|
||||
}
|
||||
if (accepted.length < buffer.length) truncated = true;
|
||||
});
|
||||
};
|
||||
const finish = () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearIdleTimer();
|
||||
if (!truncated) raw += decoder.end();
|
||||
resolve({ raw, truncated });
|
||||
};
|
||||
@@ -55,16 +102,44 @@ function readStdinRaw(stream = process.stdin, options = {}) {
|
||||
truncated = true;
|
||||
finish();
|
||||
};
|
||||
// The writer kept stdin open without sending EOF (#3665). Stop reading
|
||||
// and release the stream so a held-open pipe cannot keep the hook alive.
|
||||
// Anything short of a complete JSON object is marked incomplete so safety
|
||||
// hooks remain fail closed.
|
||||
function onIdleTimeout() {
|
||||
idleTimer = null;
|
||||
if (settled) return;
|
||||
stream.removeListener('data', onData);
|
||||
if (typeof stream.pause === 'function') stream.pause();
|
||||
if (!truncated) {
|
||||
const pending = raw + decoder.end();
|
||||
if (isCompleteJsonObject(pending)) {
|
||||
raw = pending;
|
||||
settled = true;
|
||||
resolve({ raw, truncated: false });
|
||||
return;
|
||||
}
|
||||
raw = pending;
|
||||
}
|
||||
writeDiagnostic(
|
||||
`[Hook] stdin stayed open without EOF for ${idleTimeoutMs} ms; treating the hook input as incomplete\n`
|
||||
);
|
||||
finishIncomplete();
|
||||
}
|
||||
stream.on('data', onData);
|
||||
stream.once('end', finish);
|
||||
// A transport error or premature close can leave a syntactically plausible
|
||||
// prefix behind. Mark it incomplete so safety hooks remain fail closed.
|
||||
stream.once('error', finishIncomplete);
|
||||
stream.once('close', finishIncomplete);
|
||||
armIdleTimer();
|
||||
});
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
DEFAULT_MAX_STDIN,
|
||||
DEFAULT_STDIN_IDLE_TIMEOUT_MS,
|
||||
readStdinRaw,
|
||||
resolveMaxStdin
|
||||
resolveMaxStdin,
|
||||
resolveStdinIdleTimeout
|
||||
};
|
||||
@@ -5,12 +5,20 @@
|
||||
'use strict';
|
||||
|
||||
const assert = require('assert');
|
||||
const path = require('path');
|
||||
const { spawn } = require('child_process');
|
||||
const { PassThrough } = require('stream');
|
||||
const { readStdinRaw } = require('../../scripts/hooks/hook-input');
|
||||
const {
|
||||
DEFAULT_STDIN_IDLE_TIMEOUT_MS,
|
||||
readStdinRaw,
|
||||
resolveStdinIdleTimeout
|
||||
} = require('../../scripts/hooks/hook-input');
|
||||
const { run: runConfigProtection } = require('../../scripts/hooks/config-protection');
|
||||
|
||||
const TEST_STDIN_LIMIT = 1024;
|
||||
const STREAM_SETTLEMENT_TIMEOUT_MS = 500;
|
||||
const TEST_IDLE_TIMEOUT_MS = 50;
|
||||
const PRE_BASH_DISPATCHER = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'pre-bash-dispatcher.js');
|
||||
|
||||
async function test(name, fn) {
|
||||
try {
|
||||
@@ -56,6 +64,34 @@ async function readFromClosedStream(partialInput) {
|
||||
});
|
||||
}
|
||||
|
||||
function settleWithin(promise, ms, label) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => reject(new Error(`${label} did not settle within ${ms} ms`)), ms);
|
||||
promise.then(
|
||||
result => {
|
||||
clearTimeout(timer);
|
||||
resolve(result);
|
||||
},
|
||||
error => {
|
||||
clearTimeout(timer);
|
||||
reject(error);
|
||||
}
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
// The writer keeps the stream open and never sends EOF (#3665).
|
||||
function readFromHeldOpenStream(chunks, options = {}) {
|
||||
const stream = new PassThrough();
|
||||
const resultPromise = readStdinRaw(stream, {
|
||||
maxStdin: TEST_STDIN_LIMIT,
|
||||
idleTimeoutMs: TEST_IDLE_TIMEOUT_MS,
|
||||
...options
|
||||
});
|
||||
for (const chunk of chunks) stream.write(chunk);
|
||||
return { stream, resultPromise };
|
||||
}
|
||||
|
||||
async function runTests() {
|
||||
console.log('\nHook input reader tests:');
|
||||
|
||||
@@ -118,6 +154,137 @@ async function runTests() {
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('held-open stream with no input settles as truncated after the idle deadline', async () => {
|
||||
const { resultPromise } = readFromHeldOpenStream([]);
|
||||
const result = await settleWithin(resultPromise, STREAM_SETTLEMENT_TIMEOUT_MS, 'readStdinRaw');
|
||||
|
||||
assert.deepStrictEqual(result, { raw: '', truncated: true });
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('held-open stream with a partial request settles as truncated', async () => {
|
||||
const partialInput = '{"tool_name":"Write","tool_input":{';
|
||||
const { resultPromise } = readFromHeldOpenStream([partialInput]);
|
||||
const result = await settleWithin(resultPromise, STREAM_SETTLEMENT_TIMEOUT_MS, 'readStdinRaw');
|
||||
|
||||
assert.strictEqual(result.raw, partialInput);
|
||||
assert.strictEqual(result.truncated, true);
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('held-open stream with a complete JSON object settles as complete', async () => {
|
||||
const input = '{"tool_name":"Bash","tool_input":{"command":"ls"}}';
|
||||
const { resultPromise } = readFromHeldOpenStream([input.slice(0, 10), input.slice(10)]);
|
||||
const result = await settleWithin(resultPromise, STREAM_SETTLEMENT_TIMEOUT_MS, 'readStdinRaw');
|
||||
|
||||
assert.deepStrictEqual(result, { raw: input, truncated: false });
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('a complete JSON object past the byte cap stays truncated on the idle deadline', async () => {
|
||||
const input = JSON.stringify({ padding: 'x'.repeat(TEST_STDIN_LIMIT) });
|
||||
const { resultPromise } = readFromHeldOpenStream([input]);
|
||||
const result = await settleWithin(resultPromise, STREAM_SETTLEMENT_TIMEOUT_MS, 'readStdinRaw');
|
||||
|
||||
assert.strictEqual(result.truncated, true);
|
||||
assert.strictEqual(Buffer.byteLength(result.raw), TEST_STDIN_LIMIT);
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('each chunk resets the idle deadline', async () => {
|
||||
const { stream, resultPromise } = readFromHeldOpenStream([]);
|
||||
let settled = false;
|
||||
resultPromise.then(() => {
|
||||
settled = true;
|
||||
});
|
||||
for (const part of ['{"a":', '1', '}']) {
|
||||
await new Promise(resolve => setTimeout(resolve, TEST_IDLE_TIMEOUT_MS / 2));
|
||||
stream.write(part);
|
||||
}
|
||||
assert.strictEqual(settled, false, 'reader settled while chunks were still arriving');
|
||||
stream.end();
|
||||
|
||||
assert.deepStrictEqual(await resultPromise, { raw: '{"a":1}', truncated: false });
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('idleTimeoutMs 0 disables the deadline', async () => {
|
||||
const { stream, resultPromise } = readFromHeldOpenStream([], { idleTimeoutMs: 0 });
|
||||
let settled = false;
|
||||
resultPromise.then(() => {
|
||||
settled = true;
|
||||
});
|
||||
await new Promise(resolve => setTimeout(resolve, TEST_IDLE_TIMEOUT_MS * 3));
|
||||
assert.strictEqual(settled, false, 'reader settled with the deadline disabled');
|
||||
stream.end('{}');
|
||||
|
||||
assert.deepStrictEqual(await resultPromise, { raw: '{}', truncated: false });
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('resolveStdinIdleTimeout validates ECC_HOOK_INPUT_IDLE_TIMEOUT_MS', async () => {
|
||||
const diagnostics = [];
|
||||
const writeDiagnostic = message => diagnostics.push(message);
|
||||
|
||||
assert.strictEqual(resolveStdinIdleTimeout(undefined), DEFAULT_STDIN_IDLE_TIMEOUT_MS);
|
||||
assert.strictEqual(resolveStdinIdleTimeout(''), DEFAULT_STDIN_IDLE_TIMEOUT_MS);
|
||||
assert.strictEqual(resolveStdinIdleTimeout('750'), 750);
|
||||
assert.strictEqual(resolveStdinIdleTimeout('0'), 0);
|
||||
assert.strictEqual(resolveStdinIdleTimeout('-1', { writeDiagnostic }), DEFAULT_STDIN_IDLE_TIMEOUT_MS);
|
||||
assert.strictEqual(resolveStdinIdleTimeout('soon', { writeDiagnostic }), DEFAULT_STDIN_IDLE_TIMEOUT_MS);
|
||||
assert.strictEqual(diagnostics.length, 2);
|
||||
assert.match(diagnostics[0], /ECC_HOOK_INPUT_IDLE_TIMEOUT_MS/);
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
await test('pre-bash dispatcher exits fail closed when stdin is never closed', async () => {
|
||||
const child = spawn(process.execPath, [PRE_BASH_DISPATCHER], {
|
||||
env: { ...process.env, ECC_HOOK_INPUT_IDLE_TIMEOUT_MS: '100', ECC_HOOK_PROFILE: 'standard' },
|
||||
stdio: ['pipe', 'ignore', 'pipe']
|
||||
});
|
||||
let stderr = '';
|
||||
child.stderr.on('data', chunk => {
|
||||
stderr += chunk;
|
||||
});
|
||||
child.stdin.write('{"tool_name":"Bash","tool_input":{');
|
||||
const exitCode = await settleWithin(
|
||||
new Promise(resolve => child.once('exit', code => resolve(code))),
|
||||
5000,
|
||||
'pre-bash-dispatcher'
|
||||
).finally(() => {
|
||||
child.stdin.destroy();
|
||||
child.kill();
|
||||
});
|
||||
|
||||
assert.strictEqual(exitCode, 2);
|
||||
assert.match(stderr, /stdin stayed open without EOF for 100 ms/);
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
console.log(`\nPassed: ${passed}`);
|
||||
console.log(`Failed: ${failed}\n`);
|
||||
process.exitCode = failed > 0 ? 1 : 0;
|
||||
|
||||
Reference in new issue
Block a user