feat(skills): enforce registry revocations.json on install (#2187)

* feat(skills): enforce registry revocations.json on install

The registry ships a revocations.json kill switch that nothing reads. Installs now fetch it from beside the registry (or OPENCLAUDE_SKILLS_REVOCATIONS_URL) and refuse a skill whose id, version, or sha256 matches an entry. A missing list revokes nothing; a malformed list fails the install.

* fix(skills): fail closed and validate revocation entries

Review follow-up: only a confirmed-absent revocation list (ENOENT or HTTP 404) is treated as empty; other read failures block the install. The list must be a JSON array and every entry is schema-checked (non-empty id/version strings, 64-hex sha256, at least an id or a digest). Adds tests for the OPENCLAUDE_SKILLS_REVOCATIONS_URL override, invalid entries, non-array lists, and unreadable lists.

* docs(skills): add docstrings to revocation helpers

* test(skills): isolate revocation override and cover remote failure paths
This commit is contained in:
alexverify
2026-09-04 07:35:04 +08:00
committed by GitHub
parent 3451187aed
commit 0ea8eefbd0
2 changed files with 639 additions and 1 deletions
+448
View File
@@ -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<Response>
/**
* 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<Response>,
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<T>(
handler: FetchHandler,
fn: () => Promise<T>,
): Promise<T> {
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<T>(fn: (tempDir: string) => Promise<T>): Promise<T> {
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')
+191 -1
View File
@@ -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<string> {
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<SkillRevocation[]> {
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,