Merge pull request 'feat(intake): issue named upload links and accept photo submissions (#222)' (#250) from feature/222-upload-links into main
Linting / lint (push) Successful in 2m26s
SonarQube Analysis / sonarqube (push) Successful in 22m1s

Reviewed-on: #250
This commit was merged in pull request #250.
This commit is contained in:
2026-08-31 15:47:09 -05:00
21 changed files with 1727 additions and 270 deletions
@@ -0,0 +1,80 @@
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. The
-- route defaults this to a finite number rather than null, so an
-- unbounded link is something asked for rather than something that
-- happens when nobody thought about it.
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[],
-- Kept even though a suggestion is also copied onto the item, so what
-- the model proposed stays readable after the admin has edited the
-- item's price. Without it there is no way to ask later whether the
-- model's numbers were any good.
ai_suggested_price_cents INTEGER,
-- ai | default | admin. 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 review queue reads by state; everything else reads by item, which
-- the UNIQUE constraint on item_id already indexes.
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. 80.00 applies when
-- nothing else supplies a price; the drafting worker in #223 writes a
-- model's suggestion over it when there is one.
--
-- The number lives here rather than in configuration deliberately.
-- Changing a default price is a rare, deliberate act that deserves a
-- record; an environment variable would let it drift silently between
-- environments, and a wrong default is invisible until something has
-- already sold at it.
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;
`);
};
+7 -7
View File
@@ -9,6 +9,8 @@ import adminSettingsRouter from './routes/adminSettings';
import adminEmailTemplatesRouter from './routes/adminEmailTemplates';
import adminCategoriesRouter from './routes/adminCategories';
import adminTagsRouter from './routes/adminTags';
import adminUploadLinksRouter from './routes/adminUploadLinks';
import intakeRouter from './routes/intake';
import adminVersionRouter from './routes/adminVersion';
import filtersRouter from './routes/filters';
import customersRouter from './routes/customers';
@@ -20,6 +22,7 @@ import { attachCustomer } from './middleware/customerAuth';
import { requireAdminGate } from './middleware/adminGate';
import { asyncRoute } from './asyncRoute';
import { uploadsRouter } from './uploads';
import { trimTrailingSlashes } from './utils';
const app = express();
// Express advertises itself in X-Powered-By by default, which hands an
@@ -36,13 +39,6 @@ app.use(cookieParser());
app.use(asyncRoute(attachCustomer));
app.use('/uploads', uploadsRouter(process.env.UPLOADS_DIR || '/app/uploads'));
// Trimmed with a loop rather than a `/+$/` regex, which backtracks.
function trimTrailingSlashes(value: string): string {
let trimmed = value;
while (trimmed.endsWith('/')) trimmed = trimmed.slice(0, -1);
return trimmed;
}
app.get('/api/config', (_req, res) => {
const clientId = process.env.PAYPAL_CLIENT_ID;
const isPlaceholder = !clientId || clientId.length < 10 || clientId === 'REPLACE_WITH_PAYPAL_CLIENT_ID';
@@ -68,6 +64,9 @@ app.get('/api/config', (_req, res) => {
app.use('/api/items', itemsRouter);
app.use('/api/filters', filtersRouter);
app.use('/api/cart', cartRouter);
// Public and unauthenticated by design (#222). No requireAdminGate: the token
// in the path is the whole access control, and every refusal is a 404.
app.use('/api/intake', intakeRouter);
app.use('/api/checkout/cart', cartCheckoutRouter);
// requireAdminGate is attached to each admin router rather than to a path
// prefix. Attached to the router, an admin router added later at some other
@@ -79,6 +78,7 @@ app.use('/api/admin/settings', requireAdminGate, adminSettingsRouter);
app.use('/api/admin/email-templates', requireAdminGate, adminEmailTemplatesRouter);
app.use('/api/admin/categories', requireAdminGate, adminCategoriesRouter);
app.use('/api/admin/tags', requireAdminGate, adminTagsRouter);
app.use('/api/admin/upload-links', requireAdminGate, adminUploadLinksRouter);
app.use('/api/admin/version', requireAdminGate, adminVersionRouter);
app.use('/api/admin', requireAdminGate, adminRouter);
app.use('/api/customers/me/addresses', shippingAddressesRouter);
+286
View File
@@ -0,0 +1,286 @@
/**
* The one validated path from a multipart request to files on the uploads
* volume.
*
* Extracted from routes/admin.ts when a second caller appeared (#222's public
* intake endpoint). It is deliberately one module rather than two similar
* ones: every property that makes an upload safe here — the type allowlist,
* the magic-byte check after the write, names from a CSPRNG rather than from
* `originalname`, the re-encode that strips EXIF, and the cleanup of whatever
* a refused 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, #103, #180 and #226 exist to close.
*
* Nothing below changed in the move. The comments came with it, because they
* record why the code is shaped as it is and are the most valuable part of it.
*/
import { Request, Response, NextFunction } from 'express';
import multer from 'multer';
import { promises as fs } from 'fs';
import { randomUUID } from 'crypto';
import { PoolClient } from 'pg';
import {
ALLOWED_IMAGE_TYPES,
SIGNATURE_BYTES,
extensionFor,
isAllowedImageType,
signatureMatches
} from './uploadTypes';
import { reencodeInPlace } from './imageProcessing';
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
// Multer writes to disk with no size cap unless one is given, so a single
// request could fill the uploads volume. Bound every dimension of the
// multipart body: image count, bytes per image, and the small text fields
// (name/description/price) that accompany them.
const MAX_IMAGES_PER_REQUEST = 6;
// 8 MB, not 8 MiB — this is the ceiling S5693 treats as safe, and 8 * 1024 *
// 1024 sits just over it. Plenty for a product photo either way.
const MAX_IMAGE_BYTES = 8_000_000;
const MAX_TEXT_FIELDS = 8;
const MAX_TEXT_FIELD_BYTES = 64 * 1024;
// Refused before a byte is written. This catches the honest mistake — picking a
// PDF by accident — and nothing more, because file.mimetype is whatever the
// caller wrote in the multipart headers. The bytes are checked after the write;
// see verifyUploadedImages.
class UnsupportedImageTypeError extends Error {}
const storage = multer.diskStorage({
destination: UPLOADS_DIR,
// Stored names come from a CSPRNG rather than a timestamp plus Math.random,
// which is predictable enough that a caller could guess (or collide with)
// another upload's path.
//
// The extension comes from the validated content type rather than from
// path.extname(file.originalname), so the name on disk cannot disagree with
// what the file claims to be — a caller cannot get `.html` onto the uploads
// volume by naming their file that way.
filename: (_req, file, cb) => {
const ext = extensionFor(file.mimetype);
if (!ext) {
// Unreachable while fileFilter runs first, and here so that it stays
// unreachable rather than silently writing a file with no extension.
cb(new UnsupportedImageTypeError(`unsupported image type ${file.mimetype}`), '');
return;
}
cb(null, `${randomUUID()}${ext}`);
}
});
// Reviewed for #180. Bounding one request is only half the problem — see
// discardUnlessAccepted below for the other half, which is bounding what the
// volume accumulates across requests that were refused.
const upload = multer({
storage,
limits: {
fileSize: MAX_IMAGE_BYTES,
files: MAX_IMAGES_PER_REQUEST,
fields: MAX_TEXT_FIELDS,
fieldSize: MAX_TEXT_FIELD_BYTES
},
fileFilter: (_req, file, cb) => {
if (!isAllowedImageType(file.mimetype)) {
cb(new UnsupportedImageTypeError(
`${file.mimetype} is not an accepted image type — allowed: ${ALLOWED_IMAGE_TYPES.join(', ')}`
));
return;
}
cb(null, true);
}
});
// Reads only the leading bytes — enough to identify a format, not enough to
// care how large the file is. The handle is closed before anything is unlinked,
// because an open handle makes the unlink fail on Windows.
//
// Reviewed for #180. The path is not caller-controlled despite arriving from a
// request: multer composes it from `destination`, which is a server constant,
// and `filename`, which the storage above sets to `randomUUID()` plus an
// extension looked up from the validated content type. The caller's
// `originalname` is never consulted, so no part of the path traverses anywhere.
async function readHead(filePath: string): Promise<Buffer> {
const handle = await fs.open(filePath, 'r');
try {
const buffer = Buffer.alloc(SIGNATURE_BYTES);
const { bytesRead } = await handle.read(buffer, 0, SIGNATURE_BYTES, 0);
return buffer.subarray(0, bytesRead);
} finally {
await handle.close();
}
}
// Best effort: a file that cannot be removed should not turn a 400 into a 500,
// but it must not be left behind quietly either.
async function discardUploads(files: Express.Multer.File[]): Promise<void> {
await Promise.all(
files.map((file) =>
fs.unlink(file.path).catch((err: unknown) => {
console.error(`[upload] could not remove rejected file ${file.path}:`, err);
})
)
);
}
/**
* Removes a request's uploaded files unless the request actually succeeded.
*
* multer writes to disk before any route logic runs, and its own cleanup only
* covers errors it raised itself. Everything after that — a failed signature
* check, a malformed `category_id`, a database error, a dropped connection —
* previously left the bytes on the volume with nothing referencing them: no row
* to find them by, and no bound on how many could accumulate. Bounding the size
* of one upload does not help if every refused upload is kept forever (#180).
*
* Registered as soon as multer succeeds rather than at each `return`, so a
* route added later inherits it instead of having to remember it. That is the
* whole reason it is a hook and not a call: the failure it prevents is someone
* adding a fourth early return.
*
* `close` rather than `finish`, so an aborted connection is covered too, and
* `writableEnded` distinguishes a response that completed from one that never
* did — the latter is not a success however its status code reads.
*/
function discardUnlessAccepted(req: Request, res: Response): void {
res.on('close', () => {
if (res.writableEnded && res.statusCode < 400) return;
void discardUploads((req.files as Express.Multer.File[]) || []);
});
}
/**
* Confirms each stored file actually is what it was declared to be.
*
* This cannot happen in multer's fileFilter, which runs before the stream has
* been read — there are no bytes to look at yet. So the check runs after the
* write.
*
* Checking only: removing the files is discardUnlessAccepted's job, and doing
* it here as well would unlink twice and log an ENOENT for every refused
* upload. That also covers the case this function used to miss — `readHead`
* itself throwing, which returned no message and so cleaned up nothing.
*
* Returns the message to refuse with, or null when everything checks out.
*/
async function verifyUploadedImages(req: Request): Promise<string | null> {
const files = (req.files as Express.Multer.File[]) || [];
for (const file of files) {
const head = await readHead(file.path);
if (!signatureMatches(file.mimetype, head)) {
return `${file.originalname} does not contain ${file.mimetype} data`;
}
}
return null;
}
/**
* Rebuilds every accepted file so it carries no metadata (#226).
*
* After verification, deliberately: re-encoding a file whose bytes do not match
* its declared type would be doing work on something already refused, and
* sharp's own error would replace the clearer message that check produces.
*
* A failure here refuses the upload rather than storing the original. Storing
* it would mean the one case where a photo keeps the coordinates it was taken
* at is the case nobody was told about.
*
* Returns the message to refuse with, or null when every file was rebuilt.
*/
async function stripUploadedImages(req: Request): Promise<string | null> {
const files = (req.files as Express.Multer.File[]) || [];
for (const file of files) {
try {
await reencodeInPlace(file.path, file.mimetype);
} catch (err) {
console.error(`[upload] could not re-encode ${file.path}:`, err);
return `${file.originalname} could not be processed`;
}
}
return null;
}
// No error-handling middleware is mounted on the app, so translate multer's
// limit errors here instead of letting them surface as a generic 500.
const uploadImages = (req: Request, res: Response, next: NextFunction) => {
upload.array('images', MAX_IMAGES_PER_REQUEST)(req, res, (err: unknown) => {
if (err instanceof UnsupportedImageTypeError) {
return res.status(400).json({ error: err.message });
}
if (err instanceof multer.MulterError) {
const status = err.code === 'LIMIT_FILE_SIZE' ? 413 : 400;
return res.status(status).json({ error: err.message });
}
if (err) {
return next(err);
}
// Every file is on disk by this point and multer will not clean up after
// itself again, so the bytes become this request's responsibility before
// anything else is allowed to fail.
discardUnlessAccepted(req, res);
verifyUploadedImages(req)
.then((problem) => {
if (problem) {
res.status(400).json({ error: problem });
return null;
}
return stripUploadedImages(req);
})
.then((problem) => {
// The first stage returns null both when it answered and when it found
// nothing wrong, so the response itself is what distinguishes them.
if (res.headersSent) return;
if (problem) {
res.status(400).json({ error: problem });
return;
}
next();
})
.catch(next);
});
};
/**
* Records uploaded files as an item's images.
*
* Create and update wrote this loop separately, differing only in where the id
* came from and where the sort order started — zero for a new item, one past
* the current maximum for an existing one. Both are parameters now.
*
* It also means the `/uploads/` prefix is written once. That matters more than
* it looks: #103 made the stored value the path `uploadUrl` joins an origin
* onto, so it is a contract rather than a string, and two places to change it
* is one place to forget.
*/
async function insertItemImages(
client: PoolClient,
itemId: number,
files: Express.Multer.File[],
firstSortOrder: number
): Promise<void> {
// Iterated by entry rather than by index, so there is no possibly-undefined
// element to guard — the create path used to fall back to an empty filename,
// which would have stored a path pointing at the uploads directory itself.
for (const [offset, file] of files.entries()) {
await client.query(
`INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`,
[itemId, `/uploads/${file.filename}`, firstSortOrder + offset]
);
}
}
export {
uploadImages,
verifyUploadedImages,
stripUploadedImages,
insertItemImages,
MAX_IMAGES_PER_REQUEST,
MAX_IMAGE_BYTES
};
+60
View File
@@ -124,3 +124,63 @@ export const verificationResendLimiter = rateLimit({
error: 'we have already sent several verification emails recently. Check your spam folder, and try again later.'
}
});
/**
* 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 trade is accepted
* here deliberately: the *link* is the per-caller identity, and its
* `submission_count` against `max_submissions` is the per-caller cap. This
* limiter exists for a different job — bounding what one address can throw at
* an unauthenticated endpoint that writes files to disk.
*
* ipKeyGenerator rather than `req.ip` raw, for the reason #84 records: a
* residential IPv6 customer is delegated a whole prefix and can source every
* request from a different address inside it for free, so keying on the exact
* address counts each one as a new caller and never bounds anything.
*/
export function keyByCaller(req: Request): string {
return ipKeyGenerator(req.ip ?? '');
}
/**
* Two limiters rather than one, because the two requests cost different things.
*
* Reading a link is a page load: it hits one indexed row and writes nothing.
* Submitting writes up to six files to the uploads volume. Counting them
* against a single allowance meant reloading the page consumed the budget for
* sending items, and at twenty apiece that allowance ran out after ten items —
* for exactly the person this feature is for, somebody working through a box
* of stock. The comment here used to say refusing them costs a consignment,
* while the number quietly did it.
*
* Both still key on the caller alone, since a submission carries no email. The
* `keyByCallerAndEmail` comment warns that a bare `ip:` bucket is a shared
* allowance rather than a per-caller one, and that trade is accepted here: the
* link is the per-caller identity and its `max_submissions` is the per-caller
* cap, while these bound what one address can throw at an unauthenticated
* endpoint.
*/
export const intakeViewLimiter = rateLimit({
windowMs: 15 * 60 * 1000,
// Generous, because it is a page load. Someone re-reading the form, losing
// their signal, or coming back to it should never be told to wait.
limit: 120,
keyGenerator: keyByCaller,
standardHeaders: 'draft-7',
legacyHeaders: false,
message: { error: 'too many requests — please try again shortly' }
});
export const intakeSubmitLimiter = rateLimit({
windowMs: 15 * 60 * 1000,
// Each of these writes files, so this is the one worth bounding. Thirty in a
// quarter of an hour is more than anyone photographing items can manage and
// far less than a script would want.
limit: 30,
keyGenerator: keyByCaller,
standardHeaders: 'draft-7',
legacyHeaders: false,
message: { error: 'too many submissions — please try again later' }
});
+7 -257
View File
@@ -1,7 +1,4 @@
import { Router, Request, Response, NextFunction } from 'express';
import multer from 'multer';
import { promises as fs } from 'fs';
import { randomUUID } from 'crypto';
import { Router, Request, Response } from 'express';
import { PoolClient } from 'pg';
import { pool, requireRow } from '../db';
import { ADMIN_ITEM_SELECT, AdminItemRow, ItemRecord } from '../itemSelect';
@@ -9,15 +6,13 @@ import { ItemStatus } from '../types';
import { asyncRoute } from '../asyncRoute';
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
import { tagColorFor } from '../utils';
import {
ALLOWED_IMAGE_TYPES,
SIGNATURE_BYTES,
extensionFor,
isAllowedImageType,
signatureMatches
} from '../uploadTypes';
import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecipients } from '../favoriteAlerts';
import { reencodeInPlace } from '../imageProcessing';
// The upload pipeline moved to src/imageUpload.ts when #222's public intake
// endpoint became a second caller. Mounting uploadImages gets the type
// allowlist, the magic-byte check, and the EXIF-stripping re-encode together —
// which is the point of it being one module rather than something each route
// assembles for itself.
import { uploadImages, insertItemImages } from '../imageUpload';
const router = Router();
@@ -31,223 +26,6 @@ interface ItemStatusRow {
status: ItemStatus;
}
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
// Multer writes to disk with no size cap unless one is given, so a single
// request could fill the uploads volume. Bound every dimension of the
// multipart body: image count, bytes per image, and the small text fields
// (name/description/price) that accompany them.
const MAX_IMAGES_PER_REQUEST = 6;
// 8 MB, not 8 MiB — this is the ceiling S5693 treats as safe, and 8 * 1024 *
// 1024 sits just over it. Plenty for a product photo either way.
const MAX_IMAGE_BYTES = 8_000_000;
const MAX_TEXT_FIELDS = 8;
const MAX_TEXT_FIELD_BYTES = 64 * 1024;
// Refused before a byte is written. This catches the honest mistake — picking a
// PDF by accident — and nothing more, because file.mimetype is whatever the
// caller wrote in the multipart headers. The bytes are checked after the write;
// see verifyUploadedImages.
class UnsupportedImageTypeError extends Error {}
const storage = multer.diskStorage({
destination: UPLOADS_DIR,
// Stored names come from a CSPRNG rather than a timestamp plus Math.random,
// which is predictable enough that a caller could guess (or collide with)
// another upload's path.
//
// The extension comes from the validated content type rather than from
// path.extname(file.originalname), so the name on disk cannot disagree with
// what the file claims to be — a caller cannot get `.html` onto the uploads
// volume by naming their file that way.
filename: (_req, file, cb) => {
const ext = extensionFor(file.mimetype);
if (!ext) {
// Unreachable while fileFilter runs first, and here so that it stays
// unreachable rather than silently writing a file with no extension.
cb(new UnsupportedImageTypeError(`unsupported image type ${file.mimetype}`), '');
return;
}
cb(null, `${randomUUID()}${ext}`);
}
});
// Reviewed for #180. Bounding one request is only half the problem — see
// discardUnlessAccepted below for the other half, which is bounding what the
// volume accumulates across requests that were refused.
const upload = multer({
storage,
limits: {
fileSize: MAX_IMAGE_BYTES,
files: MAX_IMAGES_PER_REQUEST,
fields: MAX_TEXT_FIELDS,
fieldSize: MAX_TEXT_FIELD_BYTES
},
fileFilter: (_req, file, cb) => {
if (!isAllowedImageType(file.mimetype)) {
cb(new UnsupportedImageTypeError(
`${file.mimetype} is not an accepted image type — allowed: ${ALLOWED_IMAGE_TYPES.join(', ')}`
));
return;
}
cb(null, true);
}
});
// Reads only the leading bytes — enough to identify a format, not enough to
// care how large the file is. The handle is closed before anything is unlinked,
// because an open handle makes the unlink fail on Windows.
//
// Reviewed for #180. The path is not caller-controlled despite arriving from a
// request: multer composes it from `destination`, which is a server constant,
// and `filename`, which the storage above sets to `randomUUID()` plus an
// extension looked up from the validated content type. The caller's
// `originalname` is never consulted, so no part of the path traverses anywhere.
async function readHead(filePath: string): Promise<Buffer> {
const handle = await fs.open(filePath, 'r');
try {
const buffer = Buffer.alloc(SIGNATURE_BYTES);
const { bytesRead } = await handle.read(buffer, 0, SIGNATURE_BYTES, 0);
return buffer.subarray(0, bytesRead);
} finally {
await handle.close();
}
}
// Best effort: a file that cannot be removed should not turn a 400 into a 500,
// but it must not be left behind quietly either.
async function discardUploads(files: Express.Multer.File[]): Promise<void> {
await Promise.all(
files.map((file) =>
fs.unlink(file.path).catch((err: unknown) => {
console.error(`[upload] could not remove rejected file ${file.path}:`, err);
})
)
);
}
/**
* Removes a request's uploaded files unless the request actually succeeded.
*
* multer writes to disk before any route logic runs, and its own cleanup only
* covers errors it raised itself. Everything after that — a failed signature
* check, a malformed `category_id`, a database error, a dropped connection —
* previously left the bytes on the volume with nothing referencing them: no row
* to find them by, and no bound on how many could accumulate. Bounding the size
* of one upload does not help if every refused upload is kept forever (#180).
*
* Registered as soon as multer succeeds rather than at each `return`, so a
* route added later inherits it instead of having to remember it. That is the
* whole reason it is a hook and not a call: the failure it prevents is someone
* adding a fourth early return.
*
* `close` rather than `finish`, so an aborted connection is covered too, and
* `writableEnded` distinguishes a response that completed from one that never
* did — the latter is not a success however its status code reads.
*/
function discardUnlessAccepted(req: Request, res: Response): void {
res.on('close', () => {
if (res.writableEnded && res.statusCode < 400) return;
void discardUploads((req.files as Express.Multer.File[]) || []);
});
}
/**
* Confirms each stored file actually is what it was declared to be.
*
* This cannot happen in multer's fileFilter, which runs before the stream has
* been read — there are no bytes to look at yet. So the check runs after the
* write.
*
* Checking only: removing the files is discardUnlessAccepted's job, and doing
* it here as well would unlink twice and log an ENOENT for every refused
* upload. That also covers the case this function used to miss — `readHead`
* itself throwing, which returned no message and so cleaned up nothing.
*
* Returns the message to refuse with, or null when everything checks out.
*/
async function verifyUploadedImages(req: Request): Promise<string | null> {
const files = (req.files as Express.Multer.File[]) || [];
for (const file of files) {
const head = await readHead(file.path);
if (!signatureMatches(file.mimetype, head)) {
return `${file.originalname} does not contain ${file.mimetype} data`;
}
}
return null;
}
/**
* Rebuilds every accepted file so it carries no metadata (#226).
*
* After verification, deliberately: re-encoding a file whose bytes do not match
* its declared type would be doing work on something already refused, and
* sharp's own error would replace the clearer message that check produces.
*
* A failure here refuses the upload rather than storing the original. Storing
* it would mean the one case where a photo keeps the coordinates it was taken
* at is the case nobody was told about.
*
* Returns the message to refuse with, or null when every file was rebuilt.
*/
async function stripUploadedImages(req: Request): Promise<string | null> {
const files = (req.files as Express.Multer.File[]) || [];
for (const file of files) {
try {
await reencodeInPlace(file.path, file.mimetype);
} catch (err) {
console.error(`[upload] could not re-encode ${file.path}:`, err);
return `${file.originalname} could not be processed`;
}
}
return null;
}
// No error-handling middleware is mounted on the app, so translate multer's
// limit errors here instead of letting them surface as a generic 500.
const uploadImages = (req: Request, res: Response, next: NextFunction) => {
upload.array('images', MAX_IMAGES_PER_REQUEST)(req, res, (err: unknown) => {
if (err instanceof UnsupportedImageTypeError) {
return res.status(400).json({ error: err.message });
}
if (err instanceof multer.MulterError) {
const status = err.code === 'LIMIT_FILE_SIZE' ? 413 : 400;
return res.status(status).json({ error: err.message });
}
if (err) {
return next(err);
}
// Every file is on disk by this point and multer will not clean up after
// itself again, so the bytes become this request's responsibility before
// anything else is allowed to fail.
discardUnlessAccepted(req, res);
verifyUploadedImages(req)
.then((problem) => {
if (problem) {
res.status(400).json({ error: problem });
return null;
}
return stripUploadedImages(req);
})
.then((problem) => {
// The first stage returns null both when it answered and when it found
// nothing wrong, so the response itself is what distinguishes them.
if (res.headersSent) return;
if (problem) {
res.status(400).json({ error: problem });
return;
}
next();
})
.catch(next);
});
};
// The multipart body carries category_id and tags as text fields. An absent
// field means "leave as-is" on update, which is why these return undefined
@@ -305,34 +83,6 @@ async function setItemTags(client: PoolClient, itemId: number, tagIds: number[])
}
}
/**
* Records uploaded files as an item's images.
*
* Create and update wrote this loop separately, differing only in where the id
* came from and where the sort order started — zero for a new item, one past
* the current maximum for an existing one. Both are parameters now.
*
* It also means the `/uploads/` prefix is written once. That matters more than
* it looks: #103 made the stored value the path `uploadUrl` joins an origin
* onto, so it is a contract rather than a string, and two places to change it
* is one place to forget.
*/
async function insertItemImages(
client: PoolClient,
itemId: number,
files: Express.Multer.File[],
firstSortOrder: number
): Promise<void> {
// Iterated by entry rather than by index, so there is no possibly-undefined
// element to guard — the create path used to fall back to an empty filename,
// which would have stored a path pointing at the uploads directory itself.
for (const [offset, file] of files.entries()) {
await client.query(
`INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`,
[itemId, `/uploads/${file.filename}`, firstSortOrder + offset]
);
}
}
/**
* The two optional fields the item form submits as multipart text.
+116
View File
@@ -0,0 +1,116 @@
import { Router, Request, Response } from 'express';
import { pool, requireRow } from '../db';
import { asyncRoute } from '../asyncRoute';
import { generateToken, hashToken } from '../uploadLinks';
import { trimTrailingSlashes } from '../utils';
const router = Router();
/**
* Issuing and retiring the links that open the public intake endpoint (#222).
*
* A link is named because provenance matters more than convenience here. When
* one is shared further than intended the question is *which* one, and the
* answer has to come from somewhere — so every submission records the link it
* arrived through, and revoking kills that link rather than the feature.
*
* The token is returned by exactly one response in this file and is
* unrecoverable afterwards. That is why the admin screen has to present it as
* a one-time reveal rather than a field to come back to, and why losing it
* means issuing a new link rather than looking the old one up.
*/
/**
* Shaped so a `SELECT *` can never leak the digest into a response.
*
* Spelling the columns out is the point: `SELECT *` here would put
* `token_hash` into every listing the moment somebody added a convenience.
*/
const LINK_SELECT = `
SELECT id, label, revoked_at, submission_count, max_submissions, last_used_at, created_at
FROM upload_links
`;
/**
* The cap a link gets when nobody chose one.
*
* Not a tuned number — large enough that an ordinary contributor never meets
* it, small enough that a link shared further than intended cannot be used
* indefinitely before anyone notices. The point is that the default is finite
* at all.
*/
const DEFAULT_MAX_SUBMISSIONS = 25;
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' });
}
// Three cases, deliberately distinct. Absent means nobody decided, which
// gets the bounded default. An explicit null means unlimited — a decision
// someone made, visible in the request. A number is itself. Reading absent
// as unlimited is what would make every link unbounded by default.
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;
}
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. Absent in local development, which yields a
// relative URL the admin screen can still show and copy usefully.
const base = trimTrailingSlashes(process.env.PUBLIC_URL ?? '');
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 neither rewrite that nor
// fail — a button that errors on a double-click teaches people to distrust
// it, which is the last thing wanted on the control that contains a leak.
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]
);
const link = rows[0];
if (!link) {
return res.status(404).json({ error: 'not found' });
}
res.json(link);
}));
export default router;
+164
View File
@@ -0,0 +1,164 @@
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 { intakeViewLimiter, intakeSubmitLimiter } from '../rateLimit';
const router = Router();
/**
* The public way in: photos of one item, from someone with no account (#222).
*
* Everything here is reachable by a stranger holding a URL, so the shape of
* every refusal matters. Unknown, revoked and exhausted links are all 404 and
* indistinguishable from outside — whether a link exists is not something a
* stranger needs to be able to learn, which 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 in #223.
*/
interface LinkRow {
id: number;
label: string;
}
/** The link resolved by `requireUsableLink`, carried through 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;
}
/**
* 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 ahead of `uploadImages` is the
* whole mitigation, and a test asserts it.
*/
const requireUsableLink = asyncRoute(
async (req: Request, res: Response, next: NextFunction) => {
const link = await usableLink(req.params.token as string);
if (!link) {
res.status(404).json({ error: 'not found' });
return;
}
(req as IntakeRequest).uploadLink = link;
next();
}
);
router.get('/:token', intakeViewLimiter, asyncRoute(async (req: Request, res: Response) => {
const link = await usableLink(req.params.token as string);
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 });
}));
router.post(
'/:token',
intakeSubmitLimiter,
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 rather
// than "Untitled" so several waiting submissions stay 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 for 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 could 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;
+43
View File
@@ -0,0 +1,43 @@
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`, both of which
* are exported for their tests because they are where the real decisions live.
*/
// 32 bytes — 256 bits. base64url so the value survives being pasted into a URL,
// a chat message and a QR code without escaping, which is the whole point of a
// link somebody is handed.
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, and the reasoning is the opposite
* of the one that governs passwords. A password hash is slow on purpose,
* because a human password carries little entropy and has to survive an
* offline dictionary attack. This is 256 bits from a CSPRNG: there is no
* dictionary to try, and guessing is not a threat that slowing the hash
* addresses.
*
* Meanwhile the digest is computed on every submission request, and the intake
* endpoint is unauthenticated. A deliberately slow hash there would be a
* denial-of-service surface rather than a protection — see #242, where cost-12
* bcrypt in the test suite was enough to push a request past its timeout under
* load.
*
* No timing-safe comparison is needed. The lookup is an indexed equality match
* on the digest rather than a byte-by-byte compare of the secret, and an
* attacker able to 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');
}
+18
View File
@@ -73,3 +73,21 @@ export function tagColorFor(name: string): string {
export const MARKETING_CONSENT_TEXT =
'I want to receive occasional emails about new one-of-a-kind items from Redefined Designs. I can unsubscribe at any time.';
/**
* Strips trailing slashes so a base URL can be joined with a stored path.
*
* A loop rather than `/\/+$/`, which backtracks: sonarjs flags that pattern as
* super-linear, and the input here is an environment variable rather than
* anything hostile, but the cheap version is no harder to read.
*
* Shared because two callers now need it — `/api/config` sends
* `uploadsBaseUrl` this way, and the upload-link routes build a submission URL
* from PUBLIC_URL. Stored paths always begin with a slash, so trimming the
* base is what stops the join producing a double.
*/
export function trimTrailingSlashes(value: string): string {
let trimmed = value;
while (trimmed.endsWith('/')) trimmed = trimmed.slice(0, -1);
return trimmed;
}
@@ -0,0 +1,209 @@
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 | null): 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 as string;
}
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<{ id: number }>(`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<{ id: number; status: string; price_cents: number }>(
`SELECT id, status, price_cents FROM items`
);
expect(items).toHaveLength(1);
expect(items[0]?.status).toBe('pending');
// The migration's default, not a price anyone chose.
expect(items[0]?.price_cents).toBe(8000);
const { rows: images } = await pool.query<{ image_path: string }>(
`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<{ submission_count: number; last_used_at: string | null }>(
`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<{ id: number }>(`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 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);
});
// #226 applies here too, and this route is exactly where it matters most:
// the photo comes from a stranger's phone rather than the shop's own camera.
it('strips metadata from a submitted photo', async () => {
const token = await issueLink();
await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
const { rows } = await pool.query<{ image_path: string }>(`SELECT image_path FROM item_images`);
const sharp = (await import('sharp')).default;
const stored = await sharp(
`${UPLOADS_DIR}/${rows[0]?.image_path.replace('/uploads/', '')}`
).metadata();
expect(stored.exif).toBeUndefined();
});
});
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);
});
});
+3 -3
View File
@@ -28,9 +28,9 @@ export async function migrate(): Promise<void> {
export async function resetDb(): Promise<void> {
await testPool.query(`
TRUNCATE TABLE orders, checkout_items, checkouts, shipping_addresses, cart_items, carts,
customer_tokens, customer_sessions, favorites, customers, item_tags, item_images, items,
tags, categories
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
`);
@@ -0,0 +1,110 @@
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 hand 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<{ token_hash: string }>(
`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);
});
// Omitting the field is the common case, so it is the case that has to 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();
});
});
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();
});
// The useful fact is when access ended, so a second click must not rewrite
// it — and it must not be an error either, because a button that fails on a
// double-click teaches people to distrust it.
it('is idempotent, keeping the original timestamp', async () => {
const created = await request(app)
.post('/api/admin/upload-links')
.send({ label: 'Temporary' });
const first = 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);
expect(second.body.revoked_at).toBe(first.body.revoked_at);
});
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);
});
});
+31
View File
@@ -0,0 +1,31 @@
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()));
});
});
@@ -188,7 +188,7 @@ This is a pure refactor. No behaviour changes; the existing tests are the safety
**Interfaces:**
- Consumes: `uploadTypes.ts` (`ALLOWED_IMAGE_TYPES`, `SIGNATURE_BYTES`, `extensionFor`, `isAllowedImageType`, `signatureMatches`)
- Produces:
- `uploadImages: (req, res, next) => void` — multer middleware, field name `images`
- `uploadImages: (req, res, next) => void` — multer middleware, field name `images`. Internally runs `verifyUploadedImages` then `stripUploadedImages`, so a caller mounting this gets validation *and* EXIF stripping without asking for either
- `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`
@@ -203,9 +203,13 @@ 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`.
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`, **`stripUploadedImages`**, `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.
**`stripUploadedImages` and its `import { reencodeInPlace } from '../imageProcessing'` did not exist when this plan was written.** They arrived with #226 after it, and they are the step that removes EXIF — including the GPS coordinates a phone writes — from every accepted upload. Leaving them behind in `admin.ts` would give the public intake route in Task 5 an upload path that skips stripping entirely, which is the one thing #226 exists to prevent, and nothing would fail to say so.
The promise chain inside `uploadImages` runs `verifyUploadedImages` and then `stripUploadedImages`, and moves as a whole. Verify after moving that `routes/admin.ts` no longer imports `imageProcessing` — if it still does, something was left behind.
Keep every existing comment verbatim. They record why the code is shaped as it is (#95, #103, #180, #226) and are the most valuable thing being moved.
Export `uploadImages`, `verifyUploadedImages`, `insertItemImages`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`. Everything else stays module-private.
+2
View File
@@ -33,6 +33,7 @@ import Emails from './Emails';
import Settings from './Settings';
import Categories from './Categories';
import Tags from './Tags';
import UploadLinks from './UploadLinks';
import BuildStamp from './BuildStamp';
import CategoryTreeSelect from './CategoryTreeSelect';
import ItemCard from '../components/ItemCard';
@@ -391,6 +392,7 @@ export default function Admin() {
{ key: 'inventory', label: 'Inventory', children: <Inventory /> },
{ key: 'categories', label: 'Categories', children: <Categories /> },
{ key: 'tags', label: 'Tags', children: <Tags /> },
{ key: 'upload-links', label: 'Upload links', children: <UploadLinks /> },
{ key: 'customers', label: 'Customers', children: <Customers /> },
{ key: 'emails', label: 'Emails', children: <Emails /> },
{ key: 'settings', label: 'Settings', children: <Settings /> }
+191
View File
@@ -0,0 +1,191 @@
import { useEffect, useState } from 'react';
import Table from 'antd/es/table';
import Button 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 Checkbox from 'antd/es/checkbox';
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;
}
/** What a link gets when the form is left alone. Mirrors the server's default. */
const DEFAULT_CAP = '25';
/**
* Issuing and retiring the links that let someone without an account send in
* photos (#222).
*
* The token is shown exactly once, at creation, and cannot be recovered — the
* server stores only a digest. That is a deliberate property rather than an
* oversight, so this screen has to make the one-time nature obvious rather
* than leaving somebody to discover it by refreshing.
*/
export default function UploadLinks() {
const [links, setLinks] = useState<UploadLink[]>([]);
const [label, setLabel] = useState('');
const [cap, setCap] = useState(DEFAULT_CAP);
const [unlimited, setUnlimited] = useState(false);
// Held only in component state and shown once. A refresh loses it, which is
// the honest behaviour: the server genuinely cannot produce it again.
const [issued, setIssued] = useState<string | null>(null);
const [error, setError] = useState<string | null>(null);
const [creating, setCreating] = useState(false);
async function load() {
const res = await fetch('/api/admin/upload-links');
if (res.ok) setLinks(await res.json());
}
// Load-on-mount, the same shape Tags and Categories use. `load` only sets
// state after its fetch resolves, so nothing here is synchronous.
// eslint-disable-next-line react-hooks/set-state-in-effect
useEffect(() => { void load(); }, []);
async function create() {
setCreating(true);
setError(null);
const res = await fetch('/api/admin/upload-links', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
// Sent explicitly in all three cases rather than omitted. null is how
// unlimited is asked for; the server's default only has to cover callers
// that are not this screen.
body: JSON.stringify({
label,
maxSubmissions: unlimited ? null : Number(cap)
})
});
setCreating(false);
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(DEFAULT_CAP);
setUnlimited(false);
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 already sent through it is kept.
</Paragraph>
<Space wrap>
<Input
placeholder="Who or what is this for?"
aria-label="Link label"
value={label}
onChange={(e) => setLabel(e.target.value)}
style={{ width: 260 }}
/>
<Input
placeholder="Max uses"
aria-label="Maximum 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>
<Button type="primary" onClick={create} loading={creating} disabled={label.trim() === ''}>
Create link
</Button>
</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."
// Named after the action rather than left as "OK", matching
// the okText this admin's other destructive confirms use.
// A confirm button that says OK makes the reader re-read the
// question to find out what they are agreeing to.
okText="Revoke"
okButtonProps={{ danger: true }}
onConfirm={() => revoke(row.id)}
>
<Button danger size="small">
Revoke
</Button>
</Popconfirm>
)
}
]}
/>
</Space>
);
}
+183
View File
@@ -0,0 +1,183 @@
import { 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 Button 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 { UploadOutlined } from '@ant-design/icons';
import type { UploadFile } from 'antd/es/upload/interface';
import { fetchIntakeLink, submitItem } from './intakeApi';
import type { LinkState } from './intakeApi';
const { Title, Paragraph } = Typography;
const { TextArea } = Input;
/**
* Where someone with no account sends in photos of one item (#222).
*
* The three types the server will accept, and the same per-request cap. Listed
* here so the file picker offers exactly what will be taken and the count is
* bounded before anything is uploaded — but the server checks both again,
* because everything on this page is under the sender's control.
*/
const ACCEPT = 'image/jpeg,image/png,image/webp';
const MAX_IMAGES = 6;
export default function Submit() {
const { token = '' } = useParams();
const [state, setState] = useState<LinkState>({ kind: 'unusable' });
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((result) => {
// The token can change if the URL does, and a late response from the
// previous one would otherwise overwrite the current answer.
if (cancelled) return;
setState(result);
setChecking(false);
});
return () => {
cancelled = true;
};
}, [token]);
async function send() {
setSending(true);
setError(null);
const result = await submitItem(
token,
// originFileObj is what antd hands back for a file it did not upload
// itself; beforeUpload returning false is what keeps them here. flatMap
// rather than map-then-filter because a type predicate cannot narrow to
// File here — antd's RcFile extends it, so the predicate would widen.
files.flatMap((f) => (f.originFileObj ? [f.originFileObj] : [])),
note
);
setSending(false);
if (result.ok) {
setSent(true);
return;
}
setError(result.error);
}
if (checking) {
return (
<div style={{ maxWidth: 640, margin: '48px auto', padding: '0 16px', textAlign: 'center' }}>
<Spin />
</div>
);
}
// Kept apart from the state below on purpose. The two need opposite
// reactions — wait a moment, versus go and ask for a different link — so
// telling a throttled sender their link was dead would send them to fetch a
// replacement that could not have helped.
if (state.kind === 'throttled') {
return (
<div style={{ maxWidth: 640, margin: '48px auto', padding: '0 16px' }}>
<Card>
<Title level={3}>Too many requests just now</Title>
<Paragraph>
Your link is fine this page has just been asked for too many times from your
connection. Wait a minute and reload.
</Paragraph>
</Card>
</div>
);
}
// One state for unknown, revoked and used-up alike, matching the server's
// single 404. Saying which it was would tell a stranger whether a link they
// guessed at exists.
if (state.kind === 'unusable') {
return (
<div style={{ maxWidth: 640, margin: '48px auto', padding: '0 16px' }}>
<Card>
<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>
</div>
);
}
if (sent) {
return (
<div style={{ maxWidth: 640, margin: '48px auto', padding: '0 16px' }}>
<Card>
<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>
<Button
onClick={() => {
setFiles([]);
setNote('');
setSent(false);
}}
>
Send another item
</Button>
</Card>
</div>
);
}
return (
<div style={{ maxWidth: 640, margin: '48px auto', padding: '0 16px' }}>
<Card>
<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 stops antd uploading each file the moment it is
// picked; they are sent together by `send` instead, which is what
// makes this one request the server can accept or refuse as a unit.
beforeUpload={() => false}
onChange={({ fileList }) => setFiles(fileList)}
>
<Button icon={<UploadOutlined />}>Choose photos</Button>
</Upload>
<TextArea
rows={4}
value={note}
onChange={(e) => setNote(e.target.value)}
aria-label="Anything you know about this item"
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 />}
<Button type="primary" onClick={send} loading={sending} disabled={files.length === 0}>
Send
</Button>
</Space>
</Card>
</div>
);
}
+59
View File
@@ -0,0 +1,59 @@
export interface IntakeLink {
label: string;
}
export type LinkState =
| { kind: 'usable'; link: IntakeLink }
| { kind: 'unusable' }
| { kind: 'throttled' };
/**
* Whether a link works, and what it is called.
*
* The server answers 404 for unknown, revoked and exhausted links alike, and
* that is deliberate — whether a link exists is not something a stranger needs
* to learn. So all three collapse into one `unusable` state here rather than
* several the page would have to explain.
*
* A 429 is deliberately *not* one of them. This used to treat any non-OK
* response as "no link", which meant a rate-limited sender was told their link
* was dead — sending them to ask for a replacement that would not have helped,
* because the problem was the address they were coming from and a minute of
* patience. Two conditions that need opposite reactions must not share a
* message.
*/
export async function fetchIntakeLink(token: string): Promise<LinkState> {
const res = await fetch(`/api/intake/${encodeURIComponent(token)}`);
if (res.status === 429) return { kind: 'throttled' };
if (!res.ok) return { kind: 'unusable' };
return { kind: 'usable', link: await res.json() };
}
export type SubmitResult = { ok: true } | { ok: false; error: string };
export async function submitItem(
token: string,
files: File[],
note: string
): Promise<SubmitResult> {
const body = new FormData();
// The field name the server's multer instance listens on. Sending several
// under one name is what makes req.files an array.
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 };
// The server's message is the useful one — it names the offending file for a
// type or content refusal. The fallback covers a response that is not JSON
// at all, which is what a proxy error looks like.
const payload = await res.json().catch(() => ({}));
return { ok: false, error: payload.error ?? 'Something went wrong. Please try again.' };
}
+3
View File
@@ -15,6 +15,7 @@ import Admin from './admin/Admin';
import AuthRouteModal from './customer/AuthRouteModal';
import Account from './customer/Account';
import PrivacyPolicy from './customer/PrivacyPolicy';
import Submit from './intake/Submit';
import VerifyEmail from './customer/VerifyEmail';
import ForgotPassword from './customer/ForgotPassword';
import ResetPassword from './customer/ResetPassword';
@@ -106,6 +107,8 @@ function AppRoutes() {
MODAL_ROUTES would put it back in the 700px box it just left. */}
<Route path="/orders" element={<Orders />} />
<Route path="/privacy" element={<PrivacyPolicy />} />
{/* Public and unauthenticated: the token is the whole access control. */}
<Route path="/submit/:token" element={<Submit />} />
<Route path="/verify-email" element={<VerifyEmail />} />
</Routes>
{/* Rendered outside the Routes above, which are showing the backdrop. */}
@@ -0,0 +1,72 @@
import { test, expect, uniqueSuffix } from './fixtures';
/**
* Issuing and revoking upload links from the admin (#222).
*
* Every link is labelled with a fresh run id and every assertion is scoped to
* that row. The suite is fullyParallel against one shared database, so an
* assertion on the table as a whole would be an assertion on whatever other
* specs happen to be doing (#241).
*/
test.describe('Managing upload links', () => {
test('issues a link, shows its token once, and lists it bounded', async ({ page, admin }) => {
const label = `Sarah ${uniqueSuffix()}`;
await admin.goto();
await page.getByRole('tab', { name: 'Upload links' }).click();
await page.getByLabel('Link label').fill(label);
await page.getByRole('button', { name: 'Create link' }).click();
// Shown exactly once. The server keeps only a digest, so there is no
// reveal to come back to — the copy has to say so.
await expect(page.getByText('Copy this link now')).toBeVisible();
await expect(page.getByText(/\/submit\//)).toBeVisible();
await expect(page.getByText(/cannot be shown again/)).toBeVisible();
const row = page.getByRole('row', { name: new RegExp(label) });
await expect(row).toBeVisible();
// The default cap, not unlimited. An unbounded link should be asked for.
await expect(row.getByText('0 of 25')).toBeVisible();
await expect(row.getByText('Active')).toBeVisible();
});
test('revokes a link, naming the action on the confirm', async ({ page, admin }) => {
const label = `Temporary ${uniqueSuffix()}`;
await admin.goto();
await page.getByRole('tab', { name: 'Upload links' }).click();
await page.getByLabel('Link label').fill(label);
await page.getByRole('button', { name: 'Create link' }).click();
const row = page.getByRole('row', { name: new RegExp(label) });
await row.getByRole('button', { name: 'Revoke' }).click();
// The confirm names what it does rather than saying OK, which is what the
// rest of this admin's destructive actions do.
await page.getByRole('tooltip').getByRole('button', { name: 'Revoke' }).click();
await expect(row.getByText('Revoked')).toBeVisible();
// The row stays: what arrived through the link is kept, and the record of
// where it came from with it.
await expect(row).toBeVisible();
});
// Blank-means-unlimited would make the least deliberate action produce the
// least bounded link, so unlimited is a checkbox rather than an empty field.
test('makes unlimited a deliberate choice', async ({ page, admin }) => {
const label = `Always on ${uniqueSuffix()}`;
await admin.goto();
await page.getByRole('tab', { name: 'Upload links' }).click();
await page.getByLabel('Link label').fill(label);
await page.getByText('No limit').click();
await page.getByRole('button', { name: 'Create link' }).click();
const row = page.getByRole('row', { name: new RegExp(label) });
await expect(row).toBeVisible();
// A bare count rather than "0 of N".
await expect(row.getByText('0 of', { exact: false })).toHaveCount(0);
});
});
+76
View File
@@ -0,0 +1,76 @@
import { test, expect, createAdminContext, uniqueSuffix } from './fixtures';
/**
* The public submission page (#222).
*
* Every fixture here carries a run id and every assertion names only what this
* run created. The suite is fullyParallel against one shared database, so a
* spec that asserts on anything catalogue-wide is asserting on other specs too
* (#241).
*/
const RUN = `i${uniqueSuffix()}`;
let token = '';
test.beforeAll(async ({ playwright }) => {
const api = await createAdminContext(playwright);
const res = await api.post('/api/admin/upload-links', {
data: { label: `Intake spec ${RUN}` }
});
expect(res.status(), 'creating the upload link').toBe(201);
token = (await res.json()).token;
await api.dispose();
});
test.describe('Sending in an item through a link', () => {
test('shows the form for a link that works', async ({ page }) => {
await page.goto(`/submit/${token}`);
await expect(page.getByRole('heading', { name: 'Send in an item' })).toBeVisible();
await expect(page.getByRole('button', { name: 'Choose photos' })).toBeVisible();
});
// Nothing to send is not a submission, and the server would refuse it — but
// the sender should not have to find that out by pressing the button.
test('will not send until a photo is chosen', async ({ page }) => {
await page.goto(`/submit/${token}`);
await expect(page.getByRole('button', { name: 'Send' })).toBeDisabled();
});
// One state for every refusal, matching the server's single 404. Saying which
// kind of dead a link is would tell a stranger whether one they guessed at
// exists.
test('explains an unusable link without saying which kind', async ({ page }) => {
await page.goto('/submit/not-a-real-token');
await expect(page.getByRole('heading', { name: 'This link is not active' })).toBeVisible();
await expect(page.getByRole('button', { name: 'Choose photos' })).toHaveCount(0);
});
test('accepts a photo and says it arrived', async ({ page }) => {
await page.goto(`/submit/${token}`);
// A real 1x1 PNG, so the server's magic-byte check sees what it expects
// rather than a buffer that merely starts correctly.
const png = Buffer.from(
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==',
'base64'
);
await page.setInputFiles('input[type="file"]', {
name: `${RUN}.png`,
mimeType: 'image/png',
buffer: png
});
await page.getByLabel('Anything you know about this item').fill(`Stoneware ${RUN}`);
await page.getByRole('button', { name: 'Send' }).click();
await expect(page.getByRole('heading', { name: 'Thank you — it arrived' })).toBeVisible();
// The wording matters: it is what stops a sender wondering why their item
// is not on the site.
await expect(page.getByText(/Nothing is listed for sale until/)).toBeVisible();
});
});