feat(intake): refuse submissions past the daily ceiling, and reset it from the admin (#227)
Linting / lint (pull_request) Successful in 2m5s
SonarQube Analysis / sonarqube (pull_request) Successful in 26m27s

The check is ordered ahead of uploadImages, for the same reason requireUsableLink is: a refused submission must write zero bytes. Ordering it after would accept the upload, store the files and then throw them away, which is the expensive half of the work the ceiling exists to prevent. A test asserts nothing is stored.

503, not 403. The sender has done nothing wrong, their link is fine, and the condition clears by itself as the window rolls — so the link stays usable and works again the moment there is room.

The ceiling never touches the admin upload path, which has its own test. Intake being throttled is an inconvenience; the shop being unable to add its own stock is an outage.

reset-ceiling is declared above /:id/revoke because Express matches in order and would otherwise read it as an id and try to revoke a link named "reset-ceiling". That has its own test too.

The per-link alert is a named function rather than the inline IIFE the plan wrote. routesAreWrapped.test.ts flags any async inside a route registration not directly wrapped in asyncRoute, and it cannot tell an inner IIFE from an unwrapped handler — nor should it have to. The guard caught this, and the extraction reads better than what it rejected.

Backend now 373 unit and 337 integration, all passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-01 16:29:00 -05:00
co-authored by Claude Opus 5
parent 55cb7eaa13
commit 92c04847d6
3 changed files with 223 additions and 0 deletions
+22
View File
@@ -94,6 +94,28 @@ router.post('/', asyncRoute(async (req: Request, res: Response) => {
res.status(201).json({ ...link, token, url: `${base}/submit/${token}` }); res.status(201).json({ ...link, token, url: `${base}/submit/${token}` });
})); }));
/**
* Forgives the current ceiling window without deleting anything.
*
* The count is derived from item_drafts rows, which are real submissions with
* real items in the review queue — so a reset moves the window's start rather
* than removing anything. Recovery is automatic as the window rolls; this is
* for the case where the ceiling was hit legitimately and waiting is not
* acceptable.
*
* Declared above `/:id/revoke` deliberately: Express matches in order, and
* `reset-ceiling` would otherwise be read as an id.
*/
router.post('/reset-ceiling', asyncRoute(async (_req: Request, res: Response) => {
await pool.query(
`INSERT INTO admin_settings (key, value, updated_at)
VALUES ('intake_ceiling_reset_at', $1, now())
ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`,
[new Date().toISOString()]
);
res.json({ reset: true });
}));
router.post('/:id/revoke', asyncRoute(async (req: Request, res: Response) => { router.post('/:id/revoke', asyncRoute(async (req: Request, res: Response) => {
// COALESCE so revoking twice keeps the original timestamp. The useful fact // COALESCE so revoking twice keeps the original timestamp. The useful fact
// is when access ended, and a second click should neither rewrite that nor // is when access ended, and a second click should neither rewrite that nor
+58
View File
@@ -5,6 +5,9 @@ import { hashToken } from '../uploadLinks';
import { uploadImages, verifyUploadedImages, insertItemImages } from '../imageUpload'; import { uploadImages, verifyUploadedImages, insertItemImages } from '../imageUpload';
import { intakeViewLimiter, intakeSubmitLimiter } from '../rateLimit'; import { intakeViewLimiter, intakeSubmitLimiter } from '../rateLimit';
import { draftQueued } from '../intake/draftingWorker'; import { draftQueued } from '../intake/draftingWorker';
import { checkCapacity, countForLinkSince, windowStart } from '../intake/capacity';
import { alertCeilingReached, alertLinkThreshold } from '../intake/abuseAlert';
import { getSettings } from '../adminSettings';
const router = Router(); const router = Router();
@@ -61,6 +64,53 @@ async function usableLink(token: string): Promise<LinkRow | null> {
* and the unlink would skip. Ordering this ahead of `uploadImages` is the * and the unlink would skip. Ordering this ahead of `uploadImages` is the
* whole mitigation, and a test asserts it. * whole mitigation, and a test asserts it.
*/ */
/**
* Refuses when the intake surface as a whole is over its ceiling.
*
* Ordered ahead of `uploadImages` for the same reason `requireUsableLink` is: a
* refused submission must write zero bytes to disk. Ordering it after would
* accept the upload, store the files and then throw them away, which is the
* expensive half of the work this exists to prevent.
*
* 503 rather than 403. The sender has done nothing wrong, their link is fine,
* and the condition clears by itself as the window rolls.
*/
/**
* Alerts when one link crosses its threshold.
*
* A named function rather than an inline IIFE in the handler. The wrapper guard
* flags any `async` inside a route registration that is not directly preceded
* by `asyncRoute(`, and it cannot tell an inner IIFE from an unwrapped handler —
* nor should it have to.
*/
async function alertIfLinkIsBusy(link: LinkRow): Promise<void> {
const { intakeLinkAlertThreshold, intakeCeilingResetAt } = await getSettings();
const used = await countForLinkSince(link.id, windowStart(new Date(), intakeCeilingResetAt));
if (used >= intakeLinkAlertThreshold) {
await alertLinkThreshold(link.id, link.label, used, intakeLinkAlertThreshold);
}
}
const requireCapacity = asyncRoute(
async (_req: Request, res: Response, next: NextFunction) => {
const verdict = await checkCapacity();
if (verdict.allowed) {
next();
return;
}
// Not awaited: an alert that fails must not become a failed request for
// somebody who has done nothing wrong, and the refusal is already decided.
void alertCeilingReached(verdict.used, verdict.ceiling).catch((err) =>
console.error('[intake] ceiling alert failed:', err)
);
res.status(503).json({
error: 'we are not able to accept submissions right now — please try again later'
});
}
);
const requireUsableLink = asyncRoute( const requireUsableLink = asyncRoute(
async (req: Request, res: Response, next: NextFunction) => { async (req: Request, res: Response, next: NextFunction) => {
const link = await usableLink(req.params.token as string); const link = await usableLink(req.params.token as string);
@@ -86,6 +136,7 @@ router.post(
'/:token', '/:token',
intakeSubmitLimiter, intakeSubmitLimiter,
requireUsableLink, requireUsableLink,
requireCapacity,
uploadImages, uploadImages,
asyncRoute(async (req: Request, res: Response) => { asyncRoute(async (req: Request, res: Response) => {
// Set by requireUsableLink above. Re-checked rather than asserted non-null, // Set by requireUsableLink above. Re-checked rather than asserted non-null,
@@ -157,6 +208,13 @@ router.post(
// is a few minutes' delay rather than a lost submission. // is a few minutes' delay rather than a lost submission.
void draftQueued(1).catch((err) => console.error('[drafting] after submission:', err)); void draftQueued(1).catch((err) => console.error('[drafting] after submission:', err));
// The signal that a link has been shared further than intended, which is
// the case the revoke mechanism exists for and which otherwise depends on
// somebody happening to look. Not awaited, for the same reason as above.
void alertIfLinkIsBusy(link).catch((err) =>
console.error('[intake] link threshold alert failed:', err)
);
// No item id in the response: the sender has no business knowing about // No item id in the response: the sender has no business knowing about
// the catalogue, and nothing they could do with it. // the catalogue, and nothing they could do with it.
res.status(201).json({ ok: true }); res.status(201).json({ ok: true });
@@ -0,0 +1,143 @@
import request from 'supertest';
import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
import { resetAlertThrottleForTests } from '../../src/intake/abuseAlert';
const PNG = Buffer.from(
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==',
'base64'
);
beforeEach(async () => {
await resetDb();
resetAlertThrottleForTests();
});
afterAll(async () => {
await pool.end();
await closeDb();
});
async function makeLink(): Promise<string> {
const res = await request(app).post('/api/admin/upload-links').send({ label: 'ceiling spec' });
expect(res.status).toBe(201);
return res.body.token;
}
/** Fills the window with drafts, as though earlier submissions had arrived. */
async function fillWindow(count: number): Promise<void> {
for (let i = 0; i < count; i++) {
const { rows } = await pool.query<{ id: number }>(
`INSERT INTO items (name, status) VALUES ('filler', 'pending') RETURNING id`
);
await pool.query(`INSERT INTO item_drafts (item_id) VALUES ($1)`, [rows[0]!.id]);
}
}
async function setCeiling(value: number): Promise<void> {
await pool.query(
`INSERT INTO admin_settings (key, value, updated_at) VALUES ('intake_daily_ceiling', $1, now())
ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`,
[String(value)]
);
}
const submit = (token: string) =>
request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
const draftCount = async (): Promise<number> =>
(await pool.query<{ c: number }>(`SELECT count(*)::int AS c FROM item_drafts`)).rows[0]?.c ?? 0;
describe('the submission ceiling', () => {
it('accepts a submission below the ceiling', async () => {
await setCeiling(5);
const token = await makeLink();
expect((await submit(token)).status).toBe(201);
});
it('refuses with 503 once the ceiling is reached', async () => {
await setCeiling(2);
await fillWindow(2);
const token = await makeLink();
const res = await submit(token);
expect(res.status).toBe(503);
expect(res.body.error).toMatch(/later/i);
});
// Nothing the sender did is wrong and the condition clears by itself, so this
// must not read as a rejection of them or of their link.
it('leaves the link usable, so it works again when there is room', async () => {
await setCeiling(1);
await fillWindow(1);
const token = await makeLink();
expect((await submit(token)).status).toBe(503);
await setCeiling(50);
expect((await submit(token)).status).toBe(201);
});
// A refused submission must write nothing. The check is ordered ahead of
// uploadImages for the same reason requireUsableLink is.
it('stores no item when it refuses', async () => {
await setCeiling(1);
await fillWindow(1);
const token = await makeLink();
await submit(token);
expect(await draftCount()).toBe(1);
});
/**
* The constraint that matters most. Intake being throttled is an
* inconvenience; the shop being unable to add its own stock is an outage.
*/
it('never applies to the admin upload path', async () => {
await setCeiling(1);
await fillWindow(5);
const res = await request(app)
.post('/api/admin/items')
.field('name', 'admin adds stock')
.field('price', '42.00')
.attach('images', PNG, 'a.png');
// 200, not 201 — this route answers with the created item rather than a
// bare created status, unlike the intake route.
expect(res.status).toBe(200);
});
it('accepts again after the window is reset', async () => {
await setCeiling(1);
await fillWindow(1);
const token = await makeLink();
expect((await submit(token)).status).toBe(503);
const reset = await request(app).post('/api/admin/upload-links/reset-ceiling');
expect(reset.status).toBe(200);
expect((await submit(token)).status).toBe(201);
});
// A reset forgives; it does not erase. Those submissions are real and their
// items are in the review queue.
it('keeps the submissions it counted after a reset', async () => {
await fillWindow(3);
await request(app).post('/api/admin/upload-links/reset-ceiling');
expect(await draftCount()).toBe(3);
});
// reset-ceiling is declared above /:id/revoke, so Express must not read it as
// an id and try to revoke a link called "reset-ceiling".
it('does not collide with the revoke route', async () => {
const res = await request(app).post('/api/admin/upload-links/reset-ceiling');
expect(res.body).toEqual({ reset: true });
});
});