diff --git a/cli/test/unit/clients/skillhub-client.test.ts b/cli/test/unit/clients/skillhub-client.test.ts index a8df4f87..a4d9fac8 100644 --- a/cli/test/unit/clients/skillhub-client.test.ts +++ b/cli/test/unit/clients/skillhub-client.test.ts @@ -1,5 +1,7 @@ import { describe, expect, test } from 'bun:test' import { SkillHubClient } from '../../../src/clients/skillhub-client' +import { CliError } from '../../../src/shared/errors' +import { EXIT } from '../../../src/shared/constants' describe('SkillHubClient', () => { test('uses the provided multipart file name when publishing', async () => { @@ -22,4 +24,138 @@ describe('SkillHubClient', () => { await expect(client.publish('team', new Blob(['zip'], { type: 'application/zip' }), 'PRIVATE', 'custom-skill.zip')) .resolves.toMatchObject({ slug: 'custom-skill' }) }) + + // --- download() error handling (P0) --- + + test('download() throws auth error on 401', async () => { + const fetchImpl = (async () => new Response(null, { status: 401 })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const err = expect(client.download('ns', 'slug')).rejects + await err.toBeInstanceOf(CliError) + await err.toHaveProperty('message', 'authentication failed') + await err.toHaveProperty('exitCode', EXIT.auth) + }) + + test('download() throws auth error on 403', async () => { + const fetchImpl = (async () => new Response(null, { status: 403 })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const err = expect(client.download('ns', 'slug')).rejects + await err.toBeInstanceOf(CliError) + await err.toHaveProperty('message', 'authentication failed') + await err.toHaveProperty('exitCode', EXIT.auth) + }) + + test('download() throws not-found error on 404', async () => { + const fetchImpl = (async () => new Response(null, { status: 404 })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const err = expect(client.download('ns', 'slug')).rejects + await err.toBeInstanceOf(CliError) + await err.toHaveProperty('message', 'skill or version not found') + await err.toHaveProperty('exitCode', EXIT.generic) + }) + + test('download() throws network error on fetch failure', async () => { + const fetchImpl = (async () => { throw new TypeError('fetch failed') }) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const err = expect(client.download('ns', 'slug')).rejects + await err.toBeInstanceOf(CliError) + await err.toHaveProperty('message', 'registry unreachable') + await err.toHaveProperty('exitCode', EXIT.network) + }) + + // --- whoami() (P1) --- + + test('whoami() returns user data', async () => { + const fetchImpl = (async () => Response.json({ + data: { handle: 'alice', displayName: 'Alice', email: 'a@b.com' } + })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const result = await client.whoami() + expect(result).toEqual({ handle: 'alice', displayName: 'Alice', email: 'a@b.com' }) + }) + + test('whoami() throws on 401', async () => { + const fetchImpl = (async () => new Response(null, { status: 401 })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const err = expect(client.whoami()).rejects + await err.toBeInstanceOf(CliError) + await err.toHaveProperty('message', 'authentication failed') + await err.toHaveProperty('exitCode', EXIT.auth) + }) + + // --- search() (P1) --- + + test('search() returns items', async () => { + const fetchImpl = (async () => Response.json({ + data: { + items: [{ namespace: 'g', slug: 's', latestVersion: '1.0', summary: 'x' }], + total: 1, + limit: 20 + } + })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const result = await client.search('test', 20) + expect(result.items).toHaveLength(1) + expect(result.items[0]).toEqual({ namespace: 'g', slug: 's', latestVersion: '1.0', summary: 'x' }) + expect(result.total).toBe(1) + expect(result.limit).toBe(20) + }) + + test('search() returns empty results', async () => { + const fetchImpl = (async () => Response.json({ + data: { items: [], total: 0, limit: 20 } + })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const result = await client.search('nothing', 20) + expect(result.items).toHaveLength(0) + expect(result.total).toBe(0) + }) + + // --- resolve() (P1) --- + + test('resolve() without version omits query param', async () => { + let capturedUrl = '' + const fetchImpl = (async (input: URL | RequestInfo) => { + capturedUrl = String(input) + return Response.json({ + data: { namespace: 'ns', slug: 'sk', version: '1.0.0', versionId: 1, fingerprint: 'abc', downloadUrl: '/dl' } + }) + }) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + await client.resolve('ns', 'sk') + expect(capturedUrl).not.toContain('?version=') + }) + + test('resolve() with version includes query param', async () => { + let capturedUrl = '' + const fetchImpl = (async (input: URL | RequestInfo) => { + capturedUrl = String(input) + return Response.json({ + data: { namespace: 'ns', slug: 'sk', version: '2.0.0', versionId: 2, fingerprint: 'def', downloadUrl: '/dl' } + }) + }) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + await client.resolve('ns', 'sk', '2.0.0') + expect(capturedUrl).toContain('?version=2.0.0') + }) + + // --- deleteRemote() (P1) --- + + test('deleteRemote() returns result on success', async () => { + const fetchImpl = (async () => Response.json({ + data: { ok: true, scope: 'remote', action: 'delete', namespace: 'global', slug: 'demo' } + })) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const result = await client.deleteRemote('global', 'demo') + expect(result).toEqual({ ok: true, scope: 'remote', action: 'delete', namespace: 'global', slug: 'demo' }) + }) + + test('deleteRemote() throws on network error', async () => { + const fetchImpl = (async () => { throw new TypeError('fetch failed') }) as unknown as typeof fetch + const client = new SkillHubClient('http://registry.test', 'token', fetchImpl) + const err = expect(client.deleteRemote('global', 'demo')).rejects + await err.toBeInstanceOf(CliError) + await err.toHaveProperty('message', 'registry unreachable') + await err.toHaveProperty('exitCode', EXIT.network) + }) }) diff --git a/cli/test/unit/platform/archive.test.ts b/cli/test/unit/platform/archive.test.ts index 1813af16..a39c3c03 100644 --- a/cli/test/unit/platform/archive.test.ts +++ b/cli/test/unit/platform/archive.test.ts @@ -31,4 +31,29 @@ describe('archive helpers', () => { await expect(extractZip(unsafe.buffer as ArrayBuffer, target)).rejects.toThrow('unsafe zip entry path') }) + + test('rejects zip entries with absolute paths', async () => { + const target = await mkdtemp(join(tmpdir(), 'skillhub-archive-abs-')) + const unsafe = zipSync({ '/etc/passwd': new TextEncoder().encode('bad') }) + + await expect(extractZip(unsafe.buffer as ArrayBuffer, target)).rejects.toThrow('unsafe zip entry path') + }) + + test('rejects zip entries with multi-level ../ traversal', async () => { + const target = await mkdtemp(join(tmpdir(), 'skillhub-archive-multi-')) + const unsafe = zipSync({ 'foo/../../escape.txt': new TextEncoder().encode('escaped') }) + + await expect(extractZip(unsafe.buffer as ArrayBuffer, target)).rejects.toThrow('unsafe zip entry path') + }) + + test('handles empty zip gracefully', async () => { + const target = await mkdtemp(join(tmpdir(), 'skillhub-archive-empty-')) + const empty = zipSync({}) + + await extractZip(empty.buffer as ArrayBuffer, target) + + const { readdir } = await import('node:fs/promises') + const entries = await readdir(target) + expect(entries).toEqual([]) + }) }) diff --git a/cli/test/unit/platform/package-manager.test.ts b/cli/test/unit/platform/package-manager.test.ts new file mode 100644 index 00000000..89c37d60 --- /dev/null +++ b/cli/test/unit/platform/package-manager.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, test } from 'bun:test' +import { detectInstallMode } from '../../../src/platform/package-manager' + +describe('detectInstallMode', () => { + test('detects npx from _npx/ in argv', () => { + const result = detectInstallMode(['/usr/bin/bun', '/tmp/_npx/abc/skillhub'], {}) + expect(result).toBe('npx') + }) + + test('detects npm-global from npm_config_prefix', () => { + const result = detectInstallMode( + ['/usr/bin/bun', '/usr/local/lib/node_modules/.bin/skillhub'], + { npm_config_prefix: '/usr/local' } + ) + expect(result).toBe('npm-global') + }) + + test('detects bun-global from BUN_INSTALL', () => { + const result = detectInstallMode( + ['/usr/bin/bun', '/home/user/.bun/bin/skillhub'], + { BUN_INSTALL: '/home/user/.bun' } + ) + expect(result).toBe('bun-global') + }) + + test('returns unknown as fallback', () => { + const result = detectInstallMode(['/usr/bin/bun', '/random/path/skillhub'], {}) + expect(result).toBe('unknown') + }) +}) diff --git a/cli/test/unit/platform/paths.test.ts b/cli/test/unit/platform/paths.test.ts new file mode 100644 index 00000000..16976950 --- /dev/null +++ b/cli/test/unit/platform/paths.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, test } from 'bun:test' +import { stat } from 'node:fs/promises' +import { mkdtemp, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { applyCredentialPermissions, userStateDir } from '../../../src/platform/paths' + +describe('userStateDir', () => { + test('throws when home is empty string', () => { + expect(() => userStateDir('')).toThrow('Cannot resolve user home directory') + }) +}) + +describe('applyCredentialPermissions', () => { + test('sets 0o600 on unix', async () => { + const tempDir = await mkdtemp(join(tmpdir(), 'skillhub-test-')) + const tempFile = join(tempDir, 'credential.json') + await writeFile(tempFile, '{}') + + await applyCredentialPermissions(tempFile) + + if (process.platform !== 'win32') { + const stats = await stat(tempFile) + expect(stats.mode & 0o777).toBe(0o600) + } + }) +}) diff --git a/cli/test/unit/platform/updater.test.ts b/cli/test/unit/platform/updater.test.ts new file mode 100644 index 00000000..7471a458 --- /dev/null +++ b/cli/test/unit/platform/updater.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, test } from 'bun:test' +import { runUpdateCommand } from '../../../src/platform/updater' + +describe('runUpdateCommand', () => { + test('succeeds with node --version', async () => { + const result = await runUpdateCommand('node --version') + expect(result.success).toBe(true) + expect(result.output).toMatch(/v\d+\.\d+/) + }) + + test('fails on invalid node flag', async () => { + const result = await runUpdateCommand('node --invalid-flag-xyz-12345') + expect(result.success).toBe(false) + }) + + test('returns failure for empty command', async () => { + const result = await runUpdateCommand('') + expect(result.success).toBe(false) + expect(result.output).toBe('empty update command') + }) + + test('handles spawn error for missing binary', async () => { + const result = await runUpdateCommand('nonexistent-binary-xyz-12345') + expect(result.success).toBe(false) + }) +}) diff --git a/cli/test/unit/services/remove-service.test.ts b/cli/test/unit/services/remove-service.test.ts index 029915a4..d1a86782 100644 --- a/cli/test/unit/services/remove-service.test.ts +++ b/cli/test/unit/services/remove-service.test.ts @@ -50,4 +50,25 @@ describe('removeLocalSkill', () => { expect(await exists(teamDir)).toBe(false) expect((await store.read()).items).toEqual([]) }) + + test('throws on path traversal in installDir', async () => { + const home = await mkdtemp(join(tmpdir(), 'skillhub-remove-traversal-')) + + const store = new InventoryStore(home) + await store.write({ + items: [ + { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'evil', + version: '1.0.0', + targets: [{ agent: 'codex', rootDir: '/safe/root', installDir: '/etc/passwd', installedAt: '2026-04-20T00:00:00Z' }] + } + ] + }) + + await expect( + removeLocalSkill({ registry: 'https://skill.xfyun.cn', slug: 'evil', home }) + ).rejects.toThrow('unsafe remove path') + }) }) diff --git a/cli/test/unit/shared/output.test.ts b/cli/test/unit/shared/output.test.ts index 146f09e0..8d051172 100644 --- a/cli/test/unit/shared/output.test.ts +++ b/cli/test/unit/shared/output.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'bun:test' import { CliError } from '../../../src/shared/errors' -import { renderError } from '../../../src/shared/output' +import { printResult, renderError } from '../../../src/shared/output' describe('renderError', () => { test('renders stable json error to stderr payload', () => { @@ -25,3 +25,18 @@ describe('renderError', () => { ].join('\n')) }) }) + +describe('printResult', () => { + test('returns string as-is in non-JSON mode', () => { + expect(printResult('hello', false)).toBe('hello') + }) + + test('wraps string in JSON envelope', () => { + expect(printResult('hello', true)).toBe(JSON.stringify({ ok: true, message: 'hello' })) + }) + + test('formats object in non-JSON mode', () => { + const result = printResult({ ok: true, handle: 'me', email: 'a@b' }, false) + expect(result).toBe('handle: me\nemail: a@b') + }) +}) diff --git a/cli/test/unit/stores/config-store.test.ts b/cli/test/unit/stores/config-store.test.ts new file mode 100644 index 00000000..15e27be8 --- /dev/null +++ b/cli/test/unit/stores/config-store.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from 'bun:test' +import { mkdtemp } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { ConfigStore } from '../../../src/stores/config-store' + +function makeTempHome() { + return mkdtemp(join(tmpdir(), 'skillhub-store-test-')) +} + +describe('ConfigStore', () => { + test('read() returns empty object when file missing', async () => { + const home = await makeTempHome() + const store = new ConfigStore(home) + + const config = await store.read() + expect(config).toEqual({}) + }) + + test('write() then read() round-trips', async () => { + const home = await makeTempHome() + const store = new ConfigStore(home) + + const original = { registry: 'https://example.com', defaultAgent: 'codex' } + await store.write(original) + + const config = await store.read() + expect(config).toEqual(original) + }) + + test('setRegistry() merges into existing config', async () => { + const home = await makeTempHome() + const store = new ConfigStore(home) + + // Write initial config with defaultAgent only + await store.write({ defaultAgent: 'codex' }) + + // setRegistry should merge, not overwrite + await store.setRegistry('https://new.com') + + const config = await store.read() + expect(config.registry).toBe('https://new.com') + expect(config.defaultAgent).toBe('codex') + }) +}) diff --git a/cli/test/unit/stores/inventory-store.test.ts b/cli/test/unit/stores/inventory-store.test.ts new file mode 100644 index 00000000..02c3f33d --- /dev/null +++ b/cli/test/unit/stores/inventory-store.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from 'bun:test' +import { mkdir, writeFile } from 'node:fs/promises' +import { mkdtemp } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join, dirname } from 'node:path' +import { InventoryStore } from '../../../src/stores/inventory-store' + +function makeTempHome() { + return mkdtemp(join(tmpdir(), 'skillhub-store-test-')) +} + +function makeTarget(slug: string) { + return { + agent: 'claude', + rootDir: `/projects/${slug}`, + installDir: `/projects/${slug}/.skillhub/skills/${slug}`, + installedAt: new Date().toISOString(), + } +} + +describe('InventoryStore', () => { + test('5 sequential upsertTarget calls all persist', async () => { + const home = await makeTempHome() + const store = new InventoryStore(home) + + for (let i = 0; i < 5; i++) { + await store.upsertTarget( + 'https://skill.xfyun.cn', + 'global', + `skill-${i}`, + '1.0.0', + makeTarget(`skill-${i}`), + ) + } + + const inventory = await store.read() + expect(inventory.items).toHaveLength(5) + }) + + test('recovers from stale lock file', async () => { + const home = await makeTempHome() + const store = new InventoryStore(home) + + // Ensure the directory for the lock file exists + await mkdir(dirname(store.path), { recursive: true }) + + // Write a stale lock: dead PID + old timestamp + const lockPath = `${store.path}.lock` + await writeFile( + lockPath, + JSON.stringify({ pid: 999999, timestamp: Date.now() - 60000 }), + ) + + // upsertTarget should recover from the stale lock and succeed + await store.upsertTarget( + 'https://skill.xfyun.cn', + 'global', + 'test-skill', + '1.0.0', + makeTarget('test-skill'), + ) + + const inventory = await store.read() + expect(inventory.items).toHaveLength(1) + expect(inventory.items[0].slug).toBe('test-skill') + }) + + test('removeTarget returns false when item not found', async () => { + const home = await makeTempHome() + const store = new InventoryStore(home) + + const result = await store.removeTarget( + 'https://skill.xfyun.cn', + 'global', + 'nonexistent', + '/some/path', + ) + expect(result).toBe(false) + }) + + test('removeTargetsByInstallDir returns 0 when no matches', async () => { + const home = await makeTempHome() + const store = new InventoryStore(home) + + // Seed one item so the inventory file exists + await store.upsertTarget( + 'https://skill.xfyun.cn', + 'global', + 'existing-skill', + '1.0.0', + makeTarget('existing-skill'), + ) + + const removed = await store.removeTargetsByInstallDir('/nonexistent/path') + expect(removed).toBe(0) + + // Original item should still be there + const inventory = await store.read() + expect(inventory.items).toHaveLength(1) + }) +})