diff --git a/.dockerignore b/.dockerignore index 27fac04..416281f 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,5 +1,6 @@ node_modules dist +tmp .DS_Store *.log .claude diff --git a/backend/.editorconfig b/.editorconfig similarity index 100% rename from backend/.editorconfig rename to .editorconfig diff --git a/.gitea/workflows/check.yml b/.gitea/workflows/check.yml index 860d9dd..08c2ebc 100644 --- a/.gitea/workflows/check.yml +++ b/.gitea/workflows/check.yml @@ -6,5 +6,7 @@ jobs: steps: # actions/checkout v4.2.2, 2026-02-22 - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - - run: script/cibuild - - run: docker build -f Dockerfile.backend . + # script/cibuild bootstraps, runs every check and builds the + # image. script/bootstrap links what it installs into + # ~/.local/bin, so that has to be on PATH for the rest. + - run: PATH="$HOME/.local/bin:$PATH" script/cibuild diff --git a/.gitignore b/.gitignore index 9451024..e676a31 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,28 @@ -node_modules/ -dist/ +# OS .DS_Store +Thumbs.db + +# Editors +*.swp +*.swo +*~ +*.bak +.idea/ +.vscode/ +*.sublime-* + +# Node +node_modules/ + +# Environment / secrets +.env +.env.* +*.pem +*.key + +# Build output +dist/ +tmp/ + +# Logs *.log diff --git a/.prettierignore b/.prettierignore index d1a0b78..d0374bc 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,5 +1,6 @@ backend/ dist/ node_modules/ +tmp/ yarn.lock .claude/ diff --git a/Dockerfile b/Dockerfile index ed37836..d13e53e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,21 +1,97 @@ +# The one image netwatch ships: nginx serves the built frontend and +# passes /api/ and /.well-known/healthcheck to netwatch-server, the Go +# backend, which runs in the same container on loopback only. +# bin/entrypoint.sh starts and watches both. + +# Lint stage — fast feedback on formatting and lint issues. The +# golangci/golangci-lint image ships Go, gofmt, make and the linter, so +# nothing is installed here. The root make lint builds this stage alone. +# golangci/golangci-lint:v2.12.2 (2026-08-10) +FROM golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS lint + +WORKDIR /src +COPY backend/go.mod backend/go.sum ./ +RUN go mod download +COPY backend/ . +RUN make fmt-check +RUN make lint + +# Backend build stage +# golang:1.25-alpine (2026-02-27) +FROM golang:1.25-alpine@sha256:f6751d823c26342f9506c03797d2527668d095b0a15f1862cddb4d927a7a4ced AS builder + +RUN apk add --no-cache make + +WORKDIR /src + +# Force BuildKit to run the lint stage before proceeding. BuildKit runs +# stages in parallel by default; without this no-op copy a lint failure +# would not gate compilation. +COPY --from=lint /src/go.sum /dev/null + +COPY backend/go.mod backend/go.sum ./ +RUN go mod download +COPY backend/ . + +RUN make test + +# make build is a shim around backend/script/build, the one definition +# of the build command: +# CGO_ENABLED=0 go build -trimpath -ldflags "-s -w -X main.Version=... -X main.Buildarch=..." +# That script reads VERSION from the environment, so it is handed over +# there rather than as a make variable. +ARG VERSION=dev +RUN VERSION="${VERSION}" make build + +# Frontend stage # node:22-alpine as of 2026-02-22 -FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS build +FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS frontend WORKDIR /app COPY package.json yarn.lock ./ RUN yarn install --frozen-lockfile RUN apk add --no-cache git make COPY . . -# make check runs script/check (test + lint + fmt-check); its test step -# is the production yarn build, so this both produces dist/ and gates the -# image on lint/fmt-check/test regressions, not merely a broken build. -RUN make check +# make frontend-check is the frontend half of make check (test + lint + +# fmt-check); its test step is the production yarn build, so this both +# produces dist/ and gates the image on lint/fmt-check/test regressions. +# This node stage has neither Go nor Docker; the lint and builder stages +# above gate the backend half. +RUN make frontend-check +# Runtime stage # nginx:stable-alpine as of 2026-02-22 FROM nginx@sha256:15e96e59aa3b0aada3a121296e3bce117721f42d88f5f64217ef4b18f458c6ab -RUN rm /etc/nginx/conf.d/default.conf -COPY nginx.conf /etc/nginx/conf.d/netwatch.conf -COPY --from=build /app/dist /usr/share/nginx/html +# netwatch-server runs as this user, which owns the report directory. +# nginx keeps the image's own arrangement: its main process runs as +# root, its worker processes as the nginx user. +RUN addgroup -g 1000 -S netwatch && \ + adduser -u 1000 -S netwatch -G netwatch + +# At start-up the nginx image renders every template here into +# conf.d; bin/entrypoint.sh says how. +RUN rm /etc/nginx/conf.d/default.conf +COPY nginx.conf /etc/nginx/templates/netwatch.conf.template +COPY security-headers.conf /etc/nginx/security-headers.conf +COPY --from=frontend /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 + +# bin/entrypoint.sh creates DATA_DIR at start and gives it and /data to +# the netwatch user, whatever is mounted there. +ENV DATA_DIR=/data/reports +VOLUME /data + +# The default public port; PORT changes it. EXPOSE 8080 -CMD ["nginx", "-g", "daemon off;"] +# Requests the backend's health check through nginx, on the port from +# PORT, so it fails unless both answer. upaas reads the result 60 +# seconds after a deploy and fails the deploy unless it is healthy. +HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ + CMD wget -q -O /dev/null "http://127.0.0.1:${PORT:-8080}/.well-known/healthcheck" + +# The nginx image stops its container with SIGQUIT; the entrypoint +# acts on TERM and INT. +STOPSIGNAL SIGTERM +ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] diff --git a/Dockerfile.backend b/Dockerfile.backend deleted file mode 100644 index d97bbbd..0000000 --- a/Dockerfile.backend +++ /dev/null @@ -1,25 +0,0 @@ -# golang:1.25-alpine (2026-02-27) -FROM golang:1.25-alpine@sha256:f6751d823c26342f9506c03797d2527668d095b0a15f1862cddb4d927a7a4ced AS builder - -RUN apk add --no-cache git make gcc musl-dev - -# golangci-lint v2.7.2 (2026-02-27) -RUN CGO_ENABLED=0 go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@9f61b0f53f80672872fced07b6874397c3ed197b - -WORKDIR /repo/backend -COPY backend/go.mod backend/go.sum ./ -RUN go mod download -COPY .git /repo/.git -COPY backend/ . - -RUN make check -RUN make build - -# alpine:3.23 (2026-02-27) -FROM alpine:3.23@sha256:25109184c71bdad752c8312a8623239686a9a2071e8825f20acb8f2198c3f659 - -RUN apk add --no-cache ca-certificates -COPY --from=builder /repo/backend/netwatch-server /usr/local/bin/netwatch-server - -EXPOSE 8080 -ENTRYPOINT ["netwatch-server"] diff --git a/Makefile b/Makefile index 353f708..eb7bb43 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,10 @@ -.PHONY: bootstrap setup dev test lint fmt fmt-check check docker hooks +.PHONY: bootstrap setup dev test lint fmt fmt-check check frontend-check \ + frontend-viewport-test docker hooks # Standard targets are thin shims; the implementations live in script/ # per the scripts-to-rule-them-all pattern (see the Entrypoints section -# of README.md). +# of README.md). test, lint, fmt, fmt-check and check cover the whole +# repo: the frontend here and the Go backend in backend/. bootstrap: @script/bootstrap @@ -28,6 +30,17 @@ fmt-check: check: @script/check +# The frontend half of check, for Dockerfile's node build stage, which +# has neither Go nor Docker. Use check everywhere else. +frontend-check: + @script/frontend-check + +# Responsive-layout verification in a containerised browser. Kept out of +# check: it needs Docker and takes minutes, where make test has to stay +# under 20 seconds. +frontend-viewport-test: + @script/frontend-viewport-test + docker: @script/docker diff --git a/README.md b/README.md index d4dbdd4..acbb62d 100644 --- a/README.md +++ b/README.md @@ -23,29 +23,63 @@ docker build -t netwatch . docker run -p 8080:8080 netwatch ``` +`yarn dev` proxies `/api` to `http://127.0.0.1:8080`, so a locally running +`netwatch-server` (see `backend/`) receives the reports the page posts. + ## Entrypoints This repository adheres to the [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all) standard: normalized scripts in `script/` are the entrypoints for the -development workflow, and the Makefile targets are thin shims that call them. We -provide: +development workflow, and the Makefile targets are thin shims that call them. +The Go backend in `backend/` has its own `script/` directory and shim Makefile +(see [backend/README.md](backend/README.md)). The root scripts cover both +halves, so the root `make check` fails if either one is broken. We provide: -- `script/bootstrap` — install all dependencies (pinned node via nvm if needed, - yarn via corepack, `yarn install --frozen-lockfile`) +- `script/bootstrap` — install all dependencies (the pinned node via nvm unless + one new enough for the frontend's dependencies is installed, yarn via + corepack, `yarn install --frozen-lockfile`, the pinned Go unless one at least + as new as `backend/go.mod` asks for is installed, and the Go modules), linking + what it installs itself into `~/.local/bin`, which has to be on `PATH`. It + installs no Go linter and not Docker: `make lint` runs the linter in Docker - `script/setup` — make a fresh clone ready for development: bootstrap plus the git pre-commit hook - `script/projectname` — print the project name (used for the Docker image tag) -- `script/test` — run the production build as the test (no unit tests yet) -- `script/lint` — run prettier in check mode -- `script/fmt` — format all files (writes) -- `script/fmt-check` — check formatting (read-only) +- `script/test` — run `script/frontend-test`, then the backend's Go tests, both + within one 30-second timeout +- `script/lint` — run `script/frontend-lint`, then golangci-lint in Docker, by + building the lint stage of `Dockerfile` without the cache +- `script/fmt` — format all files (writes): prettier, then gofmt over `backend/` +- `script/fmt-check` — check formatting (read-only): prettier, then gofmt - `script/check` — run test, lint, and fmt-check -- `script/docker` — build the Docker image tagged via `script/projectname` -- `script/cibuild` — CI entrypoint: plain `docker build .` +- `script/frontend-test` — run the production build as the frontend's test (no + unit tests yet) +- `script/frontend-lint` — run prettier in check mode +- `script/frontend-fmt` — format everything prettier understands (writes) +- `script/frontend-fmt-check` — check prettier formatting (read-only) +- `script/frontend-check` — the frontend half of `script/check`, for + `Dockerfile`, whose node build stage has neither Go nor Docker +- `script/frontend-viewport-test` — responsive-layout verification of the built + frontend in a containerised headless Chrome (see + [test/viewport/README.md](test/viewport/README.md)). Not part of + `script/check`: it needs Docker and takes minutes. +- `script/docker` — build the image from `Dockerfile` without the build cache, + tagged `netwatch` via `script/projectname` +- `script/cibuild` — CI entrypoint: runs `script/bootstrap` and `script/check`, + then builds the image as `script/docker` does, without the build cache - `script/precommit` — run by the git pre-commit hook; runs `script/check` - `script/install-precommit` — install the git pre-commit hook +## Responsive layout + +The narrow-viewport layout lives in the `max-width: 768px` media block in +`src/styles.css`. It is verified automatically by `make frontend-viewport-test`, +which drives a digest-pinned headless Chrome against the built `dist/` and +asserts on computed layout at widths derived from that CSS — one pixel either +side of every breakpoint it declares, plus a 320px floor, a desktop baseline and +two landscape sizes. See [test/viewport/README.md](test/viewport/README.md) for +what it covers and what it genuinely cannot. + ## Rationale When debugging network issues, it's useful to have a persistent at-a-glance view @@ -71,6 +105,18 @@ code lives in `src/main.js` with a class-based architecture: - **`tick()`**: Main loop — measures all hosts in parallel via `Promise.all`, pushes samples, redraws UI. When paused, pushes blank markers (no probes, no false outage) +- **`Reporter`**: Posts collected samples to the backend + +### Reporting + +Every `reportInterval` (default 60s) the page POSTs a JSON report to the +same-origin path `/api/v1/reports`: a random per-browser `clientId` kept in +`localStorage`, `geo` sent as null, and each host's unreported, non-paused +samples (timestamp, latency, error). A per-host high-water mark makes every +report a delta, so only new samples are sent; the mark advances only on a +delivered report, and while paused nothing is sent. Delivery failure is quiet — +one debug-log line per outage, retried at the next interval, never blocking +probing. The report-building step is a pure function of host state. ### Monitoring targets @@ -133,13 +179,60 @@ After running `yarn build`, deploy the contents of the `dist/` directory to any static file host (S3, GCS, Cloudflare Pages, Vercel, Netlify, GitHub Pages) or use the Docker image behind a reverse proxy. -The Docker image: +The Docker image, built from `Dockerfile`, is the whole service in one +container: nginx serves the built frontend and passes `/api/` and +`/.well-known/healthcheck` to the Go backend, `netwatch-server`, which listens +only inside the container, on `127.0.0.1:8081`. The image: - Listens on port 8080 by default (override with `PORT` env var) -- Trusts `X-Forwarded-For` from RFC1918 reverse proxies (10/8, 172.16/12, - 192.168/16) +- Takes the client address from `X-Forwarded-For` only on requests from the + reverse proxies named in `TRUSTED_PROXIES`, and by default from none - Sends access logs to stdout - Caches static assets with immutable headers +- Sends the security headers `REPO_POLICIES.md` requires on every response, as + `security-headers.conf` sets them, in place of the backend's own +- Stores reports in `DATA_DIR`, `/data/reports` by default, on the `/data` + volume. Before the backend starts, the image creates `DATA_DIR` and gives it + and `/data` to user `netwatch` (uid 1000), which the backend runs as, so a + host directory bind-mounted at `/data` ends up owned by uid 1000 +- Writes buffered reports to disk on `docker stop`, and exits non-zero if nginx + or the backend exits on its own, so the platform restarts it + +## Running under upaas + +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 + one set to a value netwatch cannot use stops the 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 + minute + - `DATA_DIR_MAX_BYTES`, default `1073741824` (1 GiB): the most room the + report files may take + - `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call + the API + - `DEBUG`, default `false`: debug logging + - `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside + `/data` do not survive a redeploy + - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy + in front of the container connects from, as an IP address or CIDR; several + are separated by commas. nginx takes the client address from + `X-Forwarded-For` only on a request from one of them, and the rate limit + counts that address. Unset, `X-Forwarded-For` is ignored and every client + behind the proxy shares the proxy's one allowance of `REPORTS_PER_MINUTE`. + Name only addresses nothing but the proxy connects from: any client that + connects from one can write its own `X-Forwarded-For`, and through a port + Docker publishes, every client may connect from the Docker network's + gateway, such as `172.17.0.1`. +- **Health check:** the image's `HEALTHCHECK` requests + `/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless + both nginx and the backend answer. upaas reads the container's health 60 + seconds after a deploy and fails the deploy unless it is `healthy`. The + container also stops when either process exits. ## Browser Compatibility diff --git a/TODO.md b/TODO.md index 587c805..8b17dd2 100644 --- a/TODO.md +++ b/TODO.md @@ -10,23 +10,190 @@ # Status -pre-1.0. No git tags. Backend work in flight on feat/reportbuf-storage (dirty: -src/main.js). Frontend is functional; backend is new and unmerged. +pre-1.0. No git tags. `feat/reportbuf-storage` is merged; the backend, the CI +workflow, and the backend repo standard files are all on `main`. Frontend and +backend are both functional. Working toward the 1.0.0 milestone by closing the +remaining repo-compliance issues on the tracker. # Next Step -Land feat/reportbuf-storage: finish the in-progress src/main.js change, get make -check green, and merge the branch to main. The branch adds the backend (buffered -zstd-compressed report storage), the CI workflow, and backend repo standard -files, so merging it also closes most compliance gaps. +Confirm the `.gitea/workflows/check.yml` run is green (main always green +policy). The workflow file is already on `main`; what is unverified is that its +latest run passes. # Completed Steps +- 2026-09-29: the container sets up its own data directory (issue #75): + `bin/entrypoint.sh`, still as root, creates `DATA_DIR` if missing and gives it + and `/data` to the `netwatch` user with mode 750 before starting the backend + as that user, so an empty host directory owned by root, or one holding files + from another uid, works with no step on the host. It stops the start instead + when a symbolic link is on the path to `DATA_DIR`, since root would change + whatever the link points to. The `README.md` first-run step that created and + chowned the host directory is gone, and the image no longer sets that + ownership at build time +- 2026-09-29: CI can no longer pass on checks that did not run (issue #37): + `script/cibuild` is now the org model, byte for byte. It runs + `script/bootstrap` and `script/check`, then builds the image with `--no-cache` + and the version from `git describe` as the `VERSION` build argument, where it + used to be a plain `docker build .` whose check steps could come from the + build cache. The workflow puts `~/.local/bin`, where bootstrap links what it + installs, on the step's `PATH`, and bootstrap now installs its pinned node + when the installed one is older than the frontend's dependencies need +- 2026-09-29: `backend/.golangci.yml` re-vendored from `sneak/prompts` (issue + #41): `gomodguard`, deprecated in golangci-lint v2.12.0, is disabled and its + successor `gomodguard_v2` enabled with the org block list, so lint runs print + no deprecation warning. The new file also turns `depguard` on with its + `test-support` rule, which keeps `net/http/httptest` out of non-test code; + netwatch adds no entries of its own to that rule. `backend/script/lint` checks + the new sha256 +- 2026-09-29: nginx sends the security headers `REPO_POLICIES.md` requires on + every response (issue #18), including errors, `/assets/` and what it passes on + from the backend, whose own copies it drops so each header goes out once. They + live in `security-headers.conf`, which `nginx.conf` includes. The content + security policy allows no inline script or style, so the status dot's grey in + `src/main.js` is now a class; `connect-src` is `*` because probed hosts + redirect to others, and the browser checks each redirect against it +- 2026-09-29: the request log is bounded (issue #60): the method, URL, protocol, + `User-Agent`, `Referer`, request ID (which chi takes from the client's + `X-Request-Id` header) and client address it writes are each cut to 128 bytes, + the bound the report handler already used, so one request can no longer put + about 1 MiB per field into a log line. That bound and its helper now live in + the `logger` package, shared by both +- 2026-09-29: nginx takes the client address from `X-Forwarded-For` only on + requests from the reverse proxies named in the container's `TRUSTED_PROXIES` + (issue #64), and by default from none, where it trusted every RFC1918 address + before, so a client could write a new address on each request and escape the + rate limit. `bin/entrypoint.sh` writes one `set_real_ip_from` line per entry + into `/etc/nginx/trusted-proxies.conf`, which `nginx.conf` includes, refusing + an entry that is not an IP address or CIDR, as `netwatch-server check-cidr` + finds; it starts the backend with `TRUSTED_PROXIES=127.0.0.1/32`, since nginx + is its only client +- 2026-09-29: report file names can no longer collide (issue #61): each is + `reports--.jsonl.zst`, where the number goes up by one for + each file the server starts to write, so two flushes in the same millisecond, + such as a flush for size and the final flush at shutdown, each get a file of + their own instead of the second one failing. A failed write uses up its + number, leaving a gap if the file could not be created and otherwise a file + under that number that may be incomplete. +- 2026-09-29: ready to run under upaas (issue #59): the image has a + `HEALTHCHECK` that requests `/.well-known/healthcheck` through nginx on the + port from `PORT`. The backend no longer reads a bad `PORT` as 0 or a bad + `DEBUG` as false: those, and a `BIND_ADDRESS` that is not an IP address, stop + it from starting with an error naming the variable, as the limits, + `CORS_ALLOWED_ORIGINS` and, now by name, `TRUSTED_PROXIES` already did. + `bin/entrypoint.sh` also refuses a `PORT` outside 1 to 65535, and `8081`, + where the backend listens inside the container, naming `PORT`. `README.md` has + a "Running under upaas" section, whose first-run steps create the host + directory for `/data` owned by uid 1000; the image does not change its owner +- 2026-09-29: nginx listens on `PORT` (issue #26), 8080 when unset or empty: the + nginx image renders `nginx.conf` as a template at container start, filling in + `PORT` and no other variable. `bin/entrypoint.sh` refuses to start when `PORT` + is not digits only. `server_tokens off` keeps the nginx version out of + responses. `script/frontend-viewport-test` renders the template the same way. + Gzip and a `50x.html` error page are not added +- 2026-09-29: bounded the report endpoint (issue #20): `POST /api/v1/reports` + still needs no credentials, but each client address, as resolved through + `TRUSTED_PROXIES`, may send `REPORTS_PER_MINUTE` (default 60) reports a + minute, counted by `go-chi/httprate`, and past that gets 429 with + `Retry-After`; the report files in `DATA_DIR`, counted from start with those + already there, may total at most `DATA_DIR_MAX_BYTES` (default 1 GiB), past + which reports get 507; and the wildcard CORS is gone: no CORS headers unless + `CORS_ALLOWED_ORIGINS` lists origins, and an entry that is not a plain + `scheme://host[:port]` origin, `*` included, stops the server from starting. + Deleting report files frees room only at the next start; pruning is issue #54 +- 2026-09-28: one container image (issue #52): the root `Dockerfile` builds the + only image, and `Dockerfile.backend` is gone. nginx serves the frontend on + port 8080 and proxies `/api/` and `/.well-known/healthcheck` to the backend, + which listens on `127.0.0.1:8081` in the same container; the new + `BIND_ADDRESS` setting sets its listen address. `bin/entrypoint.sh` starts + both, passes TERM and INT on to both, and exits non-zero when either exits on + its own. The backend runs as user `netwatch` and stores reports on the `/data` + volume. `script/docker` is the org model again +- 2026-09-28: unified the gate (issue #16): the root `make check` covers the Go + backend as well as the frontend, and the pre-commit hook with it; the backend + moved onto scripts-to-rule-them-all (`backend/script/*`, `backend/Makefile` as + shims, its duplicate hook installer removed); `script/cibuild` builds both + images and is the workflow's only build step. The root `make lint` runs + golangci-lint only in Docker, by building the lint stage of + `Dockerfile.backend` without the cache. `script/bootstrap` installs the pinned + Go unless the installed one is at least what `backend/go.mod` asks for, links + what it installs into `~/.local/bin` without replacing anything it did not + create, and installs no linter. Root `make test` runs both halves within one + 30-second timeout. When `VERSION` is unset or empty, the backend binary's + version falls back to `git describe` inside a git checkout, then to `dev` +- 2026-09-28: frontend reporting client (issue #53): a `Reporter` class posts + collected samples to `/api/v1/reports` every `reportInterval` (default 60s) as + a per-host delta, with the report-building step a pure exported function of + host state; a per-host mark advances only on a delivered POST; at most one + report POST is pending at a time and it is abandoned after half the interval, + so a slow POST never overlaps the next report and a mark never moves + backwards; the samples of an abandoned POST are sent again at the next + interval, so a backend that stored them but answered late receives them twice; + the per-browser client id works in insecure (plain-HTTP) contexts; + `vite.config.js` proxies `/api` to the local backend for `yarn dev` +- 2026-09-28: report ingest correctness (issue #23): a storage failure now + returns 500 instead of a false `ok`; oversize bodies return 413 (distinguished + from malformed JSON, which stays 400); a `MaxBodyBytes` middleware caps every + route, not just the report route; the raw attacker-controlled `geo` blob is no + longer logged (only its length), and `client_id`, `timestamp` and decode error + text are length-bounded before logging; a `decodeJSON` handler helper was + added; panic recovery now routes the stack through slog instead of chi's + plain-text stderr; and writing a report file now returns its error, so a + failed final flush on shutdown makes the process exit non-zero instead of + losing the buffered reports silently +- 2026-09-21: shutdown lifecycle correctness. The process now shuts down through + fx instead of `os.Exit`, so every component's `OnStop` runs and buffered + reports are flushed to disk on `SIGTERM` — previously a full flush window of + telemetry was silently lost on every restart. The `http.Server` is now built + before its serving goroutine starts, so shutdown can no longer race or + nil-deref it; a listen failure exits non-zero via `fx.Shutdowner`; `reportbuf` + `OnStop` is idempotent; and `writeTimeout` now exceeds the chi per-request + budget so that budget is actually reachable. Dead `startupTime`, `exitCode`, + and `cancelFunc` fields were removed +- 2026-09-21: backend HTTP hardening (issue #19): added `ReadHeaderTimeout` and + `IdleTimeout` to the server, a `SecurityHeaders` middleware (HSTS, tight CSP, + frame/sniff/referrer/permissions headers) registered before CORS, and + trusted-proxy client IP resolution honouring `X-Forwarded-For` / `X-Real-IP` + only from a `TRUSTED_PROXIES` allowlist (loopback plus RFC1918 by default) +- 2026-08-10: adopted the org-standard `backend/.golangci.yml` verbatim and + moved the pinned golangci-lint from v2.7.2 to v2.12.2 (the `lint` stage of + `Dockerfile.backend` now pins the `golangci/golangci-lint:v2.12.2` image by + digest); the previous config declared `version: "2"` but used v1 schema keys, + so every threshold in it was inert and its green result was meaningless. + `backend/Makefile`'s `lint` target now asserts the config's sha256 against the + canonical file first, so drift from the org standard fails the build instead + of silently degrading to defaults +- 2026-08-10: every interactive control now meets the 44x44 CSS px minimum tap + target (`.pin-btn`, `#interval-select`, the debug-log label and, on narrow + viewports, `#pause-btn`). The pin button's hit area grows via matching + negative margins, so its layout footprint and row density are unchanged +- 2026-08-10: per-host status line wraps below the 768px breakpoint instead of + forcing horizontal page scroll at 320px +- 2026-08-09: `Dockerfile.backend` reworked to the mandated Go multistage + lint-stage pattern: separate `lint` stage on the hash-pinned + `golangci/golangci-lint` image, `COPY --from=lint` stage dependency, + `CGO_ENABLED=0` static build driven by `ARG VERSION`, and no more `COPY .git` +- 2026-08-09: dotfile compliance — lifted `backend/.editorconfig` to the repo + root so `root = true` covers the frontend too, and replaced `.gitignore` with + the org model (OS, editor, node, and environment/secrets sections) plus this + repo's `dist/` and `*.log`. `.env`, `.env.*`, `*.pem`, and `*.key` are now + ignored repo-wide, not just under `backend/`. Excluding `.git` from + `.dockerignore` stays deferred: both images read git metadata at build time + (`COPY .git` in `Dockerfile.backend`, `git rev-parse` in `vite.config.js`) +- 2026-08-09: automated responsive-layout harness + (`make frontend-viewport-test`): digest-pinned headless Chrome driven over CDP + against the built `dist/`, viewport widths derived from the breakpoints in + `src/styles.css` ([#13](https://git.eeqj.de/sneak/netwatch/issues/13)). Every + check carries a presence guard so none of them can pass against a page it is + not actually measuring. Found two real layout defects, filed as + [#42](https://git.eeqj.de/sneak/netwatch/issues/42) and + [#43](https://git.eeqj.de/sneak/netwatch/issues/43) - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile shims, README Entrypoints section - 2026-02-27: backend with buffered zstd-compressed report storage; CI workflow and backend repo standard files; backend Dockerfile fixed (Go 1.25, - golangci-lint) and moved to repo root (feat/reportbuf-storage, unmerged) + golangci-lint) and moved to repo root (feat/reportbuf-storage) - 2026-02-26: host row layout redesigned with CSS grid; overflow and spacing fixes; nginx config extracted; port hardcoded to 8080 - 2026-02-26: debug log panel, median stats, recovery probe, Docker build fix, @@ -39,6 +206,9 @@ files, so merging it also closes most compliance gaps. # Future Steps +- Wire `script/frontend-viewport-test` into CI as its own step (deliberately not + part of `make check` today; the decision has real CI-runtime cost and is + tracked separately) - Compliance top-up as one small commit: add .editorconfig and add the hooks target to the Makefile - After merge, confirm .gitea/workflows/check.yml is on main and CI is green diff --git a/backend/.golangci.yml b/backend/.golangci.yml index 34a8e31..a7a74c2 100644 --- a/backend/.golangci.yml +++ b/backend/.golangci.yml @@ -1,32 +1,98 @@ version: "2" +# Config schema uses the golangci-lint v2 layout (settings live under +# linters.settings, not top-level linters-settings) so that the +# thresholds below are actually applied by golangci-lint >= v2. + run: timeout: 5m modules-download-mode: readonly linters: default: all + enable: + # Successor to the deprecated gomodguard. Named explicitly, rather than + # left to `default: all`, because it carries the module policy below. + - gomodguard_v2 disable: # Genuinely incompatible with project patterns - exhaustruct # Requires all struct fields - - depguard # Dependency allow/block lists - godot # Requires comments to end with periods - - wsl # Deprecated, replaced by wsl_v5 - wrapcheck # Too verbose for internal packages - varnamelen # Short names like db, id are idiomatic Go - -linters-settings: - lll: - line-length: 88 - funlen: - lines: 80 - statements: 50 - cyclop: - max-complexity: 15 - dupl: - threshold: 100 + # Deprecated: the warning is attached to the old name, so it is + # silenced by disabling that name, not by enabling the successor. + - wsl # Deprecated, replaced by wsl_v5 + - gomodguard # Deprecated, replaced by gomodguard_v2 + settings: + lll: + line-length: 88 + funlen: + lines: 80 + statements: 50 + cyclop: + max-complexity: 15 + dupl: + threshold: 100 + depguard: + # Test-support code must not be compiled into the shipped binary. A + # test-support package exists to hand a test privileges the program + # itself must never have, so a file that is not a test must not import + # one. Test files, and the files inside a package whose directory name + # ends in `test`, are where that code belongs, and are exempt. + # + # The deny list below is the one part of this file a repository is + # expected to extend, and the only part it may. depguard matches an + # import path against a list of prefixes, so it cannot be told "any path + # whose last segment ends in test"; a repository's own test-support + # packages have to be named here one at a time, by full import path, + # under a module path that differs from repository to repository. Add + # them; change nothing else. + rules: + test-support: + list-mode: lax + files: + - "$all" + - "!$test" + - "!**/*test/**" + deny: + - pkg: net/http/httptest + desc: >- + Test-support code belongs in test files and in packages whose + directory name ends in test, not in the shipped binary. + # Only decisions already recorded in the Go package defaults are + # listed here. Every entry matches the module path exactly. + gomodguard_v2: + blocked: + - module: github.com/rs/zerolog + recommendations: + - log/slog + reason: "Structured logging is stdlib log/slog." + # One entry per pre-fork module path, because the later releases + # are separate paths. A prefix match would be shorter but would + # also reach github.com/go-redis/redismock, the test double for + # the successor these entries recommend. + - module: github.com/go-redis/redis + recommendations: + - github.com/redis/go-redis/v9 + reason: "Pre-fork module; use the maintained go-redis v9." + - module: github.com/go-redis/redis/v7 + recommendations: + - github.com/redis/go-redis/v9 + reason: "Pre-fork module; use the maintained go-redis v9." + - module: github.com/go-redis/redis/v8 + recommendations: + - github.com/redis/go-redis/v9 + reason: "Pre-fork module; use the maintained go-redis v9." + - module: github.com/sergi/go-diff + recommendations: + - github.com/aymanbagabas/go-udiff + reason: "No unified diff output; use go-udiff." + - module: github.com/hexops/gotextdiff + recommendations: + - github.com/aymanbagabas/go-udiff + reason: "Unmaintained fork; use go-udiff." issues: - exclude-use-default: false max-issues-per-linter: 0 max-same-issues: 0 diff --git a/backend/Makefile b/backend/Makefile index 38cebcf..faf9ec5 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -1,53 +1,30 @@ -UNAME_S := $(shell uname -s) -VERSION := $(shell git describe --always --dirty) -BUILDARCH := $(shell uname -m) -BINARY := netwatch-server +# Thin shims; the implementations live in backend/script/ (see the +# Entrypoints section of README.md). There is no check, hooks or docker +# target here: the root Makefile's check covers this directory, its +# hooks target installs the repo's only pre-commit hook, and its docker +# target builds the one image, which contains this backend. -GOLDFLAGS += -X main.Version=$(VERSION) -GOLDFLAGS += -X main.Buildarch=$(BUILDARCH) - -ifeq ($(UNAME_S),Darwin) - GOFLAGS := -ldflags "$(GOLDFLAGS)" -else - GOFLAGS = -ldflags "-linkmode external -extldflags -static $(GOLDFLAGS)" -endif - -.PHONY: all build test lint fmt fmt-check check docker hooks run clean +.PHONY: all build test lint fmt fmt-check run clean all: build -build: ./$(BINARY) - -./$(BINARY): $(shell find . -name '*.go' -type f) go.mod go.sum - go build -o $@ $(GOFLAGS) ./cmd/netwatch-server/ +build: + @script/build test: - timeout 30 go test ./... + @script/test lint: - golangci-lint run ./... + @script/lint fmt: - go fmt ./... + @script/fmt fmt-check: - @test -z "$$(gofmt -l .)" || \ - (echo "Files not formatted:"; gofmt -l .; exit 1) + @script/fmt-check -check: test lint fmt-check - -docker: - timeout 300 docker build -t netwatch-server -f ../Dockerfile.backend .. - -hooks: - @printf '#!/bin/sh\ncd backend && make check\n' > \ - $$(git rev-parse --show-toplevel)/.git/hooks/pre-commit - @chmod +x \ - $$(git rev-parse --show-toplevel)/.git/hooks/pre-commit - @echo "Pre-commit hook installed" - -run: build - ./$(BINARY) +run: + @script/run clean: - rm -f ./$(BINARY) + @script/clean diff --git a/backend/README.md b/backend/README.md index a7de988..9914eb0 100644 --- a/backend/README.md +++ b/backend/README.md @@ -4,18 +4,51 @@ SPA and persists them as zstd-compressed JSONL files on disk. ## Getting Started +From this directory: + ```bash # Build and run locally make run +``` -# Run tests, lint, and format check +From the repo root, whose `Dockerfile` builds the one image that ships this +backend behind nginx (see [Container image](#container-image)): + +```bash +# Run tests, lint, and format check over the frontend and this backend make check -# Docker -docker build -t netwatch-server . -docker run -p 8080:8080 netwatch-server +# Build the image: nginx, the frontend and this backend +make docker +docker run -p 8080:8080 netwatch ``` +## Entrypoints + +This directory follows the same +[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all) +pattern as the repo root: the targets in `backend/Makefile` are thin shims over +`backend/script/`. The root `Dockerfile` runs them, and the root scripts call +`test`, `fmt` and `fmt-check`: + +- `script/build` — compile the static `netwatch-server` binary with its version + and architecture stamped in. The version is `VERSION` from the environment; + when that is unset or empty, it falls back to `git describe` inside a git + checkout, then to `dev` +- `script/test` — run the Go tests under a 30-second timeout +- `script/lint` — check `.golangci.yml` against its pinned sha256, then run + golangci-lint. It runs inside the golangci-lint image of the lint stage of + the root `Dockerfile`; from a checkout, run `make lint` at the repo root, + which builds that stage +- `script/fmt` — format the Go sources (writes) +- `script/fmt-check` — check Go formatting (read-only) +- `script/run` — build and run the server locally +- `script/clean` — remove build artifacts + +There is no `check`, `hooks` or `docker` target here: the root `make check` +covers this directory, the root `make hooks` installs the repo's only pre-commit +hook, and the root `make docker` builds the image that contains this backend. + ## Rationale The NetWatch frontend collects latency measurements from the browser but has no @@ -42,17 +75,95 @@ Internal packages in `internal/` follow standard Go project layout: ### Configuration -| Variable | Default | Description | -| ---------- | ------------------ | --------------------------------- | -| `PORT` | `8080` | HTTP listen port | -| `DATA_DIR` | `./data/reports` | Directory for compressed reports | -| `DEBUG` | `false` | Enable debug logging | +| Variable | Default | Description | +| ---------------------- | -------------------- | -------------------------------------------------------------------------------------------------------- | +| `BIND_ADDRESS` | empty | IP address to listen on; empty listens on every interface | +| `PORT` | `8080` | HTTP listen port | +| `DATA_DIR` | `./data/reports` | Directory for compressed reports | +| `DATA_DIR_MAX_BYTES` | `1073741824` (1 GiB) | Largest total size of the report files in `DATA_DIR`; 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 | +| `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) | + +`TRUSTED_PROXIES` defaults to `127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. +The loopback entries cover a reverse proxy on the same host. A request whose +direct peer is outside this set has its forwarded headers ignored, and the +direct peer is logged and rate-limited instead. The container image does not use +this default; see [Container image](#container-image). + +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. + +### Container image + +The root `Dockerfile` builds one image in which nginx listens on the public port +8080, serves the frontend, and proxies `/api/` and `/.well-known/healthcheck` to +this server. The image's entrypoint, `bin/entrypoint.sh`, starts the server as +user `netwatch` (uid 1000) with `BIND_ADDRESS=127.0.0.1` and `PORT=8081`, so +only nginx reaches it, and with `TRUSTED_PROXIES=127.0.0.1/32`, so it takes the +client address nginx passes on and no other. `DATA_DIR` is `/data/reports`, on +the `/data` volume; the entrypoint creates it and gives it and `/data` to +`netwatch` before starting the server. nginx replaces the security headers +this server sets with those in the root `security-headers.conf`, so those are +what clients of the image see. + +The container's own `TRUSTED_PROXIES` goes to nginx instead: IP addresses or +CIDRs, separated by commas, of the reverse proxies in front of the container. +nginx takes the client address from `X-Forwarded-For` only on a request from one +of them. Unset or empty, nginx trusts no proxy, and the client address is the +one each request comes from, so every client behind a proxy shares one rate +limit. An entry that is not an IP address or CIDR, such as a hostname or +`1.2.3`, stops the container at start with an error naming `TRUSTED_PROXIES`: +the entrypoint checks each entry with `netwatch-server check-cidr`, which parses +it as this server parses its own `TRUSTED_PROXIES`. ### Report storage -Reports are written as `reports-.jsonl.zst` files in `DATA_DIR`. -Each file contains one JSON object per line, compressed with zstd. Files are -created with `O_EXCL` to prevent overwrites. +Reports are written as `reports--.jsonl.zst` files in +`DATA_DIR`. The timestamp is in UTC to the millisecond, so the names sort by +time. The number starts at 1 when the server starts and goes up by one for each +file the server starts to write, so two files written in the same millisecond +still get different names. A failed write uses up its number, leaving a gap in +the numbers if the file could not be created and otherwise a file under that +number that may be incomplete. Each file contains one JSON object per line, +compressed with zstd. Files are created with `O_EXCL` to prevent overwrites. + +### 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 +`{"status":"error"}` body as any other error. + +- **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 + count, fading out over the current one, so an address is sure never to be + refused only while it sends at most half of `REPORTS_PER_MINUTE` in any 60 + seconds. The page sends one report a minute from each open tab, so the default + of 60 refuses nothing from up to 30 tabs behind one address, such as a + household or an office sharing it, however their reports bunch up. Report + responses also carry `X-RateLimit-Limit`, `X-RateLimit-Remaining` and + `X-RateLimit-Reset` headers. +- **Size cap.** The report files in `DATA_DIR` may total at most + `DATA_DIR_MAX_BYTES`, counting the files already there at start. Reports + waiting in memory count at their uncompressed size until they are written, so + a report that would take the total past the cap is refused with 507, and + nothing of it is stored. Deleting report files frees room only at the next + start, when the files are counted again. The default of 1 GiB is small enough + for any host; set it to the space you can give `DATA_DIR`. + +### CORS + +The page calls the API from the origin it is served from, so by default the +server sends no CORS headers, and browsers let no other origin's pages call it. +To serve the page from elsewhere, list that origin in `CORS_ALLOWED_ORIGINS` +(for example `https://netwatch.example.com`); pages from a listed origin may +`GET` and `POST` with a `Content-Type` header. Each entry must be a plain +origin, `scheme://host` with an optional `:port`, as browsers send it: no path, +not even a trailing `/`, and no `*`. Any other entry stops the server from +starting, with an error naming `CORS_ALLOWED_ORIGINS`. ## TODO diff --git a/backend/cmd/netwatch-server/main.go b/backend/cmd/netwatch-server/main.go index 3f3d86b..01c77a4 100644 --- a/backend/cmd/netwatch-server/main.go +++ b/backend/cmd/netwatch-server/main.go @@ -2,6 +2,9 @@ package main import ( + "fmt" + "os" + "sneak.berlin/go/netwatch/internal/config" "sneak.berlin/go/netwatch/internal/globals" "sneak.berlin/go/netwatch/internal/handlers" @@ -22,6 +25,19 @@ var ( ) func main() { + // "netwatch-server check-cidr CIDR" exits 1, with the error, if + // this server would refuse CIDR in its TRUSTED_PROXIES. + // bin/entrypoint.sh runs it on each entry it gives nginx. + if len(os.Args) == 3 && os.Args[1] == "check-cidr" { + _, err := middleware.ParseTrustedProxies(os.Args[2:]) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + + return + } + globals.Appname = Appname globals.Version = Version globals.Buildarch = Buildarch diff --git a/backend/go.mod b/backend/go.mod index 89e3ef4..a2462e8 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -5,6 +5,7 @@ go 1.25.5 require ( github.com/go-chi/chi/v5 v5.2.5 github.com/go-chi/cors v1.2.2 + github.com/go-chi/httprate v0.16.0 github.com/joho/godotenv v1.5.1 github.com/klauspost/compress v1.18.4 github.com/spf13/viper v1.21.0 @@ -14,6 +15,7 @@ require ( require ( github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect + github.com/klauspost/cpuid/v2 v2.2.10 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/sagikazarmark/locafero v0.11.0 // indirect github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect @@ -21,10 +23,11 @@ require ( github.com/spf13/cast v1.10.0 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect + github.com/zeebo/xxh3 v1.0.2 // indirect go.uber.org/dig v1.19.0 // indirect go.uber.org/multierr v1.10.0 // indirect go.uber.org/zap v1.26.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/sys v0.29.0 // indirect + golang.org/x/sys v0.30.0 // indirect golang.org/x/text v0.28.0 // indirect ) diff --git a/backend/go.sum b/backend/go.sum index de6ddf2..cac5e22 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -8,6 +8,8 @@ github.com/go-chi/chi/v5 v5.2.5 h1:Eg4myHZBjyvJmAFjFvWgrqDTXFyOzjj7YIm3L3mu6Ug= github.com/go-chi/chi/v5 v5.2.5/go.mod h1:X7Gx4mteadT3eDOMTsXzmI4/rwUpOwBHLpAfupzFJP0= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= +github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8= +github.com/go-chi/httprate v0.16.0/go.mod h1:A8lo+qRhk+s9LiuP5saS7XCGDXRXMcrueq0NfIuCa/I= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= @@ -16,6 +18,8 @@ github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE9a2c= github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= +github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE= +github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -42,6 +46,10 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= +github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ= +github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= +github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0= +github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA= go.uber.org/dig v1.19.0 h1:BACLhebsYdpQ7IROQ1AGPjrXcP5dF80U3gKoFzbaq/4= go.uber.org/dig v1.19.0/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= go.uber.org/fx v1.24.0 h1:wE8mruvpg2kiiL1Vqd0CC+tr0/24XIB10Iwp2lLWzkg= @@ -54,8 +62,8 @@ go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU= -golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 6966d57..a258978 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -4,7 +4,13 @@ package config import ( "errors" + "fmt" "log/slog" + "math" + "net/netip" + "net/url" + "strconv" + "strings" "sneak.berlin/go/netwatch/internal/globals" "sneak.berlin/go/netwatch/internal/logger" @@ -14,6 +20,32 @@ import ( "go.uber.org/fx" ) +// defaultTrustedProxies lists the networks whose forwarded +// headers are honoured by default: IPv4 and IPv6 loopback, +// for a reverse proxy on the same host, and the RFC1918 +// 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," + + "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" + +// Default limits on stored reports; backend/README.md gives the +// reasons for these values. +const ( + defaultReportsPerMinute = 60 + defaultDataDirMaxBytes = 1 << 30 // 1 GiB +) + +var ( + errNotPositive = errors.New("must be a positive whole number") + errNotOrigin = errors.New( + "must be an origin, scheme://host with an optional port", + ) + errNotPort = errors.New("must be a port number, 1 to 65535") + errNotBool = errors.New("must be true or false") + errNotIP = errors.New("must be an IP address, or empty") +) + // Params defines the dependencies for Config. type Params struct { fx.In @@ -24,18 +56,24 @@ type Params struct { // Config holds the resolved application configuration. type Config struct { - DataDir string - Debug bool - MetricsPassword string - MetricsUsername string - Port int - SentryDSN string - log *slog.Logger - params *Params + BindAddress string + CORSAllowedOrigins []string + DataDir string + DataDirMaxBytes int64 + Debug bool + MetricsPassword string + MetricsUsername string + Port int + ReportsPerMinute int + SentryDSN string + TrustedProxies []string + log *slog.Logger + params *Params } // New loads configuration from env, .env files, and config -// files, returning a fully resolved Config. +// files, returning a fully resolved Config. It fails, with an error +// naming the setting, on a value the server cannot use. func New( _ fx.Lifecycle, params Params, @@ -50,12 +88,19 @@ func New( viper.AutomaticEnv() + // An empty CORS_ALLOWED_ORIGINS allows no other origin. + viper.SetDefault("CORS_ALLOWED_ORIGINS", "") viper.SetDefault("DATA_DIR", "./data/reports") + viper.SetDefault("DATA_DIR_MAX_BYTES", defaultDataDirMaxBytes) viper.SetDefault("DEBUG", "false") + // An empty BIND_ADDRESS listens on every interface. + viper.SetDefault("BIND_ADDRESS", "") viper.SetDefault("PORT", "8080") + viper.SetDefault("REPORTS_PER_MINUTE", defaultReportsPerMinute) viper.SetDefault("SENTRY_DSN", "") viper.SetDefault("METRICS_USERNAME", "") viper.SetDefault("METRICS_PASSWORD", "") + viper.SetDefault("TRUSTED_PROXIES", defaultTrustedProxies) err := viper.ReadInConfig() if err != nil { @@ -66,15 +111,39 @@ func New( } } + // Read with strconv: viper's GetInt and GetBool would read a value + // they cannot parse as 0 or false instead of failing. + port, err := strconv.Atoi(viper.GetString("PORT")) + if err != nil || port < 1 || port > math.MaxUint16 { + return nil, fmt.Errorf("PORT %q: %w", + viper.GetString("PORT"), errNotPort) + } + + debug, err := strconv.ParseBool(viper.GetString("DEBUG")) + if err != nil { + return nil, fmt.Errorf("DEBUG %q: %w", + viper.GetString("DEBUG"), errNotBool) + } + s := &Config{ - DataDir: viper.GetString("DATA_DIR"), - Debug: viper.GetBool("DEBUG"), - MetricsPassword: viper.GetString("METRICS_PASSWORD"), - MetricsUsername: viper.GetString("METRICS_USERNAME"), - Port: viper.GetInt("PORT"), - SentryDSN: viper.GetString("SENTRY_DSN"), - log: log, - params: ¶ms, + BindAddress: viper.GetString("BIND_ADDRESS"), + CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")), + DataDir: viper.GetString("DATA_DIR"), + DataDirMaxBytes: viper.GetInt64("DATA_DIR_MAX_BYTES"), + Debug: debug, + MetricsPassword: viper.GetString("METRICS_PASSWORD"), + MetricsUsername: viper.GetString("METRICS_USERNAME"), + Port: port, + ReportsPerMinute: viper.GetInt("REPORTS_PER_MINUTE"), + SentryDSN: viper.GetString("SENTRY_DSN"), + TrustedProxies: splitList(viper.GetString("TRUSTED_PROXIES")), + log: log, + params: ¶ms, + } + + err = s.check() + if err != nil { + return nil, err } if s.Debug { @@ -84,3 +153,64 @@ func New( return s, nil } + +// check fails with an error naming the first setting here whose value +// the server cannot use. New checks PORT and DEBUG as it reads them, +// and the middleware checks TRUSTED_PROXIES as it parses it. +func (s *Config) check() error { + // viper reads a value that is not a number as 0, so this also + // catches a mistyped setting. + if s.ReportsPerMinute <= 0 { + return fmt.Errorf("REPORTS_PER_MINUTE %q: %w", + viper.GetString("REPORTS_PER_MINUTE"), errNotPositive) + } + + if s.DataDirMaxBytes <= 0 { + return fmt.Errorf("DATA_DIR_MAX_BYTES %q: %w", + viper.GetString("DATA_DIR_MAX_BYTES"), errNotPositive) + } + + if s.BindAddress != "" { + _, err := netip.ParseAddr(s.BindAddress) + if err != nil { + return fmt.Errorf("BIND_ADDRESS %q: %w", s.BindAddress, errNotIP) + } + } + + return checkOrigins(s.CORSAllowedOrigins) +} + +// checkOrigins fails on the first CORS_ALLOWED_ORIGINS entry that is +// not a plain origin, scheme://host with an optional port, as browsers +// send it; anything more, such as a trailing "/", would match no page. +// go-chi/cors reads a "*" anywhere in an entry as a wildcard, so no +// entry may contain one. +func checkOrigins(origins []string) error { + for _, origin := range origins { + u, err := url.Parse(origin) + if err != nil || u.Scheme == "" || u.Host == "" || + strings.Contains(origin, "*") || + origin != u.Scheme+"://"+u.Host { + return fmt.Errorf("CORS_ALLOWED_ORIGINS %q: %w", + origin, errNotOrigin) + } + } + + return nil +} + +// splitList turns a comma-separated setting into a trimmed +// slice, dropping empty entries. +func splitList(raw string) []string { + parts := strings.Split(raw, ",") + + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + out = append(out, p) + } + } + + return out +} diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go new file mode 100644 index 0000000..a75e31e --- /dev/null +++ b/backend/internal/config/config_test.go @@ -0,0 +1,121 @@ +package config_test + +import ( + "strings" + "testing" + + "sneak.berlin/go/netwatch/internal/config" + "sneak.berlin/go/netwatch/internal/globals" + "sneak.berlin/go/netwatch/internal/logger" + + "go.uber.org/fx" +) + +// requireConfigError builds the config as main does and fails the +// test unless that fails with an error naming setting. It uses +// fx.New, because fxtest.New fails the test itself on an error. +func requireConfigError(t *testing.T, setting string) { + t.Helper() + + app := fx.New( + fx.NopLogger, + fx.Provide(globals.New, logger.New, config.New), + fx.Invoke(func(*config.Config) {}), + ) + + err := app.Err() + if err == nil || !strings.Contains(err.Error(), setting) { + t.Fatalf("config error = %v, want one naming %s", err, setting) + } +} + +// TestSettingsLoadAsGiven: valid values pass the checks and are used +// as given. bin/entrypoint.sh starts the server with these +// BIND_ADDRESS and PORT values. +func TestSettingsLoadAsGiven(t *testing.T) { + t.Setenv("BIND_ADDRESS", "127.0.0.1") + t.Setenv("PORT", "8081") + t.Setenv("DEBUG", "true") + + 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 cfg.BindAddress != "127.0.0.1" || cfg.Port != 8081 || !cfg.Debug { + t.Fatalf("BindAddress, Port, Debug = %q, %d, %t; "+ + "want \"127.0.0.1\", 8081, true", + cfg.BindAddress, cfg.Port, cfg.Debug) + } +} + +// TestPortMustBeAPortNumber: viper reads a value that is not a number +// as 0, on which the server would listen on a random port. +func TestPortMustBeAPortNumber(t *testing.T) { + for _, value := range []string{"abc", "0", "65536", "8080.5"} { + t.Run(value, func(t *testing.T) { + t.Setenv("PORT", value) + + requireConfigError(t, "PORT") + }) + } +} + +// TestDebugMustBeTrueOrFalse: viper reads any other value, such as +// "yes", as false. +func TestDebugMustBeTrueOrFalse(t *testing.T) { + t.Setenv("DEBUG", "yes") + + requireConfigError(t, "DEBUG") +} + +// TestBindAddressMustBeAnIPAddress: a host name would be looked up +// only once the server starts listening, and a mistyped one would stop +// it then with an error that does not name the setting. +func TestBindAddressMustBeAnIPAddress(t *testing.T) { + t.Setenv("BIND_ADDRESS", "localhost") + + requireConfigError(t, "BIND_ADDRESS") +} + +// TestReportsPerMinuteMustBePositive: unchecked, zero would panic +// when the routes are built, and a negative rate would lift the +// limit. +func TestReportsPerMinuteMustBePositive(t *testing.T) { + t.Setenv("REPORTS_PER_MINUTE", "0") + + requireConfigError(t, "REPORTS_PER_MINUTE") +} + +// TestDataDirMaxBytesMustBeANumber: viper reads a value that is not +// a number, such as "1GB", as 0, which would refuse every report. +func TestDataDirMaxBytesMustBeANumber(t *testing.T) { + t.Setenv("DATA_DIR_MAX_BYTES", "1GB") + + requireConfigError(t, "DATA_DIR_MAX_BYTES") +} + +// TestCORSAllowedOriginsMustBeOrigins: "*" would let every origin in, +// and an entry that is not a plain origin would match no page. +func TestCORSAllowedOriginsMustBeOrigins(t *testing.T) { + for _, entry := range []string{ + "*", + "https://*.netwatch.example", + "netwatch.example", + "https://netwatch.example/", + } { + t.Run(entry, func(t *testing.T) { + t.Setenv("CORS_ALLOWED_ORIGINS", entry) + + requireConfigError(t, "CORS_ALLOWED_ORIGINS") + }) + } +} diff --git a/backend/internal/handlers/export_test.go b/backend/internal/handlers/export_test.go new file mode 100644 index 0000000..220da99 --- /dev/null +++ b/backend/internal/handlers/export_test.go @@ -0,0 +1,10 @@ +package handlers + +import "log/slog" + +// NewForTest builds a Handlers around a report sink and logger, +// bypassing the fx graph so handler behaviour (including the +// storage failure path) is exercisable in unit tests. +func NewForTest(buf reportAppender, log *slog.Logger) *Handlers { + return &Handlers{buf: buf, log: log} +} diff --git a/backend/internal/handlers/handlers.go b/backend/internal/handlers/handlers.go index a327fcd..db51059 100644 --- a/backend/internal/handlers/handlers.go +++ b/backend/internal/handlers/handlers.go @@ -18,6 +18,13 @@ import ( const jsonContentType = "application/json; charset=utf-8" +// 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. +type reportAppender interface { + Append(v any) error +} + // Params defines the dependencies for Handlers. type Params struct { fx.In @@ -30,7 +37,7 @@ type Params struct { // Handlers provides HTTP handler factories for all endpoints. type Handlers struct { - buf *reportbuf.Buffer + buf reportAppender hc *healthcheck.Healthcheck log *slog.Logger params *Params @@ -72,3 +79,15 @@ func (s *Handlers) respondJSON( } } } + +// decodeJSON decodes the request body into v. The 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 { + return json.NewDecoder(r.Body).Decode(v) +} diff --git a/backend/internal/handlers/report.go b/backend/internal/handlers/report.go index d4b54ed..e7dd4e4 100644 --- a/backend/internal/handlers/report.go +++ b/backend/internal/handlers/report.go @@ -2,10 +2,12 @@ package handlers import ( "encoding/json" + "errors" "net/http" -) -const maxReportBodyBytes = 1 << 20 // 1 MiB + "sneak.berlin/go/netwatch/internal/logger" + "sneak.berlin/go/netwatch/internal/reportbuf" +) type reportSample struct { T int64 `json:"t"` @@ -35,48 +37,84 @@ func (s *Handlers) HandleReport() http.HandlerFunc { } return func(w http.ResponseWriter, r *http.Request) { - r.Body = http.MaxBytesReader( - w, r.Body, maxReportBodyBytes, - ) - var rpt report - err := json.NewDecoder(r.Body).Decode(&rpt) + err := s.decodeJSON(w, r, &rpt) if err != nil { - s.log.Error("failed to decode report", - "error", err, - ) s.respondJSON(w, r, &response{Status: "error"}, - http.StatusBadRequest, + s.decodeErrorStatus(err), ) return } - totalSamples := 0 - for _, h := range rpt.Hosts { - totalSamples += len(h.History) - } + s.logReportReceived(rpt) - s.log.Info("report received", - "client_id", rpt.ClientID, - "timestamp", rpt.Timestamp, - "host_count", len(rpt.Hosts), - "total_samples", totalSamples, - "geo", string(rpt.Geo), - ) - - bufErr := s.buf.Append(rpt) - if bufErr != nil { - s.log.Error("failed to buffer report", - "error", bufErr, + err = s.buf.Append(rpt) + if err != nil { + s.respondJSON(w, r, + &response{Status: "error"}, + s.appendErrorStatus(err), ) + + return } - s.respondJSON(w, r, - &response{Status: "ok"}, - http.StatusOK, - ) + s.respondJSON(w, r, &response{Status: "ok"}, http.StatusOK) } } + +// decodeErrorStatus logs a report decode failure and returns the +// status to send: 413 when the body exceeded the size limit, +// otherwise 400 for malformed JSON. +func (s *Handlers) decodeErrorStatus(err error) int { + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + s.log.Warn("report body too large", "limit_bytes", tooLarge.Limit) + + return http.StatusRequestEntityTooLarge + } + + // The decoder's error text can quote request bytes (a whole + // oversized number, for example), so it is bounded too. + s.log.Error("failed to decode report", + "error", logger.BoundedForLog(err.Error()), + ) + + return http.StatusBadRequest +} + +// appendErrorStatus logs a failure to store a report and returns +// the status to send: 507 when the report files are at their size +// cap, otherwise 500. +func (s *Handlers) appendErrorStatus(err error) int { + if errors.Is(err, reportbuf.ErrFull) { + s.log.Warn("report refused: report files at their size cap") + + return http.StatusInsufficientStorage + } + + s.log.Error("failed to buffer report", "error", err) + + return http.StatusInternalServerError +} + +// logReportReceived logs an accepted report. Untrusted fields are +// bounded (client_id, timestamp) or reduced to a length +// (geo_bytes) so the raw attacker-controlled body never reaches +// the log. +func (s *Handlers) logReportReceived(rpt report) { + totalSamples := 0 + for _, h := range rpt.Hosts { + totalSamples += len(h.History) + } + + s.log.Info("report received", + "client_id", logger.BoundedForLog(rpt.ClientID), + "timestamp", logger.BoundedForLog(rpt.Timestamp), + "host_count", len(rpt.Hosts), + "total_samples", totalSamples, + "geo_bytes", len(rpt.Geo), + ) +} diff --git a/backend/internal/handlers/report_test.go b/backend/internal/handlers/report_test.go new file mode 100644 index 0000000..cd94b5c --- /dev/null +++ b/backend/internal/handlers/report_test.go @@ -0,0 +1,240 @@ +package handlers_test + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "sneak.berlin/go/netwatch/internal/handlers" + "sneak.berlin/go/netwatch/internal/logger" + "sneak.berlin/go/netwatch/internal/middleware" + "sneak.berlin/go/netwatch/internal/reportbuf" +) + +var errStorageFailed = errors.New("storage failed") + +// stubAppender drives the storage success/failure path without a +// real buffer or disk. +type stubAppender struct { + err error +} + +func (s stubAppender) Append(any) error { return s.err } + +func newTestHandlers(buf stubAppender, out io.Writer) *handlers.Handlers { + return handlers.NewForTest(buf, slog.New(slog.NewJSONHandler(out, nil))) +} + +func decodeStatus(t *testing.T, body []byte) string { + t.Helper() + + var resp struct { + Status string `json:"status"` + } + + err := json.Unmarshal(body, &resp) + if err != nil { + t.Fatalf("response body not JSON: %v (%q)", err, body) + } + + return resp.Status +} + +func TestHandleReportStorageFailureIsNon2xx(t *testing.T) { + t.Parallel() + + h := newTestHandlers(stubAppender{err: errStorageFailed}, io.Discard) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"clientId":"c1","hosts":[]}`), + ) + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code < 500 { + t.Fatalf("storage failure status = %d, want a 5xx", rec.Code) + } + + if got := decodeStatus(t, rec.Body.Bytes()); got != "error" { + t.Fatalf("status field = %q, want %q", got, "error") + } +} + +// TestHandleReportFullIs507 checks the answer when the report files +// are at their size cap: 507 and the usual error body, which tells +// the client nothing more. +func TestHandleReportFullIs507(t *testing.T) { + t.Parallel() + + h := newTestHandlers(stubAppender{err: reportbuf.ErrFull}, io.Discard) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"clientId":"c1","hosts":[]}`), + ) + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code != http.StatusInsufficientStorage { + t.Fatalf("status = %d, want %d", + rec.Code, http.StatusInsufficientStorage) + } + + if got := rec.Body.String(); got != "{\"status\":\"error\"}\n" { + t.Errorf("body = %q, want %q", got, "{\"status\":\"error\"}\n") + } +} + +func TestHandleReportMalformedJSONIs400(t *testing.T) { + t.Parallel() + + h := newTestHandlers(stubAppender{}, io.Discard) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{not json`), + ) + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("malformed status = %d, want %d", + rec.Code, http.StatusBadRequest) + } +} + +func TestHandleReportOversizeIs413(t *testing.T) { + t.Parallel() + + const limit = 32 + + h := newTestHandlers(stubAppender{}, io.Discard) + handler := (&middleware.Middleware{}).MaxBodyBytes(limit)( + h.HandleReport(), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`), + ) + // No declared length, so only the middleware's read cap can + // stop this body. + req.ContentLength = -1 + + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("oversize status = %d, want %d", + rec.Code, http.StatusRequestEntityTooLarge) + } +} + +func TestHandleReportDoesNotLogRawGeo(t *testing.T) { + t.Parallel() + + const sentinel = "SENSITIVE-GEO-BLOB" + + var logbuf bytes.Buffer + + h := newTestHandlers(stubAppender{}, &logbuf) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader( + `{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`, + ), + ) + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + if strings.Contains(logbuf.String(), sentinel) { + t.Fatal("raw geo bytes were written to the log") + } + + if !strings.Contains(logbuf.String(), "geo_bytes") { + t.Fatal("expected a bounded geo_bytes field in the log") + } +} + +func TestHandleReportLogsClientIDCutToBound(t *testing.T) { + t.Parallel() + + long := strings.Repeat("c", 2*logger.MaxLoggedFieldBytes) + + var logbuf bytes.Buffer + + h := newTestHandlers(stubAppender{}, &logbuf) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader( + `{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`, + ), + ) + + h.HandleReport().ServeHTTP(rec, req) + + var logged map[string]any + + err := json.Unmarshal(logbuf.Bytes(), &logged) + if err != nil { + t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String()) + } + + want := long[:logger.MaxLoggedFieldBytes] + + if logged["client_id"] != want { + t.Fatalf("logged client_id not cut to %d bytes: %q", + logger.MaxLoggedFieldBytes, logged["client_id"]) + } + + if logged["timestamp"] != want { + t.Fatalf("logged timestamp not cut to %d bytes: %q", + logger.MaxLoggedFieldBytes, logged["timestamp"]) + } +} + +func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) { + t.Parallel() + + // A number too large for its int64 field makes the decoder's + // error text quote the whole number. + huge := strings.Repeat("9", 2*logger.MaxLoggedFieldBytes) + + var logbuf bytes.Buffer + + h := newTestHandlers(stubAppender{}, &logbuf) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"hosts":[{"history":[{"t":`+huge+`}]}]}`), + ) + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) + } + + if strings.Contains(logbuf.String(), huge) { + t.Fatal("the whole oversized number was written to the log") + } +} diff --git a/backend/internal/logger/logger.go b/backend/internal/logger/logger.go index 9f30c1a..de6c884 100644 --- a/backend/internal/logger/logger.go +++ b/backend/internal/logger/logger.go @@ -11,6 +11,21 @@ import ( "go.uber.org/fx" ) +// MaxLoggedFieldBytes bounds untrusted text (request fields, +// header values, decode error text) before it is logged, so a +// caller cannot inflate log volume with an oversized value. +const MaxLoggedFieldBytes = 128 + +// BoundedForLog truncates an untrusted string to a fixed byte +// bound so an attacker-controlled field cannot dominate the log. +func BoundedForLog(s string) string { + if len(s) > MaxLoggedFieldBytes { + return s[:MaxLoggedFieldBytes] + } + + return s +} + // Params defines the dependencies for Logger. type Params struct { fx.In diff --git a/backend/internal/middleware/export_test.go b/backend/internal/middleware/export_test.go new file mode 100644 index 0000000..ed47271 --- /dev/null +++ b/backend/internal/middleware/export_test.go @@ -0,0 +1,31 @@ +package middleware + +import ( + "log/slog" + "net/http" + "net/netip" +) + +// Test-only wrappers exposing unexported helpers to the +// external middleware_test package. + +// NewWithLogger builds a Middleware around a logger for tests +// that exercise the logging paths without the fx graph. +func NewWithLogger(log *slog.Logger) *Middleware { + return &Middleware{log: log} +} + +// NewWithTrustedProxies builds a Middleware that honours forwarded +// headers from the given networks, for tests of the client address +// paths without the fx graph. +func NewWithTrustedProxies(trusted []netip.Prefix) *Middleware { + return &Middleware{trustedProxies: trusted} +} + +func ClientIP( + remoteAddr string, + header http.Header, + trusted []netip.Prefix, +) string { + return clientIP(remoteAddr, header, trusted) +} diff --git a/backend/internal/middleware/middleware.go b/backend/internal/middleware/middleware.go index 01cbe8d..aff7864 100644 --- a/backend/internal/middleware/middleware.go +++ b/backend/internal/middleware/middleware.go @@ -3,9 +3,15 @@ package middleware import ( + "errors" + "fmt" + "io" "log/slog" "net" "net/http" + "net/netip" + "runtime/debug" + "strings" "time" "sneak.berlin/go/netwatch/internal/config" @@ -14,11 +20,29 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/cors" + "github.com/go-chi/httprate" "go.uber.org/fx" ) const corsMaxAgeSec = 300 +// jsonErrorBody is the body written for errors raised inside +// middleware, matching the {"status":"error"} shape the handlers +// return so clients see one error contract across the API. +const ( + jsonContentType = "application/json; charset=utf-8" + jsonErrorBody = "{\"status\":\"error\"}\n" +) + +// Security header values. The backend is a JSON API with no +// HTML surface, so the CSP forbids every resource type and +// framing outright. +const ( + hstsValue = "max-age=31536000; includeSubDomains" + cspValue = "default-src 'none'; frame-ancestors 'none'" + permissionsPolicyValue = "camera=(), microphone=(), geolocation=()" +) + // Params defines the dependencies for Middleware. type Params struct { fx.In @@ -30,8 +54,9 @@ type Params struct { // Middleware holds shared state for middleware factories. type Middleware struct { - log *slog.Logger - params *Params + log *slog.Logger + params *Params + trustedProxies []netip.Prefix } // New creates a Middleware instance. @@ -39,13 +64,40 @@ func New( _ fx.Lifecycle, params Params, ) (*Middleware, error) { + trusted, err := ParseTrustedProxies(params.Config.TrustedProxies) + if err != nil { + return nil, err + } + s := new(Middleware) s.params = ¶ms s.log = params.Logger.Get() + s.trustedProxies = trusted return s, nil } +// ParseTrustedProxies converts the TRUSTED_PROXIES entries into +// prefixes, failing fast on any malformed entry. Each entry must be +// a CIDR; a lone address is refused. "netwatch-server check-cidr" +// runs it too. +func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) { + prefixes := make([]netip.Prefix, 0, len(cidrs)) + + for _, cidr := range cidrs { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + return nil, fmt.Errorf( + "TRUSTED_PROXIES %q: %w", cidr, err, + ) + } + + prefixes = append(prefixes, prefix.Masked()) + } + + return prefixes, nil +} + type loggingResponseWriter struct { http.ResponseWriter @@ -72,8 +124,75 @@ func ipFromHostPort(hostPort string) string { return host } +// clientIP resolves the caller's address. X-Forwarded-For and +// X-Real-IP are honoured only when the direct peer is a +// trusted proxy; otherwise the direct peer is returned so a +// spoofed header cannot forge the logged address. +func clientIP( + remoteAddr string, + header http.Header, + trusted []netip.Prefix, +) string { + peer := ipFromHostPort(remoteAddr) + + if !addrInAny(peer, trusted) { + return peer + } + + if xff := firstForwardedFor(header.Get("X-Forwarded-For")); xff != "" { + return xff + } + + if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) { + return xr + } + + return peer +} + +// firstForwardedFor returns the left-most valid address in an +// X-Forwarded-For list (the original client), or "" if none. +func firstForwardedFor(value string) string { + for part := range strings.SplitSeq(value, ",") { + candidate := strings.TrimSpace(part) + if validIP(candidate) { + return candidate + } + } + + return "" +} + +func validIP(s string) bool { + _, err := netip.ParseAddr(s) + + return err == nil +} + +// addrInAny reports whether s parses as an address contained +// in any of the trusted prefixes. +func addrInAny(s string, trusted []netip.Prefix) bool { + addr, err := netip.ParseAddr(s) + if err != nil { + return false + } + + addr = addr.Unmap() + + for _, prefix := range trusted { + if prefix.Contains(addr) { + return true + } + } + + return false +} + // Logging returns middleware that logs each request with -// timing, status code, and client information. +// timing, status code, and client information. Every string +// taken from the request is cut to logger.MaxLoggedFieldBytes, +// including the request ID, which chi takes from the client's +// X-Request-Id header when one is sent. func (s *Middleware) Logging() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc( @@ -86,17 +205,19 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler { latency := time.Since(start) s.log.InfoContext(ctx, "request", "request_start", start, - "method", r.Method, - "url", r.URL.String(), - "useragent", r.UserAgent(), + "method", logger.BoundedForLog(r.Method), + "url", logger.BoundedForLog(r.URL.String()), + "useragent", logger.BoundedForLog(r.UserAgent()), "request_id", - ctx.Value( - middleware.RequestIDKey, - ), - "referer", r.Referer(), - "proto", r.Proto, + logger.BoundedForLog(middleware.GetReqID(ctx)), + "referer", logger.BoundedForLog(r.Referer()), + "proto", logger.BoundedForLog(r.Proto), "remote_ip", - ipFromHostPort(r.RemoteAddr), + logger.BoundedForLog(clientIP( + r.RemoteAddr, + r.Header, + s.trustedProxies, + )), "status", lrw.statusCode, "latency_ms", latency.Milliseconds(), @@ -109,21 +230,137 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler { } } -// CORS returns middleware that adds permissive CORS headers. -func (s *Middleware) CORS() func(http.Handler) http.Handler { +// SecurityHeaders returns middleware that sets response +// security headers. It runs before CORS so the headers are +// present on preflight responses the CORS handler writes. +func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + h := w.Header() + h.Set("Strict-Transport-Security", hstsValue) + h.Set("Content-Security-Policy", cspValue) + h.Set("X-Frame-Options", "DENY") + h.Set("X-Content-Type-Options", "nosniff") + h.Set("Referrer-Policy", "no-referrer") + h.Set("Permissions-Policy", permissionsPolicyValue) + + next.ServeHTTP(w, r) + }, + ) + } +} + +// writeJSONError writes the shared JSON error body with the +// given status. Used where middleware must reject a request +// before it reaches a handler. +func writeJSONError(w http.ResponseWriter, status int) { + w.Header().Set("Content-Type", jsonContentType) + w.WriteHeader(status) + _, _ = io.WriteString(w, jsonErrorBody) +} + +// MaxBodyBytes returns middleware that caps the request body at +// limit bytes. A declared Content-Length over the limit is +// rejected immediately with 413. Bodies without a declared +// length (or that understate it) are capped as they are read, so +// a handler that reads the body sees a *http.MaxBytesError it can +// map to 413. Mounted again on a route group, it can only lower +// the limit: a cap applied earlier in the chain still holds. +func (s *Middleware) MaxBodyBytes( + limit int64, +) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + if r.ContentLength > limit { + writeJSONError( + w, + http.StatusRequestEntityTooLarge, + ) + + return + } + + r.Body = http.MaxBytesReader(w, r.Body, limit) + + next.ServeHTTP(w, r) + }, + ) + } +} + +// 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 +// 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) { + defer func() { + rec := recover() + if rec == nil { + return + } + + err, ok := rec.(error) + if ok && errors.Is(err, http.ErrAbortHandler) { + panic(rec) + } + + s.log.ErrorContext(r.Context(), + "panic recovered", + "panic", fmt.Sprintf("%v", rec), + "stack", string(debug.Stack()), + ) + + w.WriteHeader(http.StatusInternalServerError) + }() + + next.ServeHTTP(w, r) + }, + ) + } +} + +// CORS returns middleware that lets pages served from the given +// origins call the API. With no origins it adds no CORS headers at +// all, so only same-origin pages can use the API. That case must not +// reach cors.Handler, which treats an empty origin list as "allow +// every origin". +func (s *Middleware) CORS( + origins []string, +) func(http.Handler) http.Handler { + if len(origins) == 0 { + return func(next http.Handler) http.Handler { return next } + } + return cors.Handler(cors.Options{ - AllowedOrigins: []string{"*"}, - AllowedMethods: []string{ - "GET", "POST", "PUT", "DELETE", "OPTIONS", - }, - AllowedHeaders: []string{ - "Accept", - "Authorization", - "Content-Type", - "X-CSRF-Token", - }, - ExposedHeaders: []string{"Link"}, + AllowedOrigins: origins, + AllowedMethods: []string{http.MethodGet, http.MethodPost}, + AllowedHeaders: []string{"Content-Type"}, AllowCredentials: false, MaxAge: corsMaxAgeSec, }) } + +// RateLimit returns middleware that allows each client address +// perMinute requests a minute and answers the rest with 429, the +// Retry-After header httprate sets, and the usual error body. The +// address is the one clientIP resolves, so clients behind the reverse +// proxy are limited one by one, not together as the proxy. +func (s *Middleware) RateLimit( + perMinute int, +) func(http.Handler) http.Handler { + return httprate.LimitBy(perMinute, time.Minute, + func(r *http.Request) (string, error) { + return clientIP(r.RemoteAddr, r.Header, s.trustedProxies), nil + }, + httprate.WithLimitHandler( + func(w http.ResponseWriter, _ *http.Request) { + writeJSONError(w, http.StatusTooManyRequests) + }, + ), + ) +} diff --git a/backend/internal/middleware/middleware_test.go b/backend/internal/middleware/middleware_test.go new file mode 100644 index 0000000..c916cf2 --- /dev/null +++ b/backend/internal/middleware/middleware_test.go @@ -0,0 +1,571 @@ +package middleware_test + +import ( + "bytes" + "encoding/json" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "net/netip" + "strings" + "testing" + "testing/synctest" + "time" + + "sneak.berlin/go/netwatch/internal/logger" + "sneak.berlin/go/netwatch/internal/middleware" + + chimiddleware "github.com/go-chi/chi/v5/middleware" +) + +const ( + // loopbackPeer is a remote address inside the trusted-proxy allowlist. + loopbackPeer = "127.0.0.1:5000" + // forwardedIP is the client address presented via X-Forwarded-For. + forwardedIP = "203.0.113.7" + // realIP is the client address presented via X-Real-IP. + realIP = "203.0.113.9" +) + +func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix { + t.Helper() + + prefixes, err := middleware.ParseTrustedProxies(cidrs) + if err != nil { + t.Fatalf("ParseTrustedProxies(%v): %v", cidrs, err) + } + + return prefixes +} + +// TestParseTrustedProxiesRejectsMalformed includes entries nginx would +// read as another address or look up as a hostname, in the CIDR form +// bin/entrypoint.sh gives "netwatch-server check-cidr". +func TestParseTrustedProxiesRejectsMalformed(t *testing.T) { + t.Parallel() + + for _, cidr := range []string{ + "not-a-cidr", "10.0.0.1", "1.2.3/32", "172.30/32", "10/32", + "cafe/32", "999.1.1.1/32", "10.0.0.0/33", "::1/129", + "fe80::1%eth0/128", + } { + _, err := middleware.ParseTrustedProxies([]string{cidr}) + if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") { + t.Errorf("%q: error = %v, want one naming TRUSTED_PROXIES", + cidr, err) + } + } +} + +func TestParseTrustedProxiesAcceptsCIDRs(t *testing.T) { + t.Parallel() + + mustPrefixes(t, "172.17.0.1/32", "10.0.0.0/8", "2001:db8::1/128", + "2001:db8::/32", "::ffff:192.0.2.1/128") +} + +type clientIPCase struct { + name string + remoteAddr string + xff string + xRealIP string + want string +} + +func clientIPCases() []clientIPCase { + return []clientIPCase{ + { + name: "trusted proxy uses forwarded-for", + remoteAddr: loopbackPeer, + xff: forwardedIP, + want: forwardedIP, + }, + { + name: "trusted proxy uses left-most of chain", + remoteAddr: "10.1.2.3:5000", + xff: forwardedIP + ", 10.1.2.3", + want: forwardedIP, + }, + { + name: "trusted proxy falls back to x-real-ip", + remoteAddr: loopbackPeer, + xRealIP: realIP, + want: realIP, + }, + { + name: "untrusted peer ignores forwarded-for", + remoteAddr: "198.51.100.4:5000", + xff: forwardedIP, + want: "198.51.100.4", + }, + { + name: "untrusted peer ignores x-real-ip", + remoteAddr: "198.51.100.4:5000", + xRealIP: realIP, + want: "198.51.100.4", + }, + { + name: "trusted proxy with no headers uses peer", + remoteAddr: "10.1.2.3:5000", + want: "10.1.2.3", + }, + { + name: "trusted proxy with garbage header uses peer", + remoteAddr: loopbackPeer, + xff: "not-an-ip", + want: "127.0.0.1", + }, + } +} + +func TestClientIP(t *testing.T) { + t.Parallel() + + trusted := mustPrefixes(t, "127.0.0.1/32", "::1/128", "10.0.0.0/8") + + for _, tc := range clientIPCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + header := http.Header{} + if tc.xff != "" { + header.Set("X-Forwarded-For", tc.xff) + } + + if tc.xRealIP != "" { + header.Set("X-Real-IP", tc.xRealIP) + } + + got := middleware.ClientIP(tc.remoteAddr, header, trusted) + if got != tc.want { + t.Errorf("ClientIP() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestSecurityHeaders(t *testing.T) { + t.Parallel() + + handler := (&middleware.Middleware{}).SecurityHeaders()( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody) + handler.ServeHTTP(rec, req) + + want := map[string]string{ + "Strict-Transport-Security": "max-age=31536000; includeSubDomains", + "Content-Security-Policy": "default-src 'none'; frame-ancestors 'none'", + "X-Frame-Options": "DENY", + "X-Content-Type-Options": "nosniff", + "Referrer-Policy": "no-referrer", + "Permissions-Policy": "camera=(), microphone=(), geolocation=()", + } + + for name, value := range want { + if got := rec.Header().Get(name); got != value { + t.Errorf("header %s = %q, want %q", name, got, value) + } + } +} + +// TestMaxBodyBytesRejectsOversizeOnNonReadingRoute confirms the +// limit is enforced even for a handler that never reads the body +// (for example the health check), via the Content-Length check. +func TestMaxBodyBytesRejectsOversizeOnNonReadingRoute(t *testing.T) { + t.Parallel() + + const limit = 16 + + called := false + handler := (&middleware.Middleware{}).MaxBodyBytes(limit)( + http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + called = true + }), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/.well-known/healthcheck", + strings.NewReader(strings.Repeat("x", limit+1)), + ) + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want %d", + rec.Code, http.StatusRequestEntityTooLarge) + } + + if called { + t.Fatal("handler ran despite oversize body") + } + + if got := rec.Body.String(); got != "{\"status\":\"error\"}\n" { + t.Errorf("body = %q, want %q", got, "{\"status\":\"error\"}\n") + } + + got := rec.Header().Get("Content-Type") + if got != "application/json; charset=utf-8" { + t.Errorf("Content-Type = %q, want a JSON content type", got) + } +} + +func TestMaxBodyBytesAllowsWithinLimit(t *testing.T) { + t.Parallel() + + const limit = 64 + + handler := (&middleware.Middleware{}).MaxBodyBytes(limit)( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"clientId":"c1"}`), + ) + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } +} + +func TestRecovererReturns500AndLogsThroughSlog(t *testing.T) { + t.Parallel() + + var logbuf bytes.Buffer + + mw := middleware.NewWithLogger( + slog.New(slog.NewJSONHandler(&logbuf, nil)), + ) + + handler := mw.Recoverer()( + http.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) + } + + var record map[string]any + + err := json.Unmarshal(logbuf.Bytes(), &record) + if err != nil { + t.Fatalf("panic log is not one JSON record: %v (%q)", err, logbuf.String()) + } + + if record["msg"] != "panic recovered" || record["level"] != "ERROR" { + t.Errorf("log record = %v, want msg %q at level ERROR", + record, "panic recovered") + } + + if record["panic"] != "boom" { + t.Errorf("panic field = %v, want %q", record["panic"], "boom") + } + + stack, _ := record["stack"].(string) + if !strings.HasPrefix(stack, "goroutine ") { + t.Errorf("stack field = %q, want a stack trace", stack) + } +} + +// TestRecovererRepanicsOnAbortHandler checks that a handler aborting +// with http.ErrAbortHandler is not treated as a crash: Recoverer +// panics again so the server aborts the response, and logs nothing. +func TestRecovererRepanicsOnAbortHandler(t *testing.T) { + t.Parallel() + + var logbuf bytes.Buffer + + mw := middleware.NewWithLogger( + slog.New(slog.NewJSONHandler(&logbuf, nil)), + ) + + handler := mw.Recoverer()( + http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + panic(http.ErrAbortHandler) + }), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody) + + var recovered any + + func() { + defer func() { recovered = recover() }() + + handler.ServeHTTP(rec, req) + }() + + err, _ := recovered.(error) + if !errors.Is(err, http.ErrAbortHandler) { + t.Errorf("Recoverer panicked with %v, want http.ErrAbortHandler", recovered) + } + + if logbuf.Len() != 0 { + t.Errorf("abort was logged: %q", logbuf.String()) + } +} + +// TestLoggingCutsRequestStringsToBound sends an over-long URL and +// over-long header values, and checks the request log writes each +// one cut to logger.MaxLoggedFieldBytes. +func TestLoggingCutsRequestStringsToBound(t *testing.T) { + t.Parallel() + + long := strings.Repeat("a", 2*logger.MaxLoggedFieldBytes) + + var logbuf bytes.Buffer + + mw := middleware.NewWithLogger( + slog.New(slog.NewJSONHandler(&logbuf, nil)), + ) + + handler := chimiddleware.RequestID(mw.Logging()(okHandler())) + + req := httptest.NewRequestWithContext(t.Context(), + http.MethodGet, "/"+long, http.NoBody) + req.Header.Set("User-Agent", long) + req.Header.Set("Referer", long) + req.Header.Set("X-Request-Id", long) + + handler.ServeHTTP(httptest.NewRecorder(), req) + + var logged map[string]any + + err := json.Unmarshal(logbuf.Bytes(), &logged) + if err != nil { + t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String()) + } + + want := map[string]string{ + "url": ("/" + long)[:logger.MaxLoggedFieldBytes], + "useragent": long[:logger.MaxLoggedFieldBytes], + "referer": long[:logger.MaxLoggedFieldBytes], + "request_id": long[:logger.MaxLoggedFieldBytes], + } + + for field, value := range want { + if logged[field] != value { + t.Errorf("logged %s = %q, want it cut to %d bytes", + field, logged[field], logger.MaxLoggedFieldBytes) + } + } +} + +// okHandler stands in for the route a middleware guards. +func okHandler() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) +} + +// TestRateLimitRefusesPastAllowanceThenResets checks one client +// address: it may use its whole allowance at once, the next request +// is refused with 429, and later it may send again. +func TestRateLimitRefusesPastAllowanceThenResets(t *testing.T) { + t.Parallel() + + // synctest runs this on a fake clock: time.Sleep returns at once, + // with the clock moved on. + synctest.Test(t, func(t *testing.T) { + const perMinute = 2 + + handler := (&middleware.Middleware{}).RateLimit(perMinute)(okHandler()) + + post := func() *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", http.NoBody) + handler.ServeHTTP(rec, req) + + return rec + } + + for i := range perMinute { + if code := post().Code; code != http.StatusOK { + t.Fatalf("request %d: status = %d, want %d", + i+1, code, http.StatusOK) + } + } + + rec := post() + if rec.Code != http.StatusTooManyRequests { + t.Fatalf("request past the allowance: status = %d, want %d", + rec.Code, http.StatusTooManyRequests) + } + + if got := rec.Body.String(); got != "{\"status\":\"error\"}\n" { + t.Errorf("body = %q, want %q", got, "{\"status\":\"error\"}\n") + } + + if got := rec.Header().Get("Retry-After"); got != "60" { + t.Fatalf("Retry-After = %q, want %q", got, "60") + } + + // httprate also counts the previous minute's requests, fading + // them out over the current one, so two minutes on the whole + // allowance is back. + time.Sleep(2 * time.Minute) + + for i := range perMinute { + if code := post().Code; code != http.StatusOK { + t.Fatalf("two minutes later, request %d: status = %d, want %d", + i+1, code, http.StatusOK) + } + } + }) +} + +// postForwarded sends handler a report from peer that names client in +// X-Forwarded-For, and returns the status. +func postForwarded( + t *testing.T, + handler http.Handler, + peer, client string, +) int { + t.Helper() + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", http.NoBody) + req.RemoteAddr = peer + req.Header.Set("X-Forwarded-For", client) + handler.ServeHTTP(rec, req) + + return rec.Code +} + +// TestRateLimitIsPerForwardedClient checks that clients behind a +// trusted proxy each get their own allowance: the limit is keyed on +// the client address clientIP resolves, not on the proxy's. +func TestRateLimitIsPerForwardedClient(t *testing.T) { + t.Parallel() + + const otherClient = "203.0.113.8" + + mw := middleware.NewWithTrustedProxies(mustPrefixes(t, "127.0.0.1/32")) + handler := mw.RateLimit(1)(okHandler()) + + code := postForwarded(t, handler, loopbackPeer, forwardedIP) + if code != http.StatusOK { + t.Fatalf("first request: status = %d, want %d", code, http.StatusOK) + } + + code = postForwarded(t, handler, loopbackPeer, forwardedIP) + if code != http.StatusTooManyRequests { + t.Fatalf("same client again: status = %d, want %d", + code, http.StatusTooManyRequests) + } + + code = postForwarded(t, handler, loopbackPeer, otherClient) + if code != http.StatusOK { + t.Fatalf("other client behind the same proxy: status = %d, want %d", + code, http.StatusOK) + } +} + +// TestRateLimitIgnoresForwardedForFromUntrustedPeer checks that a +// peer that is not a trusted proxy cannot get a fresh allowance by +// naming a different client in X-Forwarded-For on each request. +func TestRateLimitIgnoresForwardedForFromUntrustedPeer(t *testing.T) { + t.Parallel() + + const untrustedPeer = "198.51.100.4:5000" + + mw := middleware.NewWithTrustedProxies(mustPrefixes(t, "127.0.0.1/32")) + handler := mw.RateLimit(1)(okHandler()) + + code := postForwarded(t, handler, untrustedPeer, "203.0.113.8") + if code != http.StatusOK { + t.Fatalf("first request: status = %d, want %d", code, http.StatusOK) + } + + code = postForwarded(t, handler, untrustedPeer, "203.0.113.9") + if code != http.StatusTooManyRequests { + t.Fatalf("same peer naming another client: status = %d, want %d", + code, http.StatusTooManyRequests) + } +} + +// preflight sends cors the preflight request a browser makes before +// it POSTs JSON from origin. +func preflight( + t *testing.T, + cors func(http.Handler) http.Handler, + origin string, +) *httptest.ResponseRecorder { + t.Helper() + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodOptions, "/api/v1/reports", http.NoBody) + req.Header.Set("Origin", origin) + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + req.Header.Set("Access-Control-Request-Headers", "content-type") + cors(okHandler()).ServeHTTP(rec, req) + + return rec +} + +// TestCORSWithoutOriginsAddsNoHeaders checks the default: with no +// origins configured, no origin is given any CORS header. +func TestCORSWithoutOriginsAddsNoHeaders(t *testing.T) { + t.Parallel() + + rec := preflight(t, + (&middleware.Middleware{}).CORS(nil), "https://elsewhere.example") + + for name := range rec.Header() { + if strings.HasPrefix(name, "Access-Control-") { + t.Errorf("CORS header %s set with no origins configured", name) + } + } +} + +func TestCORSAllowsOnlyListedOrigins(t *testing.T) { + t.Parallel() + + const listed = "https://netwatch.example" + + cors := (&middleware.Middleware{}).CORS([]string{listed}) + + cases := []struct { + name string + origin string + want string + }{ + {name: "listed origin allowed", origin: listed, want: listed}, + {name: "other origin refused", origin: "https://elsewhere.example"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + rec := preflight(t, cors, tc.origin) + + got := rec.Header().Get("Access-Control-Allow-Origin") + if got != tc.want { + t.Errorf("Access-Control-Allow-Origin = %q, want %q", + got, tc.want) + } + }) + } +} diff --git a/backend/internal/reportbuf/export_test.go b/backend/internal/reportbuf/export_test.go new file mode 100644 index 0000000..efe8133 --- /dev/null +++ b/backend/internal/reportbuf/export_test.go @@ -0,0 +1,15 @@ +package reportbuf + +import "time" + +// Flush writes the buffered reports to a file now, as the periodic +// flush does, so tests need not wait a minute for it. +func (b *Buffer) Flush() error { + return b.flushLocked() +} + +// StopClock makes every report file the buffer writes from now on +// carry the timestamp at, as if all were written in one millisecond. +func (b *Buffer) StopClock(at time.Time) { + b.now = func() time.Time { return at } +} diff --git a/backend/internal/reportbuf/reportbuf.go b/backend/internal/reportbuf/reportbuf.go index ff4cbad..eeb0184 100644 --- a/backend/internal/reportbuf/reportbuf.go +++ b/backend/internal/reportbuf/reportbuf.go @@ -6,12 +6,15 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io/fs" "log/slog" "os" "path/filepath" + "strings" "sync" + "sync/atomic" "time" "sneak.berlin/go/netwatch/internal/config" @@ -27,8 +30,17 @@ const ( defaultDataDir = "./data/reports" dirPerms fs.FileMode = 0o750 filePerms fs.FileMode = 0o640 + + // Report files are named filePrefix + timestamp + "-" + number + + // fileSuffix; see writeFile. + filePrefix = "reports-" + fileSuffix = ".jsonl.zst" ) +// ErrFull is returned by Append when storing the report would +// take the report files past the configured maximum size. +var ErrFull = errors.New("report files at their size cap") + // Params defines the dependencies for Buffer. type Params struct { fx.In @@ -40,11 +52,23 @@ type Params struct { // Buffer accumulates JSON lines in memory and flushes them // to zstd-compressed files on disk. type Buffer struct { - buf bytes.Buffer - dataDir string - done chan struct{} - log *slog.Logger - mu sync.Mutex + buf bytes.Buffer + dataDir string + done chan struct{} + log *slog.Logger + maxBytes int64 + mu sync.Mutex + // now is the clock report files are named by: time.Now, except + // in tests that need two flushes to share a timestamp. + now func() time.Time + // seq numbers the report files, so that two named in the same + // millisecond still get different names. + seq atomic.Uint64 + stopOnce sync.Once + // usedBytes is what Append checks against maxBytes: the size + // of the report files in dataDir, plus the reports not yet + // written to one at their uncompressed size. + usedBytes int64 } // New creates a Buffer and registers lifecycle hooks to @@ -59,9 +83,11 @@ func New( } b := &Buffer{ - dataDir: dir, - done: make(chan struct{}), - log: params.Logger.Get(), + dataDir: dir, + done: make(chan struct{}), + log: params.Logger.Get(), + maxBytes: params.Config.DataDirMaxBytes, + now: time.Now, } lc.Append(fx.Hook{ @@ -71,15 +97,30 @@ func New( return fmt.Errorf("create data dir: %w", err) } + // Report files left by earlier runs count too. + b.usedBytes, err = reportFilesSize(b.dataDir) + if err != nil { + return err + } + go b.flushLoop() return nil }, OnStop: func(_ context.Context) error { - close(b.done) - b.flushLocked() + // stopOnce makes OnStop idempotent: a second + // invocation must not close an already-closed channel + // (which would panic) or flush again. + var err error - return nil + b.stopOnce.Do(func() { + close(b.done) + err = b.flushLocked() + }) + + // A failed final flush fails the stop, so the process + // exits non-zero. + return err }, }) @@ -87,15 +128,27 @@ func New( } // Append marshals v as a single JSON line and appends it to -// the buffer. If the buffer reaches the size threshold, it is -// drained and written to disk asynchronously. +// the buffer. It stores nothing and returns ErrFull if the line +// would take usedBytes past maxBytes. If the buffer reaches the +// size threshold, it is drained and written to disk +// asynchronously. func (b *Buffer) Append(v any) error { line, err := json.Marshal(v) if err != nil { return fmt.Errorf("marshal report: %w", err) } + lineBytes := int64(len(line)) + 1 // with its newline + b.mu.Lock() + + if b.usedBytes+lineBytes > b.maxBytes { + b.mu.Unlock() + + return ErrFull + } + + b.usedBytes += lineBytes b.buf.Write(line) b.buf.WriteByte('\n') @@ -103,7 +156,12 @@ func (b *Buffer) Append(v any) error { data := b.drainBuf() b.mu.Unlock() - go b.writeFile(data) + go func() { + writeErr := b.writeFile(data) + if writeErr != nil { + b.log.Error("flush reports failed", "error", writeErr) + } + }() return nil } @@ -122,7 +180,10 @@ func (b *Buffer) flushLoop() { for { select { case <-ticker.C: - b.flushLocked() + err := b.flushLocked() + if err != nil { + b.log.Error("flush reports failed", "error", err) + } case <-b.done: return } @@ -131,19 +192,19 @@ func (b *Buffer) flushLoop() { // flushLocked acquires the lock, drains the buffer, and // writes the data to a compressed file. -func (b *Buffer) flushLocked() { +func (b *Buffer) flushLocked() error { b.mu.Lock() if b.buf.Len() == 0 { b.mu.Unlock() - return + return nil } data := b.drainBuf() b.mu.Unlock() - b.writeFile(data) + return b.writeFile(data) } // drainBuf copies the buffer contents and resets it. @@ -158,42 +219,91 @@ func (b *Buffer) drainBuf() []byte { // writeFile creates a timestamped zstd-compressed JSONL file // in the data directory. -func (b *Buffer) writeFile(data []byte) { - ts := time.Now().UTC().Format("2006-01-02T15-04-05.000Z") - name := fmt.Sprintf("reports-%s.jsonl.zst", ts) +func (b *Buffer) writeFile(data []byte) error { + // The timestamp comes first, so the names sort by time; the number + // after it tells apart files named in the same millisecond. + ts := b.now().UTC().Format("2006-01-02T15-04-05.000Z") + name := fmt.Sprintf("%s%s-%d%s", filePrefix, ts, b.seq.Add(1), fileSuffix) path := filepath.Join(b.dataDir, name) - f, err := os.OpenFile( //nolint:gosec // path built from controlled dataDir + timestamp + // path is built from the operator-supplied dataDir plus a + // generated timestamp and number, so it carries no external input. + f, err := os.OpenFile( //nolint:gosec // see comment above path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, filePerms, ) if err != nil { - b.log.Error("create report file", "error", err) - - return + return fmt.Errorf("create report file: %w", err) } + // Closes the file on the early returns below. The success + // path closes it explicitly to check the error; closing it + // a second time here is harmless. defer func() { _ = f.Close() }() enc, err := zstd.NewWriter(f) if err != nil { - b.log.Error("create zstd encoder", "error", err) - - return + return fmt.Errorf("create zstd encoder: %w", err) } - _, writeErr := enc.Write(data) - if writeErr != nil { - b.log.Error("write compressed data", "error", writeErr) - + _, err = enc.Write(data) + if err != nil { _ = enc.Close() - return + return fmt.Errorf("write compressed data: %w", err) } - closeErr := enc.Close() - if closeErr != nil { - b.log.Error("close zstd encoder", "error", closeErr) + err = enc.Close() + if err != nil { + return fmt.Errorf("close zstd encoder: %w", err) } + + info, err := f.Stat() + if err != nil { + return fmt.Errorf("stat report file: %w", err) + } + + err = f.Close() + if err != nil { + return fmt.Errorf("close report file: %w", err) + } + + // The reports counted at their uncompressed size while they + // waited; now they count as the file. After a failed write they + // stay counted as they were, which errs toward refusing reports + // early rather than letting the files pass the cap. + b.mu.Lock() + b.usedBytes += info.Size() - int64(len(data)) + b.mu.Unlock() + + return nil +} + +// reportFilesSize returns the total size of the report files in +// dir. +func reportFilesSize(dir string) (int64, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return 0, fmt.Errorf("read data dir: %w", err) + } + + var total int64 + + for _, entry := range entries { + name := entry.Name() + if !strings.HasPrefix(name, filePrefix) || + !strings.HasSuffix(name, fileSuffix) { + continue + } + + info, err := entry.Info() + if err != nil { + return 0, fmt.Errorf("stat report file: %w", err) + } + + total += info.Size() + } + + return total, nil } diff --git a/backend/internal/reportbuf/reportbuf_test.go b/backend/internal/reportbuf/reportbuf_test.go index 24a9ae9..0b56b7e 100644 --- a/backend/internal/reportbuf/reportbuf_test.go +++ b/backend/internal/reportbuf/reportbuf_test.go @@ -1,13 +1,441 @@ package reportbuf_test import ( + "encoding/json" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "sync" + "sync/atomic" "testing" + "time" - _ "sneak.berlin/go/netwatch/internal/reportbuf" + "sneak.berlin/go/netwatch/internal/config" + "sneak.berlin/go/netwatch/internal/globals" + "sneak.berlin/go/netwatch/internal/logger" + "sneak.berlin/go/netwatch/internal/reportbuf" + + "github.com/klauspost/compress/zstd" + "go.uber.org/fx" + "go.uber.org/fx/fxtest" ) -func TestImport(t *testing.T) { - t.Parallel() - // Compilation check — verifies the package parses - // and all imports resolve. +// TestFlushOnShutdown proves the flush-on-shutdown path: a +// report appended after start but before the periodic flush +// window must reach disk when the fx lifecycle stops. This is +// the exact case that silent data loss on restart used to +// destroy. +func TestFlushOnShutdown(t *testing.T) { + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + var buf *reportbuf.Buffer + + app := fxtest.New(t, + fx.Provide( + globals.New, + logger.New, + config.New, + reportbuf.New, + ), + fx.Populate(&buf), + ) + + app.RequireStart() + + err := buf.Append(map[string]string{"probe": "shutdown"}) + if err != nil { + t.Fatalf("append report: %v", err) + } + + // RequireStop runs the reportbuf OnStop hook, which is the + // only code path that flushes buffered reports on shutdown. + app.RequireStop() + + if !hasReportFile(t, dir) { + t.Fatal("no report file on disk after shutdown; " + + "the buffered report was lost") + } +} + +// TestFailedFinalFlushFailsStop proves a final flush that cannot +// write its file makes the stop fail, which makes the process +// exit non-zero instead of dropping the buffered reports silently. +func TestFailedFinalFlushFailsStop(t *testing.T) { + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + var buf *reportbuf.Buffer + + app := fxtest.New(t, + fx.Provide( + globals.New, + logger.New, + config.New, + reportbuf.New, + ), + fx.Populate(&buf), + ) + + app.RequireStart() + + err := buf.Append(map[string]string{"probe": "shutdown"}) + if err != nil { + t.Fatalf("append report: %v", err) + } + + // Removing the data directory leaves the final flush nowhere to + // write. A read-only directory would not do: tests run as root + // in the backend image, and root ignores the read-only bit. + err = os.RemoveAll(dir) + if err != nil { + t.Fatalf("remove data dir: %v", err) + } + + err = app.Stop(t.Context()) + if !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("stop error = %v, want the final flush's error", err) + } +} + +// 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() + + var buf *reportbuf.Buffer + + app := fxtest.New(t, + fx.Provide( + globals.New, + logger.New, + config.New, + reportbuf.New, + ), + fx.Populate(&buf), + ) + + app.RequireStart() + t.Cleanup(app.RequireStop) + + return buf +} + +// lineBytes is what one report takes in the buffer: its JSON and a +// newline. +func lineBytes(t *testing.T, report any) int { + t.Helper() + + line, err := json.Marshal(report) + if err != nil { + t.Fatalf("marshal report: %v", err) + } + + return len(line) + 1 +} + +func TestAppendPastCapIsRefused(t *testing.T) { + report := map[string]string{"id": "cap"} + + t.Setenv("DATA_DIR", t.TempDir()) + t.Setenv("DATA_DIR_MAX_BYTES", strconv.Itoa(lineBytes(t, report))) + + buf := startBuffer(t) + + err := buf.Append(report) + if err != nil { + t.Fatalf("report that fills the cap exactly: %v", err) + } + + err = buf.Append(report) + if !errors.Is(err, reportbuf.ErrFull) { + t.Fatalf("report past the cap: error = %v, want ErrFull", err) + } +} + +// TestCapCountsReportFilesAlreadyInDataDir starts on a data +// directory holding a report file from an earlier run, and a file +// that is not a report, which must not count. +func TestCapCountsReportFilesAlreadyInDataDir(t *testing.T) { + const earlierBytes = 100 + + report := map[string]string{"id": "cap"} + dir := t.TempDir() + + writeBytes(t, filepath.Join(dir, "reports-2026-01-01T00-00-00.000Z.jsonl.zst"), + earlierBytes) + writeBytes(t, filepath.Join(dir, "notes.txt"), 10*earlierBytes) + + t.Setenv("DATA_DIR", dir) + t.Setenv("DATA_DIR_MAX_BYTES", + strconv.Itoa(earlierBytes+lineBytes(t, report))) + + buf := startBuffer(t) + + err := buf.Append(report) + if err != nil { + t.Fatalf("report that fills the cap exactly: %v", err) + } + + err = buf.Append(report) + if !errors.Is(err, reportbuf.ErrFull) { + t.Fatalf("report past the cap: error = %v, want ErrFull", err) + } +} + +// TestWrittenReportsCountAtFileSize checks that once reports are +// written, they count as their compressed file, not their +// uncompressed size, which frees room under the cap. +func TestWrittenReportsCountAtFileSize(t *testing.T) { + // Repetitive, so its file is far smaller than its JSON. + report := map[string]string{"id": strings.Repeat("a", 1000)} + size := lineBytes(t, report) + + t.Setenv("DATA_DIR", t.TempDir()) + // Room for the report twice over only if the first one counts + // at its file's size by the time the second arrives. + t.Setenv("DATA_DIR_MAX_BYTES", strconv.Itoa(2*size-1)) + + buf := startBuffer(t) + + err := buf.Append(report) + if err != nil { + t.Fatalf("first report: %v", err) + } + + err = buf.Flush() + if err != nil { + t.Fatalf("flush: %v", err) + } + + err = buf.Append(report) + if err != nil { + t.Fatalf("second report, after the first was written: %v", err) + } +} + +// TestWrittenReportsKeepCounting writes one report file after another +// under a small cap: each report must be taken while the files on disk +// leave room for it, and refused once they do not. +func TestWrittenReportsKeepCounting(t *testing.T) { + const maxBytes = 200 + + report := map[string]string{"id": "written"} + size := int64(lineBytes(t, report)) + dir := t.TempDir() + + t.Setenv("DATA_DIR", dir) + t.Setenv("DATA_DIR_MAX_BYTES", strconv.Itoa(maxBytes)) + + buf := startBuffer(t) + + // Every file takes at least a byte, so they fill the cap within + // maxBytes rounds. + for range maxBytes { + used := reportFilesBytes(t, dir) + + err := buf.Append(report) + if used+size > maxBytes { + if !errors.Is(err, reportbuf.ErrFull) { + t.Fatalf("with %d bytes of report files: error = %v, "+ + "want ErrFull", used, err) + } + + return + } + + if err != nil { + t.Fatalf("with %d bytes of report files: %v", used, err) + } + + err = buf.Flush() + if err != nil { + t.Fatalf("flush: %v", err) + } + } + + t.Fatal("the report files never filled the cap") +} + +// TestConcurrentAppendsStopAtCap appends from many goroutines at once +// with room for exactly roomFor reports: exactly that many must be +// taken, which holds only if Append checks and counts each report +// under one lock. +func TestConcurrentAppendsStopAtCap(t *testing.T) { + const ( + roomFor = 5 + senders = 50 + ) + + // Large, so each Append takes long enough for the senders to + // overlap while the cap is reached. + report := map[string]string{"id": strings.Repeat("a", 1_000_000)} + + t.Setenv("DATA_DIR", t.TempDir()) + t.Setenv("DATA_DIR_MAX_BYTES", + strconv.Itoa(roomFor*lineBytes(t, report))) + + buf := startBuffer(t) + + var ( + taken atomic.Int64 + wg sync.WaitGroup + ) + + start := make(chan struct{}) + + for range senders { + wg.Go(func() { + <-start + + err := buf.Append(report) + if err == nil { + taken.Add(1) + } else if !errors.Is(err, reportbuf.ErrFull) { + t.Errorf("append: %v", err) + } + }) + } + + close(start) + wg.Wait() + + if got := taken.Load(); got != roomFor { + t.Fatalf("%d reports taken, want %d", got, roomFor) + } +} + +// TestTwoFlushesInOneMillisecond flushes twice within one millisecond, +// as a flush for size and the final flush at shutdown can: each flush +// must write a file of its own, and the files must hold every report. +func TestTwoFlushesInOneMillisecond(t *testing.T) { + const flushes = 2 + + dir := t.TempDir() + t.Setenv("DATA_DIR", dir) + + buf := startBuffer(t) + buf.StopClock(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)) + + for id := 1; id <= flushes; id++ { + err := buf.Append(map[string]int{"id": id}) + if err != nil { + t.Fatalf("append report %d: %v", id, err) + } + + err = buf.Flush() + if err != nil { + t.Fatalf("flush %d: %v", id, err) + } + } + + files := readReportFiles(t, dir) + if len(files) != flushes { + t.Fatalf("%d report files after %d flushes", len(files), flushes) + } + + for id := 1; id <= flushes; id++ { + want := fmt.Sprintf(`{"id":%d}`+"\n", id) + if !slices.Contains(files, want) { + t.Fatalf("no report file holds report %d alone", id) + } + } +} + +// reportFilesBytes returns the total size of the report files in dir. +func reportFilesBytes(t *testing.T, dir string) int64 { + t.Helper() + + paths, err := filepath.Glob(filepath.Join(dir, "reports-*.jsonl.zst")) + if err != nil { + t.Fatalf("list report files: %v", err) + } + + var total int64 + + for _, path := range paths { + info, statErr := os.Stat(path) + if statErr != nil { + t.Fatalf("stat %s: %v", path, statErr) + } + + total += info.Size() + } + + return total +} + +// readReportFiles returns the decompressed contents of each report +// file in dir. +func readReportFiles(t *testing.T, dir string) []string { + t.Helper() + + files := os.DirFS(dir) + + names, err := fs.Glob(files, "reports-*.jsonl.zst") + if err != nil { + t.Fatalf("list report files: %v", err) + } + + dec, err := zstd.NewReader(nil) + if err != nil { + t.Fatalf("create zstd decoder: %v", err) + } + defer dec.Close() + + contents := make([]string, 0, len(names)) + + for _, name := range names { + compressed, readErr := fs.ReadFile(files, name) + if readErr != nil { + t.Fatalf("read %s: %v", name, readErr) + } + + data, decErr := dec.DecodeAll(compressed, nil) + if decErr != nil { + t.Fatalf("decompress %s: %v", name, decErr) + } + + contents = append(contents, string(data)) + } + + return contents +} + +func writeBytes(t *testing.T, path string, n int) { + t.Helper() + + err := os.WriteFile(path, make([]byte, n), 0o600) + if err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +func hasReportFile(t *testing.T, dir string) bool { + t.Helper() + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read data dir: %v", err) + } + + for _, e := range entries { + if strings.HasSuffix(e.Name(), ".jsonl.zst") { + info, statErr := e.Info() + if statErr != nil { + t.Fatalf("stat %s: %v", e.Name(), statErr) + } + + if info.Size() > 0 { + return true + } + } + } + + return false } diff --git a/backend/internal/server/export_test.go b/backend/internal/server/export_test.go new file mode 100644 index 0000000..84cdd90 --- /dev/null +++ b/backend/internal/server/export_test.go @@ -0,0 +1,11 @@ +package server + +// MaxRequestBodyBytes exposes the router-wide body limit to the +// external tests. +const MaxRequestBodyBytes = maxRequestBodyBytes + +// ListenAddr exposes the address the server listens on to the +// external tests. +func (s *Server) ListenAddr() string { + return s.newHTTPServer().Addr +} diff --git a/backend/internal/server/http.go b/backend/internal/server/http.go index d3824c6..46f4a67 100644 --- a/backend/internal/server/http.go +++ b/backend/internal/server/http.go @@ -2,42 +2,69 @@ package server import ( "errors" - "fmt" + "net" "net/http" + "strconv" "time" + + "go.uber.org/fx" ) const ( - readTimeout = 10 * time.Second - writeTimeout = 10 * time.Second - maxHeaderBytes = 1 << 20 // 1 MiB + readTimeout = 10 * time.Second + readHeaderTimeout = 5 * time.Second + idleTimeout = 60 * time.Second + maxHeaderBytes = 1 << 20 // 1 MiB + + // requestTimeout (routes.go) is the single per-request + // processing budget, enforced by chi's middleware.Timeout. + // writeTimeout must exceed that budget so a handler can write + // its 503 when the chi timeout fires; if it were shorter the + // server would abort the write first and the chi budget would + // be unreachable dead configuration. + writeTimeout = requestTimeout + 5*time.Second ) -func (s *Server) serveUntilShutdown() { - listenAddr := fmt.Sprintf(":%d", s.params.Config.Port) +// newHTTPServer constructs the http.Server. It performs no I/O +// and does not start listening. +func (s *Server) newHTTPServer() *http.Server { + listenAddr := net.JoinHostPort( + s.params.Config.BindAddress, + strconv.Itoa(s.params.Config.Port), + ) - s.httpServer = &http.Server{ - Addr: listenAddr, - Handler: s, - MaxHeaderBytes: maxHeaderBytes, - ReadTimeout: readTimeout, - WriteTimeout: writeTimeout, + return &http.Server{ + Addr: listenAddr, + Handler: s, + MaxHeaderBytes: maxHeaderBytes, + ReadTimeout: readTimeout, + ReadHeaderTimeout: readHeaderTimeout, + WriteTimeout: writeTimeout, + IdleTimeout: idleTimeout, } +} - s.SetupRoutes() - +// listenAndServe runs the listener until the server is shut +// down. A genuine listen failure (not the expected +// ErrServerClosed from a clean shutdown) requests process +// shutdown through fx with a non-zero exit code, so the failure +// is visible to any supervisor. +func (s *Server) listenAndServe() { s.log.Info("http begin listen", - "listenaddr", listenAddr, + "listenaddr", s.httpServer.Addr, "version", s.params.Globals.Version, "buildarch", s.params.Globals.Buildarch, ) err := s.httpServer.ListenAndServe() - if err != nil && !errors.Is(err, http.ErrServerClosed) { - s.log.Error("listen error", "error", err) + if err == nil || errors.Is(err, http.ErrServerClosed) { + return + } - if s.cancelFunc != nil { - s.cancelFunc() - } + s.log.Error("listen error", "error", err) + + shutdownErr := s.shutdowner.Shutdown(fx.ExitCode(1)) + if shutdownErr != nil { + s.log.Error("request shutdown failed", "error", shutdownErr) } } diff --git a/backend/internal/server/http_test.go b/backend/internal/server/http_test.go new file mode 100644 index 0000000..d1370df --- /dev/null +++ b/backend/internal/server/http_test.go @@ -0,0 +1,32 @@ +package server_test + +import "testing" + +// TestListenAddress checks that the server listens on BIND_ADDRESS +// and PORT, and on port 8080 on every interface when neither is set. +// The container image sets both, to keep the backend on loopback +// behind nginx. +func TestListenAddress(t *testing.T) { + tests := []struct { + bindAddress string + port string + want string + }{ + {bindAddress: "", port: "", want: ":8080"}, + {bindAddress: "127.0.0.1", port: "8081", want: "127.0.0.1:8081"}, + {bindAddress: "::1", port: "8081", want: "[::1]:8081"}, + } + + for _, tt := range tests { + t.Run(tt.want, func(t *testing.T) { + // t.Setenv rules out t.Parallel. + t.Setenv("BIND_ADDRESS", tt.bindAddress) + t.Setenv("PORT", tt.port) + + got := newServer(t).ListenAddr() + if got != tt.want { + t.Errorf("listen address = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/backend/internal/server/routes.go b/backend/internal/server/routes.go index da74cf6..4c87f59 100644 --- a/backend/internal/server/routes.go +++ b/backend/internal/server/routes.go @@ -7,17 +7,26 @@ import ( "github.com/go-chi/chi/v5/middleware" ) -const requestTimeout = 60 * time.Second +const ( + requestTimeout = 60 * time.Second + + // maxRequestBodyBytes caps every request body. A route group + // can mount s.mw.MaxBodyBytes with a smaller value to lower + // its bound, but cannot raise it: this cap runs first. + maxRequestBodyBytes int64 = 1 << 20 // 1 MiB +) // SetupRoutes configures the chi router with middleware and // all application routes. func (s *Server) SetupRoutes() { s.router = chi.NewRouter() - s.router.Use(middleware.Recoverer) + s.router.Use(s.mw.Recoverer()) s.router.Use(middleware.RequestID) s.router.Use(s.mw.Logging()) - s.router.Use(s.mw.CORS()) + 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)) s.router.Get( @@ -26,6 +35,7 @@ func (s *Server) SetupRoutes() { ) s.router.Route("/api/v1", func(r chi.Router) { - r.Post("/reports", s.h.HandleReport()) + r.With(s.mw.RateLimit(s.params.Config.ReportsPerMinute)). + Post("/reports", s.h.HandleReport()) }) } diff --git a/backend/internal/server/routes_test.go b/backend/internal/server/routes_test.go new file mode 100644 index 0000000..972debd --- /dev/null +++ b/backend/internal/server/routes_test.go @@ -0,0 +1,142 @@ +package server_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "sneak.berlin/go/netwatch/internal/config" + "sneak.berlin/go/netwatch/internal/globals" + "sneak.berlin/go/netwatch/internal/handlers" + "sneak.berlin/go/netwatch/internal/healthcheck" + "sneak.berlin/go/netwatch/internal/logger" + "sneak.berlin/go/netwatch/internal/middleware" + "sneak.berlin/go/netwatch/internal/reportbuf" + "sneak.berlin/go/netwatch/internal/server" + + "go.uber.org/fx" + "go.uber.org/fx/fxtest" +) + +// newServer builds a Server from the same constructors as main, +// configured from the environment. It is never started, so nothing +// listens. +func newServer(t *testing.T) *server.Server { + t.Helper() + + var srv *server.Server + + app := fxtest.New(t, + fx.Provide( + config.New, + globals.New, + handlers.New, + healthcheck.New, + logger.New, + middleware.New, + reportbuf.New, + server.New, + ), + fx.Populate(&srv), + ) + + err := app.Err() + if err != nil { + t.Fatalf("build server: %v", err) + } + + return srv +} + +// TestReportsAreRateLimited checks that POST /api/v1/reports is +// behind the per-address rate limit, set here to two a minute. +func TestReportsAreRateLimited(t *testing.T) { + t.Setenv("REPORTS_PER_MINUTE", "2") + + srv := newServer(t) + srv.SetupRoutes() + + post := func() int { + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"clientId":"c1","hosts":[]}`), + ) + srv.ServeHTTP(rec, req) + + return rec.Code + } + + for i := range 2 { + if code := post(); code != http.StatusOK { + t.Fatalf("report %d: status = %d, want %d", + i+1, code, http.StatusOK) + } + } + + if code := post(); code != http.StatusTooManyRequests { + t.Fatalf("third report in a minute: status = %d, want %d", + code, http.StatusTooManyRequests) + } +} + +// TestCORSAllowedOriginsReachTheRouter checks that an origin listed in +// CORS_ALLOWED_ORIGINS is allowed by the router, not only when handed +// to the CORS middleware directly. +func TestCORSAllowedOriginsReachTheRouter(t *testing.T) { + const origin = "https://netwatch.example:8443" + + t.Setenv("CORS_ALLOWED_ORIGINS", origin) + + srv := newServer(t) + srv.SetupRoutes() + + // The preflight a browser sends before it POSTs JSON from origin. + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodOptions, "/api/v1/reports", http.NoBody) + req.Header.Set("Origin", origin) + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + req.Header.Set("Access-Control-Request-Headers", "content-type") + srv.ServeHTTP(rec, req) + + got := rec.Header().Get("Access-Control-Allow-Origin") + if got != origin { + t.Fatalf("Access-Control-Allow-Origin = %q, want %q", got, origin) + } +} + +// TestHealthCheckRejectsOversizeBody sends the health check, which +// never reads its body, a body one byte over the limit. Only the +// router-wide body limit can reject it. +func TestHealthCheckRejectsOversizeBody(t *testing.T) { + t.Parallel() + + srv := newServer(t) + srv.SetupRoutes() + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodGet, "/.well-known/healthcheck", + strings.NewReader( + strings.Repeat("x", int(server.MaxRequestBodyBytes)+1), + ), + ) + + srv.ServeHTTP(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want %d", + rec.Code, http.StatusRequestEntityTooLarge) + } + + if got := rec.Body.String(); got != "{\"status\":\"error\"}\n" { + t.Errorf("body = %q, want %q", got, "{\"status\":\"error\"}\n") + } + + got := rec.Header().Get("Content-Type") + if got != "application/json; charset=utf-8" { + t.Errorf("Content-Type = %q, want a JSON content type", got) + } +} diff --git a/backend/internal/server/server.go b/backend/internal/server/server.go index a67bd1e..d14dd25 100644 --- a/backend/internal/server/server.go +++ b/backend/internal/server/server.go @@ -1,16 +1,14 @@ // Package server provides the HTTP server lifecycle, -// including startup, routing, signal handling, and graceful -// shutdown. +// including startup, routing, and graceful shutdown. The +// process lifetime is owned by fx: shutdown is requested +// through fx.Shutdowner so every component's OnStop hook runs +// in dependency order. package server import ( "context" "log/slog" "net/http" - "os" - "os/signal" - "syscall" - "time" "sneak.berlin/go/netwatch/internal/config" "sneak.berlin/go/netwatch/internal/globals" @@ -31,19 +29,18 @@ type Params struct { Handlers *handlers.Handlers Logger *logger.Logger Middleware *middleware.Middleware + Shutdowner fx.Shutdowner } // Server is the top-level HTTP server orchestrator. type Server struct { - cancelFunc context.CancelFunc - exitCode int - h *handlers.Handlers - httpServer *http.Server - log *slog.Logger - mw *middleware.Middleware - params Params - router *chi.Mux - startupTime time.Time + h *handlers.Handlers + httpServer *http.Server + log *slog.Logger + mw *middleware.Middleware + params Params + router *chi.Mux + shutdowner fx.Shutdowner } // New creates a Server and registers lifecycle hooks for @@ -57,23 +54,25 @@ func New( s.mw = params.Middleware s.h = params.Handlers s.log = params.Logger.Get() + s.shutdowner = params.Shutdowner lc.Append(fx.Hook{ OnStart: func(_ context.Context) error { - s.startupTime = time.Now().UTC() + // Build the router and http.Server synchronously + // here, before spawning the serving goroutine, so + // httpServer is fully constructed by the time OnStop + // (or an early signal) can read it. fx guarantees + // OnStart returns before OnStop runs, so no + // synchronization or nil check is needed at shutdown. + s.SetupRoutes() + s.httpServer = s.newHTTPServer() - go func() { //nolint:contextcheck // fx OnStart ctx is startup-only; run() creates its own - s.run() - }() + go s.listenAndServe() return nil }, - OnStop: func(_ context.Context) error { - if s.cancelFunc != nil { - s.cancelFunc() - } - - return nil + OnStop: func(ctx context.Context) error { + return s.shutdown(ctx) }, }) @@ -88,60 +87,17 @@ func (s *Server) ServeHTTP( s.router.ServeHTTP(w, r) } -func (s *Server) run() { - exitCode := s.serve() - os.Exit(exitCode) -} - -func (s *Server) serve() int { - var ctx context.Context //nolint:wsl // ctx must be declared before multi-assign - - ctx, s.cancelFunc = context.WithCancel( - context.Background(), - ) - - go func() { - c := make(chan os.Signal, 1) - - signal.Ignore(syscall.SIGPIPE) - signal.Notify(c, os.Interrupt, syscall.SIGTERM) - - sig := <-c - s.log.Info("signal received", "signal", sig) - - if s.cancelFunc != nil { - s.cancelFunc() - } - }() - - go func() { - s.serveUntilShutdown() - }() - - <-ctx.Done() - s.cleanShutdown() - - return s.exitCode -} - -const shutdownTimeout = 5 * time.Second - -func (s *Server) cleanShutdown() { - s.exitCode = 0 - - ctxShutdown, shutdownCancel := context.WithTimeout( - context.Background(), - shutdownTimeout, - ) - defer shutdownCancel() - - err := s.httpServer.Shutdown(ctxShutdown) +// shutdown gracefully stops the HTTP server within the +// deadline of the context fx provides for OnStop. +func (s *Server) shutdown(ctx context.Context) error { + err := s.httpServer.Shutdown(ctx) if err != nil { - s.log.Error( - "server clean shutdown failed", - "error", err, - ) + s.log.Error("server clean shutdown failed", "error", err) + + return err } s.log.Info("server stopped") + + return nil } diff --git a/backend/script/build b/backend/script/build new file mode 100755 index 0000000..40d4d01 --- /dev/null +++ b/backend/script/build @@ -0,0 +1,21 @@ +#!/bin/sh +# script/build: compile the static netwatch-server binary into the +# backend project root, with its version and architecture stamped in. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + + # VERSION comes from the environment (the root Dockerfile passes its + # ARG VERSION in). Unset or empty, it is git describe, or "dev" where + # there is no git or no repository history. + version="${VERSION:-$(git describe --always --dirty 2>/dev/null || echo dev)}" + + CGO_ENABLED=0 go build -trimpath \ + -ldflags "-s -w -X main.Version=$version -X main.Buildarch=$(uname -m)" \ + -o netwatch-server ./cmd/netwatch-server/ +} + +main "$@" diff --git a/backend/script/clean b/backend/script/clean new file mode 100755 index 0000000..1f4e638 --- /dev/null +++ b/backend/script/clean @@ -0,0 +1,12 @@ +#!/bin/sh +# script/clean: remove build artifacts. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + rm -f netwatch-server +} + +main "$@" diff --git a/backend/script/fmt b/backend/script/fmt new file mode 100755 index 0000000..9fa78bf --- /dev/null +++ b/backend/script/fmt @@ -0,0 +1,12 @@ +#!/bin/sh +# script/fmt: format the Go sources (writes). +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + go fmt ./... +} + +main "$@" diff --git a/backend/script/fmt-check b/backend/script/fmt-check new file mode 100755 index 0000000..57405d0 --- /dev/null +++ b/backend/script/fmt-check @@ -0,0 +1,18 @@ +#!/bin/sh +# script/fmt-check: check Go formatting (read-only). Same scope as +# script/fmt, but fails instead of writing. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + unformatted="$(gofmt -l .)" + if [ -n "$unformatted" ]; then + echo "Files not formatted:" >&2 + echo "$unformatted" >&2 + exit 1 + fi +} + +main "$@" diff --git a/backend/script/lint b/backend/script/lint new file mode 100755 index 0000000..720b747 --- /dev/null +++ b/backend/script/lint @@ -0,0 +1,32 @@ +#!/bin/sh +# script/lint: run golangci-lint over the backend. This runs inside the +# lint stage of the root Dockerfile, whose digest-pinned golangci-lint +# image provides the linter; nothing installs golangci-lint on the host. +# From a checkout, run `make lint` at the repo root, which builds that +# stage. +# +# .golangci.yml is standardized org-wide and must never be edited here +# (REPO_POLICIES.md). Its last silent drift replaced the v2 schema with +# v1 keys, which left every threshold in the file inert while the build +# stayed green. So the file is first checked against the canonical +# copy's sha256: a local comparison, no network, nothing unpinned. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +GOLANGCI_CONFIG_SHA256="a79b63a254602a5318db5d0e9a06bc71b84bf0c1d896305229d8bfed1d1b1776" + +main() { + cd "$ROOT" + actual="$(sha256sum .golangci.yml | cut -d' ' -f1)" + if [ "$actual" != "$GOLANGCI_CONFIG_SHA256" ]; then + echo ".golangci.yml has drifted from the org standard." >&2 + echo " expected $GOLANGCI_CONFIG_SHA256" >&2 + echo " actual $actual" >&2 + echo "Restore it verbatim from sneak/prompts; do not edit it." >&2 + exit 1 + fi + golangci-lint run ./... +} + +main "$@" diff --git a/backend/script/run b/backend/script/run new file mode 100755 index 0000000..5ea87ba --- /dev/null +++ b/backend/script/run @@ -0,0 +1,13 @@ +#!/bin/sh +# script/run: build and run netwatch-server locally. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + "$ROOT/script/build" + exec ./netwatch-server "$@" +} + +main "$@" diff --git a/backend/script/test b/backend/script/test new file mode 100755 index 0000000..cbfc889 --- /dev/null +++ b/backend/script/test @@ -0,0 +1,12 @@ +#!/bin/sh +# script/test: run the backend test suite. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + timeout 30 go test ./... +} + +main "$@" diff --git a/bin/entrypoint.sh b/bin/entrypoint.sh new file mode 100755 index 0000000..6f3bdf8 --- /dev/null +++ b/bin/entrypoint.sh @@ -0,0 +1,142 @@ +#!/bin/sh +# The container's entrypoint: runs netwatch-server and nginx side by +# side. TERM or INT stops both, and the container exits 0 if both exit +# cleanly. If either exits on its own, the other is stopped too and the +# container exits non-zero, so the platform restarts it instead of +# leaving it half up. +# +# No set -e: kill and wait return non-zero here in normal operation. +set -u + +# PORT is the public port nginx listens on, 8080 when unset or empty. +# nginx would take a value such as localhost or unix:/tmp/x.sock as an +# address and start anyway, and reports a bad port without naming +# PORT, so a value that is not a usable port stops the container here, +# before either process starts. +export PORT="${PORT:-8080}" +case "$PORT" in + *[!0-9]*) + echo "entrypoint: PORT must be a port number, not '$PORT'" >&2 + exit 1 + ;; +esac +# The length is checked first because, for a number too big for it, +# the shell's test prints an error and is false, so the range checks +# alone would let it through. +if [ "${#PORT}" -gt 5 ] || [ "$PORT" -lt 1 ] || [ "$PORT" -gt 65535 ]; then + echo "entrypoint: PORT must be from 1 to 65535, not '$PORT'" >&2 + exit 1 +fi +if [ "$PORT" -eq 8081 ]; then + echo "entrypoint: PORT cannot be 8081, netwatch-server listens there" >&2 + exit 1 +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 +# unset or empty, it trusts no one. 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 +# is made a CIDR, a lone address getting /128 if it is IPv6 and /32 if +# not, and netwatch-server checks it with the parsing it gives its own +# 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:-}" +set -f +for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do + case "$proxy" in + */*) cidr="$proxy" ;; + *:*) cidr="$proxy/128" ;; + *) cidr="$proxy/32" ;; + esac + if ! netwatch-server check-cidr "$cidr" 2> /dev/null; then + echo "entrypoint: TRUSTED_PROXIES must be IP addresses or CIDRs" \ + "separated by commas; '$proxy' is neither" >&2 + exit 1 + fi + echo "set_real_ip_from $cidr;" +done > /etc/nginx/trusted-proxies.conf + +# netwatch-server keeps its report files in DATA_DIR, on the /data +# volume, which may be a host directory owned by root or by another +# uid. Both are given to the netwatch user here, with the mode the +# server gives a directory it creates, so the host directory needs no +# preparing. +# +# chown and chmod, run as root, change whatever a symbolic link on the +# path points to, anywhere in the container, and the netwatch user can +# put one in /data. So the start stops unless readlink -f, which +# follows every link on a path, gives /data and DATA_DIR back as they +# are. It also writes a path in full, so a DATA_DIR with '.', '..' or +# an extra '/' in it is refused too. +export DATA_DIR="${DATA_DIR:-/data/reports}" +mkdir -p "$DATA_DIR" || exit 1 +if [ "$(readlink -f /data)" != /data ] || + [ "$(readlink -f "$DATA_DIR")" != "$DATA_DIR" ]; then + echo "entrypoint: DATA_DIR must be a full path with no '.', '..'," \ + "extra '/' or symbolic link on it or on /data, not '$DATA_DIR'" >&2 + exit 1 +fi +chown -R netwatch:netwatch /data "$DATA_DIR" || exit 1 +chmod 750 /data "$DATA_DIR" || exit 1 + +# A stop signal is only noted here; the loop below acts on it. +stop_requested="" +trap 'stop_requested=yes' TERM INT + +# netwatch-server runs as the netwatch user and listens on loopback +# only, on a port other than the public one; nginx.conf proxies to this +# address. Its only client is nginx, so it takes the client address +# nginx passes on from 127.0.0.1 alone, whatever TRUSTED_PROXIES the +# container has. The netwatch user has no login shell, hence -s +# /bin/sh. busybox su replaces itself with the command instead of +# staying on as its parent, so $! is the server's own PID. +BIND_ADDRESS=127.0.0.1 PORT=8081 TRUSTED_PROXIES=127.0.0.1/32 \ + su -s /bin/sh netwatch -c 'exec netwatch-server' & +backend=$! + +# nginx starts through the nginx image's own entrypoint, which applies +# the image's start-up configuration and then replaces itself with +# nginx. Part of that start-up configuration renders nginx.conf into +# conf.d with nginx listening on PORT. NGINX_ENVSUBST_FILTER limits +# that rendering to PORT: a variable nginx itself uses, such as $uri, +# would otherwise be replaced by an environment variable of the same +# name. +NGINX_ENVSUBST_FILTER='^PORT$' \ + /docker-entrypoint.sh nginx -g 'daemon off;' & +nginx=$! + +running() { + kill -0 "$1" 2>/dev/null +} + +# POSIX sh cannot wait for whichever of two children exits first, so +# look once a second. The shell collects a child that has exited while +# it runs sleep, and running() is false for that child from then on. +while [ -z "$stop_requested" ] && running "$backend" && running "$nginx"; do + sleep 1 +done + +# Stop both, then wait until neither is left. +kill -TERM "$backend" "$nginx" 2>/dev/null +while running "$backend" || running "$nginx"; do + sleep 1 +done + +wait "$backend" +backend_status=$? +wait "$nginx" +nginx_status=$? +echo "entrypoint: netwatch-server exited $backend_status," \ + "nginx exited $nginx_status" + +# Success is a requested stop that both processes exited cleanly from. +if [ -n "$stop_requested" ] && [ "$backend_status" -eq 0 ] && + [ "$nginx_status" -eq 0 ]; then + exit 0 +fi +exit 1 diff --git a/nginx.conf b/nginx.conf index cb176d4..3ea963a 100644 --- a/nginx.conf +++ b/nginx.conf @@ -1,14 +1,27 @@ +# A template: the nginx image renders it into conf.d at container start, +# filling in PORT and nothing else. bin/entrypoint.sh sets PORT and that +# limit. server { - listen 8080; + listen ${PORT}; server_name _; + # Keep the nginx version out of the Server header and error pages. + server_tokens off; + + # The security headers, on every response. An add_header in a + # location drops every add_header from here, so a location with one + # of its own includes this file again. + include /etc/nginx/security-headers.conf; + root /usr/share/nginx/html; index index.html; - # Trust RFC1918 reverse proxies for X-Forwarded-For - set_real_ip_from 10.0.0.0/8; - set_real_ip_from 172.16.0.0/12; - set_real_ip_from 192.168.0.0/16; + # 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 + # leaves it empty when TRUSTED_PROXIES is unset, so that by default + # 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; @@ -24,5 +37,35 @@ server { location /assets/ { expires 1y; add_header Cache-Control "public, immutable"; + include /etc/nginx/security-headers.conf; + } + + # netwatch-server, the Go backend, runs in the same container and + # listens on loopback only: bin/entrypoint.sh starts it on + # 127.0.0.1:8081. These headers go with every request passed to it. + # X-Forwarded-For carries only the client address, as resolved by + # the real IP settings above, and not the chain the request came + # with: the backend takes the first entry, which a client can write. + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + + # netwatch-server sets the same security headers on its own + # responses. Its copies are dropped so that each header goes out + # once, as security-headers.conf sets it. + proxy_hide_header Strict-Transport-Security; + proxy_hide_header Content-Security-Policy; + proxy_hide_header X-Frame-Options; + proxy_hide_header X-Content-Type-Options; + proxy_hide_header Referrer-Policy; + proxy_hide_header Permissions-Policy; + + location /api/ { + proxy_pass http://127.0.0.1:8081; + } + + location = /.well-known/healthcheck { + proxy_pass http://127.0.0.1:8081; } } diff --git a/package.json b/package.json index a5e3225..f4cfeaf 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "autoprefixer": "^10.4.23", "postcss": "^8.5.6", "prettier": "^3.8.1", + "puppeteer-core": "25.5.0", "tailwindcss": "^4.1.18", "vite": "^7.3.1" } diff --git a/script/bootstrap b/script/bootstrap index 4df1d8c..d807ee3 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -3,19 +3,40 @@ # this repo. Idempotent: every install is guarded by a check so already # installed tools are skipped. Base tooling comes from nix, apt, brew, # or apk (detected in that order); assumes nothing is present. Node is -# used directly if installed; otherwise it is installed at a pinned -# version via nvm (installing nvm itself first, from a hash-verified -# release archive, never curl | sh). +# used directly if it is at least NODE_MIN_VERSION; otherwise it is +# installed at a pinned version via nvm (installing nvm itself first, +# from a hash-verified release archive, never curl | sh). Go, with its +# gofmt, is used directly if it is at least the version backend/go.mod +# asks for; otherwise the pinned Go release is installed from its +# hash-verified archive. +# +# What this script installs outside the system package manager lives +# under $HOME and is linked into ~/.local/bin, where make and the git +# hook find it once that directory is on PATH. Nothing in ~/.local/bin +# that this script did not create is ever replaced. +# +# golangci-lint is not installed: make lint runs it in Docker, which +# this script does not install either. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" # Pinned versions, 2026-07-07 NODE_VERSION="22.17.0" +# The oldest node the frontend's dependencies accept: the "engines" +# field of puppeteer-core 25.5.0, the most demanding of them, asks for +# 22.12.0 or newer, 2026-09-29. An older installed node is not used. +NODE_MIN_VERSION="22.12.0" NVM_VERSION="0.40.3" # sha256 of https://github.com/nvm-sh/nvm/archive/refs/tags/v0.40.3.tar.gz NVM_SHA256="5f4d6aaa04a177dc93c985e31dbc411ab6b8c6e1e21d8015dbc1372625fcd1d0" YARN_VERSION="1.22.22" +# The Go inside the golang:1.25-alpine image Dockerfile builds the +# backend with, 2026-08-09. The archive hashes are in ensure_go. +GO_VERSION="1.25.7" + +BIN_DIR="$HOME/.local/bin" +TOOLCHAIN="$HOME/.local/share/$("$ROOT/script/projectname")/toolchain" PKGMGR="" SUDO="" @@ -79,6 +100,26 @@ verify_sha256() { fi } +# link_bin : make an installed tool reachable as +# $BIN_DIR/. Only a symlink this script made, one pointing into +# $TOOLCHAIN or ~/.nvm, is ever replaced; if anything else is already +# there, bootstrap stops. +link_bin() { + link="$BIN_DIR/$2" + if [ -L "$link" ] || [ -e "$link" ]; then + case "$(readlink "$link" || true)" in + "$TOOLCHAIN"/* | "$HOME"/.nvm/*) ;; + *) + echo "bootstrap: $link was not created by this script;" >&2 + echo " remove or rename it, then re-run bootstrap" >&2 + exit 1 + ;; + esac + fi + mkdir -p "$BIN_DIR" + ln -sf "$1" "$link" +} + # nvm is a bash script; run a command in a bash with nvm loaded nvm_sh() { bash -c ". \"\$HOME/.nvm/nvm.sh\" && $*" @@ -99,43 +140,135 @@ ensure_nvm() { rm -rf "$tmp" } +# node_ok: the node on PATH is at least NODE_MIN_VERSION. node itself +# compares the two: major, then minor, then patch. +node_ok() { + if missing node; then return 1; fi + node -e ' + const have = process.versions.node.split(".").map(Number); + const want = process.argv[1].split(".").map(Number); + for (let i = 0; i < 3; i++) { + if (have[i] !== want[i]) process.exit(have[i] > want[i] ? 0 : 1); + } + ' "$NODE_MIN_VERSION" +} + +# ensure_node: unless node_ok, install NODE_VERSION and link its node. ensure_node() { - if ! missing node; then return 0; fi + if node_ok; then return 0; fi ensure_nvm nvm_sh "nvm install $NODE_VERSION" + link_bin "$HOME/.nvm/versions/node/v$NODE_VERSION/bin/node" node } +# ensure_yarn: corepack writes its shims (pnpm and yarnpkg as well as +# yarn) into $TOOLCHAIN rather than next to itself, and the npm fallback +# installs there too; only yarn is linked. ensure_yarn() { if ! missing yarn; then return 0; fi + shims="$TOOLCHAIN/corepack-shims" + mkdir -p "$shims" if ! missing corepack; then - corepack enable + corepack enable --install-directory "$shims" corepack prepare "yarn@$YARN_VERSION" --activate elif [ -s "$HOME/.nvm/nvm.sh" ]; then - nvm_sh "nvm use $NODE_VERSION >/dev/null && corepack enable && \ + nvm_sh "nvm use $NODE_VERSION >/dev/null && \ + corepack enable --install-directory \"$shims\" && \ corepack prepare yarn@$YARN_VERSION --activate" else - npm install -g "yarn@$YARN_VERSION" + npm install -g --prefix "$TOOLCHAIN/npm-global" "yarn@$YARN_VERSION" + shims="$TOOLCHAIN/npm-global/bin" fi + link_bin "$shims/yarn" yarn } -install_js_deps() { - if missing yarn && [ -s "$HOME/.nvm/nvm.sh" ]; then - nvm_sh "nvm use $NODE_VERSION >/dev/null && cd \"$ROOT\" && \ - yarn install --frozen-lockfile" - else - yarn install --frozen-lockfile +# go_ok: the go on PATH has its gofmt beside it (a Go release ships the +# two together) and is at least the version backend/go.mod asks for. +# GOTOOLCHAIN=local makes an older go fail here instead of fetching a +# newer toolchain for itself. +go_ok() { + if missing go; then return 1; fi + [ -x "$(dirname "$(command -v go)")/gofmt" ] || return 1 + (cd "$ROOT/backend" && GOTOOLCHAIN=local go list -m >/dev/null 2>&1) +} + +# ensure_go: unless go_ok, install GO_VERSION and link its go and gofmt. +# They are linked on every run that needs them, so a deleted link is put +# back, and the archive is unpacked again if either binary is missing. +ensure_go() { + if go_ok; then return 0; fi + go_dir="$TOOLCHAIN/go-$GO_VERSION" + if [ ! -x "$go_dir/bin/go" ] || [ ! -x "$go_dir/bin/gofmt" ]; then + # sha256 of each archive, from https://go.dev/dl/?mode=json + case "$(uname -s)-$(uname -m)" in + Linux-x86_64) + plat="linux-amd64" + sha="12e6d6a191091ae27dc31f6efc630e3a3b8ba409baf3573d955b196fdf086005" + ;; + Linux-aarch64) + plat="linux-arm64" + sha="ba611a53534135a81067240eff9508cd7e256c560edd5d8c2fef54f083c07129" + ;; + Darwin-x86_64) + plat="darwin-amd64" + sha="bf5050a2152f4053837b886e8d9640c829dbacbc3370f913351eb0904cb706f5" + ;; + Darwin-arm64) + plat="darwin-arm64" + sha="ff18369ffad05c57d5bed888b660b31385f3c913670a83ef557cdfd98ea9ae1b" + ;; + *) + echo "bootstrap: no pinned Go release for this platform" >&2 + exit 1 + ;; + esac + if missing curl; then pkg_install curl curl curl curl; fi + mkdir -p "$TOOLCHAIN" + curl -fsSL -o "$go_dir.tar.gz" \ + "https://go.dev/dl/go$GO_VERSION.$plat.tar.gz" + verify_sha256 "$go_dir.tar.gz" "$sha" + # Unpacked beside its final place and then moved there, so an + # interrupted run never leaves a partial Go that looks complete. + rm -rf "$go_dir.partial" + mkdir "$go_dir.partial" + tar -xzf "$go_dir.tar.gz" -C "$go_dir.partial" --strip-components=1 + rm -rf "$go_dir" "$go_dir.tar.gz" + mv "$go_dir.partial" "$go_dir" fi + link_bin "$go_dir/bin/go" go + link_bin "$go_dir/bin/gofmt" gofmt } main() { cd "$ROOT" + # Tools linked on an earlier run count as installed, and tools linked + # on this run are found by the steps after it. + path_hint="" + case ":$PATH:" in + *":$BIN_DIR:"*) ;; + *) path_hint=yes ;; + esac + PATH="$BIN_DIR:$PATH" + if missing make; then pkg_install gnumake make make make; fi if missing git; then pkg_install git git git git; fi ensure_node ensure_yarn - install_js_deps + yarn install --frozen-lockfile + + ensure_go + (cd "$ROOT/backend" && go mod download) + + if missing docker; then + echo "bootstrap: docker not found; make lint, and so make check" >&2 + echo " and the pre-commit hook, need it to run the Go linter" >&2 + fi + if [ -n "$path_hint" ] && [ -d "$BIN_DIR" ]; then + echo "bootstrap: add $BIN_DIR to the front of your PATH, e.g." >&2 + echo " export PATH=\"\$HOME/.local/bin:\$PATH\"" >&2 + fi echo "bootstrap complete" } diff --git a/script/cibuild b/script/cibuild index 966f51d..688299f 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,13 +1,29 @@ #!/bin/sh -# script/cibuild: run the CI build. The Dockerfile runs make check, so -# a successful build implies all checks pass. +# script/cibuild: run the CI build. It bootstraps first: a CI runner +# checks out and runs this and nothing else, and script/fmt-check runs +# the formatter on the host, which a pristine checkout cannot do. +# --no-cache for the same reason as script/docker: the gate phases the +# final stage depends on are RUN steps, and a cached one is a check that +# did not run. set -eu -ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - docker build . + "$SCRIPT_DIR/bootstrap" + "$SCRIPT_DIR/check" + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. VERSION is computed here because .dockerignore + # excludes .git, so `git describe` in a build stage yields an empty + # version without failing. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$("$SCRIPT_DIR/projectname")" . } main "$@" diff --git a/script/docker b/script/docker index aa9387f..c4688e8 100755 --- a/script/docker +++ b/script/docker @@ -1,6 +1,8 @@ #!/bin/sh # script/docker: build the Docker image tagged with the project name. -# The tag comes from script/projectname. +# Identical in all repos; the tag comes from script/projectname. +# --no-cache because the gate phases the final stage depends on are RUN +# steps, and a cached one is a check that did not run. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" @@ -8,7 +10,16 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - timeout 300 docker build -t "$("$SCRIPT_DIR/projectname")" . + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. VERSION is computed here because .dockerignore + # excludes .git, so `git describe` in a build stage yields an empty + # version without failing. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$("$SCRIPT_DIR/projectname")" . } main "$@" diff --git a/script/fmt b/script/fmt index e7d63e2..7270800 100755 --- a/script/fmt +++ b/script/fmt @@ -1,12 +1,14 @@ #!/bin/sh -# script/fmt: format all files (writes). +# script/fmt: format the whole repo (writes): prettier over everything +# it understands, then gofmt over the Go backend. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - yarn prettier --write . + "$ROOT/script/frontend-fmt" + "$ROOT/backend/script/fmt" } main "$@" diff --git a/script/fmt-check b/script/fmt-check index 07ad5ea..28518f7 100755 --- a/script/fmt-check +++ b/script/fmt-check @@ -1,13 +1,14 @@ #!/bin/sh -# script/fmt-check: check formatting (read-only). Same scope as -# script/fmt, but fails instead of writing. +# script/fmt-check: check formatting across the whole repo (read-only). +# Same scope as script/fmt, but fails instead of writing. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - yarn prettier --check . + "$ROOT/script/frontend-fmt-check" + "$ROOT/backend/script/fmt-check" } main "$@" diff --git a/script/frontend-check b/script/frontend-check new file mode 100755 index 0000000..0819860 --- /dev/null +++ b/script/frontend-check @@ -0,0 +1,18 @@ +#!/bin/sh +# script/frontend-check: run the frontend half of the checks only (test, +# lint, fmt-check). This exists for the frontend stage of Dockerfile, a +# node image with neither Go nor Docker; the Dockerfile's lint and +# backend build stages gate the backend half. Everywhere else, use +# script/check, which covers the whole repo. Must not modify any files. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + "$ROOT/script/frontend-test" + "$ROOT/script/frontend-lint" + "$ROOT/script/frontend-fmt-check" +} + +main "$@" diff --git a/script/frontend-fmt b/script/frontend-fmt new file mode 100755 index 0000000..1af33b9 --- /dev/null +++ b/script/frontend-fmt @@ -0,0 +1,14 @@ +#!/bin/sh +# script/frontend-fmt: format the frontend and every other file prettier +# understands, repo-wide (writes). backend/ is in .prettierignore; Go +# sources are formatted by backend/script/fmt. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + yarn prettier --write . +} + +main "$@" diff --git a/script/frontend-fmt-check b/script/frontend-fmt-check new file mode 100755 index 0000000..1501fce --- /dev/null +++ b/script/frontend-fmt-check @@ -0,0 +1,13 @@ +#!/bin/sh +# script/frontend-fmt-check: check prettier formatting (read-only). Same +# scope as script/frontend-fmt, but fails instead of writing. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + yarn prettier --check . +} + +main "$@" diff --git a/script/frontend-lint b/script/frontend-lint new file mode 100755 index 0000000..12b5b29 --- /dev/null +++ b/script/frontend-lint @@ -0,0 +1,12 @@ +#!/bin/sh +# script/frontend-lint: run the frontend linter (prettier in check mode). +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + yarn prettier --check . +} + +main "$@" diff --git a/script/frontend-test b/script/frontend-test new file mode 100755 index 0000000..4c74c65 --- /dev/null +++ b/script/frontend-test @@ -0,0 +1,14 @@ +#!/bin/sh +# script/frontend-test: run the frontend test suite. The frontend has no +# unit tests; the production build serves as the test (fails on broken +# code). +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + timeout 30 yarn build +} + +main "$@" diff --git a/script/frontend-viewport-test b/script/frontend-viewport-test new file mode 100755 index 0000000..d26e0fe --- /dev/null +++ b/script/frontend-viewport-test @@ -0,0 +1,110 @@ +#!/bin/sh +# script/frontend-viewport-test: verify the responsive layout of the built +# frontend in a real browser engine. +# +# Builds dist/, serves it with the same nginx image and the same nginx.conf +# the shipping container uses, points a containerised headless Chrome at it +# over CDP, and asserts on computed layout at every viewport width derived +# from the app's own CSS. See test/viewport/README.md for what this covers +# and what it cannot. +# +# Deliberately not part of script/check: it needs Docker and takes far +# longer than the 20s budget make test has to stay inside. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +# chromedp/headless-shell 151.0.7922.109, 2026-08-09 +BROWSER_IMAGE="chromedp/headless-shell@sha256:2d349b544a1ea6b5b5fd7c0fe99215ff662339c57407ee2e8c0a11af93516b04" +# nginx:stable-alpine, 2026-02-22 (the digest Dockerfile ships) +SERVER_IMAGE="nginx@sha256:15e96e59aa3b0aada3a121296e3bce117721f42d88f5f64217ef4b18f458c6ab" +# node:22-alpine, 2026-02-22 (the digest Dockerfile builds with) +NODE_IMAGE="node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34" + +RUN_ID="$$-$(date +%s)" +NETWORK="netwatch-viewport-$RUN_ID" +SERVER="netwatch-viewport-server-$RUN_ID" +BROWSER="netwatch-viewport-browser-$RUN_ID" +HARNESS="netwatch-viewport-harness-$RUN_ID" +ARTIFACT_DIR="$ROOT/tmp/viewport" + +# Every container is named and removed here, including the harness itself: +# `timeout` below kills the `docker run` client, not the container it +# started, and an unnamed survivor keeps the --internal network in use so +# `docker network rm` fails too. This host runs many sessions at once and +# neither may be left behind. +cleanup() { + docker rm -f "$HARNESS" > /dev/null 2>&1 || true + docker rm -f "$BROWSER" > /dev/null 2>&1 || true + docker rm -f "$SERVER" > /dev/null 2>&1 || true + docker network rm "$NETWORK" > /dev/null 2>&1 || true +} +trap cleanup EXIT INT TERM + +main() { + cd "$ROOT" + + # Test what ships: the production build, not a dev server. + "$ROOT/script/frontend-test" + if [ ! -f "$ROOT/dist/index.html" ]; then + echo "frontend-viewport-test: dist/index.html missing after build" >&2 + exit 1 + fi + + mkdir -p "$ARTIFACT_DIR" + + # An --internal network has no route off the host, so the browser + # cannot reach the real internet no matter what the page asks for. + # Latency probes are answered by the harness instead. This also means + # no port can be published from it, which is why the harness itself + # runs as a third container on the same network rather than on the + # host. + docker network create --internal "$NETWORK" > /dev/null + + # 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. nginx.conf also includes + # 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$' \ + -v "$ROOT/dist:/usr/share/nginx/html:ro" \ + -v "$ROOT/nginx.conf:/etc/nginx/templates/default.conf.template:ro" \ + -v /dev/null:/etc/nginx/trusted-proxies.conf:ro \ + -v "$ROOT/security-headers.conf:/etc/nginx/security-headers.conf:ro" \ + "$SERVER_IMAGE" > /dev/null + + # The image's own entrypoint already exposes CDP on 9222 and passes + # --no-sandbox, so only extra flags belong here; re-specifying the + # debugging port collides with it and leaves the endpoint bound to + # loopback only. --hide-scrollbars keeps innerWidth equal to + # clientWidth, so the overflow assertion has no scrollbar-sized slack + # to hide behind, and matches the overlay scrollbars phones use. + docker run -d --rm --name "$BROWSER" --init --shm-size=1g \ + --network "$NETWORK" \ + "$BROWSER_IMAGE" \ + --hide-scrollbars \ + > /dev/null + + # Chrome refuses DevTools requests whose Host header is neither + # localhost nor an IP address, so dial the container by address rather + # than by its network alias. + browser_ip="$(docker inspect \ + -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' \ + "$BROWSER")" + + timeout 900 docker run --rm --init --name "$HARNESS" \ + --network "$NETWORK" \ + --user "$(id -u):$(id -g)" \ + -v "$ROOT:/app" \ + -w /app \ + -e NETWATCH_ROOT=/app \ + -e NETWATCH_BASE_URL=http://netwatch:8080 \ + -e "NETWATCH_CDP_URL=http://$browser_ip:9222" \ + -e NETWATCH_ARTIFACT_DIR=/app/tmp/viewport \ + "$NODE_IMAGE" \ + node test/viewport/harness.js +} + +main "$@" diff --git a/script/lint b/script/lint index 054682a..1b3783d 100755 --- a/script/lint +++ b/script/lint @@ -1,12 +1,21 @@ #!/bin/sh -# script/lint: run the linter (prettier in check mode). +# script/lint: lint the whole repo: prettier over the frontend, then the +# Go linter over backend/. +# +# The Go linter runs only in Docker: this builds the lint stage of +# Dockerfile, the digest-pinned golangci-lint image, which runs the +# backend's fmt-check and lint targets. --no-cache makes the linter +# really run every time rather than reuse an earlier result, and the +# stage is built for its checks alone, so no image is kept. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - yarn prettier --check . + "$ROOT/script/frontend-lint" + timeout 300 docker build --no-cache --target lint \ + --output type=cacheonly . } main "$@" diff --git a/script/test b/script/test index 4e98401..f0ad7e8 100755 --- a/script/test +++ b/script/test @@ -1,13 +1,14 @@ #!/bin/sh -# script/test: run the test suite. This repo has no unit tests; the -# production build serves as the test (fails on broken code). +# script/test: run the test suite for the whole repo: the frontend at +# the repo root, then the Go backend in backend/. Both halves together +# get 30 seconds; each also keeps its own limit for the Dockerfiles. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - timeout 30 yarn build + timeout 30 sh -c 'script/frontend-test && backend/script/test' } main "$@" diff --git a/security-headers.conf b/security-headers.conf new file mode 100644 index 0000000..6602d6e --- /dev/null +++ b/security-headers.conf @@ -0,0 +1,24 @@ +# The security headers REPO_POLICIES.md requires on every response. +# nginx.conf includes this file, which Dockerfile copies to +# /etc/nginx/security-headers.conf. always sends each header on error +# responses too. + +add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; + +# Scripts and styles load only from the page's own origin. Inline ones +# are blocked, style attributes in markup included, so style elements +# through classes or element.style. data: images are for the favicon +# in index.html. connect-src is * because the browser checks each probe in +# src/main.js against it, and also every redirect the probe follows, +# and several of those hosts redirect to others; a list of hosts here +# would block those probes. It also covers the reports the page sends +# to its own origin. +add_header Content-Security-Policy "default-src 'self'; connect-src *; img-src 'self' data:; object-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'" always; + +add_header X-Frame-Options DENY always; +add_header X-Content-Type-Options nosniff always; + +# The probed hosts are not told where the page is served from. +add_header Referrer-Policy no-referrer always; + +add_header Permissions-Policy "accelerometer=(), camera=(), display-capture=(), geolocation=(), gyroscope=(), magnetometer=(), microphone=(), midi=(), payment=(), usb=()" always; diff --git a/src/main.js b/src/main.js index 3ab6bb8..526a544 100644 --- a/src/main.js +++ b/src/main.js @@ -7,9 +7,11 @@ import "./styles.css"; // graphMaxLatency — values above it pin to the top of the chart but still // display their real value in the latency figure. The history buffer holds // maxHistoryPoints samples (historyDuration / updateInterval). +// reportInterval is how often collected samples are POSTed to the backend. const CONFIG = { updateInterval: 3000, maxHistoryPoints: 100, + reportInterval: 60000, get historyDuration() { return (this.maxHistoryPoints * this.updateInterval) / 1000; }, @@ -346,6 +348,159 @@ class AppState { } } +// --- Reporting --------------------------------------------------------------- + +// A random UUIDv4. `crypto.randomUUID` exists only in secure contexts +// (HTTPS or localhost); over plain HTTP to any other host — the normal LAN +// deployment — it is undefined, so feature-detect it and otherwise build the +// id from `crypto.getRandomValues`, which is available in insecure contexts. +function randomId() { + if (typeof crypto !== "undefined" && crypto.randomUUID) { + return crypto.randomUUID(); + } + const bytes = new Uint8Array(16); + crypto.getRandomValues(bytes); + bytes[6] = (bytes[6] & 0x0f) | 0x40; // version 4 + bytes[8] = (bytes[8] & 0x3f) | 0x80; // variant 1 + const hex = [...bytes].map((b) => b.toString(16).padStart(2, "0")); + return ( + hex.slice(0, 4).join("") + + "-" + + hex.slice(4, 6).join("") + + "-" + + hex.slice(6, 8).join("") + + "-" + + hex.slice(8, 10).join("") + + "-" + + hex.slice(10, 16).join("") + ); +} + +// A random id identifying this browser across reports. Generated once and +// kept in localStorage; if storage is unavailable (e.g. private mode) a +// fresh id is used for this session only. +function getClientId() { + const key = "netwatch-client-id"; + try { + let id = localStorage.getItem(key); + if (!id) { + id = randomId(); + localStorage.setItem(key, id); + } + return id; + } catch { + return randomId(); + } +} + +// Build the delta report body the backend decodes, plus the new per-host +// high-water marks. Pure function of the passed state: `hosts` is an array +// of { name, url, status, history }, `since` maps a host url to the Unix-ms +// timestamp of the last sample already reported for it, and `now` is a Date. +// Only non-paused samples newer than the mark are included. Returns null +// when no host has an unreported sample. +export function buildReport(hosts, clientId, now, since) { + const reportHosts = []; + const marks = new Map(); + for (const host of hosts) { + const mark = since.get(host.url) ?? 0; + const samples = []; + let high = mark; + for (const p of host.history) { + if (p.paused) continue; + if (p.timestamp <= mark) continue; + samples.push({ + t: p.timestamp, + latency: p.latency, + error: p.error ?? null, + }); + if (p.timestamp > high) high = p.timestamp; + } + if (samples.length === 0) continue; + reportHosts.push({ + name: host.name, + url: host.url, + status: host.status, + history: samples, + }); + marks.set(host.url, high); + } + if (reportHosts.length === 0) return null; + return { + body: { + clientId, + geo: null, + hosts: reportHosts, + timestamp: now.toISOString(), + }, + marks, + }; +} + +// Periodically POSTs unreported samples to the same-origin backend. Holds +// the per-host high-water marks so each report is a delta; marks only +// advance on a delivered report, so a failed POST simply re-sends those +// samples next interval (bounded by the history window — whatever has since +// fallen out is dropped). Failure is quiet: one debug line per outage, one +// on recovery, never an alert, never a tight retry loop. +// +// Only one report is ever in flight, and it is abandoned after half the +// interval, so a slow POST can neither overlap the next report (which would +// carry the same samples) nor stall reporting for good. +class Reporter { + constructor(state, clientId, intervalMs) { + this.state = state; + this.clientId = clientId; + this.intervalMs = intervalMs; + this.marks = new Map(); + this.failing = false; + this.sending = false; + this.timerId = null; + } + + start() { + if (this.timerId) return; + this.timerId = setInterval(() => this.flush(), this.intervalMs); + } + + async flush() { + if (this.sending || this.state.paused) return; + const report = buildReport( + this.state.allHosts, + this.clientId, + new Date(), + this.marks, + ); + if (!report) return; + this.sending = true; + try { + const resp = await fetch("/api/v1/reports", { + method: "POST", + headers: { "Content-Type": "application/json" }, + credentials: "omit", + body: JSON.stringify(report.body), + signal: AbortSignal.timeout(this.intervalMs / 2), + }); + if (!resp.ok) throw new Error(`HTTP ${resp.status}`); + for (const [url, t] of report.marks) { + const current = this.marks.get(url) ?? 0; + this.marks.set(url, Math.max(current, t)); + } + if (this.failing) { + log.debug("Report delivery recovered"); + this.failing = false; + } + } catch (err) { + if (!this.failing) { + log.debug(`Report delivery failed: ${err.message}`); + this.failing = true; + } + } finally { + this.sending = false; + } + } +} + // --- Latency Measurement ----------------------------------------------------- async function measureLatency(url) { @@ -537,6 +692,12 @@ class SparklineRenderer { // --- UI Renderer ------------------------------------------------------------- +// The per-host status line must stay wrappable: its populated content is +// wider than the host column at a 320px viewport, and `whitespace-nowrap` +// here overflows the element and forces the whole document to scroll +// horizontally. +const STATUS_TEXT_CLASS = "status-text text-xs text-right col-span-2 mt-5"; + function hostRowHTML(host, index, showPin = true) { const pinColor = host.pinned ? "text-blue-500" @@ -555,14 +716,14 @@ function hostRowHTML(host, index, showPin = true) { ${pinBtn}
-
+
${host.name}
---
${host.url} -
waiting...
+
waiting...
@@ -671,7 +832,7 @@ function buildUI(state) {

${__COMMIT_HASH__}

-