From 8dd610f574e2c1f7d3ade790001f03e2d4661391 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:15:30 -0400 Subject: [PATCH] fix(plan-canvas): harden PDF export concurrency --- docs/design/plan-canvas.md | 14 ++- docs/testing/plan-canvas-pdf-export.tdd.md | 20 ++-- scripts/lib/plan-canvas/pdf.js | 40 +++++++- scripts/lib/plan-canvas/sdk.js | 15 +++ scripts/lib/plan-canvas/server.js | 58 +++++++++-- scripts/lib/plan-canvas/ui.js | 30 +++++- scripts/plan-canvas.js | 108 +++++++++++++++++---- tests/integration/plan-canvas-e2e.test.js | 29 +++++- tests/scripts/plan-canvas-pdf.test.js | 29 +++--- tests/scripts/plan-canvas.test.js | 49 +++++++++- 10 files changed, 335 insertions(+), 57 deletions(-) diff --git a/docs/design/plan-canvas.md b/docs/design/plan-canvas.md index e3a3d0f0e..077110343 100644 --- a/docs/design/plan-canvas.md +++ b/docs/design/plan-canvas.md @@ -82,7 +82,7 @@ after 30 min, `ECC_PLAN_CANVAS_IDLE_MS`). Feedback is deliver-and-drain: queued handed to exactly one `await` call and persisted to disk until then, so nothing is lost if the poll is interrupted. -- `GET /health` — `{ok, app, version, protocolVersion, runtimeId}`; the CLI reuses a detached server only when its package, protocol, and Canvas-module fingerprint match, preventing an older same-version worktree from serving stale browser code +- `GET /health` — `{ok, app, version, protocolVersion, runtimeId}`; the CLI reuses a detached server only when its package, protocol, and Canvas-module fingerprint match, preventing an older same-version worktree from serving stale browser code. A per-user, port-scoped startup lock serializes compatibility checks and replacement so concurrent opens reuse the winning server instead of racing two detached launches. - `GET /` — session list (ECC chrome) - `POST /api/sessions` `{file, reopen?}` — open/resume; `409 user-ended` unless `reopen` - `GET /canvas/` — editor chrome; `GET /artifact//` — rendered artifact @@ -127,7 +127,11 @@ unavailable, and can be repointed at a local mirror via PDF export also stays local. The server launches an installed Chrome, Chromium, or Edge executable with a new temporary profile, restricts the print target to the loopback Canvas -origin, waits for a complete `%PDF` document, terminates that private renderer, and removes -its temporary profile. Set `ECC_PLAN_CANVAS_CHROME_PATH` when auto-discovery cannot find the -browser. The export endpoint returns an actionable error instead of uploading the plan or -silently falling back to a remote service. +origin, applies an export-only CSP that disables scripts and outbound resource classes, and +routes every non-origin browser request into a local deny proxy. It waits for a complete +`%PDF` document, terminates that private renderer, closes the deny proxy, and removes its +temporary profile. One renderer is admitted at a time; overlapping requests receive HTTP 429 +with `Retry-After: 1` instead of launching unbounded browser processes. Set +`ECC_PLAN_CANVAS_CHROME_PATH` when auto-discovery cannot find the browser. The export endpoint +returns an actionable error instead of uploading the plan or silently falling back to a +remote service. diff --git a/docs/testing/plan-canvas-pdf-export.tdd.md b/docs/testing/plan-canvas-pdf-export.tdd.md index 0da4bd212..18ac1c295 100644 --- a/docs/testing/plan-canvas-pdf-export.tdd.md +++ b/docs/testing/plan-canvas-pdf-export.tdd.md @@ -14,17 +14,20 @@ artifact as a real PDF file without sending the plan to an external converter. | Only a loopback artifact URL can reach the renderer | `assertLoopbackUrl` tests | PASS | | Filenames are useful and safe across platforms | `pdfFileName` tests | PASS | | Incomplete output is never served as a PDF | `%PDF` header and `%%EOF` completion tests | PASS | +| Validation uses the opened file handle | File-descriptor regression test and CodeQL rerun | PASS | | Renderer state is private and temporary | Fake-process lifecycle test and live process/temp-state inspection | PASS | +| Artifact HTML cannot make outbound export requests | PDF-only CSP and loopback-origin deny-proxy regression tests | PASS | +| Browser renderer concurrency is bounded | Concurrent endpoint test expects HTTP 429 and `Retry-After` | PASS | | Export adds no cloud converter or npm runtime dependency | Implementation and package diff inspection | PASS | ## Red and green - RED: the focused server suite produced 30 passes and 2 failures because the Canvas had no Download PDF control or PDF endpoint. -- GREEN: renderer unit tests pass 6/6, Plan Canvas server tests pass 32/32, - and the end-to-end review workflow passes 10/10. -- FULL SUITE: `npm test` passes all 4,003 discovered tests; full ESLint, - Markdown lint, package dry-run, and `git diff --check` also pass. +- GREEN: renderer unit tests pass 7/7, Plan Canvas server tests pass 34/34, + and the end-to-end review workflow passes 11/11. +- FULL SUITE: the final review-hardened implementation passes all 4,007 + discovered tests; hosted security reruns are recorded on PR #2894. - COVERAGE: `npm run coverage` passes 4,003/4,003 with 88.97% statements, 80.58% branches, 94.22% functions, and 88.97% lines. The Plan Canvas module group reaches 96.72% statements and lines, 84.47% branches, and @@ -42,9 +45,12 @@ artifact as a real PDF file without sending the plan to an external converter. The loopback server discovers Google Chrome, Chromium, or Microsoft Edge, or uses `ECC_PLAN_CANVAS_CHROME_PATH`. It launches the executable without a shell, -with a private temporary profile and a loopback-only artifact URL. Completion -requires both a `%PDF-` header and `%%EOF` marker. The renderer is terminated and -temporary state is removed before the response is handed to the browser. +with a private temporary profile, a loopback-only artifact URL, an export-only +CSP, and a local deny proxy that allows only the exact Canvas origin. Completion +requires both a `%PDF-` header and `%%EOF` marker read from the already-open file +handle. The renderer is terminated and temporary state is removed before the +response is handed to the browser. Concurrent export attempts fail quickly with +a retryable HTTP 429 response while one renderer is active. If no renderer exists, the browser receives an actionable local error. Plan Canvas does not upload the artifact or add a hosted conversion dependency. diff --git a/scripts/lib/plan-canvas/pdf.js b/scripts/lib/plan-canvas/pdf.js index ac3828ea0..d3b370580 100644 --- a/scripts/lib/plan-canvas/pdf.js +++ b/scripts/lib/plan-canvas/pdf.js @@ -11,6 +11,7 @@ const { spawn } = require('child_process'); const fs = require('fs'); +const net = require('net'); const os = require('os'); const path = require('path'); @@ -137,6 +138,33 @@ function delay(ms) { return new Promise(resolve => setTimeout(resolve, ms)); } +function startDenyProxy(netImpl = net) { + return new Promise((resolve, reject) => { + const server = netImpl.createServer(socket => { + socket.on('error', () => {}); + socket.destroy(); + }); + const onError = error => reject(errorWithCode( + `Could not isolate PDF renderer network access (${error.message})`, + 'PDF_EXPORT_FAILED' + )); + server.once('error', onError); + server.listen(0, '127.0.0.1', () => { + server.removeListener('error', onError); + server.on('error', () => {}); + server.unref(); + resolve({ server, url: `http://127.0.0.1:${server.address().port}` }); + }); + }); +} + +function stopDenyProxy(server) { + if (!server) return Promise.resolve(); + return new Promise(resolve => { + try { server.close(resolve); } catch { resolve(); } + }); +} + async function stopChild(child) { if (!child || child.exitCode !== null || child.signalCode) return; const closed = new Promise(resolve => child.once('close', resolve)); @@ -195,9 +223,11 @@ async function exportPdf({ timeoutMs = DEFAULT_EXPORT_TIMEOUT_MS, spawnImpl = spawn, fsImpl = fs, - osImpl = os + osImpl = os, + netImpl = net } = {}) { const safeUrl = assertLoopbackUrl(url); + const exportUrl = new URL(safeUrl); const browser = executable || resolveChromiumExecutable({ env, platform, fsImpl }); if (!browser) { const override = String(env.ECC_PLAN_CANVAS_CHROME_PATH || '').trim(); @@ -214,14 +244,18 @@ async function exportPdf({ const profileDir = path.join(tempDir, 'profile'); fsImpl.mkdirSync(profileDir, { recursive: true }); let child = null; + let denyProxy = null; try { + denyProxy = await startDenyProxy(netImpl); const args = [ '--headless=new', + '--disable-background-networking', '--disable-gpu', '--disable-component-update', '--disable-default-apps', '--disable-extensions', '--disable-sync', + '--disable-quic', '--metrics-recording-only', '--mute-audio', '--no-first-run', @@ -229,6 +263,9 @@ async function exportPdf({ '--no-pdf-header-footer', '--print-to-pdf-no-header', '--hide-scrollbars', + `--proxy-server=${denyProxy.url}`, + `--proxy-bypass-list=<-loopback>;${exportUrl.origin}`, + `--host-resolver-rules=MAP * ~NOTFOUND, EXCLUDE ${exportUrl.hostname}`, `--user-data-dir=${profileDir}`, `--print-to-pdf=${outputFile}`, '--virtual-time-budget=5000', @@ -247,6 +284,7 @@ async function exportPdf({ return { buffer, filename: pdfFileName(artifactFile) }; } finally { await stopChild(child); + await stopDenyProxy(denyProxy && denyProxy.server); fsImpl.rmSync(tempDir, { recursive: true, force: true }); } } diff --git a/scripts/lib/plan-canvas/sdk.js b/scripts/lib/plan-canvas/sdk.js index a5110914d..3996446d9 100644 --- a/scripts/lib/plan-canvas/sdk.js +++ b/scripts/lib/plan-canvas/sdk.js @@ -205,6 +205,19 @@ function artifactSdkJs() { }); // --- chrome bridge --------------------------------------------------------- + function exportSnapshot() { + const clone = document.documentElement.cloneNode(true); + clone.querySelectorAll('script,iframe,object,embed,form,base,meta[http-equiv],[data-ecc-plan-canvas]').forEach(el => el.remove()); + clone.querySelectorAll('*').forEach(el => { + for (const attr of [...el.attributes]) { + if (attr.name.toLowerCase().startsWith('on') || attr.name.toLowerCase() === 'srcdoc') { + el.removeAttribute(attr.name); + } + } + }); + return '\\n' + clone.outerHTML; + } + window.addEventListener('message', e => { const msg = e.data || {}; if (msg.type === 'pc:set-mode') { @@ -212,6 +225,8 @@ function artifactSdkJs() { if (!annotate) { hl.style.display = 'none'; selhint.style.display = 'none'; closeCard(); } } else if (msg.type === 'pc:restore-scroll') { window.scrollTo(msg.x || 0, msg.y || 0); + } else if (msg.type === 'pc:export-snapshot' && typeof msg.requestId === 'string') { + post({ type: 'pc:export-snapshot-result', requestId: msg.requestId, html: exportSnapshot() }); } }); document.addEventListener('keydown', e => { diff --git a/scripts/lib/plan-canvas/server.js b/scripts/lib/plan-canvas/server.js index 7237a4da0..c93135531 100644 --- a/scripts/lib/plan-canvas/server.js +++ b/scripts/lib/plan-canvas/server.js @@ -31,6 +31,7 @@ const DEFAULT_PORT = 4517; const DEFAULT_HOST = '127.0.0.1'; const DEFAULT_IDLE_TIMEOUT_MS = 30 * 60 * 1000; const MAX_BODY_BYTES = 1024 * 1024; +const MAX_PDF_SNAPSHOT_BYTES = 5 * 1024 * 1024; // How long the "agent is thinking" indicator survives without the agent // checking back in, before presence decays to the honest queued/waiting. const DEFAULT_THINKING_STALE_MS = 90 * 1000; @@ -38,6 +39,19 @@ const DEFAULT_THINKING_STALE_MS = 90 * 1000; const DEFAULT_TYPING_EXPIRY_MS = 30 * 1000; const PLAN_CANVAS_PROTOCOL_VERSION = 4; const TYPING_STATES = new Set(['thinking', 'typing', 'idle']); +const PDF_EXPORT_CSP = [ + "default-src 'none'", + "base-uri 'none'", + "connect-src 'none'", + "font-src 'self' data:", + "form-action 'none'", + "frame-src 'none'", + "img-src 'self' data:", + "media-src 'self' data:", + "object-src 'none'", + "script-src 'none'", + "style-src 'self' 'unsafe-inline'" +].join('; '); // Package versions do not distinguish two worktrees on the same release. // Fingerprint every module loaded into the detached server so a current CLI @@ -96,13 +110,13 @@ function resolveIdleTimeoutMs(env = process.env) { return Number.isInteger(value) && value > 0 ? value : DEFAULT_IDLE_TIMEOUT_MS; } -function readJsonBody(req) { +function readJsonBody(req, maxBytes = MAX_BODY_BYTES) { return new Promise((resolve, reject) => { let size = 0; const chunks = []; req.on('data', chunk => { size += chunk.length; - if (size > MAX_BODY_BYTES) { + if (size > maxBytes) { reject(new Error('body too large')); req.destroy(); return; @@ -130,8 +144,9 @@ function sendJson(res, statusCode, payload) { function sendHtml(res, statusCode, html, { csp = true } = {}) { const headers = { 'content-type': 'text/html; charset=utf-8', 'cache-control': 'no-store' }; if (csp) { - headers['content-security-policy'] = - "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-src 'self'"; + headers['content-security-policy'] = typeof csp === 'string' + ? csp + : "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-src 'self'"; } res.writeHead(statusCode, headers); res.end(html); @@ -175,6 +190,8 @@ function createPlanCanvasServer({ const typingKeys = new Map(); // key -> ms timestamp the agent signalled composing let idleTimer = null; let closed = false; + let pdfExportActive = false; + let pdfSnapshot = null; // --- presence --------------------------------------------------------- @@ -391,10 +408,25 @@ function createPlanCanvasServer({ } const pdfMatch = pathname.match(/^\/api\/session\/([a-f0-9]{12})\/pdf$/); - if (pdfMatch && req.method === 'GET') { + if (pdfMatch && (req.method === 'GET' || req.method === 'POST')) { const session = store.get(pdfMatch[1]); if (!session) return sendJson(res, 404, { error: 'unknown session' }); + if (pdfExportActive) { + res.setHeader('retry-after', '1'); + return sendJson(res, 429, { + error: 'another PDF export is already in progress', + code: 'PDF_EXPORT_BUSY' + }); + } + pdfExportActive = true; try { + if (req.method === 'POST') { + const body = await readJsonBody(req, MAX_PDF_SNAPSHOT_BYTES); + if (typeof body.html !== 'string' || !body.html.trim()) { + return sendJson(res, 400, { error: 'html snapshot is required' }); + } + pdfSnapshot = { key: session.key, html: body.html }; + } const pdf = await pdfExporter({ url: `http://${req.headers.host}/artifact/${session.key}/?pdf=1`, artifactFile: session.file @@ -403,6 +435,9 @@ function createPlanCanvasServer({ } catch (error) { const statusCode = error.code === 'PDF_BROWSER_NOT_FOUND' ? 503 : 500; return sendJson(res, statusCode, { error: error.message, code: error.code || 'PDF_EXPORT_FAILED' }); + } finally { + pdfSnapshot = null; + pdfExportActive = false; } } @@ -474,11 +509,14 @@ function createPlanCanvasServer({ res.end(); } - function serveArtifact(res, key, assetPath) { + function serveArtifact(res, key, assetPath, { pdfExport = false } = {}) { const session = store.get(key); if (!session) return sendHtml(res, 404, '

