fix(desktop-update): atomically claim the temporary browser profile

Use mktemp -d before launching the optional UI; skip UI if allocation fails. Native Chrome collision and allocation-failure probes preserve preexisting directories.
This commit is contained in:
Teknium
2026-09-07 02:40:55 -07:00
parent b3778dbfd6
commit ca812ba3b5
2 changed files with 30 additions and 15 deletions
+4 -1
View File
@@ -245,6 +245,10 @@ start_ui() {
fi
{ [ -f "$html" ] && [ -n "$py" ] && [ -n "$browser" ]; } || { log "shim: no renderer; skipping UI"; return; }
UI_PROFILE_DIR="$(mktemp -d "${TMPDIR:-/tmp}/hermes-update-ui-XXXXXXXX")" || {
log "shim: could not allocate a browser profile; skipping UI"
return
}
publish_stage ""
# The Desktop's final teardown targets the updater process group. Put both
# UI processes in their own sessions so neither the HTTP server nor a Chrome
@@ -267,7 +271,6 @@ start_ui() {
[ -n "$port" ] || { kill -9 "$UI_SERVER_PID" 2>/dev/null; UI_SERVER_PID=""; return; }
# Throwaway profile: new window/process we own; user's browser untouched.
UI_PROFILE_DIR="${TMPDIR:-/tmp}/hermes-update-ui-$$"
"$py" -c 'import os, signal, sys; os.setsid(); signal.signal(signal.SIGTERM, signal.SIG_DFL); os.execv(sys.argv[1], sys.argv[1:])' \
"$browser" --app="http://127.0.0.1:$port/" --user-data-dir="$UI_PROFILE_DIR" \
--no-first-run --no-default-browser-check --window-size=280,320 >/dev/null 2>&1 &
@@ -1,4 +1,4 @@
"""The update hand-off cleans only the browser profile it launched."""
"""The update hand-off cleans only an atomically claimed browser profile."""
import os
from pathlib import Path
import subprocess
@@ -7,8 +7,8 @@ import pytest
@pytest.mark.linux_only
@pytest.mark.parametrize("failed", [False, True])
def test_shim_removes_owned_profile_on_exit(tmp_path, failed):
@pytest.mark.parametrize("outcome", ["success", "error", "allocation-failed"])
def test_shim_removes_only_its_owned_profile(tmp_path, outcome):
bin_dir = tmp_path / "bin"
bin_dir.mkdir()
browser = bin_dir / "google-chrome"
@@ -20,6 +20,10 @@ def test_shim_removes_owned_profile_on_exit(tmp_path, failed):
'while :; do sleep 0.1; done\n', encoding="utf-8",
)
browser.chmod(0o755)
if outcome == "allocation-failed":
allocator = bin_dir / "mktemp"
allocator.write_text("#!/bin/sh\nexit 1\n", encoding="utf-8")
allocator.chmod(0o755)
config = tmp_path / ".config"
config.mkdir()
(config / "mimeapps.list").write_text(
@@ -27,21 +31,29 @@ def test_shim_removes_owned_profile_on_exit(tmp_path, failed):
'x-scheme-handler/https=google-chrome.desktop\ntext/html=google-chrome.desktop\n',
encoding="utf-8",
)
sibling = tmp_path / "hermes-update-ui-unrelated"
sibling.mkdir()
(sibling / "keep").write_text("keep", encoding="utf-8")
install = tmp_path / "hermes-agent"
install.mkdir()
env = {**os.environ, "HOME": str(tmp_path), "HERMES_HOME": str(tmp_path),
"XDG_CONFIG_HOME": str(config), "TMPDIR": str(tmp_path),
"PATH": f"{bin_dir}:/usr/bin:/bin", "HERMES_SELFTEST_HOLD_SECONDS": "1",
"HERMES_UPDATE_SHIM_GRACE_SECONDS": "1", "HERMES_SELFTEST_FAIL": "1" if failed else ""}
result = subprocess.run(
["bash", str(Path(__file__).resolve().parents[1] / "scripts/desktop-update/posix.sh"),
"HERMES_UPDATE_SHIM_GRACE_SECONDS": "1", "HERMES_SELFTEST_FAIL": "1" if outcome == "error" else ""}
process = subprocess.Popen(
["bash", "-c", 'while [ ! -f "$HOME/start" ]; do sleep .05; done; exec bash "$@"', "probe",
str(Path(__file__).resolve().parents[1] / "scripts/desktop-update/posix.sh"),
"--install-root", str(install), "--self-test-ui"],
env=env, capture_output=True, text=True, timeout=30,
env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
)
assert result.returncode == (1 if failed else 0), result.stdout + result.stderr
owned = Path((tmp_path / "launched-profile").read_text(encoding="utf-8"))
assert not owned.exists()
assert (sibling / "keep").read_text(encoding="utf-8") == "keep"
collision = tmp_path / f"hermes-update-ui-{process.pid}"
collision.mkdir()
(collision / "keep").write_text("keep", encoding="utf-8")
(tmp_path / "start").touch()
stdout, stderr = process.communicate(timeout=30)
assert process.returncode == (1 if outcome == "error" else 0), stdout + stderr
launched = tmp_path / "launched-profile"
if outcome == "allocation-failed":
assert not launched.exists()
else:
owned = Path(launched.read_text(encoding="utf-8"))
assert owned != collision
assert not owned.exists()
assert (collision / "keep").read_text(encoding="utf-8") == "keep"