Stopping the server waits for report file writes under way (closes #122)

The report buffer's stop now waits, through a sync.WaitGroup, for
flushLoop and for the write Append starts at 10 MiB before its final
flush. It used to return at once, so the process exiting cut such a
write off and its reports were lost.

A write under way that fails puts its reports back in the buffer, so
the final flush writes them; the stop fails only if that flush fails
too.

Tests hold the periodic write and the one at 10 MiB open in a synctest
bubble and find the stop waiting until each is released; a third makes
the held write fail and finds its report written by the final flush.

Model: opus-5-5

Dockerfile

@@ -5,11 +5,11 @@
 #
 # The lint and test phases are the gates: `make lint` (script/lint)
 # builds the lint stage alone and `make test` (script/test) the test
-# stage alone, and the builder stage depends on both, so the image
+# stage alone, and the two build stages, frontend-build and builder,
-# cannot be built unless they pass. Each covers the frontend as well,
+# depend on both, so the image cannot be built unless they pass. Each
-# through a copy from a node stage. Inside them each tool is invoked
+# phase covers the frontend as well, through a copy from a node stage.
-# directly, never through make or script/, whose lint and test are
+# Inside them each tool is invoked directly, never through make or
-# themselves docker builds.
+# script/, whose lint and test are themselves docker builds.
 
 # Frontend lint stage: eslint with the rules in eslint.config.js. The
 # lint phase below runs it.
@@ -35,30 +35,26 @@ RUN golangci-lint run --config .golangci.yml ./...
 # phase run it.
 COPY --from=frontend-lint /app/yarn.lock /dev/null
 
-# Frontend stage: the unit tests in test/unit/, then the production
+# Frontend test stage: the unit tests in test/unit/. The test phase
-# build into dist/, which the runtime stage serves. The test phase below
+# below runs it, and the frontend-build stage starts from it. The tests
-# runs it. The tests print a dot each; if any fails, they run again with
+# print a dot each; if any fails, they run again with every test listed,
-# every test listed, and the step fails even if that run passes.
+# and the step fails even if that run passes. NODE_OPTIONS chooses the
-# NODE_OPTIONS chooses the reporter because yarn adds its arguments
+# reporter because yarn adds its arguments after the test files, where
-# after the test files, where node would take a reporter option for one
+# node would take a reporter option for one more file.
-# more file.
 # node:22-alpine as of 2026-02-22
-FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS frontend
+FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS frontend-test
 WORKDIR /app
 COPY package.json yarn.lock ./
 RUN yarn install --frozen-lockfile
-# vite.config.js reads the commit for the page's footer with git.
-RUN apk add --no-cache git
 COPY . .
 RUN NODE_OPTIONS=--test-reporter=dot timeout 90 yarn --silent run test || \
     { echo "--- Rerunning with every test listed for details ---"; \
       NODE_OPTIONS=--test-reporter=spec timeout 90 yarn --silent run test; \
       exit 1; }
-RUN yarn build
 
 # Test phase: the backend's tests with the race detector and coverage,
-# and the frontend's through the copy from the frontend stage at the
+# and the frontend's through the copy from the frontend-test stage at
-# end. -race needs cgo and so a C compiler, which the Debian Go image
+# the end. -race needs cgo and so a C compiler, which the Debian Go image
 # ships and the alpine one does not. -timeout 90s is a backstop above
 # the 60-second cap on the suite. The rerun with -v only shows details:
 # the step fails however it ends, because the first run already failed.
@@ -75,9 +71,21 @@ RUN --mount=type=tmpfs,target=/go/pkg/mod \
     go test -timeout 90s -race -cover ./... || \
     { echo "--- Rerunning with -v for details ---"; \
       go test -timeout 90s -race -v ./...; exit 1; }
-# Nothing is wanted from the frontend stage; the copy is what makes
+# Nothing is wanted from the frontend-test stage; the copy is what makes
 # this phase run its tests.
-COPY --from=frontend /app/yarn.lock /dev/null
+COPY --from=frontend-test /app/yarn.lock /dev/null
+
+# Frontend build stage: the production build into dist/, which the
+# runtime stage serves, on the yarn packages and source of the
+# frontend-test stage. Nothing is wanted from the two phases; the copies
+# are what make BuildKit build them first, so this stage cannot run
+# unless lint and test passed.
+FROM frontend-test AS frontend-build
+COPY --from=lint /src/go.sum /dev/null
+COPY --from=test /src/go.sum /dev/null
+# vite.config.js reads the commit for the page's footer with git.
+RUN apk add --no-cache git
+RUN yarn build
 
 # Backend build stage. Nothing is wanted from the two phases; the copies
 # are what make BuildKit build them first, so this stage cannot run
@@ -131,7 +139,7 @@ RUN addgroup -g 1000 -S netwatch && \
 RUN rm /etc/nginx/conf.d/default.conf
 COPY configs/nginx.conf /etc/nginx/templates/netwatch.conf.template
 COPY configs/security-headers.conf /etc/nginx/security-headers.conf
-COPY --from=frontend /app/dist /usr/share/nginx/html
+COPY --from=frontend-build /app/dist /usr/share/nginx/html
 COPY --from=builder /src/netwatch-server /usr/local/bin/netwatch-server
 COPY bin/entrypoint.sh /usr/local/bin/entrypoint.sh
 

README.md

@@ -59,8 +59,9 @@ either one is broken. We provide:
 - `script/clean` — remove what `script/build` writes
 - `script/projectname` — print the project name (used for the Docker image tag)
 - `script/test` — build the `test` phase of `Dockerfile` without the cache: the
-  frontend's unit tests and production build in its `frontend` stage, and the
+  frontend's unit tests in its `frontend-test` stage, and the backend's Go tests
-  backend's Go tests with the race detector and coverage
+  with the race detector and coverage. The frontend's production build is left
+  to the image build, in the `frontend-build` stage
 - `script/lint` — build the `lint` phase of `Dockerfile` without the cache:
   eslint in its `frontend-lint` stage, and golangci-lint over the Go packages
 - `script/fmt` — format all files (writes): prettier over the JavaScript, CSS,
@@ -78,8 +79,8 @@ either one is broken. We provide:
 - `script/frontend-test` — run the unit tests in `test/unit/` on the host with
   Node's built-in test runner, through the `test` script in `package.json`, and
   if any fails, run them again listing every test, and fail; then the production
-  build. Each test run has a 90-second timeout. `make test` runs the same in
+  build. Each test run has a 90-second timeout. `make test` runs the same tests
-  Docker
+  in Docker, without the production build
 - `script/frontend-fmt` — format the JavaScript, CSS, HTML and Markdown with
   prettier (writes)
 - `script/frontend-fmt-check` — check prettier formatting (read-only)
@@ -199,23 +200,25 @@ image, `make run` builds it and runs it on port 8080.
 | `DATA_DIR`             | `./data/reports`     | Directory for compressed reports                                                                         |
 | `DATA_DIR_MAX_BYTES`   | `1073741824` (1 GiB) | Most bytes of report files kept in `DATA_DIR`, oldest deleted first; see [Report limits](#report-limits) |
 | `DEBUG`                | `false`              | Enable debug logging                                                                                     |
-| `TRUSTED_PROXIES`      | loopback + RFC1918   | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
+| `TRUSTED_PROXIES`      | RFC 1918 ranges      | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
 | `REPORTS_PER_MINUTE`   | `60`                 | Reports each client address may send a minute; see [Report limits](#report-limits)                       |
 | `CORS_ALLOWED_ORIGINS` | empty                | Comma-separated origins whose pages may call the API; see [CORS](#cors)                                  |
 | `METRICS_USERNAME`     | empty                | Basic auth user name for `/metrics`; see [Metrics](#metrics)                                             |
 | `METRICS_PASSWORD`     | empty                | Basic auth password for `/metrics`; see [Metrics](#metrics)                                              |
 | `SENTRY_DSN`           | empty                | DSN of the Sentry project to send errors to; see [Sentry](#sentry)                                       |
 
-`TRUSTED_PROXIES` defaults to
+`TRUSTED_PROXIES` defaults to `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. A list
-`127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. The loopback
+replaces the default; set and empty, the server trusts no proxy. Loopback is not
-entries cover a reverse proxy on the same host. A request whose direct peer is
+in the default, so a reverse proxy on the same host has to be named, as in
-outside this set has its forwarded headers ignored, and the direct peer is
+`TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set
-logged and rate-limited instead. The container image does not use this default;
+has its forwarded headers ignored, and the direct peer is logged and
-see [Deployment](#deployment).
+rate-limited instead. The container image does not use this default; see
+[Deployment](#deployment).
 
 A variable set to a value the server cannot use, such as `PORT=abc`,
 `DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from
-starting, with an error naming the variable. An empty variable counts as unset.
+starting, with an error naming the variable. An empty variable counts as unset,
+except `TRUSTED_PROXIES`.
 
 #### Report storage
 
@@ -239,9 +242,15 @@ non-zero.
 #### Report limits
 
 `POST /api/v1/reports` takes reports from anyone who can reach it, without
-credentials, so it is bounded instead. Both refusals below answer with the same
+credentials, so it is bounded instead. Each refusal below answers with the same
 `{"status":"error"}` body as any other error.
 
+- **JSON only.** A report must be sent with `Content-Type: application/json`,
+  parameters such as `charset` allowed, as the page sends it; any other is
+  refused with 415, and nothing of it is stored. A page on another origin can
+  send a `text/plain` POST, or a form, without a CORS preflight; refusing them
+  keeps any site from making its visitors' browsers store reports (see
+  [CORS](#cors)).
 - **Rate limit.** Each client address, resolved through `TRUSTED_PROXIES`, may
   send `REPORTS_PER_MINUTE` reports a minute; past that it gets 429 with
   `Retry-After: 60`. The minute slides: reports from the minute before still
@@ -414,9 +423,10 @@ address nginx passes on and no other. The image:
 
 - Listens on port 8080 by default (override with `PORT` env var)
 - Takes the client address from `X-Forwarded-For` only on requests from the
-  reverse proxies named in `TRUSTED_PROXIES`, and by default from none.
+  reverse proxies named in `TRUSTED_PROXIES`, by default from any address in
-  `bin/entrypoint.sh` checks each entry with `netwatch-server check-cidr`, which
+  `10.0.0.0/8`, `172.16.0.0/12` or `192.168.0.0/16`. `bin/entrypoint.sh` checks
-  parses it as the backend parses its own `TRUSTED_PROXIES`
+  each entry with `netwatch-server check-cidr`, which parses it as the backend
+  parses its own `TRUSTED_PROXIES`
 - Sends access logs to stdout
 - Caches static assets with immutable headers
 - Sends the security headers `REPO_POLICIES.md` requires on every response, as
@@ -435,9 +445,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
 
 - **Port:** container port `8080`.
 - **Volume:** container path `/data`; the reports are kept in `/data/reports`.
-- **Environment variables:** none is required. An empty one counts as unset, and
+- **Environment variables:** none is required. An empty one counts as unset,
-  one set to a value netwatch cannot use stops the container at start, with the
+  except `TRUSTED_PROXIES`, and one set to a value netwatch cannot use stops the
-  reason in its log.
+  container at start, with the reason in its log.
     - `PORT`, default `8080`: the container port, from 1 to 65535. `8081` cannot
       be used: the backend listens on it inside the container
     - `REPORTS_PER_MINUTE`, default `60`: reports each client address may send a
@@ -451,16 +461,17 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
       in: `/data` or a path below it, with no `.` or `..` part and no extra `/`.
       The container also stops if the path goes through a symbolic link that
       leads out of `/data` or is written as a full path
-    - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy
+    - `TRUSTED_PROXIES`, default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`: the
-      in front of the container connects from, as an IP address or CIDR; several
+      addresses the reverse proxy in front of the container connects from, as IP
-      are separated by commas. nginx takes the client address from
+      addresses or CIDRs separated by commas. nginx takes the client address
-      `X-Forwarded-For` only on a request from one of them, and the rate limit
+      from `X-Forwarded-For` only on a request from one of them, and the rate
-      counts that address. Unset, `X-Forwarded-For` is ignored and every client
+      limit counts that address. A list replaces the default. Set and empty,
-      behind the proxy shares the proxy's one allowance of `REPORTS_PER_MINUTE`.
+      `X-Forwarded-For` is ignored and every client behind the proxy shares the
-      Name only addresses nothing but the proxy connects from: any client that
+      proxy's one allowance of `REPORTS_PER_MINUTE`. Any client that connects
-      connects from one can write its own `X-Forwarded-For`, and through a port
+      from one of these addresses can write its own `X-Forwarded-For`, and
-      Docker publishes, every client may connect from the Docker network's
+      through a port Docker publishes, every client may connect from the Docker
-      gateway, such as `172.17.0.1`.
+      network's gateway, such as `172.17.0.1`, which the default includes; where
+      that matters, name only the addresses nothing but the proxy connects from.
     - `METRICS_USERNAME` and `METRICS_PASSWORD`, default empty: with both set,
       the backend records Prometheus metrics of its requests and serves them at
       `/metrics` on the container port, to requests with this user name and

TODO.md

@@ -21,6 +21,43 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
 
 # Completed Steps
 
+- 2026-10-14: stopping the server waits for a report file write already under
+  way, the periodic flush's or the one `Append` starts when 10 MiB are waiting,
+  before the final flush
+  ([#122](https://git.eeqj.de/sneak/netwatch/issues/122)). It used to return at
+  once, and the process exiting cut the write off, losing its reports. A write
+  under way that fails puts its reports back in memory, so the final flush
+  writes them; the stop fails, and the process exits non-zero, only if that
+  flush fails too. Two tests hold the periodic write and the one at 10 MiB open
+  and find the stop waiting until each is released; a third makes the held write
+  fail and finds its report written by the final flush. They run in a
+  `testing/synctest` bubble, so the periodic flush comes without waiting a
+  minute
+- 2026-10-14: `make test` no longer runs the frontend's production build, which
+  the tests do not need ([#118](https://git.eeqj.de/sneak/netwatch/issues/118)).
+  `yarn build` moved out of the stage that runs the frontend's unit tests, now
+  named `frontend-test`, into a new `frontend-build` stage, which starts from it
+  and, like the `builder` stage, waits on the lint and test phases, so the image
+  still cannot be built unless they pass. The runtime stage copies `dist/` from
+  it, and the `git` that `vite.config.js` reads the commit with is installed
+  there. No build cache was added
+- 2026-10-14: opening the debug log no longer raises the measured latencies
+  ([#131](https://git.eeqj.de/sneak/netwatch/issues/131)). With the panel open,
+  each line written drew the panel again with every line before it, up to 1000,
+  and a check that ends while the page draws is timed only when the drawing is
+  done. Each line is now added at the end of the open panel, its first line is
+  dropped once it holds more than 1000, and it scrolls to its newest line once a
+  frame rather than once a line; the whole panel is drawn only when it is
+  opened. A unit test writes more lines than the panel keeps and checks that the
+  lines already shown are left as they are
+- 2026-10-14: unset, the backend's own `TRUSTED_PROXIES` trusts `10.0.0.0/8`,
+  `172.16.0.0/12` and `192.168.0.0/16`, and no longer loopback; a list replaces
+  that; set and empty, it trusts no proxy, where it used to get the default, as
+  viper reads an empty environment variable as unset
+  ([#129](https://git.eeqj.de/sneak/netwatch/issues/129)). A value it cannot
+  parse still stops the start, and the image still starts the backend with
+  `TRUSTED_PROXIES=127.0.0.1/32`. Tests load the config with it unset, empty and
+  set to a list
 - 2026-10-14: while report files cannot be written, `POST /api/v1/reports`
   answers 500, and storing recovers once writing works again, with no restart
   ([#123](https://git.eeqj.de/sneak/netwatch/issues/123)). A failed write puts
@@ -32,6 +69,29 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
   others check that a report appended while a write runs is kept, behind that
   write's own, when the write fails, and that the cap still holds exactly once a
   failed write's reports are written
+- 2026-10-14: a request whose handler panics is logged with status 500 and
+  counted with `code` 500 in the metrics
+  ([#125](https://git.eeqj.de/sneak/netwatch/issues/125)), not 200. `Recoverer`
+  is still the first middleware; `Write500OnPanic`, after the request log and
+  after the metrics, writes the 500 as the panic passes and lets it go on to
+  Sentry and `Recoverer`, which now writes a 500 only when no status was written
+- 2026-10-14: with `TRUSTED_PROXIES` unset, the container's nginx trusts the
+  three private ranges of RFC 1918, `10.0.0.0/8`, `172.16.0.0/12` and
+  `192.168.0.0/16` ([#119](https://git.eeqj.de/sneak/netwatch/issues/119)), as
+  sneak ruled on 2026-09-22 for every app; it trusted no proxy before. A list
+  replaces them, and set and empty it trusts none. The backend inside the
+  container still trusts only `127.0.0.1/32`
+- 2026-10-14: `POST /api/v1/reports` takes only a report sent with
+  `Content-Type: application/json`, parameters such as `charset` allowed, and
+  refuses any other with 415 and the usual error body, storing nothing
+  ([#126](https://git.eeqj.de/sneak/netwatch/issues/126)). A page on another
+  origin can send a `text/plain` POST without a CORS preflight, so any site
+  could make its visitors' browsers store reports; a JSON POST waits for the
+  preflight, which only an origin in `CORS_ALLOWED_ORIGINS` passes. The page
+  already sent `application/json`. A test posts a report as `text/plain`, as
+  `application/x-www-form-urlencoded`, as `multipart/form-data`, with no
+  `Content-Type`, and as `application/json` with and without a charset, and
+  checks which are stored
 - 2026-10-14: changing the interval no longer drops the samples not yet reported
   to the backend ([#124](https://git.eeqj.de/sneak/netwatch/issues/124)). The
   interval menu resets each host with the new `HostState.reset`, which moves the
@@ -463,3 +523,9 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
 - A configurable host list (an environment variable or a config file)
 - Export of the latency history (CSV or JSON)
 - A notification when the health status changes to DEGRADED
+- `make test` within the 20-second target
+  ([#118](https://git.eeqj.de/sneak/netwatch/issues/118)): every run compiles
+  the Go standard library and every dependency with the race detector from
+  nothing. Whether test builds may keep Go's compiled packages between runs
+  waits on sneak's ruling in
+  [prompts#113](https://git.eeqj.de/sneak/prompts/issues/113)

bin/entrypoint.sh

@@ -34,9 +34,11 @@ fi
 
 # TRUSTED_PROXIES names the reverse proxies in front of the container,
 # as IP addresses or CIDRs separated by commas. nginx takes the client
-# address from X-Forwarded-For only on a request from one of them, so
+# address from X-Forwarded-For only on a request from one of them.
-# unset or empty, it trusts no one. configs/nginx.conf includes the file
+# Unset, they are the three private ranges of RFC 1918; set and empty,
-# written here, one set_real_ip_from line per entry.
+# nginx trusts no one, which is why the default below follows - and not
+# :-. configs/nginx.conf includes the file written here, one
+# set_real_ip_from line per entry.
 #
 # nginx looks up an entry it cannot read as an address as a hostname,
 # and trusts what it finds (1.2.3 is found as 1.2.0.3). So each entry
@@ -45,7 +47,7 @@ fi
 # TRUSTED_PROXIES. Its error, naming the CIDR, is dropped for the one
 # below, naming the entry as written. set -f keeps a * in an entry from
 # becoming a list of file names.
-TRUSTED_PROXIES="${TRUSTED_PROXIES:-}"
+TRUSTED_PROXIES="${TRUSTED_PROXIES-10.0.0.0/8,172.16.0.0/12,192.168.0.0/16}"
 set -f
 for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do
     case "$proxy" in

configs/nginx.conf

@@ -18,9 +18,10 @@ server {
 
     # The client address comes from X-Forwarded-For only on a request
     # from the reverse proxies in TRUSTED_PROXIES: bin/entrypoint.sh
-    # writes one set_real_ip_from line for each into this file, and
+    # writes one set_real_ip_from line for each into this file. Unset,
-    # leaves it empty when TRUSTED_PROXIES is unset, so that by default
+    # TRUSTED_PROXIES is the three private ranges of RFC 1918; set and
-    # the client address is the one each request comes from.
+    # empty, the file is empty and the client address is the one each
+    # request comes from.
     include /etc/nginx/trusted-proxies.conf;
     real_ip_header X-Forwarded-For;
     real_ip_recursive on;

internal/config/config.go

@@ -9,6 +9,7 @@ import (
 	"math"
 	"net/netip"
 	"net/url"
+	"os"
 	"strconv"
 	"strings"
 
@@ -21,13 +22,11 @@ import (
 )
 
 // defaultTrustedProxies lists the networks whose forwarded
-// headers are honoured by default: IPv4 and IPv6 loopback,
+// headers are honoured when TRUSTED_PROXIES is unset: the
-// for a reverse proxy on the same host, and the RFC1918
+// RFC 1918 ranges. The container image does not use it:
-// ranges. The container image does not use it:
 // bin/entrypoint.sh gives the server 127.0.0.1/32, since
 // nginx is its only client there.
-const defaultTrustedProxies = "127.0.0.1/32,::1/128," +
+const defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
-	"10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
 
 // Default limits on stored reports; README.md gives the reasons for
 // these values.
@@ -135,6 +134,14 @@ func New(
 			viper.GetString("DEBUG"), errNotBool)
 	}
 
+	// An empty TRUSTED_PROXIES trusts no proxy, but viper reads an
+	// empty environment variable as unset and would give the default,
+	// so the environment is read first.
+	trustedProxies, set := os.LookupEnv("TRUSTED_PROXIES")
+	if !set {
+		trustedProxies = viper.GetString("TRUSTED_PROXIES")
+	}
+
 	s := &Config{
 		BindAddress:        viper.GetString("BIND_ADDRESS"),
 		CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")),
@@ -146,7 +153,7 @@ func New(
 		Port:               port,
 		ReportsPerMinute:   viper.GetInt("REPORTS_PER_MINUTE"),
 		SentryDSN:          viper.GetString("SENTRY_DSN"),
-		TrustedProxies:     splitList(viper.GetString("TRUSTED_PROXIES")),
+		TrustedProxies:     splitList(trustedProxies),
 		log:                log,
 		params:             &params,
 	}

internal/config/config_test.go

@@ -1,6 +1,8 @@
 package config_test
 
 import (
+	"os"
+	"slices"
 	"strings"
 	"testing"
 
@@ -29,6 +31,62 @@ func requireConfigError(t *testing.T, setting string) {
 	}
 }
 
+// requireTrustedProxies builds the config as main does and fails the
+// test unless its TrustedProxies are want.
+func requireTrustedProxies(t *testing.T, want []string) {
+	t.Helper()
+
+	var cfg *config.Config
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Provide(globals.New, logger.New, config.New),
+		fx.Populate(&cfg),
+	)
+
+	err := app.Err()
+	if err != nil {
+		t.Fatalf("config error = %v", err)
+	}
+
+	if !slices.Equal(cfg.TrustedProxies, want) {
+		t.Fatalf("TrustedProxies = %q, want %q", cfg.TrustedProxies, want)
+	}
+}
+
+// TestTrustedProxiesUnsetTrustsRFC1918: unset, the server trusts the
+// RFC 1918 ranges and nothing else, loopback included.
+func TestTrustedProxiesUnsetTrustsRFC1918(t *testing.T) {
+	// t.Setenv first, so the variable is put back as it was after the
+	// test.
+	t.Setenv("TRUSTED_PROXIES", "")
+
+	err := os.Unsetenv("TRUSTED_PROXIES")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	requireTrustedProxies(t,
+		[]string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"})
+}
+
+// TestTrustedProxiesEmptyTrustsNothing: set and empty, the server
+// trusts no proxy. viper alone would read the empty variable as unset
+// and give the default.
+func TestTrustedProxiesEmptyTrustsNothing(t *testing.T) {
+	t.Setenv("TRUSTED_PROXIES", "")
+
+	requireTrustedProxies(t, []string{})
+}
+
+// TestTrustedProxiesListReplacesDefault: a list is used as given, with
+// none of the default ranges added.
+func TestTrustedProxiesListReplacesDefault(t *testing.T) {
+	t.Setenv("TRUSTED_PROXIES", "172.17.0.1/32, 2001:db8::/32")
+
+	requireTrustedProxies(t, []string{"172.17.0.1/32", "2001:db8::/32"})
+}
+
 // TestSettingsLoadAsGiven: valid values pass the checks and are used
 // as given. bin/entrypoint.sh starts the server with these
 // BIND_ADDRESS and PORT values.

internal/handlers/handlers.go

@@ -5,7 +5,9 @@ package handlers
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"log/slog"
+	"mime"
 	"net/http"
 
 	"sneak.berlin/go/netwatch/internal/globals"
@@ -18,6 +20,10 @@ import (
 
 const jsonContentType = "application/json; charset=utf-8"
 
+// errNotJSON is what decodeJSON returns for a request whose
+// Content-Type is not application/json.
+var errNotJSON = errors.New("content type is not application/json")
+
 // reportAppender is the subset of the report buffer the handlers
 // depend on. Defining it here keeps the storage failure path
 // exercisable with a stub in tests.
@@ -80,14 +86,25 @@ func (s *Handlers) respondJSON(
 	}
 }
 
-// decodeJSON decodes the request body into v. The body is
+// decodeJSON decodes the request body into v. It returns errNotJSON,
-// expected to already be bounded by the body-size middleware, so
+// reading nothing, unless the request's Content-Type is
-// a caller can distinguish an over-limit body from malformed
+// application/json, with or without parameters such as charset. The
-// JSON by testing the returned error for *http.MaxBytesError.
+// body is expected to already be bounded by the body-size middleware,
+// so a caller can distinguish an over-limit body from malformed JSON
+// by testing the returned error for *http.MaxBytesError.
 func (s *Handlers) decodeJSON(
 	_ http.ResponseWriter,
 	r *http.Request,
 	v any,
 ) error {
+	// A page on another origin can POST text/plain or a form without
+	// a CORS preflight, so its visitors' browsers would send it
+	// whatever the CORS settings. A JSON POST waits for the
+	// preflight, so only JSON is taken.
+	mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
+	if err != nil || mediaType != "application/json" {
+		return errNotJSON
+	}
+
 	return json.NewDecoder(r.Body).Decode(v)
 }

internal/handlers/report.go

@@ -30,7 +30,7 @@ type report struct {
 }
 
 // HandleReport returns a handler that accepts telemetry
-// reports from NetWatch clients.
+// reports from NetWatch clients, sent as application/json.
 func (s *Handlers) HandleReport() http.HandlerFunc {
 	type response struct {
 		Status string `json:"status"`
@@ -66,9 +66,15 @@ func (s *Handlers) HandleReport() http.HandlerFunc {
 }
 
 // decodeErrorStatus logs a report decode failure and returns the
-// status to send: 413 when the body exceeded the size limit,
+// status to send: 415 when the report was not sent as JSON, 413 when
-// otherwise 400 for malformed JSON.
+// the body exceeded the size limit, otherwise 400 for malformed JSON.
 func (s *Handlers) decodeErrorStatus(err error) int {
+	if errors.Is(err, errNotJSON) {
+		s.log.Warn("report refused: Content-Type is not application/json")
+
+		return http.StatusUnsupportedMediaType
+	}
+
 	var tooLarge *http.MaxBytesError
 	if errors.As(err, &tooLarge) {
 		s.log.Warn("report body too large", "limit_bytes", tooLarge.Limit)

internal/handlers/report_test.go

@@ -27,6 +27,17 @@ type stubAppender struct {
 
 func (s stubAppender) Append(any) error { return s.err }
 
+// recordingAppender keeps every report the handler stores.
+type recordingAppender struct {
+	reports []any
+}
+
+func (a *recordingAppender) Append(v any) error {
+	a.reports = append(a.reports, v)
+
+	return nil
+}
+
 func newTestHandlers(buf stubAppender, out io.Writer) *handlers.Handlers {
 	return handlers.NewForTest(buf, slog.New(slog.NewJSONHandler(out, nil)))
 }
@@ -83,6 +94,7 @@ func TestHandleReportAcceptsValidReports(t *testing.T) {
 				http.MethodPost, "/api/v1/reports",
 				strings.NewReader(tt.body),
 			)
+			req.Header.Set("Content-Type", "application/json")
 
 			h.HandleReport().ServeHTTP(rec, req)
 
@@ -103,6 +115,89 @@ func TestHandleReportAcceptsValidReports(t *testing.T) {
 	}
 }
 
+// TestHandleReportStoresOnlyJSON checks that a report is stored only
+// when sent as application/json, as the page sends it. One sent as
+// text/plain or as a form, which a page on another origin can send
+// without a CORS preflight, is refused with 415.
+func TestHandleReportStoresOnlyJSON(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		contentType string
+		wantStatus  int
+		wantStored  int
+	}{
+		{
+			name:        "text/plain",
+			contentType: "text/plain;charset=UTF-8",
+			wantStatus:  http.StatusUnsupportedMediaType,
+			wantStored:  0,
+		},
+		{
+			name:        "a urlencoded form",
+			contentType: "application/x-www-form-urlencoded",
+			wantStatus:  http.StatusUnsupportedMediaType,
+			wantStored:  0,
+		},
+		{
+			name:        "a multipart form",
+			contentType: "multipart/form-data; boundary=x",
+			wantStatus:  http.StatusUnsupportedMediaType,
+			wantStored:  0,
+		},
+		{
+			name:        "no Content-Type",
+			contentType: "",
+			wantStatus:  http.StatusUnsupportedMediaType,
+			wantStored:  0,
+		},
+		{
+			name:        "application/json",
+			contentType: "application/json",
+			wantStatus:  http.StatusOK,
+			wantStored:  1,
+		},
+		{
+			name:        "application/json with a charset",
+			contentType: "application/json;charset=UTF-8",
+			wantStatus:  http.StatusOK,
+			wantStored:  1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var stored recordingAppender
+
+			h := handlers.NewForTest(&stored, slog.New(slog.DiscardHandler))
+
+			rec := httptest.NewRecorder()
+			req := httptest.NewRequestWithContext(t.Context(),
+				http.MethodPost, "/api/v1/reports",
+				strings.NewReader(`{"clientId":"c1","hosts":[]}`),
+			)
+
+			if tt.contentType != "" {
+				req.Header.Set("Content-Type", tt.contentType)
+			}
+
+			h.HandleReport().ServeHTTP(rec, req)
+
+			if rec.Code != tt.wantStatus {
+				t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus)
+			}
+
+			if len(stored.reports) != tt.wantStored {
+				t.Errorf("reports stored = %d, want %d",
+					len(stored.reports), tt.wantStored)
+			}
+		})
+	}
+}
+
 func TestHandleReportStorageFailureIsNon2xx(t *testing.T) {
 	t.Parallel()
 
@@ -113,6 +208,7 @@ func TestHandleReportStorageFailureIsNon2xx(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"c1","hosts":[]}`),
 	)
+	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -138,6 +234,7 @@ func TestHandleReportFullIs507(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"c1","hosts":[]}`),
 	)
+	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -161,6 +258,7 @@ func TestHandleReportMalformedJSONIs400(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{not json`),
 	)
+	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -185,6 +283,8 @@ func TestHandleReportOversizeIs413(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`),
 	)
+	req.Header.Set("Content-Type", "application/json")
+
 	// No declared length, so only the middleware's read cap can
 	// stop this body.
 	req.ContentLength = -1
@@ -213,6 +313,7 @@ func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
 			`{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`,
 		),
 	)
+	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -245,6 +346,7 @@ func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
 			`{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`,
 		),
 	)
+	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 
@@ -284,6 +386,7 @@ func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) {
 		http.MethodPost, "/api/v1/reports",
 		strings.NewReader(`{"hosts":[{"history":[{"t":`+huge+`}]}]}`),
 	)
+	req.Header.Set("Content-Type", "application/json")
 
 	h.HandleReport().ServeHTTP(rec, req)
 

internal/middleware/middleware.go

@@ -297,12 +297,16 @@ func (s *Middleware) MaxBodyBytes(
 
 // Recoverer returns middleware that recovers from a panic in a
 // downstream handler, logs the panic and stack trace through
-// slog, and responds 500 with no body. http.ErrAbortHandler is
+// slog, and responds 500 with no body, unless a status was already
-// re-panicked so the server can abort the response as intended.
+// written, as Write500OnPanic writes one for a handler's panic.
+// http.ErrAbortHandler is re-panicked so the server can abort the
+// response as intended.
 func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(
 			func(w http.ResponseWriter, r *http.Request) {
+				ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
+
 				defer func() {
 					rec := recover()
 					if rec == nil {
@@ -320,10 +324,43 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 						"stack", string(debug.Stack()),
 					)
 
-					w.WriteHeader(http.StatusInternalServerError)
+					if ww.Status() == 0 {
+						ww.WriteHeader(http.StatusInternalServerError)
+					}
 				}()
 
-				next.ServeHTTP(w, r)
+				next.ServeHTTP(ww, r)
+			},
+		)
+	}
+}
+
+// Write500OnPanic returns middleware that writes 500 when a downstream
+// handler panics before writing a status, and lets the panic go on to
+// Sentry and Recoverer. Recoverer, which comes first, would write its
+// 500 only after the request log and the metrics had recorded the
+// status they saw; placed after each of them, this makes them record
+// the 500.
+func (s *Middleware) Write500OnPanic() func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(
+			func(w http.ResponseWriter, r *http.Request) {
+				ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
+
+				defer func() {
+					rec := recover()
+					if rec == nil {
+						return
+					}
+
+					if ww.Status() == 0 {
+						ww.WriteHeader(http.StatusInternalServerError)
+					}
+
+					panic(rec)
+				}()
+
+				next.ServeHTTP(ww, r)
 			},
 		)
 	}

internal/middleware/middleware_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"log"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -16,7 +17,9 @@ import (
 	"sneak.berlin/go/netwatch/internal/logger"
 	"sneak.berlin/go/netwatch/internal/middleware"
 
+	"github.com/go-chi/chi/v5"
 	chimiddleware "github.com/go-chi/chi/v5/middleware"
+	"github.com/prometheus/client_golang/prometheus"
 )
 
 const (
@@ -323,6 +326,172 @@ func TestRecovererRepanicsOnAbortHandler(t *testing.T) {
 	}
 }
 
+// TestHandlerPanicIsLoggedAndCountedAs500 sends a request to a handler
+// that panics through the middleware SetupRoutes puts around a route
+// with metrics, in its order, leaving out those that write no status
+// for a panic and Sentry, which TestSentry covers. The request log line
+// and the metrics must give the 500 the client gets, and the panic must
+// still reach Recoverer, which logs it.
+func TestHandlerPanicIsLoggedAndCountedAs500(t *testing.T) {
+	t.Parallel()
+
+	var logbuf bytes.Buffer
+
+	mw := middleware.NewWithLogger(
+		slog.New(slog.NewJSONHandler(&logbuf, nil)),
+	)
+	registry := prometheus.NewRegistry()
+
+	handler := chi.Chain(
+		mw.Recoverer(),
+		chimiddleware.RequestID,
+		mw.Logging(),
+		mw.Write500OnPanic(),
+		mw.Metrics(registry),
+		mw.Write500OnPanic(),
+	).HandlerFunc(func(http.ResponseWriter, *http.Request) {
+		panic("boom")
+	})
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody)
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, want %d",
+			rec.Code, http.StatusInternalServerError)
+	}
+
+	request := findLogLine(t, logbuf.Bytes(), "request")
+	if request.Status != http.StatusInternalServerError {
+		t.Errorf("request log status = %d, want %d",
+			request.Status, http.StatusInternalServerError)
+	}
+
+	recovered := findLogLine(t, logbuf.Bytes(), "panic recovered")
+	if recovered.Panic != "boom" {
+		t.Errorf("panic recovered line has panic %q, want %q",
+			recovered.Panic, "boom")
+	}
+
+	for _, name := range []string{
+		"http_request_duration_seconds", "http_response_size_bytes",
+	} {
+		codes := metricCodes(t, registry, name)
+		if len(codes) != 1 || codes[0] != "500" {
+			t.Errorf("%s code labels = %v, want [500]", name, codes)
+		}
+	}
+}
+
+// loggedLine holds the fields the tests read from a JSON log line.
+type loggedLine struct {
+	Msg    string `json:"msg"`
+	Status int    `json:"status"`
+	Panic  string `json:"panic"`
+}
+
+// findLogLine returns the first of the JSON log lines in logged whose
+// message is msg.
+func findLogLine(t *testing.T, logged []byte, msg string) loggedLine {
+	t.Helper()
+
+	dec := json.NewDecoder(bytes.NewReader(logged))
+
+	for {
+		var line loggedLine
+
+		err := dec.Decode(&line)
+		if err != nil {
+			t.Fatalf("no %q line in the log: %v (%q)", msg, err, logged)
+		}
+
+		if line.Msg == msg {
+			return line
+		}
+	}
+}
+
+// metricCodes returns the code label of each series of the metric
+// family name in registry.
+func metricCodes(
+	t *testing.T,
+	registry *prometheus.Registry,
+	name string,
+) []string {
+	t.Helper()
+
+	families, err := registry.Gather()
+	if err != nil {
+		t.Fatalf("gather metrics: %v", err)
+	}
+
+	var codes []string
+
+	for _, family := range families {
+		if family.GetName() != name {
+			continue
+		}
+
+		for _, metric := range family.GetMetric() {
+			for _, label := range metric.GetLabel() {
+				if label.GetName() == "code" {
+					codes = append(codes, label.GetValue())
+				}
+			}
+		}
+	}
+
+	return codes
+}
+
+// TestRecovererWritesNoSecondStatus checks, on a real server, that
+// Recoverer does not write its 500 again after Write500OnPanic has
+// written one, which the server would log as a superfluous
+// WriteHeader call.
+func TestRecovererWritesNoSecondStatus(t *testing.T) {
+	t.Parallel()
+
+	var serverLog bytes.Buffer
+
+	mw := middleware.NewWithLogger(slog.New(slog.DiscardHandler))
+
+	server := httptest.NewUnstartedServer(chi.Chain(
+		mw.Recoverer(),
+		mw.Write500OnPanic(),
+	).HandlerFunc(func(http.ResponseWriter, *http.Request) {
+		panic("boom")
+	}))
+	server.Config.ErrorLog = log.New(&serverLog, "", 0)
+	server.Start()
+
+	req, err := http.NewRequestWithContext(t.Context(),
+		http.MethodGet, server.URL, http.NoBody)
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+
+	resp, err := server.Client().Do(req)
+	if err != nil {
+		t.Fatalf("request: %v", err)
+	}
+
+	_ = resp.Body.Close()
+
+	// Close waits for the handler to return, so whatever the server
+	// logs for this request is in serverLog after it.
+	server.Close()
+
+	if resp.StatusCode != http.StatusInternalServerError {
+		t.Errorf("status = %d, want %d",
+			resp.StatusCode, http.StatusInternalServerError)
+	}
+
+	if serverLog.Len() != 0 {
+		t.Errorf("server logged %q", serverLog.String())
+	}
+}
+
 // TestLoggingCutsRequestStringsToBound sends an over-long URL and
 // over-long header values, and checks the request log writes each
 // one cut to logger.MaxLoggedFieldBytes.

internal/reportbuf/reportbuf.go

@@ -96,6 +96,10 @@ type Buffer struct {
 	// refuses reports, and the reports of that write wait in buf for
 	// the next flush.
 	writeErr error
+	// writers are the goroutines that write report files: flushLoop,
+	// and each one Append starts at flushSizeThreshold. OnStop waits
+	// for them before the final flush.
+	writers sync.WaitGroup
 }
 
 // New creates a Buffer and registers lifecycle hooks to
@@ -147,7 +151,7 @@ func New(
 
 			b.mu.Unlock()
 
-			go b.flushLoop()
+			b.writers.Go(b.flushLoop)
 
 			return nil
 		},
@@ -158,7 +162,14 @@ func New(
 			var err error
 
 			b.stopOnce.Do(func() {
+				// Closing done ends flushLoop after any flush it is
+				// running. Wait for it and for the writes Append
+				// started, which the process exiting would cut off;
+				// one that fails puts its reports back in buf, for the
+				// final flush to write.
 				close(b.done)
+				b.writers.Wait()
+
 				err = b.flushLocked()
 			})
 
@@ -211,12 +222,12 @@ func (b *Buffer) Append(v any) error {
 		data := b.drainBuf()
 		b.mu.Unlock()
 
-		go func() {
+		b.writers.Go(func() {
 			writeErr := b.writeFile(data)
 			if writeErr != nil {
 				b.log.Error("flush reports failed", "error", writeErr)
 			}
-		}()
+		})
 
 		return nil
 	}

internal/reportbuf/reportbuf_test.go

@@ -15,6 +15,7 @@ import (
 	"sync"
 	"sync/atomic"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"sneak.berlin/go/netwatch/internal/config"
@@ -107,9 +108,174 @@ func TestFailedFinalFlushFailsStop(t *testing.T) {
 	}
 }
 
-// startBuffer starts a Buffer through fx, as main does, with the
+// TestStopWaitsForPeriodicWrite holds open the write the periodic flush
-// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
+// starts, then stops the app. The stop must wait until the write is
-func startBuffer(t *testing.T) *reportbuf.Buffer {
+// released, and the report must then be in its file. The test runs in
+// a synctest bubble, whose clock moves on at once when every goroutine
+// in it is waiting, so the periodic flush does not take a minute.
+func TestStopWaitsForPeriodicWrite(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	synctest.Test(t, func(t *testing.T) {
+		buf, app := startApp(t)
+
+		created := make(chan struct{})
+		release := make(chan struct{})
+
+		buf.OnFileCreated(func(*os.File) {
+			created <- struct{}{}
+
+			<-release
+		})
+
+		err := buf.Append(map[string]string{"id": "periodic"})
+		if err != nil {
+			t.Fatalf("append report: %v", err)
+		}
+
+		<-created
+
+		err = stopWhileHeld(t, app, release)
+		if err != nil {
+			t.Fatalf("stop: %v", err)
+		}
+
+		files, err := readReportFiles(dir)
+		if err != nil {
+			t.Fatalf("read report files: %v", err)
+		}
+
+		want := `{"id":"periodic"}` + "\n"
+		if len(files) != 1 || files[0] != want {
+			t.Fatalf("report files = %q, want one holding %q", files, want)
+		}
+	})
+}
+
+// TestStopWaitsForWriteAtSizeThreshold holds open the write Append
+// starts when the buffer reaches FlushSizeThreshold, then stops the
+// app. The stop must wait until the write is released, and the reports
+// must then be in its file.
+func TestStopWaitsForWriteAtSizeThreshold(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	synctest.Test(t, func(t *testing.T) {
+		buf, app := startApp(t)
+
+		created := make(chan struct{})
+		release := make(chan struct{})
+
+		buf.OnFileCreated(func(*os.File) {
+			created <- struct{}{}
+
+			<-release
+		})
+
+		appended := appendToThreshold(t, buf)
+
+		<-created
+
+		err := stopWhileHeld(t, app, release)
+		if err != nil {
+			t.Fatalf("stop: %v", err)
+		}
+
+		files, err := readReportFiles(dir)
+		if err != nil {
+			t.Fatalf("read report files: %v", err)
+		}
+
+		if len(files) != 1 || files[0] != appended {
+			t.Fatalf("%d report files, want one holding the %d bytes "+
+				"appended", len(files), len(appended))
+		}
+	})
+}
+
+// TestFinalFlushWritesReportsOfFailedWrite holds open the write the
+// periodic flush starts, makes it fail, then stops the app. The stop
+// waits for the write, which puts its report back in the buffer, and
+// the final flush writes it, so the stop succeeds with nothing lost.
+func TestFinalFlushWritesReportsOfFailedWrite(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	synctest.Test(t, func(t *testing.T) {
+		buf, app := startApp(t)
+
+		created := make(chan *os.File)
+		release := make(chan struct{})
+
+		buf.OnFileCreated(func(f *os.File) {
+			created <- f
+
+			<-release
+		})
+
+		err := buf.Append(map[string]string{"id": "failed"})
+		if err != nil {
+			t.Fatalf("append report: %v", err)
+		}
+
+		held := <-created
+
+		// Closing the file under the write makes the write fail.
+		_ = held.Close()
+
+		// The final flush's write goes through.
+		buf.OnFileCreated(func(*os.File) {})
+
+		err = stopWhileHeld(t, app, release)
+		if err != nil {
+			t.Fatalf("stop: %v", err)
+		}
+
+		files, err := readReportFiles(dir)
+		if err != nil {
+			t.Fatalf("read report files: %v", err)
+		}
+
+		want := `{"id":"failed"}` + "\n"
+		if len(files) != 1 || files[0] != want {
+			t.Fatalf("report files = %q, want one holding %q", files, want)
+		}
+	})
+}
+
+// stopWhileHeld stops app while a report file write is held open until
+// release is closed, and fails the test if the stop returns before
+// that. It then closes release and returns the stop's error. It must
+// be called in a synctest bubble.
+func stopWhileHeld(
+	t *testing.T,
+	app *fxtest.App,
+	release chan<- struct{},
+) error {
+	t.Helper()
+
+	stopped := make(chan error, 1)
+
+	go func() { stopped <- app.Stop(t.Context()) }()
+
+	// Wait returns once every other goroutine in the bubble is waiting:
+	// the stop has got as far as it can while the write is held.
+	synctest.Wait()
+
+	if len(stopped) != 0 {
+		t.Error("stop returned while a report file write was held open")
+	}
+
+	close(release)
+
+	return <-stopped
+}
+
+// startApp starts a Buffer through fx, as main does, with the DATA_DIR
+// and DATA_DIR_MAX_BYTES the calling test has set, and returns it with
+// the app, for tests that stop the app themselves.
+func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	t.Helper()
 
 	var buf *reportbuf.Buffer
@@ -127,6 +293,16 @@ func startBuffer(t *testing.T) *reportbuf.Buffer {
 	app.RequireStart()
 	t.Cleanup(app.RequireStop)
 
+	return buf, app
+}
+
+// startBuffer starts a Buffer through fx, as main does, with the
+// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
+func startBuffer(t *testing.T) *reportbuf.Buffer {
+	t.Helper()
+
+	buf, _ := startApp(t)
+
 	return buf
 }
 
@@ -892,6 +1068,32 @@ func TestFlushAtSizeThreshold(t *testing.T) {
 
 	buf := startBuffer(t)
 
+	appended := appendToThreshold(t, buf)
+
+	// Append writes the file in the background, so wait for it.
+	deadline := time.Now().Add(10 * time.Second)
+
+	for {
+		files, err := readReportFiles(dir)
+		if err == nil && len(files) == 1 && files[0] == appended {
+			return
+		}
+
+		if time.Now().After(deadline) {
+			t.Fatalf("%d report files (error: %v), want one holding the "+
+				"%d bytes appended", len(files), err, len(appended))
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
+// appendToThreshold appends reports to buf until it holds
+// FlushSizeThreshold bytes, so that the last append starts writing them
+// to a report file, and returns the lines appended.
+func appendToThreshold(t *testing.T, buf *reportbuf.Buffer) string {
+	t.Helper()
+
 	// Large reports, so the threshold takes a few hundred appends.
 	pad := strings.Repeat("a", 64<<10)
 
@@ -914,22 +1116,7 @@ func TestFlushAtSizeThreshold(t *testing.T) {
 		appended.WriteByte('\n')
 	}
 
-	// Append writes the file in the background, so wait for it.
+	return appended.String()
-	deadline := time.Now().Add(10 * time.Second)
-
-	for {
-		files, err := readReportFiles(dir)
-		if err == nil && len(files) == 1 && files[0] == appended.String() {
-			return
-		}
-
-		if time.Now().After(deadline) {
-			t.Fatalf("%d report files (error: %v), want one holding the "+
-				"%d bytes appended", len(files), err, appended.Len())
-		}
-
-		time.Sleep(10 * time.Millisecond)
-	}
 }
 
 // reportFilesBytes returns the total size of the report files in dir.

internal/server/routes.go

@@ -34,13 +34,16 @@ func (s *Server) SetupRoutes() {
 	s.router.Use(s.mw.Recoverer())
 	s.router.Use(middleware.RequestID)
 	s.router.Use(s.mw.Logging())
+	// s.mw.Recoverer would write a panic's 500 only after the panic has
+	// passed the request log, so it is written here, where the log sees it.
+	s.router.Use(s.mw.Write500OnPanic())
 	s.router.Use(s.mw.SecurityHeaders())
 	s.router.Use(s.mw.CORS(s.params.Config.CORSAllowedOrigins))
 	s.router.Use(s.mw.MaxBodyBytes(maxRequestBodyBytes))
 	s.router.Use(middleware.Timeout(requestTimeout))
 
-	// Sentry reports a panic, then panics again, so that s.mw.Recoverer
+	// Sentry reports a panic, then panics again, so that it still
-	// still answers 500.
+	// reaches s.mw.Recoverer.
 	if s.params.Config.SentryDSN != "" {
 		s.router.Use(sentryhttp.New(sentryhttp.Options{Repanic: true}).Handle)
 	}
@@ -64,6 +67,8 @@ func (s *Server) SetupRoutes() {
 		// other.
 		if s.params.Config.MetricsUsername != "" {
 			r.Use(s.mw.Metrics(registry))
+			// Likewise inside the metrics.
+			r.Use(s.mw.Write500OnPanic())
 		}
 
 		r.Get("/.well-known/healthcheck", s.h.HandleHealthCheck())

internal/server/routes_test.go

@@ -66,6 +66,7 @@ func TestReportsAreRateLimited(t *testing.T) {
 			http.MethodPost, "/api/v1/reports",
 			strings.NewReader(`{"clientId":"c1","hosts":[]}`),
 		)
+		req.Header.Set("Content-Type", "application/json")
 		srv.ServeHTTP(rec, req)
 
 		return rec.Code

script/frontend-test

@@ -2,9 +2,10 @@
 # script/frontend-test: run the frontend test suite on the host: the
 # unit tests in test/unit/, through the test script in package.json,
 # then the production build, which fails on broken code. make test runs
-# the same in the frontend stage of Dockerfile. The tests print a dot
+# the same tests in the frontend-test stage of Dockerfile, and the image
-# each; if any fails, they run again with every test listed, and the
+# build runs the production build in its frontend-build stage. The tests
-# script fails even if that run passes. NODE_OPTIONS chooses the
+# print a dot each; if any fails, they run again with every test listed,
+# and the script fails even if that run passes. NODE_OPTIONS chooses the
 # reporter because yarn adds its arguments after the test files, where
 # node would take a reporter option for one more file.
 set -eu

script/frontend-viewport-test

@@ -64,8 +64,8 @@ main() {
     # configs/nginx.conf is a template: the image renders it over its own
     # default.conf, with the same port and limit bin/entrypoint.sh uses.
     # The empty file it includes trusts no proxy, as bin/entrypoint.sh
-    # writes it when TRUSTED_PROXIES is unset. It also includes the
+    # writes it when TRUSTED_PROXIES is set and empty. It also includes
-    # security headers, so the page runs under the shipped policy.
+    # the security headers, so the page runs under the shipped policy.
     docker run -d --rm --name "$SERVER" \
         --network "$NETWORK" --network-alias netwatch \
         -e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \

test/unit/main.test.js

@@ -13,20 +13,22 @@ import {
     humanDuration,
     latencyClass,
     latencyHex,
+    log,
     measureLatency,
     Reporter,
     tick,
 } from "../../web/src/main.js";
 
 // There is no page here, so the tests stand in for it. The debug log looks
-// for its panel by id and finds none. Each element of a host's row that
+// for its panel by id and finds none, an element the page creates is a
-// tick or greyOutUI draws into is a plain object, made the first time a
+// plain object, and no frame the page asks for ever comes. Each element of
-// test looks it up and kept in elements under its selector until the next
+// a host's row that tick or greyOutUI draws into is a plain object, made
-// test starts. As on a page, writing its text replaces its markup; the
+// the first time a test looks it up and kept in elements under its
-// status dot greyOutUI looks for in it is not there. Drawing a sparkline
+// selector until the next test starts. As on a page, writing its text
-// does nothing; it looks for the pixel ratio on window and finds none. In
+// replaces its markup; the status dot greyOutUI looks for in it is not
-// each test, console.error and console.info print nothing and keep what
+// there. Drawing a sparkline does nothing; it looks for the pixel ratio on
-// they are given.
+// window and finds none. In each test, console.error and console.info
+// print nothing and keep what they are given.
 const doNothing = () => {};
 let elements;
 beforeEach((t) => {
@@ -46,8 +48,11 @@ const canvasContext = {
     arc: doNothing,
 };
 globalThis.window = {};
+globalThis.requestAnimationFrame = doNothing;
+globalThis.cancelAnimationFrame = doNothing;
 globalThis.document = {
     getElementById: () => null,
+    createElement: () => ({}),
     querySelector: (selector) =>
         (elements[selector] ??= {
             getContext: () => canvasContext,
@@ -471,6 +476,39 @@ test("of the checks interval changes cleared before they were reported, the next
     assert.equal(sent.at(-1).t, 120000);
 });
 
+// panel stands in for the open debug log panel, and for the element in it
+// that holds its lines, whichever the page looks up by id; the lines are
+// kept in order in lines. Drawing the whole panel, as opening it does,
+// replaces every line with a new one.
+test("with the debug log open, writing more lines than it keeps adds each to the panel and leaves the lines already shown as they are", (t) => {
+    const lines = [];
+    const panel = {
+        classList: { contains: () => false },
+        children: lines,
+        get firstElementChild() {
+            return lines[0];
+        },
+        append: (line) => lines.push(line),
+        removeChild: (line) => lines.splice(lines.indexOf(line), 1),
+        replaceChildren: (...newLines) =>
+            lines.splice(0, lines.length, ...newLines),
+    };
+    t.mock.method(globalThis.document, "getElementById", () => panel);
+    const kept = CONFIG.maxLogEntries;
+
+    for (let i = 1; i <= kept; i++) log.info(`line ${i}`);
+    const shown = [...lines];
+    for (let i = kept + 1; i <= kept + 5; i++) log.info(`line ${i}`);
+
+    assert.deepEqual(
+        lines.map((line) => line.textContent.match(/line \d+$/)?.[0]),
+        Array.from({ length: kept }, (_, i) => `line ${i + 6}`),
+    );
+    // The first five lines shown were dropped, and the rest are the same
+    // lines, not drawn again.
+    shown.slice(5).forEach((line, i) => assert.equal(lines[i], line));
+});
+
 // The page shows &lt; &gt; &quot; &amp; and &#39; in a row's markup as
 // < > " & and '.
 test(`a target whose name and URL hold < > " & and ' shows those characters in its row`, () => {

web/src/main.js

@@ -144,13 +144,15 @@ const WAN_HOSTS = [
 
 const debugLog = [];
 
-const log = (() => {
+export const log = (() => {
     function append(level, message) {
-        debugLog.push({ timestamp: new Date(), level, message });
+        const entry = { timestamp: new Date(), level, message };
+        debugLog.push(entry);
         if (debugLog.length > CONFIG.maxLogEntries)
             debugLog.splice(0, debugLog.length - CONFIG.maxLogEntries);
         const panel = document.getElementById("debug-panel");
-        if (panel && !panel.classList.contains("hidden")) renderDebugLog();
+        if (panel && !panel.classList.contains("hidden"))
+            appendDebugLogLine(entry);
     }
     return {
         error: (msg) => append("error", msg),
@@ -1107,9 +1109,8 @@ function updateHealthBox(state) {
 
 // --- Debug Log Renderer ------------------------------------------------------
 
-function renderDebugLog() {
+// One line of the debug log panel, its message set as text.
-    const el = document.getElementById("debug-log");
+function debugLogLine(entry) {
-    if (!el) return;
     const levelColors = {
         error: "text-red-400",
         warning: "text-orange-400",
@@ -1117,19 +1118,43 @@ function renderDebugLog() {
         info: "text-gray-300",
         debug: "text-gray-500",
     };
-    el.replaceChildren(
+    const ts = formatUTCTimestamp(entry.timestamp);
-        ...debugLog.map((entry) => {
+    const lvl = entry.level.toUpperCase().padEnd(7);
-            const ts = formatUTCTimestamp(entry.timestamp);
+    const line = document.createElement("div");
-            const lvl = entry.level.toUpperCase().padEnd(7);
+    line.className = levelColors[entry.level] || "text-gray-400";
-            const line = document.createElement("div");
+    line.textContent = `${ts} ${lvl} ${entry.message}`;
-            line.className = levelColors[entry.level] || "text-gray-400";
+    return line;
-            line.textContent = `${ts} ${lvl} ${entry.message}`;
+}
-            return line;
+
-        }),
+// Draws every line of the debug log into the panel, when it is opened.
-    );
+function renderDebugLog() {
+    const el = document.getElementById("debug-log");
+    if (!el) return;
+    el.replaceChildren(...debugLog.map((entry) => debugLogLine(entry)));
     el.scrollTop = el.scrollHeight;
 }
 
+// The request that scrolls the open panel to its newest line in the next
+// frame. One waits at most, also in a hidden tab, where frames do not run.
+let debugLogScrollFrame = 0;
+
+// Adds entry to the open panel as its last line, and drops the first line
+// once the panel holds more than maxLogEntries. A check that ends while the
+// page is busy is timed only when the page is done, so the lines already
+// shown are left as they are, and the panel is scrolled once a frame
+// rather than once a line: scrolling it lays out all of its lines again.
+function appendDebugLogLine(entry) {
+    const el = document.getElementById("debug-log");
+    if (!el) return;
+    el.append(debugLogLine(entry));
+    if (el.children.length > CONFIG.maxLogEntries)
+        el.removeChild(el.firstElementChild);
+    cancelAnimationFrame(debugLogScrollFrame);
+    debugLogScrollFrame = requestAnimationFrame(() => {
+        el.scrollTop = el.scrollHeight;
+    });
+}
+
 // --- Sorting -----------------------------------------------------------------
 
 // Sort WAN hosts: pinned hosts first (alphabetically), then unpinned hosts