From 055dda167292238892d34ac3d012aefbc72479a9 Mon Sep 17 00:00:00 2001
From: jennyf19 <19942418+jennyf19@users.noreply.github.com>
Date: Mon, 20 Jul 2026 11:28:43 -0700
Subject: [PATCH] signals-dashboard: harden desk launch (round 2 review)

Synced from the-workshop@8106dd4. Addresses the second GHCP review pass on #2363:

- isOnPath now requires a runnable file (X_OK / PATHEXT), not just existsSync.

- launchDeskConsole adds a realpath-containment check (symlink escape) and takes workshopDir.

- Dashboard copies the path only on the non-launch fallback.

- Mutating /api/* routes now require the canonical loopback Host plus a per-server capability token (DNS-rebinding defense), mirroring connector-namespaces.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 26faf13e-639c-4a21-ac05-c0dc2bff7c62
---
 extensions/signals-dashboard/extension.mjs | 130 ++++++++++++++++-----
 1 file changed, 99 insertions(+), 31 deletions(-)

diff --git a/extensions/signals-dashboard/extension.mjs b/extensions/signals-dashboard/extension.mjs
index 0fed4a7f..962b6d6b 100644
--- a/extensions/signals-dashboard/extension.mjs
+++ b/extensions/signals-dashboard/extension.mjs
@@ -4,10 +4,11 @@
 // Supports stashing desks (48hr hold) and restoring them.
 
 import { createServer } from "node:http";
-import { existsSync } from "node:fs";
+import { existsSync, statSync, accessSync, realpathSync, constants as fsConstants } from "node:fs";
 import { readdir, readFile, writeFile, stat } from "node:fs/promises";
-import { join, delimiter } from "node:path";
+import { join, delimiter, sep } from "node:path";
 import { spawn } from "node:child_process";
+import { randomBytes } from "node:crypto";
 import { joinSession, createCanvas } from "@github/copilot-sdk/extension";
 
 const servers = new Map();
@@ -74,6 +75,18 @@ function trySpawn(cmd, args, opts = {}) {
 // Resolve an executable on PATH (honoring PATHEXT on Windows), mirroring
 // WorkshopRoom's AgentClis.IsOnPath. Used to prefer Agency when the machine has
 // it installed, falling back to vanilla Copilot.
+// A PATH hit only counts if it resolves to a real, runnable file. existsSync
+// alone would treat a directory or a non-executable file named `agency` as a
+// match, so auto-detection would pick the wrapper and the terminal would then
+// fail to run it with no fallback.
+function isExecutableFile(p) {
+    try {
+        if (!statSync(p).isFile()) return false;
+        if (process.platform !== "win32") accessSync(p, fsConstants.X_OK);
+        return true;
+    } catch { return false; }
+}
+
 function isOnPath(command) {
     try {
         const dirs = (process.env.PATH || "").split(delimiter);
@@ -82,10 +95,13 @@ function isOnPath(command) {
             : [];
         for (const dir of dirs) {
             if (!dir) continue;
-            try {
-                if (existsSync(join(dir, command))) return true;
-                for (const ext of exts) if (existsSync(join(dir, command + ext))) return true;
-            } catch {}
+            // On Windows only a PATHEXT match is runnable; on POSIX check the bare
+            // name, and isExecutableFile confirms the execute bit either way.
+            if (exts.length) {
+                for (const ext of exts) if (isExecutableFile(join(dir, command + ext))) return true;
+            } else if (isExecutableFile(join(dir, command))) {
+                return true;
+            }
         }
     } catch {}
     return false;
@@ -129,12 +145,28 @@ function osaStringLiteral(s) {
     return '"' + String(s).replace(/\\/g, "\\\\").replace(/"/g, '\\"') + '"';
 }
 
-async function launchDeskConsole(deskPath, deskName) {
+// Resolve symlinks on both sides and confirm the target is the workshop root
+// itself or lives beneath it. The callers locate a desk with stat(), which
+// follows symlinks, so a committed desks/foo -> /outside symlink would otherwise
+// launch the agent with an external working directory, breaking the inside-repo
+// guarantee.
+function isInsideRoot(root, target) {
+    try {
+        const r = realpathSync(root);
+        const t = realpathSync(target);
+        return t === r || t.startsWith(r + sep);
+    } catch { return false; }
+}
+
+async function launchDeskConsole(deskPath, deskName, workshopDir) {
     // deskPath can't contain a double quote (a real Windows path never does and
     // it would break out of a quoted argument). deskName must be a plain slug so
-    // it is safe on every command line and shell below.
+    // it is safe on every command line and shell below, and the resolved desk
+    // must still live inside the workshop root (which defeats a symlinked desk
+    // that escapes the repo).
     if (!deskPath || deskPath.includes('"')) return false;
     if (!isSafeDeskNameForLaunch(deskName)) return false;
+    if (!isInsideRoot(workshopDir, deskPath)) return false;
     const run = [...deskAgentArgv(deskName), "-i", deskOrientPrompt(deskName)];
     if (process.platform === "win32") {
         // Windows Terminal is a GUI app, so it always surfaces its own visible
@@ -219,6 +251,24 @@ function isCrossSiteRequest(req) {
     return site === "cross-site" || site === "same-site";
 }
 
+// Pin the Host header to the exact loopback authority we bound. A DNS-rebinding
+// page reaches us under its own hostname (Host: attacker.example:<port>), so an
+// exact match against 127.0.0.1:<port> refuses those requests before any state
+// change — Origin/Host equality alone doesn't, since the attacker controls both.
+function isCanonicalHost(req, canonicalHost) {
+    return String(req.headers.host || "").toLowerCase() === String(canonicalHost || "").toLowerCase();
+}
+
+// Capability check for the per-server token minted at startup and embedded in
+// the page we serve. Only the loopback document we rendered knows it, so a blind
+// cross-origin/rebinding caller can't forge a mutating request even if it
+// reached the socket.
+function hasCapabilityToken(req, token) {
+    const header = req.headers["x-workshop-token"];
+    const provided = Array.isArray(header) ? header[0] : header;
+    return typeof provided === "string" && provided.length > 0 && provided === token;
+}
+
 // --- Stash management ---
 
 async function readStash(workshopDir) {
@@ -675,7 +725,7 @@ function renderStashedCard(entry) {
     </div>`;
 }
 
-function renderDashboard(signals, stashed) {
+function renderDashboard(signals, stashed, capabilityToken) {
     const activeSignals = sortSignals(signals.filter(s => !stashed.some(e => e.name === s.deskName)));
 
     const cards = activeSignals.length > 0
@@ -747,12 +797,17 @@ function renderDashboard(signals, stashed) {
     </div>
 
     <script>
+        // Minted per server, echoed on every mutating fetch so the loopback
+        // document proves it actually loaded this page (defense-in-depth with the
+        // Host pin + CSRF check on the server).
+        const WORKSHOP_TOKEN = ${JSON.stringify(capabilityToken)};
+        const POST_OPTS = { method: 'POST', headers: { 'x-workshop-token': WORKSHOP_TOKEN } };
         async function stashDesk(name) {
-            await fetch('/api/stash/' + encodeURIComponent(name), { method: 'POST' });
+            await fetch('/api/stash/' + encodeURIComponent(name), POST_OPTS);
             refresh();
         }
         async function restoreDesk(name) {
-            await fetch('/api/restore/' + encodeURIComponent(name), { method: 'POST' });
+            await fetch('/api/restore/' + encodeURIComponent(name), POST_OPTS);
             refresh();
         }
         function showToast(title, detail) {
@@ -777,25 +832,20 @@ function renderDashboard(signals, stashed) {
             setTimeout(() => toast.remove(), 4000);
         }
         async function openDesk(name) {
-            const res = await fetch('/api/open/' + encodeURIComponent(name), { method: 'POST' });
+            const res = await fetch('/api/open/' + encodeURIComponent(name), POST_OPTS);
             const data = await res.json();
             if (data.ok) {
                 const path = data.deskPath || name;
-                // Copy the path either way so there's always a usable handle, but
-                // remember whether it actually succeeded so we never claim a copy
-                // that the browser blocked.
-                let copied = false;
-                try { await navigator.clipboard.writeText(path); copied = true; } catch {}
                 if (data.launched) {
+                    // A successful open shouldn't hijack the user's clipboard.
                     showToast('opening ' + name + ' desk…', path);
-                } else if (copied) {
-                    // No terminal could be launched from here, so the copied path
-                    // is the fallback the user (or the TA) opens the desk with.
-                    showToast(name + ' · path copied', path);
                 } else {
-                    // Neither launch nor clipboard worked; the toast still shows
-                    // the path below so it stays usable.
-                    showToast(name + ' · copy this path', path);
+                    // No terminal launched from here, so copy the path as the
+                    // fallback handle, but only claim the copy when it actually
+                    // succeeded. The path shows in the toast either way.
+                    let copied = false;
+                    try { await navigator.clipboard.writeText(path); copied = true; } catch {}
+                    showToast(copied ? (name + ' · path copied') : (name + ' · copy this path'), path);
                 }
             } else {
                 showToast(name + ' · not found', '');
@@ -849,14 +899,31 @@ function renderDashboard(signals, stashed) {
 // --- Server ---
 
 async function startServer(instanceId, workshopDir) {
+    // Minted once per server: embedded in the page we render and required back on
+    // every mutating request. canonicalHost is filled in after listen() so the
+    // Host pin knows the exact port we bound.
+    const capabilityToken = randomBytes(32).toString("base64url");
+    let canonicalHost = null;
+
     const server = createServer(async (req, res) => {
         try {
         const url = new URL(req.url, `http://${req.headers.host}`);
 
-        if (req.method === "POST" && url.pathname.startsWith("/api/") && isCrossSiteRequest(req)) {
-            res.writeHead(403, { "Content-Type": "application/json" });
-            res.end(JSON.stringify({ ok: false, error: "cross_site_blocked" }));
-            return;
+        // Layered guard for the state-changing /api/* routes (cf.
+        // connector-namespaces/server.mjs): the canonical-Host pin defeats DNS
+        // rebinding, the CSRF check blocks cross-site browser POSTs, and the
+        // capability token proves the caller actually loaded our page.
+        if (req.method === "POST" && url.pathname.startsWith("/api/")) {
+            if (!isCanonicalHost(req, canonicalHost) || isCrossSiteRequest(req)) {
+                res.writeHead(403, { "Content-Type": "application/json" });
+                res.end(JSON.stringify({ ok: false, error: "cross_site_blocked" }));
+                return;
+            }
+            if (!hasCapabilityToken(req, capabilityToken)) {
+                res.writeHead(403, { "Content-Type": "application/json" });
+                res.end(JSON.stringify({ ok: false, error: "missing_capability_token" }));
+                return;
+            }
         }
 
         if (req.method === "POST" && url.pathname.startsWith("/api/stash/")) {
@@ -895,7 +962,7 @@ async function startServer(instanceId, workshopDir) {
                 try {
                     const s = await stat(deskPath);
                     if (s.isDirectory()) {
-                        const launched = await launchDeskConsole(deskPath, deskName);
+                        const launched = await launchDeskConsole(deskPath, deskName, workshopDir);
                         res.writeHead(200, { "Content-Type": "application/json" });
                         res.end(JSON.stringify({ ok: true, deskName, deskPath, launched }));
                         return;
@@ -910,7 +977,7 @@ async function startServer(instanceId, workshopDir) {
         const signals = await scanSignals(workshopDir);
         const stashed = await readStash(workshopDir);
         res.setHeader("Content-Type", "text/html; charset=utf-8");
-        res.end(renderDashboard(signals, stashed));
+        res.end(renderDashboard(signals, stashed, capabilityToken));
         } catch (err) {
             // Top-level boundary: never leave a request hanging or let a
             // rejection become an unhandled crash — e.g. malformed %-encoding
@@ -933,6 +1000,7 @@ async function startServer(instanceId, workshopDir) {
     });
     const address = server.address();
     const port = typeof address === "object" && address ? address.port : 0;
+    canonicalHost = `127.0.0.1:${port}`;
     return { server, url: `http://127.0.0.1:${port}/` };
 }
 
@@ -1037,7 +1105,7 @@ const session = await joinSession({
                             try {
                                 const s = await stat(deskPath);
                                 if (s.isDirectory()) {
-                                    const launched = await launchDeskConsole(deskPath, ctx.input.deskName);
+                                    const launched = await launchDeskConsole(deskPath, ctx.input.deskName, entry.workshopDir);
                                     return { ok: true, deskName: ctx.input.deskName, deskPath, launched, workshopDir: entry.workshopDir };
                                 }
                             } catch {}