🔒 fix: Bound /files/usage TTL Hold Instead of Clearing It (#14470)

* 🔒 fix: Bound `/files/usage` TTL Hold Instead of Clearing It

`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.

* 🔒 fix: Anchor the `/files/usage` hold to upload time

Codex review on b687922.

The hold derived each new deadline from `Date.now()`, so a caller touching
once a day advanced it by another 24h every time, far below the rate limit.
That left indefinite preservation reachable and made the PR's replay claim
wrong: the window was bounded per call but not in aggregate.

Anchor the deadline to the file's immutable `createdAt` instead of the
request clock. `extendFilesTTL` now takes a lifetime and sets
`expiresAt = max(expiresAt, createdAt + holdMs)` in an aggregation
pipeline, so the target is a fixed point per file and replay is inert
rather than merely bounded. `$max` keeps the widen-only property and the
`expiresAt: {$exists: true}` filter still refuses to resurrect a released
TTL; `createdAt: {$exists: true}` fail-closes when the anchor is absent.

The update runs with `timestamps: false`: a hold is TTL bookkeeping, not a
content write, and bumping `updatedAt` also made every re-touch count as a
modification, hiding whether the deadline actually moved.

Also drop four `.node_modules-*` symlinks that `git add -A` swept in from
an npm install. They pointed at absolute paths on one machine, so every
other checkout got dangling entries. Added the pattern to .gitignore so a
workspace install cannot reintroduce them.

* 🔒 fix: Track the configured approval window in the `/files/usage` hold

Codex review on 9277620.

`endpoints.agents.checkpointer.ttl` is a positive int with no upper bound,
and its docs invite raising it for longer review windows. It drives the
pending-action expiry, so a run can legitimately stay paused past 24h. The
fixed 24h lifetime would then let Mongo reap an attachment while its
approval was still live, and the later queue drain would send a file that
no longer exists.

Replace the fixed constant with `resolveFilesUsageHoldMs`, which adds the
configured approval window to a 24h baseline covering upload, enqueue, and
the run reaching its pause. The route reads the window from the same
`getApprovalTtlMs(checkpointerCfg)` the pending action uses, so the two
stay in lockstep.

The replay bound is unaffected: the window is a per-deployment constant and
the deadline is still `createdAt + holdMs`, so a replayed touch re-asserts
the same instant and `$max` skips the write. Only an operator config change
moves it, never a client.

* 🔒 fix: Renew the `/files/usage` hold across queued runs, under a ceiling

Codex review on 2bd3c52.

The drain sends one queued item per run completion, and each item starts a
run that may itself pause for the full approval window. Since the hold was
taken once at enqueue and pinned to the upload time, an item several places
back could sit through multiple approval windows and lose its attachment
while its chip and the live approval were still there. Another regression
from this PR: the old `$unset` made retention permanent, so deep queues
happened to work.

The queue is unbounded, so no fixed lifetime covers it. Split the hold into
a renewable window and a ceiling:

  expiresAt = max(expiresAt, min(now + renewMs, createdAt + maxLifetimeMs))

`renewMs` covers one run's wait and is granted from now, so a queue that is
still draining re-asserts it at each transition; `useQueueDrain` now marks
the remaining items' files whenever it pops one. `maxLifetimeMs` is
measured from the immutable upload time and clamps every renewal, so
repeated touches converge on a ceiling instead of advancing per call, which
keeps the replay bound from the previous round intact.

This also tightens abandonment: a queue nobody drains now lapses one
`renewMs` after its last touch instead of surviving to the ceiling.

`useQueueDrain`'s spec gained a QueryClientProvider, since the renewal goes
through react-query.

* 🔒 fix: Renew queued holds on a heartbeat, and stop dropping batches

Codex review on f616bed.

Three gaps in the renewal added last commit:

- `collectQueuedFileIds` returned early at the server's 10-id cap, so a
  remainder holding more than one batch renewed only its first message and
  left the rest on their enqueue-time hold. Collect everything and split
  into capped requests instead of truncating.
- A refused `ask()` restores the popped item, but renewal ran before the
  send and covered only the pre-existing remainder. Since the run-end signal
  is already consumed, nothing would touch that item again. Renewal now runs
  after `ask` and includes the restored item.
- A single run can interrupt for approval more than once, each pause running
  to the configured window, so renewing only at drain transitions leaves a
  gap longer than `renewMs` with no renewal in it. The ceiling cannot help
  when nothing renews.

The third is the same structural gap as the previous round along a new axis:
renewal tied to discrete events loses the file whenever two events are
further apart than the hold. Rather than hook each transition, renew on a
30 minute heartbeat while anything is queued, which is far below the
smallest hold (24h) and so covers any single gap regardless of cause.

Still bounded: every renewal is clamped against the file's upload time, so
the ceiling is unchanged. A queue nobody has open emits no heartbeat and
lapses one `renewMs` after its last touch, preserving the abandonment
behaviour.

* 🔒 fix: Cover the pre-migration queue, first tick, and `/usage/`

Codex review on 892a27d.

- The heartbeat watched only the active conversation id, but `drainNext`
  merges in the `NEW_CONVO` queue, which outlives the URL update: items
  queued during the first turn stay keyed there until that run ends. It now
  renews the union of both, deduped since they are the same atom before
  migration.
- The interval installed without firing, so returning to a conversation
  whose hold was nearly up waited out a full period before the first
  renewal. It now renews immediately, then on each tick.
- Express's non-strict routing sends `POST /files/usage/` to the same
  handler with `req.path === '/usage/'`, so the exact comparison pushed it
  onto both upload limiters. A trailing-slash client would have spent its
  upload quota, and collected file-upload violations, on metadata
  heartbeats. Matching now tolerates the trailing slash.

Firing on effect start also made the drain-time renewal redundant: popping
an item changes the held set, so the renewal effect re-runs on its own. The
one case it cannot see is a refused send, where restoring the item leaves
the set identical, so that branch keeps an explicit renewal and the rest is
removed. Net one request per transition instead of two.

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,
 };

api/server/routes/files/files.js

@@ -3,6 +3,7 @@ const express = require('express');
 const { logger, SystemCapabilities } = require('@librechat/data-schemas');
 const {
   logAxiosError,
+  getApprovalTtlMs,
   refreshS3FileUrls,
   handleFilesUsageRequest,
   shouldUseUploadSse,
@@ -144,15 +145,19 @@ 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. The approval window is passed through so a queue waiting on a paused
+ * run outlives that pause. Thin wrapper: validation, cap, hold window, and
+ * best-effort semantics live in `@librechat/api` (`handleFilesUsageRequest`).
  */
 router.post('/usage', async (req, res) => {
   try {
+    const checkpointerCfg = req.config?.endpoints?.[EModelEndpoint.agents]?.checkpointer;
     const { status, body } = await handleFilesUsageRequest(req.user ?? {}, req.body ?? {}, {
-      updateFilesUsage: db.updateFilesUsage,
+      extendFilesTTL: db.extendFilesTTL,
+      approvalTtlMs: getApprovalTtlMs(checkpointerCfg),
     });
     return res.status(status).json(body);
   } catch (error) {

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,110 @@ 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());
+      /* The 24h baseline plus the default 24h approval window, so a queue
+       * waiting on a paused run outlives that pause. Renewed from now, but
+       * never past the ceiling measured from upload time. */
+      const HOUR = 60 * 60 * 1000;
+      expect(held.expiresAt.getTime()).toBeGreaterThan(Date.now() + 47 * HOUR);
+      expect(held.expiresAt.getTime()).toBeLessThanOrEqual(
+        held.createdAt.getTime() + 24 * HOUR + 8 * 24 * HOUR,
+      );
+      /* 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));
+
+      const first = await request(app)
+        .post('/files/usage')
+        .send({ file_ids: [ownFileId] });
+      expect(first.body).toEqual({ held: 1 });
+
+      for (let i = 0; i < 5; i++) {
+        const repeat = await request(app)
+          .post('/files/usage')
+          .send({ file_ids: [ownFileId] });
+        expect(repeat.status).toBe(200);
+      }
+
+      /* Every renewal is clamped to the ceiling measured from upload time, so
+       * replay converges there instead of advancing a window per call. */
+      const HOUR = 60 * 60 * 1000;
+      const held = await File.findOne({ file_id: ownFileId }).lean();
+      expect(held.expiresAt).toBeDefined();
+      expect(held.expiresAt.getTime()).toBeLessThanOrEqual(
+        held.createdAt.getTime() + 24 * HOUR + 8 * 24 * HOUR,
+      );
+    });
+
+    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 () => {

api/server/routes/files/index.js

@@ -1,5 +1,6 @@
 const express = require('express');
 const {
+  createFileUsageLimiter,
   createFileLimiters,
   configMiddleware,
   requireJwtAuth,
@@ -30,19 +31,29 @@ const initialize = async () => {
   router.use('/speech', speech);
 
   const { fileUploadIpLimiter, fileUploadUserLimiter } = createFileLimiters();
+  const fileUsageLimiter = createFileUsageLimiter();
+
+  /** Non-strict routing means `/usage/` reaches the same handler, so match the
+   *  route the way Express does. An exact comparison would push a
+   *  trailing-slash request onto the upload quota instead. */
+  const isUsagePath = (path) => path.replace(/\/+$/, '') === '/usage';
 
   /** 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 (isUsagePath(req.path)) {
+      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);

api/server/routes/files/index.limiters.test.js

@@ -0,0 +1,129 @@
+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);
+  });
+
+  /* Non-strict routing sends `/usage/` to the same handler, so an exact path
+   * comparison would bill a trailing-slash client's heartbeats to the upload
+   * quota and hand them file-upload violations. */
+  it('routes a trailing-slash /usage/ through the usage limiter too', async () => {
+    const response = await request(app)
+      .post('/api/files/usage/')
+      .send({ file_ids: ['f1'] });
+
+    expect(response.status).toBe(200);
+    expect(hits.usage).toBe(1);
+    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);
+  });
+});

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',