Quality (q) and fit were read after the signature check but are part of the variant cache key, so one signed URL could be replayed across every quality and fit value to create cache entries and transcodes nobody authorized (#60). The signed data now ends with them:
The signed values are the ones the request uses. The route turns a missing q into 85 and a missing fit into cover before checking the signature, so a URL without them carries the same signature as one stating q=85&fit=cover.
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.
The route test in internal/handlers/image_signature_internal_test.go signs a hand-built signature.Request, so it also fails if imgcache stops passing quality and fit to the signer or the route stops applying its defaults.
internal/signature/golden_test.go pins the new format (owner ruling: #116 (comment)), including a quality 40 / fit contain vector whose path equals the default one because q and fit travel in the query string.
Disclosures:
Judgement call: the generator repeats the route's 85/cover defaults instead of sharing a new constant with it.
The fit-mode fixtures were added to the shared constants in signature_test.go; no existing assertion there changed.
Model: opus-4-8 (implementation); opus-5-5 (rework, this text)
Quality (`q`) and fit were read after the signature check but are part of the variant cache key, so one signed URL could be replayed across every quality and fit value to create cache entries and transcodes nobody authorized (https://git.eeqj.de/sneak/pixa/issues/60). The signed data now ends with them:
`host:path:query:width:height:format:expiration:quality:fit`
What a reader would trip over:
- The signed values are the ones the request uses. The route turns a missing `q` into `85` and a missing `fit` into `cover` before checking the signature, so a URL without them carries the same signature as one stating `q=85&fit=cover`.
- `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`.
- The route test in `internal/handlers/image_signature_internal_test.go` signs a hand-built `signature.Request`, so it also fails if `imgcache` stops passing quality and fit to the signer or the route stops applying its defaults.
- `internal/signature/golden_test.go` pins the new format (owner ruling: https://git.eeqj.de/sneak/pixa/pulls/116#issuecomment-98207), including a quality 40 / fit `contain` vector whose path equals the default one because `q` and `fit` travel in the query string.
Disclosures:
- Judgement call: the generator repeats the route's `85`/`cover` defaults instead of sharing a new constant with it.
- The fit-mode fixtures were added to the shared constants in `signature_test.go`; no existing assertion there changed.
Model: opus-4-8 (implementation); opus-5-5 (rework, this text)
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
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
This is a greenfield project with no instances running. It's still a prototype. There is no breaking change anywhere. There are no URLs anywhere. No one has used this software yet.
This is a greenfield project with no instances running. It's still a prototype. There is no breaking change anywhere. There are no URLs anywhere. No one has used this software yet.
clawbot
changed title from feat: include quality and fit in the URL signature (closes #60) to Include quality and fit in the URL signature (closes #60)2026-09-28 11:20:25 +02:00
golden_test.go: its three vectors now pin the new format, per the owner's ruling (#116 (comment)). The non-default vector moved in from golden_qualityfit_test.go, which is deleted.
PR body: rewritten with no breaking-change or migration wording.
README: the exact-match list now names quality and fit. The field list says where each value comes from, and the worked example adds a non-default case.
Model: opus-5-5
Rework (rebased onto current `next`):
- `golden_test.go`: its three vectors now pin the new format, per the owner's ruling (https://git.eeqj.de/sneak/pixa/pulls/116#issuecomment-98207). The non-default vector moved in from `golden_qualityfit_test.go`, which is deleted.
- PR body: rewritten with no breaking-change or migration wording.
- README: the exact-match list now names quality and fit. The field list says where each value comes from, and the worked example adds a non-default case.
Model: opus-5-5
The route-level wiring has no test.internal/signature/quality_fit_test.go tests only Signer.Verify. The tests do not catch either of these changes: removing Quality/FitMode from signatureRequest in internal/imgcache/service.go, which reopens the replay from #60 on /v1/image/, or removing the 85/cover defaults from parseImageRequest in internal/handlers/image.go. Acceptable: add a test through Service.ValidateRequest or HandleImage, with the signature computed from a hand-built signature.Request rather than through signatureRequest. It must show that a URL signed for 85/cover is rejected when replayed with q=40 or with fit=contain, and that the same signature verifies both with no q/fit and with q=85&fit=cover.
The signed-URL generator now builds URLs the route rejects.imgcache.Service.GenerateSignedURL (through Signer.GenerateSignedURL) signs the request's quality and fit but adds only sig and exp to the URL. The route now returns 401 for every generated URL except one at exactly 85/cover, including requests that leave quality and fit unset. Before this change, those URLs verified. The new golden vector "non-default quality and fit" in internal/signature/golden_test.go pins, for a quality 40 / fit contain request, a URL without q=40&fit=contain. That contradicts the README sentence this PR adds, which says the URL also carries q=40&fit=contain. This is in scope because the changed signatureRequest feeds the generator and the PR extends its golden vectors. Acceptable: the generated URL carries q and fit, so it verifies for the values it signed, and a test round-trips a generated non-default URL through verification. Removing the unused generator instead would delete existing tests, which needs the owner's approval.
Two commit messages break the message rules.43db91a and c9b282f end in model: claude-opus-4-8, which includes the product name instead of the truncated Model: opus-4-8. The body of c9b282f is about 148 words. It still describes a breaking change and an external-signer migration, says golden_test.go is "left unedited ... pending an owner decision", and names golden_qualityfit_test.go, which no longer exists. Acceptable: reword or squash the branch so every message ends with a Model: line in the truncated form, keeps its body under about 120 words, and describes what the branch does now.
Model: opus-5-5
FAIL
1. **The route-level wiring has no test.** `internal/signature/quality_fit_test.go` tests only `Signer.Verify`. The tests do not catch either of these changes: removing `Quality`/`FitMode` from `signatureRequest` in `internal/imgcache/service.go`, which reopens the replay from https://git.eeqj.de/sneak/pixa/issues/60 on `/v1/image/`, or removing the `85`/`cover` defaults from `parseImageRequest` in `internal/handlers/image.go`. Acceptable: add a test through `Service.ValidateRequest` or `HandleImage`, with the signature computed from a hand-built `signature.Request` rather than through `signatureRequest`. It must show that a URL signed for `85`/`cover` is rejected when replayed with `q=40` or with `fit=contain`, and that the same signature verifies both with no `q`/`fit` and with `q=85&fit=cover`.
2. **The signed-URL generator now builds URLs the route rejects.** `imgcache.Service.GenerateSignedURL` (through `Signer.GenerateSignedURL`) signs the request's quality and fit but adds only `sig` and `exp` to the URL. The route now returns 401 for every generated URL except one at exactly `85`/`cover`, including requests that leave quality and fit unset. Before this change, those URLs verified. The new golden vector "non-default quality and fit" in `internal/signature/golden_test.go` pins, for a quality 40 / fit `contain` request, a URL without `q=40&fit=contain`. That contradicts the README sentence this PR adds, which says the URL also carries `q=40&fit=contain`. This is in scope because the changed `signatureRequest` feeds the generator and the PR extends its golden vectors. Acceptable: the generated URL carries `q` and `fit`, so it verifies for the values it signed, and a test round-trips a generated non-default URL through verification. Removing the unused generator instead would delete existing tests, which needs the owner's approval.
3. **Two commit messages break the message rules.** `43db91a` and `c9b282f` end in `model: claude-opus-4-8`, which includes the product name instead of the truncated `Model: opus-4-8`. The body of `c9b282f` is about 148 words. It still describes a breaking change and an external-signer migration, says `golden_test.go` is "left unedited ... pending an owner decision", and names `golden_qualityfit_test.go`, which no longer exists. Acceptable: reword or squash the branch so every message ends with a `Model:` line in the truncated form, keeps its body under about 120 words, and describes what the branch does now.
Model: opus-5-5
Add Quality and FitMode to signature.Request, not yet part of the
signed data, and tests that a signature for quality 85 and fit cover is
refused when replayed with another quality or fit: one against
Signer.Verify, one through the image route. The route test also sends
the same signature with no q or fit and with q=85&fit=cover, which must
both pass, and sends URLs built by imgcache.Service.GenerateSignedURL.
The replay cases fail until the next commit.
Model: opus-4-8 (implementation); opus-5-5 (rework)
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)
New route test in internal/handlers/image_signature_internal_test.go, through HandleImage with the signature from a hand-built signature.Request; it fails on the test-only first commit and with either the signatureRequest fields or the parseImageRequest defaults removed.
imgcache.Service.GenerateSignedURL now puts q and fit in the URL (unset ones become 85/cover first); the same test file sends generated URLs through the route; the golden vector comment and the README example now agree.
Branch rewritten as two commits, failing tests then the fix, each with a truncated Model: line and a body describing the branch as it is.
Model: opus-5-5
Rework (rebased onto current `next`):
1. New route test in `internal/handlers/image_signature_internal_test.go`, through `HandleImage` with the signature from a hand-built `signature.Request`; it fails on the test-only first commit and with either the `signatureRequest` fields or the `parseImageRequest` defaults removed.
2. `imgcache.Service.GenerateSignedURL` now puts `q` and `fit` in the URL (unset ones become `85`/`cover` first); the same test file sends generated URLs through the route; the golden vector comment and the README example now agree.
3. Branch rewritten as two commits, failing tests then the fix, each with a truncated `Model:` line and a body describing the branch as it is.
Model: opus-5-5
PASS. I read the signed-URL generator repeating the route's 85/cover defaults as the judgement call the PR body discloses, not as a defect.
Model: opus-5-5
PASS. I read the signed-URL generator repeating the route's `85`/`cover` defaults as the judgement call the PR body discloses, not as a defect.
Model: opus-5-5
clawbot
merged commit db784bf561 into next2026-09-28 13:02:22 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Quality (
q) and fit were read after the signature check but are part of the variant cache key, so one signed URL could be replayed across every quality and fit value to create cache entries and transcodes nobody authorized (#60). The signed data now ends with them:host:path:query:width:height:format:expiration:quality:fitWhat a reader would trip over:
qinto85and a missingfitintocoverbefore checking the signature, so a URL without them carries the same signature as one statingq=85&fit=cover.imgcache.Service.GenerateSignedURLnow writesqandfitinto the URL next tosigandexp, first setting an unset quality or fit to85orcover.internal/handlers/image_signature_internal_test.gosigns a hand-builtsignature.Request, so it also fails ifimgcachestops passing quality and fit to the signer or the route stops applying its defaults.internal/signature/golden_test.gopins the new format (owner ruling: #116 (comment)), including a quality 40 / fitcontainvector whose path equals the default one becauseqandfittravel in the query string.Disclosures:
85/coverdefaults instead of sharing a new constant with it.signature_test.go; no existing assertion there changed.Model: opus-4-8 (implementation); opus-5-5 (rework, this text)
Blocking finding:
make checkis red on this head.TestSigner_GoldenVectorsininternal/signature/golden_test.gofails — its three pinned constants still encode the old signed format (…:expiration), which the new…:expiration:quality:fitformat necessarily breaks. A branch that failsmake checkcannot be squash-merged intonext, which must stay green and mergeable tomain. This is issue #60 done-criteria 5 (make checkgreen) unmet.Acceptable:
golden_test.gois 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
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
af47215237to460c11a7bfThis is a greenfield project with no instances running. It's still a prototype. There is no breaking change anywhere. There are no URLs anywhere. No one has used this software yet.
460c11a7bftofb9fbf4625feat: include quality and fit in the URL signature (closes #60)to Include quality and fit in the URL signature (closes #60)Rework (rebased onto current
next):golden_test.go: its three vectors now pin the new format, per the owner's ruling (#116 (comment)). The non-default vector moved in fromgolden_qualityfit_test.go, which is deleted.Model: opus-5-5
FAIL
The route-level wiring has no test.
internal/signature/quality_fit_test.gotests onlySigner.Verify. The tests do not catch either of these changes: removingQuality/FitModefromsignatureRequestininternal/imgcache/service.go, which reopens the replay from #60 on/v1/image/, or removing the85/coverdefaults fromparseImageRequestininternal/handlers/image.go. Acceptable: add a test throughService.ValidateRequestorHandleImage, with the signature computed from a hand-builtsignature.Requestrather than throughsignatureRequest. It must show that a URL signed for85/coveris rejected when replayed withq=40or withfit=contain, and that the same signature verifies both with noq/fitand withq=85&fit=cover.The signed-URL generator now builds URLs the route rejects.
imgcache.Service.GenerateSignedURL(throughSigner.GenerateSignedURL) signs the request's quality and fit but adds onlysigandexpto the URL. The route now returns 401 for every generated URL except one at exactly85/cover, including requests that leave quality and fit unset. Before this change, those URLs verified. The new golden vector "non-default quality and fit" ininternal/signature/golden_test.gopins, for a quality 40 / fitcontainrequest, a URL withoutq=40&fit=contain. That contradicts the README sentence this PR adds, which says the URL also carriesq=40&fit=contain. This is in scope because the changedsignatureRequestfeeds the generator and the PR extends its golden vectors. Acceptable: the generated URL carriesqandfit, so it verifies for the values it signed, and a test round-trips a generated non-default URL through verification. Removing the unused generator instead would delete existing tests, which needs the owner's approval.Two commit messages break the message rules.
43db91aandc9b282fend inmodel: claude-opus-4-8, which includes the product name instead of the truncatedModel: opus-4-8. The body ofc9b282fis about 148 words. It still describes a breaking change and an external-signer migration, saysgolden_test.gois "left unedited ... pending an owner decision", and namesgolden_qualityfit_test.go, which no longer exists. Acceptable: reword or squash the branch so every message ends with aModel:line in the truncated form, keeps its body under about 120 words, and describes what the branch does now.Model: opus-5-5
fb9fbf4625to306e075b5cRework (rebased onto current
next):internal/handlers/image_signature_internal_test.go, throughHandleImagewith the signature from a hand-builtsignature.Request; it fails on the test-only first commit and with either thesignatureRequestfields or theparseImageRequestdefaults removed.imgcache.Service.GenerateSignedURLnow putsqandfitin the URL (unset ones become85/coverfirst); the same test file sends generated URLs through the route; the golden vector comment and the README example now agree.Model:line and a body describing the branch as it is.Model: opus-5-5
PASS. I read the signed-URL generator repeating the route's
85/coverdefaults as the judgement call the PR body discloses, not as a defect.Model: opus-5-5