From e58bb26647793ed05c6e3b7d55d7fad63d556b4d Mon Sep 17 00:00:00 2001 From: labeedsoft-cloud Date: Fri, 21 Aug 2026 10:31:39 -0400 Subject: [PATCH] fix(config-protection): protect shared/base linter configs, not just entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PROTECTED_FILES` matches exact basenames, so it only ever guarded a tool's canonical entry point. Real repos split flat config across files — a shared `eslint.config.base.mjs` holding the ignore list and rule severities, imported by per-workspace `eslint.config.mjs` files, is the common monorepo shape. That meant the hook protected the leaves and left the trunk wide open: eslint.config.base.mjs <- ignore list + rule severities UNPROTECTED frontend/eslint.config.mjs <- imports the base protected backend/eslint.config.mjs <- imports the base protected An agent blocked from touching the two leaves could silently rewrite every rule severity in the file they both import. Hit in practice: two edits to `eslint.config.js` were correctly blocked, then an edit to `eslint.config.base.mjs` in the same repo went through unchallenged. Adds `PROTECTED_PATTERNS` alongside the existing Set, covering `.config..` and `.rc..` for the linters and formatters already listed. Case-insensitive, for the same reason the Set lookup is (#2543). Deliberately NOT matched: `vite.config.ts`, `vitest.config.ts`, `jest.config.js`, `playwright.config.ts`, `tsconfig.json`. This hook exists to stop a LINTER config being weakened in place of fixing the code; editing a bundler or test-runner config is ordinary work, and sweeping those in would make the hook obstructive. A second test pins that boundary so a future widening of the patterns cannot quietly cross it. The exact-name Set is untouched, so nothing previously protected becomes unprotected, and first-time creation stays allowed (the bootstrap path). Tests: 11 pass. The new regression test was verified failing against the unpatched hook first; the boundary test passes either way by design and is there as the control. --- scripts/hooks/config-protection.js | 36 +++++++++++++++- tests/hooks/config-protection.test.js | 60 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/scripts/hooks/config-protection.js b/scripts/hooks/config-protection.js index 2da5358c2..e6b713541 100644 --- a/scripts/hooks/config-protection.js +++ b/scripts/hooks/config-protection.js @@ -62,6 +62,40 @@ const PROTECTED_FILES = new Set([ '.markdownlintrc' ]); +/** + * Exact basenames only catch a tool's canonical entry point. Real repos split + * flat config across files: a shared `eslint.config.base.mjs` holding the + * ignore list and rule severities, imported by per-workspace + * `eslint.config.mjs` files. That is the common monorepo shape, and matching + * basenames alone protected the leaves while leaving the trunk — the file that + * actually carries the rules — freely editable. + * + * These patterns cover `.config..` and + * `.rc..` for the linters and formatters listed above. + * They are case-insensitive for the same reason the Set lookup above is. + * + * Deliberately NOT matched: build and test tooling — `vite.config.ts`, + * `vitest.config.ts`, `jest.config.js`, `playwright.config.ts`, + * `tsconfig.json`. This hook exists to stop a LINTER config being weakened in + * place of fixing the code; editing a bundler or test-runner config is + * ordinary work, and sweeping those in would make the hook obstructive. + */ +const PROTECTED_PATTERNS = [ + // eslint.config.base.mjs, prettier.config.shared.cjs, stylelint.config.local.js … + /^(eslint|prettier|stylelint|commitlint|oxlint|biome)\.config(\.[A-Za-z0-9_-]+)*\.(js|mjs|cjs|ts|mts|cts)$/i, + // .eslintrc.base.json, .prettierrc.shared.yml … + /^\.(eslintrc|prettierrc|stylelintrc|markdownlintrc)(\.[A-Za-z0-9_-]+)*\.(js|cjs|mjs|json|jsonc|yml|yaml|toml)$/i, + // biome.base.json, biome.shared.jsonc + /^biome(\.[A-Za-z0-9_-]+)*\.jsonc?$/i, +]; + +function isProtectedName(basename) { + const lower = basename.toLowerCase(); + return PROTECTED_FILES.has(basename) + || PROTECTED_FILES.has(lower) + || PROTECTED_PATTERNS.some((re) => re.test(basename)); +} + function parseInput(inputOrRaw) { if (typeof inputOrRaw === 'string') { try { @@ -101,7 +135,7 @@ function run(inputOrRaw, options = {}) { // silently overwrite the real config while the guard returned exit 0. // On genuinely case-sensitive filesystems this only costs a false positive // on a distinct file that differs from a protected name by case alone. - if (PROTECTED_FILES.has(basename) || PROTECTED_FILES.has(basename.toLowerCase())) { + if (isProtectedName(basename)) { // Allow first-time creation — there's no existing config to weaken. // The hook's purpose is blocking modifications; writing a brand-new // config file in a project that has none is a legitimate bootstrap diff --git a/tests/hooks/config-protection.test.js b/tests/hooks/config-protection.test.js index e383753ec..3e5c02c9d 100644 --- a/tests/hooks/config-protection.test.js +++ b/tests/hooks/config-protection.test.js @@ -357,6 +357,66 @@ function runTests() { passed++; else failed++; + if ( + test('blocks shared/base flat configs, not just the canonical entry point', () => { + // Monorepos split flat config: a shared `eslint.config.base.mjs` holding + // the ignore list and rule severities, imported by per-workspace + // `eslint.config.mjs` files. Matching basenames alone protected the + // leaves and left the trunk -- the file that carries the rules -- editable. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-base-')); + try { + const names = ['eslint.config.base.mjs', 'prettier.config.shared.cjs', '.eslintrc.base.json', 'ESLint.Config.Base.MJS']; + for (const name of names) { + const absPath = path.join(tmpDir, name); + fs.writeFileSync(absPath, '{}'); + + const result = runHook({ tool_name: 'Edit', tool_input: { file_path: absPath } }); + + assert.strictEqual(result.code, 2, 'Expected ' + name + ' to be blocked'); + assert.ok( + result.stderr.includes('BLOCKED: Modifying ' + name + ' is not allowed.'), + 'Expected block message for ' + name + ', got: ' + result.stderr + ); + } + } finally { + try { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } catch { + // best-effort cleanup + } + } + }) + ) + passed++; + else failed++; + + if ( + test('does not block build or test tooling configs', () => { + // Pins the boundary: this hook guards LINTER configs. A future widening + // of the patterns must not quietly start blocking ordinary work. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-allow-')); + try { + const names = ['vite.config.ts', 'vitest.config.ts', 'jest.config.js', 'playwright.config.ts', 'tsconfig.json']; + for (const name of names) { + const absPath = path.join(tmpDir, name); + fs.writeFileSync(absPath, '{}'); + + const result = runHook({ tool_name: 'Edit', tool_input: { file_path: absPath } }); + + assert.strictEqual(result.code, 0, 'Expected ' + name + ' to be allowed, stderr: ' + result.stderr); + } + } finally { + try { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } catch { + // best-effort cleanup + } + } + }) + ) + passed++; + else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); }