mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-22 07:36:33 +01:00
🚦 fix: ERR_ERL_INVALID_IP_ADDRESS and IPv6 Key Collisions in IP Rate Limiters (#12319)
* fix: Add removePorts keyGenerator to all IP-based rate limiters Six IP-based rate limiters are missing the `keyGenerator: removePorts` option that is already used by the auth-related limiters (login, register, resetPassword, verifyEmail). Without it, reverse proxies that include ports in X-Forwarded-For headers cause ERR_ERL_INVALID_IP_ADDRESS errors from express-rate-limit. Fixes #12318 * fix: make removePorts IPv6-safe to prevent rate-limit key collisions The original regex `/:\d+[^:]*$/` treated the last colon-delimited segment of bare IPv6 addresses as a port, mangling valid IPs (e.g. `::1` → `::`, `2001:db8::1` → `2001:db8::`). Distinct IPv6 clients could collapse into the same rate-limit bucket. Use `net.isIP()` as a fast path for already-valid IPs, then match bracketed IPv6+port and IPv4+port explicitly. Bare IPv6 addresses are now returned unchanged. Also fixes pre-existing property ordering inconsistency in ttsLimiters.js userLimiterOptions (keyGenerator before store). * refactor: move removePorts to packages/api as TypeScript, fix import order - Move removePorts implementation to packages/api/src/utils/removePorts.ts with proper Express Request typing - Reduce api/server/utils/removePorts.js to a thin re-export from @librechat/api for backward compatibility - Consolidate removePorts import with limiterCache from @librechat/api in all 6 limiter files, fixing import order (package imports shortest to longest, local imports longest to shortest) - Remove narrating inline comments per code style guidelines --------- Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
748fd086c1
commit
ecd6d76bc8
17 changed files with 162 additions and 28 deletions
3
api/cache/banViolation.js
vendored
3
api/cache/banViolation.js
vendored
|
|
@ -1,8 +1,7 @@
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const { isEnabled, math } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { isEnabled, math, removePorts } = require('@librechat/api');
|
||||
const { deleteAllUserSessions } = require('~/models');
|
||||
const { removePorts } = require('~/server/utils');
|
||||
const getLogStores = require('./getLogStores');
|
||||
|
||||
const { BAN_VIOLATIONS, BAN_INTERVAL } = process.env ?? {};
|
||||
|
|
|
|||
|
|
@ -1,11 +1,10 @@
|
|||
const { Keyv } = require('keyv');
|
||||
const uap = require('ua-parser-js');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { isEnabled, keyvMongo } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { removePorts } = require('~/server/utils');
|
||||
const denyRequest = require('./denyRequest');
|
||||
const { isEnabled, keyvMongo, removePorts } = require('@librechat/api');
|
||||
const { getLogStores } = require('~/cache');
|
||||
const denyRequest = require('./denyRequest');
|
||||
const { findUser } = require('~/models');
|
||||
|
||||
const banCache = new Keyv({ store: keyvMongo, namespace: ViolationTypes.BAN, ttl: 0 });
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const logViolation = require('~/cache/logViolation');
|
||||
|
||||
const getEnvironmentVariables = () => {
|
||||
|
|
@ -59,6 +59,7 @@ const createForkLimiters = () => {
|
|||
windowMs: forkIpWindowMs,
|
||||
max: forkIpMax,
|
||||
handler: createForkHandler(),
|
||||
keyGenerator: removePorts,
|
||||
store: limiterCache('fork_ip_limiter'),
|
||||
};
|
||||
const userLimiterOptions = {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const logViolation = require('~/cache/logViolation');
|
||||
|
||||
const getEnvironmentVariables = () => {
|
||||
|
|
@ -60,6 +60,7 @@ const createImportLimiters = () => {
|
|||
windowMs: importIpWindowMs,
|
||||
max: importIpMax,
|
||||
handler: createImportHandler(),
|
||||
keyGenerator: removePorts,
|
||||
store: limiterCache('import_ip_limiter'),
|
||||
};
|
||||
const userLimiterOptions = {
|
||||
|
|
@ -67,7 +68,7 @@ const createImportLimiters = () => {
|
|||
max: importUserMax,
|
||||
handler: createImportHandler(false),
|
||||
keyGenerator: function (req) {
|
||||
return req.user?.id; // Use the user ID or NULL if not available
|
||||
return req.user?.id;
|
||||
},
|
||||
store: limiterCache('import_user_limiter'),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { removePorts } = require('~/server/utils');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const { logViolation } = require('~/cache');
|
||||
|
||||
const { LOGIN_WINDOW = 5, LOGIN_MAX = 7, LOGIN_VIOLATION_SCORE: score } = process.env;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const denyRequest = require('~/server/middleware/denyRequest');
|
||||
const { logViolation } = require('~/cache');
|
||||
|
||||
|
|
@ -50,6 +50,7 @@ const ipLimiterOptions = {
|
|||
windowMs: ipWindowMs,
|
||||
max: ipMax,
|
||||
handler: createHandler(),
|
||||
keyGenerator: removePorts,
|
||||
store: limiterCache('message_ip_limiter'),
|
||||
};
|
||||
|
||||
|
|
@ -58,7 +59,7 @@ const userLimiterOptions = {
|
|||
max: userMax,
|
||||
handler: createHandler(false),
|
||||
keyGenerator: function (req) {
|
||||
return req.user?.id; // Use the user ID or NULL if not available
|
||||
return req.user?.id;
|
||||
},
|
||||
store: limiterCache('message_user_limiter'),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { removePorts } = require('~/server/utils');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const { logViolation } = require('~/cache');
|
||||
|
||||
const { REGISTER_WINDOW = 60, REGISTER_MAX = 5, REGISTRATION_VIOLATION_SCORE: score } = process.env;
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { removePorts } = require('~/server/utils');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const { logViolation } = require('~/cache');
|
||||
|
||||
const {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const logViolation = require('~/cache/logViolation');
|
||||
|
||||
const getEnvironmentVariables = () => {
|
||||
|
|
@ -54,6 +54,7 @@ const createSTTLimiters = () => {
|
|||
windowMs: sttIpWindowMs,
|
||||
max: sttIpMax,
|
||||
handler: createSTTHandler(),
|
||||
keyGenerator: removePorts,
|
||||
store: limiterCache('stt_ip_limiter'),
|
||||
};
|
||||
|
||||
|
|
@ -62,7 +63,7 @@ const createSTTLimiters = () => {
|
|||
max: sttUserMax,
|
||||
handler: createSTTHandler(false),
|
||||
keyGenerator: function (req) {
|
||||
return req.user?.id; // Use the user ID or NULL if not available
|
||||
return req.user?.id;
|
||||
},
|
||||
store: limiterCache('stt_user_limiter'),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const logViolation = require('~/cache/logViolation');
|
||||
|
||||
const getEnvironmentVariables = () => {
|
||||
|
|
@ -54,6 +54,7 @@ const createTTSLimiters = () => {
|
|||
windowMs: ttsIpWindowMs,
|
||||
max: ttsIpMax,
|
||||
handler: createTTSHandler(),
|
||||
keyGenerator: removePorts,
|
||||
store: limiterCache('tts_ip_limiter'),
|
||||
};
|
||||
|
||||
|
|
@ -61,10 +62,10 @@ const createTTSLimiters = () => {
|
|||
windowMs: ttsUserWindowMs,
|
||||
max: ttsUserMax,
|
||||
handler: createTTSHandler(false),
|
||||
store: limiterCache('tts_user_limiter'),
|
||||
keyGenerator: function (req) {
|
||||
return req.user?.id; // Use the user ID or NULL if not available
|
||||
return req.user?.id;
|
||||
},
|
||||
store: limiterCache('tts_user_limiter'),
|
||||
};
|
||||
|
||||
const ttsIpLimiter = rateLimit(ipLimiterOptions);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const logViolation = require('~/cache/logViolation');
|
||||
|
||||
const getEnvironmentVariables = () => {
|
||||
|
|
@ -60,6 +60,7 @@ const createFileLimiters = () => {
|
|||
windowMs: fileUploadIpWindowMs,
|
||||
max: fileUploadIpMax,
|
||||
handler: createFileUploadHandler(),
|
||||
keyGenerator: removePorts,
|
||||
store: limiterCache('file_upload_ip_limiter'),
|
||||
};
|
||||
|
||||
|
|
@ -68,7 +69,7 @@ const createFileLimiters = () => {
|
|||
max: fileUploadUserMax,
|
||||
handler: createFileUploadHandler(false),
|
||||
keyGenerator: function (req) {
|
||||
return req.user?.id; // Use the user ID or NULL if not available
|
||||
return req.user?.id;
|
||||
},
|
||||
store: limiterCache('file_upload_user_limiter'),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
const rateLimit = require('express-rate-limit');
|
||||
const { limiterCache } = require('@librechat/api');
|
||||
const { ViolationTypes } = require('librechat-data-provider');
|
||||
const { removePorts } = require('~/server/utils');
|
||||
const { limiterCache, removePorts } = require('@librechat/api');
|
||||
const { logViolation } = require('~/cache');
|
||||
|
||||
const {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
const removePorts = require('./removePorts');
|
||||
const handleText = require('./handleText');
|
||||
const sendEmail = require('./sendEmail');
|
||||
const queue = require('./queue');
|
||||
|
|
@ -6,7 +5,6 @@ const files = require('./files');
|
|||
|
||||
module.exports = {
|
||||
...handleText,
|
||||
removePorts,
|
||||
sendEmail,
|
||||
...files,
|
||||
...queue,
|
||||
|
|
|
|||
|
|
@ -1 +0,0 @@
|
|||
module.exports = (req) => req?.ip?.replace(/:\d+[^:]*$/, '');
|
||||
|
|
@ -18,6 +18,7 @@ export * from './math';
|
|||
export * from './oidc';
|
||||
export * from './openid';
|
||||
export * from './promise';
|
||||
export * from './ports';
|
||||
export * from './sanitizeTitle';
|
||||
export * from './tempChatRetention';
|
||||
export * from './text';
|
||||
|
|
|
|||
98
packages/api/src/utils/ports.spec.ts
Normal file
98
packages/api/src/utils/ports.spec.ts
Normal file
|
|
@ -0,0 +1,98 @@
|
|||
import type { Request } from 'express';
|
||||
import { removePorts } from './ports';
|
||||
|
||||
const req = (ip: string | undefined): Request => ({ ip }) as Request;
|
||||
|
||||
describe('removePorts', () => {
|
||||
describe('bare IPv4 (no port)', () => {
|
||||
test('returns a standard private IP unchanged', () => {
|
||||
expect(removePorts(req('192.168.1.1'))).toBe('192.168.1.1');
|
||||
});
|
||||
|
||||
test('returns a public IP unchanged', () => {
|
||||
expect(removePorts(req('149.154.20.46'))).toBe('149.154.20.46');
|
||||
});
|
||||
|
||||
test('returns loopback unchanged', () => {
|
||||
expect(removePorts(req('127.0.0.1'))).toBe('127.0.0.1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('IPv4 with port (the primary bug scenario)', () => {
|
||||
test('strips port from a private IP', () => {
|
||||
expect(removePorts(req('192.168.1.1:8080'))).toBe('192.168.1.1');
|
||||
});
|
||||
|
||||
test('strips port from the IP in the original issue report', () => {
|
||||
expect(removePorts(req('149.154.20.46:48198'))).toBe('149.154.20.46');
|
||||
});
|
||||
|
||||
test('strips a low port number', () => {
|
||||
expect(removePorts(req('10.0.0.1:80'))).toBe('10.0.0.1');
|
||||
});
|
||||
|
||||
test('strips a high port number', () => {
|
||||
expect(removePorts(req('10.0.0.1:65535'))).toBe('10.0.0.1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('bare IPv6 (no port)', () => {
|
||||
test('returns loopback unchanged', () => {
|
||||
expect(removePorts(req('::1'))).toBe('::1');
|
||||
});
|
||||
|
||||
test('returns a full address unchanged', () => {
|
||||
expect(removePorts(req('2001:db8::1'))).toBe('2001:db8::1');
|
||||
});
|
||||
|
||||
test('returns an IPv4-mapped IPv6 address unchanged', () => {
|
||||
expect(removePorts(req('::ffff:192.168.1.1'))).toBe('::ffff:192.168.1.1');
|
||||
});
|
||||
|
||||
test('returns a fully expanded IPv6 unchanged', () => {
|
||||
expect(removePorts(req('2001:0db8:85a3:0000:0000:8a2e:0370:7334'))).toBe(
|
||||
'2001:0db8:85a3:0000:0000:8a2e:0370:7334',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bracketed IPv6 with port', () => {
|
||||
test('extracts loopback from brackets with port', () => {
|
||||
expect(removePorts(req('[::1]:8080'))).toBe('::1');
|
||||
});
|
||||
|
||||
test('extracts a full address from brackets with port', () => {
|
||||
expect(removePorts(req('[2001:db8::1]:443'))).toBe('2001:db8::1');
|
||||
});
|
||||
|
||||
test('extracts address from brackets without port', () => {
|
||||
expect(removePorts(req('[::1]'))).toBe('::1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('falsy / missing ip', () => {
|
||||
test('returns undefined when ip is undefined', () => {
|
||||
expect(removePorts(req(undefined))).toBeUndefined();
|
||||
});
|
||||
|
||||
test('returns undefined when ip is empty string', () => {
|
||||
expect(removePorts({ ip: '' } as Request)).toBe('');
|
||||
});
|
||||
|
||||
test('returns undefined when req is null', () => {
|
||||
expect(removePorts(null as unknown as Request)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('IPv4-mapped IPv6 with port', () => {
|
||||
test('strips port from an IPv4-mapped IPv6 address', () => {
|
||||
expect(removePorts(req('::ffff:1.2.3.4:8080'))).toBe('::ffff:1.2.3.4');
|
||||
});
|
||||
});
|
||||
|
||||
describe('unrecognized formats fall through unchanged', () => {
|
||||
test('returns garbage input unchanged', () => {
|
||||
expect(removePorts(req('not-an-ip'))).toBe('not-an-ip');
|
||||
});
|
||||
});
|
||||
});
|
||||
38
packages/api/src/utils/ports.ts
Normal file
38
packages/api/src/utils/ports.ts
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
import type { Request } from 'express';
|
||||
|
||||
/** Strips port suffix from req.ip for use as a rate-limiter key (IPv4 and IPv6-safe) */
|
||||
export function removePorts(req: Request): string | undefined {
|
||||
const ip = req?.ip;
|
||||
if (!ip) {
|
||||
return ip;
|
||||
}
|
||||
|
||||
if (ip.charCodeAt(0) === 91) {
|
||||
const close = ip.indexOf(']');
|
||||
return close > 0 ? ip.slice(1, close) : ip;
|
||||
}
|
||||
|
||||
const lastColon = ip.lastIndexOf(':');
|
||||
if (lastColon === -1) {
|
||||
return ip;
|
||||
}
|
||||
|
||||
if (ip.indexOf('.') !== -1 && hasOnlyDigitsAfter(ip, lastColon + 1)) {
|
||||
return ip.slice(0, lastColon);
|
||||
}
|
||||
|
||||
return ip;
|
||||
}
|
||||
|
||||
function hasOnlyDigitsAfter(str: string, start: number): boolean {
|
||||
if (start >= str.length) {
|
||||
return false;
|
||||
}
|
||||
for (let i = start; i < str.length; i++) {
|
||||
const c = str.charCodeAt(i);
|
||||
if (c < 48 || c > 57) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue