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

.gitignore

@@ -52,6 +52,7 @@ __blobstorage__/**/*
 # Deployed apps should consider commenting these lines out:
 # see https://npmjs.org/doc/faq.html#Should-I-check-my-node_modules-folder-into-git
 node_modules/
+.node_modules-*
 meili_data/
 api/node_modules/
 client/node_modules/

api/.node_modules-2xgm5Cvq

@@ -1 +0,0 @@
-/Users/danny/Projects/LibreChat/api/node_modules

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

@@ -966,6 +966,8 @@ describe('File Routes - Delete with Agent Access', () => {
       /* The hold must remain a hold: still reapable, just later. */
       expect(held.expiresAt).toBeDefined();
       expect(held.expiresAt.getTime()).toBeGreaterThan(soon.getTime());
+      /* Anchored to upload time, so the deadline is a fixed point per file. */
+      expect(held.expiresAt.getTime()).toBe(held.createdAt.getTime() + 24 * 60 * 60 * 1000);
       /* A queue touch is not a send, so it must not inflate usage. */
       expect(held.usage).toBe(0);
     });
@@ -973,17 +975,25 @@ describe('File Routes - Delete with Agent Access', () => {
     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 });
+      const afterFirst = (await File.findOne({ file_id: ownFileId }).lean()).expiresAt;
+
+      /* Replay must be inert, not merely bounded: a deadline derived from the
+       * request clock would advance a window per call and never converge. */
       for (let i = 0; i < 5; i++) {
-        const response = await request(app)
+        const repeat = await request(app)
           .post('/files/usage')
           .send({ file_ids: [ownFileId] });
-        expect(response.status).toBe(200);
+        expect(repeat.status).toBe(200);
+        expect(repeat.body).toEqual({ held: 0 });
       }
 
       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);
