Files

T

bermudalambandClaude Opus 5 9e1e650786 docs(intake): design emailing an upload link to its recipient (#260 )

The issue asked for the security question to be settled before any code, and it is: the mail carries the working link.

The issue framed that as a loosening comparable to a password reset, and on inspection that framing overstates it. A reset token takes over an account. An upload token grants exactly one capability — submit photos into a queue where a person must approve them before anything reaches the storefront. It reads nothing, it is revocable, max_submissions caps it, and #227 caps the whole intake surface regardless of any single link. The worst outcome of a leaked upload link is junk in the review queue, which is bounded and reversible. That is a reasonable thing to put in an inbox, and this project already makes the much larger bet with reset links.

The address is required for new links while the column stays nullable, which is not a contradiction: links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered, and the requirement belongs in the route where new links are actually made. It lives on the link rather than on a contributor entity, because a link already carries a label naming who it is for and nothing yet suggests the same people submit repeatedly.

A failed send does not roll the link back. The token is shown exactly once, so a rollback would leave the admin retrying and holding a different link, discarding work that succeeded. The link is created, the send is attempted, and the response says which happened — which matters concretely because QA's MAIL_ALLOWLIST silently skips any address outside it and returns as though it sent. Without an explicit outcome, testing this in QA against a contributor's real address looks exactly like success, which is the afternoon the issue warned would otherwise be wasted.

Writing it turned up one thing the design had assumed and the code does not support. sendMail returns Promise<void> and returns early both when SMTP is unconfigured and when the recipient is not allowlisted, so a caller cannot tell either from success. It gains a MailOutcome return value instead. No existing caller changes — there are seven and every one ignores the result — and the alternative would have duplicated isAllowedRecipient and the SMTP check at a second site, which is the drift the guard-in-one-place comment in mailer.ts exists to prevent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

2026-09-03 17:28:45 -05:00

7.2 KiB

Raw Blame History

Emailing an upload link to the person who will use it

Issue: #260. Builds on #222 (upload links) and #224 (the intake notification, which is the closest existing mail).

Today an admin creates an upload link, the token is shown once in the console, and getting it to a contributor is a copy-and-paste into whatever the admin happens to use. This makes the address part of creating the link, and sends the link to it.

Decisions, and what each one rests on

The mail carries the working link. The issue framed this as a loosening comparable to a password reset, and that framing overstates it. A reset token takes over an account; an upload token grants exactly one capability — submit photos into a queue where a person must approve them before anything is published. It reads nothing, it is revocable, max_submissions caps it, and #227 caps the whole intake surface regardless. The worst outcome of a leaked upload link is junk in the review queue, which is bounded and reversible. That is a reasonable thing to put in an inbox; a reset link is a much larger bet and this project already makes that one.

The address is required for new links, and the column is nullable. Those are not in tension: links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered at null, and the requirement lives in the route where new links are made. Making the column NOT NULL would mean either inventing data or a backfill that lies.

The address lives on the link, not on a contributor. A link already carries a label naming who it is for. A contributor entity would be the better model only if the same people submit repeatedly through different links, which nothing yet suggests, and it is materially more work.

A failed send does not lose the link. The token is displayed exactly once, so rolling the creation back on a send failure would leave the admin retrying and getting a different link — harmless but confusing, and it throws away work that succeeded. Instead the link is created, the send is attempted, and the response says which happened.

That last point is not defensive programming for its own sake. QA sets MAIL_ALLOWLIST and silently skips any address outside it, logging [mail-blocked] and returning as though it sent. Without an explicit outcome in the response, testing this in QA against a contributor's real address looks exactly like success. The issue itself warns this would otherwise waste an afternoon.

Architecture

Admin | Upload Links
  label + email + cap
        │
        ▼
POST /api/admin/upload-links
  ├─ isValidEmail(email)            400 naming the field
  ├─ INSERT upload_links            contact_email alongside label and cap
  ├─ renderTemplate('uploadLink')   submitUrl required, as verification requires verifyUrl
  └─ sendMail(...) → outcome        'sent' | 'skipped-unconfigured' | 'skipped-blocked'
        │
        ▼
201 { ...link, token, url, mail: { sent: boolean, outcome: MailOutcome } }

`sendMail` gains a return value

sendMail currently returns Promise<void> and returns early in two cases — SMTP unconfigured, and the recipient not on MAIL_ALLOWLIST — both indistinguishable from success at the call site. It will return a small result instead:

export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';

No existing caller changes. There are seven, and every one ignores the return value; ignoring a returned value is legal, so this is additive rather than a breaking change. The alternative — re-deriving "would this address be blocked?" in the route — would duplicate isAllowedRecipient and the SMTP check at a second site, which is exactly the drift the guard-in-one-place comment in mailer.ts exists to prevent.

Data

upload_links.contact_email TEXT — nullable, no default. Null means a link made before this existed, or by a caller that predates the requirement. Nothing reads it except the admin list and the mail at creation.

The template

A new uploadLink key in TEMPLATES, editable in the admin like the others.

required: ['submitUrl'] — the same guard that stops a verification mail shipping without its link. A link email with no link is the one failure worth making impossible.
available: ['submitUrl', 'label', 'submissionsAllowed']
submissionsAllowed renders as a number for a capped link and as words for an uncapped one, so an unlimited link reads as a sentence rather than as a missing value. Pinned exactly, so it is not a coin-flip at implementation time: a capped link renders 25 items, or 1 item at a cap of one; an uncapped link renders as many items as you like. The default body uses it as "You can send {{submissionsAllowed}}."

The admin screen

Email becomes a required field in the create form, beside the label. The links table gains the address, so it is visible who a link was sent to. When the response reports the mail was not sent, the admin is told plainly and the link is still shown to copy.

Failure handling

What happens	Result
Address missing or malformed	400 naming the field. No link created.
SMTP not configured (local development)	Link created. `mail.sent` false, `mail.outcome` `'skipped-unconfigured'`.
Address not on `MAIL_ALLOWLIST` (QA)	Link created. `mail.sent` false, `mail.outcome` `'skipped-blocked'`.
SMTP rejects the message	Link created. `mail.sent` false. The admin copies the link by hand.
Everything works	Link created, `mail.sent` true, `mail.outcome` `'sent'`.

mail.outcome is always present and always one of the three values, including on success — a field that appears only on failure is one every consumer has to remember to check for.

A send failure never rolls back the link, and never turns into a 500. The admin always ends up holding a usable link and an honest statement of whether it was delivered.

Testing

Integration: the address is required; a malformed one is refused naming the field; a valid one is stored; the mail is attempted with the rendered template; a send failure still returns 201 with a usable token and mail.sent false. Existing links with a null address still list and still work.
Unit: the uploadLink template refuses a body with no submitUrl, through the existing missingPlaceholders guard.
E2E: the create form refuses to submit without an address, and a created link shows its address in the table.
Existing callers: the e2e fixtures and admin-upload-links.spec.ts create links with only a label and must be updated. That is the cost of the requirement, and the compiler and the suite find every one.

Out of scope

Re-sending a lost link. Only the digest is stored, so the original token cannot be recovered. "Re-send" would mean minting a new link and revoking the old one — a different feature with its own decisions about what the recipient is told.

Notifying the address on revoke. Not asked for, and it is a separate judgement about whether a contributor should learn their link was withdrawn.

A contributor entity. See the decisions above.

Any change to the seven existing sendMail call sites. They keep ignoring the outcome. Making each of them report delivery is a larger piece of work with no demand behind it.

7.2 KiB Raw Blame History