From 7807a12f8468b4d28653f9bd6e7a609c42875f32 Mon Sep 17 00:00:00 2001 From: Alexandre 'Kidev' Poumaroux <1204936+Kidev@users.noreply.github.com> Date: Tue, 7 Jan 2025 19:12:00 +0100 Subject: [PATCH] Try to use a static string for the installer binary --- aqt/installer.py | 197 ++++++++++++------------------------------ tests/test_install.py | 13 +-- 2 files changed, 57 insertions(+), 153 deletions(-) diff --git a/aqt/installer.py b/aqt/installer.py index b0672694..12ac0875 100644 --- a/aqt/installer.py +++ b/aqt/installer.py @@ -1451,18 +1451,10 @@ def download_bin(_base_url): class CommercialInstaller: - ALLOWED_INSTALLERS = frozenset( - {"qt-unified-windows-x64-online.exe", "qt-unified-macOS-x64-online.dmg", "qt-unified-linux-x64-online.run"} - ) - - ALLOWED_AUTO_ANSWER_OPTIONS = { - "OperationDoesNotExistError": frozenset({"Abort", "Ignore"}), - "OverwriteTargetDirectory": frozenset({"Yes", "No"}), - "stopProcessesForUpdates": frozenset({"Retry", "Ignore", "Cancel"}), - "installationErrorWithCancel": frozenset({"Retry", "Ignore", "Cancel"}), - "installationErrorWithIgnore": frozenset({"Retry", "Ignore"}), - "AssociateCommonFiletypes": frozenset({"Yes", "No"}), - "telemetry-question": frozenset({"Yes", "No"}), + ALLOWED_INSTALLERS = { + "windows": "qt-unified-windows-x64-online.exe", + "mac": "qt-unified-macOS-x64-online.dmg", + "linux": "qt-unified-linux-x64-online.run", } def __init__( @@ -1511,68 +1503,14 @@ def __init__( else: self.os_name = "windows" - self.installer_filename = self._get_installer_filename() - self.qt_account = self._get_qt_account_path() - - def _validate_installer_path(self, installer_path: Path) -> None: - """Validate the installer path is safe and exists""" - try: - # Resolve to absolute path and check for symlink attacks - real_path = installer_path.resolve(strict=True) - - if real_path.name not in self.ALLOWED_INSTALLERS: - raise ValueError(f"Invalid installer filename: {real_path.name}") - - # Additional file checks - if not real_path.is_file(): - raise ValueError("Path is not a regular file") - - # On Unix systems, check permissions - if hasattr(os, "access"): - if not os.access(real_path, os.X_OK): - raise ValueError("Installer file is not executable") - - except (RuntimeError, OSError) as e: - raise ValueError(f"Invalid installer path: {e}") - - @staticmethod - def _validate_dir_path(path: Optional[str]) -> None: - """Validate a directory path is safe""" - if path: - # Convert to Path object for validation - dir_path = Path(path) - # Ensure path is absolute and normalized - abs_path = dir_path.absolute().resolve() - if not abs_path.parts: # Empty path - raise ValueError("Empty path provided") - if ".." in abs_path.parts: # Path traversal attempt - raise ValueError("Path traversal detected") - - @staticmethod - def _validate_auto_answer_option(name: str, value: str, allowed_values: set[str]) -> None: - """Validate auto-answer option has an allowed value""" - if value not in allowed_values: - raise ValueError(f"Invalid value '{value}' for option {name}") - - def _get_installer_filename(self) -> str: - """Get OS-specific installer filename""" - base = "qt-unified" + self.installer_filename = self.ALLOWED_INSTALLERS[self.os_name] if self.os_name == "windows": - return f"{base}-windows-x64-online.exe" + self.qt_account = Path(os.environ["APPDATA"]) / "Qt" / "qtaccount.ini" elif self.os_name == "mac": - return f"{base}-macOS-x64-online.dmg" + self.qt_account = Path.home() / "Library" / "Application Support" / "Qt" / "qtaccount.ini" else: - return f"{base}-linux-x64-online.run" - - def _get_qt_account_path(self) -> Path: - """Get OS-specific qtaccount.ini path""" - if self.os_name == "windows": - return Path(os.environ["APPDATA"]) / "Qt" / "qtaccount.ini" - elif self.os_name == "mac": - return Path.home() / "Library" / "Application Support" / "Qt" / "qtaccount.ini" - else: - return Path.home() / ".local" / "share" / "Qt" / "qtaccount.ini" + self.qt_account = Path.home() / ".local" / "share" / "Qt" / "qtaccount.ini" def _download_installer(self, target_path: Path) -> None: """Download Qt online installer""" @@ -1610,20 +1548,12 @@ def _get_package_name(self) -> str: qt_version = f"{self.version.major}{self.version.minor}{self.version.patch}" return f"qt.qt{self.version.major}.{qt_version}.{self.arch}" - def _get_install_command(self, installer_path: Path) -> list[str]: + def _get_install_command(self) -> list[str]: """Build installation command with enhanced security validations""" - # Validate installer path first - self._validate_installer_path(installer_path) # Start with empty command list cmd: list[str] = [] - # Resolve and validate installer path - resolved_installer = str(installer_path.resolve(strict=True)) - if not resolved_installer: - raise ValueError("Empty installer path") - cmd.append(resolved_installer) - # Validate credentials if provided if self.username and self.password: if not isinstance(self.username, str) or not isinstance(self.password, str): @@ -1659,36 +1589,21 @@ def _get_install_command(self, installer_path: Path) -> list[str]: ("AssociateCommonFiletypes", self.associate_common_filetypes), ("telemetry-question", self.telemetry), ]: - # Check against allowed values - allowed_values = self.ALLOWED_AUTO_ANSWER_OPTIONS.get(option) - if not allowed_values or value not in allowed_values: - raise ValueError(f"Invalid value '{value}' for option {option}") auto_answers.append(f"{option}={value}") - # Add validated auto-answer string - auto_answer_string = ",".join(auto_answers) - if not auto_answer_string: - raise ValueError("Empty auto-answer string") - # Add standard arguments and package standard_args = [ "--accept-licenses", "--accept-obligations", "--confirm-command", "--auto-answer", - auto_answer_string, + ",".join(auto_answers), "install", self._get_package_name(), ] cmd.extend(standard_args) - # Final validation of complete command - if not all(isinstance(arg, str) for arg in cmd): - raise ValueError("Non-string arguments detected") - if not all(arg.strip() for arg in cmd): - raise ValueError("Empty arguments detected") - return cmd def install(self) -> None: @@ -1699,53 +1614,53 @@ def install(self) -> None: f"or ensure {self.qt_account} exists" ) - with tempfile.TemporaryDirectory(prefix="qt_install_") as temp_dir: - # Create temp dir with appropriate permissions - temp_path = Path(temp_dir) - if hasattr(os, "chmod"): - try: - os.chmod(temp_dir, 0o700) # Restrict to owner-only permissions - except OSError as e: - raise ValueError(f"Failed to set permissions on {temp_dir}: {e}") + # Create temp dir with appropriate permissions + temp_path = Path("/tmp/aqtinstall_qt_online_installer") + temp_path.mkdir(exist_ok=True) - installer_path = temp_path / self.installer_filename + installer_path = temp_path / self.installer_filename - self.logger.info(f"Downloading Qt online installer to {installer_path}") - self._download_installer(installer_path) + self.logger.info(f"Downloading Qt online installer to {installer_path}") + self._download_installer(installer_path) - self.logger.info("Starting Qt installation") + self.logger.info("Starting Qt installation") - try: - cmd = self._get_install_command(installer_path) - - if not isinstance(cmd, list) or not all(isinstance(arg, str) for arg in cmd): - raise ValueError("Command must be a list of strings") - - sanitized_cmd = [arg.strip() for arg in cmd if arg.strip()] - if any(arg.startswith(";") or arg.startswith("&") for arg in sanitized_cmd): - raise ValueError("Invalid command argument detected") - - # Log sanitized command (exclude password) - safe_cmd = list(sanitized_cmd) - if "--pw" in safe_cmd: - pw_index = safe_cmd.index("--pw") - if len(safe_cmd) > pw_index + 1: - safe_cmd[pw_index + 1] = "********" - if "--email" in safe_cmd: - email_index = safe_cmd.index("--email") - if len(safe_cmd) > email_index + 1: - safe_cmd[email_index + 1] = "********" - self.logger.info(f"Running: {' '.join(safe_cmd)}") - - subprocess.run(safe_cmd, shell=False, check=True, cwd=temp_dir) - except subprocess.CalledProcessError as e: - if e.stderr: - self.logger.error(f": {e.stderr.decode('utf-8', errors='replace')}") - self.logger.error(f"Installation failed with exit code {e.returncode}") - except subprocess.TimeoutExpired: - raise RuntimeError("Installation timed out") - finally: - if installer_path.exists(): - installer_path.unlink() - - self.logger.info("Qt installation completed successfully") + try: + cmd = self._get_install_command() + + # Log sanitized command (exclude password) + safe_cmd = list(cmd) + if "--pw" in safe_cmd: + pw_index = safe_cmd.index("--pw") + if len(safe_cmd) > pw_index + 1: + safe_cmd[pw_index + 1] = "********" + if "--email" in safe_cmd: + email_index = safe_cmd.index("--email") + if len(safe_cmd) > email_index + 1: + safe_cmd[email_index + 1] = "********" + self.logger.info(f"Running: {' '.join(safe_cmd)}") + + if self.os_name == "windows": + subprocess.run( + ["/tmp/aqtinstall_qt_online_installer/qt-unified-windows-x64-online.exe", *safe_cmd], shell=False + ) + elif self.os_name == "mac": + subprocess.run(["" + f"{temp_path}/qt-unified-macOS-x64-online.dmg", *safe_cmd], shell=False) + elif self.os_name == "linux": + subprocess.run( + ["/tmp/aqtinstall_qt_online_installer/qt-unified-linux-x64-online.run", *safe_cmd], shell=False + ) + else: + raise RuntimeError("Unsupported OS") + + except subprocess.CalledProcessError as e: + if e.stderr: + self.logger.error(f": {e.stderr.decode('utf-8', errors='replace')}") + self.logger.error(f"Installation failed with exit code {e.returncode}") + except subprocess.TimeoutExpired: + raise RuntimeError("Installation timed out") + finally: + if installer_path.exists(): + installer_path.unlink() + + self.logger.info("Qt installation completed successfully") diff --git a/tests/test_install.py b/tests/test_install.py index 17869b49..80f157f7 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -2060,9 +2060,7 @@ def mock_get_url(url: str, *args, **kwargs) -> str: "cmd, arch_dict, details, expected_command", [ ( - "install-qt-commercial desktop {} 6.8.0 " - "--outputdir ./install-qt-commercial " - "--user {} --password {}", + "install-qt-commercial desktop {} 6.8.0 " "--outputdir ./install-qt-commercial " "--user {} --password {}", {"windows": "win64_msvc2022_64", "linux": "linux_gcc_64", "mac": "clang_64"}, ["./install-qt-commercial", "qt6", "681"], "qt-unified-{}-online.run --root {} --accept-licenses --accept-obligations --confirm-command " @@ -2085,15 +2083,6 @@ def test_install_qt_commercial( cli = Cli() cli._setup_settings() - def mock_exists(*args, **kwargs): - return False - - def mock_subprocess(*args, **kwargs): - return 0 - - monkeypatch.setattr(Path, "exists", mock_exists) - monkeypatch.setattr("subprocess.check_call", mock_subprocess) - cli.run(formatted_cmd.split()) [out, _] = capsys.readouterr()