Skip to content

fix: UK GDPR/PECR erasure, deletion callback and SMS opt-out - #41

Open
Mcgyver-ai wants to merge 12 commits into
mainfrom
fix/compliance
Open

Mcgyver-ai wants to merge 12 commits into
mainfrom
fix/compliance

Conversation

@Mcgyver-ai

@Mcgyver-ai Mcgyver-ai commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

Audit fixes against e493421, for UK GDPR/PECR. Branched from main (independent of #40). main (with #42) is merged in; conflicts were resolved in fe29ce5.

# Fix Test
1 Public result PII. Superseded by #42, which deleted GET /api/result/:id outright (nothing called it) along with its OpenAPI entry. The merge takes main's side and drops this PR's toPublicResult filter and its test. app/r/[id] reads Firestore server-side and never renders ownerEmail/ownerUid. n/a (route gone)
2 Meta data-deletion callback: NOT fixed (blocked, see below). n/a
3 Erasure gaps. eraseUserData now also deletes agencies/{uid}, inbound_tokens (uid), ghl_events (uid) and invites (agencyId, acceptedBy, and email when the token email is verified). It reads users/{uid} first: any other users docs with the same stripeCustomerId (the webhook's customer-id-keyed orphans) are deleted too. If agencyId points at someone else's agency, the uid is removed from that agency's memberUids via FieldValue.arrayRemove (an already-deleted agency is tolerated). accountService.test.js extended
4 Stripe cancel failure swallowed. cancelSubscriptionBeforeErasure now uses #42's retrieve-first rules: it skips the cancel for canceled or incomplete_expired, continues on resource_missing, and returns 502 Could not cancel your subscription. Please retry or contact support. for anything else, before any erasure. The route calls the helper instead of #42's inline copy, so the logic is unit-tested. This PR's original cancel-first version called cancel on incomplete_expired subscriptions, which Stripe rejects, so those users could never delete their account. That's fixed in f6419f8's parent. Also: the cancel fires customer.subscription.deleted right after erasure, and the webhook's set(merge) recreated users/{uid} for the deleted user. It now uses update() and ignores NOT_FOUND. accountService.test.js: no sub, active cancelled, canceled / incomplete_expired skip the cancel, resource_missing continues, cancel failure → 502, status check failure → 502 with no cancel. The webhook change has no test (no webhook test harness exists).
5 SMS STOP bypass. smsSuppressed is keyed by Twilio's E.164 From, so a reminder stored as 07700 900123 could never match an opt-out. /reminders/schedule strips spaces and requires /^\+[1-9]\d{7,14}$/, otherwise 400 to must be E.164, e.g. +447700900123 (no country-code guessing). The cron marks any queued non-E.164 reminder status: "failed", error: "invalid number" and skips it, and checks and sends using the normalized number. The same rule is in reminderService.js (normalizeE164), which the live route imports. reminderService.test.js

Item 2: Meta deletion callback is blocked

Meta's user_id is the app-scoped Facebook user ID. The Instagram and Messenger connect flows only take a pasted Page access token and validate it with page/account field lookups (lib/meta.js), so no user ID is ever available to store. The callback is unchanged: it still deletes nothing and still records status: "completed". Suggested follow-up (in TASKS.md): call GET /debug_token at connect time with an app access token (META_APP_ID|META_APP_SECRET), or add Facebook Login. Until then, the callback should stop reporting completed.

Other notes

  • Dead services. reminderService gets the same E.164 fix but isn't wired into the route's cron wholesale: the service compares CRON_SECRET with !==, while the live route uses timing-safe safeEqual.
  • TASKS.md gains a line about pinning the canary/beta deps to stable releases. The deps themselves are untouched.

Test plan

  • yarn test: Test Files 20 passed (20) / Tests 160 passed (160)
  • yarn build with CI's placeholder env: ✓ Compiled successfully / ✓ Generating static pages (76/76)
  • Manually verified locally / in preview. Not done: Vercel previews have no Firebase creds, so API routes 500 there.

🤖 Generated with Claude Code

https://claude.ai/code/session_018sQyAmPXx4AVoEa3XZmC6b

…el failures

eraseUserData also deletes agencies/{uid}, inbound_tokens, ghl_events, invites, orphan users docs sharing the stripeCustomerId, and removes the uid from another owner's agency. cancelSubscriptionBeforeErasure aborts deletion unless the subscription is cancelled or already gone.
smsSuppressed is keyed by Twilio's E.164 From number. Reminders are now validated as E.164 at schedule time, and the cron fails any queued non-E.164 reminder instead of sending past an opt-out.
Copilot AI balanced review requested due to automatic review settings October 17, 2026 12:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vercel

vercel Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
dm-forge Ready Ready Preview Oct 20, 2026 8:29pm UTC

Comment thread lib/services/accountService.js Resolved
Invites hold the invitee's email and acceptedBy uid even when another user owns them.
An unverified token email could belong to anyone, so deleting that account would wipe pending invites meant for the real owner. Drops the stored-email fallback, which came from the same unverified source.
Resolves the route.js conflicts with #42 by taking main's side: GET
/api/result/:id stays deleted, and main's retrieve-first Stripe cancel
in DELETE /api/account replaces the branch's version. toPublicResult and
its test go with the deleted route.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018sQyAmPXx4AVoEa3XZmC6b
…hook recreating erased users

cancelSubscriptionBeforeErasure now retrieves first and skips the cancel
for canceled or incomplete_expired subscriptions. The cancel-first
version called cancel on incomplete_expired, which Stripe rejects, so
those users could never delete their account. The route uses the tested
helper again instead of main's inline copy.

customer.subscription.deleted lands right after erasure. It used
set(merge) on users/{uid}, which recreated the doc for a deleted user;
update() with NOT_FOUND ignored can't.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018sQyAmPXx4AVoEa3XZmC6b

This branch was successfully deployed

1 active deployment
Preview — f6419f80 Deployed Oct 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants