diff --git a/scripts/desktop-update/posix.sh b/scripts/desktop-update/posix.sh index f9182787e7..b183521a68 100755 --- a/scripts/desktop-update/posix.sh +++ b/scripts/desktop-update/posix.sh @@ -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 & diff --git a/tests/test_desktop_update_shim_profile_cleanup.py b/tests/test_desktop_update_shim_profile_cleanup.py index b4de7ff2a5..2c5b93e43d 100644 --- a/tests/test_desktop_update_shim_profile_cleanup.py +++ b/tests/test_desktop_update_shim_profile_cleanup.py @@ -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"