fix(installer): validate hooks and settings before install

Author: affaan-m

scripts/ecc.js

@@ -5,6 +5,21 @@ const path = require('path');
 
 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) {
   const mergedEntries = [];
   const seenEntries = new Set();
@@ -22,39 +37,32 @@ function mergeHookEntries(existingEntries, incomingEntries) {
   return mergedEntries;
 }
 
-function mergeHooksIntoSettings(plan) {
-  if (!plan.adapter || plan.adapter.target !== 'claude') {
-    return;
-  }
+function findHooksSourcePath(plan, hooksDestinationPath) {
+  const operation = plan.operations.find(item => item.destinationPath === hooksDestinationPath);
+  return operation ? operation.sourcePath : null;
+}
 
-  const hooksJsonPath = path.join(plan.targetRoot, 'hooks', 'hooks.json');
-  if (!fs.existsSync(hooksJsonPath)) {
-    return;
+function buildMergedSettings(plan) {
+  if (!plan.adapter || plan.adapter.target !== 'claude') {
+    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 hooksDestinationPath = path.join(plan.targetRoot, 'hooks', 'hooks.json');
+  const hooksSourcePath = findHooksSourcePath(plan, hooksDestinationPath) || hooksDestinationPath;
+  if (!fs.existsSync(hooksSourcePath)) {
+    return null;
   }
 
+  const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config');
   const incomingHooks = hooksConfig.hooks;
   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');
   let settings = {};
   if (fs.existsSync(settingsPath)) {
-    try {
-      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}`);
-    }
+    settings = readJsonObject(settingsPath, 'existing settings');
   }
 
   const existingHooks = settings.hooks && typeof settings.hooks === 'object' && !Array.isArray(settings.hooks)
@@ -73,17 +81,29 @@ function mergeHooksIntoSettings(plan) {
     hooks: mergedHooks,
   };
 
-  fs.mkdirSync(path.dirname(settingsPath), { recursive: true });
-  fs.writeFileSync(settingsPath, JSON.stringify(mergedSettings, null, 2) + '\n', 'utf8');
+  return {
+    settingsPath,
+    mergedSettings,
+  };
 }
 
 function applyInstallPlan(plan) {
+  const mergedSettingsPlan = buildMergedSettings(plan);
+
   for (const operation of plan.operations) {
     fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true });
     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);
 
   return {

scripts/install-apply.js

@@ -22,6 +22,8 @@ function readJson(filePath) {
   return JSON.parse(fs.readFileSync(filePath, 'utf8'));
 }
 
+const REPO_ROOT = path.join(__dirname, '..', '..');
+
 function run(args = [], options = {}) {
   const env = {
     ...process.env,
@@ -435,6 +437,8 @@ function runTests() {
       assert.strictEqual(result.code, 1);
       assert.ok(result.stderr.includes('Failed to parse existing settings at'));
       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 {
       cleanup(homeDir);
       cleanup(projectDir);
@@ -453,10 +457,36 @@ function runTests() {
 
       const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
       assert.strictEqual(result.code, 1);
-      assert.ok(result.stderr.includes('Failed to parse existing settings at'));
-      assert.ok(result.stderr.includes('root value must be a JSON object'));
+      assert.ok(result.stderr.includes('Invalid existing settings at'));
+      assert.ok(result.stderr.includes('expected a JSON object'));
       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 {
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) 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);
     }