Repository navigation
🛡️ feat: Azure Blob Storage URL Signing and Refresh Support - #13245
Open
JorgeCosta87 wants to merge 9 commits into
Open
JorgeCosta87 wants to merge 9 commits into
JorgeCosta87 wants to merge 9 commits into
Conversation
Adds time-limited signed (SAS) URLs and URL refresh for the Azure Blob storage strategy, achieving feature parity with the existing S3 implementation and addressing the security concerns around public Azure Blob containers raised in discussion LibreChat-AI#6704. What this brings - `getSignedAzureURL` returns a time-limited SAS URL for a blob, selecting 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 (no long-lived keys in the cluster) - `needsRefreshAzure`, `getNewAzureURL`, `refreshAzureFileUrls`, and `refreshAzureUrl` mirror the S3 functions: detect URLs about to expire, generate fresh ones, and persist them. - `AZURE_EXPIRY_INTERVAL` cache key in `api/cache/getLogStores.js` and `packages/data-provider/src/config.ts` (mirrors S3_EXPIRY_INTERVAL). - `getFileURLRefreshCacheTime` in `api/server/utils/getFileStrategy.js` shares the half-TTL refresh-cache logic across S3 and Azure. - Avatar URL refresh routed via DI maps in UserController.js (avatarNeedsRefreshBySource / getNewAvatarUrlBySource) and agents/v1.js (urlRefreshersBySource), so adding another storage provider only requires another map entry. - `refreshListAvatars` (packages/api/src/agents/avatars.ts) accepts an optional `refreshAzureUrl` alongside `refreshS3Url`. New env vars (documented in .env.example via a follow-up): - `AZURE_STORAGE_ACCOUNT_NAME` — enables Managed Identity / User Delegation SAS (no connection string required) - `AZURE_URL_EXPIRY_SECONDS` — TTL for SAS tokens (default 2 minutes) Based on the work originally proposed in LibreChat-AI#11137 by @adamfisher. This commit carries his patches forward onto a fresh upstream/main; further hardening commits follow. Co-authored-by: Jorge Costa <8352477+JorgeCosta87@users.noreply.github.com>
Refinements on top of LibreChat-AI#11137: - Flip AZURE_STORAGE_PUBLIC_ACCESS default 'true' -> 'false' so a missing/empty env var no longer silently opens public containers. - Read AZURE_STORAGE_PUBLIC_ACCESS via process.env at call time instead of capturing it in a module-load destructure constant, so runtime env changes and tests that toggle the flag in beforeEach are honored. - Pin SAS tokens to SASProtocol.Https on both signing paths (Account Key and User Delegation) so signed URLs cannot be replayed over HTTP. - Replace the silent unsigned-URL fallback when the connection string is missing AccountName/AccountKey with a thrown error, so misconfigured deployments fail loud instead of leaking unsigned URLs. - Bump the default URL expiry from 120s to 300s (5 min) in crud.js and in getFileURLRefreshCacheTime; 120s is shorter than typical large-PDF load times on corporate networks and forces a refresh round-trip on every preview. - Drop the redundant `await import('@azure/storage-blob')` since the module is already statically required two lines above; reuse the single import everywhere. - Tidy a redundant `process.env.AZURE_REFRESH_EXPIRY_MS !== null && X` env check (process.env values are string | undefined, never null). Tests: add coverage for the new behavior — throws when neither connection string nor account name is configured, throws (rather than returning an unsigned URL) when the connection string parses but lacks AccountName/AccountKey, SAS tokens include spr=https and sp=r, and the default expiry is 300s when AZURE_URL_EXPIRY_SECONDS is unset. Also mock @librechat/data-schemas in the test to keep the suite independent of the workspace's winston format mock.
Heavy-review pass found three S3-vs-Azure parity gaps that would bite
once private containers are enabled:
1) getAzureURL was returning an unsigned blob URL even when private
access was configured. It is exposed via the strategy interface as
getFileURL and its return value is persisted as `filepath` by
processFileURL, so URL-based uploads (avatars, image-generation
outputs) ended up with unsigned filepaths that 401'd until the next
refresh cycle. S3's getS3URL always signs; Azure now matches.
2) deleteFileFromAzure parsed the blob path with
`file.filepath.split(`${AZURE_CONTAINER_NAME}/`)[1]`. Once filepaths
carry SAS query strings, this concatenates the query string into the
blob name and the SDK 404s. Also breaks for path-style URLs (Azurite
emulator). Switched to `extractBlobPathFromAzureUrl`, which parses
with `new URL()` so the SAS is discarded and the container is
matched as a path segment.
3) extractBlobPathFromAzureUrl itself was using a blind "drop the first
segment" extraction, which for Azurite-style URLs
(`<host>/<account>/<container>/<path>`) silently leaked the
container name into the blob path. Anchored extraction on the
configured container name with a logged fallback to the legacy
behavior for URLs stored under a previous container name. Mirrors
the spirit of S3 PR LibreChat-AI#11894 (path-style URL support for MinIO/R2).
Also added a getNewAzureURL guard mirroring getNewS3URL's
`keyParts.length < 3` check: blob paths shallower than
basePath/userId/fileName are malformed and shouldn't be re-signed.
Tests: 6 new cases covering path-style extraction, custom container
override, legacy-container fallback, getNewAzureURL guard, and that
getAzureURL signs when private / returns plain URL when public.
24/24 azure crud tests pass.
Follow-up improvements on top of the SAS signing + refresh chain
already on this branch. All correctness and observability — no
behavior change on the happy path.
crud.js source changes:
- needsRefreshAzure now detects "is this a SAS URL" via `sig` (the
cryptographic signature) instead of `se` (just the expiry). `se`
alone doesn't prove a URL was signed; mirrors S3's
`X-Amz-Signature` check.
- Hoist `BlobServiceClient` + credential to module-level singletons
on the Managed Identity path via a new `getManagedIdentityBlobServ
iceClient(accountName)` helper. Recreating them per request
cold-started the credential's AAD token cache every time — a real
perf hotspot on any flow that signs many URLs (file list, agent
list refresh). Cache is keyed on account name and invalidates the
delegation-key cache on account change.
- Tighten the credential chain: `ManagedIdentityCredential`
explicitly in production (NODE_ENV === 'production');
`DefaultAzureCredential` kept for dev so local Azure CLI / VS
Code identities still work. Production no longer silently falls
through to whichever identity DefaultAzureCredential's chain
happens to find.
- `getUserDelegationKey` now memoises the in-flight Promise rather
than the resolved key. The old resolved-value cache had a benign
race where N concurrent first-callers each fired their own AAD
round-trip; with promise-caching they all await the same fetch.
Rejected promises are dropped from the cache so the next caller
retries.
- `getNewAzureURL(currentURL, containerName)` accepts an optional
containerName and threads it through extractBlobPathFromAzureUrl,
getSignedAzureURL, and getAzureContainerClient. Refreshes in
multi-container deployments no longer silently rewrite URLs into
the env-default container's namespace.
- Convert `await import('@azure/identity')` to a top-of-file
`require()`. The dynamic import was a leftover from when the
package was treated as optional; it's a declared dep now, and
the require form avoids Jest's experimental-vm-modules
requirement for tests.
- Drop the inner try/catch in the MI branch of getSignedAzureURL.
The outer catch already logs + rethrows; the inner one was
double-logging the same error.
- Restore `// Use default concurrency (5)` / `// Use default buffer
size (8MB)` inline comments on uploadStream that were dropped.
- Add URL/file_id context to needsRefreshAzure error log and
refreshAzureUrl debug + error logs — both useful when debugging
a real outage from logs alone.
- isNaN → Number.isNaN in the two env-var parsers.
Test coverage expanded from the prior set to 38 cases:
- deleteFileFromAzure: happy path strips SAS from filepath, throws
on user-id mismatch, swallows 404, rethrows non-404.
- refreshAzureFileUrls: no-op on empty input, skips non-azure
files, malformed entries don't crash the batch.
- refreshAzureUrl: null/undefined fileObj, non-azure source
passthrough, no-refresh passthrough.
- getNewAzureURL: containerName override (multi-container support).
- Managed Identity signing path: DefaultAzureCredential outside
production, ManagedIdentityCredential in production, and
BlobServiceClient + credential reused across concurrent calls
(the singleton invariant — exactly one credential constructed
for N concurrent first-callers).
@azure/identity is mocked at file scope so the MI path can be
exercised without trying to reach IMDS/AAD; the BlobServiceClient
constructor itself is real (no network calls — only
getUserDelegationKey is, which the tests spy on).
Two real bugs surfaced in code review on the fork side that need mirroring here so the merge is conflict-free for downstream consumers. 1) UserController could persist `undefined` into `user.avatar`. User avatars are plain strings with no `source` field. The refresh chain has no way to tell an Azure URL from a Google/OAuth provider URL or a legacy `/uploads/` path. `needsRefreshAzure` returned `true` for any non-SAS URL in private mode (the public→private transition branch), `getNewAzureURL` then returned `undefined`, and `db.updateUser` overwrote the working avatar with that `undefined`. Two-layer fix: - `crud.js` — new `isAzureBlobUrl(parsedUrl)` host-shape check (`*.blob.*`, sovereign clouds, Azurite loopback:10000). `needsRefreshAzure` calls it first and returns `false` for non-Azure URLs, short-circuiting the refresh chain. Mirrors how S3's `needsRefresh` keys on `X-Amz-Signature` — only refresh URLs we own. - `UserController.js` — defensive guard: when `getNewAvatarUrl` returns a falsy value, keep the existing avatar (don't persist). Belt-and-suspenders in case a future provider's refresh helper returns `undefined` for a different reason. 2) Agent-list avatar-refresh cache outlived Azure's URL TTL. `getListAgentsHandler` cached Azure avatar URLs in the `S3_EXPIRY_INTERVAL` namespace with `Time.THIRTY_MINUTES` hardcoded. With Azure's default 5-minute URL TTL, cached entries served expired SAS URLs for ~25 minutes of every 30-minute cache window. The avatar-upload handler's cache-invalidate call had the same problem — it only deleted from the S3 namespace, so a fresh upload on the Azure path didn't invalidate the stale Azure entry. Fix: - New `avatarRefreshCacheKeyByStrategy` map (next to `urlRefreshersBySource`) selects the cache namespace by storage source: `S3_EXPIRY_INTERVAL` for S3, `AZURE_EXPIRY_INTERVAL` for Azure. - `getListAgentsHandler` reads `req.config?.fileStrategy`, uses the per-strategy cache namespace, and uses `getFileURLRefreshCacheTime(strategy)` for the cache entry TTL (half the URL TTL, floored at 1 minute — 150s for Azure default). - The avatar-upload handler's cache-invalidate uses the same per-strategy key. Tests (38 → 47 cases): - `needsRefreshAzure` non-Azure URLs (Google/OAuth, Microsoft Graph, legacy /uploads/, S3 stragglers) → `false`. - `needsRefreshAzure` Azure + Azurite URLs in private mode → `true` (public→private transition path preserved). - `isAzureBlobUrl` host-shape coverage: standard cloud, sovereign clouds (US Gov, China), modern *.blob.storage.azure.net, Azurite loopback + port 10000, rejects non-Azure hostnames.
`batchUpdateFiles` is shared between `refreshS3FileUrls` and the new `refreshAzureFileUrls` from this PR, but the post-bulk-write log was hardcoded to "Updated N files with new S3 URLs" — misleading on the Azure path. Strategy-neutral wording. Pure log-text change, no behavior impact.
Contributor
Author
|
Hey @danny-avila, could you take a look at this? |
Collaborator
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7428559d38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+129
to
+132
| await updateUser(userId, { avatar: downloadURL }); | ||
| } | ||
|
|
||
| return url; | ||
| return downloadURL; |
There was a problem hiding this comment.
Manual avatar protection depends on the ?manual=true marker (see handleExistingUser in api/strategies/process.js, plus the OpenID/SAML strategy checks), but processAzureAvatar now saves and returns downloadURL without that marker. For users on Azure storage, a manually uploaded avatar can therefore be treated as non-manual and overwritten on the next social-login/profile-sync flow. This is a regression in user-visible behavior and should preserve an equivalent manual marker while handling pre-existing query params safely.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed in 68cb964c7. Restored manual=true on the avatar URL via URL.searchParams.set('manual', 'true') — the SAS query string is preserved and the social-login strategies still see the marker.
Comment on lines
+509
to
+510
| if (!isAzureBlobUrl(url)) { | ||
| return false; |
There was a problem hiding this comment.
needsRefreshAzure exits early for any URL whose hostname does not contain .blob. (or localhost:10000), so Azure Blob URLs served via custom domains will never refresh even if they contain SAS tokens and expire. Since refreshAzureUrl/refreshAzureFileUrls rely on this predicate, deployments using mapped custom blob domains can get permanently stale file/avatar URLs until another write path updates them.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed in 68cb964c7. Added an optional AZURE_STORAGE_BLOB_ENDPOINT env var; when set, URLs whose hostname matches it are treated as Azure Blob URLs by isAzureBlobUrl.
Comment on lines
+70
to
+77
| const expirySeconds = parseInt(process.env.S3_URL_EXPIRY_SECONDS, 10) || 120; | ||
| // 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 expirySeconds = parseInt(process.env.AZURE_URL_EXPIRY_SECONDS, 10) || 300; | ||
| // Check for refresh at half the URL expiry time: (expirySeconds * 1000ms) / 2 => expirySeconds * 500 | ||
| return Math.max(expirySeconds * 500, Time.ONE_MINUTE); |
There was a problem hiding this comment.
getFileURLRefreshCacheTime uses raw env values (S3_URL_EXPIRY_SECONDS/AZURE_URL_EXPIRY_SECONDS) without the same max bound enforced by the signers (both are capped to 7 days), so an oversized env value can make the refresh-check cache live far longer than the actual signed URL validity. In that configuration, /files and /Agents will skip refresh for an extended interval and keep serving expired signed URLs.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed in 68cb964c7. Env input is now clamped to the same 7-day max the signers enforce: Math.min(raw, MAX_SIGNED_URL_TTL_SECONDS).
Comment on lines
1022
to
1023
| if (urlCache && agent?.id && agent?.avatar && urlCache[agent.id]) { | ||
| agent.avatar = { ...agent.avatar, filepath: urlCache[agent.id] }; |
There was a problem hiding this comment.
This cache replay now applies urlCache[agent.id] to any avatar source, but the cache only contains refreshed signed URLs from prior refresh runs. If an agent avatar source changes (for example, to local) after cache creation, list responses can still overwrite it with an old signed URL until cache expiry, returning stale/incorrect avatar links. The earlier source guard prevented this class of stale rewrite.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed in 68cb964c7. Added a source guard — replay only happens when agent.avatar.source is still s3 or azure_blob.
Comment on lines
+85
to
+87
| _miBlobServiceClient = new BlobServiceClient( | ||
| `https://${accountName}.blob.core.windows.net`, | ||
| credential, |
There was a problem hiding this comment.
The Managed Identity signing path always constructs BlobServiceClient with https://${accountName}.blob.core.windows.net, which breaks non-public Azure clouds (for example, Gov/China endpoint suffixes) and any deployment using a non-default blob endpoint. In those environments, SAS refresh/signing fails outright even when AZURE_STORAGE_ACCOUNT_NAME is correctly configured.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed in 68cb964c7. Same AZURE_STORAGE_BLOB_ENDPOINT env var (also used for the isAzureBlobUrl check) overrides the default https://${accountName}.blob.core.windows.net when set.
Five review findings from chatgpt-codex-connector plus the ESLint job:
P1 — Preserve `manual=true` marker in processAzureAvatar
`samlStrategy` (and the other social-login strategies) skip
overwriting an avatar when `user.avatar?.includes('manual=true')`.
Adam's original code appended `?manual=...` to the avatar URL; that
marker was lost when SAS URLs were introduced. Restored, using the
URL API so the existing SAS query string is preserved (so the final
URL carries both `sig=…` and `manual=true`).
P2 — needsRefreshAzure now accepts custom Azure-blob hostnames
`isAzureBlobUrl` only matched `.blob.` and the Azurite emulator
endpoint, so Azure Blob URLs behind a custom CNAME never refreshed.
Added an optional `AZURE_STORAGE_BLOB_ENDPOINT` env var; URLs whose
hostname matches the configured endpoint count as Azure Blob URLs.
P2 — getFileURLRefreshCacheTime clamps env input to the 7-day max
Both signers cap `*_URL_EXPIRY_SECONDS` to 7 days. The
refresh-cache-TTL helper read the raw env value without that clamp,
so an oversized env value made the cache outlive the actual signed
URL — `/files` and `/Agents` skipped refresh and served expired
URLs for the cache window. Same clamp applied here.
P2 — Restrict urlCache replay in agents/v1.js to signed-URL sources
The avatar `urlCache` is populated by refreshListAvatars, which
only writes entries for S3 / Azure-Blob avatars. The replay site
applied the cache entry unconditionally, so an agent whose avatar
source had changed (e.g. to `local`) could be overwritten with a
stale signed URL until cache expiry. Added a source guard.
P2 — Managed Identity signing endpoint configurable
`getManagedIdentityBlobServiceClient` was hardcoded to
`https://${accountName}.blob.core.windows.net` — only valid for
the public cloud. Sovereign clouds (Azure Gov, Azure China) and
custom-domain deployments would fail to sign. Honour the same
`AZURE_STORAGE_BLOB_ENDPOINT` env var (also used by needsRefreshAzure
/ isAzureBlobUrl above) when set; otherwise default to the
public-cloud endpoint as before.
ESLint fixes:
- 3 prettier formatting nits in getLogStores.js, files.js, crud.js
- Unused `Time` import in agents/v1.js — removed (no longer referenced
after the cache TTL refactor)
47/47 unit tests still pass. ESLint on the changed files is clean.
9 of 10 tasks
getAzureFileStream did an unauthenticated axios.get(fileURL), which 403/409s on a private container (anonymous access disabled), breaking vision requests and file preview/download. Mirror getS3FileStream/deleteFileFromAzure: strip any SAS query, derive the blob path via extractBlobPathFromAzureUrl, and download through the authenticated container client (Account Key / Managed Identity). Works whether or not a SAS is present. encodeAndFormat now fails closed for azure_blob — rethrow on a server-side download failure instead of falling through to an unfetchable blob URL. S3/Firebase keep their fall-through. initializeAzureBlobService honours AZURE_STORAGE_BLOB_ENDPOINT for the authenticated client too (matching the signing path) so sovereign-cloud / custom-domain downloads target the right endpoint. Closes LibreChat-AI#13340.
Contributor
Author
|
Pushed a commit adding the authenticated server-side download — the shared backend piece from the discussion on #13411:
This makes the PR close #13340 (private-container vision + preview/download) on top of the SAS signing/refresh. Added a @danny-avila ready for another look. |
Collaborator
|
There are some merge conflicts. Happy to receive some guidance on what the merge order for these Azure Storage PR's should be, too. |
rahepler2
added a commit
to rahepler2/LibreChat
that referenced
this pull request
Jun 5, 2026
Drop AZURE_STORAGE_ENDPOINT_SUFFIX (env var, azure.ts handling, .env.example docs, and its test) to consolidate on LibreChat-AI#13245's AZURE_STORAGE_BLOB_ENDPOINT full-URL override, per review discussion. The backend authenticated download and the frontend ImageFile display proxy are unchanged.
Contributor
|
Thanks @adamfisher and @JorgeCosta87 for the groundwork here — we needed this fixed and built directly on it. We've been running a carried-forward version in production and I've pushed it here: The main difference from this PR is placement: the signing logic lives in
Happy to open this as a PR if useful, or @JorgeCosta87 feel free to pull any of it into this branch — no attachment to which vehicle lands, we'd just like to stop carrying the patch. |
rahepler2
added a commit
to rahepler2/LibreChat
that referenced
this pull request
Aug 19, 2026
Drop AZURE_STORAGE_ENDPOINT_SUFFIX (env var, azure.ts handling, .env.example docs, and its test) to consolidate on LibreChat-AI#13245's AZURE_STORAGE_BLOB_ENDPOINT full-URL override, per review discussion. The backend authenticated download and the frontend ImageFile display proxy are unchanged.
Contributor
|
@danny-avila on your merge-order question from June: four patches now fix the same break (#13245, #13411, #14956, and @mforman's branch). All four contain the same core piece:
If you would rather take one PR only, mforman's branch looks the most complete, and I will close #14956. Happy either way; the goal is to unblock private containers, which #13340 has tracked since May. |
Contributor
|
My patch now sits on top of 0.8.8-rc2 and is running in production without issue |
mforman
added a commit
to mforman/LibreChat
that referenced
this pull request
Sep 23, 2026
Azure Blob storage previously created containers with anonymous public read (`access: 'blob'`) and served raw unsigned blob URLs to the browser and model providers, with the code defaulting AZURE_STORAGE_PUBLIC_ACCESS to true. Setting it to false broke all reads since nothing signed URLs. - New `packages/api/src/storage/azure/`: HTTPS-only, read-only SAS signing via user delegation key (Managed Identity, cached) or account shared key (connection string), URL refresh mirroring S3 (`needsRefreshAzure`, `refreshAzureFileUrls`, `refreshAzureUrl`), authenticated server-side download stream, and `getAzureDownloadURL` for the `?direct=true` path - Legacy unsigned URLs persisted before cutover are detected and re-signed on read (inverse of S3's unsigned-URL semantics) - `AZURE_STORAGE_PUBLIC_ACCESS` now defaults to false in code, matching .env.example; container creation omits public access when private - Source-dispatched refresh (`refreshFileUrl`/`refreshFileUrls`) wired into /files and agent avatar refresh alongside S3 - `cdn/azure.ts`: static imports, cached DefaultAzureCredential, optional AZURE_STORAGE_BLOB_ENDPOINT for sovereign/custom endpoints - Legacy api/server/services/Files/Azure/ deleted; Azure images now use the shared ImageService like S3/CloudFront Continues the work of adamfisher (LibreChat-AI#11137) and JorgeCosta87 (LibreChat-AI#13245). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
codegraph-librechat
Bot
added
the
🗺️ File Storage
rahepler2
added a commit
to rahepler2/LibreChat
that referenced
this pull request
Oct 2, 2026
Drop AZURE_STORAGE_ENDPOINT_SUFFIX (env var, azure.ts handling, .env.example docs, and its test) to consolidate on LibreChat-AI#13245's AZURE_STORAGE_BLOB_ENDPOINT full-URL override, per review discussion. The backend authenticated download and the frontend ImageFile display proxy are unchanged.
codegraph-librechat
Bot
added
🗺️ Server Core
mforman
added a commit
to mforman/LibreChat
that referenced
this pull request
Oct 5, 2026
Azure Blob storage previously created containers with anonymous public read (`access: 'blob'`) and served raw unsigned blob URLs to the browser and model providers, with the code defaulting AZURE_STORAGE_PUBLIC_ACCESS to true. Setting it to false broke all reads since nothing signed URLs. - New `packages/api/src/storage/azure/`: HTTPS-only, read-only SAS signing via user delegation key (Managed Identity, cached) or account shared key (connection string), URL refresh mirroring S3 (`needsRefreshAzure`, `refreshAzureFileUrls`, `refreshAzureUrl`), authenticated server-side download stream, and `getAzureDownloadURL` for the `?direct=true` path - Legacy unsigned URLs persisted before cutover are detected and re-signed on read (inverse of S3's unsigned-URL semantics) - `AZURE_STORAGE_PUBLIC_ACCESS` now defaults to false in code, matching .env.example; container creation omits public access when private - Source-dispatched refresh (`refreshFileUrl`/`refreshFileUrls`) wired into /files and agent avatar refresh alongside S3 - `cdn/azure.ts`: static imports, cached DefaultAzureCredential, optional AZURE_STORAGE_BLOB_ENDPOINT for sovereign/custom endpoints - Legacy api/server/services/Files/Azure/ deleted; Azure images now use the shared ImageService like S3/CloudFront Continues the work of adamfisher (LibreChat-AI#11137) and JorgeCosta87 (LibreChat-AI#13245). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rahepler2
added a commit
to rahepler2/LibreChat
that referenced
this pull request
Oct 6, 2026
Drop AZURE_STORAGE_ENDPOINT_SUFFIX (env var, azure.ts handling, .env.example docs, and its test) to consolidate on LibreChat-AI#13245's AZURE_STORAGE_BLOB_ENDPOINT full-URL override, per review discussion. The backend authenticated download and the frontend ImageFile display proxy are unchanged.
This was referenced Oct 9, 2026
codegraph-librechat
Bot
added
🗺️ Platform Security
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds time-limited signed (SAS) URLs and URL refresh for the Azure Blob storage strategy — feature parity with the existing S3 implementation — and closes the security concerns around public Azure Blob containers raised in discussion #6704.
Why now
Production deployments need private Azure containers. Today's
azure_blobstrategy:AZURE_STORAGE_PUBLIC_ACCESSto'true'in the code (the.env.examplesaysfalsebut a missing env var hits the code-side default), so a misconfigured deploy silently creates world-readable containers.blockBlobClient.urlfrom every helper, so the only working setup is a public container.Without this PR, the only "secure" choice is to put a reverse-proxy in front. With it, S3 and Azure behave the same: TTL'd signed URLs, transparent refresh, and no long-lived storage keys in the cluster when Managed Identity is used.
What this PR brings
Foundation (Adam's work — squashed
feat:commit, authored as Adam Fisher)getSignedAzureURL: generates a time-limited SAS URL for a blob, choosing the signing method by config:AZURE_STORAGE_CONNECTION_STRINGis setAZURE_STORAGE_ACCOUNT_NAMEis set (no connection string / long-lived keys in cluster)needsRefreshAzure,getNewAzureURL,refreshAzureFileUrls,refreshAzureUrl: mirror the S3 functions for detecting near-expiry URLs, generating fresh ones, and persisting them.AZURE_EXPIRY_INTERVALcache key (api/cache/getLogStores.js,packages/data-provider/src/config.ts) — mirrorsS3_EXPIRY_INTERVAL.getFileURLRefreshCacheTimeinapi/server/utils/getFileStrategy.js— shared half-TTL refresh-cache logic for S3 and Azure.UserController.jsandagents/v1.js— adding a new storage provider only requires another map entry. (Addresses @danny-avila's review comment on 6704: Azure Blob Storage URL Signing and Refresh Support #11137.)refreshListAvatars(packages/api/src/agents/avatars.ts) accepts an optionalrefreshAzureUrlalongsiderefreshS3Url.Hardening + S3 parity + post-review polish (three
fix:commits)fix: harden Azure Blob SAS URL signing and flip insecure defaultAZURE_STORAGE_PUBLIC_ACCESSwas destructured with= 'true'at module-load time, so a missing/empty env var silently opened new containers as public. Default is now'false', and the env var is read offprocess.envat call time so tests and runtime toggles are honored.SASProtocol.Httpson every signed URL. Both signing paths pinprotocol: SASProtocol.Https, so a generated SAS can't be replayed over plain HTTP.if (!match || !keyMatch) { logger.warn(...); return blockBlobClient.url; }— a silent unsigned-URL leak. Replaced with a thrownError.getSignedAzureURLdidawait import('@azure/storage-blob')even though the same module is staticallyrequired at the top of the file.process.env.AZURE_REFRESH_EXPIRY_MS !== null && Xcheck (process.envvalues arestring | undefined, nevernull).fix: align Azure Blob refresh parity with S3 and close two filepath bugsHeavy-review pass against
api/server/services/Files/S3/crud.jssurfaced three S3-vs-Azure parity gaps that would bite once private containers are enabled:getAzureURLwas returning an unsigned blob URL even when private access was configured. This is the strategy interface'sgetFileURL— its return is persisted as the file'sfilepathbyprocessFileURL(api/server/services/Files/process.js:252). URL-based uploads (avatars, image-generation outputs) were ending up with unsigned filepaths that 401'd until the next refresh cycle. S3'sgetS3URLalways signs; Azure now matches.deleteFileFromAzureparsed the blob path withfile.filepath.split(${AZURE_CONTAINER_NAME}/)[1]. Once filepaths carry SAS query strings, this concatenates the query string into the blob name and the SDK 404s. Also breaks for path-style URLs (Azurite emulator). Switched toextractBlobPathFromAzureUrl, which parses withnew URL()so the SAS is discarded and the container is matched as a path segment.extractBlobPathFromAzureUrlitself used a blind "drop the first segment" extraction — for path-style URLs (<host>/<account>/<container>/<path>, used by Azurite and some custom blob endpoints), the container name silently leaked into the blob path. Anchored extraction on the configured container name with a logged fallback for legacy URLs. Mirrors the spirit of S3 PR #11894 (MinIO/R2 path-style support).Also added a
getNewAzureURLguard mirroringgetNewS3URL'skeyParts.length < 3check: blob paths shallower thanbasePath/userId/fileNameare malformed and shouldn't be re-signed.fix: tighten Azure SAS signing path and expand test coverage(new)Follow-up from a post-PR critical code review. All correctness and observability — no behavior change on the happy path.
needsRefreshAzurewas keying the "is this a SAS URL" check onsearchParams.has('se')(just the expiry).sealone doesn't prove a URL was signed — onlysigdoes. Now keys onsig, mirroring S3'sX-Amz-Signaturecheck.BlobServiceClient+ credential hoisted to module-level singletons on the Managed Identity path via a newgetManagedIdentityBlobServiceClient(accountName)helper. Recreating them per request cold-started the credential's AAD token cache every time — a real perf hotspot on any flow that signs many URLs (file list, agent list refresh). Cache is keyed on account name and invalidates the delegation-key cache on account change.ManagedIdentityCredentialexplicitly (NODE_ENV === 'production');DefaultAzureCredentialkept for dev so local Azure CLI / VS Code identities still work. Production no longer silently falls through to whichever identityDefaultAzureCredential's chain happens to find.getUserDelegationKeynow memoises the in-flight Promise rather than the resolved key. The old resolved-value cache had a benign race where N concurrent first-callers each fired their own AAD round-trip; with promise-caching they all await the same fetch. Rejected promises are dropped from the cache so the next caller retries.getNewAzureURL(currentURL, containerName)accepts an optionalcontainerNameand threads it throughextractBlobPathFromAzureUrl,getSignedAzureURL, andgetAzureContainerClient. Refreshes in multi-container deployments no longer silently rewrite URLs into the env-default container's namespace.await import('@azure/identity')to a top-of-filerequire(). The dynamic import was a leftover from when the package was treated as optional; it's a declared dep now, and the require form avoids Jest's--experimental-vm-modulesrequirement for tests.getSignedAzureURL(inner catch logged the same error the outer catch already logs).// Use default concurrency (5)/// Use default buffer size (8MB)inline comments onuploadStreamthat were dropped earlier.needsRefreshAzureandrefreshAzureUrllog lines — useful when debugging a real outage from logs alone.isNaN→Number.isNaNin the two env-var parsers.New / changed env vars
AZURE_STORAGE_PUBLIC_ACCESS=trueis still supported (e.g. for legacy CDN-style deployments), but the default is nowfalseand SAS signing kicks in automatically — no breaking change for existing private-container users, only for users who relied on the undocumented default-public behavior.Change Type
Testing
Test coverage:
needsRefreshAzure: SAS-token detection viasig(notse), expired URLs refresh, fresh URLs don't, invalid URLs are treated as needing refresh, public→private transitions work.extractBlobPathFromAzureUrl: virtual-hosted URLs, URLs with SAS tokens, path-style URLs (Azurite-shaped), explicit container override, legacy-container fallback, malformed inputs.getNewAzureURL: returns plain URL when public; returnsundefinedfor malformed blob paths; explicitcontainerNamefor multi-container deployments.getSignedAzureURL: throws when no config; throws (vs silently returning unsigned URL) when conn string lacks AccountName/AccountKey; generated SAS containssig,sp=r,spr=https; default expiry is 300s whenAZURE_URL_EXPIRY_SECONDSis unset.getAzureURL: returns a SAS URL when private; returns plain URL when public.deleteFileFromAzure: extracts blob path stripping any SAS query, throws on user-id mismatch, swallows 404, rethrows non-404.refreshAzureFileUrls: no-op on empty input, skips non-azure files, malformed entries don't crash the batch.refreshAzureUrl: null/undefined fileObj, non-azure source passthrough, no-refresh passthrough.DefaultAzureCredentialoutside production, viaManagedIdentityCredentialin production, and reuses theBlobServiceClient+ credential across concurrent signed-URL requests (the singleton invariant — exactly one credential constructed for N concurrent first-callers).@azure/identityis mocked at file scope so the MI path can be exercised without trying to reach IMDS/AAD; theBlobServiceClientconstructor itself is real (no network calls — onlygetUserDelegationKeyis, which the tests spy on).Also mocks
@librechat/data-schemasin the test file so the suite runs in isolation of the workspace's winston format mock.Test Configuration
v24.x(per.nvmrc)>= 10Commits
Checklist