2026-08-29 10:32:49 -05:00
3 changed files with 2792 additions and 0 deletions
@@ -0,0 +1,782 @@
 # Strip EXIF and Re-encode Uploads Implementation Plan
 > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
 **Goal:** No uploaded photo — new or already stored — carries the coordinates it was taken at, and stored bytes fall by roughly an order of magnitude.
 **Architecture:** `sharp` re-encodes every accepted upload inside the existing `uploadImages` middleware, immediately after the magic-byte verification and before any route logic runs. Re-encoding rebuilds the file, so EXIF is gone as a consequence rather than as a deletion that could miss a tag. A separate one-off script applies the same transform to the photos already on disk.
 **Tech Stack:** TypeScript, `sharp`, Express + multer, Jest + supertest, `tsx` for the script.
 **Issue:** https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/226
 ## Global Constraints
 - **Format is preserved, never converted.** JPEG stays JPEG, PNG stays PNG, WebP stays WebP. See the decision below — this is what keeps the stored extension, the served content type and `item_images.image_path` all consistent, and it is what makes the backfill a rewrite rather than a rename.
 - **`sharp` is a production dependency.** The final Docker stage runs `npm install --omit=dev`, so a dev-dependency `sharp` would be absent at runtime and every upload would fail in production only.
 - **Every route handler stays wrapped in `asyncRoute`** — `tests/unit/routesAreWrapped.test.ts` enforces it.
 - **The backfill is lossy and irreversible.** It does not run against production without a verified backup and a QA run first.
 - **Commit style:** Conventional Commits, subject ending `(#226)`, no hard wrapping in bodies.
 ## Decisions
 | Question | Decision | Why |
 | --- | --- | --- |
 | Output format | Keep the input's format | Converting everything to WebP compresses better, but changes every stored filename's extension, which changes `item_images.image_path`, which turns the backfill into a rename plus a database migration with a window where rows point at files that no longer exist. The privacy fix does not need that risk. Revisit as its own issue if storage becomes the binding problem |
 | Maximum dimension | 2000px on the longest side, never upscaled | Comfortably above what the storefront displays, and the point where a phone photo stops being several megabytes |
 | Quality | 82 for JPEG and WebP | The usual point where further reduction starts being visible on photographs |
 | PNG | Re-encoded, not converted | It still loses EXIF and still downscales. It will not shrink much, which is accepted — a PNG photograph is rare here |
 | Where it runs | Inside `uploadImages`, after verification | That middleware is the single choke point every upload route already passes through, so a route added later inherits the behaviour rather than having to remember it. This is the same reasoning the file's own comment gives for `discardUnlessAccepted` |
 | Backfill idempotency | Skip a file that already has no EXIF and is within bounds | Needs no schema change and no marker file, and is correct by construction: a file that is already stripped and already small needs nothing done to it, so a second run cannot degrade it with another lossy pass |
 | Originals | Not kept | Retaining them keeps the coordinates on the same volume, one proxy misconfiguration from being public again — which is the exposure this exists to close |
 ## File Structure
 **Created:**
 - `backend/src/imageProcessing.ts` — the re-encode policy and the transform
 - `backend/tests/unit/imageProcessing.test.ts`
 - `backend/tests/integration/exifStripping.integration.test.ts`
 - `backend/scripts/backfill-image-reencode.ts`
 **Modified:**
 - `backend/package.json` — `sharp` as a production dependency, plus a script entry
 - `backend/src/routes/admin.ts` — the re-encode call inside `uploadImages`
 ---
 ### Task 1: Add sharp and prove it installs where it has to run
 `sharp` ships prebuilt binaries per platform. This repository builds on `node:20-bookworm-slim` (Debian, glibc) and runs on a Synology NAS, so the prebuild has to exist for the image's architecture. Finding out it does not during a production deploy is the failure this task exists to prevent.
 **Files:**
 - Modify: `backend/package.json`
 - [ ] **Step 1: Install it as a production dependency**
 ```bash
 cd backend && npm install sharp
 ```
 Confirm it landed under `"dependencies"` and not `"devDependencies"` in `backend/package.json`. The final Docker stage installs with `--omit=dev`, so this being in the wrong section produces a container that fails on the first upload and nowhere else.
 - [ ] **Step 2: Check it loads**
 ```bash
 cd backend && node -e "const s=require('sharp'); console.log(s.versions); console.log(process.arch, process.platform)"
 ```
 Expected: a version object, not a build error.
 Also confirm the installed major version is **0.33 or newer**:
 ```bash
 cd backend && node -e "console.log(require('sharp/package.json').version)"
 ```
 The integration tests in Task 3 build their fixture with `.withExif()`, which does not exist before 0.33 — on an older version those tests fail in a way that looks like the stripping is broken when it is the fixture that cannot be made.
 - [ ] **Step 3: Prove it works in the actual image**
 ```bash
 docker build -t redefined-sharp-check .
 docker run --rm redefined-sharp-check node -e "require('sharp'); console.log('sharp loaded', process.arch)"
 ```
 Expected: `sharp loaded` and the architecture. **If this fails**, the prebuild is missing for the target platform and the Dockerfile needs `build-essential`, `libvips-dev` and `python3` in the build stage. Do not proceed on a dev-machine success alone — that is a different platform from the one that serves.
 - [ ] **Step 4: Confirm the deployment target's architecture matches**
 The image must run on the NAS, not only build on this machine. If the NAS is ARM and the image is x64 (or the reverse), that is a problem this repository has regardless of `sharp`, but `sharp` is where it will first become visible.
 ```bash
 docker run --rm redefined-sharp-check node -e "console.log(process.arch)"
 ```
 Compare against `uname -m` on the NAS. Record the result in the commit message.
 - [ ] **Step 5: Commit**
 ```bash
 git add backend/package.json backend/package-lock.json
 git commit -m "build(uploads): add sharp for image re-encoding (#226)"
 ```
 ---
 ### Task 2: The re-encode policy and transform
 **Files:**
 - Create: `backend/src/imageProcessing.ts`
 - Test: `backend/tests/unit/imageProcessing.test.ts`
 **Interfaces:**
 - Produces:
  - `MAX_DIMENSION: number`, `QUALITY: number`
  - `needsProcessing(meta: {width?: number; height?: number; exif?: unknown}): boolean` — pure
  - `reencodeInPlace(filePath: string, mimetype: string): Promise<void>`
 - [ ] **Step 1: Write the failing unit test**
 Create `backend/tests/unit/imageProcessing.test.ts`:
 ```ts
 import { needsProcessing, MAX_DIMENSION } from '../../src/imageProcessing';
 // The whole of the skip/process policy, kept pure so the backfill's
 // idempotency can be reasoned about without a filesystem. The backfill is
 // lossy and irreversible, so being wrong here is expensive.
 describe('needsProcessing', () => {
  it('processes anything carrying EXIF, however small', () => {
    expect(needsProcessing({ width: 10, height: 10, exif: Buffer.from('x') })).toBe(true);
  });
  it('processes an oversized image even with no EXIF', () => {
    expect(needsProcessing({ width: MAX_DIMENSION + 1, height: 100 })).toBe(true);
  });
  it('processes an image oversized on either axis', () => {
    expect(needsProcessing({ width: 100, height: MAX_DIMENSION + 1 })).toBe(true);
  });
  // The idempotency property the backfill depends on: a file already stripped
  // and already within bounds is left alone, so a second run cannot put it
  // through another lossy pass.
  it('leaves a stripped, in-bounds image alone', () => {
    expect(needsProcessing({ width: MAX_DIMENSION, height: MAX_DIMENSION })).toBe(false);
  });
  // Unknown dimensions mean sharp could not read it as an image. Processing is
  // the safe answer: the alternative is skipping a file we understand least.
  it('processes an image whose dimensions could not be read', () => {
    expect(needsProcessing({})).toBe(true);
  });
 });
 ```
 - [ ] **Step 2: Run it to verify it fails**
 ```bash
 cd backend && npx jest -c jest.unit.config.js imageProcessing
 ```
 Expected: FAIL — module not found.
 - [ ] **Step 3: Write the implementation**
 Create `backend/src/imageProcessing.ts`:
 ```ts
 import sharp from 'sharp';
 import { promises as fs } from 'fs';
 /**
 * Rebuilding an uploaded image so it carries nothing but the picture.
 *
 * A file arrives as the camera wrote it, and a camera writes EXIF — which
 * routinely includes the coordinates the photo was taken at. Those files are
 * served publicly from /uploads/, so an unmodified product photo publishes the
 * location it was taken. Nobody sending in a photograph of a vase expects that.
 *
 * The fix is to re-encode rather than to delete tags. Deleting requires knowing
 * every tag that could carry something sensitive, across formats and camera
 * makers, forever. Re-encoding builds a new file from the decoded pixels, so
 * there is nothing left that could have been missed — the same reasoning that
 * makes uploadTypes.ts an allowlist rather than a denylist.
 *
 * Format is deliberately preserved. Converting to WebP would compress better,
 * but it changes stored extensions, and therefore item_images.image_path, and
 * therefore turns the backfill into a rename with a window where rows point at
 * files that no longer exist. See #226.
 */
 /** Comfortably larger than anything the storefront renders. */
 export const MAX_DIMENSION = 2000;
 /** Where further reduction starts to show on a photograph. */
 export const QUALITY = 82;
 interface ImageFacts {
  width?: number;
  height?: number;
  exif?: unknown;
 }
 /**
 * Whether a file still needs rebuilding.
 *
 * Pure, and the backfill's entire idempotency argument: a file with no EXIF
 * that is already within bounds is already in its final state, so a re-run
 * skips it rather than putting it through a second lossy pass. Anything
 * unreadable is processed rather than skipped — a file we cannot describe is
 * not one to assume is safe.
 */
 export function needsProcessing(meta: ImageFacts): boolean {
  if (meta.exif !== undefined && meta.exif !== null) return true;
  if (meta.width === undefined || meta.height === undefined) return true;
  return meta.width > MAX_DIMENSION || meta.height > MAX_DIMENSION;
 }
 function encoderFor(instance: sharp.Sharp, mimetype: string): sharp.Sharp {
  switch (mimetype) {
    case 'image/jpeg':
      return instance.jpeg({ quality: QUALITY });
    case 'image/webp':
      return instance.webp({ quality: QUALITY });
    case 'image/png':
      // PNG is lossless, so quality does not apply and this will not shrink
      // much. It still strips EXIF and still bounds the dimensions, which are
      // the two things being bought here.
      return instance.png({ compressionLevel: 9 });
    default:
      // Unreachable: the allowlist in uploadTypes.ts is these three. Throwing
      // rather than passing the file through unmodified, because "we did not
      // recognise it so we left the metadata in" is the failure mode this
      // module exists to make impossible.
      throw new Error(`cannot re-encode unsupported type ${mimetype}`);
  }
 }
 /**
 * Rewrites the file at `filePath`, in its own format, stripped and bounded.
 *
 * Writes to a sibling temporary file and renames over the original, because
 * writing in place would leave a half-written image being served if the process
 * died mid-write — and sharp cannot read and write the same path in one pass
 * anyway.
 *
 * `withoutEnlargement` so a small image is not blown up to the cap: the ceiling
 * is a maximum, not a target.
 */
 export async function reencodeInPlace(filePath: string, mimetype: string): Promise<void> {
  const temporary = `${filePath}.reencoding`;
  try {
    await encoderFor(
      // `animated` only for WebP, which is the one allowed type that can carry
      // more than one frame. Reading an animated WebP without it decodes the
      // first frame alone and silently writes back a still — destroying the
      // uploader's image while reporting success. It is not set unconditionally
      // because it changes how `resize` interprets height (the full frame
      // strip, not one frame), which would be wrong for the other two.
      sharp(filePath, mimetype === 'image/webp' ? { animated: true } : {}).resize({
        width: MAX_DIMENSION,
        height: MAX_DIMENSION,
        fit: 'inside',
        withoutEnlargement: true
      }),
      mimetype
      // No withMetadata(): omitting it is what drops EXIF, ICC and everything
      // else. Calling it would put the metadata back.
    ).toFile(temporary);
    await fs.rename(temporary, filePath);
  } catch (err) {
    await fs.unlink(temporary).catch(() => undefined);
    throw err;
  }
 }
 ```
 - [ ] **Step 4: Run the test to verify it passes**
 ```bash
 cd backend && npx jest -c jest.unit.config.js imageProcessing
 ```
 Expected: PASS, 5 tests.
 - [ ] **Step 5: Commit**
 ```bash
 git add backend/src/imageProcessing.ts backend/tests/unit/imageProcessing.test.ts
 git commit -m "feat(uploads): re-encode images to strip metadata and bound dimensions (#226)"
 ```
 ---
 ### Task 3: Apply it to every upload
 **Files:**
 - Modify: `backend/src/routes/admin.ts:183-211` (the `uploadImages` middleware)
 - Test: `backend/tests/integration/exifStripping.integration.test.ts`
 **Interfaces:**
 - Consumes: `reencodeInPlace` from `src/imageProcessing`
 - [ ] **Step 1: Write the failing integration test**
 Create `backend/tests/integration/exifStripping.integration.test.ts`:
 ```ts
 import request from 'supertest';
 import sharp from 'sharp';
 import { promises as fs } from 'fs';
 import path from 'path';
 import app from '../../src/app';
 import { pool } from '../../src/db';
 import { resetDb, closeDb } from './setup/testDb';
 const UPLOADS_DIR = process.env.UPLOADS_DIR as string;
 beforeAll(async () => {
  await fs.mkdir(UPLOADS_DIR, { recursive: true });
 });
 beforeEach(async () => {
  await resetDb();
 });
 afterAll(async () => {
  await pool.end();
  await closeDb();
 });
 /**
 * A JPEG carrying GPS EXIF, built rather than committed as a binary fixture so
 * what it contains is readable in this file. This is the exact shape of the
 * problem: a photograph that says where it was taken.
 */
 async function photoWithLocation(): Promise<Buffer> {
  return sharp({
    create: { width: 3000, height: 2000, channels: 3, background: { r: 120, g: 90, b: 60 } }
  })
    .withExif({
      IFD0: { Make: 'TestCam', Model: 'X1' },
      GPS: { GPSLatitudeRef: 'N', GPSLatitude: '51/1 30/1 0/1', GPSLongitudeRef: 'W' }
    })
    .jpeg()
    .toBuffer();
 }
 async function storedPathFor(itemId: number): Promise<string> {
  const { rows } = await pool.query(
    `SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`,
    [itemId]
  );
  // image_path is '/uploads/<name>'; the file is that name inside UPLOADS_DIR.
  return path.join(UPLOADS_DIR, path.basename(rows[0].image_path));
 }
 describe('an uploaded photo does not keep where it was taken', () => {
  it('has no EXIF once stored', async () => {
    const withGps = await photoWithLocation();
    // Guard the fixture itself: if this ever stops carrying EXIF, the
    // assertion below would pass while testing nothing at all.
    expect((await sharp(withGps).metadata()).exif).toBeDefined();
    const res = await request(app)
      .post('/api/admin/items')
      .field('name', 'Vase')
      .field('description', '')
      .field('price', '40')
      .attach('images', withGps, 'vase.jpg');
    expect(res.status).toBe(200);
    const stored = await sharp(await storedPathFor(res.body.id)).metadata();
    expect(stored.exif).toBeUndefined();
  });
  it('is bounded to the maximum dimension', async () => {
    const res = await request(app)
      .post('/api/admin/items')
      .field('name', 'Vase')
      .field('description', '')
      .field('price', '40')
      .attach('images', await photoWithLocation(), 'vase.jpg');
    const stored = await sharp(await storedPathFor(res.body.id)).metadata();
    expect(stored.width).toBe(2000);
    expect(stored.height).toBe(1333);
  });
  it('keeps the format, so the stored extension still describes the file', async () => {
    const res = await request(app)
      .post('/api/admin/items')
      .field('name', 'Vase')
      .field('description', '')
      .field('price', '40')
      .attach('images', await photoWithLocation(), 'vase.jpg');
    const storedPath = await storedPathFor(res.body.id);
    expect(path.extname(storedPath)).toBe('.jpg');
    expect((await sharp(storedPath).metadata()).format).toBe('jpeg');
  });
  it('does not enlarge an image that is already small', async () => {
    const small = await sharp({
      create: { width: 300, height: 200, channels: 3, background: { r: 1, g: 2, b: 3 } }
    })
      .png()
      .toBuffer();
    const res = await request(app)
      .post('/api/admin/items')
      .field('name', 'Tiny')
      .field('description', '')
      .field('price', '5')
      .attach('images', small, 'tiny.png');
    const stored = await sharp(await storedPathFor(res.body.id)).metadata();
    expect(stored.width).toBe(300);
    expect(stored.height).toBe(200);
  });
 });
 ```
 - [ ] **Step 2: Run it to verify it fails**
 ```bash
 cd backend && npx jest -c jest.integration.config.js --runInBand exifStripping
 ```
 Expected: FAIL — `stored.exif` is defined, and the width is 3000.
 - [ ] **Step 3: Wire it into the middleware**
 In `backend/src/routes/admin.ts`, add the import:
 ```ts
 import { reencodeInPlace } from '../imageProcessing';
 ```
 Add this beside `verifyUploadedImages`:
 ```ts
 /**
 * Rebuilds every accepted file so it carries no metadata (#226).
 *
 * After verification, deliberately: re-encoding a file whose bytes do not match
 * its declared type would be doing work on something already refused, and
 * sharp's own error would replace the clearer message that check produces.
 *
 * A failure here refuses the upload rather than storing the original. Storing
 * it would mean the one case where a photo keeps its coordinates is the case
 * nobody was told about.
 */
 async function stripUploadedImages(req: Request): Promise<string | null> {
  const files = (req.files as Express.Multer.File[]) || [];
  for (const file of files) {
    try {
      await reencodeInPlace(file.path, file.mimetype);
    } catch (err) {
      console.error(`[upload] could not re-encode ${file.path}:`, err);
      return `${file.originalname} could not be processed`;
    }
  }
  return null;
 }
 ```
 Then extend the chain at the end of `uploadImages` — this is the single choke point every upload route passes through, so this covers the admin routes and any route added later:
 ```ts
    verifyUploadedImages(req)
      .then((problem) => {
        if (problem) {
          res.status(400).json({ error: problem });
          return null;
        }
        return stripUploadedImages(req);
      })
      .then((problem) => {
        // null from either stage means "already answered" or "nothing wrong";
        // distinguish by whether a response has gone out.
        if (res.headersSent) return;
        if (problem) {
          res.status(400).json({ error: problem });
          return;
        }
        next();
      })
      .catch(next);
 ```
 - [ ] **Step 4: Run the tests**
 ```bash
 cd backend
 npx jest -c jest.integration.config.js --runInBand exifStripping
 npm run test:unit
 npm run test:integration
 npm run lint && npm run build
 ```
 Expected: PASS throughout. `uploadValidation` and `adminInventory` matter most — they exercise this middleware hardest, and the refusal paths must still refuse with the same messages.
 - [ ] **Step 5: Commit**
 ```bash
 git add backend/src/routes/admin.ts backend/tests/integration/exifStripping.integration.test.ts
 git commit -m "feat(uploads): strip metadata from every accepted upload (#226)"
 ```
 ---
 ### Task 4: Backfill the photos already stored
 Task 3 does nothing for the existing catalogue. Without this, every photo on the storefront today keeps its coordinates.
 **Files:**
 - Create: `backend/scripts/backfill-image-reencode.ts`
 - Modify: `backend/package.json` (script entry)
 - [ ] **Step 1: Write the script**
 Create `backend/scripts/backfill-image-reencode.ts`:
 ```ts
 /**
 * Applies #226's re-encoding to the photos that were stored before it existed.
 *
 * New uploads are handled in the request path. Everything already on the volume
 * still carries whatever the camera wrote, including the coordinates the photo
 * was taken at, and is still served publicly. This is the other half.
 *
 * The transform is lossy and there is no undo, so:
 *
 *   - It reports by default and changes nothing without --apply.
 *   - It is idempotent. `needsProcessing` skips a file that is already stripped
 *     and already within bounds, so a second run is not a second lossy pass.
 *   - It writes to a temporary file and renames, so an interruption cannot
 *     leave a half-written image being served.
 *   - It never renames the stored file, so item_images.image_path stays correct
 *     and no database write is needed at all.
 *
 * Usage
 * -----
 *   npm run backfill:images            # report only
 *   npm run backfill:images -- --apply # rewrite the files
 *
 * Point it at QA first. Compare a handful of images by eye before production,
 * and take a backup that you have confirmed restores.
 */
 import sharp from 'sharp';
 import { promises as fs } from 'fs';
 import path from 'path';
 import { pool } from '../src/db';
 import { needsProcessing, reencodeInPlace } from '../src/imageProcessing';
 const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
 const APPLY = process.argv.includes('--apply');
 // The stored extension is the file's real type — uploadTypes.ts derives it from
 // the validated content type on the way in, so it can be trusted on the way
 // back out. Anything else is a file this application would refuse to serve.
 const TYPE_FOR_EXTENSION: Record<string, string> = {
  '.jpg': 'image/jpeg',
  '.png': 'image/png',
  '.webp': 'image/webp'
 };
 interface Totals {
  seen: number;
  missing: number;
  skipped: number;
  processed: number;
  failed: number;
  bytesBefore: number;
  bytesAfter: number;
 }
 async function run(): Promise<void> {
  const totals: Totals = {
    seen: 0, missing: 0, skipped: 0, processed: 0, failed: 0, bytesBefore: 0, bytesAfter: 0
  };
  const { rows } = await pool.query<{ image_path: string }>(
    `SELECT image_path FROM item_images ORDER BY id`
  );
  console.info(`[backfill] ${rows.length} image rows, ${APPLY ? 'APPLYING' : 'reporting only'}`);
  for (const row of rows) {
    totals.seen++;
    const filePath = path.join(UPLOADS_DIR, path.basename(row.image_path));
    const mimetype = TYPE_FOR_EXTENSION[path.extname(filePath).toLowerCase()];
    if (!mimetype) {
      console.warn(`[backfill] unrecognised extension, skipping: ${row.image_path}`);
      totals.skipped++;
      continue;
    }
    let before: number;
    try {
      before = (await fs.stat(filePath)).size;
    } catch {
      // A row pointing at nothing is a pre-existing inconsistency. Reported
      // rather than fatal: it is not this script's job to fix, and stopping
      // would leave the rest of the catalogue exposed.
      console.warn(`[backfill] file missing for ${row.image_path}`);
      totals.missing++;
      continue;
    }
    try {
      const meta = await sharp(filePath).metadata();
      if (!needsProcessing(meta)) {
        totals.skipped++;
        continue;
      }
      if (!APPLY) {
        console.info(
          `[backfill] would process ${row.image_path} ` +
            `(${meta.width}x${meta.height}, exif ${meta.exif ? 'present' : 'absent'}, ${before} bytes)`
        );
        totals.processed++;
        totals.bytesBefore += before;
        continue;
      }
      await reencodeInPlace(filePath, mimetype);
      const after = (await fs.stat(filePath)).size;
      totals.processed++;
      totals.bytesBefore += before;
      totals.bytesAfter += after;
      console.info(`[backfill] ${row.image_path}: ${before} -> ${after} bytes`);
    } catch (err) {
      console.error(`[backfill] failed on ${row.image_path}:`, err);
      totals.failed++;
    }
  }
  console.info('[backfill] done', totals);
  if (totals.failed > 0) {
    // A non-zero exit so a partial run is visible to whatever invoked it,
    // rather than reading as success because the summary printed.
    process.exitCode = 1;
  }
 }
 run()
  .catch((err) => {
    console.error(err);
    process.exitCode = 1;
  })
  .finally(() => pool.end());
 ```
 - [ ] **Step 2: Add the script entry**
 In `backend/package.json`, beside `bench:hashing`:
 ```json
    "backfill:images": "tsx scripts/backfill-image-reencode.ts",
 ```
 - [ ] **Step 3: Prove it on a local database first**
 Bring up the stack, add an item through `/admin` with a photo, then — because Task 3 now strips on upload — put a file carrying EXIF on the volume by hand so there is something to find:
 ```bash
 cd backend
 node -e "const sharp=require('sharp');const p=process.env.UPLOADS_DIR;sharp({create:{width:3000,height:2000,channels:3,background:{r:1,g:2,b:3}}}).withExif({IFD0:{Make:'TestCam'}}).jpeg().toFile(require('path').join(p,'legacy-test.jpg')).then(()=>console.log('written'))"
 ```
 Point an `item_images` row at it — reusing an item you already created, so the row is otherwise ordinary:
 ```bash
 psql -h localhost -p 55432 -U redefined_test -d redefined_test -c \
  "INSERT INTO item_images (item_id, image_path, sort_order) \
   SELECT id, '/uploads/legacy-test.jpg', 99 FROM items LIMIT 1"
 ```
 Then:
 ```bash
 npm run backfill:images
 ```
 Expected: it reports `would process /uploads/legacy-test.jpg` and changes nothing. Confirm the file's bytes are unchanged.
 - [ ] **Step 4: Apply, then prove idempotency**
 ```bash
 cd backend
 npm run backfill:images -- --apply
 npm run backfill:images -- --apply
 ```
 Expected: the first run processes the file and reports a smaller size; **the second run skips it**. That second run is the whole idempotency property — if it processes again, `needsProcessing` is wrong and the script will degrade images a little more on every run.
 Then confirm the metadata is gone:
 ```bash
 node -e "require('sharp')(process.env.UPLOADS_DIR+'/legacy-test.jpg').metadata().then(m=>console.log({exif:m.exif,width:m.width}))"
 ```
 Expected: `exif: undefined`, `width: 2000`.
 - [ ] **Step 5: Commit**
 ```bash
 git add backend/scripts/backfill-image-reencode.ts backend/package.json
 git commit -m "feat(uploads): backfill re-encoding over already-stored photos (#226)"
 ```
 ---
 ### Task 5: Deploy, QA first
 Not code. This is the part where the irreversible thing happens to real images, and it is written down so it is not improvised.
 - [ ] **Step 1: Deploy to QA and check new uploads**
 Deploy the built image to QA. Upload a photo with EXIF through `/admin`, then fetch it from `/uploads/` and confirm the metadata is gone and the image still looks right.
 - [ ] **Step 2: Report on QA's existing images**
 ```bash
 docker exec <qa-container> npm run backfill:images
 ```
 Read the report. The count of files it would process should be plausible against the size of QA's catalogue — a count of zero, or a count far larger than the catalogue, both mean something is wrong with the path resolution rather than with the images.
 - [ ] **Step 3: Apply on QA, then look at them**
 ```bash
 docker exec <qa-container> npm run backfill:images -- --apply
 ```
 Open the QA storefront and look at several items, including a detail view. This is the only step that checks the thing no assertion covers: whether the images still look good enough to sell from.
 - [ ] **Step 4: Back up production and confirm the backup restores**
 Take the backup. **Confirm it restores** — an untested backup is not a backup, and this is the step whose omission cannot be recovered from. Back up the uploads volume, not only the database: the database rows are unchanged by this script, and the files are the only thing at risk.
 - [ ] **Step 5: Report, then apply, on production**
 ```bash
 docker exec <prod-container> npm run backfill:images
 docker exec <prod-container> npm run backfill:images -- --apply
 ```
 Read the report before applying. Then spot-check the live storefront.
 - [ ] **Step 6: Confirm the exposure is closed**
 Pick an image URL from the live storefront, download it, and read its metadata:
 ```bash
 curl -s https://<host>/uploads/<name>.jpg -o /tmp/check.jpg
 node -e "require('sharp')('/tmp/check.jpg').metadata().then(m=>console.log(m.exif))"
 ```
 Expected: `undefined`. That is the assertion this whole issue exists to be able to make.
 ---
 ## Done when
 - A photo uploaded through `/admin` is stored with no EXIF, bounded to 2000px, in its original format.
 - A small image is not enlarged.
 - An upload that cannot be re-encoded is refused rather than stored unmodified.
 - `npm run backfill:images` reports without changing anything; `--apply` processes; a second `--apply` skips everything it already did.
 - Production images fetched over HTTP carry no EXIF.
 - `npm run test:unit`, `npm run test:integration`, `npm run lint` and `npm run build` all pass in `backend/`.
 ## Not in this issue
 Converting formats for better compression — that changes stored extensions and `item_images.image_path`, and belongs in its own issue if storage becomes the binding constraint. Thumbnails and responsive sizes. Any change to what the storefront requests.
