test(cli): add P0/P1/P2 test coverage for security and error paths

Add 36 test cases covering:
- Path traversal and symlink attack prevention in archive extraction
- SkillHubClient error handling (401/403/404/network) for all endpoints
- Inventory store concurrent writes and stale lock recovery
- Config store read/write round-trip
- Platform utilities (package-manager, updater, paths)
- Output formatting (printResult, humanize)

Tests use cross-platform commands (node) instead of shell builtins
for CI compatibility across macOS/Linux/Windows.

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)
+  })
 })

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([])
+  })
 })

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')
+  })
+})

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)
+    }
+  })
+})

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)
+  })
+})

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')
+  })
 })

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')
+  })
+})

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')
+  })
+})

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)
+  })
+})