fix: close Hookify runtime review gaps

This commit is contained in:
Affaan Mustafa
2026-07-26 05:38:49 -04:00
parent 396fadc784
commit ca26e35e9c
9 changed files with 289 additions and 46 deletions
+8 -3
View File
@@ -13,8 +13,8 @@ PreToolUse, PostToolUse, UserPromptSubmit, and Stop.
### Event Types
- `bash`: runs on Bash tool use; a simple `pattern` matches `command`
- `file`: runs on Write/Edit/MultiEdit/NotebookEdit; a simple `pattern`
matches changed content
- `file`: runs on Write/Edit/MultiEdit/NotebookEdit;
a simple `pattern` matches `file_path`
- `stop`: runs when Claude finishes a response; a simple `pattern` matches
the last assistant message
- `prompt`: runs on user message submission; a simple `pattern` matches the
@@ -100,6 +100,10 @@ non-regular files, unsupported YAML structures, unknown fields, invalid
operators, and invalid event/field combinations. It does not read `transcript_path`;
Stop rules use the bounded last assistant message.
Every condition field in an accepted hook input is evaluated completely up to
the 256 KiB input limit. A regex timeout does not discard independent
literal-only rule matches.
Limits per invocation:
- 256 directory entries inspected and 64 rule files evaluated
@@ -124,6 +128,7 @@ the warning to stderr.
- use JavaScript regex syntax; matching is case-insensitive
- for `bash`, match against the full command string
- for a file path, use a `file_path` condition
- for `file`, a simple pattern matches the full file path
- for changed text, use a `content`, `new_text`, or `old_text` condition
- keep regexes narrow even though worker isolation enforces a hard deadline
- test patterns before enabling a blocking rule
+76 -35
View File
@@ -7,13 +7,16 @@
const path = require('path');
const { Worker } = require('worker_threads');
const { evaluateTasks } = require('./hookify-regex-worker');
const WORKER_PATH = path.join(__dirname, 'hookify-regex-worker.js');
const WORKER_RESULT_BYTES = 64 * 1024;
const HEADER_BYTES = Int32Array.BYTES_PER_ELEMENT * 2;
const DEFAULT_TIMEOUT_MS = 250;
const MAX_FIELD_BYTES = 64 * 1024;
const MAX_EDIT_ITEMS = 256;
const MAX_FIELD_BYTES = 256 * 1024;
// Every JSON array entry consumes at least one input byte. This ceiling keeps
// direct callers bounded without excluding any payload accepted by the runner.
const MAX_EDIT_ENTRIES = MAX_FIELD_BYTES;
function truncateUtf8(value, maxBytes = MAX_FIELD_BYTES) {
const input = String(value);
@@ -25,38 +28,62 @@ function truncateUtf8(value, maxBytes = MAX_FIELD_BYTES) {
return encoded.subarray(0, end).toString('utf8');
}
function stringField(value) {
return typeof value === 'string' ? truncateUtf8(value) : null;
function normalizeFieldByteLimit(value) {
const requested = Number(value);
return Number.isInteger(requested) && requested > 0
? Math.min(requested, MAX_FIELD_BYTES)
: MAX_FIELD_BYTES;
}
function editValues(toolInput, field) {
function stringField(value, maxBytes) {
return typeof value === 'string' ? truncateUtf8(value, maxBytes) : null;
}
function editValues(toolInput, field, maxBytes) {
if (!Array.isArray(toolInput.edits)) return null;
const values = [];
for (const edit of toolInput.edits.slice(0, MAX_EDIT_ITEMS)) {
const chunks = [];
let acceptedValues = 0;
let bytes = 0;
let inspectedEntries = 0;
for (const edit of toolInput.edits) {
if (inspectedEntries >= MAX_EDIT_ENTRIES) break;
inspectedEntries += 1;
if (!edit || typeof edit !== 'object' || Array.isArray(edit)) continue;
const value = stringField(edit[field]);
if (value !== null) values.push(value);
if (typeof edit[field] !== 'string') continue;
const separatorBytes = acceptedValues > 0 ? 1 : 0;
const contentBudget = maxBytes - bytes - separatorBytes;
if (contentBudget < 0) break;
const value = truncateUtf8(edit[field], contentBudget);
if (edit[field].length > 0 && value.length === 0 && contentBudget === 0) break;
if (separatorBytes > 0) chunks.push('\n');
chunks.push(value);
acceptedValues += 1;
bytes += separatorBytes + Buffer.byteLength(value, 'utf8');
if (Buffer.byteLength(edit[field], 'utf8') > contentBudget) break;
}
return truncateUtf8(values.join('\n'));
return chunks.join('');
}
function fileContent(toolName, toolInput) {
function fileContent(toolName, toolInput, maxBytes) {
if (toolName === 'MultiEdit') {
return editValues(toolInput, 'new_string') || '';
return editValues(toolInput, 'new_string', maxBytes) || '';
}
if (toolName === 'NotebookEdit') {
return stringField(toolInput.new_source) ?? '';
return stringField(toolInput.new_source, maxBytes) ?? '';
}
return (
stringField(toolInput.content) ??
stringField(toolInput.new_text) ??
stringField(toolInput.new_string) ??
stringField(toolInput.content, maxBytes) ??
stringField(toolInput.new_text, maxBytes) ??
stringField(toolInput.new_string, maxBytes) ??
''
);
}
function extractConditionValue(field, input) {
function extractConditionValue(field, input, options = {}) {
if (!input || typeof input !== 'object' || Array.isArray(input)) return null;
const maxBytes = normalizeFieldByteLimit(options.maxFieldBytes);
const toolName = typeof input.tool_name === 'string' ? input.tool_name : '';
const toolInput = input.tool_input &&
typeof input.tool_input === 'object' &&
@@ -66,38 +93,42 @@ function extractConditionValue(field, input) {
switch (field) {
case 'command':
return toolName === 'Bash' ? stringField(toolInput.command) : null;
return toolName === 'Bash' ? stringField(toolInput.command, maxBytes) : null;
case 'file_path':
return ['Edit', 'Write', 'MultiEdit', 'NotebookEdit'].includes(toolName)
? stringField(toolInput.file_path) ?? stringField(toolInput.notebook_path)
? stringField(toolInput.file_path, maxBytes) ??
stringField(toolInput.notebook_path, maxBytes)
: null;
case 'new_text':
if (toolName === 'MultiEdit') return editValues(toolInput, 'new_string');
if (toolName === 'MultiEdit') return editValues(toolInput, 'new_string', maxBytes);
if (!['Edit', 'Write', 'NotebookEdit'].includes(toolName)) return null;
return (
stringField(toolInput.new_text) ??
stringField(toolInput.new_string) ??
stringField(toolInput.new_source) ??
stringField(toolInput.content)
stringField(toolInput.new_text, maxBytes) ??
stringField(toolInput.new_string, maxBytes) ??
stringField(toolInput.new_source, maxBytes) ??
stringField(toolInput.content, maxBytes)
);
case 'old_text':
if (toolName === 'MultiEdit') return editValues(toolInput, 'old_string');
if (toolName === 'MultiEdit') return editValues(toolInput, 'old_string', maxBytes);
if (!['Edit', 'Write', 'NotebookEdit'].includes(toolName)) return null;
return stringField(toolInput.old_text) ?? stringField(toolInput.old_string);
return (
stringField(toolInput.old_text, maxBytes) ??
stringField(toolInput.old_string, maxBytes)
);
case 'user_prompt':
return input.hook_event_name === 'UserPromptSubmit'
? stringField(input.prompt)
? stringField(input.prompt, maxBytes)
: null;
case 'content':
if (input.hook_event_name === 'Stop') {
return stringField(input.last_assistant_message) ?? '';
return stringField(input.last_assistant_message, maxBytes) ?? '';
}
if (input.hook_event_name === 'UserPromptSubmit') {
return stringField(input.prompt) ?? '';
return stringField(input.prompt, maxBytes) ?? '';
}
if (toolName === 'Bash') return stringField(toolInput.command) ?? '';
if (toolName === 'Bash') return stringField(toolInput.command, maxBytes) ?? '';
if (['Edit', 'Write', 'MultiEdit', 'NotebookEdit'].includes(toolName)) {
return fileContent(toolName, toolInput);
return fileContent(toolName, toolInput, maxBytes);
}
return null;
default:
@@ -213,25 +244,35 @@ function evaluateRules(rules, input, options = {}) {
for (const condition of rule.conditions) fields.add(condition.field);
}
if (tasks.length === 0) return { matches: [], diagnostics: [] };
const maxFieldBytes = normalizeFieldByteLimit(options.maxFieldBytes);
const values = {};
for (const field of fields) {
values[field] = extractConditionValue(field, input);
values[field] = extractConditionValue(field, input, { maxFieldBytes });
}
const literalTasks = tasks.filter(task =>
task.conditions.every(condition => condition.operator !== 'regex_match')
);
const regexTasks = tasks.filter(task =>
task.conditions.some(condition => condition.operator === 'regex_match')
);
const literalResult = evaluateTasks(literalTasks, values);
const requestedTimeout = Number(options.timeoutMs);
const timeoutMs = Number.isFinite(requestedTimeout) && requestedTimeout > 0
? Math.min(Math.floor(requestedTimeout), 1000)
: DEFAULT_TIMEOUT_MS;
const result = runWorker(tasks, values, timeoutMs);
const regexResult = regexTasks.length > 0
? runWorker(regexTasks, values, timeoutMs)
: { matchedIndexes: [], diagnostics: [] };
const matched = new Set(
result.matchedIndexes.filter(
[...literalResult.matchedIndexes, ...regexResult.matchedIndexes].filter(
index => Number.isInteger(index) && index >= 0 && index < rules.length
)
);
return {
matches: rules.filter((_rule, index) => matched.has(index)),
diagnostics: result.diagnostics,
diagnostics: [...literalResult.diagnostics, ...regexResult.diagnostics],
};
}
+1
View File
@@ -210,6 +210,7 @@ function validateToolMatcher(value) {
function inferredField(event) {
if (event === 'bash') return 'command';
if (event === 'file') return 'file_path';
if (event === 'prompt') return 'user_prompt';
return 'content';
}
+1
View File
@@ -316,6 +316,7 @@ function run(rawInput, context = {}) {
});
const evaluated = evaluateRules(loaded.rules, payload, {
timeoutMs: LIMITS.regexTimeoutMs,
maxFieldBytes: LIMITS.maxInputBytes,
});
const output = buildOutput(eventName, evaluated.matches, [
...loaded.diagnostics,
+8 -2
View File
@@ -40,6 +40,10 @@ Can include markdown formatting, warnings, suggestions, etc.
| conditions | Yes* | list | All field/operator/pattern entries must match (*use exactly one of pattern or conditions) |
| tool_matcher | No | `*` or exact names separated by `\|` | Limits a rule to tools such as `Bash` or `Write\|Edit` |
Simple `event: file` patterns match `file_path`.
Matching changed content requires explicit conditions using `content`,
`new_text`, or `old_text`.
### Advanced Format (Multiple Conditions)
```markdown
@@ -82,10 +86,12 @@ Match Bash command patterns:
- Permission issues: `chmod\s+777`
### file Events
Match Edit/Write/MultiEdit operations:
Simple patterns match the target path for Edit/Write/MultiEdit/NotebookEdit:
- Sensitive files: `\.env$`, `credentials`, `\.pem$`
Use explicit `content`, `new_text`, or `old_text` conditions for changed text:
- Debug code: `console\.log\(`, `debugger`
- Security risks: `eval\(`, `innerHTML\s*=`
- Sensitive files: `\.env$`, `credentials`, `\.pem$`
### stop Events
Completion checks and reminders against the last assistant message. Pattern
+9
View File
@@ -51,6 +51,15 @@ function runTests() {
}
})) passed++; else failed++;
if (test('simple file patterns retain the file-path contract', () => {
const help = read('commands/hookify-help.md');
const skill = read('skills/hookify-rules/SKILL.md');
assert.ok(help.includes('simple `pattern` matches `file_path`'));
assert.ok(skill.includes('Simple `event: file` patterns match `file_path`'));
assert.ok(skill.includes('changed content requires explicit conditions'));
})) passed++; else failed++;
if (test('authoring command and skill document the complete supported schema', () => {
const authoring = read('commands/hookify.md');
const skill = read('skills/hookify-rules/SKILL.md');
+119 -6
View File
@@ -191,6 +191,105 @@ function runTests() {
assert.strictEqual(JSON.stringify(input), snapshot);
})) passed++; else failed++;
if (test('blocking rules inspect complete accepted fields beyond the old 64 KiB boundary', () => {
const sentinel = 'HOOKIFY_BLOCK_AFTER_64_KIB';
const longPrefix = 'x'.repeat(70 * 1024);
const cases = [
{
field: 'command',
input: {
hook_event_name: 'PreToolUse',
tool_name: 'Bash',
tool_input: { command: `${longPrefix}${sentinel}` },
},
},
{
field: 'content',
input: {
hook_event_name: 'PreToolUse',
tool_name: 'Write',
tool_input: {
file_path: '/repo/output.txt',
content: `${longPrefix}${sentinel}`,
},
},
},
{
field: 'user_prompt',
input: {
hook_event_name: 'UserPromptSubmit',
prompt: `${longPrefix}${sentinel}`,
},
},
{
field: 'content',
input: {
hook_event_name: 'Stop',
last_assistant_message: `${longPrefix}${sentinel}`,
},
},
{
field: 'new_text',
input: {
hook_event_name: 'PreToolUse',
tool_name: 'MultiEdit',
tool_input: {
file_path: '/repo/output.txt',
edits: [
{ old_string: 'before', new_string: longPrefix },
{ old_string: 'after', new_string: sentinel },
],
},
},
},
];
for (const { field, input } of cases) {
assert.ok(
Buffer.byteLength(JSON.stringify(input), 'utf8') < 256 * 1024,
`${field} fixture must fit within the accepted hook input`
);
const blockingRule = rule({
action: 'block',
conditions: [{ field, operator: 'contains', pattern: sentinel }],
});
assert.deepStrictEqual(
evaluateRules([blockingRule], input).matches.map(item => item.name),
['test-rule'],
`${field} must be evaluated through its complete accepted value`
);
}
const manyEditsInput = {
hook_event_name: 'PreToolUse',
tool_name: 'MultiEdit',
tool_input: {
file_path: '/repo/output.txt',
edits: [
...Array.from(
{ length: 300 },
() => ({ old_string: 'before', new_string: 'ordinary edit' })
),
{ old_string: 'after', new_string: sentinel },
],
},
};
assert.ok(
Buffer.byteLength(JSON.stringify(manyEditsInput), 'utf8') < 256 * 1024,
'MultiEdit fixture must fit within the accepted hook input'
);
assert.deepStrictEqual(
evaluateRules([
rule({
action: 'block',
conditions: [{ field: 'new_text', operator: 'contains', pattern: sentinel }],
}),
], manyEditsInput).matches.map(item => item.name),
['test-rule'],
'every edit in an accepted MultiEdit payload must be evaluated'
);
})) passed++; else failed++;
if (test('fails open with a sanitized diagnostic for invalid regex syntax', () => {
const input = {
hook_event_name: 'PreToolUse',
@@ -211,31 +310,45 @@ function runTests() {
assert.ok(!result.diagnostics[0].message.includes('anything'));
})) passed++; else failed++;
if (test('terminates catastrophic regex evaluation at one hard total deadline', () => {
if (test('a regex timeout preserves an unrelated literal-only block match', () => {
const input = {
hook_event_name: 'PreToolUse',
tool_name: 'Bash',
tool_input: { command: `${'a'.repeat(30000)}!` },
tool_input: { command: `literal-hit ${'a'.repeat(30000)}!` },
};
const dangerousRules = Array.from({ length: 8 }, (_, index) => rule({
name: `danger-${index}`,
source: `hookify.danger-${index}.local.md`,
conditions: [{ field: 'command', operator: 'regex_match', pattern: '(a+)+$' }],
}));
const literalBlock = rule({
name: 'literal-block',
action: 'block',
conditions: [{ field: 'command', operator: 'contains', pattern: 'literal-hit' }],
});
const startedAt = Date.now();
const result = evaluateRules(dangerousRules, input, { timeoutMs: 100 });
const result = evaluateRules([...dangerousRules, literalBlock], input, { timeoutMs: 100 });
const elapsed = Date.now() - startedAt;
assert.deepStrictEqual(result.matches, []);
assert.deepStrictEqual(result.matches.map(item => item.name), ['literal-block']);
assert.ok(result.diagnostics.some(item => item.code === 'HOOKIFY_REGEX_TIMEOUT'));
assert.ok(elapsed < 1500, `regex worker should be terminated promptly, took ${elapsed}ms`);
})) passed++; else failed++;
if (test('bounds multibyte fields and handles absent or incompatible field shapes', () => {
const longValue = `${'a'.repeat(65535)}界tail`;
const acceptedValue = `${'a'.repeat(70 * 1024)}界tail`;
const preservedValue = truncateUtf8(acceptedValue);
assert.strictEqual(
Buffer.byteLength(preservedValue, 'utf8'),
Buffer.byteLength(acceptedValue, 'utf8'),
'a field within the hook input cap must not be partially evaluated'
);
assert.ok(preservedValue.endsWith('界tail'));
const longValue = `${'a'.repeat(256 * 1024 - 1)}界tail`;
const truncated = truncateUtf8(longValue);
assert.ok(Buffer.byteLength(truncated, 'utf8') <= 64 * 1024);
assert.ok(Buffer.byteLength(truncated, 'utf8') <= 256 * 1024);
assert.ok(!truncated.includes('\ufffd'));
assert.strictEqual(extractConditionValue('command', null), null);
+16
View File
@@ -112,6 +112,22 @@ function runTests() {
});
})) passed++; else failed++;
if (test('simple file patterns preserve the documented file_path matcher', () => {
const loaded = validateRule({
name: 'protect-env-files',
enabled: true,
event: 'file',
action: 'block',
pattern: '\\.env$',
}, 'Do not edit environment files.', 'hookify.protect-env-files.local.md');
assert.deepStrictEqual(loaded.conditions, [{
field: 'file_path',
operator: 'regex_match',
pattern: '\\.env$',
}]);
})) passed++; else failed++;
if (test('rejects unknown fields, ambiguous matchers, invalid operators, and event-incompatible fields', () => {
withProject(({ projectRoot, claudeDir }) => {
writeRule(
+51
View File
@@ -141,6 +141,57 @@ function runTests() {
});
})) passed++; else failed++;
if (test('PreToolUse blocks inspect accepted command text beyond 64 KiB', () => {
withProject(({ projectRoot, claudeDir }) => {
const sentinel = 'HOOKIFY_BLOCK_AFTER_64_KIB';
writeRule(claudeDir, {
action: 'block',
pattern: sentinel,
message: 'This late command content is prohibited.',
});
const payload = {
hook_event_name: 'PreToolUse',
tool_name: 'Bash',
tool_input: { command: `${'x'.repeat(70 * 1024)}${sentinel}` },
};
assert.ok(Buffer.byteLength(JSON.stringify(payload), 'utf8') < LIMITS.maxInputBytes);
const output = invoke(projectRoot, 'PreToolUse', payload);
assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
assert.ok(
output.hookSpecificOutput.permissionDecisionReason.includes(
'This late command content is prohibited.'
)
);
});
})) passed++; else failed++;
if (test('simple file patterns block matching file paths rather than changed content', () => {
withProject(({ projectRoot, claudeDir }) => {
writeRule(claudeDir, {
event: 'file',
action: 'block',
pattern: '\\.env$',
message: 'Environment files are protected.',
});
const blocked = invoke(projectRoot, 'PreToolUse', {
hook_event_name: 'PreToolUse',
tool_name: 'Write',
tool_input: { file_path: '/repo/.env', content: 'SAFE=value' },
});
assert.strictEqual(blocked.hookSpecificOutput.permissionDecision, 'deny');
const contentOnly = invoke(projectRoot, 'PreToolUse', {
hook_event_name: 'PreToolUse',
tool_name: 'Write',
tool_input: { file_path: '/repo/README.md', content: 'mentions .env' },
});
assert.deepStrictEqual(contentOnly, {});
});
})) passed++; else failed++;
if (test('PostToolUse block feedback truthfully says the completed tool is not undone', () => {
withProject(({ projectRoot, claudeDir }) => {
writeRule(claudeDir, { action: 'block', message: 'Repair the result.' });