Two tasks added to the slice-1 plan, closing the half of the upload gap the middleware ordering does not. The ordering fix stops a caller with a bad token writing anything. A caller with a working one can still send six eight-megabyte files per request against a limiter that allows twenty requests a window, and nothing checks whether the volume can take it. That volume is shared with the admin upload path, so intake filling it is a shop outage rather than an intake outage. Task 8 refuses an upload when less than a gigabyte remains, on the admin item routes as well as intake, failing closed because a volume that cannot be measured is not one to assume is empty. Task 9 makes an absent cap mean the bounded default of twenty-five rather than unlimited: the router as written treated omission as "no limit", so the ordinary act of creating a link produced an unbounded one, and a cap that has to be remembered is not a control. Two larger findings are filed rather than folded in. Re-encoding uploads to strip EXIF and cut stored bytes (#226) touches the shared pipeline and adds a native dependency; a global ceiling with an abuse alert (#227) needs its own state and an email. The EXIF one is worth stating plainly: nothing strips metadata today, so an uploaded phone photo publishes the coordinates it was taken at, at a public URL. That is already true of the admin path and is not introduced here, but this slice widens who can put such a file there. Ref #220
1805 lines
63 KiB
Markdown
1805 lines
63 KiB
Markdown
# 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<string | null>` — refusal message, or null
|
||
- `insertItemImages(client: PoolClient, itemId: number, files: Express.Multer.File[], firstSortOrder: number): Promise<void>`
|
||
- `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<UploadLinkRow>(`${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<UploadLinkRow>(
|
||
`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<UploadLinkRow>(
|
||
`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<string[]> {
|
||
return fs.readdir(UPLOADS_DIR);
|
||
}
|
||
|
||
async function issueLink(label = 'Sarah', maxSubmissions?: number): Promise<string> {
|
||
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('<html>not an image</html>'), {
|
||
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<LinkRow | null> {
|
||
const { rows } = await pool.query<LinkRow>(
|
||
`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<IntakeLink | null> {
|
||
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<string | null>(null);
|
||
const [checking, setChecking] = useState(true);
|
||
const [files, setFiles] = useState<UploadFile[]>([]);
|
||
const [note, setNote] = useState('');
|
||
const [sending, setSending] = useState(false);
|
||
const [sent, setSent] = useState(false);
|
||
const [error, setError] = useState<string | null>(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 <Spin />;
|
||
|
||
// One state for every refusal, matching the server's single 404.
|
||
if (label === null) {
|
||
return (
|
||
<Card style={{ maxWidth: 640, margin: '3rem auto' }}>
|
||
<Title level={3}>This link is not active</Title>
|
||
<Paragraph>
|
||
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.
|
||
</Paragraph>
|
||
</Card>
|
||
);
|
||
}
|
||
|
||
if (sent) {
|
||
return (
|
||
<Card style={{ maxWidth: 640, margin: '3rem auto' }}>
|
||
<Title level={3}>Thank you — it arrived</Title>
|
||
<Paragraph>
|
||
Somebody will look at your photos and write it up. Nothing is listed for sale until
|
||
they have.
|
||
</Paragraph>
|
||
<ButtonAntd
|
||
onClick={() => {
|
||
setFiles([]);
|
||
setNote('');
|
||
setSent(false);
|
||
}}
|
||
>
|
||
Send another item
|
||
</ButtonAntd>
|
||
</Card>
|
||
);
|
||
}
|
||
|
||
return (
|
||
<Card style={{ maxWidth: 640, margin: '3rem auto' }}>
|
||
<Title level={3}>Send in an item</Title>
|
||
<Paragraph>
|
||
Photos of one item, and anything you know about it. Send each item separately.
|
||
</Paragraph>
|
||
|
||
<Space direction="vertical" size="middle" style={{ width: '100%' }}>
|
||
<Upload
|
||
accept={ACCEPT}
|
||
multiple
|
||
listType="picture"
|
||
maxCount={MAX_IMAGES}
|
||
fileList={files}
|
||
// Returning false keeps antd from uploading each file on selection;
|
||
// they are sent together by `send` instead.
|
||
beforeUpload={() => false}
|
||
onChange={({ fileList }) => setFiles(fileList)}
|
||
>
|
||
<ButtonAntd>Choose photos</ButtonAntd>
|
||
</Upload>
|
||
|
||
<TextArea
|
||
rows={4}
|
||
value={note}
|
||
onChange={(e) => setNote(e.target.value)}
|
||
placeholder="What is it, what is it made of, how big, what condition, where did it come from? Anything you know helps — a photo cannot show any of it."
|
||
/>
|
||
|
||
{error && <Alert type="error" message={error} showIcon />}
|
||
|
||
<ButtonAntd type="primary" onClick={send} loading={sending} disabled={files.length === 0}>
|
||
Send
|
||
</ButtonAntd>
|
||
</Space>
|
||
</Card>
|
||
);
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 3: Add the route**
|
||
|
||
In `frontend/src/main.tsx`, import and add inside `<Routes location={backdrop}>`:
|
||
|
||
```tsx
|
||
import Submit from './intake/Submit';
|
||
```
|
||
|
||
```tsx
|
||
<Route path="/submit/:token" element={<Submit />} />
|
||
```
|
||
|
||
- [ ] **Step 4: Verify it builds and lints**
|
||
|
||
```bash
|
||
cd frontend && npm run lint && npm run build
|
||
```
|
||
|
||
Expected: PASS. The build runs `tsc` first, so a wrong antd import path fails here.
|
||
|
||
- [ ] **Step 5: Try it against a running stack**
|
||
|
||
```powershell
|
||
.\scripts\start-local.ps1
|
||
```
|
||
|
||
Issue a link, then open `http://localhost:5173/submit/<token>`, attach two photos, add a note, send. Confirm in `/admin` that a pending item appeared with both images. Then check `http://localhost:5173/submit/bogus` shows the inactive-link card.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add frontend/src/intake frontend/src/main.tsx
|
||
git commit -m "feat(intake): add the public submission page (#NNN)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 7: The admin screen for managing links
|
||
|
||
**Files:**
|
||
- Create: `frontend/src/admin/UploadLinks.tsx`
|
||
- Modify: `frontend/src/admin/Admin.tsx`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `GET/POST /api/admin/upload-links`, `POST /api/admin/upload-links/:id/revoke`
|
||
- Produces: an "Upload links" tab
|
||
|
||
- [ ] **Step 1: Write the screen**
|
||
|
||
Create `frontend/src/admin/UploadLinks.tsx`:
|
||
|
||
```tsx
|
||
import React, { useEffect, useState } from 'react';
|
||
import Table from 'antd/es/table';
|
||
import ButtonAntd from 'antd/es/button';
|
||
import Input from 'antd/es/input';
|
||
import Space from 'antd/es/space';
|
||
import Alert from 'antd/es/alert';
|
||
import Typography from 'antd/es/typography';
|
||
import Popconfirm from 'antd/es/popconfirm';
|
||
import Tag from 'antd/es/tag';
|
||
|
||
const { Paragraph, Text } = Typography;
|
||
|
||
interface UploadLink {
|
||
id: number;
|
||
label: string;
|
||
revoked_at: string | null;
|
||
submission_count: number;
|
||
max_submissions: number | null;
|
||
last_used_at: string | null;
|
||
created_at: string;
|
||
}
|
||
|
||
export default function UploadLinks() {
|
||
const [links, setLinks] = useState<UploadLink[]>([]);
|
||
const [label, setLabel] = useState('');
|
||
const [cap, setCap] = useState('');
|
||
// Held only in component state and shown once. The server cannot return it
|
||
// again — it stores a digest — so a refresh loses it deliberately.
|
||
const [issued, setIssued] = useState<string | null>(null);
|
||
const [error, setError] = useState<string | null>(null);
|
||
|
||
async function load() {
|
||
const res = await fetch('/api/admin/upload-links');
|
||
if (res.ok) setLinks(await res.json());
|
||
}
|
||
|
||
useEffect(() => {
|
||
void load();
|
||
}, []);
|
||
|
||
async function create() {
|
||
setError(null);
|
||
const res = await fetch('/api/admin/upload-links', {
|
||
method: 'POST',
|
||
headers: { 'Content-Type': 'application/json' },
|
||
body: JSON.stringify({ label, maxSubmissions: cap === '' ? undefined : Number(cap) })
|
||
});
|
||
if (!res.ok) {
|
||
const payload = await res.json().catch(() => ({}));
|
||
setError(payload.error ?? 'Could not create the link.');
|
||
return;
|
||
}
|
||
const created = await res.json();
|
||
setIssued(created.url);
|
||
setLabel('');
|
||
setCap('');
|
||
await load();
|
||
}
|
||
|
||
async function revoke(id: number) {
|
||
await fetch(`/api/admin/upload-links/${id}/revoke`, { method: 'POST' });
|
||
await load();
|
||
}
|
||
|
||
return (
|
||
<Space direction="vertical" size="large" style={{ width: '100%' }}>
|
||
<Paragraph>
|
||
Give one link per person or purpose. If a link is shared further than you meant, revoke
|
||
that one — everything sent through it is kept.
|
||
</Paragraph>
|
||
|
||
<Space wrap>
|
||
<Input
|
||
placeholder="Who or what is this for?"
|
||
value={label}
|
||
onChange={(e) => setLabel(e.target.value)}
|
||
style={{ width: 260 }}
|
||
/>
|
||
<Input
|
||
placeholder="Max uses (optional)"
|
||
value={cap}
|
||
onChange={(e) => setCap(e.target.value)}
|
||
style={{ width: 160 }}
|
||
/>
|
||
<ButtonAntd type="primary" onClick={create} disabled={label.trim() === ''}>
|
||
Create link
|
||
</ButtonAntd>
|
||
</Space>
|
||
|
||
{error && <Alert type="error" message={error} showIcon />}
|
||
|
||
{issued && (
|
||
<Alert
|
||
type="success"
|
||
showIcon
|
||
message="Copy this link now"
|
||
description={
|
||
<>
|
||
<Paragraph copyable={{ text: issued }}>
|
||
<Text code>{issued}</Text>
|
||
</Paragraph>
|
||
<Text type="secondary">
|
||
It is not stored and cannot be shown again. If you lose it, revoke this link and
|
||
make another.
|
||
</Text>
|
||
</>
|
||
}
|
||
closable
|
||
onClose={() => setIssued(null)}
|
||
/>
|
||
)}
|
||
|
||
<Table<UploadLink>
|
||
rowKey="id"
|
||
dataSource={links}
|
||
pagination={false}
|
||
columns={[
|
||
{ title: 'Label', dataIndex: 'label' },
|
||
{
|
||
title: 'Used',
|
||
render: (_, row) =>
|
||
row.max_submissions === null
|
||
? row.submission_count
|
||
: `${row.submission_count} of ${row.max_submissions}`
|
||
},
|
||
{
|
||
title: 'Status',
|
||
render: (_, row) =>
|
||
row.revoked_at ? <Tag>Revoked</Tag> : <Tag color="green">Active</Tag>
|
||
},
|
||
{
|
||
title: '',
|
||
render: (_, row) =>
|
||
row.revoked_at ? null : (
|
||
<Popconfirm
|
||
title="Revoke this link?"
|
||
description="Anyone holding it stops being able to send anything. Items already sent are kept."
|
||
onConfirm={() => revoke(row.id)}
|
||
>
|
||
<ButtonAntd danger size="small">
|
||
Revoke
|
||
</ButtonAntd>
|
||
</Popconfirm>
|
||
)
|
||
}
|
||
]}
|
||
/>
|
||
</Space>
|
||
);
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Add the tab**
|
||
|
||
In `frontend/src/admin/Admin.tsx`, add the import alongside the other tab components:
|
||
|
||
```tsx
|
||
import UploadLinks from './UploadLinks';
|
||
```
|
||
|
||
Then add the entry to the `items` array at line 388, after `tags` and before `customers` — it belongs with the catalogue-side tabs rather than the customer-side ones:
|
||
|
||
```tsx
|
||
{ key: 'tags', label: 'Tags', children: <Tags /> },
|
||
{ key: 'upload-links', label: 'Upload links', children: <UploadLinks /> },
|
||
{ key: 'customers', label: 'Customers', children: <Customers /> },
|
||
```
|
||
|
||
- [ ] **Step 3: Verify it builds and lints**
|
||
|
||
```bash
|
||
cd frontend && npm run lint && npm run build
|
||
```
|
||
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 4: Try it against a running stack**
|
||
|
||
In `/admin`, create a link, confirm the one-time reveal appears and copies, reload the page and confirm the token is gone from the table, submit through the link, confirm the used count increments, then revoke it and confirm the submission page shows the inactive card.
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add frontend/src/admin/UploadLinks.tsx frontend/src/admin/Admin.tsx
|
||
git commit -m "feat(intake): manage upload links from the admin (#NNN)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 8: Refuse uploads when the volume is nearly full
|
||
|
||
Both of the remaining tasks must come after Task 2 (the extraction). Task 8 is independent of Tasks 3–7; Task 9 edits the router Task 4 writes, so it comes after that.
|
||
|
||
The gap this closes: the ordering fix in Task 5 stops a caller with a *bad* token writing anything, but a caller with a working one can still send 6 × 8 MB per request against a limiter that allows 20 requests per window. Nothing anywhere checks whether the volume can take it, and the volume is shared with the admin upload path — so intake filling it breaks the whole shop, not just intake.
|
||
|
||
**Files:**
|
||
- Modify: `backend/src/imageUpload.ts`, `backend/src/routes/intake.ts`, `backend/src/routes/admin.ts`
|
||
- Test: `backend/tests/unit/imageUpload.test.ts` (create), `backend/tests/integration/intake.integration.test.ts` (extend)
|
||
|
||
**Interfaces:**
|
||
- Produces:
|
||
- `hasRoomFor(availableBytes: number): boolean` — pure, exported for its test
|
||
- `requireVolumeRoom` — middleware, `503` when the volume is low
|
||
|
||
- [ ] **Step 1: Write the failing unit test**
|
||
|
||
Create `backend/tests/unit/imageUpload.test.ts`:
|
||
|
||
```ts
|
||
import { hasRoomFor, MIN_FREE_BYTES } from '../../src/imageUpload';
|
||
|
||
// The threshold decision is pure and is the whole of the policy, so it is
|
||
// tested directly rather than through a request — the same reasoning that has
|
||
// keyByCallerAndEmail and isAllowedRecipient exported for their tests.
|
||
describe('hasRoomFor', () => {
|
||
it('accepts a volume with room to spare', () => {
|
||
expect(hasRoomFor(MIN_FREE_BYTES * 10)).toBe(true);
|
||
});
|
||
|
||
it('refuses a volume below the floor', () => {
|
||
expect(hasRoomFor(MIN_FREE_BYTES - 1)).toBe(false);
|
||
});
|
||
|
||
// Exactly at the floor is still enough: the floor is what must remain, and
|
||
// an off-by-one here is the difference between a shop that works and one
|
||
// that refuses every upload.
|
||
it('accepts a volume exactly at the floor', () => {
|
||
expect(hasRoomFor(MIN_FREE_BYTES)).toBe(true);
|
||
});
|
||
|
||
it('refuses an empty volume', () => {
|
||
expect(hasRoomFor(0)).toBe(false);
|
||
});
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Run it to verify it fails**
|
||
|
||
```bash
|
||
cd backend && npx jest -c jest.unit.config.js imageUpload
|
||
```
|
||
|
||
Expected: FAIL — `hasRoomFor` is not exported.
|
||
|
||
- [ ] **Step 3: Implement the guard**
|
||
|
||
Append to `backend/src/imageUpload.ts`:
|
||
|
||
```ts
|
||
import { statfs } from 'fs/promises';
|
||
|
||
/**
|
||
* How much of the uploads volume must remain free for an upload to be accepted.
|
||
*
|
||
* One gigabyte is not a tuned number; it is a floor chosen so that the volume
|
||
* filling degrades into refused uploads rather than into a database that
|
||
* cannot write, logs that cannot rotate, and a shop that is down. There is
|
||
* deliberately no environment variable: an operator who wants a different floor
|
||
* is making a decision that should be visible in a diff.
|
||
*/
|
||
export const MIN_FREE_BYTES = 1_000_000_000;
|
||
|
||
/** Pure, so the policy can be tested without a filesystem. */
|
||
export function hasRoomFor(availableBytes: number): boolean {
|
||
return availableBytes >= MIN_FREE_BYTES;
|
||
}
|
||
|
||
/**
|
||
* Refuses an upload when the uploads volume is nearly full.
|
||
*
|
||
* Fails closed. A volume that cannot be measured is not one to assume is
|
||
* empty — and in practice `statfs` failing means the directory is missing or
|
||
* unreadable, in which case the upload was going to fail anyway, just later
|
||
* and less clearly.
|
||
*
|
||
* 503 rather than 507: the condition is temporary and about the server rather
|
||
* than the request, and 507 is obscure enough that intermediaries and clients
|
||
* handle it inconsistently.
|
||
*/
|
||
export const requireVolumeRoom = asyncRoute(
|
||
async (_req: Request, res: Response, next: NextFunction) => {
|
||
try {
|
||
const stats = await statfs(UPLOADS_DIR);
|
||
if (!hasRoomFor(stats.bavail * stats.bsize)) {
|
||
console.error(`[upload] refusing: less than ${MIN_FREE_BYTES} bytes free on ${UPLOADS_DIR}`);
|
||
res.status(503).json({ error: 'storage is full — please try again later' });
|
||
return;
|
||
}
|
||
} catch (err) {
|
||
console.error(`[upload] could not measure ${UPLOADS_DIR}:`, err);
|
||
res.status(503).json({ error: 'storage is unavailable — please try again later' });
|
||
return;
|
||
}
|
||
next();
|
||
}
|
||
);
|
||
```
|
||
|
||
Add to that file's imports:
|
||
|
||
```ts
|
||
import { Request, Response, NextFunction } from 'express';
|
||
import { asyncRoute } from './asyncRoute';
|
||
```
|
||
|
||
- [ ] **Step 4: Apply it to every route that writes files**
|
||
|
||
In `backend/src/routes/intake.ts`, before `uploadImages`:
|
||
|
||
```ts
|
||
router.post('/:token', intakeLimiter, requireUsableLink, requireVolumeRoom, uploadImages, asyncRoute(async (req: Request, res: Response) => {
|
||
```
|
||
|
||
In `backend/src/routes/admin.ts`, on both routes that take files — the guard belongs on the shared pipeline, not only on the public half, since both write to the same volume:
|
||
|
||
```ts
|
||
router.post('/items', requireVolumeRoom, uploadImages, asyncRoute(async (req: Request, res: Response) => {
|
||
```
|
||
|
||
```ts
|
||
router.put('/items/:id', requireVolumeRoom, uploadImages, asyncRoute(async (req: Request, res: Response) => {
|
||
```
|
||
|
||
Import it in both files from `../imageUpload`.
|
||
|
||
- [ ] **Step 5: Run the unit test and the whole suite**
|
||
|
||
```bash
|
||
cd backend
|
||
npx jest -c jest.unit.config.js imageUpload
|
||
npm run test:unit
|
||
npm run test:integration
|
||
npm run lint && npm run build
|
||
```
|
||
|
||
Expected: PASS throughout. The integration suites exercise the admin upload routes, so a mistake in the middleware ordering surfaces there as a 503 on every upload.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add backend/src/imageUpload.ts backend/src/routes/intake.ts backend/src/routes/admin.ts backend/tests/unit/imageUpload.test.ts
|
||
git commit -m "feat(uploads): refuse uploads when the volume is nearly full (#NNN)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 9: Bound a link by default
|
||
|
||
The gap this closes: `max_submissions` is nullable and the router as written in Task 4 sends `null` whenever the field is absent, so the ordinary act of creating a link produces an unbounded one. A cap that has to be remembered is not a control.
|
||
|
||
**Files:**
|
||
- Modify: `backend/src/routes/adminUploadLinks.ts`, `frontend/src/admin/UploadLinks.tsx`
|
||
- Test: `backend/tests/integration/uploadLinks.integration.test.ts` (extend)
|
||
|
||
- [ ] **Step 1: Write the failing test**
|
||
|
||
Add to `backend/tests/integration/uploadLinks.integration.test.ts`, inside `describe('issuing an upload link')`:
|
||
|
||
```ts
|
||
// Omitting the field is the common case, so it is the case that must be
|
||
// safe. An unbounded link should be something asked for, not something that
|
||
// happens when nobody thought about it.
|
||
it('bounds a link that was created without a cap', async () => {
|
||
const res = await request(app).post('/api/admin/upload-links').send({ label: 'Sarah' });
|
||
|
||
expect(res.status).toBe(201);
|
||
expect(res.body.max_submissions).toBe(25);
|
||
});
|
||
|
||
it('allows unlimited when it is asked for explicitly', async () => {
|
||
const res = await request(app)
|
||
.post('/api/admin/upload-links')
|
||
.send({ label: 'Always on', maxSubmissions: null });
|
||
|
||
expect(res.status).toBe(201);
|
||
expect(res.body.max_submissions).toBeNull();
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Run it to verify it fails**
|
||
|
||
```bash
|
||
cd backend && npx jest -c jest.integration.config.js --runInBand uploadLinks.integration
|
||
```
|
||
|
||
Expected: FAIL — `max_submissions` is `null` where `25` was expected.
|
||
|
||
- [ ] **Step 3: Change the default**
|
||
|
||
In `backend/src/routes/adminUploadLinks.ts`, replace the cap-parsing block from Task 4 with:
|
||
|
||
```ts
|
||
/**
|
||
* The cap a link gets when nobody chose one.
|
||
*
|
||
* Not a tuned number — it is large enough that an ordinary contributor never
|
||
* meets it and small enough that a leaked link cannot be used indefinitely
|
||
* before anyone notices. The point is that the default is finite at all.
|
||
*/
|
||
const DEFAULT_MAX_SUBMISSIONS = 25;
|
||
|
||
// Three cases, and they are deliberately different. Absent means "nobody
|
||
// decided", which gets the bounded default. An explicit null means "unlimited",
|
||
// which is a decision someone made and can be seen in the request. A number is
|
||
// itself. Reading absent as unlimited is what made every link unbounded.
|
||
const rawCap = req.body?.maxSubmissions;
|
||
let maxSubmissions: number | null = DEFAULT_MAX_SUBMISSIONS;
|
||
if (rawCap === null) {
|
||
maxSubmissions = null;
|
||
} else if (rawCap !== undefined && 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;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Make the frontend say so**
|
||
|
||
In `frontend/src/admin/UploadLinks.tsx`, the cap input starts at the default and unlimited becomes a deliberate choice rather than an empty field. Replace the `cap` state and its input:
|
||
|
||
```tsx
|
||
const [cap, setCap] = useState('25');
|
||
const [unlimited, setUnlimited] = useState(false);
|
||
```
|
||
|
||
```tsx
|
||
<Input
|
||
placeholder="Max uses"
|
||
value={cap}
|
||
disabled={unlimited}
|
||
onChange={(e) => setCap(e.target.value)}
|
||
style={{ width: 140 }}
|
||
/>
|
||
<Checkbox checked={unlimited} onChange={(e) => setUnlimited(e.target.checked)}>
|
||
No limit
|
||
</Checkbox>
|
||
```
|
||
|
||
Import `Checkbox from 'antd/es/checkbox'`. In `create`, send the three cases explicitly:
|
||
|
||
```tsx
|
||
body: JSON.stringify({
|
||
label,
|
||
// null is how unlimited is asked for; a number otherwise. The field is
|
||
// never omitted from this client, so the server's default is a
|
||
// safeguard for other callers rather than this screen's behaviour.
|
||
maxSubmissions: unlimited ? null : Number(cap)
|
||
})
|
||
```
|
||
|
||
And reset it after a successful create — `setCap('25'); setUnlimited(false);` in place of `setCap('')`.
|
||
|
||
- [ ] **Step 5: Run everything**
|
||
|
||
```bash
|
||
cd backend && npx jest -c jest.integration.config.js --runInBand uploadLinks.integration && npm run lint && npm run build
|
||
cd ../frontend && npm run lint && npm run build
|
||
```
|
||
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add backend/src/routes/adminUploadLinks.ts backend/tests/integration/uploadLinks.integration.test.ts frontend/src/admin/UploadLinks.tsx
|
||
git commit -m "feat(intake): bound an upload link by default (#NNN)"
|
||
```
|
||
|
||
---
|
||
|
||
## Done when
|
||
|
||
- A link created in `/admin` accepts a submission at `/submit/<token>`, and the item appears in the admin inventory as pending with its photos and note.
|
||
- Revoking that link makes the page show the inactive card and the endpoint return 404.
|
||
- A capped link stops at its cap.
|
||
- A file whose bytes disagree with its declared type is refused and leaves nothing on disk or in the database.
|
||
- A token that does not work causes no bytes to be written at all, and the test asserting that passes.
|
||
- A link created without a stated cap comes back bounded at 25; unlimited requires asking for it.
|
||
- `npm run test:unit`, `npm run test:integration`, `npm run lint` and `npm run build` all pass in `backend/`; `npm run lint` and `npm run build` pass in `frontend/`.
|
||
|
||
## Not in this slice
|
||
|
||
The drafting worker, the notification email and its signed action links, and the review queue. `item_drafts` is created here with its AI columns unpopulated so slice 2 adds behaviour rather than schema. The `price_source` column is written as `'default'` and not yet read.
|
||
|
||
Two hardening items are deliberately separate issues rather than tasks here. **Re-encoding uploads to strip EXIF and cut stored bytes** touches the shared pipeline the admin path also uses, adds a native dependency, and has its own testing surface — it deserves its own review rather than riding along. **A global daily submission ceiling with an alert** needs state of its own and an outbound email, which is more than the two guards above. Both are filed against #220.
|
||
|
||
Until the re-encode lands, uploaded photos retain their EXIF — including GPS coordinates — and are served publicly. That is true of the existing admin upload path today and is not introduced by this slice, but this slice widens who can put such a file there.
|