diff --git a/cli/README.md b/cli/README.md index a30cefec..d54f6045 100644 --- a/cli/README.md +++ b/cli/README.md @@ -302,6 +302,12 @@ skillhub sync push --all --namespace team-a --submit-review The default workspace is `/.agents/skills`. Pull never overwrites local changes unless `--force` is supplied. Remote removals are reported as `orphaned` and are retained unless `--prune` is supplied. Both destructive cases still require explicit flags. +Sync compares both the published version and package fingerprint. An exact match is `up-to-date`, +while a newer version is `update-available` even when its content is unchanged. An older remote +version, an unorderable version pair, or changed remote content without a version bump is `blocked`. +`--force` cannot bypass these release-safety checks; verify the release and use an explicit +`skillhub install` when replacement is intentional. + Workspace push is non-overwriting: an existing namespace/slug/version is reported as a conflict, including versions that are still uploaded or pending review. Other skills in the same `--all` run continue processing. Namespace sync writes `.skillhub/namespace-sync.json` in the workspace and per-skill `.skillhub/metadata.json` files. These files contain the registry coordinate, published version, aggregate fingerprint, and file hashes used by `status` and `diff`. diff --git a/cli/src/commands/sync.ts b/cli/src/commands/sync.ts index 1c91806e..183c4a3e 100644 --- a/cli/src/commands/sync.ts +++ b/cli/src/commands/sync.ts @@ -47,10 +47,15 @@ export async function syncPullCommand(options: SyncPullOptions): Promise const output = renderPullResult(result, Boolean(options.json), Boolean(options.check)) if (result.failures.length > 0) { process.stdout.write(`${output}\n`) - throw new CliError('namespace sync completed with failures', EXIT.generic, { - namespace: context.namespace, - failures: result.failures - }) + const blocked = result.entries.filter(entry => entry.status === 'blocked') + throw new CliError( + blocked.length > 0 ? 'namespace sync blocked by remote version safety checks' : 'namespace sync completed with failures', + blocked.length > 0 ? EXIT.validation : EXIT.generic, + { + namespace: context.namespace, + failures: result.failures + } + ) } return output } diff --git a/cli/src/services/install-service.ts b/cli/src/services/install-service.ts index 88539ec3..ba98ac5a 100644 --- a/cli/src/services/install-service.ts +++ b/cli/src/services/install-service.ts @@ -114,6 +114,14 @@ export async function installSkill(options: InstallOptions): Promise 0 ? 'remote-newer' : 'remote-older' +} diff --git a/cli/src/services/sync-service.ts b/cli/src/services/sync-service.ts index 20cc9c0a..2bab0298 100644 --- a/cli/src/services/sync-service.ts +++ b/cli/src/services/sync-service.ts @@ -7,8 +7,9 @@ import { InventoryStore } from '../stores/inventory-store' import { SyncWorkspaceStore, type NamespaceSyncState } from '../stores/sync-workspace-store' import { createZip, isZipFile } from '../platform/archive' import { pathExists } from '../platform/paths' +import { compareSkillVersions } from './skill-version-order' -export type SyncStatus = 'up-to-date' | 'update-available' | 'local-changed' | 'orphaned' | 'not-installed' +export type SyncStatus = 'up-to-date' | 'update-available' | 'local-changed' | 'blocked' | 'orphaned' | 'not-installed' export interface SkillSyncMetadata { registry: string @@ -88,15 +89,46 @@ export async function inspectNamespaceWorkspace(options: { } const snapshot = await snapshotSkillDirectory(skillDir) + const changedFiles = snapshot.fingerprint === metadata.fingerprint + ? [] + : diffSkillFiles(metadata.files, snapshot.files) + const versionOrder = compareSkillVersions(metadata.version, remote.version) + if (versionOrder === 'remote-older') { + entries.push({ + ...baseEntry(remote, 'blocked'), + localVersion: metadata.version, + changedFiles, + reason: 'remote version is older than the installed version; local files were kept' + }) + continue + } + if (versionOrder === 'unknown') { + entries.push({ + ...baseEntry(remote, 'blocked'), + localVersion: metadata.version, + changedFiles, + reason: 'cannot determine version order; use explicit install after verifying the release' + }) + continue + } + if (versionOrder === 'same' && metadata.fingerprint !== remote.fingerprint) { + entries.push({ + ...baseEntry(remote, 'blocked'), + localVersion: metadata.version, + changedFiles, + reason: 'remote content changed without a newer version; use explicit install after verifying the release' + }) + continue + } if (snapshot.fingerprint !== metadata.fingerprint) { entries.push({ ...baseEntry(remote, 'local-changed'), localVersion: metadata.version, - changedFiles: diffSkillFiles(metadata.files, snapshot.files) + changedFiles }) continue } - if (metadata.fingerprint !== remote.fingerprint) { + if (versionOrder === 'remote-newer') { entries.push({ ...baseEntry(remote, 'update-available'), localVersion: metadata.version @@ -142,7 +174,9 @@ export async function pullNamespace(options: { rootDir: options.rootDir, entries: inspected.entries, actions: [], - failures: [], + failures: inspected.entries + .filter(entry => entry.status === 'blocked') + .map(entry => ({ slug: entry.slug, message: entry.reason ?? 'automatic sync is blocked' })), warnings: [] } if (options.check) return result @@ -150,6 +184,7 @@ export async function pullNamespace(options: { const remoteBySlug = new Map(inspected.remoteItems.map(item => [item.slug, item])) for (const entry of inspected.entries) { if (entry.status === 'up-to-date' || entry.status === 'orphaned') continue + if (entry.status === 'blocked') continue if (entry.status === 'local-changed' && !options.force) { result.failures.push({ slug: entry.slug, message: entry.reason ?? 'local changes detected; pass --force to overwrite' }) continue diff --git a/cli/src/services/upgrade-service.ts b/cli/src/services/upgrade-service.ts index 65cedcc0..e462c6a1 100644 --- a/cli/src/services/upgrade-service.ts +++ b/cli/src/services/upgrade-service.ts @@ -1,5 +1,4 @@ import { isAbsolute, relative, resolve } from 'node:path' -import { compare as compareSemver, valid as validSemver } from 'semver' import { canonicalizeExistingPath, pathExists } from '../platform/paths' import { SkillHubClient, type ResolveResponse } from '../clients/skillhub-client' import { InventoryStore, type InventoryItem, type InventoryTarget } from '../stores/inventory-store' @@ -9,6 +8,7 @@ import { hasExplicitNamespace, parseSkillName, resolveSkillName } from '../share import { diffSkillFiles, snapshotSkillDirectory } from './skill-fingerprint' import { installSkill } from './install-service' import { readInstalledSkillMetadata, sameInstalledSkillSource } from './installed-skill-metadata' +import { compareSkillVersions } from './skill-version-order' const MAX_UPGRADE_SELECTION = 50 @@ -166,7 +166,7 @@ export async function planSkillUpgrades(options: UpgradeSelectionOptions): Promi continue } - const versionOrder = compareVersions(selection.item.version, resolved.version) + const versionOrder = compareSkillVersions(selection.item.version, resolved.version) if (versionOrder === 'remote-older') { items.push({ ...base, @@ -407,14 +407,3 @@ function isSameOrWithin(parent: string, candidate: string): boolean { function normalizeRegistry(registry: string): string { return registry.replace(/\/+$/, '') } - -function compareVersions( - installedVersion: string, - remoteVersion: string -): 'same' | 'remote-newer' | 'remote-older' | 'unknown' { - if (installedVersion === remoteVersion) return 'same' - if (!validSemver(installedVersion) || !validSemver(remoteVersion)) return 'unknown' - const order = compareSemver(remoteVersion, installedVersion) - if (order === 0) return 'same' - return order > 0 ? 'remote-newer' : 'remote-older' -} diff --git a/cli/test/helpers/fake-registry.ts b/cli/test/helpers/fake-registry.ts index 8833d39f..024bd3a9 100644 --- a/cli/test/helpers/fake-registry.ts +++ b/cli/test/helpers/fake-registry.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto' import { unzipSync } from 'fflate' type FakeHandler = (req: Request) => Response | Promise @@ -70,12 +71,25 @@ export interface FakeSkill { version?: string /** Numeric version id returned in resolve. Defaults to 1. */ versionId?: number - /** SHA-256 fingerprint string. Defaults to 'deadbeef'. */ + /** SHA-256 fingerprint string. Defaults to the fingerprint of zipBytes. */ fingerprint?: string /** Raw bytes served as the ZIP body. Defaults to a minimal valid ZIP. */ zipBytes?: Uint8Array } +function resolveSkillFingerprint(skill: FakeSkill): string { + if (skill.fingerprint) return skill.fingerprint + const entries = unzipSync(skill.zipBytes ?? MINIMAL_ZIP) + const aggregate = createHash('sha256') + for (const path of Object.keys(entries) + .filter(path => !path.endsWith('/') && path !== '.skillhub' && !path.startsWith('.skillhub/')) + .sort((left, right) => left.localeCompare(right))) { + const fileHash = createHash('sha256').update(entries[path]!).digest('hex') + aggregate.update(`${path}:${fileHash}\n`, 'utf8') + } + return `sha256:${aggregate.digest('hex')}` +} + // Minimal valid ZIP: local file header + end-of-central-directory record with // zero entries. Enough for any consumer that just checks Content-Type / length. const MINIMAL_ZIP = new Uint8Array([ @@ -296,7 +310,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { slug: skill.slug, version: skill.version ?? '1.0.0', versionId: skill.versionId ?? index + 1, - fingerprint: skill.fingerprint ?? 'deadbeef', + fingerprint: resolveSkillFingerprint(skill), updatedAt: '2026-08-18T00:00:00Z', visibility: 'NAMESPACE_ONLY', downloadUrl: buildDownloadUrl(baseUrl, namespace, skill.slug, skill.version ?? '1.0.0') @@ -335,7 +349,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { slug, version, versionId: skill.versionId ?? 1, - fingerprint: skill.fingerprint ?? 'deadbeef', + fingerprint: resolveSkillFingerprint(skill), downloadUrl: buildDownloadUrl(baseUrl, namespace, slug, version) } }) diff --git a/cli/test/integration/install-command.test.ts b/cli/test/integration/install-command.test.ts index 27b0623c..9687e60a 100644 --- a/cli/test/integration/install-command.test.ts +++ b/cli/test/integration/install-command.test.ts @@ -42,7 +42,6 @@ describe('install command — P0', () => { slug: 'pdf-parser', version: '1.0.0', versionId: 1, - fingerprint: 'abc123', zipBytes: makeSkillZip() } ] diff --git a/cli/test/integration/sync-command.test.ts b/cli/test/integration/sync-command.test.ts index 607b2d5a..9978cc40 100644 --- a/cli/test/integration/sync-command.test.ts +++ b/cli/test/integration/sync-command.test.ts @@ -127,6 +127,230 @@ describe('sync command', () => { } }) + test('reports a newer remote version as update-available even when content is unchanged', async () => { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const fixture = makeSkill('---\nname: demo\ndescription: Demo\nversion: 1.0.0\n---\n') + const skill: FakeSkill = { + namespace: 'team-a', + slug: 'demo', + version: '1.0.0', + versionId: 1, + ...fixture + } + const registry = await startFakeRegistry({ token: 'token', skills: [skill] }) + + try { + await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token' + ], { HOME: env.home }, { cwd: env.cwd }) + skill.version = '1.1.0' + skill.versionId = 2 + + const status = await runCli([ + 'sync', 'status', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + expect(JSON.parse(status.stdout).items[0]).toMatchObject({ + status: 'update-available', + localVersion: '1.0.0', + remoteVersion: '1.1.0' + }) + } finally { + registry.stop() + } + }) + + test('blocks same-version remote content drift even with force', async () => { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const original = makeSkill('# original\n') + const changed = makeSkill('# changed without a version bump\n') + const skill: FakeSkill = { + namespace: 'team-a', + slug: 'demo', + version: '1.0.0', + versionId: 1, + ...original + } + const registry = await startFakeRegistry({ token: 'token', skills: [skill] }) + + try { + await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token' + ], { HOME: env.home }, { cwd: env.cwd }) + skill.fingerprint = changed.fingerprint + skill.zipBytes = changed.zipBytes + + const status = await runCli([ + 'sync', 'status', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + expect(JSON.parse(status.stdout).items[0]).toMatchObject({ + status: 'blocked', + reason: 'remote content changed without a newer version; use explicit install after verifying the release' + }) + + const checked = await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--check', + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + expect(checked.exitCode).toBe(6) + expect(JSON.parse(checked.stdout)).toMatchObject({ ok: false, check: true }) + + const pulled = await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + expect(pulled.exitCode).toBe(6) + expect(await readFile(join(skillsDir, 'demo', 'SKILL.md'), 'utf8')).toBe('# original\n') + } finally { + registry.stop() + } + }) + + test('blocks automatic downgrade even when remote content is unchanged', async () => { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const fixture = makeSkill('# stable content\n') + const skill: FakeSkill = { + namespace: 'team-a', + slug: 'demo', + version: '2.0.0', + versionId: 2, + ...fixture + } + const registry = await startFakeRegistry({ token: 'token', skills: [skill] }) + + try { + await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token' + ], { HOME: env.home }, { cwd: env.cwd }) + skill.version = '1.0.0' + skill.versionId = 1 + + const pulled = await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + expect(pulled.exitCode).toBe(6) + expect(JSON.parse(pulled.stdout).entries[0]).toMatchObject({ + status: 'blocked', + localVersion: '2.0.0', + remoteVersion: '1.0.0', + reason: 'remote version is older than the installed version; local files were kept' + }) + expect(await readFile(join(skillsDir, 'demo', 'SKILL.md'), 'utf8')).toBe('# stable content\n') + } finally { + registry.stop() + } + }) + + test('blocks sync when local and remote versions cannot be ordered', async () => { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const fixture = makeSkill('# stable content\n') + const skill: FakeSkill = { + namespace: 'team-a', + slug: 'demo', + version: 'release-a', + versionId: 1, + ...fixture + } + const registry = await startFakeRegistry({ token: 'token', skills: [skill] }) + + try { + await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token' + ], { HOME: env.home }, { cwd: env.cwd }) + skill.version = 'release-b' + skill.versionId = 2 + + const pulled = await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + expect(pulled.exitCode).toBe(6) + expect(JSON.parse(pulled.stdout).entries[0]).toMatchObject({ + status: 'blocked', + localVersion: 'release-a', + remoteVersion: 'release-b', + reason: 'cannot determine version order; use explicit install after verifying the release' + }) + } finally { + registry.stop() + } + }) + + test('hard remote guards cannot be bypassed by local changes and force', async () => { + const original = makeSkill('# original\n') + const variants = [ + { + name: 'downgrade', + initialVersion: '2.0.0', + remoteVersion: '1.0.0', + remote: original, + reason: 'remote version is older than the installed version; local files were kept' + }, + { + name: 'same-version drift', + initialVersion: '1.0.0', + remoteVersion: '1.0.0', + remote: makeSkill('# changed without a version bump\n'), + reason: 'remote content changed without a newer version; use explicit install after verifying the release' + }, + { + name: 'unknown version order', + initialVersion: 'release-a', + remoteVersion: 'release-b', + remote: original, + reason: 'cannot determine version order; use explicit install after verifying the release' + } + ] + + for (const variant of variants) { + const env = await createTempHome() + const skillsDir = join(env.cwd, 'team-skills') + const skill: FakeSkill = { + namespace: 'team-a', + slug: 'demo', + version: variant.initialVersion, + versionId: 1, + ...original + } + const registry = await startFakeRegistry({ token: 'token', skills: [skill] }) + try { + await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, + '--registry', registry.url, '--token', 'token' + ], { HOME: env.home }, { cwd: env.cwd }) + await writeFile(join(skillsDir, 'demo', 'SKILL.md'), `# local edit before ${variant.name}\n`) + skill.version = variant.remoteVersion + skill.versionId = 2 + skill.fingerprint = variant.remote.fingerprint + skill.zipBytes = variant.remote.zipBytes + + const pulled = await runCli([ + 'sync', 'pull', '--namespace', 'team-a', '--dir', skillsDir, '--force', + '--registry', registry.url, '--token', 'token', '--json' + ], { HOME: env.home }, { cwd: env.cwd }) + const output = JSON.parse(pulled.stdout) + expect(pulled.exitCode, variant.name).toBe(6) + expect(output.entries[0], variant.name).toMatchObject({ status: 'blocked', reason: variant.reason }) + expect(output.entries[0].changedFiles, variant.name).toEqual(['SKILL.md']) + expect(output.actions, variant.name).toEqual([]) + expect(await readFile(join(skillsDir, 'demo', 'SKILL.md'), 'utf8')) + .toBe(`# local edit before ${variant.name}\n`) + } finally { + registry.stop() + } + } + }) + test('prune removes only unchanged managed orphan skills', async () => { const env = await createTempHome() const skillsDir = join(env.cwd, 'team-skills') diff --git a/cli/test/integration/upgrade-command.test.ts b/cli/test/integration/upgrade-command.test.ts index 3f6ae2ed..8df61ff5 100644 --- a/cli/test/integration/upgrade-command.test.ts +++ b/cli/test/integration/upgrade-command.test.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto' import { access, mkdir, readFile, realpath, rm, writeFile } from 'node:fs/promises' import { isAbsolute, join } from 'node:path' import { afterEach, describe, expect, test } from 'bun:test' @@ -16,8 +17,17 @@ afterEach(() => { registries = [] }) +function makeSkill(content: string): { fingerprint: string; zipBytes: Uint8Array } { + const bytes = strToU8(content) + const fileHash = createHash('sha256').update(bytes).digest('hex') + return { + fingerprint: `sha256:${createHash('sha256').update(`SKILL.md:${fileHash}\n`).digest('hex')}`, + zipBytes: zipSync({ 'SKILL.md': bytes }) + } +} + function makeSkillZip(content: string): Uint8Array { - return zipSync({ 'SKILL.md': strToU8(content) }) + return makeSkill(content).zipBytes } async function exists(path: string): Promise { @@ -37,8 +47,7 @@ describe('upgrade command', () => { slug: 'skillhub-registry', version: '1.0.0', versionId: 1, - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -52,8 +61,7 @@ describe('upgrade command', () => { skill.version = '1.1.0' skill.versionId = 2 - skill.fingerprint = 'fp-v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const inventoryPath = join(env.home, '.skillhub', 'inventory.json') const metadataPath = join(rootDir, 'skillhub-registry', '.skillhub', 'metadata.json') @@ -92,10 +100,15 @@ describe('upgrade command', () => { expect(registry.received.downloads).toBe(2) const metadata = JSON.parse(await readFile(metadataPath, 'utf-8')) - expect(metadata).toMatchObject({ schemaVersion: 1, version: '1.1.0', versionId: 2, fingerprint: 'fp-v2' }) + expect(metadata).toMatchObject({ + schemaVersion: 1, + version: '1.1.0', + versionId: 2, + fingerprint: makeSkill('# v2').fingerprint + }) expect(Object.keys(metadata.files)).toContain('SKILL.md') const inventory = JSON.parse(await readFile(inventoryPath, 'utf-8')) - expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: 'fp-v2' }) + expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: makeSkill('# v2').fingerprint }) }) test('local changes block by default and --force replaces only the same source', async () => { @@ -104,8 +117,7 @@ describe('upgrade command', () => { namespace: 'team', slug: 'code-review', version: '1.0.0', - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -118,8 +130,7 @@ describe('upgrade command', () => { await writeFile(join(rootDir, 'code-review', 'SKILL.md'), '# locally edited') skill.version = '1.1.0' - skill.fingerprint = 'fp-v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const blocked = await runCli([ 'upgrade', '@team/code-review', '--registry', registry.url, '--agent', 'custom', '--check', '--json' @@ -143,8 +154,7 @@ describe('upgrade command', () => { slug: 'late-edit', version: '1.0.0', versionId: 1, - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -157,8 +167,7 @@ describe('upgrade command', () => { skill.version = '1.1.0' skill.versionId = 2 - skill.fingerprint = 'fp-v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const tokenForRegistry = async () => undefined const plan = await planSkillUpgrades({ coordinates: ['@global/late-edit'], @@ -175,7 +184,7 @@ describe('upgrade command', () => { expect(await readFile(join(rootDir, 'late-edit', 'SKILL.md'), 'utf-8')) .toBe('# edited after planning') const inventory = JSON.parse(await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8')) - expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: 'fp-v1' }) + expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: makeSkill('# v1').fingerprint }) }) test('a target removed after planning is not recreated by upgrade', async () => { @@ -185,8 +194,7 @@ describe('upgrade command', () => { slug: 'removed-late', version: '1.0.0', versionId: 1, - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -199,8 +207,7 @@ describe('upgrade command', () => { skill.version = '1.1.0' skill.versionId = 2 - skill.fingerprint = 'fp-v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const tokenForRegistry = async () => undefined const plan = await planSkillUpgrades({ coordinates: ['@global/removed-late'], @@ -217,7 +224,7 @@ describe('upgrade command', () => { expect(result.items[0]?.reason).toContain('installed target disappeared before upgrade commit') expect(await exists(skillDir)).toBe(false) const inventory = JSON.parse(await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8')) - expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: 'fp-v1' }) + expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: makeSkill('# v1').fingerprint }) }) test('new installs persist absolute targets and legacy relative targets are blocked safely', async () => { @@ -227,8 +234,7 @@ describe('upgrade command', () => { slug: 'portable', version: '1.0.0', versionId: 1, - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -252,8 +258,7 @@ describe('upgrade command', () => { await writeFile(inventoryPath, JSON.stringify(inventory)) skill.version = '1.1.0' skill.versionId = 2 - skill.fingerprint = 'fp-v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const otherCwd = join(env.cwd, 'other') await mkdir(otherCwd, { recursive: true }) @@ -271,8 +276,7 @@ describe('upgrade command', () => { namespace: 'global', slug: 'demo', version: '1.0.0', - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -288,7 +292,7 @@ describe('upgrade command', () => { metadata.namespace = 'another-team' await writeFile(metadataPath, JSON.stringify(metadata)) skill.version = '2.0.0' - skill.fingerprint = 'fp-v2' + skill.fingerprint = makeSkill('# v2').fingerprint const result = await runCli([ 'upgrade', '@global/demo', '--registry', registry.url, '--force', '--json' @@ -300,8 +304,8 @@ describe('upgrade command', () => { test('a bare slug must identify exactly one installed source', async () => { const env = await createTempHome() - const skillA = { namespace: 'team-a', slug: 'demo', version: '1.0.0', fingerprint: 'a', zipBytes: makeSkillZip('# A') } - const skillB = { namespace: 'team-b', slug: 'demo', version: '1.0.0', fingerprint: 'b', zipBytes: makeSkillZip('# B') } + const skillA = { namespace: 'team-a', slug: 'demo', version: '1.0.0', ...makeSkill('# A') } + const skillB = { namespace: 'team-b', slug: 'demo', version: '1.0.0', ...makeSkill('# B') } const registryA = await startFakeRegistry({ skills: [skillA] }) const registryB = await startFakeRegistry({ skills: [skillB] }) registries.push(registryA, registryB) @@ -350,7 +354,7 @@ describe('upgrade command', () => { test('target filters select deterministically and missing matches never install', async () => { const env = await createTempHome() - const skill = { namespace: 'global', slug: 'filtered', version: '1.0.0', fingerprint: 'fp', zipBytes: makeSkillZip('# v1') } + const skill = { namespace: 'global', slug: 'filtered', version: '1.0.0', ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) const rootDir = join(env.cwd, 'skills') @@ -395,8 +399,7 @@ describe('upgrade command', () => { namespace: 'global', slug: 'shared', version: '1.0.0', - fingerprint: 'fp-v1', - zipBytes: makeSkillZip('# v1') + ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -409,8 +412,7 @@ describe('upgrade command', () => { expect(registry.received.downloads).toBe(1) skill.version = '1.1.0' - skill.fingerprint = 'fp-v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const partial = await runCli([ 'upgrade', '@global/shared', '--registry', registry.url, '--agent', 'codex', '--check', '--json' @@ -429,7 +431,7 @@ describe('upgrade command', () => { expect(await readFile(join(env.home, '.codex', 'skills', 'shared', 'SKILL.md'), 'utf-8')).toBe('# v2') expect(await readFile(join(env.home, '.claude', 'skills', 'shared', 'SKILL.md'), 'utf-8')).toBe('# v2') const inventory = JSON.parse(await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8')) - expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: 'fp-v2' }) + expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: makeSkill('# v2').fingerprint }) expect(inventory.items[0].targets).toHaveLength(2) const listed = await runCli(['list', '--json', '--registry', registry.url], { @@ -447,8 +449,7 @@ describe('upgrade command', () => { slug: 'stable', version: '2.0.0', versionId: 2, - fingerprint: 'fp-v2', - zipBytes: makeSkillZip('# v2') + ...makeSkill('# v2') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) @@ -461,8 +462,7 @@ describe('upgrade command', () => { skill.version = '1.0.0' skill.versionId = 1 - skill.fingerprint = 'fp-v1' - skill.zipBytes = makeSkillZip('# v1') + Object.assign(skill, makeSkill('# v1')) const result = await runCli([ 'upgrade', '@global/stable', '--registry', registry.url, '--force', '--json' @@ -476,7 +476,7 @@ describe('upgrade command', () => { test('keeps local files when resolve is unavailable or same-version content drifts', async () => { const env = await createTempHome() const failures: { resolve?: 'server_error' } = {} - const skill = { namespace: 'global', slug: 'resilient', version: '1.0.0', fingerprint: 'fp-v1', zipBytes: makeSkillZip('# v1') } + const skill = { namespace: 'global', slug: 'resilient', version: '1.0.0', ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill], failures }) registries.push(registry) const rootDir = join(env.cwd, 'skills') @@ -495,8 +495,7 @@ describe('upgrade command', () => { expect(await readFile(join(rootDir, 'resilient', 'SKILL.md'), 'utf-8')).toBe('# v1') delete failures.resolve - skill.fingerprint = 'fp-drift' - skill.zipBytes = makeSkillZip('# changed without version bump') + Object.assign(skill, makeSkill('# changed without version bump')) const drifted = await runCli([ 'upgrade', '@global/resilient', '--registry', registry.url, '--force', '--json' ], { HOME: env.home, USERPROFILE: env.home }) @@ -508,8 +507,8 @@ describe('upgrade command', () => { test('a blocked batch reports a plan and does not claim successful writes', async () => { const env = await createTempHome() - const first = { namespace: 'global', slug: 'first', version: '1.0.0', fingerprint: 'first-v1', zipBytes: makeSkillZip('# first v1') } - const second = { namespace: 'global', slug: 'second', version: '1.0.0', fingerprint: 'second-v1', zipBytes: makeSkillZip('# second v1') } + const first = { namespace: 'global', slug: 'first', version: '1.0.0', ...makeSkill('# first v1') } + const second = { namespace: 'global', slug: 'second', version: '1.0.0', ...makeSkill('# second v1') } const registry = await startFakeRegistry({ skills: [first, second] }) registries.push(registry) const rootDir = join(env.cwd, 'skills') @@ -522,11 +521,9 @@ describe('upgrade command', () => { } first.version = '1.1.0' - first.fingerprint = 'first-v2' - first.zipBytes = makeSkillZip('# first v2') + Object.assign(first, makeSkill('# first v2')) second.version = '1.1.0' - second.fingerprint = 'second-v2' - second.zipBytes = makeSkillZip('# second v2') + Object.assign(second, makeSkill('# second v2')) await writeFile(join(rootDir, 'second', 'SKILL.md'), '# local change') const result = await runCli([ @@ -541,9 +538,9 @@ describe('upgrade command', () => { test('a runtime batch failure reports committed, failed, and unattempted skills', async () => { const env = await createTempHome() - const first = { namespace: 'global', slug: 'first', version: '1.0.0', fingerprint: 'first-v1', zipBytes: makeSkillZip('# first v1') } - const second = { namespace: 'global', slug: 'second', version: '1.0.0', fingerprint: 'second-v1', zipBytes: makeSkillZip('# second v1') } - const third = { namespace: 'global', slug: 'third', version: '1.0.0', fingerprint: 'third-v1', zipBytes: makeSkillZip('# third v1') } + const first = { namespace: 'global', slug: 'first', version: '1.0.0', ...makeSkill('# first v1') } + const second = { namespace: 'global', slug: 'second', version: '1.0.0', ...makeSkill('# second v1') } + const third = { namespace: 'global', slug: 'third', version: '1.0.0', ...makeSkill('# third v1') } const registry = await startFakeRegistry({ skills: [first, second, third] }) registries.push(registry) const rootDir = join(env.cwd, 'skills') @@ -556,14 +553,12 @@ describe('upgrade command', () => { } first.version = '1.1.0' - first.fingerprint = 'first-v2' - first.zipBytes = makeSkillZip('# first v2') + Object.assign(first, makeSkill('# first v2')) second.version = '1.1.0' - second.fingerprint = 'second-v2' + second.fingerprint = makeSkill('# second v2').fingerprint second.zipBytes = strToU8('not a zip archive') third.version = '1.1.0' - third.fingerprint = 'third-v2' - third.zipBytes = makeSkillZip('# third v2') + Object.assign(third, makeSkill('# third v2')) const result = await runCli([ 'upgrade', '@global/first', '@global/second', '@global/third', @@ -586,7 +581,7 @@ describe('upgrade command', () => { test('a committed upgrade keeps success and renders a post-commit warning', async () => { const env = await createTempHome() - const skill = { namespace: 'global', slug: 'warned', version: '1.0.0', fingerprint: 'v1', zipBytes: makeSkillZip('# v1') } + const skill = { namespace: 'global', slug: 'warned', version: '1.0.0', ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) const rootDir = join(env.cwd, 'skills') @@ -596,8 +591,7 @@ describe('upgrade command', () => { USERPROFILE: env.home }) skill.version = '1.1.0' - skill.fingerprint = 'v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const tokenForRegistry = async () => undefined const plan = await planSkillUpgrades({ coordinates: ['@global/warned'], @@ -624,12 +618,12 @@ describe('upgrade command', () => { expect(renderUpgradeResult(plan, result, false)).toContain('[warning: target lock cleanup failed') expect(await readFile(join(rootDir, 'warned', 'SKILL.md'), 'utf-8')).toBe('# v2') const inventory = JSON.parse(await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8')) - expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: 'v2' }) + expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: makeSkill('# v2').fingerprint }) }) test('legacy metadata without a file baseline requires explicit force migration', async () => { const env = await createTempHome() - const skill = { namespace: 'global', slug: 'legacy', version: '1.0.0', fingerprint: 'v1', zipBytes: makeSkillZip('# v1') } + const skill = { namespace: 'global', slug: 'legacy', version: '1.0.0', ...makeSkill('# v1') } const registry = await startFakeRegistry({ skills: [skill] }) registries.push(registry) const rootDir = join(env.cwd, 'skills') @@ -644,8 +638,7 @@ describe('upgrade command', () => { delete metadata.schemaVersion await writeFile(metadataPath, JSON.stringify(metadata)) skill.version = '1.1.0' - skill.fingerprint = 'v2' - skill.zipBytes = makeSkillZip('# v2') + Object.assign(skill, makeSkill('# v2')) const blocked = await runCli([ 'upgrade', '@global/legacy', '--registry', registry.url, '--check', '--json' diff --git a/cli/test/unit/services/install-service.test.ts b/cli/test/unit/services/install-service.test.ts index ed2eef23..a88061e4 100644 --- a/cli/test/unit/services/install-service.test.ts +++ b/cli/test/unit/services/install-service.test.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto' import { access, mkdir, mkdtemp, readFile, readdir, rm, symlink, utimes, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -7,6 +8,7 @@ import { installSkill } from '../../../src/services/install-service' import { planSkillUpgrades } from '../../../src/services/upgrade-service' import { removeLocalSkill } from '../../../src/services/remove-service' import { skillTargetLockPath } from '../../../src/services/skill-target-lock' +import { EXIT } from '../../../src/shared/constants' const originalFetch = globalThis.fetch @@ -19,7 +21,16 @@ async function exists(path: string): Promise { } } -function installFetch(zipEntries: Record): typeof fetch { +function skillFingerprint(zipEntries: Record): string { + const aggregate = createHash('sha256') + for (const [name, content] of Object.entries(zipEntries).sort(([left], [right]) => left.localeCompare(right))) { + const fileHash = createHash('sha256').update(content).digest('hex') + aggregate.update(`${name}:${fileHash}\n`, 'utf8') + } + return `sha256:${aggregate.digest('hex')}` +} + +function installFetch(zipEntries: Record, fingerprint = skillFingerprint(zipEntries)): typeof fetch { const archive = zipSync(Object.fromEntries( Object.entries(zipEntries).map(([name, content]) => [name, new TextEncoder().encode(content)]) )) @@ -27,10 +38,10 @@ function installFetch(zipEntries: Record): typeof fetch { return installFetchWithDownloadResponse(new Response( archive.buffer.slice(archive.byteOffset, archive.byteOffset + archive.byteLength) as ArrayBuffer, { status: 200 } - )) + ), fingerprint) } -function installFetchWithDownloadResponse(downloadResponse: Response): typeof fetch { +function installFetchWithDownloadResponse(downloadResponse: Response, fingerprint = 'fp'): typeof fetch { const fakeFetch = async (input: URL | RequestInfo) => { const path = new URL(String(input)).pathname if (path.endsWith('/resolve')) { @@ -41,7 +52,7 @@ function installFetchWithDownloadResponse(downloadResponse: Response): typeof fe slug: 'demo', version: '1.0.0', versionId: 1, - fingerprint: 'fp', + fingerprint, downloadUrl: '/download' } }) @@ -116,6 +127,49 @@ describe('installSkill', () => { expect(await exists(join(home, '.skillhub', 'inventory.json'))).toBe(false) }) + test('rejects a downloaded fingerprint mismatch before replacing files or inventory', async () => { + globalThis.fetch = installFetch({ 'SKILL.md': '# Unexpected' }, 'sha256:unexpected') + const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-')) + const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-install-root-')) + const skillDir = join(rootDir, 'demo') + const inventoryPath = join(home, '.skillhub', 'inventory.json') + await mkdir(skillDir, { recursive: true }) + await writeFile(join(skillDir, 'SKILL.md'), '# Existing') + await writeManagedMetadata(skillDir) + await mkdir(join(home, '.skillhub'), { recursive: true }) + const inventoryBefore = JSON.stringify({ + items: [{ + registry: 'http://registry.test', + namespace: 'global', + slug: 'demo', + version: '0.1.0', + targets: [{ + agent: 'codex', + rootDir, + installDir: skillDir, + installedAt: '2026-09-01T00:00:00Z' + }] + }] + }) + await writeFile(inventoryPath, inventoryBefore) + + await expect(installSkill({ + registry: 'http://registry.test', + namespace: 'global', + slug: 'demo', + targets: [{ agent: 'codex', rootDir, scope: 'project', source: 'explicit' }], + force: true, + home + })).rejects.toMatchObject({ + exitCode: EXIT.validation, + message: 'downloaded skill fingerprint does not match the resolved release' + }) + + expect(await readFile(join(skillDir, 'SKILL.md'), 'utf-8')).toBe('# Existing') + expect(await readFile(inventoryPath, 'utf-8')).toBe(inventoryBefore) + expect((await readdir(rootDir)).some(name => name.includes('.install-'))).toBe(false) + }) + test('rejects canonical target aliases before writing any installation', async () => { globalThis.fetch = installFetch({ 'SKILL.md': '# Demo' }) const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-')) @@ -252,7 +306,10 @@ describe('installSkill', () => { expect(result.warnings).toEqual(['target lock cleanup failed: simulated release failure']) expect(await readFile(join(skillDir, 'SKILL.md'), 'utf-8')).toBe('# Committed') const inventory = JSON.parse(await readFile(join(home, '.skillhub', 'inventory.json'), 'utf-8')) - expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: 'fp' }) + expect(inventory.items[0]).toMatchObject({ + version: '1.0.0', + fingerprint: skillFingerprint({ 'SKILL.md': '# Committed' }) + }) }) test('force rejects a different namespace at the same install directory', async () => { @@ -383,7 +440,7 @@ describe('installSkill', () => { slug: 'demo', version: '1.0.0', versionId: 1, - fingerprint: 'fp', + fingerprint: skillFingerprint({ 'SKILL.md': '# New' }), downloadUrl: '/download' } }) @@ -451,7 +508,7 @@ describe('installSkill', () => { if (path.endsWith('/resolve')) { return Response.json({ code: 0, data: { namespace: 'global', slug: 'demo', version: '1.0.0', versionId: 1, - fingerprint: 'fp', downloadUrl: '/download' + fingerprint: skillFingerprint({ 'SKILL.md': '# New' }), downloadUrl: '/download' } }) } if (path.endsWith('/download')) {