From b6879220aaddf15cf451c6284716e84b843bdb51 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 27 Jul 2026 19:07:44 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20fix:=20Bound=20`/files/usage`=20?= =?UTF-8?q?TTL=20Hold=20Instead=20of=20Clearing=20It?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /files/usage` marks queued attachments so the 1-hour upload-window TTL cannot reap them before the client queue drains. It did this by calling `updateFilesUsage`, which unsets `expiresAt` outright, turning every touched upload into a permanently retained file. The client queue is ephemeral browser state, so this also leaks in normal use: a closed tab or cleared queue leaves nothing referencing the files, but their TTL is already gone. The same mechanism let an authenticated user pin arbitrary owned uploads indefinitely, and the route was excluded from the file limiters, so the touch was entirely unmetered. Make the operation match its intent, a renewable hold rather than a release: - Add `extendFilesTTL`, which pushes `expiresAt` forward by a bounded window in a single owner-scoped `updateMany`. Two filter guards keep it safe under client-supplied ids: `$exists: true` so an already-released file never has a TTL re-added (that would schedule a live file for deletion), and `$lt` so a hold only ever moves the deadline later. The owner scope is a required argument, so an unscoped call is a no-op rather than a cross-user update. - `handleFilesUsageRequest` now holds for 24h instead of clearing, and no longer increments `usage`, since a queue touch is not a send. The real release still happens at drain, where `updateFilesUsage` marks the files used against an actual message. - Give `/usage` its own per-user limiter. Keeping it off the upload quota was intentional, leaving it unmetered was not. Abandoned queues are now reaped on schedule, and a replayed touch can only ever re-assert the same bounded window. --- api/.node_modules-2xgm5Cvq | 1 + .../middleware/limiters/uploadLimiters.js | 33 +++++ api/server/routes/files/files.js | 9 +- api/server/routes/files/files.test.js | 80 ++++++++++-- api/server/routes/files/index.js | 24 ++-- .../routes/files/index.limiters.test.js | 115 ++++++++++++++++++ api/server/routes/files/index.tenant.test.js | 1 + client/.node_modules-101IklEQ | 1 + client/src/hooks/Chat/useSteering.ts | 13 +- packages/api/.node_modules-uphMfdfi | 1 + packages/api/src/files/usage.spec.ts | 43 ++++--- packages/api/src/files/usage.ts | 48 +++++--- packages/data-provider/.node_modules-7gquYG9s | 1 + packages/data-provider/src/types/files.ts | 3 +- .../data-schemas/src/methods/file.spec.ts | 87 +++++++++++++ packages/data-schemas/src/methods/file.ts | 46 +++++++ 16 files changed, 446 insertions(+), 60 deletions(-) create mode 120000 api/.node_modules-2xgm5Cvq create mode 100644 api/server/routes/files/index.limiters.test.js create mode 120000 client/.node_modules-101IklEQ create mode 120000 packages/api/.node_modules-uphMfdfi create mode 120000 packages/data-provider/.node_modules-7gquYG9s diff --git a/api/.node_modules-2xgm5Cvq b/api/.node_modules-2xgm5Cvq new file mode 120000 index 0000000000..1fb9cb10df --- /dev/null +++ b/api/.node_modules-2xgm5Cvq @@ -0,0 +1 @@ +/Users/danny/Projects/LibreChat/api/node_modules \ No newline at end of file diff --git a/api/server/middleware/limiters/uploadLimiters.js b/api/server/middleware/limiters/uploadLimiters.js index 8c878cfa86..ab138b8679 100644 --- a/api/server/middleware/limiters/uploadLimiters.js +++ b/api/server/middleware/limiters/uploadLimiters.js @@ -80,6 +80,39 @@ const createFileLimiters = () => { return { fileUploadIpLimiter, fileUploadUserLimiter }; }; +/** + * Per-user limiter for the `/files/usage` TTL hold. Deliberately separate from + * the upload limiters: a metadata touch must not consume upload quota, but it + * still writes to the DB and so cannot go unmetered. Sized well above the + * enqueue-driven call rate a real client produces. + */ +const createFileUsageLimiter = () => { + const windowMinutes = parseInt(process.env.FILE_USAGE_USER_WINDOW) || 15; + const max = parseInt(process.env.FILE_USAGE_USER_MAX) || 120; + const windowMs = windowMinutes * 60 * 1000; + + return rateLimit({ + windowMs, + max, + handler: async (req, res) => { + const type = ViolationTypes.FILE_UPLOAD_LIMIT; + await logViolation( + req, + res, + type, + { type, max, limiter: 'user', windowInMinutes: windowMinutes }, + process.env.FILE_UPLOAD_VIOLATION_SCORE, + ); + res.status(429).json({ message: 'Too many file usage requests. Try again later' }); + }, + keyGenerator: function (req) { + return req.user?.id; + }, + store: limiterCache('file_usage_user_limiter'), + }); +}; + module.exports = { createFileLimiters, + createFileUsageLimiter, }; diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index d98ed3b6f8..d4b579d13d 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -144,15 +144,16 @@ router.get('/config', async (req, res) => { /** * POST /files/usage * - * Owner-scoped TTL touch for uploads held in a client-side queue (mid-run + * Owner-scoped TTL hold for uploads sitting in a client-side queue (mid-run * queued messages), so the upload-window TTL cannot reap them before drain. - * Thin wrapper: validation, cap, and best-effort semantics live in - * `@librechat/api` (`handleFilesUsageRequest`). + * Extends the deadline rather than clearing it; the real release happens at + * send. Thin wrapper: validation, cap, hold window, and best-effort semantics + * live in `@librechat/api` (`handleFilesUsageRequest`). */ router.post('/usage', async (req, res) => { try { const { status, body } = await handleFilesUsageRequest(req.user ?? {}, req.body ?? {}, { - updateFilesUsage: db.updateFilesUsage, + extendFilesTTL: db.extendFilesTTL, }); return res.status(status).json(body); } catch (error) { diff --git a/api/server/routes/files/files.test.js b/api/server/routes/files/files.test.js index 6cd60d43d6..90d4eb81ee 100644 --- a/api/server/routes/files/files.test.js +++ b/api/server/routes/files/files.test.js @@ -938,7 +938,7 @@ describe('File Routes - Delete with Agent Access', () => { }); describe('POST /files/usage', () => { - it('marks owned files used and clears the upload TTL', async () => { + const createQueuedFile = async (expiresAt) => { const ownFileId = uuidv4(); await createFile({ user: otherUserId, @@ -948,31 +948,93 @@ describe('File Routes - Delete with Agent Access', () => { bytes: 10, type: 'image/png', }); - await File.updateOne({ file_id: ownFileId }, { $set: { expiresAt: new Date() } }); + await File.updateOne({ file_id: ownFileId }, { $set: { expiresAt } }); + return ownFileId; + }; + + it('extends the upload TTL of owned files without clearing it', async () => { + const soon = new Date(Date.now() + 60 * 1000); + const ownFileId = await createQueuedFile(soon); const response = await request(app) .post('/files/usage') .send({ file_ids: [ownFileId] }); expect(response.status).toBe(200); - expect(response.body).toEqual({ marked: 1 }); - const marked = await File.findOne({ file_id: ownFileId }).lean(); - expect(marked.usage).toBe(1); - expect(marked.expiresAt).toBeUndefined(); + expect(response.body).toEqual({ held: 1 }); + const held = await File.findOne({ file_id: ownFileId }).lean(); + /* The hold must remain a hold: still reapable, just later. */ + expect(held.expiresAt).toBeDefined(); + expect(held.expiresAt.getTime()).toBeGreaterThan(soon.getTime()); + /* A queue touch is not a send, so it must not inflate usage. */ + expect(held.usage).toBe(0); + }); + + it('cannot be replayed to preserve a file indefinitely', async () => { + const ownFileId = await createQueuedFile(new Date(Date.now() + 60 * 1000)); + + for (let i = 0; i < 5; i++) { + const response = await request(app) + .post('/files/usage') + .send({ file_ids: [ownFileId] }); + expect(response.status).toBe(200); + } + + const held = await File.findOne({ file_id: ownFileId }).lean(); + expect(held.expiresAt).toBeDefined(); + /* Repeated touches converge on one bounded window, never unset the TTL. */ + expect(held.expiresAt.getTime()).toBeLessThan(Date.now() + 25 * 60 * 60 * 1000); + }); + + it('never re-adds a TTL to a file that was already sent', async () => { + const sentFileId = uuidv4(); + await createFile({ + user: otherUserId, + file_id: sentFileId, + filename: 'sent.png', + filepath: '/uploads/sent.png', + bytes: 10, + type: 'image/png', + }); + await File.updateOne({ file_id: sentFileId }, { $unset: { expiresAt: '' } }); + + const response = await request(app) + .post('/files/usage') + .send({ file_ids: [sentFileId] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ held: 0 }); + const permanent = await File.findOne({ file_id: sentFileId }).lean(); + expect(permanent.expiresAt).toBeUndefined(); + }); + + it('never shortens an existing hold', async () => { + const farOut = new Date(Date.now() + 90 * 24 * 60 * 60 * 1000); + const ownFileId = await createQueuedFile(farOut); + + const response = await request(app) + .post('/files/usage') + .send({ file_ids: [ownFileId] }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ held: 0 }); + const untouched = await File.findOne({ file_id: ownFileId }).lean(); + expect(untouched.expiresAt.getTime()).toBe(farOut.getTime()); }); it("is owner-scoped: another user's file stays untouched (best-effort 200)", async () => { - await File.updateOne({ file_id: fileId }, { $set: { expiresAt: new Date() } }); + const soon = new Date(Date.now() + 60 * 1000); + await File.updateOne({ file_id: fileId }, { $set: { expiresAt: soon } }); const response = await request(app) .post('/files/usage') .send({ file_ids: [fileId] }); expect(response.status).toBe(200); - expect(response.body).toEqual({ marked: 0 }); + expect(response.body).toEqual({ held: 0 }); const untouched = await File.findOne({ file_id: fileId }).lean(); expect(untouched.usage).toBe(0); - expect(untouched.expiresAt).toBeDefined(); + expect(untouched.expiresAt.getTime()).toBe(soon.getTime()); }); it('rejects a list over the cap', async () => { diff --git a/api/server/routes/files/index.js b/api/server/routes/files/index.js index f7e2428c4c..c479ff945b 100644 --- a/api/server/routes/files/index.js +++ b/api/server/routes/files/index.js @@ -1,5 +1,6 @@ const express = require('express'); const { + createFileUsageLimiter, createFileLimiters, configMiddleware, requireJwtAuth, @@ -30,19 +31,24 @@ const initialize = async () => { router.use('/speech', speech); const { fileUploadIpLimiter, fileUploadUserLimiter } = createFileLimiters(); + const fileUsageLimiter = createFileUsageLimiter(); /** Apply rate limiters to all POST routes (excluding /speech which is handled - * above, and /usage — a metadata touch that must not consume upload quota) */ + * above). `/usage` is a metadata touch, so it gets its own limiter rather + * than consuming upload quota, but it is never left unmetered. */ router.use((req, res, next) => { - if (req.method === 'POST' && !req.path.startsWith('/speech') && req.path !== '/usage') { - return fileUploadIpLimiter(req, res, (err) => { - if (err) { - return next(err); - } - return fileUploadUserLimiter(req, res, next); - }); + if (req.method !== 'POST' || req.path.startsWith('/speech')) { + return next(); } - next(); + if (req.path === '/usage') { + return fileUsageLimiter(req, res, next); + } + return fileUploadIpLimiter(req, res, (err) => { + if (err) { + return next(err); + } + return fileUploadUserLimiter(req, res, next); + }); }); router.post('/', upload.single('file'), restoreTenantContextFromReq); diff --git a/api/server/routes/files/index.limiters.test.js b/api/server/routes/files/index.limiters.test.js new file mode 100644 index 0000000000..a78be3c31b --- /dev/null +++ b/api/server/routes/files/index.limiters.test.js @@ -0,0 +1,115 @@ +const express = require('express'); +const request = require('supertest'); + +const hits = { uploadIp: 0, uploadUser: 0, usage: 0 }; + +jest.mock('~/server/middleware', () => ({ + createFileLimiters: jest.fn(() => ({ + fileUploadIpLimiter: (req, res, next) => { + hits.uploadIp++; + next(); + }, + fileUploadUserLimiter: (req, res, next) => { + hits.uploadUser++; + next(); + }, + })), + createFileUsageLimiter: jest.fn(() => (req, res, next) => { + hits.usage++; + next(); + }), + configMiddleware: (req, res, next) => { + req.config = { fileStrategy: 'local', paths: { uploads: '/tmp', images: '/tmp' } }; + next(); + }, + requireJwtAuth: (req, res, next) => { + req.user = { id: 'user-1', role: 'USER' }; + next(); + }, + uaParser: (req, res, next) => next(), + checkBan: (req, res, next) => next(), +})); + +jest.mock('./multer', () => ({ + createMulterInstance: jest.fn(async () => ({ + single: jest.fn(() => (req, res, next) => next()), + })), +})); + +const okRouter = (paths) => { + const router = express.Router(); + for (const path of paths) { + router.post(path, (req, res) => res.status(200).json({ ok: true })); + } + return router; +}; + +jest.mock('./files', () => { + const express = require('express'); + const router = express.Router(); + router.post('/', (req, res) => res.status(200).json({ ok: true })); + router.post('/usage', (req, res) => res.status(200).json({ ok: true })); + return router; +}); + +jest.mock('./images', () => okRouter(['/'])); +jest.mock('./avatar', () => okRouter(['/'])); +jest.mock('./speech', () => okRouter(['/stt'])); + +jest.mock('~/server/routes/agents/v1', () => ({ + avatar: okRouter(['/:agent_id/avatar/']), +})); +jest.mock('~/server/routes/assistants/v1', () => ({ + avatar: okRouter(['/:assistant_id/avatar/']), +})); + +describe('file route limiter wiring', () => { + let app; + + beforeAll(async () => { + const { initialize } = require('./index'); + app = express(); + app.use(express.json()); + app.use('/api/files', await initialize()); + }); + + beforeEach(() => { + hits.uploadIp = 0; + hits.uploadUser = 0; + hits.usage = 0; + }); + + it('meters POST /usage with the usage limiter, never leaving it unlimited', async () => { + const response = await request(app) + .post('/api/files/usage') + .send({ file_ids: ['f1'] }); + + expect(response.status).toBe(200); + expect(hits.usage).toBe(1); + }); + + it('keeps POST /usage off the upload quota', async () => { + await request(app) + .post('/api/files/usage') + .send({ file_ids: ['f1'] }); + + expect(hits.uploadIp).toBe(0); + expect(hits.uploadUser).toBe(0); + }); + + it('still applies the upload limiters to real uploads', async () => { + await request(app).post('/api/files').send({}); + + expect(hits.uploadIp).toBe(1); + expect(hits.uploadUser).toBe(1); + expect(hits.usage).toBe(0); + }); + + it('leaves /speech exempt from both limiters', async () => { + await request(app).post('/api/files/speech/stt').send({}); + + expect(hits.uploadIp).toBe(0); + expect(hits.uploadUser).toBe(0); + expect(hits.usage).toBe(0); + }); +}); diff --git a/api/server/routes/files/index.tenant.test.js b/api/server/routes/files/index.tenant.test.js index 9b703d7524..b2071f06d7 100644 --- a/api/server/routes/files/index.tenant.test.js +++ b/api/server/routes/files/index.tenant.test.js @@ -21,6 +21,7 @@ jest.mock('~/server/middleware', () => ({ fileUploadIpLimiter: (req, res, next) => next(), fileUploadUserLimiter: (req, res, next) => next(), })), + createFileUsageLimiter: jest.fn(() => (req, res, next) => next()), configMiddleware: (req, res, next) => { req.config = { fileStrategy: 'local', diff --git a/client/.node_modules-101IklEQ b/client/.node_modules-101IklEQ new file mode 120000 index 0000000000..49cee9a7d7 --- /dev/null +++ b/client/.node_modules-101IklEQ @@ -0,0 +1 @@ +/Users/danny/Projects/LibreChat/client/node_modules \ No newline at end of file diff --git a/client/src/hooks/Chat/useSteering.ts b/client/src/hooks/Chat/useSteering.ts index 2e5467464b..a0ad175c5a 100644 --- a/client/src/hooks/Chat/useSteering.ts +++ b/client/src/hooks/Chat/useSteering.ts @@ -251,10 +251,11 @@ export default function useSteering({ [], ); - /** Fire-and-forget TTL touch for uploads entering the client queue: a + /** Fire-and-forget TTL hold for uploads entering the client queue: a * queued message can outlive the upload window (long run, approval pause) - * and send-time marking only happens at drain. Failure is tolerated — - * the send-time marking remains the backstop. */ + * and send-time marking only happens at drain. The server extends the + * deadline rather than clearing it, so a queue this tab never drains still + * gets reaped. Failure is tolerated; send-time marking is the backstop. */ const markQueuedFilesUsage = useCallback( (files?: TMessage['files']) => { if (files == null || files.length === 0) { @@ -285,8 +286,8 @@ export default function useSteering({ files?: TMessage['files']; quotes?: string[]; manualSkills?: string[]; - /** Set when the files were ALREADY queued/steered — their usage was - * marked when they first entered the queue (or at the steer 202). */ + /** Set when the files were ALREADY queued/steered: their TTL was + * held when they first entered the queue (or at the steer 202). */ skipUsageMark?: boolean; }, ) => { @@ -694,7 +695,7 @@ export default function useSteering({ } return; } - // Files were already marked used when the item first entered the queue. + // Files were already TTL-held when the item first entered the queue. enqueue(taken.text, { front: true, files: taken.files, diff --git a/packages/api/.node_modules-uphMfdfi b/packages/api/.node_modules-uphMfdfi new file mode 120000 index 0000000000..07571e485c --- /dev/null +++ b/packages/api/.node_modules-uphMfdfi @@ -0,0 +1 @@ +/Users/danny/Projects/LibreChat/packages/api/node_modules \ No newline at end of file diff --git a/packages/api/src/files/usage.spec.ts b/packages/api/src/files/usage.spec.ts index 59de9cbffd..f705bc759a 100644 --- a/packages/api/src/files/usage.spec.ts +++ b/packages/api/src/files/usage.spec.ts @@ -1,17 +1,19 @@ -import { handleFilesUsageRequest, FILES_USAGE_MAX_IDS } from './usage'; +import { handleFilesUsageRequest, FILES_USAGE_MAX_IDS, FILES_USAGE_HOLD_MS } from './usage'; describe('handleFilesUsageRequest', () => { const user = { id: 'user-1', tenantId: 'tenant-1' }; + const NOW = 1_700_000_000_000; - const createDeps = (marked: unknown[] = []) => ({ - updateFilesUsage: jest.fn().mockResolvedValue(marked), + const createDeps = (held = 0) => ({ + extendFilesTTL: jest.fn().mockResolvedValue(held), + now: () => NOW, }); it('rejects unauthenticated requests without touching the DB', async () => { const deps = createDeps(); const result = await handleFilesUsageRequest({}, { file_ids: ['f1'] }, deps); expect(result).toEqual({ status: 401, body: { code: 'UNAUTHORIZED' } }); - expect(deps.updateFilesUsage).not.toHaveBeenCalled(); + expect(deps.extendFilesTTL).not.toHaveBeenCalled(); }); it.each([ @@ -25,7 +27,7 @@ describe('handleFilesUsageRequest', () => { const result = await handleFilesUsageRequest(user, body, deps); expect(result.status).toBe(400); expect(result.body).toEqual({ code: 'INVALID_FILE_IDS' }); - expect(deps.updateFilesUsage).not.toHaveBeenCalled(); + expect(deps.extendFilesTTL).not.toHaveBeenCalled(); }); it('caps the list at FILES_USAGE_MAX_IDS', async () => { @@ -34,24 +36,33 @@ describe('handleFilesUsageRequest', () => { const result = await handleFilesUsageRequest(user, { file_ids }, deps); expect(result.status).toBe(400); expect(result.body).toEqual({ code: 'TOO_MANY_FILES', max: FILES_USAGE_MAX_IDS }); - expect(deps.updateFilesUsage).not.toHaveBeenCalled(); + expect(deps.extendFilesTTL).not.toHaveBeenCalled(); }); - it('marks usage owner-scoped and returns best-effort 200', async () => { - const deps = createDeps([{ file_id: 'f1' }]); + it('extends the hold owner-scoped by a bounded window', async () => { + const deps = createDeps(1); const result = await handleFilesUsageRequest(user, { file_ids: ['f1', 'f2'] }, deps); - expect(deps.updateFilesUsage).toHaveBeenCalledTimes(1); - expect(deps.updateFilesUsage).toHaveBeenCalledWith( - [{ file_id: 'f1' }, { file_id: 'f2' }], - undefined, + expect(deps.extendFilesTTL).toHaveBeenCalledTimes(1); + expect(deps.extendFilesTTL).toHaveBeenCalledWith( + ['f1', 'f2'], + new Date(NOW + FILES_USAGE_HOLD_MS), { user: 'user-1', tenantId: 'tenant-1' }, ); - expect(result).toEqual({ status: 200, body: { marked: 1 } }); + expect(result).toEqual({ status: 200, body: { held: 1 } }); }); - it('returns 200 with zero marked when no id resolves to an owned file', async () => { - const deps = createDeps([]); + it('never requests an unbounded hold', async () => { + const deps = createDeps(1); + await handleFilesUsageRequest(user, { file_ids: ['f1'] }, deps); + const [, expiresAt] = deps.extendFilesTTL.mock.calls[0]; + expect(expiresAt).toBeInstanceOf(Date); + expect(Number.isFinite((expiresAt as Date).getTime())).toBe(true); + expect((expiresAt as Date).getTime()).toBeGreaterThan(NOW); + }); + + it('returns 200 with zero held when no id resolves to an owned file', async () => { + const deps = createDeps(0); const result = await handleFilesUsageRequest(user, { file_ids: ['not-owned'] }, deps); - expect(result).toEqual({ status: 200, body: { marked: 0 } }); + expect(result).toEqual({ status: 200, body: { held: 0 } }); }); }); diff --git a/packages/api/src/files/usage.ts b/packages/api/src/files/usage.ts index fdb20127ed..b0494b72c2 100644 --- a/packages/api/src/files/usage.ts +++ b/packages/api/src/files/usage.ts @@ -1,6 +1,13 @@ /** Cap per usage touch, mirroring the composer's practical attachment limit. */ export const FILES_USAGE_MAX_IDS: number = 10; +/** + * How far forward a single touch pushes the upload-window TTL. Generous + * enough to outlast any realistic queue wait (long run, approval pause), + * short enough that a queue the user abandons still gets reaped. + */ +export const FILES_USAGE_HOLD_MS: number = 24 * 60 * 60 * 1000; + export interface FilesUsageUser { id?: string; tenantId?: string; @@ -17,20 +24,30 @@ export interface FilesUsageResult { } export interface FilesUsageDeps { - /** Owner-scoped usage marker (`db.updateFilesUsage`-shaped). */ - updateFilesUsage: ( - files: Array<{ file_id: string }>, - fileIds?: string[], - options?: { user?: string; tenantId?: string | null }, - ) => Promise; + /** Owner-scoped TTL hold (`db.extendFilesTTL`-shaped). */ + extendFilesTTL: ( + fileIds: string[], + expiresAt: Date, + owner: { user: string; tenantId?: string | null }, + ) => Promise; + /** Injectable clock for deterministic tests. */ + now?: () => number; } /** - * Owner-scoped usage touch for attachments entering a client-side queue: a - * queued message can outlive the upload window (long run, approval pause), so - * marking at queue time stops the TTL from reaping files the drain will send. - * Best-effort 200 — ids that do not resolve to an owned file are not errors - * (send-time marking remains the backstop). + * Owner-scoped TTL hold for attachments entering a client-side queue: a + * queued message can outlive the upload window (long run, approval pause), + * so holding at queue time stops the TTL from reaping files the drain will + * send. + * + * A hold, not a release. The client queue is ephemeral browser state, so a + * closed tab or a cleared queue leaves nothing referencing these files, and + * clearing the TTL outright would strand them in storage permanently. The + * deadline is only pushed forward; the real release happens at send, where + * `updateFilesUsage` marks the files used against an actual message. + * + * Best-effort 200: ids that do not resolve to a held file are not errors + * (they may be already-sent files, or not owned). */ export async function handleFilesUsageRequest( user: FilesUsageUser, @@ -47,16 +64,17 @@ export async function handleFilesUsageRequest( if (raw.length > FILES_USAGE_MAX_IDS) { return { status: 400, body: { code: 'TOO_MANY_FILES', max: FILES_USAGE_MAX_IDS } }; } - const files: Array<{ file_id: string }> = []; + const fileIds: string[] = []; for (const value of raw) { if (typeof value !== 'string' || value.length === 0) { return { status: 400, body: { code: 'INVALID_FILE_IDS' } }; } - files.push({ file_id: value }); + fileIds.push(value); } - const marked = await deps.updateFilesUsage(files, undefined, { + const nowMs = deps.now?.() ?? Date.now(); + const held = await deps.extendFilesTTL(fileIds, new Date(nowMs + FILES_USAGE_HOLD_MS), { user: user.id, tenantId: user.tenantId, }); - return { status: 200, body: { marked: marked.length } }; + return { status: 200, body: { held } }; } diff --git a/packages/data-provider/.node_modules-7gquYG9s b/packages/data-provider/.node_modules-7gquYG9s new file mode 120000 index 0000000000..8b9f8e521a --- /dev/null +++ b/packages/data-provider/.node_modules-7gquYG9s @@ -0,0 +1 @@ +/Users/danny/Projects/LibreChat/packages/data-provider/node_modules \ No newline at end of file diff --git a/packages/data-provider/src/types/files.ts b/packages/data-provider/src/types/files.ts index 4d1210cd7d..b889ca6f17 100644 --- a/packages/data-provider/src/types/files.ts +++ b/packages/data-provider/src/types/files.ts @@ -242,7 +242,8 @@ export type TFilesUsageBody = { }; export type TFilesUsageResponse = { - marked: number; + /** Count of queued uploads whose TTL hold was extended. */ + held: number; }; export type DeleteFilesResponse = { diff --git a/packages/data-schemas/src/methods/file.spec.ts b/packages/data-schemas/src/methods/file.spec.ts index 71b12ff5aa..6850ee36a3 100644 --- a/packages/data-schemas/src/methods/file.spec.ts +++ b/packages/data-schemas/src/methods/file.spec.ts @@ -1123,6 +1123,93 @@ describe('File Methods', () => { }); }); + describe('extendFilesTTL', () => { + const seedTempFile = async (userId: mongoose.Types.ObjectId, expiresAt: Date) => { + const fileId = uuidv4(); + await fileMethods.createFile({ + file_id: fileId, + user: userId, + filename: `${fileId}.txt`, + filepath: `/uploads/${fileId}.txt`, + type: 'text/plain', + bytes: 100, + }); + await mongoose.models.File.updateOne({ file_id: fileId }, { $set: { expiresAt } }); + return fileId; + }; + + it('pushes the TTL forward without unsetting it', async () => { + const userId = new mongoose.Types.ObjectId(); + const soon = new Date(Date.now() + 60_000); + const fileId = await seedTempFile(userId, soon); + const target = new Date(Date.now() + 3_600_000); + + const count = await fileMethods.extendFilesTTL([fileId], target, { user: String(userId) }); + + expect(count).toBe(1); + const file = await fileMethods.findFileById(fileId); + expect(file?.expiresAt).toBeDefined(); + expect(file?.expiresAt?.getTime()).toBe(target.getTime()); + }); + + it('does not resurrect a TTL on an already-released file', async () => { + const userId = new mongoose.Types.ObjectId(); + const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000)); + await fileMethods.updateFileUsage({ file_id: fileId, user: String(userId) }); + + const count = await fileMethods.extendFilesTTL([fileId], new Date(Date.now() + 3_600_000), { + user: String(userId), + }); + + expect(count).toBe(0); + const file = await fileMethods.findFileById(fileId); + expect(file?.expiresAt).toBeUndefined(); + }); + + it('never moves an expiry earlier', async () => { + const userId = new mongoose.Types.ObjectId(); + const farOut = new Date(Date.now() + 7 * 24 * 3_600_000); + const fileId = await seedTempFile(userId, farOut); + + const count = await fileMethods.extendFilesTTL([fileId], new Date(Date.now() + 60_000), { + user: String(userId), + }); + + expect(count).toBe(0); + const file = await fileMethods.findFileById(fileId); + expect(file?.expiresAt?.getTime()).toBe(farOut.getTime()); + }); + + it("leaves another user's file untouched", async () => { + const ownerId = new mongoose.Types.ObjectId(); + const attackerId = new mongoose.Types.ObjectId(); + const soon = new Date(Date.now() + 60_000); + const fileId = await seedTempFile(ownerId, soon); + + const count = await fileMethods.extendFilesTTL([fileId], new Date(Date.now() + 3_600_000), { + user: String(attackerId), + }); + + expect(count).toBe(0); + const file = await fileMethods.findFileById(fileId); + expect(file?.expiresAt?.getTime()).toBe(soon.getTime()); + }); + + it('is a no-op without an owner scope rather than a cross-user update', async () => { + const userId = new mongoose.Types.ObjectId(); + const soon = new Date(Date.now() + 60_000); + const fileId = await seedTempFile(userId, soon); + + const count = await fileMethods.extendFilesTTL([fileId], new Date(Date.now() + 3_600_000), { + user: '', + } as { user: string }); + + expect(count).toBe(0); + const file = await fileMethods.findFileById(fileId); + expect(file?.expiresAt?.getTime()).toBe(soon.getTime()); + }); + }); + describe('deleteFile', () => { it('should delete a file by file_id', async () => { const fileId = uuidv4(); diff --git a/packages/data-schemas/src/methods/file.ts b/packages/data-schemas/src/methods/file.ts index 2e1d9159a5..68fe66c5ab 100644 --- a/packages/data-schemas/src/methods/file.ts +++ b/packages/data-schemas/src/methods/file.ts @@ -82,6 +82,11 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { fileIds?: string[], options?: { user?: string; tenantId?: string | null }, ) => Promise; + extendFilesTTL: ( + fileIds: string[], + expiresAt: Date, + owner: { user: string; tenantId?: string | null }, + ) => Promise; sweepOrphanedPreviews: (maxAgeMs?: number) => Promise; } { /** @@ -546,6 +551,46 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { return results.filter((result): result is IMongoFile => result != null); } + /** + * Pushes the upload-window TTL of owned, still-temporary files forward. + * + * A renewable hold, not a release: unlike `updateFileUsage` this never + * unsets `expiresAt`, so a file that is held but never actually sent is + * still reaped once the hold lapses. Two filter guards make the write + * safe to expose to a client: + * - `$exists: true`: a file whose TTL was already cleared (a real send) + * is permanent; re-adding `expiresAt` would schedule it for deletion. + * - `$lt: expiresAt`: the hold only ever moves the deadline later. + * + * The owner scope is required, not optional: this write is driven by a + * client-supplied id list, so an unscoped call would hold every user's + * matching file. A missing owner is a no-op rather than a wide update. + * + * @param fileIds - File IDs to hold + * @param expiresAt - New expiry; only applied where it is later than the current one + * @param owner - Owner scope; mismatches leave the TTL unchanged + * @returns Number of files whose hold was extended + */ + async function extendFilesTTL( + fileIds: string[], + expiresAt: Date, + owner: { user: string; tenantId?: string | null }, + ): Promise { + if (fileIds.length === 0 || !owner?.user) { + return 0; + } + const File = mongoose.models.File as Model; + const filter = withOwnerScope( + { + file_id: { $in: [...new Set(fileIds)] }, + expiresAt: { $exists: true, $lt: expiresAt }, + }, + { userId: owner.user, tenantId: owner.tenantId }, + ); + const result = await File.updateMany(filter, { $set: { expiresAt } }); + return result.modifiedCount ?? 0; + } + /** * Mark stale `status: 'pending'` file records as `'failed'` with * `previewError: 'orphaned'`. Recovers from the one case the @@ -594,6 +639,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { deleteFileByFilter, batchUpdateFiles, updateFilesUsage, + extendFilesTTL, sweepOrphanedPreviews, }; }