fix: backport Copilot review fixes from PR rboarescu#15 (/viz) to main

Same shape as 152e428 — production at disks runs fork main, so
review amendments to the in-flight upstream PR need to ship here too.
Eight Copilot comments addressed across main.py and static/viz.html.

## Auth (was: /viz served publicly)

main.py: /viz now goes through _check_auth on the same code path as
every other endpoint. Accepts the key from either the X-Api-Key header
(preferred) or the ?key=… query param (bookmarkable shortcut).
_check_auth is a no-op when PALACE_API_KEY is unset, so the
zero-config local-dev experience is unchanged. Production deployments
with PALACE_API_KEY set will now 401 unauthenticated /viz requests —
matches every other endpoint.

## CDN integrity (was: D3 + Mermaid loaded without SRI, version not pinned)

static/viz.html: pinned to d3@7.8.5 and mermaid@10.9.1 with SHA-384
SRI hashes via cdn.jsdelivr.net. Added crossorigin="anonymous" (required
for SRI on cross-origin scripts) and referrerpolicy="no-referrer".

## Mermaid sanitization

static/viz.html:
- mermaidSafe() now strips ASCII control chars (\\n, \\r, \\t, etc.) in
  addition to the existing parser-breaking chars.
- mermaid.initialize({ securityLevel: "strict" }) — was "loose" which
  relaxes label sanitization and enables clickable diagram nodes.
  The dashboard never needs node click handlers, and our own
  mermaidSafe() runs before labels reach Mermaid anyway.

## Mermaid render-error fallback (was: <div> appended inside <pre>)

static/viz.html: target was <pre id="hierarchy">; appending a <div>
inside it produced invalid HTML and inconsistent cross-browser
rendering. Surface the error as plain text inside the existing element
and tag with class="err" so CSS can style it.

## ?key= leakage warning

static/viz.html: comment on the KEY parsing block calls out that
?key=… leaks into browser history, referer headers, and proxy logs.
Same caveat in the /viz docstring.

## Docstring drift (was: "cached at module load" but actually lazy)

