config-audit/scanners/hook-validator.mjs
Kjell Tore Guttormsen 7a794b47eb fix(scanners)!: a finding ID names the check, not the emission (M-BUG-28)
BREAKING CHANGE: the {NNN} in CA-{SCANNER}-{NNN} identifies the check that
produced the finding. It used to be the finding's position in that scanner's
output for that run, which made it unstable across CONFIGURATIONS, not just
across releases as STATE framed it. Measured on two fixtures: "No custom
subagents" was CA-GAP-007 on minimal-project and CA-GAP-004 on healthy-project.
A user who fixed an unrelated earlier gap silently renumbered every later one,
so a .config-audit-ignore pin retargeted to a neighbouring finding with no
version change at all.

Second measured arm: README already documented the opposite scheme. It and the
scanner headers describe ~20 numbers as check codes (CA-SKL-003 = oversized
body, CA-PLH-015 = folder shadowing, CA-TOK-006 = schema deferral), and the
counter could only produce those in the all-fire case -- source-order positions
are 4, 3 and 8. The documentation described the scheme; the implementation was
what was wrong. Every published number is preserved by construction and pinned
exhaustively in tests/lib/finding-codes.test.mjs.

scanners/lib/finding-codes.mjs is the single authority. Every finding() call
passes a `code`; an undeclared or missing one THROWS. No counter fallback --
that would reproduce D1's findGapId -> 'unknown' silent degradation and let a
half-converted scanner ship IDs that look valid. findingCounter/resetCounter
are deleted outright, not left as no-ops. Retirement is now a mechanism:
RETIRED_CODES tombstones a withdrawn key so its number is never reissued,
seeded with GAP t3_8 -- the D1 removal that opened this chunk.

IDs are consequently NOT unique per finding: one check failing in three files
emits three findings sharing an ID. That inverts which consumer is correct, so
every f.id/findingId site was classified before the change. diff-engine and
most of fix-engine already keyed on scanner+title+file (drift was never lying);
fix-engine's verification did not, and keyed on the ID alone -- fixing one of
two sibling instances marked both fixed, and the untouched one, still present
in the re-scan, was reported as a REGRESSION. Red test first, then keyed on
(findingId, file), which both planFixes and applyFixes already carry.
plugin-health's crossIds Set was measured and is a clean negative: cross
findings are allFindings.slice(crossPluginStart) and codes 18/19 are emitted
only in that tail, so the partition holds by construction.

unknownSuppressions() reports a pin that names no declared check, in the
--output-file payload (ux-rules rule 2 -- a stderr-only warning is invisible to
the commands) and only when one exists, so a clean config is byte-identical.
That is what makes the break safe: a stale pin goes loud instead of dying quiet.

Frozen tests/snapshots/v5.0.0/ untouched on disk. IDs are masked out of that
comparison (mask-finding-ids.mjs) rather than re-derived -- re-deriving
positional IDs would assert the retired scheme against itself, and #58's
isGapEntry off-by-one is the measured example of that misfiring. The dead
re-derivation is removed from strip-retired-gap.mjs. default-output snapshots
re-approved after confirming the diff is IDs and nothing else.

Guards, each seen red against its own defect: a missing code (scanner errors
out mid-sweep), an orphan declaration, a resurrected retired key, and a
documented ID naming no check. The sweep asserts the union across all 16
scanners, never per scanner -- a per-scanner assertion goes green on a partial
conversion.

Fasit written before implementation: docs/mbug28-id-semantics-fasit.local.md,
including one correction made before running (CML has 12 checks over 13 call
sites -- the anchored and calibrated char-budget arms are one check, which a
repeated-title sweep found and my call-site count had missed).

Suite 1535 -> 1573, 0 failing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MyqCQKK2ornJ1jFWwqx17E
2026-08-09 23:26:36 +02:00

363 lines
14 KiB
JavaScript

