-
Notifications
You must be signed in to change notification settings - Fork 728
chore: gdpr scripts #3391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: gdpr scripts #3391
Conversation
Signed-off-by: Joana Maia <[email protected]>
Signed-off-by: Joana Maia <[email protected]>
Signed-off-by: Joana Maia <[email protected]>
Signed-off-by: Joana Maia <[email protected]>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
1 similar comment
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
| ) | ||
| if (!proceedMaintainers) { | ||
| throw new Error('User cancelled deletion from maintainersInternal') | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Interactive prompts block database transaction with held locks
The function deleteMemberFromDb is called within store.transactionally() at line 142, but it contains multiple await promptConfirmation() calls that wait for user input. This holds database locks open for an unpredictable amount of time while waiting for human response, which can cause transaction timeouts, lock contention with other database operations, and potential deadlocks. Interactive prompts should happen before the transaction begins, not within it.
Additional Locations (2)
| if (!proceedTable) { | ||
| log.info(`Skipped deletion from ${table}`) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Inconsistent cancellation allows partial deletion in transaction
When a user cancels deletion from a specific table during the loop, the code uses continue to skip to the next table while remaining in the same transaction. However, cancelling maintainersInternal (line 310) or the final member record (line 377) throws an error that rolls back the transaction. This inconsistency means a user could skip some table deletions while others proceed, resulting in partial data deletion within a committed transaction. For GDPR compliance, deletions should be all-or-nothing.
|
|
||
| if (orgResults.length > 0) { | ||
| for (const orgResult of orgResults) { | ||
| if (orgResult.organizationId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Deletion summary omits activities table that gets deleted
The tablesToDelete map in getDeletionSummary does not include the activities table, but the same map in deleteMemberFromDb includes ['activities', ['memberId']] at line 327. This means the user confirmation summary won't show the activities that will be deleted, but the deletion will actually occur. Users receive misleading information about what data will be erased.
Additional Locations (1)
| if (orgDataMap.has(eIdentity.memberId)) { | ||
| orgResults = orgDataMap.get(eIdentity.memberId) | ||
| } else { | ||
| orgResults = await store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Script exits with success code even after failure
When the deletion fails and an error is caught at line 154-155, the script logs the error but then unconditionally calls process.exit(0) at line 158, indicating success. For GDPR compliance scripts, failed deletions must be clearly indicated to automation and monitoring systems. The script should exit with a non-zero code when an error occurs.
| } | ||
| } catch (err) { | ||
| console.error(err) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Tinybird deletion continues after errors causing partial deletion
When a deletion from any Tinybird datasource fails, the error is caught and logged but the loop continues to process remaining tables. This can result in partial data deletion where some tables have the member's data removed while others retain it. For GDPR compliance, if any deletion fails (e.g., maintainersInternal fails but memberIdentities succeeds), the operation should abort rather than leave data in an inconsistent state. The script also exits with code 0 regardless of errors.
Changes proposed ✍️
What
Checklist ✅
Feature,Improvement, orBug.Note
Adds a Tinybird data erasure script and refactors the Postgres member erasure script to be memberId-only with confirmations, summaries, identity archiving, FK-safe deletions, and search/org re-sync.
services/apps/data_sink_worker/src/bin/erase-members-data-tinybird.ts):activities,activityRelations,maintainersInternal,memberIdentities, andmembers.activitiesbyactivityId(fromactivityRelations),maintainersInternalbyidentityIdsubquery.services/apps/data_sink_worker/src/bin/erase-member.ts):memberId; shows deletion summary and prompts for confirmation.memberIdentitiestorequestedForErasureMemberIdentitiesbefore deletion.activityRelations.objectMemberId/objectMemberUsername, deletesmaintainersInternalfirst, then all member-related tables, thenmembers.Written by Cursor Bugbot for commit c7bb8c2. This will update automatically on new commits. Configure here.