fix(privacy): delete-my-data scrubs the requester identity off site suggestions - #227
Open
BSalaeddin wants to merge 1 commit into
Conversation
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.
Third in the stack: #225 (capture) -> #226 (answer + notice) -> this. Base is
feat/site-suggestions-status-and-requester-notice, notdev. Closes theTODO#225 left in theSiteSuggestionmodel doc.The bug
"Delete my data" emptied three tables and left the requester's email address sitting in
site_suggestions.Scrub, not delete
The row's identifying half is nulled —
user_id,requester_email,user_agent.domain,raw_url,statusandcreated_atstay, and nothing new is stamped.A "please support this store" request is not personal data once the requester is off it, and it is the coupons pipeline's input. Deleting the row would silently retract a store request other people may also have made, and would corrupt a queue this user does not own. A scrub is also not an ANSWER to the request, so it dates nothing:
status_changed_at,notified_atandimported_atare untouched.Matched two ways, and the second is the point
user_idfinds the requests made while signed in. The requester email finds the ones made while signed OUT — where the row carries no user id at all and the address the person typed is the only thing on it. Matching byuser_idalone is the fix that looks right and leaves behind exactly the email this route exists to remove; that losing predicate is kept verbatim in the suite and run against the same rows.Case-insensitive, because the suggest form records what the visitor typed (
Shopper@Example.COM) while the account holds its own spelling — a case-sensitive compare walks straight past the row.An account with no email (the schema allows one) contributes no email branch rather than a
nullone, which would match every anonymous suggestion ever made.In the same transaction
The scrub is the fourth member of the existing
$transactionbatch, not a loose await after it. Run afterwards, a failure would leave the three tables emptied and the email still insite_suggestions, with a 500 telling the user nothing had been removed.The response reports it under its own key —
scrubbed: { siteSuggestions: n }, never folded intodeleted, because these rows still exist and calling that a deletion would be a lie the next reader believes. The profile client reads onlyres.ok, so the added key changes no UI.Verification
tsc --noEmitturbo run prettier-checkRed-proofed in three arms:
user_idaloneThe unit fake now models Prisma's lazy
PrismaPromise: array-form$transactionmembers only execute when the transaction awaits them, which is exactly why a rejected batch leaves rows untouched. With an eager fake, "a partial failure changes nothing" is untestable. That needs one hand-made thenable, carrying a datedoxlint-disablewith its reason (test-fixture scope; nothing ships). Postgres-only facts — thatmode: 'insensitive'really folds case, and that theORreally reaches a row with no user id — are pinned in the integration suite.🟡 A related gap this PR does NOT close
DataPrivacySection.tsxdisables the button and shows "Nothing to delete" when the user has zero savings events, zero followed stores and zero coupon reports. Site suggestions are not in that count, so a user whose only personal data is a suggestion cannot reach this route from the UI at all — including the signed-out requester this PR was written for.Fixing it means adding a suggestion count to the profile overview API and its query, which is a different surface (
account/overview) and a different PR. Flagging rather than widening this one.Also considered and deliberately not done
raw_urlis left as typed. It is a store URL, not an identifier, and the scope named three fields. If a pasted URL could carry a personal token, nulling it is a one-line follow-up — say the word.