diff --git a/dist/server/index.js b/dist/server/index.js index f4ca7dd..cee5000 100644 --- a/dist/server/index.js +++ b/dist/server/index.js @@ -15,10 +15,30 @@ import { startWatcher } from './scanner/watcher.js'; import { invalidateCache } from './routes/skills.js'; import { fullScan } from './scanner/discovery.js'; import { purgeExpired as purgeExpiredTrash } from './trash/store.js'; +import { registerOriginGuard } from './security.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const app = Fastify({ logger: false }); -await app.register(cors, { origin: true }); +// Track the actually-bound origin so CORS + CSRF guards know what to accept. +// The server may fall through to a higher port on EADDRINUSE, so both lists +// are mutable and populated after `listen()` resolves. +const allowedOrigins = []; +function allowedOriginsGetter() { + return allowedOrigins; +} +await app.register(cors, { + origin: (origin, cb) => { + // Same-origin requests from our own UI come through without an Origin + // header — allow those. Everything else must exactly match an origin we + // bound to. + if (!origin) + return cb(null, true); + const ok = allowedOrigins.some((o) => o.toLowerCase() === origin.toLowerCase()); + cb(null, ok); + }, + credentials: false, +}); +registerOriginGuard(app, allowedOriginsGetter); await app.register(websocket); await app.register(skillRoutes); await app.register(manageRoutes); @@ -117,6 +137,11 @@ const basePort = parseInt(process.env.PORT || '3456'); try { const actualPort = await listenWithRetry(basePort); const url = `http://localhost:${actualPort}`; + // Register the origins we accept for CORS + mutating requests. Both the + // loopback IP and the `localhost` hostname are legitimate: browsers open + // the auto-launched URL, `fetch()` from the UI, and user-pasted links all + // route to the same server but send different Origin headers. + allowedOrigins.push(`http://localhost:${actualPort}`, `http://127.0.0.1:${actualPort}`); // Purge expired trash entries on startup (best-effort, non-blocking failures) try { const removed = await purgeExpiredTrash(); diff --git a/dist/server/routes/manage.js b/dist/server/routes/manage.js index e04c0e5..cb835a4 100644 --- a/dist/server/routes/manage.js +++ b/dist/server/routes/manage.js @@ -4,8 +4,30 @@ import os from 'os'; import { invalidateCache } from './skills.js'; import { createSnapshot } from '../versioning/store.js'; import { moveToTrash } from '../trash/store.js'; +import { confinePath, confineExistingPath } from '../security.js'; const homedir = os.homedir(); const settingsPath = path.join(homedir, '.claude', 'settings.json'); +// skillName is user-supplied and gets joined into a filesystem path. Reject +// anything that could escape the intended parent dir (separators, traversal, +// NUL bytes, Windows drive letters). Also cap length — npm/git barf past this. +function sanitizeSkillName(raw) { + if (typeof raw !== 'string') + throw new Error('skillName 必须是字符串'); + const trimmed = raw.trim(); + if (trimmed.length === 0) + throw new Error('skillName 不能为空'); + if (trimmed.length > 128) + throw new Error('skillName 过长'); + if (trimmed.includes('/') || + trimmed.includes('\\') || + trimmed.includes('\0') || + trimmed === '.' || + trimmed === '..' || + /^[A-Za-z]:/.test(trimmed)) { + throw new Error(`skillName 包含非法字符: ${raw}`); + } + return trimmed; +} async function readSettings() { try { const raw = await fs.readFile(settingsPath, 'utf-8'); @@ -42,53 +64,70 @@ export async function manageRoutes(app) { return { ok: true, enabled }; }); // Update SKILL.md content - app.put('/api/skills/:id/content', async (req) => { + app.put('/api/skills/:id/content', async (req, reply) => { const { realPath, content } = req.body; - const skillMdPath = path.join(realPath, 'SKILL.md'); - // Verify the file exists + if (typeof content !== 'string') { + reply.status(400); + return { ok: false, error: 'content 必须是字符串' }; + } + let safeRealPath; + try { + safeRealPath = await confineExistingPath(realPath); + } + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '路径不合法' }; + } + const skillMdPath = path.join(safeRealPath, 'SKILL.md'); try { await fs.access(skillMdPath); } catch { + reply.status(404); return { ok: false, error: 'SKILL.md not found' }; } // Auto-snapshot before overwriting (save the old version) - const skillName = path.basename(realPath); + const skillName = path.basename(safeRealPath); try { - await createSnapshot(realPath, skillName, '编辑前自动备份', 'auto'); + await createSnapshot(safeRealPath, skillName, '编辑前自动备份', 'auto'); } catch { } await fs.writeFile(skillMdPath, content, 'utf-8'); // Snapshot the new version try { - await createSnapshot(realPath, skillName, '通过编辑器保存', 'auto'); + await createSnapshot(safeRealPath, skillName, '通过编辑器保存', 'auto'); } catch { } invalidateCache(); return { ok: true }; }); // Copy skill to another location - app.post('/api/skills/copy', async (req) => { + app.post('/api/skills/copy', async (req, reply) => { const { sourcePath, targetScope, projectPath, skillName } = req.body; + let safeName; + let safeSource; let targetDir; - if (targetScope === 'global') { - targetDir = path.join(homedir, '.claude', 'skills', skillName); - } - else if (projectPath) { - targetDir = path.join(projectPath, '.claude', 'skills', skillName); - } - else { - return { ok: false, error: 'Project path required for project scope' }; - } - // Resolve source if symlink - let realSource; try { - realSource = await fs.realpath(sourcePath); + safeName = sanitizeSkillName(skillName); + safeSource = await confineExistingPath(sourcePath); + if (targetScope === 'global') { + targetDir = path.join(homedir, '.claude', 'skills', safeName); + } + else if (projectPath) { + const safeProject = await confineExistingPath(projectPath); + targetDir = path.join(safeProject, '.claude', 'skills', safeName); + } + else { + reply.status(400); + return { ok: false, error: 'Project path required for project scope' }; + } + // Target itself must also fall inside a safe root. + targetDir = await confinePath(targetDir); } - catch { - realSource = sourcePath; + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '参数不合法' }; } - // Check if target already exists try { await fs.access(targetDir); return { ok: false, error: '目标位置已存在同名 Skill' }; @@ -96,54 +135,70 @@ export async function manageRoutes(app) { catch { // Good — doesn't exist } - // Copy directory recursively - await copyDir(realSource, targetDir); + await copyDir(safeSource, targetDir); invalidateCache(); return { ok: true, targetDir }; }); // Move skill (copy + delete source) - app.post('/api/skills/move', async (req) => { + app.post('/api/skills/move', async (req, reply) => { const { sourcePath, targetScope, projectPath, skillName } = req.body; + let safeName; + let safeSourceReal; + let safeSourceLink; let targetDir; - if (targetScope === 'global') { - targetDir = path.join(homedir, '.claude', 'skills', skillName); - } - else if (projectPath) { - targetDir = path.join(projectPath, '.claude', 'skills', skillName); - } - else { - return { ok: false, error: 'Project path required for project scope' }; - } - let realSource; try { - realSource = await fs.realpath(sourcePath); + safeName = sanitizeSkillName(skillName); + // Validate both the link location AND the symlink target; both get + // touched (source link is unlinked, real dir is copied). + safeSourceLink = await confineExistingPath(sourcePath); + safeSourceReal = await confineExistingPath(await fs.realpath(safeSourceLink).catch(() => safeSourceLink)); + if (targetScope === 'global') { + targetDir = path.join(homedir, '.claude', 'skills', safeName); + } + else if (projectPath) { + const safeProject = await confineExistingPath(projectPath); + targetDir = path.join(safeProject, '.claude', 'skills', safeName); + } + else { + reply.status(400); + return { ok: false, error: 'Project path required for project scope' }; + } + targetDir = await confinePath(targetDir); } - catch { - realSource = sourcePath; + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '参数不合法' }; } try { await fs.access(targetDir); return { ok: false, error: '目标位置已存在同名 Skill' }; } catch { } - await copyDir(realSource, targetDir); + await copyDir(safeSourceReal, targetDir); // Remove the source (if symlink, just remove the link; if dir, remove recursively) - const stat = await fs.lstat(sourcePath); + const stat = await fs.lstat(safeSourceLink); if (stat.isSymbolicLink()) { - await fs.unlink(sourcePath); + await fs.unlink(safeSourceLink); } else { - await fs.rm(sourcePath, { recursive: true }); + await fs.rm(safeSourceLink, { recursive: true }); } invalidateCache(); return { ok: true, targetDir }; }); // Delete skill (soft delete → recycle bin; 7-day TTL) app.delete('/api/skills/:id', async (req, reply) => { - const skillPath = req.body.path; const skillName = req.body.skillName; + let safePath; + try { + safePath = await confineExistingPath(req.body.path); + } + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '路径不合法' }; + } try { - const meta = await moveToTrash(skillPath, skillName); + const meta = await moveToTrash(safePath, skillName); invalidateCache(); return { ok: true, trashId: meta.id, expiresAt: meta.expiresAt }; } @@ -165,8 +220,21 @@ export async function manageRoutes(app) { results.push({ id: item?.id || '(unknown)', ok: false, error: '参数不完整' }); continue; } + let safeItemPath; + try { + safeItemPath = await confineExistingPath(item.path); + } + catch (err) { + results.push({ + id: item.id, + skillName: item.skillName, + ok: false, + error: err?.message || '路径不合法', + }); + continue; + } try { - const meta = await moveToTrash(item.path, item.skillName); + const meta = await moveToTrash(safeItemPath, item.skillName); results.push({ id: item.id, skillName: item.skillName, diff --git a/dist/server/routes/trash.js b/dist/server/routes/trash.js index 95ba95c..19acadb 100644 --- a/dist/server/routes/trash.js +++ b/dist/server/routes/trash.js @@ -1,5 +1,14 @@ import { listTrash, restoreFromTrash, purgeOne, purgeExpired, TrashConflictError, TrashNotFoundError, } from '../trash/store.js'; import { invalidateCache } from './skills.js'; +// newId() from ../trash/store.ts always matches `-`. Reject +// anything else so an attacker can't smuggle path traversal into fs.rm. +const TRASH_ID_RE = /^[a-z0-9]{1,20}-[a-f0-9]{4,32}$/; +function assertSafeTrashId(id) { + if (typeof id !== 'string' || !TRASH_ID_RE.test(id)) { + throw new Error(`非法 trash id: ${String(id)}`); + } + return id; +} export async function trashRoutes(app) { // List trash entries (also purges expired as a side effect) app.get('/api/trash', async () => { @@ -8,7 +17,14 @@ export async function trashRoutes(app) { }); // Restore a trash entry back to its original location app.post('/api/trash/:id/restore', async (req, reply) => { - const { id } = req.params; + let id; + try { + id = assertSafeTrashId(req.params.id); + } + catch (err) { + reply.status(400); + return { ok: false, error: err.message }; + } const force = req.query.force === 'true' || req.query.force === '1'; try { const meta = await restoreFromTrash(id, force); @@ -29,8 +45,16 @@ export async function trashRoutes(app) { } }); // Permanently delete a single trash entry - app.delete('/api/trash/:id', async (req) => { - const ok = await purgeOne(req.params.id); + app.delete('/api/trash/:id', async (req, reply) => { + let id; + try { + id = assertSafeTrashId(req.params.id); + } + catch (err) { + reply.status(400); + return { ok: false, error: err.message }; + } + const ok = await purgeOne(id); return { ok }; }); // Manual purge of expired entries diff --git a/dist/server/routes/versions.js b/dist/server/routes/versions.js index 9a8ce28..db428be 100644 --- a/dist/server/routes/versions.js +++ b/dist/server/routes/versions.js @@ -1,11 +1,20 @@ import { createSnapshot, getHistory, getVersion, diffVersions, diffWithCurrent, rollback, deleteVersion, } from '../versioning/store.js'; import { invalidateCache } from './skills.js'; +import { confinePath, confineExistingPath } from '../security.js'; export async function versionRoutes(app) { // 创建快照 - app.post('/api/versions/snapshot', async (req) => { + app.post('/api/versions/snapshot', async (req, reply) => { const { skillPath, skillName, message } = req.body; + let safePath; try { - const meta = await createSnapshot(skillPath, skillName, message, 'manual'); + safePath = await confineExistingPath(skillPath); + } + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '路径不合法' }; + } + try { + const meta = await createSnapshot(safePath, skillName, message, 'manual'); return { ok: true, version: meta }; } catch (e) { @@ -43,18 +52,36 @@ export async function versionRoutes(app) { return { ok: true, diff }; }); // 回滚到指定版本 - app.post('/api/versions/rollback', async (req) => { + app.post('/api/versions/rollback', async (req, reply) => { const { skillPath, versionId } = req.body; - const success = await rollback(skillPath, versionId); + let safePath; + try { + safePath = await confineExistingPath(skillPath); + } + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '路径不合法' }; + } + const success = await rollback(safePath, versionId); if (!success) return { ok: false, error: 'Rollback failed' }; invalidateCache(); return { ok: true }; }); // 删除版本 - app.delete('/api/versions', async (req) => { + app.delete('/api/versions', async (req, reply) => { const { skillPath, versionId } = req.query; - const success = await deleteVersion(skillPath, versionId); + let safePath; + try { + // version metadata is keyed by skill path — we still confine, but the + // path may have been deleted (allow non-existent), so use confinePath. + safePath = await confinePath(skillPath); + } + catch (err) { + reply.status(403); + return { ok: false, error: err?.message || '路径不合法' }; + } + const success = await deleteVersion(safePath, versionId); return { ok: success }; }); } diff --git a/dist/web/index.html b/dist/web/index.html index 9d70b66..84adad8 100644 --- a/dist/web/index.html +++ b/dist/web/index.html @@ -1,10 +1,10 @@ - - - - - - Skill Hub - + + + + + + Skill Hub + - + + - - -
-
- -
正在加载 Skill Hub...
-
如果这个画面持续不消失，打开 DevTools Console 查看错误，或访问 /api/debug
-
- - - + + +
+
+ +
正在加载 Skill Hub...
+
如果这个画面持续不消失，打开 DevTools Console 查看错误，或访问 /api/debug
+
+ + + diff --git a/package-lock.json b/package-lock.json index a83336f..74e199c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -70,6 +70,7 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -1739,6 +1740,7 @@ "integrity": "sha512-wGdMcf+vPYM6jikpS/qhg6WiqSV/OhG+jeeHT/KlVqxYfD40iYJf9/AE1uQxVWFvU7MipKRkRv8NSHiCGgPr8Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~6.21.0" } @@ -1749,6 +1751,7 @@ "integrity": "sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -1941,6 +1944,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", @@ -2676,6 +2680,7 @@ "integrity": "sha512-ekilCSN1jwRvIbgeg/57YFh8qQDNbwDb9xT/qu2DAHbFFZUicIl4ygVaAvzveMhMVr3LnpSKTNnwt8PoOfmKhQ==", "dev": true, "license": "MIT", + "peer": true, "bin": { "jiti": "lib/jiti-cli.mjs" } @@ -2803,6 +2808,7 @@ "integrity": "sha512-NXYBzinNrblfraPGyrbPoD19C1h9lfI/1mzgWYvXUTe414Gz/X1FD2XBZSZM7rRTrMA8JL3OtAaGifrIKhQ5yQ==", "dev": true, "license": "MPL-2.0", + "peer": true, "dependencies": { "detect-libc": "^2.0.3" }, @@ -3228,6 +3234,7 @@ "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -3329,6 +3336,7 @@ "integrity": "sha512-llUJLzz1zTUBrskt2pwZgLq59AemifIftw4aB7JxOqf1HY2FDaGDxgwpAPVzHU1kdWabH7FauP4i1oEeer2WCA==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -3858,6 +3866,7 @@ "integrity": "sha512-5C1sg4USs1lfG0GFb2RLXsdpXqBSEhAaA/0kPL01wxzpMqLILNxIxIOKiILz+cdg/pLnOUxFYOR5yhHU666wbw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.27.0", "get-tsconfig": "^4.7.5" @@ -3936,6 +3945,7 @@ "integrity": "sha512-2N/55r4JDJ4gdrCvGgINMy+HH3iRpNIz8K6SFwVsA+JbQScLiC+clmAxBgwiSPgcG9U15QmvqCGWzMbqda5zGQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.4.4", diff --git a/release/claude-skill-hub.tgz b/release/claude-skill-hub.tgz index 548e1a6..6c9ff7f 100644 Binary files a/release/claude-skill-hub.tgz and b/release/claude-skill-hub.tgz differ diff --git a/server/index.ts b/server/index.ts index 15e0f9c..d746ff5 100644 --- a/server/index.ts +++ b/server/index.ts @@ -15,6 +15,7 @@ import { startWatcher } from './scanner/watcher.js' import { invalidateCache } from './routes/skills.js' import { fullScan } from './scanner/discovery.js' import { purgeExpired as purgeExpiredTrash } from './trash/store.js' +import { registerOriginGuard } from './security.js' import type { WebSocket } from 'ws' const __filename = fileURLToPath(import.meta.url) @@ -22,7 +23,26 @@ const __dirname = path.dirname(__filename) const app = Fastify({ logger: false }) -await app.register(cors, { origin: true }) +// Track the actually-bound origin so CORS + CSRF guards know what to accept. +// The server may fall through to a higher port on EADDRINUSE, so both lists +// are mutable and populated after `listen()` resolves. +const allowedOrigins: string[] = [] +function allowedOriginsGetter(): string[] { + return allowedOrigins +} + +await app.register(cors, { + origin: (origin, cb) => { + // Same-origin requests from our own UI come through without an Origin + // header — allow those. Everything else must exactly match an origin we + // bound to. + if (!origin) return cb(null, true) + const ok = allowedOrigins.some((o) => o.toLowerCase() === origin.toLowerCase()) + cb(null, ok) + }, + credentials: false, +}) +registerOriginGuard(app, allowedOriginsGetter) await app.register(websocket) await app.register(skillRoutes) await app.register(manageRoutes) @@ -139,6 +159,15 @@ try { const actualPort = await listenWithRetry(basePort) const url = `http://localhost:${actualPort}` + // Register the origins we accept for CORS + mutating requests. Both the + // loopback IP and the `localhost` hostname are legitimate: browsers open + // the auto-launched URL, `fetch()` from the UI, and user-pasted links all + // route to the same server but send different Origin headers. + allowedOrigins.push( + `http://localhost:${actualPort}`, + `http://127.0.0.1:${actualPort}`, + ) + // Purge expired trash entries on startup (best-effort, non-blocking failures) try { const removed = await purgeExpiredTrash() diff --git a/server/routes/manage.ts b/server/routes/manage.ts index c10b3fb..298fc5b 100644 --- a/server/routes/manage.ts +++ b/server/routes/manage.ts @@ -5,10 +5,32 @@ import os from 'os' import { invalidateCache } from './skills.js' import { createSnapshot } from '../versioning/store.js' import { moveToTrash } from '../trash/store.js' +import { confinePath, confineExistingPath } from '../security.js' const homedir = os.homedir() const settingsPath = path.join(homedir, '.claude', 'settings.json') +// skillName is user-supplied and gets joined into a filesystem path. Reject +// anything that could escape the intended parent dir (separators, traversal, +// NUL bytes, Windows drive letters). Also cap length — npm/git barf past this. +function sanitizeSkillName(raw: unknown): string { + if (typeof raw !== 'string') throw new Error('skillName 必须是字符串') + const trimmed = raw.trim() + if (trimmed.length === 0) throw new Error('skillName 不能为空') + if (trimmed.length > 128) throw new Error('skillName 过长') + if ( + trimmed.includes('/') || + trimmed.includes('\\') || + trimmed.includes('\0') || + trimmed === '.' || + trimmed === '..' || + /^[A-Za-z]:/.test(trimmed) + ) { + throw new Error(`skillName 包含非法字符: ${raw}`) + } + return trimmed +} + async function readSettings(): Promise { try { const raw = await fs.readFile(settingsPath, 'utf-8') @@ -54,28 +76,40 @@ export async function manageRoutes(app: FastifyInstance) { app.put<{ Params: { id: string } Body: { realPath: string; content: string } - }>('/api/skills/:id/content', async (req) => { + }>('/api/skills/:id/content', async (req, reply) => { const { realPath, content } = req.body - const skillMdPath = path.join(realPath, 'SKILL.md') + if (typeof content !== 'string') { + reply.status(400) + return { ok: false, error: 'content 必须是字符串' } + } + + let safeRealPath: string + try { + safeRealPath = await confineExistingPath(realPath) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '路径不合法' } + } - // Verify the file exists + const skillMdPath = path.join(safeRealPath, 'SKILL.md') try { await fs.access(skillMdPath) } catch { + reply.status(404) return { ok: false, error: 'SKILL.md not found' } } // Auto-snapshot before overwriting (save the old version) - const skillName = path.basename(realPath) + const skillName = path.basename(safeRealPath) try { - await createSnapshot(realPath, skillName, '编辑前自动备份', 'auto') + await createSnapshot(safeRealPath, skillName, '编辑前自动备份', 'auto') } catch {} await fs.writeFile(skillMdPath, content, 'utf-8') // Snapshot the new version try { - await createSnapshot(realPath, skillName, '通过编辑器保存', 'auto') + await createSnapshot(safeRealPath, skillName, '通过编辑器保存', 'auto') } catch {} invalidateCache() @@ -90,27 +124,32 @@ export async function manageRoutes(app: FastifyInstance) { projectPath?: string skillName: string } - }>('/api/skills/copy', async (req) => { + }>('/api/skills/copy', async (req, reply) => { const { sourcePath, targetScope, projectPath, skillName } = req.body + let safeName: string + let safeSource: string let targetDir: string - if (targetScope === 'global') { - targetDir = path.join(homedir, '.claude', 'skills', skillName) - } else if (projectPath) { - targetDir = path.join(projectPath, '.claude', 'skills', skillName) - } else { - return { ok: false, error: 'Project path required for project scope' } - } - - // Resolve source if symlink - let realSource: string try { - realSource = await fs.realpath(sourcePath) - } catch { - realSource = sourcePath + safeName = sanitizeSkillName(skillName) + safeSource = await confineExistingPath(sourcePath) + + if (targetScope === 'global') { + targetDir = path.join(homedir, '.claude', 'skills', safeName) + } else if (projectPath) { + const safeProject = await confineExistingPath(projectPath) + targetDir = path.join(safeProject, '.claude', 'skills', safeName) + } else { + reply.status(400) + return { ok: false, error: 'Project path required for project scope' } + } + // Target itself must also fall inside a safe root. + targetDir = await confinePath(targetDir) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '参数不合法' } } - // Check if target already exists try { await fs.access(targetDir) return { ok: false, error: '目标位置已存在同名 Skill' } @@ -118,8 +157,7 @@ export async function manageRoutes(app: FastifyInstance) { // Good — doesn't exist } - // Copy directory recursively - await copyDir(realSource, targetDir) + await copyDir(safeSource, targetDir) invalidateCache() return { ok: true, targetDir } }) @@ -132,23 +170,35 @@ export async function manageRoutes(app: FastifyInstance) { projectPath?: string skillName: string } - }>('/api/skills/move', async (req) => { + }>('/api/skills/move', async (req, reply) => { const { sourcePath, targetScope, projectPath, skillName } = req.body + let safeName: string + let safeSourceReal: string + let safeSourceLink: string let targetDir: string - if (targetScope === 'global') { - targetDir = path.join(homedir, '.claude', 'skills', skillName) - } else if (projectPath) { - targetDir = path.join(projectPath, '.claude', 'skills', skillName) - } else { - return { ok: false, error: 'Project path required for project scope' } - } - - let realSource: string try { - realSource = await fs.realpath(sourcePath) - } catch { - realSource = sourcePath + safeName = sanitizeSkillName(skillName) + // Validate both the link location AND the symlink target; both get + // touched (source link is unlinked, real dir is copied). + safeSourceLink = await confineExistingPath(sourcePath) + safeSourceReal = await confineExistingPath( + await fs.realpath(safeSourceLink).catch(() => safeSourceLink), + ) + + if (targetScope === 'global') { + targetDir = path.join(homedir, '.claude', 'skills', safeName) + } else if (projectPath) { + const safeProject = await confineExistingPath(projectPath) + targetDir = path.join(safeProject, '.claude', 'skills', safeName) + } else { + reply.status(400) + return { ok: false, error: 'Project path required for project scope' } + } + targetDir = await confinePath(targetDir) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '参数不合法' } } try { @@ -156,14 +206,14 @@ export async function manageRoutes(app: FastifyInstance) { return { ok: false, error: '目标位置已存在同名 Skill' } } catch {} - await copyDir(realSource, targetDir) + await copyDir(safeSourceReal, targetDir) // Remove the source (if symlink, just remove the link; if dir, remove recursively) - const stat = await fs.lstat(sourcePath) + const stat = await fs.lstat(safeSourceLink) if (stat.isSymbolicLink()) { - await fs.unlink(sourcePath) + await fs.unlink(safeSourceLink) } else { - await fs.rm(sourcePath, { recursive: true }) + await fs.rm(safeSourceLink, { recursive: true }) } invalidateCache() @@ -175,11 +225,18 @@ export async function manageRoutes(app: FastifyInstance) { Params: { id: string } Body: { path: string; skillName?: string } }>('/api/skills/:id', async (req, reply) => { - const skillPath = req.body.path const skillName = req.body.skillName + let safePath: string try { - const meta = await moveToTrash(skillPath, skillName) + safePath = await confineExistingPath(req.body.path) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '路径不合法' } + } + + try { + const meta = await moveToTrash(safePath, skillName) invalidateCache() return { ok: true, trashId: meta.id, expiresAt: meta.expiresAt } } catch (err: any) { @@ -211,8 +268,20 @@ export async function manageRoutes(app: FastifyInstance) { results.push({ id: item?.id || '(unknown)', ok: false, error: '参数不完整' }) continue } + let safeItemPath: string + try { + safeItemPath = await confineExistingPath(item.path) + } catch (err: any) { + results.push({ + id: item.id, + skillName: item.skillName, + ok: false, + error: err?.message || '路径不合法', + }) + continue + } try { - const meta = await moveToTrash(item.path, item.skillName) + const meta = await moveToTrash(safeItemPath, item.skillName) results.push({ id: item.id, skillName: item.skillName, diff --git a/server/routes/trash.ts b/server/routes/trash.ts index 6f84b9c..6f28d0c 100644 --- a/server/routes/trash.ts +++ b/server/routes/trash.ts @@ -9,6 +9,17 @@ import { } from '../trash/store.js' import { invalidateCache } from './skills.js' +// newId() from ../trash/store.ts always matches `-`. Reject +// anything else so an attacker can't smuggle path traversal into fs.rm. +const TRASH_ID_RE = /^[a-z0-9]{1,20}-[a-f0-9]{4,32}$/ + +function assertSafeTrashId(id: unknown): string { + if (typeof id !== 'string' || !TRASH_ID_RE.test(id)) { + throw new Error(`非法 trash id: ${String(id)}`) + } + return id +} + export async function trashRoutes(app: FastifyInstance) { // List trash entries (also purges expired as a side effect) app.get('/api/trash', async () => { @@ -21,7 +32,13 @@ export async function trashRoutes(app: FastifyInstance) { Params: { id: string } Querystring: { force?: string } }>('/api/trash/:id/restore', async (req, reply) => { - const { id } = req.params + let id: string + try { + id = assertSafeTrashId(req.params.id) + } catch (err: any) { + reply.status(400) + return { ok: false, error: err.message } + } const force = req.query.force === 'true' || req.query.force === '1' try { const meta = await restoreFromTrash(id, force) @@ -42,8 +59,15 @@ export async function trashRoutes(app: FastifyInstance) { }) // Permanently delete a single trash entry - app.delete<{ Params: { id: string } }>('/api/trash/:id', async (req) => { - const ok = await purgeOne(req.params.id) + app.delete<{ Params: { id: string } }>('/api/trash/:id', async (req, reply) => { + let id: string + try { + id = assertSafeTrashId(req.params.id) + } catch (err: any) { + reply.status(400) + return { ok: false, error: err.message } + } + const ok = await purgeOne(id) return { ok } }) diff --git a/server/routes/versions.ts b/server/routes/versions.ts index 803a907..a6c71b5 100644 --- a/server/routes/versions.ts +++ b/server/routes/versions.ts @@ -9,15 +9,23 @@ import { deleteVersion, } from '../versioning/store.js' import { invalidateCache } from './skills.js' +import { confinePath, confineExistingPath } from '../security.js' export async function versionRoutes(app: FastifyInstance) { // 创建快照 app.post<{ Body: { skillPath: string; skillName: string; message: string } - }>('/api/versions/snapshot', async (req) => { + }>('/api/versions/snapshot', async (req, reply) => { const { skillPath, skillName, message } = req.body + let safePath: string try { - const meta = await createSnapshot(skillPath, skillName, message, 'manual') + safePath = await confineExistingPath(skillPath) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '路径不合法' } + } + try { + const meta = await createSnapshot(safePath, skillName, message, 'manual') return { ok: true, version: meta } } catch (e: any) { return { ok: false, error: e.message } @@ -66,9 +74,16 @@ export async function versionRoutes(app: FastifyInstance) { // 回滚到指定版本 app.post<{ Body: { skillPath: string; versionId: string } - }>('/api/versions/rollback', async (req) => { + }>('/api/versions/rollback', async (req, reply) => { const { skillPath, versionId } = req.body - const success = await rollback(skillPath, versionId) + let safePath: string + try { + safePath = await confineExistingPath(skillPath) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '路径不合法' } + } + const success = await rollback(safePath, versionId) if (!success) return { ok: false, error: 'Rollback failed' } invalidateCache() return { ok: true } @@ -77,9 +92,18 @@ export async function versionRoutes(app: FastifyInstance) { // 删除版本 app.delete<{ Querystring: { skillPath: string; versionId: string } - }>('/api/versions', async (req) => { + }>('/api/versions', async (req, reply) => { const { skillPath, versionId } = req.query - const success = await deleteVersion(skillPath, versionId) + let safePath: string + try { + // version metadata is keyed by skill path — we still confine, but the + // path may have been deleted (allow non-existent), so use confinePath. + safePath = await confinePath(skillPath) + } catch (err: any) { + reply.status(403) + return { ok: false, error: err?.message || '路径不合法' } + } + const success = await deleteVersion(safePath, versionId) return { ok: success } }) } diff --git a/server/security.ts b/server/security.ts new file mode 100644 index 0000000..3e717df --- /dev/null +++ b/server/security.ts @@ -0,0 +1,167 @@ +/** + * Server-side security guards. + * + * Two threats this module defends against: + * + * 1. Drive-by CSRF — any webpage the user visits in the browser can issue + * `fetch('http://localhost:3456/api/skills/', { method: 'DELETE', … })` + * and the server would happily execute it. We require either a + * `Sec-Fetch-Site: same-origin` header (sent by every modern browser for + * same-origin requests) or an `Origin` header that matches our own origin. + * Cross-origin requests from browsers will fail the check. + * + * 2. Arbitrary filesystem path injection — the mutating routes take paths + * (e.g. `body.path`) and pass them to `fs.rm` / `fs.writeFile` / rename. + * `confinePath()` resolves the candidate (following symlinks when they + * exist) and rejects anything that isn't under one of the directories the + * scanner legitimately covers (`~/.claude/…`, `~/.skill-hub/…`, discovered + * project roots, and `SKILL_HUB_EXTRA_PATHS`). + */ +import type { FastifyInstance } from 'fastify' +import fs from 'fs/promises' +import path from 'path' +import os from 'os' +import { getCachedResult } from './routes/skills.js' + +const homedir = os.homedir() +const IS_WIN = process.platform === 'win32' + +// ---------- Origin / CSRF guard ---------- + +const MUTATING = new Set(['POST', 'PUT', 'DELETE', 'PATCH']) + +export function registerOriginGuard( + app: FastifyInstance, + getAllowedOrigins: () => string[], +): void { + app.addHook('onRequest', async (req, reply) => { + const method = (req.method || '').toUpperCase() + if (!MUTATING.has(method)) return + if (req.url.startsWith('/ws')) return // websocket upgrade uses GET anyway + + const allowed = getAllowedOrigins().map((o) => o.toLowerCase()) + const origin = String(req.headers.origin || '').toLowerCase() + const fetchSite = String(req.headers['sec-fetch-site'] || '').toLowerCase() + const host = String(req.headers.host || '').toLowerCase() + + // Allow if browser reports same-origin. + if (fetchSite === 'same-origin') return + // Allow if Origin header matches one of our own origins exactly. + if (origin && allowed.includes(origin)) return + // Allow when there is NO origin header AND the Host header is loopback. + // Browsers always send Origin on cross-origin mutations, so this case + // only covers local tools (curl, tests) hitting loopback directly. + if (!origin && (host.startsWith('127.0.0.1:') || host.startsWith('localhost:'))) return + + reply.status(403) + return reply.send({ + ok: false, + error: 'Forbidden: cross-origin mutation blocked', + }) + }) +} + +// ---------- Path confinement ---------- + +function normalizeForCompare(p: string): string { + const n = path.normalize(path.resolve(p)) + return IS_WIN ? n.toLowerCase() : n +} + +function isUnderRoot(resolved: string, root: string): boolean { + const r = normalizeForCompare(root) + const c = normalizeForCompare(resolved) + if (c === r) return true + return c.startsWith(r + path.sep) +} + +function staticSafeRoots(): string[] { + return [ + path.resolve(homedir, '.claude'), + path.resolve(homedir, '.skill-hub'), + ] +} + +function extraPathRoots(): string[] { + const raw = process.env.SKILL_HUB_EXTRA_PATHS + if (!raw) return [] + return raw + .split(/[:,]/) + .map((p) => p.trim()) + .filter(Boolean) + .map((p) => (p.startsWith('~') ? path.join(homedir, p.slice(1)) : p)) + .map((p) => path.resolve(p)) +} + +/** + * Roots we'll accept as write targets. Pulls from: + * - static Claude / Skill-Hub paths + * - project roots the most recent scan actually discovered + * - SKILL_HUB_EXTRA_PATHS + * + * Falls back to static roots alone when no scan has run yet. + */ +export function getSafeRoots(): string[] { + const roots = new Set() + for (const r of staticSafeRoots()) roots.add(r) + for (const r of extraPathRoots()) roots.add(r) + const scan = getCachedResult() + if (scan) { + for (const proj of scan.projects) { + roots.add(path.resolve(proj.path)) + } + } + return Array.from(roots) +} + +/** + * Resolve a user-supplied path, follow symlinks when possible, and require + * the result to live inside one of the safe roots. Throws on any violation. + * + * Works for both existing paths (e.g. source of a copy/move, file to delete) + * and not-yet-existing paths (e.g. copy/move target). + */ +export async function confinePath( + input: unknown, + roots: string[] = getSafeRoots(), +): Promise { + if (typeof input !== 'string' || input.length === 0) { + throw new Error('路径不能为空') + } + + let resolved = path.resolve(input) + + // If the path exists, resolve symlinks so an attacker can't use a symlink + // inside a safe root to reach somewhere that isn't. + try { + resolved = await fs.realpath(resolved) + } catch { + // Path doesn't exist yet — `path.resolve` alone is fine for write targets. + } + + resolved = path.normalize(resolved) + + const fits = roots.some((r) => isUnderRoot(resolved, r)) + if (!fits) { + throw new Error(`路径超出允许范围: ${input}`) + } + return resolved +} + +/** + * Same as `confinePath`, but also asserts that the path exists right now. + * Use for sources of read/delete/move operations where a missing path is + * itself a failure. + */ +export async function confineExistingPath( + input: unknown, + roots: string[] = getSafeRoots(), +): Promise { + const resolved = await confinePath(input, roots) + try { + await fs.access(resolved) + } catch { + throw new Error(`路径不存在: ${input}`) + } + return resolved +}