diff --git a/scripts/hooks/config-protection.js b/scripts/hooks/config-protection.js index 2da5358c2..9250ee5b8 100644 --- a/scripts/hooks/config-protection.js +++ b/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) { diff --git a/tests/hooks/config-protection.test.js b/tests/hooks/config-protection.test.js index 87bce5365..787fbd346 100644 --- a/tests/hooks/config-protection.test.js +++ b/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();