From b6fe5a71e194d711d9aa8f56024ddf7ca53fad0c Mon Sep 17 00:00:00 2001 From: Haley Chen <2022hachen@gmail.com> Date: Wed, 22 Jul 2026 16:28:07 -0400 Subject: [PATCH] fix: harden plan canvas code scanning alerts --- scripts/lib/plan-canvas/server.js | 6 ++- scripts/plan-canvas.js | 61 ++++++++++++++++++++++++++----- tests/scripts/plan-canvas.test.js | 7 ++-- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/scripts/lib/plan-canvas/server.js b/scripts/lib/plan-canvas/server.js index 28e7c2b70..2c250c73e 100644 --- a/scripts/lib/plan-canvas/server.js +++ b/scripts/lib/plan-canvas/server.js @@ -248,9 +248,11 @@ function createPlanCanvasServer({ } if (req.method === 'GET' && pathname === '/api/await') { + const keyParam = url.searchParams.get('key'); const file = url.searchParams.get('file'); - if (!file) return sendJson(res, 400, { error: 'file query parameter is required' }); - const session = store.findByFile(file); + if (keyParam && !/^[a-f0-9]{12}$/.test(keyParam)) return sendJson(res, 400, { error: 'invalid session key' }); + if (!keyParam && !file) return sendJson(res, 400, { error: 'key or file query parameter is required' }); + const session = keyParam ? store.get(keyParam) : store.findByFile(file); if (!session) return sendJson(res, 200, { status: 'missing' }); const key = session.key; const timeoutRaw = url.searchParams.get('timeoutMs'); diff --git a/scripts/plan-canvas.js b/scripts/plan-canvas.js index 3f1206817..5c26fb59a 100755 --- a/scripts/plan-canvas.js +++ b/scripts/plan-canvas.js @@ -37,6 +37,16 @@ const { const VERSION = require('../package.json').version; +const SAFE_REQUEST_PATHS = new Set([ + '/', + '/health', + '/shutdown', + '/api/await', + '/api/sessions', + '/api/end' +]); +const SESSION_REPLY_PATH = /^\/api\/session\/[a-f0-9]{12}\/reply$/; + function usage() { return [ 'Plan Canvas - review plans and HTML artifacts in the browser', @@ -77,19 +87,51 @@ function readServerInfo(stateDir) { } } +function validatePort(port) { + const value = Number(port); + if (!Number.isInteger(value) || value < 0 || value > 65535) { + throw new Error(`invalid plan-canvas server port: ${port}`); + } + return value; +} + +function validateRequestPath(requestPath) { + if (typeof requestPath !== 'string' || !requestPath.startsWith('/')) { + throw new Error('plan-canvas request path must be root-relative'); + } + const url = new URL(requestPath, `http://${DEFAULT_HOST}`); + if (url.hostname !== DEFAULT_HOST) { + throw new Error('plan-canvas request path must stay on the loopback server'); + } + if (!SAFE_REQUEST_PATHS.has(url.pathname) && !SESSION_REPLY_PATH.test(url.pathname)) { + throw new Error(`unsupported plan-canvas request path: ${url.pathname}`); + } + return `${url.pathname}${url.search}`; +} + +function requestOptions(port, method, requestPath, headers) { + return { + host: DEFAULT_HOST, + port: validatePort(port), + method, + path: validateRequestPath(requestPath), + agent: false, + headers + }; +} + function request(port, method, requestPath, body = null) { return new Promise((resolve, reject) => { const payload = body === null ? null : JSON.stringify(body); const req = http.request( - { - host: DEFAULT_HOST, + requestOptions( port, method, - path: requestPath, - headers: payload + requestPath, + payload ? { 'content-type': 'application/json', 'content-length': Buffer.byteLength(payload) } : {} - }, + ), res => { let data = ''; res.on('data', chunk => { @@ -197,12 +239,13 @@ async function cmdOpen(file, args, { stateDir, port }) { }; } -function awaitRequest(port, file, timeoutMs) { - const params = new URLSearchParams({ file }); +function awaitRequest(port, key, timeoutMs) { + if (!/^[a-f0-9]{12}$/.test(key)) throw new Error('invalid plan-canvas session key'); + const params = new URLSearchParams({ key }); if (timeoutMs !== null) params.set('timeoutMs', String(timeoutMs)); return new Promise((resolve, reject) => { const req = http.request( - { host: DEFAULT_HOST, port, method: 'GET', path: `/api/await?${params}` }, + requestOptions(port, 'GET', `/api/await?${params}`, {}), res => { let data = ''; res.on('data', chunk => { @@ -236,7 +279,7 @@ async function cmdAwait(file, args, { stateDir, port }) { const timeoutRaw = valueAfter(args, '--timeout-ms'); const timeoutMs = timeoutRaw === null ? null : Number.parseInt(timeoutRaw, 10) || 0; process.stderr.write('[plan-canvas] waiting for human feedback... leave this running (re-run if interrupted; queued feedback is never lost)\n'); - const result = await awaitRequest(port, path.resolve(file), timeoutMs); + const result = await awaitRequest(port, sessionKeyFor(canonicalizeArtifactPath(file)), timeoutMs); if (result.status === 'feedback') { result.next_step = result.sessionEnded ? 'The user sent this feedback and ended the session. Address it and report in chat; do not reopen the canvas uninvited.' diff --git a/tests/scripts/plan-canvas.test.js b/tests/scripts/plan-canvas.test.js index ebe9107f9..7d164c999 100644 --- a/tests/scripts/plan-canvas.test.js +++ b/tests/scripts/plan-canvas.test.js @@ -118,7 +118,8 @@ async function main() { const htmlArtifact = path.join(tmp, 'report.html'); fs.writeFileSync(htmlArtifact, '

Report

'); fs.writeFileSync(path.join(tmp, 'style.css'), 'body { color: red }'); - fs.writeFileSync(path.join(os.tmpdir(), 'plan-canvas-outside.txt'), 'secret'); + 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') }); let idleFired = false; @@ -209,7 +210,7 @@ async function main() { const ok = await request(port, 'GET', `/artifact/${key}/style.css`); assert.strictEqual(ok.statusCode, 200); assert.ok(ok.body.includes('color: red')); - const escape = await request(port, 'GET', `/artifact/${key}/..%2Fplan-canvas-outside.txt`); + const escape = await request(port, 'GET', `/artifact/${key}/..%2F${path.basename(outsideDir)}%2Fsecret.txt`); assert.strictEqual(escape.statusCode, 403); })) passed++; else failed++; @@ -359,7 +360,7 @@ async function main() { })) passed++; else failed++; fs.rmSync(tmp, { recursive: true, force: true }); - fs.rmSync(path.join(os.tmpdir(), 'plan-canvas-outside.txt'), { force: true }); + fs.rmSync(outsideDir, { recursive: true, force: true }); console.log('\n' + '='.repeat(40)); console.log(`Passed: ${passed}`);