Conversation
MarcelGeo
commented
Feb 16, 2026
MarcelGeo
commented
- this file paths will be ignored during files extensions and mime type checks
- this file paths will be ignored during files extensions and mime type checks
There was a problem hiding this comment.
Pull request overview
Adds an upload file-path whitelist intended to bypass forbidden extension and MIME type checks for specific, configured paths.
Changes:
- Introduces
Configuration.UPLOAD_FILES_WHITELISTin sync configuration. - Adds
check_skip_validation()and integrates it intois_supported_extension()andis_supported_type(). - Adds tests covering whitelist behavior (extension + intended MIME-type skip).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
server/mergin/sync/config.py |
Adds UPLOAD_FILES_WHITELIST configuration option. |
server/mergin/sync/utils.py |
Implements whitelist check and uses it to bypass extension/MIME validation. |
server/mergin/tests/test_utils.py |
Adds unit tests for whitelist behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "mergin.sync.utils.get_mimetype", return_value="application/x-python-code" | ||
| ), patch("mergin.sync.utils.Configuration.UPLOAD_FILES_WHITELIST", ALLOWED_FILES): | ||
| for file_path in ALLOWED_FILES: | ||
| assert is_supported_extension(file_path) |
There was a problem hiding this comment.
test_mime_type_validation_skip patches get_mimetype and is meant to verify that MIME type validation is skipped for whitelisted paths, but it currently asserts is_supported_extension(file_path) inside the loop. That doesn't exercise is_supported_type at all, so the test will pass even if MIME skipping is broken. Update the assertions to call is_supported_type(...) for the whitelisted paths (and keep the non-whitelisted negative case).
| assert is_supported_extension(file_path) | |
| assert is_supported_type(file_path) |
| @@ -515,6 +528,8 @@ def is_supported_extension(filepath) -> bool: | |||
|
|
|||
| def is_supported_type(filepath) -> bool: | |||
| """Check whether the file mimetype is supported.""" | |||
| if check_skip_validation(filepath): | |||
| return True | |||
| mime_type = get_mimetype(filepath) | |||
| return mime_type.startswith("image/") or mime_type not in FORBIDDEN_MIME_TYPES | |||
There was a problem hiding this comment.
check_skip_validation compares the passed-in file_path directly against Configuration.UPLOAD_FILES_WHITELIST, but is_supported_type is called with a filesystem path in the upload flow (e.g., temporary_location in server/mergin/sync/models.py). That means the whitelist will never match and MIME type validation will not actually be skipped for the intended project-relative paths. Consider changing the API so is_supported_type/check_skip_validation can check the original project-relative path (e.g., accept both logical_path and local_path, or have the caller decide whether to skip before calling get_mimetype).
| EXCLUDED_CLONE_FILENAMES = config( | ||
| "EXCLUDED_CLONE_FILENAMES", default="qgis_cfg.xml", cast=Csv() | ||
| ) | ||
| # files that should be ignored during extension and mime type check |
There was a problem hiding this comment.
Comment grammar/casing: consider using “MIME” and pluralizing “checks” (e.g., “ignored during extension and MIME type checks”) for clarity and consistency.
| # files that should be ignored during extension and mime type check | |
| # files that should be ignored during extension and MIME type checks |
Pull Request Test Coverage Report for Build 22061299119Details
💛 - Coveralls |
3 participants
