feat: Enhance lite-skill-generator with single file output and improved validation

Author: catlog22

ccw/src/core/routes/system-routes.ts

@@ -286,19 +286,35 @@ export async function handleSystemRoutes(ctx: SystemRouteContext): Promise<boole
       return true;
     }
 
+    let validatedPath: string;
     try {
       // Validate path is within allowed directories (fix: sec-001-a1b2c3d4)
-      const validatedPath = await validateAllowedPath(filePath, {
-        mustExist: true,
-        allowedDirectories: [process.cwd(), resolvePath('.ccw', 'sessions')]
-      });
+      validatedPath = await validateAllowedPath(filePath, { mustExist: true, allowedDirectories: [initialPath] });
+    } catch (err) {
+      const message = err instanceof Error ? err.message : String(err);
+      const status = message.includes('Access denied') ? 403 : (message.includes('File not found') ? 404 : 400);
+      console.error(`[System] Path validation failed: ${message}`);
+      res.writeHead(status, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({
+        error: status === 403 ? 'Access denied' : (status === 404 ? 'File not found' : 'Invalid path')
+      }));
+      return true;
+    }
+
+    try {
       const content = await fsPromises.readFile(validatedPath, 'utf-8');
       const json = JSON.parse(content);
       res.writeHead(200, { 'Content-Type': 'application/json' });
       res.end(JSON.stringify(json));
-    } catch (err) {
-      res.writeHead(404, { 'Content-Type': 'application/json' });
-      res.end(JSON.stringify({ error: 'File not found or invalid JSON' }));
+    } catch (err: unknown) {
+      const errno = typeof err === 'object' && err !== null && 'code' in err ? String((err as any).code) : null;
+      const status = err instanceof SyntaxError ? 400 : (errno === 'EACCES' ? 403 : 404);
+      const message = err instanceof Error ? err.message : String(err);
+      console.error(`[System] Failed to read JSON file: ${message}`);
+      res.writeHead(status, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({
+        error: status === 403 ? 'Access denied' : (status === 400 ? 'Invalid JSON' : 'File not found')
+      }));
     }
     return true;
   }
@@ -449,12 +465,16 @@ export async function handleSystemRoutes(ctx: SystemRouteContext): Promise<boole
       }
 
       // Validate path is within allowed directories (fix: sec-003-c3d4e5f6)
-      const initialPath = process.cwd();
-      if (browsePath) {
+      try {
         targetPath = await validateAllowedPath(targetPath, {
           mustExist: true,
           allowedDirectories: [initialPath, os.homedir()]
         });
+      } catch (err) {
+        const message = err instanceof Error ? err.message : String(err);
+        const status = message.includes('Access denied') ? 403 : 400;
+        console.error(`[System] Path validation failed: ${message}`);
+        return { error: status === 403 ? 'Access denied' : 'Invalid path', status };
       }
 
       try {
@@ -486,7 +506,9 @@ export async function handleSystemRoutes(ctx: SystemRouteContext): Promise<boole
           homePath: os.homedir()
         };
       } catch (err) {
-        return { error: 'Cannot access directory: ' + (err as Error).message, status: 400 };
+        const message = err instanceof Error ? err.message : String(err);
+        console.error(`[System] Failed to browse directory: ${message}`);
+        return { error: 'Cannot access directory', status: 400 };
       }
     });
     return true;
@@ -518,11 +540,17 @@ export async function handleSystemRoutes(ctx: SystemRouteContext): Promise<boole
       }
 
       // Validate path is within allowed directories (fix: sec-003-c3d4e5f6)
-      const initialPath = process.cwd();
-      targetPath = await validateAllowedPath(targetPath, {
-        mustExist: true,
-        allowedDirectories: [initialPath, os.homedir()]
-      });
+      try {
+        targetPath = await validateAllowedPath(targetPath, {
+          mustExist: true,
+          allowedDirectories: [initialPath, os.homedir()]
+        });
+      } catch (err) {
+        const message = err instanceof Error ? err.message : String(err);
+        const status = message.includes('Access denied') ? 403 : 400;
+        console.error(`[System] Path validation failed: ${message}`);
+        return { error: status === 403 ? 'Access denied' : 'Invalid path', status };
+      }
 
       try {
         await fs.promises.access(targetPath, fs.constants.R_OK);
@@ -534,8 +562,10 @@ export async function handleSystemRoutes(ctx: SystemRouteContext): Promise<boole
           isFile: stat.isFile(),
           isDirectory: stat.isDirectory()
         };
