Service.Warm, the ImageCache interface, both GenerateSignedURL methods,
signature.ParseParams, imagerequest.ParseImagePath, four symbols of
internal/magic, the image processor's lists of supported formats,
IsEmpty and Count of the allowlist, ssrfSafeDialer, CacheStale,
ErrCacheMiss, ErrNegativeCache, ImageResponse.LastModified and the Load
methods of ContentStorage and VariantStorage had no caller outside
tests. They are gone, with the tests that tested only them.
The handlers tests sign their image URLs with generateSignedURL, a test
helper that builds the URL as the removed generator did. The SSRF dialer
tests call dialSSRFSafe with no operator blocklist, which is all
ssrfSafeDialer did. The tests of ParseImagePath and ContentStorage.Load
call ParseImageURL and LoadWithSize instead. MIMEToImageFormat,
ContentStorage.Store and CleanExpired stay, as pixa calls them.
Model: opus-5-5
The new package internal/imagerequest holds the request type, its two
parts Source and Transform, the Format and Fit types, the dimension and
fit checks, the /v1/image/ path parsing and the variant key; it imports
no other package of pixa. imgcache.ParsedURL, imgcache.ImageRequest,
signature.Request, imageprocessor.Request, the second Size, format and
fit types and the conversions between them are gone. The image
processor takes a Transform only. encurl keeps a private struct with the
token's short CBOR field names. VariantKey moves with the function that
makes it. Tests follow the moved and renamed code; the tests of the
removed conversions go with them.
Model: opus-5-5
The README gave no way to put a source URL's query into a signed image
URL, and GenerateSignedURL built one that failed: the route parsed
chi's wildcard, which keeps the path as the client wrote it, so one
%2F, %3B or %2C left the %3F undecoded and the signature failed. The
route now parses the decoded path, GenerateSignedURL writes Go's own
encoding of it, and the README says the ? before the query, any ? in it
and every % must be percent-encoded, with a signed example. The path
traversal check no longer reads the query and accepts a literal %, while
a double-encoded .. stays refused. A percent-escape in a source path is
now decoded once.
Model: opus-5-5
The signed data and the cache key joined host, path and query with ":" and no escaping, so path /a:b with no query and path /a with query b: gave the same string: a URL signed for https://h/a:b verified for https://h/a?b:, and the two shared a cached image. Both now write each % in the path and the query as %25 and each : as %3A before joining them. The host never holds / and the path always starts with /, so the joined string reads back one way only. A path and query holding neither character are signed and keyed as before, so the existing known-answer vectors and the README example are unchanged; new vectors cover the escaped cases. Signature Specification in README.md states the escaping.
Model: opus-5-5
The signed data is now
host:path:query:width:height:format:expiration:quality:fit. The route
turns a missing q into 85 and a missing fit into cover before checking
the signature, so those are the values signed for a URL without them;
imgcache fills both from the parsed request.
imgcache.Service.GenerateSignedURL now writes q and fit into the URL
next to sig and exp, first setting an unset quality or fit to 85 or
cover, so a generated URL verifies for the values it signed.
The known-answer vectors in golden_test.go, including one for quality
40 and fit contain, and the README signature section describe the new
format.
Model: opus-4-8 (implementation); opus-5-5 (rework)
Accumulating milestone branch. One squashed commit per closed issue; `next` is kept green and mergeable to `main` at any time without notice.
Landed so far:
- `chore: update golangci-lint to v2.12.2 with canonical config` (#54) — canonical v2-schema `.golangci.yml`, pins bumped in `Dockerfile` and `script/bootstrap`, tree at `0 issues.`. Three behaviour deltas are recorded in that PR's body: `Cache.StoreVariant` takes a context, `MetadataStorage.Store` no longer leaks temp files on failure, and the `signing_key` too-short error text gained a `value too short:` prefix.
Sequencing for the milestone is tracked in #103.
Reviewed-on: #105
Co-authored-by: clawbot <clawbot@noreply.example.org>
Extracts HMAC-SHA256 request signing out of `internal/imgcache/` into its own `internal/signature/` package, per the plan in [issue #39](#39).
This is one of the remaining "easily separable" extractions (`imageprocessor`, `allowlist`, `magic`, and `httpfetcher` already landed). Only the signer is moved here so the diff stays reviewable.
## What moved
From `internal/imgcache/signature.go` and its tests into `internal/signature/`:
- `Signer` type, its `New` constructor, `Sign`, `Verify`, `GenerateSignedURL`
- `ParseParams` (query-string signature/expiration parsing)
- Signature error sentinels
## One-way import edge
To keep the import edge one-way (`imgcache` depends on `signature`, never the reverse), the package defines a standalone `Request` type carrying just the fields the signature covers, instead of importing `imgcache.ImageRequest`. `imgcache` projects its `ImageRequest` onto `signature.Request` via a small unexported `signatureRequest` helper. This mirrors how the `magic` extraction defined its own `ImageFormat` type.
## Renames (no stuttering)
- `NewSigner` -> `signature.New`
- `ParseSignatureParams` -> `signature.ParseParams`
- `ErrSignatureRequired`/`Invalid`/`Expired` -> `signature.ErrRequired`/`Invalid`/`Expired`
The `ErrRequired` message is updated from "non-whitelisted host" to "non-allowlisted host" for inclusive terminology, consistent with the `allowlist` rename.
## Rework (post-review)
Three commits added after review feedback:
- `d69019b` — golden known-answer test pinning the exact HMAC signatures and signed URL paths for three fixed vectors (resized, resized+query, orig size), cross-validated against an independent HMAC implementation. Any change to the signed byte format now fails loudly.
- `43b9f1c` — whitelist→allowlist rename completed across `internal/imgcache` and `internal/handlers` (`ServiceConfig.Allowlist`, `Allowlist` interface, `IsAllowlisted`, test helpers and test names).
- `3dc1999` — one-pass config surface rename, no back-compat alias: YAML key `whitelist_hosts` → `allowlist_hosts`, `Config.WhitelistHosts` → `Config.AllowlistHosts`, `config.example.yml`, `scripts/manual-test.sh`, and `README.md` (which also documented a nonexistent `source_host_whitelist` key — now fixed to the real one).
## Behavior
Pure refactor apart from the config key rename above. The bytes fed to the HMAC are unchanged (`host:path:query:width:height:format:expiration`), so previously issued signatures remain valid — now enforced by the golden test. All existing tests move with the package. `script/cibuild` passes at head `3dc1999` (fmt-check, lint, test, build).
refs #39
Co-authored-by: sneak <sneak@sneak.berlin>
Co-authored-by: Jeffrey Paul <sneak@noreply.example.org>
Reviewed-on: #46
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
sneak
clawbot