From e5a1cc95069b67d69e34ba02c289a1bfc055c9a8 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:32:04 +0200 Subject: [PATCH] fix(dm): keep 1-on-1 dm_members on tombstone, drop only group membership (S1) --- docs/systems/auth.md | 6 +- .../utils/userDeletion.dmMembership.test.ts | 59 +++++++++++++++++++ packages/server/src/utils/userDeletion.ts | 23 +++++++- .../test/federation-identity-deletion.test.ts | 14 +++-- 4 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 packages/server/src/utils/userDeletion.dmMembership.test.ts diff --git a/docs/systems/auth.md b/docs/systems/auth.md index 5ae84fad..7ba31688 100644 --- a/docs/systems/auth.md +++ b/docs/systems/auth.md @@ -403,7 +403,7 @@ All cleanup runs in a single SQLite transaction: - `memberRoles` -- removes all role assignments - `friends` -- removes all friendships (both directions) - `friendRequests` -- removes all friend requests (both directions) -- `dmMembers` -- removes from all DM channels +- `dmMembers` -- **partitioned**: the row is KEPT for 1-on-1 DMs (`dm_channels.ownerId IS NULL`) so the thread survives as a readable anonymized "Deleted User" thread; it is deleted only for group DMs (`ownerId IS NOT NULL`). The in-function `userDmChannelIds` local captures the user's DM channel ids before partitioning. - `readStates` -- removes all read state records - `reactions` -- removes all message reactions - `dmReactions` -- removes all DM reactions @@ -460,8 +460,8 @@ interface TombstoneOptions { purgeContent?: boolean } function tombstoneUser(uid: string, options?: TombstoneOptions): string[] ``` -- **`purgeContent: true`** (default / omitted): full tombstone — removes the user from spaces, friends, DM membership, and read-states; then also deletes `reactions`, `dmReactions`, and the user's space `messages` with their attachments and embeds. -- **`purgeContent: false`**: soft tombstone — removes the user from spaces, friends, DM membership (`dm_members`), and read-states. The `purgeContent: false` flag skips only `reactions`, `dm_reactions`, and the user's space `messages` (with attachments + embeds); DM membership cleanup and orphaned-DM purge always run in both modes (per `userDeletion.ts:121-126, 169-202`) because zero-member DM channels are unreachable garbage regardless of authorship retention. Used by the federation identity soft-delete endpoint so remote message history is retained. +- **`purgeContent: true`** (default / omitted): full tombstone — removes the user from spaces, friends, group DM membership, and read-states; then also deletes `reactions`, `dmReactions`, and the user's space `messages` with their attachments and embeds. 1-on-1 DM membership is kept (see below). +- **`purgeContent: false`**: soft tombstone — removes the user from spaces, friends, group DM membership, and read-states. The `purgeContent: false` flag skips only `reactions`, `dm_reactions`, and the user's space `messages` (with attachments + embeds). The DM membership partition and orphaned-DM purge always run in both modes: group-DM `dm_members` rows are deleted, 1-on-1 `dm_members` rows are KEPT (so the thread survives as an anonymized "Deleted User" thread), and any resulting zero-member DM channel is purged as unreachable garbage. Used by the federation identity soft-delete endpoint so remote message history is retained. ### `resolveOrCreateReplicatedUser` and Deleted Users diff --git a/packages/server/src/utils/userDeletion.dmMembership.test.ts b/packages/server/src/utils/userDeletion.dmMembership.test.ts new file mode 100644 index 00000000..1d5a01d6 --- /dev/null +++ b/packages/server/src/utils/userDeletion.dmMembership.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import Database from 'better-sqlite3'; +import { drizzle } from 'drizzle-orm/better-sqlite3'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import * as schema from '../db/schema.js'; +import { eq } from 'drizzle-orm'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +type TestDb = ReturnType>; +let sqlite: Database.Database; +let testDb: TestDb; + +vi.mock('../db/index.js', () => ({ getDb: () => testDb, getRawDb: () => sqlite, schema })); + +function applyMigrations(db: Database.Database): void { + const dir = path.resolve(__dirname, '../../drizzle'); + for (const f of fs.readdirSync(dir).filter(x => x.endsWith('.sql')).sort()) { + for (const stmt of fs.readFileSync(path.join(dir, f), 'utf8').split(/-->\s*statement-breakpoint/)) { + const clean = stmt.trim(); + if (clean) db.exec(clean); + } + } +} + +function seedUser(id: string, extra: Partial = {}) { + testDb.insert(schema.users).values({ id, username: id, passwordHash: 'x', createdAt: Date.now(), ...extra }).run(); +} +function seedDm(id: string, ownerId: string | null) { + testDb.insert(schema.dmChannels).values({ id, ownerId, createdAt: Date.now() }).run(); +} +function seedMember(dmChannelId: string, userId: string) { + testDb.insert(schema.dmMembers).values({ dmChannelId, userId, closed: 0 }).run(); +} + +describe('tombstoneUser — DM membership partition', () => { + beforeEach(() => { + sqlite = new Database(':memory:'); + testDb = drizzle(sqlite, { schema }); + applyMigrations(sqlite); + vi.clearAllMocks(); + }); + afterEach(() => sqlite.close()); + + it('keeps the deleted user 1-on-1 membership, removes group membership', async () => { + const { tombstoneUser } = await import('./userDeletion.js'); + seedUser('victim'); seedUser('survivor'); seedUser('groupmate'); + seedDm('dm_1on1', null); seedMember('dm_1on1', 'victim'); seedMember('dm_1on1', 'survivor'); + seedDm('dm_group', 'survivor'); seedMember('dm_group', 'victim'); seedMember('dm_group', 'survivor'); seedMember('dm_group', 'groupmate'); + + tombstoneUser('victim', { purgeContent: false }); + + const memberships = testDb.select().from(schema.dmMembers).where(eq(schema.dmMembers.userId, 'victim')).all(); + expect(memberships.map(m => m.dmChannelId)).toEqual(['dm_1on1']); // 1-on-1 kept, group removed + // Survivor's 1-on-1 channel still exists and still has the survivor + expect(testDb.select().from(schema.dmChannels).where(eq(schema.dmChannels.id, 'dm_1on1')).get()).toBeTruthy(); + }); +}); diff --git a/packages/server/src/utils/userDeletion.ts b/packages/server/src/utils/userDeletion.ts index a391b5f3..82811e66 100644 --- a/packages/server/src/utils/userDeletion.ts +++ b/packages/server/src/utils/userDeletion.ts @@ -1,5 +1,5 @@ import crypto from 'crypto'; -import { eq, or, and, inArray } from 'drizzle-orm'; +import { eq, or, and, inArray, isNotNull } from 'drizzle-orm'; import { getDb, schema } from '../db/index.js'; export interface DeletionBroadcastTargets { @@ -122,7 +122,26 @@ export function tombstoneUser(uid: string, options?: TombstoneOptions): string[] tx.delete(schema.memberRoles).where(eq(schema.memberRoles.userId, uid)).run(); tx.delete(schema.friends).where(or(eq(schema.friends.userId, uid), eq(schema.friends.friendId, uid))).run(); tx.delete(schema.friendRequests).where(or(eq(schema.friendRequests.fromId, uid), eq(schema.friendRequests.toId, uid))).run(); - tx.delete(schema.dmMembers).where(eq(schema.dmMembers.userId, uid)).run(); + // DM membership: keep the row for 1-on-1 DMs (ownerId NULL) so the thread + // survives as a readable "Deleted User" thread; drop it for group DMs. + const userDmChannelIds = tx.select({ dmChannelId: schema.dmMembers.dmChannelId }) + .from(schema.dmMembers) + .where(eq(schema.dmMembers.userId, uid)) + .all() + .map(r => r.dmChannelId); + if (userDmChannelIds.length > 0) { + const groupDmChannelIds = tx.select({ id: schema.dmChannels.id }) + .from(schema.dmChannels) + .where(and(inArray(schema.dmChannels.id, userDmChannelIds), isNotNull(schema.dmChannels.ownerId))) + .all() + .map(c => c.id); + if (groupDmChannelIds.length > 0) { + tx.delete(schema.dmMembers).where(and( + eq(schema.dmMembers.userId, uid), + inArray(schema.dmMembers.dmChannelId, groupDmChannelIds), + )).run(); + } + } tx.delete(schema.readStates).where(eq(schema.readStates.userId, uid)).run(); if (purge) { tx.delete(schema.reactions).where(eq(schema.reactions.userId, uid)).run(); diff --git a/packages/server/test/federation-identity-deletion.test.ts b/packages/server/test/federation-identity-deletion.test.ts index febe093f..c184f3d0 100644 --- a/packages/server/test/federation-identity-deletion.test.ts +++ b/packages/server/test/federation-identity-deletion.test.ts @@ -317,7 +317,7 @@ describe('Federation identity deletion — server suite', () => { homeInspect.close(); }); - it('#3 soft mode: tombstone shape, messages/reactions retained, dm membership cleared', async () => { + it('#3 soft mode: tombstone shape, messages/reactions retained, 1-on-1 dm membership kept', async () => { const fx = await setupFullDeletionFixture('t3'); const { openInspector } = await import('./helpers/dbInspect.js'); @@ -358,7 +358,8 @@ describe('Federation identity deletion — server suite', () => { expect(remote.spaceMembersForUser(fx.remoteUser.id)).toEqual([]); expect(remote.messagesAuthored(fx.remoteUser.id).length).toBe(2); // RETAINED expect(remote.reactionsForUser(fx.remoteUser.id).length).toBe(2); // RETAINED - expect(remote.dmMembership(fx.remoteUser.id)).toEqual([]); // dm_members always cleared + // 1-on-1 DM membership is KEPT (anonymized) so the thread survives as "Deleted User" + expect(remote.dmMembership(fx.remoteUser.id).map(r => r.dmChannelId)).toEqual([fx.dmChannelId]); expect(remote.dmChannelExists(fx.dmChannelId)).toBe(true); // other party still member remote.close(); @@ -394,7 +395,8 @@ describe('Federation identity deletion — server suite', () => { expect(remote.messagesAuthored(fx.remoteUser.id).length).toBe(0); // PURGED expect(remote.reactionsForUser(fx.remoteUser.id).length).toBe(0); // PURGED - expect(remote.dmMembership(fx.remoteUser.id)).toEqual([]); + // 1-on-1 DM membership is KEPT (anonymized) so the thread survives as "Deleted User" + expect(remote.dmMembership(fx.remoteUser.id).map(r => r.dmChannelId)).toEqual([fx.dmChannelId]); // 1-on-1 DM with another live participant SURVIVES (other party still member) expect(remote.dmChannelExists(fx.dmChannelId)).toBe(true); remote.close(); @@ -897,7 +899,8 @@ describe('Federation identity deletion — all-remotes fan-out', () => { expect(ins.spaceMembersForUser(fixtures[i].remoteUser.id)).toEqual([]); expect(ins.messagesAuthored(fixtures[i].remoteUser.id).length).toBe(2); // RETAINED expect(ins.reactionsForUser(fixtures[i].remoteUser.id).length).toBe(2); // RETAINED - expect(ins.dmMembership(fixtures[i].remoteUser.id)).toEqual([]); + // 1-on-1 DM membership is KEPT (anonymized) so the thread survives as "Deleted User" + expect(ins.dmMembership(fixtures[i].remoteUser.id).map(r => r.dmChannelId)).toEqual([fixtures[i].dmChannelId]); expect(ins.dmChannelExists(fixtures[i].dmChannelId)).toBe(true); ins.close(); }); @@ -958,7 +961,8 @@ describe('Federation identity deletion — all-remotes fan-out', () => { expect(after!.username).toBe(`!deleted:${fixtures[i].remoteUser.id}`); expect(ins.messagesAuthored(fixtures[i].remoteUser.id).length).toBe(0); // PURGED expect(ins.reactionsForUser(fixtures[i].remoteUser.id).length).toBe(0); // PURGED - expect(ins.dmMembership(fixtures[i].remoteUser.id)).toEqual([]); + // 1-on-1 DM membership is KEPT (anonymized) so the thread survives as "Deleted User" + expect(ins.dmMembership(fixtures[i].remoteUser.id).map(r => r.dmChannelId)).toEqual([fixtures[i].dmChannelId]); // Observer remains a member, so DM channel survives. expect(ins.dmChannelExists(fixtures[i].dmChannelId)).toBe(true); ins.close();