config-audit/scanners/lib/finding-codes.mjs
Kjell Tore Guttormsen 7a794b47eb fix(scanners)!: a finding ID names the check, not the emission (M-BUG-28)
BREAKING CHANGE: the {NNN} in CA-{SCANNER}-{NNN} identifies the check that
produced the finding. It used to be the finding's position in that scanner's
output for that run, which made it unstable across CONFIGURATIONS, not just
across releases as STATE framed it. Measured on two fixtures: "No custom
subagents" was CA-GAP-007 on minimal-project and CA-GAP-004 on healthy-project.
A user who fixed an unrelated earlier gap silently renumbered every later one,
so a .config-audit-ignore pin retargeted to a neighbouring finding with no
version change at all.

Second measured arm: README already documented the opposite scheme. It and the
scanner headers describe ~20 numbers as check codes (CA-SKL-003 = oversized
body, CA-PLH-015 = folder shadowing, CA-TOK-006 = schema deferral), and the
counter could only produce those in the all-fire case -- source-order positions
are 4, 3 and 8. The documentation described the scheme; the implementation was
what was wrong. Every published number is preserved by construction and pinned
exhaustively in tests/lib/finding-codes.test.mjs.

scanners/lib/finding-codes.mjs is the single authority. Every finding() call
passes a `code`; an undeclared or missing one THROWS. No counter fallback --
that would reproduce D1's findGapId -> 'unknown' silent degradation and let a
half-converted scanner ship IDs that look valid. findingCounter/resetCounter
are deleted outright, not left as no-ops. Retirement is now a mechanism:
RETIRED_CODES tombstones a withdrawn key so its number is never reissued,
seeded with GAP t3_8 -- the D1 removal that opened this chunk.

IDs are consequently NOT unique per finding: one check failing in three files
emits three findings sharing an ID. That inverts which consumer is correct, so
every f.id/findingId site was classified before the change. diff-engine and
most of fix-engine already keyed on scanner+title+file (drift was never lying);
fix-engine's verification did not, and keyed on the ID alone -- fixing one of
two sibling instances marked both fixed, and the untouched one, still present
in the re-scan, was reported as a REGRESSION. Red test first, then keyed on
(findingId, file), which both planFixes and applyFixes already carry.
plugin-health's crossIds Set was measured and is a clean negative: cross
findings are allFindings.slice(crossPluginStart) and codes 18/19 are emitted
only in that tail, so the partition holds by construction.

unknownSuppressions() reports a pin that names no declared check, in the
--output-file payload (ux-rules rule 2 -- a stderr-only warning is invisible to
the commands) and only when one exists, so a clean config is byte-identical.
That is what makes the break safe: a stale pin goes loud instead of dying quiet.

Frozen tests/snapshots/v5.0.0/ untouched on disk. IDs are masked out of that
comparison (mask-finding-ids.mjs) rather than re-derived -- re-deriving
positional IDs would assert the retired scheme against itself, and #58's
isGapEntry off-by-one is the measured example of that misfiring. The dead
re-derivation is removed from strip-retired-gap.mjs. default-output snapshots
re-approved after confirming the diff is IDs and nothing else.

Guards, each seen red against its own defect: a missing code (scanner errors
out mid-sweep), an orphan declaration, a resurrected retired key, and a
documented ID naming no check. The sweep asserts the union across all 16
scanners, never per scanner -- a per-scanner assertion goes green on a partial
conversion.

Fasit written before implementation: docs/mbug28-id-semantics-fasit.local.md,
including one correction made before running (CML has 12 checks over 13 call
sites -- the anchored and calibrated char-budget arms are one check, which a
repeated-title sweep found and my call-site count had missed).

Suite 1535 -> 1573, 0 failing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MyqCQKK2ornJ1jFWwqx17E
2026-08-09 23:26:36 +02:00

