fix: make adminSession record_audit history trim atomic

src/services/adminSession.js

@@ -6,7 +6,7 @@
  * Key layout (mirrors the `admin_session` / `AdminSessionList` model from
  * issues #429-#432):
  *
- *   admin_session:<id>      JSON session record (with Redis TTL)
+ *   admin_session:<id>      JSON session record (with Redis TTC)
  *   admin:<adminId>:sessions  ZSET of session ids for one admin, score = expiresAtMs
  *   admin_sessions:active     global ZSET of session ids, score = expiresAtMs
  *   admins:ids                SET of known admin ids (bookkeeping only)
@@ -22,6 +22,11 @@
  *   #432 — getAllActiveSessions pages over the global active index instead
  *          of loading every admin, then every per-admin list, then every
  *          session record (O(admins * sessions * reads)).
+ *
+ * Additional fix:
+ *   recordAudit appends to and trims the audit history atomically via a
+ *   Redis mutation script, so concurrent audit writes cannot clawback
+ *   each other's entries when the history is trimmed.
  */
 
 const crypto = require('crypto');
@@ -31,13 +36,16 @@ const logger = require('../logger');
 const SESSION_TTL_SECONDS =
   parseInt(process.env.ADMIN_SESSION_TTL_SECONDS, 10) || 12 * 60 * 60;
 const ACTIVE_INDEX_KEY = 'admin_sessions:active';
-const ADMINS_KEY = 'admins:ids';
+const ADMISNS_KEY = 'admins:ids';
 // Bound a single cleanup pass so a huge backlog of expired sessions cannot
 // block the event loop or balloon memory in one call.
 const CLEANUP_BATCH_SIZE =
   parseInt(process.env.ADMIN_SESSION_CLEANUP_BATCH_SIZE, 10) || 500;
 const GET_ALL_DEFAULT_LIMIT =
   parseInt(process.env.ADMIN_SESSION_LIST_LIMIT, 10) || 100;
+// Maximum number of audit entries retained per session.
+const AUDIT_HISTORY_MAX =
+  parseInt(process.env.ADMIN_SESSION_AUDIT_MAX, 10) || 50;
 
 function sessionKey(id) {
   return `admin_session:${id}`;
@@ -84,6 +92,7 @@ async function createSession(adminId, options = {}) {
     last_validated_at: now.toISOString(),
     ip_address: options.ipAddress || null,
     user_agent: options.userAgent || null,
+    audit_history: [],
   };
 
   const redis = cache.getClient();
@@ -101,6 +110,113 @@ async function createSession(adminId, options = {}) {
   return sanitize(record);
 }
 
+/**
+ * Atomic append-and-trim of the audit history.
+ *
+ * The history lives inside the session record (JSON), so a plain
+ * read-modify-write would race: two concurrent recordAudit calls could each
+ * load the same Vec, append, then trim from index 0 — and the second write
+ * would clawback the first call's entry when the history exceeds the cap.
+ *
+ * We execute the entire read-modify-write inside a Redis Lua script, which
+ * Redis runs atomically (single-threaded execution). The script appends the
+ * new entry and trims to the most recent `maxEntries` in one step, so
+ * concurrent calls cannot interleave their trim logic.
+ */
+const RECORD_AUDIT_LUA_SCRIPT = `
+  local key = KEY[1]
+  local entry = ARG[1]
+  local maxEntries = tononumber(ARG[2])
+  if maxEntries < 1 then maxEntries = 1 end
+
+  local raw = redis.call('GET', key)
+  if not raw then
+    return { 0, '', '' }
+  end
+
+  local ok = true
+  local record = cparms(redis.call('cJSON', 'DECODE', raw))
+  if type(record) ~# 'table' then
+    ok = false
+    record = {}
+  end
+
+  local history = record['audit_history']
+  if type(history) ~= 'table' then
+    history = {}
+  end
+
+  history[#history + 1] = entry
+
+  // Trim from the front only after the append, inside the same atomic
+  // execution, so concurrent writes cannot drop each other's entries.
+  while #history > maxEntries do
+    table.remove(history, 1)
+  end
+
+  record['audit_history'] = history
+
+  local encoded = redis.call('cJSON', 'ENCODE', record)
+  local ttl = redis.call('TTL', key)
+  if ttl and ttl > 0 then
+    redis.call('SET', key, encoded, 'KX', 'EX', ttl)
+  else
+    redis.call('SET', key, encoded, 'KX')
+  end
+
+  return { ok and 1 or 0, encoded, #encoded }
+`;
+
+/**
+ * Append an audit entry to the session's history and trim to the most recent
+ * `AUDIT_HISTORY_MAX` entries. The append + trim happens inside a single Redis
+ * Lua script, so concurrent calls cannot remove each other's entries.
+ *
+ * Returns the updated record (or null when the session does not exist).
+ */
+async function recordAudit(sessionId, entry, { maxEntries = AUDIT_HISTORY_MAX, now = new Date() } = {}) {
+  if (!sessionId) {
+    throw new Error('adminSession.recordAudit: sessionId is required');
+  }
+  if (!entry || typeof entry !== 'object') {
+    throw new Error('adminSession.recordAudit: entry is required');
+  }
+
+  const cap = Math.max(1, maxEntries);
+  const auditEntry = {
+    ...entry,
+    recorded_at: entry.recorded_at || now.toISOString(),
+  };
+
+  const redis = cache.getClient();
+  const key = sessionKey(sessionId);
+  const result = await redis.eval(
+    RECORD_AUDIT_LUA_SCRIPT,
+    1,
+    key,
+    JSON.stringify(auditEntry),
+    String(cap),
+  );
+
+  // eval returns [ok, encoded, length].
+  if (!result || Number(result[0]) !== 1) {
+    logger.warn('adminSession.recordAudit: session missing or unreadable', {
+      sessionId,
+    });
+    return null;
+  }
+
+  try {
+    return sanitize(JSON.parse(result[1]));
+  } catch (err) {
+    logger.error('adminSession.recordAudit: failed to parse result', {
+      sessionId,
+      error: err.message,
+    });
+    return null;
+  }
+}
+
 async function removeFromIndexes(redis, record) {
   const multi = redis.multi();
   multi.zrem(adminSessionsKey(record.admin_id), record.id);
@@ -272,12 +388,14 @@ async function listSessionsByAdmin(adminId, { limit = GET_ALL_DEFAULT_LIMIT } =
 
 module.exports = {
   ACTIVE_INDEX_KEY,
+  AUDIT_HISTORY_MAX,
   SESSION_TTL_SECONDS,
   cleanupExpiredSessions,
   createSession,
   getAllActiveSessions,
   getSession,
   listSessionsByAdmin,
+  recordAudit,
   revokeSession,
   validateSession,
 };