diff --git a/CLAUDE.md b/CLAUDE.md index bb3b0be..c11c39e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -201,6 +201,26 @@ flagged, because Claude Code keeps scanning the folder in that case (`addressesD predicate). The v5.4.0 plan originally listed `commands/agents/skills/hooks`; that set was corrected here against the live docs (Verifiseringsplikt). +### PLH scanner — skills:-array validation (`CA-PLH-016`) + +Per-plugin check (in `scanSinglePlugin`, after the shadow check): when `plugin.json` has a +`skills` field (string or array), each entry must resolve to an **existing directory inside the +plugin root**. The value is normalized `Array.isArray(v) ? v : [v]`, so a single string is one +entry — and a non-string top-level value (e.g. `42`) is naturally caught as a single non-string +entry (no separate top-level check needed). One finding per bad entry, severity **MEDIUM**, +`category: 'plugin-hygiene'`, `details: { field: 'skills', entry, problem }` where `problem` is +one of `non-string` / `escapes-root` / `not-found` / `not-a-directory`. Mirrors +`claude plugin validate` (~2.1.145). + +Escape detection uses `skillsEntryEscapesRoot` (resolve + `startsWith(pluginDir + sep)` +containment — robust against a literal `..foo` dir name), backed by the docs' path-traversal rule +(*"Installed plugins cannot reference files outside their directory … such as `../shared-utils`"*). +`statOrNull` distinguishes missing from file-vs-dir. **Verifiseringsplikt note:** the v5.4.0 plan +claimed CC "suggests the parent directory when an entry points at a file"; that exact error text is +**not** in the primary docs, so it was dropped — the finding asserts only the four +primary-source-verified conditions. `skills` is deliberately *not* in `SHADOWING_PATH_FIELDS` +(it adds to the default scan, never shadows). + ## Gotchas - Session directories accumulate — use `/config-audit cleanup` to manage diff --git a/README.md b/README.md index ca97aa2..4b7364e 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ ![Commands](https://img.shields.io/badge/commands-18-green) ![Agents](https://img.shields.io/badge/agents-6-orange) ![Hooks](https://img.shields.io/badge/hooks-4-red) -![Tests](https://img.shields.io/badge/tests-941+-brightgreen) +![Tests](https://img.shields.io/badge/tests-944+-brightgreen) ![License](https://img.shields.io/badge/license-MIT-lightgrey) 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. @@ -383,6 +383,16 @@ By default, `/config-audit` auto-detects scope from your git context. Override w > flag a custom path that points back into the default folder (e.g. > `"commands": ["./commands/x.md"]`), because the folder is then addressed explicitly. +> **`skills:`-array validation — every listed path must be a real skill folder.** +> A plugin's `plugin.json` may list custom skill directories in a `skills` array (each entry a +> path to a folder containing `SKILL.md`). PLH validates each entry (`CA-PLH-016`, **medium**) +> and flags four ways an entry can be broken: it isn't a string, it points at a path that +> **doesn't exist**, it points at a **file** instead of a directory, or it **escapes the plugin +> root** (`../…` — installed plugins can't reference files outside their own directory, so the +> skill never loads). A valid existing directory is never flagged. This mirrors +> `claude plugin validate`. Note `skills` *adds to* the default `skills/` scan, so a custom path +> here is never a shadow — it just has to resolve to a real folder. + ### CLI Tools All tools work standalone — no Claude Code session needed: diff --git a/scanners/lib/humanizer-data.mjs b/scanners/lib/humanizer-data.mjs index 0c6c330..f550b7a 100644 --- a/scanners/lib/humanizer-data.mjs +++ b/scanners/lib/humanizer-data.mjs @@ -742,6 +742,14 @@ export const TRANSLATIONS = { recommendation: 'Either delete the unused default folder, or keep it by listing it explicitly alongside the custom path. The details show which folder and field.', }, }, + { + regex: /^plugin\.json "skills" entry /, + translation: { + title: 'A plugin lists a skill path that doesn\'t point to a real skill folder', + description: 'The plugin\'s `plugin.json` lists a `skills` path that isn\'t a folder inside the plugin — it\'s missing, points at a file, sits outside the plugin, or isn\'t text. Claude Code loads no skill from it.', + recommendation: 'Point each `skills` entry at a real folder inside the plugin that contains a `SKILL.md`. The details show which entry and what\'s wrong.', + }, + }, ], _default: { title: 'A plugin has a configuration issue', diff --git a/scanners/plugin-health-scanner.mjs b/scanners/plugin-health-scanner.mjs index 1b7a820..3503cf1 100644 --- a/scanners/plugin-health-scanner.mjs +++ b/scanners/plugin-health-scanner.mjs @@ -9,7 +9,7 @@ */ import { readdir, stat, readFile } from 'node:fs/promises'; -import { join, basename, resolve } from 'node:path'; +import { join, basename, resolve, sep } from 'node:path'; import { finding, scannerResult, resetCounter } from './lib/output.mjs'; import { SEVERITY } from './lib/severity.mjs'; import { parseFrontmatter } from './lib/yaml-parser.mjs'; @@ -71,6 +71,50 @@ async function dirExists(p) { } } +/** Stat `p`, or null when it does not exist (distinguishes missing from file/dir). */ +async function statOrNull(p) { + try { + return await stat(p); + } catch { + return null; + } +} + +/** + * True when a `skills` entry resolves outside the plugin root. Installed plugins + * cannot reference files outside their own directory (docs: path-traversal + * limitations), so "../shared" or an absolute path will not load. + */ +function skillsEntryEscapesRoot(pluginDir, entry) { + const resolved = resolve(pluginDir, entry.replace(/^\.\//, '')); + return resolved !== pluginDir && !resolved.startsWith(pluginDir + sep); +} + +// Per-problem prose for a malformed `skills` entry. Each `title` starts with +// `plugin.json "skills" entry` so the family is greppable. +const SKILLS_ENTRY_MESSAGES = { + 'non-string': { + title: e => `plugin.json "skills" entry is not a string: ${JSON.stringify(e)}`, + description: 'Each "skills" entry must be a relative path string (starting with "./") to a skill directory.', + recommendation: 'Replace the non-string entry with a path like "./my-skill/", or remove it.', + }, + 'escapes-root': { + title: e => `plugin.json "skills" entry escapes the plugin root: ${e}`, + description: 'Installed plugins cannot reference files outside their own directory, so a skills path that traverses outside the plugin root (e.g. "../shared") will not load.', + recommendation: 'Point the entry at a directory inside the plugin, or vendor the skill into the plugin.', + }, + 'not-found': { + title: e => `plugin.json "skills" entry does not exist: ${e}`, + description: 'The "skills" entry points at a path that does not exist in the plugin, so no skill loads from it.', + recommendation: 'Create the directory, fix the path, or remove the entry.', + }, + 'not-a-directory': { + title: e => `plugin.json "skills" entry is a file, not a directory: ${e}`, + description: 'A "skills" entry must be a directory containing a SKILL.md (or /SKILL.md), not a file.', + recommendation: 'Point the entry at the skill directory (the folder that contains SKILL.md), not the file.', + }, +}; + /** * Discover plugins under a path. * Looks for .claude-plugin/plugin.json pattern. @@ -204,6 +248,40 @@ async function scanSinglePlugin(pluginDir) { details: { field: key, ignoredDir: defaultDir, customPaths }, })); } + + // skills:-array validation: each entry must resolve to an existing + // directory inside the plugin root. Mirrors `claude plugin validate`. + // skills is string|array (a single string is one entry). Unlike the + // shadow check, skills ADDS to the default skills/ scan, so a custom path + // here is never a shadow — it just has to be a real directory. + if (parsed.skills !== undefined && parsed.skills !== null) { + const entries = Array.isArray(parsed.skills) ? parsed.skills : [parsed.skills]; + for (const entry of entries) { + let problem = null; + if (typeof entry !== 'string') { + problem = 'non-string'; + } else if (skillsEntryEscapesRoot(pluginDir, entry)) { + problem = 'escapes-root'; + } else { + const st = await statOrNull(resolve(pluginDir, entry.replace(/^\.\//, ''))); + if (!st) problem = 'not-found'; + else if (!st.isDirectory()) problem = 'not-a-directory'; + } + if (!problem) continue; + const m = SKILLS_ENTRY_MESSAGES[problem]; + findings.push(finding({ + scanner: SCANNER, + severity: SEVERITY.medium, + title: m.title(entry), + description: `Plugin "${pluginName}": ${m.description}`, + file: pluginJsonPath, + evidence: `skills entry=${JSON.stringify(entry)}; problem=${problem}`, + recommendation: m.recommendation, + category: 'plugin-hygiene', + details: { field: 'skills', entry, problem }, + })); + } + } } } catch { findings.push(finding({ diff --git a/tests/fixtures/plugin-skills-array/.claude-plugin/plugin.json b/tests/fixtures/plugin-skills-array/.claude-plugin/plugin.json new file mode 100644 index 0000000..c3e08d8 --- /dev/null +++ b/tests/fixtures/plugin-skills-array/.claude-plugin/plugin.json @@ -0,0 +1,6 @@ +{ + "name": "skills-array-plugin", + "description": "Fixture: plugin.json skills: array entry validation (CA-PLH-016)", + "version": "1.0.0", + "skills": ["./valid-skill/", "./a-file.md", "./missing-dir/", "../escape", 42] +} diff --git a/tests/fixtures/plugin-skills-array/CLAUDE.md b/tests/fixtures/plugin-skills-array/CLAUDE.md new file mode 100644 index 0000000..41ff765 --- /dev/null +++ b/tests/fixtures/plugin-skills-array/CLAUDE.md @@ -0,0 +1,15 @@ +# Skills Array Plugin + +Fixture for CA-PLH-016 skills:-array validation. + +## Commands + +(none) + +## Agents + +(none) + +## Hooks + +(none) diff --git a/tests/fixtures/plugin-skills-array/a-file.md b/tests/fixtures/plugin-skills-array/a-file.md new file mode 100644 index 0000000..c3bb361 --- /dev/null +++ b/tests/fixtures/plugin-skills-array/a-file.md @@ -0,0 +1 @@ +This is a plain file, not a skill directory. A skills: entry pointing here is invalid. diff --git a/tests/fixtures/plugin-skills-array/valid-skill/SKILL.md b/tests/fixtures/plugin-skills-array/valid-skill/SKILL.md new file mode 100644 index 0000000..3b9f3fd --- /dev/null +++ b/tests/fixtures/plugin-skills-array/valid-skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: skills-array-valid-skill +description: A valid skill directory referenced from the skills: array +--- + +# Valid Skill diff --git a/tests/scanners/plugin-health-scanner.test.mjs b/tests/scanners/plugin-health-scanner.test.mjs index d2747ee..0dc0414 100644 --- a/tests/scanners/plugin-health-scanner.test.mjs +++ b/tests/scanners/plugin-health-scanner.test.mjs @@ -12,6 +12,7 @@ const BROKEN_PLUGIN = resolve(FIXTURES, 'broken-plugin'); const DUP_NAME = resolve(FIXTURES, 'duplicate-plugin-name'); const DUP_CMD = resolve(FIXTURES, 'duplicate-command-name'); const SHADOW = resolve(FIXTURES, 'plugin-shadow-folder'); +const SKILLS_ARR = resolve(FIXTURES, 'plugin-skills-array'); describe('discoverPlugins', () => { it('discovers a single plugin when pointed at plugin dir', async () => { @@ -278,6 +279,48 @@ describe('plugin-folder shadowing (CA-PLH-015)', () => { }); }); +describe('skills:-array entry validation (CA-PLH-016)', () => { + // Title set by the scanner: `plugin.json "skills" entry : `. + const SKILLS_RE = /^plugin\.json "skills" entry/; + + it('flags one finding per bad entry (file, missing, escape, non-string) and none for a valid dir', async () => { + resetCounter(); + // skills: ["./valid-skill/", "./a-file.md", "./missing-dir/", "../escape", 42] + const result = await scan(SKILLS_ARR); + const bad = result.findings.filter(f => f.scanner === 'PLH' && SKILLS_RE.test(f.title || '')); + assert.equal(bad.length, 4, `Expected 4 bad-entry findings, got ${bad.length}: ${bad.map(f => f.title).join(' | ')}`); + for (const f of bad) { + assert.equal(f.severity, 'medium', `skills entry problem is medium: ${f.title}`); + assert.equal(f.category, 'plugin-hygiene'); + assert.ok(/plugin\.json$/.test(f.file || ''), `Finding should point at plugin.json: ${f.file}`); + assert.ok(f.details && f.details.field === 'skills', 'details.field should be "skills"'); + } + const problems = bad.map(f => f.details.problem).sort(); + assert.deepEqual( + problems, + ['escapes-root', 'non-string', 'not-a-directory', 'not-found'], + `Each problem type should appear once; got ${problems.join(',')}` + ); + }); + + it('does NOT flag the valid skill directory', async () => { + resetCounter(); + const result = await scan(SKILLS_ARR); + const validFlagged = result.findings.some(f => + f.scanner === 'PLH' && SKILLS_RE.test(f.title || '') && /valid-skill/.test(f.title || '') + ); + assert.equal(validFlagged, false, './valid-skill/ is an existing directory → not flagged'); + }); + + it('does NOT flag a plugin with no skills: key', async () => { + resetCounter(); + // test-plugin declares no skills: field. + const result = await scan(TEST_PLUGIN); + const skillsFindings = result.findings.filter(f => f.scanner === 'PLH' && SKILLS_RE.test(f.title || '')); + assert.equal(skillsFindings.length, 0, 'No skills: key → no skills-entry findings'); + }); +}); + describe('finding format', () => { it('findings have standard fields', async () => { resetCounter();