From 742395bca70a3ae384d0df82ac22a52b4058b08c Mon Sep 17 00:00:00 2001 From: WRG Date: Sat, 26 Sep 2026 02:14:24 +1000 Subject: [PATCH 1/5] fix(scripts): confine plan-canvas artifact assets and escape 404 paths Serve artifact pages with a sandbox-only CSP mirroring the viewer iframe, fail-closed realpath confinement for sibling assets, and escape the artifact path in the missing-artifact 404. TDD: 4 RED failures (CSP header, 404 escaping, symlink escape) before the fix, 31/31 GREEN after; markdown/sessions/hook suites and ESLint clean. --- scripts/lib/plan-canvas/server.js | 42 ++++++++++++++++++++++++------- tests/scripts/plan-canvas.test.js | 32 ++++++++++++++++++++++- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/scripts/lib/plan-canvas/server.js b/scripts/lib/plan-canvas/server.js index 11b44062a..961dd559c 100644 --- a/scripts/lib/plan-canvas/server.js +++ b/scripts/lib/plan-canvas/server.js @@ -15,7 +15,7 @@ const http = require('http'); const path = require('path'); const { buildAllowedHostnames, isAllowedHostHeader, isAllowedOrigin } = require('../loopback-guard'); -const { renderMarkdown } = require('./markdown'); +const { escapeHtml, renderMarkdown } = require('./markdown'); const { artifactSdkJs } = require('./sdk'); const { canvasCss, @@ -101,9 +101,21 @@ function sendJson(res, statusCode, payload) { res.end(body); } +// Artifact pages render inside a sandboxed iframe (no allow-same-origin) and +// legitimately run CDN scripts (Mermaid) plus inline loaders, so the default +// restrictive CSP cannot apply. A sandbox-only CSP mirrors the iframe +// attribute instead: scripts keep working, but the document gets an opaque +// origin, which neuters direct-navigation abuse of the loopback API (no CORS +// reads, JSON POSTs are preflight-blocked) without changing in-iframe +// behavior. A hostile CSP in a raw HTML artifact can only narrow this +// further, never loosen it. +const ARTIFACT_CSP = 'sandbox allow-scripts allow-forms allow-popups'; + function sendHtml(res, statusCode, html, { csp = true } = {}) { const headers = { 'content-type': 'text/html; charset=utf-8', 'cache-control': 'no-store' }; - if (csp) { + if (csp === 'artifact') { + headers['content-security-policy'] = ARTIFACT_CSP; + } else if (csp) { headers['content-security-policy'] = "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-src 'self'"; } @@ -493,7 +505,7 @@ function createPlanCanvasServer({ try { content = fs.readFileSync(session.file, 'utf8'); } catch { - return sendHtml(res, 404, `

Artifact missing

${session.file} no longer exists.

`, { csp: false }); + return sendHtml(res, 404, `

Artifact missing

${escapeHtml(session.file)} no longer exists.

`); } const ext = path.extname(session.file).toLowerCase(); if (ext === '.md' || ext === '.markdown') { @@ -501,29 +513,41 @@ function createPlanCanvasServer({ title: path.basename(session.file), sdkSrc: '/sdk.js' }); - return sendHtml(res, 200, html, { csp: false }); + return sendHtml(res, 200, html, { csp: 'artifact' }); } 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: 'artifact' }); } // Sibling assets resolve relative to the artifact's directory and must - // stay confined to it. + // stay confined to it. The prefix check alone is insufficient: a symlink + // inside the directory can point outside it, so the check is repeated + // against the real paths and fails closed when they cannot be resolved. const baseDir = path.dirname(session.file); const resolved = path.resolve(baseDir, assetPath); if (resolved !== baseDir && !resolved.startsWith(baseDir + path.sep)) { return sendJson(res, 403, { error: 'asset path escapes artifact directory' }); } - let data; + let realTarget; try { - data = fs.readFileSync(resolved); + const realBase = fs.realpathSync(baseDir); + realTarget = fs.realpathSync(resolved); + if (realTarget !== realBase && !realTarget.startsWith(realBase + path.sep)) { + return sendJson(res, 403, { error: 'asset path escapes artifact directory' }); + } } catch { return sendJson(res, 404, { error: 'asset not found' }); } - const type = CONTENT_TYPES[path.extname(resolved).toLowerCase()] || 'application/octet-stream'; + let data; + try { + data = fs.readFileSync(realTarget); + } catch { + return sendJson(res, 404, { error: 'asset not found' }); + } + const type = CONTENT_TYPES[path.extname(realTarget).toLowerCase()] || 'application/octet-stream'; res.writeHead(200, { 'content-type': type, 'cache-control': 'no-store' }); return res.end(data); } diff --git a/tests/scripts/plan-canvas.test.js b/tests/scripts/plan-canvas.test.js index 5d1e8745f..152d9c35b 100644 --- a/tests/scripts/plan-canvas.test.js +++ b/tests/scripts/plan-canvas.test.js @@ -182,7 +182,7 @@ async function main() { assert.ok(res.body.includes('

')); assert.ok(res.body.includes('')); assert.ok(res.body.includes('