Repository navigation
UN-1924 [FIX] Reject unsupported files in API deployment #2267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
215929d
UN-1924 [FIX] Reject unsupported files by sniffing MIME in API storag…
Deepak-Kesavan 7af4732
UN-1924 [FIX] Terminalise an API execution when every file is rejected
Deepak-Kesavan 63d5237
UN-1924 [FIX] Address review findings on unsupported-file rejection
Deepak-Kesavan 4b436a8
UN-1924 [FIX] Release API rate limit slots by org id, not model instance
Deepak-Kesavan 9929f88
UN-1924 [FIX] Fail one undetectable upload instead of the whole request
Deepak-Kesavan 0efd943
UN-1924 [FIX] Acknowledge results and notify subscribers on an all-re…
Deepak-Kesavan b27ef4c
UN-1924 [FIX] Keep acknowledgement and notification independent
Deepak-Kesavan 2bf3bb7
UN-1924 [MISC] Drop formatter churn on two untouched assertions
Deepak-Kesavan cb97bf6
UN-1924 [FIX] Mirror LLMWhisperer's file type gate
Deepak-Kesavan 938a957
Merge remote-tracking branch 'origin/main' into UN-1924-reject-unsupp…
Deepak-Kesavan 6d9f450
UN-1924 [FIX] Recognise a PDF that does not start at offset 0
Deepak-Kesavan 80867ea
Merge remote-tracking branch 'origin/main' into UN-1924-reject-unsupp…
Deepak-Kesavan 5cdce85
UN-1924 [FIX] Harden rejection reporting and the PDF rescue
Deepak-Kesavan 61985aa
UN-1924 [FIX] Require a PDF version header before rescuing a file
Deepak-Kesavan 1b274b5
UN-1924 [FIX] Resolve zip containers, which libmagic will not name fr…
Deepak-Kesavan 048e94f
UN-1924 [REFACTOR] Move the shared MIME gate logic into unstract/core
Deepak-Kesavan 85d3ae5
UN-1924 [FIX] Bound the ODF mimetype read so a zip bomb cannot expand…
Deepak-Kesavan 6c953a9
UN-1924 [FIX] Stop an archive nominating its own file type
Deepak-Kesavan 3b50d5e
UN-1924 [MISC] Leave one throwing call inside the raises block
Deepak-Kesavan 9c8e9ff
UN-1924 [MISC] Pin the zip-carrying-a-PDF-marker case
Deepak-Kesavan 81a9e6a
UN-1924 [FIX] Three small inconsistencies in the new terminal paths
Deepak-Kesavan f059e50
UN-1924 [FIX] Count and keep files rejected before dispatch
Deepak-Kesavan 2d70dfd
UN-1924 [FIX] Keep two rejections apart when the bytes are identical
Deepak-Kesavan efcbbf9
UN-1924 [MISC] Drop a line-number cross-reference from a comment
Deepak-Kesavan 188f3e0
UN-1924 [MISC] Shorten the new code comments
Deepak-Kesavan cbd26fe
UN-1924 [MISC] Clear the two Sonar findings
Deepak-Kesavan 9c619e7
UN-1924 [FIX] Require relationships before naming a zip an Office doc…
Deepak-Kesavan 3afd489
UN-1924 [FIX] Let neither recognised wrapper be renamed by its payload
Deepak-Kesavan 9ce3086
UN-1924 [FIX] Require a declared main part, and one row per rejected …
Deepak-Kesavan 54a5317
UN-1924 [FIX] Correlate the declared main part with what relates it
Deepak-Kesavan a732034
UN-1924 [FIX] Parse package declarations instead of matching their bytes
Deepak-Kesavan a5e664f
UN-1924 [MISC] Split the OOXML check to clear the complexity gate
Deepak-Kesavan 6d2b3ab
Merge branch 'main' into UN-1924-reject-unsupported-files
Deepak-Kesavan f4b0c85
UN-1924 [FIX] Refuse a DTD whatever encoding it is written in
Deepak-Kesavan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
140 changes: 140 additions & 0 deletions
140
backend/workflow_manager/endpoint_v2/tests/test_api_storage_mime_validation.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| """MIME validation for files staged into API storage. | ||
|
|
||
| ``SourceConnector.add_input_file_to_api_storage`` is the single funnel through | ||
| which API-deployment uploads reach the API storage bucket, so an unsupported | ||
| file has to be rejected here or it reaches the extraction step and fails there | ||
| with an error that does not name the real cause. | ||
|
|
||
| Unit tests: the real classmethod runs with its DB/storage-touching | ||
| collaborators patched on the imported module, so no database is needed. MIME | ||
| detection itself is deliberately *not* patched — sniffing the bytes with | ||
| libmagic is the behaviour under test. | ||
| """ | ||
|
|
||
| from unittest import mock | ||
| from unittest.mock import MagicMock | ||
|
|
||
| import pytest | ||
| from django.core.files.uploadedfile import SimpleUploadedFile | ||
|
|
||
| import workflow_manager.endpoint_v2.source as src_mod | ||
| from workflow_manager.endpoint_v2.constants import ApiDeploymentResultStatus | ||
| from workflow_manager.endpoint_v2.source import SourceConnector | ||
|
|
||
| # Bytes chosen from what libmagic actually reports (verified against the pinned | ||
| # python-magic): a PDF header sniffs application/pdf, an HTML document sniffs | ||
| # text/html, which is absent from AllowedFileTypes. | ||
| PDF_BYTES = b"%PDF-1.7\n1 0 obj\n<< /Type /Catalog >>\nendobj\n" | ||
| HTML_BYTES = b"<!DOCTYPE html><html><body>hello</body></html>" | ||
|
|
||
|
|
||
| API_STORAGE_DIR = "/api-storage/exec-1" | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def collaborators(): | ||
| """Patch everything the staging loop touches except MIME detection.""" | ||
| with ( | ||
| mock.patch.multiple( | ||
| src_mod, | ||
| UserContext=mock.DEFAULT, | ||
| WorkflowLog=mock.DEFAULT, | ||
| Workflow=mock.DEFAULT, | ||
| FileSystem=mock.DEFAULT, | ||
| FileHistoryHelper=mock.DEFAULT, | ||
| ResultCacheUtils=mock.DEFAULT, | ||
| ) as mocks, | ||
| mock.patch.object( | ||
| SourceConnector, | ||
| "get_api_storage_dir_path", | ||
| return_value=API_STORAGE_DIR, | ||
| ), | ||
| ): | ||
| storage = MagicMock() | ||
| mocks["FileSystem"].return_value.get_file_storage.return_value = storage | ||
| mocks["storage"] = storage | ||
| yield mocks | ||
|
|
||
|
|
||
| def _upload(name: str, content: bytes, declared: str) -> SimpleUploadedFile: | ||
| """An uploaded file whose declared Content-Type may not match its bytes.""" | ||
| return SimpleUploadedFile(name, content, content_type=declared) | ||
|
|
||
|
|
||
| def _stage(files): | ||
| return SourceConnector.add_input_file_to_api_storage( | ||
| pipeline_id="pipe-1", | ||
| workflow_id="wf-1", | ||
| execution_id="exec-1", | ||
| file_objs=files, | ||
| ) | ||
|
|
||
|
|
||
| def _staged_names(storage: MagicMock) -> set[str]: | ||
| """File names that actually had bytes written to API storage.""" | ||
| return { | ||
| call.kwargs["path"].rsplit("/", 1)[-1] for call in storage.write.call_args_list | ||
| } | ||
|
|
||
|
|
||
| def test_supported_file_is_staged(collaborators) -> None: | ||
| """A real PDF is staged and returned for dispatch.""" | ||
| result = _stage([_upload("doc.pdf", PDF_BYTES, "application/pdf")]) | ||
|
|
||
| assert set(result) == {"doc.pdf"} | ||
| assert result["doc.pdf"].mime_type == "application/pdf" | ||
| assert _staged_names(collaborators["storage"]) == {"doc.pdf"} | ||
|
|
||
|
|
||
| def test_unsupported_bytes_rejected_despite_supported_declared_type( | ||
| collaborators, | ||
| ) -> None: | ||
| """The declared Content-Type must not decide what reaches the bucket. | ||
|
|
||
| An HTML file announced as application/pdf satisfies any header-based check, | ||
| so only sniffing the bytes keeps it out. | ||
| """ | ||
| result = _stage([_upload("evil.pdf", HTML_BYTES, "application/pdf")]) | ||
|
|
||
| # Never dispatched... | ||
| assert result == {} | ||
| # ...and never written to the bucket. | ||
| collaborators["storage"].write.assert_not_called() | ||
|
|
||
|
|
||
| def test_rejection_is_reported_to_the_caller(collaborators) -> None: | ||
| """A rejected file gets its own failed entry in the API response.""" | ||
| _stage([_upload("evil.pdf", HTML_BYTES, "application/pdf")]) | ||
|
|
||
| collaborators["ResultCacheUtils"].update_api_results.assert_called_once() | ||
| api_result = collaborators["ResultCacheUtils"].update_api_results.call_args.kwargs[ | ||
| "api_result" | ||
| ] | ||
| assert api_result.file == "evil.pdf" | ||
| # The message has to name the offending type, not a downstream symptom. | ||
| assert "text/html" in api_result.error | ||
| assert api_result.status == ApiDeploymentResultStatus.FAILED | ||
|
|
||
|
|
||
| def test_missing_declared_type_falls_back_to_sniffed_type(collaborators) -> None: | ||
| """A supported file with no declared Content-Type is still staged. | ||
|
|
||
| The recorded type comes from the bytes, so an absent header neither blocks | ||
| the file nor degrades it to application/octet-stream. | ||
| """ | ||
| result = _stage([_upload("doc.pdf", PDF_BYTES, "")]) | ||
|
|
||
| assert result["doc.pdf"].mime_type == "application/pdf" | ||
|
|
||
|
|
||
| def test_supported_files_survive_a_rejected_sibling(collaborators) -> None: | ||
| """One bad file does not fail the whole request.""" | ||
| result = _stage( | ||
| [ | ||
| _upload("good.pdf", PDF_BYTES, "application/pdf"), | ||
| _upload("evil.pdf", HTML_BYTES, "application/pdf"), | ||
| ] | ||
| ) | ||
|
|
||
| assert set(result) == {"good.pdf"} | ||
| assert _staged_names(collaborators["storage"]) == {"good.pdf"} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.