From eb3c5902eb742322b437d33307590bdc852d4668 Mon Sep 17 00:00:00 2001 From: alexverify Date: Tue, 8 Sep 2026 01:52:27 +0200 Subject: [PATCH] feat(skills): add verify for revocations and eyebrow drift checks (#2215) * feat(skills): add verify for revocations and eyebrow drift checks * apply code reviews fixes * fix(skills): read execute bits through the fs seam, skip symlink tests where unsupported * fix(skills): check X_OK through the fs seam, report symlink tests as skipped --- docs/skills.md | 19 + src/cli/handlers/skills.ts | 1 + src/cli/handlers/skillsCli.ts | 44 ++- src/cli/handlers/skillsInstall.ts | 48 +-- src/cli/handlers/skillsVerify.test.ts | 496 ++++++++++++++++++++++++++ src/cli/handlers/skillsVerify.ts | 351 ++++++++++++++++++ src/main.tsx | 7 + src/utils/fsOperations.ts | 6 + 8 files changed, 950 insertions(+), 22 deletions(-) create mode 100644 src/cli/handlers/skillsVerify.test.ts create mode 100644 src/cli/handlers/skillsVerify.ts diff --git a/docs/skills.md b/docs/skills.md index 68f397f81..916d45396 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -10,6 +10,7 @@ openclaude skills show Show details for an installed openclaude skills validate Validate a local skill directory openclaude skills install [options] Install a skill openclaude skills remove [--global] Remove an installed skill +openclaude skills verify [options] Check installed skills for revocations and, with eyebrow, drift ``` ## Install from the registry @@ -69,6 +70,22 @@ $ openclaude skills list ci-fix enabled Diagnoses and fixes CI pipeline failures. ``` +## Verify installed skills + +`verify` reads the skills installed in the project's `.openclaude/skills` and in your user skills directory. Skills that `list` shows from plugins, MCP servers, or `--add-dir` directories are outside it. For each skill it reads the id, version, and digest from `skill.json` as written on disk, and matches them against the registry's `revocations.json` with the rule the installer uses. The digest is the registry pin recorded at install, not a hash of the current files, so `verify` says nothing about the contents on disk and trusts `skill.json` as it finds it. A skill whose `skill.json` has no id and no digest, such as one written by hand, is listed as skipped. A local install that copied a `skill.json` keeps that file's id and digest, so `verify` matches it like a registry install. The list comes from the default registry, from `--registry`, or from the environment variables named above. A list that cannot be read fails the command, the same as an install does. + +```text +$ openclaude skills verify +Found 2 installed skills. Revocation list: https://raw.githubusercontent.com/Gitlawb/openclaude-skills/main/revocations.json + ci-fix ok + hand-made skipped (no registry metadata) +eyebrow not found; install it to check skill contents against a lockfile (see docs/skills.md). +``` + +When an executable named `eyebrow` is on `PATH`, or `OPENCLAUDE_EYEBROW_BIN` names one, and the project has a lockfile, `verify` runs `eyebrow verify --path . --lockfile eyebrowlock.json` in the project directory, shows its output, and prints the path of the binary it ran. `--lockfile` names another lockfile and `--policy` adds `--ci` and the policy file to the eyebrow run. `verify` does not check the eyebrow version. An eyebrow older than 0.4.4 finds no OpenClaude skills and reports every locked skill as removed, so use 0.4.6 or newer, as the next section explains. That section also shows how to create the lockfile and how to read the verdict. + +Exit codes: `1` when a skill is revoked, when the revocation list cannot be read, or when eyebrow cannot be started; otherwise the eyebrow exit code; `0` when both checks pass, and also when eyebrow is absent or the lockfile is missing, in which case `verify` prints what to install or run. + ## Check installed skills after install [eyebrow](https://github.com/alexverify/eyebrow) is a separate, MIT-licensed single binary that records a hash of every skill, MCP server, hook, and rule it finds across coding tools in a lockfile you commit, and reports what changed since. Discovery of OpenClaude skills in the project's `.openclaude/skills` and in `~/.openclaude/skills` shipped in eyebrow 0.4.4, and 0.4.5 added the egress fingerprint used below; see the [changelog](https://github.com/alexverify/eyebrow/blob/main/CHANGELOG.md). Use 0.4.6 or newer: in 0.4.5 a skill folder that is a symlink appeared in the inventory with no hashed files, so a clean `verify` said nothing about the contents behind the link. 0.4.6 hashes the linked contents. @@ -86,6 +103,8 @@ Check them again at any time, or in CI: eyebrow verify --path . --lockfile eyebrowlock.json --ci ``` +`openclaude skills verify` runs this command for you when eyebrow is installed. + `verify` exit codes: `0` clean under the active policy, which includes content changes the policy permits; `1` rejected drift or a policy violation; `2` usage error, such as a bad flag; `3` internal or I/O error, such as a missing lockfile. In CI, treat `1` as a review task and `2` or `3` as a broken job. On the edited skill above, with no policy file: ```text diff --git a/src/cli/handlers/skills.ts b/src/cli/handlers/skills.ts index 103bc67cc..95442a0fe 100644 --- a/src/cli/handlers/skills.ts +++ b/src/cli/handlers/skills.ts @@ -30,6 +30,7 @@ import { import { validateSkillPath } from './skillsValidation.js' export { skillsInstallHandler } from './skillsInstall.js' +export { skillsVerifyHandler } from './skillsVerify.js' type SkillCommand = SkillListCommand type ListOptions = { json?: boolean } diff --git a/src/cli/handlers/skillsCli.ts b/src/cli/handlers/skillsCli.ts index 74b32059f..54dd99eba 100644 --- a/src/cli/handlers/skillsCli.ts +++ b/src/cli/handlers/skillsCli.ts @@ -6,6 +6,8 @@ type SkillsCliOptions = { global?: boolean help?: boolean json?: boolean + lockfile?: string + policy?: string registry?: string sha256?: string } @@ -85,7 +87,13 @@ Commands: show Show details for an installed skill validate Validate a local skill directory install [options] Install a skill (--sha256 required for HTTP(S) URLs) - remove [--global] Remove an installed skill` + remove [--global] Remove an installed skill + verify [options] Check installed skills for revocations and, with eyebrow, drift + +Verify options: + --registry Registry whose revocations.json to read + --lockfile eyebrow lockfile (default: eyebrowlock.json) + --policy eyebrow policy file; adds --ci to the eyebrow run` function parseSkillsCliArgs(args: string[]): { options: SkillsCliOptions @@ -119,6 +127,20 @@ function parseSkillsCliArgs(args: string[]): { } options.sha256 = value index += 1 + } else if (arg === '--lockfile') { + const value = args[index + 1] + if (!value || value.startsWith('--')) { + return { options, positionals, error: '--lockfile requires a value.' } + } + options.lockfile = value + index += 1 + } else if (arg === '--policy') { + const value = args[index + 1] + if (!value || value.startsWith('--')) { + return { options, positionals, error: '--policy requires a value.' } + } + options.policy = value + index += 1 } else if (arg?.startsWith('--registry=')) { const value = arg.slice('--registry='.length) if (!value) { @@ -131,6 +153,18 @@ function parseSkillsCliArgs(args: string[]): { return { options, positionals, error: '--sha256 requires a value.' } } options.sha256 = value + } else if (arg?.startsWith('--lockfile=')) { + const value = arg.slice('--lockfile='.length) + if (!value) { + return { options, positionals, error: '--lockfile requires a value.' } + } + options.lockfile = value + } else if (arg?.startsWith('--policy=')) { + const value = arg.slice('--policy='.length) + if (!value) { + return { options, positionals, error: '--policy requires a value.' } + } + options.policy = value } else if (TRAILING_GLOBAL_BOOLEAN_FLAGS.has(arg)) { continue } else if (TRAILING_GLOBAL_VALUE_FLAGS.has(arg)) { @@ -217,6 +251,7 @@ export async function runSkillsCli(args: string[]): Promise { skillsRemoveHandler, skillsShowHandler, skillsValidateHandler, + skillsVerifyHandler, } = await import('./skills.js') await runSkillsCliAction(async () => { @@ -260,6 +295,13 @@ export async function runSkillsCli(args: string[]): Promise { await skillsRemoveHandler(name, { global: options.global }) break } + case 'verify': + await skillsVerifyHandler({ + registry: options.registry, + lockfile: options.lockfile, + policy: options.policy, + }) + break default: console.error(`Unknown skills command: ${subcommand}`) process.exit(1) diff --git a/src/cli/handlers/skillsInstall.ts b/src/cli/handlers/skillsInstall.ts index 87a7327ed..f5cbbce89 100644 --- a/src/cli/handlers/skillsInstall.ts +++ b/src/cli/handlers/skillsInstall.ts @@ -45,7 +45,7 @@ type RegistryEntriesResult = { registrySource: string } -type SkillRevocation = { +export type SkillRevocation = { id?: unknown version?: unknown sha256?: unknown @@ -167,20 +167,30 @@ async function readSourceText(source: string): Promise { return getFsImplementation().readFile(resolve(source), { encoding: 'utf8' }) } -async function readRegistryEntries(source: string): Promise { - let registrySource = source - if (!isUrl(source)) { - const resolved = resolve(source) - try { - const sourceStats = await getFsImplementation().stat(resolved) - registrySource = sourceStats.isDirectory() - ? join(resolved, 'registry.json') - : resolved - } catch { - registrySource = resolved - } +/** + * The registry an install or verify reads: the given value, then the + * OPENCLAUDE_SKILLS_REGISTRY_URL env var, then the default registry. A + * local directory resolves to the registry.json inside it. + */ +export async function resolveRegistrySource(registry?: string): Promise { + const source = + registry ?? + process.env.OPENCLAUDE_SKILLS_REGISTRY_URL ?? + DEFAULT_SKILLS_REGISTRY_URL + if (isUrl(source)) { + return source } + const resolved = resolve(source) + try { + const sourceStats = await getFsImplementation().stat(resolved) + return sourceStats.isDirectory() ? join(resolved, 'registry.json') : resolved + } catch { + return resolved + } +} +async function readRegistryEntries(source?: string): Promise { + const registrySource = await resolveRegistrySource(source) const raw = await readSourceText(registrySource) const parsed = JSON.parse(raw) as unknown return { @@ -209,7 +219,7 @@ function resolveRegistryEntrySource( * OPENCLAUDE_SKILLS_REVOCATIONS_URL env var when set, otherwise a * revocations.json sibling of the registry (URL or local file). */ -function resolveRevocationsSource(registrySource: string): string { +export function resolveRevocationsSource(registrySource: string): string { return ( process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL ?? resolveRegistryEntrySource('revocations.json', registrySource) @@ -279,7 +289,7 @@ function parseRevocation( * list revokes nothing; any other read, parse, or schema failure throws * so the install fails closed. */ -async function readRevocations(registrySource: string): Promise { +export async function readRevocations(registrySource: string): Promise { const source = resolveRevocationsSource(registrySource) let raw: string try { @@ -311,11 +321,7 @@ async function resolveRegistryEntry( idOrName: string, options: InstallOptions, ): Promise<{ entry: SkillRegistryEntry; registrySource: string } | null> { - const registrySource = - options.registry ?? - process.env.OPENCLAUDE_SKILLS_REGISTRY_URL ?? - DEFAULT_SKILLS_REGISTRY_URL - const registry = await readRegistryEntries(registrySource) + const registry = await readRegistryEntries(options.registry) const entry = registry.entries.find( candidate => candidate.id === idOrName || @@ -395,7 +401,7 @@ function assertSha256Matches(text: string, expectedSha256: string, spec: string) * specifies must match (id alone covers all versions, sha256 covers the * exact content under any id). */ -function revocationApplies( +export function revocationApplies( revocation: SkillRevocation, skill: { id?: string; version?: string; sha256: string }, ): boolean { diff --git a/src/cli/handlers/skillsVerify.test.ts b/src/cli/handlers/skillsVerify.test.ts new file mode 100644 index 000000000..b61e5930c --- /dev/null +++ b/src/cli/handlers/skillsVerify.test.ts @@ -0,0 +1,496 @@ +import assert from 'node:assert/strict' +import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { delimiter, join } from 'node:path' +import { test } from 'bun:test' + +import { + acquireSharedMutationLock, + releaseSharedMutationLock, +} from '../../test/sharedMutationLock.js' +import { setClaudeConfigHomeDirForTesting } from '../../utils/envUtils.js' +import { setEyebrowRunnerForTesting, skillsVerifyHandler } from './skillsVerify.ts' + +const SKILL_MD = `--- +name: sample-skill +description: Sample skill used by verify tests. +--- + +# Sample Skill +` + +const REGISTRY_SHA256 = + '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef' + +type Captured = { out: string[]; err: string[] } + +/** + * Builds an empty project and user skills root, a registry directory, and + * clean process state: no eyebrow on PATH, no env override, exit code 0. + * Every test mutates process globals, so each one holds the shared lock. + */ +async function withVerifyFixture( + fn: (fixture: { + tempDir: string + projectDir: string + userSkills: string + projectSkills: string + registryDir: string + captured: Captured + }) => Promise, +): Promise { + await acquireSharedMutationLock('skillsVerifyHandler') + const tempDir = mkdtempSync(join(tmpdir(), 'openclaude-skill-verify-test-')) + const projectDir = join(tempDir, 'project') + const projectSkills = join(projectDir, '.openclaude', 'skills') + const userHome = join(tempDir, 'home') + const userSkills = join(userHome, 'skills') + const registryDir = join(tempDir, 'registry') + const emptyPathDir = join(tempDir, 'empty-path') + for (const dir of [projectSkills, userSkills, registryDir, emptyPathDir]) { + mkdirSync(dir, { recursive: true }) + } + writeFileSync(join(registryDir, 'registry.json'), '[]', 'utf8') + writeFileSync(join(registryDir, 'revocations.json'), '[]', 'utf8') + + const saved = { + path: process.env.PATH, + bin: process.env.OPENCLAUDE_EYEBROW_BIN, + revocationsUrl: process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL, + registryUrl: process.env.OPENCLAUDE_SKILLS_REGISTRY_URL, + log: console.log, + error: console.error, + } + process.env.PATH = emptyPathDir + delete process.env.OPENCLAUDE_EYEBROW_BIN + delete process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL + delete process.env.OPENCLAUDE_SKILLS_REGISTRY_URL + process.exitCode = 0 + setClaudeConfigHomeDirForTesting(userHome) + const captured: Captured = { out: [], err: [] } + console.log = (...parts: unknown[]) => { + captured.out.push(parts.map(String).join(' ')) + } + console.error = (...parts: unknown[]) => { + captured.err.push(parts.map(String).join(' ')) + } + try { + await fn({ tempDir, projectDir, userSkills, projectSkills, registryDir, captured }) + } finally { + console.log = saved.log + console.error = saved.error + setClaudeConfigHomeDirForTesting(undefined) + setEyebrowRunnerForTesting(undefined) + process.exitCode = 0 + restoreEnv('PATH', saved.path) + restoreEnv('OPENCLAUDE_EYEBROW_BIN', saved.bin) + restoreEnv('OPENCLAUDE_SKILLS_REVOCATIONS_URL', saved.revocationsUrl) + restoreEnv('OPENCLAUDE_SKILLS_REGISTRY_URL', saved.registryUrl) + rmSync(tempDir, { recursive: true, force: true }) + releaseSharedMutationLock() + } +} + +/** + * True when this platform lets the test create a directory symlink. + * Windows needs a privilege or developer mode for that, so the symlink + * cases are reported as skipped there instead of failing in setup. + */ +function canSymlinkDirectories(): boolean { + const probeDir = mkdtempSync(join(tmpdir(), 'openclaude-skill-verify-symlink-probe-')) + const target = join(probeDir, 'target') + mkdirSync(target) + try { + symlinkSync(target, join(probeDir, 'link'), 'dir') + return true + } catch { + return false + } finally { + rmSync(probeDir, { recursive: true, force: true }) + } +} + +const symlinkTest = test.serial.skipIf(!canSymlinkDirectories()) + +function restoreEnv(name: string, value: string | undefined): void { + if (value === undefined) { + delete process.env[name] + } else { + process.env[name] = value + } +} + +function writeSkill( + root: string, + name: string, + sidecar?: Record, +): string { + const dir = join(root, ...name.split(':')) + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'SKILL.md'), SKILL_MD, 'utf8') + if (sidecar) { + writeFileSync(join(dir, 'skill.json'), JSON.stringify(sidecar), 'utf8') + } + return dir +} + +function registrySidecar(overrides: Record = {}): Record { + return { + id: 'gitlawb/sample-skill', + name: 'sample-skill', + trust: 'official', + version: '0.1.0', + sha256: REGISTRY_SHA256, + ...overrides, + } +} + +function writeRevocations(registryDir: string, entries: unknown): void { + writeFileSync( + join(registryDir, 'revocations.json'), + typeof entries === 'string' ? entries : JSON.stringify(entries), + 'utf8', + ) +} + +test.serial('reports installed registry skills as ok when nothing is revoked', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, userSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeSkill(userSkills, 'git:commit', registrySidecar({ id: 'gitlawb/git-commit' })) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 0) + assert.match(captured.out[0]!, /^Found 2 installed skills\. Revocation list: .*revocations\.json$/) + assert.ok(captured.out.some(line => /git:commit\s+ok$/.test(line))) + assert.ok(captured.out.some(line => /sample-skill\s+ok$/.test(line))) + assert.ok(captured.out.some(line => line.startsWith('eyebrow not found'))) + assert.deepEqual(captured.err, []) + }) +}) + +test.serial('flags a skill revoked by id and version', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeRevocations(registryDir, [ + { id: 'gitlawb/sample-skill', version: '0.1.0', reason: 'compromised release' }, + ]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.ok( + captured.out.some(line => /sample-skill\s+REVOKED \(compromised release\)$/.test(line)), + ) + assert.match(captured.err[0]!, /^1 revoked skill installed\./) + }) +}) + +test.serial('flags a skill revoked by digest alone', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeRevocations(registryDir, [{ sha256: REGISTRY_SHA256.toUpperCase() }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.ok(captured.out.some(line => /sample-skill\s+REVOKED$/.test(line))) + }) +}) + +test.serial('leaves a skill alone when the revocation names another version', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeRevocations(registryDir, [{ id: 'gitlawb/sample-skill', version: '0.2.0' }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 0) + assert.ok(captured.out.some(line => /sample-skill\s+ok$/.test(line))) + }) +}) + +test.serial('skips local skills that carry no registry metadata', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'hand-made') + writeSkill(projectSkills, 'stale-sidecar', { trust: 'local' }) + writeRevocations(registryDir, [{ id: 'gitlawb/hand-made' }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 0) + assert.ok( + captured.out.some(line => /hand-made\s+skipped \(no registry metadata\)$/.test(line)), + ) + assert.ok( + captured.out.some(line => /stale-sidecar\s+skipped \(no registry metadata\)$/.test(line)), + ) + }) +}) + +test.serial('matches a copied sidecar on a local install like a registry one', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'copied', registrySidecar({ id: 'gitlawb/copied', trust: 'official' })) + writeRevocations(registryDir, [{ id: 'gitlawb/copied' }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.ok(captured.out.some(line => /^\s+copied\s+REVOKED$/.test(line))) + }) +}) + +symlinkTest('finds a skill behind a symlinked skill directory', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, projectSkills, registryDir, captured }) => { + const target = writeSkill(join(tempDir, 'elsewhere'), 'linked', registrySidecar({ id: 'gitlawb/linked' })) + symlinkSync(target, join(projectSkills, 'linked'), 'dir') + writeRevocations(registryDir, [{ id: 'gitlawb/linked' }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.ok(captured.out.some(line => /^\s+linked\s+REVOKED$/.test(line))) + }) +}) + +test.serial('finds a skill nested below another skill', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'outer', registrySidecar({ id: 'gitlawb/outer' })) + writeSkill(projectSkills, 'outer:inner', registrySidecar({ id: 'gitlawb/inner' })) + writeRevocations(registryDir, [{ id: 'gitlawb/inner' }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.match(captured.out[0]!, /^Found 2 installed skills\. /) + assert.ok(captured.out.some(line => /^\s+outer\s+ok$/.test(line))) + assert.ok(captured.out.some(line => /^\s+outer:inner\s+REVOKED$/.test(line))) + }) +}) + +symlinkTest('reports a skill once when a symlink points back at the skills root', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + symlinkSync(projectSkills, join(projectSkills, 'loop-root'), 'dir') + symlinkSync(projectDir, join(projectSkills, 'loop-ancestor'), 'dir') + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 0) + assert.match(captured.out[0]!, /^Found 1 installed skill\. /) + assert.equal(captured.out.filter(line => /sample-skill\s+ok$/.test(line)).length, 1) + }) +}) + +symlinkTest('reports a skill once when project and user roots link the same directory', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, projectSkills, userSkills, registryDir, captured }) => { + const target = writeSkill(join(tempDir, 'elsewhere'), 'shared', registrySidecar({ id: 'gitlawb/shared' })) + symlinkSync(target, join(projectSkills, 'shared'), 'dir') + symlinkSync(target, join(userSkills, 'shared'), 'dir') + writeRevocations(registryDir, [{ id: 'gitlawb/shared' }]) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.match(captured.out[0]!, /^Found 1 installed skill\. /) + assert.equal(captured.out.filter(line => /^\s+shared\s+REVOKED$/.test(line)).length, 1) + assert.match(captured.err[0]!, /^1 revoked skill installed\./) + }) +}) + +test.serial('fails closed when the revocation list cannot be parsed', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeRevocations(registryDir, '{not json') + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.match(captured.err[0]!, /Revocation list at .* is not valid JSON\./) + assert.deepEqual(captured.out, []) + }) +}) + +test.serial('reads an absent revocation list as empty', async () => { + await withVerifyFixture(async ({ projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + rmSync(join(registryDir, 'revocations.json')) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 0) + assert.ok(captured.out.some(line => /sample-skill\s+ok$/.test(line))) + }) +}) + +test.serial('hands off to eyebrow with the project path and lockfile', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, projectSkills, registryDir, captured }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeFileSync(join(projectDir, 'eyebrowlock.json'), '{}', 'utf8') + const binary = join(tempDir, 'fake-eyebrow') + writeFileSync(binary, '', 'utf8') + process.env.OPENCLAUDE_EYEBROW_BIN = binary + const calls: { binary: string; args: string[]; cwd: string }[] = [] + setEyebrowRunnerForTesting(async (bin, args, cwd) => { + calls.push({ binary: bin, args, cwd }) + return 1 + }) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(calls.length, 1) + assert.equal(calls[0]!.binary, binary) + assert.equal(calls[0]!.cwd, projectDir) + assert.deepEqual(calls[0]!.args, [ + 'verify', + '--path', + '.', + '--lockfile', + 'eyebrowlock.json', + ]) + assert.equal(process.exitCode, 1) + assert.ok(captured.out.some(line => line.startsWith(`Running ${binary} verify `))) + }) +}) + +test.serial('adds --ci and the policy path when a policy is given', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, registryDir }) => { + writeFileSync(join(projectDir, 'custom.lock.json'), '{}', 'utf8') + process.env.OPENCLAUDE_EYEBROW_BIN = join(tempDir, 'fake-eyebrow') + const calls: string[][] = [] + setEyebrowRunnerForTesting(async (_bin, args) => { + calls.push(args) + return 0 + }) + + await skillsVerifyHandler({ + projectDir, + registry: registryDir, + lockfile: 'custom.lock.json', + policy: 'eyebrow.policy.json', + }) + + assert.equal(process.exitCode, 0) + assert.deepEqual(calls[0], [ + 'verify', + '--path', + '.', + '--lockfile', + 'custom.lock.json', + '--ci', + '--policy', + 'eyebrow.policy.json', + ]) + }) +}) + +test.serial('keeps exit code 1 for a revoked skill even when eyebrow is clean', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, projectSkills, registryDir }) => { + writeSkill(projectSkills, 'sample-skill', registrySidecar()) + writeRevocations(registryDir, [{ id: 'gitlawb/sample-skill' }]) + writeFileSync(join(projectDir, 'eyebrowlock.json'), '{}', 'utf8') + process.env.OPENCLAUDE_EYEBROW_BIN = join(tempDir, 'fake-eyebrow') + setEyebrowRunnerForTesting(async () => 0) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + }) +}) + +test.serial('does not run eyebrow when the lockfile is missing', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, registryDir, captured }) => { + process.env.OPENCLAUDE_EYEBROW_BIN = join(tempDir, 'fake-eyebrow') + let ran = false + setEyebrowRunnerForTesting(async () => { + ran = true + return 1 + }) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(ran, false) + assert.equal(process.exitCode, 0) + assert.ok( + captured.out.some(line => /^eyebrow found but no lockfile at .*eyebrowlock\.json\. Run "eyebrow scan --path \. --lockfile eyebrowlock\.json" in /.test(line)), + ) + }) +}) + +test.serial('finds eyebrow on PATH', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, registryDir, captured }) => { + const binDir = join(tempDir, 'bin') + mkdirSync(binDir) + writeFileSync(join(binDir, 'eyebrow'), '', 'utf8') + chmodSync(join(binDir, 'eyebrow'), 0o755) + process.env.PATH = [process.env.PATH, binDir].join(delimiter) + writeFileSync(join(projectDir, 'eyebrowlock.json'), '{}', 'utf8') + const binaries: string[] = [] + setEyebrowRunnerForTesting(async bin => { + binaries.push(bin) + return 0 + }) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.deepEqual(binaries, [join(binDir, 'eyebrow')]) + assert.equal(process.exitCode, 0) + assert.ok(captured.out.some(line => line.startsWith(`Running ${join(binDir, 'eyebrow')} verify `))) + }) +}) + +test.serial('skips a non-executable eyebrow file earlier on PATH', async () => { + if (process.platform === 'win32') { + return + } + await withVerifyFixture(async ({ tempDir, projectDir, registryDir }) => { + const plainDir = join(tempDir, 'plain') + const execDir = join(tempDir, 'exec') + mkdirSync(plainDir) + mkdirSync(execDir) + writeFileSync(join(plainDir, 'eyebrow'), '', 'utf8') + chmodSync(join(plainDir, 'eyebrow'), 0o644) + writeFileSync(join(execDir, 'eyebrow'), '', 'utf8') + chmodSync(join(execDir, 'eyebrow'), 0o755) + process.env.PATH = [plainDir, execDir, process.env.PATH].join(delimiter) + writeFileSync(join(projectDir, 'eyebrowlock.json'), '{}', 'utf8') + const binaries: string[] = [] + setEyebrowRunnerForTesting(async bin => { + binaries.push(bin) + return 0 + }) + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.deepEqual(binaries, [join(execDir, 'eyebrow')]) + }) +}) + +test.serial('passes a real eyebrow exit code through', async () => { + if (process.platform === 'win32') { + return + } + await withVerifyFixture(async ({ tempDir, projectDir, registryDir }) => { + const binary = join(tempDir, 'eyebrow.sh') + writeFileSync(binary, '#!/bin/sh\nexit 3\n', 'utf8') + chmodSync(binary, 0o755) + process.env.OPENCLAUDE_EYEBROW_BIN = binary + writeFileSync(join(projectDir, 'eyebrowlock.json'), '{}', 'utf8') + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 3) + }) +}) + +test.serial('reports a binary that cannot be started', async () => { + await withVerifyFixture(async ({ tempDir, projectDir, registryDir, captured }) => { + process.env.OPENCLAUDE_EYEBROW_BIN = join(tempDir, 'missing-eyebrow') + writeFileSync(join(projectDir, 'eyebrowlock.json'), '{}', 'utf8') + + await skillsVerifyHandler({ projectDir, registry: registryDir }) + + assert.equal(process.exitCode, 1) + assert.match(captured.err[0]!, /^Failed to run eyebrow at .*missing-eyebrow: /) + }) +}) diff --git a/src/cli/handlers/skillsVerify.ts b/src/cli/handlers/skillsVerify.ts new file mode 100644 index 000000000..becdda41c --- /dev/null +++ b/src/cli/handlers/skillsVerify.ts @@ -0,0 +1,351 @@ +/** + * `openclaude skills verify`: checks every installed skill against the + * registry's revocation list, then hands the content check to eyebrow + * when that tool is installed. The client never re-hashes SKILL.md + * itself; the lockfile and the drift verdict stay with eyebrow. + */ + +import { spawn } from 'child_process' +import { constants as fsConstants } from 'fs' +import { delimiter, join, relative, resolve, sep } from 'path' +import { getCwd } from '../../utils/cwd.js' +import { getClaudeConfigHomeDir } from '../../utils/envUtils.js' +import { getDisplayPath } from '../../utils/file.js' +import { getFsImplementation } from '../../utils/fsOperations.js' +import { PROJECT_CONFIG_DIR_NAMES } from '../../utils/markdownConfigLoader.js' +import { + readRevocations, + resolveRegistrySource, + resolveRevocationsSource, + revocationApplies, + type SkillRevocation, +} from './skillsInstall.js' + +export type VerifyOptions = { + projectDir?: string + registry?: string + lockfile?: string + policy?: string +} + +type InstalledSkill = { + name: string + dir: string + id?: string + version?: string + sha256?: string +} + +/** + * Runs the eyebrow binary and resolves with its exit code. Replaced in + * tests so the handler's argument building and exit code handling can be + * checked without a real binary. + */ +export type EyebrowRunner = ( + binary: string, + args: string[], + cwd: string, +) => Promise + +const EYEBROW_BINARY_ENV = 'OPENCLAUDE_EYEBROW_BIN' +const DEFAULT_LOCKFILE_NAME = 'eyebrowlock.json' +const SHA256_HEX = /^[a-fA-F0-9]{64}$/ + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +async function pathExists(path: string): Promise { + try { + await getFsImplementation().stat(path) + return true + } catch { + return false + } +} + +async function isDirectory(path: string): Promise { + try { + return (await getFsImplementation().stat(path)).isDirectory() + } catch { + return false + } +} + +/** + * Reads the skill.json sidecar as written on disk. The digest is the + * registry pin recorded at install, not a hash of the current files. A + * skill with no sidecar, or one without an id or digest, carries nothing + * a revocation entry can match. + */ +async function describeSkill(root: string, dir: string): Promise { + const skill: InstalledSkill = { + name: relative(root, dir).split(sep).join(':'), + dir, + } + let parsed: unknown + try { + parsed = JSON.parse( + await getFsImplementation().readFile(join(dir, 'skill.json'), { + encoding: 'utf8', + }), + ) + } catch { + return skill + } + if (!isPlainObject(parsed)) { + return skill + } + if (typeof parsed.id === 'string' && parsed.id.trim() !== '') { + skill.id = parsed.id + } + if (typeof parsed.version === 'string' && parsed.version.trim() !== '') { + skill.version = parsed.version + } + if (typeof parsed.sha256 === 'string' && SHA256_HEX.test(parsed.sha256.trim())) { + skill.sha256 = parsed.sha256.trim().toLowerCase() + } + return skill +} + +/** + * Finds skill directories under a skills root the way the loader does: a + * directory that holds SKILL.md, nested to any depth, reached through + * symlinks too, including a skill nested below another skill. Files + * directly in the root are ignored. `visited` holds real directory paths + * and is shared across roots, so a symlink back to the root, to an + * ancestor, or to a directory another root already covered cannot loop + * or report a skill twice. + */ +async function findInstalledSkills( + root: string, + visited: Set, +): Promise { + const fs = getFsImplementation() + const found: InstalledSkill[] = [] + + function realDirectory(dir: string): string | undefined { + try { + return fs.realpathSync(dir) + } catch { + return undefined + } + } + + async function walk(dir: string): Promise { + const realDir = realDirectory(dir) + if (realDir === undefined || visited.has(realDir)) { + return + } + visited.add(realDir) + + let entries + try { + entries = await fs.readdir(dir) + } catch { + return + } + if (entries.some(entry => entry.name === 'SKILL.md')) { + found.push(await describeSkill(root, dir)) + } + for (const entry of entries) { + const entryPath = join(dir, entry.name) + if (entry.isDirectory() || (entry.isSymbolicLink() && (await isDirectory(entryPath)))) { + await walk(entryPath) + } + } + } + + const realRoot = realDirectory(root) + if (realRoot === undefined) { + return [] + } + visited.add(realRoot) + let topLevel + try { + topLevel = await fs.readdir(root) + } catch { + return [] + } + for (const entry of topLevel) { + const entryPath = join(root, entry.name) + if (entry.isDirectory() || (entry.isSymbolicLink() && (await isDirectory(entryPath)))) { + await walk(entryPath) + } + } + found.sort((a, b) => a.name.localeCompare(b.name)) + return found +} + +function skillRoots(projectDir: string): string[] { + return [ + ...PROJECT_CONFIG_DIR_NAMES.map(configDirName => + join(projectDir, configDirName, 'skills'), + ), + join(getClaudeConfigHomeDir(), 'skills'), + ] +} + +function revocationReason(match: SkillRevocation): string { + return typeof match.reason === 'string' && match.reason.trim() !== '' + ? ` (${match.reason.trim()})` + : '' +} + +/** + * True for a regular file this process may execute, checked through the + * active filesystem implementation. On Windows the name's extension + * decides, so existence is enough there. + */ +async function isExecutableFile(candidate: string): Promise { + const fs = getFsImplementation() + try { + if (!(await fs.stat(candidate)).isFile()) { + return false + } + await fs.access( + candidate, + process.platform === 'win32' ? fsConstants.F_OK : fsConstants.X_OK, + ) + return true + } catch { + return false + } +} + +/** + * Locates eyebrow: OPENCLAUDE_EYEBROW_BIN when set, otherwise the first + * executable named `eyebrow` on PATH. A file on an earlier PATH entry + * without execute permission is skipped. Returns undefined when nothing + * is found, so the caller can say so and stop instead of failing. + */ +async function findEyebrowBinary(): Promise { + const override = process.env[EYEBROW_BINARY_ENV] + if (override && override.trim() !== '') { + return override + } + // Only names a plain spawn can start: no .cmd or .bat wrappers on + // Windows, which need a shell. + const names = process.platform === 'win32' ? ['eyebrow.exe'] : ['eyebrow'] + for (const dir of (process.env.PATH ?? '').split(delimiter)) { + if (dir === '') continue + for (const name of names) { + const candidate = join(dir, name) + if (await isExecutableFile(candidate)) { + return candidate + } + } + } + return undefined +} + +const defaultEyebrowRunner: EyebrowRunner = (binary, args, cwd) => + new Promise((resolveExit, reject) => { + const child = spawn(binary, args, { cwd, stdio: 'inherit' }) + child.once('error', reject) + // A signal kill leaves no exit code; report it as eyebrow's internal + // error code rather than a clean run. + child.once('close', code => resolveExit(code ?? 3)) + }) + +let eyebrowRunner: EyebrowRunner = defaultEyebrowRunner + +export function setEyebrowRunnerForTesting(runner: EyebrowRunner | undefined): void { + eyebrowRunner = runner ?? defaultEyebrowRunner +} + +/** + * Reports each installed skill's revocation state, then runs + * `eyebrow verify` on the project when eyebrow and a lockfile exist. + * Exit code: 1 when a skill is revoked or the revocation list cannot be + * read; otherwise eyebrow's own exit code; 0 when nothing is wrong. + */ +export async function skillsVerifyHandler(options: VerifyOptions = {}): Promise { + const projectDir = resolve(options.projectDir ?? getCwd()) + const visited = new Set() + const skills: InstalledSkill[] = [] + for (const root of skillRoots(projectDir)) { + skills.push(...(await findInstalledSkills(root, visited))) + } + + const registrySource = await resolveRegistrySource(options.registry) + const revocationsSource = resolveRevocationsSource(registrySource) + let revocations: SkillRevocation[] + try { + revocations = await readRevocations(registrySource) + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + return + } + + const width = Math.max(0, ...skills.map(skill => skill.name.length)) + const revoked: InstalledSkill[] = [] + console.log( + `Found ${skills.length} installed skill${skills.length === 1 ? '' : 's'}. Revocation list: ${revocationsSource}`, + ) + for (const skill of skills) { + const label = skill.name.padEnd(width) + if (skill.id === undefined && skill.sha256 === undefined) { + console.log(` ${label} skipped (no registry metadata)`) + continue + } + const match = revocations.find(entry => + revocationApplies(entry, { + id: skill.id, + version: skill.version, + sha256: skill.sha256 ?? '', + }), + ) + if (match) { + revoked.push(skill) + console.log(` ${label} REVOKED${revocationReason(match)}`) + } else { + console.log(` ${label} ok`) + } + } + if (revoked.length > 0) { + console.error( + `${revoked.length} revoked skill${revoked.length === 1 ? '' : 's'} installed. Remove with "openclaude skills remove ".`, + ) + process.exitCode = 1 + } + + const binary = await findEyebrowBinary() + if (!binary) { + console.log( + 'eyebrow not found; install it to check skill contents against a lockfile (see docs/skills.md).', + ) + return + } + // eyebrow runs inside the project with `--path .`, the form the guide + // uses for `scan`: an artifact's id derives from the path it was found + // under, so an absolute path here would report every skill as replaced. + const lockfile = options.lockfile ?? DEFAULT_LOCKFILE_NAME + if (!(await pathExists(resolve(projectDir, lockfile)))) { + console.log( + `eyebrow found but no lockfile at ${getDisplayPath(resolve(projectDir, lockfile))}. Run "eyebrow scan --path . --lockfile ${lockfile}" in ${getDisplayPath(projectDir)} first.`, + ) + return + } + const args = ['verify', '--path', '.', '--lockfile', lockfile] + if (options.policy) { + args.push('--ci', '--policy', options.policy) + } + // Print the binary that runs: verify does not check its version, and an + // eyebrow older than 0.4.4 has no OpenClaude discovery. + console.log(`Running ${binary} ${args.join(' ')} in ${getDisplayPath(projectDir)}`) + let exitCode: number + try { + exitCode = await eyebrowRunner(binary, args, projectDir) + } catch (error) { + console.error( + `Failed to run eyebrow at ${binary}: ${error instanceof Error ? error.message : String(error)}`, + ) + process.exitCode = 1 + return + } + if (exitCode !== 0 && revoked.length === 0) { + process.exitCode = exitCode + } +} diff --git a/src/main.tsx b/src/main.tsx index 70176514e..15aa8e15c 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -4169,6 +4169,13 @@ async function run(): Promise { }) => { await runSkillsCommanderAction(({ skillsRemoveHandler }) => skillsRemoveHandler(name, options)); }); + skillsCmd.command('verify').description('Check installed skills against the registry revocation list and, when eyebrow is installed, the lockfile').option('--registry ', 'Registry JSON URL/path whose revocations.json to read').option('--lockfile ', 'eyebrow lockfile path (default: eyebrowlock.json)').option('--policy ', 'eyebrow policy file; adds --ci to the eyebrow run').action(async (options: { + registry?: string; + lockfile?: string; + policy?: string; + }) => { + await runSkillsCommanderAction(({ skillsVerifyHandler }) => skillsVerifyHandler(options)); + }); if (feature('TRANSCRIPT_CLASSIFIER')) { // Skip when tengu_auto_mode_config.enabled === 'disabled' (circuit breaker). // Reads from disk cache — GrowthBook isn't initialized at registration time. diff --git a/src/utils/fsOperations.ts b/src/utils/fsOperations.ts index 673815e63..23d096e9d 100644 --- a/src/utils/fsOperations.ts +++ b/src/utils/fsOperations.ts @@ -34,6 +34,8 @@ export type FsOperations = { stat(path: string): Promise /** Gets file stats asynchronously without following symlinks */ lstat(path: string): Promise + /** Rejects unless the process has the given fs.constants access mode on the path */ + access(path: string, mode?: number): Promise /** Lists directory contents with file type information asynchronously */ readdir(path: string): Promise /** Deletes file asynchronously */ @@ -432,6 +434,10 @@ export const NodeFsOperations: FsOperations = { return statPromise(fsPath) }, + async access(fsPath, mode) { + return fs.promises.access(fsPath, mode) + }, + async lstat(fsPath) { return lstatPromise(fsPath) },