/**
* HKV Scanner — Hook Validator
* Validates hooks.json format, script existence, event validity, timeouts.
* Finding IDs: CA-HKV-NNN
*/
import { readTextFile, discoverConfigFiles } from './lib/file-discovery.mjs';
import { finding, scannerResult } from './lib/output.mjs';
import { SEVERITY } from './lib/severity.mjs';
import { parseJson } from './lib/yaml-parser.mjs';
import { assessHookAdditionalContext } from './lib/hook-additional-context.mjs';
import { stat } from 'node:fs/promises';
import { resolve, dirname } from 'node:path';
const SCANNER = 'HKV';
/** All valid hook events — verified against code.claude.com/docs/en/hooks.md (2026-06-19) */
const VALID_EVENTS = new Set([
'SessionStart', 'Setup', 'InstructionsLoaded',
'UserPromptSubmit', 'UserPromptExpansion',
'PreToolUse', 'PermissionRequest', 'PermissionDenied',
'PostToolUse', 'PostToolUseFailure', 'PostToolBatch',
'SubagentStart', 'SubagentStop',
'TaskCreated', 'TaskCompleted',
'Stop', 'StopFailure',
'TeammateIdle', 'Notification',
'ConfigChange', 'CwdChanged', 'FileChanged',
'WorktreeCreate', 'WorktreeRemove',
'PreCompact', 'PostCompact',
'Elicitation', 'ElicitationResult',
'SessionEnd', 'MessageDisplay',
// 'post-session' deliberately EXCLUDED: the 2.1.169 changelog `post-session`
// is a self-hosted-runner workspace-lifecycle hook, NOT a settings.json hook
// event (absent from hooks.md; all settings.json events are PascalCase).
// Verified 2026-06-20.
]);
/** Valid hook handler types */
const VALID_TYPES = new Set(['command', 'http', 'prompt', 'agent']);
/** Reasonable timeout range */
const MIN_TIMEOUT = 1000;
const MAX_TIMEOUT = 300000; // 5 minutes
/** v5 M5: hook scripts that flood stdout fragment the cache prefix on every
* fire and slow Claude Code's UI. Static heuristic — count log lines. */
const VERBOSE_HOOK_LINE_THRESHOLD = 50;
const VERBOSE_HOOK_LINE_RX = /\b(?:console\.log|process\.stdout\.write)\s*\(/;
/**
* Scan all hooks.json files and hook configs in settings.json.
* @param {string} targetPath
* @param {{ files: import('./lib/file-discovery.mjs').ConfigFile[] }} discovery
* @returns {Promise<object>}
*/
export async function scan(targetPath, discovery) {
const start = Date.now();
const hooksFiles = discovery.files.filter(f => f.type === 'hooks-json');
const settingsFiles = discovery.files.filter(f => f.type === 'settings-json');
const findings = [];
let filesScanned = 0;
// Scan standalone hooks.json files
for (const file of hooksFiles) {
const content = await readTextFile(file.absPath);
if (!content) continue;
filesScanned++;
const parsed = parseJson(content);
if (parsed === null) {
findings.push(finding({
scanner: SCANNER,
code: 'invalid-json',
severity: SEVERITY.critical,
title: 'Invalid JSON in hooks.json',
description: `${file.relPath} contains invalid JSON. All hooks in this file will be ignored.`,
file: file.absPath,
recommendation: 'Fix JSON syntax errors.',
autoFixable: false,
}));
continue;
}
const hooksConfig = parsed.hooks || parsed;
await validateHooksObject(hooksConfig, file, findings, dirname(file.absPath));
}
// Scan hooks in settings.json files
for (const file of settingsFiles) {
const content = await readTextFile(file.absPath);
if (!content) continue;
const parsed = parseJson(content);
if (!parsed || !parsed.hooks) continue;
filesScanned++;
if (Array.isArray(parsed.hooks)) {
// Already reported by settings-validator, skip here
continue;
}
await validateHooksObject(parsed.hooks, file, findings, dirname(file.absPath));
}
if (hooksFiles.length === 0 && !settingsFiles.some(async f => {
const c = await readTextFile(f.absPath);
const p = c ? parseJson(c) : null;
return p && p.hooks;
})) {
// No hooks at all — this is noted but not an error
return scannerResult(SCANNER, 'ok', findings, filesScanned, Date.now() - start);
}
return scannerResult(SCANNER, 'ok', findings, filesScanned, Date.now() - start);
}
/**
* Validate a hooks object (event key → handler array).
*/
async function validateHooksObject(hooks, file, findings, baseDir) {
if (typeof hooks !== 'object' || Array.isArray(hooks)) {
findings.push(finding({
scanner: SCANNER,
code: 'hooks-not-object',
severity: SEVERITY.critical,
title: 'Hooks must be an object with event keys',
description: `${file.relPath}: hooks is ${Array.isArray(hooks) ? 'an array' : typeof hooks}. Expected object with event names as keys.`,
file: file.absPath,
recommendation: 'Use format: { "PreToolUse": [...], "Stop": [...] }',
autoFixable: false,
}));
return;
}
for (const [event, handlers] of Object.entries(hooks)) {
// Validate event name
if (!VALID_EVENTS.has(event)) {
findings.push(finding({
scanner: SCANNER,
code: 'unknown-event',
severity: SEVERITY.high,
title: 'Unknown hook event',
description: `${file.relPath}: "${event}" is not a valid hook event. This hook will never fire.`,
file: file.absPath,
evidence: event,
recommendation: `Valid events: ${[...VALID_EVENTS].slice(0, 8).join(', ')}... (${VALID_EVENTS.size} total)`,
autoFixable: false,
}));
continue;
}
if (!Array.isArray(handlers)) {
findings.push(finding({
scanner: SCANNER,
code: 'handlers-not-array',
severity: SEVERITY.high,
title: 'Hook handlers must be an array',
description: `${file.relPath}: handlers for "${event}" is not an array.`,
file: file.absPath,
evidence: `"${event}": ${typeof handlers}`,
recommendation: `Use format: "${event}": [{ "hooks": [...] }]`,
autoFixable: false,
}));
continue;
}
for (const handlerGroup of handlers) {
// Validate matcher format
if (handlerGroup.matcher !== undefined) {
if (typeof handlerGroup.matcher === 'object') {
findings.push(finding({
scanner: SCANNER,
code: 'matcher-not-string',
severity: SEVERITY.high,
title: 'Matcher must be a string, not an object',
description: `${file.relPath}: "${event}" has a matcher that is an object. Matcher should be a simple string like "Bash" or "Edit|Write".`,
file: file.absPath,
evidence: JSON.stringify(handlerGroup.matcher),
recommendation: 'Change matcher to a string: "matcher": "Bash"',
autoFixable: true,
}));
}
}
if (!handlerGroup.hooks || !Array.isArray(handlerGroup.hooks)) {
findings.push(finding({
scanner: SCANNER,
code: 'missing-hooks-array',
severity: SEVERITY.high,
title: 'Missing hooks array in handler group',
description: `${file.relPath}: "${event}" handler group is missing the "hooks" array.`,
file: file.absPath,
recommendation: 'Add "hooks": [{ "type": "command", "command": "..." }]',
autoFixable: false,
}));
continue;
}
for (const hook of handlerGroup.hooks) {
// Validate handler type
if (!hook.type || !VALID_TYPES.has(hook.type)) {
findings.push(finding({
scanner: SCANNER,
code: 'invalid-handler-type',
severity: SEVERITY.high,
title: 'Invalid hook handler type',
description: `${file.relPath}: "${event}" has handler with type "${hook.type || '(missing)'}".`,
file: file.absPath,
evidence: `type: "${hook.type || ''}"`,
recommendation: `Valid types: ${[...VALID_TYPES].join(', ')}`,
autoFixable: false,
}));
}
// For command hooks, check script existence
if (hook.type === 'command' && hook.command) {
const scriptPath = extractScriptPath(hook.command, baseDir);
if (scriptPath) {
let scriptExists = false;
try {
await stat(scriptPath);
scriptExists = true;
} catch {
findings.push(finding({
scanner: SCANNER,
code: 'script-not-found',
severity: SEVERITY.high,
title: 'Hook script not found',
description: `${file.relPath}: "${event}" references script that does not exist.`,
file: file.absPath,
evidence: hook.command,
recommendation: `Create the script at: ${scriptPath}`,
autoFixable: false,
}));
}
// v5 M5: count verbose stdout writes when the script exists.
if (scriptExists) {
const verboseCount = await countVerboseLines(scriptPath);
if (verboseCount > VERBOSE_HOOK_LINE_THRESHOLD) {
findings.push(finding({
scanner: SCANNER,
code: 'verbose-output',
severity: SEVERITY.low,
title: 'Verbose hook output (loud script)',
description:
`${file.relPath}: "${event}" runs ${scriptPath.split('/').slice(-2).join('/')} ` +
`which has ${verboseCount} console.log / process.stdout.write lines ` +
`(>${VERBOSE_HOOK_LINE_THRESHOLD}). Loud hooks slow the UI and bloat ` +
'session transcripts on every fire.',
file: scriptPath,
evidence:
`console_log_or_stdout_lines=${verboseCount}; ` +
`threshold=${VERBOSE_HOOK_LINE_THRESHOLD}`,
recommendation:
'Trim debug logging from hooks. Keep hook output to actionable signals; ' +
'route verbose diagnostics to a log file instead of stdout.',
autoFixable: false,
}));
}
// v5.10 B5: advisory (info) — a hook that injects unfiltered
// command output into hookSpecificOutput.additionalContext pays
// that whole payload into Claude's context on every fire (plain
// stdout does not). Low-precision static heuristic, so info only.
const scriptContent = await readTextFile(scriptPath);
const ac = assessHookAdditionalContext({ scriptContent });
if (ac.flagged) {
findings.push(finding({
scanner: SCANNER,
code: 'unfiltered-additional-context',
severity: SEVERITY.info,
title: 'Hook injects unfiltered output into context',
description:
`${file.relPath}: "${event}" runs ${scriptPath.split('/').slice(-2).join('/')} ` +
'which builds hookSpecificOutput.additionalContext from un-grepped command ' +
"output. That field enters Claude's context every time the hook fires (plain " +
'stdout does not), so an unfiltered payload is a recurring per-turn token cost. ' +
'Advisory only — low-precision static heuristic; verify the real payload size.',
file: scriptPath,
evidence:
'additional_context_unfiltered=true; ' +
`verbose_capture=${ac.hasVerboseCapture}; filter_applied=${ac.hasFilter}`,
recommendation:
'Filter before Claude reads: grep/head the command output down to what matters ' +
'before putting it in additionalContext (the documented filter-test-output.sh ' +
'pattern), or keep large diagnostics on plain stdout so they stay out of context.',
autoFixable: false,
}));
}
}
}
}
// Timeout validation
if (hook.timeout !== undefined) {
if (typeof hook.timeout !== 'number') {
findings.push(finding({
scanner: SCANNER,
code: 'timeout-not-number',
severity: SEVERITY.medium,
title: 'Hook timeout must be a number',
description: `${file.relPath}: "${event}" has non-numeric timeout.`,
file: file.absPath,
evidence: `timeout: ${JSON.stringify(hook.timeout)}`,
recommendation: 'Set timeout to a number (milliseconds).',
autoFixable: true,
}));
} else if (hook.timeout < MIN_TIMEOUT || hook.timeout > MAX_TIMEOUT) {
findings.push(finding({
scanner: SCANNER,
code: 'timeout-out-of-range',
severity: SEVERITY.low,
title: 'Hook timeout outside recommended range',
description: `${file.relPath}: "${event}" timeout is ${hook.timeout}ms. Recommended range: ${MIN_TIMEOUT}-${MAX_TIMEOUT}ms.`,
file: file.absPath,
evidence: `timeout: ${hook.timeout}`,
recommendation: `Set timeout between ${MIN_TIMEOUT} and ${MAX_TIMEOUT}ms.`,
autoFixable: false,
}));
}
}
}
}
}
}
/**
* Count lines containing console.log( or process.stdout.write( in a hook script.
* Static heuristic — does not execute the script.
*/
async function countVerboseLines(scriptPath) {
const content = await readTextFile(scriptPath);
if (!content) return 0;
let count = 0;
for (const line of content.split('\n')) {
if (VERBOSE_HOOK_LINE_RX.test(line)) count++;
}
return count;
}
/**
* Extract a filesystem path from a hook command string.
* Handles ${CLAUDE_PLUGIN_ROOT} variable substitution.
*/
function extractScriptPath(command, baseDir) {
// Extract the script path from common patterns:
// "bash /path/to/script.sh"
// "node ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/foo.mjs"
const match = command.match(/(?:bash|node|sh)\s+(.+?)(?:\s|$)/);
if (!match) return null;
let scriptPath = match[1].trim();
// Replace ${CLAUDE_PLUGIN_ROOT} with baseDir (best guess)
scriptPath = scriptPath.replace(/\$\{CLAUDE_PLUGIN_ROOT\}/g, resolve(baseDir, '..'));
scriptPath = scriptPath.replace(/\$CLAUDE_PLUGIN_ROOT/g, resolve(baseDir, '..'));
// Don't validate absolute paths that use env vars we can't resolve
if (scriptPath.includes('$')) return null;
return resolve(baseDir, scriptPath);
}