5 Commits
Author SHA1 Message Date
clawbot d81da05748 nginx: security headers on every response (closes #18)
check / check (push) Successful in 21s
nginx sent none of the security headers REPO_POLICIES.md requires.
security-headers.conf now sets all six with always, included at server
level and again in /assets/, whose own add_header would otherwise drop
them. nginx hides the copies netwatch-server sets, so /api/ and the
health check carry each header once. The content security policy
allows no inline script or style; the host row's status dot took its
grey from a style attribute, now a class. connect-src is * because
several probed hosts redirect to other hosts and the browser checks
every redirect against it. Referrer-Policy is no-referrer, as the
backend already sends.

Model: opus-5-5
2026-09-29 10:22:12 +02:00
clawbot d74d1e311e fix(backend): cut request log fields to the log bound (closes #60)
check / check (push) Successful in 14s
The request log wrote the URL, User-Agent, Referer and other
request-supplied strings with no length limit, and the server accepts
headers up to 1 MiB, so one request could put about 1 MiB per field
into a log line. Every string the request log takes from the request,
including the request ID chi copies from X-Request-Id, is now cut to
the 128-byte bound the report handler already used. That bound and
its helper moved from the handlers package to the logger package so
both use the one copy.

Model: opus-5-5
2026-09-29 09:39:11 +02:00
clawbot 8833603eff nginx: trust X-Forwarded-For only from TRUSTED_PROXIES (closes #64)
check / check (push) Successful in 15s
nginx trusted X-Forwarded-For from every RFC1918 address, so a client
reaching it from one could write a new address on each request and
get a fresh rate-limit allowance. The container's TRUSTED_PROXIES now
names the reverse proxies nginx trusts, none by default.
bin/entrypoint.sh makes each entry a CIDR, checks it with the new
"netwatch-server check-cidr", which runs the server's own
TRUSTED_PROXIES parsing, and writes one set_real_ip_from line per
entry into /etc/nginx/trusted-proxies.conf, which nginx.conf includes.
The backend is started with TRUSTED_PROXIES=127.0.0.1/32, since nginx
is its only client. The viewport test mounts an empty file there.

Model: opus-5-5
2026-09-29 08:55:47 +02:00
clawbot 6022cc8b02 fix(backend): give each report file a name of its own (closes #61)
check / check (push) Successful in 13s
Report files were named by a millisecond timestamp and created with
O_EXCL, so two flushes in the same millisecond, such as a flush for
size and the final flush at shutdown, got the same name and the second
failed, losing its reports. Each name now carries a number after the
timestamp that goes up by one for each file the server starts to
write, so names still sort by time and never repeat within a run. 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.

Model: opus-5-5
2026-09-29 08:05:26 +02:00
clawbot d2f219ca19 upaas: health check, settings checked at start, README section (closes #59)
check / check (push) Successful in 15s
The image's HEALTHCHECK requests /.well-known/healthcheck through
nginx on the port from PORT, so it fails unless both processes answer.
The backend reads PORT and DEBUG with strconv instead of viper, which
turned a bad PORT into 0 and a bad DEBUG into false. Those, and a
BIND_ADDRESS that is not an IP address, now stop the start with an
error naming the variable; the TRUSTED_PROXIES error names it too.
bin/entrypoint.sh also refuses a container PORT outside 1 to 65535,
or 8081, where the backend listens, naming PORT. README.md gains
"Running under upaas". Its first-run steps create the host directory
owned by uid 1000, so the image changes no ownership.

Model: opus-5-5
2026-09-29 06:39:10 +02:00
22 changed files with 571 additions and 102 deletions
+7
View File
@@ -72,6 +72,7 @@ RUN addgroup -g 1000 -S netwatch && \
# 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
@@ -83,6 +84,12 @@ VOLUME /data
# The default public port; PORT changes it.
EXPOSE 8080
# 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
+50 -2
View File
@@ -184,16 +184,64 @@ container: nginx serves the built frontend and passes `/api/` and
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. The backend runs as user `netwatch` (uid 1000), so a directory
bind-mounted at `/data` must be writable 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`.
- **First run:** upaas bind-mounts the host directory it is given and does not
create it, and the backend, which runs as uid 1000, does not start unless it
can write there. Create the directory, owned by uid 1000, before the first
deploy:
```bash
mkdir -p /path/to/data
chown 1000:1000 /path/to/data
```
- **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
Requires a modern browser with ES modules, Fetch API, Canvas API, and CSS custom
+39
View File
@@ -23,6 +23,45 @@ latest run passes.
# Completed Steps
- 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-<timestamp>-<number>.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`
+31 -8
View File
@@ -87,9 +87,14 @@ Internal packages in `internal/` follow standard Go project layout:
| `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 the reverse proxy that shares the container; the
RFC1918 ranges match `nginx.conf`. A request whose direct peer is outside this
set has its forwarded headers ignored, and the direct peer is logged instead.
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
@@ -97,14 +102,32 @@ 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. `DATA_DIR` is `/data/reports`, on the `/data` volume,
which `netwatch` owns.
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, which `netwatch` owns. 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-<timestamp>.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-<timestamp>-<number>.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
+16
View File
@@ -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
+56 -20
View File
@@ -6,7 +6,10 @@ import (
"errors"
"fmt"
"log/slog"
"math"
"net/netip"
"net/url"
"strconv"
"strings"
"sneak.berlin/go/netwatch/internal/globals"
@@ -18,10 +21,11 @@ import (
)
// defaultTrustedProxies lists the networks whose forwarded
// headers are honoured by default. It covers the RFC1918
// ranges (to match nginx.conf) plus IPv4 and IPv6 loopback,
// because the reverse proxy shares the container and reaches
// the backend over loopback.
// 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"
@@ -37,6 +41,9 @@ var (
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.
@@ -65,7 +72,8 @@ type Config struct {
}
// 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,
@@ -103,15 +111,29 @@ 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{
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: viper.GetBool("DEBUG"),
Debug: debug,
MetricsPassword: viper.GetString("METRICS_PASSWORD"),
MetricsUsername: viper.GetString("METRICS_USERNAME"),
Port: viper.GetInt("PORT"),
Port: port,
ReportsPerMinute: viper.GetInt("REPORTS_PER_MINUTE"),
SentryDSN: viper.GetString("SENTRY_DSN"),
TrustedProxies: splitList(viper.GetString("TRUSTED_PROXIES")),
@@ -119,19 +141,7 @@ func New(
params: &params,
}
// viper reads a value that is not a number as 0, so this also
// catches a mistyped setting.
if s.ReportsPerMinute <= 0 {
return nil, fmt.Errorf("REPORTS_PER_MINUTE %q: %w",
viper.GetString("REPORTS_PER_MINUTE"), errNotPositive)
}
if s.DataDirMaxBytes <= 0 {
return nil, fmt.Errorf("DATA_DIR_MAX_BYTES %q: %w",
viper.GetString("DATA_DIR_MAX_BYTES"), errNotPositive)
}
err = checkOrigins(s.CORSAllowedOrigins)
err = s.check()
if err != nil {
return nil, err
}
@@ -144,6 +154,32 @@ 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.
+57
View File
@@ -29,6 +29,63 @@ func requireConfigError(t *testing.T, setting string) {
}
}
// 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.
-3
View File
@@ -2,9 +2,6 @@ package handlers
import "log/slog"
// MaxLoggedFieldBytes exposes the log bound to the external tests.
const MaxLoggedFieldBytes = maxLoggedFieldBytes
// 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.
+4 -18
View File
@@ -5,14 +5,10 @@ import (
"errors"
"net/http"
"sneak.berlin/go/netwatch/internal/logger"
"sneak.berlin/go/netwatch/internal/reportbuf"
)
// maxLoggedFieldBytes bounds untrusted text (string fields,
// decode error text) before it is logged, so a caller cannot
// inflate log volume with an oversized value.
const maxLoggedFieldBytes = 128
type reportSample struct {
T int64 `json:"t"`
Latency *int `json:"latency"`
@@ -83,7 +79,7 @@ func (s *Handlers) decodeErrorStatus(err error) int {
// 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", boundedForLog(err.Error()),
"error", logger.BoundedForLog(err.Error()),
)
return http.StatusBadRequest
@@ -115,20 +111,10 @@ func (s *Handlers) logReportReceived(rpt report) {
}
s.log.Info("report received",
"client_id", boundedForLog(rpt.ClientID),
"timestamp", boundedForLog(rpt.Timestamp),
"client_id", logger.BoundedForLog(rpt.ClientID),
"timestamp", logger.BoundedForLog(rpt.Timestamp),
"host_count", len(rpt.Hosts),
"total_samples", totalSamples,
"geo_bytes", len(rpt.Geo),
)
}
// 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
}
+6 -5
View File
@@ -12,6 +12,7 @@ import (
"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"
)
@@ -174,7 +175,7 @@ func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
t.Parallel()
long := strings.Repeat("c", 2*handlers.MaxLoggedFieldBytes)
long := strings.Repeat("c", 2*logger.MaxLoggedFieldBytes)
var logbuf bytes.Buffer
@@ -197,16 +198,16 @@ func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String())
}
want := long[:handlers.MaxLoggedFieldBytes]
want := long[:logger.MaxLoggedFieldBytes]
if logged["client_id"] != want {
t.Fatalf("logged client_id not cut to %d bytes: %q",
handlers.MaxLoggedFieldBytes, logged["client_id"])
logger.MaxLoggedFieldBytes, logged["client_id"])
}
if logged["timestamp"] != want {
t.Fatalf("logged timestamp not cut to %d bytes: %q",
handlers.MaxLoggedFieldBytes, logged["timestamp"])
logger.MaxLoggedFieldBytes, logged["timestamp"])
}
}
@@ -215,7 +216,7 @@ func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) {
// A number too large for its int64 field makes the decoder's
// error text quote the whole number.
huge := strings.Repeat("9", 2*handlers.MaxLoggedFieldBytes)
huge := strings.Repeat("9", 2*logger.MaxLoggedFieldBytes)
var logbuf bytes.Buffer
+15
View File
@@ -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
@@ -29,7 +29,3 @@ func ClientIP(
) string {
return clientIP(remoteAddr, header, trusted)
}
func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
return parseTrustedProxies(cidrs)
}
+19 -16
View File
@@ -64,7 +64,7 @@ func New(
_ fx.Lifecycle,
params Params,
) (*Middleware, error) {
trusted, err := parseTrustedProxies(params.Config.TrustedProxies)
trusted, err := ParseTrustedProxies(params.Config.TrustedProxies)
if err != nil {
return nil, err
}
@@ -77,16 +77,18 @@ func New(
return s, nil
}
// parseTrustedProxies converts CIDR strings into prefixes,
// failing fast on any malformed entry.
func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
// 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 proxy %q: %w", cidr, err,
"TRUSTED_PROXIES %q: %w", cidr, err,
)
}
@@ -187,7 +189,10 @@ func addrInAny(s string, trusted []netip.Prefix) bool {
}
// 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(
@@ -200,21 +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",
clientIP(
logger.BoundedForLog(clientIP(
r.RemoteAddr,
r.Header,
s.trustedProxies,
),
)),
"status", lrw.statusCode,
"latency_ms",
latency.Milliseconds(),
+69 -3
View File
@@ -13,7 +13,10 @@ import (
"testing/synctest"
"time"
"sneak.berlin/go/netwatch/internal/logger"
"sneak.berlin/go/netwatch/internal/middleware"
chimiddleware "github.com/go-chi/chi/v5/middleware"
)
const (
@@ -36,15 +39,32 @@ func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
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()
_, err := middleware.ParseTrustedProxies([]string{"not-a-cidr"})
if err == nil {
t.Fatal("expected error for malformed CIDR, got nil")
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
@@ -303,6 +323,52 @@ func TestRecovererRepanicsOnAbortHandler(t *testing.T) {
}
}
// 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) {
@@ -1,7 +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 }
}
+16 -4
View File
@@ -14,6 +14,7 @@ import (
"path/filepath"
"strings"
"sync"
"sync/atomic"
"time"
"sneak.berlin/go/netwatch/internal/config"
@@ -30,7 +31,8 @@ const (
dirPerms fs.FileMode = 0o750
filePerms fs.FileMode = 0o640
// Report files are named filePrefix + timestamp + fileSuffix.
// Report files are named filePrefix + timestamp + "-" + number +
// fileSuffix; see writeFile.
filePrefix = "reports-"
fileSuffix = ".jsonl.zst"
)
@@ -56,6 +58,12 @@ type Buffer 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
@@ -79,6 +87,7 @@ func New(
done: make(chan struct{}),
log: params.Logger.Get(),
maxBytes: params.Config.DataDirMaxBytes,
now: time.Now,
}
lc.Append(fx.Hook{
@@ -211,11 +220,14 @@ func (b *Buffer) drainBuf() []byte {
// writeFile creates a timestamped zstd-compressed JSONL file
// in the data directory.
func (b *Buffer) writeFile(data []byte) error {
ts := time.Now().UTC().Format("2006-01-02T15-04-05.000Z")
path := filepath.Join(b.dataDir, filePrefix+ts+fileSuffix)
// 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)
// path is built from the operator-supplied dataDir plus a
// generated timestamp, so it carries no external input.
// 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,
+77 -8
View File
@@ -3,9 +3,11 @@ package reportbuf_test
import (
"encoding/json"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"slices"
"strconv"
"strings"
"sync"
@@ -18,6 +20,7 @@ import (
"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"
)
@@ -210,10 +213,6 @@ func TestWrittenReportsCountAtFileSize(t *testing.T) {
t.Fatalf("flush: %v", err)
}
// The second report is written at shutdown, and must not land in
// the first file's millisecond (see TestWrittenReportsKeepCounting).
time.Sleep(time.Millisecond)
err = buf.Append(report)
if err != nil {
t.Fatalf("second report, after the first was written: %v", err)
@@ -254,10 +253,6 @@ func TestWrittenReportsKeepCounting(t *testing.T) {
t.Fatalf("with %d bytes of report files: %v", used, err)
}
// Report files are named to the millisecond; two in the same
// one collide (https://git.eeqj.de/sneak/netwatch/issues/61).
time.Sleep(time.Millisecond)
err = buf.Flush()
if err != nil {
t.Fatalf("flush: %v", err)
@@ -315,6 +310,43 @@ func TestConcurrentAppendsStopAtCap(t *testing.T) {
}
}
// 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()
@@ -338,6 +370,43 @@ func reportFilesBytes(t *testing.T, dir string) int64 {
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()
+49 -6
View File
@@ -10,8 +10,9 @@ 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, so anything but digits stops the container
# here, before either process starts.
# 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]*)
@@ -19,6 +20,46 @@ case "$PORT" in
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
# A stop signal is only noted here; the loop below acts on it.
stop_requested=""
@@ -26,10 +67,12 @@ 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. 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 \
# 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=$!
+22 -4
View File
@@ -8,13 +8,20 @@ server {
# 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;
@@ -30,6 +37,7 @@ 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
@@ -43,6 +51,16 @@ server {
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;
}
+5
View File
@@ -63,11 +63,16 @@ main() {
# 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
+24
View File
@@ -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;
+1 -1
View File
@@ -716,7 +716,7 @@ function hostRowHTML(host, index, showPin = true) {
${pinBtn}
<div class="w-[420px] flex-shrink-0 grid grid-cols-[minmax(0,1fr)_auto] items-center">
<div class="flex items-center gap-2 min-w-[200px]">
<div class="w-3 h-3 rounded-full flex-shrink-0" style="background-color: ${latencyHex(null)}"></div>
<div class="w-3 h-3 rounded-full flex-shrink-0 bg-[#6b7280]"></div>
<span class="font-medium text-white truncate">${host.name}</span>
</div>
<div class="latency-value text-4xl font-bold tabular-nums text-right mt-3" data-host="${index}">