Skip to content

GDPR: fix broken PII purge, add export endpoint, document immutability - #1081

Merged
joelpeace48-cell merged 5 commits into
FinesseStudioLab:mainfrom
Armolas:feat/927-gdpr-data-export-delete
Jul 29, 2026
Merged

GDPR: fix broken PII purge, add export endpoint, document immutability#1081
joelpeace48-cell merged 5 commits into
FinesseStudioLab:mainfrom
Armolas:feat/927-gdpr-data-export-delete

Conversation

@Armolas

@Armolas Armolas commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #927. A separate, recently-merged PR (#1076, "soft-delete and retention policy") had already landed most of the delete-side plumbing for this issue (piiPurgeService.js, /pii/purge-user, /pii/purge-campaign) — but it turned out to be completely non-functional (every call 500'd) and had a real access-control hole. This PR fixes both bugs, adds the missing export side, and closes out the remaining acceptance criteria rather than building a parallel implementation.

What was broken

  • purgePiiUser/purgePiiCampaign always 500'd. They read campaignRepository.db, a property that doesn't exist on that repository — so dal was always null and every request failed with DB_UNAVAILABLE. Fixed to use the real dal.db that's already in scope in index.js.
  • Auth was effectively "any valid API key." Each purge route had two competing registrations for the same path. Express only ever dispatches the first match, so the second registration — gated by requireScope('org:manage'), a scope that isn't even in VALID_API_KEY_SCOPES and so could never actually pass — was silently dead code. The live behavior was any tenant's API key could purge any other user's PII site-wide. Replaced both with a single requireMasterKey-gated registration, consistent with every other admin-sensitive route in the codebase.
  • Zero test coverage for the actual HTTP routes. Existing tests (softDelete.test.js) called the service functions directly against a hand-built db, which is exactly why neither bug above was ever caught.
  • A third, unrelated but blocking bug: notificationService.js (from a different recently-merged feature) imports a module that doesn't exist, crashing every test/boot path that loads index.js. Fixed as a one-line prerequisite — nothing could be verified without it.

What's new

  • exportPiiForUser (piiPurgeService.js) — reuses the same PII_TABLES map as the purge function (single source of truth for which off-chain tables hold wallet/email-linked PII), returning matched rows per table. Web Push credential fields (p256dh, auth) are redacted; everything else is returned as-is.
  • POST /api/v1/pii/export-user — master-key gated, symmetric with the (now-fixed) purge-user. Audit-logged with per-table row counts only — never the exported data itself, so the erasure/access trail never becomes a second copy of the PII it's logging about.
  • Filled two gaps in PII_TABLES: notification_preferences and unsubscribe_tokens were missing.
  • idempotencyMiddleware added to both purge routes (previously absent), so a retried purge request replays the cached result instead of re-processing.
  • Docs: extended docs/EVENT_SCHEMA.md's on-chain/off-chain parity rules with the GDPR corollary — the Stellar ledger is public and permanently immutable, purge only erases this backend's off-chain copies, and a future full re-index would recreate the projection tables from chain history (erasure isn't permanent against that case). Also documented that org-staff email PII (organization_members/organization_invitations) is in scope since identifier already accepts either a wallet address or an email.
  • backend/openapi.yaml — documented all three /pii/* paths (two of which existed but were never in the spec) plus new PiiExportResponse/PiiPurgeResponse schemas.

Testing

  • New backend/src/integration/pii.test.js — the first route-level coverage this feature has ever had: master-key required, validation, export/purge round-trip against real cross-table data (referrals, credit_events, push_subscriptions, notification_channel_settings), unrelated-wallet isolation, audit log contains counts only, and Idempotency-Key replay behavior.
  • Extended backend/src/dal/softDelete.test.js with exportPiiForUser unit tests (multi-table export, referee-only matching, redaction, empty-result shape, no side effects) and purge coverage for the two newly-added tables.
  • eslint/tsc --noEmit clean on all touched files (pre-existing, unrelated warnings/errors elsewhere untouched).
  • openapi:validate passes.
  • Live smoke test: booted the real backend, created a campaign + referral for a test wallet, confirmed export returns the row, purge deletes it, a follow-up export comes back empty, and both actions appear in the audit log (tamper-evident hash chain intact) with counts-only diffs — no PII in the audit trail.

Acceptance criteria

  • Export/delete endpoints for off-chain PII.
  • On-chain immutability documented.
  • Audit-logged.

Armolas added 5 commits July 29, 2026 16:04
Every path that loads index.js (the whole app, and any test importing
createApp) crashed at startup: notificationService.js imported
{ logger } from '../lib/logger.js', which doesn't exist. The real
logger is { log } from '../middleware/logger.js'. Found while trying
to run tests for FinesseStudioLab#927 — unrelated to GDPR work, but nothing could be
verified until this was fixed.
exportPiiForUser reuses the existing PII_TABLES map from
purgePiiForUser (single source of truth for which off-chain tables
hold wallet/email-linked PII) to serve GDPR right-of-access requests.
Web Push credential fields (p256dh, auth) are redacted in export
output. Also adds the two tables PII_TABLES was missing:
notification_preferences and unsubscribe_tokens.
Two real bugs in the already-merged PII purge feature, found while
implementing FinesseStudioLab#927:

- purgePiiUser/purgePiiCampaign referenced campaignRepository.db, a
  property that doesn't exist (campaignRepository never exposes the
  raw db handle) — every call unconditionally 500'd. Fixed to use the
  real dal.db already in scope.
- Each purge route had two competing registrations for the same path;
  Express only dispatches the first match, so the intended
  requireScope('org:manage') guard (a scope that isn't even in
  VALID_API_KEY_SCOPES, so could never pass anyway) was dead code —
  live behavior was any valid tenant API key, not master-key-only.
  Replaced both with a single requireMasterKey + idempotencyMiddleware
  registration, consistent with every other admin-sensitive route.

Also adds POST /api/v1/pii/export-user, symmetric with purge-user,
audit-logged with row counts only (never the exported PII itself).
No route-level test coverage existed for these endpoints before —
the unit tests only exercised the service functions directly against
a hand-built db, never the HTTP handlers, which is how the
campaignRepository.db and dead-route-registration bugs shipped
undetected. Covers: master-key required, validation, export/purge
round-trip against real cross-table data, unrelated-wallet isolation,
audit log contains counts only (never the underlying PII), and
Idempotency-Key replay on purge.
…ab#927)

Adds the three /pii/* paths (two pre-existing but never documented,
one new) plus PiiExportResponse/PiiPurgeResponse schemas to
openapi.yaml. Extends EVENT_SCHEMA.md's on-chain/off-chain parity
rules with the GDPR-relevant corollary: the Stellar ledger is public
and permanently immutable and outside this backend's control; purge
only erases our off-chain copies; a future full re-index would
recreate the projection tables from chain history, so erasure isn't
permanent against that case. Also documents that org-staff email PII
is in scope (identifier accepts wallet or email) since PII_TABLES
already covers organization_members/organization_invitations.
@drips-wave

drips-wave Bot commented Jul 29, 2026

Copy link
Copy Markdown

@Armolas Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@joelpeace48-cell
joelpeace48-cell merged commit 6ccd1ef into FinesseStudioLab:main Jul 29, 2026
1 check passed
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.

GDPR/data-retention: user data export + delete endpoints

2 participants