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 016f3a71c6..099794821d 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,11 @@ 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), codeServerHttpAgent: new http.Agent({ keepAlive: false }), codeServerHttpsAgent: new https.Agent({ keepAlive: false }), }; diff --git a/api/server/services/Files/Code/process.spec.js b/api/server/services/Files/Code/process.spec.js index e0d7b08fd3..8b12278c9a 100644 --- a/api/server/services/Files/Code/process.spec.js +++ b/api/server/services/Files/Code/process.spec.js @@ -43,6 +43,7 @@ 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')); jest.mock('@librechat/api', () => { const http = require('http'); const https = require('https'); @@ -63,6 +64,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 }), }; diff --git a/client/src/utils/__tests__/artifacts.test.ts b/client/src/utils/__tests__/artifacts.test.ts index 455e518afa..b664bc3456 100644 --- a/client/src/utils/__tests__/artifacts.test.ts +++ b/client/src/utils/__tests__/artifacts.test.ts @@ -225,6 +225,27 @@ describe('detectArtifactTypeFromFile', () => { ).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, diff --git a/client/src/utils/artifacts.ts b/client/src/utils/artifacts.ts index 11d360a94f..c51e83cfa3 100644 --- a/client/src/utils/artifacts.ts +++ b/client/src/utils/artifacts.ts @@ -719,8 +719,19 @@ export function detectArtifactTypeFromFile( * pre-flag period and the safe default ('text' bucket) is OK for * those — they were rendering correctly before this flag, and the * markdown viewer escaping HTML is a safety upgrade, not a - * regression. Codex P1 review on PR #12934. */ + * regression. Codex P1 review on PR #12934. + * + * Empty-text exception: if the office attachment has NO text at all, + * downgrading to PLAIN_TEXT would route an empty-text-tolerant + * bucket onto the panel (a half-rendered card showing only the + * filename). The historical contract for office types with missing + * text is "fall through to the legacy download path"; preserve that + * by returning null here. The downgrade only matters when there's + * actual text content that needs safe-escaping. */ if (OFFICE_HTML_BUCKETS.has(type) && attachment.textFormat !== 'html') { + if (!attachment.text) { + return null; + } return TOOL_ARTIFACT_TYPES.PLAIN_TEXT; } if (