diff --git a/src/server/api.ts b/src/server/api.ts index 1f764bf..17e4dc1 100644 --- a/src/server/api.ts +++ b/src/server/api.ts @@ -28,6 +28,7 @@ import { parseUpdateWorkspacePageRequest, parseUpdateWorkspaceSeedSelectionRequest, parseWorkspaceId, + parseRunId, } from './validation.js'; import { startGeneration } from './generation.js'; import { readRunAsset } from './assets.js'; @@ -747,19 +748,19 @@ export const handleApiRequest: ApiHandler = async (request, response) => { const runMatch = /^\/api\/runs\/([^/]+)$/.exec(url.pathname); if (request.method === 'GET' && runMatch) { - sendJson(response, 200, { run: await readRun(decodeURIComponent(runMatch[1])) }); + sendJson(response, 200, { run: await readRun(parseRunId(runMatch[1])) }); return true; } const eventsMatch = /^\/api\/runs\/([^/]+)\/events$/.exec(url.pathname); if (request.method === 'GET' && eventsMatch) { - await streamSseRun(decodeURIComponent(eventsMatch[1]), request, response); + await streamSseRun(parseRunId(eventsMatch[1]), request, response); return true; } const assetMatch = /^\/api\/runs\/([^/]+)\/assets\/(.+)$/.exec(url.pathname); if (request.method === 'GET' && assetMatch) { - const asset = await readRunAsset(decodeURIComponent(assetMatch[1]), decodeURIComponent(assetMatch[2])); + const asset = await readRunAsset(parseRunId(assetMatch[1]), decodeURIComponent(assetMatch[2])); response.writeHead(200, { 'content-type': asset.contentType, 'cache-control': 'no-store', @@ -771,13 +772,13 @@ export const handleApiRequest: ApiHandler = async (request, response) => { const handoverMatch = /^\/api\/runs\/([^/]+)\/handover$/.exec(url.pathname); if (request.method === 'POST' && handoverMatch) { - await handleHandover(response, decodeURIComponent(handoverMatch[1]), await readJsonBody(request)); + await handleHandover(response, parseRunId(handoverMatch[1]), await readJsonBody(request)); return true; } const handoverAssetMatch = /^\/api\/runs\/([^/]+)\/handovers\/([^/]+)\/(handover\.(?:md|html))$/.exec(url.pathname); if (request.method === 'GET' && handoverAssetMatch) { - const run = await readRun(decodeURIComponent(handoverAssetMatch[1])); + const run = await readRun(parseRunId(handoverAssetMatch[1])); const designId = decodeURIComponent(handoverAssetMatch[2]); const asset = handoverAssetMatch[3] as 'handover.md' | 'handover.html'; const handover = run.handovers.find((entry) => entry.designId === designId); @@ -812,7 +813,7 @@ export const handleApiRequest: ApiHandler = async (request, response) => { const nestedHandoverAssetMatch = /^\/api\/runs\/([^/]+)\/handovers\/([^/]+)\/assets\/(.+)$/.exec(url.pathname); if (request.method === 'GET' && nestedHandoverAssetMatch) { - const run = await readRun(decodeURIComponent(nestedHandoverAssetMatch[1])); + const run = await readRun(parseRunId(nestedHandoverAssetMatch[1])); const designId = decodeURIComponent(nestedHandoverAssetMatch[2]); const assetId = decodeURIComponent(nestedHandoverAssetMatch[3]); const handover = run.handovers.find((entry) => entry.designId === designId); diff --git a/src/server/assets.test.ts b/src/server/assets.test.ts new file mode 100644 index 0000000..96a6dd7 --- /dev/null +++ b/src/server/assets.test.ts @@ -0,0 +1,44 @@ +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import path from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { readRunAsset } from './assets.js'; +import { runDir } from './runStore.js'; + +const testRunId = '11111111-1111-4111-8111-111111111111'; +const runDirs: string[] = []; + +const createAsset = async (relativePath: string, contents: string): Promise => { + const dir = runDir(testRunId); + runDirs.push(dir); + const absolute = path.join(dir, relativePath); + await mkdir(path.dirname(absolute), { recursive: true }); + await writeFile(absolute, contents, 'utf8'); +}; + +describe('run assets', () => { + afterEach(async () => { + await Promise.all(runDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); + }); + + it('reads generated assets below the run asset directory', async () => { + await createAsset('assets/design-1.png', 'png-bytes'); + + await expect(readRunAsset(testRunId, 'assets/design-1.png')).resolves.toMatchObject({ + bytes: Buffer.from('png-bytes'), + contentType: 'image/png', + }); + }); + + it('rejects asset paths outside the run asset directory', async () => { + await createAsset('assets/design-1.png', 'png-bytes'); + + await expect(readRunAsset(testRunId, '../run.json')).rejects.toThrow('A valid run asset path is required.'); + await expect(readRunAsset(testRunId, 'assets/../../run.json')).rejects.toThrow('A valid run asset path is required.'); + await expect(readRunAsset(testRunId, '/etc/passwd')).rejects.toThrow('A valid run asset path is required.'); + }); + + it('rejects invalid run ids before building filesystem paths', async () => { + await expect(readRunAsset('x', 'assets/design-1.png')).rejects.toThrow('A valid run id is required.'); + await expect(readRunAsset('../../../../etc', 'assets/design-1.png')).rejects.toThrow('A valid run id is required.'); + }); +}); diff --git a/src/server/assets.ts b/src/server/assets.ts index 9412d4a..74d7cb8 100644 --- a/src/server/assets.ts +++ b/src/server/assets.ts @@ -66,13 +66,35 @@ export const saveImageData = async ( }; }; +const resolveRunAssetPath = (runId: string, assetPath: string): { absolute: string; relative: string } => { + const relative = assetPath.trim().replace(/^assets[\\/]+/, ''); + const segments = relative.split(/[\\/]+/).filter(Boolean); + if ( + !relative + || path.isAbsolute(relative) + || path.win32.isAbsolute(relative) + || segments.some((segment) => segment === '..' || segment === '.') + ) { + throw new Error('A valid run asset path is required.'); + } + + const assetRoot = path.resolve(runDir(runId), 'assets'); + const absolute = path.resolve(assetRoot, ...segments); + const assetRootWithSeparator = `${assetRoot}${path.sep}`; + if (absolute !== assetRoot && !absolute.startsWith(assetRootWithSeparator)) { + throw new Error('A valid run asset path is required.'); + } + + return { absolute, relative: segments.join('/') }; +}; + export const readRunAsset = async ( runId: string, assetPath: string, ): Promise<{ bytes: Buffer; contentType: string }> => { - const absolute = path.join(runDir(runId), assetPath); + const { absolute, relative } = resolveRunAssetPath(runId, assetPath); const bytes = await readFile(absolute); - const extension = path.extname(assetPath).toLowerCase(); + const extension = path.extname(relative).toLowerCase(); const contentType = extension === '.jpg' || extension === '.jpeg' ? 'image/jpeg' : extension === '.webp' diff --git a/src/server/runStore.ts b/src/server/runStore.ts index b8ed2d4..156c1d1 100644 --- a/src/server/runStore.ts +++ b/src/server/runStore.ts @@ -9,6 +9,7 @@ import type { RunStatus, } from '../shared/types.js'; import { runsRoot, serverConfig } from './config.js'; +import { parseRunId } from './validation.js'; type RunPatch = Partial>; @@ -24,7 +25,7 @@ const withRunLock = async (runId: string, operation: () => Promise): Prom return next; }; -export const runDir = (runId: string): string => path.join(runsRoot, runId); +export const runDir = (runId: string): string => path.join(runsRoot, parseRunId(runId)); export const runJsonPath = (runId: string): string => path.join(runDir(runId), 'run.json'); export const ensureRunsRoot = async (): Promise => { diff --git a/src/server/validation.test.ts b/src/server/validation.test.ts index 31d0669..e417a0f 100644 --- a/src/server/validation.test.ts +++ b/src/server/validation.test.ts @@ -8,6 +8,7 @@ import { parseDirectCreateHandoverRequest, parsePlanWorkspacePagesRequest, parseDesignId, + parseRunId, } from './validation.js'; describe('validation', () => { @@ -78,6 +79,12 @@ describe('validation', () => { expect(() => parseDesignId('../design-1')).toThrow('A valid designId is required.'); }); + it('validates run ids', () => { + expect(parseRunId('11111111-1111-4111-8111-111111111111')).toBe('11111111-1111-4111-8111-111111111111'); + expect(() => parseRunId('x')).toThrow('A valid run id is required.'); + expect(() => parseRunId('%2e%2e%2f%2e%2e%2fetc')).toThrow('A valid run id is required.'); + }); + it('parses create workspaces with seed variation count', () => { expect(parseCreateWorkspaceRequest({ prompt: 'Create UI', diff --git a/src/server/validation.ts b/src/server/validation.ts index d110ca3..76f2f2e 100644 --- a/src/server/validation.ts +++ b/src/server/validation.ts @@ -158,6 +158,15 @@ export const parseDesignId = (value: unknown): string => { return designId; }; + +export const parseRunId = (value: string): string => { + const runId = decodeURIComponent(value).trim(); + if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(runId)) { + throw new Error('A valid run id is required.'); + } + return runId; +}; + export const parseWorkspaceId = (value: string): string => { const workspaceId = decodeURIComponent(value).trim(); if (!/^[a-z0-9-]{8,80}$/i.test(workspaceId)) {