fix(map): stop untrusted values reaching the marker HTML (#1820)

* fix(map): drop the casing layer before its source on teardown Tearing down the reservation overlay logged an error on every map unmount: Source "trek-reservations" cannot be removed while layer "trek-reservations-lines-transit-casing" is using it. The casing layer arrived with the transit paths in 3.2.0 but was never added to destroy(), which only removed the main line layer. The try/catch around it gave no protection either — the engine reports this by firing an error event rather than throwing — so the source was silently left behind on the style. The layer id is a named constant now, so the place that creates it and the place that removes it can no longer drift apart. The lifecycle test grew the assertion that would have caught it: the fake map now refuses to remove a source that a layer still references, the same way the real one does. Verified against the unfixed code — two tests fail without the change. * fix(map): stop untrusted values reaching the marker HTML Both map renderers build their marker HTML as a string and hand it to L.divIcon / innerHTML. Two of the interpolations carry values a user controls, and neither was escaped. place.image_url is the worse of the two, because it is not admin-only: any trip member with edit rights can set it. placeUpdateRequestSchema is a plain record of unknown values, and the length guard in places.controller covers name, description, address and notes but not image_url. The renderers do check the value, but only with startsWith('/uploads/') — a payload like /uploads/x" onerror="…" y=" satisfies that and then breaks out of the src="…" attribute. category.color is the same shape one line up, reachable by an admin. It also reaches SharedTripPage, which builds its own divIcon and answers without a guard, so that payload lands in front of people who have no account here. Colours are allow-listed rather than escaped. Escaping stops the attribute breakout but leaves a working CSS value behind, and `url(https://…)` in a background is a tracking pixel. Anything that is not #rgb/#rrggbb falls back. image_url is a URL, not an enum, so that one is escaped; the prefix check stays. Server side, the category contract now demands a hex colour, and the endpoints validate through DTO classes instead of @Body('color') — a property read carries no metatype, so the global pipe never saw it. That retires two entries from the body-contract allow list. It replaces the hand-rolled 400 for a missing name with the pipe's envelope, the same trade the places and trips migrations made. The response schema deliberately still accepts any string: the write path was open for a long time, so tightening it would make an instance fail on its own stored rows. The client allow-list is what covers those. Verified against the unpatched renderer — the three new cases fail without it. * perf(client): serialize the map icons without react-dom/server Six modules imported react-dom/server statically to turn a lucide icon into an SVG string. That put Fizz — roughly 190 kB raw, 57 kB gzip — into a chunk three lazy routes share, the Leaflet renderer among them, which is the default. For output that is always a single <svg> with a flat list of shape children. mapViewport chunk 189,532 -> 3,710 B Tooltip chunk 194,333 -> 1,970 + 4,860 B precache total 17,856 -> 17,675 KiB utils/iconMarkup.ts serializes React elements directly. It is generic over elements rather than over lucide's icon tables on purpose: iconNode is closed over inside createLucideIcon and never exported, so calling the forwardRef render function is what survives a lucide upgrade. Byte-compatibility is checked, not asserted. iconMarkup.parity.test.ts diffs the output against real Fizz for every exported icon in three prop shapes — the test file is the one place still allowed to import react-dom/server, and it never reaches a bundle. Two traps it covers: viewBox must not be kebab-cased into view-box, and Fizz emits <path …></path> rather than a self-closing tag. TripPDF loses its lazy renderer entirely — ensureRenderer, the module-level handle and the two "not ready yet" guards that returned an empty string. There is nothing left to load. An eslint rule keeps the import from creeping back. Without it the regression is invisible: the build stays green, the app keeps working, and the chunk quietly grows by 190 kB again.
2026-10-20 14:06:27 +00:00 · 2026-08-07 13:43:38 +02:00
parent c089e75f08
commit cf62567c48
19 changed files with 396 additions and 74 deletions
@@ -75,4 +75,25 @@ export default tseslint.config(
      'preserve-caught-error': 'warn',
    },
  },
+  {
+    // react-dom/server was worth ~190 KB raw / 57 KB gzip in a chunk three lazy
+    // routes share — including the Leaflet renderer, which is the default — for
+    // output that is always a single <svg>. utils/iconMarkup.ts does that job
+    // without Fizz, and this keeps the import from creeping back: nothing else
+    // would notice, the build stays green and the app keeps working.
+    files: ['src/**/*.{ts,tsx}'],
+    ignores: ['src/**/*.test.{ts,tsx}'],
+    rules: {
+      'no-restricted-imports': ['error', {
+        paths: [{
+          name: 'react-dom/server',
+          message: 'Use renderIconMarkup from utils/iconMarkup — Fizz is ~190 KB for one <svg>.',
+        }],
+      }],
+      'no-restricted-syntax': ['error', {
+        selector: "ImportExpression > Literal[value=/^react-dom\\u002F(server|static)/]",
+        message: 'Use renderIconMarkup from utils/iconMarkup — Fizz is ~190 KB for one <svg>.',
+      }],
+    },
+  },
 );
@@ -1017,3 +1017,39 @@ describe('MapView photo thumbnails', () => {
    expect(vi.mocked(photoService.fetchPhoto)).not.toHaveBeenCalled()
  })
 })
