From 569e5a36bb18c3a36924618505f0c3ef66df7c0d Mon Sep 17 00:00:00 2001 From: wellkilo Date: Mon, 7 Sep 2026 00:53:13 +0800 Subject: [PATCH 1/4] feat(install): register manual Claude hooks --- README.md | 5 +- hooks/README.md | 6 +- schemas/hooks.schema.json | 5 + schemas/install-state.schema.json | 85 +- scripts/ci/validate-hooks.js | 29 +- scripts/lib/install-lifecycle.js | 236 +++++- scripts/lib/install-state.js | 33 + scripts/lib/install-targets/claude-home.js | 49 +- scripts/lib/install-targets/claude-project.js | 49 +- scripts/lib/install/apply.js | 416 ++++++---- scripts/lib/install/claude-settings.js | 750 ++++++++++++++++++ scripts/lib/install/hook-consent.js | 5 +- scripts/lib/install/plan.js | 27 + tests/ci/validators.test.js | 137 +++- tests/lib/claude-settings.test.js | 557 +++++++++++++ tests/lib/hook-consent.test.js | 24 +- tests/lib/install-executor.test.js | 168 ++++ tests/lib/install-lifecycle.test.js | 477 ++++++++++- tests/lib/install-state.test.js | 109 +++ tests/scripts/install-apply.test.js | 323 +++++--- .../scripts/manual-hook-install-docs.test.js | 8 + 21 files changed, 3160 insertions(+), 338 deletions(-) create mode 100644 scripts/lib/install/claude-settings.js create mode 100644 tests/lib/claude-settings.test.js diff --git a/README.md b/README.md index 00d3a8ca7..2431bd418 100644 --- a/README.md +++ b/README.md @@ -555,7 +555,10 @@ Do not copy the raw repo `hooks/hooks.json` into `~/.claude/settings.json` or `~ bash ./install.sh --target claude --modules hooks-runtime --enable-hooks ``` -That writes resolved hooks to `~/.claude/hooks/hooks.json` and leaves any existing `~/.claude/settings.json` untouched. +That installs the hook scripts under `~/.claude/` and registers the resolved +hook entries in `~/.claude/settings.json`. Existing user settings and hooks are +preserved; ECC-owned entries are tracked by stable ID for idempotent updates +and safe uninstall. If you installed ECC via `/plugin install`, do not copy those hooks into `settings.json`. Claude Code v2.1+ already auto-loads plugin `hooks/hooks.json`, and duplicating them in `settings.json` causes duplicate execution and cross-platform hook conflicts. diff --git a/hooks/README.md b/hooks/README.md index 510dfa755..e540b2d24 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -33,7 +33,11 @@ bash ./install.sh --target claude --modules hooks-runtime --enable-hooks pwsh -File .\install.ps1 --target claude --modules hooks-runtime --enable-hooks ``` -That installs resolved hooks to `~/.claude/hooks/hooks.json`. On Windows, the Claude config root is `%USERPROFILE%\\.claude`. +That installs the hook scripts under `~/.claude/` and registers the resolved +hook entries in `~/.claude/settings.json`. Existing user settings and hook +entries are preserved, while ECC-owned entries are tracked by stable ID for +idempotent updates and safe uninstall. On Windows, the Claude config root is +`%USERPROFILE%\\.claude`. ### PreToolUse Hooks diff --git a/schemas/hooks.schema.json b/schemas/hooks.schema.json index 4d1192973..c325d9712 100644 --- a/schemas/hooks.schema.json +++ b/schemas/hooks.schema.json @@ -122,6 +122,11 @@ "hooks" ], "properties": { + "id": { + "type": "string", + "pattern": "\\S", + "description": "Stable identifier for a matcher entry. Required and globally unique in wrapped object format." + }, "matcher": { "oneOf": [ { diff --git a/schemas/install-state.schema.json b/schemas/install-state.schema.json index 976d5129b..9b827e144 100644 --- a/schemas/install-state.schema.json +++ b/schemas/install-state.schema.json @@ -213,9 +213,92 @@ "contentSha256": { "type": "string", "pattern": "^[a-fA-F0-9]{64}$" + }, + "managedHooks": { + "type": "object", + "minProperties": 1, + "propertyNames": { + "enum": [ + "SessionStart", + "UserPromptSubmit", + "PreToolUse", + "PermissionRequest", + "PostToolUse", + "PostToolUseFailure", + "Notification", + "SubagentStart", + "Stop", + "SubagentStop", + "PreCompact", + "InstructionsLoaded", + "TeammateIdle", + "TaskCompleted", + "ConfigChange", + "WorktreeCreate", + "WorktreeRemove", + "SessionEnd" + ] + }, + "additionalProperties": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "required": ["id", "hooks"], + "properties": { + "id": { + "type": "string", + "pattern": "\\S" + } + } + } + } + } + }, + "allOf": [ + { + "if": { + "properties": { + "kind": { "const": "update-claude-settings" } + } + }, + "then": { + "required": ["managedHooks"], + "properties": { + "moduleId": { "const": "hooks-runtime" }, + "sourceRelativePath": { "const": "hooks/hooks.json" } + } + } + } + ] + } + } + }, + "allOf": [ + { + "if": { + "properties": { + "operations": { + "contains": { + "type": "object", + "properties": { + "kind": { "const": "update-claude-settings" } + }, + "required": ["kind"] + } + } + } + }, + "then": { + "properties": { + "target": { + "properties": { + "target": { "enum": ["claude", "claude-project"] } + }, + "required": ["target"] } } } } - } + ] } diff --git a/scripts/ci/validate-hooks.js b/scripts/ci/validate-hooks.js index bc1da8020..779555a44 100644 --- a/scripts/ci/validate-hooks.js +++ b/scripts/ci/validate-hooks.js @@ -154,8 +154,17 @@ function validateHooks() { // Support both object format { hooks: {...} } and array format const hooks = data.hooks || data; + const requiresStableIds = Boolean( + data + && typeof data === 'object' + && !Array.isArray(data) + && data.hooks + && typeof data.hooks === 'object' + && !Array.isArray(data.hooks) + ); let hasErrors = false; let totalMatchers = 0; + const matcherIdLocations = new Map(); if (typeof hooks === 'object' && !Array.isArray(hooks)) { // Object format: { EventType: [matchers] } @@ -179,20 +188,32 @@ function validateHooks() { hasErrors = true; continue; } + const matcherLabel = `${eventType}[${i}]`; + if (requiresStableIds && !isNonEmptyString(matcher.id)) { + console.error(`ERROR: ${matcherLabel} missing or invalid 'id' field`); + hasErrors = true; + } else if (requiresStableIds && matcherIdLocations.has(matcher.id)) { + console.error( + `ERROR: ${matcherLabel} has duplicate id '${matcher.id}' (already used by ${matcherIdLocations.get(matcher.id)})` + ); + hasErrors = true; + } else if (requiresStableIds) { + matcherIdLocations.set(matcher.id, matcherLabel); + } if (!('matcher' in matcher) && !EVENTS_WITHOUT_MATCHER.has(eventType)) { - console.error(`ERROR: ${eventType}[${i}] missing 'matcher' field`); + console.error(`ERROR: ${matcherLabel} missing 'matcher' field`); hasErrors = true; } else if ('matcher' in matcher && typeof matcher.matcher !== 'string' && (typeof matcher.matcher !== 'object' || matcher.matcher === null)) { - console.error(`ERROR: ${eventType}[${i}] has invalid 'matcher' field`); + console.error(`ERROR: ${matcherLabel} has invalid 'matcher' field`); hasErrors = true; } if (!matcher.hooks || !Array.isArray(matcher.hooks)) { - console.error(`ERROR: ${eventType}[${i}] missing 'hooks' array`); + console.error(`ERROR: ${matcherLabel} missing 'hooks' array`); hasErrors = true; } else { // Validate each hook entry for (let j = 0; j < matcher.hooks.length; j++) { - if (validateHookEntry(matcher.hooks[j], `${eventType}[${i}].hooks[${j}]`)) { + if (validateHookEntry(matcher.hooks[j], `${matcherLabel}.hooks[${j}]`)) { hasErrors = true; } } diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index c10b1cfe3..c5ece3504 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -20,6 +20,15 @@ const { getLegacyOpencodeLocation, inspectLegacyOpencodeState, } = require('./install/opencode-legacy-migration'); +const { + acquireSettingsLock, + inspectManagedHooks, + materializeManagedHooks, + repairManagedHooks, + uninstallManagedHooks, + updateSettingsAtomic, + validateManagedHooks, +} = require('./install/claude-settings'); const { adaptAntigravityAgent } = require('./install/antigravity-agent'); const { buildInstallIndex, rewriteRelativeLinks } = require('./install/link-rewrite'); const { getInstallTargetAdapter, listInstallTargetAdapters } = require('./install-targets/registry'); @@ -523,6 +532,24 @@ function readJsonNoFollow(filePath) { return JSON.parse(readFileNoFollow(filePath, 'utf8')); } +function expectedClaudeSettingsPath(targetRoot) { + return path.join(targetRoot, 'settings.json'); +} + +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.'); + } + if (path.resolve(operation.destinationPath) !== path.resolve( + expectedClaudeSettingsPath(trustedRoot) + )) { + throw new Error( + `Refusing to manage Claude hooks outside the canonical settings file: ` + + `${operation.destinationPath}` + ); + } +} + function writeContainedFile(destinationPath, content, trustedRoot, action, mode) { const preparedDestination = prepareContainedWriteDestination(destinationPath, trustedRoot, action); const finalDestination = getManagedDestination( @@ -689,6 +716,24 @@ function deepRemoveJsonSubset(currentValue, managedValue) { function hydrateRecordedOperations(repoRoot, operations) { return operations.map(operation => { + if (operation.kind === 'update-claude-settings') { + const sourcePath = resolveOperationSourcePath(repoRoot, operation); + if (!sourcePath || !fs.existsSync(sourcePath)) { + throw new Error( + `Missing source file for repair: ${sourcePath || operation.sourceRelativePath}` + ); + } + return { + ...operation, + sourcePath, + previousManagedHooks: operation.managedHooks, + managedHooks: materializeManagedHooks( + readJsonNoFollow(sourcePath), + path.dirname(operation.destinationPath) + ), + }; + } + if (operation.kind !== 'copy-file') { return { ...operation }; } @@ -717,7 +762,14 @@ function shouldRepairFromRecordedOperations(state) { return getManagedOperations(state).some(operation => operation.kind !== 'copy-file'); } -function executeRepairOperation(repoRoot, operation, trustedRoot, linkIndex = null) { +function executeRepairOperation( + repoRoot, + operation, + trustedRoot, + linkIndex = null, + target = null, + settingsLockHeld = false +) { // Install-state is attacker-controllable; never write/delete outside the // adapter-derived trusted root, regardless of what the state file claims // (GHSA-hfpv-w6mp-5g95). @@ -770,6 +822,35 @@ function executeRepairOperation(repoRoot, operation, trustedRoot, linkIndex = nu return operation.destinationPath; } + if (operation.kind === 'update-claude-settings') { + assertClaudeSettingsDestination(operation, trustedRoot, target); + const managedHooks = validateManagedHooks(operation.managedHooks); + const previousManagedHooks = operation.previousManagedHooks + ? validateManagedHooks(operation.previousManagedHooks, 'previous managed hooks') + : null; + const existingDestination = getContainedExistingPath( + operation.destinationPath, + trustedRoot, + 'repair' + ); + const settingsPath = existingDestination + ? getManagedDestination(existingDestination, trustedRoot, 'repair').managedPath + : prepareContainedWriteDestination(operation.destinationPath, trustedRoot, 'repair'); + updateSettingsAtomic( + settingsPath, + currentSettings => repairManagedHooks(currentSettings, managedHooks, { + previousManagedHooks, + }), + { + lockHeld: settingsLockHeld, + beforeCommit() { + getManagedDestination(settingsPath, trustedRoot, 'repair'); + }, + } + ); + return operation.destinationPath; + } + if (operation.kind === 'remove') { const removedPath = removeContainedPath( operation.destinationPath, @@ -938,6 +1019,45 @@ function executeUninstallOperation(operation, trustedRoot, options = {}) { }; } + if (operation.kind === 'update-claude-settings') { + assertClaudeSettingsDestination(operation, trustedRoot, options.target); + const existingDestination = getContainedExistingPath( + operation.destinationPath, + trustedRoot, + 'uninstall' + ); + if (!existingDestination) { + return { + removedPaths: [], + cleanupTargets: [] + }; + } + + const settingsPath = getManagedDestination( + existingDestination, + trustedRoot, + 'uninstall' + ).managedPath; + const uninstalled = updateSettingsAtomic( + settingsPath, + currentSettings => uninstallManagedHooks(currentSettings, operation.managedHooks), + { + lockHeld: Boolean(options.settingsLockHeld), + beforeCommit() { + getManagedDestination(settingsPath, trustedRoot, 'uninstall'); + }, + } + ); + + return { + removedPaths: [], + cleanupTargets: [], + retainedPaths: uninstalled.retained.length > 0 + ? [operation.destinationPath] + : [] + }; + } + if (operation.kind === 'remove') { const previousContent = getOperationPreviousContent(operation); if (previousContent !== null) { @@ -966,7 +1086,7 @@ function executeUninstallOperation(operation, trustedRoot, options = {}) { throw new Error(`Unsupported uninstall operation kind: ${operation.kind}`); } -function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = null) { +function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = null, target = null) { const destinationPath = operation.destinationPath; if (!destinationPath) { return { @@ -1147,6 +1267,48 @@ function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = n }; } + if (operation.kind === 'update-claude-settings') { + try { + assertClaudeSettingsDestination(operation, trustedRoot, target); + } catch (_error) { + return { + status: 'unsafe-destination', + operation, + destinationPath, + reason: 'non-canonical-claude-settings' + }; + } + let managedHooks; + try { + managedHooks = validateManagedHooks(operation.managedHooks); + } catch (_error) { + return { + status: 'unverified', + operation, + destinationPath + }; + } + + try { + const inspection = inspectManagedHooks( + readJsonNoFollow(inspectedPath), + managedHooks + ); + return { + status: inspection.status, + operation, + destinationPath, + managedHookInspection: inspection + }; + } catch (_error) { + return { + status: 'drifted', + operation, + destinationPath + }; + } + } + return { status: 'unverified', operation, @@ -1154,11 +1316,17 @@ function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = n }; } -function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations) { +function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations, target = null) { const linkIndex = buildLinkIndexForOperations(operations, trustedRoot); return operations.reduce( (summary, operation) => { - const inspection = inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex); + const inspection = inspectManagedOperation( + repoRoot, + trustedRoot, + operation, + linkIndex, + target + ); if (inspection.status === 'missing') { summary.missing.push(inspection); } else if (inspection.status === 'drifted') { @@ -1185,6 +1353,12 @@ function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations) { ); } +function hookRepairOperations(operationHealth) { + return operationHealth.drifted + .filter(entry => entry.operation.kind === 'update-claude-settings') + .map(entry => ({ ...entry.operation })); +} + function getUnsafeManagedDestinationError(operationHealth) { const hasFinalSymlink = operationHealth.unsafeDestination.some( inspection => inspection.reason === 'final-symlink' @@ -1467,7 +1641,8 @@ function analyzeRecord(record, context) { const operationHealth = summarizeManagedOperationHealth( context.repoRoot, record.targetRoot, - managedOperations + managedOperations, + record.adapter.target ); const missingManagedOperations = operationHealth.missing; @@ -1757,7 +1932,18 @@ function repairInstalledStates(options = {}) { }; } + let releaseSettingsLock = null; try { + if ( + !options.dryRun + && getManagedOperations(record.state || {}).some( + operation => operation.kind === 'update-claude-settings' + ) + ) { + releaseSettingsLock = acquireSettingsLock( + path.join(record.targetRoot, 'settings.json') + ); + } const needsOpencodeBuild = record.adapter.target === 'opencode' && hasOpencodeBuildError(getOpencodeBuildValidationIssues(context)); const opencodeBuildRepairPath = path.join(context.repoRoot, OPENCODE_BUILD_ARTIFACT); @@ -1829,7 +2015,8 @@ function repairInstalledStates(options = {}) { const operationHealth = summarizeManagedOperationHealth( context.repoRoot, record.targetRoot, - desiredPlan.operations + desiredPlan.operations, + record.adapter.target ); const unsafeOperationResult = getUnsafeOperationResult( record, @@ -1876,7 +2063,8 @@ function repairInstalledStates(options = {}) { const operationHealth = summarizeManagedOperationHealth( context.repoRoot, record.targetRoot, - desiredPlan.operations + desiredPlan.operations, + record.adapter.target ); const unsafeOperationResult = getUnsafeOperationResult( @@ -1899,7 +2087,23 @@ function repairInstalledStates(options = {}) { }; } - const repairOperations = [...operationHealth.missing.map(entry => ({ ...entry.operation })), ...operationHealth.drifted.map(entry => ({ ...entry.operation }))]; + const repairOperations = [ + ...operationHealth.missing.map(entry => ({ ...entry.operation })), + ...operationHealth.drifted.map(entry => ({ ...entry.operation })), + ...hookRepairOperations({ + drifted: desiredPlan.operations + .filter(operation => ( + operation.kind === 'update-claude-settings' + && operation.previousManagedHooks + && JSON.stringify(operation.previousManagedHooks) + !== JSON.stringify(operation.managedHooks) + )) + .map(operation => ({ operation })), + }), + ].filter((operation, index, items) => items.findIndex(candidate => ( + candidate.kind === operation.kind + && candidate.destinationPath === operation.destinationPath + )) === index); const repairLinkIndex = buildLinkIndexForOperations(desiredPlan.operations, record.targetRoot); const legacyMigrationPaths = migration.legacyOperationsToRemove.map( operation => operation.destinationPath @@ -1934,7 +2138,9 @@ function repairInstalledStates(options = {}) { context.repoRoot, operation, record.targetRoot, - repairLinkIndex + repairLinkIndex, + record.adapter.target, + Boolean(releaseSettingsLock) ); if (repairedPath) { repairedPaths.push(repairedPath); @@ -1986,6 +2192,8 @@ function repairInstalledStates(options = {}) { plannedRepairs: [], error: error.message }; + } finally { + if (releaseSettingsLock) releaseSettingsLock(); } }); @@ -2100,15 +2308,23 @@ function uninstallInstalledStates(options = {}) { }; } + let releaseSettingsLock = null; try { const removedPaths = []; const cleanupTargets = []; const retainedPaths = []; const operations = getManagedOperations(state); + if (operations.some(operation => operation.kind === 'update-claude-settings')) { + releaseSettingsLock = acquireSettingsLock( + path.join(record.targetRoot, 'settings.json') + ); + } for (const operation of operations) { const outcome = executeUninstallOperation(operation, record.targetRoot, { preserveDriftedCopies: true, + target: record.adapter.target, + settingsLockHeld: Boolean(releaseSettingsLock), }); removedPaths.push(...outcome.removedPaths); cleanupTargets.push(...outcome.cleanupTargets); @@ -2153,6 +2369,8 @@ function uninstallInstalledStates(options = {}) { plannedRemovals, error: error.message }; + } finally { + if (releaseSettingsLock) releaseSettingsLock(); } }); diff --git a/scripts/lib/install-state.js b/scripts/lib/install-state.js index a0aa3bbe6..805943f92 100644 --- a/scripts/lib/install-state.js +++ b/scripts/lib/install-state.js @@ -1,5 +1,6 @@ const fs = require('fs'); const path = require('path'); +const { validateManagedHooks } = require('./install/claude-settings'); // Dependency-free, self-contained validation. The installer closure must not // require any non-builtin package (enterprise supply-chain vetting: the vetted @@ -209,6 +210,38 @@ function createFallbackValidator() { ) { pushError(`${instancePath}/contentSha256`, 'must be a SHA-256 hex digest'); } + if (operation.kind === 'update-claude-settings') { + if (!['claude', 'claude-project'].includes(state.target && state.target.target)) { + pushError(`${instancePath}/kind`, 'is only valid for Claude targets'); + } + if (operation.moduleId !== 'hooks-runtime') { + pushError(`${instancePath}/moduleId`, 'must equal hooks-runtime'); + } + if (String(operation.sourceRelativePath).replace(/\\/g, '/') !== 'hooks/hooks.json') { + pushError(`${instancePath}/sourceRelativePath`, 'must equal hooks/hooks.json'); + } + if ( + isNonEmptyString(state.target && state.target.root) + && isNonEmptyString(operation.destinationPath) + ) { + const expectedDestination = path.resolve(state.target.root, 'settings.json'); + const actualDestination = path.resolve(operation.destinationPath); + const pathsMatch = process.platform === 'win32' + ? expectedDestination.toLowerCase() === actualDestination.toLowerCase() + : expectedDestination === actualDestination; + if (!pathsMatch) { + pushError( + `${instancePath}/destinationPath`, + 'must equal the canonical Claude settings path' + ); + } + } + try { + validateManagedHooks(operation.managedHooks); + } catch (error) { + pushError(`${instancePath}/managedHooks`, error.message); + } + } } } diff --git a/scripts/lib/install-targets/claude-home.js b/scripts/lib/install-targets/claude-home.js index 3729b50c8..0ff84a160 100644 --- a/scripts/lib/install-targets/claude-home.js +++ b/scripts/lib/install-targets/claude-home.js @@ -1,3 +1,4 @@ +const fs = require('fs'); const path = require('path'); const { @@ -8,6 +9,39 @@ const { } = require('./helpers'); const CLAUDE_ECC_NAMESPACE = 'ecc'; +const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; + +function planClaudeHooksOperations(adapter, module, input) { + const sourceHooksRoot = path.join(input.repoRoot || '', 'hooks'); + const operations = [ + createRemappedOperation( + adapter, + module.id, + CLAUDE_HOOKS_CONFIG_PATH, + path.join(adapter.resolveRoot(input), 'settings.json'), + { + kind: 'update-claude-settings', + strategy: 'merge-hook-ids', + } + ), + ]; + + if (!input.repoRoot || !fs.existsSync(sourceHooksRoot)) { + return operations; + } + + return [ + ...operations, + ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) + .filter(entry => entry.name !== 'hooks.json') + .sort((left, right) => left.name.localeCompare(right.name)) + .map(entry => adapter.createScaffoldOperation( + module.id, + path.join('hooks', entry.name), + input + )), + ]; +} function getClaudeManagedDestinationPath(adapter, sourceRelativePath, input) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); @@ -66,7 +100,14 @@ module.exports = createInstallTargetAdapter({ const paths = Array.isArray(module.paths) ? module.paths : []; return paths .filter(p => !isForeignPlatformPath(p, adapter.target)) - .map(sourceRelativePath => { + .flatMap(sourceRelativePath => { + if ( + module.id === 'hooks-runtime' + && normalizeRelativePath(sourceRelativePath) === 'hooks' + ) { + return planClaudeHooksOperations(adapter, module, planningInput); + } + const managedDestinationPath = getClaudeManagedDestinationPath( adapter, sourceRelativePath, @@ -74,16 +115,16 @@ module.exports = createInstallTargetAdapter({ ); if (managedDestinationPath) { - return createRemappedOperation( + return [createRemappedOperation( adapter, module.id, sourceRelativePath, managedDestinationPath, { strategy: 'preserve-relative-path' } - ); + )]; } - return adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput); + return [adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput)]; }); }); }, diff --git a/scripts/lib/install-targets/claude-project.js b/scripts/lib/install-targets/claude-project.js index 051b0ae26..4c5f23a32 100644 --- a/scripts/lib/install-targets/claude-project.js +++ b/scripts/lib/install-targets/claude-project.js @@ -1,3 +1,4 @@ +const fs = require('fs'); const path = require('path'); const { @@ -8,6 +9,39 @@ const { } = require('./helpers'); const CLAUDE_ECC_NAMESPACE = 'ecc'; +const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; + +function planClaudeHooksOperations(adapter, module, input) { + const sourceHooksRoot = path.join(input.repoRoot || '', 'hooks'); + const operations = [ + createRemappedOperation( + adapter, + module.id, + CLAUDE_HOOKS_CONFIG_PATH, + path.join(adapter.resolveRoot(input), 'settings.json'), + { + kind: 'update-claude-settings', + strategy: 'merge-hook-ids', + } + ), + ]; + + if (!input.repoRoot || !fs.existsSync(sourceHooksRoot)) { + return operations; + } + + return [ + ...operations, + ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) + .filter(entry => entry.name !== 'hooks.json') + .sort((left, right) => left.name.localeCompare(right.name)) + .map(entry => adapter.createScaffoldOperation( + module.id, + path.join('hooks', entry.name), + input + )), + ]; +} function getClaudeManagedDestinationPath(adapter, sourceRelativePath, input) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); @@ -66,7 +100,14 @@ module.exports = createInstallTargetAdapter({ const paths = Array.isArray(module.paths) ? module.paths : []; return paths .filter(p => !isForeignPlatformPath(p, 'claude')) - .map(sourceRelativePath => { + .flatMap(sourceRelativePath => { + if ( + module.id === 'hooks-runtime' + && normalizeRelativePath(sourceRelativePath) === 'hooks' + ) { + return planClaudeHooksOperations(adapter, module, planningInput); + } + const managedDestinationPath = getClaudeManagedDestinationPath( adapter, sourceRelativePath, @@ -74,16 +115,16 @@ module.exports = createInstallTargetAdapter({ ); if (managedDestinationPath) { - return createRemappedOperation( + return [createRemappedOperation( adapter, module.id, sourceRelativePath, managedDestinationPath, { strategy: 'preserve-relative-path' } - ); + )]; } - return adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput); + return [adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput)]; }); }); }, diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index e755586a8..1c5d4900d 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -8,8 +8,16 @@ const { hasExplicitCommitAttributionPreference, withCommitAttributionDisabled, } = require('../claude-commit-attribution'); -const { writeInstallState } = require('../install-state'); +const { readInstallState, writeInstallState } = require('../install-state'); const { assertHookConsentReady, planMaterializesHookRuntime } = require('./hook-consent'); +const { + acquireSettingsLock, + mergeManagedHooks, + readSettings, + uninstallManagedHooks, + updateSettingsAtomic, + validateManagedHooks, +} = require('./claude-settings'); const { filterMcpConfig, parseDisabledMcpServers } = require('../mcp-config'); const { assertWithinTrustedRoot } = require('../path-safety'); const { @@ -200,69 +208,21 @@ function shouldSetClaudeCommitAttributionPreference(plan) { }); } -function writeClaudeCommitAttributionPreference(settingsPath) { - // Read once rather than probing with existsSync first. Checking for the file and - // then writing it is a file system race (CodeQL js/file-system-race), and a - // missing file is simply the fresh-install case. - let settings; +function writeClaudeCommitAttributionPreference(settingsPath, options = {}) { try { - settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); - } catch (error) { - if (error.code !== 'ENOENT') { - // Unreadable or malformed settings belong to the user; leave them untouched. - return false; - } - settings = {}; - } - - if (!settings || typeof settings !== 'object' || Array.isArray(settings)) { + let changed = false; + updateSettingsAtomic(settingsPath, settings => { + if (hasExplicitCommitAttributionPreference(settings)) { + return { settings }; + } + changed = true; + return { settings: withCommitAttributionDisabled(settings) }; + }, options); + return changed; + } catch (_error) { + // Unreadable or malformed settings belong to the user; leave them untouched. return false; } - - if (hasExplicitCommitAttributionPreference(settings)) { - return false; - } - - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - fs.writeFileSync( - settingsPath, - formatJson(withCommitAttributionDisabled(settings)), - 'utf8' - ); - return true; -} - -function replacePluginRootPlaceholders(value, pluginRoot) { - if (!pluginRoot) { - return value; - } - - if (typeof value === 'string') { - return value.split('${CLAUDE_PLUGIN_ROOT}').join(pluginRoot); - } - - if (Array.isArray(value)) { - return value.map(item => replacePluginRootPlaceholders(item, pluginRoot)); - } - - if (value && typeof value === 'object') { - return Object.fromEntries( - Object.entries(value).map(([key, nestedValue]) => [ - key, - replacePluginRootPlaceholders(nestedValue, pluginRoot), - ]) - ); - } - - return value; -} - -function findHooksOperation(plan, hooksDestinationPath) { - return plan.operations.find(item => ( - item.destinationPath === hooksDestinationPath - && item.moduleId === 'hooks-runtime' - && typeof item.sourcePath === 'string' - )); } function isMcpConfigPath(filePath) { @@ -302,40 +262,135 @@ function assertSafeInstallOperation(plan, operation) { } } -function buildResolvedClaudeHooks(plan) { - if (!plan.adapter || (plan.adapter.target !== 'claude' && plan.adapter.target !== 'claude-project')) { +function readPreviousInstallState(plan) { + if (!fs.existsSync(plan.installStatePath)) { + return null; + } + return readInstallState(plan.installStatePath); +} + +function comparablePath(filePath) { + const resolved = path.resolve(filePath); + return process.platform === 'win32' ? resolved.toLowerCase() : resolved; +} + +function findPreviousManagedHooks(previousState, plan, operation) { + if ( + !previousState + || previousState.target.id !== plan.adapter.id + || comparablePath(previousState.target.root) !== comparablePath(plan.targetRoot) + || comparablePath(previousState.target.installStatePath) !== comparablePath(plan.installStatePath) + ) { return null; } - const pluginRoot = plan.targetRoot; - const hooksDestinationPath = path.join(plan.targetRoot, 'hooks', 'hooks.json'); - const hooksOperation = findHooksOperation(plan, hooksDestinationPath); - if (!hooksOperation) { - return null; - } - const hooksSourcePath = hooksOperation.sourcePath; - if (!fs.existsSync(hooksSourcePath)) { + const previousOperation = (previousState.operations || []).find(candidate => ( + candidate.kind === operation.kind + && candidate.destinationPath === operation.destinationPath + )); + if (!previousOperation || !previousOperation.managedHooks) { return null; } - const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config'); - const resolvedHooks = replacePluginRootPlaceholders(hooksConfig.hooks, pluginRoot); - if (!resolvedHooks || typeof resolvedHooks !== 'object' || Array.isArray(resolvedHooks)) { - throw new Error(`Invalid hooks config at ${hooksSourcePath}: expected "hooks" to be a JSON object`); + return validateManagedHooks( + previousOperation.managedHooks, + 'previous managed hooks' + ); +} + +function preflightClaudeSettingsOperations(plan) { + const settingsOperations = plan.operations.filter(operation => ( + operation.kind === 'update-claude-settings' + || operation.kind === 'remove-claude-settings-hooks' + )); + if (settingsOperations.length === 0) { + return new Map(); } + const previousState = readPreviousInstallState(plan); + return new Map(settingsOperations.map(operation => { + assertSafeInstallOperation(plan, operation); + const managedHooks = validateManagedHooks(operation.managedHooks); + const settings = readSettings(operation.destinationPath); + const previousManagedHooks = findPreviousManagedHooks(previousState, plan, operation); + if (operation.kind === 'remove-claude-settings-hooks') { + const removal = uninstallManagedHooks(settings, managedHooks); + if (removal.retained.length > 0) { + throw new Error( + `Refusing to disable modified Claude hooks in ${operation.destinationPath}; ` + + 'run the ECC uninstaller to review retained entries.' + ); + } + } else { + mergeManagedHooks(settings, managedHooks, { previousManagedHooks }); + } + return [operation, { managedHooks, previousManagedHooks }]; + })); +} + +function prepareHookConsentMigration(plan, migration) { + if (plan.hookConsent !== 'declined') { + return migration; + } + const previousState = readPreviousInstallState(plan); + if (!previousState) { + return migration; + } + + const removals = (previousState.operations || []) + .filter(operation => operation.kind === 'update-claude-settings') + .map(operation => ({ + ...operation, + kind: 'remove-claude-settings-hooks', + strategy: 'remove-hook-ids', + scaffoldOnly: false, + })); + if (removals.length === 0) { + return migration; + } + const removalDestinations = new Set(removals.map(operation => comparablePath( + operation.destinationPath + ))); return { - hooksOperation, - hooksDestinationPath, - resolvedHooksConfig: { - ...hooksConfig, - hooks: resolvedHooks, + ...migration, + // Disable hooks only after every ordinary install operation succeeds so a + // partial reinstall cannot silently revoke working hooks before failing. + appliedOperations: [...migration.appliedOperations, ...removals], + finalState: { + ...migration.finalState, + operations: migration.finalState.operations.filter(operation => !( + operation.kind === 'update-claude-settings' + && removalDestinations.has(comparablePath(operation.destinationPath)) + )), }, + bridgeState: { + ...migration.bridgeState, + request: { + ...migration.bridgeState.request, + hookConsent: 'enabled', + }, + resolution: { + ...migration.bridgeState.resolution, + selectedModules: [...new Set([ + ...migration.bridgeState.resolution.selectedModules, + 'hooks-runtime', + ])], + }, + }, + requiresBridgeState: true, }; } function previewInstallPlan(plan) { - const migration = prepareClaudeSkillMigration(plan); + const migration = prepareHookConsentMigration( + plan, + prepareClaudeSkillMigration(plan) + ); + const appliedPlan = { + ...plan, + operations: migration.appliedOperations, + }; + preflightClaudeSettingsOperations(appliedPlan); const hookConsentWarnings = planMaterializesHookRuntime(plan) && plan.hookConsent !== 'enabled' ? ['Applying this plan requires an explicit hook decision: --enable-hooks or --no-hooks.'] : []; @@ -356,6 +411,25 @@ function previewInstallPlan(plan) { function applyInstallPlan(plan, dependencies = {}) { assertHookConsentReady(plan); + const isClaudeManualTarget = plan.adapter + && (plan.adapter.target === 'claude' || plan.adapter.target === 'claude-project'); + const settingsPathToLock = isClaudeManualTarget + ? path.join(plan.targetRoot, 'settings.json') + : null; + if (settingsPathToLock) { + assertSafeInstallOperation(plan, { destinationPath: settingsPathToLock }); + } + const releaseSettingsLock = settingsPathToLock + ? acquireSettingsLock(settingsPathToLock) + : null; + try { + return applyInstallPlanLocked(plan, dependencies, Boolean(releaseSettingsLock)); + } finally { + if (releaseSettingsLock) releaseSettingsLock(); + } +} + +function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = false) { const persistInstallState = dependencies.writeInstallState || writeInstallState; const beforeInstallStateRead = dependencies.beforeInstallStateRead; const beforeOperationWrite = dependencies.beforeOperationWrite; @@ -363,30 +437,36 @@ function applyInstallPlan(plan, dependencies = {}) { if (typeof beforeInstallStateRead === 'function') { beforeInstallStateRead({ plan }); } - const migration = prepareClaudeSkillMigration(plan); + const migration = prepareHookConsentMigration( + plan, + prepareClaudeSkillMigration(plan) + ); const appliedPlan = { ...plan, operations: migration.appliedOperations, }; - const resolvedClaudeHooksPlan = buildResolvedClaudeHooks(appliedPlan); + const preparedClaudeSettings = preflightClaudeSettingsOperations(appliedPlan); const disabledServers = parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS); const linkIndex = buildLinkIndexForPlan(appliedPlan); const hasLegacyMigration = migration.legacyOperationsToRemove.length > 0; - - if (migration.requiresBridgeState) { - // Own every operation that may be written during a flat-skill migration - // before the first copy. A later failure is retryable and uninstall can - // clean the entire partial install, including non-skill files. During - // legacy migration the bridge also retains the prior managed operations. - if (typeof beforeInstallStateWrite === 'function') { - beforeInstallStateWrite({ plan: appliedPlan, state: migration.bridgeState }); + const hookRemovalCount = appliedPlan.operations.filter(operation => ( + operation.kind === 'remove-claude-settings-hooks' + )).length; + let completedHookRemovalCount = 0; + if (migration.requiresBridgeState) { + // Own every operation that may be written during a flat-skill migration + // before the first copy. A later failure is retryable and uninstall can + // clean the entire partial install, including non-skill files. During + // legacy migration the bridge also retains the prior managed operations. + if (typeof beforeInstallStateWrite === 'function') { + beforeInstallStateWrite({ plan: appliedPlan, state: migration.bridgeState }); + } + persistInstallState(plan.installStatePath, migration.bridgeState); } - persistInstallState(plan.installStatePath, migration.bridgeState); - } - let finalState; - try { - for (const operation of appliedPlan.operations) { + let finalState; + try { + for (const operation of appliedPlan.operations) { assertSafeInstallOperation(appliedPlan, operation); assertSafeClaudeSkillOperation(appliedPlan, operation); fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true }); @@ -399,6 +479,42 @@ function applyInstallPlan(plan, dependencies = {}) { beforeOperationWrite({ plan: appliedPlan, operation }); } + if ( + operation.kind === 'update-claude-settings' + || operation.kind === 'remove-claude-settings-hooks' + ) { + // Re-read at the write boundary so unrelated settings added after + // planning are preserved. A same-ID change still fails closed. + const prepared = preparedClaudeSettings.get(operation); + assertSafeInstallOperation(appliedPlan, operation); + updateSettingsAtomic(operation.destinationPath, latestSettings => { + const merged = operation.kind === 'remove-claude-settings-hooks' + ? uninstallManagedHooks(latestSettings, prepared.managedHooks) + : mergeManagedHooks(latestSettings, prepared.managedHooks, { + previousManagedHooks: prepared.previousManagedHooks, + }); + if ( + operation.kind === 'remove-claude-settings-hooks' + && merged.retained.length > 0 + ) { + throw new Error( + `Refusing to disable modified Claude hooks in ${operation.destinationPath}; ` + + 'run the ECC uninstaller to review retained entries.' + ); + } + return merged; + }, { + lockHeld: settingsLockHeld, + beforeCommit() { + assertSafeInstallOperation(appliedPlan, operation); + }, + }); + if (operation.kind === 'remove-claude-settings-hooks') { + completedHookRemovalCount += 1; + } + continue; + } + if (operation.kind === 'merge-json') { const payload = cloneJsonValue(operation.mergePayload); if (payload === undefined) { @@ -450,55 +566,49 @@ function applyInstallPlan(plan, dependencies = {}) { } fs.copyFileSync(operation.sourcePath, operation.destinationPath); - } - - if (resolvedClaudeHooksPlan) { - assertSafeInstallOperation(appliedPlan, resolvedClaudeHooksPlan.hooksOperation); - fs.mkdirSync(path.dirname(resolvedClaudeHooksPlan.hooksDestinationPath), { recursive: true }); - assertSafeInstallOperation(appliedPlan, resolvedClaudeHooksPlan.hooksOperation); - if (typeof beforeOperationWrite === 'function') { - beforeOperationWrite({ plan: appliedPlan, operation: resolvedClaudeHooksPlan.hooksOperation }); } - fs.writeFileSync( - resolvedClaudeHooksPlan.hooksDestinationPath, - JSON.stringify(resolvedClaudeHooksPlan.resolvedHooksConfig, null, 2) + '\n', - 'utf8' - ); - } - if (hasLegacyMigration) { - removeLegacyClaudeSkillFiles(migration, plan.targetRoot); - } + if (hasLegacyMigration) { + removeLegacyClaudeSkillFiles(migration, plan.targetRoot); + } - if (shouldSetClaudeCommitAttributionPreference(appliedPlan)) { - writeClaudeCommitAttributionPreference(path.join(plan.targetRoot, 'settings.json')); - } - - finalState = stateWithContentDigests(migration.finalState, appliedPlan); - if (typeof beforeInstallStateWrite === 'function') { - beforeInstallStateWrite({ plan: appliedPlan, state: finalState }); - } - persistInstallState(plan.installStatePath, finalState); - } catch (error) { - if (migration.requiresBridgeState) { - try { - // The bridge was committed before any writes. Refresh it with hashes of - // files that now exist so uninstall can remove only bytes this attempt - // actually installed while preserving user changes. - persistInstallState( - plan.installStatePath, - stateWithContentDigests(migration.bridgeState, appliedPlan) - ); - } catch (checkpointError) { - throw new Error( - `${error.message} Install-state checkpoint also failed: ${checkpointError.message}`, - { cause: error } + if (shouldSetClaudeCommitAttributionPreference(appliedPlan)) { + writeClaudeCommitAttributionPreference( + path.join(plan.targetRoot, 'settings.json'), + { lockHeld: settingsLockHeld } ); } + + finalState = stateWithContentDigests(migration.finalState, appliedPlan); + if (typeof beforeInstallStateWrite === 'function') { + beforeInstallStateWrite({ plan: appliedPlan, state: finalState }); + } + persistInstallState(plan.installStatePath, finalState); + } catch (error) { + if (migration.requiresBridgeState) { + try { + // The bridge was committed before any writes. Refresh it with hashes of + // files that now exist so uninstall can remove only bytes this attempt + // actually installed while preserving user changes. + persistInstallState( + plan.installStatePath, + stateWithContentDigests( + hookRemovalCount > 0 && completedHookRemovalCount === hookRemovalCount + ? migration.finalState + : migration.bridgeState, + appliedPlan + ) + ); + } catch (checkpointError) { + throw new Error( + `${error.message} Install-state checkpoint also failed: ${checkpointError.message}`, + { cause: error } + ); + } + } + throw error; } - throw error; - } - let antigravityMigrationWarnings = []; + let antigravityMigrationWarnings = []; try { const antigravityMigration = cleanupLegacyAntigravityInstall(appliedPlan); if (antigravityMigration.detected && !antigravityMigration.complete) { @@ -528,20 +638,20 @@ function applyInstallPlan(plan, dependencies = {}) { ]; } - return { - ...plan, - statePreview: finalState, - plannedOperations: [...plan.operations], - operations: migration.appliedOperations, - skippedOperations: migration.skippedOperations, - warnings: [ - ...(Array.isArray(plan.warnings) ? plan.warnings : []), - ...migration.warnings, - ...antigravityMigrationWarnings, - ...opencodeMigrationWarnings, - ], - applied: true, - }; + return { + ...plan, + statePreview: finalState, + plannedOperations: [...plan.operations], + operations: migration.appliedOperations, + skippedOperations: migration.skippedOperations, + warnings: [ + ...(Array.isArray(plan.warnings) ? plan.warnings : []), + ...migration.warnings, + ...antigravityMigrationWarnings, + ...opencodeMigrationWarnings, + ], + applied: true, + }; } module.exports = { diff --git a/scripts/lib/install/claude-settings.js b/scripts/lib/install/claude-settings.js new file mode 100644 index 000000000..90b0791ec --- /dev/null +++ b/scripts/lib/install/claude-settings.js @@ -0,0 +1,750 @@ +'use strict'; + +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); +const { isDeepStrictEqual } = require('util'); +const { writeFileAtomic } = require('../atomic-write'); + +const PLUGIN_ROOT_PLACEHOLDER = '${CLAUDE_PLUGIN_ROOT}'; +const VALID_EVENTS = new Set([ + 'SessionStart', 'UserPromptSubmit', 'PreToolUse', 'PermissionRequest', + 'PostToolUse', 'PostToolUseFailure', 'Notification', 'SubagentStart', + 'Stop', 'SubagentStop', 'PreCompact', 'InstructionsLoaded', + 'TeammateIdle', 'TaskCompleted', 'ConfigChange', 'WorktreeCreate', + 'WorktreeRemove', 'SessionEnd', +]); +const EVENTS_WITHOUT_MATCHER = new Set([ + 'UserPromptSubmit', 'Notification', 'Stop', 'SubagentStop', +]); +const VALID_HOOK_TYPES = new Set(['command', 'http', 'prompt', 'agent']); +const INVALID_LOCK_STALE_MS = 5 * 60 * 1000; + +function isJsonObject(value) { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +function cloneValue(value) { + if (Array.isArray(value)) { + return value.map(cloneValue); + } + if (isJsonObject(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, nestedValue]) => [key, cloneValue(nestedValue)]) + ); + } + return value; +} + +function isNonEmptyString(value) { + return typeof value === 'string' && value.trim() !== ''; +} + +function validateHookHandler(hook, label) { + if (!isJsonObject(hook)) { + throw new Error(`Invalid managed hook handler at ${label}: expected a JSON object`); + } + if (!VALID_HOOK_TYPES.has(hook.type)) { + throw new Error(`Invalid managed hook handler at ${label}: unsupported type`); + } + if (hook.timeout !== undefined && (typeof hook.timeout !== 'number' || hook.timeout < 0)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid timeout`); + } + + if (hook.type === 'command') { + const validCommand = isNonEmptyString(hook.command) + || (Array.isArray(hook.command) + && hook.command.length > 0 + && hook.command.every(isNonEmptyString)); + if (!validCommand) { + throw new Error(`Invalid managed hook handler at ${label}: invalid command`); + } + if (hook.async !== undefined && typeof hook.async !== 'boolean') { + throw new Error(`Invalid managed hook handler at ${label}: invalid async flag`); + } + return; + } + + if (hook.async !== undefined) { + throw new Error(`Invalid managed hook handler at ${label}: async requires command type`); + } + if (hook.type === 'http') { + if (!isNonEmptyString(hook.url)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid url`); + } + if ( + hook.headers !== undefined + && (!isJsonObject(hook.headers) + || !Object.values(hook.headers).every(value => typeof value === 'string')) + ) { + throw new Error(`Invalid managed hook handler at ${label}: invalid headers`); + } + if ( + hook.allowedEnvVars !== undefined + && (!Array.isArray(hook.allowedEnvVars) + || !hook.allowedEnvVars.every(isNonEmptyString)) + ) { + throw new Error(`Invalid managed hook handler at ${label}: invalid allowedEnvVars`); + } + return; + } + if (!isNonEmptyString(hook.prompt)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid prompt`); + } + if (hook.model !== undefined && !isNonEmptyString(hook.model)) { + throw new Error(`Invalid managed hook handler at ${label}: invalid model`); + } +} + +function validateManagedHooks(managedHooks, label = 'managed hooks') { + if (!isJsonObject(managedHooks)) { + throw new Error(`Invalid ${label}: expected a JSON object`); + } + if (Object.keys(managedHooks).length === 0) { + throw new Error(`Invalid ${label}: expected at least one hook event`); + } + + const seenIds = new Set(); + for (const [event, entries] of Object.entries(managedHooks)) { + if (!VALID_EVENTS.has(event)) { + throw new Error(`Invalid ${label}: unsupported hook event "${event}"`); + } + if (!Array.isArray(entries)) { + throw new Error(`Invalid ${label}.${event}: expected an array`); + } + if (entries.length === 0) { + throw new Error(`Invalid ${label}.${event}: expected at least one hook entry`); + } + + entries.forEach((entry, index) => { + if (!isJsonObject(entry)) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: expected a JSON object` + ); + } + if (typeof entry.id !== 'string' || entry.id.trim() === '') { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: ` + + 'expected a non-empty unique id' + ); + } + if (seenIds.has(entry.id)) { + throw new Error(`Invalid ${label}: expected globally unique id "${entry.id}"`); + } + seenIds.add(entry.id); + if ( + !Object.prototype.hasOwnProperty.call(entry, 'matcher') + && !EVENTS_WITHOUT_MATCHER.has(event) + ) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: missing matcher` + ); + } + if ( + Object.prototype.hasOwnProperty.call(entry, 'matcher') + && typeof entry.matcher !== 'string' + && !isJsonObject(entry.matcher) + ) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: invalid matcher` + ); + } + if (!Array.isArray(entry.hooks) || entry.hooks.length === 0) { + throw new Error( + `Invalid managed hook entry at ${label}.${event}[${index}]: expected hooks` + ); + } + entry.hooks.forEach((hook, hookIndex) => { + validateHookHandler(hook, `${label}.${event}[${index}].hooks[${hookIndex}]`); + }); + }); + } + + return cloneValue(managedHooks); +} + +function validateSettings(settings, label = 'Claude settings') { + if (!isJsonObject(settings)) { + throw new Error(`Invalid ${label}: expected a JSON object`); + } + + if (Object.prototype.hasOwnProperty.call(settings, 'hooks')) { + if (!isJsonObject(settings.hooks)) { + throw new Error(`Invalid ${label}: expected "hooks" to be a JSON object`); + } + for (const [event, entries] of Object.entries(settings.hooks)) { + if (!Array.isArray(entries)) { + throw new Error(`Invalid ${label}: expected hooks.${event} to be an array`); + } + } + } + + return cloneValue(settings); +} + +function replacePluginRootPlaceholders(value, pluginRoot) { + if (typeof pluginRoot !== 'string') { + throw new Error('Invalid Claude plugin root: expected a string'); + } + if (typeof value === 'string') { + return value.split(PLUGIN_ROOT_PLACEHOLDER).join(pluginRoot); + } + if (Array.isArray(value)) { + return value.map(item => replacePluginRootPlaceholders(item, pluginRoot)); + } + if (isJsonObject(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, nestedValue]) => [ + key, + replacePluginRootPlaceholders(nestedValue, pluginRoot), + ]) + ); + } + return value; +} + +function resolveManagedHookCommands(managedHooks, targetRoot) { + const encodedRoot = Buffer.from(targetRoot, 'utf8').toString('base64'); + const rootExpression = `Buffer.from('${encodedRoot}','base64').toString('utf8')`; + return Object.fromEntries( + Object.entries(managedHooks).map(([event, entries]) => [ + event, + entries.map(entry => ({ + ...entry, + hooks: entry.hooks.map(hook => ({ + ...hook, + ...(typeof hook.command === 'string' + ? { + command: hook.command + .split('var e=process.env.CLAUDE_PLUGIN_ROOT;') + .join(`var e=${rootExpression};`), + } + : {}), + })), + })), + ]) + ); +} + +function materializeManagedHooks(hooksConfig, targetRoot) { + if (!isJsonObject(hooksConfig) || !isJsonObject(hooksConfig.hooks)) { + throw new Error('Invalid hooks config: expected a JSON object with a hooks object'); + } + if (!isNonEmptyString(targetRoot)) { + throw new Error('Invalid Claude target root: expected a non-empty string'); + } + return validateManagedHooks(resolveManagedHookCommands( + replacePluginRootPlaceholders(hooksConfig.hooks, targetRoot), + targetRoot + )); +} + +function parseSettings(rawSettings, label = 'Claude settings') { + let settings; + try { + settings = JSON.parse(rawSettings); + } catch (error) { + throw new Error(`Failed to parse ${label}: ${error.message}`, { cause: error }); + } + return validateSettings(settings, label); +} + +function readSettings(settingsPath, fileSystem = fs) { + const reader = fileSystem && fileSystem.fs ? fileSystem.fs : fileSystem; + let rawSettings; + try { + rawSettings = reader.readFileSync(settingsPath, 'utf8'); + } catch (error) { + if (error && error.code === 'ENOENT') { + return {}; + } + throw error; + } + return parseSettings(rawSettings, `Claude settings at ${settingsPath}`); +} + +function readSettingsSnapshot(settingsPath) { + const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0); + let descriptor; + try { + descriptor = fs.openSync(settingsPath, flags); + } catch (error) { + if (error && error.code === 'ENOENT') { + return { exists: false, raw: null, settings: {}, mode: 0o600 }; + } + throw error; + } + + try { + const descriptorStat = fs.fstatSync(descriptor); + let pathStat; + try { + pathStat = fs.lstatSync(settingsPath); + } catch (error) { + if (error && error.code === 'ENOENT') { + error.code = 'ECC_SETTINGS_CHANGED'; + } + throw error; + } + if ( + !descriptorStat.isFile() + || !pathStat.isFile() + || pathStat.isSymbolicLink() + || descriptorStat.dev !== pathStat.dev + || descriptorStat.ino !== pathStat.ino + ) { + const error = new Error(`Refusing to read changed Claude settings at ${settingsPath}`); + error.code = 'ECC_SETTINGS_CHANGED'; + throw error; + } + const raw = fs.readFileSync(descriptor, 'utf8'); + return { + exists: true, + raw, + settings: parseSettings(raw, `Claude settings at ${settingsPath}`), + mode: descriptorStat.mode & 0o777, + dev: descriptorStat.dev, + ino: descriptorStat.ino, + }; + } finally { + fs.closeSync(descriptor); + } +} + +function assertSettingsSnapshotUnchanged(settingsPath, snapshot) { + let current; + try { + current = readSettingsSnapshot(settingsPath); + } catch (error) { + error.code = error.code || 'ECC_SETTINGS_CHANGED'; + throw error; + } + const unchanged = current.exists === snapshot.exists + && current.raw === snapshot.raw + && (!current.exists || (current.dev === snapshot.dev && current.ino === snapshot.ino)); + if (!unchanged) { + const error = new Error(`Claude settings changed during update: ${settingsPath}`); + error.code = 'ECC_SETTINGS_CHANGED'; + throw error; + } +} + +function createSettingsLock(lockPath) { + const tempPath = `${lockPath}.create-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + let descriptor; + let ownedStats; + try { + descriptor = fs.openSync(tempPath, 'wx', 0o600); + fs.writeFileSync(descriptor, `${JSON.stringify({ + pid: process.pid, + startedAt: new Date().toISOString(), + token: crypto.randomBytes(16).toString('hex'), + })}\n`); + fs.fsyncSync(descriptor); + ownedStats = fs.fstatSync(descriptor, { bigint: true }); + fs.closeSync(descriptor); + descriptor = undefined; + fs.linkSync(tempPath, lockPath); + } catch (error) { + if (descriptor !== undefined) fs.closeSync(descriptor); + fs.rmSync(tempPath, { force: true }); + throw error; + } + fs.rmSync(tempPath, { force: true }); + + let released = false; + return () => { + if (released) return; + released = true; + const quarantinePath = `${lockPath}.release-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + fs.renameSync(lockPath, quarantinePath); + const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); + if (!sameFileIdentity(quarantinedStats, ownedStats)) { + if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); + throw new Error(`Refusing to release a changed Claude settings lock: ${lockPath}`); + } + fs.rmSync(quarantinePath, { force: true }); + }; +} + +function sameFileIdentity(left, right) { + return left.dev === right.dev && left.ino === right.ino; +} + +function inspectSettingsLock(lockPath) { + const descriptor = fs.openSync(lockPath, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + try { + const stats = fs.fstatSync(descriptor, { bigint: true }); + const pathStats = fs.lstatSync(lockPath, { bigint: true }); + if ( + !stats.isFile() + || pathStats.isSymbolicLink() + || !pathStats.isFile() + || !sameFileIdentity(stats, pathStats) + ) { + return { metadata: null, stats }; + } + let metadata = null; + try { + metadata = JSON.parse(fs.readFileSync(descriptor, 'utf8')); + } catch (_error) { + // Invalid locks may be recovered only after the bounded lease below. + } + return { metadata, stats }; + } finally { + fs.closeSync(descriptor); + } +} + +function processIsAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code !== 'ESRCH'; + } +} + +function recoverSettingsLock(lockPath) { + const recoveryPath = `${lockPath}.recover`; + try { + fs.mkdirSync(recoveryPath, { mode: 0o700 }); + } catch (error) { + if (error && error.code === 'EEXIST') return null; + throw error; + } + + const quarantinePath = `${lockPath}.stale-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + try { + let inspected; + try { + inspected = inspectSettingsLock(lockPath); + } catch (error) { + if (error && error.code === 'ENOENT') return createSettingsLock(lockPath); + throw error; + } + const validOwner = Number.isSafeInteger(inspected.metadata && inspected.metadata.pid) + && inspected.metadata.pid > 0; + const stale = validOwner + ? !processIsAlive(inspected.metadata.pid) + : Date.now() - Number(inspected.stats.mtimeMs) >= INVALID_LOCK_STALE_MS; + if (!stale) return null; + + fs.renameSync(lockPath, quarantinePath); + const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); + if (!sameFileIdentity(quarantinedStats, inspected.stats)) { + if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); + return null; + } + fs.rmSync(quarantinePath, { force: true }); + return createSettingsLock(lockPath); + } finally { + fs.rmSync(recoveryPath, { recursive: true, force: true }); + fs.rmSync(quarantinePath, { force: true }); + } +} + +function acquireSettingsLock(settingsPath) { + const lockPath = `${settingsPath}.ecc.lock`; + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + try { + return createSettingsLock(lockPath); + } catch (error) { + if (!error || error.code !== 'EEXIST') { + throw error; + } + } + const recovered = recoverSettingsLock(lockPath); + if (recovered) return recovered; + throw new Error( + `Another ECC process is updating Claude settings: ${settingsPath}. ` + + `If no ECC process is active, inspect and remove ${lockPath}.` + ); +} + +function updateSettingsAtomic(settingsPath, transform, options = {}) { + const update = () => { + const maxAttempts = options.maxAttempts || 3; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + const snapshot = readSettingsSnapshot(settingsPath); + const result = transform(snapshot.settings); + if (typeof options.beforeCommit === 'function') options.beforeCommit(); + assertSettingsSnapshotUnchanged(settingsPath, snapshot); + writeFileAtomic( + settingsPath, + `${JSON.stringify(result.settings, null, 2)}\n`, + { encoding: 'utf8', mode: snapshot.mode } + ); + return result; + } catch (error) { + if (error.code !== 'ECC_SETTINGS_CHANGED' || attempt === maxAttempts) { + throw error; + } + } + } + throw new Error(`Unable to update Claude settings at ${settingsPath}`); + }; + if (options.lockHeld) { + return update(); + } + const releaseLock = acquireSettingsLock(settingsPath); + try { + return update(); + } finally { + releaseLock(); + } +} + +function reference(event, id) { + return { event, id }; +} + +function entriesMatchingId(entries, id) { + return entries + .map((entry, index) => ({ entry, index })) + .filter(candidate => isJsonObject(candidate.entry) && candidate.entry.id === id); +} + +function assertUnambiguousMatch(entries, event, id) { + const matches = entriesMatchingId(entries, id); + if (matches.length > 1) { + throw new Error( + `Claude settings contains multiple hooks for event "${event}" and id "${id}"` + ); + } + return matches[0] || null; +} + +function managedEntryFor(managedHooks, event, id) { + const entries = managedHooks && managedHooks[event]; + if (!Array.isArray(entries)) { + return null; + } + return entries.find(entry => entry.id === id) || null; +} + +function mergeManagedHooks(settings, managedHooks, options = {}) { + const validatedSettings = validateSettings(settings); + const desiredHooks = validateManagedHooks(managedHooks); + const previousHooks = options.previousManagedHooks === undefined + || options.previousManagedHooks === null + ? null + : validateManagedHooks(options.previousManagedHooks, 'previous managed hooks'); + const repair = options.mode === 'repair' || options.repair === true; + if (options.mode !== undefined && options.mode !== 'merge' && options.mode !== 'repair') { + throw new Error(`Unknown Claude settings merge mode: ${options.mode}`); + } + + let nextHooks = validatedSettings.hooks + ? cloneValue(validatedSettings.hooks) + : {}; + const added = []; + const updated = []; + const unchanged = []; + const removed = []; + + if (previousHooks) { + for (const [event, previousEntries] of Object.entries(previousHooks)) { + let eventEntries = nextHooks[event] ? cloneValue(nextHooks[event]) : []; + for (const previousEntry of previousEntries) { + if (managedEntryFor(desiredHooks, event, previousEntry.id)) continue; + const match = assertUnambiguousMatch(eventEntries, event, previousEntry.id); + if (!match) continue; + if (!isDeepStrictEqual(match.entry, previousEntry)) { + throw new Error( + `Refusing to remove Claude hook for event "${event}" and id ` + + `"${previousEntry.id}" because the previous managed entry has drifted` + ); + } + eventEntries = eventEntries.filter((_entry, index) => index !== match.index); + removed.push(reference(event, previousEntry.id)); + } + nextHooks = eventEntries.length > 0 + ? { ...nextHooks, [event]: eventEntries } + : withoutProperty(nextHooks, event); + } + } + + for (const [event, desiredEntries] of Object.entries(desiredHooks)) { + let eventEntries = nextHooks[event] ? cloneValue(nextHooks[event]) : []; + for (const desiredEntry of desiredEntries) { + const match = assertUnambiguousMatch(eventEntries, event, desiredEntry.id); + if (!match) { + eventEntries = [...eventEntries, cloneValue(desiredEntry)]; + added.push(reference(event, desiredEntry.id)); + continue; + } + if (isDeepStrictEqual(match.entry, desiredEntry)) { + unchanged.push(reference(event, desiredEntry.id)); + continue; + } + + const previousEntry = managedEntryFor(previousHooks, event, desiredEntry.id); + if (!repair && (!previousEntry || !isDeepStrictEqual(match.entry, previousEntry))) { + const driftReason = previousEntry ? ' because the previous managed entry has drifted' : ''; + throw new Error( + `Refusing to overwrite Claude hook for event "${event}" and id ` + + `"${desiredEntry.id}"${driftReason}` + ); + } + + eventEntries = eventEntries.map((entry, index) => ( + index === match.index ? cloneValue(desiredEntry) : entry + )); + updated.push(reference(event, desiredEntry.id)); + } + if (desiredEntries.length > 0) { + nextHooks = { ...nextHooks, [event]: eventEntries }; + } + } + + const nextSettings = Object.keys(nextHooks).length > 0 + ? { ...validatedSettings, hooks: nextHooks } + : validatedSettings; + return { + settings: nextSettings, + managedHooks: cloneValue(desiredHooks), + added, + updated, + unchanged, + removed, + }; +} + +function repairManagedHooks(settings, managedHooks, options = {}) { + return mergeManagedHooks(settings, managedHooks, { + ...options, + mode: 'repair', + }); +} + +function inspectManagedHooks(settings, managedHooks) { + const validatedSettings = validateSettings(settings); + const expectedHooks = validateManagedHooks(managedHooks); + const settingsHooks = validatedSettings.hooks || {}; + const managedSubset = {}; + const matched = []; + const missing = []; + const drifted = []; + + for (const [event, expectedEntries] of Object.entries(expectedHooks)) { + const actualEntries = settingsHooks[event] || []; + const foundEntries = []; + for (const expectedEntry of expectedEntries) { + const match = assertUnambiguousMatch(actualEntries, event, expectedEntry.id); + if (!match) { + missing.push(reference(event, expectedEntry.id)); + continue; + } + + foundEntries.push(cloneValue(match.entry)); + if (isDeepStrictEqual(match.entry, expectedEntry)) { + matched.push(reference(event, expectedEntry.id)); + } else { + drifted.push({ + ...reference(event, expectedEntry.id), + expected: cloneValue(expectedEntry), + actual: cloneValue(match.entry), + }); + } + } + if (foundEntries.length > 0) { + managedSubset[event] = foundEntries; + } + } + + const ok = missing.length === 0 && drifted.length === 0; + return { + status: ok ? 'ok' : (missing.length > 0 ? 'missing' : 'drifted'), + ok, + managedHooks: managedSubset, + matched, + missing, + drifted, + }; +} + +function withoutProperty(object, omittedKey) { + return Object.fromEntries( + Object.entries(object).filter(([key]) => key !== omittedKey) + ); +} + +function uninstallManagedHooks(settings, recordedManagedHooks) { + const validatedSettings = validateSettings(settings); + const recordedHooks = validateManagedHooks(recordedManagedHooks, 'recorded managed hooks'); + const currentHooks = validatedSettings.hooks || {}; + + for (const [event, recordedEntries] of Object.entries(recordedHooks)) { + const eventEntries = currentHooks[event] || []; + for (const recordedEntry of recordedEntries) { + assertUnambiguousMatch(eventEntries, event, recordedEntry.id); + } + } + + const removed = []; + const retained = []; + const missing = []; + let nextHooks = cloneValue(currentHooks); + + for (const [event, recordedEntries] of Object.entries(recordedHooks)) { + let eventEntries = nextHooks[event] || []; + for (const recordedEntry of recordedEntries) { + const match = assertUnambiguousMatch(eventEntries, event, recordedEntry.id); + if (!match) { + missing.push(reference(event, recordedEntry.id)); + continue; + } + if (!isDeepStrictEqual(match.entry, recordedEntry)) { + retained.push({ + ...reference(event, recordedEntry.id), + expected: cloneValue(recordedEntry), + actual: cloneValue(match.entry), + reason: 'modified', + }); + continue; + } + + eventEntries = eventEntries.filter((_entry, index) => index !== match.index); + removed.push(reference(event, recordedEntry.id)); + } + nextHooks = eventEntries.length > 0 + ? { ...nextHooks, [event]: eventEntries } + : withoutProperty(nextHooks, event); + } + + nextHooks = Object.fromEntries( + Object.entries(nextHooks).filter(([, entries]) => entries.length > 0) + ); + const settingsWithoutHooks = withoutProperty(validatedSettings, 'hooks'); + const nextSettings = Object.keys(nextHooks).length > 0 + ? { ...settingsWithoutHooks, hooks: nextHooks } + : settingsWithoutHooks; + + return { + settings: nextSettings, + removed, + retained, + missing, + }; +} + +module.exports = { + acquireSettingsLock, + inspectManagedHooks, + materializeManagedHooks, + mergeManagedHooks, + parseSettings, + readSettings, + repairManagedHooks, + replacePluginRootPlaceholders, + updateSettingsAtomic, + uninstallManagedHooks, + validateManagedHooks, + validateSettings, +}; diff --git a/scripts/lib/install/hook-consent.js b/scripts/lib/install/hook-consent.js index f12bd833c..883c121d7 100644 --- a/scripts/lib/install/hook-consent.js +++ b/scripts/lib/install/hook-consent.js @@ -44,7 +44,10 @@ function normalizeOperationPath(value) { } function isHookRuntimeOperation(operation = {}) { - if (operation.moduleId === HOOK_RUNTIME_MODULE_ID) { + if ( + operation.kind === 'update-claude-settings' + || operation.moduleId === HOOK_RUNTIME_MODULE_ID + ) { return true; } diff --git a/scripts/lib/install/plan.js b/scripts/lib/install/plan.js index d98ef8f0b..08173a672 100644 --- a/scripts/lib/install/plan.js +++ b/scripts/lib/install/plan.js @@ -7,6 +7,9 @@ const { execFileSync } = require('child_process'); const { resolveInstallPlan } = require('../install-manifests'); const { getInstallTargetAdapter } = require('../install-targets/registry'); const { resolveInvocationEnvironment } = require('../invocation-environment'); +const { + materializeManagedHooks, +} = require('./claude-settings'); const EXCLUDED_GENERATED_SOURCE_SUFFIXES = ['/ecc-install-state.json', '/ecc/install-state.json']; const IGNORED_DIRECTORY_NAMES = new Set([ @@ -127,7 +130,31 @@ function readJsonObject(filePath, label) { return parsed; } +function materializeClaudeSettingsOperation(sourceRoot, operation) { + const sourcePath = path.join(sourceRoot, operation.sourceRelativePath); + if (!fs.existsSync(sourcePath)) { + return []; + } + + const hooksConfig = readJsonObject(sourcePath, operation.sourceRelativePath); + const managedHooks = materializeManagedHooks( + hooksConfig, + path.dirname(operation.destinationPath) + ); + + return [{ + ...operation, + sourcePath, + scaffoldOnly: false, + managedHooks, + }]; +} + function materializeScaffoldOperation(sourceRoot, operation) { + if (operation.kind === 'update-claude-settings') { + return materializeClaudeSettingsOperation(sourceRoot, operation); + } + if (operation.kind === 'merge-json') { return [ { diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index afac4a469..a91bfe854 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -699,7 +699,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - InvalidEventType: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo hi' }] }] + InvalidEventType: [{ id: 'test:invalid-event', matcher: 'test', hooks: [{ type: 'command', command: 'echo hi' }] }] } })); @@ -714,7 +714,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ command: 'echo hi' }] }] + PreToolUse: [{ id: 'test:missing-type', matcher: 'test', hooks: [{ command: 'echo hi' }] }] } })); @@ -729,7 +729,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command' }] }] + PreToolUse: [{ id: 'test:missing-command', matcher: 'test', hooks: [{ type: 'command' }] }] } })); @@ -744,7 +744,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', async: 'yes' }] }] + PreToolUse: [{ id: 'test:invalid-async', matcher: 'test', hooks: [{ type: 'command', command: 'echo', async: 'yes' }] }] } })); @@ -759,7 +759,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: -5 }] }] + PreToolUse: [{ id: 'test:negative-timeout', matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: -5 }] }] } })); @@ -774,7 +774,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'node -e "function {"' }] }] + PreToolUse: [{ id: 'test:invalid-inline-js', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "function {"' }] }] } })); @@ -789,7 +789,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'node -e "console.log(1+2)"' }] }] + PreToolUse: [{ id: 'test:valid-inline-js', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "console.log(1+2)"' }] }] } })); @@ -803,7 +803,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ['node', '-e', 'console.log(1)'] }] }] + PreToolUse: [{ id: 'test:array-command', matcher: 'test', hooks: [{ type: 'command', command: ['node', '-e', 'console.log(1)'] }] }] } })); @@ -829,7 +829,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test' }] + PreToolUse: [{ id: 'test:missing-hooks', matcher: 'test' }] } })); @@ -1396,7 +1396,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ' \t ' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: ' \t ' }] }] } })); @@ -1411,7 +1411,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: null }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: null }] }] } })); @@ -1426,7 +1426,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 42 }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 42 }] }] } })); @@ -1605,7 +1605,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: '' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: '' }] }] } })); @@ -1620,7 +1620,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: [] }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: [] }] }] } })); @@ -1635,7 +1635,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ['node', 123, null] }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: ['node', 123, null] }] }] } })); @@ -1650,7 +1650,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 42, command: 'echo hi' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 42, command: 'echo hi' }] }] } })); @@ -1665,7 +1665,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 'fast' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 'fast' }] }] } })); @@ -1680,7 +1680,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 0 }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo', timeout: 0 }] }] } })); @@ -1694,7 +1694,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); // data.hooks is undefined, so fallback to data itself fs.writeFileSync(hooksFile, JSON.stringify({ - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo ok' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo ok' }] }] })); const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); @@ -1796,7 +1796,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: ['node', '', 'script.js'] }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: ['node', '', 'script.js'] }] }] } })); @@ -1811,7 +1811,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo hi', timeout: -5 }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo hi', timeout: -5 }] }] } })); @@ -1826,7 +1826,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PostToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo ok', async: 'yes' }] }] + PostToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'echo ok', async: 'yes' }] }] } })); @@ -1847,7 +1847,7 @@ function runTests() { manyHooks.push({ type: 'command', command: '' }); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: manyHooks }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: manyHooks }] } })); @@ -1862,7 +1862,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'node -e "const x = 1 + 2; process.exit(0)"' }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "const x = 1 + 2; process.exit(0)"' }] }] } })); @@ -1876,9 +1876,9 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo pre' }] }], - PostToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo post' }] }], - Stop: [{ matcher: 'test', hooks: [{ type: 'command', command: 'echo stop' }] }] + PreToolUse: [{ id: 'test:multi-event-pre', matcher: 'test', hooks: [{ type: 'command', command: 'echo pre' }] }], + PostToolUse: [{ id: 'test:multi-event-post', matcher: 'test', hooks: [{ type: 'command', command: 'echo post' }] }], + Stop: [{ id: 'test:multi-event-stop', matcher: 'test', hooks: [{ type: 'command', command: 'echo stop' }] }] } })); @@ -2227,7 +2227,7 @@ function runTests() { // After unescape chain: var a = "ok"\nconsole.log(a) (real newline) — valid JS fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "var a = \\"ok\\"\\nconsole.log(a)"' }] }] } })); @@ -2243,7 +2243,7 @@ function runTests() { // After unescape this becomes: var x = { — missing closing brace fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: 'node -e "var x = {"' }] }] } })); @@ -2427,7 +2427,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ matcher: 'test', hooks: [{ type: 'command', command: { run: 'echo hi' } }] }] + PreToolUse: [{ id: 'test:fixture', matcher: 'test', hooks: [{ type: 'command', command: { run: 'echo hi' } }] }] } })); @@ -2446,7 +2446,7 @@ function runTests() { // Object format: matcher entry has hooks array but NO matcher field fs.writeFileSync(hooksFile, JSON.stringify({ hooks: { - PreToolUse: [{ hooks: [{ type: 'command', command: 'echo ok' }] }] + PreToolUse: [{ id: 'test:missing-matcher', hooks: [{ type: 'command', command: 'echo ok' }] }] } })); @@ -2554,6 +2554,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ PreToolUse: [{ + id: 'test:round72-async', matcher: 'Write', hooks: [{ type: 'command', @@ -2574,6 +2575,7 @@ function runTests() { const hooksFile = path.join(testDir, 'hooks.json'); fs.writeFileSync(hooksFile, JSON.stringify({ PostToolUse: [{ + id: 'test:round72-timeout', matcher: 'Edit', hooks: [{ type: 'command', @@ -2661,8 +2663,8 @@ function runTests() { fs.writeFileSync(hooksFile, JSON.stringify({ "$schema": "https://json.schemastore.org/claude-code-settings.json", hooks: { - PreToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo ok' }] }], - PostToolUse: [{ matcher: 'Read', hooks: [{ type: 'command', command: 'echo done' }] }] + PreToolUse: [{ id: 'test:wrapped-pre', matcher: 'Write', hooks: [{ type: 'command', command: 'echo ok' }] }], + PostToolUse: [{ id: 'test:wrapped-post', matcher: 'Read', hooks: [{ type: 'command', command: 'echo done' }] }] } })); @@ -2674,6 +2676,74 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (test('rejects wrapped matcher entry missing id', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + PreToolUse: [{ + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo missing id' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1, 'Should reject wrapped matcher entries without an id'); + assert.ok(result.stderr.includes('id'), `Should report missing id, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects wrapped matcher entry with whitespace-only id', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + PreToolUse: [{ + id: ' \t', + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo blank id' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1, 'Should reject whitespace-only matcher ids'); + assert.ok(result.stderr.includes('id'), `Should report invalid id, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects duplicate wrapped matcher ids across events', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + PreToolUse: [{ + id: 'shared:matcher', + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo pre' }] + }], + PostToolUse: [{ + id: 'shared:matcher', + matcher: 'Write', + hooks: [{ type: 'command', command: 'echo post' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1, 'Should reject matcher ids reused by another event'); + assert.ok( + result.stderr.includes("duplicate id 'shared:matcher'"), + `Should report the duplicate id, got: ${result.stderr}` + ); + assert.ok( + result.stderr.includes('PreToolUse[0]') && result.stderr.includes('PostToolUse[0]'), + `Should report both matcher locations, got: ${result.stderr}` + ); + cleanupTestDir(testDir); + })) passed++; else failed++; + // ── Round 79: validate-commands.js warnings count suffix in output ── console.log('\nRound 79: validate-commands.js (warnings count in output):'); @@ -2756,6 +2826,7 @@ function runTests() { hooks: { UserPromptSubmit: [ { + id: 'test:user-prompt-submit', hooks: [ { type: 'prompt', prompt: 'Summarize the request.' }, { type: 'agent', prompt: 'Review for security issues.', model: 'gpt-5.4' }, diff --git a/tests/lib/claude-settings.test.js b/tests/lib/claude-settings.test.js new file mode 100644 index 000000000..4fadb1c11 --- /dev/null +++ b/tests/lib/claude-settings.test.js @@ -0,0 +1,557 @@ +/** + * Focused coverage for safely managing ECC hook entries in Claude settings. + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { + inspectManagedHooks, + mergeManagedHooks, + parseSettings, + readSettings, + repairManagedHooks, + replacePluginRootPlaceholders, + uninstallManagedHooks, + updateSettingsAtomic, + validateManagedHooks, +} = require('../../scripts/lib/install/claude-settings'); + +function test(name, fn) { + try { + fn(); + console.log(` PASS ${name}`); + return true; + } catch (error) { + console.log(` FAIL ${name}`); + console.log(` Error: ${error.stack || error.message}`); + return false; + } +} + +function entry(id, command, extra = {}) { + return { + matcher: '.*', + hooks: [{ type: 'command', command }], + id, + ...extra, + }; +} + +function clone(value) { + return JSON.parse(JSON.stringify(value)); +} + +function runTests() { + console.log('\n=== Testing install/claude-settings.js ===\n'); + + let passed = 0; + let failed = 0; + + if (test('validates and clones a managed hook map without mutating it', () => { + const managed = { + SessionStart: [entry('session:start', 'node start.js')], + Stop: [entry('session:stop', 'node stop.js')], + }; + const validated = validateManagedHooks(managed); + + assert.deepStrictEqual(validated, managed); + assert.notStrictEqual(validated, managed); + assert.notStrictEqual(validated.SessionStart[0], managed.SessionStart[0]); + })) passed++; else failed++; + + if (test('strictly rejects invalid managed hook maps and globally duplicate ids', () => { + const invalidValues = [ + null, + [], + {}, + { SessionStart: [] }, + { SessionStart: {} }, + { SessionStart: [null] }, + { SessionStart: [[]] }, + { SessionStart: [{}] }, + { SessionStart: [{ id: ' ' }] }, + { BogusEvent: [entry('bad:event', 'bad')] }, + { SessionStart: [{ id: 'missing:hooks', matcher: '.*' }] }, + { SessionStart: [{ id: 'bad:command', matcher: '.*', hooks: [{ type: 'command' }] }] }, + { + SessionStart: [{ id: 'shared' }], + Stop: [{ id: 'shared' }], + }, + ]; + + for (const invalid of invalidValues) { + assert.throws(() => validateManagedHooks(invalid), /managed hooks|hook entry|unique id/i); + } + })) passed++; else failed++; + + if (test('replaces every plugin-root placeholder recursively and immutably', () => { + const source = { + SessionStart: [{ + id: 'session:start', + command: '${CLAUDE_PLUGIN_ROOT}/start.js:${CLAUDE_PLUGIN_ROOT}', + nested: ['${CLAUDE_PLUGIN_ROOT}/nested.js', 3, null], + }], + }; + const before = clone(source); + + const resolved = replacePluginRootPlaceholders(source, '/opt/ecc'); + + assert.deepStrictEqual(source, before); + assert.deepStrictEqual(resolved, { + SessionStart: [{ + id: 'session:start', + command: '/opt/ecc/start.js:/opt/ecc', + nested: ['/opt/ecc/nested.js', 3, null], + }], + }); + })) passed++; else failed++; + + if (test('parseSettings accepts an object and validates every hooks event array', () => { + assert.deepStrictEqual( + parseSettings('{"theme":"dark","hooks":{"Stop":[]}}', 'memory settings'), + { theme: 'dark', hooks: { Stop: [] } } + ); + assert.throws(() => parseSettings('{', 'memory settings'), /Failed to parse memory settings/); + assert.throws(() => parseSettings('null', 'memory settings'), /expected a JSON object/); + assert.throws(() => parseSettings('[]', 'memory settings'), /expected a JSON object/); + assert.throws( + () => parseSettings('{"hooks":{"Stop":{}}}', 'memory settings'), + /hooks\.Stop.*array/ + ); + assert.throws( + () => parseSettings('{"hooks":[]}', 'memory settings'), + /"hooks".*object/ + ); + })) passed++; else failed++; + + if (test('readSettings returns an empty object for ENOENT and rejects bad files', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-')); + try { + assert.deepStrictEqual(readSettings(path.join(tempDir, 'missing.json')), {}); + + const malformedPath = path.join(tempDir, 'malformed.json'); + fs.writeFileSync(malformedPath, '{', 'utf8'); + assert.throws(() => readSettings(malformedPath), /Failed to parse Claude settings/); + + const invalidPath = path.join(tempDir, 'invalid.json'); + fs.writeFileSync(invalidPath, '{"hooks":{"Stop":false}}', 'utf8'); + assert.throws(() => readSettings(invalidPath), /hooks\.Stop.*array/); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('readSettings propagates non-ENOENT read errors without converting them to empty settings', () => { + const denied = new Error('denied'); + denied.code = 'EACCES'; + assert.throws( + () => readSettings('/private/settings.json', { + readFileSync() { + throw denied; + }, + }), + error => error === denied + ); + })) passed++; else failed++; + + if (test('atomic settings updates retry after a concurrent change and preserve secure mode', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-atomic-')); + const settingsPath = path.join(tempDir, 'settings.json'); + let commitAttempts = 0; + try { + const result = updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, managed: true } }), + { + beforeCommit() { + commitAttempts += 1; + if (commitAttempts === 1) { + fs.writeFileSync(settingsPath, '{"theme":"concurrent"}\n', { mode: 0o600 }); + } + }, + } + ); + + assert.deepStrictEqual(result.settings, { theme: 'concurrent', managed: true }); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), result.settings); + assert.strictEqual(commitAttempts, 2); + if (process.platform !== 'win32') { + assert.strictEqual(fs.statSync(settingsPath).mode & 0o777, 0o600); + } + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('atomic settings updates recover a stale invalid lock after its lease', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-stale-lock-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + try { + fs.writeFileSync(lockPath, '', { mode: 0o600 }); + const stale = new Date(Date.now() - (10 * 60 * 1000)); + fs.utimesSync(lockPath, stale, stale); + updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, recovered: true } }) + ); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + recovered: true, + }); + assert.ok(!fs.existsSync(lockPath)); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('atomic settings updates serialize nested ECC writers and release the lock', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-lock-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + try { + updateSettingsAtomic(settingsPath, settings => { + assert.throws( + () => updateSettingsAtomic( + settingsPath, + nested => ({ settings: { ...nested, nested: true } }) + ), + /Another ECC process is updating Claude settings/ + ); + return { settings: { ...settings, outer: true } }; + }); + + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + outer: true, + }); + assert.ok(!fs.existsSync(lockPath)); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('fresh merge appends managed entries while preserving unrelated settings and hooks', () => { + const userEntry = { matcher: 'Bash', hooks: [{ type: 'command', command: 'user-hook' }] }; + const settings = { + theme: 'dark', + hooks: { + SessionStart: [userEntry], + Notification: [{ id: 'user:notification', command: 'notify' }], + }, + }; + const managed = { + SessionStart: [entry('ecc:start', 'node /opt/ecc/start.js')], + Stop: [entry('ecc:stop', 'node /opt/ecc/stop.js')], + }; + const settingsBefore = clone(settings); + const managedBefore = clone(managed); + + const result = mergeManagedHooks(settings, managed); + + assert.deepStrictEqual(settings, settingsBefore); + assert.deepStrictEqual(managed, managedBefore); + assert.deepStrictEqual(result.settings, { + theme: 'dark', + hooks: { + SessionStart: [userEntry, managed.SessionStart[0]], + Notification: settings.hooks.Notification, + Stop: managed.Stop, + }, + }); + assert.deepStrictEqual(result.added, [ + { event: 'SessionStart', id: 'ecc:start' }, + { event: 'Stop', id: 'ecc:stop' }, + ]); + assert.deepStrictEqual(result.updated, []); + })) passed++; else failed++; + + if (test('fresh merge treats a different entry with the same event and id as a conflict', () => { + const settings = { + hooks: { + Stop: [entry('ecc:stop', 'user-modified')], + }, + }; + const managed = { + Stop: [entry('ecc:stop', 'managed')], + }; + + assert.throws( + () => mergeManagedHooks(settings, managed), + /Refusing to overwrite.*Stop.*ecc:stop/ + ); + assert.deepStrictEqual(settings.hooks.Stop[0], entry('ecc:stop', 'user-modified')); + })) passed++; else failed++; + + if (test('fresh merge adopts an identical existing event and id without duplicating it', () => { + const managed = { Stop: [entry('ecc:stop', 'managed')] }; + const result = mergeManagedHooks({ hooks: clone(managed) }, managed); + + assert.deepStrictEqual(result.settings.hooks.Stop, managed.Stop); + assert.deepStrictEqual(result.unchanged, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('upgrade replaces an entry only while it still equals previous managed content', () => { + const previousManagedHooks = { Stop: [entry('ecc:stop', 'version-1')] }; + const managedHooks = { Stop: [entry('ecc:stop', 'version-2')] }; + const result = mergeManagedHooks( + { hooks: { Stop: [entry('ecc:stop', 'version-1')] } }, + managedHooks, + { previousManagedHooks } + ); + + assert.deepStrictEqual(result.settings.hooks.Stop, managedHooks.Stop); + assert.deepStrictEqual(result.updated, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('upgrade fails closed when previous managed content has drifted', () => { + const settings = { hooks: { Stop: [entry('ecc:stop', 'customer-edit')] } }; + const before = clone(settings); + + assert.throws( + () => mergeManagedHooks( + settings, + { Stop: [entry('ecc:stop', 'version-2')] }, + { previousManagedHooks: { Stop: [entry('ecc:stop', 'version-1')] } } + ), + /drifted|Refusing to overwrite/ + ); + assert.deepStrictEqual(settings, before); + })) passed++; else failed++; + + if (test('upgrade is idempotent when the desired entry is already installed', () => { + const desired = { Stop: [entry('ecc:stop', 'version-2')] }; + const result = mergeManagedHooks( + { hooks: clone(desired) }, + desired, + { previousManagedHooks: { Stop: [entry('ecc:stop', 'version-1')] } } + ); + + assert.deepStrictEqual(result.settings.hooks, desired); + assert.deepStrictEqual(result.unchanged, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('upgrade removes unchanged entries that are no longer managed', () => { + const previousManagedHooks = { + Stop: [ + entry('ecc:keep', 'version-1'), + entry('ecc:removed', 'old-command'), + ], + }; + const desired = { Stop: [entry('ecc:keep', 'version-2')] }; + const userEntry = entry('user:stop', 'keep-user'); + const result = mergeManagedHooks({ + hooks: { Stop: [userEntry, ...previousManagedHooks.Stop] }, + }, desired, { previousManagedHooks }); + + assert.deepStrictEqual(result.settings.hooks.Stop, [userEntry, desired.Stop[0]]); + assert.deepStrictEqual(result.removed, [{ event: 'Stop', id: 'ecc:removed' }]); + })) passed++; else failed++; + + if (test('upgrade removes multiple retired hooks without deleting their neighbor', () => { + const previousManagedHooks = { + Stop: [entry('ecc:a', 'a'), entry('ecc:b', 'b')], + }; + const userEntry = entry('user:c', 'keep-user'); + const result = mergeManagedHooks({ + hooks: { Stop: [...previousManagedHooks.Stop, userEntry] }, + }, { SessionStart: [entry('ecc:start', 'start')] }, { previousManagedHooks }); + + assert.deepStrictEqual(result.settings.hooks.Stop, [userEntry]); + assert.deepStrictEqual(result.removed, [ + { event: 'Stop', id: 'ecc:a' }, + { event: 'Stop', id: 'ecc:b' }, + ]); + })) passed++; else failed++; + + if (test('upgrade refuses to remove a retired entry after user drift', () => { + const previousManagedHooks = { Stop: [entry('ecc:removed', 'old-command')] }; + const settings = { hooks: { Stop: [entry('ecc:removed', 'user-edited')] } }; + + assert.throws( + () => mergeManagedHooks(settings, { SessionStart: [entry('ecc:start', 'start')] }, { + previousManagedHooks, + }), + /Refusing to remove.*ecc:removed.*drifted/ + ); + })) passed++; else failed++; + + if (test('merge fails closed when settings contains ambiguous duplicate event ids', () => { + assert.throws( + () => mergeManagedHooks( + { + hooks: { + Stop: [ + entry('ecc:stop', 'version-1'), + entry('ecc:stop', 'another-copy'), + ], + }, + }, + { Stop: [entry('ecc:stop', 'version-2')] }, + { previousManagedHooks: { Stop: [entry('ecc:stop', 'version-1')] } } + ), + /multiple.*ecc:stop/i + ); + })) passed++; else failed++; + + if (test('merge treats the same id under another event as a separate user entry', () => { + const result = mergeManagedHooks( + { hooks: { SessionStart: [entry('shared:id', 'existing')] } }, + { Stop: [entry('shared:id', 'desired')] } + ); + assert.deepStrictEqual(result.settings.hooks, { + SessionStart: [entry('shared:id', 'existing')], + Stop: [entry('shared:id', 'desired')], + }); + })) passed++; else failed++; + + if (test('repair mode overwrites a drifted same-event managed id and preserves neighbors', () => { + const userEntry = { id: 'user:hook', command: 'keep-me' }; + const result = repairManagedHooks( + { hooks: { Stop: [userEntry, entry('ecc:stop', 'drifted')] } }, + { Stop: [entry('ecc:stop', 'repaired')] } + ); + + assert.deepStrictEqual(result.settings.hooks.Stop, [ + userEntry, + entry('ecc:stop', 'repaired'), + ]); + assert.deepStrictEqual(result.updated, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('inspect reports exact, missing, and drifted managed entries plus the actual subset', () => { + const expected = { + SessionStart: [entry('ecc:start', 'start')], + Stop: [ + entry('ecc:stop', 'expected'), + entry('ecc:missing', 'missing'), + ], + }; + const actualStart = entry('ecc:start', 'start'); + const actualDrift = entry('ecc:stop', 'changed'); + const result = inspectManagedHooks({ + hooks: { + SessionStart: [actualStart, { id: 'user:start', command: 'user' }], + Stop: [actualDrift], + }, + }, expected); + + assert.strictEqual(result.status, 'missing'); + assert.deepStrictEqual(result.matched, [{ event: 'SessionStart', id: 'ecc:start' }]); + assert.deepStrictEqual(result.missing, [{ event: 'Stop', id: 'ecc:missing' }]); + assert.deepStrictEqual(result.drifted, [{ + event: 'Stop', + id: 'ecc:stop', + expected: expected.Stop[0], + actual: actualDrift, + }]); + assert.deepStrictEqual(result.managedHooks, { + SessionStart: [actualStart], + Stop: [actualDrift], + }); + })) passed++; else failed++; + + if (test('inspect fails closed on duplicate matching ids in one settings event', () => { + assert.throws( + () => inspectManagedHooks( + { hooks: { Stop: [entry('ecc:stop', 'a'), entry('ecc:stop', 'b')] } }, + { Stop: [entry('ecc:stop', 'expected')] } + ), + /multiple.*ecc:stop/i + ); + })) passed++; else failed++; + + if (test('inspect and uninstall key ownership by event plus id', () => { + const recorded = { Stop: [entry('ecc:stop', 'managed')] }; + const moved = { hooks: { SessionStart: [entry('ecc:stop', 'managed')] } }; + + const inspection = inspectManagedHooks(moved, recorded); + assert.strictEqual(inspection.status, 'missing'); + assert.deepStrictEqual(inspection.missing, [{ event: 'Stop', id: 'ecc:stop' }]); + + const uninstall = uninstallManagedHooks(moved, recorded); + assert.deepStrictEqual(uninstall.settings, moved); + assert.deepStrictEqual(uninstall.missing, [{ event: 'Stop', id: 'ecc:stop' }]); + })) passed++; else failed++; + + if (test('uninstall removes exact recorded entries, retains drift, and cleans empty events', () => { + const recorded = { + SessionStart: [entry('ecc:start', 'start')], + Stop: [entry('ecc:stop', 'recorded')], + Notification: [entry('ecc:notify', 'notify')], + }; + const userEntry = { matcher: 'Bash', hooks: [{ type: 'command', command: 'user' }] }; + const driftedStop = entry('ecc:stop', 'customer-edit'); + const settings = { + theme: 'dark', + hooks: { + SessionStart: [recorded.SessionStart[0]], + Stop: [userEntry, driftedStop], + Notification: [recorded.Notification[0]], + }, + }; + const before = clone(settings); + + const result = uninstallManagedHooks(settings, recorded); + + assert.deepStrictEqual(settings, before); + assert.deepStrictEqual(result.settings, { + theme: 'dark', + hooks: { + Stop: [userEntry, driftedStop], + }, + }); + assert.deepStrictEqual(result.removed, [ + { event: 'SessionStart', id: 'ecc:start' }, + { event: 'Notification', id: 'ecc:notify' }, + ]); + assert.deepStrictEqual(result.retained, [{ + event: 'Stop', + id: 'ecc:stop', + expected: recorded.Stop[0], + actual: driftedStop, + reason: 'modified', + }]); + })) passed++; else failed++; + + if (test('uninstall removes consecutive managed hooks without deleting a user neighbor', () => { + const recorded = { Stop: [entry('ecc:a', 'a'), entry('ecc:b', 'b')] }; + const userEntry = entry('user:c', 'keep-user'); + const result = uninstallManagedHooks({ + hooks: { Stop: [...recorded.Stop, userEntry] }, + }, recorded); + + assert.deepStrictEqual(result.settings.hooks.Stop, [userEntry]); + assert.deepStrictEqual(result.removed, [ + { event: 'Stop', id: 'ecc:a' }, + { event: 'Stop', id: 'ecc:b' }, + ]); + })) passed++; else failed++; + + if (test('uninstall removes hooks entirely after the final managed event is emptied', () => { + const recorded = { Stop: [entry('ecc:stop', 'recorded')] }; + const result = uninstallManagedHooks({ theme: 'dark', hooks: clone(recorded) }, recorded); + + assert.deepStrictEqual(result.settings, { theme: 'dark' }); + assert.deepStrictEqual(result.removed, [{ event: 'Stop', id: 'ecc:stop' }]); + assert.deepStrictEqual(result.retained, []); + })) passed++; else failed++; + + if (test('all settings transforms reject non-array hook events before changing data', () => { + const settings = { hooks: { Stop: 'invalid' } }; + const managed = { Stop: [entry('ecc:stop', 'expected')] }; + + assert.throws(() => mergeManagedHooks(settings, managed), /hooks\.Stop.*array/); + assert.throws(() => repairManagedHooks(settings, managed), /hooks\.Stop.*array/); + assert.throws(() => inspectManagedHooks(settings, managed), /hooks\.Stop.*array/); + assert.throws(() => uninstallManagedHooks(settings, managed), /hooks\.Stop.*array/); + })) passed++; else failed++; + + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); + process.exit(failed > 0 ? 1 : 0); +} + +runTests(); diff --git a/tests/lib/hook-consent.test.js b/tests/lib/hook-consent.test.js index 716a91b3a..2f44361e2 100644 --- a/tests/lib/hook-consent.test.js +++ b/tests/lib/hook-consent.test.js @@ -27,10 +27,23 @@ function test(name, fn) { } function buildHookPlan() { + const managedHooks = { + SessionStart: [{ + id: 'session:start', + matcher: '.*', + hooks: [{ type: 'command', command: 'node /target/scripts/hooks/session-start.js' }], + }], + }; return { operations: [ { kind: 'copy-file', moduleId: 'rules-core', sourceRelativePath: 'rules/common.md', destinationPath: '/target/rules/common.md' }, - { kind: 'copy-file', moduleId: 'hooks-runtime', sourceRelativePath: 'hooks/hooks.json', destinationPath: '/target/hooks/hooks.json' }, + { + kind: 'update-claude-settings', + moduleId: 'hooks-runtime', + sourceRelativePath: 'hooks/hooks.json', + destinationPath: '/target/settings.json', + managedHooks, + }, { kind: 'copy-file', moduleId: 'hooks-runtime', sourceRelativePath: 'scripts/hooks/session-start.js', destinationPath: '/target/scripts/hooks/session-start.js' }, ], selectedModuleIds: ['rules-core', 'hooks-runtime'], @@ -46,7 +59,13 @@ function buildHookPlan() { }, operations: [ { kind: 'copy-file', moduleId: 'rules-core', sourceRelativePath: 'rules/common.md', destinationPath: '/target/rules/common.md' }, - { kind: 'copy-file', moduleId: 'hooks-runtime', sourceRelativePath: 'hooks/hooks.json', destinationPath: '/target/hooks/hooks.json' }, + { + kind: 'update-claude-settings', + moduleId: 'hooks-runtime', + sourceRelativePath: 'hooks/hooks.json', + destinationPath: '/target/settings.json', + managedHooks, + }, ], resolution: { selectedModules: ['rules-core', 'hooks-runtime'], skippedModules: [] }, }, @@ -69,6 +88,7 @@ function runTests() { if (test('matches hook runtime operations by module id and source path', () => { assert.strictEqual(isHookRuntimeOperation({ moduleId: 'hooks-runtime' }), true); + assert.strictEqual(isHookRuntimeOperation({ kind: 'update-claude-settings' }), true); assert.strictEqual(isHookRuntimeOperation({ sourceRelativePath: 'hooks/hooks.json' }), true); assert.strictEqual(isHookRuntimeOperation({ sourceRelativePath: '.cursor/hooks.json' }), true); assert.strictEqual(isHookRuntimeOperation({ destinationPath: '/root/.claude/hooks/hooks.json' }), true); diff --git a/tests/lib/install-executor.test.js b/tests/lib/install-executor.test.js index 4a65ce5ef..d0acad15a 100644 --- a/tests/lib/install-executor.test.js +++ b/tests/lib/install-executor.test.js @@ -9,6 +9,7 @@ const crypto = require('crypto'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { spawnSync } = require('child_process'); const { applyInstallPlan, @@ -19,6 +20,7 @@ const { listAvailableLanguages, } = require('../../scripts/lib/install-executor'); const { applyInstallPlan: applyInstallPlanDirect } = require('../../scripts/lib/install/apply'); +const { withHookConsent } = require('../../scripts/lib/install/hook-consent'); const REPO_ROOT = path.resolve(__dirname, '..', '..'); @@ -166,6 +168,98 @@ function runTests() { } })) passed++; else failed++; + if (test('Claude settings write preserves unrelated changes made after preflight', () => { + const tempDir = createTempDir('install-executor-settings-race-'); + try { + const homeDir = path.join(tempDir, 'home'); + const projectRoot = path.join(tempDir, 'project'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + const rawPlan = createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + moduleIds: ['hooks-runtime'], + }); + const plan = { + ...rawPlan, + hookConsent: 'enabled', + statePreview: { + ...rawPlan.statePreview, + request: { ...rawPlan.statePreview.request, hookConsent: 'enabled' }, + }, + }; + const settingsPath = path.join(homeDir, '.claude', 'settings.json'); + + applyInstallPlanDirect(plan, { + beforeOperationWrite({ operation }) { + if (operation.kind === 'update-claude-settings') { + fs.writeFileSync(settingsPath, '{"theme":"added-after-preflight"}\n'); + } + }, + }); + + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + assert.strictEqual(settings.theme, 'added-after-preflight'); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + } finally { + cleanup(tempDir); + } + })) passed++; else failed++; + + if (test('failed hook disable checkpoints the previous enabled consent state', () => { + const tempDir = createTempDir('install-executor-disable-failure-'); + try { + const homeDir = path.join(tempDir, 'home'); + const projectRoot = path.join(tempDir, 'project'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + const enabledPlan = withHookConsent(createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + profileId: 'core', + }), 'enabled'); + applyInstallPlanDirect(enabledPlan); + + const declinedPlan = withHookConsent(createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + profileId: 'core', + }), 'declined'); + let injectedFailure = false; + assert.throws( + () => applyInstallPlanDirect(declinedPlan, { + beforeOperationWrite({ operation }) { + if (!injectedFailure && operation.kind === 'copy-file') { + injectedFailure = true; + throw new Error('injected copy failure'); + } + }, + }), + /injected copy failure/ + ); + + const state = JSON.parse(fs.readFileSync(declinedPlan.installStatePath, 'utf8')); + assert.strictEqual(state.request.hookConsent, 'enabled'); + assert.ok(state.resolution.selectedModules.includes('hooks-runtime')); + assert.ok(state.operations.some(operation => ( + operation.kind === 'update-claude-settings' + ))); + const settings = JSON.parse(fs.readFileSync( + path.join(homeDir, '.claude', 'settings.json'), + 'utf8' + )); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + } finally { + cleanup(tempDir); + } + })) passed++; else failed++; + if (test('rejects unknown legacy install targets before planning', () => { assert.throws( () => createLegacyInstallPlan({ target: 'not-a-target' }), @@ -400,6 +494,80 @@ function runTests() { } })) passed++; else failed++; + if (test('plans one resolved Claude settings hook registration for home and project targets', () => { + const tempDir = createTempDir('install-executor-claude-hooks-'); + try { + for (const target of ['claude', 'claude-project']) { + const homeDir = path.join(tempDir, `${target} home "quoted" $dollar %percent%`); + const projectRoot = path.join(tempDir, `${target} project "quoted" $dollar %percent%`); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + + const plan = createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target, + moduleIds: ['hooks-runtime'], + }); + const expectedRoot = target === 'claude' + ? path.join(homeDir, '.claude') + : path.join(projectRoot, '.claude'); + const settingsOperations = plan.operations.filter(operation => ( + operation.kind === 'update-claude-settings' + )); + + assert.strictEqual(settingsOperations.length, 1, `${target} should plan one settings update`); + const operation = settingsOperations[0]; + assert.strictEqual(operation.moduleId, 'hooks-runtime'); + assert.strictEqual( + operation.sourceRelativePath.split(path.sep).join('/'), + 'hooks/hooks.json' + ); + assert.strictEqual(operation.destinationPath, path.join(expectedRoot, 'settings.json')); + assert.ok(operation.managedHooks); + assert.ok(operation.managedHooks.SessionStart.some(entry => ( + entry.id === 'session:start' + ))); + const commands = Object.values(operation.managedHooks) + .flat() + .flatMap(entry => entry.hooks || []) + .map(hook => hook.command) + .filter(command => typeof command === 'string'); + const encodedRoot = Buffer.from(expectedRoot, 'utf8').toString('base64'); + assert.ok(commands.some(command => command.includes(encodedRoot))); + assert.ok(commands.every(command => !command.includes(expectedRoot))); + assert.ok( + commands.every(command => !command.includes('var e=process.env.CLAUDE_PLUGIN_ROOT;')), + `${target} commands should not depend on an unset CLAUDE_PLUGIN_ROOT` + ); + if (process.platform !== 'win32') { + for (const command of commands) { + const syntaxCheck = spawnSync('/bin/sh', ['-n', '-c', command], { + encoding: 'utf8', + }); + assert.strictEqual( + syntaxCheck.status, + 0, + `${target} hook command should remain shell-safe: ${syntaxCheck.stderr}` + ); + } + } + assert.ok(!plan.operations.some(candidate => ( + candidate.kind === 'copy-file' + && candidate.sourceRelativePath.split(path.sep).join('/') === 'hooks/hooks.json' + ))); + + const stateOperation = plan.statePreview.operations.find(candidate => ( + candidate.kind === 'update-claude-settings' + )); + assert.deepStrictEqual(stateOperation.managedHooks, operation.managedHooks); + } + } finally { + cleanup(tempDir); + } + })) passed++; else failed++; + if (test('creates legacy compatibility manifest plans from language selections', () => { const projectRoot = createTempDir('install-executor-project-'); const homeDir = createTempDir('install-executor-home-'); diff --git a/tests/lib/install-lifecycle.test.js b/tests/lib/install-lifecycle.test.js index 51d39f9e1..4e45c9597 100644 --- a/tests/lib/install-lifecycle.test.js +++ b/tests/lib/install-lifecycle.test.js @@ -23,6 +23,7 @@ const { readInstallState, writeInstallState, } = require('../../scripts/lib/install-state'); +const { materializeManagedHooks } = require('../../scripts/lib/install/claude-settings'); const REPO_ROOT = path.join(__dirname, '..', '..'); const CURRENT_PACKAGE_VERSION = JSON.parse( @@ -52,6 +53,10 @@ function cleanup(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } +function formatJson(value) { + return `${JSON.stringify(value, null, 2)}\n`; +} + function writeState(filePath, options) { const state = createInstallState(options); writeInstallState(filePath, state); @@ -100,6 +105,61 @@ function writeCursorState(projectRoot, overrides = {}) { }; } +function writeClaudeState(homeDir, overrides = {}) { + const targetRoot = overrides.targetRoot || path.join(homeDir, '.claude'); + const installStatePath = overrides.installStatePath + || path.join(targetRoot, 'ecc', 'install-state.json'); + const options = { + adapter: { id: 'claude-home', target: 'claude', kind: 'home' }, + targetRoot, + installStatePath, + request: { + profile: null, + modules: [], + includeComponents: [], + excludeComponents: [], + legacyLanguages: [], + legacyMode: true, + hookConsent: 'enabled', + ...(overrides.request || {}), + }, + resolution: { + selectedModules: ['legacy-claude-install'], + skippedModules: [], + ...(overrides.resolution || {}), + }, + operations: overrides.operations || [], + source: { + repoVersion: CURRENT_PACKAGE_VERSION, + repoCommit: 'abc123', + manifestVersion: CURRENT_MANIFEST_VERSION, + ...(overrides.source || {}), + }, + }; + + writeState(installStatePath, options); + return { + targetRoot, + installStatePath, + state: options, + }; +} + +function managedHookEntry(id, command) { + return { + id, + matcher: '.*', + hooks: [{ type: 'command', command }], + }; +} + +function currentManagedHooks(targetRoot) { + return materializeManagedHooks( + JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'hooks', 'hooks.json'), 'utf8')), + targetRoot + ); +} + function createOpencodeStateOptions(homeDir, overrides = {}) { const targetRoot = overrides.targetRoot || path.join(homeDir, '.config', 'opencode'); const installStatePath = overrides.installStatePath || path.join(targetRoot, 'ecc-install-state.json'); @@ -171,8 +231,10 @@ function withTemporarilyMovedPath(filePath, callback) { function managedOperation(kind, destinationPath, overrides = {}) { const operation = { kind, - moduleId: 'test-module', - sourceRelativePath: 'rules/common/coding-style.md', + moduleId: kind === 'update-claude-settings' ? 'hooks-runtime' : 'test-module', + sourceRelativePath: kind === 'update-claude-settings' + ? 'hooks/hooks.json' + : 'rules/common/coding-style.md', destinationPath, strategy: kind, ownership: 'managed', @@ -3231,6 +3293,417 @@ function runTests() { } })) passed++; else failed++; + if (test('doctor inspects update-claude-settings hooks by event and id', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = currentManagedHooks(targetRoot); + const stopEntry = managedHooks.Stop[0]; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + Stop: [ + { id: 'user:stop', matcher: 'Bash', hooks: [{ type: 'command', command: 'user' }] }, + ...managedHooks.Stop, + ], + ...Object.fromEntries(Object.entries(managedHooks).filter(([event]) => event !== 'Stop')), + }, + })); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + let report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(report.results[0].status, 'ok'); + + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + ...managedHooks, + Stop: [ + { id: 'user:stop', matcher: 'Bash', hooks: [{ type: 'command', command: 'user' }] }, + { ...stopEntry, description: 'drifted' }, + ...managedHooks.Stop.slice(1), + ], + }, + })); + report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(report.results[0].status, 'warning'); + assert.ok(report.results[0].issues.some(issue => issue.code === 'drifted-managed-files')); + + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + ...managedHooks, + Stop: managedHooks.Stop.filter(entry => entry.id !== stopEntry.id), + }, + })); + report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(report.results[0].status, 'error'); + assert.ok(report.results[0].issues.some(issue => issue.code === 'missing-managed-files')); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('repair restores managed Claude hooks while preserving user settings and hooks', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const userHook = { + id: 'user:stop', + matcher: 'Bash', + hooks: [{ type: 'command', command: 'user-command' }], + }; + const managedHooks = currentManagedHooks(targetRoot); + const stopEntry = managedHooks.Stop[0]; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + ...managedHooks, + Stop: [ + userHook, + { ...stopEntry, description: 'drifted' }, + ...managedHooks.Stop.slice(1), + ], + }, + })); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'repaired'); + assert.ok(result.results[0].repairedPaths.includes(settingsPath)); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + hooks: { + ...managedHooks, + Stop: [userHook, ...managedHooks.Stop], + }, + }); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('repair creates missing Claude settings with private permissions', () => { + if (process.platform === 'win32') return; + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = currentManagedHooks(targetRoot); + fs.mkdirSync(targetRoot, { recursive: true }); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'repaired'); + assert.strictEqual(fs.statSync(settingsPath).mode & 0o777, 0o600); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')).hooks, managedHooks); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('repair removes retired managed hooks using the recorded ownership snapshot', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const currentHooks = currentManagedHooks(targetRoot); + const retiredHook = managedHookEntry('ecc:retired', 'node retired.js'); + const recordedHooks = { + ...currentHooks, + Stop: [...currentHooks.Stop, retiredHook], + }; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: recordedHooks, + })); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + managedHooks: recordedHooks, + }), + ], + }); + + const result = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'repaired'); + const repaired = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + assert.ok(!repaired.hooks.Stop.some(entry => entry.id === 'ecc:retired')); + assert.deepStrictEqual(repaired.hooks, currentHooks); + const state = readInstallState(path.join(targetRoot, 'ecc', 'install-state.json')); + const settingsOperation = state.operations.find(operation => ( + operation.kind === 'update-claude-settings' + )); + assert.deepStrictEqual(settingsOperation.managedHooks, currentHooks); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('uninstall removes only unchanged managed Claude hooks and reports drift as partial', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = { + SessionStart: [managedHookEntry('ecc:start', 'node managed-start.js')], + Stop: [managedHookEntry('ecc:stop', 'node managed-stop.js')], + }; + const userHook = { + id: 'user:stop', + matcher: 'Bash', + hooks: [{ type: 'command', command: 'user-command' }], + }; + const driftedHook = managedHookEntry('ecc:stop', 'node user-edited-stop.js'); + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: { + SessionStart: managedHooks.SessionStart, + Stop: [userHook, driftedHook], + }, + })); + const { installStatePath } = writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = uninstallInstalledStates({ + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'partial'); + assert.deepStrictEqual(result.results[0].retainedPaths, [settingsPath]); + assert.ok(fs.existsSync(installStatePath)); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + hooks: { + Stop: [userHook, driftedHook], + }, + }); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('uninstall clears empty hook containers but preserves unrelated Claude settings', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = { + Stop: [managedHookEntry('ecc:stop', 'node managed-stop.js')], + }; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, formatJson({ + theme: 'dark', + hooks: managedHooks, + })); + const { installStatePath } = writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const result = uninstallInstalledStates({ + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(result.results[0].status, 'uninstalled'); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { + theme: 'dark', + }); + assert.ok(!fs.existsSync(installStatePath)); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('Claude settings lifecycle refuses a final-symlink destination', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const victimPath = path.join(targetRoot, 'victim.json'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = { + Stop: [managedHookEntry('ecc:stop', 'node managed-stop.js')], + }; + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(victimPath, formatJson({ sentinel: true, hooks: managedHooks })); + try { + fs.symlinkSync(victimPath, settingsPath, 'file'); + } catch { + console.log(' (file symlink unsupported on this platform; skipping)'); + return; + } + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const doctor = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + const repair = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + const uninstall = uninstallInstalledStates({ + homeDir, + projectRoot, + targets: ['claude'], + }); + + assert.strictEqual(doctor.results[0].status, 'error'); + assert.ok(doctor.results[0].issues.some(issue => ( + issue.code === 'unsafe-managed-destination' + || issue.code === 'invalid-install-state' + ))); + assert.strictEqual(repair.results[0].status, 'error'); + assert.match(repair.results[0].error, /final symlink/); + assert.strictEqual(uninstall.results[0].status, 'error'); + assert.match(uninstall.results[0].error, /final symlink/); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(victimPath, 'utf8')), { + sentinel: true, + hooks: managedHooks, + }); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('Claude settings lifecycle refuses a non-canonical settings destination', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const destinationPath = path.join(targetRoot, 'settings.local.json'); + const managedHooks = currentManagedHooks(targetRoot); + fs.mkdirSync(targetRoot, { recursive: true }); + assert.throws( + () => writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', destinationPath, { + managedHooks, + }), + ], + }), + /canonical Claude settings path/ + ); + assert.ok(!fs.existsSync(destinationPath)); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); } diff --git a/tests/lib/install-state.test.js b/tests/lib/install-state.test.js index 8baa6c3e5..ba8effaea 100644 --- a/tests/lib/install-state.test.js +++ b/tests/lib/install-state.test.js @@ -84,6 +84,115 @@ function runTests() { assert.strictEqual(state.operations.length, 1); })) passed++; else failed++; + if (test('validates managed hook metadata for Claude settings operations', () => { + const baseOptions = { + adapter: { id: 'claude-home', target: 'claude', kind: 'home' }, + targetRoot: '/home/test/.claude', + installStatePath: '/home/test/.claude/ecc/install-state.json', + request: { + profile: 'core', + modules: ['hooks-runtime'], + includeComponents: [], + excludeComponents: [], + legacyLanguages: [], + legacyMode: false, + hookConsent: 'enabled', + }, + resolution: { selectedModules: ['hooks-runtime'], skippedModules: [] }, + source: { repoVersion: CURRENT_PACKAGE_VERSION, repoCommit: 'abc123', manifestVersion: 1 }, + }; + const operation = { + kind: 'update-claude-settings', + moduleId: 'hooks-runtime', + sourceRelativePath: 'hooks/hooks.json', + destinationPath: '/home/test/.claude/settings.json', + strategy: 'merge-hook-ids', + ownership: 'managed', + scaffoldOnly: false, + managedHooks: { + SessionStart: [{ + id: 'session:start', + matcher: '.*', + hooks: [{ type: 'command', command: 'node start.js' }], + }], + }, + }; + + assert.doesNotThrow(() => createInstallState({ ...baseOptions, operations: [operation] })); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ ...operation, moduleId: 'not-hooks-runtime' }], + }), + /moduleId.*hooks-runtime/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ ...operation, sourceRelativePath: 'attacker.json' }], + }), + /sourceRelativePath.*hooks\/hooks\.json/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + destinationPath: '/home/test/.claude/settings.local.json', + }], + }), + /destinationPath.*canonical Claude settings path/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + adapter: { id: 'cursor-project', target: 'cursor', kind: 'project' }, + targetRoot: '/repo/.cursor', + installStatePath: '/repo/.cursor/ecc-install-state.json', + operations: [{ + ...operation, + destinationPath: '/repo/.cursor/settings.json', + }], + }), + /only valid for Claude targets/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + managedHooks: { + SessionStart: [{ + matcher: '.*', + hooks: [{ type: 'command', command: 'node start.js' }], + }], + }, + }], + }), + /managedHooks.*non-empty unique id/ + ); + assert.throws( + () => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + managedHooks: { + SessionStart: [{ + id: 'duplicate', + matcher: '.*', + hooks: [{ type: 'command', command: 'node start.js' }], + }], + Stop: [{ + id: 'duplicate', + hooks: [{ type: 'command', command: 'node stop.js' }], + }], + }, + }], + }), + /managedHooks.*globally unique id/ + ); + })) passed++; else failed++; + if (test('writes and reads install-state from disk', () => { const testDir = createTestDir(); const statePath = path.join(testDir, 'ecc-install-state.json'); diff --git a/tests/scripts/install-apply.test.js b/tests/scripts/install-apply.test.js index 0931d5c0a..270339cb9 100644 --- a/tests/scripts/install-apply.test.js +++ b/tests/scripts/install-apply.test.js @@ -115,6 +115,31 @@ function runTests() { assert.ok(result.stdout.includes('--modules ')); })) passed++; else failed++; + if (test('Claude hook dry-run validates settings without mutating malformed input', () => { + const homeDir = createTempDir('install-apply-home-'); + const projectDir = createTempDir('install-apply-project-'); + const claudeRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(claudeRoot, 'settings.json'); + + try { + fs.mkdirSync(claudeRoot, { recursive: true }); + fs.writeFileSync(settingsPath, '{ malformed\n'); + + const result = run( + ['--profile', 'core', '--enable-hooks', '--dry-run', '--json'], + { cwd: projectDir, homeDir } + ); + + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /Failed to parse Claude settings/); + assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ malformed\n'); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); + } finally { + cleanup(homeDir); + cleanup(projectDir); + } + })) passed++; else failed++; + if (test('guided dispatcher reports sanitized load and rejection failures', () => { for (const failureMode of ['load', 'reject']) { const result = runWithGuidedDispatcherFailure(failureMode); @@ -487,6 +512,19 @@ function runTests() { const parsed = JSON.parse(result.stdout); assert.strictEqual(parsed.dryRun, true); assert.ok(parsed.plan.selectedModuleIds.includes('workflow-quality')); + const settingsOperations = parsed.plan.operations.filter(operation => ( + operation.kind === 'update-claude-settings' + )); + assert.strictEqual(settingsOperations.length, 1); + assert.strictEqual( + settingsOperations[0].destinationPath, + path.join(homeDir, '.claude', 'settings.json') + ); + assert.ok(settingsOperations[0].managedHooks.SessionStart); + assert.ok(!parsed.plan.operations.some(operation => ( + operation.kind === 'copy-file' + && String(operation.sourceRelativePath || '').replace(/\\/g, '/') === 'hooks/hooks.json' + ))); assert.ok( parsed.plan.operations.some(operation => ( String(operation.sourceRelativePath || '').replace(/\\/g, '/').startsWith('skills/delivery-gate/') @@ -532,7 +570,8 @@ function runTests() { assert.ok(fs.existsSync(path.join(claudeRoot, 'rules', 'ecc', 'common', 'coding-style.md'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'agents', 'architect.md'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'commands', 'plan.md'))); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json'))); + assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json'))); + assert.ok(readJson(path.join(claudeRoot, 'settings.json')).hooks.SessionStart); assert.ok(fs.existsSync(path.join(claudeRoot, 'scripts', 'hooks', 'session-end.js'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'scripts', 'lib', 'session-manager.js'))); assert.ok(fs.existsSync(path.join(claudeRoot, 'plugin.json'))); @@ -747,7 +786,7 @@ function runTests() { assert.ok(result.stderr.includes('Unknown install module: ghost-module')); })) passed++; else failed++; - if (test('installs claude hooks and defaults commit attribution off', () => { + if (test('registers Claude hooks in settings and defaults commit attribution off', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -756,58 +795,87 @@ function runTests() { assert.strictEqual(result.code, 0, result.stderr); const claudeRoot = path.join(homeDir, '.claude'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should be copied'); - assert.deepStrictEqual( - readJson(path.join(claudeRoot, 'settings.json')), - { includeCoAuthoredBy: false } + assert.strictEqual( + fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), + false, + 'hooks.json should not be copied for Claude targets' ); + const settings = readJson(path.join(claudeRoot, 'settings.json')); + assert.strictEqual(settings.includeCoAuthoredBy, false); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + + const state = readJson(path.join(claudeRoot, 'ecc', 'install-state.json')); + const settingsOperation = state.operations.find(operation => ( + operation.kind === 'update-claude-settings' + )); + assert.ok(settingsOperation, 'state should record the settings update operation'); + assert.deepStrictEqual(settingsOperation.managedHooks, settings.hooks); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('installs claude hooks with the safe plugin bootstrap contract', () => { - const homeDir = createTempDir('install-apply-home-'); - const projectDir = createTempDir('install-apply-project-'); + if (test('resolves Claude home and project hook commands to their installed roots', () => { + for (const target of ['claude', 'claude-project']) { + const homeDir = createTempDir(`install-apply-${target}-home-`); + const projectDir = createTempDir(`install-apply-${target}-project-`); - try { - const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); - assert.strictEqual(result.code, 0, result.stderr); + try { + const result = run( + ['--target', target, '--profile', 'core', '--enable-hooks'], + { cwd: projectDir, homeDir } + ); + assert.strictEqual(result.code, 0, result.stderr); - const claudeRoot = path.join(homeDir, '.claude'); - const installedHooks = readJson(path.join(claudeRoot, 'hooks', 'hooks.json')); + const claudeRoot = target === 'claude' + ? path.join(homeDir, '.claude') + : path.join(projectDir, '.claude'); + const settings = readJson(path.join(claudeRoot, 'settings.json')); + const state = readJson(path.join(claudeRoot, 'ecc', 'install-state.json')); + const installedRoot = state.target.root; + assert.strictEqual(fs.realpathSync(installedRoot), fs.realpathSync(claudeRoot)); + const installedBashDispatcherEntry = settings.hooks.PreToolUse.find( + entry => entry.id === 'pre:bash:dispatcher' + ); + assert.ok(installedBashDispatcherEntry); + const command = installedBashDispatcherEntry.hooks[0].command; + assert.ok(command.startsWith('node -e ')); + assert.ok(command.includes('plugin-hook-bootstrap.js')); + assert.ok(command.includes('pre-bash-dispatcher.js')); + assert.ok( + command.includes(Buffer.from(installedRoot, 'utf8').toString('base64')), + `${target} command should encode its absolute root without shell interpolation` + ); + assert.ok(!command.includes(claudeRoot)); + assert.ok(!command.includes('var e=process.env.CLAUDE_PLUGIN_ROOT;')); + assert.ok(!command.includes('${CLAUDE_PLUGIN_ROOT}')); - const installedBashDispatcherEntry = installedHooks.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:dispatcher'); - assert.ok(installedBashDispatcherEntry, 'hooks/hooks.json should include the consolidated Bash dispatcher hook'); - assert.strictEqual(typeof installedBashDispatcherEntry.hooks[0].command, 'string', 'hooks/hooks.json should install string-form commands for Claude Code schema compatibility'); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.startsWith('node -e '), - 'hooks/hooks.json should use the inline node bootstrap contract' - ); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.includes('plugin-hook-bootstrap.js'), - 'hooks/hooks.json should route plugin-managed hooks through the shared bootstrap' - ); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.includes('CLAUDE_PLUGIN_ROOT'), - 'hooks/hooks.json should still consult CLAUDE_PLUGIN_ROOT for runtime resolution' - ); - assert.ok( - installedBashDispatcherEntry.hooks[0].command.includes('pre-bash-dispatcher.js'), - 'hooks/hooks.json should point the Bash preflight contract at the consolidated dispatcher' - ); - assert.ok( - !installedBashDispatcherEntry.hooks[0].command.includes('\\"'), - 'hooks/hooks.json should avoid escaped double quotes that break Windows Git Bash parsing' - ); - assert.ok( - !installedBashDispatcherEntry.hooks[0].command.includes('${CLAUDE_PLUGIN_ROOT}'), - 'hooks/hooks.json should not retain raw CLAUDE_PLUGIN_ROOT shell placeholders after install' - ); - } finally { - cleanup(homeDir); - cleanup(projectDir); + const smokeEntry = settings.hooks.PreToolUse.find( + entry => entry.id === 'pre:write:doc-file-warning' + ); + const smokeResult = spawnSync(smokeEntry.hooks[0].command, { + input: JSON.stringify({ + hook_event_name: 'PreToolUse', + tool_name: 'Write', + tool_input: { file_path: 'README.md' }, + }), + encoding: 'utf8', + cwd: projectDir, + env: { + ...process.env, + HOME: homeDir, + USERPROFILE: homeDir, + ECC_DISABLED_HOOKS: 'pre:write:doc-file-warning', + }, + shell: true, + timeout: DEFAULT_INSTALL_APPLY_TIMEOUT_MS, + }); + assert.strictEqual(smokeResult.status, 0, smokeResult.stderr); + } finally { + cleanup(homeDir); + cleanup(projectDir); + } } })) passed++; else failed++; @@ -840,12 +908,16 @@ function runTests() { assert.deepStrictEqual( settings.hooks.UserPromptSubmit, [{ matcher: '*', hooks: [{ type: 'command', command: 'echo custom-submit' }] }], - 'existing hooks should be left untouched' + 'unrelated existing hooks should be preserved' ); assert.deepStrictEqual( - settings.hooks.PreToolUse, - [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }], - 'managed Claude hooks should not be injected into settings.json' + settings.hooks.PreToolUse[0], + { matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }, + 'existing event entries should retain their order and content' + ); + assert.ok( + settings.hooks.PreToolUse.some(entry => entry.id === 'pre:bash:dispatcher'), + 'managed Claude hooks should be registered alongside user hooks' ); } finally { cleanup(homeDir); @@ -927,7 +999,7 @@ function runTests() { } })) passed++; else failed++; - if (test('reinstall keeps commit attribution disabled when only managed hooks are installed', () => { + if (test('reinstall is idempotent for managed hooks and keeps commit attribution disabled', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -938,17 +1010,17 @@ function runTests() { const secondInstall = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); assert.strictEqual(secondInstall.code, 0, secondInstall.stderr); - assert.deepStrictEqual( - readJson(path.join(homeDir, '.claude', 'settings.json')), - { includeCoAuthoredBy: false } - ); + const settings = readJson(path.join(homeDir, '.claude', 'settings.json')); + assert.strictEqual(settings.includeCoAuthoredBy, false); + const ids = Object.values(settings.hooks).flat().map(entry => entry.id); + assert.strictEqual(ids.length, new Set(ids).size, 'managed hook IDs should not duplicate'); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('reinstall leaves pre-existing hook-based settings.json untouched apart from co-author preference', () => { + if (test('reinstall preserves pre-existing hook entries while registering managed hooks', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -967,10 +1039,9 @@ function runTests() { assert.strictEqual(secondInstall.code, 0, secondInstall.stderr); const afterSecondInstall = readJson(settingsPath); - assert.deepStrictEqual(afterSecondInstall, { - ...legacySettings, - includeCoAuthoredBy: false, - }); + assert.strictEqual(afterSecondInstall.includeCoAuthoredBy, false); + assert.deepStrictEqual(afterSecondInstall.hooks.PreToolUse[0], legacySettings.hooks.PreToolUse[0]); + assert.ok(afterSecondInstall.hooks.PreToolUse.some(entry => entry.id === 'pre:bash:dispatcher')); } finally { cleanup(homeDir); cleanup(projectDir); @@ -995,7 +1066,9 @@ function runTests() { assert.strictEqual(install.code, 0, install.stderr); const afterInstall = readJson(settingsPath); - assert.deepStrictEqual(afterInstall, customSettings); + assert.strictEqual(afterInstall.includeCoAuthoredBy, true); + assert.strictEqual(afterInstall.theme, 'dark'); + assert.ok(afterInstall.hooks.SessionStart.some(entry => entry.id === 'session:start')); } finally { cleanup(homeDir); cleanup(projectDir); @@ -1022,14 +1095,17 @@ function runTests() { assert.strictEqual(install.code, 0, install.stderr); const afterInstall = readJson(settingsPath); - assert.deepStrictEqual(afterInstall, customSettings); + assert.deepStrictEqual(afterInstall.attribution, customSettings.attribution); + assert.strictEqual(afterInstall.theme, 'dark'); + assert.ok(!Object.hasOwn(afterInstall, 'includeCoAuthoredBy')); + assert.ok(afterInstall.hooks.SessionStart.some(entry => entry.id === 'session:start')); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('ignores malformed existing settings.json during claude install', () => { + if (test('malformed Claude settings aborts before any install mutation', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -1040,17 +1116,17 @@ function runTests() { fs.writeFileSync(settingsPath, '{ invalid json\n'); const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); - assert.strictEqual(result.code, 0, result.stderr); + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /Failed to parse Claude settings/); assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written'); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('ignores non-object existing settings.json during claude install', () => { + if (test('non-object Claude settings aborts before any install mutation', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -1061,76 +1137,44 @@ function runTests() { fs.writeFileSync(settingsPath, '[]\n'); const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); - assert.strictEqual(result.code, 0, result.stderr); + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /expected a JSON object/); assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied'); - assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written'); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('fails when source hooks.json root is not an object before copying files', () => { - const tempDir = createTempDir('install-apply-invalid-hooks-'); - const targetRoot = path.join(tempDir, '.claude'); - const installStatePath = path.join(targetRoot, 'ecc', 'install-state.json'); - const sourceHooksPath = path.join(tempDir, 'hooks.json'); + if (test('same-id Claude hook conflict aborts before any install mutation', () => { + const homeDir = createTempDir('install-apply-home-'); + const projectDir = createTempDir('install-apply-project-'); try { - fs.writeFileSync(sourceHooksPath, '[]\n'); - - assert.throws(() => { - applyInstallPlan({ - targetRoot, - installStatePath, - hookConsent: 'enabled', - statePreview: { - schemaVersion: 'ecc.install.v1', - installedAt: new Date().toISOString(), - target: { - id: 'claude-home', - kind: 'home', - root: targetRoot, - installStatePath, - }, - request: { - profile: 'core', - modules: [], - includeComponents: [], - excludeComponents: [], - legacyLanguages: [], - legacyMode: false, - }, - resolution: { - selectedModules: ['hooks-runtime'], - skippedModules: [], - }, - source: { - repoVersion: null, - repoCommit: null, - manifestVersion: 1, - }, - operations: [], - }, - adapter: { target: 'claude' }, - operations: [{ - kind: 'copy-file', - moduleId: 'hooks-runtime', - sourcePath: sourceHooksPath, - sourceRelativePath: 'hooks/hooks.json', - destinationPath: path.join(targetRoot, 'hooks', 'hooks.json'), - strategy: 'preserve-relative-path', - ownership: 'managed', - scaffoldOnly: false, + const claudeRoot = path.join(homeDir, '.claude'); + fs.mkdirSync(claudeRoot, { recursive: true }); + const settingsPath = path.join(claudeRoot, 'settings.json'); + const existing = { + theme: 'dark', + hooks: { + PreToolUse: [{ + id: 'pre:bash:dispatcher', + matcher: 'Bash', + hooks: [{ type: 'command', command: 'echo user-owned' }], }], - }); - }, /Invalid hooks config at .*expected a JSON object/); + }, + }; + fs.writeFileSync(settingsPath, `${JSON.stringify(existing, null, 2)}\n`); - assert.ok(!fs.existsSync(path.join(targetRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied when source hooks are invalid'); - assert.ok(!fs.existsSync(installStatePath), 'install state should not be written when source hooks are invalid'); + const result = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); + assert.notStrictEqual(result.code, 0); + assert.match(result.stderr, /Refusing to overwrite.*pre:bash:dispatcher/); + assert.deepStrictEqual(readJson(settingsPath), existing); + assert.deepStrictEqual(fs.readdirSync(claudeRoot), ['settings.json']); } finally { - cleanup(tempDir); + cleanup(homeDir); + cleanup(projectDir); } })) passed++; else failed++; @@ -1254,6 +1298,39 @@ function runTests() { assert.strictEqual(state.request.hookConsent, 'declined'); assert.ok(!state.resolution.selectedModules.includes('hooks-runtime')); assert.ok(state.resolution.selectedModules.includes('rules-core')); + assert.ok(!state.operations.some(operation => ( + operation.kind === 'update-claude-settings' + ))); + } finally { + cleanup(homeDir); + cleanup(projectDir); + } + })) passed++; else failed++; + + if (test('--no-hooks removes hooks registered by a previous enabled install', () => { + const projectDir = createTempDir('install-apply-disable-hooks-'); + const homeDir = createTempDir('install-apply-disable-hooks-home-'); + try { + const enabled = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir }); + assert.strictEqual(enabled.code, 0, enabled.stderr); + + const settingsPath = path.join(homeDir, '.claude', 'settings.json'); + const settings = readJson(settingsPath); + settings.theme = 'dark'; + fs.writeFileSync(settingsPath, `${JSON.stringify(settings, null, 2)}\n`); + + const disabled = run(['--profile', 'core', '--no-hooks'], { cwd: projectDir, homeDir }); + assert.strictEqual(disabled.code, 0, disabled.stderr); + assert.deepStrictEqual(readJson(settingsPath), { + includeCoAuthoredBy: false, + theme: 'dark', + }); + + const state = readJson(path.join(homeDir, '.claude', 'ecc', 'install-state.json')); + assert.strictEqual(state.request.hookConsent, 'declined'); + assert.ok(!state.operations.some(operation => ( + operation.kind === 'update-claude-settings' + ))); } finally { cleanup(homeDir); cleanup(projectDir); diff --git a/tests/scripts/manual-hook-install-docs.test.js b/tests/scripts/manual-hook-install-docs.test.js index 8dc531efd..f86ea670f 100644 --- a/tests/scripts/manual-hook-install-docs.test.js +++ b/tests/scripts/manual-hook-install-docs.test.js @@ -47,6 +47,10 @@ function runTests() { readme.includes('%USERPROFILE%\\\\.claude'), 'README should call out the correct Windows Claude config root' ); + assert.ok( + readme.includes('registers the resolved\nhook entries in `~/.claude/settings.json`'), + 'README should explain that manual installs register hooks in Claude settings' + ); })) passed++; else failed++; if (test('hooks/README mirrors supported manual install guidance', () => { @@ -62,6 +66,10 @@ function runTests() { hooksReadme.includes('pwsh -File .\\install.ps1 --target claude --modules hooks-runtime --enable-hooks'), 'hooks/README should document the supported PowerShell hook install path' ); + assert.ok( + hooksReadme.includes('registers the resolved\nhook entries in `~/.claude/settings.json`'), + 'hooks/README should explain that manual installs register hooks in Claude settings' + ); })) passed++; else failed++; console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); From 26d3e0038b22e48f6eb769283d293e41270f6e85 Mon Sep 17 00:00:00 2001 From: wellkilo Date: Mon, 7 Sep 2026 01:13:20 +0800 Subject: [PATCH 2/4] fix(install): surface Claude settings failures --- scripts/lib/install-lifecycle.js | 25 +++++++++++--- scripts/lib/install/apply.js | 25 +++++++++----- tests/lib/install-executor.test.js | 53 +++++++++++++++++++++++++++-- tests/lib/install-lifecycle.test.js | 48 ++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 15 deletions(-) diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index c5ece3504..5da0cd8b1 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -1300,11 +1300,12 @@ function inspectManagedOperation(repoRoot, trustedRoot, operation, linkIndex = n destinationPath, managedHookInspection: inspection }; - } catch (_error) { + } catch (error) { return { - status: 'drifted', + status: 'invalid-settings', operation, - destinationPath + destinationPath, + error: `Failed to inspect Claude settings at ${destinationPath}: ${error.message}` }; } } @@ -1337,6 +1338,8 @@ function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations, targ summary.unsafeSource.push(inspection); } else if (inspection.status === 'unsafe-destination') { summary.unsafeDestination.push(inspection); + } else if (inspection.status === 'invalid-settings') { + summary.invalidSettings.push(inspection); } else if (inspection.status === 'unverified' || inspection.status === 'invalid-destination') { summary.unverified.push(inspection); } @@ -1348,6 +1351,7 @@ function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations, targ missingSource: [], unsafeSource: [], unsafeDestination: [], + invalidSettings: [], unverified: [] } ); @@ -1374,7 +1378,9 @@ function getUnsafeOperationResult(record, operationHealth) { ? getUnsafeManagedDestinationError(operationHealth) : operationHealth.unsafeSource.length > 0 ? createUnsafeRepairSourceError().message - : null; + : operationHealth.invalidSettings.length > 0 + ? operationHealth.invalidSettings[0].error + : null; if (!error) { return null; } @@ -1666,6 +1672,17 @@ function analyzeRecord(record, context) { ); } + if (operationHealth.invalidSettings.length > 0) { + issues.push( + buildIssue( + 'error', + 'invalid-claude-settings', + operationHealth.invalidSettings[0].error, + { paths: operationHealth.invalidSettings.map(entry => entry.destinationPath) } + ) + ); + } + if (missingManagedOperations.length > 0) { issues.push( buildIssue('error', 'missing-managed-files', `${missingManagedOperations.length} managed file(s) are missing`, { diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 1c5d4900d..015b1e128 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -209,20 +209,27 @@ function shouldSetClaudeCommitAttributionPreference(plan) { } function writeClaudeCommitAttributionPreference(settingsPath, options = {}) { + let settings; try { - let changed = false; - updateSettingsAtomic(settingsPath, settings => { - if (hasExplicitCommitAttributionPreference(settings)) { - return { settings }; - } - changed = true; - return { settings: withCommitAttributionDisabled(settings) }; - }, options); - return changed; + settings = readSettings(settingsPath); } catch (_error) { // Unreadable or malformed settings belong to the user; leave them untouched. return false; } + + if (hasExplicitCommitAttributionPreference(settings)) { + return false; + } + + let changed = false; + updateSettingsAtomic(settingsPath, latestSettings => { + if (hasExplicitCommitAttributionPreference(latestSettings)) { + return { settings: latestSettings }; + } + changed = true; + return { settings: withCommitAttributionDisabled(latestSettings) }; + }, options); + return changed; } function isMcpConfigPath(filePath) { diff --git a/tests/lib/install-executor.test.js b/tests/lib/install-executor.test.js index d0acad15a..0f9c656f6 100644 --- a/tests/lib/install-executor.test.js +++ b/tests/lib/install-executor.test.js @@ -498,8 +498,9 @@ function runTests() { const tempDir = createTempDir('install-executor-claude-hooks-'); try { for (const target of ['claude', 'claude-project']) { - const homeDir = path.join(tempDir, `${target} home "quoted" $dollar %percent%`); - const projectRoot = path.join(tempDir, `${target} project "quoted" $dollar %percent%`); + const quoted = process.platform === 'win32' ? 'quoted' : '"quoted"'; + const homeDir = path.join(tempDir, `${target} home ${quoted} $dollar %percent%`); + const projectRoot = path.join(tempDir, `${target} project ${quoted} $dollar %percent%`); fs.mkdirSync(homeDir, { recursive: true }); fs.mkdirSync(projectRoot, { recursive: true }); @@ -568,6 +569,54 @@ function runTests() { } })) passed++; else failed++; + if (test('Claude commit-attribution atomic write failures abort installation', () => { + const tempDir = createTempDir('install-executor-attribution-failure-'); + const originalRenameSync = fs.renameSync; + try { + const homeDir = path.join(tempDir, 'home'); + const projectRoot = path.join(tempDir, 'project'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + const rawPlan = createManifestInstallPlan({ + sourceRoot: REPO_ROOT, + homeDir, + projectRoot, + target: 'claude', + moduleIds: ['hooks-runtime'], + }); + const plan = { + ...rawPlan, + hookConsent: 'enabled', + statePreview: { + ...rawPlan.statePreview, + request: { ...rawPlan.statePreview.request, hookConsent: 'enabled' }, + }, + }; + const settingsPath = path.join(homeDir, '.claude', 'settings.json'); + let settingsCommitCount = 0; + fs.renameSync = function failAttributionCommit(sourcePath, destinationPath) { + if (path.resolve(String(destinationPath)) === path.resolve(settingsPath)) { + settingsCommitCount += 1; + if (settingsCommitCount === 2) { + throw new Error('injected attribution rename failure'); + } + } + return originalRenameSync.call(fs, sourcePath, destinationPath); + }; + + assert.throws( + () => applyInstallPlanDirect(plan), + /injected attribution rename failure/ + ); + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + assert.ok(settings.hooks.SessionStart.some(entry => entry.id === 'session:start')); + assert.strictEqual(Object.hasOwn(settings, 'includeCoAuthoredBy'), false); + } finally { + fs.renameSync = originalRenameSync; + cleanup(tempDir); + } + })) passed++; else failed++; + if (test('creates legacy compatibility manifest plans from language selections', () => { const projectRoot = createTempDir('install-executor-project-'); const homeDir = createTempDir('install-executor-home-'); diff --git a/tests/lib/install-lifecycle.test.js b/tests/lib/install-lifecycle.test.js index 4e45c9597..b086ef2bf 100644 --- a/tests/lib/install-lifecycle.test.js +++ b/tests/lib/install-lifecycle.test.js @@ -3372,6 +3372,54 @@ function runTests() { } })) passed++; else failed++; + if (test('doctor and repair surface malformed Claude settings errors', () => { + const homeDir = createTempDir('install-lifecycle-claude-home-'); + const projectRoot = createTempDir('install-lifecycle-project-'); + + try { + const targetRoot = path.join(homeDir, '.claude'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const managedHooks = currentManagedHooks(targetRoot); + fs.mkdirSync(targetRoot, { recursive: true }); + fs.writeFileSync(settingsPath, '{ invalid json\n'); + writeClaudeState(homeDir, { + operations: [ + managedOperation('update-claude-settings', settingsPath, { + sourceRelativePath: 'hooks/hooks.json', + strategy: 'update-claude-settings', + managedHooks, + }), + ], + }); + + const report = buildDoctorReport({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + const issue = report.results[0].issues.find(candidate => ( + candidate.code === 'invalid-claude-settings' + )); + assert.strictEqual(report.results[0].status, 'error'); + assert.ok(issue, 'doctor should report an invalid Claude settings issue'); + assert.match(issue.message, /Failed to inspect Claude settings/); + + const repair = repairInstalledStates({ + repoRoot: REPO_ROOT, + homeDir, + projectRoot, + targets: ['claude'], + }); + assert.strictEqual(repair.results[0].status, 'error'); + assert.match(repair.results[0].error, /Failed to inspect Claude settings/); + assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + if (test('repair restores managed Claude hooks while preserving user settings and hooks', () => { const homeDir = createTempDir('install-lifecycle-claude-home-'); const projectRoot = createTempDir('install-lifecycle-project-'); From f59cfd57c26c65eeaba37ffdb95cd6abd055ee86 Mon Sep 17 00:00:00 2001 From: wellkilo Date: Mon, 7 Sep 2026 01:57:04 +0800 Subject: [PATCH 3/4] fix(install): harden Claude settings lifecycle --- README.md | 2 +- hooks/README.md | 2 +- schemas/hooks.schema.json | 28 ++- schemas/install-state.schema.json | 22 +- scripts/ci/validate-hooks.js | 2 +- scripts/lib/install-lifecycle.js | 62 ++--- scripts/lib/install-state.js | 12 +- scripts/lib/install-targets/claude-home.js | 35 +-- scripts/lib/install-targets/claude-project.js | 35 +-- scripts/lib/install-targets/helpers.js | 41 ++++ scripts/lib/install/apply.js | 26 +-- scripts/lib/install/claude-settings-lock.js | 170 ++++++++++++++ scripts/lib/install/claude-settings.js | 214 ++++++------------ tests/ci/validators.test.js | 31 +++ tests/lib/claude-settings.test.js | 122 +++++++++- tests/lib/install-executor.test.js | 18 +- tests/lib/install-lifecycle.test.js | 24 +- tests/lib/install-state.test.js | 32 +-- .../scripts/manual-hook-install-docs.test.js | 12 +- 19 files changed, 540 insertions(+), 350 deletions(-) create mode 100644 scripts/lib/install/claude-settings-lock.js diff --git a/README.md b/README.md index 2431bd418..091836e33 100644 --- a/README.md +++ b/README.md @@ -562,7 +562,7 @@ and safe uninstall. If you installed ECC via `/plugin install`, do not copy those hooks into `settings.json`. Claude Code v2.1+ already auto-loads plugin `hooks/hooks.json`, and duplicating them in `settings.json` causes duplicate execution and cross-platform hook conflicts. -On Windows, Claude's config root is `%USERPROFILE%\\.claude`; install the hook runtime with: +On Windows, Claude's config root is `%USERPROFILE%\.claude`; install the hook runtime with: ```powershell pwsh -File .\install.ps1 --target claude --modules hooks-runtime --enable-hooks diff --git a/hooks/README.md b/hooks/README.md index e540b2d24..144bc89ad 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -37,7 +37,7 @@ That installs the hook scripts under `~/.claude/` and registers the resolved hook entries in `~/.claude/settings.json`. Existing user settings and hook entries are preserved, while ECC-owned entries are tracked by stable ID for idempotent updates and safe uninstall. On Windows, the Claude config root is -`%USERPROFILE%\\.claude`. +`%USERPROFILE%\.claude`. ### PreToolUse Hooks diff --git a/schemas/hooks.schema.json b/schemas/hooks.schema.json index c325d9712..e3d339f77 100644 --- a/schemas/hooks.schema.json +++ b/schemas/hooks.schema.json @@ -147,6 +147,24 @@ "type": "string" } } + }, + "managedMatcherEntry": { + "allOf": [ + { "$ref": "#/$defs/matcherEntry" }, + { + "type": "object", + "required": ["id"], + "properties": { + "hooks": { "type": "array", "minItems": 1 } + } + } + ] + }, + "managedMatcherRequiredEntry": { + "allOf": [ + { "$ref": "#/$defs/managedMatcherEntry" }, + { "type": "object", "required": ["matcher"] } + ] } }, "oneOf": [ @@ -180,10 +198,18 @@ "SessionEnd" ] }, + "patternProperties": { + "^(SessionStart|PreToolUse|PermissionRequest|PostToolUse|PostToolUseFailure|SubagentStart|PreCompact|InstructionsLoaded|TeammateIdle|TaskCompleted|ConfigChange|WorktreeCreate|WorktreeRemove|SessionEnd)$": { + "type": "array", + "items": { + "$ref": "#/$defs/managedMatcherRequiredEntry" + } + } + }, "additionalProperties": { "type": "array", "items": { - "$ref": "#/$defs/matcherEntry" + "$ref": "#/$defs/managedMatcherEntry" } } } diff --git a/schemas/install-state.schema.json b/schemas/install-state.schema.json index 9b827e144..b7e48def5 100644 --- a/schemas/install-state.schema.json +++ b/schemas/install-state.schema.json @@ -218,26 +218,8 @@ "type": "object", "minProperties": 1, "propertyNames": { - "enum": [ - "SessionStart", - "UserPromptSubmit", - "PreToolUse", - "PermissionRequest", - "PostToolUse", - "PostToolUseFailure", - "Notification", - "SubagentStart", - "Stop", - "SubagentStop", - "PreCompact", - "InstructionsLoaded", - "TeammateIdle", - "TaskCompleted", - "ConfigChange", - "WorktreeCreate", - "WorktreeRemove", - "SessionEnd" - ] + "type": "string", + "pattern": "\\S" }, "additionalProperties": { "type": "array", diff --git a/scripts/ci/validate-hooks.js b/scripts/ci/validate-hooks.js index 779555a44..d59807f1d 100644 --- a/scripts/ci/validate-hooks.js +++ b/scripts/ci/validate-hooks.js @@ -207,7 +207,7 @@ function validateHooks() { console.error(`ERROR: ${matcherLabel} has invalid 'matcher' field`); hasErrors = true; } - if (!matcher.hooks || !Array.isArray(matcher.hooks)) { + if (!matcher.hooks || !Array.isArray(matcher.hooks) || matcher.hooks.length === 0) { console.error(`ERROR: ${matcherLabel} missing 'hooks' array`); hasErrors = true; } else { diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index 5da0cd8b1..99ec19614 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -3,6 +3,7 @@ const fs = require('fs'); const { execFileSync } = require('child_process'); const os = require('os'); const path = require('path'); +const { isDeepStrictEqual } = require('util'); const { loadInstallManifests } = require('./install-manifests'); const { readInstallState, validateInstallState } = require('./install-state'); @@ -22,6 +23,8 @@ const { } = require('./install/opencode-legacy-migration'); const { acquireSettingsLock, + assertClaudeSettingsPath, + getClaudeSettingsPath, inspectManagedHooks, materializeManagedHooks, repairManagedHooks, @@ -532,22 +535,11 @@ function readJsonNoFollow(filePath) { return JSON.parse(readFileNoFollow(filePath, 'utf8')); } -function expectedClaudeSettingsPath(targetRoot) { - return path.join(targetRoot, 'settings.json'); -} - 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.'); } - if (path.resolve(operation.destinationPath) !== path.resolve( - expectedClaudeSettingsPath(trustedRoot) - )) { - throw new Error( - `Refusing to manage Claude hooks outside the canonical settings file: ` - + `${operation.destinationPath}` - ); - } + assertClaudeSettingsPath(operation.destinationPath, trustedRoot); } function writeContainedFile(destinationPath, content, trustedRoot, action, mode) { @@ -714,7 +706,7 @@ function deepRemoveJsonSubset(currentValue, managedValue) { return currentValue === managedValue ? JSON_REMOVE_SENTINEL : currentValue; } -function hydrateRecordedOperations(repoRoot, operations) { +function hydrateRecordedOperations(repoRoot, operations, trustedRoot) { return operations.map(operation => { if (operation.kind === 'update-claude-settings') { const sourcePath = resolveOperationSourcePath(repoRoot, operation); @@ -729,7 +721,7 @@ function hydrateRecordedOperations(repoRoot, operations) { previousManagedHooks: operation.managedHooks, managedHooks: materializeManagedHooks( readJsonNoFollow(sourcePath), - path.dirname(operation.destinationPath) + trustedRoot ), }; } @@ -1357,12 +1349,6 @@ function summarizeManagedOperationHealth(repoRoot, trustedRoot, operations, targ ); } -function hookRepairOperations(operationHealth) { - return operationHealth.drifted - .filter(entry => entry.operation.kind === 'update-claude-settings') - .map(entry => ({ ...entry.operation })); -} - function getUnsafeManagedDestinationError(operationHealth) { const hasFinalSymlink = operationHealth.unsafeDestination.some( inspection => inspection.reason === 'final-symlink' @@ -1806,7 +1792,11 @@ function createRepairPlanFromRecord(record, context, options = {}) { record.legacyLayout !== 'opencode' && (state.request.legacyMode || shouldRepairFromRecordedOperations(state)) ) { - const operations = hydrateRecordedOperations(context.repoRoot, getManagedOperations(state)); + const operations = hydrateRecordedOperations( + context.repoRoot, + getManagedOperations(state), + record.targetRoot + ); const statePreview = buildRecordedStatePreview(state, context, operations); return { @@ -1951,15 +1941,14 @@ function repairInstalledStates(options = {}) { let releaseSettingsLock = null; try { - if ( - !options.dryRun + const settingsPathToLock = !options.dryRun && getManagedOperations(record.state || {}).some( operation => operation.kind === 'update-claude-settings' ) - ) { - releaseSettingsLock = acquireSettingsLock( - path.join(record.targetRoot, 'settings.json') - ); + ? getClaudeSettingsPath(record.targetRoot) + : null; + if (settingsPathToLock) { + releaseSettingsLock = acquireSettingsLock(settingsPathToLock); } const needsOpencodeBuild = record.adapter.target === 'opencode' && hasOpencodeBuildError(getOpencodeBuildValidationIssues(context)); @@ -2107,16 +2096,13 @@ function repairInstalledStates(options = {}) { const repairOperations = [ ...operationHealth.missing.map(entry => ({ ...entry.operation })), ...operationHealth.drifted.map(entry => ({ ...entry.operation })), - ...hookRepairOperations({ - drifted: desiredPlan.operations - .filter(operation => ( - operation.kind === 'update-claude-settings' - && operation.previousManagedHooks - && JSON.stringify(operation.previousManagedHooks) - !== JSON.stringify(operation.managedHooks) - )) - .map(operation => ({ operation })), - }), + ...desiredPlan.operations + .filter(operation => ( + operation.kind === 'update-claude-settings' + && operation.previousManagedHooks + && !isDeepStrictEqual(operation.previousManagedHooks, operation.managedHooks) + )) + .map(operation => ({ ...operation })), ].filter((operation, index, items) => items.findIndex(candidate => ( candidate.kind === operation.kind && candidate.destinationPath === operation.destinationPath @@ -2333,7 +2319,7 @@ function uninstallInstalledStates(options = {}) { const operations = getManagedOperations(state); if (operations.some(operation => operation.kind === 'update-claude-settings')) { releaseSettingsLock = acquireSettingsLock( - path.join(record.targetRoot, 'settings.json') + getClaudeSettingsPath(record.targetRoot) ); } diff --git a/scripts/lib/install-state.js b/scripts/lib/install-state.js index 805943f92..3b5b7fc23 100644 --- a/scripts/lib/install-state.js +++ b/scripts/lib/install-state.js @@ -1,6 +1,10 @@ const fs = require('fs'); const path = require('path'); -const { validateManagedHooks } = require('./install/claude-settings'); +const { + CLAUDE_HOOKS_CONFIG_PATH, + getClaudeSettingsPath, + validateRecordedManagedHooks, +} = require('./install/claude-settings'); // Dependency-free, self-contained validation. The installer closure must not // require any non-builtin package (enterprise supply-chain vetting: the vetted @@ -217,14 +221,14 @@ function createFallbackValidator() { if (operation.moduleId !== 'hooks-runtime') { pushError(`${instancePath}/moduleId`, 'must equal hooks-runtime'); } - if (String(operation.sourceRelativePath).replace(/\\/g, '/') !== 'hooks/hooks.json') { + if (String(operation.sourceRelativePath).replace(/\\/g, '/') !== CLAUDE_HOOKS_CONFIG_PATH) { pushError(`${instancePath}/sourceRelativePath`, 'must equal hooks/hooks.json'); } if ( isNonEmptyString(state.target && state.target.root) && isNonEmptyString(operation.destinationPath) ) { - const expectedDestination = path.resolve(state.target.root, 'settings.json'); + const expectedDestination = path.resolve(getClaudeSettingsPath(state.target.root)); const actualDestination = path.resolve(operation.destinationPath); const pathsMatch = process.platform === 'win32' ? expectedDestination.toLowerCase() === actualDestination.toLowerCase() @@ -237,7 +241,7 @@ function createFallbackValidator() { } } try { - validateManagedHooks(operation.managedHooks); + validateRecordedManagedHooks(operation.managedHooks); } catch (error) { pushError(`${instancePath}/managedHooks`, error.message); } diff --git a/scripts/lib/install-targets/claude-home.js b/scripts/lib/install-targets/claude-home.js index 0ff84a160..5cc426ac9 100644 --- a/scripts/lib/install-targets/claude-home.js +++ b/scripts/lib/install-targets/claude-home.js @@ -1,4 +1,3 @@ -const fs = require('fs'); const path = require('path'); const { @@ -6,42 +5,10 @@ const { createRemappedOperation, isForeignPlatformPath, normalizeRelativePath, + planClaudeHooksOperations, } = require('./helpers'); const CLAUDE_ECC_NAMESPACE = 'ecc'; -const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; - -function planClaudeHooksOperations(adapter, module, input) { - const sourceHooksRoot = path.join(input.repoRoot || '', 'hooks'); - const operations = [ - createRemappedOperation( - adapter, - module.id, - CLAUDE_HOOKS_CONFIG_PATH, - path.join(adapter.resolveRoot(input), 'settings.json'), - { - kind: 'update-claude-settings', - strategy: 'merge-hook-ids', - } - ), - ]; - - if (!input.repoRoot || !fs.existsSync(sourceHooksRoot)) { - return operations; - } - - return [ - ...operations, - ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) - .filter(entry => entry.name !== 'hooks.json') - .sort((left, right) => left.name.localeCompare(right.name)) - .map(entry => adapter.createScaffoldOperation( - module.id, - path.join('hooks', entry.name), - input - )), - ]; -} function getClaudeManagedDestinationPath(adapter, sourceRelativePath, input) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); diff --git a/scripts/lib/install-targets/claude-project.js b/scripts/lib/install-targets/claude-project.js index 4c5f23a32..a4fda3970 100644 --- a/scripts/lib/install-targets/claude-project.js +++ b/scripts/lib/install-targets/claude-project.js @@ -1,4 +1,3 @@ -const fs = require('fs'); const path = require('path'); const { @@ -6,42 +5,10 @@ const { createRemappedOperation, isForeignPlatformPath, normalizeRelativePath, + planClaudeHooksOperations, } = require('./helpers'); const CLAUDE_ECC_NAMESPACE = 'ecc'; -const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; - -function planClaudeHooksOperations(adapter, module, input) { - const sourceHooksRoot = path.join(input.repoRoot || '', 'hooks'); - const operations = [ - createRemappedOperation( - adapter, - module.id, - CLAUDE_HOOKS_CONFIG_PATH, - path.join(adapter.resolveRoot(input), 'settings.json'), - { - kind: 'update-claude-settings', - strategy: 'merge-hook-ids', - } - ), - ]; - - if (!input.repoRoot || !fs.existsSync(sourceHooksRoot)) { - return operations; - } - - return [ - ...operations, - ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) - .filter(entry => entry.name !== 'hooks.json') - .sort((left, right) => left.name.localeCompare(right.name)) - .map(entry => adapter.createScaffoldOperation( - module.id, - path.join('hooks', entry.name), - input - )), - ]; -} function getClaudeManagedDestinationPath(adapter, sourceRelativePath, input) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); diff --git a/scripts/lib/install-targets/helpers.js b/scripts/lib/install-targets/helpers.js index 9dedcf50a..dbb5b44e5 100644 --- a/scripts/lib/install-targets/helpers.js +++ b/scripts/lib/install-targets/helpers.js @@ -1,6 +1,10 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); +const { + CLAUDE_HOOKS_CONFIG_PATH, + getClaudeSettingsPath, +} = require('../install/claude-settings'); const PLATFORM_SOURCE_PATH_OWNERS = Object.freeze({ '.claude-plugin': 'claude', @@ -146,6 +150,42 @@ function createRemappedOperation(adapter, moduleId, sourceRelativePath, destinat }); } +function planClaudeHooksOperations(adapter, module, input) { + const operations = [ + createRemappedOperation( + adapter, + module.id, + CLAUDE_HOOKS_CONFIG_PATH, + getClaudeSettingsPath(adapter.resolveRoot(input)), + { + kind: 'update-claude-settings', + strategy: 'merge-hook-ids', + } + ), + ]; + + if (!input.repoRoot) { + return operations; + } + + const sourceHooksRoot = path.join(input.repoRoot, 'hooks'); + if (!fs.existsSync(sourceHooksRoot)) { + return operations; + } + + return [ + ...operations, + ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) + .filter(entry => entry.name !== 'hooks.json') + .sort((left, right) => left.name.localeCompare(right.name)) + .map(entry => adapter.createScaffoldOperation( + module.id, + path.join('hooks', entry.name), + input + )), + ]; +} + function createNamespacedFlatRuleOperations(adapter, moduleId, sourceRelativePath, input = {}) { const normalizedSourcePath = normalizeRelativePath(sourceRelativePath); const sourceRoot = path.join(input.repoRoot || '', normalizedSourcePath); @@ -373,4 +413,5 @@ module.exports = { createRemappedOperation, isForeignPlatformPath, normalizeRelativePath, + planClaudeHooksOperations, }; diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 015b1e128..8a726883e 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -11,12 +11,14 @@ const { const { readInstallState, writeInstallState } = require('../install-state'); const { assertHookConsentReady, planMaterializesHookRuntime } = require('./hook-consent'); const { - acquireSettingsLock, + getClaudeSettingsPath, mergeManagedHooks, readSettings, + runWithSettingsLock, uninstallManagedHooks, updateSettingsAtomic, validateManagedHooks, + validateRecordedManagedHooks, } = require('./claude-settings'); const { filterMcpConfig, parseDisabledMcpServers } = require('../mcp-config'); const { assertWithinTrustedRoot } = require('../path-safety'); @@ -293,13 +295,13 @@ function findPreviousManagedHooks(previousState, plan, operation) { const previousOperation = (previousState.operations || []).find(candidate => ( candidate.kind === operation.kind - && candidate.destinationPath === operation.destinationPath + && comparablePath(candidate.destinationPath) === comparablePath(operation.destinationPath) )); if (!previousOperation || !previousOperation.managedHooks) { return null; } - return validateManagedHooks( + return validateRecordedManagedHooks( previousOperation.managedHooks, 'previous managed hooks' ); @@ -421,19 +423,17 @@ function applyInstallPlan(plan, dependencies = {}) { const isClaudeManualTarget = plan.adapter && (plan.adapter.target === 'claude' || plan.adapter.target === 'claude-project'); const settingsPathToLock = isClaudeManualTarget - ? path.join(plan.targetRoot, 'settings.json') + ? getClaudeSettingsPath(plan.targetRoot) : null; if (settingsPathToLock) { assertSafeInstallOperation(plan, { destinationPath: settingsPathToLock }); } - const releaseSettingsLock = settingsPathToLock - ? acquireSettingsLock(settingsPathToLock) - : null; - try { - return applyInstallPlanLocked(plan, dependencies, Boolean(releaseSettingsLock)); - } finally { - if (releaseSettingsLock) releaseSettingsLock(); - } + return settingsPathToLock + ? runWithSettingsLock( + settingsPathToLock, + () => applyInstallPlanLocked(plan, dependencies, true) + ) + : applyInstallPlanLocked(plan, dependencies, false); } function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = false) { @@ -581,7 +581,7 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals if (shouldSetClaudeCommitAttributionPreference(appliedPlan)) { writeClaudeCommitAttributionPreference( - path.join(plan.targetRoot, 'settings.json'), + getClaudeSettingsPath(plan.targetRoot), { lockHeld: settingsLockHeld } ); } diff --git a/scripts/lib/install/claude-settings-lock.js b/scripts/lib/install/claude-settings-lock.js new file mode 100644 index 000000000..ae413fa01 --- /dev/null +++ b/scripts/lib/install/claude-settings-lock.js @@ -0,0 +1,170 @@ +'use strict'; + +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); + +const INVALID_LOCK_STALE_MS = 5 * 60 * 1000; + +function sameFileIdentity(left, right) { + return left.dev === right.dev && left.ino === right.ino; +} + +function createSettingsLock(lockPath) { + const tempPath = `${lockPath}.create-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + let descriptor; + let ownedStats; + try { + descriptor = fs.openSync(tempPath, 'wx', 0o600); + fs.writeFileSync(descriptor, `${JSON.stringify({ + pid: process.pid, + startedAt: new Date().toISOString(), + token: crypto.randomBytes(16).toString('hex'), + })}\n`); + fs.fsyncSync(descriptor); + ownedStats = fs.fstatSync(descriptor, { bigint: true }); + fs.closeSync(descriptor); + descriptor = undefined; + fs.linkSync(tempPath, lockPath); + } catch (error) { + if (descriptor !== undefined) fs.closeSync(descriptor); + fs.rmSync(tempPath, { force: true }); + throw error; + } + fs.rmSync(tempPath, { force: true }); + + let released = false; + return () => { + if (released) return; + const quarantinePath = `${lockPath}.release-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + fs.renameSync(lockPath, quarantinePath); + const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); + if (!sameFileIdentity(quarantinedStats, ownedStats)) { + if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); + throw new Error(`Refusing to release a changed Claude settings lock: ${lockPath}`); + } + released = true; + fs.rmSync(quarantinePath, { force: true }); + }; +} + +function inspectSettingsLock(lockPath) { + const descriptor = fs.openSync(lockPath, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); + try { + const stats = fs.fstatSync(descriptor, { bigint: true }); + const pathStats = fs.lstatSync(lockPath, { bigint: true }); + if ( + !stats.isFile() + || pathStats.isSymbolicLink() + || !pathStats.isFile() + || !sameFileIdentity(stats, pathStats) + ) { + return { metadata: null, stats }; + } + let metadata = null; + try { + metadata = JSON.parse(fs.readFileSync(descriptor, 'utf8')); + } catch (_error) { + // Invalid locks may be recovered only after the bounded lease below. + } + return { metadata, stats }; + } finally { + fs.closeSync(descriptor); + } +} + +function processIsAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code !== 'ESRCH'; + } +} + +function recoverSettingsLock(lockPath) { + const recoveryPath = `${lockPath}.recover`; + try { + fs.mkdirSync(recoveryPath, { mode: 0o700 }); + } catch (error) { + if (error && error.code === 'EEXIST') return null; + throw error; + } + + const quarantinePath = `${lockPath}.stale-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; + try { + let inspected; + try { + inspected = inspectSettingsLock(lockPath); + } catch (error) { + if (error && error.code === 'ENOENT') return createSettingsLock(lockPath); + throw error; + } + const validOwner = Number.isSafeInteger(inspected.metadata && inspected.metadata.pid) + && inspected.metadata.pid > 0; + const stale = validOwner + ? !processIsAlive(inspected.metadata.pid) + : Date.now() - Number(inspected.stats.mtimeMs) >= INVALID_LOCK_STALE_MS; + if (!stale) return null; + + fs.renameSync(lockPath, quarantinePath); + const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); + if (!sameFileIdentity(quarantinedStats, inspected.stats)) { + if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); + return null; + } + fs.rmSync(quarantinePath, { force: true }); + return createSettingsLock(lockPath); + } finally { + fs.rmSync(recoveryPath, { recursive: true, force: true }); + fs.rmSync(quarantinePath, { force: true }); + } +} + +function acquireSettingsLock(settingsPath) { + const lockPath = `${settingsPath}.ecc.lock`; + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + try { + return createSettingsLock(lockPath); + } catch (error) { + if (!error || error.code !== 'EEXIST') { + throw error; + } + } + const recovered = recoverSettingsLock(lockPath); + if (recovered) return recovered; + throw new Error( + `Another ECC process is updating Claude settings: ${settingsPath}. ` + + `If no ECC process is active, inspect and remove ${lockPath}.` + ); +} + +function runWithSettingsLock(settingsPath, callback) { + const releaseLock = acquireSettingsLock(settingsPath); + let primaryError = null; + let result; + try { + result = callback(); + } catch (error) { + primaryError = error; + } + + let releaseError = null; + try { + releaseLock(); + } catch (error) { + releaseError = error; + } + + if (primaryError) { + if (releaseError) primaryError.releaseError = releaseError; + throw primaryError; + } + if (releaseError) throw releaseError; + return result; +} + +module.exports = { + acquireSettingsLock, + runWithSettingsLock, +}; diff --git a/scripts/lib/install/claude-settings.js b/scripts/lib/install/claude-settings.js index 90b0791ec..dd34dd6fb 100644 --- a/scripts/lib/install/claude-settings.js +++ b/scripts/lib/install/claude-settings.js @@ -1,12 +1,16 @@ 'use strict'; -const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); const { isDeepStrictEqual } = require('util'); const { writeFileAtomic } = require('../atomic-write'); +const { acquireSettingsLock, runWithSettingsLock } = require('./claude-settings-lock'); +const CLAUDE_SETTINGS_FILENAME = 'settings.json'; +const CLAUDE_HOOKS_CONFIG_PATH = 'hooks/hooks.json'; const PLUGIN_ROOT_PLACEHOLDER = '${CLAUDE_PLUGIN_ROOT}'; +const PLUGIN_ROOT_ENV_PROLOGUE = 'var e=process.env.CLAUDE_PLUGIN_ROOT;'; +const PLUGIN_ROOT_ENV_READ = /\bprocess\.env\.CLAUDE_PLUGIN_ROOT\b(?!\s*=)/; const VALID_EVENTS = new Set([ 'SessionStart', 'UserPromptSubmit', 'PreToolUse', 'PermissionRequest', 'PostToolUse', 'PostToolUseFailure', 'Notification', 'SubagentStart', @@ -18,7 +22,6 @@ const EVENTS_WITHOUT_MATCHER = new Set([ 'UserPromptSubmit', 'Notification', 'Stop', 'SubagentStop', ]); const VALID_HOOK_TYPES = new Set(['command', 'http', 'prompt', 'agent']); -const INVALID_LOCK_STALE_MS = 5 * 60 * 1000; function isJsonObject(value) { if (!value || typeof value !== 'object' || Array.isArray(value)) { @@ -44,6 +47,23 @@ function isNonEmptyString(value) { return typeof value === 'string' && value.trim() !== ''; } +function getClaudeSettingsPath(targetRoot) { + return path.join(targetRoot, CLAUDE_SETTINGS_FILENAME); +} + +function assertClaudeSettingsPath(destinationPath, trustedRoot) { + const resolvedDestination = path.resolve(destinationPath); + const resolvedExpected = path.resolve(getClaudeSettingsPath(trustedRoot)); + const pathsMatch = process.platform === 'win32' + ? resolvedDestination.toLowerCase() === resolvedExpected.toLowerCase() + : resolvedDestination === resolvedExpected; + if (!pathsMatch) { + throw new Error( + `Refusing to manage Claude hooks outside the canonical settings file: ${destinationPath}` + ); + } +} + function validateHookHandler(hook, label) { if (!isJsonObject(hook)) { throw new Error(`Invalid managed hook handler at ${label}: expected a JSON object`); @@ -167,6 +187,30 @@ function validateManagedHooks(managedHooks, label = 'managed hooks') { return cloneValue(managedHooks); } +function validateRecordedManagedHooks(managedHooks, label = 'recorded managed hooks') { + if (!isJsonObject(managedHooks) || Object.keys(managedHooks).length === 0) { + throw new Error(`Invalid ${label}: expected a non-empty JSON object`); + } + for (const [event, entries] of Object.entries(managedHooks)) { + if (!isNonEmptyString(event) || !Array.isArray(entries) || entries.length === 0) { + throw new Error(`Invalid ${label}.${event}: expected a non-empty hook array`); + } + const seenIds = new Set(); + entries.forEach((entry, index) => { + if (!isJsonObject(entry) || !isNonEmptyString(entry.id) || !Array.isArray(entry.hooks)) { + throw new Error(`Invalid hook entry at ${label}.${event}[${index}]`); + } + if (seenIds.has(entry.id)) { + throw new Error( + `Invalid ${label}: expected unique id "${entry.id}" within event "${event}"` + ); + } + seenIds.add(entry.id); + }); + } + return cloneValue(managedHooks); +} + function validateSettings(settings, label = 'Claude settings') { if (!isJsonObject(settings)) { throw new Error(`Invalid ${label}: expected a JSON object`); @@ -210,6 +254,18 @@ function replacePluginRootPlaceholders(value, pluginRoot) { function resolveManagedHookCommands(managedHooks, targetRoot) { const encodedRoot = Buffer.from(targetRoot, 'utf8').toString('base64'); const rootExpression = `Buffer.from('${encodedRoot}','base64').toString('utf8')`; + const resolveCommand = command => { + const resolved = command + .split(PLUGIN_ROOT_ENV_PROLOGUE) + .join(`var e=${rootExpression};`); + if (PLUGIN_ROOT_ENV_READ.test(resolved)) { + throw new Error( + 'Unable to resolve CLAUDE_PLUGIN_ROOT in a managed hook command; ' + + 'the hooks.json command prologue no longer matches the expected form' + ); + } + return resolved; + }; return Object.fromEntries( Object.entries(managedHooks).map(([event, entries]) => [ event, @@ -219,9 +275,7 @@ function resolveManagedHookCommands(managedHooks, targetRoot) { ...hook, ...(typeof hook.command === 'string' ? { - command: hook.command - .split('var e=process.env.CLAUDE_PLUGIN_ROOT;') - .join(`var e=${rootExpression};`), + command: resolveCommand(hook.command), } : {}), })), @@ -333,139 +387,6 @@ function assertSettingsSnapshotUnchanged(settingsPath, snapshot) { } } -function createSettingsLock(lockPath) { - const tempPath = `${lockPath}.create-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; - let descriptor; - let ownedStats; - try { - descriptor = fs.openSync(tempPath, 'wx', 0o600); - fs.writeFileSync(descriptor, `${JSON.stringify({ - pid: process.pid, - startedAt: new Date().toISOString(), - token: crypto.randomBytes(16).toString('hex'), - })}\n`); - fs.fsyncSync(descriptor); - ownedStats = fs.fstatSync(descriptor, { bigint: true }); - fs.closeSync(descriptor); - descriptor = undefined; - fs.linkSync(tempPath, lockPath); - } catch (error) { - if (descriptor !== undefined) fs.closeSync(descriptor); - fs.rmSync(tempPath, { force: true }); - throw error; - } - fs.rmSync(tempPath, { force: true }); - - let released = false; - return () => { - if (released) return; - released = true; - const quarantinePath = `${lockPath}.release-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; - fs.renameSync(lockPath, quarantinePath); - const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); - if (!sameFileIdentity(quarantinedStats, ownedStats)) { - if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); - throw new Error(`Refusing to release a changed Claude settings lock: ${lockPath}`); - } - fs.rmSync(quarantinePath, { force: true }); - }; -} - -function sameFileIdentity(left, right) { - return left.dev === right.dev && left.ino === right.ino; -} - -function inspectSettingsLock(lockPath) { - const descriptor = fs.openSync(lockPath, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); - try { - const stats = fs.fstatSync(descriptor, { bigint: true }); - const pathStats = fs.lstatSync(lockPath, { bigint: true }); - if ( - !stats.isFile() - || pathStats.isSymbolicLink() - || !pathStats.isFile() - || !sameFileIdentity(stats, pathStats) - ) { - return { metadata: null, stats }; - } - let metadata = null; - try { - metadata = JSON.parse(fs.readFileSync(descriptor, 'utf8')); - } catch (_error) { - // Invalid locks may be recovered only after the bounded lease below. - } - return { metadata, stats }; - } finally { - fs.closeSync(descriptor); - } -} - -function processIsAlive(pid) { - try { - process.kill(pid, 0); - return true; - } catch (error) { - return error.code !== 'ESRCH'; - } -} - -function recoverSettingsLock(lockPath) { - const recoveryPath = `${lockPath}.recover`; - try { - fs.mkdirSync(recoveryPath, { mode: 0o700 }); - } catch (error) { - if (error && error.code === 'EEXIST') return null; - throw error; - } - - const quarantinePath = `${lockPath}.stale-${process.pid}-${crypto.randomBytes(8).toString('hex')}`; - try { - let inspected; - try { - inspected = inspectSettingsLock(lockPath); - } catch (error) { - if (error && error.code === 'ENOENT') return createSettingsLock(lockPath); - throw error; - } - const validOwner = Number.isSafeInteger(inspected.metadata && inspected.metadata.pid) - && inspected.metadata.pid > 0; - const stale = validOwner - ? !processIsAlive(inspected.metadata.pid) - : Date.now() - Number(inspected.stats.mtimeMs) >= INVALID_LOCK_STALE_MS; - if (!stale) return null; - - fs.renameSync(lockPath, quarantinePath); - const quarantinedStats = fs.lstatSync(quarantinePath, { bigint: true }); - if (!sameFileIdentity(quarantinedStats, inspected.stats)) { - if (!fs.existsSync(lockPath)) fs.renameSync(quarantinePath, lockPath); - return null; - } - fs.rmSync(quarantinePath, { force: true }); - return createSettingsLock(lockPath); - } finally { - fs.rmSync(recoveryPath, { recursive: true, force: true }); - fs.rmSync(quarantinePath, { force: true }); - } -} - -function acquireSettingsLock(settingsPath) { - const lockPath = `${settingsPath}.ecc.lock`; - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - try { - return createSettingsLock(lockPath); - } catch (error) { - if (!error || error.code !== 'EEXIST') { - throw error; - } - } - const recovered = recoverSettingsLock(lockPath); - if (recovered) return recovered; - throw new Error( - `Another ECC process is updating Claude settings: ${settingsPath}. ` - + `If no ECC process is active, inspect and remove ${lockPath}.` - ); -} - function updateSettingsAtomic(settingsPath, transform, options = {}) { const update = () => { const maxAttempts = options.maxAttempts || 3; @@ -492,12 +413,7 @@ function updateSettingsAtomic(settingsPath, transform, options = {}) { if (options.lockHeld) { return update(); } - const releaseLock = acquireSettingsLock(settingsPath); - try { - return update(); - } finally { - releaseLock(); - } + return runWithSettingsLock(settingsPath, update); } function reference(event, id) { @@ -534,7 +450,7 @@ function mergeManagedHooks(settings, managedHooks, options = {}) { const previousHooks = options.previousManagedHooks === undefined || options.previousManagedHooks === null ? null - : validateManagedHooks(options.previousManagedHooks, 'previous managed hooks'); + : validateRecordedManagedHooks(options.previousManagedHooks, 'previous managed hooks'); const repair = options.mode === 'repair' || options.repair === true; if (options.mode !== undefined && options.mode !== 'merge' && options.mode !== 'repair') { throw new Error(`Unknown Claude settings merge mode: ${options.mode}`); @@ -677,7 +593,7 @@ function withoutProperty(object, omittedKey) { function uninstallManagedHooks(settings, recordedManagedHooks) { const validatedSettings = validateSettings(settings); - const recordedHooks = validateManagedHooks(recordedManagedHooks, 'recorded managed hooks'); + const recordedHooks = validateRecordedManagedHooks(recordedManagedHooks); const currentHooks = validatedSettings.hooks || {}; for (const [event, recordedEntries] of Object.entries(recordedHooks)) { @@ -735,7 +651,11 @@ function uninstallManagedHooks(settings, recordedManagedHooks) { } module.exports = { + CLAUDE_HOOKS_CONFIG_PATH, + CLAUDE_SETTINGS_FILENAME, acquireSettingsLock, + assertClaudeSettingsPath, + getClaudeSettingsPath, inspectManagedHooks, materializeManagedHooks, mergeManagedHooks, @@ -743,8 +663,10 @@ module.exports = { readSettings, repairManagedHooks, replacePluginRootPlaceholders, + runWithSettingsLock, updateSettingsAtomic, uninstallManagedHooks, validateManagedHooks, + validateRecordedManagedHooks, validateSettings, }; diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index a91bfe854..8f0a92eab 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -2694,6 +2694,37 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (test('rejects wrapped matcher entry missing a required matcher', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { + SessionStart: [{ + id: 'test:missing-matcher', + hooks: [{ type: 'command', command: 'echo start' }] + }] + } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1); + assert.ok(result.stderr.includes('matcher'), result.stderr); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects wrapped matcher entry with an empty handlers array', () => { + const testDir = createTestDir(); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, JSON.stringify({ + hooks: { Stop: [{ id: 'test:empty-handlers', hooks: [] }] } + })); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 1); + assert.ok(result.stderr.includes('hooks'), result.stderr); + cleanupTestDir(testDir); + })) passed++; else failed++; + if (test('rejects wrapped matcher entry with whitespace-only id', () => { const testDir = createTestDir(); const hooksFile = path.join(testDir, 'hooks.json'); diff --git a/tests/lib/claude-settings.test.js b/tests/lib/claude-settings.test.js index 4fadb1c11..c31f191f9 100644 --- a/tests/lib/claude-settings.test.js +++ b/tests/lib/claude-settings.test.js @@ -10,6 +10,8 @@ const os = require('os'); const path = require('path'); const { + runWithSettingsLock, + materializeManagedHooks, inspectManagedHooks, mergeManagedHooks, parseSettings, @@ -78,15 +80,49 @@ function runTests() { { BogusEvent: [entry('bad:event', 'bad')] }, { SessionStart: [{ id: 'missing:hooks', matcher: '.*' }] }, { SessionStart: [{ id: 'bad:command', matcher: '.*', hooks: [{ type: 'command' }] }] }, - { - SessionStart: [{ id: 'shared' }], - Stop: [{ id: 'shared' }], - }, ]; for (const invalid of invalidValues) { assert.throws(() => validateManagedHooks(invalid), /managed hooks|hook entry|unique id/i); } + assert.throws( + () => validateManagedHooks({ + Stop: [entry('shared', 'a')], + SubagentStop: [entry('shared', 'b')], + }), + /expected globally unique id "shared"/ + ); + })) passed++; else failed++; + + if (test('materializes hook roots and rejects unresolved environment references', () => { + const source = { + hooks: { + Stop: [entry( + 'ecc:stop', + 'var e=process.env.CLAUDE_PLUGIN_ROOT; ' + + 'process.env.CLAUDE_PLUGIN_ROOT=r; ${CLAUDE_PLUGIN_ROOT}' + )], + }, + }; + const before = clone(source); + const materialized = materializeManagedHooks(source, '/opt/ecc'); + const command = materialized.Stop[0].hooks[0].command; + const encodedRoot = command.match(/Buffer\.from\('([^']+)','base64'\)/)[1]; + + assert.deepStrictEqual(source, before); + assert.ok(!command.includes('var e=process.env.CLAUDE_PLUGIN_ROOT;')); + assert.ok(!command.includes('${CLAUDE_PLUGIN_ROOT}')); + assert.strictEqual(Buffer.from(encodedRoot, 'base64').toString('utf8'), '/opt/ecc'); + assert.throws(() => materializeManagedHooks({}, '/opt/ecc'), /hooks object/); + assert.throws(() => materializeManagedHooks(source, ''), /target root/); + assert.throws( + () => materializeManagedHooks({ + hooks: { + Stop: [entry('ecc:stop', 'node -e "const e=process.env.CLAUDE_PLUGIN_ROOT"')], + }, + }, '/opt/ecc'), + /Unable to resolve CLAUDE_PLUGIN_ROOT/ + ); })) passed++; else failed++; if (test('replaces every plugin-root placeholder recursively and immutably', () => { @@ -234,6 +270,71 @@ function runTests() { } })) passed++; else failed++; + if (test('atomic settings updates honor an already-held lock', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-lock-held-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + try { + fs.writeFileSync(lockPath, JSON.stringify({ pid: process.pid }), { mode: 0o600 }); + updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, held: true } }), + { lockHeld: true } + ); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { held: true }); + assert.ok(fs.existsSync(lockPath)); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('settings lock release failures do not replace the primary update error', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-release-error-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const lockPath = `${settingsPath}.ecc.lock`; + try { + let caught; + try { + runWithSettingsLock(settingsPath, () => { + fs.rmSync(lockPath, { force: true }); + throw new Error('primary settings failure'); + }); + } catch (error) { + caught = error; + } + assert.ok(caught); + assert.strictEqual(caught.message, 'primary settings failure'); + assert.ok(caught.releaseError); + assert.strictEqual(caught.releaseError.code, 'ENOENT'); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('atomic settings updates refuse a symlinked destination', () => { + if (process.platform === 'win32') { + console.log(' (file symlink support is environment-dependent on Windows; skipping)'); + return; + } + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-symlink-')); + const realPath = path.join(tempDir, 'real.json'); + const settingsPath = path.join(tempDir, 'settings.json'); + try { + fs.writeFileSync(realPath, '{"theme":"dark"}\n', { mode: 0o600 }); + fs.symlinkSync(realPath, settingsPath); + assert.throws( + () => updateSettingsAtomic( + settingsPath, + settings => ({ settings: { ...settings, managed: true } }) + ), + error => error.code === 'ELOOP' || error.code === 'ECC_SETTINGS_CHANGED' + ); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(realPath, 'utf8')), { theme: 'dark' }); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + if (test('fresh merge appends managed entries while preserving unrelated settings and hooks', () => { const userEntry = { matcher: 'Bash', hooks: [{ type: 'command', command: 'user-hook' }] }; const settings = { @@ -540,6 +641,19 @@ function runTests() { assert.deepStrictEqual(result.retained, []); })) passed++; else failed++; + if (test('uninstall accepts structurally valid hooks from an older runtime contract', () => { + const recorded = { + LegacyEvent: [{ + id: 'ecc:legacy', + hooks: [{ type: 'legacy-handler', payload: { version: 1 } }], + }], + }; + const result = uninstallManagedHooks({ hooks: clone(recorded) }, recorded); + + assert.deepStrictEqual(result.settings, {}); + assert.deepStrictEqual(result.removed, [{ event: 'LegacyEvent', id: 'ecc:legacy' }]); + })) passed++; else failed++; + if (test('all settings transforms reject non-array hook events before changing data', () => { const settings = { hooks: { Stop: 'invalid' } }; const managed = { Stop: [entry('ecc:stop', 'expected')] }; diff --git a/tests/lib/install-executor.test.js b/tests/lib/install-executor.test.js index 0f9c656f6..6f64b540c 100644 --- a/tests/lib/install-executor.test.js +++ b/tests/lib/install-executor.test.js @@ -182,14 +182,7 @@ function runTests() { target: 'claude', moduleIds: ['hooks-runtime'], }); - const plan = { - ...rawPlan, - hookConsent: 'enabled', - statePreview: { - ...rawPlan.statePreview, - request: { ...rawPlan.statePreview.request, hookConsent: 'enabled' }, - }, - }; + const plan = withHookConsent(rawPlan, 'enabled'); const settingsPath = path.join(homeDir, '.claude', 'settings.json'); applyInstallPlanDirect(plan, { @@ -584,14 +577,7 @@ function runTests() { target: 'claude', moduleIds: ['hooks-runtime'], }); - const plan = { - ...rawPlan, - hookConsent: 'enabled', - statePreview: { - ...rawPlan.statePreview, - request: { ...rawPlan.statePreview.request, hookConsent: 'enabled' }, - }, - }; + const plan = withHookConsent(rawPlan, 'enabled'); const settingsPath = path.join(homeDir, '.claude', 'settings.json'); let settingsCommitCount = 0; fs.renameSync = function failAttributionCommit(sourcePath, destinationPath) { diff --git a/tests/lib/install-lifecycle.test.js b/tests/lib/install-lifecycle.test.js index b086ef2bf..fef01bd8a 100644 --- a/tests/lib/install-lifecycle.test.js +++ b/tests/lib/install-lifecycle.test.js @@ -23,7 +23,10 @@ const { readInstallState, writeInstallState, } = require('../../scripts/lib/install-state'); -const { materializeManagedHooks } = require('../../scripts/lib/install/claude-settings'); +const { + assertClaudeSettingsPath, + materializeManagedHooks, +} = require('../../scripts/lib/install/claude-settings'); const REPO_ROOT = path.join(__dirname, '..', '..'); const CURRENT_PACKAGE_VERSION = JSON.parse( @@ -3479,7 +3482,10 @@ function runTests() { })) passed++; else failed++; if (test('repair creates missing Claude settings with private permissions', () => { - if (process.platform === 'win32') return; + if (process.platform === 'win32') { + console.log(' (POSIX file modes unsupported on this platform; skipping)'); + return; + } const homeDir = createTempDir('install-lifecycle-claude-home-'); const projectRoot = createTempDir('install-lifecycle-project-'); @@ -3710,7 +3716,6 @@ function runTests() { assert.strictEqual(doctor.results[0].status, 'error'); assert.ok(doctor.results[0].issues.some(issue => ( issue.code === 'unsafe-managed-destination' - || issue.code === 'invalid-install-state' ))); assert.strictEqual(repair.results[0].status, 'error'); assert.match(repair.results[0].error, /final symlink/); @@ -3726,24 +3731,17 @@ function runTests() { } })) passed++; else failed++; - if (test('Claude settings lifecycle refuses a non-canonical settings destination', () => { + if (test('Claude settings path validation refuses a non-canonical destination', () => { const homeDir = createTempDir('install-lifecycle-claude-home-'); const projectRoot = createTempDir('install-lifecycle-project-'); try { const targetRoot = path.join(homeDir, '.claude'); const destinationPath = path.join(targetRoot, 'settings.local.json'); - const managedHooks = currentManagedHooks(targetRoot); fs.mkdirSync(targetRoot, { recursive: true }); assert.throws( - () => writeClaudeState(homeDir, { - operations: [ - managedOperation('update-claude-settings', destinationPath, { - managedHooks, - }), - ], - }), - /canonical Claude settings path/ + () => assertClaudeSettingsPath(destinationPath, targetRoot), + /outside the canonical settings file/ ); assert.ok(!fs.existsSync(destinationPath)); } finally { diff --git a/tests/lib/install-state.test.js b/tests/lib/install-state.test.js index ba8effaea..653a6bb10 100644 --- a/tests/lib/install-state.test.js +++ b/tests/lib/install-state.test.js @@ -169,28 +169,18 @@ function runTests() { }, }], }), - /managedHooks.*non-empty unique id/ - ); - assert.throws( - () => createInstallState({ - ...baseOptions, - operations: [{ - ...operation, - managedHooks: { - SessionStart: [{ - id: 'duplicate', - matcher: '.*', - hooks: [{ type: 'command', command: 'node start.js' }], - }], - Stop: [{ - id: 'duplicate', - hooks: [{ type: 'command', command: 'node stop.js' }], - }], - }, - }], - }), - /managedHooks.*globally unique id/ + /managedHooks.*Invalid hook entry/ ); + assert.doesNotThrow(() => createInstallState({ + ...baseOptions, + operations: [{ + ...operation, + managedHooks: { + SessionStart: [{ id: 'shared', hooks: [] }], + LegacyEvent: [{ id: 'shared', hooks: [{ type: 'legacy' }] }], + }, + }], + })); })) passed++; else failed++; if (test('writes and reads install-state from disk', () => { diff --git a/tests/scripts/manual-hook-install-docs.test.js b/tests/scripts/manual-hook-install-docs.test.js index f86ea670f..7851fdcc2 100644 --- a/tests/scripts/manual-hook-install-docs.test.js +++ b/tests/scripts/manual-hook-install-docs.test.js @@ -8,6 +8,12 @@ const path = require('path'); const README = path.join(__dirname, '..', '..', 'README.md'); const HOOKS_README = path.join(__dirname, '..', '..', 'hooks', 'README.md'); +const HOOK_REGISTRATION_PHRASE = + 'registers the resolved hook entries in `~/.claude/settings.json`'; + +function normalizeWhitespace(text) { + return text.replace(/\s+/g, ' '); +} function test(name, fn) { try { @@ -44,11 +50,11 @@ function runTests() { 'README should document the supported PowerShell hook install path' ); assert.ok( - readme.includes('%USERPROFILE%\\\\.claude'), + readme.includes('%USERPROFILE%\\.claude'), 'README should call out the correct Windows Claude config root' ); assert.ok( - readme.includes('registers the resolved\nhook entries in `~/.claude/settings.json`'), + normalizeWhitespace(readme).includes(HOOK_REGISTRATION_PHRASE), 'README should explain that manual installs register hooks in Claude settings' ); })) passed++; else failed++; @@ -67,7 +73,7 @@ function runTests() { 'hooks/README should document the supported PowerShell hook install path' ); assert.ok( - hooksReadme.includes('registers the resolved\nhook entries in `~/.claude/settings.json`'), + normalizeWhitespace(hooksReadme).includes(HOOK_REGISTRATION_PHRASE), 'hooks/README should explain that manual installs register hooks in Claude settings' ); })) passed++; else failed++; From dbe8bfbba9449e4baeefc27366b9b0eb31e3ad48 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:31:33 -0400 Subject: [PATCH 4/4] fix(install): pin Claude settings parent during atomic replacement Reject directory replacement after temporary file creation or staging, preserve unrelated files during cleanup, and retry settings edits observed before the final rename. Add three regression tests for the review findings. --- scripts/lib/atomic-write.js | 15 +++- scripts/lib/install/claude-settings.js | 23 ++++++- tests/lib/claude-settings.test.js | 95 ++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 2 deletions(-) diff --git a/scripts/lib/atomic-write.js b/scripts/lib/atomic-write.js index e3d41df0d..9e9e524fe 100644 --- a/scripts/lib/atomic-write.js +++ b/scripts/lib/atomic-write.js @@ -13,21 +13,34 @@ function writeFileAtomic(filePath, content, options = {}) { ); const mode = options.mode || 0o600; + if (options.validateParent) options.validateParent(); fs.mkdirSync(parentDir, { recursive: true }); let descriptor; try { + if (options.validateParent) options.validateParent(); descriptor = fs.openSync(tempPath, 'wx', mode); + if (options.validateParent) options.validateParent(); fs.writeFileSync(descriptor, content, { encoding: options.encoding || 'utf8' }); fs.fsyncSync(descriptor); fs.closeSync(descriptor); descriptor = undefined; + if (options.validateParent) options.validateParent(); + if (options.beforeRename) options.beforeRename(); fs.renameSync(tempPath, resolvedPath); } catch (error) { if (descriptor !== undefined) { fs.closeSync(descriptor); } - fs.rmSync(tempPath, { force: true }); + // If the parent was replaced, this pathname may now name somebody else's + // file. Leave the private staging file in its original directory. + let parentUnchanged = true; + try { + if (options.validateParent) options.validateParent(); + } catch (_error) { + parentUnchanged = false; + } + if (parentUnchanged) fs.rmSync(tempPath, { force: true }); throw error; } diff --git a/scripts/lib/install/claude-settings.js b/scripts/lib/install/claude-settings.js index dd34dd6fb..3c18668a6 100644 --- a/scripts/lib/install/claude-settings.js +++ b/scripts/lib/install/claude-settings.js @@ -389,9 +389,23 @@ function assertSettingsSnapshotUnchanged(settingsPath, snapshot) { function updateSettingsAtomic(settingsPath, transform, options = {}) { const update = () => { + const parentPath = path.dirname(path.resolve(settingsPath)); + const parentStats = fs.lstatSync(parentPath, { bigint: true }); + const validateParent = () => { + const current = fs.lstatSync(parentPath, { bigint: true }); + if ( + !current.isDirectory() || current.isSymbolicLink() + || current.dev !== parentStats.dev || current.ino !== parentStats.ino + ) { + const error = new Error(`Claude settings parent directory changed: ${parentPath}`); + error.code = 'ECC_SETTINGS_PARENT_CHANGED'; + throw error; + } + }; const maxAttempts = options.maxAttempts || 3; for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { try { + validateParent(); const snapshot = readSettingsSnapshot(settingsPath); const result = transform(snapshot.settings); if (typeof options.beforeCommit === 'function') options.beforeCommit(); @@ -399,7 +413,14 @@ function updateSettingsAtomic(settingsPath, transform, options = {}) { writeFileAtomic( settingsPath, `${JSON.stringify(result.settings, null, 2)}\n`, - { encoding: 'utf8', mode: snapshot.mode } + { + encoding: 'utf8', + mode: snapshot.mode, + validateParent, + beforeRename() { + assertSettingsSnapshotUnchanged(settingsPath, snapshot); + }, + } ); return result; } catch (error) { diff --git a/tests/lib/claude-settings.test.js b/tests/lib/claude-settings.test.js index c31f191f9..26d094fc4 100644 --- a/tests/lib/claude-settings.test.js +++ b/tests/lib/claude-settings.test.js @@ -48,6 +48,67 @@ function clone(value) { return JSON.parse(JSON.stringify(value)); } +function assertAtomicParentReplacementRejected(stage) { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-parent-race-')); + const targetRoot = path.join(tempDir, 'target'); + const parkedRoot = path.join(tempDir, 'parked'); + const victimRoot = path.join(tempDir, 'victim'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const victimPath = path.join(victimRoot, 'settings.json'); + const originalOpen = fs.openSync; + const originalFsync = fs.fsyncSync; + const targetContent = '{"target":true}\n'; + const victimContent = '{"victim":"preserve"}\n'; + let tempDescriptor; + let tempBasename; + let replaced = false; + const replaceParent = () => { + replaced = true; + fs.renameSync(targetRoot, parkedRoot); + fs.symlinkSync(victimRoot, targetRoot, process.platform === 'win32' ? 'junction' : 'dir'); + // A colliding path in the replacement directory must survive error cleanup. + fs.writeFileSync(path.join(victimRoot, tempBasename), 'unrelated replacement file'); + }; + try { + fs.mkdirSync(targetRoot); + fs.mkdirSync(victimRoot); + fs.writeFileSync(settingsPath, targetContent); + fs.writeFileSync(victimPath, victimContent); + fs.openSync = function(file, flags, ...args) { + const isTemp = typeof file === 'string' + && path.basename(file).startsWith('.settings.json.') && file.endsWith('.tmp'); + if (isTemp) tempBasename = path.basename(file); + if (isTemp && !replaced && stage === 'open') { + // Replace immediately after the temporary descriptor has been created. + const descriptor = originalOpen.call(fs, file, flags, ...args); + tempDescriptor = descriptor; + replaceParent(); + return descriptor; + } + const descriptor = originalOpen.call(fs, file, flags, ...args); + if (isTemp) tempDescriptor = descriptor; + return descriptor; + }; + fs.fsyncSync = function(descriptor) { + const result = originalFsync.call(fs, descriptor); + if (!replaced && stage === 'rename' && descriptor === tempDescriptor) replaceParent(); + return result; + }; + assert.throws( + () => updateSettingsAtomic(settingsPath, settings => ({ settings: { ...settings, managed: true } })), + /parent.*changed|changed.*parent/i + ); + assert.ok(replaced, 'must exercise a replacement inside the atomic writer'); + assert.strictEqual(fs.readFileSync(victimPath, 'utf8'), victimContent); + assert.strictEqual(fs.readFileSync(path.join(parkedRoot, 'settings.json'), 'utf8'), targetContent); + assert.strictEqual(fs.readFileSync(path.join(victimRoot, tempBasename), 'utf8'), 'unrelated replacement file'); + } finally { + fs.openSync = originalOpen; + fs.fsyncSync = originalFsync; + fs.rmSync(tempDir, { recursive: true, force: true }); + } +} + function runTests() { console.log('\n=== Testing install/claude-settings.js ===\n'); @@ -224,6 +285,40 @@ function runTests() { } })) passed++; else failed++; + for (const stage of ['open', 'rename']) { + if (test(`atomic settings updates reject parent replacement at ${stage} without touching its files`, () => { + assertAtomicParentReplacementRejected(stage); + })) passed++; else failed++; + } + + if (test('atomic settings updates preserve edits made while the replacement file is staged', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-late-edit-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const originalFsync = fs.fsyncSync; + let changed = false; + let fsyncCalls = 0; + try { + fs.writeFileSync(settingsPath, '{"theme":"initial"}\n'); + fs.fsyncSync = function(descriptor) { + const result = originalFsync.call(fs, descriptor); + // Lock creation is the first fsync; only change settings after the + // atomic writer has staged its first replacement payload. + fsyncCalls += 1; + if (!changed && fsyncCalls === 2) { + changed = true; + fs.writeFileSync(settingsPath, '{"theme":"late-edit"}\n'); + } + return result; + }; + updateSettingsAtomic(settingsPath, settings => ({ settings: { ...settings, managed: true } })); + assert.ok(changed); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { theme: 'late-edit', managed: true }); + } finally { + fs.fsyncSync = originalFsync; + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + if (test('atomic settings updates recover a stale invalid lock after its lease', () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-stale-lock-')); const settingsPath = path.join(tempDir, 'settings.json');