diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6af1a95..818215f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,7 +45,7 @@ repos: additional_dependencies: ['flake8-bugbear'] - repo: https://github.com/agritheory/test_utils - rev: v1.28.0 + rev: v1.30.1 hooks: - id: update_pre_commit_config - id: validate_frappe_project diff --git a/cloud_storage/cloud_storage/overrides/file.py b/cloud_storage/cloud_storage/overrides/file.py index 0e6e2f9..793ef9d 100644 --- a/cloud_storage/cloud_storage/overrides/file.py +++ b/cloud_storage/cloud_storage/overrides/file.py @@ -46,13 +46,34 @@ def is_remote_file(self) -> bool: return self.file_url.startswith(URL_PREFIXES) # type: ignore return not self.content + def validate_file_path(self, path=None): + """ + HASH: 48366c6ecbad44ed24e6d02bdd8f8f189ce58927 + REPO: https://github.com/frappe/frappe + PATH: frappe/core/doctype/file/file.py + METHOD: validate_file_path + """ + if path is None: + if self.is_remote_file: + return + path = self.get_full_path() + base_path = os.path.realpath(get_files_path(is_private=self.is_private)) + resolved_path = os.path.realpath(path) + if os.path.commonpath((base_path, resolved_path)) != base_path: + frappe.throw(_("The File URL you've entered is incorrect"), title=_("Invalid File URL")) + def validate(self) -> None: """ - HASH: 69a495579a729909f4df7a45855165eee4a208f4 + HASH: ca4a6912ebf7a8dafa9e56822bd304fa38c4da09 REPO: https://github.com/frappe/frappe PATH: frappe/core/doctype/file/file.py METHOD: validate """ + if self.is_folder: + if self.file_url: + frappe.throw(_("A folder cannot have a File URL")) + return + # guard against recursion: associate_files() can save another File, re-entering validate if not self.flags.associating_files: self.associate_files() @@ -312,7 +333,7 @@ def remove_file_association(self, dt: str, dn: str) -> None: @frappe.whitelist() def get_content(self) -> bytes: """ - HASH: bfbebb3d3d9c26eb34ed447112fcd46f1dadff00 + HASH: 48366c6ecbad44ed24e6d02bdd8f8f189ce58927 REPO: https://github.com/frappe/frappe PATH: frappe/core/doctype/file/file.py METHOD: get_content @@ -320,6 +341,7 @@ def get_content(self) -> bytes: if self.is_folder: frappe.throw(_("Cannot get file contents of a Folder")) + self.validate_file_path() if self.get("content"): self._content = self.content if self.decode: # type: ignore @@ -342,6 +364,7 @@ def get_content(self) -> bytes: file_path = frappe.get_site_path("public", "files", self.file_name) else: file_path = frappe.get_site_path("private", "files", self.file_name) + self.validate_file_path(file_path) with open(file_path, mode="rb") as f: self._content = f.read() try: diff --git a/cloud_storage/tests/conftest.py b/cloud_storage/tests/conftest.py index ae770ff..b772cc8 100644 --- a/cloud_storage/tests/conftest.py +++ b/cloud_storage/tests/conftest.py @@ -82,6 +82,26 @@ def patch_frappe_conf(monkeymodule): ) +@pytest.fixture(scope="session", autouse=True) +def cleanup_uploaded_files(db_instance): + watched_dirs = [ + Path(frappe.get_site_path("public", "files")), + Path(frappe.get_site_path("private", "files")), + ] + before = {d: (set(d.rglob("*")) if d.is_dir() else set()) for d in watched_dirs} + + yield + + for d in watched_dirs: + after = set(d.rglob("*")) if d.is_dir() else set() + new_paths = sorted(after - before[d], key=lambda p: len(p.parts), reverse=True) + for path in new_paths: + if path.is_file(): + path.unlink(missing_ok=True) + elif path.is_dir() and not any(path.iterdir()): + path.rmdir() + + @pytest.fixture def mocked_s3_client(): with mock_s3(): diff --git a/cloud_storage/tests/setup.py b/cloud_storage/tests/setup.py index aff7d36..293f9d0 100644 --- a/cloud_storage/tests/setup.py +++ b/cloud_storage/tests/setup.py @@ -1,4 +1,4 @@ -# Copyright (c) 2025, AgriTheory and contributors +# Copyright (c) 2026, AgriTheory and contributors # For license information, please see license.txt import frappe diff --git a/cloud_storage/tests/test_file.py b/cloud_storage/tests/test_file.py index 5232e57..447b962 100644 --- a/cloud_storage/tests/test_file.py +++ b/cloud_storage/tests/test_file.py @@ -337,7 +337,6 @@ def test_associate_files_no_duplicate_association(): doc.associate_files("Module Def", "Cloud Storage") doc.associate_files("Module Def", "Cloud Storage") - file_b = frappe.get_doc("File", "test-assoc-guard-b") matching = [ a diff --git a/cloud_storage/tests/test_local_storage.py b/cloud_storage/tests/test_local_storage.py index 0042ad5..20657c5 100644 --- a/cloud_storage/tests/test_local_storage.py +++ b/cloud_storage/tests/test_local_storage.py @@ -120,3 +120,49 @@ def test_delete_file_local(example_file): file.delete() assert not frappe.db.exists("File", file.name) + + +def test_get_content_rejects_absolute_path_escape(): + """An absolute file_name escapes the files/ sandbox via os.path.join.""" + frappe.set_user("Administrator") + file = frappe.new_doc("File") + file.file_name = "/etc/hostname" + file.file_url = "" + file.is_private = 0 + + with pytest.raises(frappe.ValidationError): + file.get_content() + + +def test_get_content_rejects_relative_path_escape(): + """Same escape, via ../ traversal instead of an absolute path.""" + frappe.set_user("Administrator") + file = frappe.new_doc("File") + file.file_name = "../" * 10 + "etc/hostname" + file.file_url = "" + file.is_private = 1 + + with pytest.raises(frappe.ValidationError): + file.get_content() + + +def test_get_content_rejects_sibling_directory_escape(): + """A sibling dir that merely shares the files/ prefix as a string (files-shadow) + must not pass containment just because startswith(base_path) is true.""" + frappe.set_user("Administrator") + shadow_dir = Path(frappe.get_site_path("private")) / "files-shadow" + shadow_dir.mkdir(exist_ok=True) + secret = shadow_dir / "secret.txt" + secret.write_text("outside the sandbox") + + file = frappe.new_doc("File") + file.file_name = "../files-shadow/secret.txt" + file.file_url = "" + file.is_private = 1 + + try: + with pytest.raises(frappe.ValidationError): + file.get_content() + finally: + secret.unlink() + shadow_dir.rmdir() diff --git a/pyproject.toml b/pyproject.toml index 4fc31fc..9fa0bf5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cloud_storage" -version = "15.10.3" +version = "15.10.5" description = "Frappe App for integrating with cloud storage applications" readme = "README.md" requires-python = ">=3.10"