fix(cron): bind profile secret scope in the delivery-targets route
GET /api/cron/delivery-targets ran cron_delivery_targets() outside any profile secret scope. Once the dashboard/desktop `serve` backend hosts a second profile home (it flips multiplex active on the first `?profile=` request), agent.secret_scope.get_secret fails closed, so every poll raised UnscopedSecretError — caught, logged as an error, and the response silently lost every configured platform, leaving only the implicit `local` entry. Run the read inside _config_profile_scope, matching the sibling cron routes, and thread an optional `profile` query param through this route and GET /api/cron/blueprints, which shares the same call. Add a regression test that reproduces the fail-closed read (red on base, green with the scope).
This commit is contained in:
@@ -34,6 +34,7 @@ load_config = late("load_config", "hermes_cli.config")
|
||||
_cron_profile_dicts = late("_cron_profile_dicts", "hermes_cli.web_server_cron")
|
||||
_cron_profile_home = late("_cron_profile_home", "hermes_cli.web_server_cron")
|
||||
_open_session_db_for_profile = late("_open_session_db_for_profile", "hermes_cli.web_server_sessions")
|
||||
_config_profile_scope = late("_config_profile_scope", "hermes_cli.web_server_profiles")
|
||||
|
||||
def _job_not_found() -> HTTPException:
|
||||
return HTTPException(status_code=404, detail="Job not found")
|
||||
@@ -250,15 +251,24 @@ async def create_cron_job(body: CronJobCreate, profile: Optional[str] = None):
|
||||
|
||||
|
||||
@router.get("/api/cron/delivery-targets")
|
||||
async def get_cron_delivery_targets():
|
||||
async def get_cron_delivery_targets(profile: Optional[str] = None):
|
||||
"""Delivery targets for the cron dropdown: implicit ``local`` plus the
|
||||
configured gateway platforms (a platform without a cron home channel is
|
||||
still listed with ``home_target_set: false`` so the UI can say so)."""
|
||||
still listed with ``home_target_set: false`` so the UI can say so).
|
||||
|
||||
``cron_delivery_targets()`` reads each platform's home channel through
|
||||
``get_secret``, which fails closed once this process hosts more than one
|
||||
profile home (the dashboard/desktop ``serve`` backend flips multi-profile
|
||||
hosting on the first ``?profile=`` request). The read must therefore run
|
||||
inside the profile scope, exactly like the sibling cron routes — otherwise
|
||||
the poll raises ``UnscopedSecretError`` on every tick and the dropdown
|
||||
silently loses every configured platform."""
|
||||
targets = [{"id": "local", "name": "Local (save only)", "home_target_set": True, "home_env_var": None}]
|
||||
try:
|
||||
from cron.scheduler_delivery import cron_delivery_targets
|
||||
|
||||
targets.extend(cron_delivery_targets())
|
||||
with _config_profile_scope(profile):
|
||||
targets.extend(cron_delivery_targets())
|
||||
except Exception:
|
||||
_log.exception("GET /api/cron/delivery-targets failed")
|
||||
return {"targets": targets}
|
||||
@@ -381,7 +391,7 @@ async def cron_fire_webhook(request: Request):
|
||||
|
||||
|
||||
@router.get("/api/cron/blueprints")
|
||||
async def list_cron_blueprints():
|
||||
async def list_cron_blueprints(profile: Optional[str] = None):
|
||||
"""Blueprint catalog as form schemas; the ``deliver`` slot's options are
|
||||
rewritten from the actually configured gateway platforms."""
|
||||
try:
|
||||
@@ -391,7 +401,8 @@ async def list_cron_blueprints():
|
||||
try:
|
||||
from cron.scheduler_delivery import cron_delivery_targets
|
||||
|
||||
platforms = [t["id"] for t in cron_delivery_targets() if t.get("id")]
|
||||
with _config_profile_scope(profile):
|
||||
platforms = [t["id"] for t in cron_delivery_targets() if t.get("id")]
|
||||
deliver_options = ["origin", "local", *platforms]
|
||||
except Exception:
|
||||
_log.debug("cron_delivery_targets unavailable; using static deliver options", exc_info=True)
|
||||
|
||||
50
tests/hermes_cli/test_cron_delivery_targets_scope.py
Normal file
50
tests/hermes_cli/test_cron_delivery_targets_scope.py
Normal file
@@ -0,0 +1,50 @@
|
||||
"""Regression: the delivery-targets route read a profile secret unscoped.
|
||||
|
||||
The dashboard/desktop ``serve`` backend flips multi-profile hosting
|
||||
(``tui_gateway.launch_profile_policy.activate_multi_profile_hosting`` →
|
||||
``agent.secret_scope.set_multiplex_active(True)``) as soon as it hosts a second
|
||||
profile home; after that ``get_secret`` fails closed for an unscoped read.
|
||||
|
||||
``GET /api/cron/delivery-targets`` calls
|
||||
``cron.scheduler_delivery.cron_delivery_targets()``, which resolves each
|
||||
platform's home chat id through ``get_secret``. It ran outside any profile
|
||||
scope, so under multi-profile hosting every poll raised ``UnscopedSecretError``
|
||||
(caught and logged as an error) and the response silently lost every configured
|
||||
platform — leaving only the implicit ``local`` entry. The read must run inside
|
||||
``_config_profile_scope``, like the sibling cron routes.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
pytest.importorskip("fastapi")
|
||||
|
||||
|
||||
def test_delivery_targets_route_binds_profile_scope(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
(tmp_path / ".env").write_text("TELEGRAM_HOME_CHANNEL=12345\n")
|
||||
|
||||
from agent import secret_scope
|
||||
|
||||
secret_scope.set_multiplex_active(True)
|
||||
|
||||
seen: dict = {}
|
||||
|
||||
def _fake_targets():
|
||||
seen["chat"] = secret_scope.get_secret("TELEGRAM_HOME_CHANNEL")
|
||||
return []
|
||||
|
||||
from cron import scheduler_delivery
|
||||
|
||||
monkeypatch.setattr(scheduler_delivery, "cron_delivery_targets", _fake_targets)
|
||||
|
||||
from hermes_cli.web_routers import cron as cron_router
|
||||
|
||||
result = asyncio.run(cron_router.get_cron_delivery_targets())
|
||||
|
||||
assert "chat" in seen, "route must call cron_delivery_targets()"
|
||||
assert seen["chat"] == "12345", "route must bind the profile secret scope before the read"
|
||||
assert result["targets"][0]["id"] == "local"
|
||||
Reference in New Issue
Block a user