From 1ef6950b9b3f403c8151e365c01659e6a92df5c8 Mon Sep 17 00:00:00 2001 From: Hari Veera Mani Kumar Vallu Date: Tue, 12 May 2026 20:49:04 +0200 Subject: [PATCH] Harden extraction and add doctor CLI --- cloakbrowser/__main__.py | 50 ++++++++++++++++++++++++++++++++++++++++ cloakbrowser/download.py | 21 +++++++++++++++-- js/src/args.ts | 5 ++-- tests/test_cli.py | 17 ++++++++++++++ tests/test_cloakserve.py | 2 +- tests/test_config.py | 6 ++--- tests/test_extract.py | 31 +++++++++++++++++++++++-- tests/test_update.py | 10 ++++---- 8 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 tests/test_cli.py diff --git a/cloakbrowser/__main__.py b/cloakbrowser/__main__.py index d9359760..dbcbdecd 100644 --- a/cloakbrowser/__main__.py +++ b/cloakbrowser/__main__.py @@ -10,7 +10,9 @@ from __future__ import annotations import argparse +import importlib.util import logging +import platform import sys @@ -72,6 +74,52 @@ def cmd_clear_cache(args: argparse.Namespace) -> None: print("Cache cleared.") +def _module_available(module: str) -> bool: + try: + return importlib.util.find_spec(module) is not None + except ModuleNotFoundError: + return False + + +def cmd_doctor(args: argparse.Namespace) -> None: + """Print environment diagnostics without downloading Chromium.""" + from .config import get_binary_path, get_cache_dir, get_platform_tag + from .download import binary_info + + print("CloakBrowser doctor") + print(f"Python: {sys.version.split()[0]}") + print(f"OS: {platform.system()} {platform.machine()}") + + try: + print(f"Platform: {get_platform_tag()}") + except RuntimeError as exc: + print(f"Platform: unsupported ({exc})") + + print(f"Cache: {get_cache_dir()}") + + try: + info = binary_info() + binary_path = get_binary_path(info["version"]) + print(f"Version: {info['version']} (bundled: {info['bundled_version']})") + print(f"Binary: {info['binary_path']}") + print(f"Installed: {info['installed']}") + print(f"Usable: {binary_path.exists() and binary_path.is_file()}") + except Exception as exc: + print(f"Binary: unavailable ({exc})") + + checks = { + "playwright": "playwright.sync_api", + "patchright": "patchright.sync_api", + "geoip2": "geoip2.database", + "aiohttp": "aiohttp", + "websockets": "websockets", + } + print("Modules:") + for label, module in checks.items(): + status = "ok" if _module_available(module) else "missing" + print(f" {label}: {status}") + + def main() -> None: parser = argparse.ArgumentParser( prog="cloakbrowser", @@ -83,6 +131,7 @@ def main() -> None: sub.add_parser("info", help="Show binary version, path, and platform") sub.add_parser("update", help="Check for and download a newer binary") sub.add_parser("clear-cache", help="Remove all cached binaries") + sub.add_parser("doctor", help="Show environment diagnostics") args = parser.parse_args() if not args.command: @@ -96,6 +145,7 @@ def main() -> None: "info": cmd_info, "update": cmd_update, "clear-cache": cmd_clear_cache, + "doctor": cmd_doctor, } try: diff --git a/cloakbrowser/download.py b/cloakbrowser/download.py index fc064eb8..54b549f4 100644 --- a/cloakbrowser/download.py +++ b/cloakbrowser/download.py @@ -312,6 +312,7 @@ def _extract_archive( def _extract_tar(archive_path: Path, dest_dir: Path) -> None: """Extract tar.gz archive with path traversal protection.""" + dest_resolved = dest_dir.resolve() with tarfile.open(archive_path, "r:gz") as tar: safe_members = [] for member in tar.getmembers(): @@ -323,7 +324,7 @@ def _extract_tar(archive_path: Path, dest_dir: Path) -> None: continue else: member_path = (dest_dir / member.name).resolve() - if not str(member_path).startswith(str(dest_dir.resolve())): + if not _path_is_relative_to(member_path, dest_resolved): raise RuntimeError(f"Archive contains path traversal: {member.name}") safe_members.append(member) @@ -334,14 +335,24 @@ def _extract_zip(archive_path: Path, dest_dir: Path) -> None: """Extract zip archive with path traversal protection.""" import zipfile + dest_resolved = dest_dir.resolve() with zipfile.ZipFile(archive_path, "r") as zf: for info in zf.infolist(): member_path = (dest_dir / info.filename).resolve() - if not str(member_path).startswith(str(dest_dir.resolve())): + if not _path_is_relative_to(member_path, dest_resolved): raise RuntimeError(f"Archive contains path traversal: {info.filename}") zf.extractall(dest_dir) +def _path_is_relative_to(path: Path, parent: Path) -> bool: + """Return True only when path is inside parent.""" + try: + path.relative_to(parent) + return True + except ValueError: + return False + + def _flatten_single_subdir(dest_dir: Path) -> None: """If extraction created a single subdirectory, move its contents up. @@ -365,6 +376,12 @@ def _flatten_single_subdir(dest_dir: Path) -> None: def _is_executable(path: Path) -> bool: """Check if a file is executable.""" + if not path.is_file(): + return False + if platform.system() == "Windows": + pathext = os.environ.get("PATHEXT", ".COM;.EXE;.BAT;.CMD") + executable_exts = {ext.lower() for ext in pathext.split(";") if ext} + return path.suffix.lower() in executable_exts return os.access(path, os.X_OK) diff --git a/js/src/args.ts b/js/src/args.ts index 9c6e9766..77e9e56d 100644 --- a/js/src/args.ts +++ b/js/src/args.ts @@ -5,7 +5,8 @@ import type { LaunchOptions } from "./types.js"; import { getDefaultStealthArgs } from "./config.js"; -const DEBUG = /\bcloakbrowser\b/.test(process.env.DEBUG ?? ""); +const nodeProcess = (globalThis as any).process; +const DEBUG = /\bcloakbrowser\b/.test(nodeProcess?.env?.DEBUG ?? ""); /** * Build deduplicated Chromium CLI args from stealth defaults + user overrides. @@ -26,7 +27,7 @@ export function buildArgs(options: LaunchOptions): string[] { // - Windows (all modes): Chromium's GPU blocklist blocks WebGPU for the // Microsoft Basic Render Driver. Dawn's adapter_blocklist bypass alone // isn't enough. Linux doesn't need it. - if (options.headless === false || process.platform === "win32") { + if (options.headless === false || nodeProcess.platform === "win32") { seen.set("--ignore-gpu-blocklist", "--ignore-gpu-blocklist"); } if (options.args) { diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 00000000..6e0a4c1e --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,17 @@ +"""Unit tests for the cloakbrowser CLI.""" + +from unittest.mock import patch + +from cloakbrowser.__main__ import cmd_doctor + + +def test_doctor_prints_environment_without_downloading(capsys): + with patch("cloakbrowser.download.ensure_binary") as mock_ensure: + cmd_doctor(None) + + mock_ensure.assert_not_called() + out = capsys.readouterr().out + assert "CloakBrowser doctor" in out + assert "Python:" in out + assert "Platform:" in out + assert "Modules:" in out diff --git a/tests/test_cloakserve.py b/tests/test_cloakserve.py index a1dccb16..367f7d07 100644 --- a/tests/test_cloakserve.py +++ b/tests/test_cloakserve.py @@ -133,7 +133,7 @@ def test_default_data_dir_docker(self, _mock): @patch("os.path.exists", return_value=False) def test_default_data_dir_bare_metal(self, _mock): result = _default_data_dir() - assert result.endswith(".cloakbrowser/cloakserve") + assert Path(result).parts[-2:] == (".cloakbrowser", "cloakserve") # --------------------------------------------------------------------------- diff --git a/tests/test_config.py b/tests/test_config.py index 2ec3e638..10be6621 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -26,17 +26,17 @@ class TestGetBinaryPath: def test_linux(self): with patch("cloakbrowser.config.platform.system", return_value="Linux"): path = get_binary_path("145.0.0.0") - assert str(path).endswith("chromium-145.0.0.0/chrome") + assert path.as_posix().endswith("chromium-145.0.0.0/chrome") def test_darwin(self): with patch("cloakbrowser.config.platform.system", return_value="Darwin"): path = get_binary_path("145.0.0.0") - assert str(path).endswith("chromium-145.0.0.0/Chromium.app/Contents/MacOS/Chromium") + assert path.as_posix().endswith("chromium-145.0.0.0/Chromium.app/Contents/MacOS/Chromium") def test_windows(self): with patch("cloakbrowser.config.platform.system", return_value="Windows"): path = get_binary_path("145.0.0.0") - assert str(path).endswith("chromium-145.0.0.0/chrome.exe") + assert path.as_posix().endswith("chromium-145.0.0.0/chrome.exe") # --------------------------------------------------------------------------- diff --git a/tests/test_extract.py b/tests/test_extract.py index b06aa49c..74c49fba 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -6,8 +6,9 @@ import stat import tarfile import zipfile +from pathlib import Path -import pytest +import pytest # type: ignore[import-not-found] from cloakbrowser.download import ( _extract_tar, @@ -55,6 +56,20 @@ def test_path_traversal_blocked(self, tmp_path): with pytest.raises(RuntimeError, match="path traversal"): _extract_tar(archive, dest) + def test_path_prefix_sibling_escape_blocked(self, tmp_path): + """Sibling paths that merely share a string prefix are still outside.""" + archive = tmp_path / "evil-prefix.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + info = tarfile.TarInfo(name="../out-evil/pwned") + info.size = 4 + tar.addfile(info, io.BytesIO(b"evil")) + + dest = tmp_path / "out" + dest.mkdir() + with pytest.raises(RuntimeError, match="path traversal"): + _extract_tar(archive, dest) + assert not (tmp_path / "out-evil").exists() + def test_suspicious_symlink_skipped(self, tmp_path): """Symlinks with absolute targets are skipped (logged as warning).""" archive = tmp_path / "symlink.tar.gz" @@ -111,6 +126,18 @@ def test_path_traversal_blocked(self, tmp_path): with pytest.raises(RuntimeError, match="path traversal"): _extract_zip(archive, dest) + def test_path_prefix_sibling_escape_blocked(self, tmp_path): + """Sibling paths that merely share a string prefix are still outside.""" + archive = tmp_path / "evil-prefix.zip" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("../out-evil/pwned", "evil") + + dest = tmp_path / "out" + dest.mkdir() + with pytest.raises(RuntimeError, match="path traversal"): + _extract_zip(archive, dest) + assert not (tmp_path / "out-evil").exists() + # --------------------------------------------------------------------------- # Directory flattening @@ -180,7 +207,7 @@ def test_make_executable(self, tmp_path): assert _is_executable(binary) def test_is_executable_true(self, tmp_path): - binary = tmp_path / "chrome" + binary = tmp_path / ("chrome.exe" if platform.system() == "Windows" else "chrome") binary.write_bytes(b"binary") binary.chmod(0o755) assert _is_executable(binary) diff --git a/tests/test_update.py b/tests/test_update.py index 7ba08618..3337f9f1 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -13,6 +13,8 @@ CHROMIUM_VERSION, _version_newer, _version_tuple, + get_archive_ext, + get_archive_name, get_chromium_version, get_download_url, get_effective_version, @@ -74,7 +76,7 @@ def test_default_url_format(self): url = get_download_url() assert "cloakbrowser.dev" in url assert f"chromium-v{get_chromium_version()}" in url - assert url.endswith(".tar.gz") + assert url.endswith(get_archive_ext()) def test_custom_version_url(self): url = get_download_url("145.0.7718.0") @@ -161,10 +163,10 @@ class TestGetLatestVersion: def _make_assets(self, platforms: list[str]) -> list[dict]: """Helper to build asset list from platform tags.""" - return [{"name": f"cloakbrowser-{p}.tar.gz"} for p in platforms] + return [{"name": f"cloakbrowser-{p}{get_archive_ext()}"} for p in platforms] def _platform_tarball(self) -> str: - return f"cloakbrowser-{get_platform_tag()}.tar.gz" + return get_archive_name() def test_parses_chromium_tag_with_platform_asset(self): mock_response = MagicMock() @@ -438,7 +440,7 @@ def test_cached_binary_found(self, _mock_update, tmp_path): # Create a fake cached binary version = get_chromium_version() with patch("cloakbrowser.download.get_binary_path") as mock_path: - fake_binary = tmp_path / "chrome" + fake_binary = tmp_path / ("chrome.exe" if os.name == "nt" else "chrome") fake_binary.write_bytes(b"binary") fake_binary.chmod(0o755) mock_path.return_value = fake_binary