Skip to content

Commit f0c1157

Browse files
Kjubikstronkclaude
andcommitted
Fix two security holes, two data-loss bugs, and the cost leak
Pre-release review by three reviewers. Most of what they found was mine. SECURITY (rules deployed) `allow update` only checked that coupleId was UNCHANGED, never that the record was yours — the other three verbs check ownership and this one didn't. Any member knowing a document id could rewrite, cancel or unschedule another couple's date. Reads being blocked made ids hard to guess, which is obscurity, not a boundary. onlyMyMemory() treated `after == null` as "memories untouched", but that is also what nulling or deleting the field looks like. Either partner could erase the other's rating with one field, which is the whole of rate-blind-then-reveal. A create can no longer arrive pre-loaded with a rating in the partner's name either. DATA LOSS "never mind" in the rating editor restored from `item.memory` — the deprecated single-memory field that nothing writes any more. It reset to empty, and the next save overwrote a real rating with blanks. Restores from `mine` now. The editor was also seeded once at mount, and this component is memoised and keyed by id, so it survives every snapshot: rate a date on your phone and an open card on your laptop still held the old values, ready to write them back over the newer rating. It re-seeds when opened. WRONG BEHAVIOUR A date happening TODAY counted as past from midnight — it offered "we went" and sat under "how did it go?" instead of "next up". The countdown even had an unreachable branch that rendered the word "today", which is what tipped the reviewer off that this was a slip rather than a choice. Signing out terminated the Firestore client, and that instance is a module singleton created once at import. Signing back in without a reload hit a dead client and failed every read and write. Now reloads. "Later" on the update pill destroyed the prompt rather than deferring it: workbox only fires `waiting` when a NEW worker arrives, so a worker already parked never announced itself again — the exact stranded-on-an-old-build case that component exists to prevent. The overflow sheet's last row, delete, sat under the iPhone home indicator inside its swipe-up region. EditSheet already handled this. COST `photos` was requested on every Place Details lookup and the resulting photoUrl was never rendered anywhere — a higher billing tier for a field with no reader. Details are also cached per place per session: every tap on a map POI fired a fresh billable lookup with no dedupe, so tapping the same café three times while deciding cost three of them. The map re-framed itself to fit all pins on every Firestore emission rather than once per mount, throwing you away from the pin you had just added. The multi-date card claimed to scroll but its parent disabled pointer events, so any place with enough dates silently truncated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent fe78744 commit f0c1157

11 files changed

Lines changed: 2380 additions & 18 deletions

File tree

‎design_handoff_nct127/NCT 127.dc.html‎

Lines changed: 562 additions & 0 deletions
Large diffs are not rendered by default.

‎design_handoff_nct127/README.md‎

Lines changed: 481 additions & 0 deletions
Large diffs are not rendered by default.

‎design_handoff_nct127/github.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
repo: Kjubikstronk/press-it
2+
branch: main
3+
4+
## Last sync
5+
date: 2026-08-08T13:20:00Z
6+
7+
### Updated in this project
8+
- Read press-it's full design system (style.css) and page structure (index.html)
9+
- Rebuilt the layout for NCT 127 with a neon green/magenta palette
10+
- Kept Anton / Inter / Space Mono / Noto Sans KR, zero border-radius, hairline rules
11+
12+
## Screen map
13+
| Project screen | Repo files |
14+
| --- | --- |
15+
| NCT 127.dc.html | index.html, assets/css/style.css, README.md |

‎design_handoff_nct127/image-slot.js‎

Lines changed: 1225 additions & 0 deletions
Large diffs are not rendered by default.

