From 531e40d97565d90b9a6f0ef4c4954945c9282633 Mon Sep 17 00:00:00 2001 From: Lautaro Juarez <38353324+lauty95@users.noreply.github.com> Date: Wed, 26 Aug 2026 09:43:00 -0300 Subject: [PATCH 1/6] test: Automatic cleanup of files uploaded during tests (#140) * test: clean up uploaded files after each test session * ci: pre-commit updated * fix(file): validate resolved path before local-disk get_content() to block path traversal * fix(file): prevent sibling-path traversal in local get_conten --- .pre-commit-config.yaml | 2 +- cloud_storage/cloud_storage/overrides/file.py | 20 +++++++- cloud_storage/tests/conftest.py | 20 ++++++++ cloud_storage/tests/test_local_storage.py | 46 +++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6af1a95..8123167 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.29.0 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 94c9c74..a6282bc 100644 --- a/cloud_storage/cloud_storage/overrides/file.py +++ b/cloud_storage/cloud_storage/overrides/file.py @@ -46,6 +46,22 @@ 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 @@ -308,7 +324,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 @@ -316,6 +332,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 @@ -338,6 +355,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/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() From ddf5036da6363c3643ac6dd91b3129f13af2d931 Mon Sep 17 00:00:00 2001 From: Dirk van der Laarse Date: Thu, 3 Sep 2026 16:54:31 +0200 Subject: [PATCH 2/6] style: apply black formatting to test_file.py Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JdVodpzz8zmCAFWajejQdL --- cloud_storage/tests/test_file.py | 1 - 1 file changed, 1 deletion(-) 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 From adc0fa76217f20bc8b31df1fc93230f222fbd921 Mon Sep 17 00:00:00 2001 From: semantic-release Date: Fri, 11 Sep 2026 13:35:10 +0000 Subject: [PATCH 3/6] 15.10.4 Automatically generated by python-semantic-release --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 4fc31fc..473100d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cloud_storage" -version = "15.10.3" +version = "15.10.4" description = "Frappe App for integrating with cloud storage applications" readme = "README.md" requires-python = ">=3.10" From d6d9a424757610606c7004ab7f881e4949c5b66a Mon Sep 17 00:00:00 2001 From: lauty95 Date: Fri, 11 Sep 2026 13:49:27 +0000 Subject: [PATCH 4/6] fix: year updated --- cloud_storage/tests/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From a0af9288dce395a983ab63134001e5d23fa85dd9 Mon Sep 17 00:00:00 2001 From: lauty95 Date: Fri, 11 Sep 2026 14:08:58 +0000 Subject: [PATCH 5/6] fix: validate folder file URLs in File override --- .pre-commit-config.yaml | 2 +- cloud_storage/cloud_storage/overrides/file.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8123167..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.29.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 c08d0f5..793ef9d 100644 --- a/cloud_storage/cloud_storage/overrides/file.py +++ b/cloud_storage/cloud_storage/overrides/file.py @@ -64,11 +64,16 @@ def validate_file_path(self, path=None): 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() From 4aba88507b5563b5bcb9b7ca0a368651e86b0cef Mon Sep 17 00:00:00 2001 From: semantic-release Date: Fri, 11 Sep 2026 14:17:20 +0000 Subject: [PATCH 6/6] 15.10.5 Automatically generated by python-semantic-release --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 473100d..9fa0bf5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cloud_storage" -version = "15.10.4" +version = "15.10.5" description = "Frappe App for integrating with cloud storage applications" readme = "README.md" requires-python = ">=3.10"