From ef7c2f8c0d050af1537ce5a63dbd02229f3dc120 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 15:07:46 +0200 Subject: [PATCH 01/16] 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 c6f31e18..a4c17074 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 @@ -298,12 +299,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: @@ -380,7 +381,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 @@ -389,7 +390,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 [] @@ -620,7 +621,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 14228232..e5e75e4a 100644 --- a/tests/unit/artifacts/apple/test_zipped_xcarchive.py +++ b/tests/unit/artifacts/apple/test_zipped_xcarchive.py @@ -8,6 +8,7 @@ import pytest from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.artifacts.providers.zip_provider import UnsafePathError @@ -16,7 +17,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" @@ -40,7 +41,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, @@ -60,7 +61,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" @@ -85,7 +86,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, @@ -106,7 +107,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" @@ -131,7 +132,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 c1c6bda1e20b4802a584dddd8838ad6a29ac8e1d Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 16:55:14 +0200 Subject: [PATCH 02/16] 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 a4c17074..ca9ce639 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 @@ -299,12 +298,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: @@ -381,7 +380,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 @@ -390,7 +389,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 [] @@ -621,7 +620,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") From 5c4940998265279e240975475acf018ef5378ff9 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 13:24:19 +0200 Subject: [PATCH 03/16] fix: Restore missing imports lost during rebase Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/apple/zipped_xcarchive.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index ca9ce639..f5d32e0e 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -17,8 +17,9 @@ from launchpad.utils.logging import get_logger from ..artifact import AppleArtifact +from ..providers.exceptions import UnsafePathError from ..providers.safe_directory import SafeDirectory -from ..providers.zip_provider import ZipProvider +from ..providers.zip_provider import ZipProvider, is_safe_path logger = get_logger(__name__) From 3beb0d3fd2440ede0ebdb24af1f7bc68cd77b003 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 13:27:16 +0200 Subject: [PATCH 04/16] ref(security): Replace is_safe_path with SafeDirectory.resolve Co-Authored-By: Claude Opus 4.6 (1M context) --- .../artifacts/apple/zipped_xcarchive.py | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index f5d32e0e..8a3aa744 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -1,5 +1,4 @@ import json -import os import plistlib import shutil import subprocess @@ -19,7 +18,7 @@ from ..artifact import AppleArtifact from ..providers.exceptions import UnsafePathError from ..providers.safe_directory import SafeDirectory -from ..providers.zip_provider import ZipProvider, is_safe_path +from ..providers.zip_provider import ZipProvider logger = get_logger(__name__) @@ -288,10 +287,7 @@ def get_binary_path(self) -> Path | None: if not executable_name: return None - if not is_safe_path(app_bundle_path, executable_name): - raise UnsafePathError(f"Unsafe CFBundleExecutable in plist: {executable_name}") - - return app_bundle_path / executable_name + return SafeDirectory(app_bundle_path).resolve(executable_name) @sentry_sdk.trace def get_app_bundle_path(self) -> Path: @@ -427,11 +423,7 @@ def _discover_extension_binaries(self, app_bundle_path: Path) -> List[Path]: extension_plist = plistlib.load(f) extension_executable = extension_plist.get("CFBundleExecutable") if extension_executable: - if not is_safe_path(extension_path, extension_executable): - raise UnsafePathError( - f"Unsafe CFBundleExecutable in extension plist: {extension_executable}" - ) - extension_binary_path = extension_path / extension_executable + extension_binary_path = SafeDirectory(extension_path).resolve(extension_executable) if extension_binary_path.exists(): extension_binaries.append(extension_binary_path) else: @@ -453,9 +445,7 @@ def _discover_watch_binaries(self, app_bundle_path: Path) -> List[Path]: watch_plist = plistlib.load(f) watch_executable = watch_plist.get("CFBundleExecutable") if watch_executable: - if not is_safe_path(watch_path, watch_executable): - raise UnsafePathError(f"Unsafe CFBundleExecutable in Watch plist: {watch_executable}") - watch_binary_path = watch_path / watch_executable + watch_binary_path = SafeDirectory(watch_path).resolve(watch_executable) if watch_binary_path.exists(): watch_binaries.append(watch_binary_path) else: @@ -485,9 +475,7 @@ def _get_main_binary_path(self) -> Path: main_executable = self.get_plist().get("CFBundleExecutable") if main_executable is None: raise RuntimeError("CFBundleExecutable not found in Info.plist") - if not is_safe_path(app_bundle_path, main_executable): - raise UnsafePathError(f"Unsafe CFBundleExecutable in plist: {main_executable}") - return Path(os.path.join(str(app_bundle_path), main_executable)) + return SafeDirectory(app_bundle_path).resolve(main_executable) def _parse_asset_element(self, item: dict[str, Any], parent_path: Path) -> AssetCatalogElement: """Parse a dictionary item into an AssetCatalogElement.""" @@ -505,9 +493,7 @@ def _parse_asset_element(self, item: dict[str, Any], parent_path: Path) -> Asset file_extension = Path(filename).suffix.lower() if filename and file_extension in {".png", ".jpg", ".jpeg", ".heic", ".heif", ".pdf", ".svg"}: candidate = f"{image_id}{file_extension}" - if not is_safe_path(parent_path, candidate): - raise UnsafePathError(f"Unsafe asset imageId in catalog: {image_id}") - potential_path = parent_path / candidate + potential_path = SafeDirectory(parent_path).resolve(candidate) if potential_path.exists(): full_path = potential_path else: From 9ce30fa250f44e313742dd2967805bd5262c21e6 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 13:30:45 +0200 Subject: [PATCH 05/16] ref(security): Remove is_safe_path in favor of SafeDirectory Replace the last usage of is_safe_path in ZipProvider with SafeDirectory.resolve() and update all imports of UnsafePathError to come from the exceptions module. Add SafeDirectory guidance to CLAUDE.md. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 8 ++++++ src/launchpad/artifacts/android/aab.py | 3 ++- src/launchpad/artifacts/android/apk.py | 3 ++- .../artifacts/providers/zip_provider.py | 24 +++--------------- .../parsers/android/icon/icon_parser.py | 2 +- tests/unit/artifacts/android/test_aab.py | 2 +- tests/unit/artifacts/android/test_apk.py | 2 +- .../artifacts/apple/test_zipped_xcarchive.py | 2 +- .../artifacts/providers/test_zip_provider.py | 25 ++----------------- .../parsers/android/icon/test_icon_parser.py | 2 +- 10 files changed, 23 insertions(+), 50 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2aff0139..74e93bf2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,6 +68,14 @@ make worker # Start Launchpad worker (TaskWorker mode) - **Empty `__init__.py` files** preferred - **Comments**: Only add comments for complex logic or important business decisions +## Security: Path Traversal Prevention + +When handling paths derived from untrusted input (zip member names, plist values, manifest entries, user-supplied filenames), use `SafeDirectory` from `launchpad.artifacts.providers.safe_directory` — never use raw `Path` operations or `is_safe_path()`. + +- **`SafeDirectory(base).resolve(untrusted)`** — validates and returns a safe `Path`, raises `UnsafePathError` on traversal attempts +- **`SafeDirectory(base).child(untrusted)`** — returns a new `SafeDirectory` scoped to a validated subdirectory +- Trusted operations (glob, `/`, iterdir) are delegated directly and don't need validation + ## Testing Strategy - **Prefer integration tests** using real artifacts from `tests/_fixtures/` over unit tests with mocks diff --git a/src/launchpad/artifacts/android/aab.py b/src/launchpad/artifacts/android/aab.py index a2869d28..f13a7190 100644 --- a/src/launchpad/artifacts/android/aab.py +++ b/src/launchpad/artifacts/android/aab.py @@ -19,7 +19,8 @@ from launchpad.utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider +from ..providers.exceptions import UnsafePathError +from ..providers.zip_provider import ZipProvider from .apk import APK from .manifest.manifest import AndroidManifest from .manifest.proto_xml import ProtoXmlUtils diff --git a/src/launchpad/artifacts/android/apk.py b/src/launchpad/artifacts/android/apk.py index 948f732f..204f3a9f 100644 --- a/src/launchpad/artifacts/android/apk.py +++ b/src/launchpad/artifacts/android/apk.py @@ -18,8 +18,9 @@ from ...parsers.android.dex.types import ClassDefinition from ...utils.logging import get_logger from ..artifact import AndroidArtifact +from ..providers.exceptions import UnsafePathError from ..providers.safe_directory import SafeDirectory -from ..providers.zip_provider import UnsafePathError, ZipProvider +from ..providers.zip_provider import ZipProvider from .manifest.axml import AxmlUtils from .manifest.manifest import AndroidManifest from .resources.binary import BinaryResourceTable diff --git a/src/launchpad/artifacts/providers/zip_provider.py b/src/launchpad/artifacts/providers/zip_provider.py index 599859a4..5ab1e938 100644 --- a/src/launchpad/artifacts/providers/zip_provider.py +++ b/src/launchpad/artifacts/providers/zip_provider.py @@ -6,7 +6,7 @@ from launchpad.utils.file_utils import cleanup_directory, create_temp_directory from launchpad.utils.logging import get_logger -from .exceptions import UnreasonableZipError, UnsafePathError +from .exceptions import UnreasonableZipError from .safe_directory import SafeDirectory logger = get_logger(__name__) @@ -45,20 +45,6 @@ def check_reasonable_zip( ) -def is_safe_path(base_dir: Path, requested_path: str) -> bool: - """ - Ensure file operations occur within the intended directory - Based on: https://medium.com/@contactomyna/securing-zip-file-operations-understanding-and-preventing-path-traversal-attacks-74d79f696c46 - """ - try: - base_dir = base_dir.resolve() - target_path = Path(base_dir, requested_path).resolve() - return target_path.is_relative_to(base_dir) - except RuntimeError: - # Resolve raises RuntimeError prior to 3.13 for symlink loops. - return False - - class ZipProvider: """Provider for handling zip file operations.""" @@ -92,14 +78,12 @@ def _safe_extract(self, zip_path: str, extract_path: str): Supports both standard compression methods and Zstandard compression. """ - base_dir = Path(extract_path) + safe_dir = SafeDirectory(Path(extract_path)) with zipfile.ZipFile(zip_path, "r") as zip_ref: check_reasonable_zip(zip_ref) for member in zip_ref.namelist(): - if is_safe_path(base_dir, member): - zip_ref.extract(member, extract_path) - else: - raise UnsafePathError(f"Potential path traversal attack: {member}") + safe_dir.resolve(member) + zip_ref.extract(member, extract_path) def __del__(self) -> None: """Clean up resources when object is destroyed.""" diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index 004bd218..b2a4ae27 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -10,8 +10,8 @@ from PIL import Image, ImageDraw +from launchpad.artifacts.providers.exceptions import UnsafePathError 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 diff --git a/tests/unit/artifacts/android/test_aab.py b/tests/unit/artifacts/android/test_aab.py index 48ac0834..18e3809e 100644 --- a/tests/unit/artifacts/android/test_aab.py +++ b/tests/unit/artifacts/android/test_aab.py @@ -5,7 +5,7 @@ from launchpad.artifacts.android.aab import AAB from launchpad.artifacts.android.manifest.manifest import AndroidApplication, AndroidManifest -from launchpad.artifacts.providers.zip_provider import UnsafePathError +from launchpad.artifacts.providers.exceptions import UnsafePathError @pytest.fixture diff --git a/tests/unit/artifacts/android/test_apk.py b/tests/unit/artifacts/android/test_apk.py index feea0895..a102ecd6 100644 --- a/tests/unit/artifacts/android/test_apk.py +++ b/tests/unit/artifacts/android/test_apk.py @@ -5,7 +5,7 @@ from launchpad.artifacts.android.apk import APK from launchpad.artifacts.android.manifest.manifest import AndroidApplication, AndroidManifest -from launchpad.artifacts.providers.zip_provider import UnsafePathError +from launchpad.artifacts.providers.exceptions import UnsafePathError @pytest.fixture diff --git a/tests/unit/artifacts/apple/test_zipped_xcarchive.py b/tests/unit/artifacts/apple/test_zipped_xcarchive.py index e5e75e4a..4f2f4116 100644 --- a/tests/unit/artifacts/apple/test_zipped_xcarchive.py +++ b/tests/unit/artifacts/apple/test_zipped_xcarchive.py @@ -8,8 +8,8 @@ import pytest from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive +from launchpad.artifacts.providers.exceptions import UnsafePathError from launchpad.artifacts.providers.safe_directory import SafeDirectory -from launchpad.artifacts.providers.zip_provider import UnsafePathError class TestZippedXCArchive: diff --git a/tests/unit/artifacts/providers/test_zip_provider.py b/tests/unit/artifacts/providers/test_zip_provider.py index cd92b76c..e9343fd3 100644 --- a/tests/unit/artifacts/providers/test_zip_provider.py +++ b/tests/unit/artifacts/providers/test_zip_provider.py @@ -5,13 +5,12 @@ import pytest +from launchpad.artifacts.providers.exceptions import UnsafePathError from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.artifacts.providers.zip_provider import ( UnreasonableZipError, - UnsafePathError, ZipProvider, check_reasonable_zip, - is_safe_path, ) @@ -68,7 +67,7 @@ def test_multiple_extractions(self, hackernews_xcarchive: Path) -> None: def test_safe_extract_blocks_traversal(self, malicious_zip: Path) -> None: provider = ZipProvider(malicious_zip) - with pytest.raises(UnsafePathError, match="Potential path traversal attack"): + with pytest.raises(UnsafePathError, match="Path traversal attempt"): provider.extract_to_temp_directory() def test_nonexistent_zip_file(self) -> None: @@ -89,26 +88,6 @@ def test_invalid_zip_file(self) -> None: provider.extract_to_temp_directory() -class TestIsSafePath: - def test_valid_paths(self) -> None: - base_dir = Path("/tmp/test") - assert is_safe_path(base_dir, "file.txt") - assert is_safe_path(base_dir, "subdir/file.txt") - assert is_safe_path(base_dir, "a/b/c/file.txt") - - def test_path_traversal_attempts(self) -> None: - base_dir = Path("/tmp/test") - assert not is_safe_path(base_dir, "../file.txt") - assert not is_safe_path(base_dir, "../../file.txt") - assert not is_safe_path(base_dir, "../../../etc/passwd") - assert not is_safe_path(base_dir, "subdir/../../file.txt") - - def test_absolute_paths(self) -> None: - base_dir = Path("/tmp/test") - assert not is_safe_path(base_dir, "/etc/passwd") - 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: diff --git a/tests/unit/parsers/android/icon/test_icon_parser.py b/tests/unit/parsers/android/icon/test_icon_parser.py index e1722a29..871f0d7d 100644 --- a/tests/unit/parsers/android/icon/test_icon_parser.py +++ b/tests/unit/parsers/android/icon/test_icon_parser.py @@ -4,8 +4,8 @@ import pytest +from launchpad.artifacts.providers.exceptions import UnsafePathError 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 From bf68c6c851f9bd93048ee44377ebcfaceab87a63 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 13:31:45 +0200 Subject: [PATCH 06/16] docs: Update CLAUDE.md SafeDirectory guidance Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 74e93bf2..b1c7d0b3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,11 +70,12 @@ make worker # Start Launchpad worker (TaskWorker mode) ## Security: Path Traversal Prevention -When handling paths derived from untrusted input (zip member names, plist values, manifest entries, user-supplied filenames), use `SafeDirectory` from `launchpad.artifacts.providers.safe_directory` — never use raw `Path` operations or `is_safe_path()`. +When handling paths derived from untrusted input (zip member names, plist values, manifest entries, user-supplied filenames), use `SafeDirectory` from `launchpad.artifacts.providers.safe_directory` — never use raw `Path` operations. - **`SafeDirectory(base).resolve(untrusted)`** — validates and returns a safe `Path`, raises `UnsafePathError` on traversal attempts - **`SafeDirectory(base).child(untrusted)`** — returns a new `SafeDirectory` scoped to a validated subdirectory - Trusted operations (glob, `/`, iterdir) are delegated directly and don't need validation +- Import `UnsafePathError` from `launchpad.artifacts.providers.exceptions` ## Testing Strategy From a6039c47537568056b8cca2345e7f1f31d23ebb4 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 13:34:06 +0200 Subject: [PATCH 07/16] ref(security): Clarify SafeDirectory validation-only call Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/apple/zipped_xcarchive.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index 8a3aa744..c7c00a59 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -131,6 +131,7 @@ def get_app_icon(self) -> bytes | None: app_bundle = SafeDirectory(self.get_app_bundle_path()) for icon_name in icon_info.primary_icon_files: + # Validate the plist-supplied name before globbing with it app_bundle.resolve(icon_name) # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) From 31e020e0628c39978ce9c0c11db7ebd87e8eaebf Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 14:01:56 +0200 Subject: [PATCH 08/16] ref(security): Validate paths in SafeDirectory.__truediv__ and return SafeDirectory from get_app_bundle_path SafeDirectory / now validates that the result stays within bounds, so developers can't accidentally bypass traversal checks with the / operator. get_app_bundle_path() returns SafeDirectory instead of Path, eliminating ad-hoc SafeDirectory wrapping at every call site. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../artifacts/apple/zipped_xcarchive.py | 19 ++++++++++--------- .../artifacts/providers/safe_directory.py | 18 +++++++++++------- .../utils/apple/code_signature_validator.py | 3 ++- .../artifacts/apple/test_zipped_xcarchive.py | 8 ++++---- .../artifacts/providers/test_zip_provider.py | 10 +++++++++- 5 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index c7c00a59..89268dd3 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -67,7 +67,7 @@ def __init__(self, path: Path) -> None: super().__init__(path) self._zip_provider = ZipProvider(path) self._extract_dir = self._zip_provider.extract_to_temp_directory() - self._app_bundle_path: Path | None = None + self._app_bundle_path: SafeDirectory | None = None self._plist: dict[str, Any] | None = None self._archive_plist: dict[str, Any] | None = None self._provisioning_profile: dict[str, Any] | None = None @@ -128,7 +128,7 @@ def get_app_icon(self) -> bytes | None: logger.warning("No icon files found in CFBundleIconFiles") return None - app_bundle = SafeDirectory(self.get_app_bundle_path()) + app_bundle = self.get_app_bundle_path() for icon_name in icon_info.primary_icon_files: # Validate the plist-supplied name before globbing with it @@ -288,10 +288,10 @@ def get_binary_path(self) -> Path | None: if not executable_name: return None - return SafeDirectory(app_bundle_path).resolve(executable_name) + return app_bundle_path.resolve(executable_name) @sentry_sdk.trace - def get_app_bundle_path(self) -> Path: + def get_app_bundle_path(self) -> SafeDirectory: """Get the path to the .app bundle.""" if self._app_bundle_path is not None: return self._app_bundle_path @@ -299,7 +299,8 @@ def get_app_bundle_path(self) -> Path: 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 + self._app_bundle_path = SafeDirectory(path) + return self._app_bundle_path raise FileNotFoundError(f"No .app bundle found in {self._extract_dir}") @@ -401,7 +402,7 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle logger.exception(f"Failed to get asset catalog details for {relative_path}") return [] - def _discover_framework_binaries(self, app_bundle_path: Path) -> List[Path]: + def _discover_framework_binaries(self, app_bundle_path: SafeDirectory) -> List[Path]: framework_binaries: List[Path] = [] for framework_path in app_bundle_path.rglob("*.framework"): if framework_path.is_dir(): @@ -413,7 +414,7 @@ def _discover_framework_binaries(self, app_bundle_path: Path) -> List[Path]: logger.warning("Framework binary not found", extra={"path": framework_binary_path}) return framework_binaries - def _discover_extension_binaries(self, app_bundle_path: Path) -> List[Path]: + def _discover_extension_binaries(self, app_bundle_path: SafeDirectory) -> List[Path]: extension_binaries: List[Path] = [] for extension_path in app_bundle_path.rglob("*.appex"): if extension_path.is_dir(): @@ -435,7 +436,7 @@ def _discover_extension_binaries(self, app_bundle_path: Path) -> List[Path]: logger.exception(f"Failed to read extension Info.plist at {extension_path}") return extension_binaries - def _discover_watch_binaries(self, app_bundle_path: Path) -> List[Path]: + def _discover_watch_binaries(self, app_bundle_path: SafeDirectory) -> List[Path]: watch_binaries: List[Path] = [] for watch_path in app_bundle_path.rglob("Watch/*.app"): if watch_path.is_dir(): @@ -476,7 +477,7 @@ def _get_main_binary_path(self) -> Path: main_executable = self.get_plist().get("CFBundleExecutable") if main_executable is None: raise RuntimeError("CFBundleExecutable not found in Info.plist") - return SafeDirectory(app_bundle_path).resolve(main_executable) + return app_bundle_path.resolve(main_executable) def _parse_asset_element(self, item: dict[str, Any], parent_path: Path) -> AssetCatalogElement: """Parse a dictionary item into an AssetCatalogElement.""" diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py index 03d30e66..9150d17b 100644 --- a/src/launchpad/artifacts/providers/safe_directory.py +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -7,11 +7,11 @@ class SafeDirectory: - """A directory wrapper that validates untrusted paths stay within bounds. + """A directory wrapper that validates all path construction stays within bounds. - Use .resolve(untrusted) for attacker-controlled input (manifest values, - plist entries, zip member names). Trusted Path operations (glob, rglob, - iterdir, /, etc.) are delegated directly. + Every path built via / or .resolve() is checked against the base directory. + glob, rglob, and iterdir return raw Paths (they come from the filesystem + and are inherently within bounds). """ def __init__(self, base: Path) -> None: @@ -38,10 +38,14 @@ 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 + try: + result = (self._base / other).resolve() + except RuntimeError: + raise UnsafePathError(f"Path traversal attempt: {other}") + if not result.is_relative_to(self._base): + raise UnsafePathError(f"Path traversal attempt: {other}") + return result def __str__(self) -> str: return str(self._base) diff --git a/src/launchpad/utils/apple/code_signature_validator.py b/src/launchpad/utils/apple/code_signature_validator.py index ddea8a90..c8a81923 100644 --- a/src/launchpad/utils/apple/code_signature_validator.py +++ b/src/launchpad/utils/apple/code_signature_validator.py @@ -12,6 +12,7 @@ import lief from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.apple.code_signature_parser import CodeSignInformation from launchpad.parsers.apple.macho_parser import MachOParser @@ -84,7 +85,7 @@ def __init__(self, archive: ZippedXCArchive) -> None: self.archive = archive self.plist = self.archive.get_plist() self.executable_name: str = self.archive.get_plist().get("CFBundleExecutable", "") - self.app_root: Path = self.archive.get_app_bundle_path() + self.app_root: SafeDirectory = self.archive.get_app_bundle_path() self.macho_parser: Optional[MachOParser] = None def validate(self) -> Tuple[bool, List[str]]: diff --git a/tests/unit/artifacts/apple/test_zipped_xcarchive.py b/tests/unit/artifacts/apple/test_zipped_xcarchive.py index 4f2f4116..030b5f52 100644 --- a/tests/unit/artifacts/apple/test_zipped_xcarchive.py +++ b/tests/unit/artifacts/apple/test_zipped_xcarchive.py @@ -153,8 +153,8 @@ def test_framework_bundle_asset_catalog_parsing(self) -> None: def test_get_binary_path_rejects_path_traversal(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - app_bundle_path = Path(tmpdir) / "Test.app" - app_bundle_path.mkdir() + app_bundle_path = SafeDirectory(Path(tmpdir) / "Test.app") + app_bundle_path.path.mkdir() with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) @@ -168,8 +168,8 @@ def test_get_binary_path_rejects_path_traversal(self) -> None: def test_get_main_binary_path_rejects_path_traversal(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - app_bundle_path = Path(tmpdir) / "Test.app" - app_bundle_path.mkdir() + app_bundle_path = SafeDirectory(Path(tmpdir) / "Test.app") + app_bundle_path.path.mkdir() with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) diff --git a/tests/unit/artifacts/providers/test_zip_provider.py b/tests/unit/artifacts/providers/test_zip_provider.py index e9343fd3..1b6e563e 100644 --- a/tests/unit/artifacts/providers/test_zip_provider.py +++ b/tests/unit/artifacts/providers/test_zip_provider.py @@ -126,13 +126,21 @@ 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: + def test_truediv_validates_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_truediv_rejects_traversal(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + with pytest.raises(UnsafePathError): + safe_dir / "../../../etc/passwd" + with pytest.raises(UnsafePathError): + safe_dir / "/etc/passwd" + def test_glob_and_rglob(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: base = Path(tmpdir) From 26e2e5db3452636244554cf732923979e34f76a7 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 14:12:01 +0200 Subject: [PATCH 09/16] fix: Add stat() delegation to SafeDirectory Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/providers/safe_directory.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py index 9150d17b..2ec4b962 100644 --- a/src/launchpad/artifacts/providers/safe_directory.py +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -65,6 +65,9 @@ def rglob(self, pattern: str) -> Generator[Path, None, None]: def iterdir(self) -> Generator[Path, None, None]: return self._base.iterdir() + def stat(self) -> os.stat_result: + return self._base.stat() + def exists(self) -> bool: return self._base.exists() From 659f446643d8030d4d559b26b2c0ced0e9e1f62c Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 14:16:48 +0200 Subject: [PATCH 10/16] fix(security): Preserve symlink semantics in SafeDirectory.__truediv__ Validate traversal using the resolved path but return the original non-resolved path so callers can still detect symlinks via is_symlink(). This fixes code signature validation which hashes symlinks by their target string rather than following them. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/providers/safe_directory.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py index 2ec4b962..93dc1482 100644 --- a/src/launchpad/artifacts/providers/safe_directory.py +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -39,12 +39,14 @@ def child(self, untrusted: str) -> "SafeDirectory": return SafeDirectory(self.resolve(untrusted)) def __truediv__(self, other: str | os.PathLike[str]) -> Path: + result = self._base / other try: - result = (self._base / other).resolve() + resolved = result.resolve() except RuntimeError: raise UnsafePathError(f"Path traversal attempt: {other}") - if not result.is_relative_to(self._base): + if not resolved.is_relative_to(self._base): raise UnsafePathError(f"Path traversal attempt: {other}") + # Return the non-resolved path so callers can still detect symlinks return result def __str__(self) -> str: From 9d743e41209c052cd42597ceca214db6a0d72a2c Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 14:25:10 +0200 Subject: [PATCH 11/16] fix: Pass Path to FileInfo instead of SafeDirectory Pydantic validates that full_path is a Path instance. Use .path to unwrap the SafeDirectory at the call site. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/size/utils/file_analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/launchpad/size/utils/file_analysis.py b/src/launchpad/size/utils/file_analysis.py index cd242be7..9a1157f9 100644 --- a/src/launchpad/size/utils/file_analysis.py +++ b/src/launchpad/size/utils/file_analysis.py @@ -42,7 +42,7 @@ def analyze_apple_files( # register root root_rel = "" - dirs[root_rel] = _make_directory_info(app_bundle_path, root_rel) + dirs[root_rel] = _make_directory_info(app_bundle_path.path, root_rel) # inode de-dup (dirs + files) seen_dir_inodes: Set[Tuple[int, int]] = set() From 903458f9e3c91d0170a82770c27fb785f9a469a7 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 18 May 2026 16:02:48 +0200 Subject: [PATCH 12/16] fix: Unwrap SafeDirectory to Path at analysis boundaries Extract .path at the top of analyze_apple_files() and the apple analyzer so downstream code receives a plain Path. Update test fixtures to return SafeDirectory to match get_app_bundle_path(). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/size/analyzers/apple.py | 2 +- src/launchpad/size/utils/file_analysis.py | 4 ++-- tests/unit/size/utils/test_file_analysis.py | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/launchpad/size/analyzers/apple.py b/src/launchpad/size/analyzers/apple.py index 7e91c42b..89f07ee5 100644 --- a/src/launchpad/size/analyzers/apple.py +++ b/src/launchpad/size/analyzers/apple.py @@ -131,7 +131,7 @@ def analyze(self, artifact: AppleArtifact) -> AppleAnalysisResults: file_analysis = analyze_apple_files(artifact) logger.debug(f"Found {len(file_analysis.files)} files, total size: {file_analysis.total_size} bytes") - app_bundle_path = artifact.get_app_bundle_path() + app_bundle_path = artifact.get_app_bundle_path().path bundle_sizes = calculate_bundle_sizes(app_bundle_path, app_info.name, app_info.app_id) total_download_size = bundle_sizes.total_download diff --git a/src/launchpad/size/utils/file_analysis.py b/src/launchpad/size/utils/file_analysis.py index 9a1157f9..be3d4526 100644 --- a/src/launchpad/size/utils/file_analysis.py +++ b/src/launchpad/size/utils/file_analysis.py @@ -34,7 +34,7 @@ def analyze_apple_files( logger.debug("Analyzing files in app bundle") - app_bundle_path = xcarchive.get_app_bundle_path() + app_bundle_path = xcarchive.get_app_bundle_path().path files: Dict[str, FileInfo] = {} dirs: Dict[str, FileInfo] = {} @@ -42,7 +42,7 @@ def analyze_apple_files( # register root root_rel = "" - dirs[root_rel] = _make_directory_info(app_bundle_path.path, root_rel) + dirs[root_rel] = _make_directory_info(app_bundle_path, root_rel) # inode de-dup (dirs + files) seen_dir_inodes: Set[Tuple[int, int]] = set() diff --git a/tests/unit/size/utils/test_file_analysis.py b/tests/unit/size/utils/test_file_analysis.py index 53436fa5..479489cf 100644 --- a/tests/unit/size/utils/test_file_analysis.py +++ b/tests/unit/size/utils/test_file_analysis.py @@ -9,6 +9,7 @@ import pytest from launchpad.artifacts.apple.zipped_xcarchive import AssetCatalogElement, ZippedXCArchive +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.size.constants import APPLE_FILESYSTEM_BLOCK_SIZE from launchpad.size.models.common import FileAnalysis from launchpad.size.models.treemap import TreemapType @@ -61,7 +62,7 @@ def temp_app_bundle(self): (resources_dir / "image.png").write_bytes(b"fake_png_data" * 20) (resources_dir / "data.json").write_text('{"key": "value"}') - yield app_path + yield SafeDirectory(app_path) def test_basic_file_analysis(self, mock_xcarchive, temp_app_bundle): """Test basic file and directory analysis.""" @@ -292,8 +293,8 @@ def test_hash_consistency(self, mock_xcarchive, temp_app_bundle): def test_empty_bundle(self, mock_xcarchive): """Test analysis of an empty app bundle.""" with tempfile.TemporaryDirectory() as temp_dir: - empty_bundle = Path(temp_dir) / "Empty.app" - empty_bundle.mkdir() + empty_bundle = SafeDirectory(Path(temp_dir) / "Empty.app") + empty_bundle.path.mkdir() mock_xcarchive.get_app_bundle_path.return_value = empty_bundle mock_xcarchive.get_asset_catalog_details.return_value = [] From dac40f8b840f2a971d82f3761584fd923ebb9fb6 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 19 May 2026 11:11:43 +0200 Subject: [PATCH 13/16] fix(security): Block glob metacharacter path traversal in icon resolution (VULN-1761) Path.resolve() treats ** as a literal directory name while Path.glob() treats it as a recursive wildcard. An attacker could craft a CFBundleIconFiles entry like **/**/../../../etc that passes is_safe_path (resolve sees **/.. as cancelling) but escapes the extraction root during glob (** matches zero directories, so .. ascends past the base). Two-layer fix: - Reject icon names containing glob metacharacters before they reach glob(), since valid iOS icon names never contain *, ?, [, or ] - SafeDirectory.glob/rglob now validate each result resolves within the base directory, as defense-in-depth for all callers Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/apple/zipped_xcarchive.py | 5 ++++- src/launchpad/artifacts/providers/safe_directory.py | 12 ++++++++---- tests/unit/artifacts/providers/test_zip_provider.py | 13 +++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index 89268dd3..8884e652 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -131,7 +131,10 @@ def get_app_icon(self) -> bytes | None: app_bundle = self.get_app_bundle_path() for icon_name in icon_info.primary_icon_files: - # Validate the plist-supplied name before globbing with it + if any(c in icon_name for c in ("*", "?", "[", "]")): + logger.warning(f"Skipping icon name with glob metacharacters: {icon_name}") + continue + app_bundle.resolve(icon_name) # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py index 93dc1482..ad3a8c74 100644 --- a/src/launchpad/artifacts/providers/safe_directory.py +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -10,8 +10,8 @@ class SafeDirectory: """A directory wrapper that validates all path construction stays within bounds. Every path built via / or .resolve() is checked against the base directory. - glob, rglob, and iterdir return raw Paths (they come from the filesystem - and are inherently within bounds). + glob and rglob validate that each result resolves within the base directory, + since glob patterns containing ".." can escape via recursive wildcards. """ def __init__(self, base: Path) -> None: @@ -59,10 +59,14 @@ def __fspath__(self) -> str: return str(self._base) def glob(self, pattern: str) -> Generator[Path, None, None]: - return self._base.glob(pattern) + for path in self._base.glob(pattern): + if path.resolve().is_relative_to(self._base): + yield path def rglob(self, pattern: str) -> Generator[Path, None, None]: - return self._base.rglob(pattern) + for path in self._base.rglob(pattern): + if path.resolve().is_relative_to(self._base): + yield path def iterdir(self) -> Generator[Path, None, None]: return self._base.iterdir() diff --git a/tests/unit/artifacts/providers/test_zip_provider.py b/tests/unit/artifacts/providers/test_zip_provider.py index 1b6e563e..93058d8b 100644 --- a/tests/unit/artifacts/providers/test_zip_provider.py +++ b/tests/unit/artifacts/providers/test_zip_provider.py @@ -151,6 +151,19 @@ def test_glob_and_rglob(self) -> None: assert len(list(safe_dir.glob("*.txt"))) == 1 assert len(list(safe_dir.rglob("*.txt"))) == 2 + def test_glob_filters_escaped_paths(self) -> None: + """Glob patterns with recursive wildcards and .. must not escape the base.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + inner = base / "inner" + inner.mkdir() + (base / "outside.txt").write_text("should not appear") + (inner / "inside.txt").write_text("ok") + safe_dir = SafeDirectory(inner) + results = list(safe_dir.glob("../*.txt")) + assert all(p.resolve().is_relative_to(inner.resolve()) for p in results) + assert not any("outside" in p.name for p in results) + def test_fspath(self) -> None: import os From 3bffc538a9e74bec2cc2cc4ffd5b78a025d4567f Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 19 May 2026 11:15:52 +0200 Subject: [PATCH 14/16] fix: Remove redundant glob metacharacter check in icon resolution SafeDirectory.resolve() and SafeDirectory.glob() already constrain paths to the base directory, making the explicit metacharacter filtering unnecessary. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/apple/zipped_xcarchive.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index 8884e652..88cb1d7d 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -131,10 +131,6 @@ def get_app_icon(self) -> bytes | None: app_bundle = self.get_app_bundle_path() for icon_name in icon_info.primary_icon_files: - if any(c in icon_name for c in ("*", "?", "[", "]")): - logger.warning(f"Skipping icon name with glob metacharacters: {icon_name}") - continue - app_bundle.resolve(icon_name) # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) From 05ab037d9adf8af638cbbf88f2aececd198e1df4 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 19 May 2026 11:30:01 +0200 Subject: [PATCH 15/16] fix: Filter escaping symlinks in SafeDirectory.iterdir() Match the safety filtering already applied by glob() and rglob() so that iterdir() also excludes entries whose resolved path falls outside the base directory. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/providers/safe_directory.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py index ad3a8c74..a85881cd 100644 --- a/src/launchpad/artifacts/providers/safe_directory.py +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -69,7 +69,9 @@ def rglob(self, pattern: str) -> Generator[Path, None, None]: yield path def iterdir(self) -> Generator[Path, None, None]: - return self._base.iterdir() + for path in self._base.iterdir(): + if path.resolve().is_relative_to(self._base): + yield path def stat(self) -> os.stat_result: return self._base.stat() From 504fbb244a9431d9d3222e14ab12f8bee80995c2 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 19 May 2026 13:29:02 +0200 Subject: [PATCH 16/16] fix(tests): Return SafeDirectory from get_app_bundle_path mocks The mocks were returning plain Path objects which don't match the production return type. This would mask regressions if code starts calling SafeDirectory-specific methods on the return value. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/unit/artifacts/apple/test_zipped_xcarchive.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/artifacts/apple/test_zipped_xcarchive.py b/tests/unit/artifacts/apple/test_zipped_xcarchive.py index 030b5f52..7bca2387 100644 --- a/tests/unit/artifacts/apple/test_zipped_xcarchive.py +++ b/tests/unit/artifacts/apple/test_zipped_xcarchive.py @@ -46,7 +46,7 @@ def test_top_level_asset_catalog_parsing(self) -> None: with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): elements = archive.get_asset_catalog_details(Path("Assets.car")) @@ -91,7 +91,7 @@ def test_nested_bundle_asset_catalog_parsing(self) -> None: with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): elements = archive.get_asset_catalog_details(Path("PlugIns/TestExtension.appex/Assets.car")) @@ -137,7 +137,7 @@ def test_framework_bundle_asset_catalog_parsing(self) -> None: with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): elements = archive.get_asset_catalog_details(Path("MyFramework.bundle/Assets.car")) @@ -230,12 +230,12 @@ def test_asset_catalog_rejects_path_traversal_in_image_id(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, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): with pytest.raises(UnsafePathError): archive.get_asset_catalog_details(Path("Assets.car"))