Code Review: Admin Audit Router (`apps/api/src/routers/admin/audit.ts`)

Reviewed Date: April 24, 2026
File: apps/api/src/routers/admin/audit.ts
Lines of Code: ~730
Endpoints: 8 admin endpoints

Overview

The audit.ts file contains 8 admin endpoints for monitoring and analytics:

/stats - Dashboard statistics with complex aggregations
/queue-depths - BullMQ queue depths
/audit-logs - Audit logs with filtering
/audit-logs/live - SSE stream for real-time audit logs
/activities - List recent system activities
/approvals - List pending approvals
/overview-stats - Overview statistics
/analytics/agents - Platform-wide agent analytics

Critical Issues

1. File Naming & Organization Mismatch ⚠️

Severity: High
Location: Entire file

Issue:

File named audit.ts but contains 8 different endpoint types (stats, queues, activities, approvals, analytics)
Violates single responsibility principle
Makes code difficult to navigate and maintain

Recommendation: Split into logical modules:

audit-logs.ts - Audit log endpoints
stats.ts - Dashboard statistics
analytics.ts - Agent analytics
activities.ts - Activities & approvals

2. Missing Error Handling ⚠️

Severity: Critical
Location: All endpoints (lines 125-730)

Issue:

No try-catch blocks around database queries or external calls
BullMQ queue operations can fail silently (lines 344-368)
SSE endpoint has no error recovery mechanism
Database errors propagate raw to client

Recommendation:


fastify.get("/stats", { schema: S.stats }, async (req, reply) => {
  try {
    const userId = await requireSuperAdmin(req, reply);
    if (!userId) return;
    
    // ... existing logic ...
    
  } catch (error) {
    logger.error("Failed to fetch admin stats", { error, userId });
    return reply.status(500).send({ 
      error: "Failed to fetch statistics",
      code: "STATS_FETCH_ERROR"
    });
  }
});

3. Security & Type Safety ⚠️

Severity: High
Location: Schema definitions & type assertions

Issue:

Schemas use additionalProperties: true - too permissive, exposes internal fields
Type assertions like as never (line 365) bypass type checking
Unsafe type casting: (l._id as { toString(): string }).toString() repeated throughout

Recommendation:


// Define proper response types
interface AuditLogResponse {
  id: string;
  actorId: string;
  actorName: string;
  action: string;
  // ... explicit fields only
}
 
// Use typed schema
const AuditLogSchema = {
  type: "object",
  properties: {
    id: { type: "string" },
    actorId: { type: "string" },
    // ... no additionalProperties
  },
  required: ["id", "actorId", "action"],
  additionalProperties: false, // Explicit!
};

4. Memory Leaks in SSE Endpoint ⚠️

Severity: Critical
Location: Lines 451-484

Issue:

Returns Promise<void>(() => {}) that never resolves
Connection stays open indefinitely without timeout
No mechanism to clean up abandoned connections
Could exhaust server resources

Recommendation:


fastify.get("/audit-logs/live", { schema: S.auditLogsLive }, async (req, reply) => {
  const userId = await requireSuperAdmin(req, reply);
  if (!userId) return;
 
  const raw = reply.raw;
  const CONNECTION_TIMEOUT = 30 * 60 * 1000; // 30 minutes
  const HEARTBEAT_INTERVAL = 25000;
  
  raw.setHeader("Content-Type", "text/event-stream");
  raw.setHeader("Cache-Control", "no-cache");
  raw.setHeader("Connection", "keep-alive");
  raw.setHeader("X-Accel-Buffering", "no");
  raw.flushHeaders();
 
  raw.write(`data: ${JSON.stringify({ type: "connected" })}\n\n`);
 
  const unsubscribe = onAuditEvent((message) => {
    try { 
      raw.write(`data: ${message}\n\n`); 
    } catch (err) {
      logger.warn("SSE write failed, client disconnected", { userId });
      cleanup();
    }
  });
 
  const heartbeat = setInterval(() => {
    try { 
      raw.write(": heartbeat\n\n"); 
    } catch {
      cleanup();
    }
  }, HEARTBEAT_INTERVAL);
 
  // Auto-disconnect after timeout
  const timeout = setTimeout(() => {
    logger.info("SSE connection timeout", { userId });
    cleanup();
  }, CONNECTION_TIMEOUT);
 
  const cleanup = () => {
    unsubscribe();
    clearInterval(heartbeat);
    clearTimeout(timeout);
    try { raw.end(); } catch {}
  };
 
  req.raw.on("close", cleanup);
  req.raw.on("error", cleanup);
 
  // Return a promise that resolves when connection closes
  return new Promise<void>((resolve) => {
    req.raw.on("close", () => {
      cleanup();
      resolve();
    });
  });
});

5. Performance Issues ⚠️

Severity: High
Location: /stats endpoint (lines 125-319)

Issue:

Makes 18 sequential database queries
Many queries could be parallelized or combined
No caching for expensive aggregations
Full table scans without proper indexes assumed

Current:


const [
  totalTenants,
  activeAgents,
  totalUsers,
  // ... 18 queries total
] = await Promise.all([...]);

