🛶 fix: Keep Trigger Bans Off Shared Loopback IPs (#16535)

Co-authored-by: Lia <lia@librechat.ai>

api/cache/banViolation.js

@@ -1,6 +1,6 @@
 const { logger } = require('@librechat/data-schemas');
 const { ViolationTypes } = require('librechat-data-provider');
-const { isEnabled, math, removePorts } = require('@librechat/api');
+const { isEnabled, math, removePorts, getBanIp } = require('@librechat/api');
 const { deleteAllUserSessions } = require('~/models');
 const getLogStores = require('./getLogStores');
 
@@ -65,6 +65,7 @@ const banViolation = async (req, res, errorMessage) => {
   }
 
   req.ip = removePorts(req);
+  const banIp = getBanIp(req);
   logger.info(
     `[BAN] Banning user ${user_id} ${req.ip ? `@ ${req.ip} ` : ''}for ${
       duration / 1000 / 60
@@ -73,8 +74,8 @@ const banViolation = async (req, res, errorMessage) => {
 
   const expiresAt = Date.now() + duration;
   await banLogs.set(user_id, { type, violation_count, duration, expiresAt });
-  if (req.ip) {
-    await banLogs.set(req.ip, { type, user_id, violation_count, duration, expiresAt });
+  if (banIp) {
+    await banLogs.set(banIp, { type, user_id, violation_count, duration, expiresAt });
   }
 
   errorMessage.ban = true;

api/cache/banViolation.spec.js

@@ -1,5 +1,8 @@
 const mongoose = require('mongoose');
 const { MongoMemoryServer } = require('mongodb-memory-server');
+const { ViolationTypes } = require('librechat-data-provider');
+const { deleteAllUserSessions } = require('~/models');
+const getLogStores = require('./getLogStores');
 const banViolation = require('./banViolation');
 
 // Mock deleteAllUserSessions since we're testing ban logic, not session deletion
@@ -81,6 +84,40 @@ describe('banViolation', () => {
     expect(errorMessage.ban).toBeTruthy();
   });
 
+  it.each(['127.0.0.1', '::1', '::ffff:127.0.0.1'])(
+    'bans only the user for a verified trigger request from %s',
+    async (ip) => {
+      const banLogs = getLogStores(ViolationTypes.BAN);
+      await banLogs.delete(ip);
+      req.ip = ip;
+      req._isAgentTrigger = true;
+      errorMessage.violation_count = 20;
+
+      await banViolation(req, res, errorMessage);
+
+      expect(await banLogs.get(errorMessage.user_id)).toEqual(
+        expect.objectContaining({ violation_count: 20 }),
+      );
+      expect(await banLogs.get(ip)).toBeUndefined();
+      expect(deleteAllUserSessions).toHaveBeenCalledWith({ userId: errorMessage.user_id });
+      expect(res.clearCookie).toHaveBeenCalledWith('refreshToken');
+      expect(errorMessage.ban).toBe(true);
+      expect(req.ip).toBe(ip);
+    },
+  );
+
+  it('still bans the IP when an ordinary request sends a trigger header', async () => {
+    req.headers = { 'x-lc-agent-trigger': '1' };
+    errorMessage.violation_count = 20;
+
+    await banViolation(req, res, errorMessage);
+
+    const banLogs = getLogStores(ViolationTypes.BAN);
+    expect(await banLogs.get(req.ip)).toEqual(
+      expect.objectContaining({ user_id: errorMessage.user_id, violation_count: 20 }),
+    );
+  });
+
   it('should handle invalid BAN_INTERVAL and default to 20', async () => {
     process.env.BAN_INTERVAL = 'invalid';
     errorMessage.prev_count = 19;

api/server/middleware/checkBan.js

@@ -2,7 +2,7 @@ const { Keyv } = require('keyv');
 const uap = require('ua-parser-js');
 const { logger } = require('@librechat/data-schemas');
 const { ErrorTypes, ViolationTypes } = require('librechat-data-provider');
-const { isEnabled, keyvMongo, removePorts } = require('@librechat/api');
+const { isEnabled, keyvMongo, removePorts, getBanIp } = require('@librechat/api');
 const { getLogStores } = require('~/cache');
 const { isOAuthNavigation, redirectOAuthFailure } = require('./oauthNavigation');
 const denyRequest = require('./denyRequest');
@@ -88,6 +88,7 @@ const checkBan = async (req, res, next = () => {}) => {
     }
 
     req.ip = removePorts(req);
+    const banIp = getBanIp(req);
     let userId = req.user?.id ?? req.user?._id?.toString() ?? null;
 
     if (!userId && req?.body?.email) {
@@ -95,12 +96,12 @@ const checkBan = async (req, res, next = () => {}) => {
       userId = user?._id ? user._id.toString() : userId;
     }
 
-    if (!userId && !req.ip) {
+    if (!userId && !banIp) {
       return next();
     }
 
     const useRedis = isEnabled(process.env.USE_REDIS);
-    const ipKey = getBanCacheKey('ip', req.ip, useRedis);
+    const ipKey = getBanCacheKey('ip', banIp, useRedis);
     const userKey = getBanCacheKey('user', userId, useRedis);
 
     const [cachedIPBan, cachedUserBan] = await Promise.all([
@@ -121,7 +122,7 @@ const checkBan = async (req, res, next = () => {}) => {
     }
 
     const [ipBan, userBan] = await Promise.all([
-      req.ip ? banLogs.get(req.ip) : undefined,
+      banIp ? banLogs.get(banIp) : undefined,
       userId ? banLogs.get(userId) : undefined,
     ]);
 
@@ -142,7 +143,7 @@ const checkBan = async (req, res, next = () => {}) => {
     if (timeLeft <= 0) {
       const cleanups = [];
       if (ipBan) {
-        cleanups.push(banLogs.delete(req.ip));
+        cleanups.push(banLogs.delete(banIp));
       }
       if (userBan) {
         cleanups.push(banLogs.delete(userId));

api/server/middleware/checkBan.spec.js

@@ -75,6 +75,38 @@ describe('checkBan with namespaced Keyv stores and ObjectId user ids', () => {
     expect(req.banned).toBeUndefined();
   });
 
+  it('isolates concurrent trigger violations to their user on the shared loopback transport', async () => {
+    const userId = new mongoose.Types.ObjectId();
+    const req = {
+      ip: '::1',
+      user: { id: userId.toString() },
+      headers: {},
+      _isAgentTrigger: true,
+    };
+    const errorMessage = { type: ViolationTypes.CONCURRENT };
+
+    await logViolation(req, createRes(), ViolationTypes.CONCURRENT, errorMessage, 20);
+    expect(errorMessage.ban).toBe(true);
+
+    const otherUserReq = {
+      ...req,
+      user: { id: new mongoose.Types.ObjectId().toString() },
+    };
+    const next = jest.fn();
+    await checkBan(otherUserReq, createRes(), next);
+
+    expect(next).toHaveBeenCalledWith();
+    expect(otherUserReq.banned).toBeUndefined();
+
+    const bannedUserRes = createRes();
+    const bannedUserNext = jest.fn();
+    await checkBan(req, bannedUserRes, bannedUserNext);
+
+    expect(bannedUserNext).not.toHaveBeenCalled();
+    expect(bannedUserRes.status).toHaveBeenCalledWith(403);
+    expect(req.banned).toBe(true);
+  });
+
   it('enforces a ban recorded for the same user from another address', async () => {
     const userId = new mongoose.Types.ObjectId();
     const violationReq = createOAuthCallbackReq(userId, '10.0.0.2');

api/test/server/middleware/checkBan.test.js

@@ -40,6 +40,7 @@ jest.mock('@librechat/api', () => ({
     return false;
   },
   keyvMongo: {},
+  getBanIp: jest.requireActual('../../../../packages/api/src/middleware/ban').getBanIp,
   removePorts: jest.fn((req) => req.ip),
   redirectToAuthFailure: (res, { clientDomain, authFailedError }) =>
     res.redirect(`${clientDomain}/login?redirect=false&error=${authFailedError}`),
@@ -141,6 +142,93 @@ describe('checkBan middleware', () => {
     });
   });
 
+  describe.each([false, true])('verified trigger requests (USE_REDIS=%s)', (useRedis) => {
+    beforeEach(() => {
+      process.env.USE_REDIS = String(useRedis);
+    });
+
+    const userKey = useRedis ? 'ban_cache:user:user123' : 'user123';
+    const ipKey = useRedis ? 'ban_cache:ip:127.0.0.1' : '127.0.0.1';
+    const createTriggerReq = () => createReq({ ip: '127.0.0.1', _isAgentTrigger: true });
+
+    it('ignores cached and persistent loopback bans without querying either IP key', async () => {
+      const ban = { expiresAt: Date.now() + 60000 };
+      mockBanCacheGet.mockImplementation(async (key) => (key === ipKey ? ban : undefined));
+      mockBanLogsGet.mockImplementation(async (key) => (key === '127.0.0.1' ? ban : undefined));
+      const req = createTriggerReq();
+      const next = jest.fn();
+
+      await checkBan(req, createRes(), next);
+
+      expect(next).toHaveBeenCalledWith();
+      expect(req.banned).toBeUndefined();
+      expect(req.ip).toBe('127.0.0.1');
+      expect(mockBanCacheGet.mock.calls).toEqual([[userKey]]);
+      expect(mockBanLogsGet.mock.calls).toEqual([['user123']]);
+      expect(mockBanCacheSet).not.toHaveBeenCalled();
+    });
+
+    it('still rejects a cached user ban', async () => {
+      mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
+      const res = createRes();
+      const next = jest.fn();
+
+      await checkBan(createTriggerReq(), res, next);
+
+      expect(next).not.toHaveBeenCalled();
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(mockBanCacheGet.mock.calls).toEqual([[userKey]]);
+      expect(mockBanLogsGet).not.toHaveBeenCalled();
+    });
+
+    it('caches a persistent user ban only under the user key', async () => {
+      const ban = { expiresAt: Date.now() + 60000 };
+      mockBanLogsGet.mockResolvedValueOnce(ban);
+      const res = createRes();
+      const next = jest.fn();
+
+      await checkBan(createTriggerReq(), res, next);
+
+      expect(next).not.toHaveBeenCalled();
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(mockBanCacheSet).toHaveBeenCalledTimes(1);
+      expect(mockBanCacheSet).toHaveBeenCalledWith(userKey, ban, expect.any(Number));
+    });
+
+    it('cleans up an expired user ban without touching the transport IP', async () => {
+      mockBanLogsGet.mockResolvedValueOnce({ expiresAt: Date.now() - 1000 });
+      const next = jest.fn();
+
+      await checkBan(createTriggerReq(), createRes(), next);
+
+      expect(next).toHaveBeenCalledWith();
+      expect(mockBanLogsDelete.mock.calls).toEqual([['user123']]);
+      expect(mockBanCacheSet).not.toHaveBeenCalled();
+    });
+
+    it('does not treat a trigger header on an ordinary request as an exemption', async () => {
+      mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
+      const req = createReq({
+        ip: '127.0.0.1',
+        headers: { 'x-lc-agent-trigger': '1' },
+        _isAgentTrigger: false,
+      });
+      const res = createRes();
+      const next = jest.fn();
+
+      await checkBan(req, res, next);
+
+      expect(next).not.toHaveBeenCalled();
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(mockBanCacheGet).toHaveBeenCalledWith(ipKey);
+    });
+
+    afterEach(() => {
+      mockBanCacheGet.mockReset().mockResolvedValue(undefined);
+      mockBanLogsGet.mockReset().mockResolvedValue(undefined);
+    });
+  });
+
   describe('cache hit path', () => {
     it('returns 403 when IP ban is cached', async () => {
       mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });

packages/api/src/middleware/ban.spec.ts

@@ -0,0 +1,25 @@
+import { getBanIp } from './ban';
+
+describe('getBanIp', () => {
+  it.each(['127.0.0.1', '::1', '::ffff:127.0.0.1'])(
+    'excludes the shared address %s only for verified trigger requests',
+    (ip) => {
+      const req = { ip, _isAgentTrigger: true };
+
+      expect(getBanIp(req)).toBeUndefined();
+      expect(req.ip).toBe(ip);
+      expect(getBanIp({ ip, _isAgentTrigger: false })).toBe(ip);
+      expect(getBanIp({ ip })).toBe(ip);
+    },
+  );
+
+  it('does not trust a trigger transport header without a verified identity', () => {
+    const req = { ip: '127.0.0.1', headers: { 'x-lc-agent-trigger': '1' } };
+
+    expect(getBanIp(req)).toBe(req.ip);
+  });
+
+  it('preserves a missing IP address', () => {
+    expect(getBanIp({ ip: undefined })).toBeUndefined();
+  });
+});

packages/api/src/middleware/ban.ts

@@ -0,0 +1,8 @@
+import type { Request } from 'express';
+
+/** Only the router-verified trigger identity exempts the shared delivery transport from IP bans. */
+export function getBanIp(
+  req: Pick<Request, 'ip'> & { _isAgentTrigger?: boolean },
+): string | undefined {
+  return req._isAgentTrigger === true ? undefined : req.ip;
+}

packages/api/src/middleware/index.ts

@@ -5,6 +5,7 @@ export * from './email';
 export * from './notFound';
 export * from './origin';
 export * from './balance';
+export * from './ban';
 export * from './json';
 export * from './capabilities';
 export * from './auth';