GDPR: fix broken PII purge, add export endpoint, document immutability - #1081
Merged
joelpeace48-cell merged 5 commits intoJul 29, 2026
Merged
Conversation
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.
|
@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! 🚀 |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/purgePiiCampaignalways 500'd. They readcampaignRepository.db, a property that doesn't exist on that repository — sodalwas alwaysnulland every request failed withDB_UNAVAILABLE. Fixed to use the realdal.dbthat's already in scope inindex.js.requireScope('org:manage'), a scope that isn't even inVALID_API_KEY_SCOPESand 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 singlerequireMasterKey-gated registration, consistent with every other admin-sensitive route in the codebase.softDelete.test.js) called the service functions directly against a hand-builtdb, which is exactly why neither bug above was ever caught.notificationService.js(from a different recently-merged feature) imports a module that doesn't exist, crashing every test/boot path that loadsindex.js. Fixed as a one-line prerequisite — nothing could be verified without it.What's new
exportPiiForUser(piiPurgeService.js) — reuses the samePII_TABLESmap 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.PII_TABLES:notification_preferencesandunsubscribe_tokenswere missing.idempotencyMiddlewareadded to both purge routes (previously absent), so a retried purge request replays the cached result instead of re-processing.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 sinceidentifieralready 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 newPiiExportResponse/PiiPurgeResponseschemas.Testing
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, andIdempotency-Keyreplay behavior.backend/src/dal/softDelete.test.jswithexportPiiForUserunit 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 --noEmitclean on all touched files (pre-existing, unrelated warnings/errors elsewhere untouched).openapi:validatepasses.Acceptance criteria