Admin security, crash and data-loss fixes

apps/admin/src/components/map/leaflet-map.test.tsx

@@ -3,7 +3,7 @@ import L from "leaflet"
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
 
 import { BoundaryMap } from "@/components/map/boundary-map"
-import { LeafletMap, type MapPin } from "@/components/map/leaflet-map"
+import { LeafletMap, tooltipText, type MapPin } from "@/components/map/leaflet-map"
 
 // jsdom has no layout engine: Leaflet sizes the map from clientWidth/clientHeight (0 in jsdom, which
 // makes fitBounds produce NaN coordinates) and the wrappers observe their container for resizes.
@@ -93,17 +93,51 @@ describe("LeafletMap", () => {
     expect(tip).toBeNull()
   })
 
-  it("parses a pin title in the tooltip as HTML markup (current behavior)", () => {
+  it("renders a pin title and place in the tooltip as literal text, never as markup", () => {
     const pin: MapPin = { id: "h", lat: 34, lng: -118, tip: "<b>x</b>", place: "<i>y</i>" }
     const { container } = render(<LeafletMap pins={[pin]} />)
 
     const tip = hoverAndReadTooltip(container, markerIcons(container)[0]!)
-    expect(tip?.querySelector("b")?.textContent).toBe("x")
-    expect(tip?.querySelector("i")?.textContent).toBe("y")
-    expect(tip?.textContent).toBe("x · y")
-    expect(tip?.textContent).not.toContain("<b>")
+    expect(tip?.querySelector("b")).toBeNull()
+    expect(tip?.querySelector("i")).toBeNull()
+    expect(tip?.textContent).toBe("<b>x</b> · <i>y</i>")
   })
 
+  it("never runs markup from a citizen-authored title in the tooltip", () => {
+    const payload = '<img src=x onerror="window.__pwned=1">'
+    const pin: MapPin = { id: "x", lat: 34, lng: -118, tip: payload, place: "<svg onload=alert(1)>" }
+    const { container } = render(<LeafletMap pins={[pin]} />)
+
+    const tip = hoverAndReadTooltip(container, markerIcons(container)[0]!)
+    expect(tip?.querySelector("img, svg")).toBeNull()
+    expect(tip?.textContent).toBe(`${payload} · <svg onload=alert(1)>`)
+  })
+
+  it("keeps a label with entity-like text literal in the tooltip", () => {
+    const pin: MapPin = { id: "a", lat: 34, lng: -118, label: "Fish &amp; Chips" }
+    const { container } = render(<LeafletMap pins={[pin]} />)
+
+    const tip = hoverAndReadTooltip(container, markerIcons(container)[0]!)
+    expect(tip?.textContent).toBe("Fish &amp; Chips")
+  })
+
+  it.each(["constructor", "toString", "__proto__"])(
+    "draws the generic glyph for the inherited object key %j as a category",
+    (category) => {
+      const { container } = render(
+        <LeafletMap
+          pins={[
+            { id: "g", lat: 34, lng: -118, category },
+            { id: "o", lat: 34.1, lng: -118.1, category: "other" },
+          ]}
+        />,
+      )
+
+      const glyphs = markerIcons(container).map((el) => el.querySelector("g path")?.getAttribute("d"))
+      expect(glyphs[0]).toBe(glyphs[1])
+    },
+  )
+
   it("calls onPinTap with the tapped pin", () => {
     const onPinTap = vi.fn()
     const { container } = render(<LeafletMap pins={PINS} onPinTap={onPinTap} />)
@@ -155,3 +189,19 @@ describe("BoundaryMap", () => {
     expect(remove).toHaveBeenCalledTimes(1)
   })
 })
+
+describe("tooltipText", () => {
+  it("joins the title and place with a middle dot", () => {
+    expect(tooltipText({ id: "1", lat: 0, lng: 0, tip: "Pothole", place: "Oakland" })).toBe(
+      "Pothole · Oakland",
+    )
+  })
+
+  it("falls back to the label and omits a missing place", () => {
+    expect(tooltipText({ id: "1", lat: 0, lng: 0, label: "Wall" })).toBe("Wall")
+  })
+
+  it("returns null when there is no title to show", () => {
+    expect(tooltipText({ id: "1", lat: 0, lng: 0, place: "Oakland" })).toBeNull()
+  })
+})

apps/admin/src/components/map/leaflet-map.tsx

@@ -82,7 +82,10 @@ function pinIcon(
   // report pin (red when it needs attention, gray when handled).
   const isEvent = kind != null && EVENT_KINDS.has(kind)
   const state = isEvent ? kind : draft ? "needs" : "routed"
-  const glyph = GLYPHS[isEvent ? kind : category || "other"] ?? CATEGORY_GLYPHS.other
+  const glyphKey = isEvent ? kind : category || "other"
+  // The icon html goes through innerHTML and the category arrives from the API unvalidated, so only a
+  // constant own entry of GLYPHS may reach it (an inherited key like "constructor" would not).
+  const glyph = Object.hasOwn(GLYPHS, glyphKey) ? GLYPHS[glyphKey]! : CATEGORY_GLYPHS.other
   const fill = PIN_FILL[state] ?? PIN_FILL.routed
   const w = active ? 40 : 31
   const h = w * (76 / 64)
@@ -102,6 +105,20 @@ function pinIcon(
   })
 }
 
+export function tooltipText(pin: MapPin): string | null {
+  const title = pin.tip || pin.label
+  if (!title) return null
+  return pin.place ? `${title} · ${pin.place}` : title
+}
+
+// Leaflet writes string tooltip content with innerHTML and pin titles/places are citizen-authored, so
+// the content must be a node whose text is set with textContent.
+function tooltipNode(text: string): HTMLElement {
+  const el = document.createElement("span")
+  el.textContent = text
+  return el
+}
+
 export interface LeafletMapProps {
   pins?: MapPin[]
   center?: [number, number]
@@ -236,9 +253,9 @@ export function LeafletMap({
       } else {
         const icon = pinIcon(p.category, { active, draft: p.draft, kind: p.kind })
         const m = L.marker([p.lat, p.lng], { icon, riseOnHover: true }).addTo(map)
-        const tip = p.tip || p.label
+        const tip = tooltipText(p)
         if (tip) {
-          m.bindTooltip(tip + (p.place ? ` · ${p.place}` : ""), {
+          m.bindTooltip(tooltipNode(tip), {
             direction: "top",
             offset: [0, -30],
             className: "pi-map-tip",

apps/admin/src/components/providers.test.tsx

@@ -0,0 +1,48 @@
+import { render, screen, within } from "@testing-library/react"
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
+
+import { Providers } from "@/components/providers"
+
+vi.mock("@/components/auth/auth-hydrator", () => ({ AuthHydrator: () => null }))
+vi.mock("@/hooks/use-admin-auth", () => ({
+  useOperatorSession: () => ({ isOperator: true, operator: null, status: "authenticated" }),
+}))
+
+function Crash(): never {
+  throw new Error("shell exploded")
+}
+
+beforeEach(() => {
+  vi.spyOn(console, "error").mockImplementation(() => undefined)
+})
+
+afterEach(() => {
+  vi.restoreAllMocks()
+})
+
+describe("Providers", () => {
+  it("renders the operator's app content", () => {
+    render(
+      <Providers>
+        <p>Dashboard content</p>
+      </Providers>,
+    )
+    expect(screen.getByText("Dashboard content")).toBeInTheDocument()
+  })
+
+  it("catches a render error anywhere in the app with a root alert offering Reload", () => {
+    render(
+      <Providers>
+        <Crash />
+      </Providers>,
+    )
+
+    const alert = screen.getByRole("alert")
+    expect(alert).toHaveTextContent("Something went wrong")
+    expect(alert).toHaveTextContent(
+      "This page hit an unexpected error. Try again, or reload the dashboard.",
+    )
+    expect(alert).not.toHaveTextContent("shell exploded")
+    expect(within(alert).getByRole("button", { name: "Reload" })).toBeInTheDocument()
+  })
+})

apps/admin/src/components/providers.tsx

@@ -5,6 +5,7 @@ import { QueryClientProvider, type QueryClient } from "@tanstack/react-query"
 
 import { makeQueryClient } from "@/lib/query"
 import { AuthHydrator } from "@/components/auth/auth-hydrator"
+import { ErrorBoundary } from "@/components/shell/error-boundary"
 import { OperatorLogin } from "@/features/auth/operator-login"
 import { useOperatorSession } from "@/hooks/use-admin-auth"
 
@@ -23,8 +24,10 @@ export function Providers({ children }: { children: React.ReactNode }) {
 
   return (
     <QueryClientProvider client={clientRef.current}>
-      <AuthHydrator />
-      <AuthGate>{children}</AuthGate>
+      <ErrorBoundary>
+        <AuthHydrator />
+        <AuthGate>{children}</AuthGate>
+      </ErrorBoundary>
     </QueryClientProvider>
   )
 }

apps/admin/src/components/shared/backdrop-dismiss.ts

@@ -0,0 +1,61 @@
+"use client"
+
+import * as React from "react"
+
+interface BackdropProps<T extends HTMLElement> {
+  ref: React.RefObject<T | null>
+  onClick: React.MouseEventHandler<T>
+}
+
+// A press that starts in the modal (a text-selection drag, typically) and is released over the
+// backdrop must not dismiss: browsers dispatch that click on the nearest common ancestor of press and
+// release, which is the backdrop itself when it wraps the modal. So only a click whose press also
+// started on the backdrop counts. The press is read on the window in the capture phase because a
+// backdrop that is a sibling of its panel never receives a press that began inside the panel.
+export function useBackdropDismiss<T extends HTMLElement = HTMLDivElement>(
+  onDismiss: () => void,
+): BackdropProps<T> {
+  const ref = React.useRef<T>(null)
+  const pressedOnBackdrop = React.useRef(false)
+
+  React.useEffect(() => {
+    const onPointerDown = (e: PointerEvent) => {
+      pressedOnBackdrop.current = ref.current !== null && e.target === ref.current
+    }
+    window.addEventListener("pointerdown", onPointerDown, true)
+    return () => window.removeEventListener("pointerdown", onPointerDown, true)
+  }, [])
+
+  const onClick = (e: React.MouseEvent<T>) => {
+    const dismiss = pressedOnBackdrop.current && e.target === e.currentTarget
+    pressedOnBackdrop.current = false
+    if (dismiss) onDismiss()
+  }
+
+  return { ref, onClick }
+}
+
+/**
+ * Escape and the backdrop dismiss a modal only while it holds nothing the operator would lose; once
+ * there is a draft, its close button and Cancel are the deliberate ways to discard it. Returns the
+ * backdrop props. The shell yields Escape to any open modal (escape-owner.ts), so Escape here closes
+ * only the modal and never navigates.
+ */
+export function usePristineDismiss<T extends HTMLElement = HTMLDivElement>(
+  onClose: () => void,
+  pristine: boolean,
+): BackdropProps<T> {
+  const dismiss = React.useCallback(() => {
+    if (pristine) onClose()
+  }, [pristine, onClose])
+
+  React.useEffect(() => {
+    const onKey = (e: KeyboardEvent) => {
+      if (e.key === "Escape") dismiss()
+    }
+    window.addEventListener("keydown", onKey)
+    return () => window.removeEventListener("keydown", onKey)
+  }, [dismiss])
+
+  return useBackdropDismiss<T>(dismiss)
+}

apps/admin/src/components/shared/data-states.tsx

@@ -29,27 +29,42 @@ export function LoadingState({ label = "Loading..." }: { label?: string }) {
 }
 
 /**
- * Error panel with an optional retry. Surfaces the normalized AppError message; when the backend is
+ * Error panel with an optional retry and an optional primary action beside it (the error boundary's
+ * Reload). Surfaces the normalized AppError message unless `message` replaces it; when the backend is
  * down this renders the friendly INTERNAL message rather than crashing.
  */
 export function ErrorState({
   error,
   onRetry,
   title = "Could not load this",
+  message,
+  action,
 }: {
   error: unknown
   onRetry?: () => void
   title?: string
+  message?: string
+  action?: { label: string; onClick: () => void }
 }) {
-  const message = React.useMemo(() => toAppError(error).message, [error])
+  const errorText = React.useMemo(() => toAppError(error).message, [error])
+  const retry = onRetry && (
+    <button type="button" className="btn sm" onClick={onRetry}>
+      Try again
+    </button>
+  )
   return (
     <div className="state-error" role="alert">
       <div className="state-error-title">{title}</div>
-      <div className="state-error-sub">{message}</div>
-      {onRetry && (
-        <button type="button" className="btn sm" onClick={onRetry}>
-          Try again
-        </button>
+      <div className="state-error-sub">{message ?? errorText}</div>
+      {action ? (
+        <div className="row-flex">
+          {retry}
+          <button type="button" className="btn sm primary" onClick={action.onClick}>
+            {action.label}
+          </button>
+        </div>
+      ) : (
+        retry
       )}
     </div>
   )