diff --git a/application/api/user/attachments/routes.py b/application/api/user/attachments/routes.py index e8d7491d..3d856cc5 100644 --- a/application/api/user/attachments/routes.py +++ b/application/api/user/attachments/routes.py @@ -42,6 +42,7 @@ from application.upload_limits import ( enforce_parseable_attachment, is_unsupported_upload_message, UnsupportedUploadTypeError, + unsupported_upload_message, upload_limit_message, UploadTooLargeError, ) @@ -128,13 +129,27 @@ def _enforce_uploaded_audio_size_limit(file, filename: str) -> None: enforce_audio_file_size_limit(size_bytes) -def _get_store_attachment_user_error(exc: Exception) -> str: +def _get_store_attachment_user_error(exc: Exception, filename: str | None = None) -> str: + """Map an upload failure to a fixed client-facing message. + + Every branch returns text this module composes itself. Nothing is read off + the exception, so no exception state — message or traceback — can reach a + response body (CodeQL py/stack-trace-exposure). + + Args: + exc: The exception raised while processing one uploaded file. + filename: That file's name, which supplies the extension for the + unsupported-type message. + + Returns: + The message to report for this failure. + """ if isinstance(exc, AudioFileTooLargeError): return build_stt_file_size_limit_message() if isinstance(exc, UploadTooLargeError): return upload_limit_message() if isinstance(exc, UnsupportedUploadTypeError): - return str(exc) + return unsupported_upload_message(filename) return "Failed to process file" @@ -251,7 +266,9 @@ class StoreAttachment(Resource): errors.append({ "upload_index": idx, "filename": file.filename, - "error": _get_store_attachment_user_error(file_err), + "error": _get_store_attachment_user_error( + file_err, file.filename + ), }) if not tasks: diff --git a/application/parser/file/constants.py b/application/parser/file/constants.py index 079a13d2..f8628694 100644 --- a/application/parser/file/constants.py +++ b/application/parser/file/constants.py @@ -28,10 +28,10 @@ SUPPORTED_SOURCE_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. +# Suffixes the attachment path has a dedicated parser for — exactly the keys +# of ``get_default_file_extractor()``. 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 @@ -44,7 +44,7 @@ SUPPORTED_SOURCE_EXTENSIONS = ( # content check like any other binary. # # Mirrored in ``frontend/src/constants/fileUpload.ts``; update both together. -SUPPORTED_ATTACHMENT_EXTENSIONS = frozenset( +ATTACHMENT_PARSER_EXTENSIONS = frozenset( { *SUPPORTED_SOURCE_EXTENSIONS, ".xhtml", @@ -57,6 +57,10 @@ SUPPORTED_ATTACHMENT_EXTENSIONS = frozenset( ".vtt", ".xml", } + # .txt has no parser of its own — it *is* the plain-text fallthrough. It + # must be sniffed like any other unparsed suffix, or renaming a video to + # notes.txt walks straight back into the bug this gate exists for. + - {".txt"} ) @@ -86,6 +90,6 @@ def has_attachment_parser(filename: str | None) -> bool: filename: The upload's filename. Returns: - True when the suffix is in ``SUPPORTED_ATTACHMENT_EXTENSIONS``. + True when the suffix is in ``ATTACHMENT_PARSER_EXTENSIONS``. """ - return attachment_extension(filename) in SUPPORTED_ATTACHMENT_EXTENSIONS + return attachment_extension(filename) in ATTACHMENT_PARSER_EXTENSIONS diff --git a/application/upload_limits.py b/application/upload_limits.py index c5f5c579..3c24c2de 100644 --- a/application/upload_limits.py +++ b/application/upload_limits.py @@ -64,6 +64,14 @@ _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}) +# A UTF-16/32 file is half NUL bytes, so it has to be recognised by its BOM +# before the NUL test — Notepad's "Unicode" .txt is ordinary text. +_TEXT_BOMS = ( + b"\xef\xbb\xbf", + b"\xff\xfe", + b"\xfe\xff", + b"\x00\x00\xfe\xff", +) def looks_like_text(sample: bytes) -> bool: @@ -74,10 +82,12 @@ def looks_like_text(sample: bytes) -> bool: Returns: False when the sample holds a NUL byte or too many other non-text - control bytes, True otherwise. + control bytes, True otherwise. A Unicode BOM settles it as text. """ if not sample: return True + if sample.startswith(_TEXT_BOMS): + return True if b"\x00" in sample: return False nontext = sum( @@ -110,10 +120,11 @@ def enforce_parseable_attachment( ) -> 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. + Suffixes with a parser (``ATTACHMENT_PARSER_EXTENSIONS``) are admitted + unconditionally — a PDF is binary and parses fine. Everything else, .txt + included, has to read as text: that is what keeps a video or an archive + out of the plain-text fallthrough while leaving source, config and log + files in, whatever the file happens to be named. Args: path: Local path to the staged upload, readable before it is stored. diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index fac83d66..c472bc58 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -50,6 +50,7 @@ import { AUDIO_FILE_ACCEPT_ATTR, getFileExtension, parseUploadErrorMessage, + parseUploadErrorsByIndex, partitionAttachmentFiles, } from '../constants/fileUpload'; import { UserToolType } from '../settings/types'; @@ -781,12 +782,20 @@ export default function MessageInput({ } } else { console.error('Upload failed', status, xhr.responseText); - const errorMessage = parseUploadErrorMessage(xhr.responseText); - Object.values(indexToUiId).forEach((id) => + // Each file gets its own reason where the server sent one; the + // top-level message is the fallback, not the answer for all of + // them — a batch can fail two files for two different reasons. + const fallbackMessage = parseUploadErrorMessage(xhr.responseText); + const errorsByIndex = parseUploadErrorsByIndex(xhr.responseText); + Object.entries(indexToUiId).forEach(([index, id]) => dispatch( updateAttachment({ id, - updates: { status: 'failed', errorMessage }, + updates: { + status: 'failed', + errorMessage: + errorsByIndex.get(Number(index)) ?? fallbackMessage, + }, }), ), ); diff --git a/frontend/src/constants/fileUpload.test.ts b/frontend/src/constants/fileUpload.test.ts index ff0b9769..02579e76 100644 --- a/frontend/src/constants/fileUpload.test.ts +++ b/frontend/src/constants/fileUpload.test.ts @@ -7,6 +7,7 @@ import { hasAttachmentParser, looksLikeText, parseUploadErrorMessage, + parseUploadErrorsByIndex, partitionAttachmentFiles, } from './fileUpload'; @@ -46,12 +47,12 @@ describe('attachment type gate', () => { expect(hasAttachmentParser(file('main.py'))).toBe(false); expect(hasAttachmentParser(file('archive.zip'))).toBe(false); expect(hasAttachmentParser(file('clip.mp4'))).toBe(false); + // .txt is the plain-text fallthrough itself, not a parser. + expect(hasAttachmentParser(file('notes.txt'))).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'); + expect(ATTACHMENT_PARSER_EXTENSIONS).toContain('.pdf'); // Parser-backed suffixes the first cut of this list missed. for (const ext of [ '.webp', @@ -62,10 +63,22 @@ describe('attachment type gate', () => { '.xml', '.mdx', ]) { + expect(ATTACHMENT_PARSER_EXTENSIONS).toContain(ext); + } + expect(ATTACHMENT_PARSER_EXTENSIONS).not.toContain('.zip'); + expect(ATTACHMENT_PARSER_EXTENSIONS).not.toContain('.mp4'); + expect(ATTACHMENT_PARSER_EXTENSIONS).not.toContain('.txt'); + }); + + it('never hides a file the upload would accept behind the picker filter', () => { + const listed = ATTACHMENT_FILE_ACCEPT_ATTR.split(','); + for (const ext of ATTACHMENT_PARSER_EXTENSIONS) { expect(listed).toContain(ext); } - expect(listed).not.toContain('.zip'); - expect(listed).not.toContain('.mp4'); + // Parserless text (.txt, .py, .log) is accepted on content, so the + // picker must not filter it out. + expect(listed).toContain('.txt'); + expect(listed).toContain('text/*'); }); }); @@ -81,6 +94,14 @@ describe('looksLikeText', () => { ); }); + it('accepts BOM-marked Unicode, which is half NUL bytes but still text', () => { + // UTF-8, UTF-16 LE/BE, UTF-32 BE. + expect(looksLikeText(bytes(0xef, 0xbb, 0xbf, 0x68, 0x69))).toBe(true); + expect(looksLikeText(bytes(0xff, 0xfe, 0x68, 0x00, 0x69, 0x00))).toBe(true); + expect(looksLikeText(bytes(0xfe, 0xff, 0x00, 0x68, 0x00, 0x69))).toBe(true); + expect(looksLikeText(bytes(0x00, 0x00, 0xfe, 0xff, 0x00, 0x68))).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); @@ -104,11 +125,13 @@ describe('partitionAttachmentFiles', () => { it('keeps text files that have no parser — the backend reads them', async () => { const { supported, unsupported } = await partitionAttachmentFiles([ + file('notes.txt', 'plain\n'), 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([ + 'notes.txt', 'main.py', 'server.log', 'Dockerfile', @@ -116,6 +139,14 @@ describe('partitionAttachmentFiles', () => { expect(unsupported).toEqual([]); }); + it('refuses a binary renamed to .txt — .txt is sniffed like any other suffix', async () => { + const { supported, unsupported } = await partitionAttachmentFiles([ + file('notes.txt', MP4_HEADER), + ]); + expect(supported).toEqual([]); + expect(unsupported.map((f) => f.name)).toEqual(['notes.txt']); + }); + 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([ @@ -155,3 +186,38 @@ describe('parseUploadErrorMessage', () => { expect(parseUploadErrorMessage('')).toBeUndefined(); }); }); + +describe('parseUploadErrorsByIndex', () => { + it('keys each reason by its upload_index', () => { + const byIndex = parseUploadErrorsByIndex( + JSON.stringify({ + success: false, + message: 'Unsupported file type: .mp4', + errors: [ + { + upload_index: 0, + filename: 'clip.mp4', + error: 'Unsupported file type: .mp4', + }, + { + upload_index: 1, + filename: 'a.zip', + error: 'Unsupported file type: .zip', + }, + ], + }), + ); + // Without this, both chips would repeat the first file's reason. + expect(byIndex.get(0)).toBe('Unsupported file type: .mp4'); + expect(byIndex.get(1)).toBe('Unsupported file type: .zip'); + }); + + it('is empty for bodies with no usable errors array', () => { + expect(parseUploadErrorsByIndex('502').size).toBe(0); + expect(parseUploadErrorsByIndex('{"message":"nope"}').size).toBe(0); + expect(parseUploadErrorsByIndex('').size).toBe(0); + expect( + parseUploadErrorsByIndex('{"errors":[{"error":"no index"}]}').size, + ).toBe(0); + }); +}); diff --git a/frontend/src/constants/fileUpload.ts b/frontend/src/constants/fileUpload.ts index b8fd7c0d..d68b2aa3 100644 --- a/frontend/src/constants/fileUpload.ts +++ b/frontend/src/constants/fileUpload.ts @@ -83,20 +83,20 @@ export const SOURCE_FILE_TREE_ACCEPT_ATTR = [ /** * Chat-attachment suffixes with a dedicated parser. Mirrors the backend's - * `SUPPORTED_ATTACHMENT_EXTENSIONS` (application/parser/file/constants.py) — + * `ATTACHMENT_PARSER_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. + * Not the whole allow-list, and `.txt` is deliberately not here: a suffix + * that isn't listed (.txt, .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', @@ -123,9 +123,17 @@ export const ATTACHMENT_PARSER_EXTENSIONS: readonly string[] = [ '.webm', ]; -/** Picker filter for the Attach button. A hint only — pickers may ignore it. */ -export const ATTACHMENT_FILE_ACCEPT_ATTR = - ATTACHMENT_PARSER_EXTENSIONS.join(','); +/** + * Picker filter for the Attach button. A hint only — pickers may ignore it, + * and it must never be narrower than what the upload accepts: `text/*` (plus + * `.txt` explicitly) keeps parserless text files such as .py and .log + * selectable, since the gate reads those happily. + */ +export const ATTACHMENT_FILE_ACCEPT_ATTR = [ + ...ATTACHMENT_PARSER_EXTENSIONS, + '.txt', + 'text/*', +].join(','); /** Lower-cased last extension including the dot, or '' (dotfiles have none). */ export function getFileExtension(name: string): string { @@ -152,10 +160,20 @@ 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]); +// A UTF-16/32 file is half NUL bytes, so it has to be recognised by its BOM +// before the NUL test — Notepad's "Unicode" .txt is ordinary text. +const TEXT_BOMS = [ + [0xef, 0xbb, 0xbf], + [0xff, 0xfe], + [0xfe, 0xff], + [0x00, 0x00, 0xfe, 0xff], +]; /** Whether a leading byte sample reads as text rather than binary. */ export function looksLikeText(sample: Uint8Array): boolean { if (sample.length === 0) return true; + if (TEXT_BOMS.some((bom) => bom.every((byte, i) => sample[i] === byte))) + return true; let nontext = 0; for (const byte of sample) { if (byte === 0x00) return false; @@ -206,3 +224,30 @@ export function parseUploadErrorMessage(body: string): string | undefined { return undefined; } } + +/** + * Per-file reasons from an error body, keyed by `upload_index`. A rejected + * batch carries one `errors` entry per file, so each chip can say why it + * failed instead of every chip repeating the first file's reason. + */ +export function parseUploadErrorsByIndex(body: string): Map { + const byIndex = new Map(); + if (!body) return byIndex; + try { + const parsed = JSON.parse(body) as { errors?: unknown }; + if (!Array.isArray(parsed?.errors)) return byIndex; + for (const entry of parsed.errors as { + upload_index?: unknown; + error?: unknown; + }[]) { + if ( + typeof entry?.upload_index === 'number' && + typeof entry.error === 'string' + ) + byIndex.set(entry.upload_index, entry.error); + } + } catch { + // Not JSON (a proxy's HTML 502, say) — the caller falls back. + } + return byIndex; +} diff --git a/tests/api/user/attachments/test_routes.py b/tests/api/user/attachments/test_routes.py index e320f58d..a32fa36d 100644 --- a/tests/api/user/attachments/test_routes.py +++ b/tests/api/user/attachments/test_routes.py @@ -2369,6 +2369,20 @@ class TestGetStoreAttachmentUserError: msg = _get_store_attachment_user_error(RuntimeError("oops")) assert msg == "Failed to process file" + @pytest.mark.unit + def test_unsupported_type_message_is_rebuilt_from_the_filename(self): + """Nothing is read off the exception, so its text cannot reach a response.""" + from application.api.user.attachments.routes import ( + _get_store_attachment_user_error, + ) + from application.upload_limits import UnsupportedUploadTypeError + + err = UnsupportedUploadTypeError("internals: /srv/app/tmp/staged-42") + msg = _get_store_attachment_user_error(err, "clip.mp4") + + assert msg == "Unsupported file type: .mp4" + assert "/srv/app" not in msg + class TestRequireLiveSttRedisUnavailable: """Cover lines 99-102: Redis unavailable returns 503.""" @@ -2421,6 +2435,29 @@ class TestStoreAttachmentTypeGate: mock_storage.save_file.assert_not_called() mock_store_attachment.assert_not_called() + @patch("application.api.user.tasks.store_attachment.delay") + def test_rejects_a_binary_renamed_to_a_text_suffix( + self, mock_store_attachment, flask_app + ): + """.txt has no parser, so it is judged on content like any other suffix.""" + 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), "notes.txt")}, + content_type="multipart/form-data", + ): + request.decoded_token = {"sub": "test_user"} + response = StoreAttachment().post() + + assert _get_response_status(response) == 400 + assert _get_response_json(response)["message"] == "Unsupported file type: .txt" + 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 diff --git a/tests/parser/file/test_constants.py b/tests/parser/file/test_constants.py index 2bf6f042..f0cf3919 100644 --- a/tests/parser/file/test_constants.py +++ b/tests/parser/file/test_constants.py @@ -1,15 +1,16 @@ """Tests for the shared file-extension constants.""" import re +from importlib.util import find_spec from pathlib import Path import pytest from application.parser.file.bulk import get_default_file_extractor from application.parser.file.constants import ( + ATTACHMENT_PARSER_EXTENSIONS, attachment_extension, has_attachment_parser, - SUPPORTED_ATTACHMENT_EXTENSIONS, SUPPORTED_SOURCE_EXTENSIONS, ) @@ -19,30 +20,38 @@ FRONTEND_CONSTANTS = ( @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. +def test_parser_extensions_match_the_extractor(): + """The 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 + ``ATTACHMENT_PARSER_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. + A suffix listed here but *without* a parser is the opposite failure: it + skips the content check and reaches the plain-text fallthrough, which is + how a binary named notes.txt got through. """ parser_suffixes = set(get_default_file_extractor()) - assert parser_suffixes <= set(SUPPORTED_ATTACHMENT_EXTENSIONS), ( + assert parser_suffixes <= set(ATTACHMENT_PARSER_EXTENSIONS), ( "parsers exist for suffixes the attachment gate refuses: " - f"{sorted(parser_suffixes - set(SUPPORTED_ATTACHMENT_EXTENSIONS))}" + f"{sorted(parser_suffixes - set(ATTACHMENT_PARSER_EXTENSIONS))}" ) + if find_spec("docling") is not None: + assert parser_suffixes == set(ATTACHMENT_PARSER_EXTENSIONS), ( + "listed as parser-backed but nothing parses them: " + f"{sorted(set(ATTACHMENT_PARSER_EXTENSIONS) - parser_suffixes)}" + ) @pytest.mark.unit -def test_frontend_mirrors_the_attachment_allow_list(): +def test_frontend_mirrors_the_parser_extensions(): """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. + the backend's. 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") @@ -56,16 +65,19 @@ def test_frontend_mirrors_the_attachment_allow_list(): 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), ( + assert frontend_extensions == set(ATTACHMENT_PARSER_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))}" + f"backend only: {sorted(set(ATTACHMENT_PARSER_EXTENSIONS) - frontend_extensions)}, " + f"frontend only: {sorted(frontend_extensions - set(ATTACHMENT_PARSER_EXTENSIONS))}" ) @pytest.mark.unit -def test_attachments_accept_everything_source_ingestion_does(): - assert set(SUPPORTED_SOURCE_EXTENSIONS) <= set(SUPPORTED_ATTACHMENT_EXTENSIONS) +def test_source_ingestion_types_are_parser_backed_apart_from_text(): + """Source ingestion's own list must not smuggle a parserless suffix past the sniff.""" + assert set(SUPPORTED_SOURCE_EXTENSIONS) - set(ATTACHMENT_PARSER_EXTENSIONS) == { + ".txt" + } @pytest.mark.unit @@ -93,3 +105,5 @@ def test_has_attachment_parser_is_case_insensitive_and_excludes_zip(): assert not has_attachment_parser("archive.zip") assert not has_attachment_parser("main.py") assert not has_attachment_parser("clip.mp4") + # .txt is the plain-text fallthrough itself, not a parser. + assert not has_attachment_parser("notes.txt") diff --git a/tests/test_upload_limits.py b/tests/test_upload_limits.py index acb1fba0..87c868f3 100644 --- a/tests/test_upload_limits.py +++ b/tests/test_upload_limits.py @@ -1,5 +1,6 @@ """Tests for bounded user-upload stream helpers.""" +import codecs import io import pytest @@ -80,10 +81,49 @@ def test_enforce_parseable_attachment_rejects_binary_without_a_parser( assert str(excinfo.value).startswith("Unsupported file type") +def test_enforce_parseable_attachment_rejects_binary_named_as_text(tmp_path): + """.txt has no parser — it *is* the fallthrough — so it is sniffed like any suffix. + + Renaming a video to notes.txt would otherwise walk straight back into the + bug this gate exists for. + """ + from application.upload_limits import ( + enforce_parseable_attachment, + UnsupportedUploadTypeError, + ) + + path = tmp_path / "notes.txt" + path.write_bytes(MP4_HEADER + bytes(range(256)) * 4) + + with pytest.raises(UnsupportedUploadTypeError) as excinfo: + enforce_parseable_attachment(path, "notes.txt") + assert str(excinfo.value) == "Unsupported file type: .txt" + + +@pytest.mark.parametrize( + "content", + [ + codecs.BOM_UTF8 + "hello — Unicode\n".encode(), + codecs.BOM_UTF16_LE + "hello\n".encode("utf-16-le"), + codecs.BOM_UTF16_BE + "hello\n".encode("utf-16-be"), + codecs.BOM_UTF32_BE + "hello\n".encode("utf-32-be"), + ], +) +def test_enforce_parseable_attachment_accepts_bom_marked_unicode_text( + content, tmp_path +): + """A UTF-16 .txt is half NUL bytes and still ordinary text — the BOM says so.""" + from application.upload_limits import enforce_parseable_attachment + + path = tmp_path / "notes.txt" + path.write_bytes(content) + + enforce_parseable_attachment(path, "notes.txt") + + @pytest.mark.parametrize( "filename", [ - "notes.txt", "Report.PDF", "photo.JPG", "slides.pptx", @@ -108,7 +148,15 @@ def test_enforce_parseable_attachment_accepts_parser_backed_types(filename, tmp_ @pytest.mark.parametrize( "filename", - ["main.py", "server.log", "config.yaml", "query.sql", "Dockerfile", "notes.unknown"], + [ + "notes.txt", + "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.""" @@ -127,6 +175,8 @@ def test_enforce_parseable_attachment_accepts_text_without_a_parser(filename, tm (b"plain text\n", True), ("café — em dash\n".encode(), True), (b"\x1b[31mred log line\x1b[0m\n", True), + (codecs.BOM_UTF16_LE + "hi\n".encode("utf-16-le"), True), + (codecs.BOM_UTF8 + b"hi\n", True), (b"text\x00with nul", False), (bytes(range(32)) * 4, False), (b"\x7f\x7f\x7f\x7f" + b"a" * 16, False),