From f59cfd57c26c65eeaba37ffdb95cd6abd055ee86 Mon Sep 17 00:00:00 2001 From: wellkilo Date: Mon, 7 Sep 2026 01:57:04 +0800 Subject: [PATCH] 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++;