diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 7d7b657..be5cd0e 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,5 +1,5 @@ { - "$schema": "https://anthropic.com/claude-code/marketplace.schema.json", + "$schema": "https://json.schemastore.org/claude-code-marketplace.json", "name": "agentic-utilities", "description": "Curated Claude Code plugins from agentic-utilities — a harness-agnostic collection of skills, plugins, and extensions across pi, Claude Code, and Codex. This marketplace exposes the Claude Code plugins.", "owner": { @@ -54,7 +54,7 @@ }, { "name": "ask-codex", - "description": "OpenAI Codex CLI integration for Claude Code. Wraps `codex exec` and `codex resume` with model selection (gpt-5.4 / gpt-5.3-codex / gpt-5.4-nano), reasoning effort control (high/medium/low), sandbox modes (read-only / workspace-write / danger-full-access), session resumption, and stderr suppression for thinking tokens.", + "description": "OpenAI Codex CLI integration for Claude Code. Wraps `codex exec` and `codex resume` with model selection (gpt-5.4 / gpt-5.3-codex / gpt-5.4-nano), reasoning effort control (high/medium/low), sandbox modes (read-only / workspace-write / danger-full-access), session resumption, and stderr suppression for thinking tokens. Requires the `codex` binary on PATH.", "version": "0.1.0", "author": { "name": "Ossie Irondi" diff --git a/.gitignore b/.gitignore index 5a11619..a9e42e0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ node_modules/ .pnpm-store/ .DS_Store +__pycache__/ +*.py[cod] .env .env.* !.env.example diff --git a/claude-code/plugins/ask-codex/.claude-plugin/plugin.json b/claude-code/plugins/ask-codex/.claude-plugin/plugin.json index 6fc83e0..2e15644 100644 --- a/claude-code/plugins/ask-codex/.claude-plugin/plugin.json +++ b/claude-code/plugins/ask-codex/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "ask-codex", "version": "0.1.0", - "description": "OpenAI Codex CLI integration for Claude Code. Wraps `codex exec` and `codex resume` with model selection (gpt-5.4 / gpt-5.3-codex / gpt-5.4-nano), reasoning effort control (high/medium/low), sandbox modes (read-only / workspace-write / danger-full-access), session resumption, and stderr suppression for thinking tokens.", + "description": "OpenAI Codex CLI integration for Claude Code. Wraps `codex exec` and `codex resume` with model selection (gpt-5.4 / gpt-5.3-codex / gpt-5.4-nano), reasoning effort control (high/medium/low), sandbox modes (read-only / workspace-write / danger-full-access), session resumption, and stderr suppression for thinking tokens. Requires the `codex` binary on PATH.", "author": { "name": "Ossie Irondi" }, diff --git a/claude-code/plugins/awesome-readme/skills/awesome-readme/SKILL.md b/claude-code/plugins/awesome-readme/skills/awesome-readme/SKILL.md index 6bb11f1..c939fc9 100644 --- a/claude-code/plugins/awesome-readme/skills/awesome-readme/SKILL.md +++ b/claude-code/plugins/awesome-readme/skills/awesome-readme/SKILL.md @@ -57,24 +57,16 @@ If neither (1) nor (2) is set up and the user picks "Yes, generate via gpt-image ### How to invoke -When a workflow needs an image, invoke the **gpt-image-2** skill directly: +When a workflow needs an image, invoke the installed **gpt-image-2** skill directly (do not try to locate or run its script yourself; the skill loader resolves that). Pass: -```bash -bash scripts/gen.sh \ - --prompt "" \ - --out docs/.png -``` +- `--prompt ""` +- `--out docs/.png` -For reference-image conditioning (remix an existing logo, restyle a screenshot, etc.): +For reference-image conditioning (remix an existing logo, restyle a screenshot, etc.), also pass: -```bash -bash scripts/gen.sh \ - --prompt "" \ - --ref /absolute/path/to/reference.png \ - --out docs/.png -``` +- `--ref /absolute/path/to/reference.png` -The script lives inside the gpt-image-2 skill's directory; the standard skill loader resolves it. Pass the user's prompt through raw — only polish wording when the user explicitly asks for it. After the image lands, display/attach it in the response; never stop at "saved to docs/x.png". +Pass the user's prompt through raw. Only polish wording when the user explicitly asks for it. After the image lands, display/attach it in the response; never stop at "saved to docs/x.png". ### Prompt patterns for README elements diff --git a/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Create.md b/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Create.md index c5aaf46..c06869a 100755 --- a/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Create.md +++ b/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Create.md @@ -99,34 +99,15 @@ Follow this 10-section order — story before installation: If the user said yes in Step 2, invoke the **gpt-image-2** skill — once per visual. Pass the user's intent through raw; only polish wording when the user explicitly asked for "fancy" or "make it look great". -**Pattern:** - -```bash -bash scripts/gen.sh \ - --prompt "" \ - --out docs/.png -``` +**Pattern:** invoke the installed gpt-image-2 skill (do not try to locate or run its script yourself) with `--prompt ""` and `--out docs/.png`. **Example invocations for a typical Story-mode README:** -```bash -# Hero banner with embedded tagline -bash scripts/gen.sh \ - --prompt " README hero banner. . Embedded tagline reads: ''." \ - --out docs/hero.png - -# Architecture diagram -bash scripts/gen.sh \ - --prompt "Architecture diagram for . connects to via . Excalidraw / hand-drawn style. Clear labels on each node." \ - --out docs/architecture.png - -# Workflow illustration -bash scripts/gen.sh \ - --prompt "Workflow illustration: step 1 , step 2 , step 3 . Numbered left-to-right flow with annotated arrows." \ - --out docs/workflow.png -``` +- Hero banner with embedded tagline: `--prompt " README hero banner. . Embedded tagline reads: ''." --out docs/hero.png` +- Architecture diagram: `--prompt "Architecture diagram for . connects to via . Excalidraw / hand-drawn style. Clear labels on each node." --out docs/architecture.png` +- Workflow illustration: `--prompt "Workflow illustration: step 1 , step 2 , step 3 . Numbered left-to-right flow with annotated arrows." --out docs/workflow.png` -For reference-conditioned remixes (e.g., "restyle this existing logo as ukiyo-e"), add `--ref /absolute/path/to/reference.png`. +For reference-conditioned remixes (e.g., "restyle this existing logo as ukiyo-e"), also pass `--ref /absolute/path/to/reference.png`. After each call: 1. Confirm the output file exists at the `--out` path. diff --git a/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Improve.md b/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Improve.md index 3f7d366..428b20d 100755 --- a/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Improve.md +++ b/claude-code/plugins/awesome-readme/skills/awesome-readme/workflows/Improve.md @@ -130,24 +130,15 @@ For each section: If the user said yes in Step 3, invoke the **gpt-image-2** skill — once per visual. See SKILL.md "Image Generation" and Create.md Step 5 for the full pattern. -```bash -# Architecture diagram -bash scripts/gen.sh \ - --prompt "Architecture diagram for . connects to via . Excalidraw / hand-drawn style. Clear labels on each node." \ - --out docs/architecture.png - -# Workflow illustration -bash scripts/gen.sh \ - --prompt "Workflow illustration: step 1 , step 2 , step 3 . Numbered left-to-right flow with annotated arrows." \ - --out docs/workflow.png - -# Comparison visual (if applicable) -bash scripts/gen.sh \ - --prompt "Side-by-side comparison: '' vs ''. Two columns, contrasting treatment, clear winner indicated." \ - --out docs/comparison.png -``` +Invoke the installed gpt-image-2 skill (do not try to locate or run its script yourself): + +- Architecture diagram: `--prompt "Architecture diagram for . connects to via . Excalidraw / hand-drawn style. Clear labels on each node." --out docs/architecture.png` +- Workflow illustration: `--prompt "Workflow illustration: step 1 , step 2 , step 3 . Numbered left-to-right flow with annotated arrows." --out docs/workflow.png` +- Comparison visual (if applicable): `--prompt "Side-by-side comparison: '' vs ''. Two columns, contrasting treatment, clear winner indicated." --out docs/comparison.png` + +For reference-conditioned remixes (e.g., restyling an existing logo or diagram), also pass `--ref /absolute/path/to/reference.png`. -Pass the user's prompt through raw — only polish wording when they explicitly asked for it. Reference each output in the README with descriptive alt text. If `gen.sh` fails, name the failing layer in one sentence and ask the user how to proceed; do not silently fall back to a weaker model. +Pass the user's prompt through raw. Only polish wording when they explicitly asked for it. Reference each output in the README with descriptive alt text. If image generation fails, name the failing layer in one sentence and ask the user how to proceed; do not silently fall back to a weaker model. ## Step 8: Validate and Deliver diff --git a/claude-code/plugins/harness-audit/skills/harness-audit/SKILL.md b/claude-code/plugins/harness-audit/skills/harness-audit/SKILL.md index 465999a..2d29624 100644 --- a/claude-code/plugins/harness-audit/skills/harness-audit/SKILL.md +++ b/claude-code/plugins/harness-audit/skills/harness-audit/SKILL.md @@ -64,7 +64,7 @@ For each artifact, score `✅ Present / ⚠️ Partial / ❌ Missing` with concr 2. **Rules directory** — per-domain coding standards in `rules/`, `.cursor/rules/`, `docs/rules/`, `docs/solutions/`, etc. Imperative rules with reasoning. For repos with exported APIs, this includes an API documentation policy or ADR. 3. **Lint config with rich error messages** — ESLint / Biome / SwiftLint / ruff / clippy with custom rules whose messages explain why and how to fix. 4. **Pre-commit hooks** — Husky / lefthook / pre-commit / native git hooks running formatters, type checks, or targeted tests before push/commit. -5. **Test suite agents can drive** — one-liner to run tests (`npm test`, `./scripts/test.sh`, `cargo test`, etc.). Fast enough and deterministic enough for agents. +5. **Test suite agents can drive**: one-liner to run tests (`npm test`, `make test`, `cargo test`, etc.). Fast enough and deterministic enough for agents. 6. **PR review automation** — CodeRabbit, AI reviewer workflows, persona reviewers, or at minimum CI that gates PRs. 7. **Repo-scoped skills/prompts** — `.agents/skills/`, `.pi/skills/`, `.claude/skills/`, `.codex/`, `.pi/prompts/`, or package-scoped resources. Prefer 5-10 deep resources over many shallow ones. 8. **Garbage-collection cadence** — documented recurring loop that converts agent/PR feedback into permanent rules, lints, docs, or skills. diff --git a/package-lock.json b/package-lock.json index ec5e255..9fb0f87 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,7 +23,8 @@ "husky": "9.1.7", "lint-staged": "16.4.0", "typebox": "latest", - "typescript": "latest" + "typescript": "latest", + "yaml": "^2.8.2" }, "engines": { "node": ">=20" diff --git a/package.json b/package.json index a87a377..7f4bf15 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ ] }, "scripts": { - "check": "npm run lint && npm run typecheck && npm test && npm run validate:plugins && npm run validate:skills && node scripts/list-resources.mjs", + "check": "npm run lint && npm run typecheck && npm test && npm run validate:plugins && node scripts/validate-package-contents.mjs && npm run validate:skills && node scripts/list-resources.mjs", "list": "node scripts/list-resources.mjs", "validate:plugins": "node scripts/validate-plugin-ports.mjs && node scripts/validate-codex-plugins.mjs", "install:bash-guard:global": "node scripts/install-bash-guard-global.mjs", @@ -53,7 +53,7 @@ "lint": "biome lint .", "lint:fix": "biome check --write .", "prepare": "husky", - "test": "node extensions/autopilot/v2-smoke-test.mjs && node extensions/question/question-smoke-test.mjs && node extensions/conditional-hooks/smoke-test.mjs && python3 skills/diataxis-docs-site/tests/test_create_site.py && node scripts/validate-package-contents.mjs", + "test": "node extensions/autopilot/v2-smoke-test.mjs && node extensions/question/question-smoke-test.mjs && node extensions/conditional-hooks/smoke-test.mjs && node scripts/lib/frontmatter-test.mjs && node scripts/lib/bundle-refs-test.mjs && node scripts/cli-entrypoint-test.mjs && python3 skills/diataxis-docs-site/tests/test_create_site.py", "typecheck": "tsc --noEmit", "validate:skills": "node scripts/validate-agent-skills.mjs" }, @@ -74,7 +74,8 @@ "husky": "9.1.7", "lint-staged": "16.4.0", "typebox": "latest", - "typescript": "latest" + "typescript": "latest", + "yaml": "^2.8.2" }, "engines": { "node": ">=20" diff --git a/scripts/cli-entrypoint-test.mjs b/scripts/cli-entrypoint-test.mjs new file mode 100644 index 0000000..61606b3 --- /dev/null +++ b/scripts/cli-entrypoint-test.mjs @@ -0,0 +1,48 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +// Regression for a URL-unsafe direct-execution guard: a naive +// `import.meta.url === \`file://${process.argv[1]}\`` comparison fails +// silently for any checkout path containing a space (import.meta.url +// percent-encodes it, the raw argv string does not), so `node +// validate-plugin-ports.mjs` would exit 0 having never called main(). +// +// This copies the real script and its lib dependencies into a temp +// directory whose path contains a space, symlinks node_modules alongside +// it so "yaml" still resolves, and runs it as a real subprocess. There is +// no claude-code/plugins directory in the copy, so a correctly-invoked +// main() must fail loudly with "plugins directory not found" — proof that +// main() actually ran, not just that the subprocess exited. + +const scriptsDir = path.resolve(path.dirname(fileURLToPath(import.meta.url))); +const repoRoot = path.resolve(scriptsDir, ".."); + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "cli-entrypoint-")); +const spacedDir = path.join(tmp, "path with a space"); +fs.mkdirSync(path.join(spacedDir, "scripts", "lib"), { recursive: true }); + +for (const file of ["validate-plugin-ports.mjs"]) { + fs.copyFileSync(path.join(scriptsDir, file), path.join(spacedDir, "scripts", file)); +} +for (const file of ["bundle-refs.mjs", "frontmatter.mjs"]) { + fs.copyFileSync(path.join(scriptsDir, "lib", file), path.join(spacedDir, "scripts", "lib", file)); +} +fs.symlinkSync(path.join(repoRoot, "node_modules"), path.join(spacedDir, "node_modules")); + +const targetScript = path.join(spacedDir, "scripts", "validate-plugin-ports.mjs"); +const result = spawnSync(process.execPath, [targetScript], { encoding: "utf8" }); + +fs.rmSync(tmp, { recursive: true, force: true }); + +assert.notEqual(result.status, 0, "main() must run and fail against the fixture's absent claude-code/plugins dir"); +assert.match( + result.stderr, + /plugins directory not found/, + "stderr must show main() actually executed listPlugins(), proving the space-containing path was recognized as direct execution", +); + +console.log("CLI entrypoint direct-execution fixtures passed"); diff --git a/scripts/lib/bundle-refs-test.mjs b/scripts/lib/bundle-refs-test.mjs new file mode 100644 index 0000000..6af3bf9 --- /dev/null +++ b/scripts/lib/bundle-refs-test.mjs @@ -0,0 +1,271 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { checkReferenceOnDisk } from "../validate-plugin-ports.mjs"; +import { extractBundleReferences, resolveBundleReference } from "./bundle-refs.mjs"; + +const SUBDIRS = ["workflows", "references", "tools", "scripts", "cli", "assets", "hooks"]; +const EXTENSIONS = [ + "md", + "ts", + "tsx", + "sh", + "json", + "js", + "mjs", + "cjs", + "py", + "png", + "jpg", + "jpeg", + "gif", + "svg", + "webp", +]; +function extract(text) { + return extractBundleReferences(text, { subdirs: SUBDIRS, extensions: EXTENSIONS }); +} + +// --- extractBundleReferences: token boundaries and asset coverage -------- + +{ + // A production-realistic image reference must actually be extracted — the + // prior REF_EXT list excluded image extensions entirely, so this never + // reached resolveBundleReference at all regardless of what it returned. + const refs = extract("See `assets/missing-logo.png` for the logo."); + assert.equal(refs.length, 1); + assert.deepEqual(refs[0], { prefixed: false, pathPart: "assets/missing-logo.png", ref: "assets/missing-logo.png" }); +} + +{ + // A reference embedded mid-path, preceded by a word/path character, must + // not "resync" and match as if it started a fresh token. + const refs = extract("xtools/outside.ts should never match as a bare reference"); + assert.equal(refs.length, 0, "mid-identifier text must not match"); +} + +{ + // Terminal boundary: a coincidental prefix of a longer filename must not + // be truncated into a match for a shorter, unrelated real asset name. + const refs = extract("See `assets/logo.png.bak` for the backup."); + assert.equal(refs.length, 0, "assets/logo.png.bak must not extract as assets/logo.png"); +} + +{ + // Same class of bug, different character: an extra word character + // directly after the extension (no separator at all) must also not + // truncate into a match. + const refs = extract("See `assets/logo.pngx` for the logo."); + assert.equal(refs.length, 0, "assets/logo.pngx must not extract as assets/logo.png"); +} + +{ + // The "/" character is itself part of the accepted path alphabet, so a + // terminal boundary that only rejects word/dot/hyphen characters still + // lets a nested path truncate into a match for its own leading segment: + // assets/logo.png/child.bin must not extract as assets/logo.png. + const refs = extract("See `assets/logo.png/child.bin` for the asset."); + assert.equal(refs.length, 0, "assets/logo.png/child.bin must not extract as assets/logo.png"); +} + +{ + // A safe leading "./" (same-directory prefix) is a common, harmless + // convention and must be captured, not silently unmatched. + const refs = extract("Run `./scripts/gen.sh` now."); + assert.equal(refs.length, 1, "./scripts/gen.sh must be extracted"); + assert.deepEqual(refs[0], { prefixed: false, pathPart: "./scripts/gen.sh", ref: "./scripts/gen.sh" }); +} + +{ + // The exact adversarial case from the review: a ${CLAUDE_PLUGIN_ROOT}/../ + // reference must keep its traversal prefix attached to the captured + // pathPart, not get reinterpreted as the bare tail `tools/outside.ts`. + // biome-ignore lint/suspicious/noTemplateCurlyInString: literal "${CLAUDE_PLUGIN_ROOT}" placeholder text, not a JS template + const refs = extract("Run ${CLAUDE_PLUGIN_ROOT}/../tools/outside.ts now."); + assert.equal(refs.length, 1, "must extract exactly one reference, not a bare-tail duplicate"); + assert.deepEqual(refs[0], { + prefixed: true, + pathPart: "../tools/outside.ts", + // biome-ignore lint/suspicious/noTemplateCurlyInString: literal "${CLAUDE_PLUGIN_ROOT}" placeholder text, not a JS template + ref: "${CLAUDE_PLUGIN_ROOT}/../tools/outside.ts", + }); +} + +// --- resolveBundleReference: pure lexical containment (no filesystem) ---- + +{ + const result = resolveBundleReference("/tmp/base", "scripts/gen.sh"); + assert.equal(result.ok, true); + assert.equal(result.resolved, path.join("/tmp/base", "scripts", "gen.sh")); +} + +{ + // A lone "." (current-dir) segment is a safe no-op and must be accepted, + // not rejected as traversal. + const result = resolveBundleReference("/tmp/base", "scripts/./gen.sh"); + assert.equal(result.ok, true, "scripts/./gen.sh must not be treated as traversal"); +} + +for (const pathPart of ["scripts/../../secret/outside.md", "../../secret/outside.md", "../tools/outside.ts"]) { + const result = resolveBundleReference("/tmp/base", pathPart); + assert.equal(result.ok, false, `${pathPart}: expected not ok`); + assert.equal(result.reason, "traversal", `${pathPart}: expected reason "traversal"`); +} + +// --- checkReferenceOnDisk: filesystem boundary (existence, regular-file, +// symlink containment) lives at the validator, not in the pure lib --------- + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "bundle-refs-")); +const base = path.join(tmp, "skills", "fixture-plugin"); +fs.mkdirSync(path.join(base, "scripts"), { recursive: true }); +fs.mkdirSync(path.join(base, "assets"), { recursive: true }); +// A real file at the shorter, truncated path a terminal-boundary bug would +// coincidentally match against (assets/logo.png.bak -> assets/logo.png). +fs.writeFileSync(path.join(base, "assets", "logo.png"), "not a real png, just fixture bytes\n"); +fs.mkdirSync(path.join(base, "tools"), { recursive: true }); +fs.writeFileSync(path.join(base, "scripts", "gen.sh"), "#!/bin/sh\necho ok\n"); +// A directory whose name matches the reference vocabulary + extension +// pattern (e.g. a stray "tool.ts" directory) must not pass as a file. +fs.mkdirSync(path.join(base, "tools", "tool.ts"), { recursive: true }); +// A real file planted at the *bare* traversal-tail location, to prove the +// adversarial same-named-file case: even though tools/outside.ts genuinely +// exists inside the bundle, a reference that actually reads +// ${CLAUDE_PLUGIN_ROOT}/../tools/outside.ts must still be rejected as +// traversal — it must never be matched against this coincidental file. +fs.writeFileSync(path.join(base, "tools", "outside.ts"), "// decoy, must never be reached via traversal\n"); +fs.mkdirSync(path.join(tmp, "secret"), { recursive: true }); +fs.writeFileSync(path.join(tmp, "secret", "real-target.md"), "should never be reachable\n"); +fs.symlinkSync(path.join(tmp, "secret", "real-target.md"), path.join(base, "scripts", "escape-link.md")); +fs.symlinkSync(path.join(base, "scripts", "gen.sh"), path.join(base, "scripts", "safe-link.sh")); +// A symlinked *parent directory* escaping the bundle, with an ordinary +// regular file sitting behind it. lstat on the final component alone can't +// see this: path resolution follows the intermediate symlink transparently, +// so the final component looks like a plain contained file unless the whole +// resolved path is realpath-checked. +fs.mkdirSync(path.join(tmp, "secret", "linked-target"), { recursive: true }); +fs.writeFileSync(path.join(tmp, "secret", "linked-target", "payload.md"), "should never be reachable\n"); +fs.symlinkSync(path.join(tmp, "secret", "linked-target"), path.join(base, "scripts", "linked-dir")); +// A dangling symlink (target does not exist). +fs.symlinkSync(path.join(base, "scripts", "does-not-exist.sh"), path.join(base, "scripts", "dangling.sh")); +// A cyclic symlink pair. +fs.symlinkSync(path.join(base, "scripts", "cycle-b"), path.join(base, "scripts", "cycle-a")); +fs.symlinkSync(path.join(base, "scripts", "cycle-a"), path.join(base, "scripts", "cycle-b")); + +{ + // missing: lexically contained, no file on disk. + const resolved = path.join(base, "assets", "missing-logo.png"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "missing"); +} + +{ + // not-a-file: a directory sitting at the exact reference path. + const resolved = path.join(base, "tools", "tool.ts"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "not-a-file"); +} + +{ + // symlink-escape: the reference's own path is inside the bundle, but it + // points at a real target outside it. + const resolved = path.join(base, "scripts", "escape-link.md"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "symlink-escape"); +} + +{ + // A symlink whose target stays inside the bundle is a legitimate file. + const resolved = path.join(base, "scripts", "safe-link.sh"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "ok"); +} + +{ + // A genuine in-bundle file. + const resolved = path.join(base, "scripts", "gen.sh"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "ok"); +} + +{ + // The exact adversarial case from the review: an escaping *parent* + // directory symlink, reached through a final component that is itself a + // plain regular file (not a symlink). + const resolved = path.join(base, "scripts", "linked-dir", "payload.md"); + assert.equal( + checkReferenceOnDisk(base, resolved).reason, + "symlink-escape", + "a file behind a symlinked parent directory must be rejected as escaping", + ); +} + +{ + // A dangling symlink must fail as a controlled validation reason, not + // throw an uncaught ENOENT out of realpathSync. + const resolved = path.join(base, "scripts", "dangling.sh"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "missing"); +} + +{ + // A cyclic symlink pair must fail as a controlled validation reason, not + // throw an uncaught ELOOP out of realpathSync. + const resolved = path.join(base, "scripts", "cycle-a"); + assert.equal(checkReferenceOnDisk(base, resolved).reason, "missing"); +} + +// --- End-to-end: extractor -> resolver -> on-disk check, chained exactly +// the way validate-plugin-ports.mjs's checkSkillReferences runs them ------- + +function validateReference(skillMdText, pluginBase) { + const [entry] = extract(skillMdText); + assert.ok(entry, "expected exactly one extracted reference"); + const lexical = resolveBundleReference(pluginBase, entry.pathPart); + if (!lexical.ok) return lexical.reason; + return checkReferenceOnDisk(pluginBase, lexical.resolved).reason; +} + +assert.equal( + validateReference("See `assets/missing-logo.png` for the logo.", base), + "missing", + "production pipeline must flag a missing PNG asset reference", +); + +assert.equal( + // biome-ignore lint/suspicious/noTemplateCurlyInString: literal "${CLAUDE_PLUGIN_ROOT}" placeholder text, not a JS template + validateReference("Run ${CLAUDE_PLUGIN_ROOT}/../tools/outside.ts now.", base), + "traversal", + "production pipeline must reject the traversal reference even though tools/outside.ts genuinely exists in the bundle", +); + +assert.equal( + validateReference("Run `./scripts/gen.sh` now.", base), + "ok", + "production pipeline must accept a safe leading ./ prefix", +); + +{ + // The exact terminal-boundary adversarial case, end to end: even with a + // real file sitting at the truncated path (assets/logo.png), the + // coincidental-prefix text must never extract a match to validate at all. + const refs = extract("See `assets/logo.png.bak` for the backup."); + assert.equal( + refs.length, + 0, + "production pipeline must never match assets/logo.png.bak against the real assets/logo.png file", + ); +} + +{ + // The exact production-path shape from the final re-review: a nested + // path whose leading segment coincides with the real assets/logo.png + // file. "/" is part of the accepted path alphabet, so a boundary that + // only rejects word/dot/hyphen characters still truncates this into a + // match for the shorter, real file — end to end, this must never + // extract a match to validate at all. + const refs = extract("See `assets/logo.png/child.bin` for the nested asset."); + assert.equal( + refs.length, + 0, + "production pipeline must never match assets/logo.png/child.bin against the real assets/logo.png file", + ); +} + +fs.rmSync(tmp, { recursive: true, force: true }); + +console.log("bundle reference resolution fixtures passed"); diff --git a/scripts/lib/bundle-refs.mjs b/scripts/lib/bundle-refs.mjs new file mode 100644 index 0000000..6c62cc7 --- /dev/null +++ b/scripts/lib/bundle-refs.mjs @@ -0,0 +1,61 @@ +import path from "node:path"; + +/** + * Extract every unambiguous local bundle reference from `text`. + * + * A reference is unambiguous only when it starts at a token boundary — not + * preceded by a word/path character, so a match can never "resync" mid-path + * and swallow part of a larger expression (e.g. `${CLAUDE_PLUGIN_ROOT}/../tools/x.ts` + * must never be reinterpreted as the bare reference `tools/x.ts`) — and ends + * at one, so a coincidental prefix of a longer path (e.g. + * `assets/logo.png.bak` or `assets/logo.png/child.bin`) can't be truncated + * into a match for a real asset (`assets/logo.png`) that happens to exist. + * The terminal boundary must reject `/` as well as word/dot/hyphen + * characters — `/` is itself part of the accepted path alphabet, so a + * shorter match immediately followed by more path is still a truncation, + * not a genuine end of token. Leading `./` and `../` segments are captured + * as part of the reference itself, not silently dropped or left unmatched, + * so both a safe same-directory prefix and real traversal are visible + * downstream instead of disappearing. + */ +export function extractBundleReferences(text, { subdirs, extensions }) { + const pattern = new RegExp( + `(? segment === ".."); +} + +/** + * Resolve a local bundle reference against `base`, purely lexically: no + * filesystem access. Rejects traversal (".." segments, or a normalized + * result outside `base`) before any I/O would happen. Callers are + * responsible for the filesystem-facing checks (existence, regular-file, + * symlink containment) once a reference resolves `ok`. + */ +export function resolveBundleReference(base, pathPart) { + if (hasTraversalSegment(pathPart)) { + return { ok: false, reason: "traversal" }; + } + const resolved = path.join(base, pathPart); + const rel = path.relative(base, resolved); + if (rel.startsWith("..") || path.isAbsolute(rel)) { + return { ok: false, reason: "traversal" }; + } + return { ok: true, resolved }; +} diff --git a/scripts/lib/frontmatter-test.mjs b/scripts/lib/frontmatter-test.mjs new file mode 100644 index 0000000..459306d --- /dev/null +++ b/scripts/lib/frontmatter-test.mjs @@ -0,0 +1,73 @@ +import assert from "node:assert/strict"; +import { isNonEmptyString, parseFrontmatter } from "./frontmatter.mjs"; + +const FILE = "fixture.md"; + +function frontmatterBlock(descriptionLine) { + return `---\nname: fixture\n${descriptionLine}\n---\n\nBody text.\n`; +} + +// Valid `description` values: parseFrontmatter must return exactly the +// expected string, and isNonEmptyString must accept it. +const VALID_DESCRIPTIONS = [ + { label: "plain scalar", line: "description: hello world", expected: "hello world" }, + { label: "double-quoted", line: 'description: "hello world"', expected: "hello world" }, + { label: "single-quoted", line: "description: 'hello world'", expected: "hello world" }, + { label: "folded block scalar", line: "description: >\n hello\n world", expected: "hello world\n" }, + { label: "literal block scalar", line: "description: |\n hello\n world", expected: "hello\nworld\n" }, +]; + +for (const { label, line, expected } of VALID_DESCRIPTIONS) { + const frontmatter = parseFrontmatter(frontmatterBlock(line), FILE); + assert.equal(frontmatter.description, expected, `${label}: unexpected parsed value`); + assert.ok(isNonEmptyString(frontmatter.description), `${label}: expected to be accepted as non-empty string`); +} + +// Invalid `description` values: parseFrontmatter still succeeds (the +// frontmatter block itself is valid YAML), but isNonEmptyString must reject +// the resulting value. These are the exact false-negative shapes the +// hand-rolled tokenizer previously accepted. +const INVALID_DESCRIPTIONS = [ + { label: "YAML null (explicit)", line: "description: null" }, + { label: "YAML null (tilde)", line: "description: ~" }, + { label: "empty sequence", line: "description: []" }, + { label: "empty mapping", line: "description: {}" }, + { label: "comment-only (parses to null)", line: "description: # just a comment" }, + { label: "empty string", line: 'description: ""' }, + { label: "whitespace-only string", line: 'description: " "' }, + { label: "field absent", line: "other: value" }, +]; + +for (const { label, line } of INVALID_DESCRIPTIONS) { + const frontmatter = parseFrontmatter(frontmatterBlock(line), FILE); + assert.ok(!isNonEmptyString(frontmatter.description), `${label}: expected to be rejected as non-empty string`); +} + +// Malformed frontmatter blocks: parseFrontmatter itself must throw. +const MALFORMED_BLOCKS = [ + { + label: "malformed YAML (unterminated quoted string)", + content: '---\nname: fixture\ndescription: "unterminated\n---\n', + }, + { label: "missing frontmatter block entirely", content: "# just a heading\n\nno frontmatter here\n" }, + { label: "unterminated frontmatter (no closing ---)", content: "---\nname: fixture\ndescription: hi\n" }, + { label: "frontmatter is a sequence, not a mapping", content: "---\n- one\n- two\n---\n" }, + // Near-miss closing delimiters: a naive `indexOf("\n---")` search treats + // any line merely *starting* with "---" as the closer. None of these are + // an exact "---" line, so none may terminate the block. + { + label: "near-miss closer: trailing text with no separator", + content: "---\nname: fixture\ndescription: hi\n---not-a-delimiter\n", + }, + { label: "near-miss closer: extra dash", content: "---\nname: fixture\ndescription: hi\n----\n" }, + { + label: "near-miss closer: trailing junk after a space", + content: "---\nname: fixture\ndescription: hi\n--- trailing junk\n", + }, +]; + +for (const { label, content } of MALFORMED_BLOCKS) { + assert.throws(() => parseFrontmatter(content, FILE), undefined, `${label}: expected parseFrontmatter to throw`); +} + +console.log("frontmatter parser fixtures passed"); diff --git a/scripts/lib/frontmatter.mjs b/scripts/lib/frontmatter.mjs new file mode 100644 index 0000000..99fec0c --- /dev/null +++ b/scripts/lib/frontmatter.mjs @@ -0,0 +1,40 @@ +import { parse as parseYaml } from "yaml"; + +/** + * Parse a leading `--- ... ---` YAML frontmatter block into a mapping. + * Throws when the block is absent, unterminated, not valid YAML, or does + * not parse to a mapping (e.g. a bare scalar or sequence document). + */ +// The closing delimiter must be an exact "---" line (optionally followed by +// trailing spaces/tabs before the newline or end of string) — not merely a +// line that starts with "---". Without this, "---not-a-delimiter", "----", +// and "--- trailing junk" all satisfy a naive `indexOf("\n---")` search and +// silently close the block early or swallow content past the real closer. +const CLOSING_DELIMITER = /^---\n([\s\S]*?)\n---[ \t]*(?:\n|$)/; + +export function parseFrontmatter(content, filePath) { + const normalized = content.replace(/\r\n/g, "\n"); + if (!normalized.startsWith("---\n")) { + throw new Error(`${filePath}: missing YAML frontmatter block`); + } + const match = CLOSING_DELIMITER.exec(normalized); + if (!match) { + throw new Error(`${filePath}: unterminated YAML frontmatter block`); + } + const raw = match[1]; + let data; + try { + data = parseYaml(raw); + } catch (err) { + throw new Error(`${filePath}: frontmatter is not valid YAML: ${err.message}`); + } + if (data === null || typeof data !== "object" || Array.isArray(data)) { + throw new Error(`${filePath}: frontmatter must be a YAML mapping`); + } + return data; +} + +/** True only for a trimmed, non-empty string value (rejects null, numbers, collections). */ +export function isNonEmptyString(value) { + return typeof value === "string" && value.trim().length > 0; +} diff --git a/scripts/validate-agent-skills.mjs b/scripts/validate-agent-skills.mjs index eaacc43..b6460a6 100644 --- a/scripts/validate-agent-skills.mjs +++ b/scripts/validate-agent-skills.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node import { readdir, readFile } from "node:fs/promises"; import { basename, join, relative } from "node:path"; +import { isNonEmptyString, parseFrontmatter } from "./lib/frontmatter.mjs"; const SKILL_NAME_RE = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; const DESCRIPTION_MAX_LENGTH = 1024; @@ -24,47 +25,6 @@ async function walk(dir) { return out; } -function parseFrontmatter(content, filePath) { - const normalized = content.replace(/\r\n/g, "\n"); - if (!normalized.startsWith("---\n")) { - throw new Error(`${filePath}: missing YAML frontmatter block`); - } - - const end = normalized.indexOf("\n---", 4); - if (end === -1) { - throw new Error(`${filePath}: unterminated YAML frontmatter block`); - } - - const lines = normalized.slice(4, end).split("\n"); - const fields = new Map(); - - for (let i = 0; i < lines.length; i += 1) { - const line = lines[i]; - if (!line.trim() || line.trimStart().startsWith("#")) continue; - if (line.startsWith(" ") || line.startsWith("\t")) continue; - - const match = line.match(/^([A-Za-z0-9_-]+):\s*(.*)$/); - if (!match) continue; - - const [, key, rawValue] = match; - const value = rawValue.trim(); - if (["|", ">", "|-", ">-"].includes(value)) { - const block = []; - for (let j = i + 1; j < lines.length; j += 1) { - const next = lines[j]; - if (next && !next.startsWith(" ") && !next.startsWith("\t")) break; - block.push(next.trim()); - i = j; - } - fields.set(key, block.filter(Boolean).join(" ").trim()); - } else { - fields.set(key, value.replace(/^['\"]|['\"]$/g, "")); - } - } - - return fields; -} - async function validateSkillsShConfig(skillNames) { const file = "skills.sh.json"; let parsed; @@ -111,19 +71,19 @@ const warnings = []; const byName = new Map(); for (const file of files) { - let fields; + let frontmatter; try { - fields = parseFrontmatter(await readFile(file, "utf8"), file); + frontmatter = parseFrontmatter(await readFile(file, "utf8"), file); } catch (error) { errors.push(error.message); continue; } - const name = fields.get("name"); - const description = fields.get("description"); + const name = frontmatter.name; + const description = frontmatter.description; const expectedName = basename(file.replace(/\/SKILL\.md$/, "")); - if (!name) errors.push(`${file}: missing required frontmatter field: name`); + if (!isNonEmptyString(name)) errors.push(`${file}: missing required frontmatter field: name`); else { if (name !== expectedName) errors.push(`${file}: name '${name}' must match parent directory '${expectedName}'`); if (!SKILL_NAME_RE.test(name)) errors.push(`${file}: name '${name}' must be lowercase kebab-case`); @@ -132,7 +92,7 @@ for (const file of files) { byName.set(name, existing); } - if (!description) errors.push(`${file}: missing required frontmatter field: description`); + if (!isNonEmptyString(description)) errors.push(`${file}: missing required frontmatter field: description`); else if (description.length > DESCRIPTION_MAX_LENGTH) { errors.push(`${file}: description is ${description.length} chars; max is ${DESCRIPTION_MAX_LENGTH}`); } diff --git a/scripts/validate-codex-plugins.mjs b/scripts/validate-codex-plugins.mjs index 601ac2e..667daa0 100644 --- a/scripts/validate-codex-plugins.mjs +++ b/scripts/validate-codex-plugins.mjs @@ -157,7 +157,11 @@ for (const plugin of plugins) { const fixtureVerifier = path.join(pluginRoot, "scripts", "verify_fixtures.py"); if (fs.existsSync(fixtureVerifier)) { - const result = spawnSync("python3", [fixtureVerifier], { cwd: repoRoot, encoding: "utf8" }); + const result = spawnSync("python3", [fixtureVerifier], { + cwd: repoRoot, + encoding: "utf8", + env: { ...process.env, PYTHONDONTWRITEBYTECODE: "1" }, + }); if (result.status !== 0) { fail(plugin, `fixture verifier failed: ${(result.stderr || result.stdout).trim()}`); } else if (result.stdout.trim()) { diff --git a/scripts/validate-package-contents.mjs b/scripts/validate-package-contents.mjs index b8d85ad..bf0e53d 100644 --- a/scripts/validate-package-contents.mjs +++ b/scripts/validate-package-contents.mjs @@ -11,7 +11,8 @@ if (result.status !== 0) { } const [pack] = JSON.parse(result.stdout); -const packedPaths = new Set(pack.files.map((file) => file.path)); +const packedPaths = pack.files.map((file) => file.path); +const packedPathSet = new Set(packedPaths); const requiredPaths = [ "skills/diataxis-docs-site/SKILL.md", "skills/diataxis-docs-site/agents/openai.yaml", @@ -20,10 +21,14 @@ const requiredPaths = [ "skills/github-wiki/SKILL.md", "skills/github-wiki/agents/openai.yaml", ]; -const missing = requiredPaths.filter((requiredPath) => !packedPaths.has(requiredPath)); +const missing = requiredPaths.filter((requiredPath) => !packedPathSet.has(requiredPath)); +const forbidden = packedPaths.filter( + (packedPath) => /(?:^|\/)__pycache__(?:\/|$)/.test(packedPath) || /\.py[cod]$/.test(packedPath), +); -if (missing.length > 0) { - console.error(`Missing required package files:\n${missing.join("\n")}`); +if (missing.length > 0 || forbidden.length > 0) { + if (missing.length > 0) console.error(`Missing required package files:\n${missing.join("\n")}`); + if (forbidden.length > 0) console.error(`Forbidden generated artifacts in package:\n${forbidden.join("\n")}`); process.exit(1); } diff --git a/scripts/validate-plugin-ports.mjs b/scripts/validate-plugin-ports.mjs index d9407e1..3466fef 100644 --- a/scripts/validate-plugin-ports.mjs +++ b/scripts/validate-plugin-ports.mjs @@ -14,6 +14,8 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { extractBundleReferences, resolveBundleReference } from "./lib/bundle-refs.mjs"; +import { isNonEmptyString, parseFrontmatter } from "./lib/frontmatter.mjs"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const pluginsDir = path.join(repoRoot, "claude-code", "plugins"); @@ -68,31 +70,6 @@ function checkReadme(plugin) { } } -/** - * Parse a leading `--- ... ---` YAML frontmatter block. - * Returns the raw frontmatter text, or null when absent. - */ -function readFrontmatter(text) { - const match = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/.exec(text); - return match ? match[1] : null; -} - -function readFrontmatterField(frontmatter, field) { - const lines = frontmatter.split(/\r?\n/); - const index = lines.findIndex((line) => line.startsWith(`${field}:`)); - if (index === -1) return null; - - const raw = lines[index].slice(field.length + 1).trim(); - if (raw !== ">" && raw !== "|") return raw.replace(/^["']|["']$/g, ""); - - const block = []; - for (const line of lines.slice(index + 1)) { - if (!/^\s+/.test(line)) break; - block.push(line.trim()); - } - return block.join(raw === ">" ? " " : "\n").trim(); -} - /** Path to a plugin's canonical SKILL.md (skills//SKILL.md). */ function skillPath(plugin) { return path.join(pluginsDir, plugin, "skills", plugin, "SKILL.md"); @@ -105,74 +82,92 @@ function checkSkillFrontmatter(plugin) { fail(plugin, `missing SKILL.md at skills/${plugin}/SKILL.md`); return; } - const frontmatter = readFrontmatter(fs.readFileSync(file, "utf8")); - if (frontmatter === null) { - fail(plugin, "SKILL.md has no YAML frontmatter"); + let frontmatter; + try { + frontmatter = parseFrontmatter(fs.readFileSync(file, "utf8"), file); + } catch (err) { + fail(plugin, err.message); return; } - const name = readFrontmatterField(frontmatter, "name"); - if (!name) { + const name = frontmatter.name; + if (!isNonEmptyString(name)) { fail(plugin, "SKILL.md frontmatter has no name field"); return; } if (name !== plugin) { fail(plugin, `SKILL.md frontmatter name "${name}" does not match plugin "${plugin}"`); } - const description = readFrontmatterField(frontmatter, "description"); - if (!description) fail(plugin, "SKILL.md frontmatter has no description field"); + if (!isNonEmptyString(frontmatter.description)) { + fail(plugin, "SKILL.md frontmatter has no description field"); + } } // --- Check 4: SKILL.md references resolve to files on disk --------------- -// A SKILL.md path reference is only validated when it is unambiguously a -// reference to *this plugin's own bundle*: -// -// - Any `${CLAUDE_PLUGIN_ROOT}/...` path — explicitly plugin-root-relative. -// - A bare `workflows/`, `references/`, or `tools/` path — these subdir -// names are skill-bundle vocabulary, effectively never used for anything -// else. -// -// Bare `scripts/`, `cli/`, `assets/`, `hooks/` paths are NOT validated unless -// `${CLAUDE_PLUGIN_ROOT}`-prefixed: those are universal repo conventions and -// appear as illustrative examples (an audit skill citing `./scripts/test.sh`) -// or cross-skill delegation (`scripts/gen.sh` belonging to another skill). -// Bare filenames (e.g. `data-layer.md`) are likewise not validated — too -// easily confused with filenames inside code examples. -const REF_EXT = "md|ts|tsx|sh|json|js|mjs|cjs|py"; +// A SKILL.md path reference is only matched when it is unambiguously a +// reference to *this plugin's own bundle*: either `${CLAUDE_PLUGIN_ROOT}/...` +// (explicitly plugin-root-relative) or a bare `workflows/`, `references/`, +// `tools/`, `scripts/`, `cli/`, `assets/`, or `hooks/` path — this fixed +// vocabulary of skill-bundle subdirectory names is what makes the match +// unambiguous. Every match is validated: it must resolve to an existing +// *regular file* whose real (symlink-resolved) path stays inside the +// plugin's own root — rejects `..` traversal, directories masquerading as +// files, and symlinks that escape the bundle. +const REF_EXT = ["md", "ts", "tsx", "sh", "json", "js", "mjs", "cjs", "py", "png", "jpg", "jpeg", "gif", "svg", "webp"]; const ALL_SUBDIRS = ["workflows", "references", "tools", "scripts", "cli", "assets", "hooks"]; -const BARE_OK_SUBDIRS = new Set(["workflows", "references", "tools"]); -const REF_PATTERN = new RegExp( - `(\\$\\{CLAUDE_PLUGIN_ROOT\\}/)?((?:skills/[a-z0-9-]+/)?(?:${ALL_SUBDIRS.join("|")})/[A-Za-z0-9._/-]+\\.(?:${REF_EXT}))`, - "g", -); -/** First bundle-subdir component of a reference path (after any skills//). */ -function refSubdir(pathPart) { - return pathPart.replace(/^skills\/[a-z0-9-]+\//, "").split("/")[0]; +/** + * Filesystem-facing half of reference validation, applied only after + * `resolveBundleReference` has already confirmed `resolved` is lexically + * inside `base`. `realpathSync` is required unconditionally (not just when + * the final path component is itself a symlink) because a symlinked + * *parent* directory resolves away silently otherwise: `lstatSync` on the + * final component follows all intermediate symlinks as a normal part of + * path resolution, so a plain file sitting behind a symlinked directory + * looks like an ordinary contained file unless the whole path is + * realpath-resolved and re-checked for containment. A dangling or cyclic + * symlink makes `realpathSync` throw (ENOENT / ELOOP); that is a controlled + * "missing" result here, not an uncaught crash. + */ +export function checkReferenceOnDisk(base, resolved) { + let real; + try { + real = fs.realpathSync(resolved); + } catch { + return { reason: "missing" }; + } + const realBase = fs.realpathSync(base); + const rel = path.relative(realBase, real); + if (rel.startsWith("..") || path.isAbsolute(rel)) { + return { reason: "symlink-escape" }; + } + if (!fs.statSync(real).isFile()) { + return { reason: "not-a-file" }; + } + return { reason: "ok" }; } -/** Resolve a SKILL.md reference to an absolute path in the plugin port. */ -function resolveRef(plugin, prefixed, pathPart) { - // `${CLAUDE_PLUGIN_ROOT}/...` is plugin-root-relative; a bare reference is - // relative to the skill directory (skills//). - const base = prefixed ? path.join(pluginsDir, plugin) : path.join(pluginsDir, plugin, "skills", plugin); - return path.join(base, pathPart); -} +const ON_DISK_MESSAGES = { + missing: (ref) => `SKILL.md references missing file: ${ref}`, + "symlink-escape": (ref) => `SKILL.md reference resolves outside the plugin bundle via symlink: ${ref}`, + "not-a-file": (ref) => `SKILL.md reference is not a regular file: ${ref}`, +}; function checkSkillReferences(plugin) { const file = skillPath(plugin); if (!fs.existsSync(file)) return; // already reported by checkSkillFrontmatter const text = fs.readFileSync(file, "utf8"); - const seen = new Set(); - for (const [, prefix, pathPart] of text.matchAll(REF_PATTERN)) { - const prefixed = Boolean(prefix); - if (!prefixed && !BARE_OK_SUBDIRS.has(refSubdir(pathPart))) continue; - // biome-ignore lint/suspicious/noTemplateCurlyInString: literal "${CLAUDE_PLUGIN_ROOT}" placeholder text, not a JS template - const ref = (prefixed ? "${CLAUDE_PLUGIN_ROOT}/" : "") + pathPart; - if (seen.has(ref)) continue; - seen.add(ref); - if (!fs.existsSync(resolveRef(plugin, prefixed, pathPart))) { - fail(plugin, `SKILL.md references missing file: ${ref}`); + const refs = extractBundleReferences(text, { subdirs: ALL_SUBDIRS, extensions: REF_EXT }); + for (const { prefixed, pathPart, ref } of refs) { + // `${CLAUDE_PLUGIN_ROOT}/...` is plugin-root-relative; a bare reference + // is relative to the skill directory (skills//). + const base = prefixed ? path.join(pluginsDir, plugin) : path.join(pluginsDir, plugin, "skills", plugin); + const lexical = resolveBundleReference(base, pathPart); + if (!lexical.ok) { + fail(plugin, `SKILL.md reference escapes the plugin bundle: ${ref}`); + continue; } + const { reason } = checkReferenceOnDisk(base, lexical.resolved); + if (reason !== "ok") fail(plugin, ON_DISK_MESSAGES[reason](ref)); } } @@ -252,26 +247,51 @@ function checkJunk() { } // --- Run ----------------------------------------------------------------- -const plugins = listPlugins(); -const manifestDescriptions = new Map(); -for (const plugin of plugins) { - const description = checkPluginManifest(plugin); - if (description) manifestDescriptions.set(plugin, description); - checkReadme(plugin); - checkSkillFrontmatter(plugin); - checkSkillReferences(plugin); +// Guarded so a test can `import { checkReferenceOnDisk } from "./validate-plugin-ports.mjs"` +// without triggering a full validation run against the real repository. +function main() { + const plugins = listPlugins(); + const manifestDescriptions = new Map(); + for (const plugin of plugins) { + const description = checkPluginManifest(plugin); + if (description) manifestDescriptions.set(plugin, description); + checkReadme(plugin); + checkSkillFrontmatter(plugin); + checkSkillReferences(plugin); + } + checkMarketplace(plugins, manifestDescriptions); + checkCatalog(plugins); + checkJunk(); + + if (failures.length > 0) { + console.error(`\n✗ plugin-port validation failed (${failures.length} issue(s)):\n`); + for (const { plugin, message } of failures) { + console.error(` [${plugin}] ${message}`); + } + console.error(""); + process.exit(1); + } + + console.log(`✓ plugin-port validation passed (${plugins.length} plugins)`); } -checkMarketplace(plugins, manifestDescriptions); -checkCatalog(plugins); -checkJunk(); -if (failures.length > 0) { - console.error(`\n✗ plugin-port validation failed (${failures.length} issue(s)):\n`); - for (const { plugin, message } of failures) { - console.error(` [${plugin}] ${message}`); +// A plain `import.meta.url === \`file://${process.argv[1]}\`` string +// comparison is not URL-safe: import.meta.url percent-encodes characters +// like spaces, while the raw argv path does not, so a checkout path +// containing a space silently fails the check and main() never runs. +// Comparing filesystem paths instead of URL strings sidesteps encoding +// entirely: fileURLToPath decodes import.meta.url back to a native path +// (already realpath-resolved by Node for a directly-executed script — e.g. +// through macOS's /tmp -> /private/tmp symlink), and realpathSync resolves +// the same way on the argv side so both sides compare on equal footing +// regardless of spaces, separators, or symlinked ancestor directories. +function isDirectExecution() { + if (!process.argv[1]) return false; + try { + return fileURLToPath(import.meta.url) === fs.realpathSync(process.argv[1]); + } catch { + return false; } - console.error(""); - process.exit(1); } -console.log(`✓ plugin-port validation passed (${plugins.length} plugins)`); +if (isDirectExecution()) main();