diff --git a/commands/hookify-help.md b/commands/hookify-help.md index 11be8ba24..1fb37234f 100644 --- a/commands/hookify-help.md +++ b/commands/hookify-help.md @@ -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 diff --git a/scripts/hooks/hookify-engine.js b/scripts/hooks/hookify-engine.js index 0fadbdd35..af8d0efb2 100644 --- a/scripts/hooks/hookify-engine.js +++ b/scripts/hooks/hookify-engine.js @@ -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], }; } diff --git a/scripts/hooks/hookify-loader.js b/scripts/hooks/hookify-loader.js index 5e4e1fe7c..64261f58a 100644 --- a/scripts/hooks/hookify-loader.js +++ b/scripts/hooks/hookify-loader.js @@ -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'; } diff --git a/scripts/hooks/hookify-runner.js b/scripts/hooks/hookify-runner.js index 7d5b7c6d3..feea80c32 100644 --- a/scripts/hooks/hookify-runner.js +++ b/scripts/hooks/hookify-runner.js @@ -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, diff --git a/skills/hookify-rules/SKILL.md b/skills/hookify-rules/SKILL.md index 2cb1b8f1b..cfa0aed64 100644 --- a/skills/hookify-rules/SKILL.md +++ b/skills/hookify-rules/SKILL.md @@ -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 diff --git a/tests/docs/hookify-runtime-docs.test.js b/tests/docs/hookify-runtime-docs.test.js index 306bc56b8..db2fc74ed 100644 --- a/tests/docs/hookify-runtime-docs.test.js +++ b/tests/docs/hookify-runtime-docs.test.js @@ -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'); diff --git a/tests/hooks/hookify-engine.test.js b/tests/hooks/hookify-engine.test.js index 8eaab5609..62717b0ae 100644 --- a/tests/hooks/hookify-engine.test.js +++ b/tests/hooks/hookify-engine.test.js @@ -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); diff --git a/tests/hooks/hookify-loader.test.js b/tests/hooks/hookify-loader.test.js index ad8d88545..43523ebb5 100644 --- a/tests/hooks/hookify-loader.test.js +++ b/tests/hooks/hookify-loader.test.js @@ -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( diff --git a/tests/hooks/hookify-runner.test.js b/tests/hooks/hookify-runner.test.js index 4045b091a..69711476d 100644 --- a/tests/hooks/hookify-runner.test.js +++ b/tests/hooks/hookify-runner.test.js @@ -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.' });