‎firestore.rules‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,17 @@ service cloud.firestore {
3131
function onlyMyMemory() {
3232
let before = resource.data.get('memories', null);
3333
let after = request.resource.data.get('memories', null);
34-
return after == null
34+
// `after == null` must NOT mean "untouched" — it is also what a write
35+
// that nulls or deletes the field looks like, which would let either
36+
// partner erase the other's rating with a single field and make
37+
// rate-blind-then-reveal meaningless.
38+
return (before == null && after == null)
3539
? true
36-
: (before is map
37-
? after.diff(before).affectedKeys().hasOnly([request.auth.uid])
38-
: after.keys().hasOnly([request.auth.uid]));
40+
: (after is map
41+
? (before is map
42+
? after.diff(before).affectedKeys().hasOnly([request.auth.uid])
43+
: after.keys().hasOnly([request.auth.uid]))
44+
: false);
3945
}
4046

4147
/**
@@ -55,12 +61,20 @@ service cloud.firestore {
5561
// must belong to your couple, and must be signed by you.
5662
allow create: if isMember()
5763
&& request.resource.data.coupleId == myCouple()
58-
&& request.resource.data.createdBy == request.auth.uid;
64+
&& request.resource.data.createdBy == request.auth.uid
65+
// Nobody starts a date with a rating already in their partner's name.
66+
&& (request.resource.data.get('memories', null) == null
67+
|| request.resource.data.memories.keys().hasOnly([request.auth.uid]));
5968

6069
allow update: if isMember()
70+
// The record must already be yours. Checking only that coupleId is
71+
// UNCHANGED left every date in the deployment writable by any member
72+
// who knew its id — the other three verbs check ownership, this one
73+
// didn't.
74+
&& resource.data.coupleId == myCouple()
6175
&& request.resource.data.createdBy == resource.data.createdBy
6276
&& onlyMyMemory()
63-
// Nothing may ever move a record from one couple to another.
77+
// Nor may anything move a record from one couple to another.
6478
&& request.resource.data.coupleId == resource.data.coupleId;
6579
}
6680

‎src/components/DateCard.tsx‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,22 @@ function DateCard({
6060
const [stars, setStars] = useState(mine?.stars ?? 0)
6161
const [memoryNote, setMemoryNote] = useState(mine?.note ?? '')
6262

63+
/**
64+
* Re-seed the editor from whatever is actually stored, each time it opens.
65+
*
66+
* These were set once at mount, and this component is memoised and keyed by
67+
* id, so it survives every snapshot. Rate a date on your phone and the open
68+
* card on your laptop still held the old values — saving there wrote them
69+
* back over the newer rating.
70+
*/
71+
useEffect(() => {
72+
if (mode !== 'remembering') return
73+
setStars(mine?.stars ?? 0)
74+
setMemoryNote(mine?.note ?? '')
75+
// Keyed on the stored memory, so a partner's write while the editor is
76+
// closed is picked up next time it opens.
77+
}, [mode, mine?.stars, mine?.note])
78+
6379
const cancelled = item.status === 'cancelled'
6480

6581
/**
@@ -82,7 +98,7 @@ function DateCard({
8298
* timezone.
8399
*/
84100
const isPast = item.scheduledFor
85-
? item.scheduledFor <= format(new Date(), 'yyyy-MM-dd')
101+
? item.scheduledFor < format(new Date(), 'yyyy-MM-dd')
86102
: false
87103

88104
function remember() {
@@ -345,7 +361,7 @@ function DateCard({
345361
</button>
346362
</header>
347363

348-
<div className="flex flex-col p-3">
364+
<div className="safe-bottom flex flex-col p-3">
349365
<SheetRow
350366
onClick={() => {
351367
closeSheet()
@@ -459,8 +475,12 @@ function DateCard({
459475
type="button"
460476
className="pixel-btn legend px-2 py-1"
461477
onClick={() => {
462-
setStars(item.memory?.stars ?? 0)
463-
setMemoryNote(item.memory?.note ?? '')
478+
// From `mine`, not the deprecated `item.memory` — that field
479+
// is never written any more, so restoring from it silently
480+
// reset the editor to empty and the next save destroyed the
481+
// real rating.
482+
setStars(mine?.stars ?? 0)
483+
setMemoryNote(mine?.note ?? '')
464484
setMode('idle')
465485
}}
466486
>
@@ -484,7 +504,10 @@ function DateCard({
484504
autoFocus
485505
onKeyDown={(e) => {
486506
if (e.key === 'Enter') callOff()
487-
if (e.key === 'Escape') setMode('idle')
507+
if (e.key === 'Escape') {
508+
setReason('')
509+
setMode('idle')
510+
}
488511
}}
489512
/>
490513
</label>

‎src/components/DateMap.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useMemo, useState } from 'react'
1+
import { useEffect, useMemo, useRef, useState } from 'react'
22
import { format, parseISO } from 'date-fns'
33
// Aliased: the library's `Map` component otherwise shadows the global Map
44
// constructor, which broke `new Map()` in this file.
@@ -92,10 +92,17 @@ function LiveMap(props: Props) {
9292
const [me, setMe] = useState<{ lat: number; lng: number } | null>(null)
9393

9494
const groups = useMemo(() => groupByPlace(props.items), [props.items])
95+
const framed = useRef(false)
9596

9697
// Frame every pin on first load, so you open the map to the overview.
9798
useEffect(() => {
9899
if (!map || groups.length === 0) return
100+
// Once per mount. `groups` derives from a fresh array on every Firestore
101+
// emission, so this used to re-run whenever either partner edited
102+
// anything — including right after adding a place from the map, which
103+
// threw you away from the pin you'd just dropped.
104+
if (framed.current) return
105+
framed.current = true
99106
const bounds = new google.maps.LatLngBounds()
100107
for (const [, items] of groups) {
101108
bounds.extend({ lat: items[0].place.lat, lng: items[0].place.lng })

‎src/components/UpdatePill.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ export default function UpdatePill() {
2525
if (!registration) return
2626

2727
const check = () => {
28+
// A worker that is already waiting will never fire `waiting` again, so
29+
// dismissing the pill once would otherwise silence it forever — the
30+
// exact stranded-on-an-old-build case this component exists to stop.
31+
// Re-raise it here so "later" means "next time" rather than "never".
32+
if (registration.waiting) setNeedRefresh(true)
2833
// Pointless while offline, and it throws on some browsers.
2934
if (navigator.onLine) void registration.update()
3035
}

‎src/lib/auth.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ export function AuthProvider({ children }: { children: ReactNode }) {
5757
// into the entry chunk and undo the login screen's code split.
5858
const { clearCache } = await import('./db')
5959
await clearCache()
60+
// clearCache() terminates the Firestore client, and that instance is a
61+
// module singleton created once at import — signing back in without a
62+
// reload would hit a dead client and fail every read and write. The
63+
// reload is what makes the wipe survivable.
64+
window.location.reload()
6065
},
6166
}),
6267
[user, loading],

‎src/lib/places.ts‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,37 @@ import { useMap, useMapsLibrary } from '@vis.gl/react-google-maps'
33
import { MAP_INSTANCE_ID, lastViewport } from './maps'
44
import type { Place } from '../types'
55

6-
/** Only the fields we store. Places bills by field tier, so asking for more
7-
than this costs real money for data we'd throw away. */
6+
/**
7+
* Only the fields we actually use. Places bills Place Details by the tier of
8+
* the highest field requested, so anything unused here costs real money per
9+
* lookup for data we throw away.
10+
*
11+
* `photos` was in this list and its result was never rendered anywhere — the
12+
* stored photoUrl had no reader. Removed.
13+
*
14+
* `rating` is the one remaining Enterprise-tier field, and it buys exactly the
15+
* "4.3 ★" line on the map's candidate card. Dropping it too would put every
16+
* lookup on the cheapest tier.
17+
*/
818
const FIELDS = [
919
'id',
1020
'displayName',
1121
'formattedAddress',
1222
'location',
13-
'photos',
1423
'rating',
1524
] as const
1625

26+
/**
27+
* Place details, once per place per session.
28+
*
29+
* Every tap on a map POI fired a fresh billable lookup, with no dedupe — so
30+
* tapping the same café three times while deciding cost three of them, as did
31+
* every stray tap while panning. Details are stable enough that a
32+
* session-lifetime cache is safe, and it turns the one uncapped spend vector
33+
* into a bounded one.
34+
*/
35+
const detailCache = new globalThis.Map<string, Place>()
36+
1737
type PlacesLib = google.maps.PlacesLibrary
1838

1939
export function toPlace(
@@ -26,7 +46,8 @@ export function toPlace(
2646
lat: p.location?.lat() ?? null,
2747
lng: p.location?.lng() ?? null,
2848
placeId: p.id ?? null,
29-
photoUrl: p.photos?.[0]?.getURI({ maxWidth: 640 }) ?? null,
49+
// Never requested any more — see FIELDS.
50+
photoUrl: null,
3051
rating: p.rating ?? null,
3152
}
3253
}
@@ -40,10 +61,14 @@ export async function fetchPlaceById(
4061
placeId: string,
4162
): Promise<Place | null> {
4263
if (!places) return null
64+
const hit = detailCache.get(placeId)
65+
if (hit) return hit
4366
try {
4467
const place = new places.Place({ id: placeId })
4568
await place.fetchFields({ fields: [...FIELDS] })
46-
return toPlace(place)
69+
const converted = toPlace(place)
70+
if (converted) detailCache.set(placeId, converted)
71+
return converted
4772
} catch {
4873
return null
4974
}

0 commit comments

Comments
 (0)