From d9251fc3b6e4b6361e672c3aa4f17fa65555b777 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 23 Jun 2026 08:33:16 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=82=EF=B8=8F=20fix:=20Cap=20Audit=20Chain?= =?UTF-8?q?=20Verification=20and=20Honor=20Client=20Cancellation=20(#13903?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/api/src/admin/auditLog.spec.ts | 34 +++++++++++++++++-- packages/api/src/admin/auditLog.ts | 27 +++++++++++++-- packages/data-schemas/src/index.ts | 1 + .../data-schemas/src/methods/auditLog.spec.ts | 24 +++++++++++++ packages/data-schemas/src/methods/auditLog.ts | 29 ++++++++++++++++ packages/data-schemas/src/methods/index.ts | 3 +- packages/data-schemas/src/types/auditLog.ts | 4 +++ 7 files changed, 115 insertions(+), 7 deletions(-) diff --git a/packages/api/src/admin/auditLog.spec.ts b/packages/api/src/admin/auditLog.spec.ts index cdb221bcc5..5c5134bf6a 100644 --- a/packages/api/src/admin/auditLog.spec.ts +++ b/packages/api/src/admin/auditLog.spec.ts @@ -1,6 +1,6 @@ import { Types } from 'mongoose'; import { EventEmitter } from 'events'; -import { MAX_AUDIT_EXPORT_ROWS } from '@librechat/data-schemas'; +import { MAX_AUDIT_EXPORT_ROWS, MAX_AUDIT_VERIFY_ROWS } from '@librechat/data-schemas'; import type { AdminAuditLogEntry, AuditChainVerification, @@ -64,11 +64,20 @@ function createReqRes( user: 'user' in overrides ? overrides.user : { _id: new Types.ObjectId(), role: 'admin' }, } as unknown as ServerRequest; + const emitter = new EventEmitter(); + const reqEmitter = new EventEmitter(); const json = jest.fn(); const status = jest.fn().mockReturnValue({ json }); - const res = { status, json } as unknown as Response; + const res = Object.assign(emitter, { + status, + json, + removeListener: emitter.removeListener.bind(emitter), + }) as unknown as Response; + const eventReq = Object.assign(reqEmitter, req, { + removeListener: reqEmitter.removeListener.bind(reqEmitter), + }) as unknown as ServerRequest; - return { req, res, status, json }; + return { req: eventReq, res, status, json, emitter, reqEmitter }; } interface CsvCaptureContext { @@ -351,10 +360,29 @@ describe('createAdminAuditLogHandlers', () => { }); await handlers.verifyAuditLog(req, res); expect((deps.verifyAuditChain as jest.Mock).mock.calls[0][0]).toBe('real-tenant'); + expect((deps.verifyAuditChain as jest.Mock).mock.calls[0][1].maxRows).toBe( + MAX_AUDIT_VERIFY_ROWS, + ); expect(status).toHaveBeenCalledWith(200); expect(json).toHaveBeenCalledWith(verification); }); + it('threads cancellation into verification and skips the response after client close', async () => { + const ctx = createReqRes(); + const deps = createDeps({ + verifyAuditChain: jest.fn(async (_tenantId, options) => { + ctx.emitter.emit('close'); + options?.isCancelled?.(); + return mockVerification({ ok: false, reason: 'verification cancelled' }); + }), + }); + const handlers = createAdminAuditLogHandlers(deps); + await handlers.verifyAuditLog(ctx.req, ctx.res); + const optionsArg = (deps.verifyAuditChain as jest.Mock).mock.calls[0][1]; + expect(optionsArg.isCancelled()).toBe(true); + expect(ctx.status).not.toHaveBeenCalled(); + }); + it('returns 500 when verification throws', async () => { const deps = createDeps({ verifyAuditChain: jest.fn().mockRejectedValue(new Error('boom')), diff --git a/packages/api/src/admin/auditLog.ts b/packages/api/src/admin/auditLog.ts index 6b21e02f78..570e70ddf0 100644 --- a/packages/api/src/admin/auditLog.ts +++ b/packages/api/src/admin/auditLog.ts @@ -7,6 +7,7 @@ import { AUDIT_ACTOR_TYPES, MAX_AUDIT_EXPORT_ROWS, MAX_AUDIT_LOG_LIMIT, + MAX_AUDIT_VERIFY_ROWS, } from '@librechat/data-schemas'; import type { AdminAuditLogEntry, @@ -59,7 +60,10 @@ export interface AdminAuditLogDeps { onEntry: (entry: AdminAuditLogEntry) => void | Promise, options?: { isCancelled?: () => boolean; maxRows?: number }, ) => Promise<{ count: number; truncated: boolean }>; - verifyAuditChain: (tenantId: string | undefined) => Promise; + verifyAuditChain: ( + tenantId: string | undefined, + options?: { isCancelled?: () => boolean; maxRows?: number }, + ) => Promise; } interface CallerContext { @@ -328,8 +332,25 @@ export function createAdminAuditLogHandlers(deps: AdminAuditLogDeps): { const caller = resolveCaller(req); if (!caller) return res.status(401).json({ error: 'Authentication required' }); - const result = await verifyAuditChain(caller.tenantId); - return res.status(200).json(result); + let clientAborted = false; + const markAborted = () => { + clientAborted = true; + }; + res.once('close', markAborted); + req.once('aborted', markAborted); + + try { + const result = await verifyAuditChain(caller.tenantId, { + isCancelled: () => clientAborted, + maxRows: MAX_AUDIT_VERIFY_ROWS, + }); + + if (clientAborted) return res; + return res.status(200).json(result); + } finally { + res.removeListener('close', markAborted); + req.removeListener('aborted', markAborted); + } } catch (err) { logger.error('[adminAuditLog] verify error:', err); return res.status(500).json({ error: 'Failed to verify audit log integrity' }); diff --git a/packages/data-schemas/src/index.ts b/packages/data-schemas/src/index.ts index e35c29e2e3..ee393b2de4 100644 --- a/packages/data-schemas/src/index.ts +++ b/packages/data-schemas/src/index.ts @@ -27,6 +27,7 @@ export { AUDIT_SCHEMA_VERSION, MAX_AUDIT_EXPORT_ROWS, MAX_AUDIT_LOG_LIMIT, + MAX_AUDIT_VERIFY_ROWS, } from './methods'; export type * from './types'; export type * from './methods'; diff --git a/packages/data-schemas/src/methods/auditLog.spec.ts b/packages/data-schemas/src/methods/auditLog.spec.ts index eb5504b738..aefdf34e81 100644 --- a/packages/data-schemas/src/methods/auditLog.spec.ts +++ b/packages/data-schemas/src/methods/auditLog.spec.ts @@ -351,6 +351,30 @@ describe('auditLog methods', () => { expect(result.checked).toBe(0); }); + it('stops verification when the caller cancels', async () => { + await seed(3); + const result = await methods.verifyAuditChain('tenant-a', { isCancelled: () => true }); + expect(result.ok).toBe(false); + expect(result.checked).toBe(0); + expect(result.reason).toBe('verification cancelled'); + }); + + it('bounds verification when rows exceed the configured cap', async () => { + await seed(3); + const result = await methods.verifyAuditChain('tenant-a', { maxRows: 2 }); + expect(result.ok).toBe(false); + expect(result.checked).toBe(2); + expect(result.brokenAt).toBe(3); + expect(result.reason).toMatch(/row limit exceeded/); + }); + + it('allows an exact-cap verification to complete', async () => { + await seed(2); + const result = await methods.verifyAuditChain('tenant-a', { maxRows: 2 }); + expect(result.ok).toBe(true); + expect(result.checked).toBe(2); + }); + it('detects a tampered field (hash mismatch)', async () => { await seed(3); // mutate a field via the raw driver, bypassing the append-only hooks diff --git a/packages/data-schemas/src/methods/auditLog.ts b/packages/data-schemas/src/methods/auditLog.ts index 6d520df07f..0ab6a7dd51 100644 --- a/packages/data-schemas/src/methods/auditLog.ts +++ b/packages/data-schemas/src/methods/auditLog.ts @@ -27,6 +27,11 @@ export const MAX_AUDIT_LOG_LIMIT = 500; * should be sliced by `from`/`to`. */ export const MAX_AUDIT_EXPORT_ROWS = 100_000; +/** + * Upper bound on rows verified in a single HTTP-triggered integrity check. Full + * offline jobs can pass a larger value explicitly when needed. + */ +export const MAX_AUDIT_VERIFY_ROWS = 100_000; /** Record-format version stamped on every new entry. */ export const AUDIT_SCHEMA_VERSION = 1; const MAX_SEARCH_LENGTH = 200; @@ -516,6 +521,8 @@ export function createAuditLogMethods(mongoose: typeof import('mongoose')): Audi .cursor({ batchSize: 500 }); const trustedCheckpoint = options?.trustedCheckpoint; + const isCancelled = options?.isCancelled; + const maxRows = options?.maxRows; let prevHash = GENESIS_HASH; let expectedSeq: number | null = null; let firstSeq: number | null = null; @@ -524,6 +531,28 @@ export function createAuditLogMethods(mongoose: typeof import('mongoose')): Audi try { for await (const doc of cursor) { + if (isCancelled?.()) { + await cursor.close(); + return { + ok: false, + chainKey, + checked, + reason: 'verification cancelled', + range: firstSeq !== null ? { firstSeq, lastSeq } : undefined, + }; + } + if (maxRows != null && checked >= maxRows) { + await cursor.close(); + return { + ok: false, + chainKey, + checked, + brokenAt: doc.seq, + reason: `verification row limit exceeded (${maxRows})`, + range: + firstSeq !== null ? { firstSeq, lastSeq } : { firstSeq: doc.seq, lastSeq: doc.seq }, + }; + } if (firstSeq === null) { firstSeq = doc.seq; expectedSeq = doc.seq; diff --git a/packages/data-schemas/src/methods/index.ts b/packages/data-schemas/src/methods/index.ts index b98dab3a36..efde53c3e9 100644 --- a/packages/data-schemas/src/methods/index.ts +++ b/packages/data-schemas/src/methods/index.ts @@ -25,6 +25,7 @@ import { AUDIT_SCHEMA_VERSION, MAX_AUDIT_EXPORT_ROWS, MAX_AUDIT_LOG_LIMIT, + MAX_AUDIT_VERIFY_ROWS, type AuditLogMethods, } from './auditLog'; import { createShareMethods, type ShareMethods } from './share'; @@ -108,7 +109,7 @@ export { deriveStructuredFrontmatterFields, inferSkillFileCategory, }; -export { AUDIT_SCHEMA_VERSION, MAX_AUDIT_EXPORT_ROWS, MAX_AUDIT_LOG_LIMIT }; +export { AUDIT_SCHEMA_VERSION, MAX_AUDIT_EXPORT_ROWS, MAX_AUDIT_LOG_LIMIT, MAX_AUDIT_VERIFY_ROWS }; export type AllMethods = UserMethods & SessionMethods & diff --git a/packages/data-schemas/src/types/auditLog.ts b/packages/data-schemas/src/types/auditLog.ts index f1f6a94dda..0a851fec58 100644 --- a/packages/data-schemas/src/types/auditLog.ts +++ b/packages/data-schemas/src/types/auditLog.ts @@ -166,6 +166,10 @@ export interface PurgeAuditLogResult { } export interface VerifyAuditChainOptions { + /** Stop verification early when the caller has gone away. */ + isCancelled?: () => boolean; + /** Maximum rows to inspect before returning a bounded, non-OK result. */ + maxRows?: number; /** When the chain no longer starts at `seq: 1` (a prefix was purged), the * verifier requires this boundary to distinguish an authorized retention purge * from an attacker deleting the oldest rows. Without it, a non-genesis start