internal/middleware: CORS, logging, and the metrics auth gate are untested #79

New Issue

2026-08-09T03:48:37+02:00

clawbot commented

2026-08-09 03:48:37 +02:00

Verified against main at 61f42e6.

internal/middleware has two tests, both on security headers: TestSecurityHeaders and TestSecurityHeaders_PreservesExistingHeaders.

Untested:

MetricsAuth (middleware.go:135-144) — the basic-auth gate on /metrics. This is an access control check with no test. Metrics can leak upstream hostnames, traffic volumes, and cache behavior; if this gate regresses, nothing catches it.
CORS (middleware.go:114) — currently hardcoded to []string{"*"}. Becomes configurable via the access_control_allow_origin work; that change should land on top of a test that pins current behavior first.
Logging (middleware.go:82-109) — including that it does not log secrets. Worth an explicit assertion that the signing key never appears in a log line, given it can arrive as a form value on POST /.
Metrics (middleware.go:124-132).

Definition of done

MetricsAuth tests: no credentials returns 401 with a WWW-Authenticate challenge; wrong credentials returns 401; correct credentials passes through. Also assert the comparison is constant-time if it is not already (and make it so if not — a timing-attackable metrics password is a small but free win).
CORS tests pinning the current * behavior, written so they extend naturally when the config key lands.
A logging test asserting that a POST / body containing the signing key does not put that value into the log output.
A test that the metrics recorder middleware is installed when expected — see the separate issue about metrics only being collected when credentials are configured.
New tests only. make check green.

Verified against `main` at `61f42e6`. `internal/middleware` has two tests, both on security headers: `TestSecurityHeaders` and `TestSecurityHeaders_PreservesExistingHeaders`. Untested: - **`MetricsAuth` (`middleware.go:135-144`)** — the basic-auth gate on `/metrics`. This is an access control check with no test. Metrics can leak upstream hostnames, traffic volumes, and cache behavior; if this gate regresses, nothing catches it. - **`CORS` (`middleware.go:114`)** — currently hardcoded to `[]string{"*"}`. Becomes configurable via the `access_control_allow_origin` work; that change should land on top of a test that pins current behavior first. - **`Logging` (`middleware.go:82-109`)** — including that it does not log secrets. Worth an explicit assertion that the signing key never appears in a log line, given it can arrive as a form value on `POST /`. - **`Metrics` (`middleware.go:124-132`)**. ## Definition of done 1. `MetricsAuth` tests: no credentials returns 401 with a `WWW-Authenticate` challenge; wrong credentials returns 401; correct credentials passes through. Also assert the comparison is constant-time if it is not already (and make it so if not — a timing-attackable metrics password is a small but free win). 2. CORS tests pinning the current `*` behavior, written so they extend naturally when the config key lands. 3. A logging test asserting that a `POST /` body containing the signing key does not put that value into the log output. 4. A test that the metrics recorder middleware is installed when expected — see the separate issue about metrics only being collected when credentials are configured. 5. New tests only. `make check` green.

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

clawbot referenced this issue

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

Tracking: 1.0.0 sequencing and critical path #103

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/pixa#79