🛠️ fix: Address audit log review feedback (CI typecheck, ISO offsets, no-op revoke, deps surface, schema, backpressure, tests)

Resolve the duplicate AuditAction export that broke the data-schemas TypeScript check by importing the canonical declaration from types/admin instead of re-declaring it in types/auditLog.

Accept timezone-offset ISO 8601 timestamps such as 2026-05-01T09:30:00+02:00 in the from and to filter params and reject local-time strings without a zone so every request resolves to an unambiguous instant.

Skip the audit emission on no-op revokes: revokeCapability now returns deletedCount so the admin handler can omit the grant_removed entry when the target grant did not exist, keeping the audit trail factually accurate. Mocks in the existing grants.spec.ts updated to the new return shape.

Drop the required recordAuditEntry from AdminAuditLogDeps since the audit-log handler factory never consumes it; the grants handler factory keeps its optional dep for the write path.

Tighten the tenantId validator on the audit log schema to require a non-empty trimmed string, and rewrite the listing-index comment to describe deterministic offset sort instead of keyset pagination.

Stream the CSV export with explicit backpressure (await drain when res.write returns false) and abort on client disconnect so a cancelled download no longer pins a Mongo cursor or buffers unbounded data in memory.

Add packages/data-schemas/src/methods/auditLog.spec.ts covering tenant and platform scoping, single and multi action filtering, partial-name filtering for actor and capability, the createdAt window, offset pagination with total, ObjectId and date stringification on the wire, regex-metacharacter escape, and streaming completeness.

api/server/routes/admin/audit.js

