From b6b3931eccc3cd4d24720494b14e5c19fc232c09 Mon Sep 17 00:00:00 2001 From: "shakespeare.diy" Date: Tue, 8 Sep 2026 00:02:30 +0700 Subject: [PATCH] Crew audit: fix member-clobber on rapid roster writes (1.2.37) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audited the whole crew surface. The trust model is genuinely solid: membership is double-sided and unforgeable (founder p-tags + the member's own NIP-32 kind-1985 attestation, cross-referenced in useCollectiveMembers), roles are founder-controlled via the crew's addressable event (useCrewRole), moderation only honours labels authored by listed moderators, and the invite/join/endorse flow is symmetric with proper author validation and batched, deduped queries throughout. One real bug found — a stale-snapshot member clobber. The crew roster is a single addressable event rewritten wholesale on every member add, but three write sites rebuilt it from the CACHED crew/profile snapshot without re-fetching first: • useApproveJoinRequest (founder "Add to crew") • useSendCrewInvite (founder pre-list on invite) • useApproveCollectiveJoinRequest (Lab collective owner add) So approving two join-requests back to back rebuilt the second write from the pre-first-write roster and silently DROPPED the member added by the first (addressable events replace by coordinate). This is exactly the fetch-before-write discipline already applied to spots/events/shops/ contacts/stash — the crew/collective roster writes were the ones missed. Fix: added fetchLatestCrewEvent() (and an inline equivalent for Lab profiles) that re-queries the freshest roster event by coordinate and merges the new member into THAT, never regressing to an older revision, falling back to the passed-in snapshot only on a relay hiccup so an add still goes through. Founder-only / owner-only authorization and the membership-attestation trust model are unchanged. Member removal / admin promotion go through the founder's own profile editor (a deliberate full-roster overwrite by the sole owner), so they aren't concurrent-merge scenarios and are left as-is. Co-authored-by: shakespeare.diy --- package.json | 2 +- src/hooks/useCollectiveJoinRequests.ts | 34 ++++++++++++--- src/hooks/useCrewInvites.ts | 59 +++++++++++++++++++++++--- 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index ddcfb43..b75227e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "skate-spots", "private": true, - "version": "1.2.36", + "version": "1.2.37", "type": "module", "homepage": "https://skate-spots.com", "scripts": { diff --git a/src/hooks/useCollectiveJoinRequests.ts b/src/hooks/useCollectiveJoinRequests.ts index d4c9250..3cd7e9f 100644 --- a/src/hooks/useCollectiveJoinRequests.ts +++ b/src/hooks/useCollectiveJoinRequests.ts @@ -109,18 +109,42 @@ export function useApproveCollectiveJoinRequest() { throw new Error('Only the collective owner can add members.'); } - const already = profile.members.some( - (m) => m.toLowerCase() === request.requester.toLowerCase(), + // Re-fetch the freshest profile before rewriting its roster, so approving + // multiple requests back to back can't rebuild from a stale snapshot and + // drop an already-added member (the whole addressable event is rewritten + // on every add). Fall back to the passed-in snapshot on a relay hiccup. + let baseEvent = profile.event; + try { + const fresh = await nostr.query( + [{ + kinds: [profile.event.kind], + authors: [profile.pubkey], + '#d': [profile.identifier], + limit: 1, + }], + { signal: AbortSignal.timeout(8000) }, + ); + let latest = baseEvent; + for (const ev of fresh) { + if (ev.created_at > latest.created_at) latest = ev; + } + baseEvent = latest; + } catch { + // keep snapshot + } + + const already = baseEvent.tags.some( + ([n, v]) => n === 'p' && v?.toLowerCase() === request.requester.toLowerCase(), ); if (!already) { // Clone the profile event, drop any stale p for this pubkey, append it. - const tags = profile.event.tags.filter( + const tags = baseEvent.tags.filter( ([n, v]) => !(n === 'p' && v?.toLowerCase() === request.requester.toLowerCase()), ); tags.push(['p', request.requester]); const signed = await user.signer.signEvent({ - kind: profile.event.kind, - content: profile.event.content, + kind: baseEvent.kind, + content: baseEvent.content, created_at: Math.floor(Date.now() / 1000), tags, }); diff --git a/src/hooks/useCrewInvites.ts b/src/hooks/useCrewInvites.ts index 8b91934..8e54db8 100644 --- a/src/hooks/useCrewInvites.ts +++ b/src/hooks/useCrewInvites.ts @@ -89,9 +89,14 @@ export function useSendCrewInvite() { // 2. Founder optimistically pre-lists the invitee so accept = instant confirm. if (isFounder && user.pubkey.toLowerCase() === crew.pubkey.toLowerCase()) { - const already = crew.members.some((m) => m.toLowerCase() === inviteePk); + // Re-fetch the freshest roster so a rapid second invite/approval can't + // rebuild from a stale snapshot and drop a just-added member. + const latest = await fetchLatestCrewEvent(nostr, crew); + const already = latest.tags.some( + ([n, v]) => n === 'p' && v?.toLowerCase() === inviteePk, + ); if (!already) { - const updated = republishCrewWithMember(crew.event, inviteePk, role); + const updated = republishCrewWithMember(latest, inviteePk, role); const signed = await user.signer.signEvent(updated); await nostr.event(signed, { signal: AbortSignal.timeout(15000) }); } @@ -130,6 +135,46 @@ function republishCrewWithMember( }; } +/** + * Fetch the FRESHEST crew event for a coordinate right before a roster write. + * + * The crew roster is a single addressable event that we rewrite wholesale on + * every member add. Basing that rewrite on a cached `crew.event` snapshot is + * unsafe: approving two join-requests (or pre-listing two invitees) back to + * back would rebuild the second write from the pre-first-write snapshot and + * silently DROP the member added by the first — the same stale-snapshot clobber + * we guard against in spots/events/shops. So we always re-fetch + merge first. + * + * Falls back to the provided snapshot if the relay returns nothing (offline / + * hiccup) so an add still goes through rather than throwing. + */ +async function fetchLatestCrewEvent( + nostr: ReturnType['nostr'], + crew: SkateCrew, +): Promise { + try { + const events = await nostr.query( + [{ + kinds: [SKATE_CREW_KIND], + authors: [crew.pubkey], + '#d': [crew.identifier], + limit: 1, + }], + { signal: AbortSignal.timeout(8000) }, + ); + // Newest wins (relays may hold multiple revisions). + let latest: NostrEvent | undefined; + for (const ev of events) { + if (!latest || ev.created_at > latest.created_at) latest = ev; + } + // Only trust a fresher-or-equal event; never regress to an older revision. + if (latest && latest.created_at >= crew.event.created_at) return latest; + } catch { + // fall through to snapshot + } + return crew.event; +} + // ─── Reading invites addressed to me ───────────────────────────────────────── /** @@ -329,11 +374,15 @@ export function useApproveJoinRequest() { if (user.pubkey.toLowerCase() !== crew.pubkey.toLowerCase()) { throw new Error('Only the crew founder can add members.'); } - const alreadyMember = crew.members.some( - (m) => m.toLowerCase() === request.requester.toLowerCase(), + // Re-fetch the freshest roster before rewriting it, so approving several + // requests in quick succession can't clobber earlier additions (each + // approval rewrites the whole addressable roster event). + const latest = await fetchLatestCrewEvent(nostr, crew); + const alreadyMember = latest.tags.some( + ([n, v]) => n === 'p' && v?.toLowerCase() === request.requester.toLowerCase(), ); if (!alreadyMember) { - const updated = republishCrewWithMember(crew.event, request.requester, 'member'); + const updated = republishCrewWithMember(latest, request.requester, 'member'); const signed = await user.signer.signEvent(updated); await nostr.event(signed, { signal: AbortSignal.timeout(15000) }); }