Commit graph

2 commits

Author SHA1 Message Date
Agent
9fa1f73316 fix: distinguish already-confirmed tokens from invalid ones; prevent duplicate active subscriptions
All checks were successful
CI / checks (pull_request) Successful in 36s
Problem
-------
1. Re-clicking an already-confirmed confirmation link (e.g. /confirm?token=…)
   returned  from confirmSubscription because the SQL WHERE
   clause required . The caller then threw an
   ActionError('invalid or expired') which rendered the 'Email not confirmed'
   error page — misleading for someone who had already confirmed.

2. A second PUT /subscription/email with the same email+frequency could
   silently bypass the upsert path when getSubscriptionByPubkey found the
   active row but updateSubscription returned it unchanged (email and
   frequency matched). While the unique index prevented a true duplicate
   INSERT, the code path was fragile and the regression test was missing.

Changes
-------
database.ts:
- confirmSubscription now returns { sub, alreadyConfirmed } | undefined.
  First it tries the existing UPDATE (unconfirmed tokens only). If that
  returns no rows, it looks up the key directly: if the row exists and is
  already confirmed, returns { sub, alreadyConfirmed: true }. If the row
  doesn't exist or is unsubscribed, returns undefined (invalid/expired).
- Exported new ConfirmResult type for callers.

actions.ts:
- confirmSubscriptionAction destructures the new return type.
- Only registers the cron job on fresh confirmation (not re-confirms).
- Returns the ConfirmResult so the route can distinguish the two cases.

server.ts:
- /confirm route checks result.alreadyConfirmed and renders
  confirm-already.html instead of confirm-success.html.

pages/confirm-already.html:
- New page with title 'Email already confirmed' and an info message
  explaining the address was already confirmed.

Tests:
- test/confirm-already-confirmed.test.ts — NEW (3 tests): first confirm
  succeeds with alreadyConfirmed=false; second confirm returns
  alreadyConfirmed=true; nonexistent token returns undefined.
- test/duplicate-subscription.test.ts — NEW (4 tests): full cycle of
  register → confirm → re-register → assert one active row with
  unchanged key, verifying the upsert is idempotent.
- Adapted 3 existing test files to destructure the new ConfirmResult.
2026-09-17 16:57:30 -04:00
Agent
13b1e9d00f Standardize tests on vitest
- Add vitest as dev dep (pnpm add -D vitest), create vitest.config.ts
  with globals:true and setupFiles
- Migrate digest-template, normalize-relay-url, reschedule-on-frequency-change,
  and event-arrival-race from raw JS + hand-rolled asserts to vitest
  describe/it/expect, as .test.ts (port all assertions without weakening)
- Create test/setup.ts with env vars needed by env.ts/mailer.ts
- DELETE test/web-ui.test.js: dead test for removed web UI (PR #1)
- package.json: add test:unit script (vitest run), keep test = bash E2E
- script/checks: add pnpm test:unit after build
- README: document pnpm test:unit / pnpm test
2026-09-14 13:32:19 -04:00