diff --git a/docs/systems/dm-system.md b/docs/systems/dm-system.md index 9c75657a..f4005e39 100644 --- a/docs/systems/dm-system.md +++ b/docs/systems/dm-system.md @@ -426,6 +426,8 @@ A Deleted-User 1-on-1 is a **read-only archive** — you can never message a tom - **Helper:** `permissions.ts:isDeadOneOnOne(dmChannelId, requesterId)` → `true` when the channel is 1-on-1 (`ownerId IS NULL`) **and** every member other than the requester has `isDeleted = 1` (returns `false` for groups and when there are no other members). - **Applied to all three message-mutation endpoints** in `dm.ts`: `POST /api/dm/:id/messages` (after the `isDmMember` gate), `PATCH /api/dm/messages/:id`, and `DELETE /api/dm/messages/:id`. Each rejects with **`403 { error: "This user's account was deleted", code: 'recipient_deleted', statusCode: 403 }`**. +- **Applied to DM reactions on the WebSocket path** in `ws/events.ts`: `handleReactionAdd` and `handleReactionRemove` call `isDeadOneOnOne(dmMsg.dmChannelId, userId)` after the `isDmMember` gate and **silently drop** the frame (WS has no response channel). Without this, a survivor could add/remove reactions on historical messages and the reaction would relay to **all** active peers (a 1-on-1 has no group target-origins, so `queueOutboxEvent(..., undefined)` fans out) — the exact mis-directed relay the read-only invariant exists to prevent. +- **Client mirror (consistency, not the boundary):** `components/chat/Message.tsx` withdraws the add-reaction affordances (hover button, emoji picker, context-menu "Add Reaction") and no-ops existing-pill toggles when the message's DM is a dead 1-on-1 (`isDeletedPartnerDm(dm, currentUser)`). Existing reactions still **display** read-only; only add/remove is disabled. ### Live update on the heal path diff --git a/packages/server/src/utils/userDeletion.dmMembership.test.ts b/packages/server/src/utils/userDeletion.dmMembership.test.ts index 9de70378..424e001f 100644 --- a/packages/server/src/utils/userDeletion.dmMembership.test.ts +++ b/packages/server/src/utils/userDeletion.dmMembership.test.ts @@ -33,12 +33,19 @@ function seedDm(id: string, ownerId: string | null) { function seedMember(dmChannelId: string, userId: string) { testDb.insert(schema.dmMembers).values({ dmChannelId, userId, closed: 0 }).run(); } +function seedDmMessage(id: string, dmChannelId: string, userId: string) { + testDb.insert(schema.dmMessages).values({ id, dmChannelId, userId, content: 'x', createdAt: Date.now() }).run(); +} describe('tombstoneUser — DM membership partition', () => { beforeEach(() => { sqlite = new Database(':memory:'); testDb = drizzle(sqlite, { schema }); applyMigrations(sqlite); + // Match production (db/index.ts:31): FK enforcement ON. better-sqlite3 defaults + // this OFF, which would silently skip the dm_channels→dm_members/dm_messages + // cascade and let orphaned rows survive a purge unnoticed. + sqlite.pragma('foreign_keys = ON'); vi.clearAllMocks(); }); afterEach(() => sqlite.close()); @@ -65,10 +72,32 @@ describe('tombstoneUser — DM membership partition', () => { seedDm('dm_live', null); seedMember('dm_live', 'victim'); seedMember('dm_live', 'survivor'); // Both-dead thread — victim + an already-deleted partner → must be purged seedDm('dm_dead', null); seedMember('dm_dead', 'victim'); seedMember('dm_dead', 'alreadyDead'); + seedDmMessage('msg_dead', 'dm_dead', 'victim'); tombstoneUser('victim', { purgeContent: false }); expect(testDb.select().from(schema.dmChannels).where(eq(schema.dmChannels.id, 'dm_live')).get()).toBeTruthy(); expect(testDb.select().from(schema.dmChannels).where(eq(schema.dmChannels.id, 'dm_dead')).get()).toBeUndefined(); + // Cascade cleanup: with FK ON, dropping the dm_channels row must also remove its + // dm_members and dm_messages — no orphaned rows may linger. + expect(testDb.select().from(schema.dmMembers).where(eq(schema.dmMembers.dmChannelId, 'dm_dead')).all()).toEqual([]); + expect(testDb.select().from(schema.dmMessages).where(eq(schema.dmMessages.dmChannelId, 'dm_dead')).all()).toEqual([]); + }); + + it('transfers owned group DM to a LIVE member, never a tombstoned one', async () => { + const { tombstoneUser } = await import('./userDeletion.js'); + seedUser('owner'); seedUser('deadmate', { isDeleted: 1 }); seedUser('livemate'); + // owner owns a group DM whose other members are one dead + one live. + // The membership rows are ordered so the dead member would be picked first + // by an unfiltered `LIMIT 1` — proving the isDeleted=0 guard is what selects livemate. + seedDm('dm_owned', 'owner'); + seedMember('dm_owned', 'owner'); + seedMember('dm_owned', 'deadmate'); + seedMember('dm_owned', 'livemate'); + + tombstoneUser('owner', { purgeContent: false }); + + const channel = testDb.select().from(schema.dmChannels).where(eq(schema.dmChannels.id, 'dm_owned')).get(); + expect(channel?.ownerId).toBe('livemate'); }); }); diff --git a/packages/server/src/utils/userDeletion.ts b/packages/server/src/utils/userDeletion.ts index 3d050a7b..5ec891e7 100644 --- a/packages/server/src/utils/userDeletion.ts +++ b/packages/server/src/utils/userDeletion.ts @@ -170,11 +170,17 @@ export function tombstoneUser(uid: string, options?: TombstoneOptions): string[] and(eq(schema.channelOverrides.targetType, 'member'), eq(schema.channelOverrides.targetId, uid)) ).run(); - // Transfer ownership of group DMs to the next remaining member + // Transfer ownership of group DMs to the next remaining member. + // Invariant: ownership must never transfer to a tombstoned member — join + // users and require isDeleted=0 so a dead incarnation can't become owner. for (const { id: dmId } of ownedGroupDms) { const nextMember = tx.select({ userId: schema.dmMembers.userId }) .from(schema.dmMembers) - .where(eq(schema.dmMembers.dmChannelId, dmId)) + .innerJoin(schema.users, eq(schema.dmMembers.userId, schema.users.id)) + .where(and( + eq(schema.dmMembers.dmChannelId, dmId), + eq(schema.users.isDeleted, 0), + )) .limit(1) .get(); if (nextMember) { diff --git a/packages/server/src/ws/events.ts b/packages/server/src/ws/events.ts index 8212a08e..54a1765d 100644 --- a/packages/server/src/ws/events.ts +++ b/packages/server/src/ws/events.ts @@ -4,7 +4,7 @@ import { getDb, schema } from '../db/index.js'; import { generateSnowflake } from '../utils/snowflake.js'; import { connectionManager } from './handler.js'; import type { VoiceRoom, DmRoomMeta, SpaceRoomMeta } from './handler.js'; -import { isMember, getChannelSpaceId, isDmMember, hasPermission, computePermissions, PermissionBits } from '../utils/permissions.js'; +import { isMember, getChannelSpaceId, isDmMember, isDeadOneOnOne, hasPermission, computePermissions, PermissionBits } from '../utils/permissions.js'; import { broadcastDmMessage, getDmMessageWithUser } from '../routes/dm.js'; import { MAX_MESSAGE_LENGTH, type MessageWithUser, type Attachment, type DmMessageWithUser, type Embed, type Activity, type ActivityType, type ActivityTimestamps, type ActivityAssets, type ServerEvent, type DmCallUndeliverableFailure, type DmCallUndeliverableReason } from '@backspace/shared'; import type { CallRelayResult, CallFanoutFailure } from '../utils/federationOutbox.js'; @@ -1138,6 +1138,9 @@ function handleReactionAdd(event: Record, userId: string, isFed if (isFederated) return; const dmMsg = db.select().from(schema.dmMessages).where(eq(schema.dmMessages.id, messageId)).get(); if (!dmMsg || !isDmMember(dmMsg.dmChannelId, userId)) return; + // Read-only enforcement: a dead 1-on-1 thread (partner tombstoned) accepts no + // reaction mutations — the relay would fan out to all peers via undefined origins. + if (isDeadOneOnOne(dmMsg.dmChannelId, userId)) return; const reactionId = generateSnowflake(); const now = Date.now(); @@ -1224,6 +1227,9 @@ function handleReactionRemove(event: Record, userId: string, is if (isFederated) return; const dmMsg = db.select().from(schema.dmMessages).where(eq(schema.dmMessages.id, messageId)).get(); if (!dmMsg || !isDmMember(dmMsg.dmChannelId, userId)) return; + // Read-only enforcement: a dead 1-on-1 thread (partner tombstoned) accepts no + // reaction mutations — the relay would fan out to all peers via undefined origins. + if (isDeadOneOnOne(dmMsg.dmChannelId, userId)) return; const result = db.delete(schema.dmReactions) .where(and( diff --git a/packages/server/test/federation-identity-deletion.test.ts b/packages/server/test/federation-identity-deletion.test.ts index 957bf6c4..2868ed3a 100644 --- a/packages/server/test/federation-identity-deletion.test.ts +++ b/packages/server/test/federation-identity-deletion.test.ts @@ -451,6 +451,50 @@ describe('Federation identity deletion — server suite', () => { expect((await remove.json()).code).toBe('recipient_deleted'); }); + it('#20 read-only: WS reaction_add on a Deleted-User 1-on-1 is silently dropped, not persisted', async () => { + const fx = await setupFullDeletionFixture('t20'); + const { openInspector } = await import('./helpers/dbInspect.js'); + + // Survivor authors a message BEFORE the deletion so there is a message living + // in the dead thread for the survivor to (attempt to) react on. + const pre = await fetch(`${harness.remote.origin}/api/dm/${fx.dmChannelId}/messages`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${fx.observerOnRemote.token}` }, + body: JSON.stringify({ content: 'before deletion' }), + }); + expect(pre.status).toBe(201); + const { id: messageId } = await pre.json() as { id: string }; + + // Tombstone the remote (federated) user via soft delete → thread becomes dead 1-on-1. + const del = await fetch(`${harness.home.origin}/api/users/@me/federation-identity/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${fx.homeUser.token}` }, + body: JSON.stringify({ origins: [harness.remote.origin], mode: 'soft' }), + }); + expect(del.status).toBe(200); + + // Sanity: no reaction by the survivor yet. + const before = openInspector(harness.remote); + expect(before.dmReactionsForUser(fx.observerOnRemote.id).filter(r => r.dmMessageId === messageId)).toEqual([]); + before.close(); + + // Survivor opens a WS and attempts to react on the message in the dead thread. + const ws = await connectWs(harness.remote.origin, fx.observerOnRemote.token); + try { + ws.send({ type: 'reaction_add', messageId, emoji: '🔥' }); + // The handler is synchronous after the frame arrives; 400ms covers transit + + // any (rejected) insert attempt. There is no S→C ack for a dropped reaction. + await new Promise(r => setTimeout(r, 400)); + } finally { + ws.close(); + } + + // Assert: NO dm_reactions row was persisted for the survivor on that message. + const after = openInspector(harness.remote); + expect(after.dmReactionsForUser(fx.observerOnRemote.id).filter(r => r.dmMessageId === messageId)).toEqual([]); + after.close(); + }); + it('#7 owned-spaces 409: ownership prevents deletion, registry preserved', async () => { const { createFederatedUser } = await import('./helpers/testUsers.js'); const { openInspector } = await import('./helpers/dbInspect.js'); diff --git a/packages/web/src/components/chat/Message.tsx b/packages/web/src/components/chat/Message.tsx index a346ddfa..1ba136bf 100644 --- a/packages/web/src/components/chat/Message.tsx +++ b/packages/web/src/components/chat/Message.tsx @@ -16,6 +16,7 @@ import { EmbedRenderer } from './EmbedRenderer'; import { Username } from '../ui/Username'; import { EmojiPicker } from './EmojiPicker'; import { hasPermissionBit, PermissionBits } from '../../utils/permissions'; +import { isDeletedPartnerDm } from '../../utils/dmFormatters'; import { isSelf, resolveDisplayIdentity } from '../../utils/identity'; import { useCanonicalUserView } from '../../utils/userViewLookup'; import { @@ -161,10 +162,21 @@ export function Message({ message, isCompact, isFirstInGroup, previousMessageId const isDmMessage = isPendingMessage(message) ? !!message.dmChannelId || !message.channelId : !!(message as MessageWithUser & { dmChannelId?: string }).dmChannelId || !message.channelId; + const dmChannelId = isPendingMessage(message) + ? message.dmChannelId + : (message as MessageWithUser & { dmChannelId?: string }).dmChannelId; + const dmChannels = useSpaceStore((s) => s.dmChannels); + // Read-only enforcement (client mirror of the server guard): a dead 1-on-1 DM + // (partner tombstoned) accepts no reaction mutations. Existing reactions still + // DISPLAY, but the add/toggle affordances are withdrawn since the server drops them. + const isDeadDmThread = !!dmChannelId && (() => { + const dm = dmChannels.find(d => d.id === dmChannelId); + return dm ? isDeletedPartnerDm(dm, currentUser) : false; + })(); const canManageMessages = hasPermissionBit(myChPerms, PermissionBits.MANAGE_MESSAGES); const canSendMessages = isDmMessage || hasPermissionBit(myChPerms, PermissionBits.SEND_MESSAGES); const canDelete = isAuthor || canManageMessages; - const canAddReactions = isDmMessage || hasPermissionBit(myChPerms, PermissionBits.ADD_REACTIONS); + const canAddReactions = (isDmMessage || hasPermissionBit(myChPerms, PermissionBits.ADD_REACTIONS)) && !isDeadDmThread; const addReaction = useChatStore((s) => s.addReaction); const removeReaction = useChatStore((s) => s.removeReaction); @@ -181,6 +193,8 @@ export function Message({ message, isCompact, isFirstInGroup, previousMessageId r.user ? isSelf(r.user, currentUser) : r.userId === currentUser?.id; const toggleReaction = (emoji: string) => { + // Read-only: a dead 1-on-1 DM accepts no reaction mutations (add OR remove). + if (isDeadDmThread) return; const hasReacted = message.reactions?.some(r => isOwnReaction(r) && r.emoji === emoji); if (hasReacted) { removeReaction(message.id, emoji);