feat(dis): flag ineffective allow wildcards; treat Tool(*) as deny-all
Extends the DIS scanner and its shared permission-rules lib with two
documented Claude Code permission footguns. Verified verbatim against
code.claude.com/docs/en/permissions (fetched 2026-06-19).
- lib/permission-rules.mjs: new isIneffectiveAllowGlob(entry) — unanchored
tool-name globs in permissions.allow (`*`, `B*`, `mcp__*`) that CC silently
skips ("does not auto-approve anything"); valid only as a glob-free
`mcp__<server>__*`. Shared with CNF.
- lib/permission-rules.mjs: dominates() now treats the `Tool(*)` deny-all glob
as equivalent to a bare deny (covers a bare allow) — CC: "Bash(*) is
equivalent to Bash ... both forms remove the tool from Claude's context".
- DIS: new finding "Ineffective allow wildcard — Claude Code ignores this rule"
(low, permissions-hygiene, CA-DIS-NNN); the existing dead-allow finding now
also catches a bare allow killed by a Tool(*) deny.
- 9 new tests (5 lib, 4 DIS) + 2 fixtures (force-added past .gitignore .claude/).
Suite 903 -> 912. Snapshot unchanged, contamination grep clean. README/CLAUDE/
scanner-internals document the broadened DIS mandate; test badge synced.
self-audit: PASS, configGrade A 96, pluginGrade A 100, readme gate passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ter3E2JSi1Khgmuf2kady8
This commit is contained in:
parent
dfe9049b55
commit
03949c6c98
9 changed files with 198 additions and 28 deletions
15
CLAUDE.md
15
CLAUDE.md
|
|
@ -109,7 +109,20 @@ Default: auto-detects scope from git context. Override with `/config-audit full|
|
|||
node --test 'tests/**/*.test.mjs'
|
||||
```
|
||||
|
||||
903 tests across 56 test files (17 lib + 29 scanner + 1 hook + 1 agent + 3 commands + 1 knowledge + 4 top-level). Test fixtures in `tests/fixtures/`. Top-level humanizer tests: `json-backcompat.test.mjs`, `raw-backcompat.test.mjs`, `scenario-read-test.test.mjs`, `snapshot-default-output.test.mjs`.
|
||||
912 tests across 56 test files (17 lib + 29 scanner + 1 hook + 1 agent + 3 commands + 1 knowledge + 4 top-level). Test fixtures in `tests/fixtures/`. Top-level humanizer tests: `json-backcompat.test.mjs`, `raw-backcompat.test.mjs`, `scenario-read-test.test.mjs`, `snapshot-default-output.test.mjs`.
|
||||
|
||||
### DIS scanner — permission-rule hygiene
|
||||
|
||||
Beyond deny/allow overlap, the DIS scanner now also flags:
|
||||
|
||||
- **Ineffective allow wildcards** — unanchored tool-name globs in `permissions.allow`
|
||||
(`*`, `B*`, `mcp__*`) that Claude Code silently skips (auto-approve nothing). Valid
|
||||
only as a glob-free `mcp__<server>__*`. New `CA-DIS` finding, severity low.
|
||||
- **`Tool(*)` deny-all glob** — treated as equivalent to a bare deny (`Bash(*)` ≡ `Bash`),
|
||||
so a bare allow killed by it is correctly reported as dead config.
|
||||
|
||||
Both predicates live in `scanners/lib/permission-rules.mjs` (shared with the CNF
|
||||
conflict-detector). Behavior verified against `code.claude.com/docs/en/permissions`.
|
||||
|
||||
## Gotchas
|
||||
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
A Claude Code plugin that checks configuration health, suggests context-aware improvements, and auto-fixes issues — `CLAUDE.md`, `settings.json`, hooks, rules, MCP servers, `@imports`, and plugins. 13 deterministic scanners across 10 quality areas, context-aware feature recommendations, auto-fix with backup/rollback, a prompt-cache-aware Token Hotspots scanner with optional API-calibrated `--accurate-tokens` mode, plus cache-prefix stability, dead-tool, and cross-plugin collision detection. Zero external dependencies.
|
||||
|
|
@ -309,7 +309,7 @@ By default, `/config-audit` auto-detects scope from your git context. Override w
|
|||
| `feature-gap-scanner.mjs` | GAP | 25 feature checks shown as opportunities, not grades — plus a conditional `disableBundledSkills` recommendation when the active skill listing is over budget |
|
||||
| `token-hotspots.mjs` | TOK | Cache-breaking volatile content, redundant tool permissions, deep import chains, oversized cascades, bloated skill descriptions, MCP tool-schema budget |
|
||||
| `cache-prefix-scanner.mjs` | CPS | Volatile content in lines 31–150 of the CLAUDE.md cascade — beyond the cache-prefix window but still re-loaded every turn |
|
||||
| `disabled-in-schema-scanner.mjs` | DIS | Tools listed in BOTH `permissions.deny` and `permissions.allow` — deny wins, allow entries are dead config |
|
||||
| `disabled-in-schema-scanner.mjs` | DIS | Dead/ineffective permission entries: (1) tools in BOTH `permissions.deny` and `permissions.allow` — deny wins (incl. the `Tool(*)` deny-all glob, equivalent to a bare deny); (2) unanchored allow wildcards (`*`, `B*`, `mcp__*`) that Claude Code silently skips — valid only as `mcp__<server>__*` |
|
||||
| `collision-scanner.mjs` | COL | Cross-plugin skill name collisions; user-vs-plugin overlaps |
|
||||
|
||||
> **Cross-scanner remediation — diagnosis meets the fix.** SKL diagnoses an over-budget
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ Scanner CLI: `node scanners/scan-orchestrator.mjs <path> [--global] [--full-mach
|
|||
| `feature-gap-scanner.mjs` | GAP | 25 feature checks across 4 tiers — shown as opportunities, not grades |
|
||||
| `token-hotspots.mjs` | TOK | Cache-breaking volatile content, redundant tool permissions, deep import chains, oversized cascade, bloated SKILL.md descriptions, MCP tool-schema budget (prompt-cache patterns) |
|
||||
| `cache-prefix-scanner.mjs` | CPS | Volatile content in lines 31–150 of CLAUDE.md cascade (beyond Pattern A's top-30 window) |
|
||||
| `disabled-in-schema-scanner.mjs` | DIS | Tools listed in BOTH `permissions.deny` AND `permissions.allow` — deny wins, allow entries are dead config |
|
||||
| `disabled-in-schema-scanner.mjs` | DIS | Dead/ineffective permission entries (low). (1) Tools in BOTH `permissions.deny` AND `permissions.allow` — deny wins; dominance is param-aware and treats the `Tool(*)` deny-all glob as equivalent to a bare deny (covers a bare allow). (2) Unanchored allow wildcards (`*`, `B*`, `mcp__*`) that Claude Code silently skips — CC accepts allow globs only after a literal glob-free `mcp__<server>__` prefix. Predicates shared with CNF live in `lib/permission-rules.mjs` |
|
||||
| `collision-scanner.mjs` | COL | Cross-plugin skill name collisions (low); user-vs-plugin overlaps (medium); `details.namespaces` payload |
|
||||
| `skill-listing-scanner.mjs` | SKL | (1) `CA-SKL-001` (medium): active skill descriptions over the verified 1,536-char listing cap (CC 2.1.105) → silently truncated in the model's skill listing. (2) `CA-SKL-002` (low): sum of active descriptions (each counted up to the cap) over the listing budget (~2% of context, CC 2.1.32), anchored on a conservative 200k window with a calibration note that the budget scales 5× on 1M-context models — leads with the measured sum, an estimate not telemetry. HOME-scoped (all user + plugin skills). Remediation surfaces `disableBundledSkills` / `skillOverrides` / trim. Distinct lens from TOK pattern F (project-local 500-char bloat heuristic) |
|
||||
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ 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';
|
||||
import { dominates, parseRule } from './lib/permission-rules.mjs';
|
||||
import { dominates, parseRule, isIneffectiveAllowGlob } from './lib/permission-rules.mjs';
|
||||
|
||||
const SCANNER = 'DIS';
|
||||
|
||||
|
|
@ -52,6 +52,19 @@ function findDenyAllowOverlaps(settings) {
|
|||
return overlaps;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find `permissions.allow` entries that are unanchored tool-name globs Claude
|
||||
* Code silently skips (e.g. `mcp__*`, `B*`, `*`). They auto-approve nothing but
|
||||
* the author usually believes they grant access. Returns array of entry strings.
|
||||
*/
|
||||
function findIneffectiveAllowGlobs(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 : [];
|
||||
return allowList.filter(e => isIneffectiveAllowGlob(e));
|
||||
}
|
||||
|
||||
/**
|
||||
* Main scanner entry point.
|
||||
*
|
||||
|
|
@ -70,28 +83,50 @@ export async function scan(targetPath, discovery) {
|
|||
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',
|
||||
}));
|
||||
const overlaps = findDenyAllowOverlaps(parsed);
|
||||
if (overlaps.length > 0) {
|
||||
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',
|
||||
}));
|
||||
}
|
||||
|
||||
const ineffective = findIneffectiveAllowGlobs(parsed);
|
||||
if (ineffective.length > 0) {
|
||||
const evidence = `allow: ${ineffective.slice(0, 5).map(e => `"${e}"`).join(', ')}`;
|
||||
findings.push(finding({
|
||||
scanner: SCANNER,
|
||||
severity: SEVERITY.low,
|
||||
title: 'Ineffective allow wildcard — Claude Code ignores this rule',
|
||||
description:
|
||||
`${f.relPath || f.absPath} has ${ineffective.length} permissions.allow ` +
|
||||
`entr${ineffective.length === 1 ? 'y' : 'ies'} that Claude Code skips: an ` +
|
||||
'unanchored tool-name wildcard auto-approves nothing. CC accepts allow ' +
|
||||
'wildcards only after a literal `mcp__<server>__` prefix.',
|
||||
file: f.absPath,
|
||||
evidence,
|
||||
recommendation:
|
||||
'Replace `*`/`mcp__*` with explicit tool names, or anchor MCP wildcards to ' +
|
||||
'a server (`mcp__<server>__*`). As written these entries grant nothing.',
|
||||
category: 'permissions-hygiene',
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
return scannerResult(SCANNER, 'ok', findings, filesScanned, Date.now() - start);
|
||||
|
|
|
|||
|
|
@ -55,8 +55,12 @@ export function paramMatches(pattern, value) {
|
|||
|
||||
/**
|
||||
* Does the deny entry fully cover the allow entry, making the allow dead config?
|
||||
* Used by DIS. Bare deny covers everything; a specific deny only covers the
|
||||
* matching (or wildcard-subsumed) param — and does NOT cover a bare allow.
|
||||
* Used by DIS. Bare deny — and the equivalent `Tool(*)` deny-all glob — covers
|
||||
* everything (including a bare allow); a specific deny only covers the matching
|
||||
* (or wildcard-subsumed) param and does NOT kill a bare allow.
|
||||
*
|
||||
* CC: "`Bash(*)` is equivalent to `Bash` ... As a deny rule, both forms remove
|
||||
* the tool from Claude's context." (code.claude.com/docs/en/permissions)
|
||||
* @param {string} denyEntry
|
||||
* @param {string} allowEntry
|
||||
* @returns {boolean}
|
||||
|
|
@ -65,7 +69,7 @@ export function dominates(denyEntry, allowEntry) {
|
|||
const d = parseRule(denyEntry);
|
||||
const a = parseRule(allowEntry);
|
||||
if (!d.tool || !a.tool || d.tool !== a.tool) return false;
|
||||
if (d.param === null) return true; // bare deny covers all params
|
||||
if (d.param === null || d.param === '*') return true; // bare / Tool(*) deny covers all params
|
||||
if (a.param === null) return false; // specific deny does not kill a bare allow
|
||||
if (d.param === a.param) return true;
|
||||
return paramMatches(d.param, a.param); // wildcard deny covers a matching literal allow
|
||||
|
|
@ -87,3 +91,35 @@ export function rulesIntersect(ruleA, ruleB) {
|
|||
if (a.param === b.param) return true;
|
||||
return paramMatches(a.param, b.param) || paramMatches(b.param, a.param);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this `permissions.allow` entry an UNANCHORED tool-name glob that Claude
|
||||
* Code silently skips? CC accepts tool-name globs in an ALLOW rule only after a
|
||||
* literal `mcp__<server>__` prefix (the server segment must be glob-free).
|
||||
* Unanchored globs like `*`, `B*`, or `mcp__*` are skipped with a warning and
|
||||
* auto-approve nothing — dead config the author believes is granting access.
|
||||
*
|
||||
* Tool-name globs apply to the bare-name form only (no `(...)` specifier); a
|
||||
* glob INSIDE a specifier such as `Bash(npm run *)` is normal and valid.
|
||||
*
|
||||
* CC: "An unanchored allow glob such as `"*"`, `"B*"`, or `"mcp__*"` is skipped
|
||||
* with a warning and does not auto-approve anything."
|
||||
* (code.claude.com/docs/en/permissions — "Tool name wildcards")
|
||||
*
|
||||
* NOTE: deny/ask rules DO accept tool-name globs, so this predicate is for the
|
||||
* allow list only.
|
||||
* @param {string} entry
|
||||
* @returns {boolean}
|
||||
*/
|
||||
export function isIneffectiveAllowGlob(entry) {
|
||||
if (typeof entry !== 'string') return false;
|
||||
if (entry.includes('(')) return false; // specifier form — glob lives inside the param
|
||||
if (!entry.includes('*')) return false; // no glob — a normal match-all-tool allow
|
||||
if (entry.startsWith('mcp__')) {
|
||||
const rest = entry.slice(5); // after 'mcp__'
|
||||
const sep = rest.indexOf('__');
|
||||
// mcp__<server>__<rest> with a glob-free server segment is anchored & valid
|
||||
if (sep > 0 && !rest.slice(0, sep).includes('*')) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
6
tests/fixtures/deny-all-glob/.claude/settings.json
vendored
Normal file
6
tests/fixtures/deny-all-glob/.claude/settings.json
vendored
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"permissions": {
|
||||
"allow": ["Bash"],
|
||||
"deny": ["Bash(*)"]
|
||||
}
|
||||
}
|
||||
5
tests/fixtures/ineffective-allow-globs/.claude/settings.json
vendored
Normal file
5
tests/fixtures/ineffective-allow-globs/.claude/settings.json
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"permissions": {
|
||||
"allow": ["mcp__*", "B*", "mcp__puppeteer__*", "Bash(npm run *)", "Read"]
|
||||
}
|
||||
}
|
||||
|
|
@ -5,6 +5,7 @@ import {
|
|||
paramMatches,
|
||||
dominates,
|
||||
rulesIntersect,
|
||||
isIneffectiveAllowGlob,
|
||||
} from '../../scanners/lib/permission-rules.mjs';
|
||||
|
||||
describe('permission-rules — parseRule', () => {
|
||||
|
|
@ -70,11 +71,44 @@ describe('permission-rules — dominates (deny fully covers allow → dead allow
|
|||
assert.equal(dominates('Agent(model:opus)', 'Agent'), false);
|
||||
});
|
||||
|
||||
it('deny-all glob Tool(*) covers a bare allow (Bash(*) ≡ bare Bash deny)', () => {
|
||||
// CC: "Bash(*) is equivalent to Bash ... As a deny rule, both forms remove
|
||||
// the tool from Claude's context." So Bash(*) deny kills a bare Bash allow.
|
||||
assert.equal(dominates('Bash(*)', 'Bash'), true);
|
||||
assert.equal(dominates('Bash(*)', 'Bash(npm:*)'), true);
|
||||
});
|
||||
|
||||
it('different tools never dominate', () => {
|
||||
assert.equal(dominates('Bash', 'Read'), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('permission-rules — isIneffectiveAllowGlob (CC silently skips these allow entries)', () => {
|
||||
it('unanchored tool-name globs are ineffective', () => {
|
||||
assert.equal(isIneffectiveAllowGlob('*'), true);
|
||||
assert.equal(isIneffectiveAllowGlob('B*'), true);
|
||||
assert.equal(isIneffectiveAllowGlob('mcp__*'), true);
|
||||
assert.equal(isIneffectiveAllowGlob('mcp__*__foo'), true); // glob in server segment
|
||||
});
|
||||
|
||||
it('MCP globs anchored to a literal server are valid (not flagged)', () => {
|
||||
assert.equal(isIneffectiveAllowGlob('mcp__puppeteer__*'), false);
|
||||
assert.equal(isIneffectiveAllowGlob('mcp__github__get_*'), false);
|
||||
});
|
||||
|
||||
it('non-glob and specifier forms are valid (not flagged)', () => {
|
||||
assert.equal(isIneffectiveAllowGlob('Bash'), false); // legit match-all-tool allow
|
||||
assert.equal(isIneffectiveAllowGlob('mcp__puppeteer'), false); // legit match-all-server
|
||||
assert.equal(isIneffectiveAllowGlob('Bash(npm run *)'), false);// glob inside specifier is fine
|
||||
assert.equal(isIneffectiveAllowGlob('Read(src/**)'), false);
|
||||
});
|
||||
|
||||
it('non-string → false', () => {
|
||||
assert.equal(isIneffectiveAllowGlob(null), false);
|
||||
assert.equal(isIneffectiveAllowGlob(undefined), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('permission-rules — rulesIntersect (cross-scope conflict)', () => {
|
||||
it('exact same rule intersects', () => {
|
||||
assert.equal(rulesIntersect('Bash(npm run *)', 'Bash(npm run *)'), true);
|
||||
|
|
|
|||
|
|
@ -95,3 +95,44 @@ describe('DIS scanner — param-qualified permissions are param-aware', () => {
|
|||
assert.doesNotMatch(String(f?.evidence || ''), /WebFetch/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DIS scanner — ineffective allow wildcards (CC skips unanchored tool-name globs)', () => {
|
||||
// settings.json allow: ["mcp__*", "B*", "mcp__puppeteer__*", "Bash(npm run *)", "Read"]
|
||||
// CC skips `mcp__*` and `B*` (unanchored tool-name globs) — dead config.
|
||||
// `mcp__puppeteer__*` (anchored), `Bash(npm run *)` (specifier glob) and `Read` are valid.
|
||||
|
||||
it('flags the ineffective allow globs with low severity', async () => {
|
||||
const result = await runScanner('ineffective-allow-globs');
|
||||
const f = result.findings.find(x => /ineffective allow wildcard/i.test(x.title || ''));
|
||||
assert.ok(f, `expected ineffective-allow-glob finding; got: ${result.findings.map(x => x.title).join(' | ')}`);
|
||||
assert.equal(f.severity, 'low', `expected low, got ${f.severity}`);
|
||||
assert.match(f.id, /^CA-DIS-\d{3}$/);
|
||||
});
|
||||
|
||||
it('evidence cites only the unanchored globs, not the valid entries', async () => {
|
||||
const result = await runScanner('ineffective-allow-globs');
|
||||
const f = result.findings.find(x => /ineffective allow wildcard/i.test(x.title || ''));
|
||||
assert.ok(f);
|
||||
assert.match(String(f.evidence || ''), /mcp__\*/);
|
||||
assert.match(String(f.evidence || ''), /B\*/);
|
||||
assert.doesNotMatch(String(f.evidence || ''), /puppeteer/); // anchored MCP glob is valid
|
||||
assert.doesNotMatch(String(f.evidence || ''), /npm run/); // specifier glob is valid
|
||||
});
|
||||
|
||||
it('clean settings (no unanchored globs) → no ineffective-allow finding', async () => {
|
||||
const result = await runScanner('healthy-project');
|
||||
const f = result.findings.find(x => /ineffective allow wildcard/i.test(x.title || ''));
|
||||
assert.equal(f, undefined, `expected no ineffective-allow finding; got: ${f?.title}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DIS scanner — deny-all glob Tool(*) kills a bare allow (end-to-end)', () => {
|
||||
// settings.json: allow: ["Bash"], deny: ["Bash(*)"]
|
||||
// Bash(*) deny ≡ bare Bash deny → the bare Bash allow is dead config.
|
||||
it('flags the bare Bash allow as dead under a Bash(*) deny', async () => {
|
||||
const result = await runScanner('deny-all-glob');
|
||||
const f = result.findings.find(x => /both permissions\.deny and permissions\.allow/i.test(x.title || ''));
|
||||
assert.ok(f, `expected the bare Bash allow to be flagged dead; got: ${result.findings.map(x => x.title).join(' | ')}`);
|
||||
assert.match(String(f.evidence || ''), /Bash/);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue