🗝️ refactor: Prefer the Recorded Storage Key for Every Download Stream (#15695)

S3 and CloudFront records have carried `storageKey` since #12987, yet six
readers still handed `file.filepath` to `getDownloadStream` and re-derived
the key by parsing a presigned or CDN URL, while four others had grown
their own `storageKey || filepath` expression. One resolver now serves all
ten: `resolveDownloadPath` returns the recorded key when present and the
path otherwise, so records without a key (local, Firebase, Azure, code
output) behave exactly as before, and the share route keeps its local-only
query-string strip on top.

`resolveStoredS3Key` reuses the same `StoredFileRef` type. Covered by unit
cases for the resolver and an S3 case proving a record with a key streams
correctly even when its stored URL no longer parses.

Closes #15693

Author: danny-avila

api/server/routes/__tests__/share.spec.js

@@ -78,6 +78,7 @@ const mockCreateShareContentPreflight = jest.fn((filters, options = {}) => {
 });
 
 jest.mock('@librechat/api', () => ({
+  resolveDownloadPath: (file) => file.storageKey || file.filepath,
   assertModelBoundContent: (...args) => mockAssertModelBoundContent(...args),
   assertSharedFileMetadataAllowed: (...args) => mockAssertSharedFileMetadataAllowed(...args),
   createShareContentPreflight: (...args) => mockCreateShareContentPreflight(...args),

api/server/routes/files/files.js

@@ -18,6 +18,7 @@ const {
   assertUploadContentAllowed,
   hasActiveFilePolicy,
   sanitizeFilename,
+  resolveDownloadPath,
 } = require('@librechat/api');
 const {
   Time,
@@ -700,7 +701,7 @@ router.get('/download/:userId/:file_id', fileAccess, async (req, res) => {
         return res.status(501).send('Not Implemented');
       }
 
-      const fileStream = await getDownloadStream(req, file.storageKey || file.filepath);
+      const fileStream = await getDownloadStream(req, resolveDownloadPath(file));
 
       fileStream.on('error', (streamError) => {
         logger.error('[DOWNLOAD ROUTE] Stream error:', streamError);

api/server/routes/share.js

@@ -24,6 +24,7 @@ const {
   createSharedLangfuseSessionResolver,
   recordShareLinkRejection,
   traceIdForMessage,
+  resolveDownloadPath,
 } = require('@librechat/api');
 const {
   logger,
@@ -297,7 +298,7 @@ const streamSharedFile = async (req, res, file, requestedDisposition) => {
 
   // Strip any cache-busting query string (e.g. code-output images add `?v=...`) so
   // the local stream resolves the real filename, not a literal `*.png?v=...` path.
-  const streamPath = (file.storageKey || file.filepath || '').split('?')[0];
+  const streamPath = (resolveDownloadPath(file) || '').split('?')[0];
   const fileStream = await getDownloadStream(req, streamPath);
 
   res.setHeader('X-Content-Type-Options', 'nosniff');

api/server/services/Files/Code/__tests__/process-traversal.spec.js

@@ -20,6 +20,7 @@ jest.mock('@librechat/api', () => {
   const http = require('http');
   const https = require('https');
   return {
+    resolveDownloadPath: (file) => file.storageKey || file.filepath,
     logAxiosError: jest.fn(),
     getBasePath: jest.fn(() => ''),
     sanitizeArtifactPath: mockSanitizeArtifactPath,

api/server/services/Files/Code/process.js

@@ -35,6 +35,7 @@ const {
   CODE_OUTPUT_PREFLIGHT_MAX_COUNT,
   sortCodeFilesByDestinationPriority,
   normalizeArtifactDeliveryFailure,
+  resolveDownloadPath,
 } = require('@librechat/api');
 const {
   Tools,
@@ -1430,7 +1431,7 @@ const primeFiles = async (options) => {
         const { handleFileUpload: uploadCodeEnvFile } = getStrategyFunctions(
           FileSources.execute_code,
         );
-        const stream = await getDownloadStream(options.req, file.filepath);
+        const stream = await getDownloadStream(options.req, resolveDownloadPath(file));
         /* Reupload preserves the resource identity from the existing
          * ref so codeapi re-buckets under the same sessionKey shape
          * (skill stays skill, user stays user). Without this, a

api/server/services/Files/Code/process.spec.js

@@ -66,6 +66,7 @@ jest.mock('@librechat/api', () => {
   const http = require('http');
   const https = require('https');
   return {
+    resolveDownloadPath: (file) => file.storageKey || file.filepath,
     logAxiosError: jest.fn(),
     /* Behaviourally identical to the real predicate in
      * `packages/api/src/files/code/errors.ts`, which owns the contract and

packages/api/src/agents/handlers.ts

@@ -108,6 +108,7 @@ import { buildSkillPrimeMessage, isSkillFilePath, SKILL_FILE_PREFIX } from './sk
 import { resolveCallerCapabilityProjectionSnapshot } from './callerCapabilities';
 import { createSkillContentDigest } from './compatibility';
 import { isMissingSandboxPathError } from '~/files/code';
+import { resolveDownloadPath } from '~/storage/path';
 import { parseFrontmatter } from '../skills/import';
 import { cleanCodeToolOutput } from './cleanup';
 import { primeSkillFiles } from './skillFiles';
@@ -3269,7 +3270,7 @@ async function loadSkillFileTextForAuthoring({
     return { status: 'error', message: 'Download is not supported for this storage backend.' };
   }
 
-  const stream = await strategy.getDownloadStream(req, file.filepath);
+  const stream = await strategy.getDownloadStream(req, resolveDownloadPath(file));
   const chunks: Uint8Array[] = [];
   let streamedBytes = 0;
   for await (const chunk of stream as AsyncIterable<Uint8Array>) {
@@ -4673,7 +4674,7 @@ async function handleReadFileCall(
       };
     }
 
-    const stream = await strategy.getDownloadStream(req, file.filepath);
+    const stream = await strategy.getDownloadStream(req, resolveDownloadPath(file));
     const chunks: Uint8Array[] = [];
     // Use the larger binary limit as streaming cap; cheaper type-specific
     // checks happen after binary detection on the assembled buffer.

packages/api/src/agents/skillFiles.ts

@@ -25,6 +25,7 @@ import { getCodeExecutionRouteKey, type CodeExecutionContext } from './execution
 import { assertSkillFileContentAllowed } from '~/skills/protection';
 import { createSkillContentDigest } from './compatibility';
 import { extractInvokedSkillsFromPayload } from './run';
+import { resolveDownloadPath } from '~/storage/path';
 import { SKILL_FILE_PREFIX } from './skills';
 
 const MAX_INSPECTABLE_SKILL_FILE_BYTES = 10 * 1024 * 1024;
@@ -197,7 +198,7 @@ async function collectSkillUploadFiles(
         );
         return null;
       }
-      const stream = await strategy.getDownloadStream(req, file.filepath);
+      const stream = await strategy.getDownloadStream(req, resolveDownloadPath(file));
       return { stream, filename: `${SKILL_FILE_PREFIX}${skill.name}/${file.relativePath}` };
     }),
   );
@@ -402,7 +403,7 @@ async function executePrimeSkillFiles(
           logger.warn('[primeSkillFiles] No download stream for stored skill file');
           continue;
         }
-        const sourceStream = await strategy.getDownloadStream(req, file.filepath);
+        const sourceStream = await strategy.getDownloadStream(req, resolveDownloadPath(file));
         const buffer = await bufferSkillFileStream(sourceStream);
         if (buffer == null) {
           throwIfStoredSkillFileMustBeInspectable(req);

packages/api/src/files/encode/utils.ts

@@ -3,6 +3,7 @@ import { Providers } from '@librechat/agents';
 import { FileSources, mergeFileConfig, getEndpointFileConfig } from 'librechat-data-provider';
 import type { IMongoFile } from '@librechat/data-schemas';
 import type { ServerRequest, StrategyFunctions, ProcessedFile } from '~/types';
+import { resolveDownloadPath } from '~/storage/path';
 
 /**
  * Extracts the configured file size limit for a specific provider from fileConfig
@@ -55,7 +56,7 @@ export async function getFileStream(
   }
 
   const { getDownloadStream } = encodingMethods[source];
-  const stream = await getDownloadStream(req, file.filepath);
+  const stream = await getDownloadStream(req, resolveDownloadPath(file));
   let buffer: Buffer | null = await getStream.buffer(stream);
   const content = buffer.toString('base64');
   buffer = null;

packages/api/src/skills/handlers.ts

@@ -36,6 +36,7 @@ import type { ServerRequest, StrategyFunctions } from '~/types';
 import { extractSkillContent, inspectContentWithTraversal } from '~/protection';
 import { contentFilterBlockResponse } from '~/middleware/contentFilter';
 import { getDeploymentSkillIds } from './deployment';
+import { resolveDownloadPath } from '~/storage/path';
 import { resolveSkillFilePathParam } from './path';
 import { parseSkillMarkdown } from './parse';
 import { isBinaryBuffer } from './binary';
@@ -702,7 +703,7 @@ export function createSkillsHandlers(deps: SkillsHandlersDeps): {
           'Content-Disposition',
           `${isImageMime ? 'inline' : 'attachment'}; filename="${safeName}"`,
         );
-        const stream = await strategy.getDownloadStream(req, file.storageKey || file.filepath);
+        const stream = await strategy.getDownloadStream(req, resolveDownloadPath(file));
         stream.on('error', (err: Error) => {
           logger.error('[downloadFile] Stream error:', err);
           if (!res.headersSent) {
@@ -736,7 +737,7 @@ export function createSkillsHandlers(deps: SkillsHandlersDeps): {
       // destroy (binary) or continue reading (text) in the same iteration.
       // N.B. breaking out of `for await...of` destroys the stream via
       // iterator.return(), so we must NOT use break + a second loop.
-      const stream = await strategy.getDownloadStream(req, file.storageKey || file.filepath);
+      const stream = await strategy.getDownloadStream(req, resolveDownloadPath(file));
       const chunks: Buffer[] = [];
       let totalBytes = 0;
       let binaryChecked = false;