diff --git a/.github/workflows/web-routes.yml b/.github/workflows/web-routes.yml new file mode 100644 index 0000000..a341c09 --- /dev/null +++ b/.github/workflows/web-routes.yml @@ -0,0 +1,27 @@ +name: Web route check + +# web/nginx.conf answers 404 for any path that isn't a route, which means +# it has to know which routes exist. Most it infers from the build output, +# but dynamic routes (dashboards/[id] and friends) and non-prerendered +# routes (/data-sources) have no file on disk and are hand-listed there. +# +# That list drifting is a production-only failure: `vite dev` and +# `npm run preview` route from the client manifest and never read +# nginx.conf, so a new dynamic route works perfectly everywhere a +# developer would look and 404s the moment it ships. This job is what +# catches it. Its own workflow rather than another job bolted onto +# license-compliance.yml, which already carries one unrelated check +# (tenant-boundary) for historical reasons worth not compounding. + +on: + push: + branches: [master, main] + pull_request: + +jobs: + web-routes: + name: nginx route allowlist check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - run: bash hack/check-web-routes.sh diff --git a/hack/README.md b/hack/README.md index cc20a84..4149362 100644 --- a/hack/README.md +++ b/hack/README.md @@ -25,6 +25,13 @@ monorepos (Kubernetes among them). history, then keeps generating in real time. Distinct from `benchmark-fixture/` (volume, for the Phase 2 latency benchmark) and `windows-fixture/` (correctness, for the Windows ingest path). +- `check-web-routes.sh` — asserts `web/nginx.conf`'s hand-maintained + route allowlists still match `web/src/routes`. `nginx.conf` 404s + unknown paths, so it has to name the routes that have no prerendered + file to match (dynamic ones, and any route without `prerender = true`). + Drift here breaks production only — dev and `npm run preview` never + read `nginx.conf` — so this runs in CI, like + `check-tenant-boundary.sh`. - `demo-seed/` — the rest of the demo deployment: its reset script, dashboards, alert rules, and the systemd unit that runs `demo-simulator`. diff --git a/hack/check-web-routes.sh b/hack/check-web-routes.sh new file mode 100755 index 0000000..7155a15 --- /dev/null +++ b/hack/check-web-routes.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# Keeps web/nginx.conf's route allowlists in sync with web/src/routes. +# +# web/nginx.conf answers 404 for any path that isn't a route (see the "404 +# enforcement" block there for why). Most routes need no help from nginx: +# adapter-static prerenders them to a flat .html that try_files +# matches on disk. Two kinds don't, and are named explicitly in nginx.conf +# because there is no file for them to match: +# +# 1. Dynamic routes -- src/routes//[param] -- listed in the +# alternation of nginx.conf's dynamic-route `location ~` regex. +# 2. Routes that never opted into prerendering (no `export const +# prerender = true`) -- each needs its own `location = /`. +# +# Both lists are hand-maintained, and getting them wrong fails *only in +# production*: `vite dev` and `npm run preview` route from the client +# manifest and never consult nginx.conf, so a new dynamic route works +# perfectly in dev and 404s the moment it ships. This script IS the +# enforcement for that. Run in CI on every change; exits non-zero, naming +# the route and the edit to make, on a mismatch. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" + +ROUTES_DIR="web/src/routes" +NGINX_CONF="web/nginx.conf" +fail=0 + +# --- what the app actually declares ----------------------------------- +declare -a dynamic_segments=() unprerendered_routes=() odd_shaped=() + +while IFS= read -r page; do + dir="$(dirname "$page")" + route="${dir#"$ROUTES_DIR"}" # "" for the root route, else /foo/bar + route="${route:-/}" + + if [[ "$route" == *"["* ]]; then + # nginx's regex covers exactly //. A rest + # param, an optional param, a param at the top level, or a param + # nested deeper all need a *different* regex, not another entry in + # the alternation -- so flag rather than silently "fix" them. + if [[ "$route" =~ ^/([^/[]+)/\[[^./][^]]*\]$ ]]; then + dynamic_segments+=("${BASH_REMATCH[1]}") + else + odd_shaped+=("$route") + fi + continue + fi + + # Non-dynamic: prerendered routes land on disk and need no nginx entry. + if [[ -f "$dir/+page.ts" ]] && \ + grep -Eq '^[[:space:]]*export[[:space:]]+const[[:space:]]+prerender[[:space:]]*=[[:space:]]*true' "$dir/+page.ts"; then + continue + fi + # "/" is handled by its own `location = /` and is never fallback-served. + [[ "$route" == "/" ]] && continue + unprerendered_routes+=("$route") +done < <(find "$ROUTES_DIR" -name '+page.svelte' | sort) + +# --- what nginx.conf claims ------------------------------------------- +# The alternation out of `location ~ ^/(?:a|b|c)/[^/]+$ {`. +nginx_segments="$(grep -oE 'location[[:space:]]+~[[:space:]]+\^/\(\?:[^)]+\)/\[\^/\]\+\$' "$NGINX_CONF" \ + | sed -E 's#.*\(\?:([^)]+)\).*#\1#' | tr '|' '\n' | sort -u || true)" +# Every `location = /foo` except the root and the fallback shell itself. +nginx_exact="$(grep -oE 'location[[:space:]]+=[[:space:]]+/[A-Za-z0-9._/-]*' "$NGINX_CONF" \ + | sed -E 's#.*=[[:space:]]+##' | grep -vx '/' | sort -u || true)" + +sorted() { printf '%s\n' "$@" | grep -v '^$' | sort -u; } +app_segments="$(sorted "${dynamic_segments[@]+"${dynamic_segments[@]}"}")" +app_exact="$(sorted "${unprerendered_routes[@]+"${unprerendered_routes[@]}"}")" + +# --- compare ----------------------------------------------------------- +if ((${#odd_shaped[@]})); then + echo "FAIL: route param shape nginx.conf's regex does not cover:" + printf ' %s\n' "${odd_shaped[@]}" + echo " nginx.conf matches only //. Widen the" + echo " dynamic-route location regex there, then update this check." + fail=1 +fi + +echo "Checking: dynamic routes are in nginx.conf's fallback allowlist..." +if [[ "$app_segments" != "$nginx_segments" ]]; then + echo "FAIL: $ROUTES_DIR and $NGINX_CONF disagree on dynamic routes." + comm -23 <(echo "$app_segments") <(echo "$nginx_segments") \ + | sed 's#^# missing from nginx.conf (would 404 in prod): /#' + comm -13 <(echo "$app_segments") <(echo "$nginx_segments") \ + | sed 's#^# stale in nginx.conf (route no longer exists): /#' + echo " Fix: edit the alternation in nginx.conf's dynamic-route location." + fail=1 +else + echo "OK: dynamic routes match (${app_segments//$'\n'/, })" +fi + +echo "Checking: non-prerendered routes have their own nginx.conf location..." +if [[ "$app_exact" != "$nginx_exact" ]]; then + echo "FAIL: $ROUTES_DIR and $NGINX_CONF disagree on non-prerendered routes." + comm -23 <(echo "$app_exact") <(echo "$nginx_exact") \ + | sed 's#^# missing from nginx.conf (would 404 in prod): #' + comm -13 <(echo "$app_exact") <(echo "$nginx_exact") \ + | sed 's#^# stale in nginx.conf (route is prerendered or gone): #' + echo " Fix: add/remove a \`location = { try_files /200.html =404; }\`" + echo " in nginx.conf -- or give the route an \`export const prerender = true\`." + fail=1 +else + echo "OK: non-prerendered routes match (${app_exact//$'\n'/, })" +fi + +exit "$fail" diff --git a/web/README.md b/web/README.md index 1a86f43..60b5403 100644 --- a/web/README.md +++ b/web/README.md @@ -124,6 +124,30 @@ The repo convention prefers distroless/scratch base images. Serving a static SPA still needs *some* HTTP server, though, and `nginx:alpine` is the boring, standard choice for that job — writing a custom static-file binary just to stay distroless would be more engineering than a Phase 0 -placeholder page justifies. `nginx.conf` here is minimal: serve `build/`, -fall back to `index.html` for client-side routing (only one route exists -today, but this is what you want the moment a second one is added). +placeholder page justifies. `nginx.conf` serves `build/`, and resolves a +request in this order: the file itself, then the flat `.html` +adapter-static prerenders each route to, then — for the handful of routes +that have no file on disk — the `200.html` SPA shell. + +### 404s + +Anything that matches none of the above answers **404**, not 200. That +took explicit work, because the natural static-SPA config falls back to +the shell unconditionally and hands every junk URL a success status; +crawlers, uptime checks and vulnerability scanners then can't tell a real +page from a miss. The 404 still *renders* the shell, so a human sees the +app's own not-found page exactly as before — only the status line +changed. + +Two kinds of route legitimately have no file to match and so are named +explicitly in `nginx.conf`: dynamic routes (`dashboards/[id]` and +friends), whose params don't exist at build time, and routes that never +opted into prerendering (`/data-sources`, which has no `+page.ts`). Those +two allowlists are the only thing here that can drift out of sync with +`src/routes` — and drift would break *only production*, since `vite dev` +and `npm run preview` route from the client manifest and never read +`nginx.conf`. `hack/check-web-routes.sh` fails CI when they disagree; run +it after adding a dynamic or non-prerendered route. + +Trailing slashes redirect (308) to the canonical no-slash form rather +than 404ing, matching SvelteKit's default `trailingSlash: 'never'`. diff --git a/web/nginx.conf b/web/nginx.conf index 1950aa5..a0a9d27 100644 --- a/web/nginx.conf +++ b/web/nginx.conf @@ -3,6 +3,18 @@ server { root /usr/share/nginx/html; index index.html; + # Every redirect this file can emit (the trailing-slash canonicaliser + # below, plus nginx's own built-in directory redirects) must be a bare + # path, not an absolute URL. With the default `absolute_redirect on`, + # nginx reconstructs the origin from its own `listen` port and answers + # a request for https://demo.cairnobs.org/settings/ with + # `Location: http://127.0.0.1:3000/settings` -- the container's + # internal address, unreachable from the client, and a downgrade to + # http on top of it. Verified by curl against the built image, and + # latent in this file before the redirect below existed too, via the + # directory redirect on /dev. + absolute_redirect off; + # Security-audit remediation (M-3): baseline browser security headers, # absent entirely before this. HSTS/nosniff/frame-options/referrer- # policy/permissions-policy carry no functional risk to this app and @@ -40,16 +52,93 @@ server { add_header Permissions-Policy "geolocation=(), camera=(), microphone=()" always; add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; connect-src *; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'" always; + # ---- 404 enforcement ------------------------------------------------- + # + # A request for a path this site doesn't have must answer 404. It used + # to answer 200: `location /`'s try_files chain ended in an + # unconditional /200.html, so every unrouted path -- /wp-login.php, + # /.env, a typo'd inbound link -- got the SPA shell back with a success + # status. In a browser that still *looked* right (the client router + # renders its not-found page either way), which is exactly why it went + # unnoticed, but nothing reading the status line could tell a real page + # from a miss: crawlers indexed junk URLs, uptime/link checkers saw + # every 404 as healthy, and every probe for a vulnerable path came back + # looking like a hit. + # + # The fix has to be selective rather than a blanket `=404`, because two + # kinds of genuinely-valid route have no file on disk to match: + # + # 1. Dynamic routes (src/routes/dashboards/[id] and friends) -- the + # param isn't known at build time, so there is nothing to + # prerender and the SPA shell IS the correct 200 response. + # 2. /data-sources -- an ordinary static route that simply never + # opted into prerendering (it has no +page.ts, so no `export const + # prerender = true`), leaving adapter-static to serve it from the + # fallback like a dynamic one. + # + # Everything else is prerendered to a flat .html that matches on + # disk, so adding a normal prerendered route needs no change here. The + # two allowlists below are the whole drift surface, and + # hack/check-web-routes.sh fails CI if a new dynamic or non-prerendered + # route isn't reflected in them. + + # 404s keep the SPA shell as their body, so what a human sees is + # unchanged from before -- SvelteKit's own not-found page, rendered + # client-side, styled like the rest of the app. Only the status line is + # corrected. `error_page` with no `=code` override preserves the 404; + # writing `error_page 404 =200 /200.html` would reintroduce the exact + # bug this block exists to fix. + error_page 404 /200.html; + + # Canonicalize trailing slashes instead of 404ing them. SvelteKit's + # default is `trailingSlash: 'never'`, so /settings is canonical and + # /settings/ is a stale-but-real inbound link shape. It used to resolve + # by falling through to the SPA shell, which the strict try_files below + # no longer does -- without this redirect, tightening the fallback + # would silently turn every trailing-slash link into a 404. Anchored so + # "/" itself (which needs at least three characters to match) is + # untouched. 308 rather than 301 to preserve the method. + location ~ ^(/.+)/$ { + return 308 $1; + } + + # Dynamic routes -- /agents/, /alerts/, /dashboards/, + # /hosts/. Exactly one trailing segment, so /dashboards/a/b is + # not a route and still 404s. $uri.html stays ahead of the fallback in + # the chain because /alerts/new is a real prerendered page that happens + # to match this same shape and must keep serving its own file. + location ~ ^/(?:agents|alerts|dashboards|hosts)/[^/]+$ { + try_files $uri $uri.html /200.html; + } + + # Non-prerendered static route (case 2. above). Exact match, so it + # can't shadow anything below it. + location = /data-sources { + try_files /200.html =404; + } + + # "/" is the one directory URL that is a real route -- served + # explicitly so `$uri/` can stay out of the chain below. + location = / { + try_files /index.html =404; + } + location / { # adapter-static writes prerendered routes as flat .html # files (e.g. /dashboards -> dashboards.html, confirmed by # actually inspecting the build output), not /index.html -- # $uri.html has to be in this chain or a request for exactly - # "/dashboards" falls straight through to the SPA fallback and - # skips the prerendered page it should be serving. 200.html (not - # index.html) is the fallback shell for genuinely dynamic routes - # (dashboards/[id] etc.) -- named differently so it doesn't - # collide with "/" -> index.html, which really is prerendered. - try_files $uri $uri.html $uri/ /200.html; + # "/dashboards" falls straight through and skips the prerendered + # page it should be serving. The chain now ends in =404 rather + # than /200.html; the locations above carry the cases that + # legitimately still need the fallback shell. + # + # `$uri/` is deliberately NOT in this chain. build/ has directories + # that are not routes (dev/, alerts/, _app/, fonts/, icons/), none + # of them containing an index.html, and matching them here made + # nginx 301 /dev -> /dev/, which the trailing-slash canonicaliser + # then bounced straight back to /dev: an infinite redirect loop on + # a path that should simply 404. Caught by curling the built image. + try_files $uri $uri.html =404; } }