Backend hardening: missing IdleTimeout/ReadHeaderTimeout, no security headers, no trusted-proxy client IP #19

New Issue

clawbot · 2026-08-09T03:39:16+02:00

clawbot commented

2026-08-09 03:39:16 +02:00

Problem

The Go backend has partial production hardening. Three gaps, all explicitly required by the REPO_POLICIES.md HTTP hardening section. Verified on main at fbfe1df.

1. Two required `http.Server` timeouts are missing

backend/internal/server/http.go:19-25 sets:

s.httpServer = &http.Server{
    Addr:           listenAddr,
    Handler:        s,
    MaxHeaderBytes: maxHeaderBytes,
    ReadTimeout:    readTimeout,
    WriteTimeout:   writeTimeout,
}

ReadTimeout, WriteTimeout, and MaxHeaderBytes are present and correct. ReadHeaderTimeout and IdleTimeout are absent. Policy names both: "ReadTimeout and ReadHeaderTimeout on the http.Server to defend against slowloris attacks" and "IdleTimeout on the http.Server."

Without IdleTimeout, keep-alive connections are never reaped and an attacker can pin server resources with idle sockets.

2. No security-headers middleware

backend/internal/server/routes.go:17-21 registers Recoverer, RequestID, Logging, CORS, Timeout. There is no middleware setting security headers, so every backend response — including /.well-known/healthcheck and /api/v1/reports — ships without HSTS, CSP, X-Content-Type-Options, X-Frame-Options, Referrer-Policy, or Permissions-Policy.

The frontend nginx gap is tracked separately in #18; this is the Go server and needs its own fix.

3. Client IP is taken from `RemoteAddr`, ignoring the reverse proxy

backend/internal/middleware/middleware.go:99 logs:

"remote_ip", ipFromHostPort(r.RemoteAddr),

ipFromHostPort (line 66) just splits host from port. Behind the reverse proxy this service is designed to run behind, every log line will record the proxy's IP, not the client's.

Policy: "True client IP detection when behind a reverse proxy (X-Forwarded-For, X-Real-IP). The application must accept forwarded headers only from a configured set of trusted proxy addresses — never trust X-Forwarded-For unconditionally."

The nginx frontend already does this correctly with set_real_ip_from for RFC1918; the Go server does not do the equivalent.

Definition of done