-      } catch {
-        return { error: 'File not accessible', status: 404 };
+      } catch (err: unknown) {
+        const errno = typeof err === 'object' && err !== null && 'code' in err ? String((err as any).code) : null;
+        const status = errno === 'EACCES' ? 403 : 404;
+        return { error: status === 403 ? 'Access denied' : 'File not accessible', status };
       }
     });
     return true;

ccw/tests/integration/system-routes.test.ts

@@ -9,12 +9,13 @@
 import { after, before, describe, it, mock } from 'node:test';
 import assert from 'node:assert/strict';
 import http from 'node:http';
-import { mkdtempSync, rmSync } from 'node:fs';
-import { tmpdir } from 'node:os';
-import { join } from 'node:path';
+import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs';
+import { tmpdir, homedir } from 'node:os';
+import { join, parse } from 'node:path';
 
 const CCW_HOME = mkdtempSync(join(tmpdir(), 'ccw-system-routes-home-'));
 const PROJECT_ROOT = mkdtempSync(join(tmpdir(), 'ccw-system-routes-project-'));
+const OUTSIDE_ROOT = mkdtempSync(join(tmpdir(), 'ccw-system-routes-outside-'));
 
 const systemRoutesUrl = new URL('../../dist/core/routes/system-routes.js', import.meta.url);
 systemRoutesUrl.searchParams.set('t', String(Date.now()));
@@ -140,6 +141,7 @@ describe('system routes integration', async () => {
     process.env.CCW_DATA_DIR = originalEnv.CCW_DATA_DIR;
     rmSync(CCW_HOME, { recursive: true, force: true });
     rmSync(PROJECT_ROOT, { recursive: true, force: true });
+    rmSync(OUTSIDE_ROOT, { recursive: true, force: true });
   });
 
   it('GET /api/health returns ok payload', async () => {
@@ -237,5 +239,60 @@ describe('system routes integration', async () => {
       await new Promise<void>((resolve) => originalClose(() => resolve()));
     }
   });
-});
 
+  it('GET /api/file reads JSON within initialPath and rejects outside paths', async () => {
+    const insideFile = join(PROJECT_ROOT, '.review', 'fixes', 'active-fix-session.json');
+    mkdirSync(join(PROJECT_ROOT, '.review', 'fixes'), { recursive: true });
+    writeFileSync(insideFile, JSON.stringify({ ok: true }), 'utf8');
+
+    const outsideFile = join(OUTSIDE_ROOT, 'outside.json');
+    writeFileSync(outsideFile, JSON.stringify({ ok: true }), 'utf8');
+
+    const { server, baseUrl } = await createServer(PROJECT_ROOT);
+    try {
+      const ok = await requestJson(baseUrl, 'GET', `/api/file?path=${encodeURIComponent(insideFile)}`);
+      assert.equal(ok.status, 200);
+      assert.equal(ok.json.ok, true);
+
+      const denied = await requestJson(baseUrl, 'GET', `/api/file?path=${encodeURIComponent(outsideFile)}`);
+      assert.equal(denied.status, 403);
+      assert.equal(denied.json.error, 'Access denied');
+    } finally {
+      await new Promise<void>((resolve) => server.close(() => resolve()));
+    }
+  });
+
+  it('POST /api/dialog/browse rejects paths outside allowed roots', async () => {
+    const { server, baseUrl } = await createServer(PROJECT_ROOT);
+    try {
+      const rootPath = parse(homedir()).root;
+      const denied = await requestJson(baseUrl, 'POST', '/api/dialog/browse', { path: rootPath, showHidden: true });
+      assert.equal(denied.status, 403);
+      assert.equal(denied.json.error, 'Access denied');
+    } finally {
+      await new Promise<void>((resolve) => server.close(() => resolve()));
+    }
+  });
+
+  it('POST /api/dialog/open-file accepts files under initialPath and rejects outside paths', async () => {
+    const allowedFile = join(PROJECT_ROOT, 'allowed.txt');
+    writeFileSync(allowedFile, 'ok', 'utf8');
+
+    const rootPath = parse(homedir()).root;
+    const deniedPath = join(rootPath, 'ccw-not-allowed.txt');
+
+    const { server, baseUrl } = await createServer(PROJECT_ROOT);
+    try {
+      const ok = await requestJson(baseUrl, 'POST', '/api/dialog/open-file', { path: allowedFile });
+      assert.equal(ok.status, 200);
+      assert.equal(ok.json.success, true);
+      assert.equal(ok.json.isFile, true);
+
+      const denied = await requestJson(baseUrl, 'POST', '/api/dialog/open-file', { path: deniedPath });
+      assert.equal(denied.status, 403);
+      assert.equal(denied.json.error, 'Access denied');
+    } finally {
+      await new Promise<void>((resolve) => server.close(() => resolve()));
+    }
+  });
+});