Manual e2e + DOM inspection on PR #12934: pptx-preview silently
produces empty `.pptx-preview-wrapper` placeholders for pptxgenjs-
generated decks. The library parses the file enough to create the
960×540 host element with a black bg, then fails to populate it.
The outer Promise resolves "successfully" — no throw, no rejection,
the bootstrap thinks rendering succeeded — and the user sees a black
rectangle with no content and no fallback message.
Fix mirrors the DOCX mammoth-fallback pattern from commit 0c0b0ce88:
1. **Server side**: `pptxToHtml` now renders the slide-list body
(`<ol class="lc-pptx-list">...`) via the new `renderPptxSlidesBody`
helper, then embeds it inside the CDN doc via the new
`buildPptxCdnDocument(base64, slideListFallbackBody)` signature.
Combined-doc size budget mirrors the DOCX pattern: if the CDN doc
would exceed `OFFICE_HTML_OUTPUT_CAP` (512 KB), drop to slide-list
only.
2. **Iframe bootstrap**: new `hasRenderedContent()` check after
`wrapSlides()` walks each `.lc-slide-wrap` looking for actual
child content inside pptx-preview's emitted slide nodes. If every
wrap is empty, fires `showFallback('renderer-produced-empty-
wrappers ...')` which reveals the embedded slide-list view
instead of the previous static "Preview unavailable" message.
3. **CSS**: slide-list rules extracted to `PPTX_SLIDE_LIST_CSS`
constant so they can be inlined into both the standalone slide-
list document AND the CDN doc's `<style>` block (CSP `style-src`
is `'unsafe-inline'` only — no external sheets).
`renderPptxSlidesHtml` now delegates to `renderPptxSlidesBody`
wrapped in `wrapAsDocument` — single source of truth for the slide
markup.
Tests (506 passing, +1 vs before): existing `pptxToHtmlViaCdn`
call sites updated for the new fallback-body argument; new
regression test pins `hasRenderedContent`, the
`renderer-produced-empty-wrappers` reason string, the embedded
fallback structure, and the inlined slide-list CSS.
Manual e2e on PR #12934: "Preview unavailable" appears in the iframe
with no way to know what actually failed. The reason was tucked into
the fallback element's `title` attribute (hover-only tooltip) — easy
to miss and impossible to copy/paste.
Now surfaces three ways:
1. Visible inline via a `<details>` element with the reason in
monospace, folded so the friendly message stays primary but the
diagnostic is one click away in the iframe itself.
2. `title` attribute (preserved) for hover tooltip.
3. `console.error('[pptx-preview] fallback fired:', reason)` so
DevTools shows it in red — also the only reliable way to see
the reason if the iframe is detached / re-mounted.
DOCX gets the same console mirror (as `console.warn` since the
fallback there is "high-fidelity unavailable, showing simplified
preview" — informational, not error). The DOCX fallback already
displays the mammoth-rendered content visibly, so no `<details>`
needed there.
Tests: regression assertions pin the diagnostic surfacing — the
`<details>` element, the `title` write, and the `console.error`
call must all be present in the bootstrap.
Manual e2e on PR #12934: every time DevTools is open while viewing a
DOCX or PPTX preview, the console fills with CSP violations like:
Connecting to 'https://cdn.jsdelivr.net/npm/docx-preview@0.3.7/
dist/docx-preview.min.js.map' violates the following Content
Security Policy directive: "connect-src 'none'". The request has
been blocked.
The actual rendering isn't affected (sourcemap fetches happen AFTER
the script has already loaded and executed via `script-src`), but
the noise is enough to make people suspect a real problem and
distracts from useful console output.
Fix: relax `connect-src` from `'none'` to `'self' https://cdn.
jsdelivr.net` in both DOCX and PPTX CDN docs. This allows:
- Same-origin fetches (sandpack-static-server) — covers any
bundler-embedded sourcemaps + same-origin runtime fetches the
renderer might make
- jsdelivr fetches — covers sourcemaps from the CDN where we
loaded the script
Exfiltration risk stays minimal: the iframe is cross-origin to
LibreChat so an attacker can't read application data anyway, and
neither 'self' (sandpack-static-server) nor jsdelivr is a useful
target for exfiltrating slide content to a host the attacker
controls.
Tests updated: assertions for `connect-src 'none'` swapped to
`connect-src 'self' https://cdn.jsdelivr.net` for both DOCX + PPTX
CDN docs. Added negative assertion for wildcard `*` in connect-src
so a future "let's allow everything" rewrite can't widen the
exfiltration surface.
Two fixes for separate manual e2e regressions on PR #12934.
**1. PPTX black screen (single slide rendering as solid black).**
The previous fix removed `body { display: flex }` thinking that was
the sole cause, but the regression persisted. Bisecting against the
last known-good commit (4e2d538b0, width-fit only), the actual culprit
is the COMBINATION of:
- `min-height: 100vh` on html/body
- `availableHeight()` reading viewport-derived dimensions
- `Math.min(sw, sh)` height-aware scale
pptx-preview's CSS injection step interacts unpredictably with
these. Reverting to width-only `scaleFor(nativeW)` and dropping the
viewport min-height restores reliable rendering. Vertical empty
space below short decks now shows the body's bg color (`var(--bg)`)
which still matches the panel theme — that's an acceptable trade-off
vs. the black-screen regression.
Negative assertions added: `Math.min(sw, sh)`, `availableHeight`,
`min-height: 100vh`, `body { display: flex }` must NOT appear in
the bootstrap. So a future "let's fill height" rewrite has to
demonstrate it doesn't break pptx-preview before it can land.
**2. DOCX body text rendering as faint / translucent grey.**
docx-preview emits page-style rendering with white pages and the
docs native text colors. The CDN doc declared
`color-scheme: light dark`, so on OS dark mode the iframes
inheritable `--fg` resolved to `#e5e7eb` (light grey). docx-preview
body text (no explicit color in the source DOCX) inherited that
light-grey on the white page bg → barely-visible "translucent"
rendering.
Fix: declare `color-scheme: light` only in `buildDocxCdnDocument`,
drop the dark-mode `@media` override. docx-preview is a light-mode-
only renderer; matching that produces correct contrast regardless
of OS theme. The mammoth-only `wrapAsDocument` path is unaffected
— it owns its own bg + text colors and continues to respect the
users OS scheme.
New regression test pins the lock: CDN doc must contain
`color-scheme: light`, must NOT contain `color-scheme: light dark`,
must NOT contain `prefers-color-scheme: dark`.
Manual e2e regression on PR #12934: the previous commit added
`body { display: flex; flex-direction: column }` plus
`#lc-render { flex: 1 0 auto }` to fill the panel height. Side effect:
pptx-preview's internal layout assumes block flow on its ancestor
elements; making body a flex container caused slides to render as
solid-black rectangles (sized correctly, but with no visible content
inside).
Fix: keep just `html, body { min-height: 100vh }` for the bg-fill
effect — that alone gives empty space below short decks the dark
theme bg without changing flow. Drop the body-flex and the
`#lc-render { flex: 1 0 auto }` directives.
The height-aware `scaleFor(nativeW, nativeH)` from the same commit
stays — it doesn't interact with pptx-preview's layout, just chooses
a per-slide scale. Each slide still grows to fit the viewport
contain-style.
Negative-assertion added to the regression test: `body { display:
flex }` must NOT appear in the bootstrap, so a future "let's flex
the body to make height work" rewrite can't silently re-introduce
this.
(Note: the user also flagged DOCX theming as faint body text; I'm
leaving that for now per their note that it may be pre-existing.
Not addressed in this commit.)
Manual e2e on PR #12934: PPTX preview filled the panel width but left
empty space below the last slide. DOCX didn't have this issue because
its content (mammoth-rendered HTML) flows naturally and either fits
exactly or overflows; PPTX slides are fixed-aspect 16:9 and don't
grow with the panel.
Two changes:
1. **Body fills the iframe viewport** — `html, body { min-height:
100vh }` plus `body { display: flex; flex-direction: column }`
and `#lc-render { flex: 1 0 auto }`. The dark theme bg now fills
the iframe even when total slide content is shorter than the
panel, so a single-slide deck never reveals a "white below" gap.
2. **Per-slide scale honors viewport height** — `scaleFor(nativeW,
nativeH)` now returns `min(width-fit, height-fit)` (largest
factor that fits without overflowing either dimension). On a
tall artifact panel with a short deck, slides grow up to the
full panel height instead of staying at the width-bound size.
Existing height-fit was always considered correct conceptually
but the previous implementation only used width-fit, leaving
half the viewport unused per slide.
Tests: regression now also asserts `availableHeight()`, the
`Math.min(sw, sh)` formula, and `min-height: 100vh` are in the
bootstrap. Negative assertion for the old `Math.min(1, ...)` upscale
cap remains.
Manual e2e on PR #12934: slides rendered correctly but didn't fill the
artifact panel — whitespace on either side. Two issues:
1. The scale was capped at `Math.min(1, available / SLIDE_W)`. On
panels wider than 960px, the cap clamped the scale to 1.0 and
slides rendered at native size with whitespace on the sides
instead of stretching.
2. The scale was computed against the constant `SLIDE_W = 960`, but
pptx-preview can emit slides whose `offsetWidth` differs from the
init param if the source PPTX has a non-16:9 layout. Per-slide
division of `available / nativeW` handles that case.
Fix: replace `computeScale()` with two helpers — `availableWidth()`
returns the panel content-box width and `scaleFor(nativeW)` returns
the per-slide scale. No upscale cap. The slide content is rendered
by pptx-preview against its 960×540 canvas using vector text /
canvas — scaling up to e.g. 1500px doesn't visibly degrade quality.
Tests: regression now also asserts:
- `availableWidth()` and `scaleFor()` exist by name
- The exact scale formula `availableWidth() / (nativeW || SLIDE_W)`
- Negative assertion that `Math.min(1, ...)` is NOT present, so a
future "let's add an upscale cap" rewrite can't silently
re-introduce the whitespace.
Manual e2e on PR #12934: regenerated PPTX showed "Preview unavailable"
in the iframe. Root cause: the MutationObserver I added in the
previous commit fired during pptx-preview's render and moved slides
out from under the library's references. pptx-preview's async
pipeline raised an unhandled rejection, the iframe's window-level
listener caught it, and the fallback message replaced the partial
render.
Fix: drop the MutationObserver. Apply the wrap+scale ONCE in a
`finalize` step that runs:
- On `previewer.preview().then` (the happy path)
- On the 8-second timeout safety net IF the container has children
(silent-failure path — pptx-preview emitted slides but never
resolved its outer promise)
To prevent the user from seeing an unscaled flash while pptx-preview
renders into the 960px-wide canvas, the container is set to
`visibility: hidden` at init and only revealed inside `finalize`
after wrap+scale completes.
Resize handling stays via `ResizeObserver` on `document.body`,
installed AFTER the wrap pass so it doesn't fire during the wrap
itself.
Tests: regression assertion now also locks in:
- `container.style.visibility = 'hidden' / 'visible'` (the flash-
prevention contract)
- Absence of MutationObserver (the bug we just removed — must NOT
creep back in via a future "let's scale during streaming" idea)
Manual e2e on PR #12934: pptx-preview rendered slides at their native
init dimensions (960×540 default). The artifact panel is much narrower
than that, so the iframe got a horizontal scrollbar and only a corner
of each slide showed at any time — the user had to drag-scroll across
each slide to read it.
Fix: keep pptx-preview's init at 960×540 so its internal layout math
stays correct, then post-process each rendered slide:
- Cache the slide's native width/height on its dataset BEFORE
applying any transform (so subsequent re-fits don't measure the
already-transformed box).
- Wrap the slide in `.lc-slide-wrap` with explicit width/height set
inline to the scaled dimensions; the wrap shrinks the layout space
the slide occupies.
- Apply `transform: scale(panel_width / 960)` to the slide itself
with `transform-origin: top left` so the rendered output shrinks
from the top-left corner into the wrap.
- Cap the scale at 1.0 so small slides don't upscale and get blurry.
Streaming + resize:
- `MutationObserver` watches the container for slide insertions so
streaming renders get scaled on arrival rather than waiting for
the entire `previewer.preview` promise to settle.
- `ResizeObserver` re-fits all wrapped slides when the iframe
resizes (panel drag, window resize).
Tests: new "bootstrap wraps + scales each slide" lock in the wrap
class, scale computation, observer setup, and native-size caching
so a future refactor can't silently re-introduce the overflow.
Two regressions surfaced after the textFormat security gate landed.
1. **Client** (`LogContent.test.tsx` "falls back to the legacy download
branch for an office file with no extracted text"):
When the security gate downgraded an office type without
`textFormat: 'html'` to PLAIN_TEXT, the lenient empty-text gate on
PLAIN_TEXT then accepted a missing `text` field and rendered a
half-empty panel card. The historical contract is "office type +
no text → legacy download UI"; the downgrade should only fire when
there's actual plain text that needs safe-escaping.
Fix in `detectArtifactTypeFromFile`: short-circuit to null when the
office type lands in the security-gate branch with no text. The
PLAIN_TEXT downgrade still fires for legacy attachments that DO
carry plain text.
2. **API** (`process.spec.js` + `process-traversal.spec.js`): the
`@librechat/api` mocks didn't expose `getExtractedTextFormat`, so
`processCodeOutput` called `undefined(...)` → TypeError → tests got
undefined results. Added the helper to both mocks with a faithful
default (returns 'text' for non-null extractor output, null
otherwise).
Tests: new regression in `artifacts.test.ts` pinning the empty-text
+ no-textFormat → null contract for all four office types
(.docx/.csv/.xlsx/.pptx), so a future refactor can't silently
re-introduce the half-empty card.
Per the plan-mode discussion: prove out a LibreOffice subprocess
pipeline as an alternative to the docx-preview / pptx-preview CDN
renderers. LibreOffice handles every office format Microsoft and
LibreOffice itself can open (DOCX, PPTX, XLSX, ODT, ODP, ODS, RTF,
many more), produces a PDF, and the host browser's built-in PDF
viewer renders it inside the Sandpack iframe via a `data:` URI.
No client-side JS dependency, no CDN dependency, true high
fidelity for any feature LibreOffice supports.
Off by default. Operators opt in by setting both:
- `OFFICE_PREVIEW_LIBREOFFICE=true`
- LibreOffice (`soffice` or `libreoffice`) on the server's `$PATH`
When either is missing, the dispatcher falls through to the
existing CDN/mammoth/slide-list pipeline so a misconfiguration
doesn't break previews.
Hardening (`packages/api/src/files/documents/libreoffice.ts`):
- Fresh subprocess per call with isolated temp dir, stripped env
(PATH/HOME/TMPDIR only), and `-env:UserInstallation` so concurrent
conversions can't collide on shared `~/.config/libreoffice` locks
- 30-second wall-time cap; SIGKILL on timeout
- 50 MB PDF output cap to bound disk pressure
- 512 KB output cap on the wrapped HTML so the SSE/cache contract
stays intact (base64 inflates ~33%, effective PDF cap ~380 KB)
- Macros disabled by default flags (`--norestore --invisible
--nodefault --nofirststartwizard --nolockcheck`)
- Tag-distinct `LibreOfficeUnavailableError` /
`LibreOfficeConversionError` so callers can swallow appropriately
Iframe wrapper (`buildPdfEmbedDocument`):
- Native browser PDF viewer via `<iframe src="data:application/pdf;
base64,...">` — works in Chrome, Edge, Safari, Firefox
- CSP locks the iframe to `default-src 'none'; frame-src data:;
connect-src 'none'; script-src 'unsafe-inline'` — no outbound
network, no eval, no external scripts
- `#view=FitH` for first-paint sizing
- 4-second heuristic timer that swaps to a "Preview unavailable"
fallback when the browser's PDF viewer is disabled (kiosk mode,
Brave Shields, etc.)
Wired into `wordDocToHtml` and `pptxToHtml` as the first branch —
returns null when disabled / unavailable / oversized so the existing
pipeline takes over. XLSX intentionally NOT routed through this
path: SheetJS's HTML output is already excellent for spreadsheets
(sortable, sticky headers) and PDF rendering of sheets is awkward.
Tests (`libreoffice.spec.ts`, 30 cases — 25 always run, 5 conditional
on the binary): env-gating parser semantics matching
`OFFICE_PREVIEW_DISABLE_CDN`, fallthrough contract (never throws,
returns null on any failure), CSP lock-down, fallback structure,
binary probe caching + missing-binary path, error tagging, and
integration tests that engage when `soffice`/`libreoffice` is on
PATH (DOCX→PDF, PPTX→PDF, output-cap fallthrough). Integration
tests skip cleanly on bare CI.
Codex P2 review on PR #12934: the CDN-rendered DOCX path always pulled
docx-preview + jszip from cdn.jsdelivr.net. Air-gapped or corporate-
filtered networks where jsdelivr is blocked would degrade to a static
"Preview unavailable" message even though the server already had a
local mammoth renderer that could produce readable output.
Now the dispatcher renders mammoth first and embeds the sanitized
output inside the CDN document as a hidden `#lc-fallback` block. The
iframe's existing `typeof docx === 'undefined'` check (which fires
when the CDN scripts can't load) un-hides the fallback so the user
sees a real preview. CDN-success path is unchanged: high-fidelity
docx-preview output owns the viewport, mammoth fallback stays hidden.
Two new safeguards in the dispatcher:
- Size budget: if base64(binary) + mammoth body + wrapper > 512 KB
(the `attachment.text` cache cap), drop to mammoth-only so a giant
document still renders. The `OFFICE_HTML_OUTPUT_CAP` constant
mirrors `MAX_TEXT_CACHE_BYTES` from extract.ts (separate constant
to avoid a circular import; pinned by a unit test).
- `lc-render` is hidden when fallback shows so the empty padded slot
doesn't sit above the mammoth content.
Tests: existing CDN-path tests updated for the new
`wordDocToHtmlViaCdn(buffer, mammothBody)` signature; new test for
the embedded fallback structure (`#lc-fallback`, mammoth body
content, "High-fidelity renderer unavailable" notice, render-slot
hide); new constant pin and per-fixture cap-respect assertion.
Codex P1 review on PR #12934: routing .docx/.csv/.xlsx/.xls/.ods/.pptx
into the office preview buckets assumed `attachment.text` was already
sanitized full-document HTML, but that guarantee only existed for the
new code-output extractor path. Existing stored attachments and other
non-code paths can still carry plain extracted text — `useArtifactProps`
would then inject that as `index.html` inside the Sandpack iframe.
Adds a `textFormat: 'html' | 'text' | null` trust flag persisted on
the file record by the code-output extractor, surfaced over the SSE
attachment payload and the TFile API type. The client's routing in
`detectArtifactTypeFromFile` requires `textFormat === 'html'` before
landing on an office HTML bucket; everything else (legacy attachments,
RAG-extracted plain text from `parseDocument`, explicitly-marked
'text' entries) falls back to the PLAIN_TEXT bucket where the
markdown viewer escapes content rather than executing it.
Tests: new `getExtractedTextFormat` helper has 14 cases covering all
office paths, legacy XLS MIME aliases, parseDocument fallthroughs,
and null-input. Client `artifacts.test.ts` adds three security-gate
tests proving downgrade behavior for missing/null/'text' textFormat,
plus a `fileToArtifact` test that legacy office attachments without
the flag end up in PLAIN_TEXT with their content escaped.
Manual e2e of the PPTX CDN renderer surfaced a black screen with
"Could not establish connection. Receiving end does not exist."
unhandled-rejection — characteristic of a Web Worker that couldn't
start.
Root cause: pptx-preview's bundled echarts dep spins up Web Workers
via blob: URLs for chart rendering. Our CSP had `default-src 'none'`
and no `worker-src`, so workers fell back to default → blocked. The
async failure deep inside echarts didn't surface through the outer
`previewer.preview()` promise, so my bootstrap's `.catch` never fired,
the loading state was removed, and the iframe sat with the body
background showing through (dark navy in dark mode = "black screen").
Three changes:
- Add `worker-src blob:` to the PPTX CSP. Allows blob:-only worker
creation without permitting arbitrary worker URLs.
- Bootstrap: window-level `unhandledrejection` and `error` listeners
so rejections from inside bundled-dep async pipelines surface as
the user-facing "Preview unavailable" fallback instead of going
silent.
- Bootstrap: 8-second timeout that checks `container.children.length`
— if the renderer hasn't appended anything visible by then, assume
silent failure and show the fallback.
Also wipe `container.innerHTML` when showing the fallback so a partial
render doesn't compete with the message.
DOCX wrapper unchanged: docx-preview doesn't use workers, so the
worker-src directive doesn't apply, and the existing fallback path
already covers its failure modes.
Tests
-----
- Existing PPTX CSP test now also asserts `worker-src blob:` is present.
- Existing fallback-message test extended to cover the new
unhandledrejection/error/timeout listeners.
packages/api/src/files: 467/467 ✅.
docx-preview defaults to rendering at the document's native page
width (8.5in for letter, 21cm for A4). In a wide artifact panel
that left whitespace on either side; in a narrow one it forced
horizontal scroll.
Two changes:
- Pass `ignoreWidth: true` to `docx.renderAsync` so the library skips
the document's pageSize width and uses its container's width.
- Defensive CSS overrides on `.docx-wrapper` and `.docx-wrapper > section.docx`
in case a future library version regresses on the option, plus
`padding: 0` on the wrapper to drop the page-edge whitespace
docx-preview otherwise reserves.
`renderHeaders`/`renderFooters`/etc. stay enabled — those still
appear in the rendered output, just inside a container that fills
the panel instead of a fixed-width "page."
Tests unchanged (100/100); manual e2e ahead of merge.
Mirrors the DOCX CDN architecture for PPTX: small files (≤350 KB
binary) embed as base64 and render via `pptx-preview` loaded from
jsdelivr inside the Sandpack iframe. Larger files and air-gapped
deployments fall back to the existing slide-list extraction.
Why
---
PPTX is the format where the gap between LibreChat's preview and
Claude.ai-style previews was most visible (slide-list of bullet
points vs. rendered slide layouts). LibreOffice → PDF → PDF.js is
still the eventual gold-standard answer for PPTX fidelity, but
client-side rendering inside the Sandpack iframe gets us a
meaningful intermediate step (~1.5/5 → ~3.5/5) without a sidecar.
What
----
- `pptx-preview@1.0.7` (ISC license, ~1.36 MB UMD bundle that
includes its echarts/lodash/uuid/jszip/tslib deps inline). Pinned
to a specific version on jsdelivr with SHA-384 SRI and
`crossorigin="anonymous"`.
- `buildPptxCdnDocument` mirrors the DOCX wrapper: same CSP locks
(`default-src 'none'`, `connect-src 'none'`, no eval, no base/form
tampering), same `id="lc-doc-data"` base64 slot, same fallback
message wiring (`typeof pptxPreview === 'undefined'` →
"Preview unavailable").
- New public `pptxToHtml(buffer)` dispatcher; `bufferToOfficeHtml`
switches its `'pptx'` case to call it. `pptxToSlideListHtml` stays
exported as the slide-list-only path (still hit by tests directly
and by the dispatcher fallback).
- `OFFICE_PREVIEW_DISABLE_CDN=true` env-var hatch applies to PPTX
too — air-gapped operators get the slide-list path. Same env-var
read at call time, same matrix of truthy values (`1` / `true` /
`yes` / case-insensitive / whitespace-trimmed).
- `_internal` re-exports moved to after the PPTX section since the
PPTX internals live further down in the file. Adds
`pptxToHtmlViaCdn`, `MAX_PPTX_CDN_BINARY_BYTES`,
`PPTX_PREVIEW_CDN`.
Honest caveats
--------------
- The 1.36 MB UMD bundle has `require("stream"/"events"/"buffer"/
"util")` references in its outer wrapper. Those are bundled-dep
artifacts (likely from `tslib` / Node-shim transforms) and don't
appear to execute on the browser code paths, but I haven't done
manual e2e on a wide range of decks. If a class of files turns up
that breaks rendering, the iframe-side fallback message catches it
and operators have `OFFICE_PREVIEW_DISABLE_CDN=true` as the bail.
- First-render CDN fetch is ~1.36 MB (browser-cached after).
- PPTX with embedded media easily exceeds the 350 KB binary cap;
those files take the slide-list path. Lifting the cap is a
follow-up (tied to the broader self-hosting work).
Tests
-----
11 new in two new describe blocks:
- `pptxToHtml dispatcher`: routing predicate (small → CDN, env-set
→ slide-list).
- `CDN-rendered path`: base64 round-trip, SRI integrity +
crossorigin, CSP locks (connect/eval/base/form), fallback message,
size-threshold lock at 350 KB.
- `OFFICE_PREVIEW_DISABLE_CDN escape hatch`: env-var matrix for
truthy values.
packages/api/src/files: 457/457 ✅ (was 446, +11).
Two follow-up review findings on PR #12934, both real.
P1 — Spreadsheet MIME aliases on client
----------------------------------------
Backend's `officeHtmlBucket` uses the broad `excelMimeTypes` regex from
`librechat-data-provider` (covers `application/x-ms-excel`,
`application/x-msexcel`, `application/msexcel`, `application/x-excel`,
`application/x-dos_ms_excel`, `application/xls`, `application/x-xls`,
plus the canonical sheet MIMEs). The client's exact-match
`MIME_TO_TOOL_ARTIFACT_TYPE` only had three of those, so an
extensionless XLS upload with a legacy MIME would have backend HTML
produced but the client would fail to route the artifact at all —
preview chip never registers.
Fix: import the same regex on the client and add it as a fallback in
`detectArtifactTypeFromFile` after the exact-match map miss. Stays in
lock-step with the backend automatically.
7 new test cases — one per legacy alias.
P2 — Hard CDN dependency on jsdelivr
-------------------------------------
Air-gapped / corporate-filtered networks where jsdelivr is unreachable
would see DOCX previews permanently degrade to "Preview unavailable"
because the iframe could never load the renderer scripts. Mammoth was
sitting right there on the server but the dispatcher always preferred
the CDN path for files under 350 KB.
Fix: `OFFICE_PREVIEW_DISABLE_CDN` env var. When truthy (`1`, `true`,
`yes`, case-insensitive, whitespace-trimmed), `wordDocToHtml`
short-circuits to the mammoth path regardless of file size. Operators
on filtered networks set the env var; default behavior is unchanged.
Read at function-call time (not module load) so jest can flip it in
`beforeEach` without `jest.resetModules()`. The cost is one property
access per render.
12 new test cases: env-unset uses CDN (default), all five truthy
forms force mammoth, five non-truthy forms (`false`/`0`/`no`/empty/
arbitrary string) leave CDN active.
Tests
-----
packages/api/src/files: 446/446 ✅ (was 434, +12 from env-var matrix).
client artifact suites: 235/235 ✅ (was 228, +7 from MIME aliases).
Switch the default DOCX render path from server-side mammoth → flat
HTML to client-side `docx-preview` loaded inside the Sandpack iframe.
Mammoth becomes the fallback for files above the cap.
Why
---
The Sandpack iframe is a real browser DOM. Server-side rendering
ceiling for DOCX→HTML is well below the source's visual fidelity —
mammoth strips cell shading, run colors, banners, and column layouts
because Word's layout model doesn't fit HTML's flow model. Pushing the
render into the iframe lifts that ceiling without paying the
server-side cost of jsdom or LibreOffice.
What
----
- New `wordDocToHtmlViaCdn(buffer)` builds a self-contained HTML doc
that embeds the binary as base64 and lets `docx-preview@0.3.7`
render it on load. CSS preserves dark/light mode handoff via
`prefers-color-scheme`. Bootstrap script falls back to a "preview
unavailable, please download" message if the CDN is unreachable or
the parse throws.
- `docx-preview` and its `jszip` peer dep are pinned to specific
versions on jsdelivr with SRI sha384 integrity hashes and
`crossorigin="anonymous"`. Refresh: re-fetch the file, run
`openssl dgst -sha384 -binary FILE | openssl base64 -A`.
- CSP locked down on the iframe: `default-src 'none'`, scripts only
from jsdelivr (no eval), `connect-src 'none'` so a parser bug in
docx-preview can't be turned into exfiltration of the embedded
document, `base-uri 'none'`, `form-action 'none'`. Defense in depth
on top of the Sandpack cross-origin sandbox.
- `wordDocToHtml` dispatches by size: ≤ 350 KB binary → CDN path
(high fidelity), larger → mammoth fallback (preserves the size cap
on `attachment.text`). 350 KB chosen so worst-case base64-inflated
output (~478 KB) plus wrapper overhead (~5 KB) fits under
MAX_TEXT_CACHE_BYTES (512 KB) with 40 KB headroom.
- Internal renderers exported as `_internal` for tests. Public API
unchanged — callers still go through `wordDocToHtml`.
PPTX intentionally NOT switched
-------------------------------
Surveyed the available client-side PPTX libraries:
- `pptx-preview@1.0.7` ships an ESM-only main entry plus a 1.36 MB
UMD that references `require("stream"/"events"/"buffer"/"util")` —
bundled for Node, not browser-clean. Could work but the runtime
references to undefined Node globals are a fragility risk worth
more validation than this PR can absorb.
- `pptxjs` is jQuery-era, requires four separate UMD scripts in a
specific order, less actively maintained.
- The honest answer for PPTX is the LibreOffice sidecar (DOCX/XLSX/
PPTX → PDF → PDF.js), which is the architecture every major
product (Google Drive, Claude.ai, ChatGPT) effectively uses and
the only path to ~5/5 fidelity for arbitrary user decks.
PPTX stays on the existing slide-list extraction for now. Open a
follow-up issue for the LibreOffice/Gotenberg sidecar.
Tests
-----
- 6 new in CDN-rendered describe block: wrapper structure, base64
round-trip, SRI integrity + crossorigin, CSP locks
(connect-src/eval/base-uri/form-action), fallback message wiring,
size-threshold lock.
- Adjusted 2 existing tests that asserted on mammoth-path artifacts
(literal document text in `<article class="lc-docx">`) — those
assertions move to the mammoth-fallback test that calls
`_internal.wordDocToHtmlViaMammoth` directly. Dispatcher tests now
assert CDN-path signatures instead.
packages/api files: 434/434 ✅, full unit suite 4473/4473 ✅.
Two UX improvements based on hands-on use of the office preview pipeline.
DOCX rendering — mammoth strips the navy banners, cell shading, and
column layouts that direct-formatted docs apply (python-docx-style
output is a common case). The flat `<p><strong>X</strong></p>` and
bare `<table><tr><td>` it emits looks washed out next to the source.
Three targeted compensations:
- Style map promotes `Title`, `Subtitle`, `Heading 1` thru `Heading 6`,
and `Quote` paragraphs to their semantic HTML equivalents (mammoth's
default only handles Heading 1-6, missing Title/Subtitle/Quote).
- Extra CSS scoped to `.lc-docx` gives the first table row sticky-
looking header styling regardless of `<thead>` (mammoth never emits
`<thead>`), adds zebra striping, and treats the python-docx
`<p><strong>X</strong></p>` section-heading idiom as a pseudo-h2 with
a thin accent left border so document structure survives the round
trip. Headings get a left accent or underline so they read as
headings instead of just bold paragraphs.
- Sanitizer's `allowedAttributes` opens `class` on the heading and
block tags the styleMap and CSS heuristics rely on. `<script>`,
event handlers, javascript: URLs, etc. are still stripped — the
existing security regression suite catches any drift.
Panel header — `Artifacts.tsx` showed a generic "Preview" pill for
preview-only artifacts. Single-tab Radio is a no-op; surfacing the
document filename there gives the user something useful in the chrome
without taking real estate. `displayFilename` handles the sandbox
dotfile suffix the upload pipeline applies.
Tests: html.spec.ts +1 (new CSS-emission lock), 71/71. Backend files
suite 428/428. Client 308/308.
Follow-up review nit on PR #12934. Jest's `it.each` substitutes `%s`
positionally, and the table rows were `[type, expected]` while the
description template read `'returns %s for type %s'` — outputting
"returns application/vnd.librechat.docx-preview for type true"
instead of the intended "type ... returns true".
Reorder the template to match the column order. Test runner output
now reads naturally: "type application/vnd.librechat.docx-preview
returns true". Pure cosmetic — runtime behavior unchanged.
Wraps up the 10-finding follow-up review. Two MAJOR + four MINOR + two
NIT addressed; one NIT skipped after verifying it was a misread of the
package.json structure.
MAJOR
- #1: Rewrite `renderOfficeHtml` JSDoc to document the HTML-or-null
contract explicitly. The pre-fix doc described a text-fallback path
that was the original XSS vector (commit b06f08a). A future
maintainer trusting the stale doc could reintroduce the fallback.
- #2: Replace byte-truncation of office HTML with a small "preview too
large" banner document. Cutting at a UTF-8 boundary lands mid-tag
(`<table><tr><td>con\n…[truncated]`) and ships malformed markup to
the iframe — unpredictable rendering, occasional broken layouts on
DOCX with embedded images / wide spreadsheets.
MINOR
- #4: Wrap `readSlidesFromZip`'s `zipfile.close()` in try/catch so a
close-time exception (mid-flight stream) doesn't replace the
original error. Mirrors the defensive pattern in zipSafety.ts.
- #5: Refactor PPTX extraction to use `yauzl.fromBuffer` directly,
eliminating the temp-file write/unlink the safety pre-flight already
proved unnecessary. Removes 4 unused imports (os, path, fs/promises,
randomUUID).
- #6: Extract `isPreviewOnlyArtifact(type)` to `client/src/utils/
artifacts.ts` so the membership check is unit-testable without
mounting the full Artifacts component (Recoil + Sandpack + media
query). 15 new test cases covering positive types, negative types,
null/undefined, and unknown strings.
NIT
- #3: Remove dead `stripColorStyles` / `COLOR_PROPERTY_PATTERN` —
unused (sanitizer's `allowedStyles` config handles color implicitly).
- #7: Remove dead `!_lc_csv_label` worksheet property write.
- #9: Remove no-op `exclusiveFilter: () => false` sanitize-html config.
- #10: Type-narrow `PREVIEW_ONLY_ARTIFACT_TYPES` to
`ReadonlySet<ToolArtifactType>` so the membership table is
compile-time checked against the enum.
SKIPPED
- #8: Reviewer flagged `sanitize-html` as duplicated in devDeps and
dependencies. The package has no `dependencies` section — only
`devDependencies` and `peerDependencies`. Existing convention
(mammoth, xlsx, yauzl, pdfjs-dist) is to appear in BOTH. Removing
the devDep entry would break local test runs.
Tests: packages/api 4406/4406, client artifacts 128/128.
Codex P1 review on #12934 caught that when `renderOfficeHtml` failed
(timeout, malformed file, zip-bomb rejection) for an office type, the
extractor fell through to `extractDocument` and returned plain text.
The client routes by extension/MIME to the office preview buckets and
feeds `attachment.text` straight into the Sandpack iframe's
`index.html`. A spreadsheet cell or document body containing the
literal string `<script>alert(1)</script>` would have been injected
as executable markup — direct XSS.
The contract for office types is now HTML-or-null with no text
fallback. Failed render returns null, the client's empty-text gate
keeps the artifact off the panel, and the file falls back to the
regular download UI (matching what PPTX already did). PDF and ODT
still go through `extractDocument` because the client routes them to
PLAIN_TEXT (which the markdown viewer escapes) or no artifact at all,
so plain text is safe there.
Test reshuffle:
- `document` describe block now uses ODT/PDF for the legacy
parseDocument-path tests (DOCX/XLSX/XLS/ODS bypass that path).
- New "does NOT call parseDocument for office HTML types" test locks
in the SEC contract for all four office HTML buckets.
- "falls back to ..." tests rewritten as "returns null when ..." with
explicit `parseDocumentCalls.length === 0` assertions to prove no
text leaks back to the client.
- New XSS regression test for the XLSX failure path.
- Mock parseDocument failure-name match relaxed to `includes()` so
ODT-named tests can use the same trigger.
Codex review on #12934 caught that the early `category === 'other'`
return short-circuited before `hasOfficeHtmlPath` was checked. The
classifier returns 'other' for inputs the new dispatcher can still
route — extensionless `application/csv` (CSV MIMEs aren't in the
classifier's text-MIME set and don't start with `text/`), and
extensionless office MIMEs with parameters like `application/vnd...
spreadsheetml.sheet; charset=binary` (the classifier's `isDocumentMime`
exact-matches these MIMEs without parameter normalization). Both would
route correctly through `officeHtmlBucket` but never reached it.
Move the office-HTML attempt above the 'other' early return, and drop
the `|| category === 'document' || category === 'pptx'` shortcut now
that `hasOfficeHtmlPath` covers the same surface (with parameter
normalization) and a wider one. ODT still routes through `extractDocument`
unchanged — `hasOfficeHtmlPath` returns false for it and the
`category === 'document'` branch below handles it.
Adds 3 regression tests:
- extensionless `application/csv` + category='other' → office HTML
- extensionless parameterized office MIME + category='other' → office HTML
- defense check: actual binary 'other' (image/jpeg) still returns null
without invoking the office producer
Two related Codex catches on PR #12934 — both about MIME-routing
inconsistencies between backend and client that would cause
extensionless CSV files to render as broken (raw text under an HTML
slot) or skip the artifact panel entirely.
P2 — backend MIME normalization:
`officeHtmlBucket` matched MIME strings exactly, so a real-world
`text/csv; charset=utf-8` Content-Type slipped through and the
backend returned raw CSV text. The client's `baseMime` helper
strips parameters before its own MIME lookup, so it routed the
same file to the SPREADSHEET bucket expecting an HTML body that
never arrived. Mirror the client's normalization on the backend
(strip everything from `;` onward, lowercase) before bucket
matching.
P3 — client legacy CSV MIME:
Backend's `CSV_MIME_PATTERN` accepts three variants (`text/csv`,
`application/csv`, `text/comma-separated-values`); the client's
`MIME_TO_TOOL_ARTIFACT_TYPE` only had the first two. An
extensionless file with `text/comma-separated-values` would have
backend HTML produced but the client would skip the artifact
panel entirely. Add the missing variant.
Tests:
- 9 new parameterized-MIME cases on backend covering charset/
boundary/case variants for every bucket.
- 1 new client routing case for `text/comma-separated-values`.
Addresses pre-existing availability vulnerability validated by
SEC review (Codex finding 275344c5...) and made worse by this PR's
HTML rendering path. A sub-1MiB compressed XLSX/DOCX/PPTX (highly
compressed run-of-zeros) inflates to 200+ MiB of XML when handed
to mammoth/xlsx — blocking the Node event loop for 10+ seconds and
spiking RSS to ~1 GiB. The existing 8s `withTimeout` wrapper uses
`Promise.race`, which can only return early; it cannot interrupt
synchronous parser CPU/RAM consumption. PoC ran an authenticated
execute_code call to OOM the API process.
Add `assertSafeZipSize(buffer)` — a yauzl-based pre-flight that
streams every entry with mid-inflate byte counting and bails on
either a per-entry or total decompressed-size cap. Mid-inflate
counting cannot be bypassed by falsifying the central directory's
`uncompressedSize` field (the technique the PoC used). Defaults:
25 MiB per entry, 100 MiB total — generous headroom for legitimate
image-heavy office files, well below the attack profile.
Hook the check into every path that hands a buffer to mammoth/xlsx
/yauzl:
- New HTML producers (`wordDocToHtml`, `excelSheetToHtml`,
`pptxToSlideListHtml`) — added by this PR
- Legacy RAG text extractors (`wordDocToText`, `excelSheetToText`
in `crud.ts`) — pre-existing path, also vulnerable
Errors propagate as a tag-distinct `ZipBombError` so callers can
distinguish a refused bomb from generic parse failures. The outer
`extractCodeArtifactText` swallows the error and returns null,
falling back to the regular download UI.
`.xls` (BIFF/CFB binary, not ZIP) is detected by magic bytes and
skipped — yauzl would reject it as malformed anyway.
Adds 15 tests:
- `zipSafety.spec.ts` (9): benign passes, per-entry cap, total cap,
ZipBombError type-tagging, malformed-zip distinction, directory-
entry handling, named-error surfacing, and the SEC-PoC pattern
(sub-1 MiB compressed → 50 MiB inflated rejected on default caps).
- `html.spec.ts` zip-bomb suite (5): each producer rejects a bomb;
dispatcher propagates correctly; legitimate fixtures still render.
- `extract.spec.ts` (1): outer extractor swallows ZipBombError and
returns null so the download UI fallback fires.
Codex review on #12934 caught that the predicate's if-chain interleaved
extension and MIME checks for each bucket — e.g. CSV's branch was
`ext === 'csv' || CSV_MIME_PATTERN.test(mimeType)`. A `deck.pptx`
shipped with `text/csv` (sandboxed tools sometimes ship generic MIMEs)
matched the CSV branch BEFORE the PPTX extension branch was reached,
so a binary PPTX would have been handed to `csvToHtml` to parse as
text — yielding garbage or a parse exception.
Restructure to a strict two-pass dispatch: an exhaustive extension
table first (one lookup, all known extensions), then MIME-only
fallback for extensionless / unknown-ext inputs. The doc comment's
"extension wins" claim is now actually enforced by the implementation.
Add 7 regression cases covering the conflicting-MIME footgun for each
bucket: deck.pptx + text/csv → pptx; workbook.xlsx + text/csv →
spreadsheet; legacy.xls + pptx-MIME → spreadsheet; report.docx +
text/csv → docx; data.csv + docx-MIME → csv; etc.
Codex review on #12934 caught that the office-render gate in
`extractCodeArtifactText` only fired when the extension was in
`OFFICE_HTML_EXTENSIONS` or the category was `document`/`pptx`. A tool
emitting `data` with `text/csv` (no extension) classifies as
`utf8-text`, so the gate was skipped and raw CSV text shipped to the
client — but the client routes by MIME to the SPREADSHEET bucket
expecting a full HTML document, so the panel rendered broken text.
Extract a shared `officeHtmlBucket(name, mime)` predicate from
`html.ts` (returns the bucket name or null). Both `bufferToOfficeHtml`
(the dispatcher) and the upstream gate in `extract.ts` now go through
this single source of truth, so they can never drift apart again. The
predicate already mirrors the dispatcher's extension/MIME logic
(extension wins; MIME is the fallback for extensionless inputs).
Adds:
- 14 cases for the new `officeHtmlBucket` predicate covering the
positive paths (each bucket via extension OR MIME) and the negative
paths (txt, py, json, jpg, pdf, zip, odt, plain noext).
- A direct regression test in `extract.spec.ts` for the Codex catch:
`data` with `text/csv` + utf8-text category routes through the
office HTML producer.
- Parameterized cases for extensionless DOCX/XLSX/XLS/ODS/PPTX files
identified by MIME alone.
Codex review on #12934 caught that `data:` lived in the global
`allowedSchemes`, which meant a smuggled `<a href="data:text/html,
<script>...</script>">` would survive sanitization. The Sandpack
iframe sandbox does not gate `target="_blank"` navigations, so a
click would open attacker-controlled HTML in a new tab.
Scope `data:` to `<img src>` only via `allowedSchemesByTag` (mammoth
inlines DOCX images as base64 `data:image/...` URIs — that path still
works). Add a regression suite (`sanitizeOfficeHtml security`) with
8 cases covering: <script> stripping, event-handler removal,
javascript:/data: rejection on anchors, data:image preservation in
<img>, http/https/mailto allowance, target=_blank rel=noopener
enforcement, and <iframe> stripping.
Render office files emitted by tools as interactive previews in the
artifact panel instead of raw extracted text. The backend produces a
sanitized HTML document via mammoth (DOCX), SheetJS (CSV/XLSX/XLS/ODS),
or yauzl-based slide extraction (PPTX) and ships it through the
existing SSE attachment payload; the client routes it through the
Sandpack `static` template's `index.html` slot — no new browser deps,
no client-side blob fetch, no React renderer components.
Two `dangerouslySetInnerHTML` sites rendered admin-supplied HTML
without sanitization:
- `Banner.tsx` rendered `banner.message` directly.
- `MCPConfigDialog.tsx` rendered each `customUserVars` description.
Wrap both with DOMPurify, allowing only the inline tags needed for
formatting (links, emphasis, line breaks). Hardens against compromised
admin or yaml supply-chain scenarios. Pattern matches the existing
`CustomUserVarsSection.tsx` and `Tooltip.tsx` sanitizer setup.
`/c/new?prompt=…&submit=true` previously auto-submitted the prompt
unconditionally. For deployments where users may receive crafted
links from external sources, an authenticated victim's click can
trigger an immediate, attacker-controlled prompt against a memory- or
tool-enabled model — providing a 1-click vector for prompt-injection
exfiltration via markdown image rendering.
Add `interface.autoSubmitFromUrl` (default `true` to preserve current
behavior). Operators handling sensitive memory/tool data can set it
to `false` so URL-supplied prompts only pre-fill the composer; the
user must press Send explicitly.
`resizeAvatar` previously called `node-fetch` on any string input with
no validation. When OIDC providers surface a user-controllable
`picture` claim, this could be used to make blind SSRF requests to
internal services on every social login.
Wrap the URL fetch with:
- An allowlist on the URL protocol (http/https only).
- The shared `createSSRFSafeAgents` utility, which blocks resolution to
private, loopback, and link-local IPs at TCP connect time
(TOCTOU-safe; works equally for hostname targets that DNS-resolve
privately and for IP-literal targets, since Node's `net.Socket`
always dispatches through the agent's `lookup` hook).
- `redirect: 'error'` so a public-IP redirect target cannot be used to
bypass the agent check on a subsequent hop.
- A 5-second total request budget (node-fetch v2's `timeout` covers
request initiation through full body receipt, bounding slow-loris
exposure rather than just the TCP connect).
- A 10 MB response cap (`size` option + `Content-Length` pre-check +
post-read length assertion) so a hostile payload cannot exhaust
memory before `sharp()` rejects it.
Fetch the canonicalized `parsed.href` rather than the raw input string
to eliminate any future parser-differential between `new URL()` and
the underlying fetch implementation.
Per-call agent construction is intentional: the avatar path runs once
per social login per user, so pooling adds complexity without a
measurable benefit. Documented inline.
Comprehensive test coverage in `avatar.spec.js`:
- Rejects malformed URLs, non-http(s) schemes (file://, data:,
javascript:).
- Asserts the happy-path canonicalization (`fetch` is called with
`parsed.href`) and the SSRF-safe agent factory routing
(https→httpsAgent, http→httpAgent).
- Rejects non-2xx HTTP status.
- Rejects an oversized Content-Length before reading the body, and
asserts `.buffer()` is never invoked in that case.
- Rejects an oversized body even when the server lies about / omits
Content-Length.
- Surfaces ESSRF, redirect, and `size` overflow errors thrown by the
fetch layer.
- Confirms Buffer inputs bypass the fetcher entirely.
* 🪟 feat: Add allowedAddresses Exemption List For SSRF-Guarded Targets
LibreChat already blocks SSRF-prone targets (private IPs, loopback,
link-local, .internal/.local TLDs) at every server-side fetch site
that consumes user-controllable URLs — custom-endpoint baseURLs, MCP
servers, OpenAPI Actions, and OAuth endpoints. The only existing
escape hatch is `allowedDomains`, but that flips the field into a
strict whitelist: adding `127.0.0.1` to permit a self-hosted Ollama
also blocks every public destination that isn't in the list.
Introduce `allowedAddresses` as the orthogonal primitive: a private-
IP-space exemption list. When a hostname or its resolved IP appears
in the list, the SSRF block is bypassed for that target. Public
destinations remain reachable. Operators can now run self-hosted
LLMs / MCP servers / Action endpoints on private addresses without
weakening the default-deny posture for everything else.
Schema additions in `packages/data-provider/src/config.ts`:
- `endpoints.allowedAddresses` (new — gates `validateEndpointURL`)
- `mcpSettings.allowedAddresses` (parallel to `allowedDomains`)
- `actions.allowedAddresses` (parallel to `allowedDomains`)
Core changes in `packages/api/src/auth/`:
- New `isAddressAllowed(hostnameOrIP, allowedAddresses)` — pure,
case-insensitive, bracket-stripped literal match.
- Threaded the list through `isSSRFTarget`, `resolveHostnameSSRF`,
`isDomainAllowedCore`, `isActionDomainAllowed`, `isMCPDomainAllowed`,
`isOAuthUrlAllowed`, and `validateEndpointURL`.
- Extended `createSSRFSafeAgents` and `createSSRFSafeUndiciConnect`
to accept the list, building an SSRF-safe DNS lookup that exempts
matching hostnames/IPs at TCP connect time (TOCTOU-safe).
Wiring:
- Custom and OpenAI endpoint initialize sites pass
`endpoints.allowedAddresses` to `validateEndpointURL`.
- `MCPServersRegistry` stores `allowedAddresses` and exposes it via
`getAllowedAddresses()`. The factory, connection class, manager,
`UserConnectionManager`, and `ConnectionsRepository` all thread
it through to the SSRF utilities.
- `MCPOAuthHandler.initiateOAuthFlow`, `refreshOAuthTokens`, and
`validateOAuthUrl` accept the list and consult it on every URL
validation along the OAuth chain.
- `ToolService`, `ActionService`, and the assistants/agents action
routes pass `actions.allowedAddresses` to `isActionDomainAllowed`
and to `createSSRFSafeAgents` for runtime action calls.
- `initializeMCPs.js` reads `mcpSettings.allowedAddresses` from the
app config and forwards it to the registry constructor.
Documentation:
- `librechat.example.yaml` shows the new field next to each existing
`allowedDomains` block, with a note clarifying that
`allowedAddresses` is an exemption list (not a whitelist).
Tests:
- Unit tests for `isAddressAllowed` covering literal IPs, hostnames,
IPv6 brackets, case insensitivity, and partial-match rejection.
- Exemption tests for every entry point: `isSSRFTarget`,
`resolveHostnameSSRF`, `validateEndpointURL`, `isActionDomainAllowed`,
`isMCPDomainAllowed`, `isOAuthUrlAllowed`.
- Existing tests updated to reflect the new optional parameter.
Default behavior is unchanged: omitted = empty list = no exemptions.
* 🩹 fix: Plumb allowedAddresses Through AppConfig endpoints Type
The initial PR added `endpoints.allowedAddresses` to the
data-provider config schema and consumed it in the endpoint
initialize sites, but the runtime `AppConfig.endpoints` shape in
`@librechat/data-schemas` was a hand-maintained subset that didn't
include the new field — so `tsc` rejected `appConfig.endpoints.allowedAddresses`.
Add the field to `AppConfig['endpoints']` in
`packages/data-schemas/src/types/app.ts` and forward it from the
loaded config in `packages/data-schemas/src/app/endpoints.ts` so the
runtime config carries the value.
Update `initializeMCPs.spec.js` to expect the third positional
argument (`allowedAddresses`) on the `createMCPServersRegistry` call.
* 🩹 fix: Enforce allowedDomains Before allowedAddresses In isOAuthUrlAllowed
The initial implementation checked the address exemption first, so a
URL whose hostname appeared in `allowedAddresses` would return true
even when the admin had configured `allowedDomains` as a strict bound
on OAuth endpoints. A malicious MCP server could advertise OAuth
metadata, token, or revocation URLs at any address the admin had
permitted for an unrelated reason (a self-hosted LLM at `127.0.0.1`,
for example) and pass validation, expanding SSRF reach beyond the
configured domain whitelist.
Reorder: when `allowedDomains` is set, treat it as authoritative —
return true only if the URL matches a domain entry, otherwise fall
through to false. The address exemption only applies when no
`allowedDomains` is configured (mirrors how the downstream SSRF check
in `validateOAuthUrl` consults `allowedAddresses`).
Add a regression test asserting that an `allowedAddresses` entry does
not broaden a configured `allowedDomains` list.
Reported by chatgpt-codex-connector on PR #12933.
* 🩹 fix: Forward allowedAddresses To Remaining OAuth Callers
Two `MCPOAuthHandler` callers still used the pre-feature signatures and
were silently dropping the new `allowedAddresses` argument:
- `api/server/routes/mcp.js` invoked `initiateOAuthFlow` with the old
5-argument shape, so OAuth flows initiated through the route handler
ignored the registry's `getAllowedAddresses()` and would reject any
metadata/authorization/token URL on a permitted private host.
- `api/server/controllers/UserController.js#maybeUninstallOAuthMCP`
invoked `revokeOAuthToken` without the address exemption, so
uninstalling an OAuth-backed MCP server on a permitted private host
would fail at the revocation step even though the rest of the MCP
connection path now permits it.
Both sites now read `allowedAddresses` from the registry alongside
`allowedDomains` and forward it. Reported by Copilot on PR #12933.
* 🩹 fix: Update Test Mocks And Assertions For OAuth allowedAddresses
The previous commit started passing `allowedAddresses` to
`MCPOAuthHandler.initiateOAuthFlow` from `api/server/routes/mcp.js`
and to `MCPOAuthHandler.revokeOAuthToken` from
`api/server/controllers/UserController.js`, but the corresponding
test files mocked the registry without `getAllowedAddresses` (causing
`TypeError`s) and asserted the old positional shape on
`toHaveBeenCalledWith`.
Update the mocks and assertions to match the new arity:
- `api/server/routes/__tests__/mcp.spec.js`: add
`getAllowedDomains`/`getAllowedAddresses` to the registry mock and
expect the additional positional args on `initiateOAuthFlow`.
- `api/server/controllers/__tests__/maybeUninstallOAuthMCP.spec.js`:
add a `getAllowedAddresses` mock alongside the existing
`getAllowedDomains` and seed it in `setupOAuthServerFound`.
- `api/server/controllers/__tests__/UserController.mcpOAuth.spec.js`:
add `getAllowedAddresses` to the registry mock and expect the
trailing `null` arg on the three `revokeOAuthToken` assertions.
* 🛡️ fix: Address Comprehensive Review — Scope allowedAddresses To Private IP Space
Major findings from the comprehensive PR review (severity → fix):
**CRITICAL — `validateOAuthUrl` SSRF fallback bypass.** When `allowedDomains`
is configured and a URL fails the whitelist, the SSRF fallback in
`validateOAuthUrl` was still passing `allowedAddresses` to `isSSRFTarget` /
`resolveHostnameSSRF`, letting a malicious MCP server advertise OAuth
endpoints at any address the admin had permitted for an unrelated reason.
Suppress `allowedAddresses` in the fallback when `allowedDomains` is active —
the address exemption is opt-in for the no-whitelist mode only.
**MAJOR — WebSocket transport SSRF check ignored exemptions.** The
`constructTransport` WebSocket branch called `resolveHostnameSSRF(wsHostname)`
without `this.allowedAddresses`, so a permitted private MCP server would
pass `isMCPDomainAllowed` but be blocked at transport creation. Forward
the exemption.
**Scope `allowedAddresses` to private IP space only (operator directive).**
The exemption list is for permitting private/internal targets; it must not
be a back-door to broaden trust to public destinations.
- Schema (`packages/data-provider/src/config.ts`): new
`allowedAddressesSchema` rejects URLs (`://`), paths/CIDR (`/`),
whitespace, and public IPv4/IPv6 literals at config-load time. Wired
into `endpoints`, `mcpSettings`, and `actions`.
- Runtime (`packages/api/src/auth/domain.ts`): `isAddressAllowed` now
drops public-IP candidates and public-IP entries on the match path —
defense in depth so a misconfigured runtime list never grants exemption.
- Hot path (`packages/api/src/auth/agent.ts`): `buildSSRFSafeLookup`
pre-normalizes the list into a `Set<string>` once at construction and
applies the same scoping filter, so the connect-time DNS lookup is an
O(1) Set membership check instead of a full re-iterate-and-normalize on
every outbound request.
**Test coverage for the connect-time and OAuth-fallback paths.**
- `agent.spec.ts`: new describe block exercising `buildSSRFSafeLookup` and
`createSSRFSafe*` with `allowedAddresses` — hostname-literal exemption,
resolved-IP exemption, public-IP scoping, URL/CIDR/whitespace rejection,
and the default no-list block.
- `handler.allowedAddresses.test.ts` (new): integration tests for
`validateOAuthUrl` — covers both the no-domains-set "permit private"
path and the strict-bound regression where `allowedAddresses` must NOT
bypass `allowedDomains`.
**Documentation & cleanup.**
- `connection.ts` redirect SSRF check: explicit comment that
`allowedAddresses` is intentionally NOT consulted for redirect targets
(server-controlled, must not inherit the admin's exemption).
- `MCPConnectionFactory.test.ts`: replaced an `eslint-disable` with a
proper `import { getTenantId } from '@librechat/data-schemas'`. The
disable was added to make a pre-existing `require()` quiet — the cleaner
fix is to use the existing top-level import.
Updated `MCPConnectionSSRF.test.ts` WebSocket SSRF assertions to match the
new two-argument call shape (`hostname, allowedAddresses`).
* 🩹 fix: Require Absolute URL Before allowedAddresses Trust Bypass In isOAuthUrlAllowed
`parseDomainSpec` is lenient — it silently prepends `https://` to
schemeless inputs so it can match patterns like bare `example.com`.
That leniency leaked into `isOAuthUrlAllowed`'s new
`allowedAddresses` short-circuit: a value like `10.0.0.5/oauth` (no
scheme) would parse successfully via the prepended default, hit the
address-exemption path, return `true`, and skip `validateOAuthUrl`'s
strict `new URL(url)` parse-or-throw — only to fail later in OAuth
discovery with a less clear runtime error.
Add a strict `new URL(url)` gate at the top of `isOAuthUrlAllowed`.
Schemeless inputs now fall through to `validateOAuthUrl`'s explicit
"Invalid OAuth <field>" rejection. Tests added in both
`auth/domain.spec.ts` (unit) and the OAuth handler integration spec
(end-to-end).
Reported by chatgpt-codex-connector (P2) on PR #12933.
* 🛡️ fix: Address Follow-Up Comprehensive Review — Schema Tests, Shared Normalization, host:port
Auditing the second comprehensive review:
**F1 MAJOR — schema validation untested.** `allowedAddressesSchema` had
zero coverage, so a regression in the three refinement stages or the
three wiring locations (`endpoints` / `mcpSettings` / `actions`) would
silently let invalid entries reach the runtime. Added a dedicated
`describe('allowedAddressesSchema')` block in `config.spec.ts` covering:
valid private IPs (v4 + v6, including the previously-missed 192.0.0.0/24
range), accepted hostnames, all rejection categories (URLs, CIDR, paths,
whitespace tabs/newlines, host:port, public IP literals), and full
`configSchema.parse()` integration at each of the three nesting points.
**F2 MINOR — `isPrivateIPv4Literal` divergence.** The schema reimpl in
`packages/data-provider` was discarding the `c` octet, so the
`192.0.0.0/24` (RFC 5736 IETF protocol assignments) range that the
authoritative `isPrivateIPv4` accepts was being rejected with a
misleading "public IP" error. Destructure `c` and add the missing range
check; covered by the new schema tests.
**F3 MINOR — DRY violation across `domain.ts` and `agent.ts`.** Both
files had independent normalization implementations with a subtle
whitespace-check divergence (`/\s/` vs `.includes(' ')`). Extracted the
shared logic into a new `packages/api/src/auth/allowedAddresses.ts`
module that both consumers import:
- `normalizeAddressEntry(entry)` — single-entry shape check
- `looksLikeHostPort(entry)` — host:port detector (used by F4)
- `normalizeAllowedAddressesSet(list)` — pre-normalized Set for the
connect-time hot path
- `isAddressInAllowedSet(candidate, set)` — membership check that
enforces private-IP scoping on the candidate
Both `isAddressAllowed` (preflight) and `buildSSRFSafeLookup` (connect)
now go through the same primitives; the whitespace divergence is gone.
To break the import cycle (`allowedAddresses` needs `isPrivateIP`,
`domain` previously owned it), extracted IP private-range detection
into a leaf `auth/ip.ts` module. `domain.ts` re-exports `isPrivateIP`
for backward compatibility with existing call sites.
**F4 MINOR — `host:port` silently misclassified.** Entries like
`localhost:8080` previously slipped through the URL/path guard, were
mis-detected as IPv6, failed `isPrivateIP`, and were silently dropped
with a misleading "public IP" schema error. Added an explicit
`looksLikeHostPort` check with a clear error: "allowedAddresses
entries must not include a port — list the bare hostname or IP only."
Bare `::1`, `[::1]`, and other valid IPv6 literals are intentionally
not matched (regex distinguishes by colon count and the bracketed
`[ipv6]:port` form).
**F5 MINOR — hostname-trust documentation gap.** Hostname entries
short-circuit `resolveHostnameSSRF` before any DNS lookup — that's a
deliberate design (admin trusts the name) but it means the exemption
follows whatever the name resolves to at runtime. Added an explicit
note in `librechat.example.yaml` for both `mcpSettings.allowedAddresses`
and `endpoints.allowedAddresses`: "a hostname entry trusts whatever IP
that name resolves to. Only list hostnames whose DNS you control.
Prefer literal IPs when you can."
**F6** (8 positional params) is flagged for follow-up; refactor to an
options object is a breaking-API change deferred to a separate PR.
**F7** (redirect/WebSocket asymmetry, NIT, conf 40) — skipping; the
existing inline comment is sufficient.
* 🧹 chore: Address Follow-Up NITs — Import Order And Mirror-Function Naming
Three NITs from the latest comprehensive review:
**NIT #1 (conf 85) — local import order.** AGENTS.md requires local
imports sorted longest-to-shortest. Both `domain.ts` and `agent.ts`
had `./ip` (shorter) before `./allowedAddresses` (longer). Swapped.
**NIT #2 (conf 60) — missing cross-reference.** The schema-side
`isHostPortShape` in `packages/data-provider/src/config.ts` had no
note pointing at the canonical runtime mirror. Added a JSDoc paragraph
explaining the mirror relationship and why a local copy exists (the
data-provider package can't import from `@librechat/api` without
creating a circular dependency).
**NIT #3 (conf 50) — naming inconsistency.** Renamed
`isHostPortShape` → `looksLikeHostPort` so the schema mirror matches
the runtime helper exactly. Kept as a separate function (not a shared
import) for the same circular-dependency reason; the matching name
makes it obvious they should stay in lockstep.
The Passport local strategy validation error logged the entire request
body (including the password) into error logs. Replace it with the
email only, matching the metadata shape used by sibling log calls in
the same function.
* 🔧 chore: Update dependencies in package-lock.json and package.json
- Bump version of @librechat/agents to 3.1.75-dev.0 in multiple package.json files.
- Upgrade various AWS SDK and Smithy dependencies to their latest versions in package-lock.json for improved stability and performance.
* 🔧 chore: Update AWS SDK and Smithy dependencies in package-lock.json
- Bump version of @aws-sdk/client-bedrock-runtime to 3.1041.0 and update related dependencies for improved performance and stability.
- Upgrade various AWS SDK and Smithy packages to their latest versions, ensuring compatibility and enhanced functionality.
* chore: Align LibreChat with agents LangChain upgrade
- Route LangChain imports through @librechat/agents facade exports
- Update @librechat/agents to 3.1.75-dev.1 and remove direct LangChain deps
- Normalize nullable agent model params and API key override typing
- Update Google thinking config typing for newer LangChain packages
- Refresh targeted audit-related dependency overrides
* chore: Add Jest types for API specs
* test: Fix LangChain upgrade CI specs
* test: Exercise agents env facade
* fix: Clean up TS preview diagnostics
* fix: Address Codex review feedback
Fixes #12912.\n\n- Clear stored MCP OAuth tokens and flow state on revoke cleanup-only paths.\n- Keep provider revocation best-effort when token and client metadata are available.\n- Add controller and function coverage for stale metadata, missing config, and cleanup failure paths.
