diff --git a/.env.example b/.env.example index 6d3c91a236..d2ef4929bf 100644 --- a/.env.example +++ b/.env.example @@ -975,6 +975,12 @@ HELP_AND_FAQ_URL=https://librechat.ai # export ships. Values above 512 MiB are ignored: V8 cannot build a string that long. # CONVERSATION_IMPORT_MAX_SHARD_SIZE_BYTES=268435456 +# How many import stages one instance runs at once, counting both archive inspection and the +# background runs, across all users. Per-user limits do not bound aggregate memory: enough +# accounts each within their own limit will still exhaust the process. Default: 3. Uploads and +# starts over the ceiling are answered with 429 and can be retried. +# CONVERSATION_IMPORT_MAX_CONCURRENT=3 + # Max size (bytes) of a code-execution artifact (docx/xlsx/csv/pptx/text/pdf) rendered as an # inline preview. Larger files fall back to download-only. Default: 2 MB (2097152). Note the # rendered HTML is independently capped at 512 KB, so very rich files may still skip preview. diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index d23dfbb747..6252985f4e 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -4,6 +4,7 @@ module.exports = { api: (overrides = {}) => ({ isEnabled: jest.fn(), resolveImportMaxFileSize: jest.fn(() => 262144000), + resolveImportMaxConcurrency: jest.fn(() => 3), createAxiosInstance: jest.fn(() => ({ get: jest.fn(), post: jest.fn(), diff --git a/api/server/routes/__tests__/convos-import-concurrency.spec.js b/api/server/routes/__tests__/convos-import-concurrency.spec.js new file mode 100644 index 0000000000..3eb546e25c --- /dev/null +++ b/api/server/routes/__tests__/convos-import-concurrency.spec.js @@ -0,0 +1,147 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const express = require('express'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const { MongoMemoryServer } = require('mongodb-memory-server'); +const { buildChatGptExportZip, cleanupChatGptExportZips } = require('~/test/chatgptExport'); +const { createModels, createMethods } = require('@librechat/data-schemas'); +const { FileSources } = require('librechat-data-provider'); + +/** One stage at a time, so a single running import puts the node at capacity + * and the next request has to be refused rather than admitted. */ +process.env.CONVERSATION_IMPORT_MAX_CONCURRENT = '1'; + +jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next()); +jest.mock('~/server/middleware', () => ({ + createImportLimiters: () => ({ + importIpLimiter: (req, res, next) => next(), + importUserLimiter: (req, res, next) => next(), + }), + createForkLimiters: () => ({ + forkIpLimiter: (req, res, next) => next(), + forkUserLimiter: (req, res, next) => next(), + }), + configMiddleware: (req, res, next) => next(), + validateConvoAccess: (req, res, next) => next(), +})); +jest.mock('~/server/utils/import/defaults', () => ({ + resolveImportDefaultModel: jest.fn().mockResolvedValue('gpt-4o-mini'), +})); +jest.mock('~/server/services/Files/strategies', () => ({ + getStrategyFunctions: jest.fn(() => ({ saveBuffer: jest.fn() })), +})); + +/** Holds the run open. Everything else in the package stays real: the point + * is to observe the ceiling while a genuine run occupies its slot, and a run + * over these fixtures finishes far too quickly to observe otherwise. */ +const mockRun = { release: null }; +jest.mock('@librechat/api', () => { + const actual = jest.requireActual('@librechat/api'); + return { + ...actual, + runImport: jest.fn( + () => + new Promise((resolve) => { + mockRun.release = () => resolve({ imported: 0, skipped: 0, failed: 0, errors: [] }); + }), + ), + }; +}); + +/** Waits for the mocked run to be entered, i.e. for the job to hold the slot. */ +async function waitForRunStart() { + for (let i = 0; i < 40; i++) { + if (mockRun.release) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error('The import run never started'); +} + +describe('import concurrency ceiling', () => { + let app; + let mongoServer; + let userId; + const uploadDirs = []; + + beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + + const models = createModels(mongoose); + Object.assign(mongoose.models, models); + await createMethods(mongoose).seedDefaultRoles(); + + const convosRouter = require('../convos'); + + app = express(); + app.use((req, res, next) => { + req.user = { id: userId, role: 'USER' }; + const uploadsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'lc-import-cap-uploads-')); + uploadDirs.push(uploadsDir); + req.config = { + paths: { uploads: uploadsDir }, + fileStrategy: FileSources.local, + interfaceConfig: {}, + }; + next(); + }); + app.use('/api/convos', convosRouter); + }); + + afterAll(async () => { + const collections = mongoose.connection.collections; + for (const key in collections) { + await collections[key].deleteMany({}); + } + await mongoose.disconnect(); + await mongoServer.stop(); + for (const dir of uploadDirs) { + fs.rmSync(dir, { recursive: true, force: true }); + } + cleanupChatGptExportZips(); + delete process.env.CONVERSATION_IMPORT_MAX_CONCURRENT; + }); + + beforeEach(() => { + userId = new mongoose.Types.ObjectId().toString(); + mockRun.release = null; + }); + + /** The per-user limit lets a second account through; the node-wide ceiling + * is what keeps two exports from being parsed in one heap at once. */ + it("refuses a second user's upload while the node is at capacity, and admits it once the run ends", async () => { + const filepath = await buildChatGptExportZip(); + + const uploaded = await request(app) + .post('/api/convos/import') + .attach('file', filepath) + .expect(202); + await request(app).post(`/api/convos/import/jobs/${uploaded.body.jobId}/start`).expect(202); + await waitForRunStart(); + + const owner = userId; + userId = new mongoose.Types.ObjectId().toString(); + const refused = await request(app) + .post('/api/convos/import') + .attach('file', filepath) + .expect(429); + expect(refused.body.message).toBe('Too many imports are running, try again shortly'); + + userId = owner; + mockRun.release(); + + for (let i = 0; i < 40; i++) { + const status = await request(app).get(`/api/convos/import/jobs/${uploaded.body.jobId}`); + if (status.body.phase === 'completed') { + break; + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + + await request(app).post('/api/convos/import').attach('file', filepath).expect(202); + }); +}); diff --git a/api/server/routes/__tests__/convos-import.spec.js b/api/server/routes/__tests__/convos-import.spec.js index 633380b119..bbc68f71c7 100644 --- a/api/server/routes/__tests__/convos-import.spec.js +++ b/api/server/routes/__tests__/convos-import.spec.js @@ -6,7 +6,11 @@ const request = require('supertest'); const mongoose = require('mongoose'); const multer = require('multer'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { buildChatGptExportZip, cleanupChatGptExportZips } = require('~/test/chatgptExport'); +const { + buildChatGptExportZip, + buildMixedAssetExportZip, + cleanupChatGptExportZips, +} = require('~/test/chatgptExport'); const { bareClaudeExport, buildClaudeExportZip, @@ -32,6 +36,9 @@ jest.mock('~/server/middleware', () => ({ jest.mock('~/server/utils/import/defaults', () => ({ resolveImportDefaultModel: jest.fn().mockResolvedValue('gpt-4o-mini'), })); +/** Every `saveBuffer` the job pipeline issued, so a test can assert which + * storage base path each asset type was written under. */ +const mockSavedAssets = []; /** * The real local storage strategy writes under the repo's `client/public` * tree. This route test only cares that the job pipeline calls `saveBuffer` @@ -40,7 +47,8 @@ jest.mock('~/server/utils/import/defaults', () => ({ */ jest.mock('~/server/services/Files/strategies', () => ({ getStrategyFunctions: jest.fn(() => ({ - saveBuffer: async ({ buffer, fileName }) => { + saveBuffer: async ({ buffer, fileName, basePath }) => { + mockSavedAssets.push({ fileName, basePath }); const nodeFs = require('fs'); const nodePath = require('path'); const nodeOs = require('os'); @@ -306,6 +314,30 @@ describe('conversation import job API (real router, real Mongo)', () => { expect(savedConvos).toBe(2); }); + /** `client/public/images` is served statically, and authentication over it + * is off unless `secureImageLinks` is enabled — so an imported PDF or audio + * file written to the strategy's default base would be anonymously + * retrievable by URL. Only images, which already render from that tree, + * belong there. */ + it('writes non-image assets under uploads and images under images', async () => { + mockSavedAssets.length = 0; + const filepath = await buildMixedAssetExportZip(); + + const uploaded = await request(app) + .post('/api/convos/import') + .attach('file', filepath) + .expect(202); + await request(app).post(`/api/convos/import/jobs/${uploaded.body.jobId}/start`).expect(202); + + const completed = await waitForTerminal(app, uploaded.body.jobId); + expect(completed.body.phase).toBe('completed'); + + const basePathFor = (suffix) => + mockSavedAssets.find((call) => call.fileName.endsWith(suffix))?.basePath; + expect(basePathFor('photo.jpg')).toBe('images'); + expect(basePathFor('voice.wav')).toBe('uploads'); + }); + it('rejects a file that is neither json nor zip', async () => { const rejected = await request(app) .post('/api/convos/import') diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index 8b0ec0a1e1..31f1ae9ef2 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -14,6 +14,7 @@ const { deleteAgentCheckpoints, sanitizeImportError, resolveImportMaxFileSize, + resolveImportMaxConcurrency, restoreTenantContextFromReq, deleteAllSharedLinksWithCleanup, deleteConvoSharedLinksWithCleanup, @@ -320,6 +321,16 @@ const importJobs = new ImportJobStore(getLogStores(CacheKeys.IMPORT_JOBS)); */ const activeImports = new Map(); const MAX_CONCURRENT_IMPORTS_PER_USER = 1; +/** + * Heavy import stages in flight across every user on this node. The per-user + * map bounds what one account can start; it says nothing about aggregate work, + * so twenty accounts each inside their own limit are still twenty concurrent + * shard parses on one process. Inspection is counted with the runs because it + * is the same cost in the same heap — it decompresses and JSON-parses the + * whole export in the upload request to build the summary. + */ +const MAX_CONCURRENT_IMPORT_STAGES = resolveImportMaxConcurrency(); +let activeImportStages = 0; /** Minimum gap between job-store progress writes. The client polls every two * seconds, so anything below that is written for nobody to read. */ const PROGRESS_WRITE_INTERVAL_MS = 500; @@ -330,9 +341,11 @@ function activeImportCount(userId) { function trackImportStart(userId) { activeImports.set(userId, activeImportCount(userId) + 1); + activeImportStages += 1; } function trackImportEnd(userId) { + activeImportStages = Math.max(activeImportStages - 1, 0); const remaining = activeImportCount(userId) - 1; if (remaining > 0) { activeImports.set(userId, remaining); @@ -341,6 +354,22 @@ function trackImportEnd(userId) { activeImports.delete(userId); } +function atImportCapacity() { + return activeImportStages >= MAX_CONCURRENT_IMPORT_STAGES; +} + +/** Runs the archive inspection under the process-wide ceiling, so a burst of + * uploads queues behind the ceiling as 429s instead of parsing every archive + * at once. */ +async function inspectUnderCapacity(filepath) { + activeImportStages += 1; + try { + return await inspectExport(filepath); + } finally { + activeImportStages = Math.max(activeImportStages - 1, 0); + } +} + function handleUpload(req, res, next) { uploadSingle(req, res, (err) => { if (err && err.code === 'LIMIT_FILE_SIZE') { @@ -453,8 +482,24 @@ async function runImportJob(context, job) { */ const imageSource = getFileStrategy(appConfig, { isImage: true }); const documentSource = getFileStrategy(appConfig, { isImage: false }); - const imageBackend = { source: imageSource, fns: getStrategyFunctions(imageSource) }; - const documentBackend = { source: documentSource, fns: getStrategyFunctions(documentSource) }; + /** + * `basePath` travels with the backend because the strategies default it to + * `images`, and on the local strategy that base is `client/public/images`, + * which is served statically with no auth unless `secureImageLinks` is on. + * A PDF or audio attachment written there is anonymously retrievable by + * URL, so non-images take the same `uploads` base every other document + * upload in the app uses, reachable only through the file download route. + */ + const imageBackend = { + source: imageSource, + fns: getStrategyFunctions(imageSource), + basePath: 'images', + }; + const documentBackend = { + source: documentSource, + fns: getStrategyFunctions(documentSource), + basePath: 'uploads', + }; const backendFor = (type) => (type ?? '').startsWith('image/') ? imageBackend : documentBackend; @@ -464,6 +509,7 @@ async function runImportJob(context, job) { userId: owner, buffer, fileName, + basePath: backend.basePath, tenantId: tenant, }); return { filepath, source: backend.source }; @@ -618,11 +664,20 @@ router.post( return; } + /** Rejected before the archive is opened rather than queued: the upload is + * already on disk and the client polls anyway, so asking it to retry costs + * a request, while admitting it costs another whole export in heap. */ + if (atImportCapacity()) { + await fs.promises.unlink(req.file.path).catch(() => undefined); + res.status(429).json({ message: 'Too many imports are running, try again shortly' }); + return; + } + const isZip = path.extname(req.file.originalname).toLowerCase() === '.zip'; let inspected; try { - inspected = await inspectExport(req.file.path); + inspected = await inspectUnderCapacity(req.file.path); } catch (error) { if (!isZip && error instanceof Error && error.message === 'Unsupported import type') { await importLegacyConversation(req, res); @@ -698,6 +753,15 @@ router.post( return; } + /** Same handling for the node-wide ceiling: this user is within their own + * limit, but the process is not, so the job waits where it was rather than + * failing. */ + if (atImportCapacity()) { + await importJobs.patch(req.user.id, result.job.jobId, { phase: 'awaiting_confirmation' }); + res.status(429).json({ message: 'Too many imports are running, try again shortly' }); + return; + } + /** Everything the background run needs, read while the request is still * the thing being handled. Holding `req` instead would pin the socket and * its buffers for the length of a multi-minute import, per concurrent job. */ diff --git a/api/test/chatgptExport.js b/api/test/chatgptExport.js index 3397536e6c..57a3b197fa 100644 --- a/api/test/chatgptExport.js +++ b/api/test/chatgptExport.js @@ -100,6 +100,56 @@ async function buildChatGptExportZip() { return filepath; } +/** + * Export archive whose single conversation references one image and one audio + * attachment, for tests that care about how each asset type is stored rather + * than about sharding or manifests. + * @returns {Promise} path to the written zip + */ +async function buildMixedAssetExportZip() { + const zip = new JSZip(); + + zip.file( + 'conversations.json', + JSON.stringify([ + conversation('ext-mixed', 'Trip notes', 1700002000, { + a1: { + id: 'a1', + parent: 'u1', + children: [], + message: { + id: 'a1', + author: { role: 'assistant', name: null }, + create_time: 1700002002, + content: { + content_type: 'multimodal_text', + parts: [ + 'Here you go.', + { content_type: 'image_asset_pointer', asset_pointer: 'file-service://asset-img' }, + { content_type: 'audio_asset_pointer', asset_pointer: 'file-service://asset-aud' }, + ], + }, + metadata: { model_slug: 'gpt-4o' }, + }, + }, + }), + ]), + ); + zip.file( + 'conversation_asset_file_names.json', + JSON.stringify({ 'asset-img.dat': 'photo.jpg', 'asset-aud.dat': 'voice.wav' }), + ); + zip.file('asset-img.dat', Buffer.from([1, 2, 3, 4])); + zip.file('asset-aud.dat', Buffer.from([5, 6, 7, 8])); + + const buffer = await zip.generateAsync({ type: 'nodebuffer' }); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'lc-import-route-fixture-')); + createdDirs.push(dir); + const filepath = path.join(dir, 'chatgpt-mixed-assets.zip'); + fs.writeFileSync(filepath, buffer); + return filepath; +} + function cleanupChatGptExportZips() { while (createdDirs.length > 0) { const dir = createdDirs.pop(); @@ -107,4 +157,8 @@ function cleanupChatGptExportZips() { } } -module.exports = { buildChatGptExportZip, cleanupChatGptExportZips }; +module.exports = { + buildChatGptExportZip, + buildMixedAssetExportZip, + cleanupChatGptExportZips, +}; diff --git a/packages/api/src/utils/__tests__/import.test.ts b/packages/api/src/utils/__tests__/import.test.ts index 2d4aac76b5..b1fdf48a7d 100644 --- a/packages/api/src/utils/__tests__/import.test.ts +++ b/packages/api/src/utils/__tests__/import.test.ts @@ -5,8 +5,10 @@ jest.mock('@librechat/data-schemas', () => ({ import { DEFAULT_IMPORT_MAX_FILE_SIZE, DEFAULT_IMPORT_MAX_SHARD_SIZE, + DEFAULT_IMPORT_MAX_CONCURRENCY, resolveImportMaxFileSize, resolveImportMaxShardSize, + resolveImportMaxConcurrency, } from '../import'; import { logger } from '@librechat/data-schemas'; @@ -107,3 +109,34 @@ describe('resolveImportMaxShardSize', () => { expect(resolveImportMaxShardSize()).toBe(DEFAULT_IMPORT_MAX_SHARD_SIZE); }); }); + +describe('resolveImportMaxConcurrency', () => { + const original = process.env.CONVERSATION_IMPORT_MAX_CONCURRENT; + + afterEach(() => { + if (original === undefined) { + delete process.env.CONVERSATION_IMPORT_MAX_CONCURRENT; + return; + } + process.env.CONVERSATION_IMPORT_MAX_CONCURRENT = original; + }); + + it('defaults to three stages', () => { + delete process.env.CONVERSATION_IMPORT_MAX_CONCURRENT; + expect(resolveImportMaxConcurrency()).toBe(DEFAULT_IMPORT_MAX_CONCURRENCY); + expect(DEFAULT_IMPORT_MAX_CONCURRENCY).toBe(3); + }); + + it('honours a configured override', () => { + process.env.CONVERSATION_IMPORT_MAX_CONCURRENT = '8'; + expect(resolveImportMaxConcurrency()).toBe(8); + }); + + /** A fractional ceiling would admit a stage the operator never allowed for: + * `2.5` reads as "two and a half imports", and the comparison rounds it up + * to three. */ + it.each(['0', '-1', '2.5', 'not-a-number'])('falls back to the default for %s', (raw) => { + process.env.CONVERSATION_IMPORT_MAX_CONCURRENT = raw; + expect(resolveImportMaxConcurrency()).toBe(DEFAULT_IMPORT_MAX_CONCURRENCY); + }); +}); diff --git a/packages/api/src/utils/import.ts b/packages/api/src/utils/import.ts index 1e1dcd2bc6..8698ff522b 100644 --- a/packages/api/src/utils/import.ts +++ b/packages/api/src/utils/import.ts @@ -19,6 +19,33 @@ export function resolveImportMaxFileSize(): number { return parsed; } +/** Default number of import stages one node will run at once. */ +export const DEFAULT_IMPORT_MAX_CONCURRENCY = 3; + +/** + * Resolves the node-wide ceiling on concurrent import stages. + * + * Per-user limits bound what one account can start and say nothing about the + * aggregate: each stage buffers, decodes and `JSON.parse`s a shard at roughly + * 3.2x its size in heap, so enough accounts each inside their own limit will + * still exhaust the process. This is the ceiling on all of them together, and + * the env var is how a deployment with more headroom raises it. + */ +export function resolveImportMaxConcurrency(): number { + const raw = process.env.CONVERSATION_IMPORT_MAX_CONCURRENT; + if (!raw) { + return DEFAULT_IMPORT_MAX_CONCURRENCY; + } + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed <= 0) { + logger.warn( + `[imports] Invalid CONVERSATION_IMPORT_MAX_CONCURRENT="${raw}"; using default ${DEFAULT_IMPORT_MAX_CONCURRENCY}`, + ); + return DEFAULT_IMPORT_MAX_CONCURRENCY; + } + return parsed; +} + /** 256 MiB — default cap on a single decompressed archive entry. */ export const DEFAULT_IMPORT_MAX_SHARD_SIZE = 268435456;