fix(desktop): configure Linux Electron sandbox helper
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).
This commit is contained in:
@@ -204,6 +204,7 @@ import argparse
|
|||||||
import hashlib
|
import hashlib
|
||||||
import json
|
import json
|
||||||
import shutil
|
import shutil
|
||||||
|
import stat
|
||||||
import subprocess
|
import subprocess
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Optional
|
from typing import Optional
|
||||||
@@ -7113,6 +7114,45 @@ def _desktop_macos_relaunchable_fixup(desktop_dir: Path) -> None:
|
|||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
print(f" (warning: macOS relaunch fixup skipped: {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):
|
def cmd_gui(args: argparse.Namespace):
|
||||||
"""Build and launch the native Electron desktop GUI."""
|
"""Build and launch the native Electron desktop GUI."""
|
||||||
desktop_dir = PROJECT_ROOT / "apps" / "desktop"
|
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.")
|
print(" Expected an unpacked Electron app for the current OS.")
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
|
if not _desktop_linux_sandbox_fixup(packaged_executable):
|
||||||
|
sys.exit(1)
|
||||||
|
|
||||||
print(f"→ Launching packaged Hermes Desktop: {packaged_executable}")
|
print(f"→ Launching packaged Hermes Desktop: {packaged_executable}")
|
||||||
launch_result = subprocess.run([str(packaged_executable)], cwd=desktop_dir, env=env, check=False)
|
launch_result = subprocess.run([str(packaged_executable)], cwd=desktop_dir, env=env, check=False)
|
||||||
sys.exit(launch_result.returncode)
|
sys.exit(launch_result.returncode)
|
||||||
|
|||||||
+44
-12
@@ -2222,10 +2222,10 @@ postinstall_mode() {
|
|||||||
fi
|
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
|
# Install-Desktop: a root-level npm install so the apps/* workspace resolves
|
||||||
# the desktop's own deps (Electron ~150MB), then `npm run pack`
|
# 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
|
# via the 'desktop' stage / --include-desktop, which the Electron app's own
|
||||||
# first-launch bootstrap never requests (it must not rebuild itself).
|
# first-launch bootstrap never requests (it must not rebuild itself).
|
||||||
install_desktop() {
|
install_desktop() {
|
||||||
@@ -2261,7 +2261,7 @@ install_desktop() {
|
|||||||
log_success "Desktop workspace dependencies installed"
|
log_success "Desktop workspace dependencies installed"
|
||||||
|
|
||||||
# 2. Build. `npm run pack` = tsc + vite build + electron-builder --dir,
|
# 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
|
# auto-discovery so electron-builder falls back to an ad-hoc signature
|
||||||
# instead of grabbing an unrelated Developer ID from the keychain; a
|
# instead of grabbing an unrelated Developer ID from the keychain; a
|
||||||
# real signed/notarized .dmg needs Apple credentials and is a separate
|
# real signed/notarized .dmg needs Apple credentials and is a separate
|
||||||
@@ -2274,21 +2274,53 @@ install_desktop() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
local app=""
|
local app=""
|
||||||
local cand
|
if [ "$OS" = "linux" ]; then
|
||||||
for cand in \
|
if [ -x "$desktop_dir/release/linux-unpacked/Hermes" ]; then
|
||||||
"$desktop_dir/release/mac-arm64/Hermes.app" \
|
app="$desktop_dir/release/linux-unpacked/Hermes"
|
||||||
"$desktop_dir/release/mac/Hermes.app"; do
|
elif [ -x "$desktop_dir/release/linux-unpacked/hermes" ]; then
|
||||||
if [ -d "$cand" ]; then
|
app="$desktop_dir/release/linux-unpacked/hermes"
|
||||||
app="$cand"
|
|
||||||
break
|
|
||||||
fi
|
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
|
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
|
return 1
|
||||||
fi
|
fi
|
||||||
log_success "Desktop app built: $app"
|
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
|
# 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
|
# self-update. An ad-hoc bundle has no stable Designated Requirement, so a
|
||||||
# later in-place rebuild (new cdhash) plus the inherited quarantine flag
|
# later in-place rebuild (new cdhash) plus the inherited quarantine flag
|
||||||
|
|||||||
@@ -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)]
|
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):
|
def test_gui_source_mode_uses_renderer_build_and_electron(tmp_path, monkeypatch):
|
||||||
root = _make_desktop_tree(tmp_path)
|
root = _make_desktop_tree(tmp_path)
|
||||||
desktop_dir = root / "apps" / "desktop"
|
desktop_dir = root / "apps" / "desktop"
|
||||||
|
|||||||
Reference in New Issue
Block a user