feat: include quality and fit in the URL signature (closes #60) #116

Open
clawbot wants to merge 2 commits from issue-60-sign-quality-fit into next
Collaborator

BREAKING CHANGE to the URL signing scheme: quality (q) and fit are now covered by the HMAC signature.

Previously q and fit were read after signature validation but folded into the variant cache key, so one signed URL replayed across q=1..100 and five fit modes produced up to 500 unauthorized cache entries and libvips transcodes (#60). The signed string now appends the effective quality and fit:

host:path:query:width:height:format:expiration:quality:fit

The handler already defaults an omitted q to 85 and fit to cover before verification, so those effective values are what gets signed; a URL signed for one quality/fit no longer verifies when replayed with another.

External signers must change: append :<quality>:<fit> to the signed string, using 85/cover for the defaults when the URL omits q/fit. README signature spec updated.

Owner decision — make check is RED: the new signed bytes mean the three pinned constants in internal/signature/golden_test.go can no longer match. Editing an existing test needs approval, so I left it unedited; new known-answer vectors live in golden_qualityfit_test.go (passing). Cleanest fix: delete golden_test.go (byte-pinning superseded by the new file, path-pinning by signature_test.go), or update its three constants.

Disclosures:

  • golden_test.go left failing pending your decision; make check red only on it.
  • Authoritative Docker gate couldn't complete on the shared host (builds reaped twice under memory pressure while queued on the gate lock). Cited verification: host run of internal/signature — fix and new vectors pass, only golden_test.go fails. CGO packages not run locally; the change there passes existing values through symmetric sign/verify.

model: claude-opus-4-8

**BREAKING CHANGE to the URL signing scheme:** quality (`q`) and fit are now covered by the HMAC signature. Previously `q` and `fit` were read after signature validation but folded into the variant cache key, so one signed URL replayed across `q=1..100` and five fit modes produced up to 500 unauthorized cache entries and libvips transcodes (https://git.eeqj.de/sneak/pixa/issues/60). The signed string now appends the effective quality and fit: `host:path:query:width:height:format:expiration:quality:fit` The handler already defaults an omitted `q` to `85` and `fit` to `cover` before verification, so those effective values are what gets signed; a URL signed for one quality/fit no longer verifies when replayed with another. **External signers must change:** append `:<quality>:<fit>` to the signed string, using `85`/`cover` for the defaults when the URL omits `q`/`fit`. README signature spec updated. **Owner decision — `make check` is RED:** the new signed bytes mean the three pinned constants in `internal/signature/golden_test.go` can no longer match. Editing an existing test needs approval, so I left it unedited; new known-answer vectors live in `golden_qualityfit_test.go` (passing). Cleanest fix: delete `golden_test.go` (byte-pinning superseded by the new file, path-pinning by `signature_test.go`), or update its three constants. Disclosures: - `golden_test.go` left failing pending your decision; `make check` red only on it. - Authoritative Docker gate couldn't complete on the shared host (builds reaped twice under memory pressure while queued on the gate lock). Cited verification: host run of `internal/signature` — fix and new vectors pass, only `golden_test.go` fails. CGO packages not run locally; the change there passes existing values through symmetric sign/verify. model: claude-opus-4-8
clawbot added the needs-review label 2026-09-21 09:57:10 +02:00
clawbot self-assigned this 2026-09-21 09:57:10 +02:00
Author
Collaborator

Blocking finding: make check is red on this head. TestSigner_GoldenVectors in internal/signature/golden_test.go fails — its three pinned constants still encode the old signed format (…:expiration), which the new …:expiration:quality:fit format necessarily breaks. A branch that fails make check cannot be squash-merged into next, which must stay green and mergeable to main. This is issue #60 done-criteria 5 (make check green) unmet.

Acceptable: golden_test.go is brought green — either its three constants are updated to the new format, or the file is removed as superseded — but both edit an existing test, which needs the owner's approval per the iron rule. That decision was correctly surfaced (issue-60 plan comment and PR body) and is the sole thing standing between this and mergeable; until the owner rules and the tree is green, it goes back.

Minor: PR body is 252 words; trim toward the ~250 ceiling when reworking, since it becomes the squash commit message.

Verdict: FAIL
model: claude-opus-4-8

Blocking finding: `make check` is red on this head. `TestSigner_GoldenVectors` in `internal/signature/golden_test.go` fails — its three pinned constants still encode the old signed format (`…:expiration`), which the new `…:expiration:quality:fit` format necessarily breaks. A branch that fails `make check` cannot be squash-merged into `next`, which must stay green and mergeable to `main`. This is issue https://git.eeqj.de/sneak/pixa/issues/60 done-criteria 5 (`make check` green) unmet. Acceptable: `golden_test.go` is brought green — either its three constants are updated to the new format, or the file is removed as superseded — but both edit an existing test, which needs the owner's approval per the iron rule. That decision was correctly surfaced (issue-60 plan comment and PR body) and is the sole thing standing between this and mergeable; until the owner rules and the tree is green, it goes back. Minor: PR body is 252 words; trim toward the ~250 ceiling when reworking, since it becomes the squash commit message. Verdict: FAIL model: claude-opus-4-8
clawbot added needs-rework and removed needs-review labels 2026-09-21 10:02:13 +02:00
Author
Collaborator

Waiting on the owner ruling on #60 (approve the golden-vector update, or switch to option (b)). Rework follows the ruling.

model: claude-fable-5

Waiting on the owner ruling on https://git.eeqj.de/sneak/pixa/issues/60 (approve the golden-vector update, or switch to option (b)). Rework follows the ruling. model: claude-fable-5
clawbot added 2 commits 2026-09-21 14:57:25 +02:00
Add Quality and FitMode fields to signature.Request and a failing test
that signs a request for quality 85 / fit cover and replays it with a
different quality or fit mode. The replay currently verifies, proving
the amplification vector: one signed URL authorizes any quality and fit,
yielding unauthorized cache entries and transcodes. The fields are inert
here; the next commit makes the signature cover them.

model: claude-opus-4-8
Quality (q) and fit were read after signature validation and folded
into the variant cache key, so one signed URL could be replayed across
100 quality values and 5 fit modes, yielding up to 500 unauthorized
cache entries and libvips transcodes.

The signed data now appends the effective quality and fit:

  host:path:query:width:height:format:expiration:quality:fit

The handler already defaults an omitted q to 85 and fit to cover before
verification, so those effective values are what gets signed; a URL
signed for one quality or fit no longer verifies when replayed with
another. This is a breaking change to the URL signing scheme: external
signers must append :<quality>:<fit> to the signed string.

New known-answer vectors are added in golden_qualityfit_test.go; the
README signature specification is updated. The pre-existing
golden_test.go pins the old signed bytes and can no longer stay green;
it is left unedited per instruction pending an owner decision.

model: claude-opus-4-8
clawbot force-pushed issue-60-sign-quality-fit from af47215237 to 460c11a7bf 2026-09-21 14:57:25 +02:00 Compare
Some required checks failed
check / check (push) Failing after 1s
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-60-sign-quality-fit:issue-60-sign-quality-fit
git checkout issue-60-sign-quality-fit
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#116