+
+// The marker HTML is a hand-built string handed to L.divIcon, i.e. innerHTML.
+// Two of its interpolations carry values a user controls.
+describe('MapView — untrusted values in the marker HTML', () => {
+  const iconHtml = () => screen.getAllByTestId('marker').map(el => el.getAttribute('data-icon-html') || '').join('')
+
+  it('FE-COMP-MAPVIEW-072: a category colour that is not a hex value is dropped, not escaped', () => {
+    // Escaping alone would stop the attribute breakout but still leave a working
+    // CSS value, so the colour is allow-listed and anything else falls back.
+    render(<MapView places={[buildMapPlace({
+      lat: 48, lng: 2, category_color: 'red" onmouseover="alert(1)',
+    })]} />)
+    expect(iconHtml()).not.toContain('onmouseover')
+    expect(iconHtml()).toContain('#6b7280')
+  })
+
+  it('FE-COMP-MAPVIEW-073: a CSS url() in the category colour never reaches the style attribute', () => {
+    render(<MapView places={[buildMapPlace({ lat: 48, lng: 2, category_color: 'url(https://evil.example/px)' })]} />)
+    expect(iconHtml()).not.toContain('evil.example')
+  })
+
+  it('FE-COMP-MAPVIEW-074: an image_url that passes the /uploads/ prefix check is still escaped', () => {
+    // The prefix check only looks at the start of the string, so a payload can
+    // satisfy it and then break out of the src="…" attribute.
+    render(<MapView places={[buildMapPlace({
+      lat: 48, lng: 2, image_url: '/uploads/x" onerror="alert(1)" y="',
+    })]} />)
+    expect(iconHtml()).not.toContain('onerror="alert(1)"')
+    expect(iconHtml()).toContain('&quot;')
+  })
+
+  it('FE-COMP-MAPVIEW-075: an ordinary hex colour is passed through untouched', () => {
+    render(<MapView places={[buildMapPlace({ lat: 48, lng: 2, category_color: '#00ff00' })]} />)
+    expect(iconHtml()).toContain('#00ff00')
+  })
+})
@@ -1,6 +1,6 @@
 import { useEffect, useRef, useState, useMemo, useCallback, createElement, memo } from 'react'
 import DOM from 'react-dom'
-import { renderToStaticMarkup } from 'react-dom/server'
+import { renderIconMarkup } from '../../utils/iconMarkup'
 import { MapContainer, TileLayer, Marker, Polyline, CircleMarker, Circle, useMap, Tooltip } from 'react-leaflet'
 import MarkerClusterGroup from 'react-leaflet-cluster'
 import L from 'leaflet'
@@ -13,6 +13,8 @@ import { PluginMapMarkers } from './MapPluginMarkers'
 import { PluginMapLayers } from './MapPluginLayers'
 import { useTransportRoutes } from '../../hooks/useTransportRoutes'
 import { visibleRouteReservations } from '../../utils/reservationRoutes'
+import { safeHexColor } from '../../utils/safeColor'
+import { escapeHtml } from '@trek/shared'
 import type { Reservation, RouteVia } from '../../types'
 import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories'
 import { resolveTrackColor, hasManualTrackColor } from './trackColors'
@@ -24,7 +26,7 @@ import { computeMapViewport, TILE_SIZE_RASTER, type ViewportPadding } from '../.
 function categoryIconSvg(iconName: string | null | undefined, size: number): string {
  const IconComponent = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin']
  try {
-    return renderToStaticMarkup(createElement(IconComponent, { size, color: 'white', strokeWidth: 2.5 }))
+    return renderIconMarkup(createElement(IconComponent, { size, color: 'white', strokeWidth: 2.5 }))
  } catch { return '' }
 }
 import type { Place } from '../../types'
