mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-29 21:15:16 +02:00
fix(config-protection): protect shared/base linter configs, not just entry points
`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
`<tool>.config.<qualifier>.<ext>` and `.<tool>rc.<qualifier>.<ext>` 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.
This commit is contained in:
@@ -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 `<tool>.config.<qualifier>.<ext>` and
|
||||
* `.<tool>rc.<qualifier>.<ext>` 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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user