feat: durable account-deletion barrier; defer destruction on unconfirmed quiesce

B2 + B9. A one-shot disable scan can never close the create race: updateMany only
touches rows that exist at that instant, and there is always a "later". What closes it
is a STANDING predicate on the parent that every admission path consults, which is why
the barrier lives on User and not on the schedule rows (the race is about new children
appearing, and a flag on children cannot govern children that do not exist yet).

Barrier (packages/data-schemas/src/methods/user.ts):
- markUserDeleting is ONE-WAY and monotonic: the timestamp is stamped only when absent,
  so concurrent/repeated deletion requests agree on one value and there is no un-delete
  or ABA race. It invalidates the auth user-doc cache, without which the barrier would
  only be as strong as the shortest cache TTL.
- isUserDeleting FAILS CLOSED: an unresolvable lookup or a missing user reports true.
  Refusing work for a live user is recoverable (the caller retries); admitting work for
  a deleting one is not.
- getUsersPendingDeletion turns an unfinished cascade into a resumable work list rather
  than a one-shot that silently leaves half-deleted accounts.

Ordering (UserController): the barrier is raised BEFORE quiescing, because quiescing is
the slow part and the whole drain window must already be refusing new work. Previously
quiesce ran first, leaving the entire drain unguarded.

B9 becomes enforceable once the barrier exists: quiesceUserSchedules now RETURNS whether
the drain was confirmed, and deletion no longer proceeds to destructive steps on an
unconfirmed drain. It responds 202 and leaves the barrier up, so nothing new accumulates
while a later pass finishes. (The specs caught this immediately: their mocks returned
undefined, which the fail-closed branch correctly treated as unconfirmed.)

Defense in depth, since cross-collection atomicity is not available on standalone Mongo:
admission (create/update/run-now handlers) shrinks the window from unbounded to roughly
one request, and the fire path re-checks the owner at the DISPATCH boundary before a
billed generation goes out.

Tests: barrier is one-way under 6 concurrent requests (single timestamp); fails closed
for an unknown user; unfinished cascades surface as a resumable work list.
This commit is contained in:
Danny Avila 2026-07-23 08:47:18 -04:00
parent 86255b8c7e
commit acf914d0da
12 changed files with 211 additions and 12 deletions

View file

