2 Commits
Author SHA1 Message Date
clawbot 28d99e89d0 nginx: security headers on every response (closes #18)
check / check (push) Successful in 1m13s
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 07:40:02 +00: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
14 changed files with 149 additions and 40 deletions
+1
View File
@@ -72,6 +72,7 @@ RUN addgroup -g 1000 -S netwatch && \
# conf.d; bin/entrypoint.sh says how. # conf.d; bin/entrypoint.sh says how.
RUN rm /etc/nginx/conf.d/default.conf RUN rm /etc/nginx/conf.d/default.conf
COPY nginx.conf /etc/nginx/templates/netwatch.conf.template 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=frontend /app/dist /usr/share/nginx/html
COPY --from=builder /src/netwatch-server /usr/local/bin/netwatch-server COPY --from=builder /src/netwatch-server /usr/local/bin/netwatch-server
COPY bin/entrypoint.sh /usr/local/bin/entrypoint.sh COPY bin/entrypoint.sh /usr/local/bin/entrypoint.sh
+2
View File
@@ -188,6 +188,8 @@ only inside the container, on `127.0.0.1:8081`. The image:
reverse proxies named in `TRUSTED_PROXIES`, and by default from none reverse proxies named in `TRUSTED_PROXIES`, and by default from none
- Sends access logs to stdout - Sends access logs to stdout
- Caches static assets with immutable headers - 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` - Stores reports in `DATA_DIR`, `/data/reports` by default, on the `/data`
volume. The backend runs as user `netwatch` (uid 1000), so a directory volume. The backend runs as user `netwatch` (uid 1000), so a directory
bind-mounted at `/data` must be writable by uid 1000 bind-mounted at `/data` must be writable by uid 1000
+13
View File
@@ -23,6 +23,19 @@ latest run passes.
# Completed Steps # 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 - 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` 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 (issue #64), and by default from none, where it trusted every RFC1918 address
+3 -1
View File
@@ -104,7 +104,9 @@ 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 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 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 client address nginx passes on and no other. `DATA_DIR` is `/data/reports`, on
the `/data` volume, which `netwatch` owns. 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 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. CIDRs, separated by commas, of the reverse proxies in front of the container.
-3
View File
@@ -2,9 +2,6 @@ package handlers
import "log/slog" 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, // NewForTest builds a Handlers around a report sink and logger,
// bypassing the fx graph so handler behaviour (including the // bypassing the fx graph so handler behaviour (including the
// storage failure path) is exercisable in unit tests. // storage failure path) is exercisable in unit tests.
+4 -18
View File
@@ -5,14 +5,10 @@ import (
"errors" "errors"
"net/http" "net/http"
"sneak.berlin/go/netwatch/internal/logger"
"sneak.berlin/go/netwatch/internal/reportbuf" "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 { type reportSample struct {
T int64 `json:"t"` T int64 `json:"t"`
Latency *int `json:"latency"` 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 // The decoder's error text can quote request bytes (a whole
// oversized number, for example), so it is bounded too. // oversized number, for example), so it is bounded too.
s.log.Error("failed to decode report", s.log.Error("failed to decode report",
"error", boundedForLog(err.Error()), "error", logger.BoundedForLog(err.Error()),
) )
return http.StatusBadRequest return http.StatusBadRequest
@@ -115,20 +111,10 @@ func (s *Handlers) logReportReceived(rpt report) {
} }
s.log.Info("report received", s.log.Info("report received",
"client_id", boundedForLog(rpt.ClientID), "client_id", logger.BoundedForLog(rpt.ClientID),
"timestamp", boundedForLog(rpt.Timestamp), "timestamp", logger.BoundedForLog(rpt.Timestamp),
"host_count", len(rpt.Hosts), "host_count", len(rpt.Hosts),
"total_samples", totalSamples, "total_samples", totalSamples,
"geo_bytes", len(rpt.Geo), "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" "testing"
"sneak.berlin/go/netwatch/internal/handlers" "sneak.berlin/go/netwatch/internal/handlers"
"sneak.berlin/go/netwatch/internal/logger"
"sneak.berlin/go/netwatch/internal/middleware" "sneak.berlin/go/netwatch/internal/middleware"
"sneak.berlin/go/netwatch/internal/reportbuf" "sneak.berlin/go/netwatch/internal/reportbuf"
) )
@@ -174,7 +175,7 @@ func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
func TestHandleReportLogsClientIDCutToBound(t *testing.T) { func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
t.Parallel() t.Parallel()
long := strings.Repeat("c", 2*handlers.MaxLoggedFieldBytes) long := strings.Repeat("c", 2*logger.MaxLoggedFieldBytes)
var logbuf bytes.Buffer var logbuf bytes.Buffer
@@ -197,16 +198,16 @@ func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String()) t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String())
} }
want := long[:handlers.MaxLoggedFieldBytes] want := long[:logger.MaxLoggedFieldBytes]
if logged["client_id"] != want { if logged["client_id"] != want {
t.Fatalf("logged client_id not cut to %d bytes: %q", 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 { if logged["timestamp"] != want {
t.Fatalf("logged timestamp not cut to %d bytes: %q", 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 // A number too large for its int64 field makes the decoder's
// error text quote the whole number. // error text quote the whole number.
huge := strings.Repeat("9", 2*handlers.MaxLoggedFieldBytes) huge := strings.Repeat("9", 2*logger.MaxLoggedFieldBytes)
var logbuf bytes.Buffer var logbuf bytes.Buffer
+15
View File
@@ -11,6 +11,21 @@ import (
"go.uber.org/fx" "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. // Params defines the dependencies for Logger.
type Params struct { type Params struct {
fx.In fx.In
+12 -11
View File
@@ -189,7 +189,10 @@ func addrInAny(s string, trusted []netip.Prefix) bool {
} }
// Logging returns middleware that logs each request with // 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 { func (s *Middleware) Logging() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler {
return http.HandlerFunc( return http.HandlerFunc(
@@ -202,21 +205,19 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
latency := time.Since(start) latency := time.Since(start)
s.log.InfoContext(ctx, "request", s.log.InfoContext(ctx, "request",
"request_start", start, "request_start", start,
"method", r.Method, "method", logger.BoundedForLog(r.Method),
"url", r.URL.String(), "url", logger.BoundedForLog(r.URL.String()),
"useragent", r.UserAgent(), "useragent", logger.BoundedForLog(r.UserAgent()),
"request_id", "request_id",
ctx.Value( logger.BoundedForLog(middleware.GetReqID(ctx)),
middleware.RequestIDKey, "referer", logger.BoundedForLog(r.Referer()),
), "proto", logger.BoundedForLog(r.Proto),
"referer", r.Referer(),
"proto", r.Proto,
"remote_ip", "remote_ip",
clientIP( logger.BoundedForLog(clientIP(
r.RemoteAddr, r.RemoteAddr,
r.Header, r.Header,
s.trustedProxies, s.trustedProxies,
), )),
"status", lrw.statusCode, "status", lrw.statusCode,
"latency_ms", "latency_ms",
latency.Milliseconds(), latency.Milliseconds(),
@@ -13,7 +13,10 @@ import (
"testing/synctest" "testing/synctest"
"time" "time"
"sneak.berlin/go/netwatch/internal/logger"
"sneak.berlin/go/netwatch/internal/middleware" "sneak.berlin/go/netwatch/internal/middleware"
chimiddleware "github.com/go-chi/chi/v5/middleware"
) )
const ( const (
@@ -320,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. // okHandler stands in for the route a middleware guards.
func okHandler() http.Handler { func okHandler() http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+16
View File
@@ -8,6 +8,11 @@ server {
# Keep the nginx version out of the Server header and error pages. # Keep the nginx version out of the Server header and error pages.
server_tokens off; 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; root /usr/share/nginx/html;
index index.html; index index.html;
@@ -32,6 +37,7 @@ server {
location /assets/ { location /assets/ {
expires 1y; expires 1y;
add_header Cache-Control "public, immutable"; add_header Cache-Control "public, immutable";
include /etc/nginx/security-headers.conf;
} }
# netwatch-server, the Go backend, runs in the same container and # netwatch-server, the Go backend, runs in the same container and
@@ -45,6 +51,16 @@ server {
proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-For $remote_addr;
proxy_set_header X-Forwarded-Proto $scheme; 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/ { location /api/ {
proxy_pass http://127.0.0.1:8081; proxy_pass http://127.0.0.1:8081;
} }
+3 -1
View File
@@ -64,13 +64,15 @@ main() {
# nginx.conf is a template: the image renders it over its own # nginx.conf is a template: the image renders it over its own
# default.conf, with the same port and limit bin/entrypoint.sh uses. # default.conf, with the same port and limit bin/entrypoint.sh uses.
# The empty file it includes trusts no proxy, as bin/entrypoint.sh # The empty file it includes trusts no proxy, as bin/entrypoint.sh
# writes it when TRUSTED_PROXIES is unset. # 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" \ docker run -d --rm --name "$SERVER" \
--network "$NETWORK" --network-alias netwatch \ --network "$NETWORK" --network-alias netwatch \
-e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \ -e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \
-v "$ROOT/dist:/usr/share/nginx/html:ro" \ -v "$ROOT/dist:/usr/share/nginx/html:ro" \
-v "$ROOT/nginx.conf:/etc/nginx/templates/default.conf.template:ro" \ -v "$ROOT/nginx.conf:/etc/nginx/templates/default.conf.template:ro" \
-v /dev/null:/etc/nginx/trusted-proxies.conf: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 "$SERVER_IMAGE" > /dev/null
# The image's own entrypoint already exposes CDP on 9222 and passes # 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} ${pinBtn}
<div class="w-[420px] flex-shrink-0 grid grid-cols-[minmax(0,1fr)_auto] items-center"> <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="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> <span class="font-medium text-white truncate">${host.name}</span>
</div> </div>
<div class="latency-value text-4xl font-bold tabular-nums text-right mt-3" data-host="${index}"> <div class="latency-value text-4xl font-bold tabular-nums text-right mt-3" data-host="${index}">