From e8afb148d305e1c1c61791f6869397d79a2219dc Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Fri, 26 Jun 2026 17:24:08 +0200 Subject: [PATCH] fix(acr): conflict-detector segregates plugin-bundled configs (M-BUG-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CNF compared every discovered settings.json/hooks.json pairwise regardless of origin, so it treated installed plugins' bundled configs — each plugin's own settings.json/hooks.json plus its shipped test fixtures and examples under ~/.claude/plugins/ — as if they were the user's authored cascade. A "conflict" between two plugins' bundled test fixtures is not something a user can resolve, yet these dominated the count: 339 CNF findings on this machine (315 high-sev permission allow/deny "conflicts", 18 duplicate-hook, 6 settings-key), almost all sourced from plugin-internal fixtures. The Conflicts grade was F on pure noise. Fix: CNF excludes any file whose path is under `.claude/plugins/` from conflict analysis (new isPluginBundled predicate; absPath marker). Kept CNF-local rather than a discovery-level skip on purpose: an active plugin's contributed hooks.json/.mcp.json legitimately lives in plugins/cache and other scanners need it — only conflict analysis must ignore plugin-bundled files. Same class as M-BUG-8 (non-live config trees treated as live). Suite 1344/0 (+3: plugin-bundled exclusion, discovery-side sanity, over-exclusion guard). Frozen v5.0.0 + SC-5 snapshots untouched (marketplace-medium has no plugins/ paths), no re-seed. Dogfood ~/.claude CNF 339->0 (F-grade was 100% plugin-bundled noise; the ~3 genuine user-scope local settings have no actual conflicting keys, matching the plan C5 "real surface ~3 files" prediction). Follow-up (not in this fix): classifyScope tags plugin-bundled files by checking basePath instead of the file's own path, so scope:'plugin' is effectively dead for a ~/.claude-rooted scan. Fixing it would let every scanner trust the scope field, but that is a discovery-layer change beyond this bug's scope. --- scanners/conflict-detector.mjs | 25 +++++- tests/scanners/conflict-detector.test.mjs | 97 ++++++++++++++++++++++- 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/scanners/conflict-detector.mjs b/scanners/conflict-detector.mjs index 44038b2..fbbcb23 100644 --- a/scanners/conflict-detector.mjs +++ b/scanners/conflict-detector.mjs @@ -5,6 +5,7 @@ * Finding IDs: CA-CNF-NNN */ +import { sep } from 'node:path'; import { readTextFile } from './lib/file-discovery.mjs'; import { finding, scannerResult } from './lib/output.mjs'; import { SEVERITY } from './lib/severity.mjs'; @@ -17,6 +18,22 @@ const SCANNER = 'CNF'; // Keys checked separately or not meaningful to compare const SKIP_KEYS = new Set(['$schema', 'hooks', 'permissions']); +// Files under `.claude/plugins/` are shipped by installed plugins — the plugin's +// own settings.json/hooks.json plus bundled test fixtures and examples. They are +// not the user's authored cascade and a "conflict" between them is not something +// the user can resolve, so they must be excluded from cross-scope conflict +// analysis. (Other scanners still need active plugin config, so this exclusion is +// CNF-local, not a discovery-level skip. M-BUG-2.) +const PLUGIN_TREE_MARKER = `.claude${sep}plugins${sep}`; + +/** + * @param {import('./lib/file-discovery.mjs').ConfigFile} file + * @returns {boolean} true if the file is shipped by an installed plugin + */ +function isPluginBundled(file) { + return file.absPath.includes(PLUGIN_TREE_MARKER); +} + /** * Flatten an object's top-level keys into a simple key→value map. * Only first level — we compare top-level settings, not nested. @@ -63,10 +80,10 @@ export async function scan(targetPath, discovery) { const start = Date.now(); const findings = []; - // Collect settings files - const settingsFiles = discovery.files.filter(f => f.type === 'settings-json'); - // Collect hooks files - const hooksFiles = discovery.files.filter(f => f.type === 'hooks-json'); + // Collect settings files (excluding plugin-bundled — see PLUGIN_TREE_MARKER) + const settingsFiles = discovery.files.filter(f => f.type === 'settings-json' && !isPluginBundled(f)); + // Collect hooks files (excluding plugin-bundled) + const hooksFiles = discovery.files.filter(f => f.type === 'hooks-json' && !isPluginBundled(f)); const totalFiles = settingsFiles.length + hooksFiles.length; diff --git a/tests/scanners/conflict-detector.test.mjs b/tests/scanners/conflict-detector.test.mjs index f0c3226..cc06cc2 100644 --- a/tests/scanners/conflict-detector.test.mjs +++ b/tests/scanners/conflict-detector.test.mjs @@ -1,6 +1,6 @@ import { describe, it, beforeEach, before, after } from 'node:test'; import assert from 'node:assert/strict'; -import { resolve, join } from 'node:path'; +import { resolve, join, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { mkdir, writeFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -208,3 +208,98 @@ describe('CNF scanner — cache exclusion drops duplicate-hook count (B3)', () = `cache exclusion must drop CNF count: include=${includeCount} exclude=${excludeCount}`); }); }); + +// M-BUG-2: CNF must segregate plugin-bundled configs from the user's authored +// cascade. Installed plugins ship their OWN settings.json / hooks.json — plus +// bundled test fixtures and examples — under ~/.claude/plugins/. None of these +// are user-authored config the user can edit, and a "conflict" between two +// plugins' bundled files is not something the user can resolve. Yet CNF compared +// every settings-json/hooks-json pairwise regardless of origin, inflating the +// Conflicts grade to F with hundreds of bogus findings (339 on this machine, +// ~315 of them high-severity permission "conflicts" between plugin test +// fixtures). CNF must exclude any file whose path is under `.claude/plugins/`. +// The exclusion belongs in CNF, NOT discovery: an active plugin's contributed +// hooks.json/.mcp.json legitimately lives in plugins/cache and other scanners +// need it — only conflict analysis must ignore it. +describe('CNF scanner — excludes plugin-bundled configs (M-BUG-2)', () => { + let dir, result, discovery; + + before(async () => { + dir = join(tmpdir(), `config-audit-cnf-mbug2-${Date.now()}`); + // Live, user-authored cascade (project-scope settings). + const liveClaude = join(dir, '.claude'); + await mkdir(liveClaude, { recursive: true }); + await writeFile(join(liveClaude, 'settings.json'), JSON.stringify({ + model: 'opus', + permissions: { deny: ['Bash(curl:*)'] }, + hooks: { PreToolUse: [{ matcher: 'Edit' }] }, + })); + // An installed plugin ships its OWN settings.json + hooks.json (active version). + const p1 = join(liveClaude, 'plugins', 'cache', 'mkt', 'p1', '1.0.0'); + await mkdir(join(p1, '.claude'), { recursive: true }); + await writeFile(join(p1, '.claude', 'settings.json'), JSON.stringify({ + model: 'sonnet', + permissions: { allow: ['Bash(curl:*)'] }, + hooks: { PreToolUse: [{ matcher: 'Edit' }] }, + })); + await mkdir(join(p1, 'hooks'), { recursive: true }); + await writeFile(join(p1, 'hooks', 'hooks.json'), JSON.stringify({ + hooks: { PreToolUse: [{ matcher: 'Edit' }] }, + })); + // Another plugin ships a TEST FIXTURE settings.json — pure noise, never live. + const p2fix = join(liveClaude, 'plugins', 'cache', 'mkt', 'p2', '2.0.0', + 'tests', 'fixtures', 'proj', '.claude'); + await mkdir(p2fix, { recursive: true }); + await writeFile(join(p2fix, 'settings.json'), JSON.stringify({ + model: 'haiku', + permissions: { allow: ['Bash(rm:*)'] }, + })); + + resetCounter(); + discovery = await discoverConfigFiles(dir); + result = await scan(dir, discovery); + }); + + after(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it('discovery surfaces the plugin-bundled settings (exclusion must be CNF-side, not discovery-side)', () => { + const pluginSettings = discovery.files.filter( + f => f.type === 'settings-json' && f.absPath.includes(`.claude${sep}plugins${sep}`)); + assert.ok(pluginSettings.length >= 2, + `expected ≥2 plugin-bundled settings discovered; got ${pluginSettings.length}`); + }); + + it('does not flag any conflict sourced from plugin-bundled configs', () => { + assert.equal(result.findings.length, 0, + `expected 0 CNF findings (only noise is plugin-bundled); got ${result.findings.length}: ${result.findings.map(f => f.title).join(', ')}`); + }); +}); + +// Guard against over-exclusion: the fix must NOT silence genuine conflicts +// between the user's own authored files (user/project/local), which are not +// under `.claude/plugins/`. +describe('CNF scanner — genuine live conflict still flagged (M-BUG-2 guard)', () => { + let dir, result; + + before(async () => { + dir = join(tmpdir(), `config-audit-cnf-mbug2-guard-${Date.now()}`); + const claude = join(dir, '.claude'); + await mkdir(claude, { recursive: true }); + await writeFile(join(claude, 'settings.json'), JSON.stringify({ model: 'opus' })); + await writeFile(join(claude, 'settings.local.json'), JSON.stringify({ model: 'haiku' })); + resetCounter(); + const discovery = await discoverConfigFiles(dir); + result = await scan(dir, discovery); + }); + + after(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it('still flags the genuine cross-scope key conflict between user-authored files', () => { + assert.ok(result.findings.some(f => f.title.includes('model')), + `expected a genuine "model" conflict; got: ${result.findings.map(f => f.title).join(', ') || '(none)'}`); + }); +});