Feature/224 notification impl #265

Merged
bermudalamb merged 5 commits from feature/224-notification-impl into main 2026-09-01 15:47:28 -05:00
17 changed files with 692 additions and 5 deletions
+7 -1
View File
@@ -34,7 +34,13 @@ const DEFINITIONS = [
name: 'draftingModel', name: 'draftingModel',
type: 'choice', type: 'choice',
fallback: DEFAULT_DRAFTING_MODEL fallback: DEFAULT_DRAFTING_MODEL
} },
// Where the intake notification goes (#224). A setting rather than an
// environment variable, for the same reason drafting_model is one: it is
// changed by whoever runs the shop, not by whoever deploys it, and a redeploy
// to change an address would be absurd. Empty means do not notify, which is
// the default and a working configuration.
{ key: 'intake_notify_email', name: 'intakeNotifyEmail', type: 'text', fallback: '' }
] as const; ] as const;
type Definition = (typeof DEFINITIONS)[number]; type Definition = (typeof DEFINITIONS)[number];
+2
View File
@@ -11,6 +11,7 @@ import adminCategoriesRouter from './routes/adminCategories';
import adminTagsRouter from './routes/adminTags'; import adminTagsRouter from './routes/adminTags';
import adminUploadLinksRouter from './routes/adminUploadLinks'; import adminUploadLinksRouter from './routes/adminUploadLinks';
import adminItemDraftsRouter from './routes/adminItemDrafts'; import adminItemDraftsRouter from './routes/adminItemDrafts';
import intakeActionsRouter from './routes/intakeActions';
import intakeRouter from './routes/intake'; import intakeRouter from './routes/intake';
import adminVersionRouter from './routes/adminVersion'; import adminVersionRouter from './routes/adminVersion';
import filtersRouter from './routes/filters'; import filtersRouter from './routes/filters';
@@ -68,6 +69,7 @@ app.use('/api/cart', cartRouter);
// Public and unauthenticated by design (#222). No requireAdminGate: the token // Public and unauthenticated by design (#222). No requireAdminGate: the token
// in the path is the whole access control, and every refusal is a 404. // in the path is the whole access control, and every refusal is a 404.
app.use('/api/intake', intakeRouter); app.use('/api/intake', intakeRouter);
app.use('/api/intake-actions', intakeActionsRouter);
app.use('/api/checkout/cart', cartCheckoutRouter); app.use('/api/checkout/cart', cartCheckoutRouter);
// requireAdminGate is attached to each admin router rather than to a path // 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 // prefix. Attached to the router, an admin router added later at some other
+40 -1
View File
@@ -17,7 +17,8 @@ export type TemplateKey =
| 'favoriteSold' | 'favoriteSold'
| 'favoriteWithdrawn' | 'favoriteWithdrawn'
| 'cartReminder' | 'cartReminder'
| 'emailChanged'; | 'emailChanged'
| 'intakeDraft';
export interface TemplateDefinition { export interface TemplateDefinition {
/** Shown in the admin so a card is identifiable without reading its body. */ /** Shown in the admin so a card is identifiable without reading its body. */
@@ -122,6 +123,36 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
'{{itemList}}\n\n' + '{{itemList}}\n\n' +
'Items are held for {{holdDuration}} from when they were added.\n\n' + 'Items are held for {{holdDuration}} from when they were added.\n\n' +
'[View your cart]({{cartUrl}}) before your reservation expires.' '[View your cart]({{cartUrl}}) before your reservation expires.'
},
intakeDraft: {
label: 'Item submitted for review',
// Only the review link. The signed shortcuts are absent whenever
// INTAKE_ACTION_SECRET is unset, and requiring them would make an
// unconfigured environment unable to send this at all.
required: ['reviewUrl'],
available: [
'itemName',
'draftName',
'draftDescription',
'price',
'submitterNote',
'linkLabel',
'reviewUrl',
'regenerateUrl',
'discardUrl'
],
defaultSubject: 'An item was submitted: {{draftName}}',
defaultBody:
'Someone sent in an item through {{linkLabel}}.\n\n' +
'**{{draftName}}**\n\n' +
'{{draftDescription}}\n\n' +
'Suggested price: {{price}}\n\n' +
"The sender's note: {{submitterNote}}\n\n" +
'[Review and publish it]({{reviewUrl}})\n\n' +
'Nothing is listed until you publish it from that screen, and the price ' +
'above is a suggestion rather than a decision.\n\n' +
'[Ask for another draft]({{regenerateUrl}}) - [Discard it]({{discardUrl}})'
} }
}; };
@@ -225,6 +256,14 @@ export const SAMPLE_VALUES: Record<string, string> = {
// Fallbacks only. The admin preview overrides both from the live settings, // Fallbacks only. The admin preview overrides both from the live settings,
// so the pane shows the duration that would actually be sent rather than a // so the pane shows the duration that would actually be sent rather than a
// plausible-looking number that disagrees with it. // plausible-looking number that disagrees with it.
draftName: 'Blue stoneware vase',
draftDescription: 'A hand-thrown vase with a chipped base.',
price: '$80.00',
submitterNote: 'Found in a loft clearance.',
linkLabel: 'Autumn drop-off',
reviewUrl: 'https://example.com/admin',
regenerateUrl: 'https://example.com/api/intake-actions/1/regenerate?expires=0&sig=sample',
discardUrl: 'https://example.com/api/intake-actions/1/discard?expires=0&sig=sample',
expiresIn: 'one hour', expiresIn: 'one hour',
holdDuration: '24 hours' holdDuration: '24 hours'
}; };
+14 -1
View File
@@ -170,6 +170,19 @@ function checkAdminGate(env: NodeJS.ProcessEnv): string[] {
// without its description written. Silence would be the wrong answer too: an // without its description written. Silence would be the wrong answer too: an
// operator who believes drafting is on and finds every item undrafted has // operator who believes drafting is on and finds every item undrafted has
// nothing to tell them why. // nothing to tell them why.
// Optional, like the drafting key below. Absent, the notification still sends
// with its review link and simply carries no shortcuts — being told an item
// arrived matters far more than being able to discard it in one click.
function checkIntakeActionSecret(env: NodeJS.ProcessEnv): string[] {
if (isPresent(env, 'INTAKE_ACTION_SECRET')) {
return [];
}
return [
'INTAKE_ACTION_SECRET is not set — intake notifications will link to the review queue ' +
'but carry no regenerate or discard shortcuts.'
];
}
function checkDraftingKey(env: NodeJS.ProcessEnv): string[] { function checkDraftingKey(env: NodeJS.ProcessEnv): string[] {
if (isPresent(env, 'ANTHROPIC_API_KEY')) { if (isPresent(env, 'ANTHROPIC_API_KEY')) {
return []; return [];
@@ -223,6 +236,6 @@ export function validateEnv(env: NodeJS.ProcessEnv): EnvValidation {
...mail.errors, ...mail.errors,
...uploads.errors ...uploads.errors
], ],
warnings: [...mail.warnings, ...checkAdminGate(env), ...uploads.warnings, ...checkDraftingKey(env)] warnings: [...mail.warnings, ...checkAdminGate(env), ...uploads.warnings, ...checkDraftingKey(env), ...checkIntakeActionSecret(env)]
}; };
} }
+75
View File
@@ -0,0 +1,75 @@
import crypto from 'crypto';
import { trimTrailingSlashes } from '../utils';
/**
* Links in the notification email that act without a login.
*
* Only two actions are signable, and neither can publish. The worst case of a
* leaked link is a wasted API call or a hide the review queue can undo — which
* is what makes it acceptable to put them in an inbox at all.
*
* Signed over the item, the action and the expiry together. Signing any subset
* would let a link be replayed against a different item or upgraded to a
* different action, and leaving the expiry out of the payload would let anyone
* holding an expired link extend it by editing the timestamp in the URL.
*/
export type IntakeAction = 'regenerate' | 'discard';
/** Thirty days. Long enough to survive a holiday, short enough to lapse. */
export const ACTION_TTL_MS = 30 * 24 * 60 * 60 * 1000;
function secret(): string | null {
const value = process.env.INTAKE_ACTION_SECRET;
return value && value.trim() !== '' ? value : null;
}
export function signAction(itemId: number, action: IntakeAction, expiresAt: number): string {
const key = secret();
if (!key) throw new Error('INTAKE_ACTION_SECRET is not set');
return crypto
.createHmac('sha256', key)
.update(`${itemId}:${action}:${expiresAt}`)
.digest('base64url');
}
/**
* Compared through a second digest rather than directly, because
* timingSafeEqual throws when the two buffers differ in length — and a
* malformed signature from a truncated link is an ordinary thing to receive
* rather than an exception. Same idiom as middleware/adminGate.ts.
*/
function digest(value: string): Buffer {
return crypto.createHash('sha256').update(value).digest();
}
export function verifyAction(
itemId: number,
action: IntakeAction,
expiresAt: number,
signature: string,
now: number = Date.now()
): boolean {
if (!secret()) return false;
if (!Number.isFinite(expiresAt) || now > expiresAt) return false;
const expected = signAction(itemId, action, expiresAt);
return crypto.timingSafeEqual(digest(expected), digest(signature));
}
/**
* The absolute link, or null when one cannot be made.
*
* Null rather than a throw or a relative path. An unconfigured environment
* still sends the notification with its review link — being told an item
* arrived matters far more than the shortcuts — and a link that could not be
* verified must never be offered in the first place.
*/
export function actionUrl(itemId: number, action: IntakeAction): string | null {
const base = process.env.PUBLIC_URL;
if (!secret() || !base || base.trim() === '') return null;
const expiresAt = Date.now() + ACTION_TTL_MS;
const sig = signAction(itemId, action, expiresAt);
const origin = trimTrailingSlashes(base);
return `${origin}/api/intake-actions/${itemId}/${action}?expires=${expiresAt}&sig=${sig}`;
}
+9
View File
@@ -6,6 +6,7 @@ import { typeForExtension } from '../uploadTypes';
import { getAnthropicClient } from './anthropicClient'; import { getAnthropicClient } from './anthropicClient';
import { draftListing } from './draftListing'; import { draftListing } from './draftListing';
import { applyDraft } from './applyDraft'; import { applyDraft } from './applyDraft';
import { notifyDraftReady } from './notifyDraft';
/** /**
* Turns queued submissions into drafts. * Turns queued submissions into drafts.
@@ -144,6 +145,14 @@ export async function draftQueued(limit = DEFAULT_BATCH): Promise<SweepResult> {
} }
await draftOne(client, row.item_id, row.submitter_note, photos); await draftOne(client, row.item_id, row.submitter_note, photos);
drafted++; drafted++;
// Fire and forget, and deliberately after the draft is committed. A mail
// failure must never mark a draft that was written correctly as failed —
// the queue is what the admin actually works from, and the email is a
// convenience on top of it.
void notifyDraftReady(row.item_id).catch((err) =>
console.error(`[drafting] notifying for item ${row.item_id}:`, err)
);
} catch (err) { } catch (err) {
const message = err instanceof Error ? err.message : String(err); const message = err instanceof Error ? err.message : String(err);
console.error(`[drafting] item ${row.item_id}: ${message}`); console.error(`[drafting] item ${row.item_id}: ${message}`);
+70
View File
@@ -0,0 +1,70 @@
import { pool } from '../db';
import { sendMail } from '../mailer';
import { renderTemplate } from '../emailTemplates';
import { loadStoredTemplate } from '../routes/adminEmailTemplates';
import { getSettings } from '../adminSettings';
import { trimTrailingSlashes } from '../utils';
import { actionUrl } from './actionLinks';
interface NotifyRow {
item_name: string;
price_cents: number;
ai_name: string | null;
ai_description: string | null;
submitter_note: string | null;
link_label: string | null;
}
/**
* Tells the admin an item arrived and has been drafted.
*
* Everything here is best-effort by design. The review queue is the source of
* truth: a ready draft is visible and actionable whether or not this ever sent,
* so a missing recipient, an SMTP outage, or a template that will not render
* must all end in a log line rather than an exception reaching the worker and
* marking a perfectly good draft as failed.
*/
export async function notifyDraftReady(itemId: number): Promise<void> {
const { intakeNotifyEmail } = await getSettings();
const to = intakeNotifyEmail?.trim();
if (!to) {
// Not an error, and deliberately not a warning either. Nobody has said
// where to send it, and the draft is waiting in the queue regardless.
return;
}
const { rows } = await pool.query<NotifyRow>(
`SELECT i.name AS item_name, i.price_cents,
d.ai_name, d.ai_description, d.submitter_note,
l.label AS link_label
FROM item_drafts d
JOIN items i ON i.id = d.item_id
LEFT JOIN upload_links l ON l.id = d.upload_link_id
WHERE d.item_id = $1`,
[itemId]
);
const row = rows[0];
if (!row) return;
const base = trimTrailingSlashes(process.env.PUBLIC_URL ?? '');
const template = renderTemplate('intakeDraft', await loadStoredTemplate('intakeDraft'), {
itemName: row.item_name,
draftName: row.ai_name ?? row.item_name,
// Said plainly rather than left blank. An empty description in a
// notification reads as a bug; "no description was drafted" reads as the
// fact that it is, and tells the admin what to expect on the screen.
draftDescription: row.ai_description ?? 'No description was drafted for this item.',
price: `$${(row.price_cents / 100).toFixed(2)}`,
submitterNote: row.submitter_note ?? 'The sender left no note.',
linkLabel: row.link_label ?? 'an upload link',
reviewUrl: `${base}/admin`,
// Empty rather than a broken link when there is no secret to sign with.
// The body renders without them; a link that could not be verified would
// be worse than none.
regenerateUrl: actionUrl(itemId, 'regenerate') ?? '',
discardUrl: actionUrl(itemId, 'discard') ?? ''
});
await sendMail(to, template.subject, template.html);
}
+129
View File
@@ -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, passwordResetHours: 1,
greetingFormat: 'Hi {{firstName}},', greetingFormat: 'Hi {{firstName}},',
greetingFallback: 'Hi,', 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', 'emailChanged',
'favoriteSold', 'favoriteSold',
'favoriteWithdrawn', 'favoriteWithdrawn',
'intakeDraft',
'passwordReset', 'passwordReset',
'verification' '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();
});
});
+85
View File
@@ -0,0 +1,85 @@
import { signAction, verifyAction, actionUrl, ACTION_TTL_MS } from '../../src/intake/actionLinks';
const SECRET = 'test-intake-secret';
const NOW = 1_800_000_000_000;
const EXPIRY = NOW + ACTION_TTL_MS;
beforeEach(() => {
process.env.INTAKE_ACTION_SECRET = SECRET;
process.env.PUBLIC_URL = 'https://shop.example.com';
});
describe('signAction / verifyAction', () => {
it('accepts a signature it produced', () => {
const sig = signAction(7, 'discard', EXPIRY);
expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(true);
});
// Each of these is a different link. A signature that survives any of these
// swaps is a signature that authorises more than it names.
it('refuses a signature reused for another item', () => {
const sig = signAction(7, 'discard', EXPIRY);
expect(verifyAction(8, 'discard', EXPIRY, sig, NOW)).toBe(false);
});
it('refuses a signature reused for another action', () => {
const sig = signAction(7, 'discard', EXPIRY);
expect(verifyAction(7, 'regenerate', EXPIRY, sig, NOW)).toBe(false);
});
// Otherwise the expiry is decoration: anyone holding an expired link could
// extend it themselves by editing the timestamp.
it('refuses a signature whose expiry was altered', () => {
const sig = signAction(7, 'discard', EXPIRY);
expect(verifyAction(7, 'discard', EXPIRY + 1000, sig, NOW)).toBe(false);
});
it('refuses an expired link even with a valid signature', () => {
const sig = signAction(7, 'discard', EXPIRY);
expect(verifyAction(7, 'discard', EXPIRY, sig, EXPIRY + 1)).toBe(false);
});
it('refuses a malformed signature without throwing', () => {
expect(verifyAction(7, 'discard', EXPIRY, 'not-a-signature', NOW)).toBe(false);
expect(verifyAction(7, 'discard', EXPIRY, '', NOW)).toBe(false);
});
// This is what makes rotating the secret revoke every outstanding link.
it('refuses a signature made with a different secret', () => {
const sig = signAction(7, 'discard', EXPIRY);
process.env.INTAKE_ACTION_SECRET = 'a-different-secret';
expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(false);
});
it('refuses everything when there is no secret at all', () => {
const sig = signAction(7, 'discard', EXPIRY);
delete process.env.INTAKE_ACTION_SECRET;
expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(false);
});
});
describe('actionUrl', () => {
it('builds an absolute url carrying the expiry and signature', () => {
const url = actionUrl(7, 'discard');
expect(url).toContain('https://shop.example.com/api/intake-actions/7/discard');
expect(url).toMatch(/expires=\d+/);
expect(url).toMatch(/sig=[A-Za-z0-9_-]+/);
});
// Absent secret is a working configuration: the notification still sends with
// its review link. A link that cannot be verified must never be offered.
it('returns null when there is no secret', () => {
delete process.env.INTAKE_ACTION_SECRET;
expect(actionUrl(7, 'discard')).toBeNull();
});
it('returns null when there is no public url to build against', () => {
delete process.env.PUBLIC_URL;
expect(actionUrl(7, 'discard')).toBeNull();
});
it('does not double the slash when PUBLIC_URL has a trailing one', () => {
process.env.PUBLIC_URL = 'https://shop.example.com/';
expect(actionUrl(7, 'discard')).toContain('https://shop.example.com/api/intake-actions/');
});
});
+34
View File
@@ -274,3 +274,37 @@ describe('every template can address the customer', () => {
); );
}); });
}); });
describe('the intake notification template', () => {
// Without the review link the email is a notification you cannot act on.
it('requires the review url', () => {
expect(missingPlaceholders('intakeDraft', 'An item arrived.')).toContain('reviewUrl');
});
it('accepts a body carrying the review url', () => {
expect(missingPlaceholders('intakeDraft', 'Review it: {{reviewUrl}}')).toEqual([]);
});
// The signed links are deliberately optional. They are absent whenever
// INTAKE_ACTION_SECRET is unset, and a template demanding them would leave an
// unconfigured environment unable to send this at all.
it('does not require the signed action links', () => {
const missing = missingPlaceholders('intakeDraft', '{{reviewUrl}}');
expect(missing).not.toContain('discardUrl');
expect(missing).not.toContain('regenerateUrl');
});
it('offers the drafted copy to the template author', () => {
for (const name of ['itemName', 'draftName', 'draftDescription', 'price', 'submitterNote']) {
expect(TEMPLATES.intakeDraft.available).toContain(name);
}
});
// The email must never be able to publish. That is what bounds the risk taken
// by pricing items on arrival, and it is a property of the copy as much as of
// the routes — a publish link here would be one nobody reviewed.
it('offers no way to publish', () => {
expect(TEMPLATES.intakeDraft.available).not.toContain('publishUrl');
expect(TEMPLATES.intakeDraft.defaultBody).not.toMatch(/publishUrl/);
});
});
+15
View File
@@ -236,4 +236,19 @@ describe('UPLOADS_BASE_URL', () => {
expect(warnings.join(' ')).not.toMatch(/ANTHROPIC_API_KEY/); expect(warnings.join(' ')).not.toMatch(/ANTHROPIC_API_KEY/);
}); });
}); });
// #224. MINIMAL has no INTAKE_ACTION_SECRET, so it is already the absent case.
describe('the intake action secret', () => {
it('is not required', () => {
expect(validateEnv(MINIMAL).errors).toEqual([]);
});
it('warns when it is absent', () => {
expect(validateEnv(MINIMAL).warnings.join(' ')).toMatch(/INTAKE_ACTION_SECRET/);
});
it('says nothing when it is set', () => {
const { warnings } = validateEnv(withEnv({ INTAKE_ACTION_SECRET: 'a-secret' }));
expect(warnings.join(' ')).not.toMatch(/INTAKE_ACTION_SECRET/);
});
});
}); });
+9
View File
@@ -92,6 +92,9 @@
# failing, so an empty value is a working configuration. # failing, so an empty value is a working configuration.
# ANTHROPIC_API_KEY Optional. Drafts a listing from a submitted photo # ANTHROPIC_API_KEY Optional. Drafts a listing from a submitted photo
# (#223). Unset means submissions still arrive and wait # (#223). Unset means submissions still arrive and wait
# INTAKE_ACTION_SECRET Optional. Signs the regenerate and discard links in the
# intake notification email (#224). Absent, the email
# still sends and carries no shortcuts.
# undrafted, which is a working configuration for the # undrafted, which is a working configuration for the
# same reason USPS is. The one credential here that # same reason USPS is. The one credential here that
# spends money per call, and on a path anybody holding # spends money per call, and on a path anybody holding
@@ -225,6 +228,12 @@ services:
# one, and #227 is the submission ceiling that bounds the volume rather # one, and #227 is the submission ceiling that bounds the volume rather
# than the bill. # than the bill.
- ANTHROPIC_API_KEY=${ANTHROPIC_API_KEY:-} - ANTHROPIC_API_KEY=${ANTHROPIC_API_KEY:-}
# Signs the regenerate and discard links in the intake notification email
# (#224). Optional: absent, the notification still sends and links to the
# review queue without shortcuts. Rotating it revokes every outstanding
# link, which is how a leaked one is dealt with.
- INTAKE_ACTION_SECRET=${INTAKE_ACTION_SECRET:-}
volumes: volumes:
# Production's own uploads directory. QA writes to # Production's own uploads directory. QA writes to
# /volume1/configs/redefined-designs-qa/uploads; sharing this one would # /volume1/configs/redefined-designs-qa/uploads; sharing this one would
+12
View File
@@ -54,6 +54,10 @@
# credential here that spends money per call, and it is # credential here that spends money per call, and it is
# reachable by anyone holding an upload link. Leave it unset # reachable by anyone holding an upload link. Leave it unset
# and submissions still arrive, undrafted. # and submissions still arrive, undrafted.
# QA_INTAKE_ACTION_SECRET — optional. Signs the regenerate and discard links
# in the notification email (#224). Absent, the email still
# sends and simply carries no shortcuts. Its own value, not
# production's: a link signed with it acts without a login.
services: services:
redefined-designs-qa: redefined-designs-qa:
@@ -144,6 +148,14 @@ services:
# consignment to an expired key would be far worse than an item arriving # consignment to an expired key would be far worse than an item arriving
# without its description written. # without its description written.
- ANTHROPIC_API_KEY=${QA_ANTHROPIC_API_KEY} - ANTHROPIC_API_KEY=${QA_ANTHROPIC_API_KEY}
# Signs the regenerate and discard links in the intake notification email
# (#224). Optional: absent, the notification still sends and simply links
# to the review queue without shortcuts. Anyone holding a link can act on
# it without signing in, so this must not be shared with production —
# rotating it revokes every outstanding link, which is the intended way to
# deal with a leak.
- INTAKE_ACTION_SECRET=${QA_INTAKE_ACTION_SECRET:-}
volumes: volumes:
# Separate uploads directory. Sharing production's would let a QA run # Separate uploads directory. Sharing production's would let a QA run
# write into, and a QA teardown delete, real product images. # write into, and a QA teardown delete, real product images.
+1 -1
View File
@@ -60,7 +60,7 @@ Everything here is lost when the stack is deleted, and the rollback in step 8 is
**The stack name**, exactly as Portainer shows it. If it is not `redefined-designs`, note that — the new stack must be created with that name, because the stack name becomes the compose project name and reusing QA's would make Compose reconcile the two against each other. **The stack name**, exactly as Portainer shows it. If it is not `redefined-designs`, note that — the new stack must be created with that name, because the stack name becomes the compose project name and reusing QA's would make Compose reconcile the two against each other.
**Every stack environment variable, name and value.** They belong to the stack, and deleting it discards them. This is the step whose omission is felt hardest. The compose file interpolates fourteen names — `DEMO_MODE`, `DB_PASSWORD`, `SMTP_USER`, `SMTP_PASSWORD`, `SMTP_FROM`, `ADMIN_GATE_SECRET`, `PAYPAL_CLIENT_ID`, `PAYPAL_CLIENT_SECRET`, `PAYPAL_WEBHOOK_ID`, `USPS_CLIENT_ID`, `USPS_CLIENT_SECRET`, `UPLOADS_BASE_URL`, `BACKUP_PASSPHRASE` and `ANTHROPIC_API_KEY` — and an unset one substitutes to an empty string rather than failing. None of it is recoverable from anything in this repository. Take everything the stack holds rather than working from this list; it is here to say how much there is, and it is checked against the file rather than from memory. **Every stack environment variable, name and value.** They belong to the stack, and deleting it discards them. This is the step whose omission is felt hardest. The compose file interpolates fifteen names — `DEMO_MODE`, `DB_PASSWORD`, `SMTP_USER`, `SMTP_PASSWORD`, `SMTP_FROM`, `ADMIN_GATE_SECRET`, `PAYPAL_CLIENT_ID`, `PAYPAL_CLIENT_SECRET`, `PAYPAL_WEBHOOK_ID`, `USPS_CLIENT_ID`, `USPS_CLIENT_SECRET`, `UPLOADS_BASE_URL`, `BACKUP_PASSPHRASE`, `ANTHROPIC_API_KEY` and `INTAKE_ACTION_SECRET` — and an unset one substitutes to an empty string rather than failing. None of it is recoverable from anything in this repository. Take everything the stack holds rather than working from this list; it is here to say how much there is, and it is checked against the file rather than from memory.
`USPS_CLIENT_ID` and `USPS_CLIENT_SECRET` deserve naming because losing them is the one failure here that is completely silent. Address validation is skipped when they are empty rather than failing, so checkout keeps working and quietly stops validating addresses. Nothing in step 7 catches it, and there is no crash loop to notice. `USPS_CLIENT_ID` and `USPS_CLIENT_SECRET` deserve naming because losing them is the one failure here that is completely silent. Address validation is skipped when they are empty rather than failing, so checkout keeps working and quietly stops validating addresses. Nothing in step 7 catches it, and there is no crash loop to notice.