feat(intake): act on a signed link from the notification (#224)

Mounted publicly, deliberately not behind requireAdminGate. These are clicked from an inbox by someone who is not signed in, which is the whole point; the signature is what protects them. GET confirms and changes nothing, POST acts. Mail scanners and corporate 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 — carrying a valid signature, looking entirely legitimate in the log, and nobody would know to go and recover it. That is the case the split exists for and it has its own test. Forged, replayed, upgraded and expired links are each refused with the same 403. Distinguishing them would tell somebody probing which of those they had achieved. There is no signable publish, and asking for one finds no handler. The two registry guard tests are updated rather than worked around: they assert the full set of settings and template keys, so adding either is exactly what should trip them. Backend now 367 unit and 329 integration, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-01 15:32:07 -05:00
parent faed47e105
commit 346e9eae4c
5 changed files with 321 additions and 1 deletions
@@ -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
@@ -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;
@@ -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: ''
    });
  });
 });
@@ -28,6 +28,7 @@ describe('GET /api/admin/email-templates', () => {
      'emailChanged',
      'favoriteSold',
      'favoriteWithdrawn',
+      'intakeDraft',
      'passwordReset',
      'verification'
    ]);
@@ -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<number> {
+  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<string | undefined> =>
+  (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();
+  });
+});