fix(privacy): stop showing guests where members live - #497
Merged
Conversation
…y member Why: join codes make the library reachable by strangers; a full roster of home addresses behind a QR code is not defensible. Also strips the now-dead email/street-address rendering in ManageMembersModal, the only UI consumer of this endpoint's member payload (city/state/zip stay, for the map). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The library page tells guests "we keep member details private until you join." GET /api/collections/[id] was not keeping that promise: it served every member's email, street address and exact home coordinates to anyone holding an invite cookie, and — via the isPublic branch — to viewers with no membership at all. Guests and anonymous viewers now get no per-member objects. In their place is `memberAreas`: coordinates rounded to two decimals (~1.1km), identical points merged with a count. Enough to show there are real neighbours on this block, nothing that points at a door or names a person. Members, admins and owners keep exact pins — neighbours seeing neighbours is the product working as intended. Deliberately not jittered: a random offset yields a wrong-but-specific coordinate that points at some other real house. Honest imprecision beats precise wrongness. email, address1 and formattedAddress come out of the payload at every role. Nothing rendered them — email was never even in the client's LibraryData interface — so they were pure weight and pure liability. Redaction lives in src/lib/member-location-privacy.ts as pure functions, so the route reads as "fetch, then redact by role" and the blurring is testable on its own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to 2ce086d, from code review: - Type the address input. neighbourProfile took Record<string, unknown> and cast every field, so a renamed Prisma column would have quietly produced null instead of a compile error — in the one module whose job is emitting a correct payload. A real RawAddress interface removes all four casts; the fatter Prisma row still assigns, since excess-property checks only apply to object literals. - Spell it "neighbor". The codebase and the user-facing copy already do, 209 times; the 21 British spellings were all mine. - Name the projection toNeighborProfile, matching toMemberAreas. Same kind of work, same shape of name. - Drop the negative-zero normalization and its comment. The comment claimed -0 would split grouping keys, but `${-0}` is already "0", so they never could have. A comment that teaches the next reader something false is worse than no comment. - Guests now get owner: null rather than { id }. A bare id is a stable identifier for a real person, which sits oddly next to members: [] telling them nothing else. Verified nothing reads the library owner's id: CollectionDetailClient touches only owner.addresses, and neither other consumer of this endpoint references owner at all. Also pins a behavior we only just understood. Coordinates are populated solely at profile-save time from client-supplied values (profile route, both writes `?? null`), and the only geocoding anywhere is client-side in the map. So the app had been leaning on map renders to backfill coords for members who typed their address by hand — and with address text no longer sent, that backfill is gone at every role. The new test asserts a member with city and state but null coordinates is omitted, so the client follow-up cannot quietly reintroduce an invented position. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The library page tells guests "we keep member details private until you join." The server now keeps that promise; the client had not caught up. Item listings: guests get a first name and nothing else — no surname, no avatar url, no owner id. The card used to overprint the name with block characters client-side, which left the real name and face sitting in the JSON one devtools panel away. Redaction moves to the route, and the card renders whatever it was sent. Borrowers and lenders keep their ids: the card compares the two to tell a self-borrow from a real loan. The map: guests were left with no map at all, because members is now [] for them. They get memberAreas instead — one counted badge per rounded coordinate, no name, no face, no click target. Insiders keep their pins. Members with no coordinates are no longer plotted at a random offset from the map's centre. Inventing a location for someone whose location we don't know is worse than showing nothing — the offset points at some other real house. They're dropped, and a caption says how many. Also removes the guest-triggered geocode-and-write-back: it was dead once address text stopped being sent, and a page view should never attempt a write to another user's record. NOTE: this was the only mechanism backfilling coordinates for members who typed their address by hand rather than picking from autocomplete. That backfill no longer happens anywhere. Known, accepted follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-ups from review of a6ec011. The important one: LibraryMap protected its central invariant with comments ("exactly one of the two is ever populated", "only reachable for insiders"). Nothing in the props enforced it, so a caller passing members for a guest silently reopened the hole the branch exists to close — including the InfoWindow with name and avatar. Props are now a discriminated union of view:'pins' | view:'areas', so there is no shape where that type-checks. Verified: all four bad prop combinations now fail tsc. That also disentangles isGuest, which had been answering two questions at once. It only ever locked the map controls, so it is now `locked`. Escaped display names out of innerHTML. LibraryMap interpolated member.name straight into markup, including into an inline onerror= attribute — names are user-controlled, so `<img src=x onerror=alert(1)>` was live script in every viewer's page. Rather than add an escape helper that only works while everyone remembers to call it, marker construction moved to library-map-markers.ts and builds nodes with createElement/textContent. The injection surface is gone, not patched. Confirmed the old construction parsed a real <img> element out of a display name; the new tests assert it renders as text. The caption precedence moved to chooseMapCaption() and is now unit-tested. It suppresses a pre-existing CTA, and I made that call without being able to render it, so it should not have been an untestable nested ternary in JSX. Also: count unplotted members from the predicate rather than as (total - plotted), which silently becomes a wrong number in a user-facing sentence the moment anything else filters that list; stable NO_MEMBERS/NO_AREAS identities, since these feed an effect dep array; drop the click affordance on avatars that have no id to navigate to; toMatchObject on the insider assertion. Deletes /api/profile/update-coordinates, now unreferenced. CORRECTION to a6ec011's commit message: it called the removed geocode "the only mechanism backfilling coordinates," which overstates it. That endpoint 403s unless userId === sessionUserId, so the write could only ever succeed for the viewer repairing their own missing pin — every other member's attempt was already failing. It was self-service repair, not a system-wide backfill. The gap is real but narrower than stated: a member who typed their address by hand no longer gets coordinates filled in by visiting a library page. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Library maps place a pin per member from Address.latitude/longitude. Those were only ever written from values the client supplied, and two reachable paths produce an address with none: a free-form address typed without picking an autocomplete suggestion, and a failed place-details lookup. Client-side geocoding used to paper over this, but it depended on shipping street addresses to the browser — which the privacy fix on this branch stopped doing. An address saved without coordinates stayed that way forever and its owner was never on the map. Geocoding belongs on the server anyway: it runs once, it's authoritative, it works for every member rather than only someone looking at their own pin, and no street address leaves the server. - src/lib/geocode.ts: geocodeAddress() against Google's Geocoding API using GOOGLE_PLACES_API_KEY. Never throws; returns null on every failure mode and logs each one distinguishably. REQUEST_DENIED names the likely cause — the Geocoding API is a separate product from the Places API and may not be enabled for that key. A silent failure here would recreate the invisible gap this closes. - Profile save (POST and PUT) resolves coordinates before opening the Prisma transaction. A round-trip to Google while holding a DB connection is how pools get exhausted. When the client already sent coordinates — the common path — no call is made and it costs nothing. A geocoding failure never blocks the save. - scripts/backfill-address-coordinates.ts for rows that predate this. Dry-run by default, --apply to write, 200ms between calls, per-row outcomes and a summary, and one bad row can't abort the run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Geocoding is billable ($5/1000 requests) and was bypassing the ceiling
that already governs the two Places routes, so profile saves on the
free-form path and — much worse — a backfill loop could spend without
limit. The cap now sits at both call sites rather than inside
geocode.ts, which stays pure I/O with no Prisma or Redis import so it
remains testable and reusable.
Profile save: checkSpendCap('places') before the lookup, recordSpend
after a successful one. A blown cap skips the geocode and saves with
null coordinates, exactly as any other geocoding failure does — nobody
should be unable to update their profile because we spent the day's
budget. Logged distinctly so it isn't misread as Google refusing us.
Backfill: the cap is checked before every call that would actually
spend, and a blown cap stops the run cleanly and prints how far it got
and how to resume, instead of grinding through hundreds of rows.
GEOCODE_COST_CENTS = 1, rounded up from 0.5¢ so the cap errs toward
under-spending. recordSpend floors every call at a whole cent anyway,
so sub-cent values can't be expressed.
OPERATOR PREREQUISITE: the Geocoding API must be enabled on
GOOGLE_PLACES_API_KEY in Google Cloud. It is a separate product from
the Places API, and enabling one does not enable the other. Until it
is, this degrades to a no-op — every lookup returns null and logs a
REQUEST_DENIED message naming this as the likely cause, and addresses
save without coordinates as they did before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The delay between calls sat at the bottom of the loop, but the failure path reached it via `continue` and skipped it. A failing row has still hit Google, so a run where every row fails degenerated into a tight loop with no delay at all — and "every row fails" is precisely what happens when the Geocoding API isn't enabled on the key, which is the most likely state of the very first run. Moving the sleep into a finally applies it to every attempted call. The too-little-address-text skip stays above it and still costs nothing, since it never calls out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ated Spend was recorded only when a lookup produced coordinates, because geocodeAddress returned Coordinates | null and "Google answered, no match" was indistinguishable from "we never got a billable answer" at the call site. Google charges for a request it answered either way. This isn't a rounding error. The backfill runs over exactly the addresses that already failed to geocode client-side, so they skew malformed and ambiguous — ZERO_RESULTS may be most of that population rather than an edge case. A run could make several hundred billable calls and record close to nothing, defeating the cap on the single operation with the highest spend risk. geocodeAddress now returns a discriminated GeocodeResult: ok — matched (billable) no_match — Google searched and found nothing (billable) failed — rejected, never sent, or never answered (not billable) isBillableOutcome() lives beside it so the two call sites can't drift on what counts. REQUEST_DENIED and a missing API key are both `failed`: a rejected request isn't charged, and a missing key sends no request at all. Failures carry a reason so the distinguishable logging survives, and the REQUEST_DENIED message still names the likely cause. geocode.ts stays pure I/O — no Prisma, no Redis. The cap stays at the call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mcull
added a commit
that referenced
this pull request
Jul 24, 2026
…ard (#508) * docs(invites): spec the guest-preview redesign (sub-project B) Restyle the guest preview so it names the inviter (or host, for join codes), states the privacy promise as hospitality, and reframes the CTA as claiming a library card. First of the invite-flow redesign sub-projects. The dossier's 'front porch' is an internal design metaphor only — never user-facing, and the component is named GuestPreview so it can't leak through the code either. Why: the guest preview never names who invited you — the dossier's §6.1 says that personal-invitation trust signal is the strongest one going unused Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(invites): implementation plan for the guest-preview redesign Why: routine — task-by-task TDD plan derived from the approved sub-project B spec Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(invites): expose guest invitationContext (inviter/host first name) The guest payload now carries who invited the visitor — a bound invite's sender, or the owner as host for a join code — first name only, so the preview can name them. #497 redaction is otherwise intact. Why: the guest preview never named who invited you; §6.1 calls that the strongest unused trust signal Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(invites): GuestPreview component — inviter header + claim-card CTA A presentational two-slot component (header / claim) that names the inviter or host, states the privacy promise, and reframes the join CTA as claiming a library card. No data fetching; wired next. Why: consolidate the three scattered guest blocks into one branded surface built around who invited you Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(invites): render GuestPreview, retire the three old guest blocks The welcome banner, the mid privacy note, and the 'Ready to join' checklist are replaced by the GuestPreview header + claim slots, which name the inviter and carry the same join behavior. Why: the guest sees one branded arrival built around who invited them, not three generic boxes Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What this is
This branch started as one task — trimming the member roster payload — as prep for an upcoming invitations feature. The first look at the code found the roster endpoint wasn't the only door, and it grew from there.
Below the library map, guests are told:
That was not true. This branch makes it true.
What was wrong
GET /api/collections/[id]returnedemail,address1,formattedAddress,latitudeandlongitudefor every member and the owner, with no role-based filtering at all. The map plotted one pin per member at their exact home coordinates. The only guest-facing treatment was a CSSblur(4px)on the avatar image — the pin position was never altered, and clicking the marker opened an InfoWindow with the member's full name and an unblurred full-resolution photo.Reachable by anyone holding an
invite_tokencookie, including a stale one from a different library against anisPubliclibrary. A stranger who scanned a QR code and never signed in couldcurlthe endpoint and receive every neighbour's home address and coordinates.Separately, member display names were interpolated unescaped into
innerHTMLwhen building map markers — including into an inlineonerror=attribute. Names are user-controlled, so this was XSS. Confirmed by reproducing it (<img src=x onerror=alert(1)>as a display name parses out a real<img>element) before fixing.What changed
Server payload, by role.
email,address1andformattedAddressare gone for everyone — nothing rendered them; item address text was only ever a geocoder input. Members, admins and owners keep exact coordinates. Guests getmembers: [],owner: null, andmemberAreas: Array<{lat, lng, count}>— coordinates rounded to 2 decimal places (~1.1 km) with identical points merged and counted. Item people are reduced to first name with no avatar.No jitter, deliberately: a wrong-but-specific coordinate points at some other real house, which is worse than an honest blob. That reasoning is a comment in
member-location-privacy.tsso nobody re-adds it.Client. Guests render count badges instead of pins. Members without coordinates are no longer placed "near centre" with a random offset — they simply aren't plotted, and
LibraryMembernow requires lat/lng so an unplaced member is unrepresentable.LibraryMapPropsis a discriminated union (view: 'pins' | 'areas') so a guest-with-pins state can't be constructed. The█client-side name blackout and the cosmetic avatar blurs are gone — the redaction is server-side now, and leaving them would imply a protection that no longer exists.Markers are built with
createElement/textContentinstead ofinnerHTML, removing the injection surface rather than escaping it.Server-side geocoding. Removing address text from the payload killed the client-side geocode-and-write-back that had been quietly backfilling coordinates for addresses typed without picking an autocomplete suggestion.
AddressAutocompleteruns infreeSolomode, so that's a reachable path, not a theoretical one. Geocoding now happens server-side at profile save — resolved before the transaction opens, so no DB connection is held across a network call to Google — plusscripts/backfill-address-coordinates.tsfor existing rows (dry-run by default).All of it is under the existing
placesspend cap at both call sites.ZERO_RESULTScounts as billable, because Google charges for answering "no match" — in the backfill population, which is by definition addresses that already failed to geocode, unmatched addresses may well be the majority.Deleted:
src/components/CollectionMap.tsx(338 lines, unreferenced, contained the same geocode-and-write-back) andsrc/app/api/profile/update-coordinates/route.ts(no remaining callers).Verification
778 tests, typecheck clean. Beyond unit tests, the guest payload was checked against a running server with real data:
All three geocoding outcomes were confirmed against live Google:
okreturns real coordinates (billable),no_matchreturns ZERO_RESULTS (billable),failedcovers rejected/unreachable/never-sent (not billable).The Geocoding API must be added to
GOOGLE_PLACES_API_KEY's API restrictions allowlist in Google Cloud Console. Enabling it under APIs & Services → Library is not sufficient — if the key has "Restrict key" set, the API must also appear in that key's list. Without it, geocoding degrades to a logged no-op (REQUEST_DENIED) and silently does nothing.Also worth confirming Redis is configured wherever the backfill runs:
checkSpendCapfails open without it, so the per-row spend protection disappears.Known follow-ups
isSelfBorrowflag would remove the needmain, undiagnosedThat last one means the guest area badges have never been seen rendered by anyone — the visual design is an unvalidated guess. Worth an eye before or shortly after merge.
Field-Note-Why: started as roster-payload prep for the library invitations work; the first look at the code found the endpoint wasn't the only door and the promise on screen wasn't being kept
Field-Note-Interesting: the geocoding "fix" we removed turned out to be load-bearing for free-typed addresses, so removing it forced building the real server-side version; separately, a test asserting a security property passed against a mutation that cut the keyspace 2^40 → 2^32
Field-Note-Friction: three rounds lost to a Google Cloud distinction — enabling an API in the project does not add it to a key's API restrictions allowlist, and the error text only says "check the API restrictions settings"
Field-Note-Deferred: #494, #495, #496; the guest area badge design is unvalidated because the map won't render locally on any branch
🤖 Generated with Claude Code