mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
fix: round nineteen: coordinator-backed shutdown gate, tombstone allowlist, login-boundary barrier recheck
- The fire dispatch gate observes the COORDINATOR's shutdown flag (isShutdownInProgress, set before the listener starts closing) in addition to the engine's own stop: the engine-local flag was set by a pre-drain task that runs after server.close() begins and after higher-priority tasks, so an active pass could still dispatch into a closing listener. - Erasure tombstones retain ONLY the replay-detection identity: the unset list is derived from the live schema paths (collapsed to top-level fields) minus an explicit identity allowlist, so optional fields like tools or cron — and any field added later — cannot survive into the tombstone. Spec asserts the tombstone's key set equals the allowlist exactly. - The local login strategy rechecks the durable deletion barrier at the successful-login boundary, sequenced after the slow bcrypt await: a barrier raised mid-comparison otherwise handed a stale user to loginController, whose session/token/balance writes recreated records the cascade had deleted.
This commit is contained in:
parent
d50d64e63d
commit
177a149a6b
5 changed files with 80 additions and 19 deletions
|
|
@ -73,6 +73,24 @@ async function passportLogin(req, email, password, done) {
|
|||
return done(null, user, { message: 'Email not verified.' });
|
||||
}
|
||||
|
||||
// FINAL barrier recheck at the successful-login boundary, sequenced after the
|
||||
// slow awaits above (bcrypt runs tens of milliseconds): a deletion barrier
|
||||
// raised mid-comparison would otherwise hand a stale user to loginController,
|
||||
// whose session/token/balance writes recreate records the cascade deleted.
|
||||
// The lookup at the top only covers barriers committed before it.
|
||||
let barrier = null;
|
||||
try {
|
||||
barrier = await findUser({ email: email.trim() }, 'deletionRequestedAt');
|
||||
} catch {
|
||||
barrier = null;
|
||||
}
|
||||
if (barrier == null || barrier.deletionRequestedAt != null) {
|
||||
logger.warn(
|
||||
`[Login] Refusing login for ${user._id}: deletion barrier raised or unverifiable`,
|
||||
);
|
||||
return done(null, false, { message: 'Account deletion in progress' });
|
||||
}
|
||||
|
||||
logger.info(`[Login] [Login successful] [Username: ${email}] [Request-IP: ${req.ip}]`);
|
||||
return done(null, user);
|
||||
} catch (err) {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,13 @@ export function registerShutdownTask(
|
|||
* safety net for long-lived connections such as SSE streams that may
|
||||
* not finish in time.
|
||||
*/
|
||||
/** Whether graceful shutdown has begun. Set BEFORE the listener starts closing, so
|
||||
* dispatch gates (e.g. the schedule engine's fire boundary) observe it ahead of any
|
||||
* pre-drain task ordering — an engine-local flag set by its own task ran too late. */
|
||||
export function isShutdownInProgress(): boolean {
|
||||
return isShuttingDown;
|
||||
}
|
||||
|
||||
export function setupGracefulShutdown(server: Server): void {
|
||||
httpServer = server;
|
||||
for (const signal of SIGNALS) {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
import { logger, runAsSystem } from '@librechat/data-schemas';
|
||||
import type { IScheduleRun } from '@librechat/data-schemas';
|
||||
import type { ScheduleEngineDeps, JobState } from './types';
|
||||
import { isShutdownInProgress, registerShutdownTask } from '~/app/shutdown';
|
||||
import { fireSchedule, BALANCE_SKIP_DISABLE_THRESHOLD } from './fire';
|
||||
import { registerShutdownTask } from '~/app/shutdown';
|
||||
import { computeNextRunAt } from './cadence';
|
||||
import { hasAbortInFlight } from './types';
|
||||
|
||||
|
|
@ -403,10 +403,11 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine {
|
|||
}
|
||||
try {
|
||||
const result = await fireSchedule(
|
||||
// The engine's stop flag reaches the dispatch boundary: a pass in flight
|
||||
// when shutdown begins releases its claim instead of POSTing at the
|
||||
// closing listener.
|
||||
{ ...deps, isShuttingDown: () => stopped },
|
||||
// The dispatch boundary observes shutdown from BOTH signals: the
|
||||
// coordinator flag flips before the listener starts closing (ahead of
|
||||
// any pre-drain task ordering), and the engine's own stop covers direct
|
||||
// runTick callers outside a coordinated shutdown.
|
||||
{ ...deps, isShuttingDown: () => stopped || isShutdownInProgress() },
|
||||
schedule,
|
||||
limits,
|
||||
scheduledFor,
|
||||
|
|
|
|||
|
|
@ -1567,8 +1567,31 @@ describe('erasure leaves an idempotency tombstone', () => {
|
|||
expect(tombstone?.erased).toBe(true);
|
||||
expect(tombstone?.deleting).toBe(true);
|
||||
expect(tombstone?.clientRequestDigest).toBe('digest-1');
|
||||
// ALLOWLIST semantics: everything except the replay-detection identity is gone,
|
||||
// derived from the live schema paths so fields added later cannot leak either.
|
||||
const contentKeys = Object.keys(tombstone ?? {}).filter(
|
||||
(key) =>
|
||||
![
|
||||
'_id',
|
||||
'__v',
|
||||
'id',
|
||||
'user',
|
||||
'tenantId',
|
||||
'clientRequestId',
|
||||
'clientRequestDigest',
|
||||
'deleting',
|
||||
'erased',
|
||||
'erasedAt',
|
||||
'enabled',
|
||||
'createdAt',
|
||||
'updatedAt',
|
||||
].includes(key),
|
||||
);
|
||||
expect(contentKeys).toEqual([]);
|
||||
expect(tombstone?.prompt).toBeUndefined();
|
||||
expect(tombstone?.name).toBeUndefined();
|
||||
expect(tombstone?.cadence).toBeUndefined();
|
||||
expect(tombstone?.timezone).toBeUndefined();
|
||||
|
||||
// Tombstones are inert to every sweep: re-erasing or re-deleting them forever
|
||||
// would pin the bounded windows.
|
||||
|
|
|
|||
|
|
@ -1589,6 +1589,24 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
|
|||
* let the worker then insert a ghost row against a gone schedule that it can no
|
||||
* longer prove it owns. Returns whether it erased.
|
||||
*/
|
||||
/** The ONLY fields an erasure tombstone keeps: replay-detection identity plus the
|
||||
* flags that keep it inert to sweeps and resolvable by the create replay path. */
|
||||
const TOMBSTONE_IDENTITY_FIELDS = new Set([
|
||||
'_id',
|
||||
'__v',
|
||||
'id',
|
||||
'user',
|
||||
'tenantId',
|
||||
'clientRequestId',
|
||||
'clientRequestDigest',
|
||||
'deleting',
|
||||
'erased',
|
||||
'erasedAt',
|
||||
'enabled',
|
||||
'createdAt',
|
||||
'updatedAt',
|
||||
]);
|
||||
|
||||
async function eraseScheduleIfDrained(id: string): Promise<boolean> {
|
||||
// A live lease (leaseUntil in the future) means a worker still holds the claim.
|
||||
// Worker clock, DocumentDB-portable: `$gt` matches neither missing nor null, so a
|
||||
|
|
@ -1620,24 +1638,18 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
|
|||
// would let the retry recreate the recurring work they just removed. The
|
||||
// tombstone keeps only the identity fields (key, digest, owner) for a bounded
|
||||
// window (TTL on erasedAt); the replay path answers "deleted" against it.
|
||||
// ALLOWLIST, not a blocklist: everything except the replay-detection identity
|
||||
// is unset, derived from the live schema paths so a field added later (tools,
|
||||
// cron, anything) cannot silently survive into the tombstone. The deletion path
|
||||
// promises that ONLY the idempotency identity remains.
|
||||
const contentFields = [
|
||||
...new Set(Object.keys(Schedule().schema.paths).map((path) => path.split('.')[0])),
|
||||
].filter((field) => !TOMBSTONE_IDENTITY_FIELDS.has(field));
|
||||
const tombstoned = await Schedule().updateOne(
|
||||
{ id, deleting: true, clientRequestId: { $exists: true } },
|
||||
{
|
||||
$set: { erased: true, erasedAt: new Date(), enabled: false },
|
||||
$unset: {
|
||||
name: 1,
|
||||
prompt: 1,
|
||||
agent_id: 1,
|
||||
cadence: 1,
|
||||
timezone: 1,
|
||||
file_ids: 1,
|
||||
lastRun: 1,
|
||||
nextRunAt: 1,
|
||||
leaseUntil: 1,
|
||||
leaseBy: 1,
|
||||
disabledReason: 1,
|
||||
countedFor: 1,
|
||||
},
|
||||
$unset: Object.fromEntries(contentFields.map((field) => [field, 1])),
|
||||
// Not a config edit; the tombstone must not surface in updated-time listings.
|
||||
},
|
||||
{ timestamps: false },
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue