fix(installer): validate hooks and settings before install

This commit is contained in:
Affaan Mustafa
2026-03-28 19:55:28 -04:00
parent d7e6bb242a
commit 47aa415b06
4 changed files with 76 additions and 26 deletions

0
scripts/ecc.js Normal file → Executable file
View File

0
scripts/install-apply.js Normal file → Executable file
View File

View File

@@ -5,6 +5,21 @@ const path = require('path');
const { writeInstallState } = require('../install-state'); const { writeInstallState } = require('../install-state');
function readJsonObject(filePath, label) {
let parsed;
try {
parsed = JSON.parse(fs.readFileSync(filePath, 'utf8'));
} catch (error) {
throw new Error(`Failed to parse ${label} at ${filePath}: ${error.message}`);
}
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
throw new Error(`Invalid ${label} at ${filePath}: expected a JSON object`);
}
return parsed;
}
function mergeHookEntries(existingEntries, incomingEntries) { function mergeHookEntries(existingEntries, incomingEntries) {
const mergedEntries = []; const mergedEntries = [];
const seenEntries = new Set(); const seenEntries = new Set();
@@ -22,39 +37,32 @@ function mergeHookEntries(existingEntries, incomingEntries) {
return mergedEntries; return mergedEntries;
} }
function mergeHooksIntoSettings(plan) { function findHooksSourcePath(plan, hooksDestinationPath) {
const operation = plan.operations.find(item => item.destinationPath === hooksDestinationPath);
return operation ? operation.sourcePath : null;
}
function buildMergedSettings(plan) {
if (!plan.adapter || plan.adapter.target !== 'claude') { if (!plan.adapter || plan.adapter.target !== 'claude') {
return; return null;
} }
const hooksJsonPath = path.join(plan.targetRoot, 'hooks', 'hooks.json'); const hooksDestinationPath = path.join(plan.targetRoot, 'hooks', 'hooks.json');
if (!fs.existsSync(hooksJsonPath)) { const hooksSourcePath = findHooksSourcePath(plan, hooksDestinationPath) || hooksDestinationPath;
return; if (!fs.existsSync(hooksSourcePath)) {
} return null;
let hooksConfig;
try {
hooksConfig = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8'));
} catch (error) {
throw new Error(`Failed to parse hooks config at ${hooksJsonPath}: ${error.message}`);
} }
const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config');
const incomingHooks = hooksConfig.hooks; const incomingHooks = hooksConfig.hooks;
if (!incomingHooks || typeof incomingHooks !== 'object' || Array.isArray(incomingHooks)) { if (!incomingHooks || typeof incomingHooks !== 'object' || Array.isArray(incomingHooks)) {
return; throw new Error(`Invalid hooks config at ${hooksSourcePath}: expected "hooks" to be a JSON object`);
} }
const settingsPath = path.join(plan.targetRoot, 'settings.json'); const settingsPath = path.join(plan.targetRoot, 'settings.json');
let settings = {}; let settings = {};
if (fs.existsSync(settingsPath)) { if (fs.existsSync(settingsPath)) {
try { settings = readJsonObject(settingsPath, 'existing settings');
settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
if (!settings || typeof settings !== 'object' || Array.isArray(settings)) {
throw new Error('root value must be a JSON object');
}
} catch (error) {
throw new Error(`Failed to parse existing settings at ${settingsPath}: ${error.message}`);
}
} }
const existingHooks = settings.hooks && typeof settings.hooks === 'object' && !Array.isArray(settings.hooks) const existingHooks = settings.hooks && typeof settings.hooks === 'object' && !Array.isArray(settings.hooks)
@@ -73,17 +81,29 @@ function mergeHooksIntoSettings(plan) {
hooks: mergedHooks, hooks: mergedHooks,
}; };
fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); return {
fs.writeFileSync(settingsPath, JSON.stringify(mergedSettings, null, 2) + '\n', 'utf8'); settingsPath,
mergedSettings,
};
} }
function applyInstallPlan(plan) { function applyInstallPlan(plan) {
const mergedSettingsPlan = buildMergedSettings(plan);
for (const operation of plan.operations) { for (const operation of plan.operations) {
fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true }); fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true });
fs.copyFileSync(operation.sourcePath, operation.destinationPath); fs.copyFileSync(operation.sourcePath, operation.destinationPath);
} }
mergeHooksIntoSettings(plan); if (mergedSettingsPlan) {
fs.mkdirSync(path.dirname(mergedSettingsPlan.settingsPath), { recursive: true });
fs.writeFileSync(
mergedSettingsPlan.settingsPath,
JSON.stringify(mergedSettingsPlan.mergedSettings, null, 2) + '\n',
'utf8'
);
}
writeInstallState(plan.installStatePath, plan.statePreview); writeInstallState(plan.installStatePath, plan.statePreview);
return { return {

View File

@@ -22,6 +22,8 @@ function readJson(filePath) {
return JSON.parse(fs.readFileSync(filePath, 'utf8')); return JSON.parse(fs.readFileSync(filePath, 'utf8'));
} }
const REPO_ROOT = path.join(__dirname, '..', '..');
function run(args = [], options = {}) { function run(args = [], options = {}) {
const env = { const env = {
...process.env, ...process.env,
@@ -435,6 +437,8 @@ function runTests() {
assert.strictEqual(result.code, 1); assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('Failed to parse existing settings at')); assert.ok(result.stderr.includes('Failed to parse existing settings at'));
assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n'); assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied on validation failure');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written on validation failure');
} finally { } finally {
cleanup(homeDir); cleanup(homeDir);
cleanup(projectDir); cleanup(projectDir);
@@ -453,15 +457,41 @@ function runTests() {
const result = run(['--profile', 'core'], { cwd: projectDir, homeDir }); const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(result.code, 1); assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('Failed to parse existing settings at')); assert.ok(result.stderr.includes('Invalid existing settings at'));
assert.ok(result.stderr.includes('root value must be a JSON object')); assert.ok(result.stderr.includes('expected a JSON object'));
assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n'); assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied on validation failure');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written on validation failure');
} finally { } finally {
cleanup(homeDir); cleanup(homeDir);
cleanup(projectDir); cleanup(projectDir);
} }
})) passed++; else failed++; })) passed++; else failed++;
if (test('fails when source hooks.json root is not an object before copying files', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
const sourceHooksPath = path.join(REPO_ROOT, 'hooks', 'hooks.json');
const originalHooks = fs.readFileSync(sourceHooksPath, 'utf8');
try {
fs.writeFileSync(sourceHooksPath, '[]\n');
const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('Invalid hooks config at'));
assert.ok(result.stderr.includes('expected a JSON object'));
const claudeRoot = path.join(homeDir, '.claude');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied when source hooks are invalid');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written when source hooks are invalid');
} finally {
fs.writeFileSync(sourceHooksPath, originalHooks);
cleanup(homeDir);
cleanup(projectDir);
}
})) passed++; else failed++;
if (test('installs from ecc-install.json and persists component selections', () => { if (test('installs from ecc-install.json and persists component selections', () => {
const homeDir = createTempDir('install-apply-home-'); const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-'); const projectDir = createTempDir('install-apply-project-');