From 29d41b81eb99d56526dd6f5e3b455a26148c8e95 Mon Sep 17 00:00:00 2001 From: "shakespeare.diy" Date: Mon, 7 Sep 2026 20:31:48 +0700 Subject: [PATCH] Stash: prevent data-loss clobber on save + confirm before deleting gear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audited the Stash — a private, NIP-44-encrypted, Amber-sensitive surface. DATA-SAFETY FIX (the important one): The stash is a single replaceable event whose encrypted content is the WHOLE item list, so every add/update/remove rewrites the entire event. The mutations built the new list from the `stash` closure value — but that's legitimately `undefined` before the decrypt completes, and with Amber the read query is gated / staleTime:Infinity, so it may never have been decrypted at mutation time. Result: adding one item while `stash` was empty/undefined could publish a list containing only that item and silently WIPE all existing gear. Fix: mutations now call `fetchLatestStash()` — a fresh relay fetch + decrypt of the current stash immediately before saving — and build the new list on top of that. It falls back to React Query cache (never an empty list) on failure, and throws rather than risk a clobber if it truly can't load. saveStash also writes the just-published value straight into the cache to keep reads consistent. The extra decrypt per mutation is an acceptable Amber tradeoff: a mutation is already a deliberate write (encrypt + sign prompt), and data integrity beats prompt count. UX: • Delete now asks for confirmation via an AlertDialog (item name + a note that public items get unlisted). Previously a single tap wiped an item outright — destructive and, with Amber, a surprise signer prompt. The rest of StashPage (login gate, encryption-required state, distinct wishlist/stash/filtered empty states, skeletons, rarity cards, grid/list) is already excellent — left as-is. Bumps to 1.2.23 (Android versionCode 10223 auto-derived). Co-authored-by: shakespeare.diy --- package.json | 2 +- src/hooks/useStash.ts | 67 ++++++++++++++++++++++++++++++++++++++--- src/pages/StashPage.tsx | 50 +++++++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index a133b29..b049572 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "skate-spots", "private": true, - "version": "1.2.22", + "version": "1.2.23", "type": "module", "homepage": "https://skate-spots.com", "scripts": { diff --git a/src/hooks/useStash.ts b/src/hooks/useStash.ts index e849348..b29de37 100644 --- a/src/hooks/useStash.ts +++ b/src/hooks/useStash.ts @@ -113,6 +113,62 @@ export function useStash(options?: { enabled?: boolean }) { refetchOnWindowFocus: false, }); + /** + * Re-fetch + decrypt the freshest stash from relays RIGHT BEFORE a mutation, + * rather than trusting the `stash` closure value. + * + * Why this matters (data-safety): the stash is one replaceable event whose + * content is the WHOLE list. Every add/update/remove rewrites that entire + * event. If we based the new list on a stale or not-yet-decrypted `stash` + * (which is legitimately `undefined` early, and — with Amber — may never have + * been decrypted because the query is gated / `staleTime: Infinity`), we'd + * publish a list missing everything the closure didn't know about and + * silently wipe the user's gear. So we always rebuild from a fresh decrypt. + * + * Falls back to the cached query data, then to an empty stash, only if the + * live fetch genuinely returns nothing. + */ + const fetchLatestStash = useCallback(async (): Promise => { + if (!user?.pubkey || !hasNip44) return { items: [], updatedAt: 0 }; + + let nip44Dec: typeof user.signer.nip44; + try { nip44Dec = user.signer?.nip44; } catch { nip44Dec = undefined; } + if (!nip44Dec) { + // Can't decrypt now — use whatever we already have cached to avoid clobber. + return ( + queryClient.getQueryData(['stash', user.pubkey]) ?? + stash ?? { items: [], updatedAt: 0 } + ); + } + + try { + const events = await nostr.query([{ + kinds: [STASH_KIND], + authors: [user.pubkey], + '#d': [STASH_D_TAG], + limit: 1, + }], { signal: AbortSignal.timeout(10000) }); + + if (events.length === 0) { + // No remote event: safe to start fresh only if we also have nothing cached. + return ( + queryClient.getQueryData(['stash', user.pubkey]) ?? + stash ?? { items: [], updatedAt: 0 } + ); + } + + const decrypted = await nip44Dec.decrypt(user.pubkey, events[0].content); + return JSON.parse(decrypted) as Stash; + } catch (err) { + // On any failure, prefer cached data over an empty list so we never wipe. + console.error('[Stash] Failed to re-fetch latest stash before save:', err); + const cached = queryClient.getQueryData(['stash', user.pubkey]) ?? stash; + if (cached) return cached; + // Nothing to fall back on — surface the error instead of risking a clobber. + throw err instanceof Error ? err : new Error('Could not load your stash to update it safely.'); + } + }, [user, hasNip44, nostr, queryClient, stash]); + // Save stash (encrypt and publish) const saveStash = useCallback(async (newStash: Stash) => { if (!user?.pubkey || !hasNip44) { @@ -137,14 +193,15 @@ export function useStash(options?: { enabled?: boolean }) { ], }); - // Invalidate cache + // Keep the cache in sync with exactly what we just published, then invalidate. + queryClient.setQueryData(['stash', user.pubkey], newStash); queryClient.invalidateQueries({ queryKey: ['stash', user.pubkey] }); - }, [user, createEvent, queryClient]); + }, [user, hasNip44, createEvent, queryClient]); // Add item mutation const addItem = useMutation({ mutationFn: async (item: Omit) => { - const currentStash = stash || { items: [], updatedAt: 0 }; + const currentStash = await fetchLatestStash(); const newItem: StashItem = { ...item, id: crypto.randomUUID(), @@ -167,7 +224,7 @@ export function useStash(options?: { enabled?: boolean }) { // Update item mutation const updateItem = useMutation({ mutationFn: async (item: StashItem) => { - const currentStash = stash || { items: [], updatedAt: 0 }; + const currentStash = await fetchLatestStash(); const newStash: Stash = { items: currentStash.items.map(i => i.id === item.id ? item : i), updatedAt: Math.floor(Date.now() / 1000), @@ -184,7 +241,7 @@ export function useStash(options?: { enabled?: boolean }) { // Remove item mutation const removeItem = useMutation({ mutationFn: async (itemId: string) => { - const currentStash = stash || { items: [], updatedAt: 0 }; + const currentStash = await fetchLatestStash(); const newStash: Stash = { items: currentStash.items.filter(i => i.id !== itemId), updatedAt: Math.floor(Date.now() / 1000), diff --git a/src/pages/StashPage.tsx b/src/pages/StashPage.tsx index 2c90c1e..98ada66 100644 --- a/src/pages/StashPage.tsx +++ b/src/pages/StashPage.tsx @@ -11,6 +11,16 @@ import { Textarea } from '@/components/ui/textarea'; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; import { Switch } from '@/components/ui/switch'; import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogTrigger, DialogFooter, DialogClose, DialogDescription } from '@/components/ui/dialog'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '@/components/ui/alert-dialog'; import { Skeleton } from '@/components/ui/skeleton'; import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { LoginArea } from '@/components/auth/LoginArea'; @@ -638,8 +648,21 @@ export default function StashPage() { }); }; - // Remove: delete from stash, and clean up any public event. + // Remove: confirm first (deleting gear is destructive and, with Amber, costs a + // signer prompt), then delete from the encrypted stash and clean up any public + // event. `handleRemoveItem` just opens the confirm; `confirmRemoveItem` commits. + const [pendingDeleteId, setPendingDeleteId] = useState(null); + const pendingDeleteItem = pendingDeleteId + ? items.find((i) => i.id === pendingDeleteId) ?? null + : null; + const handleRemoveItem = (itemId: string) => { + setPendingDeleteId(itemId); + }; + + const confirmRemoveItem = () => { + if (!pendingDeleteId) return; + const itemId = pendingDeleteId; const target = items.find((i) => i.id === itemId); removeItem.mutate(itemId, { onSuccess: () => { @@ -648,6 +671,7 @@ export default function StashPage() { } }, }); + setPendingDeleteId(null); }; const [searchQuery, setSearchQuery] = useState(''); const [filterCategory, setFilterCategory] = useState('all'); @@ -1126,6 +1150,30 @@ export default function StashPage() { {listingItem && ( !open && setListingItem(null)} onSelectSpot={handleSpotSelection} /> )} + + {/* Delete confirmation — deleting gear is destructive (and re-encrypts + + signs, so it costs an Amber prompt); confirm before committing. */} + !open && setPendingDeleteId(null)}> + + + Remove from your stash? + + {pendingDeleteItem + ? <>“{pendingDeleteItem.title}” will be removed from your stash{pendingDeleteItem.visibility === 'public' ? ' and unlisted from your public showcase' : ''}. This can’t be undone. + : 'This item will be removed from your stash. This can’t be undone.'} + + + + Cancel + + Remove + + + + ); }