From cc3d16a3a58e93a12b01b476bcecaf818e09114f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Fri, 3 Jul 2026 18:24:58 +0800 Subject: [PATCH] feat: install marketplace skills safely --- bun.lock | 3 + package-lock.json | 7 + package.json | 1 + .../__tests__/skill-market-install.test.ts | 143 ++++++++++++ src/server/services/skillMarket/installer.ts | 211 ++++++++++++++++++ 5 files changed, 365 insertions(+) create mode 100644 src/server/__tests__/skill-market-install.test.ts create mode 100644 src/server/services/skillMarket/installer.ts diff --git a/bun.lock b/bun.lock index f321d88e..8cb5e492 100644 --- a/bun.lock +++ b/bun.lock @@ -31,6 +31,7 @@ "emoji-regex": "^10.6.0", "env-paths": "^4.0.0", "execa": "^9.6.1", + "fflate": "^0.8.3", "figures": "^6.1.0", "fuse.js": "^7.1.0", "get-east-asian-width": "^1.5.0", @@ -852,6 +853,8 @@ "fetch-blob": ["fetch-blob@3.2.0", "https://registry.npmmirror.com/fetch-blob/-/fetch-blob-3.2.0.tgz", { "dependencies": { "node-domexception": "^1.0.0", "web-streams-polyfill": "^3.0.3" } }, "sha512-7yAQpD2UMJzLi1Dqv7qFYnPbaPx7ZfFK6PiIxQ4PfkGPyNyl2Ugx+a/umUonmKqjhM4DnfbMvdX6otXq83soQQ=="], + "fflate": ["fflate@0.8.3", "https://registry.npmmirror.com/fflate/-/fflate-0.8.3.tgz", {}, "sha512-tbZNuJrLwGUp3zshBtdy4W+ORxZuIh8a5ilyIEQDC5rY1f3U20JMry0Ll3WBzU58EZKsEuJFXhb5gwv8CsPvgA=="], + "figures": ["figures@6.1.0", "https://registry.npmmirror.com/figures/-/figures-6.1.0.tgz", { "dependencies": { "is-unicode-supported": "^2.0.0" } }, "sha512-d+l3qxjSesT4V7v2fh+QnmFnUWv9lSpjarhShNTgBOfA0ttejbQUAlHLitbjkoRiDulW0OPoQPYIGhIC8ohejg=="], "finalhandler": ["finalhandler@2.1.1", "https://registry.npmmirror.com/finalhandler/-/finalhandler-2.1.1.tgz", { "dependencies": { "debug": "^4.4.0", "encodeurl": "^2.0.0", "escape-html": "^1.0.3", "on-finished": "^2.4.1", "parseurl": "^1.3.3", "statuses": "^2.0.1" } }, "sha512-S8KoZgRZN+a5rNwqTxlZZePjT/4cnm0ROV70LedRHZ0p8u9fRID0hJUZQpkKLzro8LfmC8sx23bY6tVNxv8pQA=="], diff --git a/package-lock.json b/package-lock.json index 4752450d..b0a212a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34,6 +34,7 @@ "emoji-regex": "^10.6.0", "env-paths": "^4.0.0", "execa": "^9.6.1", + "fflate": "^0.8.3", "figures": "^6.1.0", "fuse.js": "^7.1.0", "get-east-asian-width": "^1.5.0", @@ -5522,6 +5523,12 @@ "node": "^12.20 || >= 14.13" } }, + "node_modules/fflate": { + "version": "0.8.3", + "resolved": "https://registry.npmmirror.com/fflate/-/fflate-0.8.3.tgz", + "integrity": "sha512-tbZNuJrLwGUp3zshBtdy4W+ORxZuIh8a5ilyIEQDC5rY1f3U20JMry0Ll3WBzU58EZKsEuJFXhb5gwv8CsPvgA==", + "license": "MIT" + }, "node_modules/figures": { "version": "6.1.0", "resolved": "https://registry.npmmirror.com/figures/-/figures-6.1.0.tgz", diff --git a/package.json b/package.json index b6dba98a..00512f16 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "emoji-regex": "^10.6.0", "env-paths": "^4.0.0", "execa": "^9.6.1", + "fflate": "^0.8.3", "figures": "^6.1.0", "fuse.js": "^7.1.0", "get-east-asian-width": "^1.5.0", diff --git a/src/server/__tests__/skill-market-install.test.ts b/src/server/__tests__/skill-market-install.test.ts new file mode 100644 index 00000000..296b1472 --- /dev/null +++ b/src/server/__tests__/skill-market-install.test.ts @@ -0,0 +1,143 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test' +import { zipSync } from 'fflate' +import * as fs from 'node:fs/promises' +import * as os from 'node:os' +import * as path from 'node:path' +import { installUserSkillFromZipBytes } from '../services/skillMarket/installer.js' + +let tmpHome: string +let originalClaudeConfigDir: string | undefined + +const encoder = new TextEncoder() + +function zip(entries: Record): Buffer { + return Buffer.from(zipSync(Object.fromEntries( + Object.entries(entries).map(([name, content]) => [name, encoder.encode(content)]), + ))) +} + +function targetPath(skillName: string): string { + return path.join(tmpHome, '.claude', 'skills', skillName) +} + +describe('skill market user installer', () => { + beforeEach(async () => { + tmpHome = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-market-install-')) + originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR + process.env.CLAUDE_CONFIG_DIR = path.join(tmpHome, '.claude') + }) + + afterEach(async () => { + if (originalClaudeConfigDir === undefined) { + delete process.env.CLAUDE_CONFIG_DIR + } else { + process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir + } + await fs.rm(tmpHome, { recursive: true, force: true }) + }) + + it('installs a complete skill directory into user skills', async () => { + const result = await installUserSkillFromZipBytes({ + skillName: 'skill-vetter', + zipBytes: zip({ + 'skill-vetter/SKILL.md': '---\ndescription: Safe\n---\n# Skill', + 'skill-vetter/scripts/check.sh': 'echo ok', + 'skill-vetter/assets/icon.txt': 'icon', + }), + }) + + expect(result).toEqual({ + installed: true, + skillName: 'skill-vetter', + targetPath: targetPath('skill-vetter'), + }) + await expect(fs.readFile(path.join(targetPath('skill-vetter'), 'SKILL.md'), 'utf-8')).resolves.toContain('# Skill') + await expect(fs.readFile(path.join(targetPath('skill-vetter'), 'scripts', 'check.sh'), 'utf-8')).resolves.toBe('echo ok') + await expect(fs.readFile(path.join(targetPath('skill-vetter'), 'assets', 'icon.txt'), 'utf-8')).resolves.toBe('icon') + }) + + it('installs a root-level SKILL.md package structure', async () => { + await installUserSkillFromZipBytes({ + skillName: 'root-skill', + zipBytes: zip({ + 'SKILL.md': '---\ndescription: Root\n---\n# Root Skill', + 'scripts/check.sh': 'echo root', + }), + }) + + await expect(fs.readFile(path.join(targetPath('root-skill'), 'SKILL.md'), 'utf-8')).resolves.toContain('Root Skill') + await expect(fs.readFile(path.join(targetPath('root-skill'), 'scripts', 'check.sh'), 'utf-8')).resolves.toBe('echo root') + }) + + it('rejects unsafe skill names', async () => { + const unsafeNames = ['', '../escape', '/absolute', 'Skill', 'bad/name', 'bad\\name'] + + for (const skillName of unsafeNames) { + await expect(installUserSkillFromZipBytes({ + skillName, + zipBytes: zip({ 'SKILL.md': 'bad' }), + })).rejects.toThrow('Invalid skill name') + } + }) + + it('rejects path traversal and absolute zip entries before writing the target', async () => { + const cases = [ + { entry: '../bad-skill/SKILL.md', message: 'Unsafe file path' }, + { entry: '/bad-skill/SKILL.md', message: 'Unsafe file path' }, + ] + + for (const testCase of cases) { + await expect(installUserSkillFromZipBytes({ + skillName: 'bad-skill', + zipBytes: zip({ [testCase.entry]: 'bad' }), + })).rejects.toThrow(testCase.message) + await expect(fs.stat(targetPath('bad-skill'))).rejects.toThrow() + } + }) + + it('rejects packages missing SKILL.md', async () => { + await expect(installUserSkillFromZipBytes({ + skillName: 'missing-entry', + zipBytes: zip({ 'missing-entry/scripts/check.sh': 'echo nope' }), + })).rejects.toThrow('SKILL.md') + + await expect(fs.stat(targetPath('missing-entry'))).rejects.toThrow() + }) + + it('does not overwrite an existing non-empty skill directory', async () => { + await fs.mkdir(targetPath('skill-vetter'), { recursive: true }) + await fs.writeFile(path.join(targetPath('skill-vetter'), 'SKILL.md'), 'existing', 'utf-8') + + await expect(installUserSkillFromZipBytes({ + skillName: 'skill-vetter', + zipBytes: zip({ 'skill-vetter/SKILL.md': 'new' }), + })).rejects.toThrow('already exists') + + await expect(fs.readFile(path.join(targetPath('skill-vetter'), 'SKILL.md'), 'utf-8')).resolves.toBe('existing') + }) + + it('does not overwrite an existing empty skill directory', async () => { + await fs.mkdir(targetPath('empty-skill'), { recursive: true }) + + await expect(installUserSkillFromZipBytes({ + skillName: 'empty-skill', + zipBytes: zip({ 'empty-skill/SKILL.md': 'new' }), + })).rejects.toThrow('already exists') + + await expect(fs.readdir(targetPath('empty-skill'))).resolves.toEqual([]) + }) + + it('strips a single different top-level directory while preserving the requested skill name', async () => { + await installUserSkillFromZipBytes({ + skillName: 'renamed-skill', + zipBytes: zip({ + 'upstream-name/SKILL.md': '---\ndescription: Renamed\n---\n# Renamed', + 'upstream-name/README.md': 'docs', + }), + }) + + await expect(fs.readFile(path.join(targetPath('renamed-skill'), 'SKILL.md'), 'utf-8')).resolves.toContain('Renamed') + await expect(fs.readFile(path.join(targetPath('renamed-skill'), 'README.md'), 'utf-8')).resolves.toBe('docs') + await expect(fs.stat(path.join(targetPath('renamed-skill'), 'upstream-name'))).rejects.toThrow() + }) +}) diff --git a/src/server/services/skillMarket/installer.ts b/src/server/services/skillMarket/installer.ts new file mode 100644 index 00000000..08e38ee8 --- /dev/null +++ b/src/server/services/skillMarket/installer.ts @@ -0,0 +1,211 @@ +import * as fs from 'node:fs/promises' +import * as os from 'node:os' +import * as path from 'node:path' +import { unzipFile } from '../../../utils/dxt/zip.js' +import { getClaudeConfigHomeDir } from '../../../utils/envUtils.js' +import type { SkillMarketInstallResult } from './types.js' + +const SKILL_NAME_PATTERN = /^[a-z0-9][a-z0-9._-]{0,80}$/ + +type NormalizedEntries = Map + +export async function installUserSkillFromZipBytes(input: { + skillName: string + zipBytes: Buffer +}): Promise { + validateSkillName(input.skillName) + + const userSkillsRoot = path.join(getClaudeConfigHomeDir(), 'skills') + const targetPath = path.join(userSkillsRoot, input.skillName) + ensurePathInside(path.resolve(userSkillsRoot), path.resolve(targetPath), 'Install target escapes user skills directory') + + if (await exists(targetPath)) { + throw new Error(`Skill "${input.skillName}" already exists at ${targetPath}`) + } + + const zipEntries = await unzipFile(input.zipBytes) + const entries = normalizeSkillEntries(input.skillName, zipEntries) + if (!entries.has('SKILL.md')) { + throw new Error('Package does not contain SKILL.md') + } + + const stagingRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'cc-haha-skill-install-')) + const stagingSkillDir = path.join(stagingRoot, input.skillName) + let targetCreated = false + + try { + await fs.mkdir(stagingSkillDir) + await writeEntries(stagingSkillDir, entries) + + await fs.mkdir(userSkillsRoot, { recursive: true }) + await createTargetDirectory(targetPath, input.skillName) + targetCreated = true + await writeEntries(targetPath, entries) + + return { installed: true, skillName: input.skillName, targetPath } + } catch (error) { + if (targetCreated) { + await fs.rm(targetPath, { recursive: true, force: true }) + } + throw error + } finally { + await fs.rm(stagingRoot, { recursive: true, force: true }) + } +} + +function validateSkillName(skillName: string): void { + if (!SKILL_NAME_PATTERN.test(skillName)) { + throw new Error(`Invalid skill name: ${skillName}`) + } +} + +function normalizeSkillEntries( + skillName: string, + entries: Record, +): NormalizedEntries { + const validatedEntries = Object.entries(entries) + .map(([entryPath, bytes]) => ({ + path: normalizeArchiveEntryPath(entryPath), + bytes, + })) + .filter((entry): entry is { path: string; bytes: Uint8Array } => entry.path !== null) + + const packageRoot = findPackageRoot(skillName, validatedEntries.map((entry) => entry.path)) + const normalized = new Map() + + for (const entry of validatedEntries) { + const relativePath = packageRoot ? stripPackageRoot(entry.path, packageRoot) : entry.path + validateRelativeInstallPath(relativePath) + if (normalized.has(relativePath)) { + throw new Error(`Duplicate file path detected: "${relativePath}"`) + } + normalized.set(relativePath, entry.bytes) + } + + return normalized +} + +function normalizeArchiveEntryPath(entryPath: string): string | null { + if (!entryPath || entryPath.includes('\0')) { + throw new Error(`Unsafe file path detected: "${entryPath}"`) + } + + if (/^[a-zA-Z]:/.test(entryPath)) { + throw new Error(`Unsafe file path detected: "${entryPath}"`) + } + + const slashed = entryPath.replace(/\\/g, '/') + if (path.posix.isAbsolute(slashed)) { + throw new Error(`Unsafe file path detected: "${entryPath}"`) + } + + const segments = slashed.split('/') + if (segments.some((segment) => segment === '..')) { + throw new Error(`Unsafe file path detected: "${entryPath}"`) + } + + const normalized = path.posix.normalize(slashed) + if (!normalized || normalized === '.' || normalized.startsWith('../') || normalized === '..' || path.posix.isAbsolute(normalized)) { + throw new Error(`Unsafe file path detected: "${entryPath}"`) + } + + if (slashed.endsWith('/')) { + return null + } + + return normalized.replace(/^(\.\/)+/, '') +} + +function findPackageRoot(skillName: string, paths: string[]): string | null { + if (paths.includes('SKILL.md')) { + return null + } + + const topLevelNames = new Set(paths.map((entryPath) => entryPath.split('/')[0]!).filter(Boolean)) + if (topLevelNames.size !== 1) { + return null + } + + const [topLevelName] = topLevelNames + if (!topLevelName) { + return null + } + + if (topLevelName === skillName || paths.includes(`${topLevelName}/SKILL.md`)) { + return topLevelName + } + + return null +} + +function stripPackageRoot(entryPath: string, packageRoot: string): string { + if (!entryPath.startsWith(`${packageRoot}/`)) { + throw new Error(`Unsafe file path detected: "${entryPath}"`) + } + return entryPath.slice(packageRoot.length + 1) +} + +function validateRelativeInstallPath(relativePath: string): void { + if (!relativePath || relativePath.includes('\0') || /^[a-zA-Z]:/.test(relativePath)) { + throw new Error(`Unsafe file path detected: "${relativePath}"`) + } + + const normalized = path.posix.normalize(relativePath.replace(/\\/g, '/')) + if ( + normalized !== relativePath + || normalized === '.' + || normalized.startsWith('../') + || normalized === '..' + || path.posix.isAbsolute(normalized) + ) { + throw new Error(`Unsafe file path detected: "${relativePath}"`) + } +} + +async function writeEntries(rootDir: string, entries: NormalizedEntries): Promise { + const resolvedRoot = path.resolve(rootDir) + + for (const [relativePath, bytes] of entries) { + const destination = path.resolve(rootDir, relativePath) + ensurePathInside(resolvedRoot, destination, `Unsafe destination path: ${relativePath}`) + await fs.mkdir(path.dirname(destination), { recursive: true }) + await fs.writeFile(destination, bytes) + } +} + +function ensurePathInside(root: string, target: string, message: string): void { + const relative = path.relative(root, target) + if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) { + throw new Error(message) + } +} + +async function createTargetDirectory(targetPath: string, skillName: string): Promise { + try { + await fs.mkdir(targetPath) + } catch (error) { + if (hasErrorCode(error, 'EEXIST')) { + throw new Error(`Skill "${skillName}" already exists at ${targetPath}`) + } + throw error + } +} + +async function exists(filePath: string): Promise { + try { + await fs.lstat(filePath) + return true + } catch (error) { + if (hasErrorCode(error, 'ENOENT')) { + return false + } + throw error + } +} + +function hasErrorCode(error: unknown, code: string): boolean { + return typeof error === 'object' + && error !== null + && 'code' in error + && (error as { code?: unknown }).code === code +}