From 092a2aedb6a0247376db5ecbd1a7305e1f24d2a0 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 15:07:46 +0200 Subject: [PATCH 1/2] ref(security): Add SafeDirectory to enforce path traversal checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ZipProvider.extract_to_temp_directory() now returns a SafeDirectory instead of a raw Path. SafeDirectory.resolve(untrusted) validates that attacker-controlled paths (manifest values, plist entries) stay within the extraction directory, replacing the manual is_safe_path() + raise pattern at each call site. This makes the safe path the default path — developers can't accidentally skip the check when joining untrusted input to an extract directory. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/android/aab.py | 17 ++-- src/launchpad/artifacts/android/apk.py | 15 ++-- src/launchpad/artifacts/android/zipped_aab.py | 4 +- src/launchpad/artifacts/android/zipped_apk.py | 4 +- .../artifacts/apple/zipped_xcarchive.py | 23 +++--- .../artifacts/providers/exceptions.py | 10 +++ .../artifacts/providers/safe_directory.py | 42 ++++++++++ .../artifacts/providers/zip_provider.py | 21 ++--- .../icon/binary_xml_drawable_parser.py | 3 +- .../parsers/android/icon/icon_parser.py | 20 ++--- .../android/icon/proto_xml_drawable_parser.py | 3 +- src/launchpad/size/analyzers/apple.py | 2 +- .../artifacts/apple/test_zipped_xcarchive.py | 13 +-- .../artifacts/providers/test_zip_provider.py | 79 ++++++++++++++----- 14 files changed, 168 insertions(+), 88 deletions(-) create mode 100644 src/launchpad/artifacts/providers/exceptions.py create mode 100644 src/launchpad/artifacts/providers/safe_directory.py diff --git a/src/launchpad/artifacts/android/aab.py b/src/launchpad/artifacts/android/aab.py index 332c191b..9b86ec8a 100644 --- a/src/launchpad/artifacts/android/aab.py +++ b/src/launchpad/artifacts/android/aab.py @@ -19,7 +19,7 @@ from launchpad.utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path +from ..providers.zip_provider import UnsafePathError, ZipProvider from .apk import APK from .manifest.manifest import AndroidManifest from .manifest.proto_xml import ProtoXmlUtils @@ -44,7 +44,7 @@ def get_manifest(self) -> AndroidManifest: if self._manifest is not None: return self._manifest - manifest_files = list(self._extract_dir.rglob("base/manifest/AndroidManifest.xml")) + manifest_files = list(self._extract_dir.path.rglob("base/manifest/AndroidManifest.xml")) if len(manifest_files) > 1: raise ValueError("Multiple AndroidManifest.xml files found in AAB") @@ -64,7 +64,7 @@ def get_resource_tables(self) -> list[ProtobufResourceTable]: # type: ignore[ov if self._resource_table is not None: return [self._resource_table] - arsc_files = list(self._extract_dir.rglob("base/resources.pb")) + arsc_files = list(self._extract_dir.path.rglob("base/resources.pb")) if len(arsc_files) > 1: raise ValueError("Multiple resources.pb files found in AAB") @@ -132,7 +132,7 @@ def get_dex_mapping(self) -> DexMapping | None: if self._dex_mapping is not None: return self._dex_mapping - dex_mapping_files = list(self._extract_dir.rglob("proguard.map")) + dex_mapping_files = list(self._extract_dir.path.rglob("proguard.map")) if len(dex_mapping_files) > 1: raise ValueError("Multiple proguard.map files found in AAB") @@ -156,11 +156,8 @@ def get_app_icon(self) -> bytes | None: logger.info("No icon path found in manifest") return None - base_dir = self._extract_dir / "base" - if not is_safe_path(base_dir, icon_path_str): - raise UnsafePathError(f"Unsafe icon path in manifest: {icon_path_str}") - - icon_path = base_dir / icon_path_str + base_dir = self._extract_dir.child("base") + icon_path = base_dir.resolve(icon_path_str) if not icon_path.exists(): logger.info(f"Icon not found in AAB: {icon_path_str}") @@ -171,7 +168,7 @@ def get_app_icon(self) -> bytes | None: try: proto_res_tables = self.get_resource_tables() - proto_xml_drawable_parser = ProtoXmlDrawableParser(self._extract_dir / "base", proto_res_tables) + proto_xml_drawable_parser = ProtoXmlDrawableParser(self._extract_dir.child("base"), proto_res_tables) icon = proto_xml_drawable_parser.render_from_path(icon_path) if icon: diff --git a/src/launchpad/artifacts/android/apk.py b/src/launchpad/artifacts/android/apk.py index 436180f8..e2b84417 100644 --- a/src/launchpad/artifacts/android/apk.py +++ b/src/launchpad/artifacts/android/apk.py @@ -18,7 +18,7 @@ from ...parsers.android.dex.types import ClassDefinition from ...utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path +from ..providers.zip_provider import UnsafePathError, ZipProvider from .manifest.axml import AxmlUtils from .manifest.manifest import AndroidManifest from .resources.binary import BinaryResourceTable @@ -54,7 +54,7 @@ def get_manifest(self) -> AndroidManifest: if self._manifest is not None: return self._manifest - manifest_files = list(self._extract_dir.rglob("AndroidManifest.xml")) + manifest_files = list(self._extract_dir.path.rglob("AndroidManifest.xml")) if len(manifest_files) > 1: raise ValueError("Multiple AndroidManifest.xml files found in APK") @@ -74,7 +74,7 @@ def get_resource_tables(self) -> list[BinaryResourceTable]: # type: ignore[over if self._resource_table is not None: return [self._resource_table] - arsc_files = list(self._extract_dir.rglob("resources.arsc")) + arsc_files = list(self._extract_dir.path.rglob("resources.arsc")) if len(arsc_files) > 1: raise ValueError("Multiple resources.arsc files found in APK") @@ -95,7 +95,7 @@ def get_class_definitions(self) -> list[ClassDefinition]: return self._class_definitions self._class_definitions = [] - dex_files = list(self._extract_dir.rglob("classes*.dex")) + dex_files = list(self._extract_dir.path.rglob("classes*.dex")) for dex_file in dex_files: try: with open(dex_file, "rb") as f: @@ -110,7 +110,7 @@ def get_class_definitions(self) -> list[ClassDefinition]: return self._class_definitions def get_extract_path(self) -> Path: - return self._extract_dir + return self._extract_dir.path def get_apksigner_certs(self) -> str: apksigner = Apksigner() @@ -128,10 +128,7 @@ def get_app_icon(self) -> bytes | None: logger.info("No icon path found in manifest") return None - if not is_safe_path(self._extract_dir, icon_path_str): - raise UnsafePathError(f"Unsafe icon path in manifest: {icon_path_str}") - - icon_path = self._extract_dir / icon_path_str + icon_path = self._extract_dir.resolve(icon_path_str) if not icon_path.exists(): logger.info(f"Icon not found in APK: {icon_path_str}") diff --git a/src/launchpad/artifacts/android/zipped_aab.py b/src/launchpad/artifacts/android/zipped_aab.py index 0ad05468..36436bcc 100644 --- a/src/launchpad/artifacts/android/zipped_aab.py +++ b/src/launchpad/artifacts/android/zipped_aab.py @@ -30,7 +30,7 @@ def get_aab(self) -> AAB: if self._aab is not None: return self._aab - for path in self._extract_dir.rglob("*.aab"): + for path in self._extract_dir.path.rglob("*.aab"): if path.is_file(): tmp_dir = Path(tempfile.mkdtemp()) new_path = tmp_dir / path.name @@ -38,7 +38,7 @@ def get_aab(self) -> AAB: self._aab = AAB(new_path, cleanup=lambda: shutil.rmtree(tmp_dir)) return self._aab - raise FileNotFoundError(f"No AAB found in {self._extract_dir}") + raise FileNotFoundError(f"No AAB found in {self._extract_dir.path}") def get_primary_apks(self) -> list[APK]: return self.get_aab().get_primary_apks() diff --git a/src/launchpad/artifacts/android/zipped_apk.py b/src/launchpad/artifacts/android/zipped_apk.py index 9b799b8b..69f33f25 100644 --- a/src/launchpad/artifacts/android/zipped_apk.py +++ b/src/launchpad/artifacts/android/zipped_apk.py @@ -29,7 +29,7 @@ def get_primary_apk(self) -> APK: if self._primary_apk is not None: return self._primary_apk - for path in self._extract_dir.rglob("*.apk"): + for path in self._extract_dir.path.rglob("*.apk"): if path.is_file(): tmp_dir = Path(tempfile.mkdtemp()) new_path = tmp_dir / path.name @@ -37,7 +37,7 @@ def get_primary_apk(self) -> APK: self._primary_apk = APK(new_path, None, cleanup=lambda: shutil.rmtree(tmp_dir)) return self._primary_apk - raise FileNotFoundError(f"No primary APK found in {self._extract_dir}") + raise FileNotFoundError(f"No primary APK found in {self._extract_dir.path}") def get_app_icon(self) -> bytes | None: return self.get_primary_apk().get_app_icon() diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index 608daa31..e8486f54 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -17,7 +17,8 @@ from launchpad.utils.logging import get_logger from ..artifact import AppleArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path +from ..providers.safe_directory import SafeDirectory +from ..providers.zip_provider import ZipProvider logger = get_logger(__name__) @@ -74,7 +75,7 @@ def __init__(self, path: Path) -> None: self._binary_uuid_cache: dict[Path, str] = {} self._lief_cache: dict[Path, lief.MachO.FatBinary] = {} - def get_extract_dir(self) -> Path: + def get_extract_dir(self) -> SafeDirectory: return self._extract_dir @sentry_sdk.trace @@ -100,9 +101,9 @@ def get_archive_plist(self) -> dict[str, Any] | None: if self._archive_plist is not None: return self._archive_plist - xcarchive_dirs = list(self._extract_dir.glob("*.xcarchive")) + xcarchive_dirs = list(self._extract_dir.path.glob("*.xcarchive")) if not xcarchive_dirs: - logger.debug(f"No .xcarchive directory found in {self._extract_dir}") + logger.debug(f"No .xcarchive directory found in {self._extract_dir.path}") return None xcarchive_dir = xcarchive_dirs[0] @@ -128,10 +129,10 @@ def get_app_icon(self) -> bytes | None: return None app_bundle_path = self.get_app_bundle_path() + app_bundle = SafeDirectory(app_bundle_path) for icon_name in icon_info.primary_icon_files: - if not is_safe_path(app_bundle_path, icon_name): - raise UnsafePathError(f"Unsafe icon name in plist: {icon_name}") + app_bundle.resolve(icon_name) # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) # Search for files matching the base name with any suffix @@ -295,12 +296,12 @@ def get_app_bundle_path(self) -> Path: if self._app_bundle_path is not None: return self._app_bundle_path - for path in self._extract_dir.rglob("*.xcarchive/Products/**/*.app"): + for path in self._extract_dir.path.rglob("*.xcarchive/Products/**/*.app"): if path.is_dir() and "__MACOSX" not in str(path): logger.debug(f"Found Apple app bundle: {path}") return path - raise FileNotFoundError(f"No .app bundle found in {self._extract_dir}") + raise FileNotFoundError(f"No .app bundle found in {self._extract_dir.path}") @sentry_sdk.trace def get_main_binary_uuid(self) -> str | None: @@ -377,7 +378,7 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle try: app_bundle_path = self.get_app_bundle_path() json_name = relative_path.with_suffix(".json") - xcarchive_dir = list(self._extract_dir.glob("*.xcarchive"))[0] + xcarchive_dir = list(self._extract_dir.path.glob("*.xcarchive"))[0] app_bundle_path = app_bundle_path.relative_to(xcarchive_dir) parent_path = xcarchive_dir / "ParsedAssets" / app_bundle_path / relative_path.parent @@ -386,7 +387,7 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle if not file_path.exists(): logger.warning( "size.apple.assets_json_not_found", - extra={"file_path": file_path.relative_to(self._extract_dir)}, + extra={"file_path": file_path.relative_to(self._extract_dir.path)}, ) return [] @@ -600,7 +601,7 @@ def _find_dsym_files(self) -> dict[str, DsymInfo]: dsym_info: dict[str, DsymInfo] = {} dsyms_dir = None - for path in self._extract_dir.rglob("dSYMs"): + for path in self._extract_dir.path.rglob("dSYMs"): if path.is_dir(): dsyms_dir = path break diff --git a/src/launchpad/artifacts/providers/exceptions.py b/src/launchpad/artifacts/providers/exceptions.py new file mode 100644 index 00000000..04bba3e4 --- /dev/null +++ b/src/launchpad/artifacts/providers/exceptions.py @@ -0,0 +1,10 @@ +class UnreasonableZipError(ValueError): + """Raised when a zip file exceeds reasonable limits.""" + + pass + + +class UnsafePathError(ValueError): + """Raised when a zip file contains unsafe path entries that could lead to path traversal attacks.""" + + pass diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py new file mode 100644 index 00000000..de36447b --- /dev/null +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -0,0 +1,42 @@ +from pathlib import Path + +from .exceptions import UnsafePathError + + +class SafeDirectory: + """A directory wrapper that validates untrusted paths stay within bounds. + + Use .resolve(untrusted) for attacker-controlled input (manifest values, + plist entries, zip member names). Use .path for trusted operations + (glob, rglob, iterdir). + """ + + def __init__(self, base: Path) -> None: + self._base = base.resolve() + + @property + def path(self) -> Path: + return self._base + + def resolve(self, untrusted: str) -> Path: + """Resolve an untrusted path within this directory. + + Raises UnsafePathError if the resolved path escapes the base. + """ + try: + target = (self._base / untrusted).resolve() + except RuntimeError: + raise UnsafePathError(f"Path traversal attempt: {untrusted}") + if not target.is_relative_to(self._base): + raise UnsafePathError(f"Path traversal attempt: {untrusted}") + return target + + def child(self, untrusted: str) -> "SafeDirectory": + """Return a new SafeDirectory scoped to a validated subdirectory.""" + return SafeDirectory(self.resolve(untrusted)) + + def __repr__(self) -> str: + return f"SafeDirectory({self._base})" + + def __str__(self) -> str: + return str(self._base) diff --git a/src/launchpad/artifacts/providers/zip_provider.py b/src/launchpad/artifacts/providers/zip_provider.py index 8ae8d72a..599859a4 100644 --- a/src/launchpad/artifacts/providers/zip_provider.py +++ b/src/launchpad/artifacts/providers/zip_provider.py @@ -6,24 +6,15 @@ from launchpad.utils.file_utils import cleanup_directory, create_temp_directory from launchpad.utils.logging import get_logger +from .exceptions import UnreasonableZipError, UnsafePathError +from .safe_directory import SafeDirectory + logger = get_logger(__name__) DEFAULT_MAX_FILE_COUNT = 100000 DEFAULT_MAX_UNCOMPRESSED_SIZE = 10 * 1024 * 1024 * 1024 -class UnreasonableZipError(ValueError): - """Raised when a zip file exceeds reasonable limits.""" - - pass - - -class UnsafePathError(ValueError): - """Raised when a zip file contains unsafe path entries that could lead to path traversal attacks.""" - - pass - - def check_reasonable_zip( zf: zipfile.ZipFile, max_file_count: int = DEFAULT_MAX_FILE_COUNT, @@ -80,13 +71,13 @@ def __init__(self, path: Path) -> None: self.path = path self._temp_dirs: List[Path] = [] - def extract_to_temp_directory(self) -> Path: + def extract_to_temp_directory(self) -> SafeDirectory: """Extract the zip contents to a temporary directory. Creates a temporary directory and extracts the zip contents to it. A new temporary directory is created for each call to this method. Returns: - Path to the temporary directory containing extracted files + SafeDirectory wrapping the temporary directory containing extracted files """ temp_dir = create_temp_directory("zip-extract-") self._temp_dirs.append(temp_dir) @@ -94,7 +85,7 @@ def extract_to_temp_directory(self) -> Path: self._safe_extract(str(self.path), str(temp_dir)) logger.debug(f"Extracted zip contents to {temp_dir}") - return temp_dir + return SafeDirectory(temp_dir) def _safe_extract(self, zip_path: str, extract_path: str): """Extract the zip contents to a temporary directory, ensuring that the paths are safe from path traversal attacks. diff --git a/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py b/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py index 2a9f075c..dff9ff48 100644 --- a/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py +++ b/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py @@ -4,6 +4,7 @@ from launchpad.artifacts.android.manifest.axml import AxmlUtils from launchpad.artifacts.android.resources.binary import BinaryResourceTable +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.android.binary.android_binary_parser import AndroidBinaryParser from launchpad.parsers.android.binary.types import XmlNode from launchpad.utils.logging import get_logger @@ -16,7 +17,7 @@ class BinaryXmlDrawableParser(IconParser): def __init__( self, - extract_dir: Path, + extract_dir: Path | SafeDirectory, binary_res_tables: list[BinaryResourceTable], ) -> None: super().__init__(extract_dir) diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index 153b6e1d..e2577a1b 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -10,7 +10,8 @@ from PIL import Image, ImageDraw -from launchpad.artifacts.providers.zip_provider import UnsafePathError, is_safe_path +from launchpad.artifacts.providers.safe_directory import SafeDirectory +from launchpad.artifacts.providers.zip_provider import UnsafePathError from launchpad.parsers.android.binary.types import XmlNode from launchpad.utils.logging import get_logger @@ -57,8 +58,11 @@ class GradientInfo: class IconParser: - def __init__(self, extract_dir: Path) -> None: - self.extract_dir = extract_dir + def __init__(self, extract_dir: Path | SafeDirectory) -> None: + if isinstance(extract_dir, SafeDirectory): + self._safe_dir = extract_dir + else: + self._safe_dir = SafeDirectory(extract_dir) def _get_attr_value(self, attributes: list, name: str, required: bool = False) -> str | None: raise NotImplementedError @@ -461,23 +465,19 @@ def _interpolate_gradient_color( return None def _find_file(self, filename: str) -> Path | None: - if not is_safe_path(self.extract_dir, filename): - raise UnsafePathError(f"Unsafe file path in drawable: {filename}") - - # Try exact match first - exact_path = self.extract_dir / filename + exact_path = self._safe_dir.resolve(filename) if exact_path.exists(): return exact_path # Try with res/ prefix if not filename.startswith("res/"): - res_path = self.extract_dir / "res" / filename + res_path = self._safe_dir.resolve("res/" + filename) if res_path.exists(): return res_path # Search recursively (last resort) filename_lower = filename.lower() - for file_path in self.extract_dir.rglob("*"): + for file_path in self._safe_dir.path.rglob("*"): if file_path.is_file() and str(file_path).lower().endswith(filename_lower): return file_path diff --git a/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py b/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py index 4559b31f..5e179e81 100644 --- a/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py +++ b/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py @@ -13,6 +13,7 @@ from launchpad.artifacts.android.resources.protos.Resources_pb2 import ( XmlNode as PbXmlNode, # type: ignore[attr-defined] ) +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.android.binary.types import ( NodeType, TypedValue, @@ -27,7 +28,7 @@ class ProtoXmlDrawableParser(IconParser): - def __init__(self, extract_dir: Path, proto_res_tables: list[ProtobufResourceTable]) -> None: + def __init__(self, extract_dir: Path | SafeDirectory, proto_res_tables: list[ProtobufResourceTable]) -> None: super().__init__(extract_dir) self.proto_res_tables = proto_res_tables diff --git a/src/launchpad/size/analyzers/apple.py b/src/launchpad/size/analyzers/apple.py index 7e91c42b..ef23faec 100644 --- a/src/launchpad/size/analyzers/apple.py +++ b/src/launchpad/size/analyzers/apple.py @@ -168,7 +168,7 @@ def analyze(self, artifact: AppleArtifact) -> AppleAnalysisResults: ) if binary_info.dsym_path: logger.debug( - f"Found dSYM file for {binary_info.name} at {binary_info.dsym_path.relative_to(artifact.get_extract_dir())}" + f"Found dSYM file for {binary_info.name} at {binary_info.dsym_path.relative_to(artifact.get_extract_dir().path)}" ) binary = self._analyze_binary(binary_info, app_bundle_path, lief_cache) if binary is not None: diff --git a/tests/unit/artifacts/apple/test_zipped_xcarchive.py b/tests/unit/artifacts/apple/test_zipped_xcarchive.py index c3b1d6f7..97d10a0f 100644 --- a/tests/unit/artifacts/apple/test_zipped_xcarchive.py +++ b/tests/unit/artifacts/apple/test_zipped_xcarchive.py @@ -5,6 +5,7 @@ from unittest.mock import patch from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive +from launchpad.artifacts.providers.safe_directory import SafeDirectory class TestZippedXCArchive: @@ -12,7 +13,7 @@ class TestZippedXCArchive: def test_top_level_asset_catalog_parsing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - tmpdir_path = Path(tmpdir) + tmpdir_path = Path(tmpdir).resolve() xcarchive_dir = tmpdir_path / "Test.xcarchive" parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app" @@ -36,7 +37,7 @@ def test_top_level_asset_catalog_parsing(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, @@ -56,7 +57,7 @@ def test_top_level_asset_catalog_parsing(self) -> None: def test_nested_bundle_asset_catalog_parsing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - tmpdir_path = Path(tmpdir) + tmpdir_path = Path(tmpdir).resolve() xcarchive_dir = tmpdir_path / "Test.xcarchive" parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app" @@ -81,7 +82,7 @@ def test_nested_bundle_asset_catalog_parsing(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, @@ -102,7 +103,7 @@ def test_nested_bundle_asset_catalog_parsing(self) -> None: def test_framework_bundle_asset_catalog_parsing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - tmpdir_path = Path(tmpdir) + tmpdir_path = Path(tmpdir).resolve() xcarchive_dir = tmpdir_path / "Test.xcarchive" parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app" @@ -127,7 +128,7 @@ def test_framework_bundle_asset_catalog_parsing(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, diff --git a/tests/unit/artifacts/providers/test_zip_provider.py b/tests/unit/artifacts/providers/test_zip_provider.py index 458c9ec8..6204e5dc 100644 --- a/tests/unit/artifacts/providers/test_zip_provider.py +++ b/tests/unit/artifacts/providers/test_zip_provider.py @@ -5,6 +5,7 @@ import pytest +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.artifacts.providers.zip_provider import ( UnreasonableZipError, UnsafePathError, @@ -32,38 +33,37 @@ def test_init(self, hackernews_xcarchive: Path) -> None: def test_extract_to_temp_directory_ios(self, hackernews_xcarchive: Path) -> None: provider = ZipProvider(hackernews_xcarchive) - temp_dir = provider.extract_to_temp_directory() + safe_dir = provider.extract_to_temp_directory() - assert temp_dir.exists() - assert temp_dir.is_dir() + assert isinstance(safe_dir, SafeDirectory) + assert safe_dir.path.exists() + assert safe_dir.path.is_dir() assert len(provider._temp_dirs) == 1 - assert provider._temp_dirs[0] == temp_dir - extracted_files = list(temp_dir.rglob("*")) + extracted_files = list(safe_dir.path.rglob("*")) assert len(extracted_files) > 0 def test_extract_to_temp_directory_android(self, zipped_apk: Path) -> None: provider = ZipProvider(zipped_apk) - temp_dir = provider.extract_to_temp_directory() + safe_dir = provider.extract_to_temp_directory() - assert temp_dir.exists() - assert temp_dir.is_dir() + assert isinstance(safe_dir, SafeDirectory) + assert safe_dir.path.exists() + assert safe_dir.path.is_dir() assert len(provider._temp_dirs) == 1 - extracted_files = list(temp_dir.rglob("*")) + extracted_files = list(safe_dir.path.rglob("*")) assert len(extracted_files) > 0 def test_multiple_extractions(self, hackernews_xcarchive: Path) -> None: provider = ZipProvider(hackernews_xcarchive) - temp_dir1 = provider.extract_to_temp_directory() - temp_dir2 = provider.extract_to_temp_directory() + safe_dir1 = provider.extract_to_temp_directory() + safe_dir2 = provider.extract_to_temp_directory() - assert temp_dir1 != temp_dir2 + assert safe_dir1.path != safe_dir2.path assert len(provider._temp_dirs) == 2 - assert temp_dir1 in provider._temp_dirs - assert temp_dir2 in provider._temp_dirs - assert temp_dir1.exists() - assert temp_dir2.exists() + assert safe_dir1.path.exists() + assert safe_dir2.path.exists() def test_safe_extract_blocks_traversal(self, malicious_zip: Path) -> None: provider = ZipProvider(malicious_zip) @@ -109,6 +109,45 @@ def test_absolute_paths(self) -> None: assert not is_safe_path(base_dir, "/tmp/other/file.txt") +class TestSafeDirectory: + def test_resolve_valid_paths(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + assert safe_dir.resolve("file.txt") == Path(tmpdir).resolve() / "file.txt" + assert safe_dir.resolve("subdir/file.txt") == Path(tmpdir).resolve() / "subdir" / "file.txt" + + def test_resolve_rejects_traversal(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + with pytest.raises(UnsafePathError): + safe_dir.resolve("../file.txt") + with pytest.raises(UnsafePathError): + safe_dir.resolve("../../etc/passwd") + with pytest.raises(UnsafePathError): + safe_dir.resolve("subdir/../../file.txt") + + def test_resolve_rejects_absolute_paths(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + with pytest.raises(UnsafePathError): + safe_dir.resolve("/etc/passwd") + + def test_child_creates_scoped_directory(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + (base / "sub").mkdir() + safe_dir = SafeDirectory(base) + child = safe_dir.child("sub") + assert child.path == base.resolve() / "sub" + with pytest.raises(UnsafePathError): + child.resolve("../../etc/passwd") + + def test_path_property(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + assert safe_dir.path == Path(tmpdir).resolve() + + class TestCheckReasonableZip: def test_reasonable_zip_passes(self, hackernews_xcarchive: Path) -> None: with zipfile.ZipFile(hackernews_xcarchive, "r") as zf: @@ -137,10 +176,10 @@ def test_extract_zstd_zip(self) -> None: try: provider = ZipProvider(temp_path) - temp_dir = provider.extract_to_temp_directory() + safe_dir = provider.extract_to_temp_directory() - assert temp_dir.exists() - assert (temp_dir / "test.txt").exists() - assert (temp_dir / "test.txt").read_text() == "content" + assert safe_dir.path.exists() + assert (safe_dir.path / "test.txt").exists() + assert (safe_dir.path / "test.txt").read_text() == "content" finally: temp_path.unlink(missing_ok=True) From 8a90e9fe49dca33244a9d2d832bc49ab5901d859 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 16:55:14 +0200 Subject: [PATCH 2/2] ref(security): Make SafeDirectory a drop-in for Path SafeDirectory now delegates glob, rglob, iterdir, /, exists, is_dir, relative_to, __fspath__, and common properties (name, stem, suffix, parent) to the underlying Path. This eliminates the .path accessor from all call sites. Icon parsers (IconParser, BinaryXmlDrawableParser, ProtoXmlDrawableParser) now accept only SafeDirectory instead of Path | SafeDirectory, enforcing the safety boundary at the type level. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/android/aab.py | 6 +-- src/launchpad/artifacts/android/apk.py | 11 ++-- src/launchpad/artifacts/android/zipped_aab.py | 4 +- src/launchpad/artifacts/android/zipped_apk.py | 4 +- .../artifacts/apple/zipped_xcarchive.py | 19 ++++--- .../artifacts/providers/safe_directory.py | 54 +++++++++++++++++-- .../icon/binary_xml_drawable_parser.py | 4 +- .../parsers/android/icon/icon_parser.py | 9 ++-- .../android/icon/proto_xml_drawable_parser.py | 4 +- src/launchpad/size/analyzers/android.py | 2 +- src/launchpad/size/analyzers/apple.py | 2 +- .../artifacts/providers/test_zip_provider.py | 46 ++++++++++++---- .../parsers/android/icon/test_icon_parser.py | 4 +- 13 files changed, 117 insertions(+), 52 deletions(-) diff --git a/src/launchpad/artifacts/android/aab.py b/src/launchpad/artifacts/android/aab.py index 9b86ec8a..a2869d28 100644 --- a/src/launchpad/artifacts/android/aab.py +++ b/src/launchpad/artifacts/android/aab.py @@ -44,7 +44,7 @@ def get_manifest(self) -> AndroidManifest: if self._manifest is not None: return self._manifest - manifest_files = list(self._extract_dir.path.rglob("base/manifest/AndroidManifest.xml")) + manifest_files = list(self._extract_dir.rglob("base/manifest/AndroidManifest.xml")) if len(manifest_files) > 1: raise ValueError("Multiple AndroidManifest.xml files found in AAB") @@ -64,7 +64,7 @@ def get_resource_tables(self) -> list[ProtobufResourceTable]: # type: ignore[ov if self._resource_table is not None: return [self._resource_table] - arsc_files = list(self._extract_dir.path.rglob("base/resources.pb")) + arsc_files = list(self._extract_dir.rglob("base/resources.pb")) if len(arsc_files) > 1: raise ValueError("Multiple resources.pb files found in AAB") @@ -132,7 +132,7 @@ def get_dex_mapping(self) -> DexMapping | None: if self._dex_mapping is not None: return self._dex_mapping - dex_mapping_files = list(self._extract_dir.path.rglob("proguard.map")) + dex_mapping_files = list(self._extract_dir.rglob("proguard.map")) if len(dex_mapping_files) > 1: raise ValueError("Multiple proguard.map files found in AAB") diff --git a/src/launchpad/artifacts/android/apk.py b/src/launchpad/artifacts/android/apk.py index e2b84417..948f732f 100644 --- a/src/launchpad/artifacts/android/apk.py +++ b/src/launchpad/artifacts/android/apk.py @@ -18,6 +18,7 @@ from ...parsers.android.dex.types import ClassDefinition from ...utils.logging import get_logger from ..artifact import AndroidArtifact +from ..providers.safe_directory import SafeDirectory from ..providers.zip_provider import UnsafePathError, ZipProvider from .manifest.axml import AxmlUtils from .manifest.manifest import AndroidManifest @@ -54,7 +55,7 @@ def get_manifest(self) -> AndroidManifest: if self._manifest is not None: return self._manifest - manifest_files = list(self._extract_dir.path.rglob("AndroidManifest.xml")) + manifest_files = list(self._extract_dir.rglob("AndroidManifest.xml")) if len(manifest_files) > 1: raise ValueError("Multiple AndroidManifest.xml files found in APK") @@ -74,7 +75,7 @@ def get_resource_tables(self) -> list[BinaryResourceTable]: # type: ignore[over if self._resource_table is not None: return [self._resource_table] - arsc_files = list(self._extract_dir.path.rglob("resources.arsc")) + arsc_files = list(self._extract_dir.rglob("resources.arsc")) if len(arsc_files) > 1: raise ValueError("Multiple resources.arsc files found in APK") @@ -95,7 +96,7 @@ def get_class_definitions(self) -> list[ClassDefinition]: return self._class_definitions self._class_definitions = [] - dex_files = list(self._extract_dir.path.rglob("classes*.dex")) + dex_files = list(self._extract_dir.rglob("classes*.dex")) for dex_file in dex_files: try: with open(dex_file, "rb") as f: @@ -109,8 +110,8 @@ def get_class_definitions(self) -> list[ClassDefinition]: return self._class_definitions - def get_extract_path(self) -> Path: - return self._extract_dir.path + def get_extract_path(self) -> SafeDirectory: + return self._extract_dir def get_apksigner_certs(self) -> str: apksigner = Apksigner() diff --git a/src/launchpad/artifacts/android/zipped_aab.py b/src/launchpad/artifacts/android/zipped_aab.py index 36436bcc..0ad05468 100644 --- a/src/launchpad/artifacts/android/zipped_aab.py +++ b/src/launchpad/artifacts/android/zipped_aab.py @@ -30,7 +30,7 @@ def get_aab(self) -> AAB: if self._aab is not None: return self._aab - for path in self._extract_dir.path.rglob("*.aab"): + for path in self._extract_dir.rglob("*.aab"): if path.is_file(): tmp_dir = Path(tempfile.mkdtemp()) new_path = tmp_dir / path.name @@ -38,7 +38,7 @@ def get_aab(self) -> AAB: self._aab = AAB(new_path, cleanup=lambda: shutil.rmtree(tmp_dir)) return self._aab - raise FileNotFoundError(f"No AAB found in {self._extract_dir.path}") + raise FileNotFoundError(f"No AAB found in {self._extract_dir}") def get_primary_apks(self) -> list[APK]: return self.get_aab().get_primary_apks() diff --git a/src/launchpad/artifacts/android/zipped_apk.py b/src/launchpad/artifacts/android/zipped_apk.py index 69f33f25..9b799b8b 100644 --- a/src/launchpad/artifacts/android/zipped_apk.py +++ b/src/launchpad/artifacts/android/zipped_apk.py @@ -29,7 +29,7 @@ def get_primary_apk(self) -> APK: if self._primary_apk is not None: return self._primary_apk - for path in self._extract_dir.path.rglob("*.apk"): + for path in self._extract_dir.rglob("*.apk"): if path.is_file(): tmp_dir = Path(tempfile.mkdtemp()) new_path = tmp_dir / path.name @@ -37,7 +37,7 @@ def get_primary_apk(self) -> APK: self._primary_apk = APK(new_path, None, cleanup=lambda: shutil.rmtree(tmp_dir)) return self._primary_apk - raise FileNotFoundError(f"No primary APK found in {self._extract_dir.path}") + raise FileNotFoundError(f"No primary APK found in {self._extract_dir}") def get_app_icon(self) -> bytes | None: return self.get_primary_apk().get_app_icon() diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index e8486f54..fcf5666e 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -101,9 +101,9 @@ def get_archive_plist(self) -> dict[str, Any] | None: if self._archive_plist is not None: return self._archive_plist - xcarchive_dirs = list(self._extract_dir.path.glob("*.xcarchive")) + xcarchive_dirs = list(self._extract_dir.glob("*.xcarchive")) if not xcarchive_dirs: - logger.debug(f"No .xcarchive directory found in {self._extract_dir.path}") + logger.debug(f"No .xcarchive directory found in {self._extract_dir}") return None xcarchive_dir = xcarchive_dirs[0] @@ -128,8 +128,7 @@ def get_app_icon(self) -> bytes | None: logger.warning("No icon files found in CFBundleIconFiles") return None - app_bundle_path = self.get_app_bundle_path() - app_bundle = SafeDirectory(app_bundle_path) + app_bundle = SafeDirectory(self.get_app_bundle_path()) for icon_name in icon_info.primary_icon_files: app_bundle.resolve(icon_name) @@ -137,7 +136,7 @@ def get_app_icon(self) -> bytes | None: # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) # Search for files matching the base name with any suffix # e.g., "AppIcon60x60" matches "AppIcon60x60@2x.png" or "AppIcon60x60.png" - matching_files = list(app_bundle_path.glob(f"{icon_name}*.png")) + matching_files = list(app_bundle.glob(f"{icon_name}*.png")) if not matching_files: continue @@ -296,12 +295,12 @@ def get_app_bundle_path(self) -> Path: if self._app_bundle_path is not None: return self._app_bundle_path - for path in self._extract_dir.path.rglob("*.xcarchive/Products/**/*.app"): + for path in self._extract_dir.rglob("*.xcarchive/Products/**/*.app"): if path.is_dir() and "__MACOSX" not in str(path): logger.debug(f"Found Apple app bundle: {path}") return path - raise FileNotFoundError(f"No .app bundle found in {self._extract_dir.path}") + raise FileNotFoundError(f"No .app bundle found in {self._extract_dir}") @sentry_sdk.trace def get_main_binary_uuid(self) -> str | None: @@ -378,7 +377,7 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle try: app_bundle_path = self.get_app_bundle_path() json_name = relative_path.with_suffix(".json") - xcarchive_dir = list(self._extract_dir.path.glob("*.xcarchive"))[0] + xcarchive_dir = list(self._extract_dir.glob("*.xcarchive"))[0] app_bundle_path = app_bundle_path.relative_to(xcarchive_dir) parent_path = xcarchive_dir / "ParsedAssets" / app_bundle_path / relative_path.parent @@ -387,7 +386,7 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle if not file_path.exists(): logger.warning( "size.apple.assets_json_not_found", - extra={"file_path": file_path.relative_to(self._extract_dir.path)}, + extra={"file_path": file_path.relative_to(self._extract_dir)}, ) return [] @@ -601,7 +600,7 @@ def _find_dsym_files(self) -> dict[str, DsymInfo]: dsym_info: dict[str, DsymInfo] = {} dsyms_dir = None - for path in self._extract_dir.path.rglob("dSYMs"): + for path in self._extract_dir.rglob("dSYMs"): if path.is_dir(): dsyms_dir = path break diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py index de36447b..03d30e66 100644 --- a/src/launchpad/artifacts/providers/safe_directory.py +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -1,4 +1,7 @@ +import os + from pathlib import Path +from typing import Generator from .exceptions import UnsafePathError @@ -7,8 +10,8 @@ class SafeDirectory: """A directory wrapper that validates untrusted paths stay within bounds. Use .resolve(untrusted) for attacker-controlled input (manifest values, - plist entries, zip member names). Use .path for trusted operations - (glob, rglob, iterdir). + plist entries, zip member names). Trusted Path operations (glob, rglob, + iterdir, /, etc.) are delegated directly. """ def __init__(self, base: Path) -> None: @@ -35,8 +38,53 @@ def child(self, untrusted: str) -> "SafeDirectory": """Return a new SafeDirectory scoped to a validated subdirectory.""" return SafeDirectory(self.resolve(untrusted)) + # -- Trusted Path delegations -- + + def __truediv__(self, other: str | os.PathLike[str]) -> Path: + return self._base / other + + def __str__(self) -> str: + return str(self._base) + def __repr__(self) -> str: return f"SafeDirectory({self._base})" - def __str__(self) -> str: + def __fspath__(self) -> str: return str(self._base) + + def glob(self, pattern: str) -> Generator[Path, None, None]: + return self._base.glob(pattern) + + def rglob(self, pattern: str) -> Generator[Path, None, None]: + return self._base.rglob(pattern) + + def iterdir(self) -> Generator[Path, None, None]: + return self._base.iterdir() + + def exists(self) -> bool: + return self._base.exists() + + def is_dir(self) -> bool: + return self._base.is_dir() + + def is_file(self) -> bool: + return self._base.is_file() + + def relative_to(self, other: str | os.PathLike[str]) -> Path: + return self._base.relative_to(other) + + @property + def name(self) -> str: + return self._base.name + + @property + def stem(self) -> str: + return self._base.stem + + @property + def suffix(self) -> str: + return self._base.suffix + + @property + def parent(self) -> Path: + return self._base.parent diff --git a/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py b/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py index dff9ff48..ff71b1f2 100644 --- a/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py +++ b/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py @@ -1,7 +1,5 @@ from __future__ import annotations -from pathlib import Path - from launchpad.artifacts.android.manifest.axml import AxmlUtils from launchpad.artifacts.android.resources.binary import BinaryResourceTable from launchpad.artifacts.providers.safe_directory import SafeDirectory @@ -17,7 +15,7 @@ class BinaryXmlDrawableParser(IconParser): def __init__( self, - extract_dir: Path | SafeDirectory, + extract_dir: SafeDirectory, binary_res_tables: list[BinaryResourceTable], ) -> None: super().__init__(extract_dir) diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index e2577a1b..004bd218 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -58,11 +58,8 @@ class GradientInfo: class IconParser: - def __init__(self, extract_dir: Path | SafeDirectory) -> None: - if isinstance(extract_dir, SafeDirectory): - self._safe_dir = extract_dir - else: - self._safe_dir = SafeDirectory(extract_dir) + def __init__(self, extract_dir: SafeDirectory) -> None: + self._safe_dir = extract_dir def _get_attr_value(self, attributes: list, name: str, required: bool = False) -> str | None: raise NotImplementedError @@ -477,7 +474,7 @@ def _find_file(self, filename: str) -> Path | None: # Search recursively (last resort) filename_lower = filename.lower() - for file_path in self._safe_dir.path.rglob("*"): + for file_path in self._safe_dir.rglob("*"): if file_path.is_file() and str(file_path).lower().endswith(filename_lower): return file_path diff --git a/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py b/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py index 5e179e81..3d661930 100644 --- a/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py +++ b/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py @@ -1,7 +1,5 @@ from __future__ import annotations -from pathlib import Path - from launchpad.artifacts.android.manifest.proto_xml import ProtoXmlUtils from launchpad.artifacts.android.resources.proto import ProtobufResourceTable from launchpad.artifacts.android.resources.protos.Resources_pb2 import ( # type: ignore[attr-defined] @@ -28,7 +26,7 @@ class ProtoXmlDrawableParser(IconParser): - def __init__(self, extract_dir: Path | SafeDirectory, proto_res_tables: list[ProtobufResourceTable]) -> None: + def __init__(self, extract_dir: SafeDirectory, proto_res_tables: list[ProtobufResourceTable]) -> None: super().__init__(extract_dir) self.proto_res_tables = proto_res_tables diff --git a/src/launchpad/size/analyzers/android.py b/src/launchpad/size/analyzers/android.py index 24f57126..09656c4b 100644 --- a/src/launchpad/size/analyzers/android.py +++ b/src/launchpad/size/analyzers/android.py @@ -276,7 +276,7 @@ def _get_hermes_reports(self, apks: list[APK]) -> dict[str, HermesReport]: all_reports: dict[str, HermesReport] = {} for apk in apks: extract_path = apk.get_extract_path() - apk_reports = make_hermes_reports(extract_path) + apk_reports = make_hermes_reports(extract_path.path) for relative_path, report in apk_reports.items(): if relative_path in all_reports: logger.warning(f"Duplicate Hermes report key found: {relative_path}, overwriting") diff --git a/src/launchpad/size/analyzers/apple.py b/src/launchpad/size/analyzers/apple.py index ef23faec..7e91c42b 100644 --- a/src/launchpad/size/analyzers/apple.py +++ b/src/launchpad/size/analyzers/apple.py @@ -168,7 +168,7 @@ def analyze(self, artifact: AppleArtifact) -> AppleAnalysisResults: ) if binary_info.dsym_path: logger.debug( - f"Found dSYM file for {binary_info.name} at {binary_info.dsym_path.relative_to(artifact.get_extract_dir().path)}" + f"Found dSYM file for {binary_info.name} at {binary_info.dsym_path.relative_to(artifact.get_extract_dir())}" ) binary = self._analyze_binary(binary_info, app_bundle_path, lief_cache) if binary is not None: diff --git a/tests/unit/artifacts/providers/test_zip_provider.py b/tests/unit/artifacts/providers/test_zip_provider.py index 6204e5dc..cd92b76c 100644 --- a/tests/unit/artifacts/providers/test_zip_provider.py +++ b/tests/unit/artifacts/providers/test_zip_provider.py @@ -36,10 +36,10 @@ def test_extract_to_temp_directory_ios(self, hackernews_xcarchive: Path) -> None safe_dir = provider.extract_to_temp_directory() assert isinstance(safe_dir, SafeDirectory) - assert safe_dir.path.exists() - assert safe_dir.path.is_dir() + assert safe_dir.exists() + assert safe_dir.is_dir() assert len(provider._temp_dirs) == 1 - extracted_files = list(safe_dir.path.rglob("*")) + extracted_files = list(safe_dir.rglob("*")) assert len(extracted_files) > 0 def test_extract_to_temp_directory_android(self, zipped_apk: Path) -> None: @@ -47,11 +47,11 @@ def test_extract_to_temp_directory_android(self, zipped_apk: Path) -> None: safe_dir = provider.extract_to_temp_directory() assert isinstance(safe_dir, SafeDirectory) - assert safe_dir.path.exists() - assert safe_dir.path.is_dir() + assert safe_dir.exists() + assert safe_dir.is_dir() assert len(provider._temp_dirs) == 1 - extracted_files = list(safe_dir.path.rglob("*")) + extracted_files = list(safe_dir.rglob("*")) assert len(extracted_files) > 0 def test_multiple_extractions(self, hackernews_xcarchive: Path) -> None: @@ -62,8 +62,8 @@ def test_multiple_extractions(self, hackernews_xcarchive: Path) -> None: assert safe_dir1.path != safe_dir2.path assert len(provider._temp_dirs) == 2 - assert safe_dir1.path.exists() - assert safe_dir2.path.exists() + assert safe_dir1.exists() + assert safe_dir2.exists() def test_safe_extract_blocks_traversal(self, malicious_zip: Path) -> None: provider = ZipProvider(malicious_zip) @@ -147,6 +147,30 @@ def test_path_property(self) -> None: safe_dir = SafeDirectory(Path(tmpdir)) assert safe_dir.path == Path(tmpdir).resolve() + def test_truediv_delegates_to_path(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + result = safe_dir / "subdir" / "file.txt" + assert result == Path(tmpdir).resolve() / "subdir" / "file.txt" + assert isinstance(result, Path) + + def test_glob_and_rglob(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + (base / "a.txt").write_text("a") + (base / "sub").mkdir() + (base / "sub" / "b.txt").write_text("b") + safe_dir = SafeDirectory(base) + assert len(list(safe_dir.glob("*.txt"))) == 1 + assert len(list(safe_dir.rglob("*.txt"))) == 2 + + def test_fspath(self) -> None: + import os + + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + assert os.fspath(safe_dir) == str(Path(tmpdir).resolve()) + class TestCheckReasonableZip: def test_reasonable_zip_passes(self, hackernews_xcarchive: Path) -> None: @@ -178,8 +202,8 @@ def test_extract_zstd_zip(self) -> None: provider = ZipProvider(temp_path) safe_dir = provider.extract_to_temp_directory() - assert safe_dir.path.exists() - assert (safe_dir.path / "test.txt").exists() - assert (safe_dir.path / "test.txt").read_text() == "content" + assert safe_dir.exists() + assert (safe_dir / "test.txt").exists() + assert (safe_dir / "test.txt").read_text() == "content" finally: temp_path.unlink(missing_ok=True) diff --git a/tests/unit/parsers/android/icon/test_icon_parser.py b/tests/unit/parsers/android/icon/test_icon_parser.py index 6d50ca96..e1722a29 100644 --- a/tests/unit/parsers/android/icon/test_icon_parser.py +++ b/tests/unit/parsers/android/icon/test_icon_parser.py @@ -4,6 +4,7 @@ import pytest +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.artifacts.providers.zip_provider import UnsafePathError from launchpad.parsers.android.icon.icon_parser import IconParser @@ -11,8 +12,7 @@ class TestIconParserFindFile: def test_find_file_rejects_path_traversal(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - extract_dir = Path(tmpdir) - parser = IconParser(extract_dir) + parser = IconParser(SafeDirectory(Path(tmpdir))) with pytest.raises(UnsafePathError): parser._find_file("../../etc/passwd")