fix(privacy): danger zone counts store requests, and the scrub drops the pasted URL - #228
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.
Fourth in the stack: #225 -> #226 -> #227 -> this. Base is
feat/site-suggestions-delete-my-data-scrub, notdev.Two follow-ups from #227's report.
1. The danger zone can now be reached by the user it was written for
#227 made "Delete my data" scrub the requester identity off
site_suggestions, and the button that reaches it was disabled whenever savings, followed stores and coupon reports were all zero. A request made while signed out carries nothing but the email typed into the form, so the one user whose only personal data is that email read "Nothing to delete" and could never reach the route that removes it.GET /api/account/overviewnow returnssiteSuggestions.identifyingCount, and the gate counts it.The count and the scrub are one predicate, not two. Both call
siteSuggestionIdentityWherefrom the new leafsrc/lib/siteSuggestionIdentity.ts(types from@prisma/clientand nothing else). Two hand-written copies would let the page say "Nothing to delete" about rows the delete route would happily have scrubbed — and both readings look fine in isolation.No second "has anything left to scrub" condition. The spec asked for one, and it would be dead code: every branch of the identity predicate requires a non-null column, so a row already scrubbed (user id, email and user agent all null) matches neither branch, and an anonymous row carrying only a
user_agentbelongs to nobody and is not matched either. Rather than add a guard that reads as a second condition, both facts are pinned — including on real Postgres, where the count goes 1 -> 0 across an actual delete with the row still present.Copy stays honest. Store requests are deliberately not in the dialog's "This permanently removes ..." list, because the request is not removed. They get their own sentence: "It also takes your email and device details off 2 store requests you sent — the requests themselves stay, just without you attached." A suggestion-only user's dialog never says "permanently removes" at all, and a user with no requests sees no sentence about them.
2.
raw_urlis scrubbed tooOwner call. A pasted URL can carry a session or affiliate token, and the pipeline only ever keys on
domain.Emptied, not nulled: the column is
NOT NULLandrawUrlis a required string on the wire the coupons repo reads, so making it nullable would be a cross-repo contract change for no gain. The empty string is unambiguous — the suggest route's body schema requires at least one character, so no live row can be born with one.domainandstatussurvive; nothing new is stamped.Verification
tsc --noEmitturbo run prettier-checkRed-proofed in three arms:
raw_urlThe danger-zone pins render the real section against a real
ProfileOverview, and include the pre-change gate kept verbatim so the pair is a pair.ConfirmDialogis built on the native<dialog>, whoseshowModal()/close()jsdom does not implement; those are stubbed to theopenattribute a browser sets, so the component's own effect still runs and the copy is queryable.Note for the reviewer
ProfileOverviewgained a required field, so every construction site had to supply it. That is the point of the shared type, but it does mean this PR touchessrc/lib/profile/types.ts, which other profile work also lives in.