+      expect(held.expiresAt.getTime()).toBe(afterFirst.getTime());
     });
 
     it('never re-adds a TTL to a file that was already sent', async () => {

client/.node_modules-101IklEQ

@@ -1 +0,0 @@
-/Users/danny/Projects/LibreChat/client/node_modules

packages/api/.node_modules-uphMfdfi

@@ -1 +0,0 @@
-/Users/danny/Projects/LibreChat/packages/api/node_modules

packages/api/src/files/usage.spec.ts

@@ -2,11 +2,9 @@ import { handleFilesUsageRequest, FILES_USAGE_MAX_IDS, FILES_USAGE_HOLD_MS } fro
 
 describe('handleFilesUsageRequest', () => {
   const user = { id: 'user-1', tenantId: 'tenant-1' };
-  const NOW = 1_700_000_000_000;
 
   const createDeps = (held = 0) => ({
     extendFilesTTL: jest.fn().mockResolvedValue(held),
-    now: () => NOW,
   });
 
   it('rejects unauthenticated requests without touching the DB', async () => {
@@ -39,25 +37,30 @@ describe('handleFilesUsageRequest', () => {
     expect(deps.extendFilesTTL).not.toHaveBeenCalled();
   });
 
-  it('extends the hold owner-scoped by a bounded window', async () => {
+  it('requests an owner-scoped hold of the fixed lifetime', async () => {
     const deps = createDeps(1);
     const result = await handleFilesUsageRequest(user, { file_ids: ['f1', 'f2'] }, deps);
     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(deps.extendFilesTTL).toHaveBeenCalledWith(['f1', 'f2'], FILES_USAGE_HOLD_MS, {
+      user: 'user-1',
+      tenantId: 'tenant-1',
+    });
     expect(result).toEqual({ status: 200, body: { held: 1 } });
   });
 
-  it('never requests an unbounded hold', async () => {
+  /** The hold must be a lifetime the data layer anchors to the upload, not a
+   *  deadline derived here: a request-clock deadline would let a caller walk
+   *  the file's lifetime forward one window per call. */
+  it('passes a constant lifetime, never a request-derived deadline', 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);
+    await handleFilesUsageRequest(user, { file_ids: ['f1'] }, deps);
+
+    const [, firstHold] = deps.extendFilesTTL.mock.calls[0];
+    const [, secondHold] = deps.extendFilesTTL.mock.calls[1];
+    expect(firstHold).toBe(FILES_USAGE_HOLD_MS);
+    expect(secondHold).toBe(firstHold);
+    expect(typeof firstHold).toBe('number');
   });
 
   it('returns 200 with zero held when no id resolves to an owned file', async () => {

packages/api/src/files/usage.ts

@@ -2,9 +2,13 @@
 export const FILES_USAGE_MAX_IDS: number = 10;
 
 /**
- * How far forward a single touch pushes the upload-window TTL. Generous
+ * Total lifetime a held upload gets, measured from its upload time. Generous
  * enough to outlast any realistic queue wait (long run, approval pause),
  * short enough that a queue the user abandons still gets reaped.
+ *
+ * Measured from upload rather than from the request, so the deadline is a
+ * fixed point per file: replaying the touch re-asserts the same instant
+ * instead of walking the file's lifetime forward a window at a time.
  */
 export const FILES_USAGE_HOLD_MS: number = 24 * 60 * 60 * 1000;
 
@@ -27,11 +31,9 @@ export interface FilesUsageDeps {
   /** Owner-scoped TTL hold (`db.extendFilesTTL`-shaped). */
   extendFilesTTL: (
     fileIds: string[],
-    expiresAt: Date,
+    holdMs: number,
     owner: { user: string; tenantId?: string | null },
   ) => Promise<number>;
-  /** Injectable clock for deterministic tests. */
-  now?: () => number;
 }
 
 /**
@@ -43,7 +45,8 @@ export interface FilesUsageDeps {
  * 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
+ * hold only widens the window to a fixed point measured from upload, so it
+ * is idempotent under replay; 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
@@ -71,8 +74,7 @@ export async function handleFilesUsageRequest(
     }
     fileIds.push(value);
   }
-  const nowMs = deps.now?.() ?? Date.now();
-  const held = await deps.extendFilesTTL(fileIds, new Date(nowMs + FILES_USAGE_HOLD_MS), {
+  const held = await deps.extendFilesTTL(fileIds, FILES_USAGE_HOLD_MS, {
     user: user.id,
     tenantId: user.tenantId,
   });

packages/data-provider/.node_modules-7gquYG9s

@@ -1 +0,0 @@
-/Users/danny/Projects/LibreChat/packages/data-provider/node_modules

packages/data-schemas/src/methods/file.spec.ts

@@ -1124,6 +1124,8 @@ describe('File Methods', () => {
   });
 
   describe('extendFilesTTL', () => {
+    const HOLD_MS = 24 * 3_600_000;
+
     const seedTempFile = async (userId: mongoose.Types.ObjectId, expiresAt: Date) => {
       const fileId = uuidv4();
       await fileMethods.createFile({
@@ -1138,18 +1140,48 @@ describe('File Methods', () => {
       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 readCreatedAt = async (fileId: string) => {
+      const doc = await mongoose.models.File.findOne({ file_id: fileId })
+        .lean<{ createdAt: Date }>()
+        .exec();
+      return doc!.createdAt;
+    };
 
-      const count = await fileMethods.extendFilesTTL([fileId], target, { user: String(userId) });
+    it('widens the TTL to createdAt + holdMs without unsetting it', async () => {
+      const userId = new mongoose.Types.ObjectId();
+      const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000));
+      const createdAt = await readCreatedAt(fileId);
+
+      const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) });
 
       expect(count).toBe(1);
       const file = await fileMethods.findFileById(fileId);
       expect(file?.expiresAt).toBeDefined();
-      expect(file?.expiresAt?.getTime()).toBe(target.getTime());
+      expect(file?.expiresAt?.getTime()).toBe(createdAt.getTime() + HOLD_MS);
+    });
+
+    /** The bound that makes the endpoint safe to expose: the deadline is a
+     *  function of the immutable createdAt, so replaying the call can never
+     *  walk a file's lifetime forward one window at a time. */
+    it('is idempotent under replay, never advancing the deadline', async () => {
+      const userId = new mongoose.Types.ObjectId();
+      const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000));
+      const createdAt = await readCreatedAt(fileId);
+
+      const first = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) });
+      const afterFirst = (await fileMethods.findFileById(fileId))?.expiresAt;
+
+      for (let i = 0; i < 5; i++) {
+        const repeat = await fileMethods.extendFilesTTL([fileId], HOLD_MS, {
+          user: String(userId),
+        });
+        expect(repeat).toBe(0);
+      }
+
+      expect(first).toBe(1);
+      const file = await fileMethods.findFileById(fileId);
+      expect(file?.expiresAt?.getTime()).toBe(afterFirst?.getTime());
+      expect(file?.expiresAt?.getTime()).toBe(createdAt.getTime() + HOLD_MS);
     });
 
     it('does not resurrect a TTL on an already-released file', async () => {
@@ -1157,9 +1189,7 @@ describe('File Methods', () => {
       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),
-      });
+      const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) });
 
       expect(count).toBe(0);
       const file = await fileMethods.findFileById(fileId);
@@ -1171,9 +1201,7 @@ describe('File Methods', () => {
       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),
-      });
+      const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) });
 
       expect(count).toBe(0);
       const file = await fileMethods.findFileById(fileId);
