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
b54fb4f891 POST /notify/🆔 standardize response shape — always include stored field
Bug: when the event was not found at the relay, the handler returned
{ ok: true, skipped: true }, which did not match the documented contract
{ ok: true, stored: boolean } in README.md.

Fix: change the 'skipped' response to { ok: true, stored: false }, so the
response shape is consistent across all code paths.

- src/server.ts: changed line 287 from { ok: true, skipped: true } to
  { ok: true, stored: false }, with updated comment
- README.md: added a note documenting the skip case (stored: false)
- test/notify-response-shape.test.ts: new test that asserts stored=false
  and that skipped is never present
2026-09-14 13:42:11 -04:00