From b15d40e3e4770567a4c5a90ba9d251a61e668d61 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 26 Jun 2026 12:19:03 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=A3=20refactor:=20Rate-Limit=20Token?= =?UTF-8?q?=20Routes=20and=20Cap=20Remote=20File=20Downloads=20(#13978)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * harden token and remote file handling * sort s3 storage imports * split token submission rate limits --- api/server/middleware/limiters/index.js | 4 + .../resetPasswordSubmissionLimiter.js | 39 ++++++ .../limiters/verifyEmailSubmissionLimiter.js | 39 ++++++ api/server/routes/auth.2fa-ratelimit.test.js | 1 + api/server/routes/auth.cloudfront.test.js | 1 + api/server/routes/auth.js | 1 + .../auth.reset-password-ratelimit.test.js | 121 ++++++++++++++++++ api/server/routes/user.js | 3 +- .../routes/user.verify-ratelimit.test.js | 71 ++++++++++ api/server/services/Files/Azure/crud.js | 23 +++- api/server/services/Files/Firebase/crud.js | 22 +++- api/server/services/Files/Local/crud.js | 19 ++- packages/api/src/storage/index.ts | 1 + .../api/src/storage/s3/__tests__/crud.test.ts | 51 +++++++- packages/api/src/storage/s3/crud.ts | 40 ++++-- packages/api/src/storage/url.ts | 103 +++++++++++++++ 16 files changed, 518 insertions(+), 21 deletions(-) create mode 100644 api/server/middleware/limiters/resetPasswordSubmissionLimiter.js create mode 100644 api/server/middleware/limiters/verifyEmailSubmissionLimiter.js create mode 100644 api/server/routes/auth.reset-password-ratelimit.test.js create mode 100644 api/server/routes/user.verify-ratelimit.test.js create mode 100644 packages/api/src/storage/url.ts diff --git a/api/server/middleware/limiters/index.js b/api/server/middleware/limiters/index.js index 4a569e2698..76afc3ce37 100644 --- a/api/server/middleware/limiters/index.js +++ b/api/server/middleware/limiters/index.js @@ -12,6 +12,8 @@ const promptUsageLimiter = require('./promptUsageLimiter'); const verifyEmailLimiter = require('./verifyEmailLimiter'); const resetPasswordLimiter = require('./resetPasswordLimiter'); const twoFactorTempLimiter = require('./twoFactorTempLimiter'); +const verifyEmailSubmissionLimiter = require('./verifyEmailSubmissionLimiter'); +const resetPasswordSubmissionLimiter = require('./resetPasswordSubmissionLimiter'); module.exports = { ...uploadLimiters, @@ -26,5 +28,7 @@ module.exports = { createSTTLimiters, verifyEmailLimiter, resetPasswordLimiter, + verifyEmailSubmissionLimiter, + resetPasswordSubmissionLimiter, twoFactorTempLimiter, }; diff --git a/api/server/middleware/limiters/resetPasswordSubmissionLimiter.js b/api/server/middleware/limiters/resetPasswordSubmissionLimiter.js new file mode 100644 index 0000000000..4eb3bb2967 --- /dev/null +++ b/api/server/middleware/limiters/resetPasswordSubmissionLimiter.js @@ -0,0 +1,39 @@ +const rateLimit = require('express-rate-limit'); +const { ViolationTypes } = require('librechat-data-provider'); +const { limiterCache, removePorts } = require('@librechat/api'); +const { logViolation } = require('~/cache'); + +const { + RESET_PASSWORD_SUBMISSION_WINDOW = process.env.RESET_PASSWORD_WINDOW ?? 2, + RESET_PASSWORD_SUBMISSION_MAX = process.env.RESET_PASSWORD_MAX ?? 2, + RESET_PASSWORD_SUBMISSION_VIOLATION_SCORE: score, +} = process.env; +const windowMs = RESET_PASSWORD_SUBMISSION_WINDOW * 60 * 1000; +const max = RESET_PASSWORD_SUBMISSION_MAX; +const windowInMinutes = windowMs / 60000; +const message = `Too many attempts, please try again after ${windowInMinutes} minute(s)`; + +const handler = async (req, res) => { + const type = ViolationTypes.RESET_PASSWORD_LIMIT; + const errorMessage = { + type, + max, + windowInMinutes, + limiter: 'submission', + }; + + await logViolation(req, res, type, errorMessage, score); + return res.status(429).json({ message }); +}; + +const limiterOptions = { + windowMs, + max, + handler, + keyGenerator: removePorts, + store: limiterCache('reset_password_submission_limiter'), +}; + +const resetPasswordSubmissionLimiter = rateLimit(limiterOptions); + +module.exports = resetPasswordSubmissionLimiter; diff --git a/api/server/middleware/limiters/verifyEmailSubmissionLimiter.js b/api/server/middleware/limiters/verifyEmailSubmissionLimiter.js new file mode 100644 index 0000000000..e3a1f8cb22 --- /dev/null +++ b/api/server/middleware/limiters/verifyEmailSubmissionLimiter.js @@ -0,0 +1,39 @@ +const rateLimit = require('express-rate-limit'); +const { ViolationTypes } = require('librechat-data-provider'); +const { limiterCache, removePorts } = require('@librechat/api'); +const { logViolation } = require('~/cache'); + +const { + VERIFY_EMAIL_SUBMISSION_WINDOW = process.env.VERIFY_EMAIL_WINDOW ?? 2, + VERIFY_EMAIL_SUBMISSION_MAX = process.env.VERIFY_EMAIL_MAX ?? 2, + VERIFY_EMAIL_SUBMISSION_VIOLATION_SCORE: score, +} = process.env; +const windowMs = VERIFY_EMAIL_SUBMISSION_WINDOW * 60 * 1000; +const max = VERIFY_EMAIL_SUBMISSION_MAX; +const windowInMinutes = windowMs / 60000; +const message = `Too many attempts, please try again after ${windowInMinutes} minute(s)`; + +const handler = async (req, res) => { + const type = ViolationTypes.VERIFY_EMAIL_LIMIT; + const errorMessage = { + type, + max, + windowInMinutes, + limiter: 'submission', + }; + + await logViolation(req, res, type, errorMessage, score); + return res.status(429).json({ message }); +}; + +const limiterOptions = { + windowMs, + max, + handler, + keyGenerator: removePorts, + store: limiterCache('verify_email_submission_limiter'), +}; + +const verifyEmailSubmissionLimiter = rateLimit(limiterOptions); + +module.exports = verifyEmailSubmissionLimiter; diff --git a/api/server/routes/auth.2fa-ratelimit.test.js b/api/server/routes/auth.2fa-ratelimit.test.js index 35cc0e8840..3d11189a83 100644 --- a/api/server/routes/auth.2fa-ratelimit.test.js +++ b/api/server/routes/auth.2fa-ratelimit.test.js @@ -62,6 +62,7 @@ jest.mock('~/server/middleware', () => { checkInviteUser: pass, validateRegistration: pass, resetPasswordLimiter: pass, + resetPasswordSubmissionLimiter: pass, validatePasswordReset: pass, requireJwtAuth: pass, }; diff --git a/api/server/routes/auth.cloudfront.test.js b/api/server/routes/auth.cloudfront.test.js index ae78629104..7456501119 100644 --- a/api/server/routes/auth.cloudfront.test.js +++ b/api/server/routes/auth.cloudfront.test.js @@ -59,6 +59,7 @@ jest.mock('~/server/middleware', () => { checkInviteUser: pass, validateRegistration: pass, resetPasswordLimiter: pass, + resetPasswordSubmissionLimiter: pass, validatePasswordReset: pass, requireJwtAuth: jest.fn((req, res, next) => { if (req.headers.authorization !== 'Bearer ok') { diff --git a/api/server/routes/auth.js b/api/server/routes/auth.js index 14c4e86389..191c1d3ee1 100644 --- a/api/server/routes/auth.js +++ b/api/server/routes/auth.js @@ -80,6 +80,7 @@ router.post( ); router.post( '/resetPassword', + middleware.resetPasswordSubmissionLimiter, middleware.checkBan, middleware.validatePasswordReset, resetPasswordController, diff --git a/api/server/routes/auth.reset-password-ratelimit.test.js b/api/server/routes/auth.reset-password-ratelimit.test.js new file mode 100644 index 0000000000..86ec583ec5 --- /dev/null +++ b/api/server/routes/auth.reset-password-ratelimit.test.js @@ -0,0 +1,121 @@ +const express = require('express'); +const request = require('supertest'); + +const mockCheckBan = jest.fn((req, res, next) => next()); +const mockResetPasswordSubmissionLimiter = jest.fn((req, res, next) => next()); +const mockValidatePasswordReset = jest.fn((req, res, next) => next()); +const mockResetPasswordController = jest.fn((req, res) => res.status(204).end()); + +jest.mock('@librechat/api', () => ({ + createSetBalanceConfig: jest.fn(() => (req, res, next) => next()), + forceRefreshCloudFrontAuthCookies: jest.fn(), +})); + +jest.mock('~/server/controllers/AuthController', () => ({ + refreshController: jest.fn((req, res) => res.status(204).end()), + registrationController: jest.fn((req, res) => res.status(204).end()), + resetPasswordController: (...args) => mockResetPasswordController(...args), + resetPasswordRequestController: jest.fn((req, res) => res.status(204).end()), + graphTokenController: jest.fn((req, res) => res.status(204).end()), +})); + +jest.mock('~/server/controllers/TwoFactorController', () => ({ + enable2FA: jest.fn((req, res) => res.status(204).end()), + verify2FA: jest.fn((req, res) => res.status(204).end()), + confirm2FA: jest.fn((req, res) => res.status(204).end()), + disable2FA: jest.fn((req, res) => res.status(204).end()), + regenerateBackupCodes: jest.fn((req, res) => res.status(204).end()), +})); + +jest.mock('~/server/controllers/auth/TwoFactorAuthController', () => ({ + verify2FAWithTempToken: jest.fn((req, res) => res.status(204).end()), +})); + +jest.mock('~/server/controllers/auth/LogoutController', () => ({ + logoutController: jest.fn((req, res) => res.status(204).end()), +})); + +jest.mock('~/server/controllers/auth/LoginController', () => ({ + loginController: jest.fn((req, res) => res.status(204).end()), +})); + +jest.mock('~/models', () => ({ + findBalanceByUser: jest.fn(), + upsertBalanceFields: jest.fn(), +})); + +jest.mock('~/server/services/Config', () => ({ + getAppConfig: jest.fn(), +})); + +jest.mock('~/server/middleware', () => { + const pass = (req, res, next) => next(); + return { + logHeaders: pass, + loginLimiter: pass, + setTwoFactorTempUser: pass, + twoFactorTempLimiter: pass, + checkBan: (...args) => mockCheckBan(...args), + requireLocalAuth: pass, + requireLdapAuth: pass, + registerLimiter: pass, + checkInviteUser: pass, + validateRegistration: pass, + resetPasswordLimiter: pass, + resetPasswordSubmissionLimiter: (...args) => mockResetPasswordSubmissionLimiter(...args), + validatePasswordReset: (...args) => mockValidatePasswordReset(...args), + requireJwtAuth: pass, + }; +}); + +const authRouter = require('./auth'); + +describe('POST /api/auth/resetPassword rate limiting', () => { + let app; + + beforeEach(() => { + jest.clearAllMocks(); + mockCheckBan.mockImplementation((req, res, next) => next()); + mockResetPasswordSubmissionLimiter.mockImplementation((req, res, next) => next()); + mockValidatePasswordReset.mockImplementation((req, res, next) => next()); + mockResetPasswordController.mockImplementation((req, res) => res.status(204).end()); + + app = express(); + app.use(express.json()); + app.use('/api/auth', authRouter); + }); + + it('limits before ban checks, validation, and password reset work', async () => { + await request(app).post('/api/auth/resetPassword').send({ token: 'token' }).expect(204); + + expect(mockResetPasswordSubmissionLimiter).toHaveBeenCalledTimes(1); + expect(mockCheckBan).toHaveBeenCalledTimes(1); + expect(mockValidatePasswordReset).toHaveBeenCalledTimes(1); + expect(mockResetPasswordController).toHaveBeenCalledTimes(1); + expect(mockResetPasswordSubmissionLimiter.mock.invocationCallOrder[0]).toBeLessThan( + mockCheckBan.mock.invocationCallOrder[0], + ); + expect(mockCheckBan.mock.invocationCallOrder[0]).toBeLessThan( + mockValidatePasswordReset.mock.invocationCallOrder[0], + ); + expect(mockValidatePasswordReset.mock.invocationCallOrder[0]).toBeLessThan( + mockResetPasswordController.mock.invocationCallOrder[0], + ); + }); + + it('does not validate or reset passwords after the limiter rejects the request', async () => { + mockResetPasswordSubmissionLimiter.mockImplementation((req, res) => + res.status(429).json({ message: 'Too many password reset attempts' }), + ); + + const response = await request(app) + .post('/api/auth/resetPassword') + .send({ token: 'token' }) + .expect(429); + + expect(response.body).toEqual({ message: 'Too many password reset attempts' }); + expect(mockCheckBan).not.toHaveBeenCalled(); + expect(mockValidatePasswordReset).not.toHaveBeenCalled(); + expect(mockResetPasswordController).not.toHaveBeenCalled(); + }); +}); diff --git a/api/server/routes/user.js b/api/server/routes/user.js index 1858be22be..dbe42f1164 100644 --- a/api/server/routes/user.js +++ b/api/server/routes/user.js @@ -10,6 +10,7 @@ const { } = require('~/server/controllers/UserController'); const { verifyEmailLimiter, + verifyEmailSubmissionLimiter, configMiddleware, canDeleteAccount, requireJwtAuth, @@ -25,7 +26,7 @@ router.get('/terms', requireJwtAuth, getTermsStatusController); router.post('/terms/accept', requireJwtAuth, acceptTermsController); router.post('/plugins', requireJwtAuth, updateUserPluginsController); router.delete('/delete', requireJwtAuth, canDeleteAccount, configMiddleware, deleteUserController); -router.post('/verify', verifyEmailController); +router.post('/verify', verifyEmailSubmissionLimiter, verifyEmailController); router.post('/verify/resend', verifyEmailLimiter, resendVerificationController); module.exports = router; diff --git a/api/server/routes/user.verify-ratelimit.test.js b/api/server/routes/user.verify-ratelimit.test.js new file mode 100644 index 0000000000..c61ade5720 --- /dev/null +++ b/api/server/routes/user.verify-ratelimit.test.js @@ -0,0 +1,71 @@ +const express = require('express'); +const request = require('supertest'); + +const mockVerifyEmailSubmissionLimiter = jest.fn((req, res, next) => next()); +const mockVerifyEmailController = jest.fn((req, res) => res.status(204).end()); + +jest.mock('~/server/controllers/UserController', () => ({ + getUserController: jest.fn((req, res) => res.status(204).end()), + deleteUserController: jest.fn((req, res) => res.status(204).end()), + acceptTermsController: jest.fn((req, res) => res.status(204).end()), + verifyEmailController: (...args) => mockVerifyEmailController(...args), + getTermsStatusController: jest.fn((req, res) => res.status(204).end()), + updateUserPluginsController: jest.fn((req, res) => res.status(204).end()), + resendVerificationController: jest.fn((req, res) => res.status(204).end()), +})); + +jest.mock('~/server/middleware', () => { + const pass = (req, res, next) => next(); + return { + requireJwtAuth: pass, + canDeleteAccount: pass, + configMiddleware: pass, + verifyEmailLimiter: pass, + verifyEmailSubmissionLimiter: (...args) => mockVerifyEmailSubmissionLimiter(...args), + }; +}); + +jest.mock('./settings', () => { + const express = require('express'); + return express.Router(); +}); + +const userRouter = require('./user'); + +describe('POST /api/user/verify rate limiting', () => { + let app; + + beforeEach(() => { + jest.clearAllMocks(); + mockVerifyEmailSubmissionLimiter.mockImplementation((req, res, next) => next()); + mockVerifyEmailController.mockImplementation((req, res) => res.status(204).end()); + + app = express(); + app.use(express.json()); + app.use('/api/user', userRouter); + }); + + it('limits email verification before checking the token', async () => { + await request(app).post('/api/user/verify').send({ token: 'token' }).expect(204); + + expect(mockVerifyEmailSubmissionLimiter).toHaveBeenCalledTimes(1); + expect(mockVerifyEmailController).toHaveBeenCalledTimes(1); + expect(mockVerifyEmailSubmissionLimiter.mock.invocationCallOrder[0]).toBeLessThan( + mockVerifyEmailController.mock.invocationCallOrder[0], + ); + }); + + it('does not check the verification token after the limiter rejects the request', async () => { + mockVerifyEmailSubmissionLimiter.mockImplementation((req, res) => + res.status(429).json({ message: 'Too many verification attempts' }), + ); + + const response = await request(app) + .post('/api/user/verify') + .send({ token: 'token' }) + .expect(429); + + expect(response.body).toEqual({ message: 'Too many verification attempts' }); + expect(mockVerifyEmailController).not.toHaveBeenCalled(); + }); +}); diff --git a/api/server/services/Files/Azure/crud.js b/api/server/services/Files/Azure/crud.js index 8f681bd06c..59ffed9891 100644 --- a/api/server/services/Files/Azure/crud.js +++ b/api/server/services/Files/Azure/crud.js @@ -4,7 +4,14 @@ const mime = require('mime'); const axios = require('axios'); const fetch = require('node-fetch'); const { logger } = require('@librechat/data-schemas'); -const { getAzureContainerClient, deleteRagFile } = require('@librechat/api'); +const { + deleteRagFile, + assertRemoteFileURL, + getAzureContainerClient, + getRemoteFileFetchMaxBytes, + getRemoteFileFetchTimeoutMs, + assertRemoteFileContentLength, +} = require('@librechat/api'); const defaultBasePath = 'images'; const { AZURE_STORAGE_PUBLIC_ACCESS = 'true', AZURE_CONTAINER_NAME = 'files' } = process.env; @@ -63,8 +70,20 @@ async function saveURLToAzure({ containerName, }) { try { - const response = await fetch(URL); + const maxBytes = getRemoteFileFetchMaxBytes(); + const response = await fetch(assertRemoteFileURL(URL), { + timeout: getRemoteFileFetchTimeoutMs(), + size: maxBytes, + }); + if (!response.ok) { + throw new Error(`Failed to fetch URL: ${response.status} ${response.statusText}`); + } + assertRemoteFileContentLength(response.headers, maxBytes); const buffer = await response.buffer(); + if (buffer.length > maxBytes) { + throw new Error(`Remote file response too large: ${buffer.length} bytes`); + } + return await saveBufferToAzure({ userId, buffer, fileName, basePath, containerName }); } catch (error) { logger.error('[saveURLToAzure] Error uploading file from URL:', error); diff --git a/api/server/services/Files/Firebase/crud.js b/api/server/services/Files/Firebase/crud.js index d5e5a409bf..9fbdb91dbe 100644 --- a/api/server/services/Files/Firebase/crud.js +++ b/api/server/services/Files/Firebase/crud.js @@ -3,7 +3,14 @@ const path = require('path'); const axios = require('axios'); const fetch = require('node-fetch'); const { logger } = require('@librechat/data-schemas'); -const { getFirebaseStorage, deleteRagFile } = require('@librechat/api'); +const { + deleteRagFile, + getFirebaseStorage, + assertRemoteFileURL, + getRemoteFileFetchMaxBytes, + getRemoteFileFetchTimeoutMs, + assertRemoteFileContentLength, +} = require('@librechat/api'); const { ref, uploadBytes, getDownloadURL, deleteObject } = require('firebase/storage'); const { getBufferMetadata } = require('~/server/utils'); @@ -57,8 +64,19 @@ async function saveURLToFirebase({ userId, URL, fileName, basePath = 'images' }) } const storageRef = ref(storage, `${basePath}/${userId.toString()}/${fileName}`); - const response = await fetch(URL); + const maxBytes = getRemoteFileFetchMaxBytes(); + const response = await fetch(assertRemoteFileURL(URL), { + timeout: getRemoteFileFetchTimeoutMs(), + size: maxBytes, + }); + if (!response.ok) { + throw new Error(`Failed to fetch URL: ${response.status} ${response.statusText}`); + } + assertRemoteFileContentLength(response.headers, maxBytes); const buffer = await response.buffer(); + if (buffer.length > maxBytes) { + throw new Error(`Remote file response too large: ${buffer.length} bytes`); + } try { await uploadBytes(storageRef, buffer); diff --git a/api/server/services/Files/Local/crud.js b/api/server/services/Files/Local/crud.js index c86774d472..799bdfc3f7 100644 --- a/api/server/services/Files/Local/crud.js +++ b/api/server/services/Files/Local/crud.js @@ -1,7 +1,13 @@ const fs = require('fs'); const path = require('path'); const axios = require('axios'); -const { deleteRagFile } = require('@librechat/api'); +const { + deleteRagFile, + assertRemoteFileURL, + getRemoteFileFetchMaxBytes, + getRemoteFileFetchTimeoutMs, + assertRemoteFileContentLength, +} = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { EModelEndpoint } = require('librechat-data-provider'); const { resizeImageBuffer } = require('~/server/services/Files/images/resize'); @@ -115,12 +121,21 @@ async function saveLocalBuffer({ userId, buffer, fileName, basePath = 'images' } */ async function saveFileFromURL({ userId, URL, fileName, basePath = 'images' }) { try { + const maxBytes = getRemoteFileFetchMaxBytes(); const response = await axios({ - url: URL, + url: assertRemoteFileURL(URL), responseType: 'arraybuffer', + timeout: getRemoteFileFetchTimeoutMs(), + maxContentLength: maxBytes, + maxBodyLength: maxBytes, }); + assertRemoteFileContentLength(response.headers, maxBytes); const buffer = Buffer.from(response.data, 'binary'); + if (buffer.length > maxBytes) { + throw new Error(`Remote file response too large: ${buffer.length} bytes`); + } + const { bytes, type, dimensions, extension } = await getBufferMetadata(buffer); // Construct the outputPath based on the basePath and userId diff --git a/packages/api/src/storage/index.ts b/packages/api/src/storage/index.ts index 2c8df83f0a..53ccbe2eab 100644 --- a/packages/api/src/storage/index.ts +++ b/packages/api/src/storage/index.ts @@ -4,3 +4,4 @@ export * from './types'; export * from './images'; export * from './avatar'; export * from './metadata'; +export * from './url'; diff --git a/packages/api/src/storage/s3/__tests__/crud.test.ts b/packages/api/src/storage/s3/__tests__/crud.test.ts index 5004446625..ecf60b8bc1 100644 --- a/packages/api/src/storage/s3/__tests__/crud.test.ts +++ b/packages/api/src/storage/s3/__tests__/crud.test.ts @@ -580,7 +580,10 @@ describe('S3 CRUD', () => { fileName: 'downloaded.jpg', }); - expect(global.fetch).toHaveBeenCalledWith('https://example.com/image.jpg'); + expect(global.fetch).toHaveBeenCalledWith( + 'https://example.com/image.jpg', + expect.objectContaining({ signal: expect.any(Object) }), + ); expect(s3Mock.commandCalls(PutObjectCommand)).toHaveLength(1); expect(result).toBe('https://bucket.s3.amazonaws.com/test-key?signed=true'); }); @@ -593,7 +596,10 @@ describe('S3 CRUD', () => { fileName: 'downloaded.jpg', }); - expect(global.fetch).toHaveBeenCalledWith('https://example.com/image.jpg'); + expect(global.fetch).toHaveBeenCalledWith( + 'https://example.com/image.jpg', + expect.objectContaining({ signal: expect.any(Object) }), + ); expect(s3Mock.commandCalls(PutObjectCommand)).toHaveLength(1); expect(result).toEqual({ filepath: 'https://bucket.s3.amazonaws.com/test-key?signed=true', @@ -779,6 +785,47 @@ describe('S3 CRUD', () => { ).rejects.toThrow('Failed to fetch URL'); }); + it('rejects non-http remote file URLs before fetching', async () => { + const { saveURLToS3 } = await import('../crud'); + await expect( + saveURLToS3({ + userId: 'user123', + URL: 'file:///etc/passwd', + fileName: 'passwd', + }), + ).rejects.toThrow('Refusing to fetch remote file over file:'); + + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('rejects remote responses larger than the configured cap', async () => { + process.env.REMOTE_FILE_FETCH_MAX_BYTES = '4'; + try { + (global.fetch as unknown as jest.Mock).mockResolvedValueOnce({ + ok: true, + headers: { + get: (name: string) => + ({ + 'content-length': '8', + 'content-type': 'image/jpeg', + })[name.toLowerCase()] ?? null, + }, + arrayBuffer: jest.fn().mockResolvedValue(new ArrayBuffer(8)), + }); + + const { saveURLToS3 } = await import('../crud'); + await expect( + saveURLToS3({ + userId: 'user123', + URL: 'https://example.com/image.jpg', + fileName: 'downloaded.jpg', + }), + ).rejects.toThrow('Remote file response too large: 8 bytes'); + } finally { + delete process.env.REMOTE_FILE_FETCH_MAX_BYTES; + } + }); + it('handles fetch errors', async () => { (global.fetch as unknown as jest.Mock).mockRejectedValueOnce(new Error('Network error')); diff --git a/packages/api/src/storage/s3/crud.ts b/packages/api/src/storage/s3/crud.ts index 22833f3b20..271801a51a 100644 --- a/packages/api/src/storage/s3/crud.ts +++ b/packages/api/src/storage/s3/crud.ts @@ -1,5 +1,8 @@ import fs from 'fs'; import { Readable } from 'stream'; +import { logger } from '@librechat/data-schemas'; +import { FileSources } from 'librechat-data-provider'; +import { getSignedUrl } from '@aws-sdk/s3-request-presigner'; import { UploadPartCommand, PutObjectCommand, @@ -10,16 +13,12 @@ import { HeadObjectCommand, DeleteObjectCommand, } from '@aws-sdk/client-s3'; -import { logger } from '@librechat/data-schemas'; -import { FileSources } from 'librechat-data-provider'; -import { getSignedUrl } from '@aws-sdk/s3-request-presigner'; import type { CompletedPart, GetObjectCommandInput, PutObjectCommandInput, } from '@aws-sdk/client-s3'; import type { TFile } from 'librechat-data-provider'; -import type { ServerRequest } from '~/types'; import type { UploadFileParams, SaveBufferParams, @@ -32,6 +31,20 @@ import type { UrlBuilder, S3FileRef, } from '~/storage/types'; +import type { ServerRequest } from '~/types'; +import { + assertRemoteFileURL, + getRemoteFileFetchMaxBytes, + getRemoteFileFetchTimeoutMs, + assertRemoteFileContentLength, + createRemoteFileByteLimitTransform, +} from '~/storage/url'; +import { + AVATAR_BASE_PATH, + DEFAULT_BASE_PATH as defaultBasePath, + INLINE_AVATAR_PATH_PREFIX, + INLINE_IMAGE_PATH_PREFIX, +} from '~/storage/constants'; import { assertS3FileName, assertPathSegment, @@ -39,12 +52,6 @@ import { } from '~/storage/validation'; import { initializeS3 } from '~/cdn/s3'; import { deleteRagFile } from '~/files'; -import { - AVATAR_BASE_PATH, - DEFAULT_BASE_PATH as defaultBasePath, - INLINE_AVATAR_PATH_PREFIX, - INLINE_IMAGE_PATH_PREFIX, -} from '~/storage/constants'; import { s3Config } from './s3Config'; const { @@ -551,15 +558,20 @@ export async function saveURLToS3WithMetadata({ urlBuilder, }: SaveURLParams & { urlBuilder?: UrlBuilder }): Promise { try { - const response = await fetch(URL); + const maxBytes = getRemoteFileFetchMaxBytes(); + const response = await fetch(assertRemoteFileURL(URL), { + signal: AbortSignal.timeout(getRemoteFileFetchTimeoutMs()), + }); if (!response.ok) { throw new Error(`Failed to fetch URL: ${response.status} ${response.statusText}`); } + assertRemoteFileContentLength(response.headers, maxBytes); + const contentType = response.headers.get('content-type') ?? ''; if (response.body) { const source = Readable.fromWeb( response.body as unknown as Parameters[0], - ); + ).pipe(createRemoteFileByteLimitTransform(maxBytes)); const result = await saveReadableToS3({ userId, body: source, @@ -582,6 +594,10 @@ export async function saveURLToS3WithMetadata({ } const buffer = Buffer.from(await response.arrayBuffer()); + if (buffer.length > maxBytes) { + throw new Error(`Remote file response too large: ${buffer.length} bytes`); + } + const filepath = await saveBufferToS3({ userId, buffer, diff --git a/packages/api/src/storage/url.ts b/packages/api/src/storage/url.ts new file mode 100644 index 0000000000..b1d74a45c4 --- /dev/null +++ b/packages/api/src/storage/url.ts @@ -0,0 +1,103 @@ +import { Transform } from 'stream'; +import type { TransformCallback } from 'stream'; + +export const DEFAULT_REMOTE_FILE_FETCH_TIMEOUT_MS: number = 15000; +export const DEFAULT_REMOTE_FILE_FETCH_MAX_BYTES: number = 512 * 1024 * 1024; + +const REMOTE_FILE_PROTOCOLS = new Set(['http:', 'https:']); + +type HeaderGetter = { + get: (name: string) => string | null; +}; + +type HeaderRecord = Record; + +export type RemoteFileHeaders = HeaderGetter | HeaderRecord; + +function readPositiveIntEnv(name: string, fallback: number): number { + const value = process.env[name]; + if (!value) { + return fallback; + } + + const parsed = Number.parseInt(value, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; +} + +function isHeaderGetter(headers: RemoteFileHeaders): headers is HeaderGetter { + return typeof (headers as HeaderGetter).get === 'function'; +} + +function getHeader(headers: RemoteFileHeaders, name: string): string | null { + if (isHeaderGetter(headers)) { + return headers.get(name); + } + + const value = headers[name] ?? headers[name.toLowerCase()] ?? headers[name.toUpperCase()]; + if (Array.isArray(value)) { + return value[0] ?? null; + } + + return value == null ? null : String(value); +} + +export function getRemoteFileFetchTimeoutMs(): number { + return readPositiveIntEnv('REMOTE_FILE_FETCH_TIMEOUT_MS', DEFAULT_REMOTE_FILE_FETCH_TIMEOUT_MS); +} + +export function getRemoteFileFetchMaxBytes(): number { + return readPositiveIntEnv('REMOTE_FILE_FETCH_MAX_BYTES', DEFAULT_REMOTE_FILE_FETCH_MAX_BYTES); +} + +export function assertRemoteFileURL(input: string): string { + let parsed: URL; + try { + parsed = new URL(input); + } catch { + throw new Error('Invalid remote file URL'); + } + + if (!REMOTE_FILE_PROTOCOLS.has(parsed.protocol)) { + throw new Error(`Refusing to fetch remote file over ${parsed.protocol}`); + } + + return parsed.href; +} + +export function assertRemoteFileContentLength( + headers: RemoteFileHeaders, + maxBytes: number = getRemoteFileFetchMaxBytes(), +): void { + const contentLength = Number.parseInt(getHeader(headers, 'content-length') ?? '0', 10); + if (contentLength > maxBytes) { + throw new Error(`Remote file response too large: ${contentLength} bytes`); + } +} + +class RemoteFileByteLimitTransform extends Transform { + private bytes = 0; + + constructor(private readonly maxBytes: number) { + super(); + } + + override _transform( + chunk: Buffer | string, + encoding: BufferEncoding, + callback: TransformCallback, + ): void { + this.bytes += typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength; + if (this.bytes > this.maxBytes) { + callback(new Error(`Remote file response too large: ${this.bytes} bytes`)); + return; + } + + callback(null, chunk); + } +} + +export function createRemoteFileByteLimitTransform( + maxBytes: number = getRemoteFileFetchMaxBytes(), +): Transform { + return new RemoteFileByteLimitTransform(maxBytes); +}