bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

fix(uploads): re-encode uploaded images to strip EXIF and cut stored bytes #226

New Issue
Closed
opened 2026-08-29 09:03:02 -05:00 by bermudalamb · 1 comment
bermudalamb commented 2026-08-29 09:03:02 -05:00
Owner
Copy Link
Copy Source

Two problems with one fix, and the privacy one is the more serious. This is live on production today and is not gated behind #220 — it is first in the sequence for that reason.

Uploaded photos carry GPS coordinates, and are served publicly

Nothing in this codebase strips EXIF or re-encodes an uploaded image. A file is stored exactly as it arrived and served from /uploads/. A phone photo carries EXIF, and EXIF routinely carries the coordinates the photo was taken at.

Every stock photo currently on the storefront is affected. Once #222 ships, so is every photo a stranger sends through a link — someone photographing an item at home would publish their home location at a public URL.

Nobody involved has any reason to expect a product photo to be a location beacon, which is what makes it worth fixing rather than documenting.

The same fix bounds the storage problem

Uploads are capped at 6 files × 8 MB per request, against a limiter allowing 20 requests per 15 minutes per IP — roughly 960 MB per IP per window, multiplied by however many addresses. Re-encoding an 8 MB phone photo at a sane dimension and quality yields somewhere around 300 KB, cutting stored bytes by roughly 25×. That does not remove the need for the volume guard in #222; it changes the scale of the thing being guarded against.

Part 1 — new uploads

Add sharp to the backend. In the upload path in routes/admin.ts — or in src/imageUpload.ts if #222's extraction has already landed; either way this does not wait for #222, and the extraction simply moves already-correct code — after the magic-byte verification and before the file is recorded, downscale to a maximum dimension of around 2000px and re-encode.

Re-encoding drops EXIF as a side effect of rebuilding the file, which is a stronger guarantee than deleting the tags from the original: there is nothing left to have missed.

Applies to every upload path. The admin path has the same problem as intake will.

Part 2 — the photos already on the storefront

Part 1 does nothing for what is already there. A one-off script over the existing item_images rows is needed, or the exposure simply persists for the whole current catalogue.

Recommended: re-encode in place, keeping no original. Retaining originals keeps the coordinates on the volume, and a "private" copy on the same disk is one proxy misconfiguration from being public again — which is the exposure this issue exists to close. Storage is also the secondary problem here, and keeping both halves solves neither.

This is lossy and irreversible, so:

  • Take a backup first and confirm it restores. The prod stack already has BACKUP_PASSPHRASE wired up.
  • Make the script re-runnable and idempotent — a file already re-encoded should be skipped, not degraded a second time by another lossy pass.
  • Run it against QA first and compare a handful of images by eye before touching production.
  • Report what it changed. A silent bulk rewrite of customer-visible images is not something to discover later.

Watch out for

  • The stored extension and the served content type must stay in agreement. uploadTypes.ts derives the extension from the validated MIME type and uploads.ts refuses to serve anything it cannot map back. Changing the output format means changing both ends together, or serving files the app then declines to hand out. The backfill has the same constraint, plus the item_images.image_path rows to keep consistent if any name changes.
  • sharp is a native dependency. It must build in the container image and in Gitea Actions, not only on the dev machine.
  • Animated or unusual inputs. The allowlist is JPEG, PNG and WebP; WebP can be animated, and re-encoding one naively yields a still frame.
  • Test with a real photo carrying real EXIF, and assert the tags are gone from the stored file. An assertion that the file merely got smaller would pass on a re-encode that preserved the metadata.

No dependency on #222. Related to #220

