diff --git a/src/documents/admin.py b/src/documents/admin.py index d0a43ce9c..719d33c16 100644 --- a/src/documents/admin.py +++ b/src/documents/admin.py @@ -198,6 +198,7 @@ class ShareLinksAdmin(GuardedModelAdmin): class ShareLinkBundleAdmin(GuardedModelAdmin): list_display = ("created", "status", "expiration", "owner", "slug") list_filter = ("status", "created", "expiration", "owner") + readonly_fields = ("file_path",) search_fields = ("slug",) def get_queryset(self, request): # pragma: no cover diff --git a/src/documents/models.py b/src/documents/models.py index 0819b9984..a77447512 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -1019,7 +1019,17 @@ class ShareLinkBundle(models.Model): def absolute_file_path(self) -> Path | None: if not self.file_path: return None - return (settings.SHARE_LINK_BUNDLE_DIR / Path(self.file_path)).resolve() + relative_path = Path(self.file_path) + if relative_path.is_absolute(): + return None + + bundle_dir = settings.SHARE_LINK_BUNDLE_DIR.resolve() + absolute_path = (bundle_dir / relative_path).resolve() + try: + absolute_path.relative_to(bundle_dir) + except ValueError: + return None + return absolute_path def remove_file(self) -> None: if self.absolute_file_path is not None and self.absolute_file_path.exists(): diff --git a/src/documents/tests/test_share_link_bundles.py b/src/documents/tests/test_share_link_bundles.py index 4f0eb057e..0040c5030 100644 --- a/src/documents/tests/test_share_link_bundles.py +++ b/src/documents/tests/test_share_link_bundles.py @@ -124,7 +124,7 @@ class ShareLinkBundleAPITests(DirectoriesMixin, APITestCase): self.assertIn("document_ids", response.data) def test_download_ready_bundle_streams_file(self) -> None: - bundle_file = Path(self.dirs.media_dir) / "bundles" / "ready.zip" + bundle_file = settings.SHARE_LINK_BUNDLE_DIR / "bundles" / "ready.zip" bundle_file.parent.mkdir(parents=True, exist_ok=True) bundle_file.write_bytes(b"binary-zip-content") @@ -132,7 +132,7 @@ class ShareLinkBundleAPITests(DirectoriesMixin, APITestCase): slug="readyslug", file_version=ShareLink.FileVersion.ARCHIVE, status=ShareLinkBundle.Status.READY, - file_path=str(bundle_file), + file_path=str(bundle_file.relative_to(settings.SHARE_LINK_BUNDLE_DIR)), ) bundle.documents.set([self.document]) @@ -199,11 +199,11 @@ class ShareLinkBundleTaskTests(DirectoriesMixin, APITestCase): self.document = DocumentFactory.create() def test_cleanup_expired_share_link_bundles(self) -> None: - expired_path = Path(self.dirs.media_dir) / "expired.zip" + expired_path = settings.SHARE_LINK_BUNDLE_DIR / "expired.zip" expired_path.parent.mkdir(parents=True, exist_ok=True) expired_path.write_bytes(b"expired") - active_path = Path(self.dirs.media_dir) / "active.zip" + active_path = settings.SHARE_LINK_BUNDLE_DIR / "active.zip" active_path.write_bytes(b"active") expired_bundle = ShareLinkBundle.objects.create( @@ -211,7 +211,7 @@ class ShareLinkBundleTaskTests(DirectoriesMixin, APITestCase): file_version=ShareLink.FileVersion.ARCHIVE, status=ShareLinkBundle.Status.READY, expiration=timezone.now() - timedelta(days=1), - file_path=str(expired_path), + file_path=expired_path.name, ) expired_bundle.documents.set([self.document]) @@ -220,7 +220,7 @@ class ShareLinkBundleTaskTests(DirectoriesMixin, APITestCase): file_version=ShareLink.FileVersion.ARCHIVE, status=ShareLinkBundle.Status.READY, expiration=timezone.now() + timedelta(days=1), - file_path=str(active_path), + file_path=active_path.name, ) active_bundle.documents.set([self.document]) @@ -424,7 +424,7 @@ class ShareLinkBundleFilterSetTests(DirectoriesMixin, APITestCase): class ShareLinkBundleModelTests(DirectoriesMixin, APITestCase): - def test_absolute_file_path_handles_relative_and_absolute(self) -> None: + def test_absolute_file_path_handles_relative_path(self) -> None: relative_path = Path("relative.zip") bundle = ShareLinkBundle.objects.create( slug="relative-bundle", @@ -437,10 +437,23 @@ class ShareLinkBundleModelTests(DirectoriesMixin, APITestCase): (settings.SHARE_LINK_BUNDLE_DIR / relative_path).resolve(), ) - absolute_path = Path(self.dirs.media_dir) / "absolute.zip" - bundle.file_path = str(absolute_path) + def test_absolute_file_path_rejects_absolute_path(self) -> None: + bundle = ShareLinkBundle.objects.create( + slug="absolute-bundle", + file_version=ShareLink.FileVersion.ORIGINAL, + file_path=str(Path(self.dirs.media_dir) / "absolute.zip"), + ) - self.assertEqual(bundle.absolute_file_path.resolve(), absolute_path.resolve()) + self.assertIsNone(bundle.absolute_file_path) + + def test_absolute_file_path_rejects_traversal_outside_bundle_dir(self) -> None: + bundle = ShareLinkBundle.objects.create( + slug="traversal-bundle", + file_version=ShareLink.FileVersion.ORIGINAL, + file_path="../escaped.zip", + ) + + self.assertIsNone(bundle.absolute_file_path) def test_str_returns_translated_slug(self) -> None: bundle = ShareLinkBundle.objects.create(