Merge reviewed PR2845 configuration filename protections

Source-PR: https://github.com/affaan-m/ECC/pull/2845

scripts/hooks/config-protection.js

@@ -57,9 +57,40 @@ const PROTECTED_FILES = new Set([
   '.stylelintrc',
   '.stylelintrc.json',
   '.stylelintrc.yml',
+  '.stylelintrc.yaml',
+  '.stylelintrc.js',
+  '.stylelintrc.cjs',
+  '.stylelintrc.mjs',
+  // Stylelint's current spelling; only the legacy `.stylelintrc*` forms were
+  // listed, so a project using the documented `stylelint.config.js` had no
+  // protection at all.
+  'stylelint.config.js',
+  'stylelint.config.cjs',
+  'stylelint.config.mjs',
+  'stylelint.config.ts',
+  'stylelint.config.mts',
+  'stylelint.config.cts',
   '.markdownlint.json',
+  '.markdownlint.jsonc',
   '.markdownlint.yaml',
-  '.markdownlintrc'
+  '.markdownlint.yml',
+  '.markdownlint.cjs',
+  '.markdownlint.mjs',
+  '.markdownlintrc',
+  // markdownlint-cli2 reads its own config names, not `.markdownlint.*`.
+  '.markdownlint-cli2.jsonc',
+  '.markdownlint-cli2.yaml',
+  '.markdownlint-cli2.cjs',
+  '.markdownlint-cli2.mjs',
+  // Ignore files are the cheapest way to make a check pass without touching
+  // the code OR the config: adding one path to .eslintignore silences the
+  // failing file outright. Blocking the config while leaving its ignore list
+  // open left the hook's whole purpose one line away from being defeated.
+  // First-time creation stays allowed by the same existence check below.
+  '.eslintignore',
+  '.prettierignore',
+  '.stylelintignore',
+  '.markdownlintignore'
 ]);
 
 function parseInput(inputOrRaw) {

tests/hooks/config-protection.test.js

@@ -10,21 +10,58 @@ const { spawnSync } = require('child_process');
 
 const runner = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js');
 
+const SKIPPED = Symbol('skipped');
+
+function describeError(error) {
+  try {
+    return String(error && error.message ? error.message : error);
+  } catch {
+    return 'Unprintable thrown value';
+  }
+}
+
+function withOwnedDirectory(directory, action) {
+  let value;
+  let failed = false;
+  let primary;
+  try {
+    value = action(directory);
+  } catch (error) {
+    failed = true;
+    primary = error;
+  }
+  try {
+    fs.rmSync(directory, { recursive: true, force: true });
+  } catch (error) {
+    if (!failed) {
+      failed = true;
+      primary = error;
+    } else {
+      console.error(`    Cleanup error: ${describeError(error)}`);
+    }
+  }
+  if (failed) throw primary;
+  return value;
+}
+
 function test(name, fn) {
   try {
-    fn();
-    console.log(`  ✓ ${name}`);
-    return true;
+    if (fn() === SKIPPED) {
+      console.log(`  SKIP ${name}`);
+      return 'skipped';
+    }
+    console.log(`  PASS ${name}`);
+    return 'passed';
   } catch (error) {
-    console.log(`  ✗ ${name}`);
-    console.log(`    Error: ${error.message}`);
-    return false;
+    console.log(`  FAIL ${name}`);
+    console.log(`    Error: ${describeError(error)}`);
+    return 'failed';
   }
 }
 
 function runHook(input, env = {}) {
   const rawInput = typeof input === 'string' ? input : JSON.stringify(input);
-  const result = spawnSync('node', [runner, 'pre:config-protection', 'scripts/hooks/config-protection.js', 'standard,strict'], {
+  const result = spawnSync(process.execPath, [runner, 'pre:config-protection', 'scripts/hooks/config-protection.js', 'standard,strict'], {
     input: rawInput,
     encoding: 'utf8',
     env: {
@@ -45,7 +82,7 @@ function runHook(input, env = {}) {
 
 function runCustomHook(pluginRoot, hookId, relScriptPath, input, env = {}) {
   const rawInput = typeof input === 'string' ? input : JSON.stringify(input);
-  const result = spawnSync('node', [runner, hookId, relScriptPath, 'standard,strict'], {
+  const result = spawnSync(process.execPath, [runner, hookId, relScriptPath, 'standard,strict'], {
     input: rawInput,
     encoding: 'utf8',
     env: {
@@ -68,13 +105,11 @@ function runCustomHook(pluginRoot, hookId, relScriptPath, input, env = {}) {
 function runTests() {
   console.log('\n=== Testing config-protection ===\n');
 
-  let passed = 0;
-  let failed = 0;
+  const results = [];
 
-  if (
+  results.push(
     test('blocks protected config file edits through run-with-flags', () => {
-      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
         const absPath = path.join(tmpDir, '.eslintrc.js');
         fs.writeFileSync(absPath, 'module.exports = {};');
 
@@ -90,19 +125,11 @@ function runTests() {
         assert.strictEqual(result.code, 2, 'Expected protected config edit to be blocked');
         assert.strictEqual(result.stdout, '', 'Blocked hook should not echo raw input');
         assert.ok(result.stderr.includes('BLOCKED: Modifying .eslintrc.js is not allowed.'), `Expected block message, got: ${result.stderr}`);
-      } finally {
-        try {
-          fs.rmSync(tmpDir, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('passes through safe file edits unchanged', () => {
       const input = {
         tool_name: 'Write',
@@ -117,11 +144,9 @@ function runTests() {
       assert.strictEqual(result.stdout, '', 'Allowed edits should not echo raw hook input');
       assert.strictEqual(result.stderr, '', 'Expected no stderr for safe edits');
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('blocks truncated protected config payloads instead of failing open', () => {
       const rawInput = JSON.stringify({
         tool_name: 'Write',
@@ -137,14 +162,11 @@ function runTests() {
       assert.ok(result.stderr.includes('Hook input exceeded 1048576 bytes'), `Expected size warning, got: ${result.stderr}`);
       assert.ok(result.stderr.includes('truncated payload'), `Expected truncated payload warning, got: ${result.stderr}`);
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('allows first-time creation of a protected config file', () => {
-      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
         const absPath = path.join(tmpDir, 'eslint.config.mjs');
         const input = {
           tool_name: 'Write',
@@ -158,22 +180,13 @@ function runTests() {
         assert.strictEqual(result.code, 0, `Expected exit 0 for first-time creation, got ${result.code}; stderr: ${result.stderr}`);
         assert.strictEqual(result.stdout, '', 'Allowed creation should not echo raw hook input');
         assert.strictEqual(result.stderr, '', `Expected no stderr for first-time creation, got: ${result.stderr}`);
-      } finally {
-        try {
-          fs.rmSync(tmpDir, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('allows first-time creation when the parent directory does not exist yet', () => {
-      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
         // Path under a non-existent subdirectory — statSync returns ENOENT
         // on the final segment, which should be treated as "does not exist"
         // and allow the write. (Agent or CLI is expected to create parents
@@ -190,22 +203,13 @@ function runTests() {
         const result = runHook(input);
         assert.strictEqual(result.code, 0, `Expected exit 0 for ENOENT path, got ${result.code}; stderr: ${result.stderr}`);
         assert.strictEqual(result.stdout, '', 'Allowed missing paths should not echo raw hook input');
-      } finally {
-        try {
-          fs.rmSync(tmpDir, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('blocks protected paths that exist as a dangling symlink', () => {
-      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
         const missingTarget = path.join(tmpDir, 'nowhere.js');
         const linkPath = path.join(tmpDir, '.eslintrc.js');
         try {
@@ -215,7 +219,7 @@ function runTests() {
           // symlinks. Skip cleanly rather than fail the suite.
           if (err.code === 'EPERM' || err.code === 'EACCES') {
             console.log('    (skipped: symlink creation not permitted here)');
-            return;
+            return SKIPPED;
           }
           throw err;
         }
@@ -232,22 +236,13 @@ function runTests() {
         assert.strictEqual(result.code, 2, `Expected exit 2 for dangling symlink, got ${result.code}; stderr: ${result.stderr}`);
         assert.strictEqual(result.stdout, '', 'Blocked hook should not echo raw input');
         assert.ok(result.stderr.includes('BLOCKED: Modifying .eslintrc.js is not allowed.'), `Expected block message, got: ${result.stderr}`);
-      } finally {
-        try {
-          fs.rmSync(tmpDir, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('blocks case-variant writes that resolve to an existing protected config', () => {
-      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
         const realPath = path.join(tmpDir, '.eslintrc.js');
         const variantPath = path.join(tmpDir, '.ESLINTRC.JS');
         fs.writeFileSync(realPath, 'module.exports = { rules: { "no-explicit-any": "error" } };');
@@ -264,7 +259,7 @@ function runTests() {
         }
         if (!sameFile) {
           console.log('    (skipped: case-sensitive filesystem)');
-          return;
+          return SKIPPED;
         }
 
         const result = runHook({
@@ -277,22 +272,13 @@ function runTests() {
 
         assert.strictEqual(result.code, 2, `Case-variant write must be blocked: it overwrites ${path.basename(realPath)} on this filesystem. Got ${result.code}; stderr: ${result.stderr}`);
         assert.strictEqual(result.stdout, '', 'Blocked hook should not echo raw input');
-      } finally {
-        try {
-          fs.rmSync(tmpDir, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
     test('still blocks writes to an existing protected config file', () => {
-      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
         const absPath = path.join(tmpDir, '.eslintrc.js');
         fs.writeFileSync(absPath, 'module.exports = { rules: {} };');
 
@@ -308,25 +294,111 @@ function runTests() {
         assert.strictEqual(result.code, 2, 'Expected exit 2 when modifying an existing protected config');
         assert.strictEqual(result.stdout, '', 'Blocked hook should not echo raw input');
         assert.ok(result.stderr.includes('BLOCKED: Modifying .eslintrc.js is not allowed.'), `Expected block message, got: ${result.stderr}`);
-      } finally {
-        try {
-          fs.rmSync(tmpDir, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  if (
+  results.push(
+    test('blocks edits to an existing linter ignore file', () => {
+      // Adding one path to .eslintignore silences a failing file without
+      // touching the code or the config — the exact move this hook exists to
+      // stop, and it was allowed. Measured before the fix: .eslintignore,
+      // .prettierignore, .stylelintignore and .markdownlintignore all
+      // returned exit 0.
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
+        for (const name of [
+          '.eslintignore',
+          '.prettierignore',
+          '.stylelintignore',
+          '.markdownlintignore'
+        ]) {
+          const absPath = path.join(tmpDir, name);
+          fs.writeFileSync(absPath, 'dist/\n');
+
+          const result = runHook({
+            tool_name: 'Edit',
+            tool_input: { file_path: absPath, content: 'dist/\nsrc/failing-file.ts\n' }
+          });
+
+          assert.strictEqual(result.code, 2, `Expected exit 2 for ${name}, got ${result.code}`);
+          assert.ok(
+            result.stderr.includes(`BLOCKED: Modifying ${name} is not allowed.`),
+            `Expected block message for ${name}, got: ${result.stderr}`
+          );
+        }
+      });
+    })
+  );
+
+  results.push(
+    test('blocks the current stylelint and markdownlint config spellings', () => {
+      // Only the legacy `.stylelintrc*` / `.markdownlint.json` names were
+      // listed, so a project on the documented `stylelint.config.js` or
+      // markdownlint-cli2 had no protection at all.
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
+        for (const name of [
+          'stylelint.config.js',
+          'stylelint.config.cjs',
+          'stylelint.config.mjs',
+          'stylelint.config.ts',
+          'stylelint.config.mts',
+          'stylelint.config.cts',
+          '.stylelintrc.yaml',
+          '.stylelintrc.js',
+          '.stylelintrc.cjs',
+          '.stylelintrc.mjs',
+          '.markdownlint.jsonc',
+          '.markdownlint.yml',
+          '.markdownlint.cjs',
+          '.markdownlint.mjs',
+          '.markdownlint-cli2.jsonc',
+          '.markdownlint-cli2.yaml',
+          '.markdownlint-cli2.cjs',
+          '.markdownlint-cli2.mjs',
+          '.ESLINTIGNORE'
+        ]) {
+          const absPath = path.join(tmpDir, name);
+          fs.writeFileSync(absPath, '{}');
+
+          const result = runHook({
+            tool_name: 'Edit',
+            tool_input: { file_path: absPath, content: '{"rules": {}}' }
+          });
+
+          assert.strictEqual(result.code, 2, `Expected exit 2 for ${name}, got ${result.code}`);
+        }
+      });
+    })
+  );
+
+  results.push(
+    test('a first-time ignore file and a lookalike name are still allowed', () => {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-')), tmpDir => {
+        // Scaffolding a brand-new ignore file is the same legitimate bootstrap
+        // path the hook already allows for configs.
+        const fresh = runHook({
+          tool_name: 'Write',
+          tool_input: { file_path: path.join(tmpDir, '.prettierignore'), content: 'dist/\n' }
+        });
+        assert.strictEqual(fresh.code, 0, `Expected exit 0 for a new ignore file, got ${fresh.code}`);
+
+        // A file that merely looks like one must not be swept up.
+        const lookalike = path.join(tmpDir, '.eslintignore.bak');
+        fs.writeFileSync(lookalike, 'dist/\n');
+        const result = runHook({
+          tool_name: 'Edit',
+          tool_input: { file_path: lookalike, content: 'dist/\nsrc/\n' }
+        });
+        assert.strictEqual(result.code, 0, `Expected exit 0 for ${path.basename(lookalike)}`);
+      });
+    })
+  );
+
+  results.push(
     test('legacy hooks do not echo raw input when they fail without stdout', () => {
-      const pluginRoot = path.join(__dirname, '..', `tmp-runner-plugin-${Date.now()}`);
-      const scriptDir = path.join(pluginRoot, 'scripts', 'hooks');
-      const scriptPath = path.join(scriptDir, 'legacy-block.js');
-
-      try {
+      return withOwnedDirectory(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-legacy-')), pluginRoot => {
+        const scriptDir = path.join(pluginRoot, 'scripts', 'hooks');
+        const scriptPath = path.join(scriptDir, 'legacy-block.js');
         fs.mkdirSync(scriptDir, { recursive: true });
         fs.writeFileSync(scriptPath, '#!/usr/bin/env node\nprocess.stderr.write("blocked by legacy hook\\n");\nprocess.exit(2);\n');
 
@@ -342,20 +414,15 @@ function runTests() {
         assert.strictEqual(result.code, 2, 'Expected failing legacy hook exit code to propagate');
         assert.strictEqual(result.stdout, '', 'Expected failing legacy hook to avoid raw passthrough');
         assert.ok(result.stderr.includes('blocked by legacy hook'), `Expected legacy hook stderr, got: ${result.stderr}`);
-      } finally {
-        try {
-          fs.rmSync(pluginRoot, { recursive: true, force: true });
-        } catch {
-          // best-effort cleanup
-        }
-      }
+      });
     })
-  )
-    passed++;
-  else failed++;
+  );
 
-  console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
-  process.exit(failed > 0 ? 1 : 0);
+  const passed = results.filter(result => result === 'passed').length;
+  const failed = results.filter(result => result === 'failed').length;
+  const skipped = results.filter(result => result === 'skipped').length;
+  console.log(`\nResults: Passed: ${passed}, Failed: ${failed}, Skipped: ${skipped}`);
+  process.exitCode = failed > 0 ? 1 : 0;
 }
 
 runTests();