-
Notifications
You must be signed in to change notification settings - Fork 64
Introduce UPLOAD_FILES_WHITELIST #581
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,8 @@ | |
| from flask import current_app | ||
| from pathlib import Path | ||
|
|
||
| from .config import Configuration | ||
|
|
||
|
|
||
| def generate_checksum(file, chunk_size=4096): | ||
| """ | ||
|
|
@@ -349,6 +351,8 @@ def has_trailing_space(filepath: str) -> bool: | |
|
|
||
| def is_supported_extension(filepath) -> bool: | ||
| """Check whether file's extension is supported.""" | ||
| if check_skip_validation(filepath): | ||
| return True | ||
| ext = os.path.splitext(filepath)[1].lower() | ||
| return ext and ext not in FORBIDDEN_EXTENSIONS | ||
|
|
||
|
|
@@ -491,6 +495,15 @@ def is_supported_extension(filepath) -> bool: | |
| ".xnk", | ||
| } | ||
|
|
||
|
|
||
| def check_skip_validation(file_path: str) -> bool: | ||
| """ | ||
| Check if we can skip validation for this file path. | ||
| Some files are allowed even if they have forbidden extension or mime type. | ||
| """ | ||
| return file_path in Configuration.UPLOAD_FILES_WHITELIST | ||
|
|
||
|
|
||
| FORBIDDEN_MIME_TYPES = { | ||
| "application/x-msdownload", | ||
| "application/x-sh", | ||
|
|
@@ -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 | ||
|
Comment on lines
499
to
534
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -22,10 +22,13 @@ | |||||
| has_valid_characters, | ||||||
| has_valid_first_character, | ||||||
| check_filename, | ||||||
| is_supported_extension, | ||||||
| is_supported_type, | ||||||
| is_valid_path, | ||||||
| get_x_accel_uri, | ||||||
| wkb2wkt, | ||||||
| has_trailing_space, | ||||||
| check_skip_validation, | ||||||
| ) | ||||||
| from ..auth.models import LoginHistory, User | ||||||
| from . import json_headers | ||||||
|
|
@@ -322,3 +325,46 @@ class TestSchema(Schema): | |||||
| "size": "disk_usage", | ||||||
| } | ||||||
| assert schema_map == expected_map | ||||||
|
|
||||||
|
|
||||||
| def test_check_skip_validation(): | ||||||
| ALLOWED_FILES = ["script.js", "config/script.js"] | ||||||
|
|
||||||
| # We patch the Configuration class attribute directly | ||||||
| with patch("mergin.sync.utils.Configuration.UPLOAD_FILES_WHITELIST", ALLOWED_FILES): | ||||||
|
|
||||||
| # Test allowed files | ||||||
| for file_path in ALLOWED_FILES: | ||||||
| assert check_skip_validation(file_path) | ||||||
|
|
||||||
| # Test not allowed files | ||||||
| assert not check_skip_validation("test.py") | ||||||
| assert not check_skip_validation("/some/path/test.py") | ||||||
| assert not check_skip_validation("image.png") | ||||||
|
|
||||||
|
|
||||||
| def test_is_supported_extension(): | ||||||
| ALLOWED_FILES = ["script.js", "config/script.js"] | ||||||
|
|
||||||
| with patch("mergin.sync.utils.Configuration.UPLOAD_FILES_WHITELIST", ALLOWED_FILES): | ||||||
| for file_path in ALLOWED_FILES: | ||||||
| assert is_supported_extension(file_path) | ||||||
|
|
||||||
| # Allowed normal file | ||||||
| assert is_supported_extension("image.png") | ||||||
|
|
||||||
| # Forbidden file | ||||||
| assert not is_supported_extension("test.js") | ||||||
|
|
||||||
|
|
||||||
| def test_mime_type_validation_skip(): | ||||||
| ALLOWED_FILES = ["script.js", "config/script.js"] | ||||||
| # Mocking get_mimetype to return forbidden mime type | ||||||
| with patch( | ||||||
| "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) | ||||||
|
||||||
| assert is_supported_extension(file_path) | |
| assert is_supported_type(file_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment grammar/casing: consider using “MIME” and pluralizing “checks” (e.g., “ignored during extension and MIME type checks”) for clarity and consistency.