316 lines
11 KiB
JavaScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* Finding-code registry — the authority for the {NNN} in `CA-{SCANNER}-{NNN}`.
*
* A finding ID names the CHECK, not the finding's position in a run (M-BUG-28).
* Before this registry, `{NNN}` came from an emission counter, so the same check
* carried different IDs on different configurations: fixing an unrelated earlier
* gap silently renumbered every later one, and a `.config-audit-ignore` entry
* retargeted to a neighbouring finding without the user changing anything.
*
* Rules for editing this file:
*
* 1. A number, once published, belongs to its check forever. Adding a check
* takes the next free number for that scanner — never the next source-order
* position, and never a number listed in RETIRED.
* 2. Removing a check moves its key to RETIRED. The number is never reissued;
* a user's suppression must go dead rather than quietly point at a
* different finding. (D1 retired GAP `t3_8` under the old scheme, which is
* the incident that motivated the registry.)
* 3. Several call sites may share one code when they are arms of one check —
* e.g. the forward/reverse arms of a permission conflict. Duplicate
* emission is legal; a finding is identified by (id, file, line).
* 4. Numbers below are NOT all source order: the ones marked "documented"
* are pinned by README / command copy that shipped before the registry.
*
* GAP keys are the `GAP_CHECKS[].id` values from `feature-gap-scanner.mjs`,
* which were already stable. They are declared here rather than derived, so
* numbers live in exactly one place; `tests/lib/finding-codes.test.mjs` binds
* the two together instead of a second copy of the table drifting.
*/
/**
* @type {Record<string, Record<string, number>>}
* scanner prefix → check key → number
*/
export const FINDING_CODES = {
// ── CML: claude-md-linter (source order) ────────────────────────────────
// `over-char-budget` has two call sites: the conservative 200k anchor and the
// `--context-window` calibrated variant. One check, one code.
CML: {
'no-claude-md': 1,
'nested-not-reinjected': 2,
'over-500-lines': 3,
'over-200-lines': 4,
'over-char-budget': 5,
'nearly-empty': 6,
'missing-sections': 7,
'no-headings': 8,
'deep-relative-import': 9,
'html-comments': 10,
'repeated-content': 11,
'todo-markers': 12,
},
// ── SET: settings-validator (source order) ──────────────────────────────
SET: {
'invalid-json': 1,
'key-typo': 2,
'deprecated-key': 3,
'type-mismatch': 4,
'invalid-effort-level': 5,
'missing-schema': 6,
'no-deny-rules': 7,
'no-allow-rules': 8,
'many-additional-dirs': 9,
'automode-not-object': 10,
'automode-unknown-subkey': 11,
'automode-subkey-not-string-array': 12,
'automode-in-shared-settings': 13,
'hooks-as-array': 14,
},
// ── HKV: hook-validator (source order) ──────────────────────────────────
HKV: {
'invalid-json': 1,
'hooks-not-object': 2,
'unknown-event': 3,
'handlers-not-array': 4,
'matcher-not-string': 5,
'missing-hooks-array': 6,
'invalid-handler-type': 7,
'script-not-found': 8,
'verbose-output': 9,
'unfiltered-additional-context': 10,
'timeout-not-number': 11,
'timeout-out-of-range': 12,
},
// ── RUL: rules-validator (source order) ─────────────────────────────────
RUL: {
'no-frontmatter': 1,
'globs-instead-of-paths': 2,
'pattern-matches-nothing': 3,
'nearly-empty': 4,
'large-unscoped': 5,
'large-scoped-lost-after-compaction': 6,
'not-markdown': 7,
},
// ── MCP: mcp-config-validator (source order) ────────────────────────────
MCP: {
'invalid-json': 1,
'unknown-server-type': 2,
'sse-transport': 3,
'unreferenced-env-var': 4,
'unknown-server-field': 5,
},
// ── IMP: import-resolver (source order) ─────────────────────────────────
IMP: {
'tilde-path': 1,
'broken-link': 2,
'circular-reference': 3,
'deep-chain': 4,
},
// ── CNF: conflict-detector ──────────────────────────────────────────────
// `permission-allow-deny` covers both arms (allow-in-A/deny-in-B and reverse).
CNF: {
'settings-key-conflict': 1,
'permission-allow-deny': 2,
'duplicate-hook': 3,
},
// ── DIS: disabled-in-schema-scanner (source order) ──────────────────────
DIS: {
'deny-and-allow': 1,
'ineffective-allow-wildcard': 2,
'forbidden-param-deny': 3,
'forbidden-param-allow': 4,
},
// ── CPS: cache-prefix-scanner (CPS-001 documented) ──────────────────────
CPS: {
'volatile-in-prefix': 1,
'volatile-in-import': 2,
},
// ── COL: collision-scanner (source order) ───────────────────────────────
COL: {
'skill-user-vs-plugin': 1,
'skill-multi-plugin': 2,
},
// ── AGT: agent-listing-scanner (both documented; source order matches) ──
AGT: {
'description-bloat': 1,
'aggregate-listing-budget': 2,
},
// ── OST: output-style-scanner (all three documented) ────────────────────
OST: {
'strips-coding-instructions': 1,
'plugin-forces-style': 2,
'style-not-found': 3,
},
// ── OPT: optimization-lens-scanner (documented) ─────────────────────────
OPT: {
'procedure-should-be-skill': 1,
},
// ── SKL: skill-listing-scanner (all three documented) ───────────────────
// `aggregate-listing-budget` has two call sites: the conservative 200k anchor
// and the calibrated `--context-window` variant. One check, one code.
SKL: {
'description-over-cap': 1,
'aggregate-listing-budget': 2,
'oversized-body': 3,
},
// ── TOK: token-hotspots ─────────────────────────────────────────────────
// 1/2/3/5/6 are documented (README + commands/tokens.md). `mcp-schema-deferral`
// is documented as 006 although it is the 8th call site in source order, so
// `cascade-over-budget` and `stale-plugin-cache` take the free 7 and 8.
TOK: {
'volatile-top': 1,
'redundant-permissions': 2,
'deep-import-chain': 3,
'bloated-skill-description': 4,
'mcp-schema-budget': 5,
'mcp-schema-deferral': 6,
'cascade-over-budget': 7,
'stale-plugin-cache': 8,
},
// ── PLH: plugin-health-scanner ──────────────────────────────────────────
// 15 and 16 are documented (README v5.4.0 entry) but sit at source positions
// 3 and 4; the remaining checks take {1…14, 17, 18, 19} in source order.
PLH: {
'invalid-plugin-json': 1,
'missing-required-field': 2,
'missing-plugin-json': 3,
'claude-md-missing-section': 4,
'missing-claude-md': 5,
'command-missing-frontmatter': 6,
'command-missing-field': 7,
'agent-missing-frontmatter': 8,
'agent-missing-field': 9,
'agent-ignored-key': 10,
'hooks-json-invalid-structure': 11,
'hooks-json-array': 12,
'hooks-json-invalid': 13,
'unknown-plugin-file': 14,
'plugin-json-shadows-default': 15,
'skills-array-entry': 16,
'no-plugins-found': 17,
'command-name-collision': 18,
'namespace-collision': 19,
},
// ── GAP: feature-gap-scanner ────────────────────────────────────────────
// Keys are GAP_CHECKS[].id (already stable). Dimensions 124 in table order,
// then the three conditional levers, which the scanner emits after the loop.
GAP: {
t1_1: 1,
t1_2: 2,
t1_3: 3,
t1_4: 4,
t1_5: 5,
t2_1: 6,
t2_2: 7,
t2_3: 8,
t2_4: 9,
t2_5: 10,
t2_6: 11,
t2_7: 12,
t3_1: 13,
t3_2: 14,
t3_3: 15,
t3_4: 16,
t3_5: 17,
t3_6: 18,
t3_7: 19,
t4_1: 20,
t4_2: 21,
t4_3: 22,
t4_4: 23,
t4_5: 24,
'bundled-skills-lever': 25,
'cli-over-mcp-lever': 26,
'filter-hook-output-lever': 27,
},
};
/**
* Keys withdrawn from a scanner. Their numbers are never reissued, so a stale
* suppression goes dead instead of silently naming a different check.
* @type {Record<string, string[]>}
*/
export const RETIRED_CODES = {
// D1 (4027cdc, 2026-08-09): "No autoMode classifier" — /doctor Check 8 covers
// auto mode with usage-weighted judgement, so the nudge went. The number it
// occupied under the old counter scheme is not reused.
GAP: ['t3_8'],
};
/**
* Resolve a check key to its published number.
* Throws rather than falling back: a fallback would let a half-converted scanner
* ship IDs that look valid, which is the silent-degradation class this registry
* exists to remove.
* @param {string} scanner - scanner prefix, e.g. 'GAP'
* @param {string} code - check key, e.g. 't3_7'
* @returns {number}
*/
export function codeNumber(scanner, code) {
const table = FINDING_CODES[scanner];
if (!table) {
throw new Error(`finding(): unknown scanner "${scanner}" — add it to FINDING_CODES`);
}
if (code === undefined || code === null || code === '') {
throw new Error(`finding(): missing "code" for scanner ${scanner} — every finding must name its check`);
}
if (!Object.prototype.hasOwnProperty.call(table, code)) {
const retired = (RETIRED_CODES[scanner] || []).includes(code);
throw new Error(
retired
? `finding(): check "${code}" is RETIRED for ${scanner} — retired numbers are never reissued`
: `finding(): undeclared check "${code}" for ${scanner} — add it to FINDING_CODES with the next free number`
);
}
return table[code];
}
/**
* Render a finding ID from a check key.
* @param {string} scanner
* @param {string} code
* @returns {string} e.g. 'CA-GAP-019'
*/
export function findingId(scanner, code) {
return `CA-${scanner}-${String(codeNumber(scanner, code)).padStart(3, '0')}`;
}
/**
* Every declared ID, as a flat set — used to validate suppression patterns so a
* stale pin is reported instead of silently matching nothing.
* @returns {Set<string>}
*/
export function allFindingIds() {
const ids = new Set();
for (const [scanner, table] of Object.entries(FINDING_CODES)) {
for (const n of Object.values(table)) {
ids.add(`CA-${scanner}-${String(n).padStart(3, '0')}`);
}
}
return ids;
}
/**
* Scanner prefixes the registry knows about.
* @returns {string[]}
*/
export function knownScanners() {
return Object.keys(FINDING_CODES);
}