refactor(server): plan 4 — unify the two user-data-erasure enqueue paths

UserCleanupService.erasePluginUserData (the account-deletion transaction's
own UC2/UC3 enqueue) and PluginRuntimeService.enqueueUserErasure (the
emitUserDeleted sink's post-commit enqueue) independently walked the
identical SELECT id, permissions FROM plugins + JSON.parse + hook:user-data
filter + INSERT OR IGNORE — and both genuinely run for every account
deletion, since every emitUserDeleted call site (auth.service.ts,
admin.service.ts, trip-members.service.ts's guest removal) fires
immediately after deleteUserCompletely.

Extracted the shared filter into enqueueHookUserDataErasures
(nest/plugins/user-erasure-enqueue.ts) and pointed both call sites at it.
UC2/UC3 also come off DatabaseService onto PluginsRepository/
PluginUserErasureQueueRepository in the same change (they were the last
raw statements standing in the way of sharing one implementation) — this
narrows the Plan 3b Task 5 "stays raw, owned by Plan 3j" ruling to UC1
only (plugin_user_config/plugin_oauth_tokens/plugin_oauth_state), which
still stays raw, unaffected. UserCleanupService's orphan-data-directory
scan (a plugin uninstalled with data retained) has no equivalent on the
enqueueUserErasure side and stays there, now via the same repository.

Both enqueue calls still run per deletion (INSERT OR IGNORE keeps this
idempotent) — this unifies the IMPLEMENTATION, not the call count; not
touching the emitUserDeleted event-sink wiring itself was a deliberate,
smaller-blast-radius choice for this task.
This commit is contained in:
jubnl
2026-09-24 06:36:43 +02:00
parent c7df22ca7f
commit 40ede38ea7
16 changed files with 134 additions and 41 deletions
+9 -2
View File
@@ -17,6 +17,8 @@ import { JourneyContributors } from '../../db/entities/JourneyContributors.entit
import { ShareTokens } from '../../db/entities/ShareTokens.entity';
import { Trips } from '../../db/entities/Trips.entity';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import { Plugins } from '../../db/entities/Plugins.entity';
import { PluginUserErasureQueue } from '../../db/entities/PluginUserErasureQueue.entity';
import { TokensModule } from '../tokens/tokens.module';
import { AuthPublicController } from './auth-public.controller';
import { AuthController } from './auth.controller';
@@ -103,8 +105,13 @@ import { AllowedFileTypesModule } from '../files/allowed-file-types.module';
// TripMembers: Plan 4 Task 1, the SAME cross-domain shape as
// JourneyShareTokens/ShareTokens above — `UserCleanupService
// .cleanupUserReferences`'s UC4 erasure update (owned by
// `nest/trip-membership`).
MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens, Trips, TripMembers])],
// `nest/trip-membership`). Plugins/PluginUserErasureQueue: Plan 4 Task
// 8a, the SAME cross-domain shape again — `UserCleanupService
// .erasePluginUserData`'s UC2/UC3 erasure-enqueue half (owned by
// `nest/plugins`), narrowed off the Plan 3b Task 5 "stays raw" ruling to
// share `enqueueHookUserDataErasures` with `PluginRuntimeService
// .enqueueUserErasure` rather than re-implementing the same filter twice.
MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens, Trips, TripMembers, Plugins, PluginUserErasureQueue])],
controllers: [AuthPublicController, AuthController, PasskeyController],
providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp],
exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService],
+29 -13
View File
@@ -24,6 +24,11 @@ import { JourneyContributors } from '../../db/entities/JourneyContributors.entit
import type { JourneyContributorsRepository } from '../../db/repositories/JourneyContributors.repository';
import { ShareTokens } from '../../db/entities/ShareTokens.entity';
import type { ShareTokensRepository } from '../../db/repositories/ShareTokens.repository';
import { Plugins } from '../../db/entities/Plugins.entity';
import type { PluginsRepository } from '../../db/repositories/Plugins.repository';
import { PluginUserErasureQueue } from '../../db/entities/PluginUserErasureQueue.entity';
import type { PluginUserErasureQueueRepository } from '../../db/repositories/PluginUserErasureQueue.repository';
import { enqueueHookUserDataErasures } from '../plugins/user-erasure-enqueue';
/**
* Account erasure — everything that has to happen around `DELETE FROM users`
@@ -55,6 +60,10 @@ export class UserCleanupService {
@InjectRepository(JourneyContributors) private readonly journeyContributorsRepo: JourneyContributorsRepository,
// Plan 3h Task 6 (UC6, R10) — the genuinely-share GDPR-erasure delete.
@InjectRepository(ShareTokens) private readonly shareTokensRepo: ShareTokensRepository,
// Plan 4 Task 8a — UC2/UC3's erasure-enqueue half only (see erasePluginUserData's
// own docstring for why this narrows the Plan 3b Task 5 "stays raw" ruling).
@InjectRepository(Plugins) private readonly pluginsRepo: PluginsRepository,
@InjectRepository(PluginUserErasureQueue) private readonly pluginUserErasureQueueRepo: PluginUserErasureQueueRepository,
) {}
/**
@@ -68,27 +77,34 @@ export class UserCleanupService {
* Best-effort per table so a slimmed-down schema (some tests) can't fail the user
* deletion itself.
*
* UC1 (`plugin_user_config`/`plugin_oauth_tokens`/`plugin_oauth_state`),
* UC2 (`plugins`) and UC3 (`plugin_user_erasure_queue`) stay raw
* `DatabaseService` calls — every one of these tables is owned by
* UC1 (`plugin_user_config`/`plugin_oauth_tokens`/`plugin_oauth_state`)
* stays a raw `DatabaseService` call — that table trio is owned by
* `nest/plugins`, which lands in Plan 3j, not this plan (Plan 3b Task 5
* ruling; inventory §6 "the single largest 'stays raw' carve-out in Plan
* 3b"). Only `DELETE FROM users` below (UC11) is this domain's own and
* converts.
* 3b"), and nothing about this method's OTHER half needs it converted.
*
* UC2 (`plugins`) and UC3 (`plugin_user_erasure_queue`) — the erasure-
* ENQUEUE half — narrow that ruling (Plan 4 Task 8a): this method's queue
* writes were an independent re-implementation of the exact same
* `SELECT id, permissions FROM plugins` + `hook:user-data` filter +
* `INSERT OR IGNORE` that `PluginRuntimeService.enqueueUserErasure` (the
* `emitUserDeleted` sink's post-commit half — every account-deletion call
* site fires BOTH) already ran a second time for the very same user.
* Converted onto `PluginsRepository`/`PluginUserErasureQueueRepository` so
* both paths can share `enqueueHookUserDataErasures` (Plan 4 Task 8a) —
* ONE implementation of the filter, not two silently drifting copies. The
* orphan-directory scan below (a plugin uninstalled with its data
* retained) has no equivalent on the `enqueueUserErasure` side and is kept
* here, now via the same repository.
*/
async erasePluginUserData(userId: number): Promise<void> {
for (const table of ['plugin_user_config', 'plugin_oauth_tokens', 'plugin_oauth_state']) {
try { this.db.run(`DELETE FROM ${table} WHERE user_id = ?`, userId); } catch { /* table absent (slim schema) */ } // UC1 — Plan 3j
}
try {
const rows = this.db.all<{ id: string; permissions: string | null }>('SELECT id, permissions FROM plugins'); // UC2 — Plan 3j
const rows = await this.pluginsRepo.listIdsAndPermissions(); // UC2 — Plan 4 Task 8a
const installed = new Set(rows.map((r) => r.id));
const insert = this.db.prepare('INSERT OR IGNORE INTO plugin_user_erasure_queue (plugin_id, user_id) VALUES (?, ?)'); // UC3 — Plan 3j
for (const r of rows) {
let perms: unknown;
try { perms = JSON.parse(r.permissions ?? '[]'); } catch { perms = []; }
if (Array.isArray(perms) && perms.includes('hook:user-data')) insert.run(r.id, userId);
}
await enqueueHookUserDataErasures(this.pluginUserErasureQueueRepo, rows, userId); // UC3 — Plan 4 Task 8a
// Also enqueue for plugins UNINSTALLED with their data retained (deleteData=false):
// their data dir still holds the user's rows and a same-id reinstall would re-adopt
// them. No permissions record survives uninstall, so we can't check hook:user-data —
@@ -96,7 +112,7 @@ export class UserCleanupService {
// that id is reinstalled + active (erasure delivery is a duty, not grant-gated).
try {
for (const entry of fs.readdirSync(pluginsDataRoot(), { withFileTypes: true })) {
if (entry.isDirectory() && !installed.has(entry.name)) insert.run(entry.name, userId);
if (entry.isDirectory() && !installed.has(entry.name)) await this.pluginUserErasureQueueRepo.insertIgnore(entry.name, userId);
}
} catch { /* no plugin data root yet */ }
} catch { /* plugins / queue table absent (slim schema) */ }
@@ -57,6 +57,7 @@ import { ForbiddenResource } from './host/rpc-host';
import { removePluginData } from './host/plugin-data.service';
import { isKnownPermission } from './protocol/envelope';
import { discoverPlugins, type DiscoveryRepos } from './install/discovery';
import { enqueueHookUserDataErasures } from './user-erasure-enqueue';
import { parseJsonText, parseManifest, parseMcpToolCapabilities } from './install/manifest';
import { scanForNativeBinaries } from './install/native-scan';
import { devLinkEnabled, DEV_LINK_SOURCE } from './dev-link';
@@ -502,11 +503,7 @@ export class PluginRuntimeService implements OnApplicationBootstrap, OnModuleDes
private async enqueueUserErasure(userId: number): Promise<void> {
try {
const rows = await this.plugins.listIdsAndPermissions();
for (const r of rows) {
let perms: unknown;
try { perms = JSON.parse(r.permissions ?? '[]'); } catch { perms = []; }
if (Array.isArray(perms) && perms.includes('hook:user-data')) await this.pluginUserErasureQueue.insertIgnore(r.id, userId);
}
await enqueueHookUserDataErasures(this.pluginUserErasureQueue, rows, userId); // Plan 4 Task 8a — shared with UserCleanupService.erasePluginUserData
} catch {
/* enqueue is best-effort; a later sweep reconciles from whatever landed */
}
@@ -0,0 +1,43 @@
import type { PluginIdPermissionRow } from '../../db/repositories/Plugins.repository';
import type { PluginUserErasureQueueRepository } from '../../db/repositories/PluginUserErasureQueue.repository';
/**
* Enqueue a durable GDPR erasure (`INSERT OR IGNORE`, idempotent) for every
* plugin in `rows` that holds `hook:user-data`. Shared primitive (Plan 4
* Task 8a, a 3j carry): before this, `UserCleanupService.erasePluginUserData`
* (the account-deletion transaction's own enqueue, UC2/UC3) and
* `PluginRuntimeService.enqueueUserErasure` (the `emitUserDeleted` sink's
* post-commit enqueue) walked the IDENTICAL `SELECT id, permissions FROM
* plugins` + `JSON.parse` + `hook:user-data` filter independently — and both
* genuinely run for every account deletion: `deleteUserCompletely` calls the
* first inside its own transaction, and `emitUserDeleted` fires the second
* immediately after, from all three account-deletion call sites
* (`auth.service.ts`, `admin.service.ts`, `trip-members.service.ts`'s guest
* removal). The duplication was live, not theoretical — CLAUDE.md's "single
* source of truth over manual synchronization".
*
* `rows` is caller-supplied, not fetched here:
* `UserCleanupService.erasePluginUserData`'s own orphan-directory scan (its
* second half, enqueuing for a plugin that was uninstalled with its data
* retained) needs the SAME `plugins` read to compute which plugin ids are
* still installed, so the caller fetches once (`PluginsRepository
* .listIdsAndPermissions()`) and this function never re-queries.
*
* Never throws itself — a malformed `permissions` JSON value is treated as
* `[]` and skipped, matching both legacy implementations exactly.
*/
export async function enqueueHookUserDataErasures(
queue: PluginUserErasureQueueRepository,
rows: readonly Pick<PluginIdPermissionRow, 'id' | 'permissions'>[],
userId: number,
): Promise<void> {
for (const r of rows) {
let perms: unknown;
try {
perms = JSON.parse(r.permissions ?? '[]');
} catch {
perms = [];
}
if (Array.isArray(perms) && perms.includes('hook:user-data')) await queue.insertIgnore(r.id, userId);
}
}