@@ -1186,7 +1214,7 @@ describe('File Methods', () => {
       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), {
+      const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, {
         user: String(attackerId),
       });
 
@@ -1200,14 +1228,25 @@ describe('File Methods', () => {
       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 });
+      const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: '' } as {
+        user: string;
+      });
 
       expect(count).toBe(0);
       const file = await fileMethods.findFileById(fileId);
       expect(file?.expiresAt?.getTime()).toBe(soon.getTime());
     });
+
+    it('is a no-op for a non-positive hold', async () => {
+      const userId = new mongoose.Types.ObjectId();
+      const soon = new Date(Date.now() + 60_000);
+      const fileId = await seedTempFile(userId, soon);
+
+      expect(await fileMethods.extendFilesTTL([fileId], 0, { user: String(userId) })).toBe(0);
+      expect(await fileMethods.extendFilesTTL([fileId], -1, { user: String(userId) })).toBe(0);
+      const file = await fileMethods.findFileById(fileId);
+      expect(file?.expiresAt?.getTime()).toBe(soon.getTime());
+    });
   });
 
   describe('deleteFile', () => {

packages/data-schemas/src/methods/file.ts

@@ -84,7 +84,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
   ) => Promise<IMongoFile[]>;
   extendFilesTTL: (
     fileIds: string[],
-    expiresAt: Date,
+    holdMs: number,
     owner: { user: string; tenantId?: string | null },
   ) => Promise<number>;
   sweepOrphanedPreviews: (maxAgeMs?: number) => Promise<number>;
@@ -552,42 +552,59 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
   }
 
   /**
-   * Pushes the upload-window TTL of owned, still-temporary files forward.
+   * Widens the upload-window TTL of owned, still-temporary files to
+   * `createdAt + holdMs`.
    *
    * 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.
+   * still reaped once the hold lapses. Three properties hold by
+   * construction, which is what makes the write safe to drive from a
+   * client-supplied id list:
+   * - the new deadline is anchored to the immutable `createdAt`, never to
+   *   the request clock, so replaying the call is idempotent and cannot
+   *   walk a file's lifetime forward indefinitely;
+   * - `$max` against the current value means a hold only ever widens;
+   * - `expiresAt: { $exists: true }` means a file whose TTL was already
+   *   cleared by a real send stays permanent. Re-adding `expiresAt` there
+   *   would schedule a live file for deletion.
    *
-   * 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.
+   * `createdAt` is required rather than defaulted: without the anchor there
+   * is no bound to enforce, so such a file is skipped instead of held.
+   *
+   * The owner scope is required, not optional: an unscoped call would hold
+   * every user's matching file. A missing owner is a no-op, not a wide
+   * update.
    *
    * @param fileIds - File IDs to hold
-   * @param expiresAt - New expiry; only applied where it is later than the current one
+   * @param holdMs - Lifetime granted from upload time
    * @param owner - Owner scope; mismatches leave the TTL unchanged
-   * @returns Number of files whose hold was extended
+   * @returns Number of files whose hold was widened
    */
   async function extendFilesTTL(
     fileIds: string[],
-    expiresAt: Date,
+    holdMs: number,
     owner: { user: string; tenantId?: string | null },
   ): Promise<number> {
-    if (fileIds.length === 0 || !owner?.user) {
+    if (fileIds.length === 0 || !owner?.user || !(holdMs > 0)) {
       return 0;
     }
     const File = mongoose.models.File as Model<IMongoFile>;
     const filter = withOwnerScope(
       {
         file_id: { $in: [...new Set(fileIds)] },
-        expiresAt: { $exists: true, $lt: expiresAt },
+        expiresAt: { $exists: true },
+        createdAt: { $exists: true },
       },
       { userId: owner.user, tenantId: owner.tenantId },
     );
-    const result = await File.updateMany(filter, { $set: { expiresAt } });
+    const result = await File.updateMany(
+      filter,
+      [{ $set: { expiresAt: { $max: ['$expiresAt', { $add: ['$createdAt', holdMs] }] } } }],
+      /** `timestamps: false`: a hold is TTL bookkeeping, not a content write.
+       *  Bumping `updatedAt` would also make every re-touch count as a
+       *  modification, hiding whether the deadline actually moved. */
+      { timestamps: false },
+    );
     return result.modifiedCount ?? 0;
   }