diff --git a/application/api/user/attachments/routes.py b/application/api/user/attachments/routes.py index f3e7a8b5..e8d7491d 100644 --- a/application/api/user/attachments/routes.py +++ b/application/api/user/attachments/routes.py @@ -39,6 +39,9 @@ from application.stt.stt_creator import STTCreator from application.tts.tts_creator import TTSCreator from application.upload_limits import ( copy_upload_to_path, + enforce_parseable_attachment, + is_unsupported_upload_message, + UnsupportedUploadTypeError, upload_limit_message, UploadTooLargeError, ) @@ -130,6 +133,8 @@ def _get_store_attachment_user_error(exc: Exception) -> str: return build_stt_file_size_limit_message() if isinstance(exc, UploadTooLargeError): return upload_limit_message() + if isinstance(exc, UnsupportedUploadTypeError): + return str(exc) return "Failed to process file" @@ -214,6 +219,10 @@ class StoreAttachment(Resource): with tempfile.TemporaryDirectory() as temp_dir: staged_path = os.path.join(temp_dir, original_filename) copy_upload_to_path(file, staged_path) + # Refuse here — nothing is stored or queued yet — so a + # video the picker let through never reaches the + # worker's reader, which would open it as text. + enforce_parseable_attachment(staged_path, original_filename) with open(staged_path, "rb") as staged_stream: staged_upload = FileStorage( stream=staged_stream, @@ -263,6 +272,22 @@ class StoreAttachment(Resource): ), 413, ) + if errors and all( + is_unsupported_upload_message(error.get("error")) for error in errors + ): + # Tell the user which type was refused rather than the + # generic copy — this is the branch a phone picker that + # ignores ``accept`` lands in. + return make_response( + jsonify( + { + "success": False, + "message": errors[0]["error"], + "errors": errors, + } + ), + 400, + ) return make_response( jsonify({"status": "error", "message": "No valid files to upload"}), 400, diff --git a/application/parser/file/constants.py b/application/parser/file/constants.py index 4656727b..079a13d2 100644 --- a/application/parser/file/constants.py +++ b/application/parser/file/constants.py @@ -1,5 +1,7 @@ """Shared file-extension constants for parsing and ingestion flows.""" +import os + from application.stt.constants import SUPPORTED_AUDIO_EXTENSIONS @@ -25,3 +27,65 @@ SUPPORTED_SOURCE_EXTENSIONS = ( *SUPPORTED_SOURCE_IMAGE_EXTENSIONS, *SUPPORTED_AUDIO_EXTENSIONS, ) + +# Suffixes the attachment path has a dedicated parser for — every key of +# ``get_default_file_extractor()`` plus ``.txt``, which needs none. Kept as a +# literal (importing ``bulk`` here would drag docling into the API process), +# with ``tests/parser/file/test_constants.py`` asserting the two agree. +# +# This is *not* the whole attachment allow-list: a suffix with no parser is +# read by ``SimpleDirectoryReader``'s plain-text fallthrough, which is right +# for a .py or a .log and catastrophic for a video — the reader "extracts" +# megabytes of binary garbage, truncates it, and stores it with +# ``extraction.status == "ok"``. So unparsed suffixes are admitted on +# content instead (``upload_limits.enforce_parseable_attachment``): text +# passes, binary is refused. Zip is deliberately absent — source ingestion +# extracts archives, the attachment path does not, and a zip fails the +# content check like any other binary. +# +# Mirrored in ``frontend/src/constants/fileUpload.ts``; update both together. +SUPPORTED_ATTACHMENT_EXTENSIONS = frozenset( + { + *SUPPORTED_SOURCE_EXTENSIONS, + ".xhtml", + ".adoc", + ".asciidoc", + ".tiff", + ".tif", + ".bmp", + ".webp", + ".vtt", + ".xml", + } +) + + +def attachment_extension(filename: str | None) -> str: + """Return the lower-cased extension of ``filename`` including the dot, or ``""``. + + Args: + filename: A bare filename or path; ``None`` and empty strings yield ``""``. + + Returns: + The last suffix in lower case (``".pdf"``), or ``""`` when there is none. + """ + if not filename: + return "" + return os.path.splitext(os.path.basename(str(filename)))[1].lower() + + +def has_attachment_parser(filename: str | None) -> bool: + """Return whether an attachment's suffix has a dedicated parser. + + A False result does not mean the file is refused: it means nothing but the + plain-text fallthrough will read it, so it has to earn its place on + content. The decision is by extension only, never by the mime type a + browser reports (mobile pickers ignore ``accept`` and lie). + + Args: + filename: The upload's filename. + + Returns: + True when the suffix is in ``SUPPORTED_ATTACHMENT_EXTENSIONS``. + """ + return attachment_extension(filename) in SUPPORTED_ATTACHMENT_EXTENSIONS diff --git a/application/upload_limits.py b/application/upload_limits.py index 98736c29..c5f5c579 100644 --- a/application/upload_limits.py +++ b/application/upload_limits.py @@ -8,6 +8,10 @@ from contextlib import suppress from typing import BinaryIO from application.core.settings import settings +from application.parser.file.constants import ( + attachment_extension, + has_attachment_parser, +) _COPY_CHUNK_BYTES = 64 * 1024 @@ -26,6 +30,106 @@ class UploadTooLargeError(ValueError): """Raised when one uploaded file exceeds the configured byte cap.""" +class UnsupportedUploadTypeError(ValueError): + """Raised when an uploaded attachment has a file type the worker cannot parse.""" + + +_UNSUPPORTED_UPLOAD_PREFIX = "Unsupported file type" + + +def unsupported_upload_message(filename: str | None) -> str: + """Return the stable client-facing rejection message for an unparseable upload. + + Args: + filename: The rejected upload's filename. + + Returns: + ``"Unsupported file type: .mp4"`` style text, naming the extension. + """ + extension = attachment_extension(filename) + return f"{_UNSUPPORTED_UPLOAD_PREFIX}: {extension or '(no extension)'}" + + +def is_unsupported_upload_message(message: str | None) -> bool: + """Return whether ``message`` is one produced by :func:`unsupported_upload_message`.""" + return bool(message) and str(message).startswith(_UNSUPPORTED_UPLOAD_PREFIX) + + +# A suffix with no parser is read by ``SimpleDirectoryReader``'s plain-text +# fallthrough, so it is admitted on content: enough of the head to recognise a +# container header, and a tolerance that keeps real text (UTF-8 accents, an +# ANSI-coloured log) in while a random binary — ~12.5% of bytes below 0x20 — +# stays out. +_TEXT_SNIFF_BYTES = 8192 +_MAX_NONTEXT_RATIO = 0.10 +# Control bytes that occur in ordinary text: tab, LF, VT, FF, CR, ESC. +_TEXT_CONTROL_BYTES = frozenset({0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x1B}) + + +def looks_like_text(sample: bytes) -> bool: + """Return whether a leading byte sample reads as text rather than binary. + + Args: + sample: The first bytes of a file; an empty sample counts as text. + + Returns: + False when the sample holds a NUL byte or too many other non-text + control bytes, True otherwise. + """ + if not sample: + return True + if b"\x00" in sample: + return False + nontext = sum( + 1 + for byte in sample + if (byte < 0x20 and byte not in _TEXT_CONTROL_BYTES) or byte == 0x7F + ) + return nontext / len(sample) <= _MAX_NONTEXT_RATIO + + +def file_looks_like_text(path: str | os.PathLike[str]) -> bool: + """Return whether a file on disk reads as text, by its leading bytes. + + Args: + path: Filesystem path to sample. + + Returns: + The :func:`looks_like_text` verdict for the file's head; True when the + file cannot be read, leaving that failure to the parser to report. + """ + try: + with open(path, "rb") as handle: + return looks_like_text(handle.read(_TEXT_SNIFF_BYTES)) + except OSError: + return True + + +def enforce_parseable_attachment( + path: str | os.PathLike[str], filename: str | None +) -> None: + """Reject an attachment that no parser handles and that is not plain text. + + Suffixes with a parser (``SUPPORTED_ATTACHMENT_EXTENSIONS``) are admitted + unconditionally — a PDF is binary and parses fine. Everything else has to + read as text, which is what keeps a video or an archive out of the + plain-text fallthrough while leaving source, config and log files in. + + Args: + path: Local path to the staged upload, readable before it is stored. + filename: The upload's filename, used for the suffix and the message. + + Raises: + UnsupportedUploadTypeError: When the file has no parser and its + contents are binary. + """ + if has_attachment_parser(filename): + return + if file_looks_like_text(path): + return + raise UnsupportedUploadTypeError(unsupported_upload_message(filename)) + + class _LimitedRawReader(io.RawIOBase): """Expose a binary stream while enforcing an exact cumulative read cap.""" diff --git a/application/worker.py b/application/worker.py index 0e7551f0..a1507239 100755 --- a/application/worker.py +++ b/application/worker.py @@ -55,6 +55,10 @@ from application.storage.db.repositories.wiki_pages import ( from application.storage.db.session import db_readonly, db_session from application.storage.db.source_config import SourceConfig from application.storage.storage_creator import StorageCreator +from application.upload_limits import ( + enforce_parseable_attachment, + UnsupportedUploadTypeError, +) from application.utils import ( count_tokens_docs, get_encoding, @@ -1554,6 +1558,26 @@ class AttachmentRejectedError(Exception): """ +def _reject_unparseable_attachment(local_path: str, filename: str) -> None: + """Reject an attachment with no parser whose contents are binary. + + Defence in depth behind the route's gate: ``SimpleDirectoryReader`` opens + any suffix it has no parser for as plain text, so a binary must fail here, + visibly, rather than "succeed" as garbage. + + Args: + local_path: Filesystem path of the attachment about to be parsed. + filename: The upload's original filename, which carries the suffix. + + Raises: + AttachmentRejectedError: If the file has no parser and is not text. + """ + try: + enforce_parseable_attachment(local_path, filename) + except UnsupportedUploadTypeError as exc: + raise AttachmentRejectedError(str(exc)) from exc + + def _reject_attachment_zip_bomb(local_path: str) -> None: """Reject a zip-container attachment that decompresses to too much. @@ -1751,6 +1775,7 @@ def attachment_worker(self, file_info, user): parser_name = type(_parser).__name__ if _parser is not None else "SimpleDirectoryReader" def _parse_local_file(local_path: str, **kwargs) -> Document: + _reject_unparseable_attachment(local_path, filename) _reject_attachment_zip_bomb(local_path) parse_path, is_temp_copy = _bounded_attachment_copy(local_path) try: diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index 2dc6d364..fac83d66 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -48,7 +48,9 @@ import { useArmedSend } from './message-input/armedSend'; import { handleAbort } from '../conversation/conversationSlice'; import { AUDIO_FILE_ACCEPT_ATTR, - FILE_UPLOAD_ACCEPT, + getFileExtension, + parseUploadErrorMessage, + partitionAttachmentFiles, } from '../constants/fileUpload'; import { UserToolType } from '../settings/types'; import { isChatToolVisible } from '../utils/toolUtils'; @@ -497,8 +499,31 @@ export default function MessageInput({ ); const uploadFiles = useCallback( - (files: File[]) => { - if (!files || files.length === 0) return; + async (incomingFiles: File[]) => { + if (!incomingFiles || incomingFiles.length === 0) return; + + // Run the server's own rule here, not just the input's `accept`: + // mobile pickers ignore `accept`, and a file the server will refuse + // should say so before it costs an upload. Surface the refusal as a + // failed chip so the user sees why instead of a silent drop. + const { supported, unsupported } = + await partitionAttachmentFiles(incomingFiles); + unsupported.forEach((file) => { + dispatch( + addAttachment({ + id: generateId(), + fileName: file.name, + progress: 0, + status: 'failed' as const, + taskId: '', + errorMessage: t('conversation.attachments.unsupportedType', { + extension: getFileExtension(file.name) || '?', + }), + }), + ); + }); + if (supported.length === 0) return; + const files = supported; const apiHost = import.meta.env.VITE_API_HOST; @@ -579,9 +604,14 @@ export default function MessageInput({ tasksByIndex.set(uploadIndex, task); }); + const errorsByIndex = new Map(); errors.forEach((errorItem) => { if (typeof errorItem.upload_index === 'number') { failedIndices.add(errorItem.upload_index); + errorsByIndex.set( + errorItem.upload_index, + errorItem.error, + ); } }); @@ -616,7 +646,10 @@ export default function MessageInput({ dispatch( updateAttachment({ id: uiId, - updates: { status: 'failed' }, + updates: { + status: 'failed', + errorMessage: errorsByIndex.get(index), + }, }), ); return; @@ -748,11 +781,12 @@ export default function MessageInput({ } } else { console.error('Upload failed', status, xhr.responseText); + const errorMessage = parseUploadErrorMessage(xhr.responseText); Object.values(indexToUiId).forEach((id) => dispatch( updateAttachment({ id, - updates: { status: 'failed' }, + updates: { status: 'failed', errorMessage }, }), ), ); @@ -871,7 +905,10 @@ export default function MessageInput({ dispatch( updateAttachment({ id: uniqueId, - updates: { status: 'failed' }, + updates: { + status: 'failed', + errorMessage: parseUploadErrorMessage(xhr.responseText), + }, }), ); } @@ -891,7 +928,7 @@ export default function MessageInput({ xhr.send(formData); }); }, - [dispatch, token, trackAttachment], + [dispatch, t, token, trackAttachment], ); const handleFileAttachment = (e: React.ChangeEvent) => { @@ -928,7 +965,10 @@ export default function MessageInput({ setHandleDragActive(false); }, maxSize: 25000000, - accept: FILE_UPLOAD_ACCEPT, + // No `accept`: react-dropzone would drop a rejected file on the floor + // with no feedback, and its mime matching disagrees with the server for + // text files that have no parser (.py, .log). uploadFiles applies the + // server's rule and reports what it refuses. }); const handleInput = useCallback(() => { @@ -1594,7 +1634,13 @@ export default function MessageInput({ onChange={handleVoiceFileAttachment} /> -
+ {/* translate="no": keep Chrome's page translator out of the composer — + it rewrites text nodes into wrappers and React loses the + controls (dead Attach button, see AttachFileButton). */} +
) => void; @@ -11,7 +11,14 @@ export default function AttachFileButton({ onChange }: AttachFileButtonProps) { const { t } = useTranslation(); return ( -
+ ); + })} +
+ + {failures.length > 0 && ( +
+ {failures.map((attachment) => ( + + {attachment.fileName}: {attachment.errorMessage} + + ))} +
+ )} + ); } diff --git a/frontend/src/constants/fileUpload.test.ts b/frontend/src/constants/fileUpload.test.ts new file mode 100644 index 00000000..ff0b9769 --- /dev/null +++ b/frontend/src/constants/fileUpload.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it } from 'vitest'; + +import { + ATTACHMENT_FILE_ACCEPT_ATTR, + ATTACHMENT_PARSER_EXTENSIONS, + getFileExtension, + hasAttachmentParser, + looksLikeText, + parseUploadErrorMessage, + partitionAttachmentFiles, +} from './fileUpload'; + +const file = (name: string, body: BlobPart = 'x', type = '') => + new File([body], name, { type }); + +const bytes = (...values: number[]) => new Uint8Array(values); + +const MP4_HEADER = bytes( + 0x00, + 0x00, + 0x00, + 0x18, + 0x66, + 0x74, + 0x79, + 0x70, + 0x69, + 0x73, + 0x6f, + 0x6d, +); + +describe('attachment type gate', () => { + it('extracts a lower-cased extension', () => { + expect(getFileExtension('Photo.JPG')).toBe('.jpg'); + expect(getFileExtension('a.tar.gz')).toBe('.gz'); + expect(getFileExtension('noext')).toBe(''); + expect(getFileExtension('.env')).toBe(''); + }); + + it('recognises parser-backed suffixes by name, case-insensitively', () => { + expect(hasAttachmentParser(file('Report.PDF'))).toBe(true); + expect(hasAttachmentParser(file('scan.WebP'))).toBe(true); + expect(hasAttachmentParser(file('subs.vtt'))).toBe(true); + // No parser — these are judged on content, not on the suffix. + expect(hasAttachmentParser(file('main.py'))).toBe(false); + expect(hasAttachmentParser(file('archive.zip'))).toBe(false); + expect(hasAttachmentParser(file('clip.mp4'))).toBe(false); + }); + + it('mirrors the backend list: no zip, no video, images and markup included', () => { + const listed = ATTACHMENT_FILE_ACCEPT_ATTR.split(','); + expect(listed).toEqual([...ATTACHMENT_PARSER_EXTENSIONS]); + expect(listed).toContain('.pdf'); + // Parser-backed suffixes the first cut of this list missed. + for (const ext of [ + '.webp', + '.tiff', + '.tif', + '.bmp', + '.vtt', + '.xml', + '.mdx', + ]) { + expect(listed).toContain(ext); + } + expect(listed).not.toContain('.zip'); + expect(listed).not.toContain('.mp4'); + }); +}); + +describe('looksLikeText', () => { + it('accepts empty, plain, accented and ANSI-coloured text', () => { + expect(looksLikeText(new Uint8Array())).toBe(true); + expect(looksLikeText(new TextEncoder().encode('plain text\n'))).toBe(true); + expect(looksLikeText(new TextEncoder().encode('café — em dash\n'))).toBe( + true, + ); + expect(looksLikeText(bytes(0x1b, 0x5b, 0x33, 0x31, 0x6d, 0x6f, 0x6b))).toBe( + true, + ); + }); + + it('rejects a NUL byte and dense control bytes', () => { + expect(looksLikeText(MP4_HEADER)).toBe(false); + expect(looksLikeText(bytes(0x74, 0x78, 0x00, 0x74))).toBe(false); + expect( + looksLikeText( + new Uint8Array(Array.from({ length: 64 }, (_, i) => i + 1)), + ), + ).toBe(false); + }); +}); + +describe('partitionAttachmentFiles', () => { + it('refuses video and archives whatever the picker claims the type is', async () => { + const { supported, unsupported } = await partitionAttachmentFiles([ + file('clip.mp4', MP4_HEADER, 'text/plain'), + file('archive.zip', bytes(0x50, 0x4b, 0x03, 0x04, 0x00, 0x00)), + ]); + expect(supported).toEqual([]); + expect(unsupported.map((f) => f.name)).toEqual(['clip.mp4', 'archive.zip']); + }); + + it('keeps text files that have no parser — the backend reads them', async () => { + const { supported, unsupported } = await partitionAttachmentFiles([ + file('main.py', 'def main():\n return 1\n'), + file('server.log', '2026-09-02 ERROR boom\n'), + file('Dockerfile', 'FROM python:3.12\n'), + ]); + expect(supported.map((f) => f.name)).toEqual([ + 'main.py', + 'server.log', + 'Dockerfile', + ]); + expect(unsupported).toEqual([]); + }); + + it('admits parser-backed types on their suffix, binary contents and all', async () => { + // A PDF is binary; the sniff must never be applied to it. + const { supported, unsupported } = await partitionAttachmentFiles([ + file('paper.pdf', MP4_HEADER), + file('photo.jpeg', MP4_HEADER), + ]); + expect(supported.map((f) => f.name)).toEqual(['paper.pdf', 'photo.jpeg']); + expect(unsupported).toEqual([]); + }); + + it('partitions a mixed selection and keeps the original order', async () => { + const mp4 = file('clip.mp4', MP4_HEADER); + const txt = file('notes.txt'); + const pdf = file('paper.pdf'); + const { supported, unsupported } = await partitionAttachmentFiles([ + mp4, + txt, + pdf, + ]); + expect(supported).toEqual([txt, pdf]); + expect(unsupported).toEqual([mp4]); + }); +}); + +describe('parseUploadErrorMessage', () => { + it('returns the server message when present', () => { + expect( + parseUploadErrorMessage( + '{"success":false,"message":"Unsupported file type: .mp4"}', + ), + ).toBe('Unsupported file type: .mp4'); + }); + + it('returns undefined for non-JSON bodies or a missing message', () => { + expect(parseUploadErrorMessage('502')).toBeUndefined(); + expect(parseUploadErrorMessage('{"ok":1}')).toBeUndefined(); + expect(parseUploadErrorMessage('')).toBeUndefined(); + }); +}); diff --git a/frontend/src/constants/fileUpload.ts b/frontend/src/constants/fileUpload.ts index e8d6f501..b8fd7c0d 100644 --- a/frontend/src/constants/fileUpload.ts +++ b/frontend/src/constants/fileUpload.ts @@ -80,3 +80,129 @@ export const SOURCE_FILE_TREE_ACCEPT_ATTR = [ '.jpg', '.jpeg', ].join(','); + +/** + * Chat-attachment suffixes with a dedicated parser. Mirrors the backend's + * `SUPPORTED_ATTACHMENT_EXTENSIONS` (application/parser/file/constants.py) — + * update both together. Zip is absent: source ingestion extracts archives, + * the attachment path does not. + * + * Not the whole allow-list. A suffix that isn't here (.py, .log, .yaml) is + * read by the backend's plain-text fallthrough, so it is judged on content + * by `partitionAttachmentFiles` instead, exactly as the server does. + */ +export const ATTACHMENT_PARSER_EXTENSIONS: readonly string[] = [ + '.rst', + '.md', + '.mdx', + '.pdf', + '.txt', + '.docx', + '.csv', + '.epub', + '.html', + '.xhtml', + '.json', + '.xlsx', + '.pptx', + '.adoc', + '.asciidoc', + '.png', + '.jpg', + '.jpeg', + '.tiff', + '.tif', + '.bmp', + '.webp', + '.vtt', + '.xml', + '.wav', + '.mp3', + '.m4a', + '.ogg', + '.webm', +]; + +/** Picker filter for the Attach button. A hint only — pickers may ignore it. */ +export const ATTACHMENT_FILE_ACCEPT_ATTR = + ATTACHMENT_PARSER_EXTENSIONS.join(','); + +/** Lower-cased last extension including the dot, or '' (dotfiles have none). */ +export function getFileExtension(name: string): string { + const base = name.split(/[\\/]/).pop() ?? ''; + const dot = base.lastIndexOf('.'); + if (dot <= 0) return ''; + return base.slice(dot).toLowerCase(); +} + +/** + * Whether the backend has a parser for this suffix. False is not a refusal: + * the file then has to read as text. Never decided by the mime type the + * picker reports — mobile pickers ignore `accept` and lie about types. + */ +export function hasAttachmentParser(file: { name: string }): boolean { + const ext = getFileExtension(file.name); + return ext !== '' && ATTACHMENT_PARSER_EXTENSIONS.includes(ext); +} + +// Mirrors application/upload_limits.py: enough of the head to recognise a +// container header, and a tolerance that keeps real text (UTF-8 accents, an +// ANSI-coloured log) in while a random binary stays out. +const TEXT_SNIFF_BYTES = 8192; +const MAX_NONTEXT_RATIO = 0.1; +// Control bytes that occur in ordinary text: tab, LF, VT, FF, CR, ESC. +const TEXT_CONTROL_BYTES = new Set([0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x1b]); + +/** Whether a leading byte sample reads as text rather than binary. */ +export function looksLikeText(sample: Uint8Array): boolean { + if (sample.length === 0) return true; + let nontext = 0; + for (const byte of sample) { + if (byte === 0x00) return false; + if ((byte < 0x20 && !TEXT_CONTROL_BYTES.has(byte)) || byte === 0x7f) + nontext += 1; + } + return nontext / sample.length <= MAX_NONTEXT_RATIO; +} + +async function isSupportedAttachmentFile(file: File): Promise { + if (hasAttachmentParser(file)) return true; + try { + const head = await file.slice(0, TEXT_SNIFF_BYTES).arrayBuffer(); + return looksLikeText(new Uint8Array(head)); + } catch { + // Can't read it here — let the upload run and the server decide. + return true; + } +} + +/** + * Split a selection into what the backend can read and what it will refuse, + * preserving order. Files with a parser pass on their suffix; the rest are + * sniffed, so source, config and log files go through and a video does not. + */ +export async function partitionAttachmentFiles(files: File[]): Promise<{ + supported: File[]; + unsupported: File[]; +}> { + const verdicts = await Promise.all(files.map(isSupportedAttachmentFile)); + const supported: File[] = []; + const unsupported: File[] = []; + files.forEach((file, index) => { + (verdicts[index] ? supported : unsupported).push(file); + }); + return { supported, unsupported }; +} + +/** The ``message`` of a JSON error body, if the server sent one. */ +export function parseUploadErrorMessage(body: string): string | undefined { + if (!body) return undefined; + try { + const parsed = JSON.parse(body) as { message?: unknown }; + return typeof parsed?.message === 'string' && parsed.message + ? parsed.message + : undefined; + } catch { + return undefined; + } +} diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index 71a35311..4e8b300d 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -1101,7 +1101,8 @@ "waitingToSend_one": "Will send when {{count}} file finishes processing…", "waitingToSend_other": "Will send when {{count}} files finish processing…", "cancelQueuedSend": "Cancel", - "sendBlockedByFailed": "{{names}} couldn't be processed — remove to send" + "sendBlockedByFailed": "{{names}} couldn't be processed — remove to send", + "unsupportedType": "Not a supported file type ({{extension}})" }, "retry": "Retry", "reasoning": "Reasoning", diff --git a/frontend/src/upload/uploadSlice.ts b/frontend/src/upload/uploadSlice.ts index 468a8353..f512e385 100644 --- a/frontend/src/upload/uploadSlice.ts +++ b/frontend/src/upload/uploadSlice.ts @@ -46,6 +46,8 @@ export interface Attachment { */ attachmentId?: string; token_count?: number; + /** Why a ``failed`` attachment failed, when known (server or client gate). */ + errorMessage?: string; } export type UploadTaskStatus = diff --git a/tests/api/user/attachments/test_routes.py b/tests/api/user/attachments/test_routes.py index d28989af..e320f58d 100644 --- a/tests/api/user/attachments/test_routes.py +++ b/tests/api/user/attachments/test_routes.py @@ -2386,3 +2386,97 @@ class TestRequireLiveSttRedisUnavailable: result = _require_live_stt_redis() assert hasattr(result, "status_code") assert result.status_code == 503 + + +MP4_BYTES = b"\x00\x00\x00\x18ftypisom\x00\x00\x02\x00isomiso2avc1mp41\x00\x00\x00\x08free" + + +class TestStoreAttachmentTypeGate: + """Unparseable uploads are refused before anything is stored or queued.""" + + @patch("application.api.user.tasks.store_attachment.delay") + def test_rejects_unsupported_file_type_before_storage( + self, mock_store_attachment, flask_app + ): + from application.api.user.attachments.routes import StoreAttachment + + app = Flask(__name__) + mock_storage = MagicMock() + with patch("application.api.user.base.storage", mock_storage), app.test_request_context( + "/api/store_attachment", + method="POST", + data={"file": (io.BytesIO(MP4_BYTES), "clip.mp4")}, + content_type="multipart/form-data", + ): + request.decoded_token = {"sub": "test_user"} + response = StoreAttachment().post() + + assert _get_response_status(response) == 400 + body = _get_response_json(response) + assert body["success"] is False + assert body["message"] == "Unsupported file type: .mp4" + assert body["errors"] == [ + {"upload_index": 0, "filename": "clip.mp4", "error": "Unsupported file type: .mp4"} + ] + mock_storage.save_file.assert_not_called() + mock_store_attachment.assert_not_called() + + @patch("application.api.user.tasks.store_attachment.delay") + def test_accepts_a_text_file_with_no_dedicated_parser( + self, mock_store_attachment, flask_app + ): + """A .py or .log is read by the plain-text fallthrough and must stay allowed.""" + from application.api.user.attachments.routes import StoreAttachment + + app = Flask(__name__) + mock_storage = MagicMock() + mock_storage.save_file.return_value = {"storage_type": "local"} + mock_store_attachment.return_value = SimpleNamespace(id="task-py") + + with patch("application.api.user.base.storage", mock_storage), app.test_request_context( + "/api/store_attachment", + method="POST", + data={"file": (io.BytesIO(b"def main():\n return 1\n"), "main.py")}, + content_type="multipart/form-data", + ): + request.decoded_token = {"sub": "test_user"} + response = StoreAttachment().post() + + assert _get_response_status(response) == 200 + assert _get_response_json(response)["task_id"] == "task-py" + assert mock_store_attachment.call_count == 1 + + @patch("application.api.user.tasks.store_attachment.delay") + def test_batch_skips_unsupported_file_and_keeps_the_rest( + self, mock_store_attachment, flask_app + ): + from application.api.user.attachments.routes import StoreAttachment + + app = Flask(__name__) + mock_storage = MagicMock() + mock_storage.save_file.return_value = {"storage_type": "local"} + mock_store_attachment.return_value = SimpleNamespace(id="task-notes") + + with patch("application.api.user.base.storage", mock_storage), app.test_request_context( + "/api/store_attachment", + method="POST", + data={ + "file": [ + (io.BytesIO(MP4_BYTES), "clip.mp4"), + (io.BytesIO(b"hello"), "notes.txt"), + ] + }, + content_type="multipart/form-data", + ): + request.decoded_token = {"sub": "test_user"} + response = StoreAttachment().post() + + assert _get_response_status(response) == 200 + body = _get_response_json(response) + assert [task["upload_index"] for task in body["tasks"]] == [1] + assert body["tasks"][0]["filename"] == "notes.txt" + assert body["errors"] == [ + {"upload_index": 0, "filename": "clip.mp4", "error": "Unsupported file type: .mp4"} + ] + assert mock_storage.save_file.call_count == 1 + assert mock_store_attachment.call_count == 1 diff --git a/tests/parser/file/test_constants.py b/tests/parser/file/test_constants.py new file mode 100644 index 00000000..2bf6f042 --- /dev/null +++ b/tests/parser/file/test_constants.py @@ -0,0 +1,95 @@ +"""Tests for the shared file-extension constants.""" + +import re +from pathlib import Path + +import pytest + +from application.parser.file.bulk import get_default_file_extractor +from application.parser.file.constants import ( + attachment_extension, + has_attachment_parser, + SUPPORTED_ATTACHMENT_EXTENSIONS, + SUPPORTED_SOURCE_EXTENSIONS, +) + +FRONTEND_CONSTANTS = ( + Path(__file__).resolve().parents[3] / "frontend/src/constants/fileUpload.ts" +) + + +@pytest.mark.unit +def test_every_parser_backed_suffix_is_an_allowed_attachment(): + """The allow-list is a hand-kept literal; this is what keeps it honest. + + ``SUPPORTED_ATTACHMENT_EXTENSIONS`` cannot import the extractor (that + would pull docling into the API process), so a parser added to + ``get_default_file_extractor`` would otherwise stay refused as an + attachment — which is how .webp, .tiff, .vtt and .xml were first missed. + """ + parser_suffixes = set(get_default_file_extractor()) + + assert parser_suffixes <= set(SUPPORTED_ATTACHMENT_EXTENSIONS), ( + "parsers exist for suffixes the attachment gate refuses: " + f"{sorted(parser_suffixes - set(SUPPORTED_ATTACHMENT_EXTENSIONS))}" + ) + + +@pytest.mark.unit +def test_frontend_mirrors_the_attachment_allow_list(): + """The composer gates uploads client-side; a divergent list refuses valid files. + + ``ATTACHMENT_PARSER_EXTENSIONS`` in the frontend is a hand-kept copy of + ``SUPPORTED_ATTACHMENT_EXTENSIONS``. Nothing but this test connects them, + and .mdx, .xhtml and .adoc were blocked in the UI while the API accepted + them before it existed. + """ + if not FRONTEND_CONSTANTS.exists(): + pytest.skip("frontend sources not present in this checkout") + + source = FRONTEND_CONSTANTS.read_text(encoding="utf-8") + match = re.search( + r"ATTACHMENT_PARSER_EXTENSIONS:\s*readonly string\[\]\s*=\s*\[(.*?)\]", + source, + re.DOTALL, + ) + assert match, "ATTACHMENT_PARSER_EXTENSIONS not found in fileUpload.ts" + frontend_extensions = set(re.findall(r"'(\.[^']+)'", match.group(1))) + + assert frontend_extensions == set(SUPPORTED_ATTACHMENT_EXTENSIONS), ( + "frontend and backend attachment lists disagree — " + f"backend only: {sorted(set(SUPPORTED_ATTACHMENT_EXTENSIONS) - frontend_extensions)}, " + f"frontend only: {sorted(frontend_extensions - set(SUPPORTED_ATTACHMENT_EXTENSIONS))}" + ) + + +@pytest.mark.unit +def test_attachments_accept_everything_source_ingestion_does(): + assert set(SUPPORTED_SOURCE_EXTENSIONS) <= set(SUPPORTED_ATTACHMENT_EXTENSIONS) + + +@pytest.mark.unit +@pytest.mark.parametrize( + ("filename", "expected"), + [ + ("Photo.JPG", ".jpg"), + ("/tmp/dir.d/report.PDF", ".pdf"), + ("archive.tar.gz", ".gz"), + ("Dockerfile", ""), + (".env", ""), + ("trailing.", "."), + (None, ""), + ], +) +def test_attachment_extension(filename, expected): + assert attachment_extension(filename) == expected + + +@pytest.mark.unit +def test_has_attachment_parser_is_case_insensitive_and_excludes_zip(): + assert has_attachment_parser("Report.PDF") + assert has_attachment_parser("scan.WebP") + # No parser: admitted (or not) on content instead, never on the suffix. + assert not has_attachment_parser("archive.zip") + assert not has_attachment_parser("main.py") + assert not has_attachment_parser("clip.mp4") diff --git a/tests/test_upload_limits.py b/tests/test_upload_limits.py index 46818421..acb1fba0 100644 --- a/tests/test_upload_limits.py +++ b/tests/test_upload_limits.py @@ -37,3 +37,126 @@ def test_limited_text_read_decodes_incrementally_and_rejects_overflow(): with pytest.raises(UploadTooLargeError): read_text_upload_limited(_ShortReadStream(b"12345"), max_bytes=4) + + +# --- attachment type gate ------------------------------------------------- +# +# ``SimpleDirectoryReader`` falls through to a plain-text ``open()`` for any +# suffix without a parser. That is what a .py or a .log attachment relies on, +# and it is also how a phone-uploaded video used to be "parsed" into +# megabytes of binary garbage, truncated, and stored with +# ``extraction.status == "ok"``. So a suffix with no parser is admitted on +# content: text in, binary out. + +MP4_HEADER = b"\x00\x00\x00\x18ftypisom\x00\x00\x02\x00isomiso2avc1mp41" + + +@pytest.mark.parametrize( + ("filename", "content"), + [ + ("clip.mp4", MP4_HEADER), + ("movie.MOV", MP4_HEADER), + ("archive.zip", b"PK\x03\x04\x14\x00\x00\x00\x08\x00" + bytes(range(32))), + ("binary", bytes(range(32)) * 8), + ("trailing.", b"\x00\x01\x02\x03"), + ("x.tar.gz", b"\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03"), + ], +) +def test_enforce_parseable_attachment_rejects_binary_without_a_parser( + filename, content, tmp_path +): + from application.upload_limits import ( + enforce_parseable_attachment, + UnsupportedUploadTypeError, + unsupported_upload_message, + ) + + path = tmp_path / "staged.bin" + path.write_bytes(content) + + with pytest.raises(UnsupportedUploadTypeError) as excinfo: + enforce_parseable_attachment(path, filename) + assert str(excinfo.value) == unsupported_upload_message(filename) + assert str(excinfo.value).startswith("Unsupported file type") + + +@pytest.mark.parametrize( + "filename", + [ + "notes.txt", + "Report.PDF", + "photo.JPG", + "slides.pptx", + "voice.ogg", + "page.xhtml", + "doc.adoc", + "scan.webp", + "fax.tiff", + "subs.vtt", + "feed.xml", + ], +) +def test_enforce_parseable_attachment_accepts_parser_backed_types(filename, tmp_path): + """A parser-backed suffix is admitted on its name — a PDF is binary and parses fine.""" + from application.upload_limits import enforce_parseable_attachment + + path = tmp_path / "staged.bin" + path.write_bytes(MP4_HEADER) + + enforce_parseable_attachment(path, filename) + + +@pytest.mark.parametrize( + "filename", + ["main.py", "server.log", "config.yaml", "query.sql", "Dockerfile", "notes.unknown"], +) +def test_enforce_parseable_attachment_accepts_text_without_a_parser(filename, tmp_path): + """The plain-text fallthrough reads these correctly, so they must stay allowed.""" + from application.upload_limits import enforce_parseable_attachment + + path = tmp_path / "staged.txt" + path.write_text("def main():\n\treturn 'café — ok'\n", encoding="utf-8") + + enforce_parseable_attachment(path, filename) + + +@pytest.mark.parametrize( + ("sample", "expected"), + [ + (b"", True), + (b"plain text\n", True), + ("café — em dash\n".encode(), True), + (b"\x1b[31mred log line\x1b[0m\n", True), + (b"text\x00with nul", False), + (bytes(range(32)) * 4, False), + (b"\x7f\x7f\x7f\x7f" + b"a" * 16, False), + ], +) +def test_looks_like_text(sample, expected): + from application.upload_limits import looks_like_text + + assert looks_like_text(sample) is expected + + +def test_file_looks_like_text_only_samples_the_head(tmp_path): + """Binary past the sampled head is the parser's problem, not the gate's.""" + from application.upload_limits import file_looks_like_text + + path = tmp_path / "staged.log" + path.write_bytes(b"a" * 9000 + b"\x00" * 100) + + assert file_looks_like_text(path) is True + + +def test_file_looks_like_text_allows_an_unreadable_file(tmp_path): + from application.upload_limits import file_looks_like_text + + assert file_looks_like_text(tmp_path / "missing.txt") is True + + +def test_unsupported_upload_message_names_the_extension(): + from application.upload_limits import unsupported_upload_message + + assert unsupported_upload_message("clip.mp4") == "Unsupported file type: .mp4" + assert unsupported_upload_message("Clip.MP4") == "Unsupported file type: .mp4" + assert unsupported_upload_message("binary") == "Unsupported file type: (no extension)" diff --git a/tests/worker/test_attachment_worker.py b/tests/worker/test_attachment_worker.py index 93e34d97..e5790cae 100644 --- a/tests/worker/test_attachment_worker.py +++ b/tests/worker/test_attachment_worker.py @@ -326,3 +326,86 @@ class TestAttachmentZipBombGuard: # BadZipFile → return quietly; the format parser surfaces a clean error. worker._reject_attachment_zip_bomb(str(path)) + + +@pytest.mark.unit +class TestAttachmentTypeGuard: + """A binary with no parser must fail rather than be read as text. + + ``SimpleDirectoryReader`` falls through to a plain-text ``open()`` for a + suffix it has no parser for, which is how a phone-uploaded ``.mp4`` was + once stored as 100k tokens of binary with ``extraction.status == "ok"``. + The same fallthrough is what makes a ``.py`` or ``.log`` attachment work, + so the guard admits those on content. + """ + + def test_binary_without_a_parser_fails_instead_of_being_read_as_text( + self, pg_conn, patch_worker_db, task_self, monkeypatch, tmp_path + ): + from application import worker + + local_path = tmp_path / "clip.mp4" + local_path.write_bytes(b"\x00\x00\x00\x18ftypisom\x00\x00\x02\x00isomavc1") + + events = [] + fake_storage = MagicMock(name="storage") + fake_storage.process_file.side_effect = lambda path, callback: callback( + str(local_path) + ) + monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage) + monkeypatch.setattr( + worker, + "get_default_file_extractor", + lambda ocr_enabled=False, pdf_text_fast_path=False: {}, + ) + monkeypatch.setattr( + worker, + "publish_user_event", + lambda user, name, payload, **kwargs: events.append((name, payload)), + ) + + file_info = { + "filename": "clip.mp4", + "attachment_id": "507f1f77bcf86cd799439012", + "path": "uploads/user1/attachments/clip.mp4", + "metadata": {"source": "chat"}, + } + + with pytest.raises( + worker.AttachmentRejectedError, match=r"Unsupported file type: \.mp4" + ): + worker.attachment_worker(task_self, file_info, "user1") + + failed = [payload for name, payload in events if name == "attachment.failed"] + assert failed and failed[0]["error"] == "Unsupported file type: .mp4" + + def test_text_without_a_parser_is_parsed( + self, pg_conn, patch_worker_db, task_self, monkeypatch, tmp_path + ): + from application import worker + + local_path = tmp_path / "server.log" + local_path.write_text("2026-09-02 ERROR boom\n", encoding="utf-8") + + fake_storage = MagicMock(name="storage") + fake_storage.process_file.side_effect = lambda path, callback: callback( + str(local_path) + ) + monkeypatch.setattr(worker.StorageCreator, "get_storage", lambda: fake_storage) + monkeypatch.setattr( + worker, + "get_default_file_extractor", + lambda ocr_enabled=False, pdf_text_fast_path=False: {}, + ) + + file_info = { + "filename": "server.log", + "attachment_id": "507f1f77bcf86cd799439013", + "path": "uploads/user1/attachments/server.log", + "metadata": {"source": "chat"}, + } + + result = worker.attachment_worker(task_self, file_info, "user1") + + assert result["filename"] == "server.log" + assert result["token_count"] > 0