From 498b66d9cceaf34a36e81403fa15b73093eda2c9 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 23 Feb 2026 23:22:29 +0700 Subject: [PATCH] fix(web-server): harden shared route symlink scanning --- src/utils/claude-config-path.ts | 26 ++ src/web-server/shared-routes.ts | 127 ++++++-- tests/unit/web-server/shared-routes.test.ts | 331 ++++++++++++++++---- 3 files changed, 396 insertions(+), 88 deletions(-) create mode 100644 src/utils/claude-config-path.ts diff --git a/src/utils/claude-config-path.ts b/src/utils/claude-config-path.ts new file mode 100644 index 00000000..e084cceb --- /dev/null +++ b/src/utils/claude-config-path.ts @@ -0,0 +1,26 @@ +import * as os from 'os'; +import * as path from 'path'; + +/** + * Resolve Claude config directory with test/dev overrides. + * Precedence: + * 1. CLAUDE_CONFIG_DIR (explicit override) + * 2. CCS_HOME compatibility path (/.claude) + * 3. ~/.claude (default) + */ +export function getClaudeConfigDir(): string { + if (process.env.CLAUDE_CONFIG_DIR) { + return path.resolve(process.env.CLAUDE_CONFIG_DIR); + } + + if (process.env.CCS_HOME) { + return path.join(path.resolve(process.env.CCS_HOME), '.claude'); + } + + return path.join(os.homedir(), '.claude'); +} + +/** Resolve Claude settings.json path. */ +export function getClaudeSettingsPath(): string { + return path.join(getClaudeConfigDir(), 'settings.json'); +} diff --git a/src/web-server/shared-routes.ts b/src/web-server/shared-routes.ts index 6e1d965a..1682cab0 100644 --- a/src/web-server/shared-routes.ts +++ b/src/web-server/shared-routes.ts @@ -7,8 +7,8 @@ import { Router, Request, Response } from 'express'; import * as fs from 'fs'; import * as path from 'path'; -import * as os from 'os'; import { getCcsDir } from '../utils/config-manager'; +import { getClaudeConfigDir } from '../utils/claude-config-path'; export const sharedRoutes = Router(); @@ -69,38 +69,76 @@ function getSharedItems(type: 'commands' | 'skills' | 'agents'): SharedItem[] { } const items: SharedItem[] = []; + const sharedDirRoot = safeRealPath(sharedDir) ?? path.resolve(sharedDir); + const allowedSkillAgentRoots = new Set([ + sharedDirRoot, + ...[ + path.join(getClaudeConfigDir(), type), + path.join(ccsDir, '.claude', type), + path.join(ccsDir, 'shared', type), + ] + .map((dirPath) => safeRealPath(dirPath)) + .filter((dirPath): dirPath is string => typeof dirPath === 'string'), + ]); try { const entries = fs.readdirSync(sharedDir, { withFileTypes: true }); for (const entry of entries) { - const entryPath = path.join(sharedDir, entry.name); + try { + const entryPath = path.join(sharedDir, entry.name); + + if (type === 'commands') { + if (!entry.name.endsWith('.md')) { + continue; + } + if (!entry.isFile() && !entry.isSymbolicLink()) { + continue; + } + + const commandPath = safeRealPath(entryPath); + if (!commandPath || !isPathWithin(commandPath, sharedDirRoot)) { + continue; + } + + const description = readMarkdownDescription(commandPath, sharedDirRoot); + if (!description) { + continue; + } - // Skills/Agents are directory-based and may be symlinked directories - if (type !== 'commands' && (entry.isDirectory() || entry.isSymbolicLink())) { - const markdownFile = type === 'skills' ? 'SKILL.md' : 'prompt.md'; - const promptPath = path.join(entryPath, markdownFile); - if (fs.existsSync(promptPath)) { - const content = fs.readFileSync(promptPath, 'utf8'); - const description = extractDescription(content); items.push({ - name: entry.name, + name: entry.name.replace('.md', ''), description, path: entryPath, - type: type === 'skills' ? 'skill' : 'agent', + type: 'command', }); + continue; } - } else if (type === 'commands' && entry.name.endsWith('.md')) { - // Commands are markdown files - const filePath = entryPath; - const content = fs.readFileSync(filePath, 'utf8'); - const description = extractDescription(content); + + // Skills/agents are directory-based and may be symlinked directories. + if (!entry.isDirectory() && !entry.isSymbolicLink()) { + continue; + } + + const entryRoot = safeRealPath(entryPath); + if (!entryRoot || !isPathWithinAny(entryRoot, allowedSkillAgentRoots)) { + continue; + } + + const markdownFile = type === 'skills' ? 'SKILL.md' : 'prompt.md'; + const description = readMarkdownDescription(path.join(entryRoot, markdownFile), entryRoot); + if (!description) { + continue; + } + items.push({ - name: entry.name.replace('.md', ''), + name: entry.name, description, - path: filePath, - type: 'command', + path: entryPath, + type: type === 'skills' ? 'skill' : 'agent', }); + } catch { + // Fail soft per entry so one bad item does not hide valid results. } } } catch { @@ -122,6 +160,53 @@ function extractDescription(content: string): string { return 'No description'; } +function readMarkdownDescription(markdownPath: string, allowedRoot: string): string | null { + try { + const resolvedMarkdownPath = safeRealPath(markdownPath); + if (!resolvedMarkdownPath || !isPathWithin(resolvedMarkdownPath, allowedRoot)) { + return null; + } + + const stats = fs.statSync(resolvedMarkdownPath); + if (!stats.isFile()) { + return null; + } + const content = fs.readFileSync(resolvedMarkdownPath, 'utf8'); + return extractDescription(content); + } catch { + return null; + } +} + +function safeRealPath(targetPath: string): string | null { + try { + return fs.realpathSync(targetPath); + } catch { + return null; + } +} + +function isPathWithin(candidatePath: string, basePath: string): boolean { + const normalizedCandidate = normalizeForPathComparison(candidatePath); + const normalizedBase = normalizeForPathComparison(basePath); + const relative = path.relative(normalizedBase, normalizedCandidate); + return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); +} + +function isPathWithinAny(candidatePath: string, basePaths: Set): boolean { + for (const basePath of basePaths) { + if (isPathWithin(candidatePath, basePath)) { + return true; + } + } + return false; +} + +function normalizeForPathComparison(targetPath: string): string { + const normalized = path.resolve(targetPath); + return process.platform === 'win32' ? normalized.toLowerCase() : normalized; +} + function checkSymlinkStatus(): { valid: boolean; message: string } { const ccsDir = getCcsDir(); const sharedDir = path.join(ccsDir, 'shared'); @@ -142,8 +227,8 @@ function checkSymlinkStatus(): { valid: boolean; message: string } { const stats = fs.lstatSync(linkPath); if (stats.isSymbolicLink()) { const target = fs.readlinkSync(linkPath); - // Check if it points to ~/.claude/{linkType} - const expectedTarget = path.join(os.homedir(), '.claude', linkType); + // Check if it points to Claude config dir. + const expectedTarget = path.join(getClaudeConfigDir(), linkType); if (path.resolve(path.dirname(linkPath), target) === path.resolve(expectedTarget)) { validLinks++; } diff --git a/tests/unit/web-server/shared-routes.test.ts b/tests/unit/web-server/shared-routes.test.ts index 0fc0352d..b1824b16 100644 --- a/tests/unit/web-server/shared-routes.test.ts +++ b/tests/unit/web-server/shared-routes.test.ts @@ -1,42 +1,94 @@ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'bun:test'; +import express from 'express'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import type { Server } from 'http'; import { sharedRoutes } from '../../../src/web-server/shared-routes'; -function getGetRouteHandler(routePath: string): (req: unknown, res: unknown) => void { - const layer = (sharedRoutes as { stack?: unknown[] }).stack?.find((entry) => { - const route = (entry as { route?: { path?: string; methods?: Record } }).route; - return route?.path === routePath && route.methods?.get; - }) as - | { - route?: { - stack?: Array<{ handle: (req: unknown, res: unknown) => void }>; - }; - } - | undefined; +function createDirectorySymlink(targetDir: string, linkPath: string): void { + const symlinkType = process.platform === 'win32' ? 'junction' : 'dir'; - const handler = layer?.route?.stack?.[0]?.handle; - if (!handler) { - throw new Error(`GET handler not found for route: ${routePath}`); + try { + fs.symlinkSync(targetDir, linkPath, symlinkType as fs.symlink.Type); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + throw new Error( + `Symlink creation is not permitted in this environment (${code}) for ${linkPath}` + ); + } + throw error; } +} - return handler; +function createFileSymlink(targetFile: string, linkPath: string): void { + const symlinkType = process.platform === 'win32' ? 'file' : 'file'; + + try { + fs.symlinkSync(targetFile, linkPath, symlinkType as fs.symlink.Type); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + throw new Error( + `File symlink creation is not permitted in this environment (${code}) for ${linkPath}` + ); + } + throw error; + } +} + +async function getJson(baseUrl: string, routePath: string): Promise { + const response = await fetch(`${baseUrl}${routePath}`); + expect(response.status).toBe(200); + return (await response.json()) as T; } describe('web-server shared-routes', () => { + let server: Server; + let baseUrl = ''; let tempHome: string; let ccsDir: string; let originalCcsHome: string | undefined; + let originalClaudeConfigDir: string | undefined; + + beforeAll(async () => { + const app = express(); + app.use('/api/shared', sharedRoutes); + + await new Promise((resolve, reject) => { + server = app.listen(0, '127.0.0.1'); + const handleError = (error: Error) => { + reject(error); + }; + + server.once('error', handleError); + server.once('listening', () => { + server.off('error', handleError); + resolve(); + }); + }); + + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Unable to resolve test server port'); + } + baseUrl = `http://127.0.0.1:${address.port}`; + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); beforeEach(() => { tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-shared-routes-test-')); originalCcsHome = process.env.CCS_HOME; + originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR; process.env.CCS_HOME = tempHome; + delete process.env.CLAUDE_CONFIG_DIR; ccsDir = path.join(tempHome, '.ccs'); - fs.mkdirSync(path.join(ccsDir, 'shared', 'skills'), { recursive: true }); - fs.mkdirSync(path.join(ccsDir, 'shared', 'agents'), { recursive: true }); + fs.mkdirSync(path.join(ccsDir, 'shared'), { recursive: true }); }); afterEach(() => { @@ -46,88 +98,233 @@ describe('web-server shared-routes', () => { delete process.env.CCS_HOME; } + if (originalClaudeConfigDir !== undefined) { + process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir; + } else { + delete process.env.CLAUDE_CONFIG_DIR; + } + if (tempHome && fs.existsSync(tempHome)) { fs.rmSync(tempHome, { recursive: true, force: true }); } }); - it('lists symlinked skill directories', () => { + it('lists symlinked skill directories', async () => { const sharedSkillsDir = path.join(ccsDir, 'shared', 'skills'); - const targetDir = path.join(tempHome, 'skill-targets', 'my-skill'); + fs.mkdirSync(sharedSkillsDir, { recursive: true }); + const targetDir = path.join(ccsDir, '.claude', 'skills', 'my-skill'); fs.mkdirSync(targetDir, { recursive: true }); fs.writeFileSync(path.join(targetDir, 'SKILL.md'), 'name: my-skill\n\nMy test skill'); const linkPath = path.join(sharedSkillsDir, 'my-skill'); - const symlinkType = process.platform === 'win32' ? 'junction' : 'dir'; - fs.symlinkSync(targetDir, linkPath, symlinkType as fs.symlink.Type); + createDirectorySymlink(targetDir, linkPath); - const handler = getGetRouteHandler('/skills'); - let payload: { items: Array<{ name: string; type: string }> } | undefined; + const payload = await getJson<{ + items: Array<{ name: string; type: string; description: string; path: string }>; + }>(baseUrl, '/api/shared/skills'); - handler( - {}, - { - json: (data: { items: Array<{ name: string; type: string }> }) => { - payload = data; - }, - } - ); - - expect(payload).toBeDefined(); - expect(payload?.items).toHaveLength(1); - expect(payload?.items[0]).toMatchObject({ + expect(payload.items).toHaveLength(1); + expect(payload.items[0]).toMatchObject({ name: 'my-skill', type: 'skill', }); + expect(payload.items[0].description.length).toBeGreaterThan(0); + expect(payload.items[0].path).toBe(linkPath); }); - it('lists symlinked agent directories', () => { + it('lists symlinked agent directories', async () => { const sharedAgentsDir = path.join(ccsDir, 'shared', 'agents'); - const targetDir = path.join(tempHome, 'agent-targets', 'my-agent'); + fs.mkdirSync(sharedAgentsDir, { recursive: true }); + const targetDir = path.join(ccsDir, '.claude', 'agents', 'my-agent'); fs.mkdirSync(targetDir, { recursive: true }); fs.writeFileSync(path.join(targetDir, 'prompt.md'), 'My test agent prompt'); const linkPath = path.join(sharedAgentsDir, 'my-agent'); - const symlinkType = process.platform === 'win32' ? 'junction' : 'dir'; - fs.symlinkSync(targetDir, linkPath, symlinkType as fs.symlink.Type); + createDirectorySymlink(targetDir, linkPath); - const handler = getGetRouteHandler('/agents'); - let payload: { items: Array<{ name: string; type: string }> } | undefined; + const payload = await getJson<{ + items: Array<{ name: string; type: string; description: string; path: string }>; + }>(baseUrl, '/api/shared/agents'); - handler( - {}, - { - json: (data: { items: Array<{ name: string; type: string }> }) => { - payload = data; - }, - } - ); - - expect(payload).toBeDefined(); - expect(payload?.items).toHaveLength(1); - expect(payload?.items[0]).toMatchObject({ + expect(payload.items).toHaveLength(1); + expect(payload.items[0]).toMatchObject({ name: 'my-agent', type: 'agent', }); + expect(payload.items[0].description.length).toBeGreaterThan(0); + expect(payload.items[0].path).toBe(linkPath); }); - it('ignores markdown files in shared skills root', () => { + it('ignores markdown files in shared skills root', async () => { const sharedSkillsDir = path.join(ccsDir, 'shared', 'skills'); + fs.mkdirSync(sharedSkillsDir, { recursive: true }); fs.writeFileSync(path.join(sharedSkillsDir, 'CLAUDE.md'), 'not a skill directory'); - const handler = getGetRouteHandler('/skills'); - let payload: { items: Array<{ name: string }> } | undefined; - - handler( - {}, - { - json: (data: { items: Array<{ name: string }> }) => { - payload = data; - }, - } + const payload = await getJson<{ items: Array<{ name: string }> }>( + baseUrl, + '/api/shared/skills' ); + expect(payload.items).toEqual([]); + }); - expect(payload).toBeDefined(); - expect(payload?.items).toEqual([]); + it('ignores invalid command markdown entries and keeps valid files', async () => { + const commandsDir = path.join(ccsDir, 'shared', 'commands'); + fs.mkdirSync(commandsDir, { recursive: true }); + fs.writeFileSync(path.join(commandsDir, 'build.md'), 'Run build command'); + fs.mkdirSync(path.join(commandsDir, 'directory.md'), { recursive: true }); + + const linkedDirTarget = path.join(tempHome, 'linked-command-dir.md'); + fs.mkdirSync(linkedDirTarget, { recursive: true }); + const linkedDirPath = path.join(commandsDir, 'linked-dir.md'); + createDirectorySymlink(linkedDirTarget, linkedDirPath); + + const payload = await getJson<{ + items: Array<{ name: string; type: string; description: string; path: string }>; + }>(baseUrl, '/api/shared/commands'); + + expect(payload.items).toEqual([ + { + name: 'build', + type: 'command', + description: 'Run build command', + path: path.join(commandsDir, 'build.md'), + }, + ]); + }); + + it('ignores skill symlink targets outside allowed roots', async () => { + const skillsDir = path.join(ccsDir, 'shared', 'skills'); + fs.mkdirSync(skillsDir, { recursive: true }); + + const outsideSkillDir = path.join(tempHome, 'external-skills', 'outside-skill'); + fs.mkdirSync(outsideSkillDir, { recursive: true }); + fs.writeFileSync(path.join(outsideSkillDir, 'SKILL.md'), 'Outside skill should be ignored'); + + const linkPath = path.join(skillsDir, 'outside-skill'); + createDirectorySymlink(outsideSkillDir, linkPath); + + const payload = await getJson<{ items: Array<{ name: string }> }>( + baseUrl, + '/api/shared/skills' + ); + expect(payload.items).toEqual([]); + }); + + it('ignores agent symlink targets outside allowed roots', async () => { + const agentsDir = path.join(ccsDir, 'shared', 'agents'); + fs.mkdirSync(agentsDir, { recursive: true }); + + const outsideAgentDir = path.join(tempHome, 'external-agents', 'outside-agent'); + fs.mkdirSync(outsideAgentDir, { recursive: true }); + fs.writeFileSync(path.join(outsideAgentDir, 'prompt.md'), 'Outside agent should be ignored'); + + const linkPath = path.join(agentsDir, 'outside-agent'); + createDirectorySymlink(outsideAgentDir, linkPath); + + const payload = await getJson<{ items: Array<{ name: string }> }>( + baseUrl, + '/api/shared/agents' + ); + expect(payload.items).toEqual([]); + }); + + it('ignores symlinked SKILL.md that escapes an allowed skill directory', async () => { + const skillsDir = path.join(ccsDir, 'shared', 'skills'); + fs.mkdirSync(skillsDir, { recursive: true }); + + const entryTargetDir = path.join(ccsDir, '.claude', 'skills', 'safe-skill'); + fs.mkdirSync(entryTargetDir, { recursive: true }); + + const outsideMarkdown = path.join(tempHome, 'outside-skill.md'); + fs.writeFileSync(outsideMarkdown, 'Leaked content should never be read'); + createFileSymlink(outsideMarkdown, path.join(entryTargetDir, 'SKILL.md')); + + createDirectorySymlink(entryTargetDir, path.join(skillsDir, 'safe-skill')); + + const payload = await getJson<{ items: Array<{ name: string }> }>( + baseUrl, + '/api/shared/skills' + ); + expect(payload.items).toEqual([]); + }); + + it('ignores symlinked command markdown targets outside commands root', async () => { + const commandsDir = path.join(ccsDir, 'shared', 'commands'); + fs.mkdirSync(commandsDir, { recursive: true }); + + const outsideMarkdown = path.join(tempHome, 'outside-command.md'); + fs.writeFileSync(outsideMarkdown, 'Outside command should not be read'); + createFileSymlink(outsideMarkdown, path.join(commandsDir, 'outside.md')); + + const payload = await getJson<{ items: Array<{ name: string }> }>( + baseUrl, + '/api/shared/commands' + ); + expect(payload.items).toEqual([]); + }); + + it('summary uses CLAUDE_CONFIG_DIR for symlink status and counts', async () => { + const sharedDir = path.join(ccsDir, 'shared'); + const claudeConfigDir = path.join(tempHome, 'custom-claude'); + process.env.CLAUDE_CONFIG_DIR = claudeConfigDir; + + for (const linkType of ['commands', 'skills', 'agents']) { + const targetDir = path.join(claudeConfigDir, linkType); + fs.mkdirSync(targetDir, { recursive: true }); + + const linkPath = path.join(sharedDir, linkType); + createDirectorySymlink(targetDir, linkPath); + } + + fs.writeFileSync(path.join(claudeConfigDir, 'commands', 'lint.md'), 'Run lint command'); + + const skillDir = path.join(claudeConfigDir, 'skills', 'custom-skill'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), 'Custom skill description'); + + const agentDir = path.join(claudeConfigDir, 'agents', 'custom-agent'); + fs.mkdirSync(agentDir, { recursive: true }); + fs.writeFileSync(path.join(agentDir, 'prompt.md'), 'Custom agent prompt'); + + const payload = await getJson<{ + commands: number; + skills: number; + agents: number; + total: number; + symlinkStatus: { valid: boolean; message: string }; + }>(baseUrl, '/api/shared/summary'); + + expect(payload.commands).toBe(1); + expect(payload.skills).toBe(1); + expect(payload.agents).toBe(1); + expect(payload.total).toBe(3); + expect(payload.symlinkStatus).toEqual({ + valid: true, + message: 'Symlinks active', + }); + }); + + it('summary uses CCS_HOME fallback Claude path when CLAUDE_CONFIG_DIR is unset', async () => { + const sharedDir = path.join(ccsDir, 'shared'); + const claudeConfigDir = path.join(tempHome, '.claude'); + + for (const linkType of ['commands', 'skills', 'agents']) { + const targetDir = path.join(claudeConfigDir, linkType); + fs.mkdirSync(targetDir, { recursive: true }); + createDirectorySymlink(targetDir, path.join(sharedDir, linkType)); + } + + fs.writeFileSync(path.join(claudeConfigDir, 'commands', 'test.md'), 'Command description'); + + const payload = await getJson<{ + symlinkStatus: { valid: boolean; message: string }; + commands: number; + }>(baseUrl, '/api/shared/summary'); + + expect(payload.commands).toBe(1); + expect(payload.symlinkStatus).toEqual({ + valid: true, + message: 'Symlinks active', + }); }); });