@@ -10,7 +10,6 @@ const router = express.Router();
 const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN);
 
 const handlers = createAdminAuditLogHandlers({
-  recordAuditEntry: db.recordAuditEntry,
   listAuditLogPage: db.listAuditLogPage,
   findAuditLogEntry: db.findAuditLogEntry,
   streamAuditLogEntries: db.streamAuditLogEntries,

packages/api/src/admin/auditLog.ts

@@ -1,11 +1,6 @@
 import { PrincipalType } from 'librechat-data-provider';
 import { logger } from '@librechat/data-schemas';
-import type {
-  AdminAuditLogEntryWire,
-  AuditAction,
-  AuditLogPage,
-  RecordAuditEntryInput,
-} from '@librechat/data-schemas';
+import type { AdminAuditLogEntryWire, AuditAction, AuditLogPage } from '@librechat/data-schemas';
 import type { Response } from 'express';
 import type { ServerRequest } from '~/types/http';
 
@@ -15,11 +10,14 @@ const CSV_BOM = '';
 const VALID_ACTIONS = new Set<AuditAction>(['grant_assigned', 'grant_removed']);
 const VALID_PRINCIPAL_TYPES = new Set<string>(Object.values(PrincipalType));
 
-const ISO_DATE_RE = /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}(:\d{2}(\.\d{1,3})?)?Z?)?$/;
+// Accepts `YYYY-MM-DD` (interpreted as UTC by the downstream Date parse) or a
+// full ISO 8601 / RFC 3339 timestamp that includes either `Z` or a `±HH:MM`
+// offset. Local-time strings without a zone are rejected so every input maps
+// to an unambiguous instant.
+const ISO_DATE_RE = /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}(:\d{2}(\.\d{1,3})?)?(Z|[+-]\d{2}:\d{2}))?$/;
 const OBJECT_ID_RE = /^[a-fA-F0-9]{24}$/;
 
 export interface AdminAuditLogDeps {
-  recordAuditEntry: (input: RecordAuditEntryInput) => Promise<unknown>;
   listAuditLogPage: (
     tenantId: string | undefined,
     filters: {
@@ -253,16 +251,36 @@ export function createAdminAuditLogHandlers(deps: AdminAuditLogDeps) {
       res.setHeader('Content-Disposition', `attachment; filename="audit-log-${filenameStamp}.csv"`);
       res.setHeader('Cache-Control', 'no-store');
 
-      res.write(CSV_BOM);
-      res.write(formatCsvHeader());
-      res.write('\r\n');
+      // Stop pulling rows from Mongo as soon as the client disconnects so a
+      // cancelled download doesn't pin a cursor open or burn through memory.
+      let clientAborted = false;
+      const markAborted = () => {
+        clientAborted = true;
+      };
+      res.once('close', markAborted);
+      req.once('aborted', markAborted);
 
-      await streamAuditLogEntries(caller.tenantId, filters.value, (entry) => {
-        res.write(formatCsvRow(entry));
-        res.write('\r\n');
+      // Wait for `drain` when the kernel/socket buffer is full so we never
+      // queue an unbounded amount of CSV in Node memory for slow consumers.
+      const writeChunk = (chunk: string): Promise<void> => {
+        if (clientAborted) return Promise.resolve();
+        if (res.write(chunk)) return Promise.resolve();
+        return new Promise<void>((resolve) => res.once('drain', () => resolve()));
+      };
+
+      await writeChunk(CSV_BOM);
+      await writeChunk(formatCsvHeader());
+      await writeChunk('\r\n');
+
+      await streamAuditLogEntries(caller.tenantId, filters.value, async (entry) => {
+        if (clientAborted) return;
+        await writeChunk(formatCsvRow(entry));
+        await writeChunk('\r\n');
       });
 
-      res.end();
+      res.removeListener('close', markAborted);
+      req.removeListener('aborted', markAborted);
+      if (!clientAborted) res.end();
     } catch (err) {
       logger.error('[adminAuditLog] export error:', err);
       if (!res.headersSent) {

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

@@ -54,7 +54,7 @@ function createDeps(overrides: Partial<AdminGrantsDeps> = {}): AdminGrantsDeps {
     getCapabilitiesForPrincipal: jest.fn().mockResolvedValue([]),
     getCapabilitiesForPrincipals: jest.fn().mockResolvedValue([]),
     grantCapability: jest.fn().mockResolvedValue(mockGrant()),
-    revokeCapability: jest.fn().mockResolvedValue(undefined),
+    revokeCapability: jest.fn().mockResolvedValue({ deletedCount: 1 }),
     getUserPrincipals: jest.fn().mockResolvedValue([
       { principalType: PrincipalType.USER, principalId: new Types.ObjectId() },
       { principalType: PrincipalType.ROLE, principalId: 'admin' },
@@ -969,7 +969,7 @@ describe('createAdminGrantsHandlers', () => {
 
     it('returns 200 idempotently even if the grant does not exist', async () => {
       const deps = createDeps({
-        revokeCapability: jest.fn().mockResolvedValue(undefined),
+        revokeCapability: jest.fn().mockResolvedValue({ deletedCount: 1 }),
       });
       const handlers = createAdminGrantsHandlers(deps);
       const { req, res, status, json } = createReqRes({ params: validParams });

packages/api/src/admin/grants.ts

@@ -50,7 +50,7 @@ export interface AdminGrantsDeps {
     principalId: string | Types.ObjectId;
     capability: SystemCapability;
     tenantId?: string;
-  }) => Promise<void>;
+  }) => Promise<{ deletedCount: number }>;
   getUserPrincipals: (params: {
     userId: string;
     role?: string | null;
@@ -439,19 +439,24 @@ export function createAdminGrantsHandlers(deps: AdminGrantsDeps) {
         return res.status(403).json({ error: 'Insufficient permissions' });
       }
 
-      await revokeCapability({
+      const revokeResult = await revokeCapability({
         principalType: principalType as PrincipalType,
         principalId,
         capability: capability as SystemCapability,
         tenantId,
       });
-      await emitAudit({
-        action: 'grant_removed',
-        caller,
-        principalType: principalType as PrincipalType,
-        principalId,
-        capability: capability as SystemCapability,
-      });
+      // Only emit the audit entry when a grant document was actually deleted —
+      // 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,
+        });
+      }
       return res.status(200).json({ success: true });
     } catch (error) {
       logger.error('[adminGrants] revokeGrant error:', error);

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

@@ -0,0 +1,171 @@
+import mongoose, { Types } from 'mongoose';
+import { PrincipalType } from 'librechat-data-provider';
+import { MongoMemoryServer } from 'mongodb-memory-server';
+import type * as t from '~/types';
+import { createAuditLogMethods, type RecordAuditEntryInput } from './auditLog';
+import auditLogSchema from '~/schema/auditLog';
+
+jest.mock('~/config/winston', () => ({
+  error: jest.fn(),
+  warn: jest.fn(),
+  info: jest.fn(),
+  debug: jest.fn(),
+}));
+
+let mongoServer: MongoMemoryServer;
+let AuditLog: mongoose.Model<t.IAuditLog>;
+let methods: ReturnType<typeof createAuditLogMethods>;
+
+const actorObjectId = new Types.ObjectId();
+const targetObjectId = new Types.ObjectId();
+
+function baseInput(over: Partial<RecordAuditEntryInput> = {}): RecordAuditEntryInput {
+  return {
+    action: 'grant_assigned',
+    actorId: actorObjectId,
+    actorName: 'Alice Admin',
+    targetPrincipalType: PrincipalType.USER,
+    targetPrincipalId: targetObjectId,
+    targetName: 'Bob User',
+    capability: 'manage:users',
+    tenantId: 'tenant-a',
+    ...over,
+  };
+}
+
+beforeAll(async () => {
+  mongoServer = await MongoMemoryServer.create();
+  AuditLog = mongoose.models.AuditLog || mongoose.model<t.IAuditLog>('AuditLog', auditLogSchema);
+  methods = createAuditLogMethods(mongoose);
+  await mongoose.connect(mongoServer.getUri());
+});
+
+afterAll(async () => {
+  await mongoose.disconnect();
+  await mongoServer.stop();
+});
+
+beforeEach(async () => {
+  await AuditLog.deleteMany({});
+});
+
+describe('auditLog methods', () => {
+  describe('recordAuditEntry', () => {
+    it('persists a tenant-scoped entry and wires denormalized fields through', async () => {
+      const doc = await methods.recordAuditEntry(baseInput());
+      expect(doc).not.toBeNull();
+      const persisted = await AuditLog.findOne({ tenantId: 'tenant-a' }).lean();
+      expect(persisted?.actorName).toBe('Alice Admin');
+      expect(persisted?.targetName).toBe('Bob User');
+      expect(persisted?.capability).toBe('manage:users');
+      expect(persisted?.createdAt).toBeInstanceOf(Date);
+    });
+
+    it('omits the tenantId field entirely for platform-level entries', async () => {
+      await methods.recordAuditEntry(baseInput({ tenantId: undefined }));
+      const raw = await AuditLog.collection.findOne({});
+      expect(raw && Object.prototype.hasOwnProperty.call(raw, 'tenantId')).toBe(false);
+    });
+  });
+
+  describe('listAuditLogPage', () => {
+    beforeEach(async () => {
+      await methods.recordAuditEntry(baseInput({ capability: 'manage:users' }));
+      await methods.recordAuditEntry(
+        baseInput({ action: 'grant_removed', capability: 'manage:roles' }),
+      );
+      await methods.recordAuditEntry(baseInput({ tenantId: 'tenant-b', capability: 'read:users' }));
+      await methods.recordAuditEntry(
+        baseInput({ tenantId: undefined, capability: 'access:admin' }),
+      );
+    });
+
+    it('returns only the calling tenant by default', async () => {
+      const page = await methods.listAuditLogPage('tenant-a', {});
+      expect(page.total).toBe(2);
+      const tenants = new Set(page.entries.map((e) => e.capability));
+      expect(tenants).toEqual(new Set(['manage:users', 'manage:roles']));
+    });
+
+    it('only returns platform-level entries when tenantId is omitted', async () => {
+      const page = await methods.listAuditLogPage(undefined, {});
+      expect(page.total).toBe(1);
+      expect(page.entries[0]?.capability).toBe('access:admin');
+    });
+
+    it('filters by a single action', async () => {
+      const page = await methods.listAuditLogPage('tenant-a', { action: ['grant_removed'] });
+      expect(page.total).toBe(1);
+      expect(page.entries[0]?.action).toBe('grant_removed');
+    });
+
+    it('filters by multiple actions via $in', async () => {
+      const page = await methods.listAuditLogPage('tenant-a', {
+        action: ['grant_assigned', 'grant_removed'],
+      });
+      expect(page.total).toBe(2);
+    });
+
+    it('partial-matches the actorId param against actorName', async () => {
+      await methods.recordAuditEntry(baseInput({ actorName: 'Charlotte Auditor' }));
+      const page = await methods.listAuditLogPage('tenant-a', { actorId: 'charlotte' });
+      expect(page.total).toBe(1);
+      expect(page.entries[0]?.actorName).toBe('Charlotte Auditor');
+    });
+
+    it('partial-matches the capability filter case-insensitively', async () => {
+      const page = await methods.listAuditLogPage('tenant-a', { capability: 'ROLES' });
+      expect(page.total).toBe(1);
+      expect(page.entries[0]?.capability).toBe('manage:roles');
+    });
+
+    it('applies a `from` / `to` window using createdAt', async () => {
+      const allBefore = await AuditLog.find({ tenantId: 'tenant-a' }).sort({ createdAt: 1 }).lean();
+      const cutoff = allBefore[0]!.createdAt;
+      const after = new Date(cutoff.getTime() + 1);
+      const page = await methods.listAuditLogPage('tenant-a', { from: after });
+      expect(page.total).toBeLessThan(2);
+    });
+
+    it('paginates by offset and limit and reports the total of the full match set', async () => {
+      const page1 = await methods.listAuditLogPage('tenant-a', { offset: 0, limit: 1 });
+      const page2 = await methods.listAuditLogPage('tenant-a', { offset: 1, limit: 1 });
+      expect(page1.total).toBe(2);
+      expect(page2.total).toBe(2);
+      expect(page1.entries.length).toBe(1);
+      expect(page2.entries.length).toBe(1);
+      expect(page1.entries[0]?.id).not.toBe(page2.entries[0]?.id);
+    });
+
+    it('stringifies ObjectIds and dates on the wire', async () => {
+      const page = await methods.listAuditLogPage('tenant-a', { limit: 1 });
+      const entry = page.entries[0]!;
+      expect(typeof entry.id).toBe('string');
+      expect(typeof entry.actorId).toBe('string');
+      expect(typeof entry.targetPrincipalId).toBe('string');
+      expect(typeof entry.timestamp).toBe('string');
+      expect(() => new Date(entry.timestamp).toISOString()).not.toThrow();
+    });
+
+    it('escapes regex metacharacters in the search input', async () => {
+      await methods.recordAuditEntry(baseInput({ actorName: 'risky.dot+plus' }));
+      const page = await methods.listAuditLogPage('tenant-a', { search: 'risky.dot+plus' });
+      expect(page.total).toBe(1);
+    });
+  });
+
+  describe('streamAuditLogEntries', () => {
+    it('invokes onEntry for every match without skipping tenant boundaries', async () => {
+      await methods.recordAuditEntry(baseInput({ capability: 'manage:users' }));
+      await methods.recordAuditEntry(baseInput({ capability: 'manage:roles' }));
+      await methods.recordAuditEntry(baseInput({ tenantId: 'tenant-b' }));
+
+      const collected: string[] = [];
+      const count = await methods.streamAuditLogEntries('tenant-a', {}, (entry) => {
+        collected.push(entry.capability);
+      });
+      expect(count).toBe(2);
+      expect(new Set(collected)).toEqual(new Set(['manage:users', 'manage:roles']));
+    });
+  });
+});

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

@@ -226,7 +226,9 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')) {
   }
 
   /**
-   * Revoke a capability from a principal.
+   * Revoke a capability from a principal. Returns the deletion result so
+   * callers (e.g. audit emission) can distinguish a real revoke from a
+   * no-op against a grant that didn't exist.
    */
   async function revokeCapability(
     {
@@ -241,7 +243,7 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')) {
       tenantId?: string;
     },
     session?: ClientSession,
-  ): Promise<void> {
+  ): Promise<{ deletedCount: number }> {
     const SystemGrant = mongoose.models.SystemGrant as Model<ISystemGrant>;
 
     const normalizedPrincipalId = normalizePrincipalId(principalId, principalType);
@@ -254,7 +256,8 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')) {
     };
 
     const options = session ? { session } : {};
-    await SystemGrant.deleteOne(filter, options);
+    const result = await SystemGrant.deleteOne(filter, options);
+    return { deletedCount: result.deletedCount ?? 0 };
   }
 
   /**

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

@@ -51,15 +51,20 @@ const auditLogSchema = new Schema<IAuditLog>(
       type: String,
       required: false,
       validate: {
-        validator: (v: unknown) => v !== null && v !== '',
-        message: 'tenantId must be a non-empty string or omitted entirely — never null or empty',
+        validator: (v: unknown) => typeof v === 'string' && v.trim().length > 0,
+        message:
+          'tenantId must be a non-empty string or omitted entirely — never null, empty, or a non-string value',
       },
     },
   },
   { timestamps: true },
 );
 
-/** Primary listing: tenant + newest-first + tiebreak on _id for keyset pagination. */
+/**
+ * Primary listing: tenant + newest-first sort with `_id` as a deterministic
+ * tiebreak for offset pagination so adjacent pages never duplicate or skip rows
+ * when two entries share a `createdAt` timestamp.
+ */
 auditLogSchema.index({ tenantId: 1, createdAt: -1, _id: -1 });
 
 /** Target facet: filter by who was affected. */

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

@@ -1,7 +1,6 @@
 import type { Document, Types } from 'mongoose';
 import type { PrincipalType } from 'librechat-data-provider';
-
-export type AuditAction = 'grant_assigned' | 'grant_removed';
+import type { AuditAction } from './admin';
 
 export type AuditLog = {
   action: AuditAction;