fix(permissions): param-aware DIS dead-allow + CNF conflict matching

The DIS scanner collapsed Tool(param) rules to the bare tool name, so
Agent(model:opus) deny + Agent(model:sonnet) allow (and the same for
WebFetch(domain:...)) were flagged as dead config — a false positive now
that CC 2.1.178 matches Tool(param:value) and 2.1.172 adds domain rules.
The conflict-detector shared the blind spot from the other side: a
wildcard deny like WebFetch(domain:*) did not cover a
WebFetch(domain:good.com) allow, so a genuine cross-scope conflict was
missed (false negative).

New shared scanners/lib/permission-rules.mjs:
- parseRule / paramMatches (glob)
- dominates(deny, allow) -> DIS dead-allow (deny fully covers allow)
- rulesIntersect(a, b)   -> CNF cross-scope conflict (match sets intersect)

DIS now delegates to dominates; conflict-detector :156 delegates to
rulesIntersect. A bare deny still covers all params, so true positives
are preserved (Bash deny + Bash(npm:*) allow still flagged).

Re-seeded the marketplace-medium snapshots: the false-positive CA-DIS
finding (Read(src/**) allow + Read(./.env) deny) is correctly gone. This
changes snapshot CONTENT only — envelope schema is unchanged, so --json
and --raw stay byte-stable.

Full suite: 837/837 green (+25). self-audit PASS, A(100)/A(97).
This commit is contained in:
Kjell Tore Guttormsen 2026-06-18 13:06:20 +02:00
commit bec3f45329
16 changed files with 324 additions and 145 deletions

View file

@ -0,0 +1,6 @@
{
"permissions": {
"allow": ["WebFetch(domain:good.com)", "Agent(model:sonnet)"],
"deny": []
}
}

View file

@ -0,0 +1,6 @@
{
"permissions": {
"allow": [],
"deny": ["WebFetch(domain:*)", "Agent(model:opus)"]
}
}

View file

@ -0,0 +1,6 @@
{
"permissions": {
"allow": ["Agent(model:sonnet)", "WebFetch(domain:good.com)", "Bash(npm:*)"],
"deny": ["Agent(model:opus)", "WebFetch(domain:evil.com)", "Bash"]
}
}

View file

@ -0,0 +1,100 @@
import { describe, it } from 'node:test';
import assert from 'node:assert/strict';
import {
parseRule,
paramMatches,
dominates,
rulesIntersect,
} from '../../scanners/lib/permission-rules.mjs';
describe('permission-rules — parseRule', () => {
it('bare tool → param null', () => {
assert.deepEqual(parseRule('Bash'), { tool: 'Bash', param: null });
});
it('param-qualified tool → tool + param', () => {
assert.deepEqual(parseRule('Bash(npm:*)'), { tool: 'Bash', param: 'npm:*' });
});
it('domain rule → tool + param', () => {
assert.deepEqual(parseRule('WebFetch(domain:example.com)'), {
tool: 'WebFetch',
param: 'domain:example.com',
});
});
it('non-string → tool null', () => {
assert.equal(parseRule(null).tool, null);
});
});
describe('permission-rules — paramMatches (glob)', () => {
it('wildcard matches any suffix', () => {
assert.equal(paramMatches('domain:*', 'domain:good.com'), true);
assert.equal(paramMatches('npm:*', 'npm:install'), true);
});
it('exact literal matches', () => {
assert.equal(paramMatches('domain:good.com', 'domain:good.com'), true);
});
it('distinct literals do not match', () => {
assert.equal(paramMatches('domain:evil.com', 'domain:good.com'), false);
assert.equal(paramMatches('model:opus', 'model:sonnet'), false);
});
it('literal pattern does not match a wildcard value', () => {
assert.equal(paramMatches('domain:good.com', 'domain:*'), false);
});
});
describe('permission-rules — dominates (deny fully covers allow → dead allow)', () => {
it('bare deny covers any param-qualified allow', () => {
assert.equal(dominates('Bash', 'Bash(npm:*)'), true);
});
it('exact param deny covers identical allow', () => {
assert.equal(dominates('Agent(model:opus)', 'Agent(model:opus)'), true);
});
it('wildcard deny covers matching literal allow', () => {
assert.equal(dominates('WebFetch(domain:*)', 'WebFetch(domain:example.com)'), true);
});
it('distinct param deny does NOT cover a different param allow (the false positive)', () => {
assert.equal(dominates('Agent(model:opus)', 'Agent(model:sonnet)'), false);
assert.equal(dominates('WebFetch(domain:evil.com)', 'WebFetch(domain:good.com)'), false);
});
it('specific deny does NOT cover a bare allow (sonnet still allowed)', () => {
assert.equal(dominates('Agent(model:opus)', 'Agent'), false);
});
it('different tools never dominate', () => {
assert.equal(dominates('Bash', 'Read'), false);
});
});
describe('permission-rules — rulesIntersect (cross-scope conflict)', () => {
it('exact same rule intersects', () => {
assert.equal(rulesIntersect('Bash(npm run *)', 'Bash(npm run *)'), true);
});
it('bare tool intersects any param of same tool', () => {
assert.equal(rulesIntersect('Bash', 'Bash(npm:*)'), true);
assert.equal(rulesIntersect('Agent(model:opus)', 'Agent'), true);
});
it('wildcard intersects matching literal (currently a false negative)', () => {
assert.equal(rulesIntersect('WebFetch(domain:*)', 'WebFetch(domain:good.com)'), true);
assert.equal(rulesIntersect('WebFetch(domain:good.com)', 'WebFetch(domain:*)'), true);
});
it('distinct literal params are disjoint (no conflict)', () => {
assert.equal(rulesIntersect('Agent(model:opus)', 'Agent(model:sonnet)'), false);
});
it('different tools never intersect', () => {
assert.equal(rulesIntersect('Read', 'Write'), false);
});
});

View file

@ -122,3 +122,37 @@ describe('CNF scanner — minimal project', () => {
assert.equal(result.findings.length, 0);
});
});
describe('CNF scanner — param-qualified cross-scope conflicts', () => {
// project allow: WebFetch(domain:good.com), Agent(model:sonnet)
// local deny: WebFetch(domain:*), Agent(model:opus)
// WebFetch: deny domain:* covers allow domain:good.com → genuine conflict.
// Agent: deny model:opus vs allow model:sonnet → disjoint → NO conflict.
let result;
beforeEach(async () => {
resetCounter();
const discovery = await discoverConfigFiles(resolve(FIXTURES, 'param-conflict-project'));
result = await scan(resolve(FIXTURES, 'param-conflict-project'), discovery);
});
it('detects the WebFetch wildcard-domain conflict (currently a false negative)', () => {
const perm = result.findings.filter(f => f.title.includes('Permission allow/deny'));
assert.ok(
perm.some(f => /WebFetch/.test(`${f.description} ${f.evidence}`)),
`expected a WebFetch allow/deny conflict; got: ${perm.map(f => f.evidence).join(' | ') || '(none)'}`,
);
});
it('does NOT flag Agent(model:sonnet) vs Agent(model:opus) as a conflict', () => {
const perm = result.findings.filter(f => f.title.includes('Permission allow/deny'));
assert.ok(
!perm.some(f => /Agent/.test(`${f.description} ${f.evidence}`)),
'distinct model params must not conflict',
);
});
it('reports exactly one permission conflict', () => {
const perm = result.findings.filter(f => f.title.includes('Permission allow/deny'));
assert.equal(perm.length, 1);
});
});

View file

@ -66,3 +66,32 @@ describe('DIS scanner — orchestrator wiring', () => {
assert.ok(dis, `expected DIS in orchestrator results; got: ${env.scanners.map(r => r.scanner).join(', ')}`);
});
});
describe('DIS scanner — param-qualified permissions are param-aware', () => {
// settings.json:
// allow: Agent(model:sonnet), WebFetch(domain:good.com), Bash(npm:*)
// deny: Agent(model:opus), WebFetch(domain:evil.com), Bash
// Only the bare `Bash` deny dominates an allow (Bash(npm:*)). The param-
// qualified deny/allow pairs are DISTINCT specs (CC 2.1.178 param-matching,
// 2.1.172 domain rules) and must NOT be flagged as dead config.
it('flags exactly the Bash overlap (bare deny covers param allow)', async () => {
const result = await runScanner('param-qualified-permissions');
const f = result.findings.find(x => /both permissions/i.test(x.title || ''));
assert.ok(f, 'expected the Bash overlap to still be flagged');
assert.match(String(f.evidence || ''), /Bash/);
assert.match(String(f.description || ''), /contains 1 tool\b/);
});
it('does NOT flag Agent(model:opus)/Agent(model:sonnet) as dead config', async () => {
const result = await runScanner('param-qualified-permissions');
const f = result.findings.find(x => /both permissions/i.test(x.title || ''));
assert.doesNotMatch(String(f?.evidence || ''), /Agent/);
});
it('does NOT flag WebFetch(domain:good.com)/WebFetch(domain:evil.com) as dead config', async () => {
const result = await runScanner('param-qualified-permissions');
const f = result.findings.find(x => /both permissions/i.test(x.title || ''));
assert.doesNotMatch(String(f?.evidence || ''), /WebFetch/);
});
});

View file

@ -1,4 +1,4 @@
{
"kind": "text",
"payload": "`[CML] CLAUDE.md Linter`: 1 finding(s) (0ms)\n `[SET] Settings Validator`: 0 finding(s) (0ms)\n `[HKV] Hook Validator`: 0 finding(s) (0ms)\n `[RUL] Rules Validator`: 0 finding(s) (0ms)\n `[MCP] MCP Config Validator`: 0 finding(s) (0ms)\n `[IMP] Import Resolver`: 0 finding(s) (0ms)\n `[CNF] Conflict Detector`: 0 finding(s) (0ms)\n `[GAP] Feature Gap Scanner`: 17 finding(s) (0ms)\n `[TOK] Token Hotspots`: 1 finding(s) (0ms)\n `[CPS] Cache-Prefix Stability`: 0 finding(s) (0ms)\n `[DIS] Disabled-In-Schema`: 1 finding(s) (0ms)\n `[COL] Plugin Skill Collision`: 0 finding(s) (0ms)\n\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n Configuration health\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n\n Health: A (97/100) — Healthy setup, only minor polish needed\n 9 areas reviewed\n\n Area scores\n ───────────\n `CLAUDE.md` ........... A (90) `Settings` ............ A (90)\n `Hooks` ............... A (100) `Rules` ............... A (100)\n `MCP` ................. A (100) `Imports` ............. A (100)\n `Conflicts` ........... A (100) `Token Efficiency` .... A (90)\n `Plugin Hygiene` ...... A (100)\n\n 17 ways you could get more out of Claude Code — see /config-audit feature-gap\n\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
"payload": "`[CML] CLAUDE.md Linter`: 1 finding(s) (0ms)\n `[SET] Settings Validator`: 0 finding(s) (0ms)\n `[HKV] Hook Validator`: 0 finding(s) (0ms)\n `[RUL] Rules Validator`: 0 finding(s) (0ms)\n `[MCP] MCP Config Validator`: 0 finding(s) (0ms)\n `[IMP] Import Resolver`: 0 finding(s) (0ms)\n `[CNF] Conflict Detector`: 0 finding(s) (0ms)\n `[GAP] Feature Gap Scanner`: 17 finding(s) (0ms)\n `[TOK] Token Hotspots`: 1 finding(s) (0ms)\n `[CPS] Cache-Prefix Stability`: 0 finding(s) (0ms)\n `[DIS] Disabled-In-Schema`: 0 finding(s) (0ms)\n `[COL] Plugin Skill Collision`: 0 finding(s) (0ms)\n\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n Configuration health\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n\n Health: A (98/100) — Healthy setup, only minor polish needed\n 9 areas reviewed\n\n Area scores\n ───────────\n `CLAUDE.md` ........... A (90) `Settings` ............ A (100)\n `Hooks` ............... A (100) `Rules` ............... A (100)\n `MCP` ................. A (100) `Imports` ............. A (100)\n `Conflicts` ........... A (100) `Token Efficiency` .... A (90)\n `Plugin Hygiene` ...... A (100)\n\n 17 ways you could get more out of Claude Code — see /config-audit feature-gap\n\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
}

View file

@ -517,29 +517,12 @@
"status": "ok",
"files_scanned": 1,
"duration_ms": 0,
"findings": [
{
"id": "CA-DIS-001",
"scanner": "DIS",
"severity": "low",
"title": "A tool is in both the let-in list and the shut-out list",
"description": "When a tool is in both lists, the shut-out always wins, so the let-in entry does nothing. It looks like the tool is approved, but it isn't.",
"file": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium/.claude/settings.json",
"line": null,
"evidence": "Read: allow=\"Read(src/**)\" + deny=\"Read(./.env)\"",
"category": "permissions-hygiene",
"recommendation": "Decide whether the tool should be allowed or denied, and remove it from the other list.",
"autoFixable": false,
"userImpactCategory": "Dead config",
"userActionLanguage": "Optional cleanup",
"relevanceContext": "test-fixture-no-impact"
}
],
"findings": [],
"counts": {
"critical": 0,
"high": 0,
"medium": 0,
"low": 1,
"low": 0,
"info": 0
}
},
@ -559,15 +542,15 @@
}
],
"aggregate": {
"total_findings": 20,
"total_findings": 19,
"counts": {
"critical": 0,
"high": 0,
"medium": 1,
"low": 8,
"low": 7,
"info": 11
},
"risk_score": 12,
"risk_score": 11,
"risk_band": "Medium",
"verdict": "PASS",
"scanners_ok": 11,

View file

@ -1,4 +1,4 @@
[CML] CLAUDE.md Linter: 1 finding(s) (13ms)
[CML] CLAUDE.md Linter: 1 finding(s) (14ms)
[SET] Settings Validator: 0 finding(s) (1ms)
[HKV] Hook Validator: 0 finding(s) (1ms)
[RUL] Rules Validator: 0 finding(s) (1ms)
@ -8,18 +8,18 @@
[GAP] Feature Gap Scanner: 17 finding(s) (2ms)
[TOK] Token Hotspots: 1 finding(s) (8ms)
[CPS] Cache-Prefix Stability: 0 finding(s) (1ms)
[DIS] Disabled-In-Schema: 1 finding(s) (1ms)
[COL] Plugin Skill Collision: 0 finding(s) (0ms)
[DIS] Disabled-In-Schema: 0 finding(s) (0ms)
[COL] Plugin Skill Collision: 0 finding(s) (1ms)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Config-Audit Health Score
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Health: A (97/100) 9 areas scanned
Health: A (98/100) 9 areas scanned
Area Scores
───────────
CLAUDE.md ........... A (90) Settings ............ A (90)
CLAUDE.md ........... A (90) Settings ............ A (100)
Hooks ............... A (100) Rules ............... A (100)
MCP ................. A (100) Imports ............. A (100)
Conflicts ........... A (100) Token Efficiency .... A (90)

View file

@ -248,29 +248,16 @@
"category": "token-efficiency",
"recommendation": "Install the package locally (so detect-mcp-tool-count can read its manifest), or run the server once and cache its tools/list response under ~/.claude/config-audit/mcp-cache/<name>.json. See knowledge/cache-telemetry-recipe.md.",
"autoFixable": false
},
{
"id": "CA-DIS-001",
"scanner": "DIS",
"severity": "low",
"title": "Tool listed in both permissions.deny and permissions.allow",
"description": ".claude/settings.json contains 1 tool 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": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium/.claude/settings.json",
"line": null,
"evidence": "Read: allow=\"Read(src/**)\" + deny=\"Read(./.env)\"",
"category": "permissions-hygiene",
"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.",
"autoFixable": false
}
],
"movedFindings": [],
"scoreChange": {
"before": {
"score": 91,
"score": 92,
"grade": "A"
},
"after": {
"score": 91,
"score": 92,
"grade": "A"
},
"delta": 0
@ -291,11 +278,11 @@
{
"name": "Settings",
"before": {
"score": 90,
"score": 100,
"grade": "A"
},
"after": {
"score": 90,
"score": 100,
"grade": "A"
},
"delta": 0
@ -398,8 +385,8 @@
}
],
"summary": {
"totalBefore": 20,
"totalAfter": 20,
"totalBefore": 19,
"totalAfter": 19,
"newCount": 0,
"resolvedCount": 0,
"trend": "stable"

View file

@ -118,12 +118,6 @@
"title": "High MCP tool-schema budget on server \"memory\"",
"file": ".mcp.json",
"recommendation": "Install the package locally (so detect-mcp-tool-count can read its manifest), or run the server once and cache its tools/list response under ~/.claude/config-audit/mcp-cache/<name>.json. See knowledge/cache-telemetry-recipe.md."
},
{
"findingId": "CA-DIS-001",
"title": "Tool listed in both permissions.deny and permissions.allow",
"file": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium/.claude/settings.json",
"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."
}
],
"backupId": null

View file

@ -24,8 +24,8 @@
"id": "settings",
"name": "Settings",
"grade": "A",
"score": 90,
"findingCount": 1
"score": 100,
"findingCount": 0
},
{
"id": "hooks",
@ -94,7 +94,7 @@
"scannerEnvelope": {
"meta": {
"target": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium",
"timestamp": "2026-06-18T10:18:35.634Z",
"timestamp": "2026-06-18T11:03:14.339Z",
"version": "2.2.0",
"tool": "config-audit"
},
@ -103,7 +103,7 @@
"scanner": "CML",
"status": "ok",
"files_scanned": 1,
"duration_ms": 3,
"duration_ms": 2,
"findings": [
{
"id": "CA-CML-001",
@ -187,7 +187,7 @@
"scanner": "IMP",
"status": "ok",
"files_scanned": 1,
"duration_ms": 2,
"duration_ms": 1,
"findings": [],
"counts": {
"critical": 0,
@ -551,26 +551,12 @@
"status": "ok",
"files_scanned": 1,
"duration_ms": 0,
"findings": [
{
"id": "CA-DIS-001",
"scanner": "DIS",
"severity": "low",
"title": "Tool listed in both permissions.deny and permissions.allow",
"description": ".claude/settings.json contains 1 tool 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": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium/.claude/settings.json",
"line": null,
"evidence": "Read: allow=\"Read(src/**)\" + deny=\"Read(./.env)\"",
"category": "permissions-hygiene",
"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.",
"autoFixable": false
}
],
"findings": [],
"counts": {
"critical": 0,
"high": 0,
"medium": 0,
"low": 1,
"low": 0,
"info": 0
}
},
@ -578,7 +564,7 @@
"scanner": "COL",
"status": "ok",
"files_scanned": 0,
"duration_ms": 0,
"duration_ms": 1,
"findings": [],
"counts": {
"critical": 0,
@ -590,15 +576,15 @@
}
],
"aggregate": {
"total_findings": 20,
"total_findings": 19,
"counts": {
"critical": 0,
"high": 0,
"medium": 1,
"low": 8,
"low": 7,
"info": 11
},
"risk_score": 12,
"risk_score": 11,
"risk_band": "Medium",
"verdict": "PASS",
"scanners_ok": 11,

View file

@ -1,7 +1,7 @@
{
"meta": {
"target": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium",
"timestamp": "2026-06-18T10:18:35.439Z",
"timestamp": "2026-06-18T11:03:14.159Z",
"version": "2.2.0",
"tool": "config-audit"
},
@ -122,7 +122,7 @@
"scanner": "GAP",
"status": "ok",
"files_scanned": 4,
"duration_ms": 2,
"duration_ms": 1,
"findings": [
{
"id": "CA-GAP-001",
@ -358,7 +358,7 @@
"scanner": "TOK",
"status": "ok",
"files_scanned": 2,
"duration_ms": 8,
"duration_ms": 7,
"findings": [
{
"id": "CA-TOK-001",
@ -457,27 +457,13 @@
"scanner": "DIS",
"status": "ok",
"files_scanned": 1,
"duration_ms": 1,
"findings": [
{
"id": "CA-DIS-001",
"scanner": "DIS",
"severity": "low",
"title": "Tool listed in both permissions.deny and permissions.allow",
"description": ".claude/settings.json contains 1 tool 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": "/Users/ktg/repos/ktg-plugin-marketplace/config-audit/tests/fixtures/marketplace-medium/.claude/settings.json",
"line": null,
"evidence": "Read: allow=\"Read(src/**)\" + deny=\"Read(./.env)\"",
"category": "permissions-hygiene",
"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.",
"autoFixable": false
}
],
"duration_ms": 0,
"findings": [],
"counts": {
"critical": 0,
"high": 0,
"medium": 0,
"low": 1,
"low": 0,
"info": 0
}
},
@ -485,7 +471,7 @@
"scanner": "COL",
"status": "ok",
"files_scanned": 0,
"duration_ms": 0,
"duration_ms": 1,
"findings": [],
"counts": {
"critical": 0,
@ -497,15 +483,15 @@
}
],
"aggregate": {
"total_findings": 20,
"total_findings": 19,
"counts": {
"critical": 0,
"high": 0,
"medium": 1,
"low": 8,
"low": 7,
"info": 11
},
"risk_score": 12,
"risk_score": 11,
"risk_band": "Medium",
"verdict": "PASS",
"scanners_ok": 11,