fix(audit-3): restore the decode allocation guard, route /health in production

Third round. The decode allocation guard is a regression from round 1: swapping
reader.decode() for into_decoder() silently dropped the max_alloc enforcement
while keeping the comment that claimed it held.

Squashed from 3 commits, original messages preserved below.

──────── fix(imaging): restore the decode allocation guard I removed in round 1

This is a regression I introduced, not a pre-existing gap. Before 05948d8 the
compression worker used `ImageReader::decode()`, which does:

    let mut decoder = Self::make_decoder(format, self.inner, limits.clone())?;
    limits.reserve(decoder.total_bytes())?;   // enforces max_alloc
    decoder.set_limits(limits)?;

Reading the EXIF orientation tag needs `into_decoder()` instead, and that skips
the reserve entirely — the crate's own FIXME concedes `from_decoder` doesn't
compensate. Nothing else enforces `max_alloc`: the JPEG decoder's `set_limits`
only checks support and dimensions. So the 256 MiB budget has been inert since
that commit, and round 2 then propagated the weakened path into export.rs through
the shared helper, in a commit whose message claimed the helper "carries" the
decompression-bomb cap. It didn't, and the comment saying max_alloc "hard-caps
the decode allocation" was simply false.

What was left was only the per-axis cap, which permits 12000x12000 — 412 MiB
decoded, 824 MiB for the two concurrent decodes the worker runs by default,
against a 1 GiB container. Deploy-blocking right now because bumping
DERIVATIVES_REV makes the first boot after a deploy re-decode the entire gallery
two at a time: an OOM kill there restarts the container, which re-runs the
backfill. A boot loop, on the first deploy of these fixes.