@@ -0,0 +1,206 @@
 # Intake Pipeline — Design
 **Issue:** [#220 — Shared upload links, AI-drafted listings, and an admin review queue](https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/220)
 **Date:** 2026-08-29
 **Status:** Draft
 ## Goal
 Someone who is not the admin can send in photos of one item through a link they were given. A draft listing is written for it automatically. The admin is told by email, and the item sits unpublished until the admin has read the copy, checked the price and published it deliberately.
 ## Where this starts from
 Most of the lifecycle this feature needs is already built, which is why the design below adds a pipeline in front of it rather than a parallel one beside it.
 - **`pending` is already the unpublished state.** `items.status` defaults to `'pending'` since #90, `NON_PUBLIC_STATUSES` keeps it out of every public and storefront query, and `POST /admin/items/:id/mark-available` and `/unpublish` already move an item across that line. Nothing here needs a new visibility concept, and inventing one would mean a second thing to keep correct.
 - **Uploads are already hardened.** Multer writes to `UPLOADS_DIR`, `uploadTypes.ts` checks the declared type against the file's leading bytes, the allowlist is three image types, and `uploads.ts` refuses to serve anything it does not recognise. A new upload path that did not go through those is a new hole.
 - **Mail already exists,** including admin-editable markdown bodies with placeholder validation, the `MAIL_ALLOWLIST` guard that stops non-production environments mailing real people, and `PUBLIC_URL` for building links.
 - **Background work has a pattern.** `cron.schedule('0 9 * * *', ...)` in `server.ts` drives the cart reminders. This is a single-instance deployment, which is what makes in-process scheduling and the in-memory rate-limit store tenable.
 Three things do not exist: a way in for someone without an admin account, any LLM integration at all, and anywhere to review a draft.
 ## Decisions
 Settled in conversation before this was written, and recorded on the issue.
 | Question | Decision |
 | --- | --- |
 | What the "shared folder" is | A web upload page, not a watched NAS share or a mailbox |
 | Who can upload | Anyone holding a named, revocable link — no account |
 | What the uploader supplies | Photos of one item, plus one free-text note |
 | What the AI drafts | Name, marketing description, category and tags from the existing taxonomy, and a suggested price |
 | Where a submission lives pre-approval | An `items` row at `status='pending'`, not a separate submissions table |
 | How the admin acts on it | Signed one-click links in the email, plus a review queue in the admin |
 | Whether one click can publish | No. Publishing always happens from the review queue |
 | What an item is priced at on arrival | The AI's suggestion, or 80.00 when it has none |
 | Model | Sonnet 5, in an env var |
 ## The invariant
 **Nothing reaches the storefront without the admin publishing it from the queue.** The item arrives `pending`, which is already invisible to every public query, and only `mark-available` moves it. The email carries no publish button, so the admin has necessarily seen the item and its price before anything ships.
 The price is the deliberate exception, and it is worth being precise about what was traded. An arriving item is priced immediately — the model's suggestion if it made one, otherwise 80.00 — so the review queue's price field is pre-filled rather than blank. The admin can change it, but is not forced to, and an unchanged field publishes at a number the admin did not choose.
 That is a real risk and it is accepted knowingly. It is bounded by the queue: publishing is a deliberate act on a screen showing the price, not something that can happen from an inbox or by a timeout. What is given up is the stronger guarantee that a machine-guessed price could never reach the storefront at all.
 Two consequences follow, and both are load-bearing:
 - **80.00 is a plausible price, not an obvious sentinel.** `0` would render as "$0.00" and read as a bug to anyone who saw it; 80.00 renders as a decision. A default that went unnoticed therefore sells the item rather than announcing itself. This is the argument for surfacing it loudly in the queue rather than for choosing a different number.
 - **The queue must make the price's provenance visible.** A price field is not enough. The queue shows whether the number came from the model, from the 80.00 default, or from the admin, so "nobody has looked at this price" is legible at a glance instead of being indistinguishable from a considered one.
 ## Architecture
 ### Schema
 Three migrations.
 ```sql
 CREATE TABLE upload_links (
  id SERIAL PRIMARY KEY,
  label TEXT NOT NULL,
  -- The token is never stored. A leaked database is not also a leaked set of
  -- working upload links, and the admin screen can show a token exactly once,
  -- at creation, for the same reason a password reset link is not re-readable.
  token_hash TEXT NOT NULL UNIQUE,
  revoked_at TIMESTAMPTZ,
  submission_count INTEGER NOT NULL DEFAULT 0,
  -- Null means no cap. A link handed to a regular contributor is open-ended;
  -- one handed out for a single box of stock is not.
  max_submissions INTEGER,
  last_used_at TIMESTAMPTZ,
  created_at TIMESTAMPTZ NOT NULL DEFAULT now()
 );
 CREATE TABLE item_drafts (
  id SERIAL PRIMARY KEY,
  item_id INTEGER NOT NULL UNIQUE REFERENCES items(id) ON DELETE CASCADE,
  -- SET NULL rather than CASCADE: deleting a link must not delete the items
  -- that came in through it. Provenance is lost; the goods are not.
  upload_link_id INTEGER REFERENCES upload_links(id) ON DELETE SET NULL,
  submitter_note TEXT,
  state TEXT NOT NULL DEFAULT 'queued',   -- queued | drafting | ready | failed | discarded
  attempts INTEGER NOT NULL DEFAULT 0,
  model TEXT,
  ai_name TEXT,
  ai_description TEXT,
  ai_category_id INTEGER REFERENCES categories(id) ON DELETE SET NULL,
  ai_tag_names TEXT[],
  -- Kept even though the suggestion is also copied onto the item, so what the
  -- model proposed stays readable after the admin has edited the item's price.
  -- Without it there is no way to ask later whether the model's numbers were
  -- any good.
  ai_suggested_price_cents INTEGER,
  -- ai | default | admin. What the item's current price actually came from.
  -- Derivable by comparing three numbers, but only fragilely: a model that
  -- happens to suggest exactly 8000, or an admin who deliberately types the
  -- model's number, both collapse the comparison. Recorded rather than
  -- inferred, because the queue uses it to say "nobody has chosen this price".
  price_source TEXT NOT NULL DEFAULT 'default',
  ai_error TEXT,
  input_tokens INTEGER,
  output_tokens INTEGER,
  cost_micros INTEGER,
  drafted_at TIMESTAMPTZ,
  reviewed_at TIMESTAMPTZ,
  created_at TIMESTAMPTZ NOT NULL DEFAULT now()
 );
 -- An arriving item is always priced, so the column stays NOT NULL and only
 -- gains a fallback. 80.00 applies when the model declined to suggest anything;
 -- a suggestion, when there is one, is written over it by the worker.
 ALTER TABLE items ALTER COLUMN price_cents SET DEFAULT 8000;
 ```
 **On `price_cents` staying NOT NULL.** The alternative considered was making it nullable so that "no price yet" was expressible, with the publish path refusing an unpriced item. That was rejected in favour of always having a price.
 The practical effect is that this design costs far less to build. `price_cents` is referenced across fifteen files including the cart and the checkout, and every one of them keeps working untouched — the column's type does not change, so nothing downstream has to learn about an item that has no price. The migration adds a default and nothing else.
 What it costs is covered under the invariant above: the storefront can now be reached by a price the admin never chose, so the protection moves from the schema into the queue's presentation, where it is weaker. `price_source` exists to make that presentation possible.
 The number lives in the migration rather than in configuration. Changing a default price is a rare, deliberate act with a record attached, which is the right amount of friction; an env var would let it drift silently between environments, and a wrong default is not visible anywhere until something has already sold at it.
 ### Ingestion
 `POST /api/intake/:token` — public, unauthenticated, `uploadImages` for the files.
 1. Hash the token, look up a matching `upload_links` row that is not revoked. No match, revoked, or over its cap: `404`. Not `403` — whether a link exists is not something a stranger needs to be able to distinguish, which is the reasoning `uploads.ts` already applies.
 2. Validate the photos through the existing `uploadTypes` checks. Caps: at most 10 photos, at most 10 MB each.
 3. In one transaction: insert the `items` row (`status='pending'`, no `price_cents` given so the 8000 default applies, a placeholder name — the submission timestamp — replaced by the draft or the admin), insert `item_images`, insert `item_drafts` at `state='queued'` and `price_source='default'`, bump the link's counter.
 4. Respond immediately. The uploader is told it arrived and that a person will look at it. **The AI is not called here** — a slow or failing API call must not turn into a failed upload for someone who did nothing wrong.
 Rate limited by IP through a new limiter in `rateLimit.ts`. `keyByCallerAndEmail` does not fit — there is no email — so this needs its own key function over `ipKeyGenerator` alone, and the existing comment about a bare `ip:` bucket being a shared allowance applies and is accepted here: the link itself is the per-caller identity, and its counter is the per-caller cap.
 ### The drafting worker
 A new `backend/src/intakeDrafting.ts`, driven both by a call at the end of a successful submission and by a `node-cron` sweeper that picks up anything left `queued` or stuck in `drafting`. The sweeper is what makes a restart mid-draft recoverable rather than a permanently stalled row.
 The request, via `@anthropic-ai/sdk` and `client.messages.parse()` with `zodOutputFormat`, so the shape is validated rather than parsed out of prose:
 - Every photo as a base64 `image` block.
 - The submitter's note verbatim, clearly framed as **the only trustworthy factual claims available**. The system prompt says plainly: describe what is visible, use the note for anything not visible, and never state a material, age, maker or provenance that is in neither. On a one-of-a-kind item, an invented "1930s hand-thrown stoneware" is not a cosmetic error — it is a false claim on a storefront.
 - The existing categories and tags as the closed set to choose from, so the draft lands inside the taxonomy the filters already work on.
 - The suggested price framed as a rough starting point.
 The note is untrusted input from an unauthenticated stranger. It is passed as data, and nothing the model returns is executed, interpolated into SQL, or rendered as HTML — the drafted description goes through the same markdown-with-`html: false` treatment as every other stored body, which is what makes "a model wrote this" and "a person wrote this" equally safe to render.
 Result: `state='ready'`, the fields populated, token counts and computed cost recorded. Where the model returned a price, it is written onto the item and `price_source='ai'`; where it did not, the item keeps the 8000 default and `price_source` stays `'default'`. The suggested price is recorded on the draft either way.
 Failure: `attempts` incremented, `ai_error` stored, and after three attempts `state='failed'` — which still leaves a perfectly good pending item with photos in the review queue, just with no draft copy and priced at the default. A failed AI call must never lose someone's submission.
 **Spend ceiling.** A monthly cap (`INTAKE_MONTHLY_BUDGET_USD`) summed from `cost_micros`. Past it, submissions still save and still notify; only the API call is skipped, with `state='failed'` and an error saying so. The ceiling protects the bill, and it must not be the thing that loses inventory.
 ### Notification and signed links
 Reuses `emailTemplates.ts` with a new `intakeDraft` key, so the copy is editable from the settings screen like every other template, with `reviewUrl` required.
 Action links are HMAC-signed with a new `INTAKE_ACTION_SECRET` over `(draftId, action, expiry)`, verified with a timing-safe comparison, good for 30 days.
 - **Regenerate** — back to `state='queued'`.
 - **Discard** — `state='discarded'` and the item unpublished; recoverable from the queue, because a one-click destructive action reachable from an inbox should not be final.
 - **Review & publish** — an ordinary deep link into the admin queue, behind authentik like the rest of `/admin`. It carries no signature and grants nothing.
 The two signed actions are deliberately the ones whose worst case is a wasted API call or a recoverable hide. Nothing a single click can do puts an item on the storefront.
 ### The review queue
 A new admin screen listing drafts by state: photos, the submitter's note, which link it came from, the drafted copy in editable fields, the price, and Publish / Regenerate / Discard. Publish writes the edited values onto the item and calls the existing `mark-available`.
 The price field carries the weight the schema no longer does, so it is not an ordinary input. It is pre-filled, and it is labelled with where the number came from — the model, or the 80.00 default, or the admin — with anything that is not `price_source='admin'` marked visibly as unconfirmed. Editing it sets `price_source='admin'`. Publishing something still marked unconfirmed is allowed, because that is the decision taken, but it says so plainly at the point of publishing rather than after.
 Reuses the existing admin item components where they fit rather than growing a second item editor.
 ### Configuration
 New variables, added to `envValidation.ts` and — per #107, enforced by `composeEnvironment.test.ts` — to both compose files if they go on the required list.
 | Variable | Requirement |
 | --- | --- |
 | `ANTHROPIC_API_KEY` | Required when intake is enabled |
 | `INTAKE_MODEL` | Optional, defaults to `claude-sonnet-5` |
 | `INTAKE_MONTHLY_BUDGET_USD` | Optional, defaults to `20` |
 | `INTAKE_ACTION_SECRET` | Required when intake is enabled |
 Intake is feature-flagged off by default, so none of these become required for an environment that does not run it. `PUBLIC_URL` is already required alongside SMTP and is what the review links are built from.
 ## Error handling
 The through-line: **a submission is the only irreplaceable thing here.** Photos of a one-of-a-kind object may not be retakeable — the item may not be in the sender's hands any more. Every failure mode below is arranged so the photos survive it.
 | Failure | Behaviour |
 | --- | --- |
 | API call fails or times out | Retry to three attempts, then `state='failed'`. Item and photos intact, reviewable with no draft, priced at the default |
 | Budget exhausted | Submission saved, draft skipped, admin still notified, item priced at the default |
 | Malformed model output | Rejected by the schema, counts as a failed attempt |
 | SMTP down | Draft still `ready` and visible in the queue; the queue, not the email, is the source of truth |
 | Link revoked mid-upload | `404`. Already-submitted items are unaffected |
 | Restart mid-draft | The cron sweeper re-queues anything stuck in `drafting` |
 ## Testing
 - **Unit** — token hashing and constant-time verification, HMAC signing and expiry, the budget calculation, the prompt builder's handling of an empty note, and the `price_source` transitions. All pure, in the style of `keyByCallerAndEmail` and `isAllowedRecipient` being exported specifically to be tested directly.
 - **Integration** — submission through a valid link creates a pending item with images, a queued draft and the 8000 default; a model suggestion overwrites that price and sets `price_source='ai'`; a failed or budget-skipped draft leaves the default in place; a revoked link gets `404`; a signed action link works once and an expired or tampered one does not. The Anthropic client is stubbed; no test spends money.
 - **E2E** — the submission page, and the queue round trip from draft to published. Assertions scoped to the item under test rather than the whole grid, since the dev database never truncates.
 ## Out of scope
 Contributor accounts, a watched NAS folder, email-in submission, multi-item submissions, image editing or cropping, and regenerating with a steer ("try again, warmer"). All are additive later; none change the schema above.