From 8d77e0be492f534531f04f021492c80495fd70bd Mon Sep 17 00:00:00 2001 From: Michael Lam Date: Wed, 6 May 2026 08:47:16 -0700 Subject: [PATCH] fix: isolate in-process cron scheduler profiles --- api/profiles.py | 83 +++++++++++++++++ .../test_scheduled_jobs_profile_isolation.py | 88 +++++++++++++++++++ 2 files changed, 171 insertions(+) diff --git a/api/profiles.py b/api/profiles.py index 52c552b2..fe0e1023 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -248,6 +248,82 @@ def get_active_hermes_home() -> Path: _cron_env_lock = threading.Lock() +def _cron_profile_context_depth() -> int: + return int(getattr(_tls, 'cron_profile_depth', 0) or 0) + + +def _push_cron_profile_context_depth() -> None: + _tls.cron_profile_depth = _cron_profile_context_depth() + 1 + + +def _pop_cron_profile_context_depth() -> None: + depth = _cron_profile_context_depth() + _tls.cron_profile_depth = max(0, depth - 1) + + +def _home_for_scheduled_cron_job(job: dict) -> Path: + """Resolve the profile home an auto-fired scheduler job should execute in. + + Legacy jobs with no profile keep the scheduler's server-default profile. + Jobs pinned to a named profile execute under that profile's HERMES_HOME, so + an in-process WebUI scheduler thread does not leak process-global config or + .env into the agent run. If a profile was deleted after the job was saved, + fall back to the server default rather than crashing every scheduler tick. + """ + raw = str((job or {}).get('profile') or '').strip() + if not raw: + return get_active_hermes_home() + if _is_root_profile(raw): + return _DEFAULT_HERMES_HOME + if not _PROFILE_ID_RE.fullmatch(raw): + logger.warning( + "Cron job %s has invalid profile %r; falling back to server default", + (job or {}).get('id', '?'), raw, + ) + return get_active_hermes_home() + home = _resolve_named_profile_home(raw) + if not home.is_dir(): + logger.warning( + "Cron job %s references missing profile %r; falling back to server default", + (job or {}).get('id', '?'), raw, + ) + return get_active_hermes_home() + return home + + +def install_cron_scheduler_profile_isolation() -> None: + """Patch cron.scheduler.run_job for WebUI in-process scheduler safety. + + Standard WebUI deployments do not start the scheduler thread in-process, but + if a future/single-process deployment calls cron.scheduler.tick() from the + WebUI worker, tick's background job path has no request TLS context. Wrap + run_job so each auto-fired job's persisted ``profile`` field gets the same + HERMES_HOME isolation as the manual /api/crons/run path. + """ + try: + import cron.scheduler as _cs + except ImportError: + logger.debug("install_cron_scheduler_profile_isolation: cron.scheduler unavailable") + return + + original = getattr(_cs, 'run_job', None) + if original is None or getattr(original, '_webui_profile_isolated', False): + return + + def _webui_profile_isolated_run_job(job, *args, **kwargs): + # Manual WebUI runs already enter cron_profile_context_for_home before + # calling run_job. Avoid nesting the non-reentrant env lock or changing + # the explicitly selected manual execution profile. + if _cron_profile_context_depth() > 0: + return original(job, *args, **kwargs) + with cron_profile_context_for_home(_home_for_scheduled_cron_job(job)): + return original(job, *args, **kwargs) + + _webui_profile_isolated_run_job._webui_profile_isolated = True + _webui_profile_isolated_run_job._webui_original_run_job = original + _cs.run_job = _webui_profile_isolated_run_job + + class cron_profile_context_for_home: """Context manager that pins HERMES_HOME to an explicit profile home path. @@ -261,6 +337,7 @@ class cron_profile_context_for_home: def __enter__(self): _cron_env_lock.acquire() + _push_cron_profile_context_depth() try: self._prev_env = os.environ.get('HERMES_HOME') os.environ['HERMES_HOME'] = str(self._home) @@ -296,6 +373,7 @@ class cron_profile_context_for_home: except (ImportError, AttributeError): logger.debug("cron_profile_context_for_home: cron.scheduler unavailable") except Exception: + _pop_cron_profile_context_depth() _cron_env_lock.release() raise return self @@ -319,6 +397,7 @@ class cron_profile_context_for_home: except (ImportError, AttributeError): pass finally: + _pop_cron_profile_context_depth() _cron_env_lock.release() return False @@ -337,6 +416,7 @@ class cron_profile_context: def __enter__(self): _cron_env_lock.acquire() + _push_cron_profile_context_depth() try: self._prev_env = os.environ.get('HERMES_HOME') home = get_active_hermes_home() @@ -371,6 +451,7 @@ class cron_profile_context: except (ImportError, AttributeError): logger.debug("cron_profile_context: cron.scheduler unavailable; env-var only") except Exception: + _pop_cron_profile_context_depth() _cron_env_lock.release() raise return self @@ -397,6 +478,7 @@ class cron_profile_context: except (ImportError, AttributeError): pass finally: + _pop_cron_profile_context_depth() _cron_env_lock.release() return False @@ -573,6 +655,7 @@ def init_profile_state() -> None: _active_profile = _read_active_profile_file() home = get_active_hermes_home() _set_hermes_home(home) + install_cron_scheduler_profile_isolation() _reload_dotenv(home) diff --git a/tests/test_scheduled_jobs_profile_isolation.py b/tests/test_scheduled_jobs_profile_isolation.py index 0d05cc4d..04f69224 100644 --- a/tests/test_scheduled_jobs_profile_isolation.py +++ b/tests/test_scheduled_jobs_profile_isolation.py @@ -199,6 +199,94 @@ def test_cron_run_does_not_silently_swallow_profile_resolution_errors(): ) +def test_webui_installs_profile_context_on_in_process_scheduler_run_job(tmp_path, monkeypatch): + """If WebUI ever runs cron.scheduler.tick in-process, scheduled run_job calls + must execute under the job's selected profile home, not the process-global + HERMES_HOME that happened to be active when the scheduler thread fired. + """ + import types + + from api import profiles as p + + default_home = tmp_path / "home" + research_home = default_home / "profiles" / "research" + research_home.mkdir(parents=True) + events = [] + + class Ctx: + def __init__(self, home): + self.home = str(home) + + def __enter__(self): + events.append(("enter", self.home)) + return self + + def __exit__(self, exc_type, exc, tb): + events.append(("exit", self.home)) + return False + + cron_pkg = types.ModuleType("cron") + cron_pkg.__path__ = [] + cron_scheduler = types.ModuleType("cron.scheduler") + cron_scheduler.run_job = lambda job: events.append(("run", job["id"])) or "ok" + + monkeypatch.setitem(sys.modules, "cron", cron_pkg) + monkeypatch.setitem(sys.modules, "cron.scheduler", cron_scheduler) + monkeypatch.setattr(p, "_DEFAULT_HERMES_HOME", default_home) + monkeypatch.setattr(p, "cron_profile_context_for_home", Ctx) + + p.install_cron_scheduler_profile_isolation() + + assert cron_scheduler.run_job({"id": "job1575", "profile": "research"}) == "ok" + assert events == [ + ("enter", str(research_home)), + ("run", "job1575"), + ("exit", str(research_home)), + ] + + +def test_scheduler_run_job_wrapper_does_not_reenter_manual_cron_context(tmp_path, monkeypatch): + """Manual /api/crons/run already pins run_job before calling it. + + The scheduler safety wrapper must detect that existing context and delegate + directly, otherwise the non-reentrant env lock would deadlock or override the + manual execution profile. + """ + import types + + from api import profiles as p + + events = [] + + class Ctx: + def __init__(self, home): + self.home = str(home) + + def __enter__(self): + events.append(("unexpected-enter", self.home)) + return self + + def __exit__(self, exc_type, exc, tb): + events.append(("unexpected-exit", self.home)) + return False + + cron_pkg = types.ModuleType("cron") + cron_pkg.__path__ = [] + cron_scheduler = types.ModuleType("cron.scheduler") + cron_scheduler.run_job = lambda job: events.append(("run", job["id"])) or "ok" + + monkeypatch.setitem(sys.modules, "cron", cron_pkg) + monkeypatch.setitem(sys.modules, "cron.scheduler", cron_scheduler) + monkeypatch.setattr(p, "_DEFAULT_HERMES_HOME", tmp_path / "home") + monkeypatch.setattr(p, "cron_profile_context_for_home", Ctx) + monkeypatch.setattr(p._tls, "cron_profile_depth", 1, raising=False) + + p.install_cron_scheduler_profile_isolation() + + assert cron_scheduler.run_job({"id": "manual1575", "profile": "research"}) == "ok" + assert events == [("run", "manual1575")] + + def test_cron_worker_does_not_silently_fall_back_on_profile_context_failure(): """_run_cron_tracked must NOT silently set ctx=None when cron_profile_context_for_home(...).__enter__() raises.