fix(hooks): keep hooks.json within Claude Code's schema

Move stable hook metadata to a validated sidecar while preserving hook commands and installer identity. Reject moved fingerprints and duplicate IDs, and validate before updating metadata. Independent local review passed at c315271624a1fd055b992f2bff889ad2a0ff8a6b; CI run 34678210149 passed. Rollback: revert this squash commit.
This commit is contained in:
zpearce-2814
2026-09-12 02:59:02 -04:00
committed by GitHub
parent c4904e3f63
commit 1ac07903ec
19 changed files with 1060 additions and 104 deletions
+160 -9
View File
@@ -8,8 +8,49 @@ const path = require('path');
const vm = require('vm');
const Ajv = require('ajv');
/**
* Resolve a module by its repo-relative path.
*
* Test harnesses copy this validator to the repo root before running it, so a
* plain relative require would break. Walk up from __dirname until the module
* is found instead.
*
* @param {string} repoRelativePath - e.g. 'scripts/lib/hooks-config.js'
* @returns {string} absolute path to the module
*/
function resolveRepoModule(repoRelativePath) {
let dir = __dirname;
for (;;) {
const candidate = path.join(dir, repoRelativePath);
if (fs.existsSync(candidate)) {
return candidate;
}
const parent = path.dirname(dir);
if (parent === dir) {
throw new Error(`Cannot locate ${repoRelativePath} above ${__dirname}`);
}
dir = parent;
}
}
const {
METADATA_FILENAME,
applyHooksMetadata,
findMetadataMismatches,
metadataPathFor,
withRefreshedFingerprints,
} = require(resolveRepoModule('scripts/lib/hooks-config.js'));
const HOOKS_FILE = path.join(__dirname, '../../hooks/hooks.json');
const HOOKS_SCHEMA_PATH = path.join(__dirname, '../../schemas/hooks.schema.json');
const METADATA_SCHEMA_PATH = path.join(__dirname, '../../schemas/hooks-metadata.schema.json');
// `--update-fingerprints` rewrites the sidecar's fingerprints from the current
// hooks.json instead of validating. Run it after changing a hook command.
const UPDATE_FINGERPRINTS = process.argv.includes('--update-fingerprints');
// Keys Claude Code's own hooks schema rejects. Keeping them out of hooks.json is
// what stops "unknown keys ... ignored" warnings when the plugin loads.
const HARNESS_UNKNOWN_ROOT_KEYS = ['$schema'];
const HARNESS_UNKNOWN_MATCHER_KEYS = ['id', 'description'];
const VALID_EVENTS = [
'SessionStart',
'UserPromptSubmit',
@@ -124,6 +165,78 @@ function validateHookEntry(hook, label) {
return hasErrors;
}
/**
* Reject keys the Claude Code harness does not understand.
*
* Claude Code validates a plugin's hooks.json against its own schema and prints
* every unrecognised key at load time. Once a hooks.metadata.json sidecar is
* present it owns the stable ids and descriptions, so hooks.json must not
* carry them as well.
*
* @param {object} data - Parsed hooks.json.
* @returns {boolean} true if errors were found
*/
function validateHarnessCompatibility(data) {
if (!data || typeof data !== 'object' || Array.isArray(data)) {
return false;
}
let hasErrors = false;
for (const key of HARNESS_UNKNOWN_ROOT_KEYS) {
if (key in data) {
console.error(
`ERROR: hooks.json must not define "${key}" - Claude Code reports it as an unknown key`
);
hasErrors = true;
}
}
const events = data.hooks && typeof data.hooks === 'object' && !Array.isArray(data.hooks)
? data.hooks
: {};
for (const [eventType, matchers] of Object.entries(events)) {
if (!Array.isArray(matchers)) continue;
matchers.forEach((matcher, index) => {
if (!matcher || typeof matcher !== 'object') return;
for (const key of HARNESS_UNKNOWN_MATCHER_KEYS) {
if (key in matcher) {
console.error(
`ERROR: hooks.json ${eventType}[${index}] must not define "${key}" - `
+ `move it to ${METADATA_FILENAME}`
);
hasErrors = true;
}
}
});
}
return hasErrors;
}
/**
* Validate a parsed document against a JSON schema file, if the schema exists.
*
* @param {object} document - Parsed JSON to validate.
* @param {string} schemaPath - Path to the schema; skipped when absent.
* @param {string} label - Name used in error output.
* @returns {boolean} true if errors were found
*/
function validateAgainstSchema(document, schemaPath, label) {
if (!fs.existsSync(schemaPath)) {
return false;
}
const schema = JSON.parse(fs.readFileSync(schemaPath, 'utf-8'));
const ajv = new Ajv({ allErrors: true });
const validate = ajv.compile(schema);
if (validate(document)) {
return false;
}
for (const err of validate.errors) {
console.error(`ERROR: ${label} schema: ${err.instancePath || '/'} ${err.message}`);
}
return true;
}
function validateHooks() {
if (!fs.existsSync(HOOKS_FILE)) {
console.log('No hooks.json found, skipping validation');
@@ -138,18 +251,51 @@ function validateHooks() {
process.exit(1);
}
// Validate against JSON schema
if (fs.existsSync(HOOKS_SCHEMA_PATH)) {
const schema = JSON.parse(fs.readFileSync(HOOKS_SCHEMA_PATH, 'utf-8'));
const ajv = new Ajv({ allErrors: true });
const validate = ajv.compile(schema);
const valid = validate(data);
if (!valid) {
for (const err of validate.errors) {
console.error(`ERROR: hooks.json schema: ${err.instancePath || '/'} ${err.message}`);
// Without a sidecar, hooks.json keeps its legacy inline ids. With one, the
// sidecar is the sole owner of id/description and hooks.json must stay
// within Claude Code's schema.
let metadata = null;
const metadataPath = metadataPathFor(HOOKS_FILE);
if (fs.existsSync(metadataPath)) {
try {
metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf-8'));
} catch (e) {
console.error(`ERROR: Invalid JSON in ${METADATA_FILENAME}: ${e.message}`);
process.exit(1);
}
if (validateHarnessCompatibility(data)) {
process.exit(1);
}
if (UPDATE_FINGERPRINTS) {
try {
metadata = withRefreshedFingerprints(data, metadata);
} catch (error) {
console.error(`ERROR: ${error.message}`);
process.exit(1);
}
}
if (validateAgainstSchema(metadata, METADATA_SCHEMA_PATH, METADATA_FILENAME)) {
process.exit(1);
}
const mismatches = findMetadataMismatches(data, metadata);
if (mismatches.length > 0) {
for (const mismatch of mismatches) {
console.error(`ERROR: ${mismatch}`);
}
process.exit(1);
}
// Validate the merged view so the id/description rules below still apply.
data = applyHooksMetadata(data, metadata);
}
// Validate against JSON schema
if (validateAgainstSchema(data, HOOKS_SCHEMA_PATH, 'hooks.json')) {
process.exit(1);
}
// Support both object format { hooks: {...} } and array format
@@ -254,6 +400,11 @@ function validateHooks() {
process.exit(1);
}
if (UPDATE_FINGERPRINTS && metadata) {
fs.writeFileSync(metadataPath, `${JSON.stringify(metadata, null, 2)}\n`);
console.log(`Updated fingerprints in ${METADATA_FILENAME}`);
}
console.log(`Validated ${totalMatchers} hook matchers`);
}
+4 -1
View File
@@ -19,6 +19,7 @@ const {
isAllowedOrigin,
} = require('./lib/loopback-guard');
const { normalizeAgentTools } = require('./lib/agent-tools');
const { readHooksConfig } = require('./lib/hooks-config');
const DEFAULT_HOST = '127.0.0.1';
@@ -129,7 +130,9 @@ function loadHooks(_root) {
const hooksPath = path.join(root, 'hooks', 'hooks.json');
if (!fs.existsSync(hooksPath)) return [];
try {
const data = JSON.parse(fs.readFileSync(hooksPath, 'utf8'));
// Ids and descriptions live in hooks/hooks.metadata.json so that hooks.json
// stays within the key set Claude Code's hooks schema accepts.
const data = readHooksConfig(hooksPath);
const hooks = [];
for (const [eventName, entries] of Object.entries(data.hooks || {})) {
for (const entry of entries || []) {
+318
View File
@@ -0,0 +1,318 @@
'use strict';
/**
* Read hooks/hooks.json together with its sibling hooks/hooks.metadata.json.
*
* Claude Code validates a plugin's hooks.json against its own schema and warns
* about every key it does not recognise, so ECC's stable matcher ids and
* human-readable descriptions cannot live in that file. They are kept in a
* sidecar keyed by event name and aligned with hooks.json entry order, and
* merged back here so the rest of ECC keeps seeing one object with `id` and
* `description` on each matcher entry.
*
* Index alignment alone cannot tell a reordered hooks.json from a correct one,
* so every sidecar entry also carries a fingerprint of the matcher entry it
* describes. A mismatch means the two files drifted apart.
*/
const crypto = require('crypto');
const fs = require('fs');
const path = require('path');
const HOOKS_FILENAME = 'hooks.json';
const METADATA_FILENAME = 'hooks.metadata.json';
const FINGERPRINT_LENGTH = 12;
const FINGERPRINT_PATTERN = /^[0-9a-f]{12}$/;
function readJsonObject(filePath, label) {
let raw;
try {
raw = fs.readFileSync(filePath, 'utf8');
} catch (error) {
throw new Error(`Unable to read ${label} at ${filePath}: ${error.message}`);
}
let parsed;
try {
parsed = JSON.parse(raw);
} catch (error) {
throw new Error(`Invalid JSON in ${label} at ${filePath}: ${error.message}`);
}
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
throw new Error(`Invalid ${label} at ${filePath}: expected a JSON object`);
}
return parsed;
}
function metadataPathFor(hooksPath) {
return path.join(path.dirname(hooksPath), METADATA_FILENAME);
}
/**
* JSON.stringify with object keys sorted, so a fingerprint does not change when
* someone reorders the keys inside a hook object.
*/
function stableStringify(value) {
if (Array.isArray(value)) {
return `[${value.map(stableStringify).join(',')}]`;
}
if (value && typeof value === 'object') {
return `{${Object.keys(value).sort().map(
key => `${JSON.stringify(key)}:${stableStringify(value[key])}`
).join(',')}}`;
}
return JSON.stringify(value);
}
/**
* Fingerprint the parts of a hooks.json matcher entry that identify it: its
* matcher and its hook commands. Ids and descriptions are excluded so the
* fingerprint is the same whether or not metadata has been merged in.
*
* @param {object} entry - A matcher entry from hooks.json.
* @returns {string} short hex digest
*/
function fingerprintHookEntry(entry) {
const subject = {
matcher: entry && 'matcher' in entry ? entry.matcher : null,
hooks: entry && Array.isArray(entry.hooks) ? entry.hooks : [],
};
return crypto.createHash('sha256')
.update(stableStringify(subject))
.digest('hex')
.slice(0, FINGERPRINT_LENGTH);
}
function eventsOf(hooksConfig) {
return hooksConfig && typeof hooksConfig.hooks === 'object' && hooksConfig.hooks
&& !Array.isArray(hooksConfig.hooks)
? hooksConfig.hooks
: null;
}
function metadataEntriesOf(metadata) {
return metadata && typeof metadata.entries === 'object' && metadata.entries
&& !Array.isArray(metadata.entries)
? metadata.entries
: null;
}
/**
* Merge sidecar metadata into a parsed hooks.json object.
*
* Neither argument is mutated; the returned config shares untouched matcher
* entries with the input and copies the ones that receive metadata.
*
* @param {object} hooksConfig - Parsed hooks.json.
* @param {object|null} metadata - Parsed hooks.metadata.json, or null when absent.
* @returns {object} a new hooks configuration with id/description restored.
*/
function applyHooksMetadata(hooksConfig, metadata) {
const events = eventsOf(hooksConfig);
const entriesByEvent = metadataEntriesOf(metadata);
if (!events || !entriesByEvent) {
return hooksConfig;
}
const mergedEvents = {};
for (const [event, entries] of Object.entries(events)) {
const eventMetadata = entriesByEvent[event];
if (!Array.isArray(entries) || !Array.isArray(eventMetadata)) {
mergedEvents[event] = entries;
continue;
}
mergedEvents[event] = entries.map((entry, index) => {
const entryMetadata = eventMetadata[index];
if (!entry || typeof entry !== 'object') return entry;
if (!entryMetadata || typeof entryMetadata !== 'object') return entry;
const merged = { ...entry };
if (typeof entryMetadata.id === 'string' && !('id' in entry)) {
merged.id = entryMetadata.id;
}
if (typeof entryMetadata.description === 'string' && !('description' in entry)) {
merged.description = entryMetadata.description;
}
return merged;
});
}
return { ...hooksConfig, hooks: mergedEvents };
}
/**
* Report entries whose metadata is missing, misaligned, or bound to a
* different matcher entry than the one at the same index.
*
* @param {object} hooksConfig - Parsed hooks.json.
* @param {object|null} metadata - Parsed hooks.metadata.json.
* @returns {string[]} human-readable problems; empty when the sidecar lines up.
*/
function findMetadataMismatches(hooksConfig, metadata) {
const problems = [];
const idLocations = new Map();
const events = eventsOf(hooksConfig) || {};
const entriesByEvent = metadataEntriesOf(metadata) || {};
for (const [event, entries] of Object.entries(events)) {
if (!Array.isArray(entries)) continue;
const eventMetadata = entriesByEvent[event];
if (!Array.isArray(eventMetadata)) {
problems.push(`${METADATA_FILENAME} is missing entries for event "${event}"`);
continue;
}
if (eventMetadata.length !== entries.length) {
problems.push(
`${METADATA_FILENAME} lists ${eventMetadata.length} entr(ies) for event "${event}" `
+ `but ${HOOKS_FILENAME} has ${entries.length}`
);
continue;
}
eventMetadata.forEach((entry, index) => {
const label = `${METADATA_FILENAME} ${event}[${index}]`;
if (!entry || typeof entry !== 'object') {
problems.push(`${label} is not an object`);
return;
}
if (typeof entry.id !== 'string' || entry.id.trim() === '') {
problems.push(`${label} is missing a non-empty "id"`);
} else if (idLocations.has(entry.id)) {
problems.push(`${label} has duplicate id "${entry.id}" already used by ${idLocations.get(entry.id)}`);
} else {
idLocations.set(entry.id, label);
}
if ('description' in entry && typeof entry.description !== 'string') {
problems.push(`${label} has a non-string "description"`);
}
if (typeof entry.fingerprint !== 'string' || !FINGERPRINT_PATTERN.test(entry.fingerprint)) {
problems.push(`${label} is missing a valid "fingerprint"`);
return;
}
const expected = fingerprintHookEntry(entries[index]);
if (entry.fingerprint !== expected) {
problems.push(
`${label} (id "${entry.id}") fingerprint ${entry.fingerprint} does not match `
+ `${HOOKS_FILENAME} ${event}[${index}] (${expected}); the entries were reordered `
+ 'or the hook command changed - regenerate with '
+ 'node scripts/ci/validate-hooks.js --update-fingerprints'
);
}
});
}
for (const event of Object.keys(entriesByEvent)) {
if (!Array.isArray(events[event])) {
problems.push(`${METADATA_FILENAME} describes event "${event}" which ${HOOKS_FILENAME} does not define`);
}
}
return problems;
}
/**
* Return a copy of the sidecar with every fingerprint recomputed from the
* matcher entry at the same index. Used to refresh the sidecar after hook
* commands change.
*
* @param {object} hooksConfig - Parsed hooks.json.
* @param {object} metadata - Parsed hooks.metadata.json.
* @returns {object} a new metadata object
*/
function withRefreshedFingerprints(hooksConfig, metadata) {
const events = eventsOf(hooksConfig) || {};
const entriesByEvent = metadataEntriesOf(metadata) || {};
const refreshed = {};
// A known fingerprint at another position signals a reorder, not a command
// edit. Require the author to move its metadata before refreshing anything.
const positions = new Map();
for (const [event, entries] of Object.entries(events)) {
if (!Array.isArray(entries)) continue;
entries.forEach((entry, index) => {
const fingerprint = fingerprintHookEntry(entry);
const locations = positions.get(fingerprint) || [];
positions.set(fingerprint, [...locations, `${event}[${index}]`]);
});
}
for (const [event, entries] of Object.entries(entriesByEvent)) {
if (!Array.isArray(entries)) continue;
entries.forEach((entry, index) => {
const locations = positions.get(entry?.fingerprint);
const location = `${event}[${index}]`;
if (locations && !locations.includes(location)) {
throw new Error(`Metadata reorder detected at ${location}; move the matching sidecar entry before refreshing fingerprints`);
}
});
}
for (const [event, eventMetadata] of Object.entries(entriesByEvent)) {
const entries = Array.isArray(events[event]) ? events[event] : [];
refreshed[event] = Array.isArray(eventMetadata)
? eventMetadata.map((entry, index) => (
entry && typeof entry === 'object' && index < entries.length
? { ...entry, fingerprint: fingerprintHookEntry(entries[index]) }
: entry
))
: eventMetadata;
}
return { ...metadata, entries: refreshed };
}
function assertMetadataAligned(hooksConfig, metadata, hooksPath) {
const mismatches = findMetadataMismatches(hooksConfig, metadata);
if (mismatches.length > 0) {
throw new Error(
`${METADATA_FILENAME} does not line up with ${hooksPath}:\n ${mismatches.join('\n ')}`
);
}
}
/**
* Merge a sidecar into a hooks config, rejecting a sidecar that does not line
* up. Shared by the readers below so a truncated or reordered sidecar fails
* loudly instead of producing entries with the wrong or missing ids.
*
* @param {object} hooksConfig - Parsed hooks.json.
* @param {object} metadata - Parsed hooks.metadata.json.
* @param {string} hooksPath - Used in the error message.
* @returns {object} a new merged hooks configuration
*/
function mergeHooksMetadata(hooksConfig, metadata, hooksPath = HOOKS_FILENAME) {
assertMetadataAligned(hooksConfig, metadata, hooksPath);
return applyHooksMetadata(hooksConfig, metadata);
}
/**
* Read hooks.json and return it with sidecar metadata merged in.
*
* @param {string} hooksPath - Path to hooks/hooks.json.
* @param {string} [label] - Label used in error messages.
* @returns {object} the merged hooks configuration.
*/
function readHooksConfig(hooksPath, label = HOOKS_FILENAME) {
const hooksConfig = readJsonObject(hooksPath, label);
const metadataPath = metadataPathFor(hooksPath);
if (!fs.existsSync(metadataPath)) {
return hooksConfig;
}
return mergeHooksMetadata(hooksConfig, readJsonObject(metadataPath, METADATA_FILENAME), hooksPath);
}
module.exports = {
HOOKS_FILENAME,
METADATA_FILENAME,
applyHooksMetadata,
findMetadataMismatches,
fingerprintHookEntry,
mergeHooksMetadata,
metadataPathFor,
readHooksConfig,
readJsonObject,
withRefreshedFingerprints,
};
+21 -1
View File
@@ -36,6 +36,7 @@ const { adaptAntigravityAgent } = require('./install/antigravity-agent');
const { buildInstallIndex, rewriteRelativeLinks } = require('./install/link-rewrite');
const { getInstallTargetAdapter, listInstallTargetAdapters } = require('./install-targets/registry');
const { resolveInvocationEnvironment } = require('./invocation-environment');
const { mergeHooksMetadata, metadataPathFor } = require('./hooks-config');
const OPENCODE_BUILD_ARTIFACT = path.join('.opencode', 'dist');
const OPENCODE_BUILD_SCRIPT = path.join('scripts', 'build-opencode.js');
const OPENCODE_PLUGIN_NOT_BUILT_CODE = 'opencode-plugin-not-built';
@@ -535,6 +536,25 @@ function readJsonNoFollow(filePath) {
return JSON.parse(readFileNoFollow(filePath, 'utf8'));
}
/**
* Read hooks.json and merge in hooks/hooks.metadata.json without following
* symlinks. The sidecar holds the stable matcher ids that hooks.json cannot
* carry, because Claude Code reports unknown keys when the plugin loads. A
* sidecar that does not line up with hooks.json is rejected before repair can
* reconcile matchers under the wrong ids.
*
* @param {string} hooksPath - Path to the source hooks.json.
* @returns {object} the hooks configuration with ids and descriptions restored.
*/
function readHooksConfigNoFollow(hooksPath) {
const hooksConfig = readJsonNoFollow(hooksPath);
const metadataPath = metadataPathFor(hooksPath);
if (!fs.existsSync(metadataPath)) {
return hooksConfig;
}
return mergeHooksMetadata(hooksConfig, readJsonNoFollow(metadataPath), hooksPath);
}
function assertClaudeSettingsDestination(operation, trustedRoot, target = null) {
if (target && target !== 'claude' && target !== 'claude-project') {
throw new Error('Refusing to manage Claude hooks for a non-Claude target.');
@@ -720,7 +740,7 @@ function hydrateRecordedOperations(repoRoot, operations, trustedRoot) {
sourcePath,
previousManagedHooks: operation.managedHooks,
managedHooks: materializeManagedHooks(
readJsonNoFollow(sourcePath),
readHooksConfigNoFollow(sourcePath),
trustedRoot
),
};
+4 -1
View File
@@ -5,6 +5,7 @@ const {
CLAUDE_HOOKS_CONFIG_PATH,
getClaudeSettingsPath,
} = require('../install/claude-settings');
const { METADATA_FILENAME } = require('../hooks-config');
const PLATFORM_SOURCE_PATH_OWNERS = Object.freeze({
'.claude-plugin': 'claude',
@@ -176,7 +177,9 @@ function planClaudeHooksOperations(adapter, module, input) {
return [
...operations,
...fs.readdirSync(sourceHooksRoot, { withFileTypes: true })
.filter(entry => entry.name !== 'hooks.json')
// hooks.json is merged into settings.json above, and its metadata sidecar
// is consumed with it, so neither is scaffolded into the target hooks dir.
.filter(entry => entry.name !== 'hooks.json' && entry.name !== METADATA_FILENAME)
.sort((left, right) => left.name.localeCompare(right.name))
.map(entry => adapter.createScaffoldOperation(
module.id,
+4 -1
View File
@@ -7,6 +7,7 @@ const { execFileSync } = require('child_process');
const { resolveInstallPlan } = require('../install-manifests');
const { getInstallTargetAdapter } = require('../install-targets/registry');
const { resolveInvocationEnvironment } = require('../invocation-environment');
const { readHooksConfig } = require('../hooks-config');
const {
materializeManagedHooks,
} = require('./claude-settings');
@@ -136,7 +137,9 @@ function materializeClaudeSettingsOperation(sourceRoot, operation) {
return [];
}
const hooksConfig = readJsonObject(sourcePath, operation.sourceRelativePath);
// Stable ids and descriptions live in hooks/hooks.metadata.json; readHooksConfig
// merges them back so managed settings entries keep their ids.
const hooksConfig = readHooksConfig(sourcePath, operation.sourceRelativePath);
const managedHooks = materializeManagedHooks(
hooksConfig,
path.dirname(operation.destinationPath)