http.Server sets ReadHeaderTimeout and IdleTimeout alongside the existing timeouts. Values are named constants next to the existing ones in http.go, not magic numbers.
A SecurityHeaders() middleware exists in internal/middleware/ following the established func (s *Middleware) X() func(http.Handler) http.Handler convention, and is registered in SetupRoutes(). It sets HSTS (max-age >= 31536000, includeSubDomains), CSP, X-Frame-Options: DENY, X-Content-Type-Options: nosniff, Referrer-Policy, and Permissions-Policy.
- The backend is a JSON API, not an HTML surface, so a tight default-src 'none'; frame-ancestors 'none' CSP is appropriate here — this endpoint does not have the probe-origin problem that #18 does. Justify whatever you choose in the PR.
- Register it before the CORS middleware so headers are present on preflight responses too; state in the PR that you verified this.
Client IP resolution honours X-Forwarded-For / X-Real-IP only from a configured trusted-proxy allowlist, defaulting to the RFC1918 ranges (10/8, 172.16/12, 192.168/16) to match nginx.conf. The trusted set is configurable via environment variable through the existing viper config in internal/config/config.go, following the established config conventions.
X-Forwarded-For from an untrusted source is ignored, and the log records the direct peer instead. Add a unit test proving both directions: trusted proxy -> forwarded IP is used; untrusted peer -> header is ignored.
The new middleware and IP resolution have unit tests. Given issue #21 (test stubs), do not leave these untested.
cd backend && make check passes; root make check passes.
TODO.md updated in the same commit.
Commit title ends with (closes #N).

Implementation requirements

Follow GO_HTTP_SERVER_CONVENTIONS.md: fx params structs, New factory pattern, slog for logging, viper for config with AutomaticEnv() and a SetDefault for any new setting.
Prefer stdlib and the already-vendored libraries. Do not add a new dependency for trusted-proxy parsing if net/netip and net.ParseCIDR suffice — they do.
Document any new environment variable in backend/README.md.
Do not change the report endpoint's authentication or CORS posture here — that is a separate issue.
make targets only; never raw go invocations.
No attribution trailers in the commit message.

## Problem The Go backend has partial production hardening. Three gaps, all explicitly required by the `REPO_POLICIES.md` HTTP hardening section. Verified on `main` at `fbfe1df`. ### 1. Two required `http.Server` timeouts are missing `backend/internal/server/http.go:19-25` sets: ```go s.httpServer = &http.Server{ Addr: listenAddr, Handler: s, MaxHeaderBytes: maxHeaderBytes, ReadTimeout: readTimeout, WriteTimeout: writeTimeout, } ``` `ReadTimeout`, `WriteTimeout`, and `MaxHeaderBytes` are present and correct. **`ReadHeaderTimeout` and `IdleTimeout` are absent.** Policy names both: "`ReadTimeout` and `ReadHeaderTimeout` on the `http.Server` to defend against slowloris attacks" and "`IdleTimeout` on the `http.Server`." Without `IdleTimeout`, keep-alive connections are never reaped and an attacker can pin server resources with idle sockets. ### 2. No security-headers middleware `backend/internal/server/routes.go:17-21` registers `Recoverer`, `RequestID`, `Logging`, `CORS`, `Timeout`. There is **no middleware setting security headers**, so every backend response — including `/.well-known/healthcheck` and `/api/v1/reports` — ships without HSTS, CSP, `X-Content-Type-Options`, `X-Frame-Options`, `Referrer-Policy`, or `Permissions-Policy`. The frontend nginx gap is tracked separately in #18; this is the Go server and needs its own fix. ### 3. Client IP is taken from `RemoteAddr`, ignoring the reverse proxy `backend/internal/middleware/middleware.go:99` logs: ```go "remote_ip", ipFromHostPort(r.RemoteAddr), ``` `ipFromHostPort` (line 66) just splits host from port. Behind the reverse proxy this service is designed to run behind, **every log line will record the proxy's IP**, not the client's. Policy: "True client IP detection when behind a reverse proxy (`X-Forwarded-For`, `X-Real-IP`). The application must accept forwarded headers only from a configured set of trusted proxy addresses — never trust `X-Forwarded-For` unconditionally." The nginx frontend already does this correctly with `set_real_ip_from` for RFC1918; the Go server does not do the equivalent. ## Definition of done - [ ] `http.Server` sets `ReadHeaderTimeout` and `IdleTimeout` alongside the existing timeouts. Values are named constants next to the existing ones in `http.go`, not magic numbers. - [ ] A `SecurityHeaders()` middleware exists in `internal/middleware/` following the established `func (s *Middleware) X() func(http.Handler) http.Handler` convention, and is registered in `SetupRoutes()`. It sets HSTS (`max-age` >= 31536000, `includeSubDomains`), CSP, `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy`, and `Permissions-Policy`. - The backend is a JSON API, not an HTML surface, so a tight `default-src 'none'; frame-ancestors 'none'` CSP is appropriate here — this endpoint does not have the probe-origin problem that #18 does. Justify whatever you choose in the PR. - Register it **before** the CORS middleware so headers are present on preflight responses too; state in the PR that you verified this. - [ ] Client IP resolution honours `X-Forwarded-For` / `X-Real-IP` **only** from a configured trusted-proxy allowlist, defaulting to the RFC1918 ranges (10/8, 172.16/12, 192.168/16) to match `nginx.conf`. The trusted set is configurable via environment variable through the existing `viper` config in `internal/config/config.go`, following the established config conventions. - [ ] `X-Forwarded-For` from an untrusted source is ignored, and the log records the direct peer instead. Add a unit test proving both directions: trusted proxy -> forwarded IP is used; untrusted peer -> header is ignored. - [ ] The new middleware and IP resolution have unit tests. Given issue #21 (test stubs), do not leave these untested. - [ ] `cd backend && make check` passes; root `make check` passes. - [ ] `TODO.md` updated in the same commit. - [ ] Commit title ends with ` (closes #N)`. ## Implementation requirements - Follow `GO_HTTP_SERVER_CONVENTIONS.md`: fx params structs, `New` factory pattern, `slog` for logging, `viper` for config with `AutomaticEnv()` and a `SetDefault` for any new setting. - Prefer stdlib and the already-vendored libraries. Do not add a new dependency for trusted-proxy parsing if `net/netip` and `net.ParseCIDR` suffice — they do. - Document any new environment variable in `backend/README.md`. - Do not change the report endpoint's authentication or CORS posture here — that is a separate issue. - `make` targets only; never raw `go` invocations. - No attribution trailers in the commit message.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:39:16 +02:00

clawbot referenced this issue

2026-08-09 03:40:06 +02:00

POST /api/v1/reports is unauthenticated, wildcard-CORS, and unrated-limited #20

clawbot referenced this issue

2026-08-09 03:41:43 +02:00

Shutdown is broken: os.Exit races fx OnStop hooks, losing buffered reports on every restart #22

clawbot referenced this issue

2026-08-09 03:43:57 +02:00

Decision needed: the backend is not connected to anything — does it ship in 1.0? #25

Sign in to join this conversation.