@ -25,6 +25,7 @@ const { getMCPManager, getFlowStateManager, getMCPServersRegistry } = require('~
const { invalidateCachedTools } = require('~/server/services/Config/getCachedTools');
const { processDeleteRequest } = require('~/server/services/Files/process');
const { quiesceUserSchedules } = require('~/server/services/Schedules');
const { markUserDeleting } = require('~/models');
const { getAppConfig } = require('~/server/services/Config');
const { getLogStores } = require('~/cache');
const db = require('~/models');
@ -349,10 +350,35 @@ const deleteUserController = async (req, res) => {
// in-flight loopback runs, so a scheduled generation can't persist messages
// after the messages/conversations below are deleted. Best-effort — a failure
// here must not block account deletion (deleteSchedulesByUser still erases rows).
await quiesceUserSchedules(user.id).catch((error) =>
logger.error('[deleteUserController] Failed to quiesce scheduled chats', error),
// BARRIER FIRST, before anything slow. Quiescing takes time, and the whole drain
// window has to already be refusing new work: a one-shot disable scan cannot close
// the create race (a schedule created after the scan simply is not in it), so every
// scheduling admission consults this durable user-level flag instead. Raising it
// also invalidates the auth user-doc cache, without which the barrier would only be
// as strong as the shortest cache TTL.
await markUserDeleting(user.id).catch((error) =>
logger.error('[deleteUserController] Failed to raise the deletion barrier', error),
);
const quiesced = await quiesceUserSchedules(user.id).catch((error) => {
logger.error('[deleteUserController] Failed to quiesce scheduled chats', error);
return false;
});
if (!quiesced) {
// DEFER rather than destroy on an unconfirmed drain. A scheduled generation that
// could not be confirmed settled may still persist messages, and deleting now
// would let it resurrect data for a deleted account. The barrier is durable and
// stays up, so nothing new accumulates; `getUsersPendingDeletion` makes this a
// resumable work list for a later pass to finish.
logger.warn(
`[deleteUserController] Deferring destructive deletion for ${user.id}: scheduled runs ` +
'did not confirm settlement. The deletion barrier remains in place.',
);
return res.status(202).json({
message: 'Account deletion started; in-flight work is still settling.',
});
}
await db.deleteMessages({ user: user.id });
await db.deleteAllUserSessions({ userId: user.id });
await db.deleteTransactions({ user: user.id });

View file

@ -2,7 +2,7 @@ const mongoose = require('mongoose');
const { MongoMemoryServer } = require('mongodb-memory-server');
jest.mock('~/server/services/Schedules', () => ({
quiesceUserSchedules: jest.fn().mockResolvedValue(undefined),
quiesceUserSchedules: jest.fn().mockResolvedValue(true),
}));
jest.mock('@librechat/data-schemas', () => {
@ -21,6 +21,7 @@ jest.mock('@librechat/data-schemas', () => {
jest.mock('~/models', () => {
const _mongoose = require('mongoose');
return {
markUserDeleting: jest.fn().mockResolvedValue(new Date()),
deleteAllUserSessions: jest.fn().mockResolvedValue(undefined),
deleteAllSharedLinks: jest.fn().mockResolvedValue(undefined),
deleteAllAgentApiKeys: jest.fn().mockResolvedValue(undefined),

View file

@ -43,10 +43,11 @@ jest.mock('@librechat/api', () => ({
}));
jest.mock('~/server/services/Schedules', () => ({
quiesceUserSchedules: jest.fn().mockResolvedValue(undefined),
quiesceUserSchedules: jest.fn().mockResolvedValue(true),
}));
jest.mock('~/models', () => ({
markUserDeleting: jest.fn().mockResolvedValue(new Date()),
deleteAllUserSessions: (...args) => mockDeleteAllUserSessions(...args),
deleteAllSharedLinks: (...args) => mockDeleteAllSharedLinks(...args),
updateUserPlugins: (...args) => mockUpdateUserPlugins(...args),

View file

@ -64,6 +64,9 @@ const handlers = createSchedulesHandlers({
// Quiesce-then-erase delete: stops new claims, aborts in-flight loopback runs,
// and erases once drained (reconciler completes drain) so evidence is preserved.
deleteSchedule: deleteScheduleForOwner,
// Durable account-deletion barrier. A one-shot disable scan cannot close the
// create race, so every scheduling WRITE consults the user-level flag instead.
isUserDeleting: methods.isUserDeleting,
});
router.get('/', checkSchedulesAccess, handlers.listSchedules);

View file

@ -170,6 +170,8 @@ function makeDeps(
abortScheduledJob: async () => undefined,
clearReconciledJob: async () => undefined,
isJobStoreShared: () => true,
isOwnerDeleting: async () => false,
isGloballyDisabled: async () => false,
countActiveRunsGlobal: async () => methods.countActiveRuns(),
withGlobalCapacitySlot: (cap: number, claim: (slot: number) => Promise<unknown>) =>
withCapacitySlot(

View file

@ -264,6 +264,15 @@ export async function fireSchedule(
return { fired: false, skipped: 'disabled' as const };
}
// Account-deletion barrier, re-checked at the DISPATCH boundary. Admission (the
// create/update/run-now handlers) is the primary gate, but there is always a window
// between admission and persistence, so the owner is re-checked immediately before a
// billed generation is dispatched. Skips silently: the deletion cascade owns the row.
if (await deps.isOwnerDeleting(user.id)) {
await advance();
return { fired: false, skipped: 'user_deleting' as const };
}
// Re-check the owner's live schedule permission: a role that lost
// SCHEDULES access after the schedule was created must stop firing.
if (!(await deps.hasScheduleAccess(user))) {

View file

@ -24,6 +24,25 @@ export interface SchedulesHandlersDeps {
* runs, and erases once drained. Returns false when not found / already deleting.
*/
deleteSchedule: (id: string, userId: string) => Promise<boolean>;
/** Whether this user's account deletion has begun. Fail-closed (unknown == true). */
isUserDeleting: (userId: string) => Promise<boolean>;
}
/**
* Refuses a scheduling WRITE once the owner's account deletion has begun. A one-shot
* disable scan can never close this race (a create landing after the scan is simply not
* in it), so admission consults the durable user-level barrier instead. Fail-closed.
*/
async function rejectIfUserDeleting(
deps: SchedulesHandlersDeps,
userId: string,
res: Response,
): Promise<boolean> {
if (!(await deps.isUserDeleting(userId))) {
return false;
}
res.status(410).json({ error: 'This account is being deleted' });
return true;
}
/** Bounded attempts to clear the upload TTL on a schedule's attachments. */
@ -174,6 +193,9 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
return;
}
const user = requestUser(req);
if (await rejectIfUserDeleting(deps, user.id, res)) {
return;
}
const limits = await deps.getLimits(user);
if (!limits.enabled) {
res.status(403).json({ error: 'Scheduled chats are disabled' });
@ -239,6 +261,9 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
}
const { id } = req.params as { id: string };
const user = requestUser(req);
if (await rejectIfUserDeleting(deps, user.id, res)) {
return;
}
const existing = await deps.methods.getScheduleById(id, user.id);
if (existing == null) {
res.status(404).json({ error: 'Schedule not found' });
@ -328,6 +353,9 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
async function runScheduleNow(req: ServerRequest, res: Response): Promise<void> {
const { id } = req.params as { id: string };
if (await rejectIfUserDeleting(deps, requestUser(req).id, res)) {
return;
}
const schedule = await deps.methods.getScheduleById(id, requestUser(req).id);
if (schedule == null) {
res.status(404).json({ error: 'Schedule not found' });

View file

@ -30,6 +30,7 @@ function makeService(
findBalance: jest.fn(async () => null),
upsertBalance: jest.fn(async () => null),
resolveAgentFireAccess: jest.fn(async () => 'ok' as const),
isUserDeleting: jest.fn(async () => false),
} as unknown as SchedulesServiceDeps;
return createSchedulesService(deps);
}
@ -60,7 +61,9 @@ describe('quiesceUserSchedules drain wait', () => {
// Each poll waits one interval; advance twice so the loop observes the drain.
await jest.advanceTimersByTimeAsync(250);
await jest.advanceTimersByTimeAsync(250);
await expect(pending).resolves.toBeUndefined();
// The rows drained, but this harness has no job store so the aborts could not be
// CONFIRMED delivered — quiesce reports false and the caller must defer destruction.
await expect(pending).resolves.toBe(false);
// Initial read + at least one poll that observed a non-empty set + the empty one.
expect(getActive.mock.calls.length).toBeGreaterThanOrEqual(3);
@ -73,9 +76,10 @@ describe('quiesceUserSchedules drain wait', () => {
const service = makeService(getActive);
const pending = service.quiesceUserSchedules('user-1');
// Advance past the full bounded timeout; the loop must give up, not hang.
// Advance past the full bounded timeout; the loop must give up, not hang, and must
// report the drain as UNCONFIRMED so deletion defers rather than destroying.
await jest.advanceTimersByTimeAsync(10_000);
await expect(pending).resolves.toBeUndefined();
await expect(pending).resolves.toBe(false);
// It polled repeatedly (bounded by the deadline) and surfaced the un-drained runs.
expect(getActive.mock.calls.length).toBeGreaterThan(1);
@ -87,7 +91,9 @@ describe('quiesceUserSchedules drain wait', () => {
const getActive = jest.fn<Promise<ActiveRun[]>, [string]>().mockResolvedValue([]);
const service = makeService(getActive);
await expect(service.quiesceUserSchedules('user-1')).resolves.toBeUndefined();
// Nothing to abort and nothing to drain, so the quiesce is trivially CONFIRMED and
// the deletion cascade may proceed to its destructive steps.
await expect(service.quiesceUserSchedules('user-1')).resolves.toBe(true);
// Only the initial collection read; the drain loop is skipped for an empty set.
expect(getActive).toHaveBeenCalledTimes(1);
});

View file

@ -102,6 +102,8 @@ export interface SchedulesServiceDeps {
agentId: string,
user: ScheduleUserContext,
) => Promise<'ok' | 'missing' | 'forbidden'>;
/** Whether this user's account deletion has begun. Fail-closed (unknown == true). */
isUserDeleting: (userId: string) => Promise<boolean>;
}
export interface SchedulesService {
@ -154,8 +156,13 @@ export interface SchedulesService {
) => Promise<boolean>;
/** Soft-deletes an owner's schedule: stop claims, abort active runs, drain, erase. */
deleteScheduleForOwner: (scheduleId: string, userId: string) => Promise<boolean>;
/** Quiesces all of a user's schedules ahead of account deletion (stop + abort). */
quiesceUserSchedules: (userId: string) => Promise<void>;
/**
* Quiesces all of a user's schedules ahead of account deletion (stop + abort + drain).
* Returns whether the drain was CONFIRMED: false means at least one run could not be
* confirmed settled, and the caller must NOT proceed to destructive deletion the
* durable barrier keeps refusing new work while a later pass finishes the cascade.
*/
quiesceUserSchedules: (userId: string) => Promise<boolean>;
initializeScheduleEngine: (options?: {
clustered?: boolean;
}) => Promise<ReturnType<typeof startScheduleEngine> | undefined>;
@ -370,6 +377,7 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
// Counted in system scope so the cap is GLOBAL — a per-owner (tenant-scoped)
// count would let multiple tenants collectively exceed fireConcurrency.
countActiveRunsGlobal: () => runAsSystem(() => methods.countActiveRuns()),
isOwnerDeleting: (userId) => deps.isUserDeleting(userId),
isGloballyDisabled: async () => {
// Env first: an incident lever that must work even if the DB/config plane is the
// thing failing (a kill switch that needs a healthy DB is the one that fails when
@ -699,7 +707,7 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
* the loopback jobs of any in-flight runs, so a scheduled generation cannot keep
* persisting messages after the account's messages/conversations are deleted.
*/
async function quiesceUserSchedules(userId: string): Promise<void> {
async function quiesceUserSchedules(userId: string): Promise<boolean> {
await methods.disableUserSchedulesForDeletion(userId);
const active = await methods.getActiveRunsForUser(userId);
const unconfirmed: string[] = [];
@ -728,7 +736,8 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
// deployment without a shared stream store the run's generation may live on a
// peer worker and keep persisting for the now-deleted account. Known unshared-
// topology limitation (see the init warning); make it visible.
if (remaining > 0 || unconfirmed.length > 0) {
const confirmed = remaining === 0 && unconfirmed.length === 0;
if (!confirmed) {
logger.warn(
`[schedules] account-deletion quiesce did not confirm ${Math.max(remaining, unconfirmed.length)} ` +
`in-flight scheduled run(s) settled${unconfirmed.length ? ` [${unconfirmed.join(', ')}]` : ''} ` +
@ -736,6 +745,7 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
'shared stream store (USE_REDIS_STREAMS).',
);
}
return confirmed;
}
return {

View file

@ -105,6 +105,8 @@ export interface ScheduleEngineDeps {
* re-enable it. Checked once per engine tick, so the uncached read is negligible.
*/
isGloballyDisabled: () => Promise<boolean>;
/** Whether the run owner's account deletion has begun. Fail-closed (unknown == true). */
isOwnerDeleting: (userId: string) => Promise<boolean>;
/**
* Runs `claim` against the lowest free GLOBAL capacity slot, retrying the next slot
* when the unique partial index rejects a collision. Enforces fireConcurrency in the
@ -142,6 +144,7 @@ export interface FireResult {
| 'superseded'
| 'agent_deleted'
| 'user_missing'
| 'user_deleting'
| 'permission_revoked'
| 'disabled';
error?: string;

View file

@ -9,6 +9,7 @@ import type {
} from '~/types/schedule';
import type { ScheduleMethods } from './schedule';
import { createScheduleMethods } from './schedule';
import { createUserMethods } from './user';
import { createModels } from '../models';
jest.mock('~/config/winston', () => ({
@ -22,6 +23,7 @@ let mongoServer: MongoMemoryServer;
let Schedule: Model<IScheduleDocument>;
let ScheduleRun: Model<IScheduleRunDocument>;
let methods: ScheduleMethods;
let userMethods: ReturnType<typeof createUserMethods>;
beforeAll(async () => {
mongoServer = await MongoMemoryServer.create();
@ -32,6 +34,7 @@ beforeAll(async () => {
await Schedule.init();
await ScheduleRun.init();
methods = createScheduleMethods(mongoose);
userMethods = createUserMethods(mongoose);
});
afterAll(async () => {
@ -1046,3 +1049,41 @@ describe('deletion quiescing (soft-delete, drain, erase)', () => {
expect(b).not.toBe('limit');
});
});
describe('account-deletion barrier (delete vs create)', () => {
it('is one-way and monotonic under concurrent deletion requests', async () => {
const User = mongoose.models.User;
const user = await User.create({ email: `barrier-${Date.now()}@test.dev`, name: 'B' });
expect(await userMethods.isUserDeleting(user._id.toString())).toBe(false);
// Six concurrent deletion requests must agree on ONE timestamp: the barrier is
// stamped only when absent, so there is no un-delete race and no ABA window.
const stamps = await Promise.all(
Array.from({ length: 6 }, () => userMethods.markUserDeleting(user._id.toString())),
);
const unique = new Set(stamps.map((d) => d?.getTime()));
expect(unique.size).toBe(1);
expect(await userMethods.isUserDeleting(user._id.toString())).toBe(true);
});
it('fails CLOSED for an unknown user so admission never leaks', async () => {
// Refusing work for a live user is recoverable (the caller retries); admitting work
// for a deleting one is not, so an unresolvable lookup must report "deleting".
expect(await userMethods.isUserDeleting(new mongoose.Types.ObjectId().toString())).toBe(true);
});
it('surfaces unfinished cascades as a resumable work list', async () => {
const User = mongoose.models.User;
const pendingUser = await User.create({
email: `pending-${Date.now()}@test.dev`,
name: 'P',
});
await userMethods.markUserDeleting(pendingUser._id.toString());
// A deletion deferred on an unconfirmed quiesce (or crashed part-way) leaves the
// barrier up with the document still present — exactly what a later pass queries.
const pending = await userMethods.getUsersPendingDeletion(50);
expect(pending.some((u) => u._id.toString() === pendingUser._id.toString())).toBe(true);
});
});

View file

@ -116,6 +116,13 @@ export function createUserMethods(
getUserById: (userId: string, fieldsToSelect?: string | string[] | null) => Promise<IUser | null>;
generateToken: (user: IUser, expiresIn?: number) => Promise<string>;
deleteUserById: (userId: string) => Promise<UserDeleteResult>;
/** Raises the one-way account-deletion barrier (monotonic) and invalidates the
* auth user-doc cache. Returns the effective timestamp. */
markUserDeleting: (userId: string) => Promise<Date | null>;
/** Whether deletion has begun for this user. Fail-closed: unknown means true. */
isUserDeleting: (userId: string) => Promise<boolean>;
/** Users whose barrier is up but whose document still exists (unfinished cascades). */
getUsersPendingDeletion: (limit: number) => Promise<IUser[]>;
updateUserPlugins: (
userId: string,
plugins: string[] | undefined,
@ -294,6 +301,65 @@ export function createUserMethods(
}
}
/**
* Raises the durable account-deletion barrier. ONE-WAY and monotonic: the timestamp
* is stamped only when absent, so a repeated or concurrent deletion request never
* moves it and there is no un-delete race. Returns the effective timestamp.
*
* Must be called BEFORE quiescing anything: quiescing is the slow part, and the
* whole drain window has to already be refusing new work. The auth user-doc cache is
* invalidated here because the barrier is only as strong as the shortest cache TTL
* a request holding a stale user document would otherwise sail straight past it.
*/
async function markUserDeleting(userId: string): Promise<Date | null> {
const User = mongoose.models.User;
const updated = await User.findOneAndUpdate(
{ _id: userId, deletionRequestedAt: { $exists: false } },
{ $set: { deletionRequestedAt: new Date() } },
{ new: true },
).lean<IUser>();
await invalidateAuthUserDocCache(userId);
if (updated?.deletionRequestedAt != null) {
return updated.deletionRequestedAt;
}
// Already raised by a prior/concurrent request: report the existing timestamp
// rather than overwriting it, so the barrier stays monotonic.
const existing = await User.findById(userId)
.select('deletionRequestedAt')
.lean<Pick<IUser, 'deletionRequestedAt'>>();
return existing?.deletionRequestedAt ?? null;
}
/**
* Whether this user's account deletion has begun. FAIL-CLOSED: a lookup failure or a
* missing user reports `true`, because refusing work for a live user is recoverable
* (the caller retries) while admitting work for a deleting one is not.
*/
async function isUserDeleting(userId: string): Promise<boolean> {
const User = mongoose.models.User;
try {
const user = await User.findById(userId)
.select('deletionRequestedAt')
.lean<Pick<IUser, 'deletionRequestedAt'>>();
return user == null || user.deletionRequestedAt != null;
} catch {
return true;
}
}
/**
* Users whose deletion barrier is up but whose document still exists, i.e. cascades
* that never finished (deferred on an unconfirmed quiesce, or crashed part-way).
* Makes the destructive cascade a resumable work list instead of a one-shot.
*/
async function getUsersPendingDeletion(limit: number): Promise<IUser[]> {
const User = mongoose.models.User;
return User.find({ deletionRequestedAt: { $exists: true } })
.sort({ deletionRequestedAt: 1 })
.limit(limit)
.lean<IUser[]>();
}
/**
* Atomically records terms acceptance for a user.
* Sets termsAccepted and, only when no timestamp is already stored, stamps
@ -588,6 +654,9 @@ export function createUserMethods(
getUserById,
generateToken,
deleteUserById,
markUserDeleting,
isUserDeleting,
getUsersPendingDeletion,
updateUserPlugins,
toggleUserMemories,
};