diff --git a/skill/scripts/live-server.mjs b/skill/scripts/live-server.mjs index c6e951086..e2eb871da 100644 --- a/skill/scripts/live-server.mjs +++ b/skill/scripts/live-server.mjs @@ -846,7 +846,13 @@ function createRequestHandler({ detectScript, liveScriptParts }) { const filePath = url.searchParams.get('path'); if (!filePath || filePath.includes('..')) { res.writeHead(400); res.end('Bad path'); return; } const absPath = path.resolve(process.cwd(), filePath); - if (!absPath.startsWith(process.cwd())) { res.writeHead(403); res.end('Forbidden'); return; } + // Confine to the project root. A bare `startsWith(cwd)` string check lets a + // sibling dir whose name extends the root name (projeto -> projeto-backup) + // slip through; compare on the relative path instead (same pattern as + // sessionFileMetadataFromPollReply below). An empty rel means the request + // resolved to the root directory itself, which this file route never serves. + const rel = path.relative(process.cwd(), absPath); + if (!rel || rel.startsWith('..') || path.isAbsolute(rel)) { res.writeHead(403); res.end('Forbidden'); return; } let content; try { content = fs.readFileSync(absPath, 'utf-8'); } catch { res.writeHead(404); res.end('File not found'); return; } diff --git a/tests/live-server.test.mjs b/tests/live-server.test.mjs index 29ba5adc2..8adb5f089 100644 --- a/tests/live-server.test.mjs +++ b/tests/live-server.test.mjs @@ -5,7 +5,7 @@ import { describe, it, before, after } from 'node:test'; import assert from 'node:assert/strict'; -import { existsSync, mkdtempSync, readFileSync, writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, writeFileSync, mkdirSync, rmSync, realpathSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { execFileSync, execSync, spawn } from 'node:child_process'; @@ -3080,6 +3080,48 @@ colors: {} } }); + it('/source rejects an absolute path to a sibling directory sharing the root prefix', async () => { + // Sibling dir whose name extends the project dir name (projeto -> projeto-evil): + // a plain string prefix check on the resolved path lets it escape the root. + // Build it off the server's real cwd (macOS symlinks /var -> /private/var, + // and the server guards against its own process.cwd(), i.e. the realpath). + const siblingDir = realpathSync(serverCwd) + '-evil'; + mkdirSync(siblingDir, { recursive: true }); + const secretPath = join(siblingDir, 'secret.txt'); + writeFileSync(secretPath, 'TOP SECRET SIBLING'); + try { + const res = await fetch(`http://localhost:${server.port}/source?token=${server.token}&path=${encodeURIComponent(secretPath)}`); + // Drain the body so the socket doesn't hang regardless of status. + await res.text().catch(() => {}); + assert.equal(res.status, 403); + } finally { + rmSync(siblingDir, { recursive: true, force: true }); + } + }); + + it('/source rejects the project root itself (directory, not a file)', async () => { + // `.` resolves exactly to cwd; the route only serves files, so an empty + // relative path is not a legitimate request and must be forbidden. + const res = await fetch(`http://localhost:${server.port}/source?token=${server.token}&path=${encodeURIComponent('.')}`); + await res.text().catch(() => {}); + assert.equal(res.status, 403); + }); + + it('/source still serves a legitimate nested in-root file', async () => { + const nestedDir = join(serverCwd, 'nested'); + mkdirSync(nestedDir, { recursive: true }); + const nestedPath = join(nestedDir, 'page.html'); + writeFileSync(nestedPath, '

in root

\n'); + try { + const res = await fetch(`http://localhost:${server.port}/source?token=${server.token}&path=${encodeURIComponent('nested/page.html')}`); + assert.equal(res.status, 200); + const text = await res.text(); + assert.ok(text.includes('in root')); + } finally { + rmSync(nestedDir, { recursive: true, force: true }); + } + }); + it('/modern-screenshot.js serves the vendored UMD build', async () => { const res = await fetch(`http://localhost:${server.port}/modern-screenshot.js`); assert.equal(res.status, 200);