main.py: docstring rewritten to describe the actual lazy-load behaviour
("lazy-loaded on first request and cached in-process thereafter; one
disk read per daemon process").

All five fixes are amendments on the corresponding upstream PR rboarescu#15
branch (force-pushed earlier today). No behaviour change for the
healthy unauthenticated-local case; the changes harden security
surface that was caught in review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jphein

main.py

@@ -885,20 +885,36 @@ async def _direct_under_sem(work):
 
 
 @app.get("/viz", response_class=HTMLResponse)
-async def viz():
+async def viz(
+    key: str | None = None,
+    x_api_key: str | None = Header(default=None),
+):
     """Self-contained status dashboard at /viz.
 
-    Returns the HTML page from static/viz.html. The page client-side fetches
-    /graph, /repair/status, and /health in parallel and renders five panels:
+    Returns the HTML page from static/viz.html. The page then fetches
+    /graph, /repair/status, and /health client-side and renders five panels:
     KG force-graph (D3), wings bar chart, wing/room hierarchy (Mermaid),
-    tunnels list, KG stats. Auth happens on the data endpoints — the HTML
-    shell itself is public so /viz?key=... in the URL works for ergonomic
-    bookmarking. Cached at module load to avoid disk reads per request.
+    tunnels list, KG stats.
+
+    Auth: same as every other endpoint — ``X-Api-Key`` header. As an
+    ergonomic shortcut for browser bookmarking, ``?key=...`` is also
+    accepted; the page reads it from the URL and re-supplies it to the
+    data endpoints. The ``?key=...`` shape leaks the key into browser
+    history, proxy logs, and referer headers — prefer the header for
+    anything beyond a personal bookmark.
+
+    The HTML template is read from disk lazily on the first request and
+    cached in-process thereafter (one disk read per daemon process).
 
     Inspired by upstream PRs #1022 (D3 KG viz), #393 (Mermaid diagrams),
     #431 (CLI stats), #256 (sync_status MCP), #601 (brief overview) — none
     cherry-picked, just patterns synthesized over the daemon's /graph.
     """
+    # Accept the API key from either the X-Api-Key header (preferred) or
+    # the ?key= query parameter (bookmarkable). _check_auth is a no-op
+    # when PALACE_API_KEY is unset, so this preserves the
+    # zero-config-local-dev experience.
+    _check_auth(x_api_key or key)
     global _VIZ_HTML_CACHE
     if _VIZ_HTML_CACHE is None:
         try:

static/viz.html

@@ -35,8 +35,22 @@
   <meta name="viewport" content="width=device-width, initial-scale=1" />
   <title>palace-daemon — viz</title>
   <link rel="icon" href="data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32'><text y='26' font-size='28'>◈</text></svg>" />
-  <script src="https://d3js.org/d3.v7.min.js"></script>
-  <script src="https://cdn.jsdelivr.net/npm/mermaid@10/dist/mermaid.min.js"></script>
+  <!--
+    CDN scripts are pinned to specific versions and verified via SRI so a
+    compromised CDN response can't run arbitrary JS in the dashboard.
+    `crossorigin="anonymous"` is required for SRI to work on cross-origin
+    scripts. Bump the version + integrity hash together when upgrading.
+  -->
+  <script
+    src="https://cdn.jsdelivr.net/npm/d3@7.8.5/dist/d3.min.js"
+    integrity="sha384-su5kReKyYlIFrI62mbQRKXHzFobMa7BHp1cK6julLPbnYcCW9NIZKJiTODjLPeDh"
+    crossorigin="anonymous"
+    referrerpolicy="no-referrer"></script>
+  <script
+    src="https://cdn.jsdelivr.net/npm/mermaid@10.9.1/dist/mermaid.min.js"
+    integrity="sha384-WmdflGW9aGfoBdHc4rRyWzYuAjEmDwMdGdiPNacbwfGKxBW/SO6guzuQ76qjnSlr"
+    crossorigin="anonymous"
+    referrerpolicy="no-referrer"></script>
   <style>
     :root {
       --bg:        #0a0e14;
@@ -218,6 +232,12 @@ <h2>KG stats</h2>
 //------------------------------------------------------------------ params + auth
 const params = new URLSearchParams(location.search);
 const REFRESH_SECONDS = parseInt(params.get("refresh") || "0", 10);
+// `?key=...` is supported as a bookmarkable shortcut for the API key. This
+// is convenient but the key will appear in the browser's history, in any
+// referer header sent to a third-party origin, and in the access logs of
+// any forward proxy or reverse proxy on the path. **Do not use ?key= when
+// the dashboard might be screen-shared, screencast, or proxied through a
+// shared intermediary.** Prefer the `X-Api-Key` header for those cases.
 const KEY = params.get("key") || "";
 
 function withAuth(opts) {
@@ -341,9 +361,15 @@ <h2>KG stats</h2>
 
 // Mermaid label sanitizer — allow only chars that don't blow up the parser.
 // (Mermaid escapes its own labels but a label containing `]` or `"` will
-// terminate the node definition early.)
+// terminate the node definition early.) Also strips ASCII control chars
+// (newline, carriage return, tab, etc.) since Mermaid's label parser
+// breaks on raw newlines and we'd rather collapse than render half a
+// label on each side of the break.
 function mermaidSafe(s) {
-  return String(s == null ? "" : s).replace(/["\[\]<>|`]/g, "_").slice(0, 60);
+  return String(s == null ? "" : s)
+    .replace(/[\x00-\x1f\x7f]/g, "_")  // ASCII control chars (incl. \n \r \t)
+    .replace(/["\[\]<>|`]/g, "_")
+    .slice(0, 60);
 }
 
 function renderHierarchy(graph) {
@@ -371,8 +397,12 @@ <h2>KG stats</h2>
   target.textContent = lines.join("\n");
   if (window.mermaid && mermaid.run) {
     mermaid.run({ querySelector: "#hierarchy" }).catch(e => {
+      // The target is a <pre>; appending a <div> here would be invalid HTML.
+      // Surface the failure as plain text inside the existing element and
+      // tag the container so CSS can style it as an error if desired.
       clear(target);
-      target.appendChild(el("div", { class: "err" }, "mermaid render failed: " + e.message));
+      target.classList.add("err");
+      target.textContent = "mermaid render failed: " + (e && e.message ? e.message : e);
     });
   }
 }
@@ -466,10 +496,15 @@ <h2>KG stats</h2>
 
 document.addEventListener("DOMContentLoaded", () => {
   if (window.mermaid) {
+    // securityLevel "strict" is the default and the safe choice — it sandboxes
+    // labels through Mermaid's HTML escaping and disables click handlers on
+    // diagram nodes. We never need clickable nodes here (this is a read-only
+    // visual of /graph), and our labels are already sanitised by mermaidSafe()
+    // before they reach the diagram. Using "loose" was over-permissive.
     mermaid.initialize({
       startOnLoad: false,
       theme: matchMedia("(prefers-color-scheme: light)").matches ? "default" : "dark",
-      securityLevel: "loose",
+      securityLevel: "strict",
     });
   }
   loadAll();