fix(dm): enforce read-only guard on DM reactions (WS) + harden purge/ownership tests
Finding 1: handleReactionAdd/Remove now drop reactions on a dead 1-on-1 (isDeadOneOnOne) — previously a survivor could react on a Deleted-User thread and the relay fanned out to all peers via undefined target-origins. Client Message.tsx withdraws add/toggle reaction affordances for dead DMs (existing reactions still display read-only). Finding 2: dmMembership purge test now runs with foreign_keys=ON (matches prod) and asserts dm_members/dm_messages cascade cleanup on channel purge. Finding 3: tombstone group-DM ownership transfer filters isDeleted=0 so ownership can never move to a tombstoned member; covered by a new test.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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<string, unknown>, 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<string, unknown>, 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(
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user