🪒 fix: Strip Cache-Bust Suffixes From Every Local File Read (#15991)

* 🧼 fix: Strip Cache-Bust Query Before Local Vision Encode

Reused code-interpreter images persist filepath with a ?v= suffix.
prepareImagesLocal now strips it before disk encode, matching crud/share.

Co-authored-by: Zsanz3 <Zsanz3@users.noreply.github.com>

* 🧪 test: Isolate prepareImagesLocal query-strip coverage

Mock sharp and resize so the local vision-encode test does not load
the rest of the file-strategy graph.

Co-authored-by: Zsanz3 <Zsanz3@users.noreply.github.com>

* 🪒 fix: Strip Cache-Bust Suffixes From Every Local File Read

Reused code-interpreter outputs persist a `?v=<timestamp>` suffix on the file
document's filepath, and local storage resolves that field into a filesystem path.
prepareImagesLocal already handled it after the cherry-picked commits; getLocalFileStream
did not, so every download-stream consumer (the download route, provisioning, agent and
skill file reads) still hit ENOENT for a regenerated image.

Move the strip into one documented helper and call it from all three local read paths.

* 🧱 refactor: Move Cache-Bust Stripping Into the Storage Package

`/api` holds wiring, not behavior, so `stripCacheBust` moves out of a new CJS helper and
into `packages/api/src/storage/path.ts`, beside `resolveDownloadPath` — the function that
already decides what a stored filepath means to a download. Its doc comment now states why
the two differ: a remote strategy's query string can carry a presigned signature, so only
the local paths may strip.

The local storage modules keep just the call into it. Semantics are covered by the storage
package's own test; the `/api` specs pin the wiring.

Addresses the Codex P1 finding on 5f8dbf3.

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Zsanz3 <Zsanz3@users.noreply.github.com>
Co-authored-by: Lia <lia@librechat.ai>

Author: 4 people

api/server/services/Files/Local/__tests__/cache-bust.spec.js

@@ -0,0 +1,89 @@
+/**
+ * Wiring coverage: `stripCacheBust`'s own behavior is covered in
+ * `packages/api/src/storage/__tests__/path.test.ts`. These tests assert that the local read paths
+ * hand their raw filepath to it, so a reused code output resolves to the file that exists on disk.
+ */
+jest.mock('@librechat/api', () => ({
+  deleteRagFile: jest.fn(),
+  stripCacheBust: jest.fn((filepath) => filepath.split('?')[0]),
+}));
+jest.mock('@librechat/data-schemas', () => ({
+  logger: { warn: jest.fn(), error: jest.fn() },
+}));
+
+const mockTmpBase = require('fs').mkdtempSync(
+  require('path').join(require('os').tmpdir(), 'local-cache-bust-'),
+);
+
+jest.mock('~/config/paths', () => {
+  const path = require('path');
+  return {
+    publicPath: path.join(mockTmpBase, 'public'),
+    uploads: path.join(mockTmpBase, 'uploads'),
+    imageOutput: path.join(mockTmpBase, 'public', 'images'),
+  };
+});
+
+const fs = require('fs');
+const path = require('path');
+const { stripCacheBust } = require('@librechat/api');
+const { getLocalFileStream } = require('../crud');
+
+const imageOutput = path.join(mockTmpBase, 'public', 'images');
+const uploads = path.join(mockTmpBase, 'uploads');
+
+const makeReq = () => ({
+  user: { id: 'user-1' },
+  config: { paths: { publicPath: path.join(mockTmpBase, 'public'), uploads, imageOutput } },
+});
+
+const readStream = (stream) =>
+  new Promise((resolve, reject) => {
+    const chunks = [];
+    stream.on('data', (chunk) => chunks.push(chunk));
+    stream.on('error', reject);
+    stream.on('end', () => resolve(Buffer.concat(chunks).toString()));
+  });
+
+describe('getLocalFileStream cache-busted filepaths', () => {
+  beforeAll(() => {
+    fs.mkdirSync(path.join(imageOutput, 'user-1'), { recursive: true });
+    fs.mkdirSync(path.join(uploads, 'user-1'), { recursive: true });
+    fs.writeFileSync(path.join(imageOutput, 'user-1', 'chart.png'), 'image-bytes');
+    fs.writeFileSync(path.join(uploads, 'user-1', 'doc.pdf'), 'upload-bytes');
+  });
+
+  afterAll(() => {
+    fs.rmSync(mockTmpBase, { recursive: true, force: true });
+  });
+
+  it('streams a reused code-output image whose filepath carries `?v=`', async () => {
+    const requested = '/images/user-1/chart.png?v=1789460622697';
+
+    const stream = await getLocalFileStream(makeReq(), requested);
+
+    await expect(readStream(stream)).resolves.toBe('image-bytes');
+    expect(stripCacheBust).toHaveBeenCalledWith(requested);
+  });
+
+  it('streams an upload whose filepath carries a query string', async () => {
+    const requested = '/uploads/user-1/doc.pdf?manual=true';
+
+    const stream = await getLocalFileStream(makeReq(), requested);
+
+    await expect(readStream(stream)).resolves.toBe('upload-bytes');
+    expect(stripCacheBust).toHaveBeenCalledWith(requested);
+  });
+
+  it('still streams a filepath without a query string', async () => {
+    const stream = await getLocalFileStream(makeReq(), '/images/user-1/chart.png');
+
+    await expect(readStream(stream)).resolves.toBe('image-bytes');
+  });
+
+  it('still rejects traversal hidden behind a query string', async () => {
+    await expect(getLocalFileStream(makeReq(), '/images/../../../etc/passwd?v=1')).rejects.toThrow(
+      'Invalid file path',
+    );
+  });
+});

api/server/services/Files/Local/__tests__/images.spec.js

@@ -0,0 +1,64 @@
+jest.mock('sharp', () => ({}));
+jest.mock('@librechat/api', () => ({
+  stripCacheBust: jest.fn((filepath) => filepath.split('?')[0]),
+}));
+jest.mock('../../images/resize', () => ({ resizeImageBuffer: jest.fn() }));
+jest.mock('~/models', () => ({
+  updateUser: jest.fn(),
+  updateFile: jest.fn(async (doc) => doc),
+}));
+
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { updateFile } = require('~/models');
+const { prepareImagesLocal } = require('../images');
+
+describe('prepareImagesLocal', () => {
+  let tmpDir;
+  let publicPath;
+  let imageOutput;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'prepare-images-local-'));
+    publicPath = path.join(tmpDir, 'public');
+    imageOutput = path.join(tmpDir, 'images');
+    fs.mkdirSync(path.join(publicPath, 'images', 'user-1'), { recursive: true });
+  });
+
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  const makeReq = () => ({
+    user: { id: 'user-1' },
+    config: { paths: { publicPath, imageOutput } },
+  });
+
+  it('strips a cache-busting query string before encoding from disk', async () => {
+    const relativePath = '/images/user-1/chart.png';
+    fs.writeFileSync(path.join(publicPath, relativePath), Buffer.from('fake-png-bytes'));
+
+    const [updated, encoded] = await prepareImagesLocal(makeReq(), {
+      file_id: 'file-1',
+      filepath: `${relativePath}?v=1789460622697`,
+    });
+
+    expect(updateFile).toHaveBeenCalledWith({ file_id: 'file-1' });
+    expect(updated).toEqual({ file_id: 'file-1' });
+    expect(encoded).toBe(Buffer.from('fake-png-bytes').toString('base64'));
+  });
+
+  it('encodes a filepath without a query string', async () => {
+    const relativePath = '/images/user-1/chart.png';
+    fs.writeFileSync(path.join(publicPath, relativePath), Buffer.from('plain-png-bytes'));
+
+    const [, encoded] = await prepareImagesLocal(makeReq(), {
+      file_id: 'file-2',
+      filepath: relativePath,
+    });
+
+    expect(encoded).toBe(Buffer.from('plain-png-bytes').toString('base64'));
+  });
+});

