diff --git a/api/package.json b/api/package.json index 7007791088c4..3abdbc83e655 100644 --- a/api/package.json +++ b/api/package.json @@ -36,14 +36,16 @@ "dependencies": { "@anthropic-ai/vertex-sdk": "^0.14.3", "@aws-sdk/client-bedrock-runtime": "^3.1013.0", + "@aws-sdk/client-cloudfront": "^3.1042.0", "@aws-sdk/client-s3": "^3.980.0", + "@aws-sdk/cloudfront-signer": "^3.1036.0", "@aws-sdk/s3-request-presigner": "^3.758.0", "@azure/identity": "^4.13.1", "@azure/search-documents": "^12.0.0", "@azure/storage-blob": "^12.30.0", "@google/genai": "^1.19.0", "@keyv/redis": "^4.3.3", - "@librechat/agents": "^3.1.77", + "@librechat/agents": "^3.1.78", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", @@ -105,6 +107,7 @@ "passport-local": "^1.0.0", "pdfjs-dist": "^5.4.624", "rate-limit-redis": "^4.2.0", + "sanitize-html": "^2.13.0", "sharp": "^0.33.5", "traverse": "^0.6.7", "ua-parser-js": "^1.0.36", @@ -116,6 +119,7 @@ "zod": "^3.22.4" }, "devDependencies": { + "@types/sanitize-html": "^2.13.0", "jest": "^30.2.0", "mongodb-memory-server": "^11.0.1", "nodemon": "^3.0.3", diff --git a/api/server/controllers/AuthController.js b/api/server/controllers/AuthController.js index eb44feffa4c0..d61bcc28449d 100644 --- a/api/server/controllers/AuthController.js +++ b/api/server/controllers/AuthController.js @@ -2,7 +2,7 @@ const cookies = require('cookie'); const jwt = require('jsonwebtoken'); const openIdClient = require('openid-client'); const { logger } = require('@librechat/data-schemas'); -const { isEnabled, findOpenIDUser } = require('@librechat/api'); +const { isEnabled, findOpenIDUser, getOpenIdIssuer } = require('@librechat/api'); const { requestPasswordReset, setOpenIDAuthTokens, @@ -85,10 +85,12 @@ const refreshController = async (req, res) => { refreshParams, ); const claims = tokenset.claims(); + const openidIssuer = getOpenIdIssuer(claims, openIdConfig); const { user, error, migration } = await findOpenIDUser({ findUser, email: getOpenIdEmail(claims), openidId: claims.sub, + openidIssuer, idOnTheSource: claims.oid, strategyName: 'refreshController', }); @@ -111,6 +113,7 @@ const refreshController = async (req, res) => { await updateUser(user._id.toString(), { provider: 'openid', openidId: claims.sub, + ...(openidIssuer ? { openidIssuer } : {}), }); logger.info( `[refreshController] Updated user ${user.email} openidId (${reason}): ${user.openidId ?? 'null'} -> ${claims.sub}`, diff --git a/api/server/controllers/AuthController.spec.js b/api/server/controllers/AuthController.spec.js index 964947def9d1..8b19c28f36f8 100644 --- a/api/server/controllers/AuthController.spec.js +++ b/api/server/controllers/AuthController.spec.js @@ -23,6 +23,7 @@ jest.mock('~/models', () => ({ jest.mock('@librechat/api', () => ({ isEnabled: jest.fn(), findOpenIDUser: jest.fn(), + getOpenIdIssuer: jest.fn(() => 'https://issuer.example.com'), })); const openIdClient = require('openid-client'); @@ -157,6 +158,7 @@ describe('refreshController – OpenID path', () => { }; const baseClaims = { + iss: 'https://issuer.example.com', sub: 'oidc-sub-123', oid: 'oid-456', email: 'user@example.com', @@ -204,7 +206,10 @@ describe('refreshController – OpenID path', () => { expect(getOpenIdEmail).toHaveBeenCalledWith(baseClaims); expect(findOpenIDUser).toHaveBeenCalledWith( - expect.objectContaining({ email: baseClaims.email }), + expect.objectContaining({ + email: baseClaims.email, + openidIssuer: baseClaims.iss, + }), ); expect(res.status).toHaveBeenCalledWith(200); }); @@ -225,7 +230,10 @@ describe('refreshController – OpenID path', () => { expect(getOpenIdEmail).toHaveBeenCalledWith(claimsWithUpn); expect(findOpenIDUser).toHaveBeenCalledWith( - expect.objectContaining({ email: 'user@corp.example.com' }), + expect.objectContaining({ + email: 'user@corp.example.com', + openidIssuer: baseClaims.iss, + }), ); expect(res.status).toHaveBeenCalledWith(200); }); @@ -236,7 +244,10 @@ describe('refreshController – OpenID path', () => { await refreshController(req, res); expect(findOpenIDUser).toHaveBeenCalledWith( - expect.objectContaining({ email: baseClaims.email }), + expect.objectContaining({ + email: baseClaims.email, + openidIssuer: baseClaims.iss, + }), ); }); @@ -267,7 +278,11 @@ describe('refreshController – OpenID path', () => { expect(updateUser).toHaveBeenCalledWith( 'user-db-id', - expect.objectContaining({ provider: 'openid', openidId: baseClaims.sub }), + expect.objectContaining({ + provider: 'openid', + openidId: baseClaims.sub, + openidIssuer: baseClaims.iss, + }), ); expect(res.status).toHaveBeenCalledWith(200); }); diff --git a/api/server/controllers/agents/__tests__/callbacks.spec.js b/api/server/controllers/agents/__tests__/callbacks.spec.js index 8bd711f9c6ea..0ba20d409cd2 100644 --- a/api/server/controllers/agents/__tests__/callbacks.spec.js +++ b/api/server/controllers/agents/__tests__/callbacks.spec.js @@ -28,6 +28,28 @@ jest.mock('~/server/services/Files/Citations', () => ({ jest.mock('~/server/services/Files/Code/process', () => ({ processCodeOutput: jest.fn(), + /* `runPreviewFinalize` is the runtime pairing for `finalize` (defined + * alongside processCodeOutput in process.js). The callback wires + * the deferred render through it; reproduce the basic happy-path here so the + * SSE-emit assertions still work. The catch/defensive-updateFile + * branch is unit-tested directly against the real helper in + * process.spec.js — exercising it here would add test coupling + * without coverage benefit. */ + runPreviewFinalize: ({ finalize, onResolved }) => { + if (typeof finalize !== 'function') { + return; + } + finalize() + .then((updated) => { + if (!updated || !onResolved) { + return; + } + onResolved(updated); + }) + .catch(() => { + /* swallowed in the mock — see process.spec.js for catch coverage */ + }); + }, })); jest.mock('~/server/services/Tools/credentials', () => ({ @@ -326,4 +348,266 @@ describe('createToolEndCallback', () => { expect(res.write).not.toHaveBeenCalled(); }); }); + + describe('code execution deferred-preview emit', () => { + /* The deferred-preview code-execution flow emits the attachment twice over + * SSE: the initial emit with `status: 'pending'` and the current run's + * messageId, the deferred render with the resolved record. The preview update emit + * must use the CURRENT run's messageId (not the persisted DB one) + * because `processCodeOutput` intentionally preserves the original + * `messageId` on cross-turn filename reuse — `getCodeGeneratedFiles` + * needs that for prior-turn priming. + * + * Codex P1 review on PR #12957: shipping `updated.messageId` + * straight from the DB record routed preview-update patches to the wrong + * message slot, leaving the current turn's pending chip stuck. */ + + const { processCodeOutput } = require('~/server/services/Files/Code/process'); + + function makeCodeExecutionEvent({ runId, threadId, toolCallId, fileId, name }) { + return { + output: { + name: 'execute_code', + tool_call_id: toolCallId, + artifact: { + session_id: 'sess-1', + files: [{ id: fileId, name, session_id: 'sess-1' }], + }, + }, + metadata: { run_id: runId, thread_id: threadId }, + }; + } + + /** Parse the SSE frame `res.write` produces back to a payload object. */ + function parseSseAttachment(call) { + const frame = call[0]; + const dataLine = frame.split('\n').find((l) => l.startsWith('data: ')); + return JSON.parse(dataLine.slice('data: '.length)); + } + + it('the preview update emit uses the current run messageId, not the persisted DB messageId (cross-turn filename reuse)', async () => { + /* Simulate turn-2 reusing `output.csv` from turn-1. The DB record + * surfaced by `updateFile` carries the original `turn-1-msg` + * messageId; the runtime emit must rewrite to `turn-2-msg`. */ + res.headersSent = true; + const finalize = jest.fn().mockResolvedValue({ + file_id: 'fid-shared', + filename: 'output.csv', + filepath: '/uploads/output.csv', + type: 'text/csv', + conversationId: 'thread789', + messageId: 'turn-1-original-msg', // persisted DB id (older turn) + status: 'ready', + text: '
', + textFormat: 'html', + }); + processCodeOutput.mockResolvedValue({ + file: { + file_id: 'fid-shared', + filename: 'output.csv', + filepath: '/uploads/output.csv', + type: 'text/csv', + conversationId: 'thread789', + messageId: 'turn-2-current-run', // runtime overlay (current turn) + toolCallId: 'tool-2', + status: 'pending', + text: null, + textFormat: null, + }, + finalize, + }); + + const toolEndCallback = createToolEndCallback({ req, res, artifactPromises }); + const event = makeCodeExecutionEvent({ + runId: 'turn-2-current-run', + threadId: 'thread789', + toolCallId: 'tool-2', + fileId: 'fid-shared', + name: 'output.csv', + }); + await toolEndCallback({ output: event.output }, event.metadata); + await Promise.all(artifactPromises); + // Wait one more tick so the fire-and-forget finalize() chain settles. + await new Promise((resolve) => setImmediate(resolve)); + + // Two SSE writes: the initial emit (pending) and the deferred render (ready). + expect(res.write).toHaveBeenCalledTimes(2); + const phase1 = parseSseAttachment(res.write.mock.calls[0]); + const phase2 = parseSseAttachment(res.write.mock.calls[1]); + + // Initial emit already used the runtime messageId (sourced from result.file). + expect(phase1.messageId).toBe('turn-2-current-run'); + expect(phase1.status).toBe('pending'); + + /* The preview update MUST also route to the current run's messageId so the + * frontend's `useAttachmentHandler` upserts under the same + * messageAttachmentsMap slot as the initial emit. Routing to + * `turn-1-original-msg` would land the patch on a stale message + * and leave turn-2's pending chip stuck. */ + expect(phase2.messageId).toBe('turn-2-current-run'); + expect(phase2.file_id).toBe('fid-shared'); + expect(phase2.status).toBe('ready'); + expect(phase2.text).toBe('
'); + expect(phase2.toolCallId).toBe('tool-2'); + /* Wire-shape parity with the initial emit: preview update emits the full updated + * record so the client doesn't see one shape on the initial emit and a + * narrower projection on the deferred render. (Codex audit on PR #12957 + * Finding 1.) */ + expect(phase2.filename).toBe('output.csv'); + expect(phase2.filepath).toBe('/uploads/output.csv'); + expect(phase2.type).toBe('text/csv'); + expect(phase2.conversationId).toBe('thread789'); + expect(phase2.textFormat).toBe('html'); + }); + + it('the preview update emit is skipped when finalize resolves to null (no DB update happened)', async () => { + res.headersSent = true; + processCodeOutput.mockResolvedValue({ + file: { + file_id: 'fid-1', + filename: 'data.xlsx', + filepath: '/uploads/data.xlsx', + type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + messageId: 'run-1', + toolCallId: 'tool-1', + status: 'pending', + }, + finalize: jest.fn().mockResolvedValue(null), + }); + + const toolEndCallback = createToolEndCallback({ req, res, artifactPromises }); + const event = makeCodeExecutionEvent({ + runId: 'run-1', + threadId: 'thread-1', + toolCallId: 'tool-1', + fileId: 'fid-1', + name: 'data.xlsx', + }); + await toolEndCallback({ output: event.output }, event.metadata); + await Promise.all(artifactPromises); + await new Promise((resolve) => setImmediate(resolve)); + + // Only the initial emit fired; preview update noop'd because finalize returned null. + expect(res.write).toHaveBeenCalledTimes(1); + }); + + it('the preview update emit is skipped when the response stream has already closed', async () => { + res.headersSent = true; + /* Hand-rolled deferred so we can hold finalize() open until + * AFTER setting `res.writableEnded = true`. Otherwise the mock + * resolves synchronously, the .then() runs in the same microtask + * queue as the artifactPromises await, and writableEnded is set + * too late. */ + let resolveFinalize; + const finalizeDeferred = new Promise((resolve) => { + resolveFinalize = resolve; + }); + processCodeOutput.mockResolvedValue({ + file: { + file_id: 'fid-1', + filename: 'data.xlsx', + filepath: '/uploads/data.xlsx', + type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + messageId: 'run-1', + toolCallId: 'tool-1', + status: 'pending', + }, + finalize: jest.fn().mockReturnValue(finalizeDeferred), + }); + + const toolEndCallback = createToolEndCallback({ req, res, artifactPromises }); + const event = makeCodeExecutionEvent({ + runId: 'run-1', + threadId: 'thread-1', + toolCallId: 'tool-1', + fileId: 'fid-1', + name: 'data.xlsx', + }); + await toolEndCallback({ output: event.output }, event.metadata); + await Promise.all(artifactPromises); + // Simulate the response closing AFTER the initial emit fires but BEFORE + // the deferred render lands. The frontend's polling path will catch the + // resolved record on its next tick. + res.writableEnded = true; + // Now resolve finalize and let the .then() chain run. + resolveFinalize({ + file_id: 'fid-1', + filename: 'data.xlsx', + messageId: 'run-1', + status: 'ready', + text: '', + textFormat: 'html', + }); + await new Promise((resolve) => setImmediate(resolve)); + + // Initial emit wrote; preview update noop'd because writableEnded. + expect(res.write).toHaveBeenCalledTimes(1); + }); + + it('does not call finalize for a non-office file (no preview expected)', async () => { + res.headersSent = true; + processCodeOutput.mockResolvedValue({ + file: { + file_id: 'fid-txt', + filename: 'note.txt', + filepath: '/uploads/note.txt', + type: 'text/plain', + messageId: 'run-1', + toolCallId: 'tool-1', + // No status — non-office files skip the deferred render entirely. + }, + // No finalize key — caller should not call anything. + }); + + const toolEndCallback = createToolEndCallback({ req, res, artifactPromises }); + const event = makeCodeExecutionEvent({ + runId: 'run-1', + threadId: 'thread-1', + toolCallId: 'tool-1', + fileId: 'fid-txt', + name: 'note.txt', + }); + await toolEndCallback({ output: event.output }, event.metadata); + await Promise.all(artifactPromises); + await new Promise((resolve) => setImmediate(resolve)); + + expect(res.write).toHaveBeenCalledTimes(1); + }); + }); +}); + +describe('isStreamWritable', () => { + /* Direct parametric coverage of the predicate that gates SSE writes + * in both the chat-completions and Open Responses callbacks. The + * existing deferred-preview tests cover this indirectly via the + * `writeAttachmentUpdate` writableEnded path; these tests pin down + * each individual branch so a future modification (e.g. adding a + * new condition) can't silently regress. + * (Comprehensive review NIT on PR #12957.) */ + const { isStreamWritable } = require('../callbacks'); + + it('returns true when streamId is truthy regardless of res state', () => { + /* Resumable mode writes go to the job emitter; res state is + * irrelevant. Even a closed res with no headers should not block. */ + expect(isStreamWritable(null, 'stream-1')).toBe(true); + expect(isStreamWritable({ headersSent: false, writableEnded: true }, 'stream-1')).toBe(true); + expect(isStreamWritable(undefined, 'stream-1')).toBe(true); + }); + + it('returns false when streamId is falsy and res is null/undefined', () => { + expect(isStreamWritable(null, null)).toBe(false); + expect(isStreamWritable(undefined, null)).toBe(false); + }); + + it('returns false when headers have not been sent yet', () => { + expect(isStreamWritable({ headersSent: false, writableEnded: false }, null)).toBe(false); + }); + + it('returns false when the stream has already ended', () => { + expect(isStreamWritable({ headersSent: true, writableEnded: true }, null)).toBe(false); + }); + + it('returns true on the happy path: headers sent, not ended, no streamId', () => { + expect(isStreamWritable({ headersSent: true, writableEnded: false }, null)).toBe(true); + }); }); diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 2af4d5b45198..c9612c6b6248 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -15,7 +15,7 @@ const { createToolExecuteHandler, } = require('@librechat/api'); const { processFileCitations } = require('~/server/services/Files/Citations'); -const { processCodeOutput } = require('~/server/services/Files/Code/process'); +const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process'); const { saveBase64Image } = require('~/server/services/Files/process'); class ModelEndHandler { @@ -397,6 +397,55 @@ function writeAttachment(res, streamId, attachment) { } } +/** + * Predicate: is it safe to push an SSE write to the caller right now? + * + * In `streamId` (resumable) mode, writes go to the job emitter and the + * `res` state is irrelevant — always writable. + * + * In standard mode, the caller's `res` must have headers sent (the + * stream has been opened) and not yet be `writableEnded` (the response + * hasn't closed). Writing to a closed stream raises + * `ERR_STREAM_WRITE_AFTER_END`. + * + * Used by deferred preview emits in both `createToolEndCallback` + * (chat-completions) and `createResponsesToolEndCallback` (Open + * Responses) so the gate logic stays in one place. (Comprehensive + * review #3 on PR #12957.) + */ +function isStreamWritable(res, streamId) { + if (streamId) { + return true; + } + return !!res && res.headersSent && !res.writableEnded; +} + +/** + * Emit an update for an attachment that was previously sent with + * `status: 'pending'`. Fire-and-forget: if the response stream has + * already closed (the agent finished generating before the deferred + * preview resolved) the frontend's React Query polling on + * `/api/files/:file_id/preview` picks up the resolved record on its + * next tick. Skipping the write in that case avoids + * `ERR_STREAM_WRITE_AFTER_END`. + * + * Reuses the `attachment` SSE event name with a discriminated payload: + * the frontend's `useAttachmentHandler` upserts by `file_id`, so a + * second event with the same id and `status: 'ready' | 'failed'` + * overwrites the pending placeholder in place. No new event type, no + * new client listener. + * + * @param {ServerResponse} res + * @param {string | null} streamId + * @param {Object} attachment - Updated attachment payload (must carry `file_id`). + */ +function writeAttachmentUpdate(res, streamId, attachment) { + if (!isStreamWritable(res, streamId)) { + return; + } + writeAttachment(res, streamId, attachment); +} + /** * * @param {Object} params @@ -556,14 +605,15 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null }) continue; } const { id, name } = file; + const toolCallId = output.tool_call_id; artifactPromises.push( (async () => { - const fileMetadata = await processCodeOutput({ + const result = await processCodeOutput({ req, id, name, messageId: metadata.run_id, - toolCallId: output.tool_call_id, + toolCallId, conversationId: metadata.thread_id, /** * Use the FILE's session_id (storage session), not the @@ -583,15 +633,54 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null }) */ session_id: file.session_id ?? output.artifact.session_id, }); - if (!streamId && !res.headersSent) { - return fileMetadata; - } - + const fileMetadata = result?.file ?? null; + const finalize = result?.finalize; if (!fileMetadata) { return null; } - - writeAttachment(res, streamId, fileMetadata); + /* Initial emit: ship the attachment to the client immediately + * (carries `status: 'pending'` for office buckets so the UI + * shows "preparing preview…"). The agent's response stops + * blocking on extraction here. + * + * Use the shared `isStreamWritable` predicate rather than the + * narrower `streamId || res.headersSent` check that lived + * here before — a client disconnect mid-stream + * (`res.writableEnded`) would otherwise hit `res.write` and + * raise `ERR_STREAM_WRITE_AFTER_END` (caught by the outer + * IIFE catch but logged as noise). Same gate the Responses + * path uses below. */ + if (isStreamWritable(res, streamId)) { + writeAttachment(res, streamId, fileMetadata); + } + /* Deferred preview rendering: extraction continues running + * even after the HTTP response closes. If the stream is still + * open when the preview resolves, push an `attachment` + * update event so the UI patches in place; otherwise React + * Query polling on `/api/files/:file_id/preview` picks it up. + * + * Spread the full updated record (mirroring the initial emit + * shape) and overlay `messageId`/`toolCallId` from the + * current run. The DB record preserves the original + * `messageId` across cross-turn filename reuse so + * `getCodeGeneratedFiles` can trace the file back to its + * original assistant message; routing the update SSE by the + * persisted id would land the patch on a stale message + * slot — turn-N's pending placeholder would stay stuck while + * turn-1's already-resolved attachment got re-merged. + * (Codex P1 review on PR #12957.) */ + runPreviewFinalize({ + finalize, + fileId: fileMetadata.file_id, + previewRevision: result?.previewRevision, + onResolved: (updated) => { + writeAttachmentUpdate(res, streamId, { + ...updated, + messageId: metadata.run_id, + toolCallId, + }); + }, + }); return fileMetadata; })().catch((error) => { logger.error('Error processing code output:', error); @@ -782,14 +871,15 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) continue; } const { id, name } = file; + const toolCallId = output.tool_call_id; artifactPromises.push( (async () => { - const fileMetadata = await processCodeOutput({ + const result = await processCodeOutput({ req, id, name, messageId: metadata.run_id, - toolCallId: output.tool_call_id, + toolCallId, conversationId: metadata.thread_id, /** * Use the FILE's session_id (storage session), not the @@ -809,25 +899,45 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) */ session_id: file.session_id ?? output.artifact.session_id, }); - + const fileMetadata = result?.file ?? null; + const finalize = result?.finalize; if (!fileMetadata) { return null; } - // For Responses API, emit attachment during streaming - if (res.headersSent && !res.writableEnded) { - const attachment = { - file_id: fileMetadata.file_id, - filename: fileMetadata.filename, - type: fileMetadata.type, - url: fileMetadata.filepath, - width: fileMetadata.width, - height: fileMetadata.height, - tool_call_id: output.tool_call_id, - }; - writeResponsesAttachment(res, tracker, attachment, metadata); + /* Initial emit (Open Responses extension format). The agent's + * response no longer blocks on extraction. */ + if (isStreamWritable(res, null)) { + writeResponsesAttachment( + res, + tracker, + buildResponsesAttachment(fileMetadata, toolCallId), + metadata, + ); } + /* Deferred preview rendering: extract HTML in the background + * and emit a follow-up `librechat:attachment` with the same + * `file_id` so the client merges the resolved record over the + * pending placeholder. Fire-and-forget — survives response + * close; polling covers the post-close gap. */ + runPreviewFinalize({ + finalize, + fileId: fileMetadata.file_id, + previewRevision: result?.previewRevision, + onResolved: (updated) => { + if (!isStreamWritable(res, null)) { + return; + } + writeResponsesAttachment( + res, + tracker, + buildResponsesAttachment(updated, toolCallId), + metadata, + ); + }, + }); + return fileMetadata; })().catch((error) => { logger.error('Error processing code output:', error); @@ -838,6 +948,28 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) }; } +/** + * Project a file metadata record into the Open Responses attachment + * shape. Mirrors the legacy inline projection but adds `status` and + * `previewError` so deferred preview updates carry the lifecycle + * signal the client uses to upsert by `file_id`. + */ +function buildResponsesAttachment(fileMetadata, toolCallId) { + return { + file_id: fileMetadata.file_id, + filename: fileMetadata.filename, + type: fileMetadata.type, + url: fileMetadata.filepath, + width: fileMetadata.width, + height: fileMetadata.height, + tool_call_id: toolCallId, + text: fileMetadata.text ?? null, + textFormat: fileMetadata.textFormat ?? null, + status: fileMetadata.status, + previewError: fileMetadata.previewError, + }; +} + const ALLOWED_LOG_LEVELS = new Set(['debug', 'info', 'warn', 'error']); function agentLogHandler(_event, data) { @@ -893,6 +1025,7 @@ module.exports = { agentLogHandlerObj, getDefaultHandlers, createToolEndCallback, + isStreamWritable, markSummarizationUsage, buildSummarizationHandlers, createResponsesToolEndCallback, diff --git a/api/server/controllers/auth/LogoutController.js b/api/server/controllers/auth/LogoutController.js index 381bfc58b2db..ae1c94a7c9aa 100644 --- a/api/server/controllers/auth/LogoutController.js +++ b/api/server/controllers/auth/LogoutController.js @@ -1,5 +1,5 @@ const cookies = require('cookie'); -const { isEnabled } = require('@librechat/api'); +const { isEnabled, clearCloudFrontCookies } = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { logoutUser } = require('~/server/services/AuthService'); const { getOpenIdConfig } = require('~/strategies'); @@ -44,6 +44,7 @@ const logoutController = async (req, res) => { res.clearCookie('openid_id_token'); res.clearCookie('openid_user_id'); res.clearCookie('token_provider'); + clearCloudFrontCookies(res); const response = { message }; if ( isOpenIdUser && diff --git a/api/server/controllers/auth/LogoutController.spec.js b/api/server/controllers/auth/LogoutController.spec.js index c9294fdcec67..ff02f5237e6d 100644 --- a/api/server/controllers/auth/LogoutController.spec.js +++ b/api/server/controllers/auth/LogoutController.spec.js @@ -4,9 +4,13 @@ const mockLogoutUser = jest.fn(); const mockLogger = { warn: jest.fn(), error: jest.fn(), debug: jest.fn() }; const mockIsEnabled = jest.fn(); const mockGetOpenIdConfig = jest.fn(); +const mockClearCloudFrontCookies = jest.fn(); jest.mock('cookie'); -jest.mock('@librechat/api', () => ({ isEnabled: (...args) => mockIsEnabled(...args) })); +jest.mock('@librechat/api', () => ({ + isEnabled: (...args) => mockIsEnabled(...args), + clearCloudFrontCookies: (...args) => mockClearCloudFrontCookies(...args), +})); jest.mock('@librechat/data-schemas', () => ({ logger: mockLogger })); jest.mock('~/server/services/AuthService', () => ({ logoutUser: (...args) => mockLogoutUser(...args), @@ -255,6 +259,15 @@ describe('LogoutController', () => { expect(res.clearCookie).toHaveBeenCalledWith('openid_user_id'); expect(res.clearCookie).toHaveBeenCalledWith('token_provider'); }); + + it('calls clearCloudFrontCookies on successful logout', async () => { + const req = buildReq(); + const res = buildRes(); + + await logoutController(req, res); + + expect(mockClearCloudFrontCookies).toHaveBeenCalledWith(res); + }); }); describe('URL length limit and logout_hint fallback', () => { diff --git a/api/server/controllers/tools.js b/api/server/controllers/tools.js index 8124894584cc..07be1210c141 100644 --- a/api/server/controllers/tools.js +++ b/api/server/controllers/tools.js @@ -10,7 +10,7 @@ const { } = require('librechat-data-provider'); const { getRoleByName, createToolCall, getToolCallsByConvo, getMessage } = require('~/models'); const { processFileURL, uploadImageBuffer } = require('~/server/services/Files/process'); -const { processCodeOutput } = require('~/server/services/Files/Code/process'); +const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process'); const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { loadTools } = require('~/app/clients/tools/util'); @@ -192,7 +192,7 @@ const callTool = async (req, res) => { const { id, name } = file; artifactPromises.push( (async () => { - const fileMetadata = await processCodeOutput({ + const result = await processCodeOutput({ req, id, name, @@ -201,11 +201,22 @@ const callTool = async (req, res) => { conversationId, session_id: artifact.session_id, }); - + const fileMetadata = result?.file ?? null; + const finalize = result?.finalize; if (!fileMetadata) { return null; } - + /* This endpoint is non-streaming and its contract is "give + * me the artifacts" — return the persisted record immediately + * (with `status: 'pending'` for office buckets) and run the + * preview render in the background. The client polls + * `/api/files/:file_id/preview` for the resolved record. + * No `onResolved` — there's no live stream to write to here. */ + runPreviewFinalize({ + finalize, + fileId: fileMetadata.file_id, + previewRevision: result?.previewRevision, + }); return fileMetadata; })().catch((error) => { logger.error('Error processing code output:', error); diff --git a/api/server/experimental.js b/api/server/experimental.js index cd594c169676..b12b9deffe1e 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -10,7 +10,7 @@ const express = require('express'); const passport = require('passport'); const compression = require('compression'); const cookieParser = require('cookie-parser'); -const { logger } = require('@librechat/data-schemas'); +const { logger, runAsSystem } = require('@librechat/data-schemas'); const mongoSanitize = require('express-mongo-sanitize'); const { isEnabled, @@ -26,7 +26,12 @@ const initializeOAuthReconnectManager = require('./services/initializeOAuthRecon const createValidateImageRequest = require('./middleware/validateImageRequest'); const { jwtLogin, ldapLogin, passportLogin } = require('~/strategies'); const { updateInterfacePermissions: updateInterfacePerms } = require('@librechat/api'); -const { getRoleByName, updateAccessPermissions, seedDatabase } = require('~/models'); +const { + getRoleByName, + updateAccessPermissions, + seedDatabase, + sweepOrphanedPreviews, +} = require('~/models'); const { checkMigrations } = require('./services/start/migration'); const initializeMCPs = require('./services/initializeMCPs'); const configureSocialLogins = require('./socialLogins'); @@ -220,6 +225,11 @@ if (cluster.isMaster) { /** Seed database (idempotent) */ await seedDatabase(); + /* Mirrors `server/index.js`; `runAsSystem` for tenant-isolated File. */ + runAsSystem(sweepOrphanedPreviews).catch((err) => { + logger.error('[sweepOrphanedPreviews] Background sweep failed:', err); + }); + /** Initialize app configuration */ const appConfig = await getAppConfig(); initializeFileStorage(appConfig); diff --git a/api/server/index.js b/api/server/index.js index d798f1a1661a..6bc4a131e6ad 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -25,7 +25,12 @@ const { } = require('@librechat/api'); const { connectDb, indexSync } = require('~/db'); const initializeOAuthReconnectManager = require('./services/initializeOAuthReconnectManager'); -const { getRoleByName, updateAccessPermissions, seedDatabase } = require('~/models'); +const { + getRoleByName, + updateAccessPermissions, + seedDatabase, + sweepOrphanedPreviews, +} = require('~/models'); const { capabilityContextMiddleware } = require('./middleware/roles/capabilities'); const createValidateImageRequest = require('./middleware/validateImageRequest'); const { jwtLogin, ldapLogin, passportLogin } = require('~/strategies'); @@ -69,6 +74,13 @@ const startServer = async () => { } await runAsSystem(seedDatabase); + /* Recover stuck `status: 'pending'` records from a crash mid-render. + * `runAsSystem` is required — `File` is tenant-isolated and strict + * mode rejects unscoped queries. Lazy sweep in the preview endpoint + * covers anything younger than the boot cutoff. */ + runAsSystem(sweepOrphanedPreviews).catch((err) => { + logger.error('[sweepOrphanedPreviews] Background sweep failed:', err); + }); const appConfig = await getAppConfig({ baseOnly: true }); initializeFileStorage(appConfig); await runAsSystem(async () => { diff --git a/api/server/routes/agents/middleware.js b/api/server/routes/agents/middleware.js new file mode 100644 index 000000000000..f71c25c6f8fd --- /dev/null +++ b/api/server/routes/agents/middleware.js @@ -0,0 +1,41 @@ +const { PermissionTypes, Permissions } = require('librechat-data-provider'); +const { + generateCheckAccess, + preAuthTenantMiddleware, + createRequireApiKeyAuth, + createRemoteAgentAuth, + createCheckRemoteAgentAccess, +} = require('@librechat/api'); +const { getEffectivePermissions } = require('~/server/services/PermissionService'); +const { getAppConfig } = require('~/server/services/Config'); +const db = require('~/models'); + +const apiKeyMiddleware = createRequireApiKeyAuth({ + validateAgentApiKey: db.validateAgentApiKey, + findUser: db.findUser, +}); + +const requireRemoteAgentAuth = createRemoteAgentAuth({ + apiKeyMiddleware, + findUser: db.findUser, + updateUser: db.updateUser, + getAppConfig, +}); + +const checkRemoteAgentsFeature = generateCheckAccess({ + permissionType: PermissionTypes.REMOTE_AGENTS, + permissions: [Permissions.USE], + getRoleByName: db.getRoleByName, +}); + +const checkAgentPermission = createCheckRemoteAgentAccess({ + getAgent: db.getAgent, + getEffectivePermissions, +}); + +module.exports = { + checkAgentPermission, + preAuthTenantMiddleware, + requireRemoteAgentAuth, + checkRemoteAgentsFeature, +}; diff --git a/api/server/routes/agents/openai.js b/api/server/routes/agents/openai.js index 72e3da6c5a0e..fa7f9b26c811 100644 --- a/api/server/routes/agents/openai.js +++ b/api/server/routes/agents/openai.js @@ -17,40 +17,23 @@ * } */ const express = require('express'); -const { PermissionTypes, Permissions } = require('librechat-data-provider'); -const { - generateCheckAccess, - createRequireApiKeyAuth, - createCheckRemoteAgentAccess, -} = require('@librechat/api'); const { OpenAIChatCompletionController, ListModelsController, GetModelController, } = require('~/server/controllers/agents/openai'); -const { getEffectivePermissions } = require('~/server/services/PermissionService'); const { configMiddleware } = require('~/server/middleware'); -const db = require('~/models'); +const { + checkAgentPermission, + preAuthTenantMiddleware, + requireRemoteAgentAuth, + checkRemoteAgentsFeature, +} = require('./middleware'); const router = express.Router(); -const requireApiKeyAuth = createRequireApiKeyAuth({ - validateAgentApiKey: db.validateAgentApiKey, - findUser: db.findUser, -}); - -const checkRemoteAgentsFeature = generateCheckAccess({ - permissionType: PermissionTypes.REMOTE_AGENTS, - permissions: [Permissions.USE], - getRoleByName: db.getRoleByName, -}); - -const checkAgentPermission = createCheckRemoteAgentAccess({ - getAgent: db.getAgent, - getEffectivePermissions, -}); - -router.use(requireApiKeyAuth); +router.use(preAuthTenantMiddleware); +router.use(requireRemoteAgentAuth); router.use(configMiddleware); router.use(checkRemoteAgentsFeature); diff --git a/api/server/routes/agents/responses.js b/api/server/routes/agents/responses.js index 2c118e059712..401025bfd62a 100644 --- a/api/server/routes/agents/responses.js +++ b/api/server/routes/agents/responses.js @@ -20,40 +20,23 @@ * @see https://openresponses.org/specification */ const express = require('express'); -const { PermissionTypes, Permissions } = require('librechat-data-provider'); -const { - generateCheckAccess, - createRequireApiKeyAuth, - createCheckRemoteAgentAccess, -} = require('@librechat/api'); const { createResponse, getResponse, listModels, } = require('~/server/controllers/agents/responses'); -const { getEffectivePermissions } = require('~/server/services/PermissionService'); const { configMiddleware } = require('~/server/middleware'); -const db = require('~/models'); +const { + checkAgentPermission, + preAuthTenantMiddleware, + requireRemoteAgentAuth, + checkRemoteAgentsFeature, +} = require('./middleware'); const router = express.Router(); -const requireApiKeyAuth = createRequireApiKeyAuth({ - validateAgentApiKey: db.validateAgentApiKey, - findUser: db.findUser, -}); - -const checkRemoteAgentsFeature = generateCheckAccess({ - permissionType: PermissionTypes.REMOTE_AGENTS, - permissions: [Permissions.USE], - getRoleByName: db.getRoleByName, -}); - -const checkAgentPermission = createCheckRemoteAgentAccess({ - getAgent: db.getAgent, - getEffectivePermissions, -}); - -router.use(requireApiKeyAuth); +router.use(preAuthTenantMiddleware); +router.use(requireRemoteAgentAuth); router.use(configMiddleware); router.use(checkRemoteAgentsFeature); diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index 5c26f65b81c9..285f78390971 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -295,6 +295,86 @@ router.get('/code/download/:session_id/:fileId', async (req, res) => { } }); +/* Lazy-sweep cutoff: pending records older than this are marked failed + * on the next poll. 2min is well past the 60s render ceiling, so any + * `pending` past it is definitively orphaned. Tighter than the boot + * sweep (5min) since this runs per-request, not per-instance. */ +const PREVIEW_LAZY_SWEEP_CUTOFF_MS = 2 * 60 * 1000; + +/** + * Poll the lifecycle status of a code-execution file's inline preview. + * + * Deferred-preview flow: the immediate persist step writes the file + * record at `status: 'pending'`; the background render transitions + * it to `'ready'` (with `text` + `textFormat`) or `'failed'` (with + * `previewError`). The frontend's `useFilePreview` React Query hook + * polls this endpoint at ~2.5s intervals while `status === 'pending'`, + * then auto-stops on terminal status. + * + * Returns the smallest viable shape: + * - `status` always present (defaults to `'ready'` for legacy records + * that never had the field — clients treat absent as ready). + * - `text` and `textFormat` only when status is 'ready' AND text + * is non-null (preserves the security contract from PR #12934 — + * office bucket files MUST NOT receive plain-text fallbacks). + * - `previewError` only when status is 'failed'. + * + * Lazy-sweeps stale `pending` records on the spot — see + * `PREVIEW_LAZY_SWEEP_CUTOFF_MS` for the rationale. + * + * Reuses the `fileAccess` middleware so ACL is identical to download. + * + * @route GET /files/:file_id/preview + */ +router.get('/:file_id/preview', fileAccess, async (req, res) => { + try { + const { file_id } = req.params; + /* `fileAccess` already fetched the record (sans `text`, the default + * projection drops it). Reuse for the lifecycle check; only re-fetch + * with `text` on a terminal ready response — the typical lifecycle + * is N pending polls + 1 ready, so this avoids ~N redundant text + * reads per file. */ + let file = req.fileAccess.file; + /* Lazy sweep: if stuck `pending` past the cutoff, mark `failed` + * conditional on the observed `updatedAt` (concurrent legitimate + * updates win). */ + if (file.status === 'pending' && file.updatedAt instanceof Date) { + const ageMs = Date.now() - file.updatedAt.getTime(); + if (ageMs > PREVIEW_LAZY_SWEEP_CUTOFF_MS) { + const swept = await db.updateFile( + { file_id, status: 'failed', previewError: 'orphaned' }, + { status: 'pending', updatedAt: file.updatedAt }, + ); + if (swept) { + file = swept; + logger.info( + `[/files/:file_id/preview] Lazy-swept orphaned pending record ${file_id} (age ${Math.round(ageMs / 1000)}s)`, + ); + } + } + } + /* Default to 'ready' for back-compat: legacy records pre-date the + * field, and non-office files never get a status set on persist. */ + const status = file.status ?? 'ready'; + const payload = { file_id, status }; + if (status === 'ready') { + const withText = await db.findFileById(file_id); + if (withText?.text != null) { + payload.text = withText.text; + payload.textFormat = withText.textFormat ?? null; + } + } else if (status === 'failed' && file.previewError) { + payload.previewError = file.previewError; + } + return res.status(200).json(payload); + } catch (error) { + logger.error('[/files/:file_id/preview] Error fetching preview status:', error); + return res + .status(500) + .json({ error: 'Internal Server Error', message: 'Failed to fetch preview status' }); + } +}); + router.get('/download/:userId/:file_id', fileAccess, async (req, res) => { try { const { userId, file_id } = req.params; diff --git a/api/server/routes/files/preview.spec.js b/api/server/routes/files/preview.spec.js new file mode 100644 index 000000000000..f86c128d4bff --- /dev/null +++ b/api/server/routes/files/preview.spec.js @@ -0,0 +1,372 @@ +/** + * Coverage for the new GET /files/:file_id/preview endpoint. + * + * Deferred-preview code-execution flow: the immediate persist step + * emits a file record at `status: 'pending'`; the background render + * transitions it to `'ready'` (with text) or `'failed'` (with + * previewError). The frontend polls this endpoint until status is + * terminal. This suite asserts the response shape across all four + * states (pending, ready, failed, legacy/back-compat) and the auth + * boundary (404 vs 403). + */ + +jest.mock('@librechat/data-schemas', () => ({ + logger: { warn: jest.fn(), debug: jest.fn(), error: jest.fn(), info: jest.fn() }, + SystemCapabilities: {}, +})); + +jest.mock('@librechat/api', () => ({ + refreshS3FileUrls: jest.fn(), + resolveUploadErrorMessage: jest.fn(), + verifyAgentUploadPermission: jest.fn(), +})); + +const mockFindFileById = jest.fn(); +const mockGetFiles = jest.fn(); +const mockUpdateFile = jest.fn(); +jest.mock('~/models', () => ({ + findFileById: (...args) => mockFindFileById(...args), + getFiles: (...args) => mockGetFiles(...args), + updateFile: (...args) => mockUpdateFile(...args), + getAgents: jest.fn().mockResolvedValue([]), + batchUpdateFiles: jest.fn(), +})); + +jest.mock('~/server/services/Files/process', () => ({ + filterFile: jest.fn(), + processFileUpload: jest.fn(), + processDeleteRequest: jest.fn(), + processAgentFileUpload: jest.fn(), +})); + +jest.mock('~/server/services/Files/strategies', () => ({ + getStrategyFunctions: jest.fn(() => ({})), +})); + +jest.mock('~/server/controllers/assistants/helpers', () => ({ + getOpenAIClient: jest.fn(), +})); + +jest.mock('~/server/middleware/roles/capabilities', () => ({ + hasCapability: jest.fn(() => (_req, _res, next) => next()), +})); + +jest.mock('~/server/services/PermissionService', () => ({ + checkPermission: jest.fn(() => (_req, _res, next) => next()), + getEffectivePermissions: jest.fn().mockResolvedValue(0), +})); + +jest.mock('~/server/services/Files', () => ({ + hasAccessToFilesViaAgent: jest.fn(), +})); + +jest.mock('~/server/utils/files', () => ({ + cleanFileName: (name) => name, +})); + +jest.mock('~/cache', () => ({ + getLogStores: jest.fn(() => ({ get: jest.fn(), set: jest.fn() })), +})); + +const express = require('express'); +const request = require('supertest'); +const filesRouter = require('./files'); + +/** + * Mount the router with a per-request user injector so we can simulate + * a logged-in user without spinning up the full auth stack. + */ +function buildApp({ user = { id: 'user-123', role: 'user' } } = {}) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.user = user; + req.config = { fileStrategy: 'local' }; + next(); + }); + app.use('/files', filesRouter); + return app; +} + +const OWNER_USER_ID = 'user-123'; + +describe('GET /files/:file_id/preview', () => { + beforeEach(() => { + mockFindFileById.mockReset(); + mockGetFiles.mockReset(); + mockUpdateFile.mockReset(); + }); + + it('returns 404 when the file does not exist (auth check fails first via fileAccess)', async () => { + /* `fileAccess` middleware does its own getFiles lookup and returns + * 404 before our handler ever runs. This test asserts the boundary + * lives there, not that the handler duplicates the check. */ + mockGetFiles.mockResolvedValueOnce([]); + const res = await request(buildApp()).get('/files/missing-id/preview'); + expect(res.status).toBe(404); + expect(res.body).toMatchObject({ error: 'Not Found' }); + expect(mockFindFileById).not.toHaveBeenCalled(); + }); + + it('returns 403 when the requester does not own the file and has no agent-based access', async () => { + /* fileAccess returns 403 — the file exists but belongs to someone + * else and no agent grants access. The preview handler should + * never run. */ + mockGetFiles.mockResolvedValueOnce([ + { file_id: 'someone-elses', user: 'other-user', filename: 'x.xlsx' }, + ]); + const res = await request(buildApp()).get('/files/someone-elses/preview'); + expect(res.status).toBe(403); + expect(mockFindFileById).not.toHaveBeenCalled(); + }); + + it('returns status:pending without text/textFormat while the deferred render is in flight', async () => { + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-pending', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'pending', + }, + ]); + const res = await request(buildApp()).get('/files/fid-pending/preview'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ file_id: 'fid-pending', status: 'pending' }); + /* Pending must NOT leak `text` and must NOT trigger the text re-fetch. */ + expect(res.body).not.toHaveProperty('text'); + expect(mockFindFileById).not.toHaveBeenCalled(); + }); + + it('returns status:ready with text + textFormat when the deferred render succeeded', async () => { + mockGetFiles.mockResolvedValueOnce([ + { file_id: 'fid-ready', user: OWNER_USER_ID, filename: 'data.xlsx', status: 'ready' }, + ]); + /* Text is fetched only on the terminal ready response. */ + mockFindFileById.mockResolvedValueOnce({ + file_id: 'fid-ready', + text: '
1
', + textFormat: 'html', + }); + const res = await request(buildApp()).get('/files/fid-ready/preview'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + file_id: 'fid-ready', + status: 'ready', + text: '
1
', + textFormat: 'html', + }); + }); + + it('returns status:failed with previewError when the deferred render errored', async () => { + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-failed', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'failed', + previewError: 'parser-error', + }, + ]); + const res = await request(buildApp()).get('/files/fid-failed/preview'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + file_id: 'fid-failed', + status: 'failed', + previewError: 'parser-error', + }); + expect(mockFindFileById).not.toHaveBeenCalled(); + }); + + it('defaults to status:ready for legacy records with no status field (back-compat)', async () => { + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-legacy', + user: OWNER_USER_ID, + filename: 'old.csv', + // status intentionally absent + }, + ]); + mockFindFileById.mockResolvedValueOnce({ + file_id: 'fid-legacy', + text: 'csv,header\n1,2', + textFormat: 'text', + }); + const res = await request(buildApp()).get('/files/fid-legacy/preview'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + file_id: 'fid-legacy', + status: 'ready', + text: 'csv,header\n1,2', + textFormat: 'text', + }); + }); + + it('returns status:ready with no text when the record is ready but text is null (binary/oversized)', async () => { + mockGetFiles.mockResolvedValueOnce([ + { file_id: 'fid-binary', user: OWNER_USER_ID, filename: 'image.bin' }, + ]); + mockFindFileById.mockResolvedValueOnce({ + file_id: 'fid-binary', + text: null, + textFormat: null, + }); + const res = await request(buildApp()).get('/files/fid-binary/preview'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ file_id: 'fid-binary', status: 'ready' }); + }); + + it('returns ready with no text when ready record was deleted between fileAccess and text fetch', async () => { + /* `fileAccess` saw the record but the concurrent delete removed it + * before the text fetch. Surface ready-without-text rather than + * 500 — the client routes to download-only and stops polling. */ + mockGetFiles.mockResolvedValueOnce([ + { file_id: 'fid-race', user: OWNER_USER_ID, filename: 'data.xlsx', status: 'ready' }, + ]); + mockFindFileById.mockResolvedValueOnce(null); + const res = await request(buildApp()).get('/files/fid-race/preview'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ file_id: 'fid-race', status: 'ready' }); + }); + + it('returns 500 with a stable shape if the text fetch throws unexpectedly', async () => { + mockGetFiles.mockResolvedValueOnce([ + { file_id: 'fid-boom', user: OWNER_USER_ID, filename: 'data.xlsx', status: 'ready' }, + ]); + mockFindFileById.mockRejectedValueOnce(new Error('mongo down')); + const res = await request(buildApp()).get('/files/fid-boom/preview'); + expect(res.status).toBe(500); + expect(res.body).toMatchObject({ error: 'Internal Server Error' }); + }); + + describe('lazy sweep for stale pending records', () => { + /* The boot-time `sweepOrphanedPreviews` only runs once at startup + * with a 5-min cutoff. A backend crash + quick restart can leave + * `pending` records younger than 5 min that never get touched + * again. This endpoint sweeps them on the spot whenever a polling + * request lands on one — the user is exactly the consumer who + * cares, so on-demand sweep is the right shape. (Codex P2 review + * on PR #12957.) */ + const STALE_MS = 6 * 60 * 1000; + const FRESH_MS = 30 * 1000; + + it('marks a stale pending record as failed:orphaned and returns the swept state', async () => { + const updatedAt = new Date(Date.now() - STALE_MS); + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-stale', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'pending', + updatedAt, + }, + ]); + mockUpdateFile.mockResolvedValueOnce({ + file_id: 'fid-stale', + status: 'failed', + previewError: 'orphaned', + }); + + const res = await request(buildApp()).get('/files/fid-stale/preview'); + + expect(mockUpdateFile).toHaveBeenCalledWith( + { file_id: 'fid-stale', status: 'failed', previewError: 'orphaned' }, + { status: 'pending', updatedAt }, + ); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + file_id: 'fid-stale', + status: 'failed', + previewError: 'orphaned', + }); + }); + + it('does NOT sweep a fresh pending record (within the cutoff window)', async () => { + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-fresh', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'pending', + updatedAt: new Date(Date.now() - FRESH_MS), + }, + ]); + + const res = await request(buildApp()).get('/files/fid-fresh/preview'); + + expect(mockUpdateFile).not.toHaveBeenCalled(); + expect(res.status).toBe(200); + expect(res.body).toEqual({ file_id: 'fid-fresh', status: 'pending' }); + }); + + it('sweeps a record past the 2min cutoff but below the 5min boot-sweep threshold', async () => { + /* Pins the cutoff change from 5min to 2min — without this, a + * future revert wouldn't fail the suite. */ + const updatedAt = new Date(Date.now() - 3 * 60 * 1000); + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-mid', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'pending', + updatedAt, + }, + ]); + mockUpdateFile.mockResolvedValueOnce({ + file_id: 'fid-mid', + status: 'failed', + previewError: 'orphaned', + }); + + const res = await request(buildApp()).get('/files/fid-mid/preview'); + + expect(mockUpdateFile).toHaveBeenCalled(); + expect(res.body).toEqual({ + file_id: 'fid-mid', + status: 'failed', + previewError: 'orphaned', + }); + }); + + it('does NOT sweep a stale ready record (only pending qualifies)', async () => { + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-ready', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'ready', + updatedAt: new Date(Date.now() - STALE_MS), + }, + ]); + mockFindFileById.mockResolvedValueOnce({ + file_id: 'fid-ready', + text: 'final', + textFormat: 'html', + }); + + const res = await request(buildApp()).get('/files/fid-ready/preview'); + + expect(mockUpdateFile).not.toHaveBeenCalled(); + expect(res.body).toMatchObject({ status: 'ready', text: 'final' }); + }); + + it('falls through to the original pending payload if the conditional sweep loses the race', async () => { + const updatedAt = new Date(Date.now() - STALE_MS); + mockGetFiles.mockResolvedValueOnce([ + { + file_id: 'fid-race', + user: OWNER_USER_ID, + filename: 'data.xlsx', + status: 'pending', + updatedAt, + }, + ]); + mockUpdateFile.mockResolvedValueOnce(null); + + const res = await request(buildApp()).get('/files/fid-race/preview'); + + expect(mockUpdateFile).toHaveBeenCalled(); + expect(res.status).toBe(200); + expect(res.body).toEqual({ file_id: 'fid-race', status: 'pending' }); + }); + }); +}); diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 816a0eac5b10..40b3c1a72526 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -11,6 +11,7 @@ const { math, isEnabled, checkEmailConfig, + setCloudFrontCookies, isEmailDomainAllowed, shouldUseSecureCookie, resolveAppConfigForUser, @@ -440,6 +441,9 @@ const setAuthTokens = async (userId, res, _session = null) => { secure: shouldUseSecureCookie(), sameSite: 'strict', }); + + setCloudFrontCookies(res); + return token; } catch (error) { logger.error('[setAuthTokens] Error in setting authentication tokens:', error); @@ -557,6 +561,9 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) = sameSite: 'strict', }); } + + setCloudFrontCookies(res); + return appAuthToken; } catch (error) { logger.error('[setOpenIDAuthTokens] Error in setting authentication tokens:', error); diff --git a/api/server/services/AuthService.spec.js b/api/server/services/AuthService.spec.js index c8abafdbe59f..df89f3f2c9c8 100644 --- a/api/server/services/AuthService.spec.js +++ b/api/server/services/AuthService.spec.js @@ -15,6 +15,7 @@ jest.mock('@librechat/api', () => ({ math: jest.fn((val, fallback) => (val ? Number(val) : fallback)), shouldUseSecureCookie: jest.fn(() => false), resolveAppConfigForUser: jest.fn(async (_getAppConfig, _user) => ({})), + setCloudFrontCookies: jest.fn(() => true), })); jest.mock('~/models', () => ({ findUser: jest.fn(), @@ -40,10 +41,17 @@ const { shouldUseSecureCookie, isEmailDomainAllowed, resolveAppConfigForUser, + setCloudFrontCookies, } = require('@librechat/api'); -const { findUser } = require('~/models'); +const { + findUser, + getUserById, + generateToken, + generateRefreshToken, + createSession, +} = require('~/models'); const { getAppConfig } = require('~/server/services/Config'); -const { setOpenIDAuthTokens, requestPasswordReset } = require('./AuthService'); +const { setOpenIDAuthTokens, requestPasswordReset, setAuthTokens } = require('./AuthService'); /** Helper to build a mock Express response */ function mockResponse() { @@ -339,3 +347,67 @@ describe('requestPasswordReset', () => { expect(result.message).toContain('If an account with that email exists'); }); }); + +describe('CloudFront cookie integration', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('setOpenIDAuthTokens', () => { + const validTokenset = { + id_token: 'the-id-token', + access_token: 'the-access-token', + refresh_token: 'the-refresh-token', + }; + + it('calls setCloudFrontCookies with response object', () => { + const req = mockRequest(); + const res = mockResponse(); + + setOpenIDAuthTokens(validTokenset, req, res, 'user-123'); + + expect(setCloudFrontCookies).toHaveBeenCalledWith(res); + }); + + it('succeeds even when setCloudFrontCookies returns false', () => { + setCloudFrontCookies.mockReturnValue(false); + + const req = mockRequest(); + const res = mockResponse(); + + const result = setOpenIDAuthTokens(validTokenset, req, res, 'user-123'); + + expect(result).toBe('the-id-token'); + }); + }); + + describe('setAuthTokens', () => { + beforeEach(() => { + getUserById.mockResolvedValue({ _id: 'user-123' }); + generateToken.mockResolvedValue('mock-access-token'); + generateRefreshToken.mockReturnValue('mock-refresh-token'); + createSession.mockResolvedValue({ + session: { expiration: new Date(Date.now() + 604800000) }, + refreshToken: 'mock-refresh-token', + }); + }); + + it('calls setCloudFrontCookies with response object', async () => { + const res = mockResponse(); + + await setAuthTokens('user-123', res); + + expect(setCloudFrontCookies).toHaveBeenCalledWith(res); + }); + + it('succeeds even when setCloudFrontCookies returns false', async () => { + setCloudFrontCookies.mockReturnValue(false); + + const res = mockResponse(); + + const result = await setAuthTokens('user-123', res); + + expect(result).toBe('mock-access-token'); + }); + }); +}); diff --git a/api/server/services/Files/Code/__tests__/process-traversal.spec.js b/api/server/services/Files/Code/__tests__/process-traversal.spec.js index 016f3a71c636..b6fcc2636f8c 100644 --- a/api/server/services/Files/Code/__tests__/process-traversal.spec.js +++ b/api/server/services/Files/Code/__tests__/process-traversal.spec.js @@ -27,6 +27,18 @@ jest.mock('@librechat/api', () => { createAxiosInstance: jest.fn(() => mockAxios), classifyCodeArtifact: jest.fn(() => 'other'), extractCodeArtifactText: jest.fn(async () => null), + /* `processCodeOutput` calls this to derive the trust flag persisted + * on `IMongoFile.textFormat` — Codex P1 review on PR #12934. The + * mock returns null in lockstep with the null `text` above so + * downstream consumers don't see a phantom format. */ + getExtractedTextFormat: jest.fn(() => null), + /* Pass-through `withTimeout`: this suite asserts traversal sanitization, + * not deferred preview timing. */ + withTimeout: async (promise) => promise, + /* These traversal cases all use non-office filenames — keep the + * inline (non-finalize) path so existing assertions on a single + * createFile call hold. */ + hasOfficeHtmlPath: jest.fn(() => false), codeServerHttpAgent: new http.Agent({ keepAlive: false }), codeServerHttpsAgent: new https.Agent({ keepAlive: false }), }; diff --git a/api/server/services/Files/Code/process.js b/api/server/services/Files/Code/process.js index 5ae78a7e9a9d..4efc938223ee 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -3,8 +3,10 @@ const { v4 } = require('uuid'); const { logger } = require('@librechat/data-schemas'); const { getCodeBaseURL } = require('@librechat/agents'); const { + withTimeout, getBasePath, logAxiosError, + hasOfficeHtmlPath, sanitizeArtifactPath, flattenArtifactPath, createAxiosInstance, @@ -12,6 +14,7 @@ const { codeServerHttpAgent, codeServerHttpsAgent, extractCodeArtifactText, + getExtractedTextFormat, } = require('@librechat/api'); const { Tools, @@ -68,8 +71,235 @@ const createDownloadFallback = ({ }; /** - * Process code execution output files - downloads and saves both images and non-image files. - * All files are saved to local storage with fileIdentifier metadata for code env re-upload. + * Hard ceiling on the deferred preview rendering (HTML extraction + DB + * update). The inner office-render path already has its own 12s timeout + * and a concurrency-limited queue; this is the outer guard that catches + * pathological cases where queue wait + render + DB write would + * otherwise hang the file in `status: 'pending'` indefinitely. + * + * If the timeout fires the record is updated to `status: 'failed'` + * with `previewError: 'timeout'` and the UI shows download-only. + */ +const PREVIEW_FINALIZE_TIMEOUT_MS = 60_000; + +/** + * Render the inline HTML preview for a code-execution file (or plain + * text for non-office buckets that still benefit from caching), then + * atomically transition the DB record to `status: 'ready'` (with + * `text`/`textFormat`) or `status: 'failed'` (with `previewError`). + * + * Decoupled from `processCodeOutput` so the agent's final response is + * not blocked on potentially slow office rendering. The caller fires + * this without awaiting; promises continue running after the HTTP + * response closes (Node doesn't kill them) and the frontend learns of + * completion via the `attachment` update SSE event (if the stream is + * still open) or via React Query polling otherwise. Process restart + * is the only thing that can lose progress — covered by the boot-time + * orphan sweep. + * + * @param {object} params + * @param {Buffer} params.buffer - The full downloaded file contents, + * bounded by the server's `fileSizeLimit` config (defaults far above + * the 1MB extractor cap). The buffer is captured by the closure + * returned in `{ finalize }`, so when many office files queue behind + * the inner concurrency limiter (cap 2), all queued buffers stay + * resident until each one's slot frees. For a tool result emitting + * N office files, peak heap usage from this path is up to + * `N * fileSizeLimit`. Acceptable for typical agent runs (a handful + * of files at a few hundred KB each); pathological cases are bounded + * by the inner per-file 12s timeout and the outer 60s render cap. + * @param {string} params.leafName - Basename for classification. + * @param {string} params.mimeType - Detected/inferred MIME. + * @param {string} params.category - Classifier output. + * @param {string} params.file_id - The DB record key for the update. + * @param {string} [params.previewRevision] - Generation marker stamped + * by the immediate persist step. The DB commit is conditional on + * this — if a newer emit (cross-turn filename reuse) has rotated + * the revision before this render finishes, `updateFile` returns + * null and the stale render is silently discarded rather than + * overwriting the newer record. + * @returns {Promise} The post-update record on + * success; `null` if the DB update itself failed (extraction failure + * is reflected as `status: 'failed'`, not a thrown error) or if the + * `previewRevision` guard rejected the write. + */ +const finalizePreview = async ({ + buffer, + leafName, + mimeType, + category, + file_id, + previewRevision, +}) => { + let text = null; + let previewError; + try { + text = await withTimeout( + extractCodeArtifactText(buffer, leafName, mimeType, category), + PREVIEW_FINALIZE_TIMEOUT_MS, + `Preview extraction exceeded ${PREVIEW_FINALIZE_TIMEOUT_MS}ms`, + ); + } catch (_error) { + /* `extractCodeArtifactText` swallows its own errors and returns null, + * so the only way to reach here is a `withTimeout` rejection — i.e. + * the queue + render combined exceeded the outer 60s ceiling. */ + previewError = 'timeout'; + logger.warn( + `[finalizePreview] ${file_id}: extraction timed out after ${PREVIEW_FINALIZE_TIMEOUT_MS}ms`, + ); + } + /* HTML-or-null contract (PR #12934): null result on an office file + * must NOT fall back to plain text — surface as failed. Caller gates + * on `hasOfficeHtmlPath`, so reaching here always means office. */ + const textFormat = getExtractedTextFormat(leafName, mimeType, text); + const failed = text == null; + const status = failed ? 'failed' : 'ready'; + if (failed && !previewError) { + previewError = 'parser-error'; + } + try { + /* Conditional update: commit only if `previewRevision` still + * matches what the immediate persist step stamped. If a newer + * emit has rotated the revision (cross-turn filename reuse), + * `updateFile` returns null and the stale render is silently + * discarded. (Codex P1 review on PR #12957.) */ + const updated = await updateFile( + { + file_id, + text, + textFormat, + status, + previewError: failed ? previewError : null, + }, + previewRevision ? { previewRevision } : undefined, + ); + if (!updated && previewRevision) { + logger.debug( + `[finalizePreview] ${file_id}: stale render skipped — newer emit has superseded revision ${previewRevision}`, + ); + } + return updated; + } catch (error) { + logger.error( + `[finalizePreview] ${file_id}: failed to persist preview result: ${error?.message ?? error}`, + ); + return null; + } +}; + +/** + * Run the background `finalize` thunk returned by `processCodeOutput` + * and route the resolved record to the caller's emit logic. Shared + * between `callbacks.js` (chat-completions + Open Responses) and + * `tools.js` (direct tool endpoint) so the fire-and-forget pattern + * doesn't drift across callsites. + * + * `onResolved` receives the post-update DB record and is the only piece + * that varies — chat-completions writes the legacy `attachment` SSE + * event, Open Responses writes the spec-shaped `librechat:attachment` + * event with a sequence number, and the direct tool endpoint has no + * stream to write to (caller passes a no-op). + * + * The catch path is the safety net for unexpected programming errors + * inside `finalizePreview` ONLY. The function is designed to never + * throw (extraction and DB failures are translated to `status: 'failed'` + * inside it), but a ref error or future regression would otherwise + * leave the DB record stuck at `'pending'` until the boot-time orphan + * sweep — potentially hours away on a stable server. We attempt a + * best-effort `updateFile` to mark the record `'failed'` with + * `previewError: 'unexpected'` so the UI stops polling and the + * next-turn LLM context surfaces the failure. + * + * `onResolved` errors are deliberately isolated in their own try/catch. + * Without that isolation, a transient transport-side failure (SSE write + * race after the stream closed, an emitter listener throwing) would + * propagate into the finalize catch and downgrade an *already-resolved* + * record to `failed` with `previewError: 'unexpected'` — surfacing + * "preview unavailable" in the UI even though extraction succeeded + * and the file is on disk. The emit failure is logged but the DB + * record stays at whatever `finalizePreview` wrote (typically + * `'ready'`), so the polling layer / next page load still sees the + * resolved preview. + * + * @param {object} params + * @param {(() => Promise) | undefined} params.finalize - The + * thunk returned by `processCodeOutput`. No-op when undefined. + * @param {string | undefined} params.fileId - DB key for the failure + * marker; if absent the catch only logs. + * @param {string | undefined} [params.previewRevision] - Generation + * marker stamped by the immediate persist step. The defensive + * `updateFile` in the catch is conditional on this — if a newer + * emit has rotated the revision, the stale failure marker is + * silently discarded so a programming error from an older render + * doesn't override a newer turn's record. + * @param {(updated: object) => void} [params.onResolved] - Called once + * on success with the post-update record. + */ +const runPreviewFinalize = ({ finalize, fileId, previewRevision, onResolved }) => { + if (typeof finalize !== 'function') { + return; + } + finalize() + .then((updated) => { + if (!updated || !onResolved) { + return; + } + /* Isolated try/catch — a throw inside `onResolved` (transport-side + * SSE write race, emitter listener error) MUST NOT propagate to + * the outer `.catch`, which would downgrade an already-resolved + * record to `failed` with `previewError: 'unexpected'`. + * Extraction succeeded at this point and `finalizePreview` has + * already persisted the terminal status; the polling layer / next + * page load will surface the resolved preview even if this turn's + * SSE emit didn't land. */ + try { + onResolved(updated); + } catch (emitError) { + logger.error( + `[runPreviewFinalize] onResolved threw for ${fileId}; record stays at the finalized status:`, + emitError, + ); + } + }) + .catch((error) => { + logger.error('Error rendering deferred preview:', error); + if (!fileId) { + return; + } + updateFile( + { + file_id: fileId, + status: 'failed', + previewError: 'unexpected', + }, + previewRevision ? { previewRevision } : undefined, + ).catch((updateErr) => { + logger.error( + `[runPreviewFinalize] also failed to mark ${fileId} as failed after error:`, + updateErr, + ); + }); + }); +}; + +/** + * Process code execution output files — downloads and saves both images + * and non-image files. All files are saved to local storage with + * `fileIdentifier` metadata for code env re-upload. + * + * Returns a two-part shape so callers can ship the attachment to the + * client immediately and run preview extraction in the background: + * - `file`: persisted metadata (file is on disk, downloadable, and + * has `status: 'pending'` if a preview is still being rendered). + * - `finalize` (optional): a thunk returning the deferred preview + * result promise. Present only when an inline HTML preview is + * expected (office buckets — DOCX/XLSX/XLS/ODS/CSV/PPTX). Caller + * decides whether to await or fire-and-forget. + * + * Existing fallback paths (size limit, missing storage strategy, error + * catch) return `{ file }` with no `finalize` — there's nothing to + * extract. + * * @param {ServerRequest} params.req - The Express request object. * @param {string} params.id - The file ID from the code environment. * @param {string} params.name - The filename. @@ -77,7 +307,7 @@ const createDownloadFallback = ({ * @param {string} params.session_id - The code execution session ID. * @param {string} params.conversationId - The current conversation ID. * @param {string} params.messageId - The current message ID. - * @returns {Promise} The file metadata or undefined if an error occurs. + * @returns {Promise<{ file: MongoFile & { messageId: string, toolCallId: string }, finalize?: () => Promise }>} */ const processCodeOutput = async ({ req, @@ -122,15 +352,17 @@ const processCodeOutput = async ({ logger.warn( `[processCodeOutput] File "${name}" (${(buffer.length / megabyte).toFixed(2)} MB) exceeds size limit of ${(fileSizeLimit / megabyte).toFixed(2)} MB, falling back to download URL`, ); - return createDownloadFallback({ - id, - name, - messageId, - toolCallId, - session_id, - conversationId, - expiresAt: currentDate.getTime() + 86400000, - }); + return { + file: createDownloadFallback({ + id, + name, + messageId, + toolCallId, + session_id, + conversationId, + expiresAt: currentDate.getTime() + 86400000, + }), + }; } const fileIdentifier = `${session_id}/${id}`; @@ -205,7 +437,7 @@ const processCodeOutput = async ({ metadata: { fileIdentifier }, }; await createFile(file, true); - return Object.assign(file, { messageId, toolCallId }); + return { file: Object.assign(file, { messageId, toolCallId }) }; } const { saveBuffer } = getStrategyFunctions(appConfig.fileStrategy); @@ -213,15 +445,17 @@ const processCodeOutput = async ({ logger.warn( `[processCodeOutput] saveBuffer not available for strategy ${appConfig.fileStrategy}, falling back to download URL`, ); - return createDownloadFallback({ - id, - name, - messageId, - toolCallId, - session_id, - conversationId, - expiresAt: currentDate.getTime() + 86400000, - }); + return { + file: createDownloadFallback({ + id, + name, + messageId, + toolCallId, + session_id, + conversationId, + expiresAt: currentDate.getTime() + 86400000, + }), + }; } const detectedType = await determineFileType(buffer, true); @@ -270,9 +504,17 @@ const processCodeOutput = async ({ * what it would have gotten with the old flat-name flow. */ const leafName = path.basename(safeName); const category = classifyCodeArtifact(leafName, mimeType); - const text = await extractCodeArtifactText(buffer, leafName, mimeType, category); - const file = { + /* Office-bucket files (DOCX/XLSX/XLS/ODS/CSV/PPTX) route through + * `bufferToOfficeHtml` which is CPU-heavy. Persist the record now + * with `status: 'pending'` and `text: null` so the agent's response + * isn't blocked, then return a `finalize` thunk the caller can run + * in the background. Non-office files have cheap or no extraction + * — run it inline so the caller gets a fully-resolved record + * without juggling a finalize step. */ + const expectsPreview = hasOfficeHtmlPath(leafName, mimeType); + + const baseFile = { file_id, filepath, messageId: persistedMessageId, @@ -288,15 +530,70 @@ const processCodeOutput = async ({ context: FileContext.execute_code, usage: isUpdate ? (claimed.usage ?? 0) + 1 : 1, createdAt: isUpdate ? claimed.createdAt : formattedDate, - // Always set `text` explicitly (string or null) so that an update which - // produces a binary or oversized artifact clears any previously cached - // text — `createFile` uses findOneAndUpdate with $set semantics, which - // would otherwise leave a stale value behind. + }; + + if (expectsPreview) { + /* Persist with `status: 'pending'` and explicit + * `text: null` / `textFormat: null` so an update that previously + * had cached text gets cleared. The deferred finalize transitions + * to 'ready' (with text/textFormat) or 'failed' (with + * previewError). + * + * `previewRevision` is a fresh UUID stamped on every emit. The + * deferred finalize's `updateFile` is conditional on this — if + * a newer turn (cross-turn filename reuse) has rotated the + * revision before this render finishes, the stale render is + * silently discarded rather than overwriting the newer record. + * (Codex P1 review on PR #12957.) */ + const previewRevision = v4(); + const file = { + ...baseFile, + text: null, + textFormat: null, + status: 'pending', + previewError: null, + previewRevision, + }; + await createFile(file, true); + return { + file: Object.assign(file, { messageId, toolCallId }), + finalize: () => + finalizePreview({ buffer, leafName, mimeType, category, file_id, previewRevision }), + previewRevision, + }; + } + + /* Non-office path: extraction is cheap (utf8 decode, parseDocument + * for PDF/ODT, or null for binaries). Run inline and return a + * fully-resolved record — no `finalize` needed. */ + const text = await extractCodeArtifactText(buffer, leafName, mimeType, category); + /* `textFormat` accompanies `text` so the client can gate + * office-HTML-bucket routing on a trusted signal — clients MUST + * NOT inject `text` into the iframe as HTML unless `textFormat === + * 'html'`. RAG-uploaded `.docx` etc. arrive with plain text from + * mammoth.extractRawText and would otherwise be hijacked by the + * extension-based office routing into the HTML-injection path + * (Codex P1 review on PR #12934). null on extract failure — the + * client treats absence as 'text' for safety. */ + const textFormat = getExtractedTextFormat(leafName, mimeType, text); + const file = { + ...baseFile, + // Always set explicitly so an update which produces a binary or + // oversized artifact clears any previously cached text — createFile + // uses findOneAndUpdate with $set semantics. text: text ?? null, + textFormat: textFormat ?? null, + // Clear deferred-preview lifecycle fields in case the prior emit + // at this (filename, conversationId) was an office file — + // otherwise stale `pending`/`failed` would persist and the client + // would render the wrong state for the now non-office artifact. + status: null, + previewError: null, + previewRevision: null, }; await createFile(file, true); - return Object.assign(file, { messageId, toolCallId }); + return { file: Object.assign(file, { messageId, toolCallId }) }; } catch (error) { if (error?.message === 'Path traversal detected in filename') { logger.warn( @@ -309,15 +606,17 @@ const processCodeOutput = async ({ }); // Fallback for download errors - return download URL so user can still manually download - return createDownloadFallback({ - id, - name, - messageId, - toolCallId, - session_id, - conversationId, - expiresAt: currentDate.getTime() + 86400000, - }); + return { + file: createDownloadFallback({ + id, + name, + messageId, + toolCallId, + session_id, + conversationId, + expiresAt: currentDate.getTime() + 86400000, + }), + }; } }; @@ -419,9 +718,14 @@ const primeFiles = async (options) => { const [path, queryString] = file.metadata.fileIdentifier.split('?'); const [session_id, id] = path.split('/'); + let queryParams = {}; + if (queryString) { + queryParams = Object.fromEntries(new URLSearchParams(queryString).entries()); + } + /** * `pushFile` accepts optional overrides so the reupload path can - * push the FRESH `(session_id, id)` parsed off the new + * push the FRESH `(session_id, id, entity_id)` parsed off the new * `fileIdentifier`. Without these overrides, the closure would * capture the stale pre-reupload refs from the outer loop and * the in-memory `files` array (now consumed by @@ -430,8 +734,12 @@ const primeFiles = async (options) => { * gets the new identifier via `updateFile`, but the seed would * still inject the old one — bash_tool / read_file would 404 * trying to mount the file until the next turn re-reads metadata. + * + * `entity_id` is forwarded so codeapi can resolve sessionKey + * per-file, allowing one execute to mix files uploaded under + * different entities (e.g. a skill bundle plus a user attachment). */ - const pushFile = (overrideSessionId, overrideId) => { + const pushFile = (overrideSessionId, overrideId, overrideEntityId) => { if (!toolContext) { toolContext = `- Note: The following files are available in the "${Tools.execute_code}" tool environment:`; } @@ -444,11 +752,29 @@ const primeFiles = async (options) => { : ' (attached by user)'; } - toolContext += `\n\t- /mnt/data/${file.filename}${fileSuffix}`; + const entity_id = overrideEntityId ?? queryParams.entity_id; + + /* Surface the preview lifecycle so the LLM knows when a + * prior-turn artifact's rich preview didn't materialize. The + * file blob is always available (`processCodeOutput` persists + * it before returning), so the model can still tell the user + * "you can download it" even when the preview never resolved. + * Absent status means legacy or non-office — render normally. */ + let previewSuffix = ''; + if (file.status === 'pending') { + previewSuffix = ' (preview not yet generated)'; + } else if (file.status === 'failed') { + previewSuffix = file.previewError + ? ` (preview unavailable: ${file.previewError})` + : ' (preview unavailable)'; + } + + toolContext += `\n\t- /mnt/data/${file.filename}${fileSuffix}${previewSuffix}`; files.push({ id: overrideId ?? id, session_id: overrideSessionId ?? session_id, name: file.filename, + ...(entity_id ? { entity_id } : {}), }); }; @@ -457,11 +783,6 @@ const primeFiles = async (options) => { continue; } - let queryParams = {}; - if (queryString) { - queryParams = Object.fromEntries(new URLSearchParams(queryString).entries()); - } - const reuploadFile = async () => { try { const { getDownloadStream } = getStrategyFunctions(file.source); @@ -493,11 +814,18 @@ const primeFiles = async (options) => { * top of this iteration refer to the old, expired/missing * sandbox object — using them here would silently re-introduce * the bug `Graph.sessions` seeding is supposed to fix. + * + * `entity_id` survives the round-trip: the upload was tagged + * with `queryParams.entity_id` above, so the new identifier + * carries the same scope. */ - const [newPath] = fileIdentifier.split('?'); + const [newPath, newQuery] = fileIdentifier.split('?'); const [newSessionId, newId] = newPath.split('/'); + const newQueryParams = newQuery + ? Object.fromEntries(new URLSearchParams(newQuery).entries()) + : {}; sessions.set(newSessionId, true); - pushFile(newSessionId, newId); + pushFile(newSessionId, newId, newQueryParams.entity_id); } catch (error) { logger.error( `Error re-uploading file ${id} in session ${session_id}: ${error.message}`, @@ -600,4 +928,5 @@ module.exports = { getSessionInfo, processCodeOutput, readSandboxFile, + runPreviewFinalize, }; diff --git a/api/server/services/Files/Code/process.spec.js b/api/server/services/Files/Code/process.spec.js index e0d7b08fd344..98cbbf9fffbe 100644 --- a/api/server/services/Files/Code/process.spec.js +++ b/api/server/services/Files/Code/process.spec.js @@ -43,6 +43,16 @@ mockAxios.isAxiosError = jest.fn(() => false); const mockClassifyCodeArtifact = jest.fn(() => 'other'); const mockExtractCodeArtifactText = jest.fn(async () => null); +const mockGetExtractedTextFormat = jest.fn((_name, _mime, text) => (text == null ? null : 'text')); +/* `hasOfficeHtmlPath` gates the persist-then-render split: when true, processCodeOutput + * returns `{ file, finalize }` with the file persisted at `status: 'pending'` + * and `finalize` runs the background extraction. Default false here so the + * legacy single-phase tests below (txt/png/etc) exercise the inline path + * unchanged. The dedicated office/finalize describe block toggles it on. */ +const mockHasOfficeHtmlPath = jest.fn(() => false); +/* Pass-through `withTimeout`: tests don't drive timeouts here (those live + * in promise.spec.ts and the finalizePreview unit tests below). */ +const passthroughWithTimeout = async (promise) => promise; jest.mock('@librechat/api', () => { const http = require('http'); const https = require('https'); @@ -52,6 +62,8 @@ jest.mock('@librechat/api', () => { sanitizeArtifactPath: jest.fn((name) => name), flattenArtifactPath: jest.fn((name) => name.replace(/\//g, '__')), createAxiosInstance: jest.fn(() => mockAxios), + withTimeout: (...args) => passthroughWithTimeout(...args), + hasOfficeHtmlPath: (...args) => mockHasOfficeHtmlPath(...args), /** * Arrow-function indirection (vs. a direct `jest.fn()` reference) so * tests can per-case `mockReturnValueOnce` / `mockImplementationOnce` @@ -63,6 +75,15 @@ jest.mock('@librechat/api', () => { */ classifyCodeArtifact: (...args) => mockClassifyCodeArtifact(...args), extractCodeArtifactText: (...args) => mockExtractCodeArtifactText(...args), + /* `processCodeOutput` derives the `textFormat` trust flag for + * `IMongoFile` from this helper — Codex P1 review on PR #12934. + * The mock returns 'text' for non-null extractor output and null + * otherwise so the downstream `file.textFormat` field is set to + * a believable shape without modeling the office-HTML branch + * (the dispatcher under test isn't exercising that path). Per- + * test overrides via `mockGetExtractedTextFormat.mockReturnValue` + * if a case needs to assert the 'html' value. */ + getExtractedTextFormat: (...args) => mockGetExtractedTextFormat(...args), codeServerHttpAgent: new http.Agent({ keepAlive: false }), codeServerHttpsAgent: new https.Agent({ keepAlive: false }), }; @@ -168,7 +189,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(mockClaimCodeFile).toHaveBeenCalledWith({ filename: 'test-file.txt', @@ -191,7 +212,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.file_id).toBe('mock-uuid-1234'); expect(result.usage).toBe(1); @@ -211,7 +232,7 @@ describe('Code Process', () => { }; convertImage.mockResolvedValue(convertedFile); - const result = await processCodeOutput(imageParams); + const { file: result } = await processCodeOutput(imageParams); expect(convertImage).toHaveBeenCalledWith( mockReq, @@ -236,7 +257,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: imageBuffer }); convertImage.mockResolvedValue({ filepath: '/images/user-123/existing-img-id.webp' }); - const result = await processCodeOutput(imageParams); + const { file: result } = await processCodeOutput(imageParams); expect(convertImage).toHaveBeenCalledWith( mockReq, @@ -262,7 +283,7 @@ describe('Code Process', () => { getStrategyFunctions.mockReturnValue({ saveBuffer: mockSaveBuffer }); determineFileType.mockResolvedValue({ mime: 'text/plain' }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(mockSaveBuffer).toHaveBeenCalledWith({ userId: 'user-123', @@ -289,7 +310,7 @@ describe('Code Process', () => { const mockSaveBuffer = jest.fn().mockResolvedValue('/uploads/saved.txt'); getStrategyFunctions.mockReturnValue({ saveBuffer: mockSaveBuffer }); - const result = await processCodeOutput({ + const { file: result } = await processCodeOutput({ ...baseParams, name: 'test_folder/test_file.txt', }); @@ -368,7 +389,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: smallBuffer }); determineFileType.mockResolvedValue({ mime: 'application/pdf' }); - const result = await processCodeOutput({ ...baseParams, name: 'document.pdf' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'document.pdf' }); expect(determineFileType).toHaveBeenCalledWith(smallBuffer, true); expect(result.type).toBe('application/pdf'); @@ -379,7 +400,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: smallBuffer }); determineFileType.mockResolvedValue(null); - const result = await processCodeOutput({ ...baseParams, name: 'unknown.xyz' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'unknown.xyz' }); expect(result.type).toBe('application/octet-stream'); }); @@ -393,7 +414,7 @@ describe('Code Process', () => { mockClassifyCodeArtifact.mockReturnValueOnce('utf8-text'); mockExtractCodeArtifactText.mockResolvedValueOnce('hello world\n'); - const result = await processCodeOutput({ ...baseParams, name: 'note.txt' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'note.txt' }); expect(mockClassifyCodeArtifact).toHaveBeenCalledWith('note.txt', 'text/plain'); expect(mockExtractCodeArtifactText).toHaveBeenCalledWith( @@ -416,7 +437,7 @@ describe('Code Process', () => { mockClassifyCodeArtifact.mockReturnValueOnce('other'); mockExtractCodeArtifactText.mockResolvedValueOnce(null); - const result = await processCodeOutput({ ...baseParams, name: 'archive.zip' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'archive.zip' }); expect(result.text).toBeNull(); const createCall = createFile.mock.calls[0][0]; @@ -455,6 +476,31 @@ describe('Code Process', () => { expect(mockClassifyCodeArtifact).not.toHaveBeenCalled(); expect(mockExtractCodeArtifactText).not.toHaveBeenCalled(); }); + + it('clears deferred-preview lifecycle fields so a prior office record at this file_id stops looking pending', async () => { + /* Codex P2: same (filename, conversationId) was previously an + * office artifact, leaving status/previewError/previewRevision + * populated. The non-office update must reset them or the + * client renders the wrong state for the now non-office file. */ + mockClaimCodeFile.mockResolvedValueOnce({ + file_id: 'reused-id', + filename: 'output.txt', + usage: 1, + createdAt: '2024-01-01T00:00:00.000Z', + }); + mockAxios.mockResolvedValue({ data: Buffer.from('hello') }); + determineFileType.mockResolvedValue({ mime: 'text/plain' }); + mockClassifyCodeArtifact.mockReturnValueOnce('text'); + mockHasOfficeHtmlPath.mockReturnValueOnce(false); + mockExtractCodeArtifactText.mockResolvedValueOnce('hello'); + + await processCodeOutput({ ...baseParams, name: 'output.txt' }); + + const createCall = createFile.mock.calls[0][0]; + expect(createCall).toHaveProperty('status', null); + expect(createCall).toHaveProperty('previewError', null); + expect(createCall).toHaveProperty('previewRevision', null); + }); }); describe('file size limit enforcement', () => { @@ -465,7 +511,7 @@ describe('Code Process', () => { const largeBuffer = Buffer.alloc(5000); // 5KB - exceeds 1KB limit mockAxios.mockResolvedValue({ data: largeBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('exceeds size limit')); expect(result.filepath).toContain('/api/files/code/download/session-123/file-id-123'); @@ -484,7 +530,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: smallBuffer }); getStrategyFunctions.mockReturnValue({ saveBuffer: null }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(logger.warn).toHaveBeenCalledWith( expect.stringContaining('saveBuffer not available'), @@ -496,7 +542,7 @@ describe('Code Process', () => { it('should fallback to download URL on axios error', async () => { mockAxios.mockRejectedValue(new Error('Network error')); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.filepath).toContain('/api/files/code/download/session-123/file-id-123'); expect(result.conversationId).toBe('conv-123'); @@ -510,7 +556,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.usage).toBe(1); }); @@ -524,7 +570,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.usage).toBe(6); }); @@ -537,7 +583,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.usage).toBe(1); }); @@ -548,7 +594,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.metadata).toEqual({ fileIdentifier: 'session-123/file-id-123', @@ -559,7 +605,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.context).toBe(FileContext.execute_code); }); @@ -568,7 +614,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.toolCallId).toBe('tool-call-123'); expect(result.messageId).toBe('msg-123'); @@ -708,7 +754,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput({ + const { file: result } = await processCodeOutput({ ...baseParams, name: 'sentinel.txt', messageId: 'turn-2-current-run-msg', @@ -779,6 +825,329 @@ describe('Code Process', () => { expect(callConfig.httpsAgent.keepAlive).toBe(false); }); }); + + describe('deferred-preview flow (office-bucket files)', () => { + /* Office-bucket files (DOCX/XLSX/etc.) split into: + * the initial emit (await): persist `text: null, status: 'pending'`, + * return `{ file, finalize }` so the caller can ship the + * attachment to the client immediately; + * the deferred render (background): finalize() invokes the extractor and + * transitions the record to 'ready' (with text/textFormat) or + * 'failed' (with previewError). The agent's final response + * never blocks on the deferred render. + * + * The `hasOfficeHtmlPath` mock is the gate. Other tests keep it + * at `false` (legacy single-phase path); we flip it on here. */ + const { updateFile } = require('~/models'); + + beforeEach(() => { + mockHasOfficeHtmlPath.mockReturnValue(true); + updateFile.mockResolvedValue({ file_id: 'mock-uuid-1234', status: 'ready' }); + }); + + afterEach(() => { + mockHasOfficeHtmlPath.mockReturnValue(false); + }); + + it('persists the initial emit with status:pending and text:null, deferring extraction', async () => { + mockAxios.mockResolvedValue({ data: Buffer.alloc(100) }); + determineFileType.mockResolvedValue({ + mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + }); + + const result = await processCodeOutput({ ...baseParams, name: 'data.xlsx' }); + + expect(result.file).toMatchObject({ + file_id: 'mock-uuid-1234', + filename: 'data.xlsx', + status: 'pending', + text: null, + textFormat: null, + }); + expect(typeof result.finalize).toBe('function'); + // Extractor MUST NOT have been called yet — that's deferred preview work. + expect(mockExtractCodeArtifactText).not.toHaveBeenCalled(); + // Persisted record with the pending status. + expect(createFile).toHaveBeenCalledWith( + expect.objectContaining({ status: 'pending', text: null, textFormat: null }), + true, + ); + }); + + it('finalize() runs the extractor, transitions to ready with text+textFormat on success', async () => { + mockAxios.mockResolvedValue({ data: Buffer.alloc(100) }); + determineFileType.mockResolvedValue({ + mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + }); + mockExtractCodeArtifactText.mockResolvedValueOnce('
1
'); + mockGetExtractedTextFormat.mockReturnValueOnce('html'); + + const { finalize } = await processCodeOutput({ ...baseParams, name: 'data.xlsx' }); + await finalize(); + + expect(mockExtractCodeArtifactText).toHaveBeenCalledTimes(1); + /* Update is conditional on `previewRevision` so an older render + * can't overwrite a newer turn's record on cross-turn filename + * reuse. The uuid mock returns the same value for every v4() + * call, so file_id and previewRevision happen to coincide here + * — what matters is the second arg carries the revision filter. */ + expect(updateFile).toHaveBeenCalledWith( + { + file_id: 'mock-uuid-1234', + text: '
1
', + textFormat: 'html', + status: 'ready', + previewError: null, + }, + { previewRevision: 'mock-uuid-1234' }, + ); + }); + + it('finalize() transitions to failed with previewError when extractor returns null (HTML-or-null contract)', async () => { + mockAxios.mockResolvedValue({ data: Buffer.alloc(100) }); + determineFileType.mockResolvedValue({ + mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + }); + mockExtractCodeArtifactText.mockResolvedValueOnce(null); + // Office bucket + null text → must be 'failed', NEVER raw text fallback + // (PR #12934 SEC fix: prevents ', + textFormat: 'text', + }), + ).toBe(TOOL_ARTIFACT_TYPES.PLAIN_TEXT); + }); + + it('downgrades office types to PLAIN_TEXT when textFormat is null (DB legacy)', () => { + /* Mongoose returns `null` for fields the document was saved without + * — covers attachments persisted before the textFormat field existed. */ + expect( + detectArtifactTypeFromFile({ + filename: 'workbook.xlsx', + type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + text: '
1
', + textFormat: null, + }), + ).toBe(TOOL_ARTIFACT_TYPES.PLAIN_TEXT); + }); + + /* Regression: an office attachment with no `text` AND no `textFormat` + * must NOT downgrade to PLAIN_TEXT (which has the lenient empty-text + * gate and would render as a half-empty panel card). The historical + * contract for office types with missing extraction is "fall through + * to the legacy download UI"; the security gate's empty-text exception + * preserves that. CI regression on PR #12934 — `LogContent.test.tsx` + * "falls back to the legacy download branch for an office file with + * no extracted text" was the canary. */ + it.each([ + ['report.docx', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + ['data.csv', 'text/csv'], + ['workbook.xlsx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + ['slides.pptx', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'], + ])( + '%s with no text and no textFormat returns null (preserves legacy download path)', + (filename, type) => { + expect(detectArtifactTypeFromFile({ filename, type, text: '' })).toBeNull(); + expect(detectArtifactTypeFromFile({ filename, type, text: undefined })).toBeNull(); + }, + ); + it('falls back to MIME when the extension is unknown', () => { expect(detectArtifactTypeFromFile({ filename: 'noext', type: 'text/html', text: 'x' })).toBe( TOOL_ARTIFACT_TYPES.HTML, @@ -114,11 +277,13 @@ describe('detectArtifactTypeFromFile', () => { }); it('returns null for unsupported types', () => { + /* PDFs have no rich preview path on the client (the artifact panel + * doesn't host a PDF viewer); they fall back to the download UI. */ expect( - detectArtifactTypeFromFile({ filename: 'output.csv', type: 'text/csv', text: 'a,b' }), + detectArtifactTypeFromFile({ filename: 'doc.pdf', type: 'application/pdf', text: 'x' }), ).toBeNull(); expect( - detectArtifactTypeFromFile({ filename: 'doc.pdf', type: 'application/pdf', text: 'x' }), + detectArtifactTypeFromFile({ filename: 'photo.jpg', type: 'image/jpeg', text: 'binary' }), ).toBeNull(); }); @@ -177,12 +342,11 @@ describe('detectArtifactTypeFromFile', () => { ); }); - it('does NOT route data formats to CODE (CSV / JSON / YAML / TOML / XML)', () => { + it('does NOT route data formats to CODE (JSON / YAML / TOML / XML)', () => { /* These get dedicated viewers in follow-ups; for now they fall - * through to inline rendering (return null). */ - expect( - detectArtifactTypeFromFile({ filename: 'data.csv', type: 'text/csv', text: 'a,b' }), - ).toBeNull(); + * through to inline rendering (return null). CSV is the exception: + * it routes to the SPREADSHEET preview bucket — covered separately + * in the "classifies %s by extension" suite above. */ expect( detectArtifactTypeFromFile({ filename: 'data.json', type: 'application/json', text: '{}' }), ).toBeNull(); @@ -397,7 +561,10 @@ describe('fileToArtifact', () => { }); it('returns null for unsupported types so callers can fall through', () => { - expect(fileToArtifact({ ...baseFile, filename: 'data.csv', type: 'text/csv' })).toBeNull(); + expect(fileToArtifact({ ...baseFile, filename: 'photo.jpg', type: 'image/jpeg' })).toBeNull(); + expect( + fileToArtifact({ ...baseFile, filename: 'doc.pdf', type: 'application/pdf' }), + ).toBeNull(); }); /* End-to-end test for the CODE bucket. The classification path is @@ -502,13 +669,18 @@ describe('fileToArtifact', () => { }); it('uses the caller-provided placeholder when a deferred-extraction file has no text', () => { - // Backend extractor returns null for pptx (deferred). Client sees - // `text === null` and substitutes the localized placeholder. + /* Plain-text and markdown remain on the lenient empty-text gate so the + * artifact card can render a "preparing preview…" placeholder while + * extraction is in flight. (Office preview buckets — DOCX, SPREADSHEET, + * PRESENTATION — use the strict gate instead: their renderers need + * server-rendered HTML, so the artifact stays unregistered until the + * `text` field arrives. See the strict-gate test in the + * `detectArtifactTypeFromFile` suite.) */ const artifact = fileToArtifact( { ...baseFile, - filename: 'slides.pptx', - type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + filename: 'notes.txt', + type: 'text/plain', text: null as unknown as string, }, { placeholder: '_Coming soon_' }, @@ -521,13 +693,115 @@ describe('fileToArtifact', () => { it('falls back to empty content when no placeholder is supplied and text is missing', () => { const artifact = fileToArtifact({ ...baseFile, - filename: 'slides.pptx', - type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + filename: 'notes.txt', + type: 'text/plain', text: undefined, }); + expect(artifact).not.toBeNull(); expect(artifact!.content).toBe(''); }); + it('returns null for office preview buckets without text (strict gate)', () => { + /* `textFormat: 'html'` is required for routing to land on the office + * bucket in the first place; without it the security gate downgrades + * to PLAIN_TEXT (which has the lenient empty-text gate). The strict + * empty-text gate then fires and the artifact stays unregistered. */ + expect( + fileToArtifact({ + ...baseFile, + filename: 'slides.pptx', + type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + text: undefined, + textFormat: 'html', + }), + ).toBeNull(); + expect( + fileToArtifact({ + ...baseFile, + filename: 'report.docx', + type: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + text: '', + textFormat: 'html', + }), + ).toBeNull(); + expect( + fileToArtifact({ + ...baseFile, + filename: 'data.csv', + type: 'text/csv', + text: undefined, + textFormat: 'html', + }), + ).toBeNull(); + }); + + it('builds a SPREADSHEET artifact for csv/xlsx with backend-rendered HTML in text', () => { + const csv = fileToArtifact({ + ...baseFile, + filename: 'data.csv', + type: 'text/csv', + text: '
1
', + textFormat: 'html', + }); + expect(csv).not.toBeNull(); + expect(csv!.type).toBe(TOOL_ARTIFACT_TYPES.SPREADSHEET); + expect(csv!.title).toBe('data.csv'); + expect(csv!.content).toContain(''); + + const xlsx = fileToArtifact({ + ...baseFile, + filename: 'workbook.xlsx', + type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + text: 'sheet', + textFormat: 'html', + }); + expect(xlsx).not.toBeNull(); + expect(xlsx!.type).toBe(TOOL_ARTIFACT_TYPES.SPREADSHEET); + }); + + it('builds a DOCX artifact for .docx with backend-rendered HTML in text', () => { + const artifact = fileToArtifact({ + ...baseFile, + filename: 'report.docx', + type: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + text: '

hello

', + textFormat: 'html', + }); + expect(artifact).not.toBeNull(); + expect(artifact!.type).toBe(TOOL_ARTIFACT_TYPES.DOCX); + }); + + it('builds a PRESENTATION artifact for .pptx with backend-rendered HTML in text', () => { + const artifact = fileToArtifact({ + ...baseFile, + filename: 'deck.pptx', + type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + text: '
  1. Slide 1
', + textFormat: 'html', + }); + expect(artifact).not.toBeNull(); + expect(artifact!.type).toBe(TOOL_ARTIFACT_TYPES.PRESENTATION); + }); + + /* Codex P1 SECURITY companion: legacy/RAG path where `textFormat` is + * missing — the security gate downgrades to PLAIN_TEXT, which has the + * lenient empty-text gate. The text round-trips into the markdown + * viewer as escaped content rather than being injected as HTML. */ + it('downgrades to a PLAIN_TEXT artifact when an office file has text but no textFormat flag', () => { + const artifact = fileToArtifact({ + ...baseFile, + filename: 'report.docx', + type: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + /* This is what mammoth.extractRawText returns for a RAG upload — + * not sanitized HTML, just the document's flowed text. The + * security gate ensures it's never injected as HTML. */ + text: 'Document body text from extractRawText. ', + }); + expect(artifact).not.toBeNull(); + expect(artifact!.type).toBe(TOOL_ARTIFACT_TYPES.PLAIN_TEXT); + expect(artifact!.content).toContain('` would then execute inside + * the Sandpack iframe. The fix returns null instead, and the + * client's empty-text gate keeps the artifact off the panel. */ + mockOfficeHtml.mockResolvedValueOnce(null); + const text = await extractCodeArtifactText( + Buffer.from('PKfake-xlsx'), + 'attack.xlsx', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + 'document', + ); + expect(text).toBeNull(); + expect(parseDocumentCalls.length).toBe(0); + }); + + it('substitutes a "preview too large" banner when HTML exceeds the cache cap', async () => { + /* Regression for review finding #2 on PR #12934. The earlier + * implementation byte-truncated the producer's HTML at the + * UTF-8 boundary, which would land mid-tag and ship malformed + * markup like `
con\n…[truncated]` to the iframe. + * The new behavior swaps the entire payload for a small valid + * HTML banner — under the cap by construction. */ + const huge = 'X'.repeat(MAX_TEXT_CACHE_BYTES + 5_000); + mockOfficeHtml.mockResolvedValueOnce(huge); + const text = await extractCodeArtifactText( + Buffer.from('PK'), + 'big.docx', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'document', + ); + expect(text).not.toBeNull(); + // Always under the cap. + expect(Buffer.byteLength(text!, 'utf-8')).toBeLessThanOrEqual(MAX_TEXT_CACHE_BYTES); + // Valid HTML doc, not byte-truncated markup. + expect(text!).toMatch(/^/); + expect(text!).toContain(''); + // No truncation marker (which would only appear on the legacy path). + expect(text!).not.toContain('…[truncated]'); + // User-facing banner content. + expect(text!).toContain('Preview exceeds the size limit'); + }); + + it('passes through HTML output unchanged when within the cache cap', async () => { + const small = '

hello

'; + mockOfficeHtml.mockResolvedValueOnce(small); + const text = await extractCodeArtifactText( + Buffer.from('PK'), + 'small.docx', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'document', + ); + expect(text).toBe(small); + }); + + it('falls back to parseDocument for pdf (HTML producer returns null for unsupported types)', async () => { + // Default mock returns null — the producer's own dispatcher would do the + // same for PDF since pdf has no HTML rendering. Whether we call it or + // skip it is an implementation detail; what matters is that PDF still + // routes to parseDocument and yields the docx-mock text. + const text = await extractCodeArtifactText( + Buffer.from('%PDF-1.4'), + 'doc.pdf', + 'application/pdf', + 'document', + ); + expect(text).toBe(docxText); + expect(parseDocumentCalls[0]?.originalname).toBe('doc.pdf'); + }); + + it('does not call the office HTML producer for plain .txt utf8-text', async () => { + const text = await extractCodeArtifactText( + Buffer.from('hello world\n', 'utf-8'), + 'note.txt', + 'text/plain', + 'utf8-text', + ); + expect(mockOfficeHtml).not.toHaveBeenCalled(); + expect(text).toBe('hello world\n'); + }); + + it('routes extensionless CSV-by-MIME (utf8-text category) through the office HTML path', async () => { + /* Regression for the Codex review on PR #12934. A tool emitting + * `data` with `text/csv` classifies as `utf8-text` (csv has no + * extension here, MIME is text/* which the classifier treats as + * utf8-text). The previous gate skipped the office-render branch + * because the extension wasn't in OFFICE_HTML_EXTENSIONS — so the + * raw CSV text shipped to the client, which routes by MIME to the + * SPREADSHEET bucket and expects a full HTML document. The fix + * shares the dispatcher's MIME-aware predicate so the gate fires + * here too. */ + mockOfficeHtml.mockResolvedValueOnce('
1
'); + const text = await extractCodeArtifactText( + Buffer.from('a,b\n1,2', 'utf-8'), + 'data', + 'text/csv', + 'utf8-text', + ); + expect(mockOfficeHtml).toHaveBeenCalledWith(expect.any(Buffer), 'data', 'text/csv'); + expect(text).toContain(''); + }); + + it.each([ + ['workbook', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + ['workbook', 'application/vnd.ms-excel'], + ['workbook', 'application/vnd.oasis.opendocument.spreadsheet'], + ['report', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + ['deck', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'], + ])('routes extensionless office files by MIME alone (%s, %s)', async (name, mime) => { + mockOfficeHtml.mockResolvedValueOnce('x'); + const category = mime.includes('presentation') + ? 'pptx' + : mime.startsWith('text/') + ? 'utf8-text' + : 'document'; + const text = await extractCodeArtifactText(Buffer.from('PK'), name, mime, category); + expect(mockOfficeHtml).toHaveBeenCalledWith(expect.any(Buffer), name, mime); + expect(text).toContain(''); + }); + + it('returns null (and falls back to download UI) when the producer rejects a zip bomb', async () => { + /* Defense-in-depth check for SEC review on PR #12934. When + * `bufferToOfficeHtml` throws `ZipBombError` (because a zip-bomb + * DOCX/XLSX/PPTX got through the compressed-size gate), the outer + * extractor must swallow it and return null — that signals to the + * code-output controller to register the file as a regular + * download instead of a panel artifact. Crucially, it must NOT + * fall back to `extractDocument` text either: the client would + * inject that text into `index.html` and a literal `after

'); + expect(out).not.toMatch(/ { + const out = await sanitizeOfficeHtml(''); + expect(out).not.toMatch(/onerror=/i); + expect(out).toContain('https://x.test/a.png'); + }); + + test('strips javascript: URLs from anchors', async () => { + const out = await sanitizeOfficeHtml('click'); + expect(out).not.toMatch(/javascript:/i); + // The text survives even when the href is dropped. + expect(out).toContain('click'); + }); + + test('rejects data: URLs in (only may use data:)', async () => { + /* The Sandpack iframe sandbox does NOT gate `target="_blank"` + * navigations. A surviving `` would + * open attacker-controlled HTML in a new tab on click. The + * sanitizer scopes `data:` to only — global schemes are + * http/https/mailto. Regression guard for the Codex review on + * PR #12934. */ + const out = await sanitizeOfficeHtml( + 'click', + ); + expect(out).not.toContain('data:'); + expect(out).not.toContain('script'); + expect(out).toContain('click'); + }); + + test('preserves data: URLs in (mammoth inlines DOCX images as base64)', async () => { + const tinyPng = + 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII='; + const out = await sanitizeOfficeHtml(`dot`); + expect(out).toContain(tinyPng); + expect(out).toContain('alt="dot"'); + }); + + test('preserves http(s) and mailto links on anchors', async () => { + const out = await sanitizeOfficeHtml( + 'site mail', + ); + expect(out).toContain('https://example.com'); + expect(out).toContain('mailto:a@b.test'); + }); + + test('forces target=_blank rel=noopener on surviving anchors', async () => { + const out = await sanitizeOfficeHtml('link'); + expect(out).toContain('target="_blank"'); + expect(out).toContain('rel="noopener noreferrer"'); + }); + + test('strips '); + expect(out).not.toMatch(/ { + if (sanitizeHtmlModule) { + return sanitizeHtmlModule; + } + const mod = await import('sanitize-html'); + sanitizeHtmlModule = (mod.default ?? mod) as typeof import('sanitize-html'); + return sanitizeHtmlModule; +} + +/** + * Sanitize HTML produced by mammoth / SheetJS / our own pptx renderer for + * embedding in the Sandpack `static` iframe. Allows the structural and + * formatting tags those producers emit, plus inline `data:` images (mammoth + * inlines DOCX images as base64). Strips `">` smuggled through DOCX/PPTX would + * survive sanitization and open attacker-controlled HTML in a new + * tab when clicked — the Sandpack iframe sandbox doesn't gate + * `target="_blank"` navigations. */ + allowedSchemes: ['http', 'https', 'mailto'], + allowedSchemesByTag: { + img: ['http', 'https', 'data'], + }, + transformTags: { + a: (tagName, attribs) => ({ + tagName, + attribs: { ...attribs, rel: 'noopener noreferrer', target: '_blank' }, + }), + }, + /* sanitize-html runs `allowedAttributes` BEFORE per-attribute filtering, so + * `style` only survives where we explicitly allow it (td/th/col/colgroup). + * For those we still want to drop color declarations — defense against the + * mammoth/SheetJS hardcoded color-on-white issue. */ + allowedStyles: { + '*': { + 'text-align': [/^left$|^right$|^center$|^justify$/], + 'font-weight': [/^[1-9]00$|^bold$|^normal$/], + 'font-style': [/^italic$|^normal$/], + width: [/^\d+(?:\.\d+)?(?:px|%|em|rem)?$/], + height: [/^\d+(?:\.\d+)?(?:px|%|em|rem)?$/], + 'min-width': [/^\d+(?:\.\d+)?(?:px|%|em|rem)?$/], + 'vertical-align': [/^top$|^middle$|^bottom$|^baseline$/], + }, + }, + }); +} + +/** + * Wrap a sanitized HTML body in a complete document with the styles we want + * inside the Sandpack iframe. The CSS palette uses `prefers-color-scheme` so + * the iframe inherits dark/light from its parent (Sandpack iframes inherit + * the prefers-color-scheme media query from the host document). + */ +function wrapAsDocument(bodyHtml: string, extraHeadHtml = ''): string { + return ` + + + + +Preview + + + +${bodyHtml} + +`; +} + +/* ============================================================================= + * DOCX → HTML + * ============================================================================= */ + +/** + * Style-map directives that broaden mammoth's default heading detection. + * Mammoth's stock map only promotes paragraphs whose ms-word style name + * matches `Heading 1`/`Heading 2`/etc. — useless for code-generated + * docs (e.g. python-docx output) that apply direct character formatting + * instead of named styles. The `Title`/`Subtitle` mappings catch the + * built-in title styles used by Word's Insert > Cover Page workflow; + * explicit `Heading 1` thru `Heading 6` mappings retain mammoth's + * defaults; and `:fresh` tells mammoth not to merge consecutive + * matching paragraphs into a single heading element. + */ +const DOCX_STYLE_MAP = [ + "p[style-name='Title'] => h1.lc-docx-title:fresh", + "p[style-name='Subtitle'] => h2.lc-docx-subtitle:fresh", + "p[style-name='Heading 1'] => h1:fresh", + "p[style-name='Heading 2'] => h2:fresh", + "p[style-name='Heading 3'] => h3:fresh", + "p[style-name='Heading 4'] => h4:fresh", + "p[style-name='Heading 5'] => h5:fresh", + "p[style-name='Heading 6'] => h6:fresh", + "p[style-name='Quote'] => blockquote:fresh", +]; + +/** + * CSS layered on top of `wrapAsDocument`'s base styles to give mammoth's + * flat output more visual structure. Mammoth strips the navy banners, + * cell shading, and column layouts that direct-formatted docs apply, so + * the source loses most of its presentation. We compensate with three + * targeted heuristics: + * + * 1. The first row of any `
` gets sticky-header styling + * regardless of `` (mammoth never emits ``). + * 2. Tables get alternating row stripes so dense data blocks stay + * scannable even without the source's hand-tuned shading. + * 3. A bold-only-child paragraph (`

X

`) is the + * python-docx idiom for a "section heading"; styled as a + * pseudo-h2 with a thin accent border so the document's structure + * survives the round-trip. + */ +const DOCX_EXTRA_CSS = ` +.lc-docx h1 { font-size: 1.5rem; font-weight: 700; margin: 1em 0 0.5em; padding-bottom: 0.3em; border-bottom: 2px solid var(--border); } +.lc-docx h2 { font-size: 1.2rem; font-weight: 600; margin: 1em 0 0.4em; padding-left: 0.6em; border-left: 3px solid var(--link); } +.lc-docx h3 { font-size: 1.05rem; font-weight: 600; margin: 0.8em 0 0.3em; color: var(--link); } +.lc-docx-title { text-align: center; border-bottom: none; } +.lc-docx-subtitle { text-align: center; border-left: none; padding-left: 0; color: var(--muted); font-style: italic; font-weight: 400; } +.lc-docx p { margin: 0.5em 0; } +.lc-docx p:has(> strong:only-child) { margin: 1em 0 0.4em; padding-left: 0.6em; border-left: 3px solid var(--link); font-size: 1.05rem; } +.lc-docx p:has(> strong:only-child) strong { font-weight: 600; } +.lc-docx table { width: 100%; max-width: 100%; } +.lc-docx td, .lc-docx th { white-space: normal; } +.lc-docx table tr:first-child td { background: var(--header-bg); font-weight: 600; } +.lc-docx table tr:nth-child(even):not(:first-child) td { background: var(--row-alt); } +.lc-docx ul, .lc-docx ol { margin: 0.5em 0; padding-left: 1.6em; } +.lc-docx li { margin: 0.15em 0; } +.lc-docx blockquote { border-left: 3px solid var(--border); color: var(--muted); margin: 0.8em 0; padding: 0.2em 0 0.2em 0.8em; } +`.trim(); + +/* ============================================================================= + * DOCX CDN renderer (high-fidelity path) + * + * Embeds the binary as base64 inside a self-contained HTML document and + * relies on `docx-preview` loaded from a pinned CDN URL with SRI + * integrity to render it inside the Sandpack iframe. The iframe is a + * real browser DOM — `docx-preview`'s "browser-first" design is a + * feature here, not a limitation: we get cell shading, run-level + * colors/fonts, headers/footers, columns, and inline images at no + * server-side parsing cost. + * + * Trade-offs vs the mammoth path: + * + Far higher visual fidelity (4/5 vs 2/5). + * + No server-side jsdom; no extra Node deps; iframe sandbox isolates + * any parser bug from the API process. + * − base64 inflates the binary by ~33%, so files above + * `MAX_DOCX_CDN_BINARY_BYTES` fall back to the mammoth path so the + * wrapped HTML doesn't blow the `MAX_TEXT_CACHE_BYTES` (512KB) cap + * on `attachment.text`. Telemetry should track how often we hit the + * fallback — if it's frequent, the next move is to lift the cap + * for office types specifically rather than embed via signed URL. + * + * Library + version pinning (jsdelivr SRI hashes computed at the + * version listed; refresh by `openssl dgst -sha384 -binary FILE | + * openssl base64 -A` on the file at the URL): + * docx-preview 0.3.7 — Apache-2.0, ~75KB UMD + * jszip 3.10.1 — MIT, ~98KB (peer dep of docx-preview) + * Both are pinned to specific minor versions; SRI guarantees the byte + * content can't change underneath us even if the version were ever + * republished. + */ +const DOCX_PREVIEW_CDN = { + jszip: { + src: 'https://cdn.jsdelivr.net/npm/jszip@3.10.1/dist/jszip.min.js', + integrity: 'sha384-+mbV2IY1Zk/X1p/nWllGySJSUN8uMs+gUAN10Or95UBH0fpj6GfKgPmgC5EXieXG', + }, + docxPreview: { + src: 'https://cdn.jsdelivr.net/npm/docx-preview@0.3.7/dist/docx-preview.min.js', + integrity: 'sha384-Fw+ZM2MtvxCe867uRzZY5GtGP+gs0NLvrlJS768RZWuKhOHMN4Fln3i3gMt1NSyQ', + }, +} as const; + +/** + * Maximum DOCX binary size (in bytes) we'll embed via the CDN-rendered + * path. Empirical headroom: with ~33% base64 inflation and ~5KB of + * wrapper boilerplate, 350KB of binary fits well under the 512KB + * `MAX_TEXT_CACHE_BYTES` cap on `attachment.text` with margin to spare. + */ +const MAX_DOCX_CDN_BINARY_BYTES = 350 * 1024; + +/** + * Mirror of `MAX_TEXT_CACHE_BYTES` from `~/files/code/extract` — the + * 512 KB ceiling that `attachment.text` is truncated to before hitting + * the SSE wire and the database. We mirror (rather than import) to + * avoid the cycle: `extract.ts` already imports `bufferToOfficeHtml` + * from this module. The dispatcher uses this to drop CDN-with-fallback + * docs that would exceed the cap and fall back to mammoth-only. + * + * If the upstream constant ever changes, update this value too. The + * `cap-mirrors-extract` test in `html.spec.ts` pins the relationship. + */ +const OFFICE_HTML_OUTPUT_CAP = 512 * 1024; + +/** + * Build the CDN-rendered HTML document for a DOCX. The base64 payload + * lives inside a `` substring inside the binary. + * + * The CSP locks the page down to the pinned CDN host: scripts only + * from jsdelivr (with SRI), no outbound `fetch`/`XHR`, no eval, images + * only `self`/`data:`/`blob:` (docx-preview uses `URL.createObjectURL` + * for inline images), styles inline (`docx-preview` injects per-doc + * styles into `` at render time). + */ +function buildDocxCdnDocument(base64: string, mammothFallbackHtml: string): string { + /* `connect-src` allows fetches to: + * - `'self'`: the sandpack-static-server origin the iframe runs in + * (covers any same-origin sourcemap fetches the bundler embedded) + * - `https://cdn.jsdelivr.net`: where the renderer script came from + * (DevTools fetches `.min.js.map` from the same host as the script + * to map minified line numbers; with `'none'` the console fills + * with CSP violations every time DevTools is open) + * + * Exfiltration risk is minimal: the iframe is cross-origin to the + * LibreChat host so an attacker can't read application data from it, + * and the only meaningful target ('self' or jsdelivr) isn't useful + * for exfiltrating slide content to a host the attacker controls. */ + const csp = [ + "default-src 'none'", + "script-src https://cdn.jsdelivr.net 'unsafe-inline'", + "style-src 'unsafe-inline'", + "img-src 'self' data: blob:", + 'font-src data:', + "connect-src 'self' https://cdn.jsdelivr.net", + "base-uri 'none'", + "form-action 'none'", + ].join('; '); + /* Body styling for the embedded mammoth fallback. The CDN-rendered + * path normally hides this content, but on air-gapped networks where + * `cdn.jsdelivr.net` is blocked the fallback handler reveals it so + * the user gets a readable preview instead of the legacy "Preview + * unavailable" message — Codex P2 review on PR #12934. We inline the + * shared `DOCX_EXTRA_CSS` rules here (rather than `` to a + * cross-origin sheet) because the CSP locks `style-src` to inline + * only and the wrapped mammoth output uses the same `.lc-docx` + * classes. */ + return ` + + + + + +Preview + + + + + +
Loading preview…
+ + + + +`; +} + +/** + * Run mammoth + sanitization to produce the inner DOCX body HTML + * (the `
` contents). Shared between the standalone mammoth + * path and the CDN path's fallback embedding so both render through + * the exact same pipeline — no diverging sanitization rules. Codex P2 + * review on PR #12934. + */ +async function renderMammothBody(buffer: Buffer): Promise { + const { convertToHtml } = await import('mammoth'); + const result = await convertToHtml({ buffer }, { styleMap: DOCX_STYLE_MAP }); + return sanitizeOfficeHtml(result.value); +} + +async function wordDocToHtmlViaCdn(buffer: Buffer, mammothFallbackBody: string): Promise { + return buildDocxCdnDocument(buffer.toString('base64'), mammothFallbackBody); +} + +async function wordDocToHtmlViaMammoth(buffer: Buffer): Promise { + const sanitized = await renderMammothBody(buffer); + return wrapAsDocument(`
${sanitized}
`, DOCX_EXTRA_CSS); +} + +/** + * Whether the CDN-rendered DOCX path is enabled for this process. + * Operators on air-gapped or filtered corporate networks (where + * `cdn.jsdelivr.net` is blocked) should set + * `OFFICE_PREVIEW_DISABLE_CDN=true` so DOCX previews fall back to the + * server-side mammoth renderer instead of degrading to "Preview + * unavailable" in the iframe — Codex P2 review on PR #12934. + * + * Read at call time (rather than module load) so jest tests can flip + * the env in a `beforeEach` without `jest.resetModules()`. The cost is + * a single property access per render. Truthy values: `true`, `1`, + * `yes` (case-insensitive). Anything else (including unset) means + * "use the CDN path when the size dispatcher picks it." + */ +function isOfficePreviewCdnDisabled(): boolean { + const v = process.env.OFFICE_PREVIEW_DISABLE_CDN; + if (v == null) { + return false; + } + return /^(1|true|yes)$/i.test(v.trim()); +} + +/** + * Convert a `.docx` buffer to a sandboxed HTML document. Two render + * paths, chosen by file size: + * + * 1. **CDN-rendered (default for files ≤ 350 KB binary)**: embeds the + * binary as base64 and lets `docx-preview` render it inside the + * Sandpack iframe. High visual fidelity — preserves cell shading, + * run-level colors/fonts, headers/footers, columns, and images. + * The mammoth-rendered HTML is *also* embedded as a hidden + * `
` block; the iframe's bootstrap script + * reveals it whenever `docx-preview` fails to load (corporate + * firewall blocking jsdelivr, offline desktop, etc.) so air- + * gapped deployments still get a readable preview instead of a + * "Preview unavailable" message — Codex P2 review on PR #12934. + * 2. **Mammoth-only (fallback for larger files, files where the + * combined CDN-doc-with-fallback would blow the cache cap, OR + * when the CDN path is explicitly disabled via + * `OFFICE_PREVIEW_DISABLE_CDN=true`)**: server-side semantic HTML + * conversion. Lower fidelity (flat paragraphs, no shading) but + * produces compact output that fits the `MAX_TEXT_CACHE_BYTES` + * (512 KB) cap on `attachment.text` even for large documents, + * and works without external network. + * + * Both paths pre-flight through `assertSafeZipSize` so a zip-bomb DOCX + * is rejected before either renderer touches it — mammoth's internal + * extraction has no decompressed-size cap and would happily inflate a + * sub-1MB compressed bomb to 200+ MB of XML. See SEC review on PR + * #12934 for the original DoS finding. + */ +export async function wordDocToHtml(buffer: Buffer): Promise { + await assertSafeZipSize(buffer, { name: 'docx' }); + /* Opt-in LibreOffice path: highest fidelity for any DOCX feature + * mammoth/docx-preview can't reproduce (complex tables, drawing + * objects, charts, embedded objects). Returns null when disabled, + * binary missing, conversion failed, or output exceeds the cache + * cap — falls through to the existing pipeline in any of those + * cases so a misconfiguration doesn't break previews. */ + const lo = await tryLibreOfficePreview(buffer, 'docx', OFFICE_HTML_OUTPUT_CAP); + if (lo) { + return lo; + } + if (isOfficePreviewCdnDisabled() || buffer.length > MAX_DOCX_CDN_BINARY_BYTES) { + return wordDocToHtmlViaMammoth(buffer); + } + /* Render mammoth first so its sanitized output can be embedded as + * the iframe's air-gapped fallback. If the combined size would + * exceed the 512 KB cache cap, drop to mammoth-only — the user + * loses high-fidelity rendering but still sees the document. The + * size budget applies after mammoth runs because we can't know its + * output size from the binary size alone. */ + const mammothBody = await renderMammothBody(buffer); + const cdnDoc = await wordDocToHtmlViaCdn(buffer, mammothBody); + if (Buffer.byteLength(cdnDoc, 'utf-8') > OFFICE_HTML_OUTPUT_CAP) { + return wrapAsDocument(`
${mammothBody}
`, DOCX_EXTRA_CSS); + } + return cdnDoc; +} + +/* ============================================================================= + * XLSX / XLS / ODS → HTML (multi-sheet with pure-CSS tab strip) + * ============================================================================= */ + +/** A workbook sheet rendered to its `
` HTML and metadata. */ +interface RenderedSheet { + name: string; + html: string; + truncated: boolean; + totalRows: number; +} + +async function renderWorkbookSheets( + workbook: import('xlsx').WorkBook, + XLSX: typeof import('xlsx'), +): Promise { + const sheets: RenderedSheet[] = []; + for (const sheetName of workbook.SheetNames) { + const ws = workbook.Sheets[sheetName]; + const ref = ws['!ref']; + let totalRows = 0; + let truncated = false; + if (ref) { + const range = XLSX.utils.decode_range(ref); + totalRows = range.e.r - range.s.r + 1; + if (totalRows > SPREADSHEET_MAX_ROWS_PER_SHEET) { + const cappedEnd = range.s.r + SPREADSHEET_MAX_ROWS_PER_SHEET - 1; + ws['!ref'] = XLSX.utils.encode_range({ + s: range.s, + e: { r: cappedEnd, c: range.e.c }, + }); + truncated = true; + } + } + const html = XLSX.utils.sheet_to_html(ws, { editable: false, header: '', footer: '' }); + sheets.push({ name: sheetName, html, truncated, totalRows }); + } + return sheets; +} + +/** + * Build a self-contained HTML document with a pure-CSS tab strip for + * multi-sheet workbooks. Sheet switching uses radio inputs + `:checked ~` + * sibling selectors so no JavaScript is needed inside the iframe. + */ +function renderSpreadsheetHtml(sheets: RenderedSheet[]): string { + if (sheets.length === 0) { + return wrapAsDocument('

This workbook contains no sheets.

'); + } + + const tabStripCss = ` +.lc-sheet-tabs { display: flex; flex-wrap: wrap; gap: 2px; padding: 0 0 8px; border-bottom: 1px solid var(--border); margin-bottom: 12px; } +.lc-sheet-tab-radio { position: absolute; opacity: 0; pointer-events: none; } +.lc-sheet-tab-label { + cursor: pointer; + padding: 4px 12px; + border-radius: 6px 6px 0 0; + font-size: 0.8rem; + color: var(--muted); + border: 1px solid transparent; + border-bottom: none; + user-select: none; +} +.lc-sheet-tab-label:hover { color: var(--fg); background: var(--row-alt); } +.lc-sheet-panel { display: none; } +${sheets + .map( + (_, i) => + `#lc-sheet-tab-${i}:checked ~ .lc-sheet-tabs label[for="lc-sheet-tab-${i}"] { color: var(--fg); background: var(--tab-active-bg); border-color: var(--border); font-weight: 600; } +#lc-sheet-tab-${i}:checked ~ #lc-sheet-panel-${i} { display: block; }`, + ) + .join('\n')} +`.trim(); + + const radios = sheets + .map( + (_, i) => + ``, + ) + .join('\n'); + + const tabs = + sheets.length > 1 + ? `` + : ''; + + const panels = sheets + .map((s, i) => { + const banner = s.truncated + ? `
Showing first ${SPREADSHEET_MAX_ROWS_PER_SHEET.toLocaleString()} of ${s.totalRows.toLocaleString()} rows. Download the file to see the rest.
` + : ''; + return `
${banner}
${s.html}
`; + }) + .join('\n'); + + return wrapAsDocument(`${radios}\n${tabs}\n${panels}`, tabStripCss); +} + +function escapeHtml(input: string): string { + return input + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + +/** + * Convert a workbook buffer (`.xlsx`, `.xls`, `.ods`) to a sandboxed HTML + * document. Each sheet is rendered as its own `
` and the document + * carries a pure-CSS tab strip for sheet switching. + * + * Pre-flights ZIP-backed formats (`.xlsx`/`.ods`) through + * `assertSafeZipSize` to reject zip bombs before SheetJS reaches them. + * `.xls` is a binary CFB format, not a ZIP — it doesn't have the + * decompression-amplification attack surface, so the safety check is + * skipped for it (yauzl would reject it as malformed anyway). + */ +export async function excelSheetToHtml(buffer: Buffer): Promise { + /* Cheap magic-byte check so we only run the ZIP validator on actual + * ZIP-backed inputs. `.xls` (BIFF/CFB) starts with `D0 CF 11 E0`; ZIPs + * start with `PK\x03\x04`. Skipping the validator on a non-ZIP input + * also avoids confusing yauzl errors leaking out as ZipBombError. */ + if (buffer.length >= 4 && buffer[0] === 0x50 && buffer[1] === 0x4b) { + await assertSafeZipSize(buffer, { name: 'spreadsheet' }); + } + const XLSX = await import('xlsx'); + const workbook = XLSX.read(buffer, { type: 'buffer' }); + const sheets = await renderWorkbookSheets(workbook, XLSX); + /* The per-sheet HTML from `sheet_to_html` is generally well-formed but we + * still sanitize it (defense in depth). The chrome (tab strip, banners) is + * our own code and doesn't need sanitization. */ + const sanitizedSheets = await Promise.all( + sheets.map(async (s) => ({ ...s, html: await sanitizeOfficeHtml(s.html) })), + ); + return renderSpreadsheetHtml(sanitizedSheets); +} + +/* ============================================================================= + * CSV → HTML + * ============================================================================= */ + +/** Convert a CSV buffer to a sandboxed HTML document with a single table. */ +export async function csvToHtml(buffer: Buffer): Promise { + const XLSX = await import('xlsx'); + const text = buffer.toString('utf-8'); + /* `XLSX.read` with `type: 'string'` accepts CSV as well as XML/JSON + * formats; the default sheet name for CSV is `Sheet1` which we relabel + * below for a friendlier tab. */ + const workbook = XLSX.read(text, { type: 'string', raw: true }); + const sheets = await renderWorkbookSheets(workbook, XLSX); + // Single sheet for CSV — relabel to "CSV" for clarity, no tab strip emitted. + const sanitized = await Promise.all( + sheets.map(async (s) => ({ ...s, name: 'CSV', html: await sanitizeOfficeHtml(s.html) })), + ); + return renderSpreadsheetHtml(sanitized); +} + +/* ============================================================================= + * PPTX → slide-list HTML + * ============================================================================= */ + +interface PptxSlide { + number: number; + title: string; + body: string[]; +} + +/** + * Stream `ppt/slides/slide*.xml` out of a PPTX buffer using yauzl. Mirrors + * the anti-zip-bomb pattern used by `extractOdtContentXml` in `crud.ts` — + * counts real decompressed bytes mid-inflate so a falsified central-directory + * `uncompressedSize` cannot bypass the cap. Returns slides in slide-number + * order; ignores everything else in the archive. + * + * Uses `yauzl.fromBuffer` (no disk I/O) — the safety pre-flight in + * `assertSafeZipSize` already proved the buffer is well-formed enough + * to walk, and fromBuffer keeps the hot path memory-only. + */ +function extractPptxSlideXml(buffer: Buffer): Promise> { + return new Promise((resolve, reject) => { + yauzl.fromBuffer(buffer, { lazyEntries: true }, (err, zipfile) => { + if (err) { + return reject(err); + } + if (!zipfile) { + return reject(new Error('Failed to open PPTX buffer')); + } + + let settled = false; + const slides: Array<{ number: number; xml: string }> = []; + const finish = (error: Error | null) => { + if (settled) { + return; + } + settled = true; + try { + zipfile.close(); + } catch { + /* zipfile.close() is best-effort — yauzl will throw if a + * stream is mid-flight. We've already settled the outer + * promise, so swallow this so the original error (if any) + * isn't replaced by a close-time exception. Mirrors + * `assertSafeZipSize`'s defensive pattern in zipSafety.ts. */ + } + if (error) { + reject(error); + } else { + slides.sort((a, b) => a.number - b.number); + resolve(slides); + } + }; + + zipfile.readEntry(); + zipfile.on('entry', (entry: yauzl.Entry) => { + const slideMatch = entry.fileName.match(/^ppt\/slides\/slide(\d+)\.xml$/); + if (!slideMatch) { + zipfile.readEntry(); + return; + } + if (slides.length >= PPTX_MAX_SLIDES) { + // Cap reached — drain remaining entries silently. + zipfile.readEntry(); + return; + } + const slideNumber = Number.parseInt(slideMatch[1], 10); + zipfile.openReadStream(entry, (streamErr, readStream) => { + if (streamErr || !readStream) { + return finish(streamErr ?? new Error('Failed to open slide stream')); + } + + let totalBytes = 0; + const chunks: Buffer[] = []; + + readStream.on('data', (chunk: Buffer) => { + totalBytes += chunk.byteLength; + if (totalBytes > PPTX_MAX_ENTRY_SIZE) { + readStream.destroy( + new Error( + `PPTX slide${slideNumber}.xml exceeds the ${PPTX_MAX_ENTRY_SIZE / megabyte}MB decompressed limit`, + ), + ); + return; + } + chunks.push(chunk); + }); + + readStream.on('end', () => { + slides.push({ number: slideNumber, xml: Buffer.concat(chunks).toString('utf-8') }); + zipfile.readEntry(); + }); + readStream.on('error', (readErr: Error) => finish(readErr)); + }); + }); + + zipfile.on('end', () => finish(null)); + zipfile.on('error', (zipErr: Error) => finish(zipErr)); + }); + }); +} + +/** + * Pull the visible text out of a slide XML. PPTX text lives in `` + * elements, grouped by `` (paragraph). We keep paragraph boundaries so + * the rendered slide preserves bullet/line structure. + * + * Pure regex (no DOMParser available in Node by default; the markup is + * tightly defined by the OOXML spec, so regex is robust enough for the text- + * only preview). + */ +function extractSlideText(xml: string): { title: string; body: string[] } { + const paragraphs: string[] = []; + const paragraphMatches = xml.matchAll(/]*>([\s\S]*?)<\/a:p>/g); + for (const match of paragraphMatches) { + const innerXml = match[1]; + const runs: string[] = []; + const runMatches = innerXml.matchAll(/]*>([\s\S]*?)<\/a:t>/g); + for (const run of runMatches) { + runs.push(decodeXmlEntities(run[1])); + } + const text = runs.join('').trim(); + if (text.length > 0) { + paragraphs.push(text); + } + } + const title = paragraphs[0] ?? ''; + const body = paragraphs.slice(1); + return { title, body }; +} + +const XML_ENTITIES: Record = { + '<': '<', + '>': '>', + '&': '&', + '"': '"', + ''': "'", +}; +function decodeXmlEntities(input: string): string { + return input.replace(/&(?:lt|gt|amp|quot|apos);/g, (m) => XML_ENTITIES[m] ?? m); +} + +/* CSS for the slide-list view. Extracted to a constant so it can also + * be inlined into the CDN doc when the slide-list is embedded as a + * fallback there — Codex feedback / manual e2e on PR #12934 ("pptx- + * preview created an empty wrapper for the pptxgenjs deck"). */ +const PPTX_SLIDE_LIST_CSS = ` +.lc-pptx-list { display: flex; flex-direction: column; gap: 16px; padding: 0; margin: 0; list-style: none; } +.lc-pptx-slide { + border: 1px solid var(--border); + border-radius: 8px; + padding: 16px 20px; + background: var(--bg); + position: relative; +} +.lc-pptx-slide-number { + display: inline-block; + font-size: 0.7rem; + text-transform: uppercase; + letter-spacing: 0.06em; + color: var(--muted); + margin-bottom: 6px; +} +.lc-pptx-slide-title { font-size: 1.15rem; font-weight: 600; margin: 0 0 8px; } +.lc-pptx-slide-body { margin: 0; padding-left: 1.25em; } +.lc-pptx-slide-body li { margin: 0.2em 0; } +.lc-pptx-slide-empty { color: var(--muted); font-style: italic; margin: 0; } +`.trim(); + +/** + * Build the slide-list body HTML (just the `
    ` element, no document + * wrapper). Used both for standalone slide-list rendering and for + * embedding inside the CDN doc as an air-gap / parser-failure + * fallback — same pattern as the mammoth fallback inside the DOCX CDN + * doc. Returns an empty-state banner for zero-slide decks. + */ +function renderPptxSlidesBody(slides: PptxSlide[]): string { + if (slides.length === 0) { + return '

    This presentation contains no readable slides.

    '; + } + const items = slides + .map((slide) => { + const titleHtml = slide.title + ? `

    ${escapeHtml(slide.title)}

    ` + : ''; + const bodyHtml = + slide.body.length > 0 + ? `
      ${slide.body.map((line) => `
    • ${escapeHtml(line)}
      • `).join('')}
    ` + : !slide.title + ? '

    (empty slide)

    ' + : ''; + return `
  1. + Slide ${slide.number} + ${titleHtml} + ${bodyHtml} +
    2. `; + }) + .join('\n'); + return `
      ${items}
    `; +} + +function renderPptxSlidesHtml(slides: PptxSlide[]): string { + return wrapAsDocument(renderPptxSlidesBody(slides), PPTX_SLIDE_LIST_CSS); +} + +/** + * Convert a `.pptx` buffer to a slide-list HTML document — text-only + * preview rendering each slide as a card (slide number, title, body + * bullets). Honest about NOT preserving complex visual layouts, + * charts, theming, or embedded media. Used as the safe fallback path + * when the CDN renderer is disabled, the binary exceeds the embed + * size cap, or the high-fidelity path can't be reached. + * + * Pre-flights through `assertSafeZipSize` so a zip-bomb PPTX can't blow + * up the slide-XML extraction pass. + */ +export async function pptxToSlideListHtml(buffer: Buffer): Promise { + await assertSafeZipSize(buffer, { name: 'pptx' }); + const rawSlides = await extractPptxSlideXml(buffer); + const slides: PptxSlide[] = rawSlides.map(({ number, xml }) => { + const { title, body } = extractSlideText(xml); + return { number, title, body }; + }); + return renderPptxSlidesHtml(slides); +} + +/* ============================================================================= + * PPTX CDN renderer (high-fidelity path) + * + * Mirrors the DOCX CDN architecture: embeds the binary as base64 and + * lets `pptx-preview` (loaded from a pinned jsdelivr URL with SRI) do + * the rendering inside the Sandpack iframe. The library is ISC-licensed + * (npm package permits commercial use) and ships a UMD bundle that + * exposes a global `pptxPreview.init(container, options)` → + * `previewer.preview(arrayBuffer)` API per the package README. + * + * Trade-offs vs the slide-list path: + * + Far higher visual fidelity — preserves slide layouts, theme + * colors, fonts, basic shape positioning, embedded images. + * + No server-side rendering cost; iframe sandbox isolates parser + * bugs from the API process. + * − UMD bundle is ~1.36 MB (contains echarts, tslib, lodash, uuid, + * jszip). Loaded once per iframe lifetime; browser-cached after. + * − Renderer hasn't been broadly browser-tested by us across the + * full spectrum of decks. See PR #12934 commit notes — fallback + * paths cover the cases where we hit a class of files that don't + * render: client-side "Preview unavailable" UX (CDN unreachable + * or `typeof pptxPreview === 'undefined'`) and the + * `OFFICE_PREVIEW_DISABLE_CDN` env-var hatch (forces the slide- + * list server-side). + */ +const PPTX_PREVIEW_CDN = { + pptxPreview: { + src: 'https://cdn.jsdelivr.net/npm/pptx-preview@1.0.7/dist/pptx-preview.umd.js', + integrity: 'sha384-CwntHHT2FbwZXuCmbf6K93YaEB9xRVVLaqFJ7pdMykeQABb/3MA0sbt2lGbgi1Mr', + }, +} as const; + +/** + * Same 350 KB binary cap as DOCX — keeps the base64-inflated wrapped + * HTML under `MAX_TEXT_CACHE_BYTES` (512 KB) on `attachment.text`. + * PPTX files often exceed this once embedded media is involved; the + * dispatcher's slide-list fallback handles the larger cases. + */ +const MAX_PPTX_CDN_BINARY_BYTES = 350 * 1024; + +/** + * Build the CDN-rendered HTML document for a PPTX. Same wrapper shape + * and CSP as the DOCX equivalent — see `buildDocxCdnDocument` for the + * security-relevant rationale on script-src/connect-src/base-uri/etc. + * + * Slides need a fixed-size container for `pptx-preview` to render into + * — it computes per-slide layouts against the configured width/height + * rather than against the iframe's variable size. We use 960×540 + * (16:9 standard) and let CSS scale the rendered output to fit the + * iframe via `transform: scale(...)`. The slides scroll vertically + * once the renderer paints them. + */ +function buildPptxCdnDocument(base64: string, slideListFallbackBody: string): string { + /* PPTX-specific CSP relaxations vs DOCX: + * - `worker-src blob:` — pptx-preview's bundled echarts dep spins up + * Web Workers via blob: URLs for chart rendering. Without this, + * workers default to `default-src 'none'`, get blocked, and + * echarts throws an unhandled-rejection deep inside its async + * pipeline that we can't catch from the bootstrap script. The + * visible symptom is a black iframe with the renderer half- + * started. Allowing blob:-only workers is the minimum surface to + * unblock rendering without permitting arbitrary worker URLs. */ + /* `connect-src` allows fetches to: + * - `'self'`: the sandpack-static-server origin the iframe runs in + * (covers any same-origin fetches pptx-preview or its bundled + * echarts make at render time, plus same-origin sourcemap loads) + * - `https://cdn.jsdelivr.net`: where the renderer script came from + * (DevTools fetches `.min.js.map` from there to map minified + * line numbers; with `'none'` the console fills with CSP + * violations every time DevTools is open) + * + * Exfiltration risk is minimal: the iframe is cross-origin to the + * LibreChat host so an attacker can't read application data, and + * the only meaningful targets ('self' or jsdelivr) aren't useful + * for exfiltrating slide content to a host the attacker controls. */ + const csp = [ + "default-src 'none'", + "script-src https://cdn.jsdelivr.net 'unsafe-inline'", + "style-src 'unsafe-inline'", + 'worker-src blob:', + "img-src 'self' data: blob:", + 'font-src data:', + "connect-src 'self' https://cdn.jsdelivr.net", + "base-uri 'none'", + "form-action 'none'", + ].join('; '); + return ` + + + + + +Preview + + + + +
    Loading preview…
    + + + + +`; +} + +/** + * Render the slide-list body (just the `
      ` HTML) for a buffer. + * Used by `pptxToHtml` to embed as a fallback inside the CDN doc and + * by `pptxToSlideListHtmlInternal` for the standalone path. */ +async function renderPptxSlidesBodyForBuffer(buffer: Buffer): Promise { + const rawSlides = await extractPptxSlideXml(buffer); + const slides: PptxSlide[] = rawSlides.map(({ number, xml }) => { + const { title, body } = extractSlideText(xml); + return { number, title, body }; + }); + return renderPptxSlidesBody(slides); +} + +async function pptxToHtmlViaCdn(buffer: Buffer, slideListFallbackBody: string): Promise { + return buildPptxCdnDocument(buffer.toString('base64'), slideListFallbackBody); +} + +/** + * Convert a `.pptx` buffer to a sandboxed HTML document. Two render + * paths, chosen by file size and the `OFFICE_PREVIEW_DISABLE_CDN` + * escape hatch: + * + * 1. **CDN-rendered (default for files ≤ 350 KB binary)**: embeds + * the binary as base64 and lets `pptx-preview` render it inside + * the Sandpack iframe. High visual fidelity. + * 2. **Slide-list (fallback)**: server-side text-only extraction + * (each slide → card with title + bullets). Lower fidelity but + * compact, network-independent, and proven against arbitrary + * decks. Triggered when the binary exceeds the embed cap, when + * `OFFICE_PREVIEW_DISABLE_CDN=true` is set (air-gapped / + * filtered networks), or when the CDN script fails to load + * (handled iframe-side). + * + * Pre-flights through `assertSafeZipSize` so a zip-bomb PPTX is + * rejected before either renderer touches it. + */ +export async function pptxToHtml(buffer: Buffer): Promise { + await assertSafeZipSize(buffer, { name: 'pptx' }); + /* Opt-in LibreOffice path: PDF rendering of slides preserves layout, + * fonts, charts, and embedded objects far better than the slide-list + * fallback or even pptx-preview's canvas approach. Falls through on + * any error per the contract in `tryLibreOfficePreview`. */ + const lo = await tryLibreOfficePreview(buffer, 'pptx', OFFICE_HTML_OUTPUT_CAP); + if (lo) { + return lo; + } + if (isOfficePreviewCdnDisabled() || buffer.length > MAX_PPTX_CDN_BINARY_BYTES) { + return pptxToSlideListHtmlInternal(buffer); + } + /* Render the slide-list first so we can embed it inside the CDN doc + * as a runtime fallback. pptx-preview is unreliable on PPTXs not + * generated by Microsoft PowerPoint (e.g. pptxgenjs decks) — it + * silently produces empty wrappers. The iframe bootstrap detects + * the empty-render case and reveals this slide-list fallback so the + * user always gets readable content. Manual e2e on PR #12934. */ + const slideListBody = await renderPptxSlidesBodyForBuffer(buffer); + const cdnDoc = await pptxToHtmlViaCdn(buffer, slideListBody); + /* Combined size budget: if base64 binary + slide-list fallback + + * wrapper would exceed the cache cap, drop CDN entirely and ship + * the slide-list standalone. Same pattern as the DOCX dispatcher's + * size budget. */ + if (Buffer.byteLength(cdnDoc, 'utf-8') > OFFICE_HTML_OUTPUT_CAP) { + return pptxToSlideListHtmlInternal(buffer); + } + return cdnDoc; +} + +/** + * Slide-list path that skips its own `assertSafeZipSize` call — used + * by `pptxToHtml` after the dispatcher has already validated the + * buffer. `pptxToSlideListHtml` (public, validating) stays the entry + * point for tests and direct callers. + */ +async function pptxToSlideListHtmlInternal(buffer: Buffer): Promise { + const rawSlides = await extractPptxSlideXml(buffer); + const slides: PptxSlide[] = rawSlides.map(({ number, xml }) => { + const { title, body } = extractSlideText(xml); + return { number, title, body }; + }); + return renderPptxSlidesHtml(slides); +} + +/** + * Test-only re-exports. Each path has distinct visual-fidelity and + * size characteristics that warrant direct testing rather than + * round-tripping through the size-based dispatcher with synthetically + * padded fixtures. Not part of the public API — callers in production + * code should always go through `wordDocToHtml` / `pptxToHtml`. + */ +export const _internal = { + wordDocToHtmlViaCdn, + wordDocToHtmlViaMammoth, + MAX_DOCX_CDN_BINARY_BYTES, + OFFICE_HTML_OUTPUT_CAP, + pptxToHtmlViaCdn, + MAX_PPTX_CDN_BINARY_BYTES, +}; + +/* ============================================================================= + * Dispatcher + * ============================================================================= */ + +const DOCX_MIME = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'; +const PPTX_MIME = 'application/vnd.openxmlformats-officedocument.presentationml.presentation'; +const ODS_MIME = 'application/vnd.oasis.opendocument.spreadsheet'; +const CSV_MIME_PATTERN = /^(text\/csv|application\/csv|text\/comma-separated-values)$/i; + +function extensionOf(name: string): string { + const dot = name.lastIndexOf('.'); + if (dot < 0 || dot === name.length - 1) { + return ''; + } + return name.slice(dot + 1).toLowerCase(); +} + +/** + * Strip MIME parameters (`; charset=utf-8`, `; boundary=...`) and + * lowercase the result so exact-match MIME comparisons survive servers + * and tool sandboxes that decorate Content-Type headers. Without this, + * an extensionless `text/csv; charset=utf-8` would slip past the bucket + * predicate (returning null) while the client's MIME router — which + * already strips parameters via `baseMime` — would route it to + * SPREADSHEET expecting an HTML body that the backend never produced. + * Mirrors `client/src/utils/artifacts.ts:baseMime`. + */ +function baseMime(mime: string): string { + if (!mime) { + return ''; + } + const semi = mime.indexOf(';'); + return (semi < 0 ? mime : mime.slice(0, semi)).trim().toLowerCase(); +} + +/** + * The four buckets the dispatcher can route to. Used by both + * `bufferToOfficeHtml` (which actually renders) and `officeHtmlBucket` + * (which the upstream gate in `extract.ts` calls to decide whether to + * invoke the dispatcher at all). Sharing one source of truth prevents the + * gate from going out of sync with the dispatcher — a previous version + * had a narrower extension-only gate that missed extensionless office + * files identified solely by MIME (e.g. a tool emitting `data` with + * `text/csv`), which then routed to the SPREADSHEET bucket on the + * client expecting full HTML and got raw text instead. + */ +export type OfficeHtmlBucket = 'docx' | 'spreadsheet' | 'csv' | 'pptx'; + +const OFFICE_EXTENSIONS: Record = { + docx: 'docx', + csv: 'csv', + xlsx: 'spreadsheet', + xls: 'spreadsheet', + ods: 'spreadsheet', + pptx: 'pptx', +}; + +/** + * Classify a file by extension OR MIME into the bucket the office HTML + * dispatcher would route it to, or `null` if no producer applies. + * + * **Extension always wins** over MIME for any known office extension. + * Without this precedence, a `deck.pptx` whose `Content-Type` was + * mislabeled `text/csv` (a known footgun when tool sandboxes ship + * generic MIMEs) would be routed to `csvToHtml` and try to parse a + * binary PPTX as CSV — yielding garbage at best, a parse exception at + * worst. + * + * **MIME fallback fires ONLY when the extension is empty.** This + * mirrors the client's `detectArtifactTypeFromFile` precedence: the + * client routes any file with a known non-office extension (`.txt`, + * `.md`, `.json`, etc.) to its non-office bucket REGARDLESS of MIME. + * If the server produced office HTML for `.txt + DOCX MIME`, the + * client would route by extension to PLAIN_TEXT, escape the HTML + * through the markdown viewer, and the user would see raw `...` + * markup instead of the rendered preview. Codex P2 review on PR + * #12934 — symmetric server/client extension precedence is the only + * way to avoid this kind of mismatch without a shared routing table. + * + * Trade-off: a true DOCX renamed to `myfile.bin` + DOCX MIME no longer + * routes through office HTML on the server (it falls to the existing + * extract path → `textFormat: 'text'` → client renders raw bytes via + * PLAIN_TEXT). Acceptable: extension-vs-MIME mismatch is user error, + * and the previous behavior would have surfaced as a security-gate + * fall-through to PLAIN_TEXT on the client anyway (no preview either + * way; the new behavior just avoids producing HTML the client can't + * use). + */ +export function officeHtmlBucket(name: string, mimeType: string): OfficeHtmlBucket | null { + const ext = extensionOf(name); + const byExtension = OFFICE_EXTENSIONS[ext]; + if (byExtension) { + return byExtension; + } + if (ext !== '') { + /* Non-empty non-office extension wins — see contract above. */ + return null; + } + /* MIME fallback for genuinely extensionless files (tool-emitted + * blobs with generic names, etc.). Strip parameters first — + * Content-Type headers routinely carry `; charset=utf-8` or + * `; boundary=...` decorations that would otherwise fail exact- + * match comparisons. */ + const normalized = baseMime(mimeType); + if (normalized === DOCX_MIME) { + return 'docx'; + } + if (CSV_MIME_PATTERN.test(normalized)) { + return 'csv'; + } + if (excelMimeTypes.test(normalized) || normalized === ODS_MIME) { + return 'spreadsheet'; + } + if (normalized === PPTX_MIME) { + return 'pptx'; + } + return null; +} + +/** + * Route an office-format buffer to the matching HTML producer. Returns `null` + * if the file isn't a recognized office type — caller should fall through to + * its existing text/binary handling. + */ +export async function bufferToOfficeHtml( + buffer: Buffer, + name: string, + mimeType: string, +): Promise { + const bucket = officeHtmlBucket(name, mimeType); + switch (bucket) { + case 'docx': + return wordDocToHtml(buffer); + case 'csv': + return csvToHtml(buffer); + case 'spreadsheet': + return excelSheetToHtml(buffer); + case 'pptx': + return pptxToHtml(buffer); + default: + return null; + } +} diff --git a/packages/api/src/files/documents/libreoffice.spec.ts b/packages/api/src/files/documents/libreoffice.spec.ts new file mode 100644 index 000000000000..449ade21bc11 --- /dev/null +++ b/packages/api/src/files/documents/libreoffice.spec.ts @@ -0,0 +1,385 @@ +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import { + _resetLibreOfficeProbeCache, + buildPdfEmbedDocument, + convertOfficeToPdf, + isLibreOfficeEnabled, + isLibreOfficeEnabledFor, + LibreOfficeConversionError, + LibreOfficeUnavailableError, + LIBREOFFICE_TIMEOUT_MS, + MAX_LIBREOFFICE_PDF_BYTES, + probeLibreOfficeBinary, + tryLibreOfficePreview, +} from './libreoffice'; + +/* Detect whether the host has a LibreOffice binary so the integration + * tests below can conditionally engage. Most CI runners don't have + * LibreOffice installed (it's a 500 MB dependency); we still exercise + * the env-gating, the wrapper builder, and the failure-fallthrough + * contract in those environments. */ +function hasLibreOfficeOnPath(): boolean { + for (const candidate of ['soffice', 'libreoffice']) { + const result = spawnSync(candidate, ['--version'], { stdio: 'ignore' }); + if (result.status === 0) { + return true; + } + } + return false; +} +const LIBREOFFICE_INSTALLED = hasLibreOfficeOnPath(); +const itIfLibreOffice = LIBREOFFICE_INSTALLED ? it : it.skip; + +/* Fixtures sit next to the source files (matches `html.spec.ts` resolution). */ +const FIXTURES_DIR = __dirname; +function readFixture(name: string): Buffer { + return fs.readFileSync(path.join(FIXTURES_DIR, name)); +} + +describe('libreoffice (env gating + wrapper)', () => { + const ORIGINAL_FLAG = process.env.OFFICE_PREVIEW_LIBREOFFICE; + + afterEach(() => { + _resetLibreOfficeProbeCache(); + if (ORIGINAL_FLAG === undefined) { + delete process.env.OFFICE_PREVIEW_LIBREOFFICE; + } else { + process.env.OFFICE_PREVIEW_LIBREOFFICE = ORIGINAL_FLAG; + } + }); + + describe('isLibreOfficeEnabled (any-format check)', () => { + it.each([['true'], ['1'], ['yes'], ['TRUE'], ['Yes'], [' true ']])( + 'returns true for %j (truthy = all formats enabled)', + (value) => { + process.env.OFFICE_PREVIEW_LIBREOFFICE = value; + expect(isLibreOfficeEnabled()).toBe(true); + }, + ); + + it.each([['false'], ['0'], ['no'], ['']])('returns false for %j', (value) => { + process.env.OFFICE_PREVIEW_LIBREOFFICE = value; + expect(isLibreOfficeEnabled()).toBe(false); + }); + + it('returns false when the env var is unset', () => { + delete process.env.OFFICE_PREVIEW_LIBREOFFICE; + expect(isLibreOfficeEnabled()).toBe(false); + }); + + it('returns true for a comma-separated format list (any format counts)', () => { + process.env.OFFICE_PREVIEW_LIBREOFFICE = 'pptx'; + expect(isLibreOfficeEnabled()).toBe(true); + process.env.OFFICE_PREVIEW_LIBREOFFICE = 'pptx,docx'; + expect(isLibreOfficeEnabled()).toBe(true); + }); + }); + + describe('isLibreOfficeEnabledFor (per-format opt-in)', () => { + /* Per-format gating lets operators trade off cold-start latency + * against fidelity per format. DOCX renders ~instantly via + * docx-preview, but PPTX needs LibreOffice for any reasonable + * fidelity (pptx-preview chokes on pptxgenjs decks). Operators + * should be able to enable LibreOffice for PPTX only. */ + it.each([ + ['true', 'pptx', true], + ['true', 'docx', true], + ['true', 'xlsx', true], + ['1', 'pptx', true], + ['yes', 'docx', true], + ['false', 'pptx', false], + ['', 'pptx', false], + ['pptx', 'pptx', true], + ['pptx', 'docx', false], + ['docx', 'pptx', false], + ['docx', 'docx', true], + ['pptx,docx', 'pptx', true], + ['pptx,docx', 'docx', true], + ['pptx,docx', 'xlsx', false], + [' pptx , docx ', 'pptx', true], // whitespace-tolerant + ['PPTX', 'pptx', true], // case-insensitive + ['pptx, ,docx', 'docx', true], // empty list entries dropped + ['pptx, ,docx', 'pptx', true], + ['pptx, ,docx', 'xlsx', false], + ])('env=%j format=%j → %s', (envValue, format, expected) => { + process.env.OFFICE_PREVIEW_LIBREOFFICE = envValue; + expect(isLibreOfficeEnabledFor(format)).toBe(expected); + }); + + it('returns false for any format when the env var is unset', () => { + delete process.env.OFFICE_PREVIEW_LIBREOFFICE; + expect(isLibreOfficeEnabledFor('pptx')).toBe(false); + expect(isLibreOfficeEnabledFor('docx')).toBe(false); + expect(isLibreOfficeEnabledFor('xlsx')).toBe(false); + }); + + it('matches format names case-insensitively', () => { + process.env.OFFICE_PREVIEW_LIBREOFFICE = 'pptx'; + expect(isLibreOfficeEnabledFor('PPTX')).toBe(true); + expect(isLibreOfficeEnabledFor('Pptx')).toBe(true); + }); + }); + + describe('buildPdfEmbedDocument', () => { + /* The wrapper structure is the security-relevant surface. These + * tests lock down the CSP, the iframe shape, and the fallback + * contract — independent of whether LibreOffice is actually + * installed. Same posture as the docx-preview / pptx-preview CDN + * wrappers. */ + const FAKE_PDF_B64 = 'JVBERi0xLjQK'; // "%PDF-1.4\n" base64 + + it('emits a complete sandboxed HTML document that renders via pdf.js to canvas', () => { + const html = buildPdfEmbedDocument(FAKE_PDF_B64); + expect(html).toMatch(/^/); + expect(html).toContain('Preview'); + /* PDF bytes embedded as a base64 data block — pdf.js decodes + * them at runtime and renders to canvas. We do NOT use any + * `