Two problems with one fix, and the privacy one is the more serious. **This is live on production today and is not gated behind #220** — it is first in the sequence for that reason. ## Uploaded photos carry GPS coordinates, and are served publicly Nothing in this codebase strips EXIF or re-encodes an uploaded image. A file is stored exactly as it arrived and served from `/uploads/`. A phone photo carries EXIF, and EXIF routinely carries the coordinates the photo was taken at. Every stock photo currently on the storefront is affected. Once #222 ships, so is every photo a stranger sends through a link — someone photographing an item at home would publish their home location at a public URL. Nobody involved has any reason to expect a product photo to be a location beacon, which is what makes it worth fixing rather than documenting. ## The same fix bounds the storage problem Uploads are capped at 6 files × 8 MB per request, against a limiter allowing 20 requests per 15 minutes per IP — roughly 960 MB per IP per window, multiplied by however many addresses. Re-encoding an 8 MB phone photo at a sane dimension and quality yields somewhere around 300 KB, cutting stored bytes by roughly 25×. That does not remove the need for the volume guard in #222; it changes the scale of the thing being guarded against. ## Part 1 — new uploads Add `sharp` to the backend. In the upload path in `routes/admin.ts` — or in `src/imageUpload.ts` if #222's extraction has already landed; **either way this does not wait for #222**, and the extraction simply moves already-correct code — after the magic-byte verification and before the file is recorded, downscale to a maximum dimension of around 2000px and re-encode. Re-encoding drops EXIF as a side effect of rebuilding the file, which is a stronger guarantee than deleting the tags from the original: there is nothing left to have missed. Applies to every upload path. The admin path has the same problem as intake will. ## Part 2 — the photos already on the storefront Part 1 does nothing for what is already there. A one-off script over the existing `item_images` rows is needed, or the exposure simply persists for the whole current catalogue. **Recommended: re-encode in place, keeping no original.** Retaining originals keeps the coordinates on the volume, and a "private" copy on the same disk is one proxy misconfiguration from being public again — which is the exposure this issue exists to close. Storage is also the secondary problem here, and keeping both halves solves neither. This is lossy and irreversible, so: - Take a backup first and confirm it restores. The prod stack already has `BACKUP_PASSPHRASE` wired up. - Make the script re-runnable and idempotent — a file already re-encoded should be skipped, not degraded a second time by another lossy pass. - Run it against QA first and compare a handful of images by eye before touching production. - Report what it changed. A silent bulk rewrite of customer-visible images is not something to discover later. ## Watch out for - **The stored extension and the served content type must stay in agreement.** `uploadTypes.ts` derives the extension from the validated MIME type and `uploads.ts` refuses to serve anything it cannot map back. Changing the output format means changing both ends together, or serving files the app then declines to hand out. The backfill has the same constraint, plus the `item_images.image_path` rows to keep consistent if any name changes. - **`sharp` is a native dependency.** It must build in the container image and in Gitea Actions, not only on the dev machine. - **Animated or unusual inputs.** The allowlist is JPEG, PNG and WebP; WebP can be animated, and re-encoding one naively yields a still frame. - **Test with a real photo carrying real EXIF**, and assert the tags are gone from the stored file. An assertion that the file merely got smaller would pass on a re-encode that preserved the metadata. No dependency on #222. Related to #220
bermudalamb commented 2026-08-29 10:24:37 -05:00
Author
Owner
Copy Link
Copy Source

Plan written

docs/superpowers/plans/2026-08-29-strip-exif-and-reencode.md — five tasks.

  1. Add sharp and prove it installs where it runs. Not on the dev machine — in the actual image, and against the NAS's architecture. sharp ships per-platform prebuilds, and discovering one is missing during a production deploy is the failure this task exists to prevent.
  2. The re-encode policy as a pure module. needsProcessing is a pure function with its own unit test, because it is the whole of the backfill's idempotency argument and being wrong about it degrades every image a little more on every run.
  3. Wire it into uploadImages. That middleware is where verifyUploadedImages already runs, and it is the single choke point every upload route passes through — so the admin routes and the future intake route are both covered without either having to remember. The same reasoning the file already gives for discardUnlessAccepted.
  4. The backfill. Reports by default, needs --apply, writes to a temporary file and renames so an interruption cannot leave a half-written image being served, and skips anything already stripped and in bounds.
  5. Deploy, QA first. Written down rather than improvised, since this is where the irreversible thing happens to real images.