Recommendation:

Add Redis caching:


const CACHE_KEY = "admin:stats";
const CACHE_TTL = 5 * 60; // 5 minutes
 
const cached = await redis.get(CACHE_KEY);
if (cached) {
  return reply.send(JSON.parse(cached));
}
 
// ... compute stats ...
 
await redis.setex(CACHE_KEY, CACHE_TTL, JSON.stringify(result));

Combine related queries:


// Instead of separate counts, use groupBy
const tenantStats = await db.tenant.groupBy({
  by: ["status"],
  _count: { _all: true },
  where: {
    OR: [
      { createdAt: { gte: monthStart } },
      { createdAt: { gte: lastMonthStart, lt: monthStart } }
    ]
  }
});

Major Issues

6. Hardcoded Values

Severity: Medium
Location: Throughout file

Issue:

Magic numbers: 25000 (heartbeat), 100 (max limit), 8 (recent logs), 5 (stuck tenants)
Hardcoded agent roles list (lines 334-346) duplicates type from @leadmetrics/queue
Status strings like "onboarding", "active" should be enums/constants

Recommendation:


// constants.ts
export const ADMIN_CONSTANTS = {
  SSE_HEARTBEAT_MS: 25_000,
  MAX_PAGE_SIZE: 100,
  DEFAULT_PAGE_SIZE: 50,
  RECENT_LOGS_LIMIT: 8,
  STUCK_TENANT_THRESHOLD_HOURS: 48,
  STATS_CACHE_TTL_SECONDS: 300,
} as const;
 
// Use AgentRole from queue package
import type { AgentRole } from "@leadmetrics/queue";
const MONITORED_AGENT_ROLES: AgentRole[] = [
  "client-researcher",
  "context-file-writer",
  // ...
];

7. Code Duplication

Severity: Medium
Location: Lines 506-520, 546-560, 587-601

Issue:

Pagination logic repeated 3 times
Date range calculations duplicated
Similar query patterns for activities/approvals endpoints

Recommendation:


// utils/pagination.ts
export function parsePaginationParams(query: Record<string, string | undefined>) {
  const page = Math.max(1, parseInt(query.page ?? "1", 10));
  const limit = Math.min(
    ADMIN_CONSTANTS.MAX_PAGE_SIZE, 
    Math.max(1, parseInt(query.limit ?? String(ADMIN_CONSTANTS.DEFAULT_PAGE_SIZE), 10))
  );
  const skip = (page - 1) * limit;
  return { page, limit, skip };
}
 
export function buildPaginationResponse<T>(
  data: T[],
  total: number,
  page: number,
  limit: number
) {
  return {
    data,
    pagination: {
      total,
      page,
      limit,
      totalPages: Math.ceil(total / limit),
    },
  };
}

8. Complex Business Logic in Routes

Severity: High
Location: /stats endpoint (lines 125-319)

Issue:

200+ lines of aggregation logic directly in route handler
MRR calculation, failure analysis, daily stats should be in service layer
Makes testing difficult
Violates separation of concerns

Recommendation:


// services/admin-stats.service.ts
export class AdminStatsService {
  async getDashboardStats() {
    const [baseStats, businessHealth, operational] = await Promise.all([
      this.getBaseStats(),
      this.getBusinessHealth(),
      this.getOperationalMetrics(),
    ]);
    
    return {
      ...baseStats,
      ...businessHealth,
      ...operational,
      recentActivity: await this.getRecentActivity(),
    };
  }
  
  private async getBusinessHealth() {
    // MRR calculation logic
  }
  
  private async getOperationalMetrics() {
    // Agent run stats
  }
}
 
// In route handler
fastify.get("/stats", { schema: S.stats }, async (req, reply) => {
  const userId = await requireSuperAdmin(req, reply);
  if (!userId) return;
  
  const stats = await adminStatsService.getDashboardStats();
  return reply.send(stats);
});

9. Input Validation Gaps

Severity: Medium
Location: Query parameter parsing

Issue:

Query params parsed but not validated (e.g., date format on line 412)
No bounds checking beyond schema
Invalid period values default silently (line 620)

Recommendation:


// Add proper validation
const querySchema = {
  type: "object",
  properties: {
    from: { type: "string", format: "date-time" },
    to: { type: "string", format: "date-time" },
    period: { 
      type: "string", 
      enum: ["day", "week", "month", "3month"] 
    },
  },
};
 
// Validate dates
if (query.from) {
  const fromDate = new Date(query.from);
  if (isNaN(fromDate.getTime())) {
    return reply.status(400).send({ error: "Invalid 'from' date format" });
  }
}

10. Inconsistent Response Formats

Severity: Medium
Location: All endpoints

Issue:

Some endpoints use { data, pagination } pattern
Others return flat objects
No TypeScript interfaces for responses

Recommendation:


// types/admin-responses.ts
export interface PaginatedResponse<T> {
  data: T[];
  pagination: {
    total: number;
    page: number;
    limit: number;
    totalPages: number;
  };
}
 
export interface StatsResponse {
  totalTenants: number;
  activeAgents: number;
  // ... all fields explicitly typed
}
 
