fix(security): the first tirith scan no longer waits on its download (#128150)
* fix(security): the first tirith scan no longer waits on its download
A process that never ran the CLI startup install (desktop serve, the
gateway, a newly routed profile) claimed the install on its first scan and
ran pm.ensure("tirith") inline. The first terminal call then waited on the
PM worker, the runtime toolchain and the tirith download before it ran.
The cold scan now starts the same background install as startup and scans
with the default path, so the call fails open at once like any scan during
a startup download.
* fix(security): a scan during the tirith download never trips the breaker
The previous commit let a cold scan run while the background install was
still downloading. That scan spawned the bare default "tirith", got
FileNotFoundError and counted it as a crash. Three terminal calls during
the download opened the circuit breaker, and every scan then returned
allow unscanned for 300 s, even after the download finished. With
tirith_fail_open: false the first three commands were blocked and the
breaker then let commands run unscanned.
A scan that finds the default binary missing while this home's install
thread is alive no longer spawns or counts a crash. Fail-open allows the
command at once ("tirith installing"). Fail-closed waits for the install,
as the inline install did before, and then scans with the real binary.
So only the fail-open case skips the scan during a download; the earlier
commit's "fails open at once" holds only for fail_open.
The PM consumer test now joins the background install before it reads
the recorded download requests.
This commit is contained in:
@@ -314,6 +314,8 @@ def test_tirith_failed_cold_scans_make_one_attempt_then_explicit_can_retry(consu
|
||||
real_get(handler)
|
||||
monkeypatch.setattr(RangeHandler, "do_GET", record)
|
||||
tirith.check_command_security("echo hello")
|
||||
for thread in tirith._install_threads.values():
|
||||
thread.join(10)
|
||||
first_attempt = list(requests)
|
||||
assert first_attempt
|
||||
tirith.check_command_security("echo hello")
|
||||
|
||||
@@ -348,8 +348,11 @@ class TestPmInstall:
|
||||
ensure.side_effect = lambda *_a, **_k: setattr(
|
||||
installed, "return_value", MagicMock(binary="/pm/tirith"))
|
||||
|
||||
assert _tirith_mod._resolve_tirith_path("tirith") == "/pm/tirith"
|
||||
assert _tirith_mod._resolve_tirith_path("tirith") == "tirith"
|
||||
for thread in _tirith_mod._install_threads.values():
|
||||
thread.join(5)
|
||||
ensure.assert_called_once_with("tirith")
|
||||
assert _tirith_mod._resolve_tirith_path("tirith") == "/pm/tirith"
|
||||
|
||||
def test_failed_install_is_not_retried(self, pm_tirith):
|
||||
"""After a failed install, subsequent resolves fall back without retrying."""
|
||||
@@ -357,6 +360,8 @@ class TestPmInstall:
|
||||
ensure.side_effect = RuntimeError("download failed")
|
||||
|
||||
assert _tirith_mod._resolve_tirith_path("tirith") == "tirith"
|
||||
for thread in _tirith_mod._install_threads.values():
|
||||
thread.join(5)
|
||||
assert _tirith_mod._resolve_tirith_path("tirith") == "tirith"
|
||||
assert ensure.call_count == 1
|
||||
|
||||
|
||||
@@ -64,7 +64,7 @@ _circuit_open_at: float = 0.0
|
||||
_breaker_lock = threading.Lock()
|
||||
|
||||
# Warn-once: spawn/path warnings sit in the hot path and would otherwise repeat once per
|
||||
# terminal command while tirith is unavailable (e.g. install thread still running).
|
||||
# terminal command while tirith is unavailable.
|
||||
_warned_messages: set[str] = set()
|
||||
_warned_lock = threading.Lock()
|
||||
|
||||
@@ -144,6 +144,11 @@ def _claim_install_attempt() -> bool:
|
||||
return True
|
||||
|
||||
|
||||
def _install_in_flight() -> threading.Thread | None:
|
||||
thread = _install_threads.get(hermes_home_key())
|
||||
return thread if thread is not None and thread.is_alive() else None
|
||||
|
||||
|
||||
def is_platform_supported() -> bool:
|
||||
"""Whether PM has a managed Tirith build for this host."""
|
||||
import pm
|
||||
@@ -174,15 +179,8 @@ def _resolve_tirith_path(configured_path: str) -> str:
|
||||
if configured_path == "tirith":
|
||||
import pm
|
||||
|
||||
if not pm.lazy_installs_allowed() or not _claim_install_attempt():
|
||||
return os.path.expanduser(configured_path)
|
||||
try:
|
||||
pm.ensure("tirith")
|
||||
selected = pm.installed_package("tirith")
|
||||
if selected and selected.binary:
|
||||
return str(selected.binary)
|
||||
except Exception as exc:
|
||||
_warn_once("tirith_install", "tirith install unavailable: %s", exc)
|
||||
if pm.lazy_installs_allowed():
|
||||
_start_background_install(log_failures=True)
|
||||
return os.path.expanduser(configured_path)
|
||||
|
||||
|
||||
@@ -196,6 +194,17 @@ def _background_install(*, log_failures: bool) -> None:
|
||||
log("tirith install failed: %s", exc)
|
||||
|
||||
|
||||
def _start_background_install(*, log_failures: bool) -> None:
|
||||
if _claim_install_attempt():
|
||||
context = copy_context()
|
||||
thread = threading.Thread(
|
||||
target=context.run, args=(_background_install,),
|
||||
kwargs={"log_failures": log_failures}, daemon=True,
|
||||
)
|
||||
_install_threads[hermes_home_key()] = thread
|
||||
thread.start()
|
||||
|
||||
|
||||
def ensure_installed(*, log_failures: bool = True, explicit: bool = False):
|
||||
"""Opt-in startup is non-blocking. Explicit setup waits and reports errors.
|
||||
|
||||
@@ -218,14 +227,7 @@ def ensure_installed(*, log_failures: bool = True, explicit: bool = False):
|
||||
return found
|
||||
if not is_platform_supported() or not pm.lazy_installs_allowed():
|
||||
return None
|
||||
if _claim_install_attempt():
|
||||
context = copy_context()
|
||||
thread = threading.Thread(
|
||||
target=context.run, args=(_background_install,),
|
||||
kwargs={"log_failures": log_failures}, daemon=True,
|
||||
)
|
||||
_install_threads[hermes_home_key()] = thread
|
||||
thread.start()
|
||||
_start_background_install(log_failures=log_failures)
|
||||
return None
|
||||
|
||||
|
||||
@@ -241,8 +243,7 @@ def missing_is_expected() -> bool:
|
||||
configured = _load_security_config()["tirith_path"]
|
||||
if configured != "tirith":
|
||||
return False
|
||||
thread = _install_threads.get(hermes_home_key())
|
||||
if thread is not None and thread.is_alive():
|
||||
if _install_in_flight():
|
||||
return True
|
||||
return _local_tirith(configured) is not None or not pm.lazy_installs_allowed()
|
||||
|
||||
@@ -310,6 +311,11 @@ def check_command_security(command: str) -> dict:
|
||||
if tirith_path is None:
|
||||
_warn_once("tirith_path_none", "tirith path resolved to None; scanning disabled")
|
||||
return _fail(fail_open, "tirith path unavailable", "tirith path unavailable (fail-closed)")
|
||||
if tirith_path == "tirith" and (install := _install_in_flight()):
|
||||
if fail_open:
|
||||
return _verdict("allow", "tirith installing")
|
||||
install.join()
|
||||
tirith_path = _resolve_tirith_path(cfg["tirith_path"])
|
||||
try:
|
||||
result = subprocess.run(
|
||||
[tirith_path, "check", "--json", "--non-interactive", "--shell", "posix", "--", command],
|
||||
|
||||
Reference in New Issue
Block a user