Add the imgproxy delivery layer - #68
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f950a0718b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f950a07 to
4196886
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4196886fb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| apply_client_hints(&mut parsed_options, &request.hints); | ||
| ( | ||
| parsed_options.resize.map(|resize| resize.width).unwrap_or(0), | ||
| (parsed_options.dpr_factor() * 1000.0).round() as u32, | ||
| ) |
There was a problem hiding this comment.
Client-hint DPR is applied at full f32 precision when processing but rounded to thousandths in the cache key. For example, at width 3840, DPR values 1.0001 and 1.0004 share the 1000 key while apply_dpr can round them to different output widths, causing one client's cached bytes to be returned to the other. Fresh evidence beyond the earlier missing-key issue is this lossy quantization; key by the exact DPR bits or by the effective scaled dimensions.
Useful? React with 👍 / 👎.
4196886 to
f3193fa
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3193fadec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f3193fa to
6d5aaff
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d5aaffddb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
6d5aaff to
015c42a
Compare
015c42a to
b7daac2
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7daac24ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b7daac2 to
1e23fa1
Compare
imgforge processed images correctly and then said almost nothing about them. A response carried a content type and, on a cache hit, a cache status; everything a CDN or a browser uses to avoid asking again was missing, and one URL could only ever mean one format. Content negotiation reads Accept and can serve WebP or AVIF to clients that advertise them, either where the URL left the format open (detection) or over the top of an explicit one (enforcement). The chosen format is part of the cache key and the response carries Vary: Accept, because otherwise a shared cache hands an AVIF to a client that said it could not read one. Caching headers follow: a configured TTL or the origin's own Cache-Control, an ETag over the response bytes with If-None-Match answered as a 304, Last-Modified, and a canonical Link back to the source. Client hints let the browser size the image for a URL that did not. Alongside those, the settings a deployment needs around them: a base URL and an allow list for sources, a path prefix and a health endpoint, a CORS origin, truncated signatures, debug headers, and development error detail.
34d36cb to
3eb4444
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eb4444e46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An allow-list entry is a prefix of the whole URL, but parsing kept only its path, so https://api.example/render?tenant=public also admitted ?tenant=private and no query at all — wider than the boundary the operator wrote down. The query is now kept and prefix-matched, and an entry that names one pins its path exactly: as a prefix of the raw URL, more path would have to come before the ?, and nothing can. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A cached composite validated only the main image on a hit, so a watermark_url host removed from IMGFORGE_ALLOWED_SOURCES kept serving its pixels out of the cache. The watermark URL is now checked before the lookup alongside the main URL, and the entry remembers where the watermark was actually fetched from — redirects included — so the hit filter rechecks both sources. Metadata entries had the same gap one layer down: CachedMetadata carried no final fetch URL, so an /info answer that had followed a redirect outlived the policy that admitted its destination. The entry now records it and a hit revalidates it, the same rule the image cache applies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The default ratio was stored as Some(1.0), identical to a URL that explicitly asked for dpr:1, so the hint gate told them apart by value and overwrote both. The default is now None — presence is the signal — and a URL that names any ratio, 1 included, keeps it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9e0ad571d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A raw or skip_processing miss always returned debug: None, so enabling IMGFORGE_ENABLE_DEBUG_HEADERS silently did nothing for exactly the requests whose passthrough nature the headers would have made visible. A passthrough returns the source as the result, so both halves of the diagnostics describe the same bytes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcd36aeb15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
With IMGFORGE_SECRET set, the TTL policy said public — an explicit invitation for a CDN to store the authorised answer and replay it to a request carrying no token. Bearer deployments now always say private: the TTL keeps its max-age, passthrough no longer forwards the origin's policy (the origin cannot know a token guards it now), and even with nothing configured the refusal is said out loud so heuristic caching cannot decide otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Authorization is not a safelisted request header, so a browser holding a bearer token asks with OPTIONS before it will send the real request. The router knew only GET, so the preflight got a 405 — and no header on the eventual GET could repair that. With IMGFORGE_ALLOW_ORIGIN set, an OPTIONS handler now grants the origin, GET, and the Authorization header; without it, OPTIONS keeps meaning 405. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27cc355bc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The successful /info JSON was the one response without Access-Control-Allow-Origin, so a cross-origin request could pass the preflight, complete at the server, and still be unreadable to the caller. And Accept parameter names are case-insensitive, so Q=0 is the same refusal as q=0 — reading only the lowercase spelling turned an explicit refusal into the default full weight and served the refused format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 442336521b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An entry that failed URL parsing fell back to raw-prefix text matching — the exact bug class the parsed matcher exists to end. A mistyped https://trusted.example:bad text-matched https://trusted.example:bad@evil.test/x, whose real host hides behind userinfo. Parsing is now fallible: a boundary that cannot be enforced as written is a configuration error naming the entry, not a different boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 377c79369f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Every ETag-enabled cache hit re-hashed the whole body on the async worker — a per-hit SHA-256 for exactly the requests the cache made cheap. The tag is now computed on the miss, which just produced the bytes anyway, stored with the entry regardless of the current setting (the entry outlives the configuration), and served back on hits. The CORS preflight also granted only Authorization, but If-None-Match and If-Modified-Since are not safelisted either — a preflight that blocked them blocked exactly the revalidation this change shipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf08442a8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Four hardenings from review. An out-of-range signature_size set by a library caller was clamped, so 0 silently became a one-byte HMAC — brute-forceable in about 256 requests; validation now fails closed on it. The resolved watermark_url joins the cache identity, because a relative reference composites a different overlay the moment IMGFORGE_BASE_URL changes while the path stays identical. CORS preflights no longer spend a rate-limit token — the browser asking permission is not a fetch, and at a limit of 1 the preflight starved the request it was asking for. And the miss-path entity tag is hashed once, inside the blocking task that produced the bytes, instead of twice on the async worker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e45793b75d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A hit rebuilt its headers from empty source metadata, so a passthrough no-store — the origin explicitly forbidding storage — vanished the moment the cache started answering, and Last-Modified went with it. The entry now stores the origin's Cache-Control and Last-Modified and a hit keeps saying what the origin said. The Width client hint is attacker-adjacent input: a signature covers the path, not the headers, so a reusable signed URL that leaves its width to hints must not let Width: 1000000 size the pipeline. The hint is now bounded by the configured result ceiling when one is in force and by a 16384px hard cap when none is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Negotiation offers only what the build can encode, so the two tests that expect AVIF to win assumed an encoder the CI runner's libvips does not have — they have been failing there since they were written. The same guard the save tests use skips them where the offer cannot exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Stacked on #67.
imgforge processed images correctly and then said almost nothing about them. A response carried a content type and, on a cache hit, a cache status; everything a CDN or a browser uses to avoid asking again was missing, and one URL could only ever mean one format.
This is the largest PR in the stack. The four groups below were built together because they share the config and service plumbing — the request has to carry the client's headers, and the response has to carry what was decided — but they are independent features and can be reviewed one at a time.
Content negotiation
IMGFORGE_ENABLE_WEBP_DETECTIONandIMGFORGE_ENABLE_AVIF_DETECTIONserve a modern format to clients whoseAcceptadvertises it, where the URL left the format open;IMGFORGE_ENFORCE_WEBP/_AVIFdo so over the top of an explicit one.IMGFORGE_ENABLE_CLIENT_HINTShonoursWidthandDPR.The negotiated format is part of the cache key and the response carries
Vary: Accept. Without both, a shared cache hands an AVIF to a client that said it could not read one. A format this libvips build cannot encode is skipped rather than attempted, so enabling AVIF on a build with no AV1 encoder degrades to the URL's own format instead of returning 400 to every modern browser.Caching headers
IMGFORGE_TTL,IMGFORGE_CACHE_CONTROL_PASSTHROUGH,IMGFORGE_USE_ETAG(withIf-None-Matchanswered as a 304),IMGFORGE_LAST_MODIFIED_ENABLED,IMGFORGE_SET_CANONICAL_HEADER. All off by default: turning on long-lived client caching during an upgrade can pin stale images in browsers for a year, which is not something to do silently.Source controls
IMGFORGE_BASE_URL,IMGFORGE_ALLOWED_SOURCES(wherehttps://*.example.com/matches exactly one subdomain label),IMGFORGE_USER_AGENT,IMGFORGE_MAX_REDIRECTS.Deployment settings
IMGFORGE_PATH_PREFIX,IMGFORGE_HEALTH_CHECK_PATH(default/health, alongside the existing/status),IMGFORGE_ALLOW_ORIGIN,IMGFORGE_SIGNATURE_SIZE,IMGFORGE_ENABLE_DEBUG_HEADERS,IMGFORGE_DEVELOPMENT_ERRORS_MODE, and per-option defaults forauto_rotate,strip_metadata,keep_copyright,strip_color_profile,preserve_hdr,enforce_thumbnail,return_attachmentandquality.Notes for review
tests/delivery_integration_tests.rsis new and covers all four groups end to end.Validation