feat: add Snyk agent-scan for dynamic skill scanning based on toxicskills

packages/clawdhub/src/cli/commands/skills.test.ts

@@ -1,6 +1,6 @@
 /* @vitest-environment node */
 
-import { afterEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest'
 import { ApiRoutes } from '../../schema/index.js'
 import type { GlobalOpts } from '../types'
 
@@ -35,7 +35,7 @@ vi.mock('../ui.js', () => ({
     throw new Error(message)
   },
   formatError: (error: unknown) => (error instanceof Error ? error.message : String(error)),
-  isInteractive: () => false,
+  isInteractive: vi.fn(() => false),
   promptConfirm: vi.fn(async () => false),
 }))
 
@@ -50,12 +50,20 @@ vi.mock('../../skills.js', () => ({
 }))
 
 vi.mock('node:fs/promises', () => ({
+  cp: vi.fn(),
   mkdir: vi.fn(),
+  rename: vi.fn(),
   rm: vi.fn(),
   stat: vi.fn(),
 }))
 
-const { clampLimit, cmdExplore, cmdInstall, cmdUpdate, formatExploreLine } = await import('./skills')
+vi.mock('node:os', () => ({
+  tmpdir: () => '/tmp',
+}))
+
+const { clampLimit, cmdExplore, cmdInstall, cmdUpdate, formatExploreLine } = await import(
+  './skills'
+)
 const {
   extractZipToDir,
   hashSkillFiles,
@@ -65,9 +73,16 @@ const {
   writeLockfile,
   writeSkillOrigin,
 } = await import('../../skills.js')
-const { rm, stat } = await import('node:fs/promises')
+const { cp, rename, rm, stat } = await import('node:fs/promises')
+const { isInteractive, promptConfirm } = await import('../ui.js')
+const { execFileSync } = await import('node:child_process')
+
+vi.mock('node:child_process', () => ({
+  execFileSync: vi.fn(),
+}))
 
 const mockLog = vi.spyOn(console, 'log').mockImplementation(() => {})
+const mockWarn = vi.spyOn(console, 'warn').mockImplementation(() => {})
 
 function makeOpts(): GlobalOpts {
   return {
@@ -83,6 +98,145 @@ afterEach(() => {
   vi.clearAllMocks()
 })
 
+describe('cmdInstall', () => {
+  beforeEach(() => {
+    // Default mocks for a successful installation path
+    mockApiRequest.mockResolvedValue({
+      latestVersion: { version: '1.0.0' },
+      moderation: { isMalwareBlocked: false, isSuspicious: false },
+    })
+    mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3]))
+    vi.mocked(readLockfile).mockResolvedValue({ version: 1, skills: {} })
+    vi.mocked(writeLockfile).mockResolvedValue()
+    vi.mocked(writeSkillOrigin).mockResolvedValue()
+    vi.mocked(extractZipToDir).mockResolvedValue()
+    vi.mocked(stat).mockRejectedValue(new Error('missing')) // Simulate file not existing
+    vi.mocked(rm).mockResolvedValue()
+    vi.mocked(rename).mockResolvedValue()
+    vi.mocked(cp).mockResolvedValue()
+    vi.mocked(execFileSync).mockReturnValue('{}') // Clean scan
+  })
+
+  it('installs a skill successfully when scan finds no violations', async () => {
+    await cmdInstall(makeOpts(), 'test-skill')
+    expect(extractZipToDir).toHaveBeenCalledWith(
+      expect.any(Uint8Array),
+      expect.stringMatching(/\/tmp\/clawhub-install-test-skill-.*/),
+    )
+    expect(execFileSync).toHaveBeenCalledWith(
+      'uvx',
+      [
+        'mcp-scan@latest',
+        '--skills',
+        expect.stringMatching(/\/tmp\/clawhub-install-test-skill-.*/),
+        '--json',
+      ],
+      { encoding: 'utf-8' },
+    )
+    expect(rename).toHaveBeenCalledWith(
+      expect.stringMatching(/\/tmp\/clawhub-install-test-skill-.*/),
+      '/work/skills/test-skill',
+    )
+    expect(mockSpinner.succeed).toHaveBeenCalledWith(
+      'OK. Installed test-skill -> /work/skills/test-skill',
+    )
+  })
+
+  it('installs a skill if user accepts after a violation warning', async () => {
+    const violation = { issues: [{ code: 'W011', message: 'Third-party content' }] }
+    vi.mocked(execFileSync).mockReturnValue(JSON.stringify({ '/path/to/skill': violation }))
+    vi.mocked(isInteractive).mockReturnValue(true)
+    vi.mocked(promptConfirm).mockResolvedValue(true)
+
+    await cmdInstall(makeOpts(), 'test-skill')
+
+    expect(mockLog).toHaveBeenCalledWith(expect.stringContaining('⚠️  Warning'))
+    expect(promptConfirm).toHaveBeenCalledWith('Install anyway?')
+    expect(rename).toHaveBeenCalled()
+    expect(mockSpinner.succeed).toHaveBeenCalledWith(
+      'OK. Installed test-skill -> /work/skills/test-skill',
+    )
+  })
+
+  it('aborts installation if user rejects after a violation warning', async () => {
+    const violation = { issues: [{ code: 'W011', message: 'Third-party content' }] }
+    vi.mocked(execFileSync).mockReturnValue(JSON.stringify({ '/path/to/skill': violation }))
+    vi.mocked(isInteractive).mockReturnValue(true)
+    vi.mocked(promptConfirm).mockResolvedValue(false)
+
+    await expect(cmdInstall(makeOpts(), 'test-skill')).rejects.toThrow('Installation cancelled')
+
+    expect(promptConfirm).toHaveBeenCalledWith('Install anyway?')
+    expect(rename).not.toHaveBeenCalled()
+    expect(mockSpinner.succeed).not.toHaveBeenCalled()
+  })
+
+  it('skips scan and continues installation if scanner fails', async () => {
+    vi.mocked(execFileSync).mockImplementation(() => {
+      throw new Error('Rate limit exceeded')
+    })
+
+    await cmdInstall(makeOpts(), 'test-skill')
+
+    expect(mockWarn).toHaveBeenCalledWith(
+      expect.stringContaining('⚠️  Skipping Snyk Agent Scan: Rate limit exceeded'),
+    )
+    expect(rename).toHaveBeenCalled()
+    expect(mockSpinner.succeed).toHaveBeenCalledWith(
+      'OK. Installed test-skill -> /work/skills/test-skill',
+    )
+  })
+
+  it('falls back to copy when rename fails with EXDEV', async () => {
+    const exdevError = new Error('Cross-device link')
+    ;(exdevError as any).code = 'EXDEV'
+    vi.mocked(rename).mockRejectedValueOnce(exdevError)
+
+    await cmdInstall(makeOpts(), 'test-skill')
+
+    expect(rename).toHaveBeenCalled()
+    expect(cp).toHaveBeenCalledWith(
+      expect.stringMatching(/\/tmp\/clawhub-install-test-skill-.*/),
+      '/work/skills/test-skill',
+      { recursive: true },
+    )
+    expect(mockSpinner.succeed).toHaveBeenCalledWith(
+      'OK. Installed test-skill -> /work/skills/test-skill',
+    )
+  })
+
+  it('fails installation if rename fails with non-EXDEV error', async () => {
+    const otherError = new Error('Permission denied')
+    vi.mocked(rename).mockRejectedValueOnce(otherError)
+
+    await expect(cmdInstall(makeOpts(), 'test-skill')).rejects.toThrow('Permission denied')
+
+    expect(rename).toHaveBeenCalled()
+    expect(cp).not.toHaveBeenCalled()
+    expect(mockSpinner.succeed).not.toHaveBeenCalled()
+  })
+
+  it('passes optional auth token to API + download requests', async () => {
+    mockGetOptionalAuthToken.mockResolvedValue('tkn')
+    // Re-setup mocks as they might be overwritten by beforeEach if they clash,
+    // but here we are specific about return values.
+    mockApiRequest.mockResolvedValue({
+      skill: { slug: 'demo', displayName: 'Demo', summary: null, tags: {}, stats: {}, createdAt: 0, updatedAt: 0 },
+      latestVersion: { version: '1.0.0' },
+      owner: null,
+      moderation: null,
+    })
+    mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3]))
+
+    await cmdInstall(makeOpts(), 'demo')
+
+    const [, requestArgs] = mockApiRequest.mock.calls[0] ?? []
+    expect(requestArgs?.token).toBe('tkn')
+    const [, zipArgs] = mockDownloadZip.mock.calls[0] ?? []
+    expect(zipArgs?.token).toBe('tkn')
+  })
+})
+
 describe('explore helpers', () => {
   it('clamps explore limits and handles non-finite values', () => {
     expect(clampLimit(-5)).toBe(1)
@@ -194,29 +348,3 @@ describe('cmdUpdate', () => {
     expect(args?.url).toBeUndefined()
   })
 })
-
-describe('cmdInstall', () => {
-  it('passes optional auth token to API + download requests', async () => {
-    mockGetOptionalAuthToken.mockResolvedValue('tkn')
-    mockApiRequest.mockResolvedValue({
-      skill: { slug: 'demo', displayName: 'Demo', summary: null, tags: {}, stats: {}, createdAt: 0, updatedAt: 0 },
-      latestVersion: { version: '1.0.0' },
-      owner: null,
-      moderation: null,
-    })
-    mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3]))
-    vi.mocked(readLockfile).mockResolvedValue({ version: 1, skills: {} })
-    vi.mocked(writeLockfile).mockResolvedValue()
-    vi.mocked(writeSkillOrigin).mockResolvedValue()
-    vi.mocked(extractZipToDir).mockResolvedValue()
-    vi.mocked(stat).mockRejectedValue(new Error('missing'))
-    vi.mocked(rm).mockResolvedValue()
-
-    await cmdInstall(makeOpts(), 'demo')
-
-    const [, requestArgs] = mockApiRequest.mock.calls[0] ?? []
-    expect(requestArgs?.token).toBe('tkn')
-    const [, zipArgs] = mockDownloadZip.mock.calls[0] ?? []
-    expect(zipArgs?.token).toBe('tkn')
-  })
-})