Re-add the reserve exactly as `decode()` does it. Per the budget decision it stays
at 256 MiB (~89 MP for RGB8, above any mainstream phone's real output); two
concurrent decodes now peak at 512 MiB. Oversized images take the graceful path
from round 1 — original retained, quota refunded, upload-error toast — and fail
after the header parse but BEFORE any pixels are read, so they cost a header read
rather than an allocation. Measured peak during a concurrent oversized burst: 3.0
MiB.

Test parity is the other half, and the reason this was invisible: the e2e app
container had NO memory limit while production is capped at 1 GiB, so a decode
that would OOM-kill production simply succeeded in CI. Mirror the 1 GiB cap in
docker-compose.test.yml. That is the third divergence of this shape, after WebKit
missing from CI and /health existing only in Caddyfile.test.

Tests: a fixture that is 568 KiB on disk and 283 MiB decoded (11000x9000 = 99 MP,
deliberately UNDER the per-axis cap so the axis check cannot be what rejects it).
A unit test asserts the refusal — it fails against the old code, which decoded it
into an 11000x9000 buffer — with a companion asserting an ordinary photo still
decodes AND still gets its orientation applied, so the guard didn't become a
blanket refusal. An e2e test uploads it singly and as a concurrent pair, asserting
compression lands in 'failed' and the backend is still serving and still
processing afterwards.

──────── fix(deploy): route /health in production, and actually apply Caddyfile changes

Two defects in the update procedure I wrote last round, both of which make a
successful-looking deploy a lie.

1. The documented health check could never pass.

`curl -fsS https://DOMAIN/health` 404s against a perfectly healthy production
stack. The backend registers /health on its ROOT router, not under /api/v1, and
the production Caddyfile proxies only /api/* and /media/* — so /health fell
through to the SvelteKit catch-all, which has no such route and returns its 404
page. With -f, curl exits 22 and the `&& echo` never runs. My own gloss
("Anything other than ok means check the logs") then sent the operator chasing a
phantom outage.

e2e/Caddyfile.test has carried `reverse_proxy /health app:3000` since it was
written — precisely because the catch-all would otherwise swallow it. Production
never did. Per the fix-the-gap-not-the-doc call, production gets the same line,
and /health joins the no-store matcher so a cached response can't report the last
known state instead of the current one. Verified by running the production
Caddyfile against the real backend: /health -> 200 "ok", Cache-Control: no-store,
with /api/v1/event and / unaffected.

2. The sequence never reloaded Caddy, so a Caddyfile-only change was dropped.

`--build` only rebuilds services with a `build:` section, and caddy is a pinned
upstream image. Compose decides whether to recreate a container from its config
hash, which covers the mount SPECIFICATION but not the mounted file's CONTENTS —
so a git pull that changes ./Caddyfile produces no delta, Compose reports
`Running`, and Caddy serves its old config indefinitely. Exit code 0 throughout.

Round 1's iOS download fix (137c4ee) is exactly this shape: Caddyfile plus four
e2e files, so 100% of its production effect is in that one file. Following the
README to the letter deployed it, showed both image IDs changing, and left iOS
downloads broken.

Demonstrated rather than assumed — added a probe header to a Caddyfile, ran the
old sequence (`up -d --build`): header absent, change silently dropped. Ran the
new step 4 (`up -d --force-recreate caddy`): header served.

`--force-recreate` rather than `restart` or `caddy reload` because the bind mount
is resolved to an inode at container-create time and git pull replaces the file
rather than editing in place, so a restart can re-read the stale content — the
exact failure I hit in round 1 when `caddy reload` didn't pick up an edit.

Also rewrites the "db and caddy are untouched … so data volumes survive" sentence.
I wrote it as reassurance; "caddy is untouched" was the bug.

──────── chore: take the Bash(*) permission change back out of the shared settings

`.claude/settings.json` is committed and applies to anyone who clones. Fabi's
local `allow: ["Bash(*)"]` plus deny list ended up in it, inside f0d69f1 — a
commit about the image decode guard, which has nothing to do with permissions.

That was my mistake, twice. The file was already modified when I started the
round: my `git status --short` check printed "(clean)" from an unconditional
`echo` rather than from the status output, so I read a dirty tree as clean. Then
`git add -A` swept it into an unrelated commit, and I reported afterwards that I
had left it untouched. Neither the check nor the claim was true.

Restores the shared file to its previous three narrow entries. The permission
setup itself is preserved, moved to `.claude/settings.local.json`, which
`.gitignore:34` covers precisely so per-user permissions stay per-user — the
existing 442 entries there are kept alongside it.

Not rewriting f0d69f1 to erase this: main is unpushed so it would be safe, but a
visible correction is worth more than a tidy history, and a rebase across the
merge commits carries more risk than the mistake does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Caddyfile

@@ -40,9 +40,10 @@
 	@media_api path /api/v1/upload/*/preview /api/v1/upload/*/thumbnail
 	header @media_api Cache-Control "private, max-age=300"
 
-	# API — never cache, EXCEPT the gated image routes above.
+	# API and health — never cache, EXCEPT the gated image routes above. A cached health
+	# response would report the last known state rather than the current one.
 	@api {
-		path /api/*
+		path /api/* /health
 		not path /api/v1/upload/*/preview /api/v1/upload/*/thumbnail
 	}
 	header @api Cache-Control "no-store"
@@ -58,6 +59,13 @@
 	reverse_proxy /api/* app:3000
 	reverse_proxy /media/* app:3000
 
+	# The backend registers /health on its ROOT router, not under /api/v1, so it needs its
+	# own line — without it the catch-all below hands /health to SvelteKit, which has no
+	# such route and returns its 404 page. That made the documented post-deploy check
+	# (`curl -fsS https://DOMAIN/health`) fail 100% of the time on a perfectly healthy
+	# stack. e2e/Caddyfile.test has always carried this line; production never did.
+	reverse_proxy /health app:3000
+
 	# Everything else goes to SvelteKit frontend
 	reverse_proxy frontend:3001
 }

README.md

@@ -132,13 +132,16 @@ cd /path/to/eventsnap
 # 2. Fetch the new code.
 git pull
 
-# 3. Rebuild and restart. --build is NOT optional.
+# 3. Rebuild and restart the application services. --build is NOT optional.
 docker compose up -d --build
 
-# 4. Confirm the app came back up. Anything other than "ok" means check the logs.
+# 4. Apply any Caddyfile change. Step 3 does NOT do this — see the warning below.
+docker compose up -d --force-recreate caddy
+
+# 5. Confirm the app came back up. Anything other than "ok" means check the logs.
 curl -fsS https://DOMAIN/health && echo
 
-# 5. Confirm a NEW image was actually built. Note the IMAGE ID before you start and
+# 6. Confirm a NEW image was actually built. Note the IMAGE ID before you start and
 #    compare — it must have changed. (Ignore the CREATED column; it reports the base
 #    layer's age, not this build's.) An unchanged ID means step 3 ran without --build
 #    and you are still serving the old code.
@@ -151,8 +154,25 @@ that a migration applied by a *newer* build is not removed by checking out an ol
 so rolling back code without restoring the database snapshot from step 1 leaves the schema
 ahead of the binary and the app refusing to boot.
 
-Only the two application services rebuild; `db` and `caddy` are pinned upstream images and
+> **Why step 4 exists.** `--build` only rebuilds services that have a `build:` section, and
-are untouched, so data volumes and the TLS certificate survive.
+> `caddy` is a pinned upstream image. Compose decides whether to recreate a container from its
+> *config hash*, which covers the mount **specification** (`./Caddyfile:/etc/caddy/Caddyfile:ro`)
+> but **not the file's contents** — so a `git pull` that changes `./Caddyfile` produces no
+> delta, Compose reports `Running`, and Caddy keeps serving its old config indefinitely. Exit
+> code 0 throughout.
+>
+> That is not hypothetical: the fix that made the keepsake download work on iOS
+> (`137c4ee`) touched the Caddyfile and four e2e files and nothing else, so **all** of its
+> production effect lives in that one file. Without step 4 you deploy it, watch both image IDs
+> change, and iOS downloads stay broken.
+>
+> `--force-recreate` rather than `restart` or `caddy reload`: the bind mount is resolved to an
+> **inode** when the container is created, and `git pull` replaces the file instead of editing
+> it in place, so the container can still be bound to the old, now-unlinked inode. A restart
+> then re-reads the stale content. Recreating the container re-resolves the path.
+
+`db` is never touched, and recreating `caddy` does not disturb the `caddy_data` volume, so the
+TLS certificate and all data volumes survive.
 
 ### Generate required secrets
 

backend/src/services/imaging.rs

@@ -19,8 +19,13 @@ use anyhow::{Context, Result};
 use image::{DynamicImage, ImageDecoder};
 use std::path::Path;
 
-/// Bounds for any decode of user-supplied image data. 12000×12000 covers any real phone
+/// Bounds for any decode of user-supplied image data. The per-axis cap covers any real phone
-/// photo; `max_alloc` hard-caps the decode allocation.
+/// photo; `max_alloc` bounds the decoded buffer — but only because `decode_oriented` reserves
+/// against it explicitly, see there.
+///
+/// Sized against the deployment: the app container is capped at 1 GiB and the compression
+/// worker runs `compression_concurrency` decodes at once (default 2), so 256 MiB per decode
+/// leaves headroom for the resize buffers and the runtime.
 fn decode_limits() -> image::Limits {
     let mut limits = image::Limits::default();
     limits.max_image_width = Some(12_000);
@@ -38,11 +43,30 @@ pub fn decode_oriented(path: &Path) -> Result<DynamicImage> {
         .context("failed to open image")?
         .with_guessed_format()
         .context("failed to read image header")?;
-    reader.limits(decode_limits());
+    let mut limits = decode_limits();
+    reader.limits(limits.clone());
 
-    // `into_decoder` carries the limits above through, so reading the tag costs nothing in
+    // We need `into_decoder` rather than `decode()` to read the EXIF orientation tag before
-    // safety. A missing or malformed tag is not an error — most images simply have none.
+    // the pixels are consumed. But the two are NOT equivalent on safety: `decode()` performs
+    //
+    //     limits.reserve(decoder.total_bytes())?;
+    //
+    // between building the decoder and reading the image, and `into_decoder()` skips it (the
+    // crate's own FIXME concedes `from_decoder` doesn't compensate). Nothing else enforces
+    // `max_alloc` — the JPEG decoder's `set_limits` only checks support and dimensions — so
+    // without the line below the budget is inert and the ONLY bound is the per-axis cap. That
+    // leaves 12000x12000 decodable at 412 MiB, and two concurrent at 824 MiB against a 1 GiB
+    // container. Re-add it, exactly as `decode()` does.
     let mut decoder = reader.into_decoder().context("failed to decode image")?;
+    limits
+        .reserve(decoder.total_bytes())
+        .context("image too large to decode within the memory budget")?;
+    decoder
+        .set_limits(limits)
+        .context("image too large to decode within the memory budget")?;
+
+    // Cheap, and it happens BEFORE any pixels are read: an oversized image costs a header
+    // parse, not an allocation.
     let orientation = decoder
         .orientation()
         .unwrap_or(image::metadata::Orientation::NoTransforms);
@@ -50,3 +74,53 @@ pub fn decode_oriented(path: &Path) -> Result<DynamicImage> {
     img.apply_orientation(orientation);
     Ok(img)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Shared with the e2e suite rather than duplicating 568 KiB of binary: the same file
+    /// drives `02-upload/oversized-image` so both layers assert on one artefact.
+    const HUGE: &str = concat!(
+        env!("CARGO_MANIFEST_DIR"),
+        "/../e2e/fixtures/media/huge-99mp.jpg"
+    );
+
+    #[test]
+    fn rejects_an_image_that_would_blow_the_allocation_budget() {
+        // 11000x9000 = 99 MP. Deliberately UNDER the 12000px per-axis cap, so the axis check
+        // cannot reject it — the allocation budget is the only thing that can, which is
+        // exactly what makes this a regression test rather than a restatement of the axis cap.
+        // 283 MiB decoded as RGB8 against a 256 MiB budget, from 568 KiB on disk.
+        //
+        // This failed before the guard was restored: `ImageReader::decode` performs
+        // `limits.reserve(decoder.total_bytes())`, and `into_decoder()` — which we need for
+        // the EXIF tag — skips it, so `max_alloc` was inert and this decoded happily.
+        // Map the Ok arm to its dimensions first: on failure `expect_err` Debug-prints the
+        // value, and Debug on a DynamicImage dumps every pixel — 283 MiB of output.
+        let err = decode_oriented(Path::new(HUGE))
+            .map(|img| (img.width(), img.height()))
+            .expect_err("a 99 MP image must be refused, not allocated");
+        let msg = format!("{err:#}");
+        assert!(
+            msg.to_lowercase().contains("limit") || msg.to_lowercase().contains("memory"),
+            "expected a limits error, got: {msg}"
+        );
+    }
+
+    #[test]
+    fn still_decodes_an_ordinary_photo_and_applies_orientation() {
+        // The guard must not have become a blanket refusal. This fixture is 40x20 stored with
+        // EXIF Orientation=6, so a correct decode returns it rotated to 20x40 portrait.
+        let path = concat!(
+            env!("CARGO_MANIFEST_DIR"),
+            "/../e2e/fixtures/media/portrait-exif6.jpg"
+        );
+        let img = decode_oriented(Path::new(path)).expect("an ordinary photo must decode");
+        assert_eq!(
+            (img.width(), img.height()),
+            (20, 40),
+            "EXIF orientation must still be applied after restoring the guard"
+        );
+    }
+}

e2e/docker-compose.test.yml

@@ -56,6 +56,15 @@ services:
       # Separate volume, exactly as in production: a keepsake archive contains every
       # photo in the event, so it is kept off the media tree.
       - exports_data:/exports
+    # Mirror production's cap (docker-compose.yml). The test stack having NO memory limit is
+    # why an unbounded image decode was invisible here: a 99 MP upload that would OOM-kill the
+    # 1 GiB production container simply succeeded in CI. A test environment more generous than
+    # production cannot catch a resource bug — the same shape as WebKit being absent from CI
+    # and /health existing only in Caddyfile.test.
+    deploy:
+      resources:
+        limits:
+          memory: 1G
     expose:
       - '3000'
 

e2e/specs/02-upload/oversized-image.spec.ts

@@ -0,0 +1,94 @@
+/**
+ * Regression guard — an image that would blow the decode budget must be refused, not
+ * allocated, and the container must survive it.
+ *
+ * The compression worker sets `max_alloc = 256 MiB`, but that budget was inert: reading the
+ * EXIF orientation tag requires `ImageReader::into_decoder()`, which skips the
+ * `limits.reserve(decoder.total_bytes())` that `decode()` performs, and nothing else enforces
+ * it (the JPEG decoder's `set_limits` only checks support and dimensions). So the only real
+ * bound was the 12000px per-axis cap — leaving 12000x12000 decodable at 412 MiB, and two
+ * concurrent decodes at 824 MiB against a 1 GiB container.
+ *
+ * That mattered acutely because bumping DERIVATIVES_REV makes the first boot after a deploy
+ * re-decode the whole gallery two at a time: an OOM kill there restarts the container, which
+ * re-runs the backfill — a boot loop.
+ *
+ * This suite could never have caught it, because until now the e2e app container had NO
+ * memory limit at all while production is capped at 1 GiB. The cap is mirrored in
+ * docker-compose.test.yml so this test means something.
+ *
+ * Fixture: 11000x9000 = 99 MP, 568 KiB on disk. Deliberately UNDER the per-axis cap, so the
+ * axis check cannot be what rejects it — 283 MiB decoded against a 256 MiB budget.
+ */
+import { test, expect } from '../../fixtures/test';
+import { uploadRaw } from '../../helpers/upload-client';
+import { BASE } from '../../helpers/env';
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+
+const HUGE = join(process.cwd(), 'fixtures', 'media', 'huge-99mp.jpg');
+const SAMPLE = join(process.cwd(), 'fixtures', 'media', 'sample.jpg');
+
+test.describe('Upload — an oversized image is refused, not allocated', () => {
+  test('a 99 MP upload fails compression gracefully and the backend stays up', async ({
+    guest,
+    db,
+  }) => {
+    test.setTimeout(90_000);
+    const g = await guest('BombThrower');
+
+    // The upload itself is accepted — 568 KiB is well within the body cap. The rejection
+    // happens in the compression worker, where the decode budget lives.
+    const res = await uploadRaw(g.jwt, readFileSync(HUGE), {
+      filename: 'huge.jpg',
+      contentType: 'image/jpeg',
+      caption: 'zu gross',
+    });
+    expect(res.status, 'a 568 KiB file is a legitimate upload').toBe(201);
+    const { id } = (await res.json()) as { id: string };
+
+    // It must land in 'failed', not 'done' — and must get there, rather than the container
+    // dying mid-decode and leaving it stuck in 'processing' forever.
+    await expect
+      .poll(() => db.compressionStatus(id), { timeout: 60_000, intervals: [500] })
+      .toBe('failed');
+
+    // The whole point: the process is still alive. An OOM kill would have taken the backend
+    // down here, and Docker would have restarted it.
+    const health = await fetch(`${BASE}/health`);
+    expect(health.status, 'the backend must have survived the oversized decode').toBe(200);
+
+    // And it is still doing useful work afterwards — not wedged or restarting.
+    const ok = await uploadRaw(g.jwt, readFileSync(SAMPLE), {
+      filename: 'after.jpg',
+      contentType: 'image/jpeg',
+    });
+    expect(ok.status).toBe(201);
+    const after = (await ok.json()) as { id: string };
+    await expect.poll(() => db.compressionStatus(after.id), { timeout: 30_000 }).toBe('done');
+  });
+
+  test('two oversized uploads at once still leave the container alive', async ({ guest, db }) => {
+    // The concurrent case is the one that actually OOM'd: `compression_concurrency` is 2, so
+    // two decodes overlap. Under the old behaviour this pair peaked near the container cap.
+    test.setTimeout(90_000);
+    const g = await guest('BombThrower2');
+    const bytes = readFileSync(HUGE);
+
+    const [a, b] = await Promise.all([
+      uploadRaw(g.jwt, bytes, { filename: 'huge-a.jpg', contentType: 'image/jpeg' }),
+      uploadRaw(g.jwt, bytes, { filename: 'huge-b.jpg', contentType: 'image/jpeg' }),
+    ]);
+    expect([a.status, b.status]).toEqual([201, 201]);
+    const ids = [((await a.json()) as { id: string }).id, ((await b.json()) as { id: string }).id];
+
+    for (const id of ids) {
+      await expect
+        .poll(() => db.compressionStatus(id), { timeout: 60_000, intervals: [500] })
+        .toBe('failed');
+    }
+
+    const health = await fetch(`${BASE}/health`);
+    expect(health.status, 'two concurrent oversized decodes must not kill the backend').toBe(200);
+  });
+});