sneak/pixa
1
0
Fork 0
Code Issues 11 Pull Requests 4 Actions 11 Packages Projects Releases Wiki Activity

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

New Issue
Closed
opened 2026-08-09 03:48:12 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 03:48:12 +02:00
Collaborator
Copy Link
Copy Source

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.

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 commented 2026-10-04 14:58:45 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan, checked against next at 604b51e. Since this issue was filed, internal/encurl, internal/seal and internal/session got package tests (expired, tampered, wrong-key tokens; cookie attributes), and internal/handlers got tests for the CSRF check, generator field limits, /v1/e/ ETag, 304, HEAD and bad fit or size, and the login log. Still untested at the handler layer, and what this unit adds (new tests only, t.Parallel() where allowed, the mock fetcher, no network):

  1. /v1/e/: a valid token serves the image; an expired token answers 410; a corrupt or truncated token answers 400; a token sealed with another signing key is refused (with whatever status README.md documents for it).
  2. Login: GET / shows the login form; a wrong key shows the form again with an error and sets no session cookie; the right key sets a session cookie, with its Secure, HttpOnly and SameSite attributes checked one by one, as the handler sends it; GET /logout clears the cookie (Max-Age 0 or an expiry in the past, as the code does it); POST /generate without a session is refused as README.md says.
  3. Round trip, in-process: POST /generate with a session makes a URL whose token /v1/e/ accepts and serves.

Where the code answers differently from README.md, the test pins the code and the PR names the difference. scripts/manual-test.sh is left to #97.

Model: opus-5-5

Plan, checked against `next` at `604b51e`. Since this issue was filed, `internal/encurl`, `internal/seal` and `internal/session` got package tests (expired, tampered, wrong-key tokens; cookie attributes), and `internal/handlers` got tests for the CSRF check, generator field limits, `/v1/e/` `ETag`, 304, `HEAD` and bad `fit` or size, and the login log. Still untested at the handler layer, and what this unit adds (new tests only, `t.Parallel()` where allowed, the mock fetcher, no network): 1. `/v1/e/`: a valid token serves the image; an expired token answers 410; a corrupt or truncated token answers 400; a token sealed with another signing key is refused (with whatever status `README.md` documents for it). 2. Login: `GET /` shows the login form; a wrong key shows the form again with an error and sets no session cookie; the right key sets a session cookie, with its `Secure`, `HttpOnly` and `SameSite` attributes checked one by one, as the handler sends it; `GET /logout` clears the cookie (`Max-Age` 0 or an expiry in the past, as the code does it); `POST /generate` without a session is refused as `README.md` says. 3. Round trip, in-process: `POST /generate` with a session makes a URL whose token `/v1/e/` accepts and serves. Where the code answers differently from `README.md`, the test pins the code and the PR names the difference. `scripts/manual-test.sh` is left to https://git.eeqj.de/sneak/pixa/issues/97. Model: opus-5-5
clawbot commented 2026-10-04 17:28:30 +02:00
Author
Collaborator
Copy Link
Copy Source

Built as planned in #191: new tests only; the code answers as README.md says everywhere they look.

Model: opus-5-5

Built as planned in https://git.eeqj.de/sneak/pixa/pulls/191: new tests only; the code answers as `README.md` says everywhere they look. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#77