Merge pull request #19406 from nsoranzo/TarFile.extraction_filter

Set safe default extraction filter for tar archives

Author: nsoranzo

lib/galaxy/datatypes/isa.py

@@ -247,7 +247,8 @@ def groom_dataset_content(self, file_name: str) -> None:
             # perform extraction
             # For some ZIP files CompressedFile::extract() extract the file inside <output_folder>/<file_name> instead of outputing it inside <output_folder>. So we first create a temporary folder, extract inside it, and move content to final destination.
             temp_folder = tempfile.mkdtemp()
-            CompressedFile(file_name).extract(temp_folder)
+            with CompressedFile(file_name) as cf:
+                cf.extract(temp_folder)
             shutil.rmtree(output_path)
             extracted_files = os.listdir(temp_folder)
             logger.debug(" ".join(extracted_files))

lib/galaxy/model/store/__init__.py

@@ -3075,7 +3075,8 @@ def source_to_import_store(
             if ModelStoreFormat.is_compressed(model_store_format):
                 try:
                     temp_dir = mkdtemp()
-                    target_dir = CompressedFile(target_path).extract(temp_dir)
+                    with CompressedFile(target_path) as cf:
+                        target_dir = cf.extract(temp_dir)
                 finally:
                     if delete:
                         os.remove(target_path)

lib/galaxy/tool_util/verify/interactor.py

@@ -5,7 +5,6 @@
 import re
 import shutil
 import sys
-import tarfile
 import tempfile
 import time
 import urllib.parse
@@ -45,6 +44,7 @@
 )
 from galaxy.util import requests
 from galaxy.util.bunch import Bunch