// Use in route
async (req, reply): Promise<PaginatedResponse<AuditLog>> => {
  // ...
}

Minor Issues

11. Poor Variable Naming

Severity: Low
Location: Throughout

Issue:

S for schemas (line 7) is cryptic
s, r, l for aggregate/run/log variables
entry, key used inconsistently

Recommendation:


// Before
const S = { getAuditLogs: { ... } };
for (const r of agentFailures24hRaw) { ... }
 
// After
const AdminAuditSchemas = { getAuditLogs: { ... } };
for (const failedRun of agentFailures24hRaw) { ... }

12. Regex Security

Severity: Low
Location: Line 404

Issue:

Manual regex escaping - error-prone
Case-insensitive search creates RegExp per request

Recommendation:


import escapeStringRegexp from "escape-string-regexp";
 
if (query.search) {
  const re = new RegExp(escapeStringRegexp(query.search), "i");
  filter["$or"] = [
    { actorName: re },
    { tenantName: re },
    { resourceName: re },
    { action: re },
  ];
}

13. Missing Documentation

Severity: Low
Location: All functions

Issue:

No JSDoc comments
Schema descriptions minimal
Complex calculations unexplained

Recommendation:


/**
 * Get platform-wide dashboard statistics.
 * 
 * Includes:
 * - Tenant/user counts by status
 * - MRR by currency
 * - Agent run metrics (7-day success rate)
 * - Financial health (pending/overdue invoices)
 * - Operational alerts (stuck onboarding, failures)
 * 
 * @returns Comprehensive stats object
 * @cache 5 minutes (Redis)
 * @performance ~200ms for 10K tenants
 */
fastify.get("/stats", ...);

14. Testing Concerns

Severity: Medium
Location: Entire file

Issue:

Tight coupling to database makes unit testing hard
No dependency injection
Time-based logic uses new Date() directly

Recommendation:


// Use dependency injection
export function createAdminAuditRouter(
  db: PrismaClient,
  auditLog: typeof AuditLog,
  queueFactory: typeof getQueue,
  timeProvider: () => Date = () => new Date()
) {
  return async function adminAuditRouter(fastify: FastifyInstance) {
    fastify.get("/stats", async (req, reply) => {
      const now = timeProvider();
      // ... rest of logic
    });
  };
}
 
// In tests
const mockTimeProvider = () => new Date("2026-04-24T00:00:00Z");
const router = createAdminAuditRouter(mockDb, mockAuditLog, mockQueue, mockTimeProvider);

Recommended Improvements

Immediate (High Priority)

✅ Add try-catch blocks with proper error logging
✅ Fix SSE endpoint memory leak - add timeout & connection limit
✅ Extract hardcoded values to constants file
✅ Add proper TypeScript interfaces for all responses
✅ Fix type safety issues - remove as casts

Short-term (Medium Priority)

✅ Split file into logical modules: stats.ts, audit-logs.ts, analytics.ts, activities.ts
✅ Extract business logic to service layer
✅ Create shared pagination utility
✅ Add caching for expensive /stats aggregations (Redis with 5min TTL)
✅ Use AgentRole type from @leadmetrics/queue instead of hardcoded list

Long-term (Architecture)

🔄 Implement repository pattern for database queries
🔄 Add integration tests for all endpoints
🔄 Consider GraphQL or tRPC for better type safety
🔄 Implement rate limiting for expensive analytics endpoints
🔄 Add OpenTelemetry tracing for performance monitoring

Code Style

📝 Use descriptive variable names
📝 Add JSDoc comments for complex functions
📝 Extract magic numbers to named constants
📝 Consistent error response format

Summary

Overall Assessment: ⚠️ Needs Significant Improvement

The file is functional but has several critical issues that should be addressed:

Security: Missing error handling, type safety issues
Performance: No caching, inefficient queries
Maintainability: Poor organization, complex logic in routes
Reliability: Memory leaks in SSE endpoint

Priority Actions:

Fix memory leak in SSE endpoint (Critical)
Add comprehensive error handling (Critical)
Split into multiple files (High)
Add caching layer (High)
Extract to service layer (Medium)

Estimated Refactor Time: 2-3 days for high-priority items

Legend:

✅ Can be done immediately
🔄 Requires architectural planning
📝 Quick wins / code quality improvements

Code Review: Admin Audit Router (apps/api/src/routers/admin/audit.ts)

Overview

Critical Issues

1. File Naming & Organization Mismatch ⚠️

2. Missing Error Handling ⚠️

3. Security & Type Safety ⚠️

4. Memory Leaks in SSE Endpoint ⚠️

5. Performance Issues ⚠️

Major Issues

6. Hardcoded Values

7. Code Duplication

8. Complex Business Logic in Routes

9. Input Validation Gaps

10. Inconsistent Response Formats

Minor Issues

11. Poor Variable Naming

12. Regex Security

13. Missing Documentation

14. Testing Concerns

Recommended Improvements

Immediate (High Priority)

Short-term (Medium Priority)

Long-term (Architecture)

Code Style

Summary

Code Review: Admin Audit Router (`apps/api/src/routers/admin/audit.ts`)