The auth UI and encrypted-URL flows have zero Go tests — only a manual shell script #77

New Issue

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

clawbot commented

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

Verified against main at 61f42e6.

Neither of these has a single Go test:

HandleImageEnc (internal/handlers/imageenc.go) — the /v1/e/{token} encrypted-URL route
HandleRoot, handleLoginPost, HandleLogout, HandleGenerateURL (internal/handlers/auth.go) — the whole login, session, and URL-generation surface

The only verification these flows have ever received is the manual pass recorded in TODO.md under 2026-08-07 (closing #49) and scripts/manual-test.sh. A manual pass against one build is not a regression test: it verified the code that existed that day and cannot fail in CI when someone breaks it tomorrow.

The code under test is cryptographic — CBOR + NaCl secretbox + HKDF across internal/encurl, internal/seal, and internal/session. A silent failure here is not a 500; it is either a token that should not validate validating, or session cookies that do not do what they claim. The recent #47 fix (Secure/HttpOnly/SameSite on session cookies) is exactly the kind of property that should have a test asserting it forever.

Definition of done

/v1/e/ tests: a valid token serves the image; an expired token returns 410; a corrupt or truncated token returns 400; a token sealed with a different key does not validate.
Auth tests: GET / renders the login form; a wrong key re-renders with an error and sets no session cookie; the correct key sets a session cookie carrying Secure, HttpOnly, and SameSite=Strict (pinning #47); GET /logout clears the cookie with Max-Age=0; an unauthenticated POST /generate is rejected.
POST /generate produces a token that the /v1/e/ route accepts — round-trip, in-process.
Assert cookie attributes explicitly rather than only checking that a cookie exists.
New tests only; no existing test modified. t.Parallel() where the test does not use t.Setenv/t.Chdir.
make check green.

Note

Once these exist, the manual checklist in TODO.md becomes a starting point for automation rather than the primary evidence. Consider whether scripts/manual-test.sh still earns its place afterwards, or should be reduced to the parts that genuinely need a running daemon.

Verified against `main` at `61f42e6`. Neither of these has a single Go test: - `HandleImageEnc` (`internal/handlers/imageenc.go`) — the `/v1/e/{token}` encrypted-URL route - `HandleRoot`, `handleLoginPost`, `HandleLogout`, `HandleGenerateURL` (`internal/handlers/auth.go`) — the whole login, session, and URL-generation surface The only verification these flows have ever received is the manual pass recorded in `TODO.md` under 2026-08-07 (closing #49) and `scripts/manual-test.sh`. A manual pass against one build is not a regression test: it verified the code that existed that day and cannot fail in CI when someone breaks it tomorrow. The code under test is cryptographic — CBOR + NaCl secretbox + HKDF across `internal/encurl`, `internal/seal`, and `internal/session`. A silent failure here is not a 500; it is either a token that should not validate validating, or session cookies that do not do what they claim. The recent #47 fix (Secure/HttpOnly/SameSite on session cookies) is exactly the kind of property that should have a test asserting it forever. ## Definition of done 1. `/v1/e/` tests: a valid token serves the image; an expired token returns 410; a corrupt or truncated token returns 400; a token sealed with a different key does not validate. 2. Auth tests: `GET /` renders the login form; a wrong key re-renders with an error and sets no session cookie; the correct key sets a session cookie carrying `Secure`, `HttpOnly`, and `SameSite=Strict` (pinning #47); `GET /logout` clears the cookie with `Max-Age=0`; an unauthenticated `POST /generate` is rejected. 3. `POST /generate` produces a token that the `/v1/e/` route accepts — round-trip, in-process. 4. Assert cookie attributes explicitly rather than only checking that a cookie exists. 5. New tests only; no existing test modified. `t.Parallel()` where the test does not use `t.Setenv`/`t.Chdir`. 6. `make check` green. ## Note Once these exist, the manual checklist in `TODO.md` becomes a starting point for automation rather than the primary evidence. Consider whether `scripts/manual-test.sh` still earns its place afterwards, or should be reduced to the parts that genuinely need a running daemon.

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