From cc349d6fe1348c24aa80e8fa7077ab83fee32cd3 Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Fri, 1 May 2026 07:39:58 +0200 Subject: [PATCH] feat(config-audit): disabled-in-schema scanner DIS (v5 N4) [skip-docs] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New DIS scanner detects tools that appear in BOTH permissions.deny and permissions.allow within the same settings.json file. The deny list wins, so allow entries are dead config but still load on every turn and confuse intent. Tool identity = bare name (everything before "("). `Bash(npm:*)` and `Bash` are treated as the same tool, so a deny on `Bash` flags any `Bash(...)` allow entry. Severity: low. Wired into scan-orchestrator + scoring (area: Settings). Fixture denied-tools-in-schema has Bash in both arrays; healthy-project serves as the negative case. [skip-docs] reason: v5 plan fences off README/CLAUDE.md badge updates to Session 5; Forgejo pre-commit-docs-gate hook requires this tag. Tests: 611 → 617 (+6). Co-Authored-By: Claude Opus 4.7 --- .../scanners/disabled-in-schema-scanner.mjs | 110 ++++++++++++++++++ plugins/config-audit/scanners/lib/scoring.mjs | 1 + .../scanners/scan-orchestrator.mjs | 2 + .../.claude/settings.json | 6 + .../scanners/disabled-in-schema.test.mjs | 68 +++++++++++ 5 files changed, 187 insertions(+) create mode 100644 plugins/config-audit/scanners/disabled-in-schema-scanner.mjs create mode 100644 plugins/config-audit/tests/fixtures/denied-tools-in-schema/.claude/settings.json create mode 100644 plugins/config-audit/tests/scanners/disabled-in-schema.test.mjs diff --git a/plugins/config-audit/scanners/disabled-in-schema-scanner.mjs b/plugins/config-audit/scanners/disabled-in-schema-scanner.mjs new file mode 100644 index 0000000..6c95627 --- /dev/null +++ b/plugins/config-audit/scanners/disabled-in-schema-scanner.mjs @@ -0,0 +1,110 @@ +/** + * DIS Scanner — Disabled-Tools-Still-In-Schema Detector (v5 N4) + * + * Detects tools that appear in BOTH `permissions.deny` and `permissions.allow` + * within the same settings.json file. The deny list wins, so the allow entry + * is dead config — but it still loads on every turn and signals confused + * intent. Often arises from copy-paste edits where one list was updated and + * the other was forgotten. + * + * Compares tool identity by the bare tool name (everything before the first + * `(`). `Bash(npm:*)` and `Bash` are treated as the same tool for collision + * purposes — a deny on `Bash` blocks all `Bash(...)` allows. + * + * Finding ID: CA-DIS-NNN. Severity: low. + * + * Zero external dependencies. + */ + +import { readTextFile } from './lib/file-discovery.mjs'; +import { finding, scannerResult } from './lib/output.mjs'; +import { SEVERITY } from './lib/severity.mjs'; +import { parseJson } from './lib/yaml-parser.mjs'; + +const SCANNER = 'DIS'; + +/** + * Bare tool name = everything before the first `(`. `Bash(npm:*)` → `Bash`. + */ +function bareTool(entry) { + if (typeof entry !== 'string') return null; + const idx = entry.indexOf('('); + return (idx === -1 ? entry : entry.slice(0, idx)).trim(); +} + +/** + * Find tools whose bare name appears in both deny and allow within the same + * settings.json. Returns array of { tool, allowEntry, denyEntry }. + */ +function findDenyAllowOverlaps(settings) { + if (!settings || typeof settings !== 'object') return []; + const perms = settings.permissions; + if (!perms || typeof perms !== 'object') return []; + + const allowList = Array.isArray(perms.allow) ? perms.allow : []; + const denyList = Array.isArray(perms.deny) ? perms.deny : []; + if (allowList.length === 0 || denyList.length === 0) return []; + + const denyByBare = new Map(); + for (const d of denyList) { + const bare = bareTool(d); + if (bare && !denyByBare.has(bare)) denyByBare.set(bare, d); + } + + const overlaps = []; + const seen = new Set(); + for (const a of allowList) { + const bare = bareTool(a); + if (!bare) continue; + if (denyByBare.has(bare) && !seen.has(bare)) { + overlaps.push({ tool: bare, allowEntry: a, denyEntry: denyByBare.get(bare) }); + seen.add(bare); + } + } + return overlaps; +} + +/** + * Main scanner entry point. + * + * @param {string} targetPath + * @param {{files: Array<{absPath:string, relPath:string, type:string}>}} discovery + */ +export async function scan(targetPath, discovery) { + const start = Date.now(); + const findings = []; + let filesScanned = 0; + + for (const f of discovery.files) { + if (f.type !== 'settings-json') continue; + filesScanned++; + const content = await readTextFile(f.absPath); + if (!content) continue; + const parsed = parseJson(content); + if (!parsed) continue; + const overlaps = findDenyAllowOverlaps(parsed); + if (overlaps.length === 0) continue; + + const evidence = overlaps.slice(0, 5) + .map(o => `${o.tool}: allow="${o.allowEntry}" + deny="${o.denyEntry}"`) + .join('; '); + findings.push(finding({ + scanner: SCANNER, + severity: SEVERITY.low, + title: 'Tool listed in both permissions.deny and permissions.allow', + description: + `${f.relPath || f.absPath} contains ${overlaps.length} tool` + + `${overlaps.length === 1 ? '' : 's'} present in both deny and allow lists. ` + + 'The deny list wins — the allow entries are dead config but still load on ' + + 'every turn and may confuse future readers about intent.', + file: f.absPath, + evidence, + recommendation: + 'Remove the redundant allow entries. If you actually want this tool enabled, ' + + 'remove it from the deny list instead. Settings should express intent clearly.', + category: 'permissions-hygiene', + })); + } + + return scannerResult(SCANNER, 'ok', findings, filesScanned, Date.now() - start); +} diff --git a/plugins/config-audit/scanners/lib/scoring.mjs b/plugins/config-audit/scanners/lib/scoring.mjs index a00fee4..4e68449 100644 --- a/plugins/config-audit/scanners/lib/scoring.mjs +++ b/plugins/config-audit/scanners/lib/scoring.mjs @@ -152,6 +152,7 @@ const SCANNER_AREA_MAP = { GAP: 'Feature Coverage', TOK: 'Token Efficiency', CPS: 'Token Efficiency', + DIS: 'Settings', }; /** diff --git a/plugins/config-audit/scanners/scan-orchestrator.mjs b/plugins/config-audit/scanners/scan-orchestrator.mjs index 03b168d..808695c 100644 --- a/plugins/config-audit/scanners/scan-orchestrator.mjs +++ b/plugins/config-audit/scanners/scan-orchestrator.mjs @@ -25,6 +25,7 @@ import { scan as scanConflicts } from './conflict-detector.mjs'; import { scan as scanGap } from './feature-gap-scanner.mjs'; import { scan as scanTokenHotspots } from './token-hotspots.mjs'; import { scan as scanCachePrefix } from './cache-prefix-scanner.mjs'; +import { scan as scanDisabledInSchema } from './disabled-in-schema-scanner.mjs'; // Directory names that identify test fixture / example directories const FIXTURE_DIR_NAMES = ['tests', 'examples', '__tests__', 'test-fixtures']; @@ -57,6 +58,7 @@ const SCANNERS = [ { name: 'GAP', fn: scanGap, label: 'Feature Gap Scanner' }, { name: 'TOK', fn: scanTokenHotspots, label: 'Token Hotspots' }, { name: 'CPS', fn: scanCachePrefix, label: 'Cache-Prefix Stability' }, + { name: 'DIS', fn: scanDisabledInSchema, label: 'Disabled-In-Schema' }, ]; /** diff --git a/plugins/config-audit/tests/fixtures/denied-tools-in-schema/.claude/settings.json b/plugins/config-audit/tests/fixtures/denied-tools-in-schema/.claude/settings.json new file mode 100644 index 0000000..799dfe0 --- /dev/null +++ b/plugins/config-audit/tests/fixtures/denied-tools-in-schema/.claude/settings.json @@ -0,0 +1,6 @@ +{ + "permissions": { + "allow": ["Bash(npm:*)", "Read", "Write"], + "deny": ["Bash", "Edit"] + } +} diff --git a/plugins/config-audit/tests/scanners/disabled-in-schema.test.mjs b/plugins/config-audit/tests/scanners/disabled-in-schema.test.mjs new file mode 100644 index 0000000..f4be4f3 --- /dev/null +++ b/plugins/config-audit/tests/scanners/disabled-in-schema.test.mjs @@ -0,0 +1,68 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { resetCounter } from '../../scanners/lib/output.mjs'; +import { scan } from '../../scanners/disabled-in-schema-scanner.mjs'; +import { discoverConfigFiles } from '../../scanners/lib/file-discovery.mjs'; + +const __dirname = fileURLToPath(new URL('.', import.meta.url)); +const FIXTURES = resolve(__dirname, '../fixtures'); + +async function runScanner(fixtureName) { + resetCounter(); + const path = resolve(FIXTURES, fixtureName); + const discovery = await discoverConfigFiles(path); + return scan(path, discovery); +} + +describe('DIS scanner — basic structure', () => { + it('reports scanner prefix DIS', async () => { + const result = await runScanner('denied-tools-in-schema'); + assert.equal(result.scanner, 'DIS'); + }); + + it('finding IDs match CA-DIS-NNN pattern', async () => { + const result = await runScanner('denied-tools-in-schema'); + for (const f of result.findings) { + assert.match(f.id, /^CA-DIS-\d{3}$/); + } + }); +}); + +describe('DIS scanner — Bash in both arrays → finding', () => { + it('flags Bash overlap with low severity', async () => { + const result = await runScanner('denied-tools-in-schema'); + const f = result.findings.find(x => /both permissions\.deny and permissions\.allow/i.test(x.title || '')); + assert.ok(f, `expected DIS finding; got: ${result.findings.map(x => x.title).join(' | ')}`); + assert.equal(f.severity, 'low', `expected low, got ${f.severity}`); + assert.match(String(f.evidence || ''), /Bash/); + }); + + it('evidence references the allow + deny entries', async () => { + const result = await runScanner('denied-tools-in-schema'); + const f = result.findings.find(x => /both permissions/i.test(x.title || '')); + assert.ok(f); + assert.match(String(f.evidence || ''), /allow=/); + assert.match(String(f.evidence || ''), /deny=/); + }); +}); + +describe('DIS scanner — clean settings → no finding', () => { + it('healthy-project has no DIS findings', async () => { + const result = await runScanner('healthy-project'); + const f = result.findings.find(x => /both permissions/i.test(x.title || '')); + assert.equal(f, undefined, + `expected no DIS finding for healthy-project; got: ${f?.title}`); + }); +}); + +describe('DIS scanner — orchestrator wiring', () => { + it('DIS appears in scan-orchestrator scanner list', async () => { + const orch = await import('../../scanners/scan-orchestrator.mjs'); + const path = resolve(FIXTURES, 'denied-tools-in-schema'); + const env = await orch.runAllScanners(path, { filterFixtures: false }); + const dis = env.scanners.find(r => r.scanner === 'DIS'); + assert.ok(dis, `expected DIS in orchestrator results; got: ${env.scanners.map(r => r.scanner).join(', ')}`); + }); +});