diff --git a/backend/src/app.ts b/backend/src/app.ts index c254692..b0177dd 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -11,6 +11,7 @@ import adminCategoriesRouter from './routes/adminCategories'; import adminTagsRouter from './routes/adminTags'; import adminUploadLinksRouter from './routes/adminUploadLinks'; import adminItemDraftsRouter from './routes/adminItemDrafts'; +import intakeActionsRouter from './routes/intakeActions'; import intakeRouter from './routes/intake'; import adminVersionRouter from './routes/adminVersion'; import filtersRouter from './routes/filters'; @@ -68,6 +69,7 @@ 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/intake-actions', intakeActionsRouter); 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 diff --git a/backend/src/routes/intakeActions.ts b/backend/src/routes/intakeActions.ts new file mode 100644 index 0000000..7e60867 --- /dev/null +++ b/backend/src/routes/intakeActions.ts @@ -0,0 +1,129 @@ +import { Router, Request, Response } from 'express'; +import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { IntakeAction, verifyAction } from '../intake/actionLinks'; + +const router = Router(); + +const ACTIONS: readonly IntakeAction[] = ['regenerate', 'discard']; + +function isAction(value: string): value is IntakeAction { + return (ACTIONS as readonly string[]).includes(value); +} + +interface Checked { + itemId: number; + action: IntakeAction; +} + +/** + * Public, and protected by the signature rather than by the admin gate. + * + * These are clicked from an inbox by someone who is not signed in, which is the + * whole point of them. Neither action can publish: the worst outcome of a + * leaked link is a wasted API call or a hide the review queue can undo, and + * that is exactly what makes putting them in an email acceptable. + */ +function check(req: Request, res: Response): Checked | null { + const action = req.params.action ?? ''; + if (!isAction(action)) { + res.status(404).json({ error: 'unknown action' }); + return null; + } + + const itemId = Number(req.params.itemId); + const expiresAt = Number(req.query.expires); + const sig = typeof req.query.sig === 'string' ? req.query.sig : ''; + + if (!Number.isInteger(itemId) || !verifyAction(itemId, action, expiresAt, sig)) { + // One response for a forged signature, an expired link and an unconfigured + // secret alike. Distinguishing them would tell somebody probing which of + // those they had achieved. + res.status(403).json({ error: 'this link is not valid, or has expired' }); + return null; + } + + return { itemId, action }; +} + +/** + * Confirms, and changes nothing. + * + * Mail scanners and corporate link-rewriting gateways issue a GET against every + * URL in a message before a human ever sees it. A GET that discarded a draft + * would therefore fire itself on delivery, carrying a valid signature and + * looking entirely legitimate in the log — and nobody would know to go and + * recover it. So the state change lives on POST, and this exists only to let a + * person confirm what they are about to do. + */ +router.get( + '/:itemId/:action', + asyncRoute(async (req: Request, res: Response) => { + const checked = check(req, res); + if (!checked) return; + + const { rows } = await pool.query<{ item_name: string; state: string }>( + `SELECT i.name AS item_name, d.state + FROM item_drafts d JOIN items i ON i.id = d.item_id + WHERE d.item_id = $1`, + [checked.itemId] + ); + if (!rows[0]) return res.status(404).json({ error: 'no draft for this item' }); + + res.json({ + itemId: checked.itemId, + action: checked.action, + itemName: rows[0].item_name, + state: rows[0].state, + confirmWith: 'POST to this same url' + }); + }) +); + +router.post( + '/:itemId/:action', + asyncRoute(async (req: Request, res: Response) => { + const checked = check(req, res); + if (!checked) return; + + if (checked.action === 'regenerate') { + // attempts cleared with the state, for the same reason the admin route + // does it: the worker only picks up rows below the attempt cap, so + // re-queueing an exhausted draft without clearing them would do nothing + // and say nothing. + const { rowCount } = await pool.query( + `UPDATE item_drafts SET state = 'queued', attempts = 0, ai_error = NULL WHERE item_id = $1`, + [checked.itemId] + ); + if (rowCount === 0) return res.status(404).json({ error: 'no draft for this item' }); + return res.json({ state: 'queued' }); + } + + const client = await pool.connect(); + try { + await client.query('BEGIN'); + const { rowCount } = await client.query( + `UPDATE item_drafts SET state = 'discarded' WHERE item_id = $1`, + [checked.itemId] + ); + if (rowCount === 0) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'no draft for this item' }); + } + // Nothing is deleted, here or in the admin route. Discard is reachable in + // one click from an inbox, and the photographs are often the only copy of + // something no longer in the sender's hands. + await client.query(`UPDATE items SET status = 'pending' WHERE id = $1`, [checked.itemId]); + await client.query('COMMIT'); + res.json({ state: 'discarded' }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } + }) +); + +export default router; diff --git a/backend/tests/integration/adminSettings.integration.test.ts b/backend/tests/integration/adminSettings.integration.test.ts index 43d41e3..63470e1 100644 --- a/backend/tests/integration/adminSettings.integration.test.ts +++ b/backend/tests/integration/adminSettings.integration.test.ts @@ -30,7 +30,10 @@ describe('GET /api/admin/settings', () => { passwordResetHours: 1, greetingFormat: 'Hi {{firstName}},', greetingFallback: 'Hi,', - draftingModel: 'claude-sonnet-5' + draftingModel: 'claude-sonnet-5', + // Empty by default: nowhere to send the intake notification is a working + // configuration, and means simply do not send one (#224). + intakeNotifyEmail: '' }); }); }); diff --git a/backend/tests/integration/emailTemplates.integration.test.ts b/backend/tests/integration/emailTemplates.integration.test.ts index d4738c8..bd07bce 100644 --- a/backend/tests/integration/emailTemplates.integration.test.ts +++ b/backend/tests/integration/emailTemplates.integration.test.ts @@ -28,6 +28,7 @@ describe('GET /api/admin/email-templates', () => { 'emailChanged', 'favoriteSold', 'favoriteWithdrawn', + 'intakeDraft', 'passwordReset', 'verification' ]); diff --git a/backend/tests/integration/intakeActions.integration.test.ts b/backend/tests/integration/intakeActions.integration.test.ts new file mode 100644 index 0000000..00c8c02 --- /dev/null +++ b/backend/tests/integration/intakeActions.integration.test.ts @@ -0,0 +1,185 @@ +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; +import { signAction, ACTION_TTL_MS } from '../../src/intake/actionLinks'; +import { notifyDraftReady } from '../../src/intake/notifyDraft'; + +const SECRET = 'integration-intake-secret'; +const original = process.env.INTAKE_ACTION_SECRET; + +beforeAll(() => { + process.env.INTAKE_ACTION_SECRET = SECRET; +}); + +afterAll(async () => { + if (original === undefined) delete process.env.INTAKE_ACTION_SECRET; + else process.env.INTAKE_ACTION_SECRET = original; + await pool.end(); + await closeDb(); +}); + +beforeEach(async () => { + await resetDb(); +}); + +async function seedDraft(): Promise { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Submission', 'pending') RETURNING id` + ); + const itemId = rows[0]!.id; + await pool.query( + `INSERT INTO item_drafts (item_id, state, ai_name) VALUES ($1, 'ready', 'Blue vase')`, + [itemId] + ); + return itemId; +} + +const soon = (): number => Date.now() + ACTION_TTL_MS; + +function link(itemId: number, action: 'regenerate' | 'discard', expiresAt: number): string { + const sig = signAction(itemId, action, expiresAt); + return `/api/intake-actions/${itemId}/${action}?expires=${expiresAt}&sig=${sig}`; +} + +const stateOf = async (itemId: number): Promise => + (await pool.query<{ state: string }>(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId])) + .rows[0]?.state; + +describe('the signed action links', () => { + /** + * The reason GET does not act. Mail scanners and link-rewriting gateways + * issue a GET against every URL in a message before a human sees it, so a GET + * that discarded a draft would fire itself on delivery — with a valid + * signature, looking entirely legitimate in the log, and nobody would know to + * go and recover it. + */ + it('GET confirms without changing anything', async () => { + const itemId = await seedDraft(); + + const res = await request(app).get(link(itemId, 'discard', soon())); + + expect(res.status).toBe(200); + expect(res.body.action).toBe('discard'); + expect(await stateOf(itemId)).toBe('ready'); + }); + + it('POST discards and unpublishes the item', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post(link(itemId, 'discard', soon())); + + expect(res.status).toBe(200); + expect(await stateOf(itemId)).toBe('discarded'); + const item = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(item.rows[0]?.status).toBe('pending'); + }); + + // Nothing is deleted, which is what makes a link in an inbox acceptable. + it('POST discard keeps the item and its row', async () => { + const itemId = await seedDraft(); + await request(app).post(link(itemId, 'discard', soon())); + + const item = await pool.query(`SELECT id FROM items WHERE id = $1`, [itemId]); + expect(item.rows).toHaveLength(1); + }); + + it('POST regenerates and clears the attempts', async () => { + const itemId = await seedDraft(); + await pool.query(`UPDATE item_drafts SET attempts = 3 WHERE item_id = $1`, [itemId]); + + await request(app).post(link(itemId, 'regenerate', soon())); + + const { rows } = await pool.query( + `SELECT state, attempts FROM item_drafts WHERE item_id = $1`, + [itemId] + ); + expect(rows[0]).toMatchObject({ state: 'queued', attempts: 0 }); + }); + + it('refuses a forged signature', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post( + `/api/intake-actions/${itemId}/discard?expires=${soon()}&sig=forged` + ); + + expect(res.status).toBe(403); + expect(await stateOf(itemId)).toBe('ready'); + }); + + // The signature names the item, so one link must not act on another. + it('refuses a signature minted for a different item', async () => { + const mine = await seedDraft(); + const other = await seedDraft(); + const expires = soon(); + const sig = signAction(other, 'discard', expires); + + const res = await request(app).post( + `/api/intake-actions/${mine}/discard?expires=${expires}&sig=${sig}` + ); + + expect(res.status).toBe(403); + expect(await stateOf(mine)).toBe('ready'); + }); + + // And it names the action, so a regenerate link cannot be upgraded. + it('refuses a signature minted for a different action', async () => { + const itemId = await seedDraft(); + const expires = soon(); + const sig = signAction(itemId, 'regenerate', expires); + + const res = await request(app).post( + `/api/intake-actions/${itemId}/discard?expires=${expires}&sig=${sig}` + ); + + expect(res.status).toBe(403); + expect(await stateOf(itemId)).toBe('ready'); + }); + + it('refuses an expired link', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post(link(itemId, 'discard', Date.now() - 1000)); + + expect(res.status).toBe(403); + expect(await stateOf(itemId)).toBe('ready'); + }); + + // There is no signable publish, and asking for one must not find a handler. + it('refuses an action it does not recognise', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post( + `/api/intake-actions/${itemId}/publish?expires=${soon()}&sig=anything` + ); + + expect(res.status).toBe(404); + }); + + it('404s for an item with no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary') RETURNING id` + ); + + const res = await request(app).post(link(rows[0]!.id, 'discard', soon())); + + expect(res.status).toBe(404); + }); +}); + +describe('the notification itself', () => { + // Nowhere to send it is a working configuration, and must not throw into the + // worker and fail a draft that was written correctly. + it('does nothing when no recipient is configured', async () => { + const itemId = await seedDraft(); + await expect(notifyDraftReady(itemId)).resolves.toBeUndefined(); + }); + + it('does not throw when the item has no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary') RETURNING id` + ); + await expect(notifyDraftReady(rows[0]!.id)).resolves.toBeUndefined(); + }); +});