diff --git a/docs/superpowers/plans/2026-08-29-intake-upload-links.md b/docs/superpowers/plans/2026-08-29-intake-upload-links.md new file mode 100644 index 0000000..77e8b0c --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-intake-upload-links.md @@ -0,0 +1,1804 @@ +# Intake — Upload Links and Submission Page 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:** Someone holding a named, revocable link can send in photos of one item plus a note, and it arrives in the admin inventory as a pending item. + +**Architecture:** A new `upload_links` table holds hashed tokens the admin issues and revokes. A public, rate-limited endpoint accepts a multipart submission through the *existing* validated image-upload pipeline — extracted from `routes/admin.ts` into a shared module rather than duplicated — and writes an `items` row at `status='pending'` with its images and an `item_drafts` row carrying the note and provenance. No AI and no email in this slice: the submission lands in the inventory screen that already exists. + +**Tech Stack:** Express 4 + TypeScript, Postgres via `pg`, `node-pg-migrate`, multer, `express-rate-limit`, Jest + supertest (backend), React + antd + Vite, Vitest + Playwright (frontend). + +**Spec:** `docs/superpowers/specs/2026-08-29-intake-pipeline-design.md` + +## Global Constraints + +- **Node 20+ required** for any `npm run migrate:*` command. `node-pg-migrate` pulls an `lru-cache` calling `diagnostics_channel.tracingChannel()`, absent before Node 19.9; on Node 18 it dies inside minified library code. Use `scripts/start-local.ps1`, which handles the version switch. +- **Every route handler must be wrapped in `asyncRoute`.** `tests/unit/routesAreWrapped.test.ts` enforces this by scanning source and will fail the build otherwise. Express 4 does not forward rejected promises; an unwrapped async handler hangs the request forever. +- **antd imports are deep ESM paths** — `import Button from 'antd/es/button'`. Never `import { Button } from 'antd'`. Note this repo uses `antd/es/`, not `antd/lib/`. +- **Uploaded files must go through the existing validation** — allowlist of `image/jpeg`, `image/png`, `image/webp`; magic-byte check after write; stored filename from `randomUUID()` plus an extension derived from the validated type, never from `originalname`. +- **Image caps are the existing constants**, not new ones: `MAX_IMAGES_PER_REQUEST = 6`, `MAX_IMAGE_BYTES = 8_000_000`. (The spec proposed 10 and 10 MB; those were invented numbers and are superseded by the values already in the codebase. One set of caps, not two.) +- **SQL uses bound parameters.** Never format a caller-supplied value into a query string. +- **Commit style:** Conventional Commits, subject ending `(#NNN)` with the sub-issue number, no hard wrapping in bodies. + +## Deviations from the spec, and why + +| Spec says | This plan does | Why | +| --- | --- | --- | +| Caps of 10 photos / 10 MB | Reuses existing 6 / 8 MB constants | The codebase already defines these for the admin upload path. Two different caps for the same pipeline is a bug waiting to happen | +| "Three migrations" | One migration file | All three DDL changes ship together and a partial application is meaningless. node-pg-migrate runs a file in one transaction | +| Intake feature-flagged off | No flag in this slice | Nothing is reachable until the admin creates a link, and an absent or revoked token 404s. The flag earns its keep in slice 2, where a paid API call appears | + +## File Structure + +**Created:** +- `backend/migrations/1787500000000_add-intake-pipeline.js` — `upload_links`, `item_drafts`, and the `items.price_cents` default +- `backend/src/imageUpload.ts` — the shared multer pipeline, extracted from `routes/admin.ts` +- `backend/src/uploadLinks.ts` — token generation and hashing, pure +- `backend/src/routes/adminUploadLinks.ts` — admin CRUD and revoke +- `backend/src/routes/intake.ts` — the public submission endpoint +- `backend/tests/unit/uploadLinks.test.ts` +- `backend/tests/unit/imageUpload.test.ts` +- `backend/tests/integration/uploadLinks.integration.test.ts` +- `backend/tests/integration/intake.integration.test.ts` +- `frontend/src/intake/Submit.tsx` — the public submission page +- `frontend/src/intake/intakeApi.ts` +- `frontend/src/admin/UploadLinks.tsx` — the admin link-management screen + +**Modified:** +- `backend/src/routes/admin.ts` — the upload pipeline moves out; imports it back in +- `backend/src/rateLimit.ts` — a limiter keyed on caller alone +- `backend/src/app.ts` — mounts the two new routers +- `backend/tests/integration/setup/testDb.ts` — new tables in the truncate list +- `frontend/src/main.tsx` — the `/submit/:token` route +- `frontend/src/admin/Admin.tsx` — a tab for upload links + +--- + +### Task 1: Schema + +**Files:** +- Create: `backend/migrations/1787500000000_add-intake-pipeline.js` +- Modify: `backend/tests/integration/setup/testDb.ts:31-39` + +**Interfaces:** +- Consumes: nothing +- Produces: tables `upload_links` and `item_drafts`; `items.price_cents` defaults to `8000` + +- [ ] **Step 1: Write the migration** + +Create `backend/migrations/1787500000000_add-intake-pipeline.js`: + +```js +exports.up = (pgm) => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS upload_links ( + id SERIAL PRIMARY KEY, + label TEXT NOT NULL, + -- The token itself is never stored, only its digest. A leaked database + -- is then 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 IF NOT EXISTS 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 arrived 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', + 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[], + ai_suggested_price_cents INTEGER, + -- ai | default | admin. Where the item's current price came from. + -- Recorded rather than inferred: a model that happens to suggest exactly + -- 8000, or an admin who deliberately types the model's number, both + -- collapse any comparison-based guess. + 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() + ); + + -- The queue reads by state, and everything else reads by item. + CREATE INDEX IF NOT EXISTS item_drafts_state_idx ON item_drafts (state); + + -- A submitted item is priced on arrival rather than left unpriced, so the + -- column keeps NOT NULL and only gains a fallback. The AI columns above + -- stay null until the drafting worker exists. + ALTER TABLE items ALTER COLUMN price_cents SET DEFAULT 8000; + `); +}; + +exports.down = (pgm) => { + pgm.sql(` + ALTER TABLE items ALTER COLUMN price_cents DROP DEFAULT; + DROP TABLE IF EXISTS item_drafts; + DROP TABLE IF EXISTS upload_links; + `); +}; +``` + +- [ ] **Step 2: Add the new tables to the integration truncate list** + +In `backend/tests/integration/setup/testDb.ts`, the `TRUNCATE` in `resetDb()` becomes — note `item_drafts` and `upload_links` lead, since `item_drafts` references `items`: + +```ts + await testPool.query(` + TRUNCATE TABLE item_drafts, upload_links, orders, checkout_items, checkouts, shipping_addresses, + cart_items, carts, customer_tokens, customer_sessions, favorites, customers, item_tags, + item_images, items, tags, categories + RESTART IDENTITY CASCADE + `); +``` + +- [ ] **Step 3: Run the migration up and back down** + +```bash +cd backend +npm run migrate:up +npm run migrate:down +npm run migrate:up +``` + +Expected: three clean runs. The down must succeed — an irreversible migration is a migration that cannot be tested. + +- [ ] **Step 4: Confirm the existing suite still passes** + +```bash +cd backend && npm run test:integration +``` + +Expected: PASS. The price default is additive, so nothing existing should shift. + +- [ ] **Step 5: Commit** + +```bash +git add backend/migrations/1787500000000_add-intake-pipeline.js backend/tests/integration/setup/testDb.ts +git commit -m "feat(intake): add upload_links and item_drafts, and default an item's price (#NNN)" +``` + +--- + +### Task 2: Extract the shared image-upload pipeline + +This is a pure refactor. No behaviour changes; the existing tests are the safety net. + +**Files:** +- Create: `backend/src/imageUpload.ts` +- Modify: `backend/src/routes/admin.ts:1-300` + +**Interfaces:** +- Consumes: `uploadTypes.ts` (`ALLOWED_IMAGE_TYPES`, `SIGNATURE_BYTES`, `extensionFor`, `isAllowedImageType`, `signatureMatches`) +- Produces: + - `uploadImages: (req, res, next) => void` — multer middleware, field name `images` + - `verifyUploadedImages(req: Request): Promise` — refusal message, or null + - `insertItemImages(client: PoolClient, itemId: number, files: Express.Multer.File[], firstSortOrder: number): Promise` + - `MAX_IMAGES_PER_REQUEST: number`, `MAX_IMAGE_BYTES: number` + +- [ ] **Step 1: Run the upload tests first, to know they pass before you touch anything** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand uploadValidation adminInventory +``` + +Expected: PASS. Record this — it is the comparison for Step 4. + +- [ ] **Step 2: Move the code** + +Create `backend/src/imageUpload.ts` and move into it, unchanged, from `routes/admin.ts`: `UnsupportedImageTypeError`, `UPLOADS_DIR`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`, `MAX_TEXT_FIELDS`, `MAX_TEXT_FIELD_BYTES`, `storage`, `upload`, `readHead`, `discardUploads`, `discardUnlessAccepted`, `verifyUploadedImages`, `uploadImages`, and `insertItemImages`. + +Keep every existing comment verbatim. They record why the code is shaped as it is (#95, #103, #180) and are the most valuable thing being moved. + +Export `uploadImages`, `verifyUploadedImages`, `insertItemImages`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`. Everything else stays module-private. + +Add this at the top of the new file: + +```ts +/** + * The one validated path from a multipart request to files on the uploads + * volume. + * + * Extracted from routes/admin.ts when a second caller appeared (#220's public + * intake endpoint). It is deliberately one module rather than two similar ones: + * every property that makes uploads safe here — the type allowlist, the + * magic-byte check after the write, names from a CSPRNG rather than from + * `originalname`, and the cleanup of anything a failed request left behind — + * is a property a second implementation would have to reproduce exactly. A + * near-copy that drifted would be precisely the gap #95 and #103 exist to + * close. + */ +``` + +- [ ] **Step 3: Import them back into `routes/admin.ts`** + +```ts +import { + uploadImages, + verifyUploadedImages, + insertItemImages, + MAX_IMAGES_PER_REQUEST +} from '../imageUpload'; +``` + +Remove the now-unused imports from `../uploadTypes`, `multer`, `fs`, and `randomUUID` from `routes/admin.ts` — but only those genuinely no longer referenced. Let `npm run lint` tell you which. + +- [ ] **Step 4: Run the same tests and the whole suite** + +```bash +cd backend +npm run lint +npm run build +npx jest -c jest.integration.config.js --runInBand uploadValidation adminInventory +npm run test:unit +``` + +Expected: identical results to Step 1, plus a clean lint and build. `routesAreWrapped` must still pass. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/imageUpload.ts backend/src/routes/admin.ts +git commit -m "refactor(uploads): extract the validated image pipeline for a second caller (#NNN)" +``` + +--- + +### Task 3: Token generation and hashing + +**Files:** +- Create: `backend/src/uploadLinks.ts` +- Test: `backend/tests/unit/uploadLinks.test.ts` + +**Interfaces:** +- Consumes: nothing +- Produces: + - `generateToken(): string` — 43-character base64url, 256 bits of CSPRNG entropy + - `hashToken(token: string): string` — 64-character lowercase hex SHA-256 + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/unit/uploadLinks.test.ts`: + +```ts +import { generateToken, hashToken } from '../../src/uploadLinks'; + +describe('generateToken', () => { + it('produces a URL-safe token with no padding', () => { + expect(generateToken()).toMatch(/^[A-Za-z0-9_-]{43}$/); + }); + + // The token is the entire access control on the intake endpoint. If two + // calls could collide, one person's link would open another's. + it('does not repeat', () => { + const seen = new Set(Array.from({ length: 1000 }, () => generateToken())); + expect(seen.size).toBe(1000); + }); +}); + +describe('hashToken', () => { + it('is a lowercase hex sha256 digest', () => { + expect(hashToken('abc')).toBe( + 'ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad' + ); + }); + + it('is stable across calls, so a stored digest keeps matching', () => { + const token = generateToken(); + expect(hashToken(token)).toBe(hashToken(token)); + }); + + it('gives different tokens different digests', () => { + expect(hashToken(generateToken())).not.toBe(hashToken(generateToken())); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.unit.config.js uploadLinks +``` + +Expected: FAIL — `Cannot find module '../../src/uploadLinks'`. + +- [ ] **Step 3: Write the implementation** + +Create `backend/src/uploadLinks.ts`: + +```ts +import crypto from 'crypto'; + +/** + * Issuing and recognising the tokens that open the public intake endpoint. + * + * Kept apart from the routes so the rules are pure and testable directly, the + * same reasoning as `uploadTypes.ts` and `keyByCallerAndEmail`. + */ + +// 32 bytes — 256 bits. base64url so the value survives being pasted into a URL, +// a chat message and a QR code without escaping. +const TOKEN_BYTES = 32; + +export function generateToken(): string { + return crypto.randomBytes(TOKEN_BYTES).toString('base64url'); +} + +/** + * The digest stored against a link. + * + * SHA-256 rather than bcrypt, deliberately. A password hash is slow on purpose + * because a human password has little entropy and must survive an offline + * dictionary attack. This is 256 bits from a CSPRNG: there is no dictionary, + * and guessing is not a threat that slowing the hash addresses. Meanwhile the + * digest is computed on every submission request, so a deliberately slow hash + * would be a denial-of-service surface on an unauthenticated endpoint. + * + * No timing-safe comparison is needed here because the lookup is an indexed + * equality match on the digest, not a byte-by-byte compare of the secret — and + * an attacker who could mount a timing attack against a 256-bit random value + * would still need the value. + */ +export function hashToken(token: string): string { + return crypto.createHash('sha256').update(token).digest('hex'); +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +cd backend && npx jest -c jest.unit.config.js uploadLinks +``` + +Expected: PASS, 5 tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/uploadLinks.ts backend/tests/unit/uploadLinks.test.ts +git commit -m "feat(intake): generate and hash upload link tokens (#NNN)" +``` + +--- + +### Task 4: Admin API for issuing and revoking links + +**Files:** +- Create: `backend/src/routes/adminUploadLinks.ts` +- Modify: `backend/src/app.ts:74-80` +- Test: `backend/tests/integration/uploadLinks.integration.test.ts` + +**Interfaces:** +- Consumes: `generateToken`, `hashToken` from `src/uploadLinks`; `pool`, `requireRow` from `src/db`; `asyncRoute` +- Produces: + - `GET /api/admin/upload-links` → `UploadLinkRow[]`, never including a token + - `POST /api/admin/upload-links` `{label, maxSubmissions?}` → `201` with `{...row, token, url}` — the only time the token is ever returned + - `POST /api/admin/upload-links/:id/revoke` → `200` with the updated row + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/integration/uploadLinks.integration.test.ts`: + +```ts +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +describe('issuing an upload link', () => { + it('returns the token exactly once, at creation', async () => { + const created = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Sarah' }); + + expect(created.status).toBe(201); + expect(created.body.label).toBe('Sarah'); + expect(created.body.token).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(created.body.url).toContain(`/submit/${created.body.token}`); + + const listed = await request(app).get('/api/admin/upload-links'); + expect(listed.status).toBe(200); + expect(listed.body).toHaveLength(1); + // The whole point of storing a digest: the listing cannot leak it back. + expect(listed.body[0].token).toBeUndefined(); + expect(listed.body[0].token_hash).toBeUndefined(); + }); + + it('stores the digest rather than the token', async () => { + const created = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Estate sale box 3' }); + + const { rows } = await pool.query(`SELECT token_hash FROM upload_links`); + expect(rows[0].token_hash).not.toBe(created.body.token); + expect(rows[0].token_hash).toMatch(/^[a-f0-9]{64}$/); + }); + + it('refuses a link with no label', async () => { + const res = await request(app).post('/api/admin/upload-links').send({ label: ' ' }); + expect(res.status).toBe(400); + }); + + it('refuses a non-positive submission cap', async () => { + const res = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Bad cap', maxSubmissions: 0 }); + expect(res.status).toBe(400); + }); +}); + +describe('revoking an upload link', () => { + it('stamps revoked_at and reports it in the listing', async () => { + const created = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Temporary' }); + + const revoked = await request(app) + .post(`/api/admin/upload-links/${created.body.id}/revoke`); + + expect(revoked.status).toBe(200); + expect(revoked.body.revoked_at).not.toBeNull(); + }); + + it('is idempotent, so a second click is not an error', async () => { + const created = await request(app) + .post('/api/admin/upload-links') + .send({ label: 'Temporary' }); + + await request(app).post(`/api/admin/upload-links/${created.body.id}/revoke`); + const second = await request(app).post(`/api/admin/upload-links/${created.body.id}/revoke`); + + expect(second.status).toBe(200); + }); + + it('404s for a link that does not exist', async () => { + const res = await request(app).post('/api/admin/upload-links/9999/revoke'); + expect(res.status).toBe(404); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand uploadLinks.integration +``` + +Expected: FAIL — 404s, because the router is not mounted. + +- [ ] **Step 3: Write the router** + +Create `backend/src/routes/adminUploadLinks.ts`: + +```ts +import { Router, Request, Response } from 'express'; +import { pool, requireRow } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { generateToken, hashToken } from '../uploadLinks'; + +const router = Router(); + +/** + * Issuing and retiring the links that open the public intake endpoint. + * + * A link is named because provenance matters more than convenience here: when + * one leaks, the question is which one, and the answer has to come from + * somewhere. The token is returned by exactly one response in this file and is + * unrecoverable afterwards, which is why the admin screen has to present it as + * a one-time reveal rather than a field to come back to. + */ + +/** Shaped so a `SELECT *` can never leak the digest into a response. */ +const LINK_SELECT = ` + SELECT id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at + FROM upload_links +`; + +interface UploadLinkRow { + id: number; + label: string; + revoked_at: string | null; + submission_count: number; + max_submissions: number | null; + last_used_at: string | null; + created_at: string; +} + +router.get('/', asyncRoute(async (_req: Request, res: Response) => { + const { rows } = await pool.query(`${LINK_SELECT} ORDER BY created_at DESC`); + res.json(rows); +})); + +router.post('/', asyncRoute(async (req: Request, res: Response) => { + const label = typeof req.body?.label === 'string' ? req.body.label.trim() : ''; + if (label === '') { + return res.status(400).json({ error: 'a label is required' }); + } + + // Absent means no cap, which is a different thing from a cap of zero — a + // link that can never be used is a mistake rather than an intent. + const rawCap = req.body?.maxSubmissions; + let maxSubmissions: number | null = null; + if (rawCap !== undefined && rawCap !== null && rawCap !== '') { + const parsed = Number(rawCap); + if (!Number.isInteger(parsed) || parsed < 1) { + return res.status(400).json({ error: 'maxSubmissions must be a positive whole number' }); + } + maxSubmissions = parsed; + } + + const token = generateToken(); + const { rows } = await pool.query( + `INSERT INTO upload_links (label, token_hash, max_submissions) + VALUES ($1, $2, $3) + RETURNING id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at`, + [label, hashToken(token), maxSubmissions] + ); + const link = requireRow(rows, 'the upload_links INSERT'); + + // PUBLIC_URL is already required alongside SMTP and is what every other + // outbound link is built from. Empty in a local environment, which yields a + // relative URL the admin screen can still render usefully. + const base = (process.env.PUBLIC_URL ?? '').replace(/\/+$/, ''); + res.status(201).json({ ...link, token, url: `${base}/submit/${token}` }); +})); + +router.post('/:id/revoke', asyncRoute(async (req: Request, res: Response) => { + // COALESCE so revoking twice keeps the original timestamp: the useful fact is + // when access ended, and a second click should not rewrite that. + const { rows } = await pool.query( + `UPDATE upload_links SET revoked_at = COALESCE(revoked_at, now()) + WHERE id = $1 + RETURNING id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at`, + [req.params.id] + ); + + if (rows.length === 0) { + return res.status(404).json({ error: 'not found' }); + } + res.json(rows[0]); +})); + +export default router; +``` + +- [ ] **Step 4: Mount it** + +In `backend/src/app.ts`, beside the other admin routers and **above** the catch-all `app.use('/api/admin', ...)`: + +```ts +import adminUploadLinksRouter from './routes/adminUploadLinks'; +``` + +```ts +app.use('/api/admin/upload-links', requireAdminGate, adminUploadLinksRouter); +``` + +Order matters: `/api/admin` is mounted last and would otherwise swallow the path. `requireAdminGate` goes on the router itself, per the reasoning in `middleware/adminGate.ts`. + +- [ ] **Step 5: Run the tests** + +```bash +cd backend +npx jest -c jest.integration.config.js --runInBand uploadLinks.integration +npm run test:unit +npm run lint && npm run build +``` + +Expected: PASS, 7 integration tests. `routesAreWrapped` must pass — every handler above is wrapped. + +- [ ] **Step 6: Commit** + +```bash +git add backend/src/routes/adminUploadLinks.ts backend/src/app.ts backend/tests/integration/uploadLinks.integration.test.ts +git commit -m "feat(intake): issue and revoke named upload links (#NNN)" +``` + +--- + +### Task 5: The public submission endpoint + +**Files:** +- Create: `backend/src/routes/intake.ts` +- Modify: `backend/src/rateLimit.ts` (append), `backend/src/app.ts` +- Test: `backend/tests/integration/intake.integration.test.ts` + +**Interfaces:** +- Consumes: `uploadImages`, `verifyUploadedImages`, `insertItemImages` from `src/imageUpload`; `hashToken` from `src/uploadLinks`; `intakeLimiter` from `src/rateLimit` +- Produces: + - `GET /api/intake/:token` → `{label}` for a usable link, else `404` + - `POST /api/intake/:token` (multipart: `images[]`, `note`) → `201 {ok: true}`, else `404` / `400` + - `keyByCaller(req: Request): string` exported from `rateLimit.ts` + +- [ ] **Step 1: Write the failing test** + +Create `backend/tests/integration/intake.integration.test.ts`: + +```ts +import request from 'supertest'; +import { promises as fs } from 'fs'; +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; + +// The same 1x1 PNG the upload validation suite uses, so the accepted case +// exercises the whole path rather than a buffer that merely starts right. +const PNG = Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', + 'base64' +); + +// multer.diskStorage does not create its destination. +beforeAll(async () => { + await fs.mkdir(UPLOADS_DIR, { recursive: true }); +}); + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +async function storedFiles(): Promise { + return fs.readdir(UPLOADS_DIR); +} + +async function issueLink(label = 'Sarah', maxSubmissions?: number): Promise { + const res = await request(app) + .post('/api/admin/upload-links') + .send({ label, ...(maxSubmissions === undefined ? {} : { maxSubmissions }) }); + expect(res.status).toBe(201); + return res.body.token; +} + +describe('checking a link before showing the form', () => { + it('names the link so the page can greet the sender', async () => { + const token = await issueLink('Sarah'); + const res = await request(app).get(`/api/intake/${token}`); + + expect(res.status).toBe(200); + expect(res.body.label).toBe('Sarah'); + }); + + // 404 rather than 403 throughout: whether a link exists is not something a + // stranger needs to be able to distinguish. Same reasoning as uploads.ts. + it('404s an unknown token', async () => { + const res = await request(app).get('/api/intake/not-a-real-token'); + expect(res.status).toBe(404); + }); + + it('404s a revoked link', async () => { + const token = await issueLink(); + const { rows } = await pool.query(`SELECT id FROM upload_links`); + await request(app).post(`/api/admin/upload-links/${rows[0].id}/revoke`); + + const res = await request(app).get(`/api/intake/${token}`); + expect(res.status).toBe(404); + }); +}); + +describe('submitting an item', () => { + it('creates a pending item with its images, note and provenance', async () => { + const token = await issueLink('Sarah'); + + const res = await request(app) + .post(`/api/intake/${token}`) + .field('note', 'Hand-thrown stoneware, chip on the base') + .attach('images', PNG, 'front.png') + .attach('images', PNG, 'back.png'); + + expect(res.status).toBe(201); + expect(res.body.ok).toBe(true); + + const { rows: items } = await pool.query( + `SELECT id, status, price_cents FROM items` + ); + expect(items).toHaveLength(1); + expect(items[0].status).toBe('pending'); + // The default from Task 1, not a price anyone chose. + expect(items[0].price_cents).toBe(8000); + + const { rows: images } = await pool.query( + `SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`, + [items[0].id] + ); + expect(images).toHaveLength(2); + expect(images[0].image_path).toMatch(/^\/uploads\/[a-f0-9-]+\.png$/); + + const { rows: drafts } = await pool.query( + `SELECT submitter_note, state, price_source, upload_link_id FROM item_drafts WHERE item_id = $1`, + [items[0].id] + ); + expect(drafts[0].submitter_note).toBe('Hand-thrown stoneware, chip on the base'); + expect(drafts[0].state).toBe('queued'); + expect(drafts[0].price_source).toBe('default'); + expect(drafts[0].upload_link_id).not.toBeNull(); + }); + + it('counts the submission against the link', async () => { + const token = await issueLink(); + await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + + const { rows } = await pool.query(`SELECT submission_count, last_used_at FROM upload_links`); + expect(rows[0].submission_count).toBe(1); + expect(rows[0].last_used_at).not.toBeNull(); + }); + + it('refuses a submission with no photos', async () => { + const token = await issueLink(); + const res = await request(app).post(`/api/intake/${token}`).field('note', 'nothing attached'); + + expect(res.status).toBe(400); + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(0); + }); + + // The file is named .png and declared image/png, but the bytes are not. + // This is the check that cannot happen before the write. + it('refuses a file whose bytes disagree with its type', async () => { + const token = await issueLink(); + const res = await request(app) + .post(`/api/intake/${token}`) + .attach('images', Buffer.from('not an image'), { + filename: 'evil.png', + contentType: 'image/png' + }); + + expect(res.status).toBe(400); + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(0); + }); + + it('404s a revoked link without creating anything', async () => { + const token = await issueLink(); + const { rows: links } = await pool.query(`SELECT id FROM upload_links`); + await request(app).post(`/api/admin/upload-links/${links[0].id}/revoke`); + + const res = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + + expect(res.status).toBe(404); + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(0); + }); + + // The reason `requireUsableLink` is ordered ahead of `uploadImages`. Without + // that ordering this still returns 404 and still creates no item — the bytes + // just reach the disk first and are deleted afterwards. This asserts they + // never arrive, so a future reordering of the middleware fails here rather + // than quietly handing an unauthenticated caller control of disk churn. + it('writes nothing to the uploads volume for a token that does not work', async () => { + const before = await storedFiles(); + + const res = await request(app) + .post('/api/intake/not-a-real-token') + .attach('images', PNG, 'a.png'); + + expect(res.status).toBe(404); + expect(await storedFiles()).toEqual(before); + }); + + it('stops accepting once the link hits its cap', async () => { + const token = await issueLink('One shot', 1); + + const first = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + expect(first.status).toBe(201); + + const second = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'b.png'); + expect(second.status).toBe(404); + + const { rows } = await pool.query(`SELECT id FROM items`); + expect(rows).toHaveLength(1); + }); +}); + +describe('a submitted item does not reach the storefront', () => { + it('is absent from the public catalogue', async () => { + const token = await issueLink(); + await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png'); + + const res = await request(app).get('/api/items'); + expect(res.status).toBe(200); + expect(res.body).toHaveLength(0); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand intake.integration +``` + +Expected: FAIL — 404s on every intake path, since the router is not mounted. + +- [ ] **Step 3: Add the rate limiter** + +Append to `backend/src/rateLimit.ts`: + +```ts +/** + * Keyed on the caller alone, because an intake submission carries no email. + * + * The comment on `keyByCallerAndEmail` warns that a bare `ip:` bucket is a + * shared allowance rather than a per-caller one, and that is accepted here: the + * link is the per-caller identity, and its `submission_count` and + * `max_submissions` are the per-caller cap. This limiter exists for a different + * job — bounding what a single address can throw at an unauthenticated endpoint + * that writes files to disk. + */ +export function keyByCaller(req: Request): string { + return ipKeyGenerator(req.ip ?? ''); +} + +// Deliberately looser than the password-reset allowance. Someone photographing +// a box of stock legitimately submits several items in a row, and the cost of +// refusing them is a lost consignment. +export const intakeLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 20, + keyGenerator: keyByCaller, + standardHeaders: true, + legacyHeaders: false, + message: { error: 'too many submissions — please try again later' } +}); +``` + +- [ ] **Step 4: Write the router** + +Create `backend/src/routes/intake.ts`: + +```ts +import { Router, Request, Response, NextFunction } from 'express'; +import { pool, requireRow } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { hashToken } from '../uploadLinks'; +import { uploadImages, verifyUploadedImages, insertItemImages } from '../imageUpload'; +import { intakeLimiter } from '../rateLimit'; + +const router = Router(); + +/** + * The public way in: photos of one item, from someone with no account. + * + * Everything here is reachable by a stranger holding a URL, so the shape of + * every refusal matters. Refusals are 404 rather than 403 throughout — + * unknown, revoked and exhausted links are indistinguishable from the outside, + * because whether a link exists is not something a stranger needs to be able + * to learn. That is the same reasoning `uploads.ts` applies to files. + * + * The AI is deliberately not called here. A slow or failing model request must + * not turn into a failed upload for someone who did nothing wrong, and the + * photos may be the only copy — the item is often no longer in the sender's + * hands. The row is left at `state='queued'` for the worker (#220, slice 2). + */ + +interface LinkRow { + id: number; + label: string; +} + +/** The link resolved by `requireUsableLink`, carried to the handler. */ +interface IntakeRequest extends Request { + uploadLink?: LinkRow; +} + +/** + * The link a token opens, or null. + * + * The cap is applied in SQL rather than in a later branch so that "usable" is + * one concept with one definition, used identically by the GET and the POST. + */ +async function usableLink(token: string): Promise { + const { rows } = await pool.query( + `SELECT id, label FROM upload_links + WHERE token_hash = $1 + AND revoked_at IS NULL + AND (max_submissions IS NULL OR submission_count < max_submissions)`, + [hashToken(token)] + ); + return rows[0] ?? null; +} + +router.get('/:token', intakeLimiter, asyncRoute(async (req: Request, res: Response) => { + const link = await usableLink(req.params.token); + if (!link) { + return res.status(404).json({ error: 'not found' }); + } + // The label only. Nothing about the catalogue, the admin, or other links. + res.json({ label: link.label }); +})); + +/** + * Resolves the link *before* multer runs, so a stranger holding a bad token + * cannot cause a single byte to be written to the uploads volume. + * + * `discardUnlessAccepted` would delete those files afterwards, but "written + * then deleted" is a materially worse position than "never written" on an + * endpoint the whole internet can reach: it is disk churn an unauthenticated + * caller controls, and it leans on a cleanup that a crash between the write + * and the unlink would skip. Ordering this middleware ahead of `uploadImages` + * is the whole mitigation. + */ +const requireUsableLink = asyncRoute(async (req: Request, res: Response, next: NextFunction) => { + const link = await usableLink(req.params.token); + if (!link) { + res.status(404).json({ error: 'not found' }); + return; + } + (req as IntakeRequest).uploadLink = link; + next(); +}); + +router.post('/:token', intakeLimiter, requireUsableLink, uploadImages, asyncRoute(async (req: Request, res: Response) => { + // Set by requireUsableLink above. Re-checked rather than asserted non-null, + // so a future reordering of the middleware fails as a 404 rather than as a + // crash on undefined. + const link = (req as IntakeRequest).uploadLink; + if (!link) { + return res.status(404).json({ error: 'not found' }); + } + + const files = (req.files as Express.Multer.File[]) || []; + if (files.length === 0) { + return res.status(400).json({ error: 'at least one photo is required' }); + } + + const refusal = await verifyUploadedImages(req); + if (refusal) { + return res.status(400).json({ error: refusal }); + } + + const note = typeof req.body?.note === 'string' ? req.body.note.trim() : ''; + + const client = await pool.connect(); + try { + await client.query('BEGIN'); + + // A placeholder name. `items.name` is NOT NULL and nobody has named this + // yet — the drafting worker or the admin replaces it. A timestamp is used + // rather than "Untitled" so several waiting submissions are still tellable + // apart in the inventory list. + const { rows } = await client.query<{ id: number }>( + `INSERT INTO items (name, description, status) + VALUES ($1, $2, 'pending') + RETURNING id`, + [`Submission ${new Date().toISOString()}`, null] + ); + const itemId = requireRow(rows, 'the intake item INSERT').id; + + await insertItemImages(client, itemId, files, 0); + + await client.query( + `INSERT INTO item_drafts (item_id, upload_link_id, submitter_note) + VALUES ($1, $2, $3)`, + [itemId, link.id, note === '' ? null : note] + ); + + // Counted inside the transaction and guarded on the same conditions as the + // lookup, so two submissions racing on the last slot of a capped link + // cannot both succeed. + const counted = await client.query( + `UPDATE upload_links + SET submission_count = submission_count + 1, last_used_at = now() + WHERE id = $1 + AND revoked_at IS NULL + AND (max_submissions IS NULL OR submission_count < max_submissions)`, + [link.id] + ); + if (counted.rowCount === 0) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'not found' }); + } + + await client.query('COMMIT'); + // No item id in the response: the sender has no business knowing about + // the catalogue, and nothing they can do with it. + res.status(201).json({ ok: true }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } +})); + +export default router; +``` + +- [ ] **Step 5: Mount it** + +In `backend/src/app.ts`, with the other public routers — no `requireAdminGate`: + +```ts +import intakeRouter from './routes/intake'; +``` + +```ts +app.use('/api/intake', intakeRouter); +``` + +- [ ] **Step 6: Run the tests** + +```bash +cd backend +npx jest -c jest.integration.config.js --runInBand intake.integration +npm run test:unit +npm run lint && npm run build +``` + +Expected: PASS, 11 integration tests, and the full unit suite including `routesAreWrapped`. + +- [ ] **Step 7: Run the whole backend suite before committing** + +```bash +cd backend && npm run test:integration +``` + +Expected: PASS. This is the point where a mistake in Task 2's extraction would surface in the admin inventory tests. + +- [ ] **Step 8: Commit** + +```bash +git add backend/src/routes/intake.ts backend/src/rateLimit.ts backend/src/app.ts backend/tests/integration/intake.integration.test.ts +git commit -m "feat(intake): accept photo submissions through a shared link (#NNN)" +``` + +--- + +### Task 6: The public submission page + +**Files:** +- Create: `frontend/src/intake/intakeApi.ts`, `frontend/src/intake/Submit.tsx` +- Modify: `frontend/src/main.tsx:101-110` + +**Interfaces:** +- Consumes: `GET/POST /api/intake/:token` +- Produces: route `/submit/:token` + +- [ ] **Step 1: Write the API client** + +Create `frontend/src/intake/intakeApi.ts`: + +```ts +export interface IntakeLink { + label: string; +} + +export async function fetchIntakeLink(token: string): Promise { + const res = await fetch(`/api/intake/${encodeURIComponent(token)}`); + // Every refusal is a 404 by design, so there is one "this link does not + // work" state rather than several the page would have to explain. + if (!res.ok) return null; + return res.json(); +} + +export async function submitItem( + token: string, + files: File[], + note: string +): Promise<{ ok: true } | { ok: false; error: string }> { + const body = new FormData(); + for (const file of files) body.append('images', file); + body.append('note', note); + + const res = await fetch(`/api/intake/${encodeURIComponent(token)}`, { method: 'POST', body }); + if (res.ok) return { ok: true }; + + const payload = await res.json().catch(() => ({})); + return { ok: false, error: payload.error ?? 'Something went wrong. Please try again.' }; +} +``` + +- [ ] **Step 2: Write the page** + +Create `frontend/src/intake/Submit.tsx`: + +```tsx +import React, { useEffect, useState } from 'react'; +import { useParams } from 'react-router-dom'; +import Typography from 'antd/es/typography'; +import Card from 'antd/es/card'; +import Upload from 'antd/es/upload'; +import ButtonAntd from 'antd/es/button'; +import Input from 'antd/es/input'; +import Alert from 'antd/es/alert'; +import Spin from 'antd/es/spin'; +import Space from 'antd/es/space'; +import type { UploadFile } from 'antd/es/upload/interface'; +import { fetchIntakeLink, submitItem } from './intakeApi'; + +const { Title, Paragraph } = Typography; +const { TextArea } = Input; + +// The three types the server will accept. Listed here so the file picker +// offers exactly those; the server checks the bytes regardless. +const ACCEPT = 'image/jpeg,image/png,image/webp'; +const MAX_IMAGES = 6; + +export default function Submit() { + const { token = '' } = useParams(); + const [label, setLabel] = useState(null); + const [checking, setChecking] = useState(true); + const [files, setFiles] = useState([]); + const [note, setNote] = useState(''); + const [sending, setSending] = useState(false); + const [sent, setSent] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + void fetchIntakeLink(token).then((link) => { + if (cancelled) return; + setLabel(link?.label ?? null); + setChecking(false); + }); + return () => { + cancelled = true; + }; + }, [token]); + + async function send() { + setSending(true); + setError(null); + const result = await submitItem( + token, + files.map((f) => f.originFileObj as File).filter(Boolean), + note + ); + setSending(false); + if (result.ok) { + setSent(true); + return; + } + setError(result.error); + } + + if (checking) return ; + + // One state for every refusal, matching the server's single 404. + if (label === null) { + return ( + + This link is not active + + It may have been turned off, or already used as many times as it was meant for. Ask + whoever sent it to you for a new one. + + + ); + } + + if (sent) { + return ( + + Thank you — it arrived + + Somebody will look at your photos and write it up. Nothing is listed for sale until + they have. + + { + setFiles([]); + setNote(''); + setSent(false); + }} + > + Send another item + + + ); + } + + return ( + + Send in an item + + Photos of one item, and anything you know about it. Send each item separately. + + + + false} + onChange={({ fileList }) => setFiles(fileList)} + > + Choose photos + + +