feat(plh): validate plugin.json skills:-array entries (CA-PLH-016)
PLH now validates each entry of a plugin.json `skills` field (string|array):
every entry must resolve to an existing directory inside the plugin root.
One finding per bad entry (medium, plugin-hygiene), problem ∈ {non-string,
escapes-root, not-found, not-a-directory}. Mirrors `claude plugin validate`.
String|array normalized so a non-string top-level value is caught too.
Verifiseringsplikt: the plan's "CC suggests the parent directory" error text
is NOT in the primary docs — dropped. Only the four primary-source-verified
conditions are asserted (escape backed by the path-traversal rule).
Tests +3 (941->944). Scanner count unchanged (13). --json/--raw byte-stable.
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
9b63125f6a
commit
76d5eda101
9 changed files with 189 additions and 2 deletions
20
CLAUDE.md
20
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
|
||||
|
|
|
|||
12
README.md
12
README.md
|
|
@ -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.
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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 <name>/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({
|
||||
|
|
|
|||
6
tests/fixtures/plugin-skills-array/.claude-plugin/plugin.json
vendored
Normal file
6
tests/fixtures/plugin-skills-array/.claude-plugin/plugin.json
vendored
Normal file
|
|
@ -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]
|
||||
}
|
||||
15
tests/fixtures/plugin-skills-array/CLAUDE.md
vendored
Normal file
15
tests/fixtures/plugin-skills-array/CLAUDE.md
vendored
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
# Skills Array Plugin
|
||||
|
||||
Fixture for CA-PLH-016 skills:-array validation.
|
||||
|
||||
## Commands
|
||||
|
||||
(none)
|
||||
|
||||
## Agents
|
||||
|
||||
(none)
|
||||
|
||||
## Hooks
|
||||
|
||||
(none)
|
||||
1
tests/fixtures/plugin-skills-array/a-file.md
vendored
Normal file
1
tests/fixtures/plugin-skills-array/a-file.md
vendored
Normal file
|
|
@ -0,0 +1 @@
|
|||
This is a plain file, not a skill directory. A skills: entry pointing here is invalid.
|
||||
6
tests/fixtures/plugin-skills-array/valid-skill/SKILL.md
vendored
Normal file
6
tests/fixtures/plugin-skills-array/valid-skill/SKILL.md
vendored
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
name: skills-array-valid-skill
|
||||
description: A valid skill directory referenced from the skills: array
|
||||
---
|
||||
|
||||
# Valid Skill
|
||||
|
|
@ -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 <problem>: <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();
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue