From 41558c8d53378e46fbe5acc96540eba6a6f30f42 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 07:43:41 +0000 Subject: [PATCH] Test the image route's signature check and error answers (closes #76) New tests in internal/handlers, with no network: the status and JSON error body the image route answers for a missing, wrong, unpadded, upper-case or expired signature on a host not on the allowlist, or a valid one sent for its parent domain, a sibling host, a subdomain or the host with another domain appended; an unparseable path, localhost as the upstream host, and an upstream error; that an allowlisted host is served without a signature and another host only with a valid one; and the answers of /robots.txt and the health check. An expired signature is answered 401, as the code and README.md say, where the issue body expected 410. No code changes. Model: opus-5-5 --- TODO.md | 9 + .../handlers/image_errors_internal_test.go | 267 ++++++++++++++++++ .../robots_healthcheck_internal_test.go | 90 ++++++ 3 files changed, 366 insertions(+) create mode 100644 internal/handlers/image_errors_internal_test.go create mode 100644 internal/handlers/robots_healthcheck_internal_test.go diff --git a/TODO.md b/TODO.md index 0c4ac74..1b0842a 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,15 @@ P2: security: referer blacklist # Completed Steps +- 2026-10-04 the image route's signature check and error answers are tested + (closes #76): new tests in `internal/handlers`, with no network, check the + status and JSON error body for a missing, wrong, unpadded, upper-case or + expired signature on a host not on the allowlist, or a valid one sent for + its parent domain, a sibling host, a subdomain or the host with another + domain appended (401), an unparseable path (400), `localhost` as the + upstream host (403) and an upstream error (502); that an allowlisted host is + served without a signature, another host only with a valid one; and the + answers of `/robots.txt` and the health check. No code changes. - 2026-10-04 routes, encrypted URLs and config file documented (closes #75): "Routes" in `README.md` lists every route with its method, purpose, what it needs and the status codes it answers with, and says `q` and `fit` are part diff --git a/internal/handlers/image_errors_internal_test.go b/internal/handlers/image_errors_internal_test.go new file mode 100644 index 0000000..6f4cac3 --- /dev/null +++ b/internal/handlers/image_errors_internal_test.go @@ -0,0 +1,267 @@ +package handlers + +import ( + "encoding/json" + "fmt" + "image/color" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + "testing/fstest" + "time" + + "github.com/go-chi/chi/v5" + "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/signature" +) + +// allowlistedHost is the only host on the allowlist of the image route +// newImageRoute builds. +const allowlistedHost = "allowed.example.com" + +// newImageRoute returns the image route of a Handlers whose service fetches +// with fetcher and checks signatures with testSigningKey. +func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler { + t.Helper() + + cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ + StateDir: t.TempDir(), + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, + }) + if err != nil { + t.Fatalf("failed to create cache: %v", err) + } + + svc, err := imgcache.NewService(&imgcache.ServiceConfig{ + Cache: cache, + Fetcher: fetcher, + SigningKey: testSigningKey, + Allowlist: []string{allowlistedHost}, + }) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + h := &Handlers{imgSvc: svc, log: slog.New(slog.DiscardHandler)} + + r := chi.NewRouter() + r.Get("/v1/image/*", h.HandleImage()) + + return r +} + +// newPhotoFetcher returns a mock fetcher that serves a JPEG at photoPath on +// each of hosts, and answers any other URL with an upstream error. +func newPhotoFetcher(t *testing.T, hosts ...string) *httpfetcher.MockFetcher { + t.Helper() + + photo := &fstest.MapFile{ + Data: generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255}), + } + + files := fstest.MapFS{} + for _, host := range hosts { + files[host+photoPath] = photo + } + + return httpfetcher.NewMock(files) +} + +// photoURL returns the image route URL of photoPath on host, as a 50x50 JPEG. +func photoURL(host string) string { + return "/v1/image/" + host + photoPath + "/50x50.jpeg" +} + +// signedPhotoURL returns photoURL(host) with sig and expires as its sig and +// exp. +func signedPhotoURL(host, sig string, expires time.Time) string { + return fmt.Sprintf("%s?sig=%s&exp=%d", photoURL(host), sig, expires.Unix()) +} + +// photoSignature returns the signature of photoURL(host) at the default +// quality and fit, made with key and expiring at expires. +func photoSignature(key, host string, expires time.Time) string { + return signature.New(key).Sign(&signature.Request{ + SourceHost: host, + SourcePath: photoPath, + Width: 50, + Height: 50, + Format: string(imgcache.FormatJPEG), + Quality: 85, + FitMode: string(imgcache.FitCover), + Expires: expires, + }) +} + +// sendGet sends a GET for target to route and returns the response. +func sendGet( + t *testing.T, route http.Handler, target string, +) *httptest.ResponseRecorder { + t.Helper() + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil) + rec := httptest.NewRecorder() + + route.ServeHTTP(rec, req) + t.Logf("GET %s: %d", target, rec.Code) + + return rec +} + +// checkErrorBody checks that rec has status wantStatus and the JSON error body +// the image route sends: wantError, wantStatus and the time in RFC 3339. +func checkErrorBody( + t *testing.T, rec *httptest.ResponseRecorder, wantStatus int, wantError string, +) { + t.Helper() + + if rec.Code != wantStatus { + t.Errorf("status = %d, want %d", rec.Code, wantStatus) + } + + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + + var body struct { + Error string `json:"error"` + Status int `json:"status"` + Timestamp string `json:"timestamp"` + } + + err := json.NewDecoder(rec.Body).Decode(&body) + if err != nil { + t.Fatalf("decoding response body: %v", err) + } + + if body.Error != wantError || body.Status != wantStatus { + t.Errorf("body error and status = %q %d, want %q %d", + body.Error, body.Status, wantError, wantStatus) + } + + _, err = time.Parse(time.RFC3339, body.Timestamp) + if err != nil { + t.Errorf("body timestamp: %v", err) + } +} + +// TestHandleImage_ErrorAnswers checks the status and the JSON error body the +// image route answers each request below with. The JPEG at photoPath exists on +// signedHost and on each host below that differs from it, so a request refused +// with 401 would otherwise be served. +func TestHandleImage_ErrorAnswers(t *testing.T) { + t.Parallel() + + // A signature for signedHost must not verify for any of these. + parentHost := "example.com" + siblingHost := "other.example.com" + subdomainHost := "img." + signedHost + appendedHost := signedHost + ".example.net" + + photos := newPhotoFetcher(t, + signedHost, parentHost, siblingHost, subdomainHost, appendedHost) + // The real fetcher refuses localhost before any lookup or connection. + realFetcher := httpfetcher.New(httpfetcher.DefaultConfig()) + + exp := time.Now().Add(time.Hour) + expired := time.Now().Add(-time.Hour) + sig := photoSignature(testSigningKey, signedHost, exp) + otherKeySig := photoSignature("another-signing-key", signedHost, exp) + expiredSig := photoSignature(testSigningKey, signedHost, expired) + localhostSig := photoSignature(testSigningKey, "localhost", exp) + + // The error every request refused for its signature gets. + const unauthorized = "unauthorized" + + tests := []struct { + name string + fetcher httpfetcher.Fetcher + target string + wantStatus int + wantError string + }{ + {"no sig or exp", photos, photoURL(signedHost), + http.StatusUnauthorized, unauthorized}, + {"exp but no sig", photos, + fmt.Sprintf("%s?exp=%d", photoURL(signedHost), exp.Unix()), + http.StatusUnauthorized, unauthorized}, + {"sig made with another key", photos, + signedPhotoURL(signedHost, otherKeySig, exp), + http.StatusUnauthorized, unauthorized}, + {"sig without its = padding", photos, + signedPhotoURL(signedHost, strings.TrimRight(sig, "="), exp), + http.StatusUnauthorized, unauthorized}, + {"sig in upper case", photos, + signedPhotoURL(signedHost, strings.ToUpper(sig), exp), + http.StatusUnauthorized, unauthorized}, + {"expired sig", photos, signedPhotoURL(signedHost, expiredSig, expired), + http.StatusUnauthorized, unauthorized}, + {"sig sent for the parent domain", photos, + signedPhotoURL(parentHost, sig, exp), + http.StatusUnauthorized, unauthorized}, + {"sig sent for a sibling host", photos, + signedPhotoURL(siblingHost, sig, exp), + http.StatusUnauthorized, unauthorized}, + {"sig sent for a subdomain", photos, + signedPhotoURL(subdomainHost, sig, exp), + http.StatusUnauthorized, unauthorized}, + {"sig sent with another domain appended", photos, + signedPhotoURL(appendedHost, sig, exp), + http.StatusUnauthorized, unauthorized}, + {"unparseable path", photos, + "/v1/image/" + allowlistedHost + photoPath + "/big.jpeg", + http.StatusBadRequest, "invalid image URL: invalid size format"}, + {"blocked upstream address", realFetcher, + signedPhotoURL("localhost", localhostSig, exp), + http.StatusForbidden, "forbidden"}, + {"upstream error", photos, + "/v1/image/" + allowlistedHost + "/images/missing.jpg/50x50.jpeg", + http.StatusBadGateway, "upstream error"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rec := sendGet(t, newImageRoute(t, tt.fetcher), tt.target) + checkErrorBody(t, rec, tt.wantStatus, tt.wantError) + }) + } +} + +// TestHandleImage_AllowlistOrSignature checks that the image route serves an +// image without a signature for a host on the allowlist only, and for another +// host only with a valid signature. +func TestHandleImage_AllowlistOrSignature(t *testing.T) { + t.Parallel() + + photos := newPhotoFetcher(t, allowlistedHost, signedHost) + exp := time.Now().Add(time.Hour) + sig := photoSignature(testSigningKey, signedHost, exp) + + tests := []struct { + name string + target string + wantStatus int + }{ + {"allowlisted host, no sig", photoURL(allowlistedHost), http.StatusOK}, + {"other host, no sig", photoURL(signedHost), http.StatusUnauthorized}, + {"other host, valid sig", signedPhotoURL(signedHost, sig, exp), + http.StatusOK}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rec := sendGet(t, newImageRoute(t, photos), tt.target) + if rec.Code != tt.wantStatus { + t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus) + } + }) + } +} diff --git a/internal/handlers/robots_healthcheck_internal_test.go b/internal/handlers/robots_healthcheck_internal_test.go new file mode 100644 index 0000000..5ef13d5 --- /dev/null +++ b/internal/handlers/robots_healthcheck_internal_test.go @@ -0,0 +1,90 @@ +package handlers + +import ( + "encoding/json" + "log/slog" + "net/http" + "testing" + + "go.uber.org/fx/fxtest" + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/globals" + "sneak.berlin/go/pixa/internal/healthcheck" + "sneak.berlin/go/pixa/internal/logger" +) + +// TestHandleRobotsTxt checks that /robots.txt asks every crawler to stay off +// the whole site. +func TestHandleRobotsTxt(t *testing.T) { + t.Parallel() + + h := &Handlers{log: slog.New(slog.DiscardHandler)} + rec := sendGet(t, h.HandleRobotsTxt(), "/robots.txt") + + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want %d", rec.Code, http.StatusOK) + } + + if ct := rec.Header().Get("Content-Type"); ct != "text/plain" { + t.Errorf("Content-Type = %q, want text/plain", ct) + } + + want := "User-agent: *\nDisallow: /\n" + if rec.Body.String() != want { + t.Errorf("body = %q, want %q", rec.Body.String(), want) + } +} + +// TestHandleHealthCheck checks that the health check answers 200 with status +// ok, the app's name and version, now, uptime_seconds, uptime_human and +// maintenance_mode, which is true here: the health check stays 200 while +// maintenance mode is on. +func TestHandleHealthCheck(t *testing.T) { + t.Parallel() + + lc := fxtest.NewLifecycle(t) + + log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}}) + if err != nil { + t.Fatalf("logger.New() error = %v", err) + } + + hc, err := healthcheck.New(lc, healthcheck.Params{ + Globals: &globals.Globals{Appname: "pixad", Version: "v1.2.3"}, + Config: &config.Config{MaintenanceMode: true}, + Logger: log, + }) + if err != nil { + t.Fatalf("healthcheck.New() error = %v", err) + } + + h := &Handlers{hc: hc, log: slog.New(slog.DiscardHandler)} + rec := sendGet(t, h.HandleHealthCheck(), "/.well-known/healthcheck.json") + + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want %d", rec.Code, http.StatusOK) + } + + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + + var body map[string]any + + err = json.NewDecoder(rec.Body).Decode(&body) + if err != nil { + t.Fatalf("decoding response body: %v", err) + } + + if body["status"] != "ok" || body["appname"] != "pixad" || + body["version"] != "v1.2.3" || body["maintenance_mode"] != true { + t.Errorf("body = %v, want status ok, appname pixad, version v1.2.3 "+ + "and maintenance_mode true", body) + } + + for _, key := range []string{"now", "uptime_seconds", "uptime_human"} { + if _, ok := body[key]; !ok { + t.Errorf("body = %v, has no %s", body, key) + } + } +}