Commit Graph

2 Commits

Author SHA1 Message Date
Arnas Donauskas
be7edfd180
fix(webhook): require META_APP_SECRET, fail closed
The webhook signature verifier used to fall open with a warning log
when META_APP_SECRET was unset — a deliberate soft-rollout choice that
is unsafe for a public template. A forker who forgets the env var
would be running a fully spoofable webhook: anyone knowing the URL
could POST fabricated status updates and drift broadcast counts.

Now:
- `verifyMetaWebhookSignature` returns false when the secret is missing
  and logs at error level, so the route responds 401 to every POST
  until the operator configures it.
- Docs mark META_APP_SECRET as **required**:
  - docs/environment-variables.md lists it under Required, not
    Recommended.
  - docs/whatsapp-setup.md §6 is now "Signature verification (required)".
  - docs/troubleshooting.md updated accordingly.
- .env.local.example lists the var with guidance on where to find it.

Also fixes a pre-existing gitignore bug: `.env*` was swallowing
`.env.local.example`, which meant forkers running
`cp .env.local.example .env.local` from the getting-started guide
hit "no such file". Added a negation for `*.example`.

Upgrade note for existing deploys: set META_APP_SECRET (Meta → App
Settings → Basic → App Secret) before updating, or inbound webhooks
will 401 until it's configured.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 18:54:52 +03:00
Arnas Donauskas
66dd4ef97e
fix(broadcasts): senior-review hardening pass
Fixes 8 real issues found during a thorough review of the broadcast
feature (PRs #21-#29).

Security
  * Webhook POSTs had zero spoof protection. Anyone could forge a
    status update and drift broadcast counts arbitrarily. Added
    HMAC-SHA256 signature verification via the X-Hub-Signature-256
    header (Meta's standard). New helper src/lib/whatsapp/webhook-
    signature.ts with timing-safe comparison. Rollout fails open
    with a warning when META_APP_SECRET is unset so existing
    deploys don't break at rollout; set the env var to enforce.

Correctness
  * CSV audience was completely broken — synthesized "csv-N" strings
    as contact_id, which failed the UUID cast on insert. Every CSV
    broadcast silently created zero recipients. Replaced with a
    real find-or-create against the contacts table (batched lookup
    by phone, single INSERT for missing rows, de-duplicates CSV
    inputs). CSV contacts now end up as real contacts.rows with
    real UUIDs flowing through the rest of the pipeline.

  * Recipient insert errors were logged with console.error and the
    loop marched on — broadcasts then ran with incomplete recipient
    sets, webhook updates couldn't find rows, and aggregate counts
    drifted. Now: on batch insert failure, flip the broadcast to
    failed + failed_count=total + throw so handleSend's toast fires.

  * Webhook status guard ordered "failed" above "replied", which
    meant a webhook replay delivering "failed" for an already-
    replied recipient would upgrade replied → failed. Split the
    ladder: forward-only ordering across pending → sent →
    delivered → read → replied; failed is a terminal side branch
    that can only be reached from pending or sent (never from any
    success state). Net effect: a message that was read can no
    longer regress to failed on a replay.

  * Template sync only pulled page 1 (ignored paging.next). Users
    with >100 templates silently missed some, and broadcasts with
    those names failed with Meta's "template not found". Now
    follows paging.next with a 20-page safety cap, reports
    truncated=true in the response if hit.

  * Template sync reported success:true even when individual
    upserts failed. Now accumulates per-template errors and
    surfaces them via a separate toast.error in the UI — green
    toast no longer hides silent drift.

Performance
  * Migration 005: replace the broadcast_recipients aggregate
    trigger installed in migration 003. Old version ran
    COUNT(*) FILTER on every row change — O(n²) across a send
    loop (10k recipients × 10k index scans). New version
    increments / decrements by ±1 per status transition: O(1)
    per change. Kept the old recompute_broadcast_counts() as a
    callable safety-net in case of drift.

  * List page poll kept running while the tab was hidden. Added
    visibilitychange handler — polling pauses when hidden,
    resumes (and does an immediate fetch) on refocus.

UX
  * Step 3 (Personalize) let users skip past unmapped placeholders
    → broadcasts then sent to Meta with empty-string params.
    Next is now disabled until every {{N}} has a value; an amber
    warning lists the missing placeholders.

Documented accepted risks in the PR body.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-17 17:38:52 +03:00