@@ -75,12 +77,14 @@ function createPlaceIcon(place, orderNumbers, isSelected) {
  const cached = iconCache.get(cacheKey)
  if (cached) return cached
  const size = isSelected ? 44 : 36
-  const borderColor = isSelected ? '#111827' : (place.category_color || 'white')
+  // Allow-listed, not escaped: the value lands in style="…" of a divIcon, where
+  // escaping stops the attribute breakout but still permits a CSS url().
+  const borderColor = isSelected ? '#111827' : safeHexColor(place.category_color, 'white')
  const borderWidth = isSelected ? 3 : 2.5
  const shadow = isSelected
    ? '0 0 0 3px rgba(17,24,39,0.25), 0 4px 14px rgba(0,0,0,0.3)'
    : '0 2px 8px rgba(0,0,0,0.22)'
-  const bgColor = place.category_color || '#6b7280'
+  const bgColor = safeHexColor(place.category_color, '#6b7280')

  // Number badges (bottom-right)
  let badgeHtml = ''
@@ -115,7 +119,7 @@ function createPlaceIcon(place, orderNumbers, isSelected) {
          box-shadow:${shadow};
          overflow:hidden;background:${bgColor};
        ">
-          <img src="${place.image_url}" width="${size}" height="${size}" style="display:block;border-radius:50%;object-fit:cover;" />
+          <img src="${escapeHtml(place.image_url)}" width="${size}" height="${size}" style="display:block;border-radius:50%;object-fit:cover;" />
        </div>
        ${badgeHtml}
      </div>`,
@@ -157,7 +161,7 @@ function createPoiIcon(category: string) {
  if (cached) return cached
  const cat = POI_CATEGORY_BY_KEY[category]
  const color = cat?.color || '#6b7280'
-  const svg = cat ? renderToStaticMarkup(createElement(cat.Icon, { size: 13, color: 'white', strokeWidth: 2.5 })) : ''
+  const svg = cat ? renderIconMarkup(createElement(cat.Icon, { size: 13, color: 'white', strokeWidth: 2.5 })) : ''
  const icon = L.divIcon({
    className: '',
    html: `<div style="width:26px;height:26px;border-radius:50%;background:${color};border:2px solid white;box-shadow:0 1px 5px rgba(0,0,0,0.3);display:flex;align-items:center;justify-content:center;cursor:pointer;">${svg}</div>`,
@@ -1,5 +1,5 @@
 import { useEffect, useRef, useMemo, useState, createElement } from 'react'
-import { renderToStaticMarkup } from 'react-dom/server'
+import { renderIconMarkup } from '../../utils/iconMarkup'
 import type mapboxgl from 'mapbox-gl'
 import { useSettingsStore } from '../../store/settingsStore'
 import { useAuthStore } from '../../store/authStore'
@@ -11,6 +11,8 @@ import { attachLocationMarker, type LocationMarkerHandle } from './locationMarke
 import { ReservationMapboxOverlay } from './reservationsMapbox'
 import { useTransportRoutes } from '../../hooks/useTransportRoutes'
 import { visibleRouteReservations } from '../../utils/reservationRoutes'
+import { safeHexColor } from '../../utils/safeColor'
+import { escapeHtml } from '@trek/shared'
 import { MAPBOX_DEFAULT_STYLE, styleForActiveProvider, basemapLanguage, type GlMapProvider } from './glProviders'
 import LocationButton from './LocationButton'
 import { useGeolocation } from '../../hooks/useGeolocation'
@@ -25,7 +27,7 @@ import { computeMapViewport, TILE_SIZE_GL } from '../../utils/mapViewport'
 function categoryIconSvg(iconName: string | null | undefined, size: number): string {
  const IconComponent = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin']
  try {
-    return renderToStaticMarkup(createElement(IconComponent, { size, color: 'white', strokeWidth: 2.5 }))
+    return renderIconMarkup(createElement(IconComponent, { size, color: 'white', strokeWidth: 2.5 }))
  } catch { return '' }
 }

@@ -130,12 +132,13 @@ interface Props {

 function createMarkerElement(place: Place & { category_color?: string; category_icon?: string }, photoUrl: string | null, orderNumbers: number[] | null, selected: boolean): HTMLDivElement {
  const size = selected ? 44 : 36
-  const borderColor = selected ? '#111827' : (place.category_color || 'white')
+  // See MapView: allow-listed rather than escaped, because this is a CSS context.
+  const borderColor = selected ? '#111827' : safeHexColor(place.category_color, 'white')
  const borderWidth = selected ? 3 : 2.5
  const shadow = selected
    ? '0 0 0 3px rgba(17,24,39,0.25), 0 4px 14px rgba(0,0,0,0.3)'
    : '0 2px 8px rgba(0,0,0,0.22)'
-  const bgColor = place.category_color || '#6b7280'
+  const bgColor = safeHexColor(place.category_color, '#6b7280')

  // The visual circle is `size` + 2*border on each side. To make the
  // mapbox `anchor: 'center'` land on the real visual middle of the marker
@@ -182,7 +185,7 @@ function createMarkerElement(place: Place & { category_color?: string; category_
        overflow:hidden;background:${bgColor};
        box-sizing:content-box;
      ">
-        <img src="${photoUrl}" width="${size}" height="${size}" style="display:block;border-radius:50%;object-fit:cover;" />
+        <img src="${escapeHtml(photoUrl)}" width="${size}" height="${size}" style="display:block;border-radius:50%;object-fit:cover;" />
      </div>
      ${badgeHtml}
    `
@@ -325,7 +328,7 @@ function buildPluginMarkerPopup(mk: PluginMapMarker): HTMLDivElement {
 function createPoiMarkerElement(category: string): HTMLDivElement {
  const cat = POI_CATEGORY_BY_KEY[category]
  const color = cat?.color || '#6b7280'
-  const svg = cat ? renderToStaticMarkup(createElement(cat.Icon, { size: 13, color: 'white', strokeWidth: 2.5 })) : ''
+  const svg = cat ? renderIconMarkup(createElement(cat.Icon, { size: 13, color: 'white', strokeWidth: 2.5 })) : ''
  const el = document.createElement('div')
  el.style.cssText = 'width:26px;height:26px;cursor:pointer;'
  el.innerHTML = `<div style="width:26px;height:26px;border-radius:50%;background:${color};border:2px solid #fff;box-shadow:0 1px 5px rgba(0,0,0,0.3);display:flex;align-items:center;justify-content:center;box-sizing:border-box;">${svg}</div>`
@@ -1,5 +1,5 @@
 import { Fragment, createElement, useMemo, useState } from 'react'
-import { renderToStaticMarkup } from 'react-dom/server'
+import { renderIconMarkup } from '../../utils/iconMarkup'
 import { Marker, Polyline, Tooltip, useMap, useMapEvents } from 'react-leaflet'
 import L from 'leaflet'
 import { Plane, Train, Ship, Car, Bus, Sailboat, Bike, CarTaxiFront, Route, TramFront } from 'lucide-react'
@@ -43,7 +43,7 @@ function useEndpointPane() {

 function endpointIcon(type: TransportType, label: string | null): L.DivIcon {
  const { icon: IconCmp, color } = TYPE_META[type]
-  const svg = renderToStaticMarkup(createElement(IconCmp, { size: 13, color: 'white', strokeWidth: 2.5 }))
+  const svg = renderIconMarkup(createElement(IconCmp, { size: 13, color: 'white', strokeWidth: 2.5 }))
  const labelHtml = label ? `<span style="display:inline-flex;align-items:center;line-height:1">${escapeHtml(label)}</span>` : ''
  const estWidth = label ? Math.max(40, label.length * 6 + 28) : 26
  return L.divIcon({
@@ -1,5 +1,5 @@
 import { createElement } from 'react'
-import { renderToStaticMarkup } from 'react-dom/server'
+import { renderIconMarkup } from '../../utils/iconMarkup'
 import { CATEGORY_ICON_MAP } from '../shared/categoryIcons'
 import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories'
 import type { Place } from '../../types'
@@ -28,7 +28,7 @@ function esc(s: string | null | undefined): string {
 function iconSvg(iconName: string | null | undefined, size: number, color: string): string {
  const Icon = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin']
  try {
-    return renderToStaticMarkup(createElement(Icon, { size, color, strokeWidth: 2 }))
+    return renderIconMarkup(createElement(Icon, { size, color, strokeWidth: 2 }))
  } catch {
    return ''
  }
@@ -61,7 +61,7 @@ export function buildPlacePopupHtml(place: PlaceWithCategory, photoUrl: string |
 export function buildPoiPopupHtml(poi: Poi): string {
  const cat = POI_CATEGORY_BY_KEY[poi.category]
  const color = cat?.color || '#6b7280'
-  const icon = cat ? renderToStaticMarkup(createElement(cat.Icon, { size: 12, color, strokeWidth: 2 })) : ''
+  const icon = cat ? renderIconMarkup(createElement(cat.Icon, { size: 12, color, strokeWidth: 2 })) : ''
  const head = `<div style="display:flex;align-items:center;gap:5px;"><span style="flex-shrink:0;display:inline-flex;line-height:0;">${icon}</span><span style="${NAME_STYLE}">${esc(poi.name)}</span></div>`
  const address = poi.address ? `<div style="${ADDR_STYLE}">${esc(poi.address)}</div>` : ''
  return `${CARD_OPEN}${head}${address}</div>`
@@ -3,6 +3,7 @@ import {
  ReservationMapboxOverlay,
  RESERVATION_SOURCE_ID,
  RESERVATION_LINE_LAYER_ID,
+  TRANSIT_CASING_LAYER_ID,
  type ReservationOverlayOptions,
 } from './reservationsMapbox'
 import type { Reservation, ReservationEndpoint } from '../../types'
@@ -165,7 +166,17 @@ function freshMap(projection: Projection = spread) {
    addLayer: vi.fn((layer: { id: string }) => { layers.set(layer.id, layer) }),
    getLayer: (id: string) => layers.get(id),
    removeLayer: vi.fn((id: string) => { layers.delete(id) }),
-    removeSource: vi.fn((id: string) => { sources.delete(id) }),
+    // The real engine refuses to drop a source that a layer still points at, and
+    // reports it by firing an error rather than throwing. Modelled as a throw here
+    // so a missed layer fails the test instead of only dirtying a console.
+    removeSource: vi.fn((id: string) => {
+      for (const [layerId, layer] of layers) {
+        if ((layer as { source?: string }).source === id) {
+          throw new Error(`Source "${id}" cannot be removed while layer "${layerId}" is using it.`)
+        }
+      }
+      sources.delete(id)
+    }),
    on: vi.fn(function (this: void, event: string, fn: Handler) { (handlers[event] ||= []).push(fn) }),
    off: vi.fn((event: string, fn: Handler) => { handlers[event] = (handlers[event] || []).filter(h => h !== fn) }),
    getZoom: () => 12,
@@ -265,7 +276,12 @@ describe('ReservationMapboxOverlay lifecycle', () => {
    expect(map.listeners('zoomend')).toBe(0)
    expect(map.listeners('moveend')).toBe(0)
    expect(map.removeLayer).toHaveBeenCalledWith(RESERVATION_LINE_LAYER_ID)
+    expect(map.removeLayer).toHaveBeenCalledWith(TRANSIT_CASING_LAYER_ID)
    expect(map.removeSource).toHaveBeenCalledWith(RESERVATION_SOURCE_ID)
+    // Nothing left behind: the casing layer used to survive teardown, which kept
+    // the source alive with it and made every map unmount log an error.
+    expect(map._layers.size).toBe(0)
+    expect(map._sources.size).toBe(0)
  })

  it('FE-COMP-RESGL-004: destroy survives a map whose style is already gone', () => {
@@ -7,7 +7,7 @@
 // renderers produce the same visual result on the globe or a flat projection.

 import { createElement } from 'react'
-import { renderToStaticMarkup } from 'react-dom/server'
+import { renderIconMarkup } from '../../utils/iconMarkup'
 import type mapboxgl from 'mapbox-gl'
 import { Plane, Train, Ship, Car, Bus, Sailboat, Bike, CarTaxiFront, Route, TramFront } from 'lucide-react'
 import { getTransitMapSegments } from './transitGeometry'
@@ -17,6 +17,8 @@ import type { Reservation, ReservationEndpoint } from '../../types'

 export const RESERVATION_SOURCE_ID = 'trek-reservations'
 export const RESERVATION_LINE_LAYER_ID = 'trek-reservations-lines'
+/** Sits under the coloured transit lines; named here so teardown can find it. */
+export const TRANSIT_CASING_LAYER_ID = `${RESERVATION_LINE_LAYER_ID}-transit-casing`

 type TransportType = 'flight' | 'train' | 'cruise' | 'car' | 'bus' | 'taxi' | 'bicycle' | 'ferry' | 'transit' | 'transport_other'
 const TRANSPORT_TYPES: TransportType[] = ['flight', 'train', 'cruise', 'car', 'bus', 'taxi', 'bicycle', 'ferry', 'transit', 'transport_other']
@@ -153,7 +155,7 @@ function buildItems(reservations: Reservation[]): TransportItem[] {
 // ── DOM helpers for HTML markers ──────────────────────────────────────────
 function endpointMarkerHtml(type: TransportType, label: string | null): string {
  const { icon: IconCmp } = TYPE_META[type]
-  const svg = renderToStaticMarkup(createElement(IconCmp, { size: 13, color: 'white', strokeWidth: 2.5 }))
+  const svg = renderIconMarkup(createElement(IconCmp, { size: 13, color: 'white', strokeWidth: 2.5 }))
  const labelHtml = label ? `<span style="display:inline-flex;align-items:center;line-height:1">${escapeHtml(label)}</span>` : ''
  return `<div style="
    display:inline-flex;align-items:center;justify-content:center;gap:4px;
@@ -218,7 +220,14 @@ export class ReservationMapboxOverlay {
    this.endpointMarkers.forEach(m => m.remove())
    this.endpointMarkers = []
    try {
-      if (this.map.getLayer(RESERVATION_LINE_LAYER_ID)) this.map.removeLayer(RESERVATION_LINE_LAYER_ID)
+      // Every layer before the source: the engine refuses to drop a source while
+      // anything still references it, and it reports that by firing an error event
+      // rather than throwing — so the catch below never saw it and the source was
+      // left behind. The casing layer arrived with the transit paths and was missed
+      // here, which is what made the console complain on every map teardown.
+      for (const id of [TRANSIT_CASING_LAYER_ID, RESERVATION_LINE_LAYER_ID]) {
+        if (this.map.getLayer(id)) this.map.removeLayer(id)
+      }
      if (this.map.getSource(RESERVATION_SOURCE_ID)) this.map.removeSource(RESERVATION_SOURCE_ID)
    } catch { /* map already gone */ }
  }
@@ -229,7 +238,7 @@ export class ReservationMapboxOverlay {
    map.addSource(RESERVATION_SOURCE_ID, { type: 'geojson', data: { type: 'FeatureCollection', features: [] } })
    // White casing under real transit paths so the colored lines read cleanly.
    map.addLayer({
-      id: RESERVATION_LINE_LAYER_ID + '-transit-casing',
+      id: TRANSIT_CASING_LAYER_ID,
      type: 'line',
      source: RESERVATION_SOURCE_ID,
      filter: ['all', ['==', ['get', 'transitPath'], true], ['!=', ['get', 'walk'], true]] as any,
@@ -5,6 +5,8 @@ import { FileText, Info, Clock, MapPin, Navigation, Train, Plane, Bus, Car, Ship
 import { accommodationsApi, mapsApi, pluginsApi } from '../../api/client'
 import type { Trip, Day, Place, Category, AssignmentsMap, DayNote } from '../../types'
 import { isDayInAccommodationRange, getDayOrder } from '../../utils/dayOrder'
+import { safeHexColor } from '../../utils/safeColor'
+import { renderIconMarkup } from '../../utils/iconMarkup'
 import { formatMoney, formatMoneySum, splitReservationDateTime, type MoneyEntry } from '../../utils/formatters'
 import { fetchExchangeRates } from '../../hooks/useExchangeRates'
 import { getFlightLegs, getTrainLegs } from '../../utils/flightLegs'
@@ -41,8 +43,7 @@ const trackColour = (on: boolean) => (on ? 'var(--accent, #111827)' : 'var(--bor
 const knobOffset = (on: boolean) => (on ? '22px' : '2px')

 function renderLucideIcon(icon:LucideIcon, props = {}) {
-  if (!_renderToStaticMarkup) return ''
-  return _renderToStaticMarkup(
+  return renderIconMarkup(
    createElement(icon, props)
  );
 }
@@ -96,17 +97,9 @@ function safeImg(url) {
 }

 // Generate SVG string from Lucide icon name (for category thumbnails)
-let _renderToStaticMarkup = null
-async function ensureRenderer() {
-  if (!_renderToStaticMarkup) {
-    const mod = await import('react-dom/server')
-    _renderToStaticMarkup = mod.renderToStaticMarkup
-  }
-}
 function categoryIconSvg(iconName, color = '#6366f1', size = 24) {
-  if (!_renderToStaticMarkup) return ''
  const Icon = getCategoryIcon(iconName)
-  return _renderToStaticMarkup(
+  return renderIconMarkup(
    createElement(Icon, { size, strokeWidth: 1.8, color: 'rgba(255,255,255,0.92)' })
  )
 }
@@ -180,7 +173,6 @@ interface downloadTripPDFProps {
 // `assignments` is normalised here once — every read below (and fetchPlacePhotos)
 // relies on it being an object.
 export async function downloadTripPDF({ trip, days, places, assignments = {}, categories, dayNotes, reservations = [], t: _t, locale: _locale }: downloadTripPDFProps) {
-  await ensureRenderer()
  const breaksPerDay = pageBreakPerDay()
  const loc = _locale || undefined
  const tr = _t || (k => k)
@@ -354,7 +346,7 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
          const place = item.data.place
          if (!place) return ''
          const cat = categories.find(c => c.id === place.category_id)
-          const color = cat?.color || '#6366f1'
+          const color = safeHexColor(cat?.color, '#6366f1')

          // Image: direct > google photo > fallback icon. Both go through safeImg
          // so the proxy path is resolved to an absolute URL the PDF can load.
@@ -16,13 +16,14 @@ import {
  Wallet,
 } from 'lucide-react';
 import { createElement, useEffect, useRef } from 'react';
-import { renderToStaticMarkup } from 'react-dom/server';
+import { renderIconMarkup } from '../utils/iconMarkup';
 import { MapContainer, Marker, TileLayer, Tooltip, useMap } from 'react-leaflet';
 import { getCategoryIcon } from '../components/shared/categoryIcons';
 import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../constants/mapDefaults';
 import { SUPPORTED_LANGUAGES, useTranslation } from '../i18n';
 import { useSettingsStore } from '../store/settingsStore';
 import { avatarSrc } from '../utils/avatarSrc';
+import { safeHexColor } from '../utils/safeColor';
 import { getMergedItems, getTransportForDay } from '../utils/dayMerge';
 import { isDayInAccommodationRange } from '../utils/dayOrder';
 import { getFlightLegs, getTrainLegs } from '../utils/flightLegs';
@@ -34,9 +35,11 @@ const TRANSPORT_ICONS = { flight: Plane, train: Train, bus: Bus, car: Car, cruis

 function createMarkerIcon(place: any) {
  const cat = place.category;
-  const color = cat?.color || '#6366f1';
+  // This page answers without a guard, so an unescaped colour here reaches
+  // people who have no account on the instance at all.
+  const color = safeHexColor(cat?.color, '#6366f1');
  const CatIcon = getCategoryIcon(cat?.icon);
-  const iconSvg = renderToStaticMarkup(createElement(CatIcon, { size: 14, strokeWidth: 2, color: 'white' }));
+  const iconSvg = renderIconMarkup(createElement(CatIcon, { size: 14, strokeWidth: 2, color: 'white' }));
  return L.divIcon({
    className: '',
    iconSize: [28, 28],
@@ -0,0 +1,69 @@
+import { describe, it, expect } from 'vitest'
+import { createElement } from 'react'
+import { renderToStaticMarkup } from 'react-dom/server'
+import * as lucide from 'lucide-react'
+import { renderIconMarkup } from './iconMarkup'
+
+/**
+ * The whole point of iconMarkup.ts is to stop shipping Fizz to the browser, so
+ * this is the one place allowed to import it — in a test, which never reaches a
+ * bundle. It diffs our serializer against the real thing for every exported
+ * icon, which is what makes "byte-compatible" a checked claim rather than a
+ * comment.
+ */
+
+type IconComponent = Parameters<typeof createElement>[0]
+
+const ICONS = Object.entries(lucide).filter(
+  ([name, value]) =>
+    /^[A-Z]/.test(name) &&
+    typeof value === 'object' &&
+    value !== null &&
+    (value as { $$typeof?: symbol }).$$typeof === Symbol.for('react.forward_ref')
+) as [string, IconComponent][]
+
+// The three prop shapes the call sites actually use.
+const PROP_SHAPES: Record<string, unknown>[] = [
+  {},
+  { size: 13, strokeWidth: 2.5, color: 'white' },
+  { size: 24, strokeWidth: 1.8, color: 'rgba(255,255,255,0.92)', className: 'x' },
+]
+
+describe('renderIconMarkup parity with react-dom/server', () => {
+  it('FE-UTIL-ICONMARKUP-001: the icon set is actually found', () => {
+    // Guards against a lucide restructure silently turning this suite into a no-op.
+    expect(ICONS.length).toBeGreaterThan(1000)
+  })
+
+  it('FE-UTIL-ICONMARKUP-002: every exported icon serializes byte-identically', () => {
+    const mismatches: string[] = []
+    for (const [name, Icon] of ICONS) {
+      for (const props of PROP_SHAPES) {
+        const expected = renderToStaticMarkup(createElement(Icon, props))
+        const actual = renderIconMarkup(createElement(Icon, props))
+        if (actual !== expected) {
+          mismatches.push(`${name} ${JSON.stringify(props)}\n  fizz: ${expected}\n  ours: ${actual}`)
+        }
+      }
+    }
+    expect(mismatches.slice(0, 5).join('\n')).toBe('')
+  })
+
+  it('FE-UTIL-ICONMARKUP-003: viewBox keeps its camel case', () => {
+    // A naive prop-name-to-kebab rule turns this into view-box, and the icon
+    // renders as an invisible zero-size box.
+    expect(renderIconMarkup(createElement(lucide.Utensils))).toContain('viewBox="0 0 24 24"')
+  })
+
+  it('FE-UTIL-ICONMARKUP-004: attribute values are escaped like React escapes them', () => {
+    const hostile = 'red" onload="alert(1)'
+    const expected = renderToStaticMarkup(createElement(lucide.Utensils, { color: hostile }))
+    expect(renderIconMarkup(createElement(lucide.Utensils, { color: hostile }))).toBe(expected)
+    expect(renderIconMarkup(createElement(lucide.Utensils, { color: hostile }))).not.toContain('onload="alert(1)"')
+  })
+
+  it('FE-UTIL-ICONMARKUP-005: a throwing component yields an empty string, not an exception', () => {
+    const Boom = () => { throw new Error('boom') }
+    expect(renderIconMarkup(createElement(Boom))).toBe('')
+  })
+})
@@ -0,0 +1,109 @@
+import type { ReactElement, ReactNode } from 'react'
+
+/**
+ * Static-markup serializer for the lucide icons the map and PDF code turn into
+ * SVG strings.
+ *
+ * It replaces renderToStaticMarkup, whose Fizz runtime is ~190 kB raw / ~57 kB
+ * gzip of a chunk that three lazy route chunks share — including the Leaflet
+ * renderer, which is the default. For output that is always a single <svg> with
+ * a flat list of shape children, that is a poor trade.
+ *
+ * Deliberately generic over React elements rather than over lucide's icon
+ * tables: `iconNode` is closed over inside createLucideIcon and not exported, so
+ * calling the forwardRef render function is what keeps this working across
+ * lucide upgrades. iconMarkup.parity.test.ts diffs the output against real Fizz
+ * for every exported icon, so drift fails the suite instead of shipping.
+ *
+ * Byte-compatibility notes, all verified against react-dom 19.2.6:
+ *  - React escapes & " ' < > the same way in attribute values and in text, so
+ *    one escape function covers both.
+ *  - Only the props in ATTRIBUTE_NAMES are renamed. Everything else keeps its
+ *    literal name, which is what preserves the camelCase SVG attributes —
+ *    viewBox has to stay viewBox and must NOT become view-box.
+ *  - Fizz emits <path …></path>, never a self-closing tag. So do we.
+ */
+
+const FORWARD_REF = Symbol.for('react.forward_ref')
+
+/** React's DOM property -> attribute renames, limited to what an SVG carries. */
+const ATTRIBUTE_NAMES: Record<string, string> = {
+  className: 'class',
+  htmlFor: 'for',
+  strokeWidth: 'stroke-width',
+  strokeLinecap: 'stroke-linecap',
+  strokeLinejoin: 'stroke-linejoin',
+  strokeDasharray: 'stroke-dasharray',
+  strokeDashoffset: 'stroke-dashoffset',
+  strokeOpacity: 'stroke-opacity',
+  strokeMiterlimit: 'stroke-miterlimit',
+  fillOpacity: 'fill-opacity',
+  fillRule: 'fill-rule',
+  clipRule: 'clip-rule',
+  clipPath: 'clip-path',
+  vectorEffect: 'vector-effect',
+  paintOrder: 'paint-order',
+  shapeRendering: 'shape-rendering',
+  stopColor: 'stop-color',
+  stopOpacity: 'stop-opacity',
+}
+
+const SKIP_PROPS = new Set(['children', 'key', 'ref', 'dangerouslySetInnerHTML'])
+
+type ElementProps = Record<string, unknown> & { children?: ReactNode }
+type ForwardRefComponent = { $$typeof: symbol; render: (props: ElementProps, ref: null) => ReactNode }
+
+function escape(value: string | number): string {
+  return String(value)
+    .replace(/&/g, '&amp;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#x27;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+}
+
+function serializeProps(props: ElementProps): string {
+  let out = ''
+  for (const name of Object.keys(props)) {
+    if (SKIP_PROPS.has(name)) continue
+    const value = props[name]
+    if (value === undefined || value === null || value === false) continue
+    if (typeof value === 'function' || typeof value === 'symbol' || typeof value === 'object') continue
+    out += ` ${ATTRIBUTE_NAMES[name] ?? name}="${escape(value === true ? '' : (value as string | number))}"`
+  }
+  return out
+}
+
+function serialize(node: unknown): string {
+  if (node === null || node === undefined || typeof node === 'boolean') return ''
+  if (typeof node === 'string' || typeof node === 'number') return escape(node)
+  if (Array.isArray(node)) return node.map(serialize).join('')
+
+  const element = node as ReactElement<ElementProps>
+  const type = element.type as unknown
+  const props = (element.props ?? {}) as ElementProps
+
+  if (typeof type === 'string') {
+    return `<${type}${serializeProps(props)}>${serialize(props.children)}</${type}>`
+  }
+  if (typeof type === 'object' && type !== null && (type as ForwardRefComponent).$$typeof === FORWARD_REF) {
+    return serialize((type as ForwardRefComponent).render(props, null))
+  }
+  if (typeof type === 'function') {
+    return serialize((type as (p: ElementProps) => ReactNode)(props))
+  }
+  throw new Error(`renderIconMarkup: unsupported element type ${String(type)}`)
+}
+
+/**
+ * Empty string when the element cannot be rendered: a throwing icon component
+ * should cost its own glyph, not the marker it sits in. That matches how the
+ * call sites already guarded renderToStaticMarkup.
+ */
+export function renderIconMarkup(element: ReactElement): string {
+  try {
+    return serialize(element)
+  } catch {
+    return ''
+  }
+}
@@ -0,0 +1,19 @@
+import { hexColorSchema } from '@trek/shared'
+
+/**
+ * Allow-list a colour before it goes into a hand-built HTML string.
+ *
+ * Category and tag colours are plain strings in the older contracts, and both map
+ * renderers paste them straight into a style="…" attribute of markup that ends up
+ * in L.divIcon / innerHTML / setHTML. Escaping alone would stop the attribute
+ * breakout but still let a CSS value through — `url(https://…)` in a background
+ * is a working tracking pixel — so a colour is allow-listed instead: anything
+ * that is not #rgb or #rrggbb falls back.
+ *
+ * Server-side validation closes the write path. This closes the read path, which
+ * is what covers rows written before that validation existed, instances that have
+ * not updated, and anything a compromised admin account left behind.
+ */
+export function safeHexColor(value: string | null | undefined, fallback: string): string {
+  return typeof value === 'string' && hexColorSchema.safeParse(value).success ? value : fallback
+}
@@ -2,6 +2,7 @@ import { Body, Controller, Delete, Get, HttpException, Param, Post, Put, UseGuar
 import type { Category, CategoryListResponse } from '@trek/shared';
 import type { User } from '../../types';
 import { CategoriesService } from './categories.service';
+import { CategoryCreateDto, CategoryUpdateDto } from './categories.dto';
 import { JwtAuthGuard } from '../auth/jwt-auth.guard';
 import { AdminGuard } from '../auth/admin.guard';
 import { CurrentUser } from '../auth/current-user.decorator';
@@ -9,11 +10,17 @@ import { CurrentUser } from '../auth/current-user.decorator';
 /**
 * /api/categories — place-category palette CRUD.
 *
- * Byte-identical to the legacy Express route (server/src/routes/categories.ts):
- * listing is open to any authenticated user; create/update/delete require admin
- * (JwtAuthGuard + AdminGuard). Status codes match the Nest defaults the legacy
- * route also used (201 on create, 200 elsewhere), and the bespoke 400/404 bodies
- * are reproduced exactly.
+ * Listing is open to any authenticated user; create/update/delete require admin
+ * (JwtAuthGuard + AdminGuard). Status codes match the legacy Express route
+ * (201 on create, 200 elsewhere) and the bespoke 404 body is reproduced exactly.
+ *
+ * Bodies now validate against the @trek/shared schemas through the DTO classes
+ * and the global ZodValidationPipe. That replaced the hand-rolled 400 ("Category
+ * name is required") with the pipe's envelope — the same trade the places, todo
+ * and trips migrations made. It matters here because `color` was reaching a
+ * style="…" attribute of hand-built marker HTML on both map renderers and on the
+ * share page, which answers without a guard; `@Body('color')` carries no
+ * metatype, so nothing validated it.
 */
@Controller('api/categories')
 export class CategoriesController {
@@ -27,30 +34,17 @@ export class CategoriesController {

  @Post()
  @UseGuards(JwtAuthGuard, AdminGuard)
-  create(
-    @CurrentUser() user: User,
-    @Body('name') name?: string,
-    @Body('color') color?: string,
-    @Body('icon') icon?: string,
-  ): { category: Category } {
-    if (!name) {
-      throw new HttpException({ error: 'Category name is required' }, 400);
-    }
-    return { category: this.categories.create(user.id, name, color, icon) };
+  create(@CurrentUser() user: User, @Body() body: CategoryCreateDto): { category: Category } {
+    return { category: this.categories.create(user.id, body.name, body.color, body.icon) };
  }

  @Put(':id')
  @UseGuards(JwtAuthGuard, AdminGuard)
-  update(
-    @Param('id') id: string,
-    @Body('name') name?: string,
-    @Body('color') color?: string,
-    @Body('icon') icon?: string,
-  ): { category: Category } {
+  update(@Param('id') id: string, @Body() body: CategoryUpdateDto): { category: Category } {
    if (!this.categories.getById(id)) {
      throw new HttpException({ error: 'Category not found' }, 404);
    }
-    return { category: this.categories.update(id, name, color, icon) };
+    return { category: this.categories.update(id, body.name, body.color, body.icon) };
  }

  @Delete(':id')
@@ -0,0 +1,13 @@
+import { createZodDto } from 'nestjs-zod';
+import { createCategoryRequestSchema, updateCategoryRequestSchema } from '@trek/shared';
+
+/**
+ * createZodDto wrappers over the @trek/shared category contracts.
+ *
+ * Typing a controller parameter with one of these is what lets the global
+ * ZodValidationPipe validate it by metatype — `@Body('color')` reads a property
+ * and carries no metatype, so it was never validated at all. The schemas in
+ * shared/ stay the single source of truth.
+ */
+export class CategoryCreateDto extends createZodDto(createCategoryRequestSchema) {}
+export class CategoryUpdateDto extends createZodDto(updateCategoryRequestSchema) {}
@@ -12,8 +12,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [
  'BookingImportController.confirm',
  'BookingImportController.preview',
  'BookingImportController.previewAsync',
-  'CategoriesController.create',
-  'CategoriesController.update',
  'ImmichMemoriesController.putSettings',
  'ImmichMemoriesController.search',
  'ImmichMemoriesController.test',
@@ -35,6 +35,7 @@ vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: (
 import { CategoriesModule } from '../../src/nest/categories/categories.module';
 import { DatabaseModule } from '../../src/nest/database/database.module';
 import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
+import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe';

 function insertCategory(name: string, color = '#6366f1', icon = '📍', userId = 1): number {
  const res = db
@@ -52,6 +53,11 @@ describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () =>
    const nest = moduleRef.createNestApplication();
    nest.use(cookieParser());
    nest.useGlobalFilters(new TrekExceptionFilter());
+    // The harness builds a container around the one domain module, so the APP_PIPE
+    // from app.module.ts is not in it. Registering it here is what the other e2e
+    // suites do (see places.e2e.test.ts) and it is required for the DTO bodies to
+    // be validated at all.
+    nest.useGlobalPipes(new ZodValidationPipe());
    await nest.init();
    return nest;
  }
@@ -107,7 +113,27 @@ describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () =>
  it('400 when an admin creates without a name', async () => {
    const res = await request(server).post('/api/categories').set('Cookie', sessionCookie(1)).send({});
    expect(res.status).toBe(400);
-    expect(res.body).toEqual({ error: 'Category name is required' });
+  });
+
+  // The colour is pasted into a style="…" attribute of hand-built marker HTML on
+  // both map renderers and on the share page, which answers without a guard. It
+  // was reaching the database unvalidated, because @Body('color') reads a property
+  // and carries no metatype for the pipe to work with.
+  it('400 when an admin sends a colour that is not a hex value', async () => {
+    const res = await request(server)
+      .post('/api/categories')
+      .set('Cookie', sessionCookie(1))
+      .send({ name: 'Food', color: 'red" onmouseover="alert(1)' });
+    expect(res.status).toBe(400);
+  });
+
+  it('400 when an admin updates a category to a non-hex colour', async () => {
+    const id = insertCategory('Food', '#fff', '🍔');
+    const res = await request(server)
+      .put(`/api/categories/${id}`)
+      .set('Cookie', sessionCookie(1))
+      .send({ color: 'url(https://evil.example/px)' });
+    expect(res.status).toBe(400);
  });

  it('200 when an admin updates, COALESCE preserving omitted fields', async () => {
@@ -31,17 +31,13 @@ describe('CategoriesController (parity with the legacy /api/categories route)',
  });

  describe('POST /', () => {
-    it('400 when name is missing', () => {
-      const create = vi.fn();
-      expect(thrown(() => makeController({ create }).create(admin, undefined))).toEqual({
-        status: 400, body: { error: 'Category name is required' },
-      });
-      expect(create).not.toHaveBeenCalled();
-    });
+    // A body without a name no longer reaches the handler: createCategoryRequestSchema
+    // requires it, and the global ZodValidationPipe rejects it by metatype. The
+    // contract itself is covered by shared/src/category/category.schema.spec.ts.

    it('creates and returns { category }', () => {
      const create = vi.fn().mockReturnValue(cat);
-      expect(makeController({ create }).create(admin, 'Food', '#fff', '🍔')).toEqual({ category: cat });
+      expect(makeController({ create }).create(admin, { name: 'Food', color: '#fff', icon: '🍔' })).toEqual({ category: cat });
      expect(create).toHaveBeenCalledWith(1, 'Food', '#fff', '🍔');
    });
  });
@@ -59,7 +55,7 @@ describe('CategoriesController (parity with the legacy /api/categories route)',
    it('updates and returns { category }', () => {
      const getById = vi.fn().mockReturnValue(cat);
      const update = vi.fn().mockReturnValue({ ...cat, name: 'Drinks' });
-      expect(makeController({ getById, update }).update('1', 'Drinks')).toEqual({ category: { ...cat, name: 'Drinks' } });
+      expect(makeController({ getById, update }).update('1', { name: 'Drinks' })).toEqual({ category: { ...cat, name: 'Drinks' } });
      expect(update).toHaveBeenCalledWith('1', 'Drinks', undefined, undefined);
    });
  });
@@ -1,4 +1,5 @@
 import { z } from 'zod';
+import { hexColorSchema } from '../place/place.schema';

 /**
 * Category API contract — single source of truth for the /api/categories endpoints.
@@ -12,6 +13,13 @@ import { z } from 'zod';
 * messages are reproduced in the controller so the bodies stay byte-identical.
 */

+/**
+ * Note the asymmetry below: the response keeps `color: z.string()` while the
+ * requests demand a hex value. Tightening the response would make an instance
+ * fail on its own stored rows — the write path was unvalidated for a long time,
+ * so anything could be in there. The client allow-lists on read for the same
+ * reason (client/src/utils/safeColor.ts).
+ */
 export const categorySchema = z.object({
  id: z.number(),
  name: z.string(),
@@ -24,7 +32,10 @@ export type Category = z.infer<typeof categorySchema>;

 export const createCategoryRequestSchema = z.object({
  name: z.string().min(1),
-  color: z.string().optional(),
+  // Hex only: the value is interpolated into a style="…" attribute of the
+  // hand-built marker HTML on both map renderers, and on the share page that
+  // answers without a guard.
+  color: hexColorSchema.optional(),
  icon: z.string().optional(),
 });
 export type CreateCategoryRequest = z.infer<typeof createCategoryRequestSchema>;
@@ -32,7 +43,10 @@ export type CreateCategoryRequest = z.infer<typeof createCategoryRequestSchema>;
 /** All fields optional — the service COALESCEs each against the stored value. */
 export const updateCategoryRequestSchema = z.object({
  name: z.string().optional(),
-  color: z.string().optional(),
+  // Hex only: the value is interpolated into a style="…" attribute of the
+  // hand-built marker HTML on both map renderers, and on the share page that
+  // answers without a guard.
+  color: hexColorSchema.optional(),
  icon: z.string().optional(),
 });
 export type UpdateCategoryRequest = z.infer<typeof updateCategoryRequestSchema>;