Crew audit: fix member-clobber on rapid roster writes (1.2.37)

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 <assistant@shakespeare.diy>
This commit is contained in:
shakespeare.diy 2026-09-08 00:02:30 +07:00
parent 85cb76b458
commit b6b3931ecc
3 changed files with 84 additions and 11 deletions

View file

@ -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": {

View file

@ -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,
});

View file

@ -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<typeof useNostr>['nostr'],
crew: SkateCrew,
): Promise<NostrEvent> {
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) });
}