packages/clawdhub/src/cli/commands/skills.ts

@@ -1,4 +1,6 @@
-import { mkdir, rm, stat } from 'node:fs/promises'
+import { execFileSync } from 'node:child_process'
+import { cp, mkdir, rename, rm, stat } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import semver from 'semver'
 import { apiRequest, downloadZip } from '../../http.js'
@@ -53,6 +55,55 @@ export async function cmdSearch(opts: GlobalOpts, query: string, limit?: number)
   }
 }
 
+// mcp-scan types
+interface ScanIssue {
+  code: string
+  message: string
+}
+
+interface ServerResult {
+  error?: { message: string }
+}
+
+interface PathResult {
+  error?: { message: string }
+  servers?: ServerResult[]
+  issues: ScanIssue[]
+}
+
+type ScanResult = Record<string, PathResult>
+
+async function checkSkillSecurity(targetPath: string): Promise<string[]> {
+  const scanOutput = execFileSync('uvx', ['mcp-scan@latest', '--skills', targetPath, '--json'], {
+    encoding: 'utf-8',
+  })
+  const scanResult = JSON.parse(scanOutput) as ScanResult
+
+  // Internal codes to ignore
+  const IGNORE_CODES = ['W003', 'W004', 'W005', 'W006', 'X002']
+  const violations: string[] = []
+
+  for (const [path, result] of Object.entries(scanResult)) {
+    // 1. Check for top-level execution failures
+    if (result.error) {
+      throw new Error(`Scan failed for ${path}: ${result.error.message}`)
+    }
+
+    // 2. Check for server-level startup failures
+    const startupError = result.servers?.find((s) => s.error)?.error
+    if (startupError) {
+      throw new Error(`Server failed to start: ${startupError.message}`)
+    }
+
+    // 3. Filter for real policy violations (Errors or Warnings)
+    const filteredViolations = result.issues.filter((issue) => !IGNORE_CODES.includes(issue.code))
+    if (filteredViolations.length > 0) {
+      violations.push(...filteredViolations.map((v) => `[${v.code}] ${v.message}`))
+    }
+  }
+  return violations
+}
+
 export async function cmdInstall(
   opts: GlobalOpts,
   slug: string,
@@ -110,7 +161,54 @@ export async function cmdInstall(
 
     spinner.text = `Downloading ${trimmed}@${resolvedVersion}`
     const zip = await downloadZip(registry, { slug: trimmed, version: resolvedVersion, token })
-    await extractZipToDir(zip, target)
+
+    const tempTarget = join(
+      tmpdir(),
+      `clawhub-install-${trimmed}-${Math.random().toString(36).slice(2, 7)}`,
+    )
+    try {
+      await extractZipToDir(zip, tempTarget)
+
+      spinner.text = `Scanning ${trimmed}@${resolvedVersion}`
+      let scanViolations: string[] = []
+      try {
+        scanViolations = await checkSkillSecurity(tempTarget)
+      } catch (error) {
+        spinner.stop()
+        console.warn(`⚠️  Skipping Snyk Agent Scan: ${formatError(error)}`)
+        spinner.start(`Resolving ${trimmed}`)
+      }
+
+      if (scanViolations.length > 0 && !force) {
+        spinner.stop()
+        console.log(
+          `\n⚠️  Warning: "${trimmed}" has security policy violations by Snyk Agent Scan:\n` +
+            scanViolations.map((msg) => `   - ${msg}`).join('\n') +
+            '\n   Review the skill code before use.\n',
+        )
+        if (isInteractive()) {
+          const confirm = await promptConfirm('Install anyway?')
+          if (!confirm) fail('Installation cancelled')
+          spinner.start(`Resolving ${trimmed}`)
+        } else {
+          fail(
+            'Use --force to install skills with security policy violations in non-interactive mode',
+          )
+        }
+      }
+
+      try {
+        await rename(tempTarget, target)
+      } catch (err: any) {
+        if (err.code === 'EXDEV') {
+          await cp(tempTarget, target, { recursive: true })
+        } else {
+          throw err
+        }
+      }
+    } finally {
+      await rm(tempTarget, { recursive: true, force: true })
+    }
 
     await writeSkillOrigin(target, {
       version: 1,