One decision worth confirming

Format is preserved rather than normalised to WebP. Converting compresses better, but it changes every stored extension, and therefore item_images.image_path, which turns the backfill into a rename with a window where database rows point at files that no longer exist.

Preserving the format means the backfill rewrites bytes in place and touches no database rows at all, which removes an entire category of failure from an operation that cannot be undone. The cost is that PNG photographs will not shrink much — they still lose their EXIF and still get bounded, which are the two things actually being bought here. Worth its own issue later if storage turns out to be the binding constraint.

Two traps the issue named but did not resolve

Animated WebP. Read without sharp's animated flag, an animated WebP decodes to its first frame and is silently written back as a still — destroying the uploader's image while reporting success. The flag is set for WebP and only WebP, because it changes how resize interprets height and would be wrong for JPEG and PNG.

sharp before 0.33 has no .withExif(), which the tests use to build a fixture that actually carries GPS data. On an older version those tests fail in a way that reads as "the stripping is broken" when in fact the fixture cannot be made. Task 1 checks the version explicitly.

Verification the plan ends on

Download an image from the live storefront over HTTP and read its metadata. exif: undefined is the assertion this whole issue exists to be able to make — anything short of that is checking the code rather than the exposure.

## Plan written `docs/superpowers/plans/2026-08-29-strip-exif-and-reencode.md` — five tasks. 1. **Add sharp and prove it installs where it runs.** Not on the dev machine — in the actual image, and against the NAS's architecture. `sharp` ships per-platform prebuilds, and discovering one is missing during a production deploy is the failure this task exists to prevent. 2. **The re-encode policy as a pure module.** `needsProcessing` is a pure function with its own unit test, because it is the whole of the backfill's idempotency argument and being wrong about it degrades every image a little more on every run. 3. **Wire it into `uploadImages`.** That middleware is where `verifyUploadedImages` already runs, and it is the single choke point every upload route passes through — so the admin routes and the future intake route are both covered without either having to remember. The same reasoning the file already gives for `discardUnlessAccepted`. 4. **The backfill.** Reports by default, needs `--apply`, writes to a temporary file and renames so an interruption cannot leave a half-written image being served, and skips anything already stripped and in bounds. 5. **Deploy, QA first.** Written down rather than improvised, since this is where the irreversible thing happens to real images. ## One decision worth confirming **Format is preserved rather than normalised to WebP.** Converting compresses better, but it changes every stored extension, and therefore `item_images.image_path`, which turns the backfill into a rename with a window where database rows point at files that no longer exist. Preserving the format means the backfill rewrites bytes in place and **touches no database rows at all**, which removes an entire category of failure from an operation that cannot be undone. The cost is that PNG photographs will not shrink much — they still lose their EXIF and still get bounded, which are the two things actually being bought here. Worth its own issue later if storage turns out to be the binding constraint. ## Two traps the issue named but did not resolve **Animated WebP.** Read without sharp's `animated` flag, an animated WebP decodes to its first frame and is silently written back as a still — destroying the uploader's image while reporting success. The flag is set for WebP and only WebP, because it changes how `resize` interprets height and would be wrong for JPEG and PNG. **`sharp` before 0.33 has no `.withExif()`**, which the tests use to build a fixture that actually carries GPS data. On an older version those tests fail in a way that reads as "the stripping is broken" when in fact the fixture cannot be made. Task 1 checks the version explicitly. ## Verification the plan ends on Download an image from the live storefront over HTTP and read its metadata. `exif: undefined` is the assertion this whole issue exists to be able to make — anything short of that is checking the code rather than the exposure.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
calamb (Chelsea Lamb) bermudalamb (Thom Lamb)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#226