internal/handlers has 4 tests: the 401 signature boundary and every error path are untested #76

New Issue

clawbot · 2026-08-09T03:48:01+02:00

clawbot commented

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

Verified against main at 61f42e6.

internal/handlers has exactly four test functions, all covering ETag/HEAD behavior on HandleImage: TestHandleImage_HEAD_ReturnsHeadersOnly, ..._IfNoneMatch_Returns304, ..._IfNoneMatch_DifferentETag, ..._ETagHeader.

This is the thinnest-tested package in the repo and the one with the most user-visible surface. Untested:

401 on a missing or invalid signature for a non-allowlisted host. This is the product's core security boundary — the thing standing between pixa and being an open proxy — and no Go test exercises it.
400 on an unparseable path or bad fit mode
403 on ErrSSRFBlocked
502 on ErrUpstreamError
410 on an expired signature
HandleRobotsTxt, HandleHealthCheck

Status-code mapping is exactly the kind of logic that silently regresses during refactors, and #39 has more package extraction planned.

Definition of done

Table-driven tests over the error paths above, asserting status code and the JSON error body shape, using the mock fetcher (internal/httpfetcher.MockFetcher) so no network is required.
Explicit positive and negative signature cases: valid signature succeeds; tampered signature, absent signature, and expired signature each produce their documented status. Exercise the exact-match guarantee from #40 at the handler layer, not just in internal/signature.
Allowlisted host without a signature succeeds (the documented bypass) — pinning that the bypass is scoped to allowlisted hosts only.
New tests only; no existing test is modified (no owner approval needed for new tests).
t.Parallel() on each test, matching the convention the canonical lint config enforces.
make check green.

Filed separately: coverage for /v1/e/ and the auth flows, which are also entirely untested in Go.

Verified against `main` at `61f42e6`. `internal/handlers` has exactly four test functions, all covering ETag/HEAD behavior on `HandleImage`: `TestHandleImage_HEAD_ReturnsHeadersOnly`, `..._IfNoneMatch_Returns304`, `..._IfNoneMatch_DifferentETag`, `..._ETagHeader`. This is the thinnest-tested package in the repo and the one with the most user-visible surface. Untested: - **401 on a missing or invalid signature for a non-allowlisted host.** This is the product's core security boundary — the thing standing between pixa and being an open proxy — and no Go test exercises it. - 400 on an unparseable path or bad fit mode - 403 on `ErrSSRFBlocked` - 502 on `ErrUpstreamError` - 410 on an expired signature - `HandleRobotsTxt`, `HandleHealthCheck` Status-code mapping is exactly the kind of logic that silently regresses during refactors, and #39 has more package extraction planned. ## Definition of done 1. Table-driven tests over the error paths above, asserting status code **and** the JSON error body shape, using the mock fetcher (`internal/httpfetcher.MockFetcher`) so no network is required. 2. Explicit positive and negative signature cases: valid signature succeeds; tampered signature, absent signature, and expired signature each produce their documented status. Exercise the exact-match guarantee from #40 at the handler layer, not just in `internal/signature`. 3. Allowlisted host without a signature succeeds (the documented bypass) — pinning that the bypass is scoped to allowlisted hosts only. 4. New tests only; no existing test is modified (no owner approval needed for new tests). 5. `t.Parallel()` on each test, matching the convention the canonical lint config enforces. 6. `make check` green. ## Related Filed separately: coverage for `/v1/e/` and the auth flows, which are also entirely untested in Go.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:48:01 +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#76