From 46e513ef518587bfb4ddc610cef7d64bd5337f8d Mon Sep 17 00:00:00 2001 From: ethernet Date: Tue, 2 Jun 2026 18:18:28 -0400 Subject: [PATCH] fix(desktop): configure Linux Electron sandbox helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Electron's chrome-sandbox helper must be root:root 4755 on Linux or the sandboxed renderer aborts before the desktop app starts. The existing installer only searched for macOS .app bundles, so a successful Linux build was reported as missing. Changes: - Add _desktop_linux_sandbox_fixup() to hermes_cli/main.py, called before launching a packaged desktop app on Linux. - Use lstat() + S_ISREG check to reject symlinks — chown/chmod on a symlink target would set SUID on an arbitrary path. - Update install.sh to recognize Linux unpacked artifacts and configure chrome-sandbox with proper error handling (the original PR silently ignored chown/chmod failures). - Add regression tests: normal fixup flow, symlink rejection, and already-configured skip path. Closes #37529 (rebased, merge conflicts resolved, copilot review feedback addressed). --- hermes_cli/main.py | 43 +++++++++++++++++ scripts/install.sh | 56 +++++++++++++++++----- tests/hermes_cli/test_gui_command.py | 69 ++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 12 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index e752dff57..53f066cb9 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -204,6 +204,7 @@ import argparse import hashlib import json import shutil +import stat import subprocess from pathlib import Path from typing import Optional @@ -7113,6 +7114,45 @@ def _desktop_macos_relaunchable_fixup(desktop_dir: Path) -> None: except Exception as exc: print(f" (warning: macOS relaunch fixup skipped: {exc})") + +def _desktop_linux_sandbox_fixup(packaged_executable: Path) -> bool: + """Configure Electron's Linux SUID sandbox helper when required.""" + if sys.platform != "linux": + return True + + sandbox = packaged_executable.parent / "chrome-sandbox" + if not sandbox.exists(): + print(f"✗ Hermes Desktop is missing Electron's Linux sandbox helper: {sandbox}") + return False + + # Reject symlinks — chown/chmod must not follow an attacker-controlled + # link to an arbitrary path. Use lstat() so we inspect the link itself + # rather than the target, and require a regular file. + try: + sandbox_lstat = sandbox.lstat() + except OSError: + print(f"✗ Cannot stat Electron's Linux sandbox helper: {sandbox}") + return False + if not stat.S_ISREG(sandbox_lstat.st_mode): + print(f"✗ Electron's Linux sandbox helper is not a regular file: {sandbox}") + return False + + if sandbox_lstat.st_uid == 0 and stat.S_IMODE(sandbox_lstat.st_mode) == 0o4755: + return True + + sudo = shutil.which("sudo") + if not sudo: + print("✗ Hermes Desktop requires sudo to configure Electron's Linux sandbox helper.") + return False + + print("→ Configuring Electron Linux sandbox helper (sudo required)...") + for command in ([sudo, "chown", "root:root", str(sandbox)], [sudo, "chmod", "4755", str(sandbox)]): + if subprocess.run(command, check=False).returncode != 0: + print(f"✗ Failed to configure Electron's Linux sandbox helper: {sandbox}") + return False + return True + + def cmd_gui(args: argparse.Namespace): """Build and launch the native Electron desktop GUI.""" desktop_dir = PROJECT_ROOT / "apps" / "desktop" @@ -7238,6 +7278,9 @@ def cmd_gui(args: argparse.Namespace): print(" Expected an unpacked Electron app for the current OS.") sys.exit(1) + if not _desktop_linux_sandbox_fixup(packaged_executable): + sys.exit(1) + print(f"→ Launching packaged Hermes Desktop: {packaged_executable}") launch_result = subprocess.run([str(packaged_executable)], cwd=desktop_dir, env=env, check=False) sys.exit(launch_result.returncode) diff --git a/scripts/install.sh b/scripts/install.sh index df9b259d0..b62d87d46 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -2222,10 +2222,10 @@ postinstall_mode() { fi } -# Build apps/desktop into a launchable Hermes.app. Mirrors install.ps1's +# Build apps/desktop into a launchable native app. Mirrors install.ps1's # Install-Desktop: a root-level npm install so the apps/* workspace resolves # the desktop's own deps (Electron ~150MB), then `npm run pack` -# (electron-builder --dir) which emits release/mac*/Hermes.app. Only invoked +# (electron-builder --dir) which emits an unpacked app for the current OS. Only invoked # via the 'desktop' stage / --include-desktop, which the Electron app's own # first-launch bootstrap never requests (it must not rebuild itself). install_desktop() { @@ -2261,7 +2261,7 @@ install_desktop() { log_success "Desktop workspace dependencies installed" # 2. Build. `npm run pack` = tsc + vite build + electron-builder --dir, - # producing an unpacked release/mac*/Hermes.app. We disable signing + # producing an unpacked app for the current OS. We disable signing # auto-discovery so electron-builder falls back to an ad-hoc signature # instead of grabbing an unrelated Developer ID from the keychain; a # real signed/notarized .dmg needs Apple credentials and is a separate @@ -2274,21 +2274,53 @@ install_desktop() { } local app="" - local cand - for cand in \ - "$desktop_dir/release/mac-arm64/Hermes.app" \ - "$desktop_dir/release/mac/Hermes.app"; do - if [ -d "$cand" ]; then - app="$cand" - break + if [ "$OS" = "linux" ]; then + if [ -x "$desktop_dir/release/linux-unpacked/Hermes" ]; then + app="$desktop_dir/release/linux-unpacked/Hermes" + elif [ -x "$desktop_dir/release/linux-unpacked/hermes" ]; then + app="$desktop_dir/release/linux-unpacked/hermes" fi - done + else + local cand + for cand in \ + "$desktop_dir/release/mac-arm64/Hermes.app" \ + "$desktop_dir/release/mac/Hermes.app"; do + if [ -d "$cand" ]; then + app="$cand" + break + fi + done + fi if [ -z "$app" ]; then - log_error "Desktop build completed but no Hermes.app was found under $desktop_dir/release/" + log_error "Desktop build completed but no app was found under $desktop_dir/release/" return 1 fi log_success "Desktop app built: $app" + # Linux: Electron's chrome-sandbox helper needs root:root 4755 or the + # sandboxed renderer will abort on startup. Check the file is a regular + # file (not a symlink) before chown/chmod so we don't follow an + # attacker-controlled link to an arbitrary path. + if [ "$OS" = "linux" ]; then + local sandbox="$desktop_dir/release/linux-unpacked/chrome-sandbox" + if [ -f "$sandbox" ] && [ ! -L "$sandbox" ]; then + if [ "$(id -u)" -eq 0 ]; then + chown root:root "$sandbox" && chmod 4755 "$sandbox" || { + log_error "Cannot configure Electron sandbox helper: $sandbox" + return 1 + } + elif command -v sudo >/dev/null 2>&1; then + sudo chown root:root "$sandbox" && sudo chmod 4755 "$sandbox" || { + log_error "Cannot configure Electron sandbox helper (sudo failed): $sandbox" + return 1 + } + else + log_error "Cannot configure Electron sandbox helper without sudo: $sandbox" + return 1 + fi + fi + fi + # macOS: make the locally-built (ad-hoc) app relaunchable after an in-place # self-update. An ad-hoc bundle has no stable Designated Requirement, so a # later in-place rebuild (new cdhash) plus the inherited quarantine flag diff --git a/tests/hermes_cli/test_gui_command.py b/tests/hermes_cli/test_gui_command.py index 6954f4b40..fb3b72b60 100644 --- a/tests/hermes_cli/test_gui_command.py +++ b/tests/hermes_cli/test_gui_command.py @@ -153,6 +153,75 @@ def test_gui_skip_build_launches_existing_packaged_app_without_npm(tmp_path, mon assert mock_run.call_args.args[0] == [str(packaged_exe)] +def test_gui_linux_configures_sandbox_before_launch(tmp_path, monkeypatch): + root = _make_desktop_tree(tmp_path) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) + packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux") + sandbox = packaged_exe.parent / "chrome-sandbox" + sandbox.write_text("", encoding="utf-8") + sandbox.chmod(0o755) + ok = subprocess.CompletedProcess([], 0) + + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/sudo"), \ + patch("hermes_cli.main.subprocess.run", return_value=ok) as mock_run, \ + pytest.raises(SystemExit) as exc: + cli_main.cmd_gui(_ns(skip_build=True)) + + assert exc.value.code == 0 + assert mock_run.call_args_list[0].args[0] == ["/usr/bin/sudo", "chown", "root:root", str(sandbox)] + assert mock_run.call_args_list[1].args[0] == ["/usr/bin/sudo", "chmod", "4755", str(sandbox)] + assert mock_run.call_args_list[2].args[0] == [str(packaged_exe)] + + +def test_gui_linux_rejects_symlink_sandbox(tmp_path, monkeypatch): + root = _make_desktop_tree(tmp_path) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) + packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux") + # Point chrome-sandbox at an unrelated file via symlink + target = tmp_path / "dangerous" + target.write_text("pwned", encoding="utf-8") + sandbox = packaged_exe.parent / "chrome-sandbox" + sandbox.symlink_to(target) + + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/sudo"), \ + patch("hermes_cli.main.subprocess.run") as mock_run, \ + pytest.raises(SystemExit) as exc: + cli_main.cmd_gui(_ns(skip_build=True)) + + assert exc.value.code == 1 + # Must NOT have called sudo chown/chmod on the symlink target + for call in mock_run.call_args_list: + assert "chown" not in call.args[0] + assert "chmod" not in call.args[0] + + +def test_gui_linux_skips_fixup_when_already_configured(tmp_path, monkeypatch): + root = _make_desktop_tree(tmp_path) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) + packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux") + sandbox = packaged_exe.parent / "chrome-sandbox" + sandbox.write_text("", encoding="utf-8") + # Simulate root-owned 4755 — lstat().st_uid==0 and mode==0o4755 + # We can't actually chown to root in tests, so mock lstat to return + # the expected values directly. + import stat as stat_mod + fake_stat = type("s", (), {"st_uid": 0, "st_mode": 0o4755 | stat_mod.S_IFREG})() + sandbox_lstat_orig = type(sandbox).lstat + monkeypatch.setattr(type(sandbox), "lstat", lambda self: fake_stat) + + launch_ok = subprocess.CompletedProcess([str(packaged_exe)], 0) + + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/sudo"), \ + patch("hermes_cli.main.subprocess.run", return_value=launch_ok) as mock_run, \ + pytest.raises(SystemExit) as exc: + cli_main.cmd_gui(_ns(skip_build=True)) + + assert exc.value.code == 0 + # Only the launch call — no sudo chown/chmod + mock_run.assert_called_once() + assert mock_run.call_args.args[0] == [str(packaged_exe)] + + def test_gui_source_mode_uses_renderer_build_and_electron(tmp_path, monkeypatch): root = _make_desktop_tree(tmp_path) desktop_dir = root / "apps" / "desktop"