diff --git a/src/cli/handlers/skills.test.ts b/src/cli/handlers/skills.test.ts index ca8b798c4..7449fe710 100644 --- a/src/cli/handlers/skills.test.ts +++ b/src/cli/handlers/skills.test.ts @@ -161,12 +161,76 @@ function assertNoNewStagedInstallDirs(before: string[]): void { assert.deepEqual(stagedInstallTempDirs().sort(), before.sort()) } +const REMOTE_REGISTRY_URL = 'https://registry.test/registry.json' +const REMOTE_REVOCATIONS_URL = 'https://registry.test/revocations.json' +const REMOTE_SKILL_URL = 'https://registry.test/skills/sample-skill/SKILL.md' + +type FetchHandler = (url: string) => Response | Promise + +/** + * Serves a registry and its skill through a mocked fetch, so the install + * resolves revocations.json as the registry's remote sibling and reads it + * over HTTP. `revocations` decides that one route; every requested URL is + * recorded so a test can prove the transport boundary was exercised. + */ +function remoteRegistryFetch( + revocations: () => Response | Promise, + requested: string[], +): FetchHandler { + return url => { + requested.push(url) + switch (url) { + case REMOTE_REGISTRY_URL: + return new Response( + JSON.stringify([ + buildRegistryEntry('', { + source: 'skills/sample-skill/SKILL.md', + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + { status: 200 }, + ) + case REMOTE_SKILL_URL: + return new Response(VALID_SKILL, { status: 200 }) + case REMOTE_REVOCATIONS_URL: + return revocations() + default: + throw new Error(`Unexpected fetch during install: ${url}`) + } + } +} + +async function withMockedFetch( + handler: FetchHandler, + fn: () => Promise, +): Promise { + const originalFetch = globalThis.fetch + globalThis.fetch = (async (input: string | URL | Request) => + handler(input instanceof Request ? input.url : String(input))) as typeof fetch + try { + return await fn() + } finally { + globalThis.fetch = originalFetch + } +} + async function withTempDir(fn: (tempDir: string) => Promise): Promise { const tempDir = mkdtempSync(join(tmpdir(), 'openclaude-skill-install-test-')) + // The revocation override is process-global input that the fixtures below + // resolve against, and production gives it precedence over the sibling + // revocations.json. Own it here: clear any value inherited from the host + // process so each fixture reads its own list, and hand it back afterwards. + const inheritedRevocationsUrl = process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL + delete process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL try { return await fn(tempDir) } finally { process.exitCode = 0 + if (inheritedRevocationsUrl === undefined) { + delete process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL + } else { + process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL = inheritedRevocationsUrl + } rmSync(tempDir, { recursive: true, force: true }) } } @@ -629,6 +693,390 @@ test.serial('rejects registry skills that require a newer OpenClaude version', a }) }) +test.serial('rejects registry skills revoked by id and version', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync( + join(tempDir, 'revocations.json'), + JSON.stringify([ + { + id: 'gitlawb/sample-skill', + version: '0.1.0', + sha256: sha256OfSkillSource(VALID_SKILL), + reason: 'compromised release', + }, + ]), + 'utf8', + ) + + const stagedBefore = stagedInstallTempDirs() + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + assertNoNewStagedInstallDirs(stagedBefore) + }) +}) + +test.serial('rejects registry skills revoked by digest alone', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync( + join(tempDir, 'revocations.json'), + JSON.stringify([{ sha256: sha256OfSkillSource(VALID_SKILL) }]), + 'utf8', + ) + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) +}) + +test.serial('installs registry skills when a revocation targets another version', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync( + join(tempDir, 'revocations.json'), + JSON.stringify([{ id: 'gitlawb/sample-skill', version: '0.0.9' }]), + 'utf8', + ) + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal( + existsSync(join(cwd, '.openclaude', 'skills', 'sample-skill', 'SKILL.md')), + true, + ) + }) +}) + +test.serial('rejects registry installs when revocations.json is malformed', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync(join(tempDir, 'revocations.json'), '{not json', 'utf8') + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) +}) + +test.serial('OPENCLAUDE_SKILLS_REVOCATIONS_URL takes precedence over the registry sibling', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + // The sibling list revokes nothing; the override list revokes the skill. + writeFileSync(join(tempDir, 'revocations.json'), '[]', 'utf8') + const overridePath = join(tempDir, 'override-revocations.json') + writeFileSync( + overridePath, + JSON.stringify([{ id: 'gitlawb/sample-skill' }]), + 'utf8', + ) + + // Set inside the fixture, which cleared any inherited value on entry and + // restores it on exit. + process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL = overridePath + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) +}) + +test.serial('an inherited OPENCLAUDE_SKILLS_REVOCATIONS_URL cannot redirect sibling-list fixtures', async () => { + // Simulate a host process (developer shell, CI runner) that already + // configures the override before the suite runs. Production gives that + // variable precedence, so without fixture isolation every sibling-list + // test would silently read this absent path instead of its own fixture. + const hostValue = process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL + const inheritedAbsentPath = join(tmpdir(), 'openclaude-revocations-never-written.json') + process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL = inheritedAbsentPath + try { + await withTempDir(async tempDir => { + assert.equal(process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL, undefined) + + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync( + join(tempDir, 'revocations.json'), + JSON.stringify([{ id: 'gitlawb/sample-skill' }]), + 'utf8', + ) + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + // The sibling list revoked the skill: the inherited override was not read. + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) + // The fixture hands the host's value back once it is done. + assert.equal(process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL, inheritedAbsentPath) + } finally { + if (hostValue === undefined) { + delete process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL + } else { + process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL = hostValue + } + } +}) + +test.serial('rejects registry installs when a revocation entry is invalid', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync( + join(tempDir, 'revocations.json'), + JSON.stringify([{ id: 'gitlawb/sample-skill', sha256: 'not-a-digest' }]), + 'utf8', + ) + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) +}) + +test.serial('rejects registry installs when revocations.json is not an array', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + writeFileSync(join(tempDir, 'revocations.json'), '{}', 'utf8') + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) +}) + +test.serial('fails closed when the revocation list exists but cannot be read', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + const sourceDir = writeSkillDir(join(tempDir, 'registry-source')) + const registryPath = join(tempDir, 'registry.json') + mkdirSync(cwd, { recursive: true }) + writeFileSync( + registryPath, + JSON.stringify([ + buildRegistryEntry(sourceDir, { + sha256: sha256OfSkillSource(VALID_SKILL), + }), + ]), + 'utf8', + ) + // A directory at the revocations path reads as EISDIR, not ENOENT: + // unreadable is not the same as absent, and must block the install. + mkdirSync(join(tempDir, 'revocations.json')) + + await skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: registryPath, + }) + + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + }) +}) + +test.serial('installs a non-revoked registry skill when the remote revocation list is HTTP 404', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + mkdirSync(cwd, { recursive: true }) + const requested: string[] = [] + + await withMockedFetch( + remoteRegistryFetch(() => new Response('not found', { status: 404 }), requested), + () => + skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: REMOTE_REGISTRY_URL, + }), + ) + + // A registry that publishes no list at all is a confirmed absence: it + // revokes nothing, and the sha256 pin still governs the install. + assert.ok(requested.includes(REMOTE_REVOCATIONS_URL)) + assert.equal(process.exitCode, 0) + assert.equal( + readFileSync( + join(cwd, '.openclaude', 'skills', 'sample-skill', 'SKILL.md'), + 'utf8', + ), + VALID_SKILL, + ) + }) +}) + +test.serial('fails closed when the remote revocation list is HTTP 500', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + mkdirSync(cwd, { recursive: true }) + const requested: string[] = [] + + const stagedBefore = stagedInstallTempDirs() + await withMockedFetch( + remoteRegistryFetch(() => new Response('upstream error', { status: 500 }), requested), + () => + skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: REMOTE_REGISTRY_URL, + }), + ) + + // A list that exists but cannot be served is not an empty list. + assert.ok(requested.includes(REMOTE_REVOCATIONS_URL)) + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + assertNoNewStagedInstallDirs(stagedBefore) + }) +}) + +test.serial('fails closed when the remote revocation list is unreachable', async () => { + await withTempDir(async tempDir => { + const cwd = join(tempDir, 'project') + mkdirSync(cwd, { recursive: true }) + const requested: string[] = [] + + const stagedBefore = stagedInstallTempDirs() + await withMockedFetch( + remoteRegistryFetch(() => { + throw new TypeError('fetch failed') + }, requested), + () => + skillsInstallHandler('sample-skill', { + projectDir: cwd, + registry: REMOTE_REGISTRY_URL, + }), + ) + + assert.ok(requested.includes(REMOTE_REVOCATIONS_URL)) + assert.equal(process.exitCode, 1) + assert.equal(existsSync(join(cwd, '.openclaude', 'skills')), false) + assertNoNewStagedInstallDirs(stagedBefore) + }) +}) + test.serial('rejects path-like skill names before installing raw markdown', async () => { await withTempDir(async tempDir => { const cwd = join(tempDir, 'project') diff --git a/src/cli/handlers/skillsInstall.ts b/src/cli/handlers/skillsInstall.ts index 97c6fb387..87a7327ed 100644 --- a/src/cli/handlers/skillsInstall.ts +++ b/src/cli/handlers/skillsInstall.ts @@ -45,6 +45,28 @@ type RegistryEntriesResult = { registrySource: string } +type SkillRevocation = { + id?: unknown + version?: unknown + sha256?: unknown + reason?: unknown +} + +/** + * A remote source answered with a non-success status. Carries the status + * so callers can tell a confirmed-absent source (404) from a failing one + * without parsing the message. + */ +class RemoteSourceHttpError extends Error { + status: number + + constructor(source: string, status: number) { + super(`Failed to fetch ${source}: HTTP ${status}`) + this.name = 'RemoteSourceHttpError' + this.status = status + } +} + const DEFAULT_SKILLS_REGISTRY_URL = 'https://raw.githubusercontent.com/Gitlawb/openclaude-skills/main/registry.json' const VALID_INSTALL_SKILL_NAME = /^[a-z0-9][a-z0-9-]*(?::[a-z0-9][a-z0-9-]*)*$/ @@ -105,7 +127,7 @@ async function readSourceText(source: string): Promise { try { const response = await fetch(url, { signal }) if (!response.ok) { - throw new Error(`Failed to fetch ${source}: HTTP ${response.status}`) + throw new RemoteSourceHttpError(source, response.status) } const contentLength = response.headers.get('content-length') @@ -182,6 +204,109 @@ function resolveRegistryEntrySource( return resolve(dirname(registrySource), entrySource) } +/** + * Resolves where the revocation list lives: the + * 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 { + return ( + process.env.OPENCLAUDE_SKILLS_REVOCATIONS_URL ?? + resolveRegistryEntrySource('revocations.json', registrySource) + ) +} + +/** + * True when an error means the source does not exist (local ENOENT or + * remote HTTP 404), as opposed to existing but being unreadable. + */ +function isAbsentSourceError(error: unknown): boolean { + if ((error as NodeJS.ErrnoException | null)?.code === 'ENOENT') { + return true + } + return error instanceof RemoteSourceHttpError && error.status === 404 +} + +/** + * Validates one revocation entry: optional non-empty id/version strings, + * an optional 64-hex sha256, an optional reason string, and at least an + * id or a digest to pin on. Throws naming the entry index otherwise. + */ +function parseRevocation( + value: unknown, + index: number, + source: string, +): SkillRevocation { + function reject(problem: string): never { + throw new Error( + `Revocation list at ${source} has an invalid entry at index ${index}: ${problem}.`, + ) + } + if (!isPlainObject(value)) { + reject('not an object') + } + const entry = value as SkillRevocation + if ( + entry.id !== undefined && + (typeof entry.id !== 'string' || entry.id.trim() === '') + ) { + reject('id must be a non-empty string') + } + if ( + entry.version !== undefined && + (typeof entry.version !== 'string' || entry.version.trim() === '') + ) { + reject('version must be a non-empty string') + } + if ( + entry.sha256 !== undefined && + (typeof entry.sha256 !== 'string' || + !/^[a-fA-F0-9]{64}$/.test(entry.sha256.trim())) + ) { + reject('sha256 must be a 64-character hex digest') + } + if (entry.reason !== undefined && typeof entry.reason !== 'string') { + reject('reason must be a string') + } + if (entry.id === undefined && entry.sha256 === undefined) { + reject('must pin at least an id or a sha256') + } + return entry +} + +/** + * Reads and validates the registry's revocation list. A confirmed-absent + * list revokes nothing; any other read, parse, or schema failure throws + * so the install fails closed. + */ +async function readRevocations(registrySource: string): Promise { + const source = resolveRevocationsSource(registrySource) + let raw: string + try { + raw = await readSourceText(source) + } catch (error) { + // A registry may publish no revocation list at all, so a confirmed + // absence revokes nothing and the sha256 pin is still enforced. Any + // other failure fails closed: an unreachable kill switch must not + // read as an empty one. + if (isAbsentSourceError(error)) { + return [] + } + const message = error instanceof Error ? error.message : String(error) + throw new Error(`Failed to read revocation list at ${source}: ${message}`) + } + let parsed: unknown + try { + parsed = JSON.parse(raw) as unknown + } catch { + throw new Error(`Revocation list at ${source} is not valid JSON.`) + } + if (!Array.isArray(parsed)) { + throw new Error(`Revocation list at ${source} must be a JSON array.`) + } + return parsed.map((entry, index) => parseRevocation(entry, index, source)) +} + async function resolveRegistryEntry( idOrName: string, options: InstallOptions, @@ -265,6 +390,61 @@ function assertSha256Matches(text: string, expectedSha256: string, spec: string) } } +/** + * True when a revocation entry matches the skill: every field the entry + * specifies must match (id alone covers all versions, sha256 covers the + * exact content under any id). + */ +function revocationApplies( + revocation: SkillRevocation, + skill: { id?: string; version?: string; sha256: string }, +): boolean { + const revokedId = typeof revocation.id === 'string' ? revocation.id : undefined + const revokedVersion = + typeof revocation.version === 'string' ? revocation.version : undefined + const revokedSha256 = + typeof revocation.sha256 === 'string' + ? revocation.sha256.trim().toLowerCase() + : undefined + // An entry must pin at least an id or a digest; every field it does + // specify has to match. + if (revokedId === undefined && revokedSha256 === undefined) { + return false + } + if (revokedId !== undefined && revokedId !== skill.id) { + return false + } + if (revokedVersion !== undefined && revokedVersion !== skill.version) { + return false + } + if (revokedSha256 !== undefined && revokedSha256 !== skill.sha256) { + return false + } + return true +} + +/** + * Throws with the entry's reason when any revocation entry applies to + * the skill being installed. + */ +function assertNotRevoked( + revocations: SkillRevocation[], + skill: { id?: string; version?: string; sha256: string }, + spec: string, +): void { + const match = revocations.find(entry => revocationApplies(entry, skill)) + if (!match) { + return + } + const reason = + typeof match.reason === 'string' && match.reason.trim() !== '' + ? ` Reason: ${match.reason.trim()}` + : '' + throw new Error( + `Skill "${spec}" is revoked in the registry. Refusing to install.${reason}`, + ) +} + function assertCompatibleOpenClaudeVersion(entry: SkillRegistryEntry, spec: string): string | undefined { if ( typeof entry.min_openclaude_version !== 'string' || @@ -513,6 +693,16 @@ async function prepareInstallCandidate( const expectedSha256 = requireRegistrySha256(entry, spec) const minOpenClaudeVersion = assertCompatibleOpenClaudeVersion(entry, spec) + const revocations = await readRevocations(registryMatch.registrySource) + assertNotRevoked( + revocations, + { + id: typeof entry.id === 'string' ? entry.id : undefined, + version: typeof entry.version === 'string' ? entry.version : undefined, + sha256: expectedSha256, + }, + spec, + ) const entrySource = resolveRegistryEntrySource( entry.source, registryMatch.registrySource,