diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cd2c5f..74b59a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) - `skillforge inspect ` now accepts a `.skill` archive in addition to a directory or a `SKILL.md` file — same recognition rule as `cat` and `install`. `.skill` / `.zip` extension is treated as a zip; `--from-bundle` forces archive interpretation against a renamed bundle (with a first-four-byte zip-magic sniff so a plain text file gets a clear refusal rather than a cryptic JSZip error). Archive mode opens the zip in memory, materializes `SKILL.md` to a one-off OS-tempdir file just long enough to drive `validateSkill` / `lintSkill` against it, then tears the tempdir down — no archive entries are extracted to a user-visible location. The result grows a `source` discriminator (`"archive" | "directory" | "file"`) and the human-readable header prints `Archive: …` / `Directory: …` / `File: …` as its first line so the reader can tell which mode `inspect` is in at a glance. Directory-only `attachedFiles` inventory unchanged — a bundle is opaque; use `tree` or `install --dry-run` to peek at its file list. Loader logic carved into a new internal `src/skill-loader.ts` (~30 LOC of essence) so `inspect` and `cat` agree on what counts as a `.skill`. No changes to `validate.ts` / `lint.ts` / `pack.ts` / `install.ts` / `cat.ts`. ### Added +- `skillforge audit [dir]`: fleet security + quality scan across every skill directory at the top level of `--from ` (default `~/.claude/skills/`). Six built-in rules — `security/embedded-binary` (error: any non-text file > 1 MB, refuse to ship binaries), `security/shell-shebang` (warning: `#!/bin/sh|bash|zsh` at top of any non-SKILL.md file), `security/exec-bit` (warning: any file with the exec bit on), `quality/todo-marker` (info: SKILL.md body still says TODO/FIXME), `quality/missing-examples` (info: no `## Examples` section), `quality/vague-description` (warning: frontmatter `description` < 40 chars or still says TODO). `--severity error|warning|info` filters output; `--json` emits the full structured report. Exit 0 if zero errors, 1 if any error. Composition over refactor — doesn't import internals of `lint`/`validate`/`pack`/`install`/`cat`/`inspect`. - `skillforge cat `: print the bundled `SKILL.md` of a `.skill` archive (or a `SKILL.md` file, or a directory containing one) to stdout — the `tar -xOf` analogue for skill bundles. Opens the archive in memory via `jszip`, locates `SKILL.md` at the root, and emits its bytes without extracting any other files. `--section ` (default `all`) slices the output to just the YAML (no `---` fences) or just the markdown body. `--json` returns `{ name, version, frontmatter, body }` for CI / scripted reviewers. Always validates frontmatter against `SkillFrontmatterSchema` before emitting — a broken `.skill` is refused with a clear list of issues rather than spilling garbage. Diagnostic noise stays on stderr so the output pipes cleanly. Side-effect free — `cat` never writes to disk. - `skillforge tree `: pre-pack file inventory preview. Walks the directory using the *same* exclusion rule as `pack` (`shouldExcludeEntry` reused from `pack.ts` — no duplicated walker, no chance of drift) and returns every non-excluded entry with relative POSIX path, byte size, and `isDir` flag. Optional `--sort path` (default, alphabetised tree order) or `--sort size` (files-only, descending — useful for spotting heavy fixtures). Human-readable mode prints a box-drawing tree (`├──` / `└──`, 2-space indent per level) with a right-aligned size column (bytes under 1 KB, KB with one decimal otherwise) and a `N files · X.Y KB` summary line. `--json` emits the full `TreeResult` for CI. Side-effect free — `tree` never writes. - `skillforge diff `: structural comparison of two SKILL.md files. Reports per-field frontmatter changes (added / removed / changed with before/after), `##` and `###` heading-level changes (added / removed / reordered with `from`/`to` positions, computed over the intersection so a single insert doesn't cascade as N moves), and a coarse body line-count delta. Refuses to diff either file if its frontmatter fails schema validation — fix the file first. Human-readable mode prints a tidy multi-section report with kleur colour (green adds, red removes, yellow changes); `--json` emits the full `DiffResult` for CI. Exits 0 when structurally identical, 1 when they differ (matches `mcp-devtools diff`), 2 on validation/IO error. Side-effect free. diff --git a/src/audit.test.ts b/src/audit.test.ts new file mode 100644 index 0000000..ae1734c --- /dev/null +++ b/src/audit.test.ts @@ -0,0 +1,138 @@ +import { realpathSync } from "node:fs"; +import { chmod, mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { beforeEach, describe, expect, it } from "vitest"; +import { auditSkills } from "./audit.js"; + +let workDir: string; + +beforeEach(async () => { + workDir = realpathSync(await mkdtemp(join(tmpdir(), "skillforge-audit-"))); +}); + +async function writeSkillMd( + dir: string, + opts: { name: string; description?: string; body?: string }, +) { + await mkdir(dir, { recursive: true }); + const description = + opts.description ?? + "A long-enough description so the schema validator is happy with this for tests."; + const body = + opts.body ?? + `# ${opts.name}\n\nReal body content for the test.\nLine two.\nLine three.\nLine four.\nLine five.\n\n## Examples\n\nExample usage.\n`; + await writeFile( + join(dir, "SKILL.md"), + `---\nname: ${opts.name}\ndescription: ${description}\nversion: 0.0.1\ntags: []\n---\n\n${body}`, + ); +} + +describe("auditSkills", () => { + it("returns zero findings for an empty directory", async () => { + const r = await auditSkills({ fromDir: workDir }); + expect(r.skillCount).toBe(0); + expect(r.findings).toEqual([]); + expect(r.summary).toEqual({ error: 0, warning: 0, info: 0 }); + }); + + it("returns zero findings for a clean skill", async () => { + await writeSkillMd(join(workDir, "clean"), { name: "clean" }); + const r = await auditSkills({ fromDir: workDir }); + expect(r.skillCount).toBe(1); + expect(r.findings).toEqual([]); + }); + + it("flags an embedded binary > 1MB as security/embedded-binary error", async () => { + const d = join(workDir, "binary"); + await writeSkillMd(d, { name: "binary" }); + await writeFile(join(d, "model.bin"), Buffer.alloc(2 * 1024 * 1024, 0)); + const r = await auditSkills({ fromDir: workDir }); + const f = r.findings.find((x) => x.ruleId === "security/embedded-binary"); + expect(f).toBeDefined(); + expect(f?.severity).toBe("error"); + expect(f?.filePath).toBe("model.bin"); + }); + + it("flags a shell-shebang script as security/shell-shebang warning", async () => { + const d = join(workDir, "shellish"); + await writeSkillMd(d, { name: "shellish" }); + await writeFile(join(d, "run.sh"), "#!/bin/bash\necho hi\n"); + const r = await auditSkills({ fromDir: workDir }); + const f = r.findings.find((x) => x.ruleId === "security/shell-shebang"); + expect(f).toBeDefined(); + expect(f?.severity).toBe("warning"); + expect(f?.filePath).toBe("run.sh"); + }); + + it.skipIf(process.platform === "win32")( + "flags an exec-bit file as security/exec-bit warning (POSIX-only)", + async () => { + const d = join(workDir, "execbit"); + await writeSkillMd(d, { name: "execbit" }); + const fp = join(d, "tool"); + await writeFile(fp, "tool body\n"); + await chmod(fp, 0o755); + const r = await auditSkills({ fromDir: workDir }); + const f = r.findings.find((x) => x.ruleId === "security/exec-bit"); + expect(f).toBeDefined(); + expect(f?.severity).toBe("warning"); + }, + ); + + it("flags a TODO marker as quality/todo-marker info", async () => { + await writeSkillMd(join(workDir, "todo"), { + name: "todo", + body: "# todo\n\nReal body content with a TODO that should surface.\nLine two.\nLine three.\nLine four.\nLine five.\n\n## Examples\n\nexample\n", + }); + const r = await auditSkills({ fromDir: workDir }); + const f = r.findings.find((x) => x.ruleId === "quality/todo-marker"); + expect(f).toBeDefined(); + expect(f?.severity).toBe("info"); + }); + + it("flags missing Examples heading as quality/missing-examples info", async () => { + await writeSkillMd(join(workDir, "noexamples"), { + name: "noexamples", + body: "# noexamples\n\nReal body content without an examples header.\nLine two.\nLine three.\nLine four.\nLine five.\n", + }); + const r = await auditSkills({ fromDir: workDir }); + expect(r.findings.find((x) => x.ruleId === "quality/missing-examples")).toBeDefined(); + }); + + it("severityFilter=error filters out info + warning", async () => { + await writeSkillMd(join(workDir, "todo"), { + name: "todo", + body: "# todo\n\nReal body content with a TODO marker.\nLine two.\nLine three.\nLine four.\nLine five.\n\n## Examples\n\nexample\n", + }); + const r = await auditSkills({ fromDir: workDir, severityFilter: "error" }); + expect(r.findings.every((f) => f.severity === "error")).toBe(true); + }); + + it("aggregates findings per skill across a multi-skill directory", async () => { + await writeSkillMd(join(workDir, "a"), { name: "a" }); + await writeSkillMd(join(workDir, "b"), { + name: "b", + body: "# b\n\nTODO body shorter line counts present here for the test.\nLine two.\nLine three.\nLine four.\nLine five.\n", + }); + const r = await auditSkills({ fromDir: workDir }); + expect(r.skillCount).toBe(2); + const bFindings = r.findings.filter((f) => f.skillName === "b"); + expect(bFindings.length).toBeGreaterThan(0); + }); + + it("rejects a non-directory fromDir with a clear error", async () => { + const fp = join(workDir, "notadir"); + await writeFile(fp, "x"); + await expect(auditSkills({ fromDir: fp })).rejects.toThrow(/not a directory/); + }); + + it("returns AuditReport shape suitable for --json output", async () => { + await writeSkillMd(join(workDir, "x"), { name: "x" }); + const r = await auditSkills({ fromDir: workDir }); + expect(r).toHaveProperty("fromDir"); + expect(r).toHaveProperty("scanned"); + expect(r).toHaveProperty("findings"); + expect(r).toHaveProperty("summary"); + }); +}); diff --git a/src/audit.ts b/src/audit.ts new file mode 100644 index 0000000..e893278 --- /dev/null +++ b/src/audit.ts @@ -0,0 +1,204 @@ +/** + * `skillforge audit ` — fleet security + quality scan over every skill + * directory at the top level of `fromDir`. Each skill directory is walked + * once; built-in rules emit findings keyed by ``. The report is consumed by the CLI for the human-readable + * table and by callers that want structured findings via `--json`. + * + * Composition, not refactor — re-uses `lint`/`validate` only by way of file + * conventions, never by importing their internals. + */ +import { readFile, readdir, stat } from "node:fs/promises"; +import { basename, join, relative, resolve } from "node:path"; +import matter from "gray-matter"; + +export type AuditSeverity = "error" | "warning" | "info"; + +export interface AuditFinding { + skillName: string; + ruleId: string; + severity: AuditSeverity; + message: string; + filePath?: string; +} + +export interface AuditOptions { + fromDir: string; + severityFilter?: AuditSeverity; +} + +export interface AuditReport { + fromDir: string; + skillCount: number; + scanned: string[]; + findings: AuditFinding[]; + summary: { error: number; warning: number; info: number }; +} + +const ONE_MB = 1024 * 1024; + +const TEXT_EXTS = new Set([".md", ".markdown", ".yaml", ".yml", ".json", ".txt"]); + +export async function auditSkills(opts: AuditOptions): Promise { + const fromDir = resolve(opts.fromDir); + const st = await stat(fromDir).catch(() => null); + if (!st || !st.isDirectory()) { + throw new Error(`audit: ${opts.fromDir} is not a directory`); + } + + const entries = await readdir(fromDir, { withFileTypes: true }); + const scanned: string[] = []; + const findings: AuditFinding[] = []; + + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const skillDir = join(fromDir, entry.name); + const skillName = await readSkillName(skillDir); + if (!skillName) continue; // not a skill — silently skip + scanned.push(skillName); + await walkAndAudit(skillDir, skillName, findings); + } + + const filtered = opts.severityFilter + ? findings.filter((f) => f.severity === opts.severityFilter) + : findings; + + const summary = { + error: filtered.filter((f) => f.severity === "error").length, + warning: filtered.filter((f) => f.severity === "warning").length, + info: filtered.filter((f) => f.severity === "info").length, + }; + + return { + fromDir, + skillCount: scanned.length, + scanned, + findings: filtered, + summary, + }; +} + +async function readSkillName(skillDir: string): Promise { + const skillMd = join(skillDir, "SKILL.md"); + const exists = await stat(skillMd).catch(() => null); + if (!exists) return null; + try { + const raw = await readFile(skillMd, "utf8"); + const parsed = matter(raw); + const name = (parsed.data as { name?: unknown }).name; + if (typeof name === "string" && name.length > 0) return name; + } catch { + // fall through + } + return basename(skillDir); +} + +async function walkAndAudit( + skillDir: string, + skillName: string, + findings: AuditFinding[], +): Promise { + // Pre-audit the SKILL.md frontmatter + body for quality rules + const skillMd = join(skillDir, "SKILL.md"); + try { + const raw = await readFile(skillMd, "utf8"); + const parsed = matter(raw); + const desc = (parsed.data as { description?: unknown }).description; + if (typeof desc !== "string" || desc.length < 40 || /TODO/i.test(desc)) { + findings.push({ + skillName, + ruleId: "quality/vague-description", + severity: "warning", + message: "frontmatter `description` is too short or still says TODO", + filePath: "SKILL.md", + }); + } + if (!/^##\s+Examples/im.test(parsed.content)) { + findings.push({ + skillName, + ruleId: "quality/missing-examples", + severity: "info", + message: "SKILL.md has no `## Examples` section", + filePath: "SKILL.md", + }); + } + if (/\b(TODO|FIXME)\b/.test(parsed.content)) { + findings.push({ + skillName, + ruleId: "quality/todo-marker", + severity: "info", + message: "SKILL.md body contains a TODO/FIXME marker", + filePath: "SKILL.md", + }); + } + } catch { + // SKILL.md unreadable — already gated by readSkillName, no-op + } + + await walkFiles(skillDir, skillDir, skillName, findings); +} + +async function walkFiles( + rootDir: string, + currentDir: string, + skillName: string, + findings: AuditFinding[], +): Promise { + const entries = await readdir(currentDir, { withFileTypes: true }); + for (const entry of entries) { + if (entry.name === ".git" || entry.name === "node_modules") continue; + const abs = join(currentDir, entry.name); + if (entry.isDirectory()) { + await walkFiles(rootDir, abs, skillName, findings); + continue; + } + if (!entry.isFile()) continue; + const rel = relative(rootDir, abs); + const st = await stat(abs); + const lowerName = entry.name.toLowerCase(); + const ext = lowerName.includes(".") ? `.${lowerName.split(".").pop() ?? ""}` : ""; + + // security/embedded-binary — > 1MB and not a known text extension + if (st.size > ONE_MB && !TEXT_EXTS.has(ext)) { + findings.push({ + skillName, + ruleId: "security/embedded-binary", + severity: "error", + message: `embedded ${(st.size / ONE_MB).toFixed(1)} MB file at ${rel} — refuse to ship binaries`, + filePath: rel, + }); + } + + // security/exec-bit — file with any exec permission bit set. + // Windows NTFS doesn't expose Unix exec bits via stat.mode, so this rule + // is POSIX-only. On Windows process.platform === "win32" → skip. + if (process.platform !== "win32" && (st.mode & 0o111) !== 0) { + findings.push({ + skillName, + ruleId: "security/exec-bit", + severity: "warning", + message: `file ${rel} has the exec bit set — review before shipping`, + filePath: rel, + }); + } + + // security/shell-shebang — content starts with #!/bin/sh or #!/bin/bash + if (entry.name === "SKILL.md") continue; + try { + const head = await readFile(abs, { encoding: "utf8" }).catch(() => ""); + const firstLine = head.split(/\r?\n/, 1)[0] ?? ""; + // matches any of: `#!/bin/sh`, `#!/usr/bin/bash`, `#!/usr/bin/env zsh`, … + if (/^#!\s*(\/\S*\/)?(env\s+)?(sh|bash|zsh)\b/.test(firstLine)) { + findings.push({ + skillName, + ruleId: "security/shell-shebang", + severity: "warning", + message: `shell shebang at top of ${rel} — review before shipping`, + filePath: rel, + }); + } + } catch { + // unreadable, skip + } + } +} diff --git a/src/cli.ts b/src/cli.ts index 3bd2df2..5c892b5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -15,9 +15,13 @@ * cat print the bundled SKILL.md of a .skill archive * ls list installed skills in ~/.claude/skills/ * uninstall remove an installed skill from ~/.claude/skills/ + * audit fleet security + quality scan across all skills in a dir */ +import { homedir } from "node:os"; +import { join } from "node:path"; import { cac } from "cac"; import kleur from "kleur"; +import { type AuditReport, type AuditSeverity, auditSkills } from "./audit.js"; import { type CatSection, catSkill } from "./cat.js"; import { type DiffResult, diffSkills } from "./diff.js"; import { formatSkill } from "./format.js"; @@ -762,6 +766,63 @@ async function readYesNo(prompt: string): Promise { }); } +cli + .command("audit [dir]", "Fleet security + quality scan across all skills in a directory") + .option("--from ", "Override the default ~/.claude/skills/ scan root") + .option("--severity ", "Only show findings at this severity: error | warning | info") + .option("--json", "Emit a single JSON envelope to stdout (no colors, no table)") + .action(async (dir: string | undefined, opts) => { + const fromDir = opts.from ?? dir ?? join(homedir(), ".claude", "skills"); + try { + const r = await auditSkills({ + fromDir, + severityFilter: opts.severity as AuditSeverity | undefined, + }); + if (opts.json) { + process.stdout.write(`${JSON.stringify(r, null, 2)}\n`); + } else { + process.stdout.write(`${formatAuditReport(r)}\n`); + } + process.exit(r.summary.error > 0 ? 1 : 0); + } catch (err) { + process.stderr.write(`${kleur.red("error:")} ${(err as Error).message}\n`); + process.exit(2); + } + }); + +function formatAuditReport(r: AuditReport): string { + const lines: string[] = []; + lines.push(kleur.dim(`Scanning ${r.fromDir}`)); + lines.push(`${r.skillCount} skill${r.skillCount === 1 ? "" : "s"} scanned`); + lines.push(""); + if (r.findings.length === 0) { + lines.push(kleur.green("No findings.")); + } else { + for (const name of r.scanned) { + const skillFindings = r.findings.filter((f) => f.skillName === name); + if (skillFindings.length === 0) continue; + lines.push(kleur.bold(name)); + for (const f of skillFindings) { + const sev = + f.severity === "error" + ? kleur.red("ERROR") + : f.severity === "warning" + ? kleur.yellow("WARN") + : kleur.cyan("INFO"); + const where = f.filePath ? kleur.dim(` (${f.filePath})`) : ""; + lines.push(` ${sev} ${f.ruleId} ${f.message}${where}`); + } + } + } + lines.push(""); + lines.push( + `Summary: ${kleur.red(`${r.summary.error} error`)} · ` + + `${kleur.yellow(`${r.summary.warning} warning`)} · ` + + `${kleur.cyan(`${r.summary.info} info`)}`, + ); + return lines.join("\n"); +} + cli.help(); cli.version(VERSION); cli.parse();