diff --git a/api/server/routes/agents/index.js b/api/server/routes/agents/index.js index c71d35ffa7..48dfc9721e 100644 --- a/api/server/routes/agents/index.js +++ b/api/server/routes/agents/index.js @@ -11,6 +11,8 @@ const { attachAskUserQuestionArgs, createMessageFilterPii, readScheduleFireClaims, + exemptFromIpLimiter, + exemptFromUserLimiter, } = require('@librechat/api'); const { createSseStreamTelemetry } = require('@librechat/api/telemetry'); const { logger } = require('@librechat/data-schemas'); @@ -495,36 +497,16 @@ chatRouter.use((req, _res, next) => { }); chatRouter.use(configMiddleware); -/** - * AUTOMATIC scheduled fires are exempt from the interactive message limiters (the - * token's scope claim is signature-verified): the scheduler's own caps govern them - * (cadence floor, global fireConcurrency), and stacking both throttles would record - * legitimate fires as errors that walk schedules toward auto-disable. - * - * Run Now is NOT exempt. It dispatches the same billed generation over the same - * schedule-scoped token, but it enforces no cadence floor and no per-user time window, - * so a permitted user could re-trigger as prior runs finish — or rotate across their - * schedules — and bypass LIMIT_MESSAGE_USER/LIMIT_MESSAGE_IP entirely. It is user-paced - * request volume wearing a scheduled token, so it belongs under the interactive limits. - * - * Reads the flags captured above so a token that expired during config/middleware still - * classifies a valid fire; the re-read fallback is fail-safe (an unverifiable token - * limits rather than exempts). - */ -const skipAutomaticFires = (limiter) => (req, res, next) => { - const claims = - typeof req._isScheduledFire === 'boolean' - ? { scheduled: req._isScheduledFire, manual: req._isManualScheduledFire === true } - : readScheduleFireClaims(req); - return claims.scheduled && !claims.manual ? next() : limiter(req, res, next); -}; +/** Applies `limiter` unless `isExempt` says this fire should skip it. */ +const unless = (isExempt, limiter) => (req, res, next) => + isExempt(req) ? next() : limiter(req, res, next); if (isEnabled(LIMIT_MESSAGE_IP)) { - chatRouter.use(skipAutomaticFires(messageIpLimiter)); + chatRouter.use(unless(exemptFromIpLimiter, messageIpLimiter)); } if (isEnabled(LIMIT_MESSAGE_USER)) { - chatRouter.use(skipAutomaticFires(messageUserLimiter)); + chatRouter.use(unless(exemptFromUserLimiter, messageUserLimiter)); } chatRouter.use('/', chat); diff --git a/api/server/routes/schedules.js b/api/server/routes/schedules.js index 93878c5ec9..6dac17a1f1 100644 --- a/api/server/routes/schedules.js +++ b/api/server/routes/schedules.js @@ -1,7 +1,7 @@ const express = require('express'); const { Permissions, PermissionTypes } = require('librechat-data-provider'); -const { createSchedulesHandlers, generateCheckAccess } = require('@librechat/api'); -const { requireJwtAuth, configMiddleware } = require('~/server/middleware'); +const { isEnabled, createSchedulesHandlers, generateCheckAccess } = require('@librechat/api'); +const { requireJwtAuth, configMiddleware, messageIpLimiter } = require('~/server/middleware'); const { getLimits, fireScheduleNow, @@ -71,6 +71,16 @@ router.post('/', checkSchedulesCreate, handlers.createSchedule); router.patch('/:id', checkSchedulesCreate, handlers.updateSchedule); router.delete('/:id', checkSchedulesCreate, handlers.deleteSchedule); // Run-now mutates runtime state; gate it on CREATE like the UI does (not USE). -router.post('/:id/run', checkSchedulesCreate, handlers.runScheduleNow); +// This is also where LIMIT_MESSAGE_IP has to apply to a manual run: the fire itself is a +// loopback POST carrying the server's address, so limiting by IP there would pool every +// user into one bucket. Here the initiating client's address is still on the request. +// The USER limiter is NOT duplicated here; the fire token carries the authenticated id, +// so the chat router applies it to the loopback exactly once. +router.post( + '/:id/run', + ...(isEnabled(process.env.LIMIT_MESSAGE_IP) ? [messageIpLimiter] : []), + checkSchedulesCreate, + handlers.runScheduleNow, +); module.exports = router; diff --git a/packages/api/src/crypto/jwt.ts b/packages/api/src/crypto/jwt.ts index d9e562c44a..287320f09d 100644 --- a/packages/api/src/crypto/jwt.ts +++ b/packages/api/src/crypto/jwt.ts @@ -67,3 +67,44 @@ export const readScheduleFireClaims = (req: { export const isScheduleFireRequest = (req: { headers: Record; }): boolean => readScheduleFireClaims(req).scheduled; + +/** + * A request whose fire classification the chat router already captured. Re-verifying + * downstream would demote a valid fire whose short-lived token expired during the + * slower middleware chain, so the captured decision wins when present. + */ +interface ClassifiedFireRequest { + headers: Record; + _isScheduledFire?: boolean; + _isManualScheduledFire?: boolean; +} + +const classify = (req: ClassifiedFireRequest): ScheduleFireClaims => + typeof req._isScheduledFire === 'boolean' + ? { scheduled: req._isScheduledFire, manual: req._isManualScheduledFire === true } + : readScheduleFireClaims(req); + +/** + * Whether the IP-based message limiter should be SKIPPED. + * + * Every fire — automatic or manual — reaches the chat router over the server's own + * loopback, so its address is never the initiating client's. Keying an IP limiter there + * would put all users in ONE bucket, letting any user's Run Now volume reject everyone + * else's. Manual runs are IP-limited at `/api/schedules/:id/run` instead, where the real + * address is still on the request. + */ +export const exemptFromIpLimiter = (req: ClassifiedFireRequest): boolean => classify(req).scheduled; + +/** + * Whether the USER-based message limiter should be SKIPPED. + * + * Only AUTOMATIC occurrences are exempt: the scheduler's own caps (cadence floor, global + * fireConcurrency) already bound them, and limiting them again would record legitimate + * fires as errors that walk schedules toward auto-disable. Run Now has neither cap, and + * this limiter keys on the authenticated id, which the fire token carries intact across + * the loopback — so it is the one that actually bounds manual volume. + */ +export const exemptFromUserLimiter = (req: ClassifiedFireRequest): boolean => { + const claims = classify(req); + return claims.scheduled && !claims.manual; +}; diff --git a/packages/api/src/crypto/limiters.spec.ts b/packages/api/src/crypto/limiters.spec.ts new file mode 100644 index 0000000000..49b3317a65 --- /dev/null +++ b/packages/api/src/crypto/limiters.spec.ts @@ -0,0 +1,88 @@ +import { + exemptFromIpLimiter, + SCHEDULE_FIRE_SCOPE, + exemptFromUserLimiter, + SCHEDULE_MANUAL_CLAIM, + readScheduleFireClaims, + generateShortLivedToken, +} from './jwt'; + +type FireKind = 'automatic' | 'manual' | 'interactive'; + +/** Builds a request as the chat router sees it, including its capture middleware. */ +function request(kind: FireKind) { + if (kind === 'interactive') { + return { headers: {} }; + } + const token = generateShortLivedToken('user-1', '60s', { + scope: SCHEDULE_FIRE_SCOPE, + ...(kind === 'manual' ? { [SCHEDULE_MANUAL_CLAIM]: '1' } : {}), + }); + const req: { + headers: Record; + _isScheduledFire?: boolean; + _isManualScheduledFire?: boolean; + } = { headers: { 'x-lc-scheduled': '1', authorization: `Bearer ${token}` } }; + const claims = readScheduleFireClaims(req); + req._isScheduledFire = claims.scheduled; + req._isManualScheduledFire = claims.manual; + return req; +} + +describe('message-limiter exemptions for scheduled fires', () => { + const original = process.env.JWT_SECRET; + + beforeAll(() => { + process.env.JWT_SECRET = 'test-secret'; + }); + + afterAll(() => { + process.env.JWT_SECRET = original; + }); + + describe('IP limiter', () => { + it.each(['automatic', 'manual'])('exempts a %s fire', (kind) => { + // Every fire reaches the chat router over the SERVER's loopback, so its address is + // never the initiating client's. Keying here would pool all users into one bucket + // and let any user's Run Now volume reject everyone else's. Manual runs are + // IP-limited at /api/schedules/:id/run, where the real address still exists. + expect(exemptFromIpLimiter(request(kind))).toBe(true); + }); + + it('does not exempt an ordinary interactive turn', () => { + expect(exemptFromIpLimiter(request('interactive'))).toBe(false); + }); + }); + + describe('user limiter', () => { + it('exempts an automatic occurrence', () => { + // Already bounded by the cadence floor and the global fireConcurrency slot; + // limiting again would record legitimate fires as errors toward auto-disable. + expect(exemptFromUserLimiter(request('automatic'))).toBe(true); + }); + + it('does NOT exempt a manual Run Now', () => { + // Keyed on the authenticated id, which the fire token carries intact across the + // loopback — so this is the limiter that actually bounds Run Now volume. + expect(exemptFromUserLimiter(request('manual'))).toBe(false); + }); + + it('does not exempt an ordinary interactive turn', () => { + expect(exemptFromUserLimiter(request('interactive'))).toBe(false); + }); + }); + + it('limits rather than exempts a request whose token cannot be verified', () => { + const req = { headers: { 'x-lc-scheduled': '1', authorization: 'Bearer not-a-jwt' } }; + expect(exemptFromIpLimiter(req)).toBe(false); + expect(exemptFromUserLimiter(req)).toBe(false); + }); + + it('trusts the captured classification over a token that expired mid-chain', () => { + // The short-lived token can expire during the slower chat middleware; re-verifying + // would demote a valid automatic fire into a limited interactive turn. + const expired = { headers: { authorization: 'Bearer expired' }, _isScheduledFire: true }; + expect(exemptFromIpLimiter(expired)).toBe(true); + expect(exemptFromUserLimiter(expired)).toBe(true); + }); +});