Unknown session

'); if (!assetPath) { + if (pdfExport && pdfSnapshot && pdfSnapshot.key === key) { + return sendHtml(res, 200, pdfSnapshot.html, { csp: PDF_EXPORT_CSP }); + } let content; try { content = fs.readFileSync(session.file, 'utf8'); @@ -491,13 +529,13 @@ function createPlanCanvasServer({ title: path.basename(session.file), sdkSrc: '/sdk.js' }); - return sendHtml(res, 200, html, { csp: false }); + return sendHtml(res, 200, html, { csp: pdfExport ? PDF_EXPORT_CSP : false }); } const sdkTag = ''; const injected = content.includes('') ? content.replace('', `${sdkTag}\n`) : `${content}\n${sdkTag}`; - return sendHtml(res, 200, injected, { csp: false }); + return sendHtml(res, 200, injected, { csp: pdfExport ? PDF_EXPORT_CSP : false }); } // Sibling assets resolve relative to the artifact's directory and must @@ -574,7 +612,9 @@ function createPlanCanvasServer({ const artifactMatch = pathname.match(/^\/artifact\/([a-f0-9]{12})\/(.*)$/); if (req.method === 'GET' && artifactMatch) { const assetPath = decodeURIComponent(artifactMatch[2]); - return serveArtifact(res, artifactMatch[1], assetPath || null); + return serveArtifact(res, artifactMatch[1], assetPath || null, { + pdfExport: url.searchParams.get('pdf') === '1' + }); } if (pathname.startsWith('/api/')) { return handleApi(req, res, url); diff --git a/scripts/lib/plan-canvas/ui.js b/scripts/lib/plan-canvas/ui.js index dbf4ed5b8..e858399e1 100644 --- a/scripts/lib/plan-canvas/ui.js +++ b/scripts/lib/plan-canvas/ui.js @@ -271,6 +271,7 @@ function canvasClientJs() { function postToFrame(msg) { if (frame.contentWindow) frame.contentWindow.postMessage(msg, '*'); } + const snapshotWaiters = new Map(); window.addEventListener('message', e => { if (e.source !== frame.contentWindow) return; const msg = e.data || {}; @@ -278,11 +279,32 @@ function canvasClientJs() { else if (msg.type === 'pc:queue-and-send' && msg.item) { addToQueue(msg.item); send(); } else if (msg.type === 'pc:scroll') lastScroll = { x: msg.x || 0, y: msg.y || 0 }; else if (msg.type === 'pc:toggle-mode') setAnnotate(!annotate); + else if (msg.type === 'pc:export-snapshot-result' && typeof msg.requestId === 'string') { + const waiter = snapshotWaiters.get(msg.requestId); + if (waiter) { + snapshotWaiters.delete(msg.requestId); + waiter(msg.html); + } + } else if (msg.type === 'pc:ready') { postToFrame({ type: 'pc:set-mode', annotate }); postToFrame({ type: 'pc:restore-scroll', x: lastScroll.x, y: lastScroll.y }); } }); + function requestArtifactSnapshot() { + return new Promise(resolve => { + const requestId = Date.now().toString(36) + Math.random().toString(36).slice(2); + const timer = setTimeout(() => { + snapshotWaiters.delete(requestId); + resolve(null); + }, 1500); + snapshotWaiters.set(requestId, html => { + clearTimeout(timer); + resolve(typeof html === 'string' ? html : null); + }); + postToFrame({ type: 'pc:export-snapshot', requestId }); + }); + } // --- queue ---------------------------------------------------------- function persistQueue() { try { sessionStorage.setItem(QKEY, JSON.stringify(queue)); } catch { /* full */ } } @@ -442,7 +464,13 @@ function canvasClientJs() { downloadPdfBtn.textContent = 'Preparing PDF\u2026'; reportExportStatus('Rendering locally\u2026'); try { - const res = await fetch('/api/session/' + key + '/pdf', { cache: 'no-store' }); + const snapshot = await requestArtifactSnapshot(); + const res = await fetch('/api/session/' + key + '/pdf', { + method: snapshot ? 'POST' : 'GET', + headers: snapshot ? { 'content-type': 'application/json' } : undefined, + body: snapshot ? JSON.stringify({ html: snapshot }) : undefined, + cache: 'no-store' + }); if (!res.ok) { const detail = await res.json().catch(() => ({})); throw new Error(detail.error || ('HTTP ' + res.status)); diff --git a/scripts/plan-canvas.js b/scripts/plan-canvas.js index 1b28b1883..fddce1471 100755 --- a/scripts/plan-canvas.js +++ b/scripts/plan-canvas.js @@ -19,6 +19,7 @@ const fs = require('fs'); const http = require('http'); +const os = require('os'); const path = require('path'); const { spawn } = require('child_process'); @@ -171,6 +172,73 @@ function sleep(ms) { return new Promise(resolve => setTimeout(resolve, ms)); } +function serverStartLockPath(port, lockDir = os.tmpdir()) { + const userId = typeof process.getuid === 'function' ? process.getuid() : 'user'; + return path.join(lockDir, `ecc-plan-canvas-${userId}-${validatePort(port)}.lock`); +} + +function processIsAlive(pid) { + if (!Number.isInteger(pid) || pid <= 0) return false; + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code === 'EPERM'; + } +} + +function removeStaleServerStartLock(lockPath, staleAfterMs = 60 * 1000) { + try { + const stat = fs.statSync(lockPath); + let owner = null; + try { owner = JSON.parse(fs.readFileSync(lockPath, 'utf8')); } catch { /* incomplete lock owner */ } + const oldEnough = Date.now() - stat.mtimeMs > staleAfterMs; + if (!oldEnough && (!owner || processIsAlive(owner.pid))) return false; + fs.rmSync(lockPath, { force: true }); + return true; + } catch { + return false; + } +} + +async function withServerStartLock(port, task, { + lockDir = os.tmpdir(), + timeoutMs = 15 * 1000 +} = {}) { + fs.mkdirSync(lockDir, { recursive: true }); + const lockPath = serverStartLockPath(port, lockDir); + const token = `${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`; + const startedAt = Date.now(); + + while (true) { + try { + fs.writeFileSync(lockPath, JSON.stringify({ pid: process.pid, token, createdAt: new Date().toISOString() }), { + flag: 'wx', + mode: 0o600 + }); + break; + } catch (error) { + if (error.code !== 'EEXIST') throw error; + if (removeStaleServerStartLock(lockPath)) continue; + if (Date.now() - startedAt >= timeoutMs) { + throw new Error(`timed out waiting for Plan Canvas startup lock on port ${port}`); + } + await sleep(50); + } + } + + try { + return await task(); + } finally { + try { + const owner = JSON.parse(fs.readFileSync(lockPath, 'utf8')); + if (owner.token === token) fs.rmSync(lockPath, { force: true }); + } catch { + // A stale-lock recovery may already have removed it. + } + } +} + function serverIsCompatible(health) { return Boolean( health && @@ -186,24 +254,28 @@ function serverIsCompatible(health) { async function ensureServer({ stateDir, port }) { const health = await healthCheck(port); if (serverIsCompatible(health)) return port; - if (health) { - await request(port, 'POST', '/shutdown').catch(() => {}); - for (let i = 0; i < 20 && (await healthCheck(port)); i++) await sleep(100); - } - fs.mkdirSync(stateDir, { recursive: true }); - const logFd = fs.openSync(path.join(stateDir, 'server.log'), 'a'); - const child = spawn(process.execPath, [__filename, 'server', '--port', String(port)], { - detached: true, - stdio: ['ignore', logFd, logFd], - env: { ...process.env, ECC_PLAN_CANVAS_STATE_DIR: stateDir } + return withServerStartLock(port, async () => { + const lockedHealth = await healthCheck(port); + if (serverIsCompatible(lockedHealth)) return port; + if (lockedHealth) { + await request(port, 'POST', '/shutdown').catch(() => {}); + for (let i = 0; i < 20 && (await healthCheck(port)); i++) await sleep(100); + } + fs.mkdirSync(stateDir, { recursive: true }); + const logFd = fs.openSync(path.join(stateDir, 'server.log'), 'a'); + const child = spawn(process.execPath, [__filename, 'server', '--port', String(port)], { + detached: true, + stdio: ['ignore', logFd, logFd], + env: { ...process.env, ECC_PLAN_CANVAS_STATE_DIR: stateDir } + }); + child.unref(); + fs.closeSync(logFd); + for (let i = 0; i < 50; i++) { + await sleep(100); + if (serverIsCompatible(await healthCheck(port))) return port; + } + throw new Error(`plan-canvas server did not become compatible on port ${port}; check ${path.join(stateDir, 'server.log')}`); }); - child.unref(); - fs.closeSync(logFd); - for (let i = 0; i < 50; i++) { - await sleep(100); - if (serverIsCompatible(await healthCheck(port))) return port; - } - throw new Error(`plan-canvas server did not become compatible on port ${port}; check ${path.join(stateDir, 'server.log')}`); } function openBrowser(url) { @@ -434,4 +506,4 @@ if (require.main === module) { }); } -module.exports = { main, ensureServer, healthCheck }; +module.exports = { main, ensureServer, healthCheck, withServerStartLock }; diff --git a/tests/integration/plan-canvas-e2e.test.js b/tests/integration/plan-canvas-e2e.test.js index c72dacc83..4a1566c89 100644 --- a/tests/integration/plan-canvas-e2e.test.js +++ b/tests/integration/plan-canvas-e2e.test.js @@ -25,6 +25,7 @@ const { spawn, spawnSync } = require('child_process'); const CLI = path.join(__dirname, '..', '..', 'scripts', 'plan-canvas.js'); const HOOK = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'plan-canvas-sessions.js'); +const { withServerStartLock } = require('../../scripts/plan-canvas'); const results = []; async function test(name, fn) { @@ -142,7 +143,22 @@ async function main() { let key = null; try { - await test('same-version legacy server is replaced before a canvas opens', async () => { + await test('port-scoped startup lock serializes server replacement callers', async () => { + let active = 0; + let maximumActive = 0; + const runLocked = label => withServerStartLock(port + 2, async () => { + active += 1; + maximumActive = Math.max(maximumActive, active); + await new Promise(resolve => setTimeout(resolve, 40)); + active -= 1; + return label; + }, { lockDir: tmp, timeoutMs: 2000 }); + assert.deepStrictEqual(await Promise.all([runLocked('first'), runLocked('second')]), ['first', 'second']); + assert.strictEqual(maximumActive, 1); + assert.ok(!fs.readdirSync(tmp).some(name => name.endsWith('.lock'))); + }); + + await test('concurrent opens serialize replacement of a same-version legacy server', async () => { const legacyPort = port + 1; const legacyStateDir = path.join(tmp, 'legacy-state'); const legacyEnv = { @@ -171,9 +187,14 @@ async function main() { }); try { - const result = await cliAsync(legacyEnv, ['open', plan, '--no-open']); - assert.strictEqual(result.status, 0, result.stderr); - assert.strictEqual(result.parsed.status, 'open'); + const results = await Promise.all([ + cliAsync(legacyEnv, ['open', plan, '--no-open']), + cliAsync(legacyEnv, ['open', plan, '--no-open']) + ]); + for (const result of results) { + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(result.parsed.status, 'open'); + } assert.strictEqual(shutdownRequested, true, 'current CLI should retire the stale server'); const health = JSON.parse((await request(legacyPort, 'GET', '/health')).body); assert.strictEqual(health.protocolVersion, 4); diff --git a/tests/scripts/plan-canvas-pdf.test.js b/tests/scripts/plan-canvas-pdf.test.js index f52100b9e..b820c4ee9 100644 --- a/tests/scripts/plan-canvas-pdf.test.js +++ b/tests/scripts/plan-canvas-pdf.test.js @@ -15,17 +15,15 @@ const { resolveChromiumExecutable } = require('../../scripts/lib/plan-canvas/pdf'); -const results = []; - async function test(name, fn) { try { await fn(); console.log(` ✓ ${name}`); - results.push(true); + return true; } catch (error) { console.log(` ✗ ${name}`); console.log(` ${error.stack || error.message}`); - results.push(false); + return false; } } @@ -48,15 +46,19 @@ function fakeChild(onSpawn) { async function main() { console.log('\n=== Testing Plan Canvas PDF export ===\n'); + let results = []; + const record = async (name, fn) => { + results = [...results, await test(name, fn)]; + }; - await test('builds safe, useful PDF filenames', () => { + await record('builds safe, useful PDF filenames', () => { assert.strictEqual(pdfFileName('/tmp/feature-fleet-2.2.plan.md'), 'feature-fleet-2.2.pdf'); assert.strictEqual(pdfFileName('/tmp/release-preview.html'), 'release-preview.pdf'); assert.strictEqual(pdfFileName('/tmp/bad:name?.md'), 'bad-name-.pdf'); assert.strictEqual(pdfFileName(''), 'plan.pdf'); }); - await test('restricts the renderer to loopback artifact URLs', () => { + await record('restricts the renderer to loopback artifact URLs', () => { assert.strictEqual( assertLoopbackUrl('http://127.0.0.1:4518/artifact/abc/?pdf=1'), 'http://127.0.0.1:4518/artifact/abc/?pdf=1' @@ -65,7 +67,7 @@ async function main() { assert.throws(() => assertLoopbackUrl('http://example.com/artifact/abc'), { code: 'PDF_EXPORT_INVALID_URL' }); }); - await test('honors an explicit Chromium executable override', () => { + await record('honors an explicit Chromium executable override', () => { const fsImpl = { accessSync(file) { assert.strictEqual(file, '/opt/test/chrome'); }, statSync() { return { isFile: () => true }; } @@ -80,7 +82,7 @@ async function main() { ); }); - await test('fails actionably when no local PDF renderer is installed', async () => { + await record('fails actionably when no local PDF renderer is installed', async () => { await assert.rejects( exportPdf({ url: 'http://127.0.0.1:4517/artifact/abc123/?pdf=1', @@ -92,7 +94,7 @@ async function main() { ); }); - await test('recognizes only complete PDF output', () => { + await record('recognizes only complete PDF output', () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-pdf-complete-')); const file = path.join(tmp, 'artifact.pdf'); fs.writeFileSync(file, '%PDF-1.4\npartial'); @@ -102,7 +104,7 @@ async function main() { fs.rmSync(tmp, { recursive: true, force: true }); }); - await test('validates PDF metadata from the opened file handle', () => { + await record('validates PDF metadata from the opened file handle', () => { const content = Buffer.from('%PDF-1.4\nlocal plan\n%%EOF\n'); const fsImpl = { openSync(file, flags) { @@ -124,7 +126,7 @@ async function main() { assert.strictEqual(isCompletePdf('/private/export.pdf', fsImpl), true); }); - await test('renders, terminates its private browser, and removes temporary state', async () => { + await record('renders, isolates network access, terminates its browser, and removes temporary state', async () => { const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-pdf-test-')); let spawned = null; let child = null; @@ -151,6 +153,11 @@ async function main() { assert.strictEqual(spawned.command, '/opt/test/chrome'); assert.strictEqual(spawned.options.shell, false); assert.ok(spawned.args.includes('--headless=new')); + assert.ok(spawned.args.includes('--disable-background-networking')); + assert.ok(spawned.args.includes('--disable-quic')); + assert.ok(spawned.args.some(arg => /^--proxy-server=http:\/\/127\.0\.0\.1:\d+$/.test(arg))); + assert.ok(spawned.args.includes('--proxy-bypass-list=<-loopback>;http://localhost:4517')); + assert.ok(spawned.args.includes('--host-resolver-rules=MAP * ~NOTFOUND, EXCLUDE localhost')); assert.ok(spawned.args.includes('http://localhost:4517/artifact/abc123/?pdf=1')); assert.strictEqual(child.signalCode, 'SIGTERM'); assert.deepStrictEqual(fs.readdirSync(tempRoot), []); diff --git a/tests/scripts/plan-canvas.test.js b/tests/scripts/plan-canvas.test.js index 7967497e6..b460e7ef7 100644 --- a/tests/scripts/plan-canvas.test.js +++ b/tests/scripts/plan-canvas.test.js @@ -92,13 +92,18 @@ async function main() { const artifact = path.join(tmp, 'demo.plan.md'); fs.writeFileSync(artifact, '# Plan: Demo\n\n## Files to Change\n\n| File | Action |\n|---|---|\n| `a.js` | UPDATE |\n'); const htmlArtifact = path.join(tmp, 'report.html'); - fs.writeFileSync(htmlArtifact, '

Report

'); + fs.writeFileSync( + htmlArtifact, + '

Report

' + ); fs.writeFileSync(path.join(tmp, 'style.css'), 'body { color: red }'); const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plan-canvas-outside-')); fs.writeFileSync(path.join(outsideDir, 'secret.txt'), 'secret'); const store = createSessionStore({ stateDir: path.join(tmp, 'state') }); const pdfRequests = []; + let holdPdfExport = false; + let releasePdfExport = null; let idleFired = false; const canvas = createPlanCanvasServer({ store, @@ -107,6 +112,7 @@ async function main() { idleTimeoutMs: 0, pdfExporter: async options => { pdfRequests.push(options); + if (holdPdfExport) await new Promise(resolve => { releasePdfExport = resolve; }); return { buffer: Buffer.from('%PDF-1.4\n%%EOF\n'), filename: 'demo.pdf' }; }, onIdleShutdown: () => { @@ -200,6 +206,19 @@ async function main() { assert.ok(res.body.includes('\n')); })) passed++; else failed++; + if (await test('PDF artifact responses block remote images and inline beacon egress', async () => { + const res = await request(port, 'GET', `/artifact/${htmlKey}/?pdf=1`); + const csp = res.headers['content-security-policy']; + assert.strictEqual(res.statusCode, 200); + assert.ok(csp.includes("default-src 'none'")); + assert.ok(csp.includes("img-src 'self' data:")); + assert.ok(csp.includes("connect-src 'none'")); + assert.ok(csp.includes("form-action 'none'")); + assert.ok(csp.includes("script-src 'none'")); + assert.ok(res.body.includes('https://example.invalid/tracker.png')); + assert.ok(res.body.includes('navigator.sendBeacon')); + })) passed++; else failed++; + if (await test('sibling assets are served, traversal is blocked', async () => { const ok = await request(port, 'GET', `/artifact/${key}/style.css`); assert.strictEqual(ok.statusCode, 200); @@ -213,11 +232,17 @@ async function main() { const res = await request(port, 'GET', asset); assert.strictEqual(res.statusCode, 200, `${asset} should be 200`); } + const sdk = await request(port, 'GET', '/sdk.js'); + assert.doesNotThrow(() => new Function(sdk.body)); + assert.ok(sdk.body.includes("msg.type === 'pc:export-snapshot'")); + assert.ok(sdk.body.includes("querySelectorAll('script,iframe,object,embed,form,base,meta[http-equiv]")); })) passed++; else failed++; if (await test('Download PDF fetches a generated PDF and starts a browser download', async () => { const client = await request(port, 'GET', '/client.js'); assert.ok(client.body.includes("'/api/session/' + key + '/pdf'")); + assert.ok(client.body.includes("method: snapshot ? 'POST' : 'GET'")); + assert.ok(client.body.includes("type: 'pc:export-snapshot'")); assert.ok(client.body.includes('URL.createObjectURL')); const res = await request(port, 'GET', `/api/session/${key}/pdf`); @@ -231,6 +256,28 @@ async function main() { assert.strictEqual(pdfRequests[0].url, `http://127.0.0.1:${port}/artifact/${key}/?pdf=1`); })) passed++; else failed++; + if (await test('concurrent PDF exports return a bounded retryable overload response', async () => { + holdPdfExport = true; + const snapshot = '

Already rendered diagram

Local SVG'; + const first = request(port, 'POST', `/api/session/${key}/pdf`, { body: { html: snapshot } }); + await waitFor(() => typeof releasePdfExport === 'function'); + const printable = await request(port, 'GET', `/artifact/${key}/?pdf=1`); + assert.ok(printable.body.includes('Already rendered diagram')); + assert.ok(printable.body.includes('Local SVG')); + assert.ok(printable.headers['content-security-policy'].includes("script-src 'none'")); + const overloaded = await request(port, 'GET', `/api/session/${key}/pdf`); + assert.strictEqual(overloaded.statusCode, 429); + assert.strictEqual(overloaded.headers['retry-after'], '1'); + assert.deepStrictEqual(jsonBody(overloaded), { + error: 'another PDF export is already in progress', + code: 'PDF_EXPORT_BUSY' + }); + releasePdfExport(); + assert.strictEqual((await first).statusCode, 200); + holdPdfExport = false; + releasePdfExport = null; + })) passed++; else failed++; + if (await test('browser client uses finite polling instead of one permanent connection per canvas', async () => { const res = await request(port, 'GET', '/client.js'); assert.ok(res.body.includes("'/api/session/' + key + '/state'"));