+from galaxy.util.compression_utils import CompressedFile
 from galaxy.util.hash_util import (
     memory_bound_hexdigest,
     parse_checksum_hash,
@@ -434,7 +434,14 @@ def test_data_path(self, tool_id, filename, tool_version=None):
             return result
         raise Exception(result["err_msg"])
 
-    def test_data_download(self, tool_id, filename, mode="file", is_output=True, tool_version=None):
+    def test_data_download(
+        self,
+        tool_id: str,
+        filename: str,
+        mode: Literal["directory", "file"] = "file",
+        is_output: bool = True,
+        tool_version: Optional[str] = None,
+    ):
         result = None
         local_path = None
 
@@ -453,7 +460,7 @@ def test_data_download(self, tool_id, filename, mode="file", is_output=True, too
                             contents.extractall(path=path)
                     else:
                         # Galaxy < 21.01
-                        with tarfile.open(fileobj=fileobj) as tar_contents:
+                        with CompressedFile.open_tar(fileobj) as tar_contents:
                             tar_contents.extractall(path=path)
                     result = path
         else:

lib/galaxy/tools/data_fetch.py

@@ -442,8 +442,8 @@ def _decompress_target(upload_config: "UploadConfig", target: Dict[str, Any]):
     # fuzzy_root to False to literally interpret the target.
     fuzzy_root = target.get("fuzzy_root", True)
     temp_directory = os.path.abspath(tempfile.mkdtemp(prefix=elements_from_name, dir=upload_config.working_directory))
-    cf = CompressedFile(elements_from_path)
-    result = cf.extract(temp_directory)
+    with CompressedFile(elements_from_path) as cf:
+        result = cf.extract(temp_directory)
     return result if fuzzy_root else temp_directory
 
 

lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py

@@ -16,6 +16,7 @@
 
 from galaxy.files import ConfiguredFileSources
 from galaxy.files.uris import stream_url_to_file
+from galaxy.util.compression_utils import CompressedFile
 
 # Set max size of archive/file that will be handled to be 100 GB. This is
 # arbitrary and should be adjusted as needed.
@@ -52,19 +53,6 @@ def check_archive(archive_file, dest_dir):
     return True
 
 
-def unpack_archive(archive_file, dest_dir):
-    """
-    Unpack a tar and/or gzipped archive into a destination directory.
-    """
-    if zipfile.is_zipfile(archive_file):
-        with zipfile.ZipFile(archive_file, "r") as zip_archive:
-            zip_archive.extractall(path=dest_dir)
-    else:
-        archive_fp = tarfile.open(archive_file, mode="r")
-        archive_fp.extractall(path=dest_dir)
-        archive_fp.close()
-
-
 def main(options, args):
     is_url = bool(options.is_url)
     is_file = bool(options.is_file)
@@ -84,7 +72,8 @@ def main(options, args):
 
     # Unpack archive.
     check_archive(archive_file, dest_dir)
-    unpack_archive(archive_file, dest_dir)
+    with CompressedFile(archive_file) as cf:
+        cf.archive.extractall(dest_dir)
 
 
 if __name__ == "__main__":

lib/galaxy/util/compression_utils.py

@@ -8,6 +8,7 @@
 import tarfile
 import tempfile
 import zipfile
+from types import TracebackType
 from typing import (
     Any,
     cast,
@@ -18,10 +19,14 @@
     Optional,
     overload,
     Tuple,
+    Type,
     Union,
 )
 
-from typing_extensions import Literal
+from typing_extensions import (
+    Literal,
+    Self,
+)
 
 from galaxy.util.path import (
     safe_relpath,
@@ -167,12 +172,16 @@ def decompress_bytes_to_directory(content: bytes) -> str:
     with tempfile.NamedTemporaryFile(delete=False) as fp:
         fp.write(content)
         fp.close()
-        return CompressedFile(fp.name).extract(temp_directory)
+        with CompressedFile(fp.name) as cf:
+            outdir = cf.extract(temp_directory)
+        return outdir
 
 
 def decompress_path_to_directory(path: str) -> str:
     temp_directory = tempfile.mkdtemp()
-    return CompressedFile(path).extract(temp_directory)
+    with CompressedFile(path) as cf:
+        outdir = cf.extract(temp_directory)
+    return outdir
 
 
 class CompressedFile:
@@ -336,13 +345,24 @@ def isfile(self, member: ArchiveMemberType) -> bool:
             return True
         return False
 
-    def open_tar(self, filepath: StrPath, mode: Literal["a", "r", "w", "x"]) -> tarfile.TarFile:
-        return tarfile.open(filepath, mode, errorlevel=0)
+    @staticmethod
+    def open_tar(file: Union[StrPath, IO[bytes]], mode: Literal["a", "r", "w", "x"] = "r") -> tarfile.TarFile:
+        if isinstance(file, (str, os.PathLike)):
+            tf = tarfile.open(file, mode=mode, errorlevel=0)
+        else:
+            tf = tarfile.open(mode=mode, fileobj=file, errorlevel=0)
+        # Set a safe default ("data_filter") for the extraction filter if
+        # available, reverting to Python 3.11 behavior otherwise, see
+        # https://docs.python.org/3/library/tarfile.html#supporting-older-python-versions
+        tf.extraction_filter = getattr(tarfile, "data_filter", (lambda member, path: member))
+        return tf
 
-    def open_zip(self, filepath: StrPath, mode: Literal["a", "r", "w", "x"]) -> zipfile.ZipFile:
-        return zipfile.ZipFile(filepath, mode)
+    @staticmethod
+    def open_zip(file: Union[StrPath, IO[bytes]], mode: Literal["a", "r", "w", "x"] = "r") -> zipfile.ZipFile:
+        return zipfile.ZipFile(file, mode)
 
-    def zipfile_ok(self, path_to_archive: StrPath) -> bool:
+    @staticmethod
+    def zipfile_ok(path_to_archive: StrPath) -> bool:
         """
         This function is a bit pedantic and not functionally necessary.  It checks whether there is
         no file pointing outside of the extraction, because ZipFile.extractall() has some potential
@@ -356,6 +376,21 @@ def zipfile_ok(self, path_to_archive: StrPath) -> bool:
                 return False
         return True
 
+    def __enter__(self) -> Self:
+        return self
+
+    def __exit__(
+        self,
+        exc_type: Optional[Type[BaseException]],
+        exc_value: Optional[BaseException],
+        traceback: Optional[TracebackType],
+    ) -> bool:
+        try:
+            self.archive.close()
+            return exc_type is None
+        except Exception:
+            return False
+
 
 class FastZipFile(zipfile.ZipFile):
     """

lib/tool_shed/test/base/twilltestcase.py

@@ -55,6 +55,8 @@
     DEFAULT_SOCKET_TIMEOUT,
     smart_str,
 )
+from galaxy.util.compression_utils import CompressedFile
+from galaxy.util.resources import as_file
 from galaxy_test.base.api_asserts import assert_status_code_is_ok
 from galaxy_test.base.api_util import get_admin_api_key
 from galaxy_test.base.populators import wait_on_assertion
@@ -64,7 +66,6 @@
     hgweb_config,
     xml_util,
 )
-from tool_shed.util.repository_content_util import tar_open
 from tool_shed.webapp.model import Repository as DbRepository
 from tool_shed_client.schema import (
     Category,
@@ -1146,7 +1147,8 @@ def add_file_to_repository(
                 target = os.path.basename(source)
             full_target = os.path.join(temp_directory, target)
             full_source = TEST_DATA_REPO_FILES.joinpath(source)
-            shutil.copyfile(str(full_source), full_target)
+            with as_file(full_source) as full_source_path:
+                shutil.copyfile(full_source_path, full_target)
             commit_message = commit_message or "Uploaded revision with added file."
             self._upload_dir_to_repository(
                 repository, temp_directory, commit_message=commit_message, strings_displayed=strings_displayed
@@ -1155,9 +1157,9 @@ def add_file_to_repository(
     def add_tar_to_repository(self, repository: Repository, source: str, strings_displayed=None):
         with self.cloned_repo(repository) as temp_directory:
             full_source = TEST_DATA_REPO_FILES.joinpath(source)
-            tar = tar_open(full_source)
-            tar.extractall(path=temp_directory)
-            tar.close()
+            with full_source.open("rb") as full_source_fileobj:
+                with CompressedFile.open_tar(full_source_fileobj) as tar:
+                    tar.extractall(path=temp_directory)
             commit_message = "Uploaded revision with added files from tar."
             self._upload_dir_to_repository(
                 repository, temp_directory, commit_message=commit_message, strings_displayed=strings_displayed

lib/tool_shed/test/functional/test_shed_repositories.py

@@ -228,7 +228,8 @@ def test_repository_search(self):
 
     def test_repo_tars(self):
         for index, repo_path in enumerate(repo_tars("column_maker")):
-            path = CompressedFile(repo_path).extract(tempfile.mkdtemp())
+            with CompressedFile(repo_path) as cf:
+                path = cf.extract(tempfile.mkdtemp())
             tool_xml_path = os.path.join(path, "column_maker.xml")
             tool_source = get_tool_source(config_file=tool_xml_path)
             tool_version = tool_source.parse_version()

lib/tool_shed/util/repository_content_util.py

@@ -1,6 +1,5 @@
 import os
 import shutil
-import tarfile
 import tempfile
 from typing import (
     Optional,
@@ -9,7 +8,7 @@
 
 import tool_shed.repository_types.util as rt_util
 from galaxy.tool_shed.util.hg_util import clone_repository
-from galaxy.util import checkers
+from galaxy.util.compression_utils import CompressedFile
 from tool_shed.dependencies.attribute_handlers import (
     RepositoryDependencyAttributeHandler,
     ToolDependencyAttributeHandler,
@@ -26,20 +25,6 @@
     from tool_shed.webapp.model import Repository
 
 
-def tar_open(uploaded_file):
-    isgzip = False
-    isbz2 = False
-    isgzip = checkers.is_gzip(uploaded_file)
-    if not isgzip:
-        isbz2 = checkers.is_bz2(uploaded_file)
-    if isgzip or isbz2:
-        # Open for reading with transparent compression.
-        tar = tarfile.open(uploaded_file, "r:*")
-    else:
-        tar = tarfile.open(uploaded_file)
-    return tar
-
-
 def upload_tar(
     trans: "ProvidesRepositoriesContext",
     username: str,
@@ -49,14 +34,12 @@ def upload_tar(
     dry_run: bool = False,
     remove_repo_files_not_in_tar: bool = True,
     new_repo_alert: bool = False,
-    tar=None,
     rdah: Optional[RepositoryDependencyAttributeHandler] = None,
     tdah: Optional[ToolDependencyAttributeHandler] = None,
 ) -> ChangeResponseT:
     host = trans.repositories_hostname
     app = trans.app
-    if tar is None:
-        tar = tar_open(uploaded_file)
+    tar = CompressedFile.open_tar(uploaded_file)
     rdah = rdah or RepositoryDependencyAttributeHandler(trans, unpopulate=False)
     tdah = tdah or ToolDependencyAttributeHandler(trans, unpopulate=False)
     # Upload a tar archive of files.

test/integration/test_workflow_tasks.py

@@ -60,11 +60,13 @@ def test_export_import_invocation_with_input_as_output_sts(self):
 
     def test_export_ro_crate_basic(self):
         ro_crate_path = self._export_invocation_to_format(extension="rocrate.zip", to_uri=False)
-        assert CompressedFile(ro_crate_path).file_type == "zip"
+        with CompressedFile(ro_crate_path) as cf:
+            assert cf.file_type == "zip"
 
     def test_export_ro_crate_to_uri(self):
         ro_crate_path = self._export_invocation_to_format(extension="rocrate.zip", to_uri=True)
-        assert CompressedFile(ro_crate_path).file_type == "zip"
+        with CompressedFile(ro_crate_path) as cf:
+            assert cf.file_type == "zip"
 
     def test_export_bco_basic(self):
         bco_path = self._export_invocation_to_format(extension="bco.json", to_uri=False)

test/unit/data/model/test_model_store.py

@@ -646,9 +646,9 @@ def test_export_invocation_to_ro_crate_archive(tmp_path):
     crate_zip = tmp_path / "crate.zip"
     with store.ROCrateArchiveModelExportStore(crate_zip, app=app, export_files="symlink") as export_store:
         export_store.export_workflow_invocation(workflow_invocation)
-    compressed_file = CompressedFile(crate_zip)
-    assert compressed_file.file_type == "zip"
-    compressed_file.extract(tmp_path)
+    with CompressedFile(crate_zip) as compressed_file:
+        assert compressed_file.file_type == "zip"
+        compressed_file.extract(tmp_path)
     crate_directory = tmp_path / "crate"
     validate_invocation_crate_directory(crate_directory)
 
@@ -1249,7 +1249,8 @@ class Options:
 
 def import_archive(archive_path, app, user, import_options=None):
     dest_parent = mkdtemp()
-    dest_dir = CompressedFile(archive_path).extract(dest_parent)
+    with CompressedFile(archive_path) as cf:
+        dest_dir = cf.extract(dest_parent)
 
     import_options = import_options or store.ImportOptions()
     model_store = store.get_import_model_store_for_directory(

test/unit/tool_shed/test_shed_index.py

@@ -1,6 +1,5 @@
 import os
 import shutil
-import tarfile
 import tempfile
 from collections import namedtuple
 from io import BytesIO
@@ -9,6 +8,7 @@
 import requests
 from whoosh import index
 
+from galaxy.util.compression_utils import CompressedFile
 from tool_shed.util.shed_index import build_index
 
 URL = "https://github.com/mvdbeek/toolshed-test-data/blob/master/toolshed_community_files.tgz?raw=true"
@@ -29,7 +29,8 @@ def community_file_dir():
     response = requests.get(URL)
     response.raise_for_status()
     b = BytesIO(response.content)
-    tarfile.open(fileobj=b, mode="r:gz").extractall(extracted_archive_dir)
+    with CompressedFile.open_tar(b) as tar:
+        tar.extractall(extracted_archive_dir)
     try:
         yield extracted_archive_dir
     finally:

test/unit/util/test_compression_util.py

@@ -15,6 +15,7 @@ def test_compression_safety(self):
         self.assert_safety("test-data/unsafe.zip", False)
         self.assert_safety("test-data/4.bed.zip", True)
         self.assert_safety("test-data/testdir.tar", True)
+        self.assert_safety("test-data/testdir1.tar.gz", True)
         self.assert_safety("test-data/safetar_with_symlink.tar", True)
         self.assert_safety("test-data/safe_relative_symlink.tar", True)
 
@@ -30,10 +31,12 @@ def assert_safety(self, path, expected_to_be_safe):
         temp_dir = tempfile.mkdtemp()
         try:
             if expected_to_be_safe:
-                CompressedFile(path).extract(temp_dir)
+                with CompressedFile(path) as cf:
+                    cf.extract(temp_dir)
             else:
                 with self.assertRaisesRegex(Exception, "is blocked"):
-                    CompressedFile(path).extract(temp_dir)
+                    with CompressedFile(path) as cf:
+                        cf.extract(temp_dir)
         finally:
             shutil.rmtree(temp_dir, ignore_errors=True)