feat(labeler): publisher-notification token endpoints (W10.4 slice C)#2071
Conversation
Add the three public, token-authenticated landing endpoints an email recipient clicks: /notifications/confirm, /notifications/unsubscribe, and /notifications/not-me. Mounted outside /admin/* so neither the Cloudflare Access edge policy nor the operator guard applies -- the capability in the link is the sole authority. GET renders a confirmation page with a POST form; only POST mutates, so an email scanner's automated GET never confirms or suppresses. Responses are neutral and uniform regardless of token validity or on-file status, with the real outcome logged server-side only (never the token). Confirm always runs confirmContact's constant-time compare and never confirms a suppressed contact. Unsubscribe/not-me record an idempotent, hash-keyed suppression and decline any pending confirmation. Adds hashConfirmToken to notification-contacts (SHA-256 of the raw token, the stored confirm_token_hash form) as the shared verify/send contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address the adversary pass on W10.4 slice C. Unsubscribe/not-me now write a suppression only when a contact row already exists for the hash. A recipient who legitimately received mail always has one (ensureContact created it, and contacts are never swept), so gating costs legit users nothing while an attacker who fabricates a well-formed but unseen recipient hash writes no orphan suppression row. The response stays byte-identical (the caller only ever sees the neutral done page). Adds a read-only contactExists helper. Document the token-entropy cross-slice dependency at hashConfirmToken (and the confirm path): the pepper-less SHA-256 lookup with no in-worker rate limit is safe only if the W10.5 send path draws the token from a CSPRNG at >=128 bits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Scope checkThis PR changes 568 lines across 4 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | c8ed862 | Jul 16 2026, 11:33 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | c8ed862 | Jul 16 2026, 11:34 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | c8ed862 | Jul 16 2026, 11:34 AM |
There was a problem hiding this comment.
This PR adds the public notification landing endpoints described in W10.4 slice C and the implementation is generally careful: GET renders the form, only POST mutates, responses are uniform, the confirm path stays timing-uniform by always running the constant-time compare, and suppressions are gated on an existing contact row to avoid orphan writes. Tests cover the safety properties claimed in the PR description.
I did not run the test suite or lint (no shell/tooling), so I’m treating the author’s test/lint claims as unverified.
The one real gap I found is security: the HTML landing pages are state-changing forms served with no clickjacking protection. Because the deliverable here is an HTML form, an attacker can frame the page and trick a recipient into clicking “Confirm” / “Unsubscribe” / “Do not contact me”. Since the capability is in the link, the attacker only needs the user to have opened the link while the attacker’s site is framing it. Adding X-Frame-Options: DENY plus a Content-Security-Policy with frame-ancestors 'none' closes this cleanly.
No SQL-injection, localization, API-envelope, or changeset issues: all DB access is parameterized, these pages are intentionally outside the admin UI/localization scope, and @emdash-cms/labeler is private: true.
| headers: { | ||
| "content-type": "text/html; charset=utf-8", | ||
| "cache-control": "no-store", | ||
| "referrer-policy": "no-referrer", | ||
| "x-content-type-options": "nosniff", | ||
| }, |
There was a problem hiding this comment.
[needs fixing] The HTML landing pages carry a state-changing form but do not protect against clickjacking. Because the confirm/unsubscribe/not-me capability travels in the email link, an attacker who gets the user to open the link while framing it in a malicious page can trick the user into submitting the form.
Add framing protections to htmlResponse so these pages cannot be embedded:
| headers: { | |
| "content-type": "text/html; charset=utf-8", | |
| "cache-control": "no-store", | |
| "referrer-policy": "no-referrer", | |
| "x-content-type-options": "nosniff", | |
| }, | |
| headers: { | |
| "content-type": "text/html; charset=utf-8", | |
| "cache-control": "no-store", | |
| "referrer-policy": "no-referrer", | |
| "x-content-type-options": "nosniff", | |
| "x-frame-options": "DENY", | |
| "content-security-policy": "frame-ancestors 'none'", | |
| }, |
…g defense-in-depth)
|
There was a problem hiding this comment.
The PR addresses the one blocking issue from the previous review: the notification landing pages now set X-Frame-Options: DENY and Content-Security-Policy: frame-ancestors 'none', so the state-changing forms are no longer frameable. The rest of the implementation remains sound: GET renders a form and only POST mutates, responses stay uniform across valid/invalid/unknown/malformed input, the confirm path continues to run a constant-time compare for suppressed or malformed cases, and suppression writes are still gated on an existing contact row. Tests cover the new framing headers plus the existing safety properties.
I did not run the test suite or linter (no shell/tooling available), so the author’s test/lint claims are unverified here.
One defense-in-depth suggestion: the CSP currently only sets frame-ancestors 'none'. Because these pages have no external scripts, styles, fonts, or frames, a stricter policy (default-src 'none'; script-src 'none'; object-src 'none'; frame-ancestors 'none'; form-action 'self') would close off an entire class of XSS escalation paths if the HTML escaping were ever bypassed, at no functional cost. Not a blocker.
| "cache-control": "no-store", | ||
| "referrer-policy": "no-referrer", | ||
| "x-content-type-options": "nosniff", | ||
| "x-frame-options": "DENY", |
There was a problem hiding this comment.
[suggestion] The clickjacking fix from the previous review is in place and correct. Consider hardening the CSP further while you're touching this header: these landing pages carry no external scripts, styles, fonts, or frames, so a restrictive policy costs nothing.
| "x-frame-options": "DENY", | |
| "x-frame-options": "DENY", | |
| "content-security-policy": "default-src 'none'; script-src 'none'; object-src 'none'; frame-ancestors 'none'; form-action 'self'", |
frame-ancestors already prevents framing; adding default-src 'none', script-src, object-src, and form-action 'self' removes an entire XSS blast radius if the HTML escaping were ever bypassed, while keeping the self-submitting form functional.
There was a problem hiding this comment.
Re-review of #2071. The previous blocking issue — that the public notification landing pages were frameable — is resolved. The new handleNotificationRequest handler now sets X-Frame-Options: DENY and a strict Content-Security-Policy that includes frame-ancestors 'none', form-action 'self', and default-src 'none' on every response. The tests added for this slice also assert those headers.
Beyond framing, the implementation remains sound for the stated threat model:
- GET renders a form and only POST mutates.
- POST responses are byte-identical for valid/wrong/unknown/malformed input.
- The confirm path still routes malformed hashes and suppressed contacts through the same constant-time compare as valid requests, so timing does not leak token validity.
- Confirm only flips
unconfirmedrows; suppression gate exists before writing a suppression record.
I checked the code against AGENTS.md conventions: SQL uses D1 parameter binding; these are public HTML landing pages outside the admin UI, so the Lingui/Tailwind rules for the admin console do not apply; no content-table queries are introduced; @emdash-cms/labeler is private, so a changeset is not required.
No remaining blockers. The slice looks ready to merge.
e95281e
into
feat/plugin-registry-labelling-service
What does this PR do?
Adds the three public, token-authorized landing endpoints that back publisher-notification email links (plan W10.4 slice C):
/notifications/confirm,/notifications/unsubscribe,/notifications/not-me. They drive the double-opt-in and universal-suppression state transitions on thenotification_contacts/notification_suppressionstables from slice B (#2068). No email sending (W10.5) and no contact resolution (slice A) here.Mounted outside
/admin/*, so neither the Cloudflare Access edge policy nor the operator guard applies — the capability in the link is the sole authority. The recipient hash (HMAC-SHA256 under the secret pepper) is unforgeable without the pepper; confirm additionally carries a single-use token whose SHA-256 the send path stores.Security properties (an adversarial review pass verified all of these):
unconfirmed → confirmedonly, never a suppressed contact; unsubscribe/not-me are idempotent hash-keyed suppressions.Cross-slice dependency recorded (for W10.5): the confirm-token lookup is a pepper-less
SHA-256(token)with no in-worker rate limit, so its safety rests entirely on token entropy — the W10.5 send path MUST generate the token from a CSPRNG with ≥128 bits. Documented athashConfirmTokenand in the plan's W10.5 section.Part of the plugin-registry labelling service (RFC #694, umbrella #1909). Targets the integration branch.
Type of change
Covered by the approved RFC #694 umbrella.
Checklist
pnpm typecheckpassespnpm lintpasses (zero diagnostics in changed files; pre-existing baseline unchanged)pnpm testpasses (notification-endpoints 15/15; full labeler suite green)pnpm formathas been run (touched paths)@emdash-cms/labeleris unpublished (private: true)AI-generated code disclosure
Screenshots / test output
vitest run test/notification-endpoints.test.ts→ 15/15: Access-bypass, GET-renders-form/POST-mutates, uniform-neutral-response equality, confirm state guards (unconfirmed-only, suppressed-never-confirmed, null-token), idempotent suppression, the gated no-orphan-row assertion (random hash writes nothing yet returns the identical done page), XSS-escaping, and 405 for other methods.Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
feat/labeler-notification-endpoints. Updated automatically when the playground redeploys.