api/server/services/Files/Local/crud.js

@@ -3,6 +3,7 @@ const path = require('path');
 const axios = require('axios');
 const {
   deleteRagFile,
+  stripCacheBust,
   assertRemoteFileURL,
   getRemoteFileFetchMaxBytes,
   getRemoteFileFetchTimeoutMs,
@@ -231,8 +232,8 @@ const deleteLocalFile = async (req, file) => {
   const appConfig = req.config;
   const { publicPath, uploads } = appConfig.paths;
 
-  /** Filepath stripped of query parameters (e.g., ?manual=true) */
-  const cleanFilepath = file.filepath.split('?')[0];
+  /** Filepath stripped of query parameters (e.g., ?manual=true, ?v=<timestamp>) */
+  const cleanFilepath = stripCacheBust(file.filepath);
 
   await deleteRagFile({ userId: req.user.id, file });
 
@@ -321,12 +322,14 @@ async function uploadLocalFile({ req, file, file_id }) {
  * Retrieves a readable stream for a file from local storage.
  *
  * @param {ServerRequest} req - The request object from Express
- * @param {string} filepath - The filepath.
+ * @param {string} requestedFilepath - The filepath, which may carry a cache-busting query string.
  * @returns {ReadableStream} A readable stream of the file.
  */
-async function getLocalFileStream(req, filepath) {
+async function getLocalFileStream(req, requestedFilepath) {
   try {
     const appConfig = req.config;
+    /** Reused code outputs persist a `?v=<timestamp>` suffix that no file on disk carries */
+    const filepath = stripCacheBust(requestedFilepath);
     if (filepath.includes('/uploads/')) {
       const basePath = filepath.split('/uploads/')[1];
 

api/server/services/Files/Local/images.js

@@ -1,6 +1,7 @@
 const fs = require('fs');
 const path = require('path');
 const sharp = require('sharp');
+const { stripCacheBust } = require('@librechat/api');
 const { resizeImageBuffer } = require('../images/resize');
 const { updateUser, updateFile } = require('~/models');
 
@@ -97,7 +98,7 @@ async function prepareImagesLocal(req, file) {
   if (!fs.existsSync(userPath)) {
     fs.mkdirSync(userPath, { recursive: true });
   }
-  const filepath = path.join(publicPath, file.filepath);
+  const filepath = path.join(publicPath, stripCacheBust(file.filepath));
 
   const promises = [];
   promises.push(updateFile({ file_id: file.file_id }));

packages/api/src/storage/__tests__/path.test.ts

@@ -1,4 +1,4 @@
-import { resolveDownloadPath } from '../path';
+import { resolveDownloadPath, stripCacheBust } from '../path';
 
 describe('resolveDownloadPath', () => {
   it('prefers the recorded object key over the stored URL', () => {
@@ -28,3 +28,21 @@ describe('resolveDownloadPath', () => {
     expect(resolveDownloadPath({ filepath: url })).toBe(url);
   });
 });
+
+describe('stripCacheBust', () => {
+  it('removes the cache-buster a regenerated code output persists', () => {
+    expect(stripCacheBust('/images/u1/chart.png?v=1789460622697')).toBe('/images/u1/chart.png');
+  });
+
+  it('removes everything from the first question mark onward', () => {
+    expect(stripCacheBust('/uploads/u1/doc.pdf?manual=true&v=2')).toBe('/uploads/u1/doc.pdf');
+  });
+
+  it('leaves a path without a query string untouched', () => {
+    expect(stripCacheBust('/images/u1/chart.png')).toBe('/images/u1/chart.png');
+  });
+
+  it('keeps a name whose sanitized form cannot contain a question mark', () => {
+    expect(stripCacheBust('/uploads/u1/what_is_this_.pdf')).toBe('/uploads/u1/what_is_this_.pdf');
+  });
+});

packages/api/src/storage/path.ts

@@ -12,3 +12,22 @@ export type StoredFileRef = Pick<TFile, 'filepath'> & { storageKey?: string | nu
 export function resolveDownloadPath(file: StoredFileRef): string {
   return file.storageKey || file.filepath;
 }
+
+/**
+ * Strips the query string from a stored `filepath` before it is resolved on disk.
+ *
+ * A regenerated code-interpreter output persists a cache-busting `?v=<timestamp>` suffix on its
+ * file document's `filepath` (`processCodeOutput`), and shared-link cache validators fold that
+ * field in deliberately (`buildShareFileEtag`), so the suffix cannot be dropped at write time.
+ * Local storage is the only strategy that turns `filepath` into a filesystem path, so it is the
+ * only one that has to remove the suffix before reading or deleting. A remote strategy receives a
+ * URL whose query string can carry a presigned signature, which is why `resolveDownloadPath`
+ * hands its result over untouched.
+ *
+ * Unambiguous for local paths because `sanitizeFilename` replaces `?` with `_` in every stored
+ * name, so no file on disk carries one.
+ */
+export function stripCacheBust(filepath: string): string {
+  const queryIndex = filepath.indexOf('?');
+  return queryIndex === -1 ? filepath : filepath.slice(0, queryIndex);
+}