mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 20:24:21 +00:00
🛑 fix: Separate Agent Event Backpressure From User Bans (#15200)
* 🛑 fix: Separate Agent Event Backpressure From User Bans
* fix: Address Agent Event Review Findings
* fix: Mirror Case-Insensitive Agent Control Routing
This commit is contained in:
parent
e9dec7749a
commit
d9e6250d05
4 changed files with 199 additions and 13 deletions
|
|
@ -9,6 +9,8 @@ const { findUser } = require('~/models');
|
|||
|
||||
const banCache = new Keyv({ store: keyvMongo, namespace: ViolationTypes.BAN, ttl: 0 });
|
||||
const message = 'Your account has been temporarily banned due to violations of our service.';
|
||||
const AGENT_CHAT_PATH = '/api/agents/chat';
|
||||
const AGENT_CHAT_POST_CONTROL_ROUTES = new Set(['abort', 'steer']);
|
||||
|
||||
/** @returns {string} Cache key for ban lookups, prefixed for Redis or raw for MongoDB */
|
||||
const getBanCacheKey = (prefix, value, useRedis) => {
|
||||
|
|
@ -18,6 +20,28 @@ const getBanCacheKey = (prefix, value, useRedis) => {
|
|||
return useRedis ? `ban_cache:${prefix}:${value}` : value;
|
||||
};
|
||||
|
||||
/** Returns whether this request starts or resumes an interactive agent chat turn. */
|
||||
const isInteractiveAgentChatRequest = (req) => {
|
||||
if (req.method !== 'POST' || req.baseUrl !== '/api/agents' || req.body == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const pathname = req.originalUrl.split('?')[0].replace(/\/$/, '');
|
||||
if (pathname === AGENT_CHAT_PATH) {
|
||||
return true;
|
||||
}
|
||||
if (!pathname.startsWith(`${AGENT_CHAT_PATH}/`)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const route = pathname.slice(`${AGENT_CHAT_PATH}/`.length);
|
||||
return (
|
||||
route.length > 0 &&
|
||||
!route.includes('/') &&
|
||||
!AGENT_CHAT_POST_CONTROL_ROUTES.has(route.toLowerCase())
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Respond to the request if the user is banned.
|
||||
*
|
||||
|
|
@ -26,14 +50,13 @@ const getBanCacheKey = (prefix, value, useRedis) => {
|
|||
* @param {Object} req - Express Request object.
|
||||
* @param {Object} res - Express Response object.
|
||||
*
|
||||
* @returns {Promise<Object>} - Returns a Promise which when resolved sends a response status of 403 with a specific message if request is not of api/agents/chat. If it is, calls `denyRequest()` function.
|
||||
* @returns {Promise<Object>} - Returns a Promise which sends a JSON 403 unless this is an interactive browser agent chat request, in which case it calls `denyRequest()`.
|
||||
*/
|
||||
const banResponse = async (req, res) => {
|
||||
const ua = uap(req.headers['user-agent']);
|
||||
const { baseUrl, originalUrl } = req;
|
||||
if (!ua.browser.name) {
|
||||
return res.status(403).json({ message });
|
||||
} else if (baseUrl === '/api/agents' && originalUrl.startsWith('/api/agents/chat')) {
|
||||
} else if (isInteractiveAgentChatRequest(req)) {
|
||||
return await denyRequest(req, res, { type: ViolationTypes.BAN });
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -87,16 +87,22 @@ const agentEventUserLimiter = (req, res, next) => {
|
|||
configuredAgentEventUserLimiter = rateLimit({
|
||||
windowMs: windowInMinutes * 60 * 1000,
|
||||
max,
|
||||
handler: async (limitedReq, limitedRes) => {
|
||||
const type = ViolationTypes.MESSAGE_LIMIT;
|
||||
const errorMessage = {
|
||||
type,
|
||||
max,
|
||||
limiter: 'agent_event_principal',
|
||||
windowInMinutes,
|
||||
};
|
||||
await logViolation(limitedReq, limitedRes, type, errorMessage, score);
|
||||
return await denyRequest(limitedReq, limitedRes, errorMessage);
|
||||
handler: (limitedReq, limitedRes) => {
|
||||
const resetAt = limitedReq.rateLimit?.resetTime?.getTime?.();
|
||||
const retryAfterSeconds = Number.isFinite(resetAt)
|
||||
? Math.max(1, Math.ceil((resetAt - Date.now()) / 1000))
|
||||
: Math.max(1, Math.ceil(windowInMinutes * 60));
|
||||
limitedRes.set('Retry-After', String(retryAfterSeconds));
|
||||
return limitedRes
|
||||
.status(429)
|
||||
.type('application/json')
|
||||
.json({
|
||||
error: {
|
||||
code: 'agent_event_rate_limited',
|
||||
message: 'Agent event admission rate limit exceeded.',
|
||||
type: 'rate_limit_error',
|
||||
},
|
||||
});
|
||||
},
|
||||
keyGenerator: (limitedReq) => String(limitedReq.apiKeyId ?? limitedReq.user?.id),
|
||||
store: limiterCache('agent_event_user_limiter'),
|
||||
|
|
|
|||
|
|
@ -1,3 +1,6 @@
|
|||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
const mockLimiter = jest.fn((_req, _res, next) => next());
|
||||
const mockRateLimit = jest.fn(() => mockLimiter);
|
||||
|
||||
|
|
@ -10,10 +13,23 @@ jest.mock('~/server/middleware/denyRequest', () => jest.fn());
|
|||
jest.mock('~/cache', () => ({ logViolation: jest.fn() }));
|
||||
|
||||
describe('agent event rate limiter', () => {
|
||||
let originalEnv;
|
||||
|
||||
beforeEach(() => {
|
||||
originalEnv = { ...process.env };
|
||||
jest.resetModules();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = originalEnv;
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('reads YAML-projected limits lazily after startup configuration', () => {
|
||||
process.env.AGENT_EVENT_USER_MAX = '80';
|
||||
process.env.AGENT_EVENT_USER_WINDOW = '2';
|
||||
const { agentEventUserLimiter } = require('./messageLimiters');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const next = jest.fn();
|
||||
|
||||
agentEventUserLimiter({ apiKeyId: 'key-1' }, {}, next);
|
||||
|
|
@ -21,6 +37,39 @@ describe('agent event rate limiter', () => {
|
|||
expect(mockRateLimit).toHaveBeenLastCalledWith(
|
||||
expect.objectContaining({ max: 80, windowMs: 120_000 }),
|
||||
);
|
||||
expect(limiterCache).toHaveBeenCalledWith('agent_event_user_limiter');
|
||||
expect(mockLimiter).toHaveBeenCalledWith({ apiKeyId: 'key-1' }, {}, next);
|
||||
});
|
||||
|
||||
it('returns an actionable JSON 429 without recording a message violation', async () => {
|
||||
process.env.AGENT_EVENT_USER_MAX = '80';
|
||||
process.env.AGENT_EVENT_USER_WINDOW = '2';
|
||||
const { agentEventUserLimiter } = require('./messageLimiters');
|
||||
const { logViolation } = require('~/cache');
|
||||
const denyRequest = require('~/server/middleware/denyRequest');
|
||||
|
||||
agentEventUserLimiter({ apiKeyId: 'key-1' }, {}, jest.fn());
|
||||
const options = mockRateLimit.mock.calls.at(-1)[0];
|
||||
const app = express();
|
||||
app.use((req, _res, next) => {
|
||||
req.rateLimit = { resetTime: new Date(Date.now() + 30_000) };
|
||||
next();
|
||||
});
|
||||
app.post('/api/agents/v1/events', options.handler);
|
||||
const response = await request(app).post('/api/agents/v1/events');
|
||||
|
||||
expect(response.status).toBe(429);
|
||||
expect(response.headers['content-type']).toMatch(/^application\/json/);
|
||||
expect(response.headers['retry-after']).toBe('30');
|
||||
expect(response.body).toEqual({
|
||||
error: {
|
||||
code: 'agent_event_rate_limited',
|
||||
message: 'Agent event admission rate limit exceeded.',
|
||||
type: 'rate_limit_error',
|
||||
},
|
||||
});
|
||||
expect(response.text).not.toContain('event:');
|
||||
expect(logViolation).not.toHaveBeenCalled();
|
||||
expect(denyRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -53,11 +53,15 @@ jest.mock('ua-parser-js', () => jest.fn(() => ({ browser: { name: 'Chrome' } }))
|
|||
|
||||
const checkBan = require('~/server/middleware/checkBan');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { findUser } = require('~/models');
|
||||
const denyRequest = require('~/server/middleware/denyRequest');
|
||||
const uap = require('ua-parser-js');
|
||||
|
||||
const createReq = (overrides = {}) => ({
|
||||
ip: '192.168.1.1',
|
||||
user: { id: 'user123' },
|
||||
method: 'GET',
|
||||
headers: { 'user-agent': 'Mozilla/5.0' },
|
||||
body: {},
|
||||
baseUrl: '/api',
|
||||
|
|
@ -170,6 +174,110 @@ describe('checkBan middleware', () => {
|
|||
|
||||
expect(mockBanLogsGet).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(['/api/agents/chat/stream/stream-123', '/api/agents/chat/status/conversation-1'])(
|
||||
'returns JSON for a banned browser GET without a request body: %s',
|
||||
async (originalUrl) => {
|
||||
mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
|
||||
const req = createReq({
|
||||
body: undefined,
|
||||
baseUrl: '/api/agents',
|
||||
originalUrl,
|
||||
});
|
||||
const res = createRes();
|
||||
|
||||
await checkBan(req, res, jest.fn());
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
message: 'Your account has been temporarily banned due to violations of our service.',
|
||||
});
|
||||
expect(denyRequest).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it('preserves SSE denial for a banned browser interactive chat request', async () => {
|
||||
mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
|
||||
const req = createReq({
|
||||
method: 'POST',
|
||||
baseUrl: '/api/agents',
|
||||
originalUrl: '/api/agents/chat/agents',
|
||||
});
|
||||
const res = createRes();
|
||||
|
||||
await checkBan(req, res, jest.fn());
|
||||
|
||||
expect(denyRequest).toHaveBeenCalledWith(req, res, { type: ViolationTypes.BAN });
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(['active', 'status', 'stream'])(
|
||||
'preserves SSE denial when a custom endpoint uses the POST-only name %s',
|
||||
async (endpoint) => {
|
||||
mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
|
||||
const req = createReq({
|
||||
method: 'POST',
|
||||
baseUrl: '/api/agents',
|
||||
originalUrl: `/api/agents/chat/${endpoint}`,
|
||||
});
|
||||
const res = createRes();
|
||||
|
||||
await checkBan(req, res, jest.fn());
|
||||
|
||||
expect(denyRequest).toHaveBeenCalledWith(req, res, { type: ViolationTypes.BAN });
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it('returns JSON for a bodyless browser POST to an interactive chat path', async () => {
|
||||
mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
|
||||
const req = createReq({
|
||||
body: undefined,
|
||||
method: 'POST',
|
||||
baseUrl: '/api/agents',
|
||||
originalUrl: '/api/agents/chat/agents',
|
||||
});
|
||||
const res = createRes();
|
||||
|
||||
await checkBan(req, res, jest.fn());
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(denyRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(['abort', 'Abort', 'STEER', 'sTeEr'])(
|
||||
'returns JSON for a banned browser agent control request: %s',
|
||||
async (route) => {
|
||||
mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
|
||||
const req = createReq({
|
||||
method: 'POST',
|
||||
baseUrl: '/api/agents',
|
||||
originalUrl: `/api/agents/chat/${route}`,
|
||||
});
|
||||
const res = createRes();
|
||||
|
||||
await checkBan(req, res, jest.fn());
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(denyRequest).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it('keeps non-browser agent chat denial as JSON', async () => {
|
||||
uap.mockReturnValueOnce({ browser: {} });
|
||||
mockBanCacheGet.mockResolvedValueOnce({ expiresAt: Date.now() + 60000 });
|
||||
const req = createReq({
|
||||
method: 'POST',
|
||||
baseUrl: '/api/agents',
|
||||
originalUrl: '/api/agents/chat/agents',
|
||||
});
|
||||
const res = createRes();
|
||||
|
||||
await checkBan(req, res, jest.fn());
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(denyRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('active ban (positive timeLeft)', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue