From 967f0d8ede481d91d36d715cb0981edd7fbef689 Mon Sep 17 00:00:00 2001 From: jennyf19 <19942418+jennyf19@users.noreply.github.com> Date: Sat, 18 Jul 2026 19:44:45 -0700 Subject: [PATCH] fix(signals-dashboard): harden canvas against XSS, path traversal, and malformed signals Address PR review findings on the Cairn canvas extension: - XSS: replace inline onclick handlers (which used HTML-escape that does not escape single quotes) with event delegation via data-act/data-desk attributes and one document click listener that survives the innerHTML auto-refresh. - Path traversal: add isValidDeskName() and enforce it in every HTTP and canvas-action handler that takes a desk name (reject empty, /, \\, null byte, '.' and '..'). - Crash safety: String()-coerce in esc()/truncate() and guard outcomeIssues with Array.isArray so a malformed signal cannot take down the whole dashboard render. - Honest UI: the per-desk button no longer claims to 'open' a desk; it is relabeled 'path' and copies the desk's filesystem path to the clipboard with an accurate toast built via textContent (not innerHTML). - Correctness: outcome signals only pair with a signal when emitted at or after it (within 1hr), and activeCount is computed by excluding stashed desks instead of subtracting counts (no longer goes negative). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26faf13e-639c-4a21-ac05-c0dc2bff7c62 --- extensions/signals-dashboard/extension.mjs | 94 +++++++++++++++++----- 1 file changed, 76 insertions(+), 18 deletions(-) diff --git a/extensions/signals-dashboard/extension.mjs b/extensions/signals-dashboard/extension.mjs index 39a593c0..74d0ab6e 100644 --- a/extensions/signals-dashboard/extension.mjs +++ b/extensions/signals-dashboard/extension.mjs @@ -11,6 +11,14 @@ import { joinSession, createCanvas } from "@github/copilot-sdk/extension"; const servers = new Map(); const STASH_TTL_MS = 48 * 60 * 60 * 1000; +// Desk names are single path segments (folder names under desks/ or classroom/). +// Reject anything that could escape the workshop dir via path traversal. +function isValidDeskName(name) { + return typeof name === "string" && name.length > 0 && name.length <= 128 && + !name.includes("/") && !name.includes("\\") && !name.includes("\0") && + name !== "." && name !== ".."; +} + // --- Stash management --- async function readStash(workshopDir) { @@ -116,8 +124,8 @@ async function scanSignals(workshopDir) { // Also check for any recent outcome (within 1hr of latest signal) if no run_id match if (!outcome) { const recentOutcomes = allSignals - .filter(s => s.parsed.signal_type === "outcome" && Math.abs(s.mtimeMs - latestTime) < 3600000) - .sort((a, b) => b.mtimeMs - a.mtimeMs); + .filter(s => s.parsed.signal_type === "outcome" && s.mtimeMs >= latestTime && (s.mtimeMs - latestTime) < 3600000) + .sort((a, b) => a.mtimeMs - b.mtimeMs); if (recentOutcomes.length > 0) outcome = recentOutcomes[0].parsed; } @@ -155,7 +163,7 @@ async function scanSignals(workshopDir) { // Outcome signal fields outcomeRating: outcome?.quality_rating || null, outcomeEffort: outcome?.effort_to_merge || null, - outcomeIssues: outcome?.issues_found || [], + outcomeIssues: Array.isArray(outcome?.issues_found) ? outcome.issues_found : [], outcomeAgent: outcome?.agent_name || null, honestyGap: honestyGap, }); @@ -188,10 +196,11 @@ function sortSignals(signals) { // --- HTML rendering --- function esc(s) { - return s.replace(/&/g, "&").replace(//g, ">").replace(/"/g, """); + return String(s).replace(/&/g, "&").replace(//g, ">").replace(/"/g, """).replace(/'/g, "'"); } function truncate(s, len) { - return s.length > len ? s.slice(0, len) + "โ€ฆ" : s; + const str = String(s); + return str.length > len ? str.slice(0, len) + "โ€ฆ" : str; } function formatTokens(n) { if (!n) return null; @@ -308,7 +317,7 @@ function renderSignalCard(sig) { ? `โœ“ done` : `โœ“ checkpoint`; - const stashBtn = ``; + onmouseout="this.style.background='${isEscalation ? '#7f1d1d' : 'transparent'}'" + title="Copy this desk's filesystem path to the clipboard">path`; let escalationBlock = ""; if (isEscalation && sig.escalationReason) { @@ -440,7 +450,7 @@ function renderStashedCard(entry) { ${esc(entry.name)} ${timeRemaining(entry.stashedAt)} -