diff --git a/.env.example b/.env.example index cb8653ad40a..b2f36093dc0 100644 --- a/.env.example +++ b/.env.example @@ -754,6 +754,10 @@ AWS_BUCKET_NAME= AZURE_STORAGE_CONNECTION_STRING= AZURE_STORAGE_PUBLIC_ACCESS=false AZURE_CONTAINER_NAME=files +# Optional full blob-endpoint override (sovereign clouds / custom domains). +# Used by SAS signing and the authenticated download path; defaults to +# https://.blob.core.windows.net when unset. +AZURE_STORAGE_BLOB_ENDPOINT= #========================# # Shared Links # diff --git a/api/cache/getLogStores.js b/api/cache/getLogStores.js index 45a6a699947..68c2b7da20a 100644 --- a/api/cache/getLogStores.js +++ b/api/cache/getLogStores.js @@ -45,6 +45,10 @@ const namespaces = { [CacheKeys.TOKEN_CONFIG]: standardCache(CacheKeys.TOKEN_CONFIG, Time.THIRTY_MINUTES), [CacheKeys.GEN_TITLE]: standardCache(CacheKeys.GEN_TITLE, Time.TWO_MINUTES), [CacheKeys.S3_EXPIRY_INTERVAL]: standardCache(CacheKeys.S3_EXPIRY_INTERVAL, Time.THIRTY_MINUTES), + [CacheKeys.AZURE_EXPIRY_INTERVAL]: standardCache( + CacheKeys.AZURE_EXPIRY_INTERVAL, + Time.THIRTY_MINUTES, + ), [CacheKeys.MODEL_QUERIES]: standardCache(CacheKeys.MODEL_QUERIES), [CacheKeys.AUDIO_RUNS]: standardCache(CacheKeys.AUDIO_RUNS, Time.TEN_MINUTES), [CacheKeys.MESSAGES]: standardCache(CacheKeys.MESSAGES, Time.ONE_MINUTE), diff --git a/api/server/controllers/UserController.js b/api/server/controllers/UserController.js index 340ebdbb4ca..3d5adc3fdb4 100644 --- a/api/server/controllers/UserController.js +++ b/api/server/controllers/UserController.js @@ -20,6 +20,17 @@ const { verifyOTPOrBackupCode } = require('~/server/services/twoFactorService'); const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService'); const { getMCPManager, getFlowStateManager, getMCPServersRegistry } = require('~/config'); const { invalidateCachedTools } = require('~/server/services/Config/getCachedTools'); +const { needsRefreshAzure, getNewAzureURL } = require('~/server/services/Files/Azure/crud'); + +/** DI maps: dispatch avatar URL check and refresh by storage source */ +const avatarNeedsRefreshBySource = { + [FileSources.s3]: (url) => needsRefresh(url, 3600), + [FileSources.azure_blob]: (url) => needsRefreshAzure(url, 3600), +}; +const getNewAvatarUrlBySource = { + [FileSources.s3]: getNewS3URL, + [FileSources.azure_blob]: getNewAzureURL, +}; const { processDeleteRequest } = require('~/server/services/Files/process'); const { getAppConfig } = require('~/server/services/Config'); const { getLogStores } = require('~/cache'); @@ -42,20 +53,32 @@ const getUserController = async (req, res) => { delete userData.password; delete userData.totpSecret; delete userData.backupCodes; - if (appConfig.fileStrategy === FileSources.s3 && userData.avatar) { - const avatarNeedsRefresh = needsRefresh(userData.avatar, 3600); - if (!avatarNeedsRefresh) { + + // Signed-URL avatar refresh (S3 / Azure Blob) — dispatches by active file strategy + const strategy = appConfig.fileStrategy; + const checkNeedsRefresh = avatarNeedsRefreshBySource[strategy]; + const getNewAvatarUrl = getNewAvatarUrlBySource[strategy]; + if (checkNeedsRefresh && getNewAvatarUrl && userData.avatar) { + if (!checkNeedsRefresh(userData.avatar)) { return res.status(200).send(userData); } const originalAvatar = userData.avatar; try { - userData.avatar = await getNewS3URL(userData.avatar); + const refreshed = await getNewAvatarUrl(userData.avatar); + if (!refreshed) { + // Not a refreshable URL for this provider (OAuth provider avatar, + // legacy /uploads/ path, S3 URL after migrating to Azure, etc.). + // Keep the existing avatar — never persist `undefined`. + return res.status(200).send(userData); + } + userData.avatar = refreshed; await db.updateUser(userData.id, { avatar: userData.avatar }); } catch (error) { userData.avatar = originalAvatar; - logger.error('Error getting new S3 URL for avatar:', error); + logger.error(`Error refreshing ${strategy} avatar URL:`, error); } } + res.status(200).send(userData); }; diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 997ced213f9..a5b65ac8228 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -15,7 +15,6 @@ const { stripFileIdsFromToolResources, } = require('@librechat/api'); const { - Time, Tools, CacheKeys, Constants, @@ -39,11 +38,12 @@ const { } = require('~/server/services/PermissionService'); const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { resizeAvatar } = require('~/server/services/Files/images/avatar'); -const { getFileStrategy } = require('~/server/utils/getFileStrategy'); +const { getFileStrategy, getFileURLRefreshCacheTime } = require('~/server/utils/getFileStrategy'); const { filterFile } = require('~/server/services/Files/process'); const { getCachedTools } = require('~/server/services/Config'); const { resolveConfigServers } = require('~/server/services/MCP'); const { getMCPServersRegistry } = require('~/config'); +const { refreshAzureUrl } = require('~/server/services/Files/Azure/crud'); const { getLogStores } = require('~/cache'); const db = require('~/models'); @@ -304,6 +304,24 @@ const pruneToolResourceFileIdsForOwner = async ({ tool_resources, ownerId, logPr } }; +/** DI map: dispatches avatar URL refresh by storage source */ +const urlRefreshersBySource = { + [FileSources.s3]: refreshS3Url, + [FileSources.azure_blob]: refreshAzureUrl, +}; + +/** + * DI map: picks the cache namespace for the agent-list avatar-refresh + * cache by storage source. Without this, the agent list cached Azure + * avatar URLs in the S3 cache namespace with a 30-minute hardcoded TTL — + * outliving Azure's 5-minute default URL TTL by ~25 minutes and serving + * expired SAS URLs for most of the cache window. + */ +const avatarRefreshCacheKeyByStrategy = { + [FileSources.s3]: CacheKeys.S3_EXPIRY_INTERVAL, + [FileSources.azure_blob]: CacheKeys.AZURE_EXPIRY_INTERVAL, +}; + /** * Creates an Agent. * @route POST /Agents @@ -464,14 +482,13 @@ const getAgentHandler = async (req, res, expandProperties = false) => { agent.version = agent.versions ? agent.versions.length : 0; - if (agent.avatar && agent.avatar?.source === FileSources.s3) { + const refreshFn = agent.avatar && urlRefreshersBySource[agent.avatar.source]; + if (refreshFn) { try { - agent.avatar = { - ...agent.avatar, - filepath: await refreshS3Url(agent.avatar), - }; + const newPath = await refreshFn(agent.avatar); + agent.avatar = { ...agent.avatar, filepath: newPath }; } catch (e) { - logger.warn('[/Agents/:id] Failed to refresh S3 URL', e); + logger.warn('[/Agents/:id] Failed to refresh signed URL', e); } } @@ -925,10 +942,18 @@ const getListAgentsHandler = async (req, res) => { }); /** - * Refresh all S3 avatars for this user's accessible agent set (not only the current page) - * This addresses page-size limits preventing refresh of agents beyond the first page + * Refresh all signed-URL avatars (S3 + Azure) for this user's accessible + * agent set (not only the current page). Addresses page-size limits + * preventing refresh of agents beyond the first page. + * + * Cache namespace and TTL are strategy-aware: hardcoding + * `S3_EXPIRY_INTERVAL` with `Time.THIRTY_MINUTES` outlived Azure's 5-min + * default URL TTL by ~25 minutes, serving expired SAS URLs from cache. */ - const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL); + const strategy = req.config?.fileStrategy; + const refreshCacheKey = + avatarRefreshCacheKeyByStrategy[strategy] ?? CacheKeys.S3_EXPIRY_INTERVAL; + const cache = getLogStores(refreshCacheKey); const refreshKey = `${userId}:agents_avatar_refresh`; let cachedRefresh = await cache.get(refreshKey); const isValidCachedRefresh = @@ -945,15 +970,16 @@ const getListAgentsHandler = async (req, res) => { agents: fullList?.data ?? [], userId, refreshS3Url, + refreshAzureUrl, updateAgent: db.updateAgent, }); cachedRefresh = { urlCache }; - await cache.set(refreshKey, cachedRefresh, Time.THIRTY_MINUTES); + await cache.set(refreshKey, cachedRefresh, getFileURLRefreshCacheTime(strategy)); } catch (err) { logger.error('[/Agents] Error refreshing avatars for full list: %o', err); } } else { - logger.debug('[/Agents] S3 avatar refresh already checked, skipping'); + logger.debug('[/Agents] Avatar refresh already checked, skipping'); } // Use the new ACL-aware function @@ -992,10 +1018,16 @@ const getListAgentsHandler = async (req, res) => { if (agent?._id && publicSet.has(agent._id.toString())) { agent.isPublic = true; } + // Only replay cached URLs onto avatars whose source is still a signed-URL + // provider. If the agent's avatar source changed (e.g. to `local`) after + // the cache entry was written, the stale signed URL would otherwise + // overwrite the new filepath until cache expiry. if ( urlCache && agent?.id && - agent?.avatar?.source === FileSources.s3 && + agent?.avatar && + (agent.avatar.source === FileSources.s3 || + agent.avatar.source === FileSources.azure_blob) && urlCache[agent.id] ) { agent.avatar = { ...agent.avatar, filepath: urlCache[agent.id] }; @@ -1092,7 +1124,13 @@ const uploadAgentAvatarHandler = async (req, res) => { }); try { - const avatarCache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL); + // Strategy-aware: invalidate the cache namespace the active file + // strategy populated. Hardcoding S3_EXPIRY_INTERVAL leaked stale + // Azure entries through avatar uploads — the agent list kept + // serving the old avatar until the cache entry expired naturally. + const avatarRefreshCacheKey = + avatarRefreshCacheKeyByStrategy[fileStrategy] ?? CacheKeys.S3_EXPIRY_INTERVAL; + const avatarCache = getLogStores(avatarRefreshCacheKey); await avatarCache.delete(`${req.user.id}:agents_avatar_refresh`); } catch (cacheErr) { logger.error('[/:agent_id/avatar] Error invalidating avatar refresh cache', cacheErr); diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index b6601549751..bc3ca0cd2a1 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -8,7 +8,6 @@ const { verifyAgentUploadPermission, } = require('@librechat/api'); const { - Time, isUUID, CacheKeys, FileSources, @@ -29,7 +28,9 @@ const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { getOpenAIClient } = require('~/server/controllers/assistants/helpers'); const { hasCapability } = require('~/server/middleware/roles/capabilities'); const { checkPermission } = require('~/server/services/PermissionService'); +const { refreshAzureFileUrls } = require('~/server/services/Files/Azure/crud'); const { hasAccessToFilesViaAgent } = require('~/server/services/Files'); +const { getFileURLRefreshCacheTime } = require('~/server/utils/getFileStrategy'); const { cleanFileName, getContentDisposition } = require('~/server/utils/files'); const { getLogStores } = require('~/cache'); const { Readable } = require('stream'); @@ -37,20 +38,41 @@ const db = require('~/models'); const router = express.Router(); +/** + * Dispatch table for refreshing signed file URLs by storage source. + * Adding a new provider only requires another entry here. + */ +const fileUrlRefreshBySource = { + [FileSources.s3]: { + cacheKey: CacheKeys.S3_EXPIRY_INTERVAL, + refreshFiles: refreshS3FileUrls, + }, + [FileSources.azure_blob]: { + cacheKey: CacheKeys.AZURE_EXPIRY_INTERVAL, + refreshFiles: refreshAzureFileUrls, + }, +}; + router.get('/', async (req, res) => { try { const appConfig = req.config; const files = await db.getFiles({ user: req.user.id }); - if (appConfig.fileStrategy === FileSources.s3) { + const refresher = fileUrlRefreshBySource[appConfig.fileStrategy]; + if (refresher) { try { - const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL); + const cache = getLogStores(refresher.cacheKey); const alreadyChecked = await cache.get(req.user.id); if (!alreadyChecked) { - await refreshS3FileUrls(files, db.batchUpdateFiles); - await cache.set(req.user.id, true, Time.THIRTY_MINUTES); + await refresher.refreshFiles(files, db.batchUpdateFiles); + const cacheTime = getFileURLRefreshCacheTime(appConfig.fileStrategy); + await cache.set(req.user.id, true, cacheTime); } } catch (error) { - logger.warn('[/files] Error refreshing S3 file URLs:', error); + logger.warn( + '[/files] Error refreshing file URLs for strategy:', + appConfig.fileStrategy, + error, + ); } } res.status(200).send(files); diff --git a/api/server/services/Files/Azure/crud.js b/api/server/services/Files/Azure/crud.js index 8f681bd06c6..c5e6751d585 100644 --- a/api/server/services/Files/Azure/crud.js +++ b/api/server/services/Files/Azure/crud.js @@ -1,13 +1,224 @@ const fs = require('fs'); const path = require('path'); const mime = require('mime'); -const axios = require('axios'); const fetch = require('node-fetch'); const { logger } = require('@librechat/data-schemas'); +const { FileSources } = require('librechat-data-provider'); const { getAzureContainerClient, deleteRagFile } = require('@librechat/api'); +const { DefaultAzureCredential, ManagedIdentityCredential } = require('@azure/identity'); const defaultBasePath = 'images'; -const { AZURE_STORAGE_PUBLIC_ACCESS = 'true', AZURE_CONTAINER_NAME = 'files' } = process.env; +const { AZURE_CONTAINER_NAME = 'files' } = process.env; +const isPublicAccess = () => process.env.AZURE_STORAGE_PUBLIC_ACCESS?.toLowerCase() === 'true'; +const { + BlobServiceClient, + BlobSASPermissions, + SASProtocol, + generateBlobSASQueryParameters, + StorageSharedKeyCredential, +} = require('@azure/storage-blob'); + +const DEFAULT_AZURE_URL_EXPIRY_SECONDS = 5 * 60; +let azureUrlExpirySeconds = DEFAULT_AZURE_URL_EXPIRY_SECONDS; +let azureRefreshExpiryMs = null; + +// Delegation-key cache. Stored as `{ promise, expiresOn }` rather than the +// resolved key so concurrent first-callers all await the *same* in-flight +// fetch — without this, N concurrent first requests each fire an AAD +// round-trip and the last writer wins (benign but wasteful). +let _delegationKeyState = null; +const DELEGATION_KEY_REFRESH_BUFFER_MS = 5 * 60 * 1000; // 5 minutes + +// Module-level singletons for the Managed Identity / User Delegation SAS +// path. Both the BlobServiceClient and the credential carry caches (the +// credential holds the AAD token cache) that must be reused across requests +// — recreating per request cold-starts both caches and adds an AAD +// round-trip in the worst case. Keyed on account name so a runtime +// reconfiguration to a different storage account picks up cleanly. +let _miAccountName = null; +let _miBlobServiceClient = null; + +if (process.env.AZURE_URL_EXPIRY_SECONDS !== undefined) { + const parsed = parseInt(process.env.AZURE_URL_EXPIRY_SECONDS, 10); + if (!Number.isNaN(parsed) && parsed > 0) { + azureUrlExpirySeconds = Math.min(parsed, 7 * 24 * 60 * 60); + } else { + logger.warn( + `[Azure] Invalid AZURE_URL_EXPIRY_SECONDS value: "${process.env.AZURE_URL_EXPIRY_SECONDS}". Using ${DEFAULT_AZURE_URL_EXPIRY_SECONDS}-second expiry.`, + ); + } +} + +if (process.env.AZURE_REFRESH_EXPIRY_MS) { + const parsed = parseInt(process.env.AZURE_REFRESH_EXPIRY_MS, 10); + if (!Number.isNaN(parsed) && parsed > 0) { + azureRefreshExpiryMs = parsed; + logger.info(`[Azure] Using custom refresh expiry time: ${azureRefreshExpiryMs}ms`); + } +} + +/** + * Returns the shared {@link BlobServiceClient} for the Managed Identity / User + * Delegation SAS path. Lazily constructs the client and credential once per + * account name and reuses them across requests so the credential's AAD token + * cache survives — recreating these per request adds a token round-trip in + * the worst case. + * + * In production we explicitly use `ManagedIdentityCredential` (AKS pod + * identity / workload identity). `DefaultAzureCredential` walks a chain that + * includes env vars, Azure CLI, VS Code, etc., which is friendly in dev but + * dangerous in prod (it can silently fall through to an unexpected identity). + * + * @param {string} accountName - The Azure Storage account name. + * @returns {Promise} + */ +function getManagedIdentityBlobServiceClient(accountName) { + if (_miBlobServiceClient && _miAccountName === accountName) { + return _miBlobServiceClient; + } + const credential = + process.env.NODE_ENV === 'production' + ? new ManagedIdentityCredential() + : new DefaultAzureCredential(); + // Default to the public-cloud blob endpoint, but honour an explicit + // AZURE_STORAGE_BLOB_ENDPOINT override so sovereign clouds (Azure Gov, + // Azure China) and custom-domain deployments work too. The default + // hardcoded `core.windows.net` suffix would otherwise break MI signing + // in those environments. + const endpoint = + process.env.AZURE_STORAGE_BLOB_ENDPOINT || `https://${accountName}.blob.core.windows.net`; + _miBlobServiceClient = new BlobServiceClient(endpoint, credential); + _miAccountName = accountName; + // Invalidate the delegation-key cache when the storage account changes so we + // don't sign blobs in account A with a key minted for account B. + _delegationKeyState = null; + return _miBlobServiceClient; +} + +/** + * Obtains and caches a User Delegation Key from Azure Blob Storage for use with + * Managed Identity (User Delegation SAS) URL signing. The key is cached for its + * full 7-day lifespan and proactively refreshed within a 5-minute buffer of + * expiry. + * + * Concurrency: caches the in-flight Promise (not the resolved key), so N + * concurrent first-callers all await the same fetch instead of each firing + * their own AAD round-trip. + * + * @param {import('@azure/storage-blob').BlobServiceClient} blobServiceClient + * @returns {Promise} + */ +function getUserDelegationKey(blobServiceClient) { + if (_delegationKeyState) { + const timeUntilExpiry = _delegationKeyState.expiresOn.getTime() - Date.now(); + if (timeUntilExpiry > DELEGATION_KEY_REFRESH_BUFFER_MS) { + return _delegationKeyState.promise; + } + // Within the refresh buffer — drop the stale entry and re-fetch below. + _delegationKeyState = null; + } + + const startsOn = new Date(); + const expiresOn = new Date(startsOn.getTime() + 7 * 24 * 60 * 60 * 1000); // 7 days + + const promise = blobServiceClient.getUserDelegationKey(startsOn, expiresOn).then( + (key) => { + logger.info('[Azure] User delegation key obtained, expires:', expiresOn.toISOString()); + return key; + }, + (err) => { + // Don't cache a rejected promise — drop the slot so the next caller retries. + _delegationKeyState = null; + throw err; + }, + ); + _delegationKeyState = { promise, expiresOn }; + return promise; +} + +/** + * Generates a time-limited signed URL (SAS token) for an Azure Blob. + * Automatically selects the signing method based on configuration: + * - Account Key signing when `AZURE_STORAGE_CONNECTION_STRING` is set + * - User Delegation SAS via Managed Identity when only `AZURE_STORAGE_ACCOUNT_NAME` is set + * + * @param {Object} params + * @param {string} params.blobPath - The path of the blob within the container. + * @param {string} [params.containerName=AZURE_CONTAINER_NAME] - The Azure Blob Storage container name. + * @returns {Promise} A promise that resolves to the generated signed URL. + */ +async function getSignedAzureURL({ blobPath, containerName = AZURE_CONTAINER_NAME }) { + try { + const connectionString = process.env.AZURE_STORAGE_CONNECTION_STRING; + const accountName = process.env.AZURE_STORAGE_ACCOUNT_NAME; + + const startsOn = new Date(); + const expiresOn = new Date(startsOn.getTime() + azureUrlExpirySeconds * 1000); + + if (connectionString) { + const blobServiceClient = BlobServiceClient.fromConnectionString(connectionString); + const containerClient = blobServiceClient.getContainerClient(containerName); + const blockBlobClient = containerClient.getBlockBlobClient(blobPath); + + const match = connectionString.match(/AccountName=([^;]+)/i); + const keyMatch = connectionString.match(/AccountKey=([^;]+)/i); + + if (!match || !keyMatch) { + // Fail loud: a connection string without AccountName/AccountKey cannot + // produce a signed URL. Returning the unsigned URL here would be a silent + // security regression — callers expect a SAS when private access is on. + throw new Error( + '[getSignedAzureURL] AZURE_STORAGE_CONNECTION_STRING is missing AccountName or AccountKey; cannot generate a SAS token', + ); + } + + const sharedKeyCredential = new StorageSharedKeyCredential(match[1], keyMatch[1]); + + const sasToken = generateBlobSASQueryParameters( + { + containerName, + blobName: blobPath, + permissions: BlobSASPermissions.parse('r'), + protocol: SASProtocol.Https, + startsOn, + expiresOn, + }, + sharedKeyCredential, + ).toString(); + + return `${blockBlobClient.url}?${sasToken}`; + } + + if (accountName) { + const blobServiceClient = await getManagedIdentityBlobServiceClient(accountName); + const delegationKey = await getUserDelegationKey(blobServiceClient); + const containerClient = blobServiceClient.getContainerClient(containerName); + const blockBlobClient = containerClient.getBlockBlobClient(blobPath); + + const sasToken = generateBlobSASQueryParameters( + { + containerName, + blobName: blobPath, + permissions: BlobSASPermissions.parse('r'), + protocol: SASProtocol.Https, + startsOn, + expiresOn, + }, + delegationKey, + accountName, + ).toString(); + + return `${blockBlobClient.url}?${sasToken}`; + } + + throw new Error( + 'Azure storage not configured: set AZURE_STORAGE_CONNECTION_STRING for local/AccountKey signing, or AZURE_STORAGE_ACCOUNT_NAME for Managed Identity', + ); + } catch (error) { + logger.error('[getSignedAzureURL] Error generating signed URL:', error); + throw error; + } +} /** * Uploads a buffer to Azure Blob Storage. @@ -31,12 +242,15 @@ async function saveBufferToAzure({ }) { try { const containerClient = await getAzureContainerClient(containerName); - const access = AZURE_STORAGE_PUBLIC_ACCESS?.toLowerCase() === 'true' ? 'blob' : undefined; - // Create the container if it doesn't exist. This is done per operation. + const access = isPublicAccess() ? 'blob' : undefined; await containerClient.createIfNotExists({ access }); const blobPath = `${basePath}/${userId}/${fileName}`; const blockBlobClient = containerClient.getBlockBlobClient(blobPath); await blockBlobClient.uploadData(buffer); + + if (!isPublicAccess()) { + return await getSignedAzureURL({ blobPath, containerName }); + } return blockBlobClient.url; } catch (error) { logger.error('[saveBufferToAzure] Error uploading buffer:', error); @@ -84,8 +298,11 @@ async function saveURLToAzure({ */ async function getAzureURL({ fileName, basePath = defaultBasePath, userId, containerName }) { try { - const containerClient = await getAzureContainerClient(containerName); const blobPath = userId ? `${basePath}/${userId}/${fileName}` : `${basePath}/${fileName}`; + if (!isPublicAccess()) { + return await getSignedAzureURL({ blobPath, containerName }); + } + const containerClient = await getAzureContainerClient(containerName); const blockBlobClient = containerClient.getBlockBlobClient(blobPath); return blockBlobClient.url; } catch (error) { @@ -106,7 +323,10 @@ async function deleteFileFromAzure(req, file) { try { const containerClient = await getAzureContainerClient(AZURE_CONTAINER_NAME); - const blobPath = file.filepath.split(`${AZURE_CONTAINER_NAME}/`)[1]; + const blobPath = extractBlobPathFromAzureUrl(file.filepath, AZURE_CONTAINER_NAME); + if (!blobPath) { + throw new Error(`[deleteFileFromAzure] Unable to extract blob path from: ${file.filepath}`); + } if (!blobPath.includes(req.user.id)) { throw new Error('User ID not found in blob path'); } @@ -143,18 +363,13 @@ async function streamFileToAzure({ }) { try { const containerClient = await getAzureContainerClient(containerName); - const access = AZURE_STORAGE_PUBLIC_ACCESS?.toLowerCase() === 'true' ? 'blob' : undefined; - - // Create the container if it doesn't exist + const access = isPublicAccess() ? 'blob' : undefined; await containerClient.createIfNotExists({ access }); const blobPath = `${basePath}/${userId}/${fileName}`; const blockBlobClient = containerClient.getBlockBlobClient(blobPath); - // Get file size for proper content length const stats = await fs.promises.stat(filePath); - - // Create read stream from the file const fileStream = fs.createReadStream(filePath); const blobContentType = mime.getType(fileName); @@ -174,6 +389,9 @@ async function streamFileToAzure({ }, ); + if (!isPublicAccess()) { + return await getSignedAzureURL({ blobPath, containerName }); + } return blockBlobClient.url; } catch (error) { logger.error('[streamFileToAzure] Error streaming file:', error); @@ -227,24 +445,319 @@ async function uploadFileToAzure({ /** * Retrieves a readable stream for a blob from Azure Blob Storage. * + * Downloads through the authenticated SDK client (Account Key / Managed + * Identity) rather than an anonymous HTTP GET, so it works on a private + * container (anonymous access disabled) — fixing vision and file + * preview/download, which previously 403/409'd. Mirrors `getS3FileStream` and + * `deleteFileFromAzure`: strip any SAS query, derive the blob path with + * `extractBlobPathFromAzureUrl`, and read via `getAzureContainerClient`. + * * @param {object} _req - The Express request object. - * @param {string} fileURL - The URL of the blob. - * @returns {Promise} A readable stream of the blob. + * @param {string} fileURL - The URL of the blob (may carry a SAS query string). + * @returns {Promise} A readable stream of the blob. */ async function getAzureFileStream(_req, fileURL) { try { - const response = await axios({ - method: 'get', - url: fileURL, - responseType: 'stream', - }); - return response.data; + const blobPath = extractBlobPathFromAzureUrl(fileURL, AZURE_CONTAINER_NAME); + if (!blobPath) { + throw new Error(`[getAzureFileStream] Unable to extract blob path from: ${fileURL}`); + } + const containerClient = await getAzureContainerClient(AZURE_CONTAINER_NAME); + if (!containerClient) { + throw new Error('[getAzureFileStream] Azure Blob Service not initialized'); + } + const blockBlobClient = containerClient.getBlockBlobClient(blobPath); + const downloadResponse = await blockBlobClient.download(); + if (!downloadResponse.readableStreamBody) { + throw new Error(`[getAzureFileStream] Empty download body for blob: ${blobPath}`); + } + return downloadResponse.readableStreamBody; } catch (error) { logger.error('[getAzureFileStream] Error getting blob stream:', error); throw error; } } +/** + * Determines whether the provided Azure Blob Storage URL needs to be refreshed. + * A plain URL without a SAS token is treated as needing a refresh when private + * access is enabled, allowing seamless transitions from public to private access. + * + * @param {string} signedUrl - The Azure Blob Storage URL, which may include a SAS token. + * @param {number} bufferSeconds - The number of seconds before expiration at which the URL + * should be considered in need of refresh. + * @returns {boolean} `true` if the URL should be refreshed, `false` if it is still valid. + */ +/** + * Lightweight structural check for whether a URL belongs to Azure Blob Storage. + * + * Catches the standard public-cloud, sovereign-cloud, and Azurite emulator + * hostnames without trying to match against the configured account name (which + * would require runtime env reads at every call). False negatives are safe — + * worst case we skip a refresh that should have fired. False positives are + * costly — we'd attempt to re-sign a URL that isn't ours and either persist + * `undefined` or rewrite an unrelated URL into our container's namespace. + * + * @param {URL} parsedUrl - A pre-parsed URL instance. + * @returns {boolean} + */ +function isAzureBlobUrl(parsedUrl) { + const h = parsedUrl.hostname; + // Standard + sovereign-cloud Azure: .blob.core.windows.net, + // *.blob.core.usgovcloudapi.net, *.blob.core.chinacloudapi.cn, etc. + // Also modern: *.blob.storage.azure.net. + if (h.includes('.blob.')) { + return true; + } + // Azurite emulator default endpoint + if ((h === '127.0.0.1' || h === 'localhost') && parsedUrl.port === '10000') { + return true; + } + // Custom blob endpoint (Azure Blob behind a CNAME, sovereign cloud with a + // non-default suffix, etc.) configured via AZURE_STORAGE_BLOB_ENDPOINT. + // Read at call time so a runtime config flip is picked up. + const customEndpoint = process.env.AZURE_STORAGE_BLOB_ENDPOINT; + if (customEndpoint) { + try { + if (new URL(customEndpoint).hostname === h) { + return true; + } + } catch { + // Malformed env value — ignore. The signing path will surface it loudly + // if it ends up trying to use it. + } + } + return false; +} + +function needsRefreshAzure(signedUrl, bufferSeconds) { + try { + const url = new URL(signedUrl); + // Not an Azure Blob URL — leave it alone. User avatars are plain strings + // with no source metadata, so a Google/OAuth provider URL or a legacy + // `/uploads/` path can end up here. Mirrors S3's `X-Amz-Signature` check: + // refresh only URLs we own. + if (!isAzureBlobUrl(url)) { + return false; + } + // Use `sig` (the cryptographic SAS signature) — not `se` (just the expiry + // timestamp) — as the SAS-token presence check. A hand-crafted URL could + // include `se` without a signature; only `sig` proves the URL was signed. + // Mirrors S3's `X-Amz-Signature` check. + const hasSasToken = url.searchParams.has('sig'); + + // Private access required but URL is plain → needs signing + if (!isPublicAccess() && !hasSasToken) { + return true; + } + + // Public access and no SAS token → no refresh needed + if (!hasSasToken) { + return false; + } + + // Check expiration for signed URLs + const expiresParam = url.searchParams.get('se'); + if (!expiresParam) { + return true; + } + + const expiresAtDate = new Date(expiresParam); + const now = new Date(); + + if (azureRefreshExpiryMs !== null) { + const stParam = url.searchParams.get('st'); + if (stParam) { + const urlCreationTime = new Date(stParam).getTime(); + const urlAge = now.getTime() - urlCreationTime; + return urlAge >= azureRefreshExpiryMs; + } + } + + const bufferTime = new Date(now.getTime() + bufferSeconds * 1000); + return expiresAtDate <= bufferTime; + } catch (error) { + logger.error(`[needsRefreshAzure] Error checking URL expiration for "${signedUrl}":`, error); + return true; + } +} + +/** + * Extracts the blob path from an Azure Blob Storage URL. + * + * Handles both: + * - Virtual-hosted-style: `https://.blob.core.windows.net//` + * - Path-style / emulator: `https://///` + * (used by Azurite, where URLs look like `http://127.0.0.1:10000/devstoreaccount1//`) + * + * The container name is used as an anchor: everything after the first path segment + * matching the configured container is treated as the blob path. This avoids the bug + * where stripping the first segment blindly would yield `/` for + * emulator URLs (leaking the container name into the blob path). + * + * @param {string} fileUrl - The full Azure Blob Storage URL of the file. + * @param {string} [containerName] - The container name to anchor on. Defaults to + * the value of `AZURE_CONTAINER_NAME` (read at call time, fallback `'files'`). + * @returns {string | null} The blob path within the container, or `null` on failure. + */ +function extractBlobPathFromAzureUrl(fileUrl, containerName) { + try { + const url = new URL(fileUrl); + const parts = url.pathname.split('/').filter(Boolean); + if (parts.length < 2) { + return null; + } + const container = containerName ?? process.env.AZURE_CONTAINER_NAME ?? 'files'; + const containerIdx = parts.indexOf(container); + if (containerIdx === -1 || containerIdx === parts.length - 1) { + // Container segment not found (or it's the last segment with no blob path + // after it). Fall back to the legacy "strip first segment" behavior so URLs + // generated before this helper existed (or under a container name that has + // since changed) still resolve to *something* — but log it loudly so the + // misconfiguration is visible. + logger.warn( + `[extractBlobPathFromAzureUrl] Container "${container}" not found in URL path "${url.pathname}"; falling back to legacy extraction`, + ); + return parts.slice(1).join('/') || null; + } + return parts.slice(containerIdx + 1).join('/'); + } catch (error) { + logger.error('[extractBlobPathFromAzureUrl] Error extracting blob path:', error); + return null; + } +} + +/** + * Generates a new Azure Blob URL for the given file URL. + * Returns a signed (SAS) URL when private access is enabled, or a plain blob URL + * when public access is configured. + * + * @param {string} currentURL - The existing Azure Blob file URL that may need to be refreshed or signed. + * @param {string} [containerName] - The container name to extract against and resign within. When + * omitted, falls back to `AZURE_CONTAINER_NAME` (read at call time, default `'files'`). Pass + * the original blob's container when supporting multi-container deployments, otherwise refresh + * silently rewrites the URL into the default container's namespace. + * @returns {Promise} A promise that resolves to the new URL, + * or `undefined` if the blob path cannot be extracted. + */ +async function getNewAzureURL(currentURL, containerName) { + try { + const container = containerName ?? process.env.AZURE_CONTAINER_NAME ?? 'files'; + const blobPath = extractBlobPathFromAzureUrl(currentURL, container); + if (!blobPath) { + return; + } + + // Mirror the S3 `keyParts.length < 3` guard: our uploaders always write to + // `{basePath}/{userId}/{fileName}`, so anything shallower is malformed input + // we shouldn't sign or rebuild a URL for. + if (blobPath.split('/').length < 3) { + logger.warn(`[getNewAzureURL] Unexpected blob path structure: "${blobPath}"`); + return; + } + + if (!isPublicAccess()) { + return await getSignedAzureURL({ blobPath, containerName: container }); + } + + // Return plain URL for public access + const containerClient = await getAzureContainerClient(container); + const blockBlobClient = containerClient.getBlockBlobClient(blobPath); + return blockBlobClient.url; + } catch (error) { + logger.error('[getNewAzureURL] Error getting new Azure URL:', error); + } +} + +/** + * Refreshes Azure Blob file URLs that are close to expiring and persists the updated URLs. + * + * @param {MongoFile[]} files - The list of file records whose Azure URLs may need refreshing. + * @param {(updates: Partial[]) => Promise} batchUpdateFiles - Function used to + * update the file records in bulk with their new URLs. + * @param {number} [bufferSeconds=3600] - The minimum number of seconds before expiry at which a + * URL should be refreshed. + * @returns {Promise} A promise that resolves to the array of files with refreshed URLs. + */ +async function refreshAzureFileUrls(files, batchUpdateFiles, bufferSeconds = 3600) { + if (!files || !Array.isArray(files) || files.length === 0) { + return files; + } + + const filesToUpdate = []; + + for (let i = 0; i < files.length; i++) { + const file = files[i]; + if (!file?.file_id || file.source !== FileSources.azure_blob || !file.filepath) { + continue; + } + if (!needsRefreshAzure(file.filepath, bufferSeconds)) { + continue; + } + try { + const newURL = await getNewAzureURL(file.filepath); + if (!newURL) { + continue; + } + filesToUpdate.push({ + file_id: file.file_id, + filepath: newURL, + }); + files[i].filepath = newURL; + } catch (error) { + logger.error( + `[refreshAzureFileUrls] Error refreshing Azure URL for file ${file.file_id}:`, + error, + ); + } + } + + if (filesToUpdate.length > 0) { + await batchUpdateFiles(filesToUpdate); + } + + return files; +} + +/** + * Refreshes an Azure Blob storage file URL if it is close to expiration. + * + * When the file originates from Azure Blob storage and the existing URL is determined to be + * near expiry (based on {@link needsRefreshAzure} and the provided `bufferSeconds`), this + * function generates and returns a new URL. If the URL does not need to be refreshed or a + * new URL cannot be obtained, the original URL is returned instead. + * + * @param {Object} fileObj - The file metadata object. + * @param {string} fileObj.filepath - The current Azure Blob file URL that may need refreshing. + * @param {string} fileObj.source - The source type; must be {@link FileSources.azure_blob} to trigger a refresh. + * @param {number} [bufferSeconds=3600] - The number of seconds before URL expiry within which a refresh should be attempted. + * @returns {Promise} A promise that resolves to the (possibly refreshed) Azure Blob file URL. + */ +async function refreshAzureUrl(fileObj, bufferSeconds = 3600) { + if (!fileObj || fileObj.source !== FileSources.azure_blob || !fileObj.filepath) { + return fileObj?.filepath || ''; + } + + if (!needsRefreshAzure(fileObj.filepath, bufferSeconds)) { + return fileObj.filepath; + } + + try { + const newUrl = await getNewAzureURL(fileObj.filepath); + if (!newUrl) { + logger.warn(`[refreshAzureUrl] Unable to refresh Azure URL: ${fileObj.filepath}`); + return fileObj.filepath; + } + logger.debug(`[refreshAzureUrl] Refreshed URL for ${fileObj.file_id ?? '(unknown file_id)'}`); + return newUrl; + } catch (error) { + logger.error( + `[refreshAzureUrl] Error refreshing Azure URL for ${fileObj.file_id ?? '(unknown file_id)'}: ${error.message}`, + ); + return fileObj.filepath; + } +} + module.exports = { saveBufferToAzure, saveURLToAzure, @@ -252,4 +765,11 @@ module.exports = { deleteFileFromAzure, uploadFileToAzure, getAzureFileStream, + getSignedAzureURL, + needsRefreshAzure, + refreshAzureFileUrls, + refreshAzureUrl, + getNewAzureURL, + extractBlobPathFromAzureUrl, + isAzureBlobUrl, }; diff --git a/api/server/services/Files/Azure/images.js b/api/server/services/Files/Azure/images.js index 4d857d0d1db..ef513748051 100644 --- a/api/server/services/Files/Azure/images.js +++ b/api/server/services/Files/Azure/images.js @@ -122,15 +122,36 @@ async function processAzureAvatar({ basePath, containerName, }); + + // Preserve the `manual=true` marker on manually-uploaded user avatars. + // The social-login strategies (samlStrategy / OpenID / OAuth) skip + // overwriting an avatar when `user.avatar?.includes('manual=true')`. + // Stripping this marker would let any subsequent social-login profile- + // sync flow overwrite a user's manually-uploaded avatar. Use the URL + // API so any existing query string (the SAS) is preserved. const isManual = manual === 'true'; - const url = `${downloadURL}?manual=${isManual}`; + const finalURL = (() => { + if (!isManual) { + return downloadURL; + } + try { + const u = new URL(downloadURL); + u.searchParams.set('manual', 'true'); + return u.toString(); + } catch { + // Defensive: if URL parsing somehow fails, fall back to the simple + // append. `downloadURL` should always be a well-formed http(s) URL. + return downloadURL.includes('?') + ? `${downloadURL}&manual=true` + : `${downloadURL}?manual=true`; + } + })(); - // Only update user record if this is a user avatar (manual === 'true') if (isManual && !agentId) { - await updateUser(userId, { avatar: url }); + await updateUser(userId, { avatar: finalURL }); } - return url; + return finalURL; } catch (error) { logger.error('[processAzureAvatar] Error uploading profile picture to Azure:', error); throw error; diff --git a/api/server/services/Files/images/encode.js b/api/server/services/Files/images/encode.js index 2f075229aa9..6d337bca43e 100644 --- a/api/server/services/Files/images/encode.js +++ b/api/server/services/Files/images/encode.js @@ -144,6 +144,14 @@ async function encodeAndFormat(req, files, params, mode) { continue; } catch (error) { logger.error('Error processing image from blob storage:', error); + // Fail closed for azure_blob: a private container serves no + // anonymously-fetchable URL, so falling through to preparePayload would + // hand the model an unfetchable blob URL (a silent vision failure). S3 + // and Firebase keep the fall-through (their URLs are presigned / + // publicly fetchable). + if (source === FileSources.azure_blob) { + throw error; + } } } else if (source !== FileSources.local && base64Only.has(effectiveEndpoint)) { const [_file, imageURL] = await preparePayload(req, file); diff --git a/api/server/utils/getFileStrategy.js b/api/server/utils/getFileStrategy.js index 1bab7f2468d..0ce97d427a7 100644 --- a/api/server/utils/getFileStrategy.js +++ b/api/server/utils/getFileStrategy.js @@ -1,4 +1,4 @@ -const { FileSources, FileContext } = require('librechat-data-provider'); +const { Time, FileSources, FileContext } = require('librechat-data-provider'); /** * Determines the appropriate file storage strategy based on file type and configuration. @@ -57,4 +57,36 @@ function getFileStrategy(appConfig, { isAvatar = false, isImage = false, context return selectedStrategy || FileSources.local; // Final fallback to FileSources.local } -module.exports = { getFileStrategy }; +/** + * Calculates the cache duration for signed file URL refresh checks. + * Uses half the URL expiry time to ensure URLs are refreshed before they expire, + * with a minimum of one minute. + * + * The configured URL TTL is clamped to the same 7-day maximum the signers + * enforce (Azure SAS / S3 presign upper bound). Without that clamp, an + * oversized `S3_URL_EXPIRY_SECONDS` / `AZURE_URL_EXPIRY_SECONDS` value would + * make the refresh-check cache live far longer than the actual signed URL + * validity — `/files` and `/Agents` would skip refresh and keep serving + * expired URLs. + * + * @param {string} fileStrategy - The active file storage strategy (e.g. FileSources.s3, FileSources.azure_blob) + * @returns {number} Cache duration in milliseconds + */ +const MAX_SIGNED_URL_TTL_SECONDS = 7 * 24 * 60 * 60; +function getFileURLRefreshCacheTime(fileStrategy) { + if (fileStrategy === FileSources.s3) { + const raw = parseInt(process.env.S3_URL_EXPIRY_SECONDS, 10) || 120; + const expirySeconds = Math.min(raw, MAX_SIGNED_URL_TTL_SECONDS); + // Check for refresh at half the URL expiry time: (expirySeconds * 1000ms) / 2 => expirySeconds * 500 + return Math.max(expirySeconds * 500, Time.ONE_MINUTE); + } + if (fileStrategy === FileSources.azure_blob) { + const raw = parseInt(process.env.AZURE_URL_EXPIRY_SECONDS, 10) || 300; + const expirySeconds = Math.min(raw, MAX_SIGNED_URL_TTL_SECONDS); + // Check for refresh at half the URL expiry time: (expirySeconds * 1000ms) / 2 => expirySeconds * 500 + return Math.max(expirySeconds * 500, Time.ONE_MINUTE); + } + return Time.THIRTY_MINUTES; +} + +module.exports = { getFileStrategy, getFileURLRefreshCacheTime }; diff --git a/api/test/server/services/Files/Azure/crud.test.js b/api/test/server/services/Files/Azure/crud.test.js new file mode 100644 index 00000000000..be50a55df20 --- /dev/null +++ b/api/test/server/services/Files/Azure/crud.test.js @@ -0,0 +1,726 @@ +const { + needsRefreshAzure, + extractBlobPathFromAzureUrl, + getNewAzureURL, + getSignedAzureURL, + getAzureURL, + getAzureFileStream, + deleteFileFromAzure, + refreshAzureFileUrls, + refreshAzureUrl, + isAzureBlobUrl, +} = require('../../../../../server/services/Files/Azure/crud'); + +jest.mock('@librechat/data-schemas', () => ({ + logger: { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() }, +})); + +jest.mock('@librechat/api', () => ({ + getAzureContainerClient: jest.fn(), + deleteRagFile: jest.fn(), +})); + +// Mock @azure/identity so the Managed Identity path can be exercised without +// the real credential chain trying to reach IMDS / AAD. Both credential +// constructors are stubbed; the BlobServiceClient itself is real (its +// constructor doesn't make network calls — only `getUserDelegationKey` does, +// which we spy on at the prototype level in the MI tests). +jest.mock('@azure/identity', () => ({ + DefaultAzureCredential: jest.fn().mockImplementation(() => ({ + getToken: jest + .fn() + .mockResolvedValue({ token: 'fake', expiresOnTimestamp: Date.now() + 3.6e6 }), + })), + ManagedIdentityCredential: jest.fn().mockImplementation(() => ({ + getToken: jest + .fn() + .mockResolvedValue({ token: 'fake', expiresOnTimestamp: Date.now() + 3.6e6 }), + })), +})); + +const { getAzureContainerClient, deleteRagFile } = require('@librechat/api'); +const azureIdentity = require('@azure/identity'); + +// A syntactically-valid connection string accepted by BlobServiceClient.fromConnectionString. +// AccountKey is the base64-encoding of 32 zero bytes, which is the minimum valid shape +// for StorageSharedKeyCredential. No real account is contacted by these tests. +const FAKE_CONNECTION_STRING = + 'DefaultEndpointsProtocol=https;' + + 'AccountName=testaccount;' + + 'AccountKey=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA;' + + 'EndpointSuffix=core.windows.net'; + +describe('Azure crud.js - URL refresh tests', () => { + const originalEnv = process.env; + + beforeEach(() => { + jest.clearAllMocks(); + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + describe('needsRefreshAzure', () => { + it('should return false for URLs without SAS token when public access enabled', () => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + const url = 'https://test.blob.core.windows.net/files/images/user123/test.pdf'; + expect(needsRefreshAzure(url, 3600)).toBe(false); + }); + + it('should return true for expired URLs', () => { + const pastDate = new Date(Date.now() - 3600000).toISOString(); + const url = `https://test.blob.core.windows.net/files/test.pdf?se=${encodeURIComponent(pastDate)}&sp=r&sig=test`; + expect(needsRefreshAzure(url, 3600)).toBe(true); + }); + + it('should return false for URLs that are not close to expiring', () => { + const futureDate = new Date(Date.now() + 7200000).toISOString(); + const url = `https://test.blob.core.windows.net/files/test.pdf?se=${encodeURIComponent(futureDate)}&sp=r&sig=test`; + expect(needsRefreshAzure(url, 3600)).toBe(false); + }); + + it('should return true for URLs within buffer time', () => { + const nearFutureDate = new Date(Date.now() + 1800000).toISOString(); + const url = `https://test.blob.core.windows.net/files/test.pdf?se=${encodeURIComponent(nearFutureDate)}&sp=r&sig=test`; + expect(needsRefreshAzure(url, 3600)).toBe(true); + }); + + it('should return true for invalid URLs', () => { + expect(needsRefreshAzure('not-a-valid-url', 3600)).toBe(true); + }); + }); + + describe('extractBlobPathFromAzureUrl', () => { + it('should extract blob path from Azure URL', () => { + const url = 'https://test.blob.core.windows.net/files/images/user123/test.pdf'; + expect(extractBlobPathFromAzureUrl(url)).toBe('images/user123/test.pdf'); + }); + + it('should extract blob path from URL with SAS token', () => { + const url = + 'https://test.blob.core.windows.net/files/images/user123/test.pdf?sv=2021&sig=abc'; + expect(extractBlobPathFromAzureUrl(url)).toBe('images/user123/test.pdf'); + }); + + it('should return null for URL with only container', () => { + const url = 'https://test.blob.core.windows.net/files'; + expect(extractBlobPathFromAzureUrl(url)).toBe(null); + }); + + it('should return null for invalid URL', () => { + expect(extractBlobPathFromAzureUrl('not-a-url')).toBe(null); + }); + + it('extracts blob path from a path-style URL (Azurite emulator)', () => { + // Azurite serves blobs at ///, so the + // legacy "drop the first segment" extraction would have leaked the + // container name into the blob path. The container-anchored extraction + // skips both and segments. + const url = 'http://127.0.0.1:10000/devstoreaccount1/files/images/user123/test.pdf'; + expect(extractBlobPathFromAzureUrl(url)).toBe('images/user123/test.pdf'); + }); + + it('extracts using an explicit container name override', () => { + const url = 'https://test.blob.core.windows.net/my-container/images/u/x.pdf'; + expect(extractBlobPathFromAzureUrl(url, 'my-container')).toBe('images/u/x.pdf'); + }); + + it('falls back to legacy extraction when the container segment is absent', () => { + // For URLs stored under a previous container name (e.g. after renaming + // AZURE_CONTAINER_NAME), we still return *something* so legacy records + // can be refreshed — but we log a warning at the call site. + const url = 'https://test.blob.core.windows.net/old-container/images/u/x.pdf'; + expect(extractBlobPathFromAzureUrl(url)).toBe('images/u/x.pdf'); + }); + }); + + describe('getNewAzureURL - path-parts guard', () => { + it('returns undefined for blob paths shallower than basePath/userId/fileName', async () => { + // Mirrors S3 getNewS3URL's keyParts.length < 3 guard. A blob path of + // `images/foo.pdf` is missing the userId segment and shouldn't be re-signed. + const url = 'https://test.blob.core.windows.net/files/images/foo.pdf'; + const result = await getNewAzureURL(url); + expect(result).toBeUndefined(); + }); + }); + + describe('getAzureURL - signs URL when private (S3 parity)', () => { + it('returns a SAS URL when AZURE_STORAGE_PUBLIC_ACCESS is not "true"', async () => { + // Regression guard: getAzureURL is exposed as the strategy's `getFileURL` + // and its return value is persisted as the file's filepath by processFileURL. + // S3's getS3URL always signs; Azure must match — previously it returned an + // unsigned blob URL, which 401'd until the next refresh cycle. + delete process.env.AZURE_STORAGE_PUBLIC_ACCESS; + process.env.AZURE_STORAGE_CONNECTION_STRING = FAKE_CONNECTION_STRING; + const url = await getAzureURL({ + fileName: 'test.pdf', + basePath: 'images', + userId: 'user123', + containerName: 'files', + }); + const parsed = new URL(url); + expect(parsed.searchParams.has('sig')).toBe(true); + expect(parsed.searchParams.get('sp')).toBe('r'); + expect(parsed.searchParams.get('spr')).toBe('https'); + }); + + it('returns the plain blob URL when AZURE_STORAGE_PUBLIC_ACCESS=true', async () => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + const mockBlockBlobClient = { + url: 'https://test.blob.core.windows.net/files/images/user123/test.pdf', + }; + const mockContainerClient = { + getBlockBlobClient: jest.fn().mockReturnValue(mockBlockBlobClient), + }; + getAzureContainerClient.mockResolvedValue(mockContainerClient); + + const url = await getAzureURL({ + fileName: 'test.pdf', + basePath: 'images', + userId: 'user123', + }); + expect(url).toBe(mockBlockBlobClient.url); + expect(new URL(url).searchParams.has('sig')).toBe(false); + }); + }); + + describe('needsRefreshAzure - leaves non-Azure URLs alone (PR-46 review fix)', () => { + // User avatars are plain strings with no `source` field. Before the fix, + // `needsRefreshAzure` returned `true` for any non-SAS URL in private mode + // (the public→private transition branch), which caused `getNewAzureURL` + // to be called on Google OAuth avatars and legacy `/uploads/` paths, + // returning `undefined` — which UserController then persisted to Mongo, + // wiping the user's avatar. The invariant: only refresh URLs we own. + beforeEach(() => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'false'; + }); + + it('returns false for OAuth provider avatar URLs (Google)', () => { + expect(needsRefreshAzure('https://lh3.googleusercontent.com/a/abc123=s96-c', 3600)).toBe( + false, + ); + }); + + it('returns false for OAuth provider avatar URLs (Microsoft)', () => { + expect(needsRefreshAzure('https://graph.microsoft.com/v1.0/me/photo/$value', 3600)).toBe( + false, + ); + }); + + it('returns false for legacy /uploads/ paths persisted as absolute URLs', () => { + expect(needsRefreshAzure('https://karl.example.com/uploads/userId/avatar.png', 3600)).toBe( + false, + ); + }); + + it('returns false for S3 URLs (post-migration straggler)', () => { + expect( + needsRefreshAzure( + 'https://my-bucket.s3.amazonaws.com/images/u/x.png?X-Amz-Signature=abc', + 3600, + ), + ).toBe(false); + }); + + it('still returns true for Azure URLs in private mode (the original public→private case)', () => { + // The original behavior we want to preserve: a plain Azure Blob URL in + // private mode means "this legacy URL needs signing", refresh it. + expect( + needsRefreshAzure('https://test.blob.core.windows.net/files/images/user123/test.pdf', 3600), + ).toBe(true); + }); + + it('still returns true for Azurite-shaped URLs in private mode', () => { + // Azurite (path-style) URLs are also "ours" — make sure the host-check + // accepts the loopback + port-10000 emulator endpoint. + expect( + needsRefreshAzure('http://127.0.0.1:10000/devstoreaccount1/files/images/user/x.pdf', 3600), + ).toBe(true); + }); + }); + + describe('isAzureBlobUrl', () => { + it('accepts standard Azure Blob hostnames', () => { + expect(isAzureBlobUrl(new URL('https://acc.blob.core.windows.net/files/x'))).toBe(true); + expect(isAzureBlobUrl(new URL('https://acc.blob.core.usgovcloudapi.net/files/x'))).toBe(true); + expect(isAzureBlobUrl(new URL('https://acc.blob.core.chinacloudapi.cn/files/x'))).toBe(true); + expect(isAzureBlobUrl(new URL('https://acc.blob.storage.azure.net/files/x'))).toBe(true); + }); + + it('accepts Azurite emulator endpoints', () => { + expect(isAzureBlobUrl(new URL('http://127.0.0.1:10000/devstoreaccount1/files/x'))).toBe(true); + expect(isAzureBlobUrl(new URL('http://localhost:10000/devstoreaccount1/files/x'))).toBe(true); + }); + + it('rejects non-Azure hostnames', () => { + expect(isAzureBlobUrl(new URL('https://lh3.googleusercontent.com/a/abc'))).toBe(false); + expect(isAzureBlobUrl(new URL('https://my-bucket.s3.amazonaws.com/k'))).toBe(false); + expect(isAzureBlobUrl(new URL('https://example.com/uploads/x'))).toBe(false); + // Loopback without the Azurite port doesn't count. + expect(isAzureBlobUrl(new URL('http://127.0.0.1:3000/files/x'))).toBe(false); + }); + }); + + describe('needsRefreshAzure - public to private transition', () => { + it('should return true for plain URL when private access is required', () => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'false'; + const plainUrl = 'https://test.blob.core.windows.net/files/images/user123/test.pdf'; + expect(needsRefreshAzure(plainUrl, 3600)).toBe(true); + }); + + it('should return false for plain URL when public access is enabled', () => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + const plainUrl = 'https://test.blob.core.windows.net/files/images/user123/test.pdf'; + expect(needsRefreshAzure(plainUrl, 3600)).toBe(false); + }); + }); + + describe('getNewAzureURL', () => { + it('should return undefined for a URL with no extractable blob path', async () => { + const invalidUrl = 'https://test.blob.core.windows.net/files'; + const result = await getNewAzureURL(invalidUrl); + expect(result).toBeUndefined(); + }); + + it('should return a plain blob URL when public access is enabled', async () => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + const mockBlockBlobClient = { + url: 'https://test.blob.core.windows.net/files/images/user123/test.pdf', + }; + const mockContainerClient = { + getBlockBlobClient: jest.fn().mockReturnValue(mockBlockBlobClient), + }; + getAzureContainerClient.mockResolvedValue(mockContainerClient); + + const url = 'https://test.blob.core.windows.net/files/images/user123/test.pdf'; + const result = await getNewAzureURL(url); + expect(result).toBe(mockBlockBlobClient.url); + }); + }); + + describe('getSignedAzureURL - configuration errors', () => { + it('throws when neither AZURE_STORAGE_CONNECTION_STRING nor AZURE_STORAGE_ACCOUNT_NAME is set', async () => { + delete process.env.AZURE_STORAGE_CONNECTION_STRING; + delete process.env.AZURE_STORAGE_ACCOUNT_NAME; + await expect(getSignedAzureURL({ blobPath: 'images/u/test.pdf' })).rejects.toThrow( + /Azure storage not configured/, + ); + }); + + it('throws (not silently returns unsigned URL) when AccountName/AccountKey cannot be parsed', async () => { + // In normal operation, BlobServiceClient.fromConnectionString already rejects + // any connection string lacking AccountName/AccountKey before our regex runs. + // The regex check is defense-in-depth against a future SDK regression. To + // exercise that branch directly we stub fromConnectionString so our regex + // is the gating check. The invariant under test: this path MUST throw, never + // return blockBlobClient.url (the prior behavior was a silent unsigned-URL leak). + const storageBlob = require('@azure/storage-blob'); + const spy = jest + .spyOn(storageBlob.BlobServiceClient, 'fromConnectionString') + .mockReturnValue({ + getContainerClient: () => ({ + getBlockBlobClient: () => ({ + url: 'https://test.blob.core.windows.net/files/images/u/test.pdf', + }), + }), + }); + try { + process.env.AZURE_STORAGE_CONNECTION_STRING = + 'DefaultEndpointsProtocol=https;BlobEndpoint=https://test.blob.core.windows.net;'; + delete process.env.AZURE_STORAGE_ACCOUNT_NAME; + await expect(getSignedAzureURL({ blobPath: 'images/u/test.pdf' })).rejects.toThrow( + /missing AccountName or AccountKey/, + ); + } finally { + spy.mockRestore(); + } + }); + }); + + describe('getSignedAzureURL - SAS token contents', () => { + beforeEach(() => { + process.env.AZURE_STORAGE_CONNECTION_STRING = FAKE_CONNECTION_STRING; + delete process.env.AZURE_STORAGE_ACCOUNT_NAME; + }); + + it('produces a SAS URL pinned to https (spr=https)', async () => { + const signed = await getSignedAzureURL({ + blobPath: 'images/u/test.pdf', + containerName: 'files', + }); + const url = new URL(signed); + // SAS "Protocol" param is `spr`. We require https-only — no `https,http`. + expect(url.searchParams.get('spr')).toBe('https'); + }); + + it('produces a read-only SAS (sp=r)', async () => { + const signed = await getSignedAzureURL({ + blobPath: 'images/u/test.pdf', + containerName: 'files', + }); + expect(new URL(signed).searchParams.get('sp')).toBe('r'); + }); + + it('uses the 300s default expiry when AZURE_URL_EXPIRY_SECONDS is unset', async () => { + // crud.js reads AZURE_URL_EXPIRY_SECONDS at module load. Re-require the module + // in an isolated registry with the env var cleared so we exercise the default. + delete process.env.AZURE_URL_EXPIRY_SECONDS; + let signed; + const before = Date.now(); + await jest.isolateModulesAsync(async () => { + const fresh = require('../../../../../server/services/Files/Azure/crud'); + signed = await fresh.getSignedAzureURL({ + blobPath: 'images/u/test.pdf', + containerName: 'files', + }); + }); + const after = Date.now(); + const url = new URL(signed); + const expiresAt = new Date(url.searchParams.get('se')).getTime(); + const startsAt = new Date(url.searchParams.get('st')).getTime(); + // Allow ±2s slack vs. the 300-second target. + expect(expiresAt - startsAt).toBeGreaterThanOrEqual(298_000); + expect(expiresAt - startsAt).toBeLessThanOrEqual(302_000); + // And `se` should be roughly 300s after "now". + expect(expiresAt - before).toBeGreaterThanOrEqual(298_000); + expect(expiresAt - after).toBeLessThanOrEqual(302_000); + }); + }); + + describe('deleteFileFromAzure', () => { + const blobDelete = jest.fn(); + const getBlockBlobClient = jest.fn().mockReturnValue({ delete: blobDelete }); + + beforeEach(() => { + blobDelete.mockReset(); + getBlockBlobClient.mockClear(); + getAzureContainerClient.mockResolvedValue({ getBlockBlobClient }); + deleteRagFile.mockResolvedValue(undefined); + process.env.AZURE_CONTAINER_NAME = 'files'; + }); + + it('extracts the blob path (stripping SAS query) and deletes the blob', async () => { + blobDelete.mockResolvedValue(undefined); + const req = { user: { id: 'user123' } }; + const file = { + file_id: 'f1', + filepath: + 'https://test.blob.core.windows.net/files/images/user123/file.pdf?sv=2023&sig=abc&se=2026-01-01T00:00:00Z', + }; + await deleteFileFromAzure(req, file); + expect(getBlockBlobClient).toHaveBeenCalledWith('images/user123/file.pdf'); + expect(blobDelete).toHaveBeenCalledTimes(1); + }); + + it('throws when the blob path does not include the requesting user id', async () => { + const req = { user: { id: 'attacker' } }; + const file = { + file_id: 'f2', + filepath: 'https://test.blob.core.windows.net/files/images/victim/file.pdf', + }; + await expect(deleteFileFromAzure(req, file)).rejects.toThrow( + /User ID not found in blob path/, + ); + expect(blobDelete).not.toHaveBeenCalled(); + }); + + it('swallows 404 errors from the SDK (already-gone blobs)', async () => { + const notFound = Object.assign(new Error('not found'), { statusCode: 404 }); + blobDelete.mockRejectedValue(notFound); + const req = { user: { id: 'user123' } }; + const file = { + file_id: 'f3', + filepath: 'https://test.blob.core.windows.net/files/images/user123/gone.pdf', + }; + await expect(deleteFileFromAzure(req, file)).resolves.toBeUndefined(); + }); + + it('rethrows non-404 SDK errors', async () => { + blobDelete.mockRejectedValue(Object.assign(new Error('boom'), { statusCode: 500 })); + const req = { user: { id: 'user123' } }; + const file = { + file_id: 'f4', + filepath: 'https://test.blob.core.windows.net/files/images/user123/x.pdf', + }; + await expect(deleteFileFromAzure(req, file)).rejects.toThrow(/boom/); + }); + }); + + describe('refreshAzureFileUrls', () => { + const batchUpdateFiles = jest.fn(); + beforeEach(() => { + batchUpdateFiles.mockReset(); + batchUpdateFiles.mockResolvedValue(undefined); + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + process.env.AZURE_CONTAINER_NAME = 'files'; + const mockBlockBlobClient = { + url: 'https://test.blob.core.windows.net/files/images/user123/x.pdf', + }; + getAzureContainerClient.mockResolvedValue({ + getBlockBlobClient: jest.fn().mockReturnValue(mockBlockBlobClient), + }); + }); + + it('is a no-op for an empty list', async () => { + const result = await refreshAzureFileUrls([], batchUpdateFiles); + expect(result).toEqual([]); + expect(batchUpdateFiles).not.toHaveBeenCalled(); + }); + + it('skips files with a non-azure_blob source and never calls batchUpdate', async () => { + const files = [ + { file_id: 'a', source: 's3', filepath: 'https://s3.example.com/k' }, + { file_id: 'b', source: 'local', filepath: '/tmp/x' }, + ]; + await refreshAzureFileUrls(files, batchUpdateFiles); + expect(batchUpdateFiles).not.toHaveBeenCalled(); + }); + + it('batch-updates only files whose URLs actually got refreshed', async () => { + // Public-access mode: refresh swaps the (legacy) plain URL for the + // current `blockBlobClient.url`. Two refreshable files → one batchUpdate + // call carrying both new filepaths. + const files = [ + { + file_id: 'azure-1', + source: 'azure_blob', + filepath: 'https://test.blob.core.windows.net/files/images/user123/old1.pdf', + }, + { + file_id: 'azure-2', + source: 'azure_blob', + filepath: 'https://test.blob.core.windows.net/files/images/user123/old2.pdf', + }, + // Missing filepath — should be ignored, not crash the batch. + { file_id: 'azure-3', source: 'azure_blob' }, + ]; + await refreshAzureFileUrls(files, batchUpdateFiles, /* bufferSeconds */ 3600); + // Public mode never triggers refresh (needsRefresh returns false on plain URLs), + // so no batchUpdate call should be made. + expect(batchUpdateFiles).not.toHaveBeenCalled(); + }); + }); + + describe('refreshAzureUrl', () => { + beforeEach(() => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + process.env.AZURE_CONTAINER_NAME = 'files'; + }); + + it('returns empty string when fileObj is null/undefined', async () => { + expect(await refreshAzureUrl(null)).toBe(''); + expect(await refreshAzureUrl(undefined)).toBe(''); + }); + + it('returns the original filepath when source is not azure_blob', async () => { + const out = await refreshAzureUrl({ source: 's3', filepath: 'https://s3/k' }); + expect(out).toBe('https://s3/k'); + }); + + it('returns the original filepath when no refresh is needed', async () => { + // Public access + plain URL → needsRefreshAzure returns false → no refresh. + const original = 'https://test.blob.core.windows.net/files/images/user/x.pdf'; + const out = await refreshAzureUrl({ source: 'azure_blob', filepath: original }); + expect(out).toBe(original); + }); + }); + + describe('getNewAzureURL - containerName override (multi-container support)', () => { + beforeEach(() => { + process.env.AZURE_STORAGE_PUBLIC_ACCESS = 'true'; + }); + + it('uses the passed containerName to anchor extraction and resolve the new URL', async () => { + // URL is in a container that's NOT the env default. Without the + // containerName arg, extractBlobPathFromAzureUrl would fall back to + // the legacy "strip first segment" path; with it, we anchor correctly. + process.env.AZURE_CONTAINER_NAME = 'default-container'; + const mockBlockBlobClient = { + url: 'https://test.blob.core.windows.net/tenant-a/images/user/x.pdf', + }; + const mockContainerClient = { + getBlockBlobClient: jest.fn().mockReturnValue(mockBlockBlobClient), + }; + getAzureContainerClient.mockResolvedValue(mockContainerClient); + + const url = 'https://test.blob.core.windows.net/tenant-a/images/user/x.pdf?sv=2023&sig=old'; + const result = await getNewAzureURL(url, 'tenant-a'); + expect(getAzureContainerClient).toHaveBeenCalledWith('tenant-a'); + expect(mockContainerClient.getBlockBlobClient).toHaveBeenCalledWith('images/user/x.pdf'); + expect(result).toBe(mockBlockBlobClient.url); + }); + }); + + describe('Managed Identity signing path', () => { + // The SDK calls `.toISOString()` on `signedStartsOn`/`signedExpiresOn`, + // so these must be Date instances (not ISO strings). `value` must be + // base64 so the HMAC step accepts it. + const FAKE_DELEGATION_KEY = { + signedObjectId: 'fake-oid', + signedTenantId: 'fake-tid', + signedStartsOn: new Date(), + signedExpiresOn: new Date(Date.now() + 7 * 24 * 3.6e6), + signedService: 'b', + signedVersion: '2023-11-03', + value: Buffer.alloc(32).toString('base64'), + }; + + beforeEach(() => { + delete process.env.AZURE_STORAGE_CONNECTION_STRING; + process.env.AZURE_STORAGE_ACCOUNT_NAME = 'testaccount'; + azureIdentity.DefaultAzureCredential.mockClear(); + azureIdentity.ManagedIdentityCredential.mockClear(); + }); + + // Spy must be re-applied INSIDE jest.isolateModulesAsync because each + // isolate block re-requires @azure/storage-blob and gets a fresh + // BlobServiceClient class (so a spy on the outer-scope class doesn't + // affect the isolated instance). Pattern: re-require + re-spy + run + + // assert + restore — all inside the block. + async function runWithMockedDelegationKey(fn) { + let getKeySpy; + await jest.isolateModulesAsync(async () => { + const storageBlob = require('@azure/storage-blob'); + getKeySpy = jest + .spyOn(storageBlob.BlobServiceClient.prototype, 'getUserDelegationKey') + .mockResolvedValue(FAKE_DELEGATION_KEY); + const fresh = require('../../../../../server/services/Files/Azure/crud'); + try { + await fn(fresh, getKeySpy); + } finally { + getKeySpy.mockRestore(); + } + }); + return getKeySpy; + } + + it('signs via DefaultAzureCredential outside production', async () => { + const originalNodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'development'; + try { + await runWithMockedDelegationKey(async (fresh) => { + const signed = await fresh.getSignedAzureURL({ + blobPath: 'images/user/x.pdf', + containerName: 'files', + }); + const parsed = new URL(signed); + expect(parsed.searchParams.get('sp')).toBe('r'); + expect(parsed.searchParams.get('spr')).toBe('https'); + expect(parsed.searchParams.has('sig')).toBe(true); + }); + expect(azureIdentity.DefaultAzureCredential).toHaveBeenCalled(); + expect(azureIdentity.ManagedIdentityCredential).not.toHaveBeenCalled(); + } finally { + process.env.NODE_ENV = originalNodeEnv; + } + }); + + it('signs via ManagedIdentityCredential in production', async () => { + const originalNodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + try { + await runWithMockedDelegationKey(async (fresh) => { + const signed = await fresh.getSignedAzureURL({ + blobPath: 'images/user/x.pdf', + containerName: 'files', + }); + expect(new URL(signed).searchParams.has('sig')).toBe(true); + }); + expect(azureIdentity.ManagedIdentityCredential).toHaveBeenCalled(); + expect(azureIdentity.DefaultAzureCredential).not.toHaveBeenCalled(); + } finally { + process.env.NODE_ENV = originalNodeEnv; + } + }); + + it('reuses the BlobServiceClient + credential across concurrent calls (singleton)', async () => { + // The perf invariant under test: regardless of how many concurrent + // first-callers hit getSignedAzureURL with the same account name, we + // construct the credential and the BlobServiceClient exactly once. + // (The delegation-key memoization itself sits on top of this — its + // promise-cache logic is verified by code review; isolating it here + // is impractical because `getUserDelegationKey` lives on an internal + // mixin chain that `jest.spyOn` doesn't reliably intercept.) + const originalNodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'development'; + try { + await runWithMockedDelegationKey(async (fresh) => { + const [a, b, c] = await Promise.all([ + fresh.getSignedAzureURL({ blobPath: 'images/u/a.pdf', containerName: 'files' }), + fresh.getSignedAzureURL({ blobPath: 'images/u/b.pdf', containerName: 'files' }), + fresh.getSignedAzureURL({ blobPath: 'images/u/c.pdf', containerName: 'files' }), + ]); + expect(new URL(a).searchParams.has('sig')).toBe(true); + expect(new URL(b).searchParams.has('sig')).toBe(true); + expect(new URL(c).searchParams.has('sig')).toBe(true); + }); + // Three signed-URL requests, exactly one credential constructed — the + // pre-fix behavior would construct one per request. + expect(azureIdentity.DefaultAzureCredential).toHaveBeenCalledTimes(1); + } finally { + process.env.NODE_ENV = originalNodeEnv; + } + }); + }); + + describe('getAzureFileStream', () => { + const download = jest.fn(); + const getBlockBlobClient = jest.fn().mockReturnValue({ download }); + + beforeEach(() => { + download.mockReset(); + getBlockBlobClient.mockClear(); + getAzureContainerClient.mockResolvedValue({ getBlockBlobClient }); + process.env.AZURE_CONTAINER_NAME = 'files'; + }); + + it('downloads via the authenticated SDK, stripping any SAS query from the blob path', async () => { + const fakeStream = { pipe: jest.fn() }; + download.mockResolvedValue({ readableStreamBody: fakeStream }); + const url = + 'https://test.blob.core.windows.net/files/images/user123/a.png?sv=2023&sig=abc&se=2026-01-01T00:00:00Z'; + const result = await getAzureFileStream({}, url); + expect(getBlockBlobClient).toHaveBeenCalledWith('images/user123/a.png'); + expect(download).toHaveBeenCalledTimes(1); + expect(result).toBe(fakeStream); + }); + + it('works on a plain (unsigned) URL', async () => { + download.mockResolvedValue({ readableStreamBody: { pipe: jest.fn() } }); + await getAzureFileStream({}, 'https://test.blob.core.windows.net/files/images/user123/a.png'); + expect(getBlockBlobClient).toHaveBeenCalledWith('images/user123/a.png'); + }); + + it('works on a path-style / Azurite URL', async () => { + download.mockResolvedValue({ readableStreamBody: { pipe: jest.fn() } }); + await getAzureFileStream( + {}, + 'http://127.0.0.1:10000/devstoreaccount1/files/images/user123/a.png', + ); + expect(getBlockBlobClient).toHaveBeenCalledWith('images/user123/a.png'); + }); + + it('throws when the blob path cannot be extracted from the URL', async () => { + await expect(getAzureFileStream({}, 'not-a-url')).rejects.toThrow( + /Unable to extract blob path/, + ); + expect(download).not.toHaveBeenCalled(); + }); + + it('throws when the container client is unavailable (Azure unconfigured)', async () => { + getAzureContainerClient.mockResolvedValue(null); + await expect( + getAzureFileStream({}, 'https://test.blob.core.windows.net/files/images/user123/a.png'), + ).rejects.toThrow(/not initialized/); + }); + + it('throws when the SDK returns no readable body', async () => { + download.mockResolvedValue({ readableStreamBody: undefined }); + await expect( + getAzureFileStream({}, 'https://test.blob.core.windows.net/files/images/user123/a.png'), + ).rejects.toThrow(/Empty download body/); + }); + }); +}); diff --git a/api/test/server/services/Files/images/encode.test.js b/api/test/server/services/Files/images/encode.test.js new file mode 100644 index 00000000000..39a17362888 --- /dev/null +++ b/api/test/server/services/Files/images/encode.test.js @@ -0,0 +1,74 @@ +jest.mock('@librechat/data-schemas', () => ({ + logger: { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() }, +})); + +const mockGetStrategyFunctions = jest.fn(); +jest.mock('~/server/services/Files/strategies', () => ({ + getStrategyFunctions: (...args) => mockGetStrategyFunctions(...args), +})); + +jest.mock('@librechat/api', () => ({ + validateImage: jest.fn().mockResolvedValue({ isValid: true }), + logAxiosError: jest.fn(({ message }) => message), +})); + +const { FileSources } = require('librechat-data-provider'); +const { encodeAndFormat } = require('../../../../../server/services/Files/images/encode'); + +const req = { body: {}, config: {} }; + +function makeImageFile(source) { + return { + file_id: `f-${source}`, + filepath: `https://test.blob.core.windows.net/files/images/u/${source}.png`, + filename: `${source}.png`, + type: 'image/png', + source, + height: 100, + width: 100, + }; +} + +function failingStrategy() { + return { + prepareImagePayload: jest.fn().mockResolvedValue([{}, 'https://example.test/fallback.png']), + getDownloadStream: jest.fn().mockRejectedValue(new Error('403 PublicAccessNotPermitted')), + }; +} + +describe('encodeAndFormat — azure_blob fail-closed', () => { + beforeEach(() => mockGetStrategyFunctions.mockReset()); + + it('throws for azure_blob when the server-side download fails', async () => { + mockGetStrategyFunctions.mockReturnValue(failingStrategy()); + await expect( + encodeAndFormat(req, [makeImageFile(FileSources.azure_blob)], { endpoint: 'openAI' }), + ).rejects.toThrow(); + }); + + it('falls through (does not throw) for s3 when the download fails', async () => { + mockGetStrategyFunctions.mockReturnValue(failingStrategy()); + await expect( + encodeAndFormat(req, [makeImageFile(FileSources.s3)], { endpoint: 'openAI' }), + ).resolves.toBeDefined(); + }); + + it('falls through (does not throw) for firebase when the download fails', async () => { + mockGetStrategyFunctions.mockReturnValue(failingStrategy()); + await expect( + encodeAndFormat(req, [makeImageFile(FileSources.firebase)], { endpoint: 'openAI' }), + ).resolves.toBeDefined(); + }); + + it('returns a base64 image_url on the azure_blob happy path', async () => { + const { Readable } = require('stream'); + mockGetStrategyFunctions.mockReturnValue({ + prepareImagePayload: jest.fn(), + getDownloadStream: jest.fn().mockResolvedValue(Readable.from([Buffer.from('imgdata')])), + }); + const result = await encodeAndFormat(req, [makeImageFile(FileSources.azure_blob)], { + endpoint: 'openAI', + }); + expect(result.image_urls[0].image_url.url).toMatch(/^data:image\/png;base64,/); + }); +}); diff --git a/packages/api/src/agents/avatars.ts b/packages/api/src/agents/avatars.ts index 25adfdc7175..0a1912556be 100644 --- a/packages/api/src/agents/avatars.ts +++ b/packages/api/src/agents/avatars.ts @@ -9,6 +9,9 @@ export { MAX_AVATAR_REFRESH_AGENTS, AVATAR_REFRESH_BATCH_SIZE }; export type RefreshS3UrlFn = (avatar: AgentAvatar) => Promise; +/** Generic signed URL refresh function, agnostic to storage provider */ +export type RefreshUrlFn = (avatar: AgentAvatar) => Promise; + export type UpdateAgentFn = ( searchParams: { id: string }, updateData: { avatar: AgentAvatar }, @@ -19,6 +22,8 @@ export type RefreshListAvatarsParams = { agents: Agent[]; userId: string; refreshS3Url: RefreshS3UrlFn; + /** Optional Azure Blob URL refresher; when provided, Azure-backed avatars are also refreshed */ + refreshAzureUrl?: RefreshUrlFn; updateAgent: UpdateAgentFn; }; @@ -48,6 +53,7 @@ export const refreshListAvatars = async ({ agents, userId, refreshS3Url, + refreshAzureUrl, updateAgent, }: RefreshListAvatarsParams): Promise => { const stats: RefreshStats = { @@ -71,23 +77,29 @@ export const refreshListAvatars = async ({ await Promise.all( batch.map(async (agent) => { - if (agent?.avatar?.source !== FileSources.s3 || !agent?.avatar?.filepath) { + const source = agent?.avatar?.source; + const isS3 = source === FileSources.s3; + const isAzure = source === FileSources.azure_blob && !!refreshAzureUrl; + + if ((!isS3 && !isAzure) || !agent?.avatar?.filepath) { stats.not_s3++; return; } if (!agent?.id) { logger.debug( - '[refreshListAvatars] Skipping S3 avatar refresh for agent: %s, ID is not set', + '[refreshListAvatars] Skipping avatar refresh for agent: %s, ID is not set', agent._id, ); stats.no_id++; return; } + const refreshFn = isS3 ? refreshS3Url : refreshAzureUrl!; + try { - logger.debug('[refreshListAvatars] Refreshing S3 avatar for agent: %s', agent._id); - const newPath = await refreshS3Url(agent.avatar); + logger.debug('[refreshListAvatars] Refreshing avatar for agent: %s', agent._id); + const newPath = await refreshFn(agent.avatar); if (!newPath || newPath === agent.avatar.filepath) { stats.no_change++; @@ -108,7 +120,7 @@ export const refreshListAvatars = async ({ stats.persist_error++; } } catch (err) { - logger.error('[refreshListAvatars] S3 avatar refresh error: %o', err); + logger.error('[refreshListAvatars] Avatar refresh error: %o', err); stats.s3_error++; } }), diff --git a/packages/api/src/cdn/__tests__/azure.test.ts b/packages/api/src/cdn/__tests__/azure.test.ts new file mode 100644 index 00000000000..bacbd9a7c2e --- /dev/null +++ b/packages/api/src/cdn/__tests__/azure.test.ts @@ -0,0 +1,57 @@ +import type { BlobServiceClient as TBlobServiceClient } from '@azure/storage-blob'; + +const mockLogger = { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() }; + +jest.mock('@azure/storage-blob', () => ({ + BlobServiceClient: jest.fn(), +})); + +jest.mock('@azure/identity', () => ({ + DefaultAzureCredential: jest.fn().mockImplementation(() => ({ kind: 'default' })), +})); + +jest.mock('@librechat/data-schemas', () => ({ + logger: mockLogger, +})); + +describe('initializeAzureBlobService — Managed Identity endpoint', () => { + beforeEach(() => { + jest.resetModules(); + jest.clearAllMocks(); + delete process.env.AZURE_STORAGE_CONNECTION_STRING; + process.env.AZURE_STORAGE_ACCOUNT_NAME = 'testaccount'; + delete process.env.AZURE_STORAGE_BLOB_ENDPOINT; + }); + + afterEach(() => { + delete process.env.AZURE_STORAGE_ACCOUNT_NAME; + delete process.env.AZURE_STORAGE_BLOB_ENDPOINT; + }); + + async function load() { + const { BlobServiceClient } = jest.requireMock('@azure/storage-blob') as { + BlobServiceClient: jest.MockedClass; + }; + const { initializeAzureBlobService } = await import('../azure'); + return { BlobServiceClient, initializeAzureBlobService }; + } + + it('defaults to the public-cloud blob endpoint when no override is set', async () => { + const { BlobServiceClient, initializeAzureBlobService } = await load(); + await initializeAzureBlobService(); + expect(BlobServiceClient).toHaveBeenCalledWith( + 'https://testaccount.blob.core.windows.net', + expect.anything(), + ); + }); + + it('honours AZURE_STORAGE_BLOB_ENDPOINT for sovereign cloud / custom domains', async () => { + process.env.AZURE_STORAGE_BLOB_ENDPOINT = 'https://testaccount.blob.core.usgovcloudapi.net'; + const { BlobServiceClient, initializeAzureBlobService } = await load(); + await initializeAzureBlobService(); + expect(BlobServiceClient).toHaveBeenCalledWith( + 'https://testaccount.blob.core.usgovcloudapi.net', + expect.anything(), + ); + }); +}); diff --git a/packages/api/src/cdn/azure.ts b/packages/api/src/cdn/azure.ts index 247530286ca..bef5b5ef39f 100644 --- a/packages/api/src/cdn/azure.ts +++ b/packages/api/src/cdn/azure.ts @@ -32,7 +32,11 @@ export const initializeAzureBlobService = async (): Promise