🛠️ fix: Address Codex review on the audit-log substrate

- F1 (fail-closed atomicity): assign/revoke now compensate (rollback grant /
  restore grant) when a fail-closed audit write fails, so a 5xx never leaves an
  unaudited mutation.
- F5: only emit grant.assigned for a real change — skip the audit when the role
  already holds the capability (idempotent re-assert).
- F7: verifyAuditChain no longer silently trusts a non-genesis start; a purged
  prefix must be authorized by a trusted checkpoint (purge now returns
  {throughSeq, prevHash}), else verification fails as tampering.
- F4: block Model.bulkWrite on AuditLog (would bypass the append-only middleware).
- F3: CSV export appends an explicit TRUNCATED marker + logs when the row cap is hit.
- F6: reject out-of-range date-only filters (2025-02-31) instead of normalizing.
- F2: regenerate package-lock.json for the 0.0.54 data-schemas bump.

Tests: +1 methods (bulkWrite) +2 verify (deleted-prefix / checkpoint mismatch),
updated purge test for checkpoint flow; +4 api (re-assert skip, assign/revoke
fail-closed rollback, date reject, CSV truncation marker).

package-lock.json

@@ -44784,7 +44784,7 @@
     },
     "packages/data-schemas": {
       "name": "@librechat/data-schemas",
-      "version": "0.0.53",
+      "version": "0.0.54",
       "license": "MIT",
       "devDependencies": {
         "@rollup/plugin-alias": "^5.1.0",

packages/api/src/admin/auditLog.spec.ts

@@ -1,5 +1,6 @@
 import { Types } from 'mongoose';
 import { EventEmitter } from 'events';
+import { MAX_AUDIT_EXPORT_ROWS } from '@librechat/data-schemas';
 import type {
   AdminAuditLogEntry,
   AuditChainVerification,
@@ -205,6 +206,15 @@ describe('createAdminAuditLogHandlers', () => {
       expect(status).toHaveBeenCalledWith(400);
     });
 
+    it('rejects an out-of-range date-only value instead of normalizing it', async () => {
+      const deps = createDeps();
+      const handlers = createAdminAuditLogHandlers(deps);
+      const { req, res, status } = createReqRes({ query: { from: '2025-02-31' } });
+      await handlers.listAuditLog(req, res);
+      expect(status).toHaveBeenCalledWith(400);
+      expect(deps.listAuditLogPage).not.toHaveBeenCalled();
+    });
+
     it.each([
       ['action', { action: 'nope' }],
       ['category', { category: 'nope' }],
@@ -412,5 +422,17 @@ describe('createAdminAuditLogHandlers', () => {
       expect(ctx.res.status as jest.Mock).toHaveBeenCalledWith(400);
       expect(deps.streamAuditLogEntries).not.toHaveBeenCalled();
     });
+
+    it('appends an explicit marker when the export hits the row cap', async () => {
+      const deps = createDeps({
+        streamAuditLogEntries: jest.fn().mockResolvedValue(MAX_AUDIT_EXPORT_ROWS),
+      });
+      const handlers = createAdminAuditLogHandlers(deps);
+      const ctx = createCsvContext();
+      await handlers.exportAuditLogCsv(ctx.req, ctx.res);
+      const body = ctx.chunks.join('');
+      expect(body).toContain('TRUNCATED');
+      expect(ctx.endCalled()).toBe(true);
+    });
   });
 });

packages/api/src/admin/auditLog.ts

@@ -124,6 +124,17 @@ function parseIsoDate(
   if (!ISO_DATE_RE.test(raw)) return { ok: false, error: 'Date must be ISO 8601' };
   const d = new Date(raw);
   if (Number.isNaN(d.getTime())) return { ok: false, error: 'Invalid date' };
+  if (DATE_ONLY_RE.test(raw)) {
+    /**
+     * `new Date('2025-02-31')` silently normalizes to March 3 rather than
+     * throwing. Round-trip the parsed Y/M/D so an out-of-range day is rejected
+     * instead of quietly shifting the queried window.
+     */
+    const [year, month, day] = raw.split('-').map(Number);
+    if (d.getUTCFullYear() !== year || d.getUTCMonth() + 1 !== month || d.getUTCDate() !== day) {
+      return { ok: false, error: 'Invalid date' };
+    }
+  }
   if (boundary === 'end' && DATE_ONLY_RE.test(raw)) {
     d.setUTCHours(23, 59, 59, 999);
   }
@@ -378,7 +389,7 @@ export function createAdminAuditLogHandlers(deps: AdminAuditLogDeps): {
       await writeChunk(formatCsvHeader());
       await writeChunk('\r\n');
 
-      await streamAuditLogEntries(
+      const written = await streamAuditLogEntries(
         caller.tenantId,
         filters.value,
         async (entry) => {
@@ -391,7 +402,23 @@ export function createAdminAuditLogHandlers(deps: AdminAuditLogDeps): {
 
       res.removeListener('close', markAborted);
       req.removeListener('aborted', markAborted);
-      if (!clientAborted) res.end();
+      if (!clientAborted) {
+        /**
+         * Hitting the row cap means the export is incomplete. Surfacing this is a
+         * compliance requirement — a silently truncated export reads as a
+         * complete record. Emit an explicit trailing marker and log it; callers
+         * should narrow the date range for the full set.
+         */
+        if (written >= MAX_AUDIT_EXPORT_ROWS) {
+          logger.warn('[adminAuditLog] CSV export truncated at row cap', {
+            maxRows: MAX_AUDIT_EXPORT_ROWS,
+          });
+          await writeChunk(
+            `# TRUNCATED: export capped at ${MAX_AUDIT_EXPORT_ROWS} rows — narrow the from/to range to export the complete set\r\n`,
+          );
+        }
+        res.end();
+      }
     } catch (err) {
       logger.error('[adminAuditLog] export error:', err);
       if (!res.headersSent) {

packages/api/src/admin/grants.spec.ts

@@ -1243,6 +1243,23 @@ describe('createAdminGrantsHandlers', () => {
       expect(status).toHaveBeenCalledWith(201);
     });
 
+    it('does not audit re-assigning a capability the role already holds', async () => {
+      const recordAuditEntry = jest.fn().mockResolvedValue(undefined);
+      const deps = createDeps({
+        recordAuditEntry,
+        getCapabilitiesForPrincipal: jest
+          .fn()
+          .mockResolvedValue([{ capability: SystemCapabilities.READ_USERS }]),
+      });
+      const handlers = createAdminGrantsHandlers(deps);
+      const { req, res, status } = createReqRes({ body: assignBody, user: reqUser() });
+
+      await handlers.assignGrant(req, res);
+
+      expect(status).toHaveBeenCalledWith(201);
+      expect(recordAuditEntry).not.toHaveBeenCalled();
+    });
+
     it('fail-open: a failed audit write does not fail the grant', async () => {
       const recordAuditEntry = jest.fn().mockRejectedValue(new Error('audit down'));
       const deps = createDeps({ recordAuditEntry });
@@ -1254,7 +1271,7 @@ describe('createAdminGrantsHandlers', () => {
       expect(status).toHaveBeenCalledWith(201);
     });
 
-    it('fail-closed: a failed audit write surfaces as a 500', async () => {
+    it('fail-closed: a failed assign audit rolls back the grant and 500s', async () => {
       const recordAuditEntry = jest.fn().mockRejectedValue(new Error('audit down'));
       const deps = createDeps({ recordAuditEntry, auditFailClosed: true });
       const handlers = createAdminGrantsHandlers(deps);
@@ -1266,6 +1283,39 @@ describe('createAdminGrantsHandlers', () => {
         expect.anything(),
         expect.objectContaining({ failClosed: true }),
       );
+      // compensating rollback removes the grant we just created
+      expect(deps.revokeCapability).toHaveBeenCalledWith(
+        expect.objectContaining({
+          principalType: PrincipalType.ROLE,
+          principalId: 'editor',
+          capability: SystemCapabilities.READ_USERS,
+        }),
+      );
+      expect(status).toHaveBeenCalledWith(500);
+    });
+
+    it('fail-closed: a failed revoke audit restores the grant and 500s', async () => {
+      const recordAuditEntry = jest.fn().mockRejectedValue(new Error('audit down'));
+      const grantCapability = jest.fn().mockResolvedValue({ id: 'g1' });
+      const deps = createDeps({
+        recordAuditEntry,
+        auditFailClosed: true,
+        grantCapability,
+        revokeCapability: jest.fn().mockResolvedValue({ deletedCount: 1 }),
+      });
+      const handlers = createAdminGrantsHandlers(deps);
+      const { req, res, status } = createReqRes({ params: assignBody, user: reqUser() });
+
+      await handlers.revokeGrant(req, res);
+
+      // compensating re-grant restores access removed before the audit failed
+      expect(grantCapability).toHaveBeenCalledWith(
+        expect.objectContaining({
+          principalType: PrincipalType.ROLE,
+          principalId: 'editor',
+          capability: SystemCapabilities.READ_USERS,
+        }),
+      );
       expect(status).toHaveBeenCalledWith(500);
     });
   });

packages/api/src/admin/grants.ts

@@ -422,6 +422,15 @@ export function createAdminGrantsHandlers(deps: AdminGrantsDeps): {
         }
       }
 
+      /** Capture prior state before the idempotent upsert so a re-assignment of
+       * a capability the role already holds is not audited as a new change. */
+      const priorGrants = await getCapabilitiesForPrincipal({
+        principalType,
+        principalId,
+        tenantId,
+      });
+      const alreadyHeld = priorGrants.some((g) => g.capability === capability);
+
       const grant = await grantCapability({
         principalType,
         principalId,
@@ -432,14 +441,29 @@ export function createAdminGrantsHandlers(deps: AdminGrantsDeps): {
       if (!grant) {
         return res.status(500).json({ error: 'Grant operation returned no result' });
       }
-      await emitAudit({
-        action: 'grant.assigned',
-        caller,
-        principalType,
-        principalId,
-        capability,
-        context: buildAuditContext(req),
-      });
+      if (!alreadyHeld) {
+        try {
+          await emitAudit({
+            action: 'grant.assigned',
+            caller,
+            principalType,
+            principalId,
+            capability,
+            context: buildAuditContext(req),
+          });
+        } catch (auditErr) {
+          /** Fail-closed only (emitAudit rethrows here): roll back the grant we
+           * just created so a 5xx never leaves an unaudited grant in place. */
+          await revokeCapability({ principalType, principalId, capability, tenantId }).catch((e) =>
+            logger.error('[adminGrants] compensating revoke after audit failure failed', e),
+          );
+          logger.error(
+            '[adminGrants] assignGrant audit failed (fail-closed) — rolled back grant',
+            auditErr,
+          );
+          return res.status(500).json({ error: 'Failed to record audit entry' });
+        }
+      }
       return res.status(201).json({ grant });
     } catch (error) {
       logger.error('[adminGrants] assignGrant error:', error);
@@ -490,14 +514,35 @@ export function createAdminGrantsHandlers(deps: AdminGrantsDeps): {
       // a no-op revoke (the grant never existed) must not appear in the audit
       // trail or the forensic record becomes misleading.
       if (revokeResult.deletedCount > 0) {
-        await emitAudit({
-          action: 'grant.removed',
-          caller,
-          principalType: principalType as PrincipalType,
-          principalId,
-          capability: capability as SystemCapability,
-          context: buildAuditContext(req),
-        });
+        try {
+          await emitAudit({
+            action: 'grant.removed',
+            caller,
+            principalType: principalType as PrincipalType,
+            principalId,
+            capability: capability as SystemCapability,
+            context: buildAuditContext(req),
+          });
+        } catch (auditErr) {
+          /** Fail-closed only (emitAudit rethrows here): restore the grant we
+           * just removed so a 5xx never leaves an unaudited revoke. The original
+           * `grantedBy`/`grantedAt` are not recoverable here; restoring access is
+           * the priority on this rare error path. */
+          await grantCapability({
+            principalType: principalType as PrincipalType,
+            principalId,
+            capability: capability as SystemCapability,
+            tenantId,
+            grantedBy: caller.userId,
+          }).catch((e) =>
+            logger.error('[adminGrants] compensating re-grant after audit failure failed', e),
+          );
+          logger.error(
+            '[adminGrants] revokeGrant audit failed (fail-closed) — restored grant',
+            auditErr,
+          );
+          return res.status(500).json({ error: 'Failed to record audit entry' });
+        }
       }
       return res.status(200).json({ success: true });
     } catch (error) {

packages/data-schemas/src/methods/auditLog.spec.ts

@@ -173,6 +173,20 @@ describe('auditLog methods', () => {
       await expect(doc!.save()).rejects.toThrow(/append-only/);
       await expect(doc!.deleteOne()).rejects.toThrow(/append-only/);
     });
+
+    it('rejects bulkWrite (which would bypass query/document middleware)', async () => {
+      await methods.recordAuditEntry(baseInput());
+      await expect(
+        AuditLog.bulkWrite([
+          {
+            updateOne: {
+              filter: { chainKey: 'tenant-a' },
+              update: { $set: { 'actor.name': 'x' } },
+            },
+          },
+        ]),
+      ).rejects.toThrow(/append-only/);
+    });
   });
 
   describe('listAuditLogPage', () => {
@@ -338,6 +352,26 @@ describe('auditLog methods', () => {
       expect(result.ok).toBe(false);
       expect(result.brokenAt).toBe(2);
     });
+
+    it('does not silently trust a deleted prefix without a checkpoint', async () => {
+      await seed(3);
+      // attacker deletes the oldest row through the raw driver
+      await AuditLog.collection.deleteOne({ chainKey: 'tenant-a', seq: 1 });
+      const result = await methods.verifyAuditChain('tenant-a');
+      expect(result.ok).toBe(false);
+      expect(result.brokenAt).toBe(2);
+      expect(result.reason).toMatch(/non-genesis|prefix/);
+    });
+
+    it('rejects a checkpoint that does not match the remaining chain', async () => {
+      await seed(3);
+      await AuditLog.collection.deleteOne({ chainKey: 'tenant-a', seq: 1 });
+      const result = await methods.verifyAuditChain('tenant-a', {
+        trustedCheckpoint: { throughSeq: 1, prevHash: 'f'.repeat(64) },
+      });
+      expect(result.ok).toBe(false);
+      expect(result.reason).toMatch(/checkpoint mismatch/);
+    });
   });
 
   describe('purgeAuditLogEntries', () => {
@@ -365,11 +399,18 @@ describe('auditLog methods', () => {
         confirm: true,
       });
       expect(result.deletedCount).toBeGreaterThanOrEqual(1);
-      expect(result.checkpoint?.seq).toBeGreaterThan(1);
+      expect(result.checkpoint?.throughSeq).toBeGreaterThanOrEqual(1);
 
-      const verification = await methods.verifyAuditChain('tenant-a');
-      expect(verification.ok).toBe(true);
-      expect(verification.range?.firstSeq).toBeGreaterThan(1);
+      // Without the checkpoint, the now non-genesis start is flagged, not trusted.
+      const unverified = await methods.verifyAuditChain('tenant-a');
+      expect(unverified.ok).toBe(false);
+
+      // With the checkpoint the purge returned, the shorter chain verifies.
+      const verified = await methods.verifyAuditChain('tenant-a', {
+        trustedCheckpoint: result.checkpoint,
+      });
+      expect(verified.ok).toBe(true);
+      expect(verified.range?.firstSeq).toBeGreaterThan(1);
     });
   });
 });

packages/data-schemas/src/methods/auditLog.ts

@@ -12,6 +12,7 @@ import type {
   PurgeAuditLogResult,
   RecordAuditEntryInput,
   RecordAuditEntryOptions,
+  VerifyAuditChainOptions,
 } from '~/types';
 import { GENESIS_HASH, PLATFORM_CHAIN_KEY } from '~/schema/auditLog';
 import { AUDIT_ACTION_CATEGORY } from '~/types/admin';
@@ -52,7 +53,10 @@ export interface AuditLogMethods {
     onEntry: (entry: AdminAuditLogEntry) => void | Promise<void>,
     options?: { isCancelled?: () => boolean; maxRows?: number },
   ) => Promise<number>;
-  verifyAuditChain: (tenantId: string | undefined) => Promise<AuditChainVerification>;
+  verifyAuditChain: (
+    tenantId: string | undefined,
+    options?: VerifyAuditChainOptions,
+  ) => Promise<AuditChainVerification>;
   purgeAuditLogEntries: (
     tenantId: string | undefined,
     options: PurgeAuditLogOptions,
@@ -450,7 +454,10 @@ export function createAuditLogMethods(mongoose: typeof import('mongoose')): Audi
     return count;
   }
 
-  async function verifyAuditChain(tenantId: string | undefined): Promise<AuditChainVerification> {
+  async function verifyAuditChain(
+    tenantId: string | undefined,
+    options?: VerifyAuditChainOptions,
+  ): Promise<AuditChainVerification> {
     const AuditLog = model();
     const chainKey = resolveChainKey(tenantId);
     const cursor = AuditLog.find({ chainKey })
@@ -458,6 +465,7 @@ export function createAuditLogMethods(mongoose: typeof import('mongoose')): Audi
       .lean<IAuditLog[]>()
       .cursor({ batchSize: 500 });
 
+    const trustedCheckpoint = options?.trustedCheckpoint;
     let prevHash = GENESIS_HASH;
     let expectedSeq: number | null = null;
     let firstSeq: number | null = null;
@@ -469,9 +477,33 @@ export function createAuditLogMethods(mongoose: typeof import('mongoose')): Audi
         if (firstSeq === null) {
           firstSeq = doc.seq;
           expectedSeq = doc.seq;
-          /** Genesis must link to GENESIS_HASH; a purged prefix (firstSeq > 1)
-           * makes the earliest remaining entry's prevHash a trusted checkpoint. */
-          prevHash = doc.seq === 1 ? GENESIS_HASH : doc.prevHash;
+          /**
+           * A non-genesis start (firstSeq > 1) means a prefix is gone. Only a
+           * matching trusted checkpoint proves it was an authorized retention
+           * purge; otherwise this is an attacker deleting the oldest rows, so we
+           * must NOT silently trust the first row's prevHash.
+           */
+          if (doc.seq === 1) {
+            prevHash = GENESIS_HASH;
+          } else if (
+            trustedCheckpoint &&
+            trustedCheckpoint.throughSeq === doc.seq - 1 &&
+            trustedCheckpoint.prevHash === doc.prevHash
+          ) {
+            prevHash = doc.prevHash;
+          } else {
+            await cursor.close();
+            return {
+              ok: false,
+              chainKey,
+              checked,
+              brokenAt: doc.seq,
+              reason: trustedCheckpoint
+                ? 'checkpoint mismatch: purged prefix does not match the trusted checkpoint'
+                : `non-genesis start at seq ${doc.seq}: prefix purged or deleted (supply a trusted checkpoint to verify)`,
+              range: { firstSeq, lastSeq: doc.seq },
+            };
+          }
         }
         if (doc.seq !== expectedSeq) {
           return {
@@ -556,7 +588,13 @@ export function createAuditLogMethods(mongoose: typeof import('mongoose')): Audi
 
     return {
       deletedCount: result.deletedCount ?? 0,
-      checkpoint: newFirst ? { seq: newFirst.seq, prevHash: newFirst.prevHash } : undefined,
+      /** Boundary for `verifyAuditChain`: the new earliest entry resumes the
+       * chain at `seq`, so the purged prefix ran through `seq - 1` and its last
+       * hash is this entry's `prevHash`. Callers persist this to later prove the
+       * purge was authorized. */
+      checkpoint: newFirst
+        ? { throughSeq: newFirst.seq - 1, prevHash: newFirst.prevHash }
+        : undefined,
     };
   }
 

packages/data-schemas/src/schema/auditLog.ts

@@ -143,6 +143,18 @@ auditLogSchema.pre('save', function (next) {
   next();
 });
 
+/**
+ * `Model.bulkWrite()` dispatches update/delete operations through the raw driver
+ * and skips the query/document middleware above. Block it wholesale: audit rows
+ * are only ever written via `recordAuditEntry` (`Model.create`), and a bulk
+ * insert would also break the sequential hash chain, so there is no legitimate
+ * bulkWrite path. The privileged retention purge uses `Model.collection` and is
+ * unaffected by this hook.
+ */
+auditLogSchema.pre('bulkWrite', function (next) {
+  next(new Error(APPEND_ONLY_MESSAGE));
+});
+
 /**
  * Unique per-chain sequence. This is both the keyset-pagination key and the
  * integrity backstop: concurrent appends race to claim the next `seq`, one wins,

packages/data-schemas/src/types/auditLog.ts

@@ -148,11 +148,29 @@ export interface PurgeAuditLogOptions {
   confirm: boolean;
 }
 
+/** A trusted boundary marker proving a prefix was purged by an authorized
+ * retention run rather than deleted by an attacker. Persisted by the caller
+ * (e.g. alongside retention-job records) and passed back to `verifyAuditChain`. */
+export interface AuditCheckpoint {
+  /** Highest `seq` that was purged; the chain resumes at `throughSeq + 1`. */
+  throughSeq: number;
+  /** Hash of the last purged entry === `prevHash` of the new earliest entry. */
+  prevHash: string;
+}
+
 export interface PurgeAuditLogResult {
   deletedCount: number;
-  /** The new earliest remaining entry, which becomes the trusted checkpoint the
-   * verifier chains forward from. Absent when the chain is now empty. */
-  checkpoint?: { seq: number; prevHash: string };
+  /** The boundary the verifier must be given to accept the now-shorter chain.
+   * Absent when the chain is now empty or nothing was purged. */
+  checkpoint?: AuditCheckpoint;
+}
+
+export interface VerifyAuditChainOptions {
+  /** 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
+   * fails verification rather than being silently trusted. */
+  trustedCheckpoint?: AuditCheckpoint;
 }
 
 /** Re-exported so consumers can import the entry shape and its supporting types