fix(dashboard): sanction plugin WS/upload auth via SDK helpers (gated mode)
Dashboard plugins (kanban, hermes-achievements) read window.__HERMES_SESSION_TOKEN__ directly and hand-assembled WebSocket URLs with ?token=. That works in loopback/--insecure mode but is rejected on OAuth-gated deployments, where the session token is absent and _ws_auth_ok only accepts single-use ?ticket= auth. The result was 401s on plugin REST calls and 1008/403 on the kanban live-events WS whenever the dashboard ran behind OAuth (e.g. hosted Fly agents). Make the plugin SDK the single sanctioned auth surface: - web/src/lib/api.ts: add authedFetch() (raw Response for FormData uploads / blob downloads, token-or-cookie auth, no throw / no 401 redirect) and buildWsUrl() (assembles a ws(s):// URL with the correct auth param for the active mode — fresh single-use ticket in gated mode, token in loopback). - web/src/plugins/registry.ts: expose authedFetch, buildWsUrl, buildWsAuthParam, and sdkVersion on window.__HERMES_PLUGIN_SDK__; add SDK_CONTRACT_VERSION. - web/src/plugins/sdk.d.ts: hand-authored typed contract for the plugin SDK + registry globals (single source of truth for the Window declarations). - plugins/kanban + hermes-achievements dist bundles: stop reading the session token directly; route uploads/downloads through SDK.authedFetch and the live-events WS through SDK.buildWsUrl. - plugins/kanban plugin_api.py: _ws_upgrade_authorized() delegates the /events WS upgrade to the canonical web_server._ws_auth_ok gate, so it transparently accepts loopback token / gated ticket / internal credential and can never drift from core auth again. - tests: guard test asserting no plugin dist reads __HERMES_SESSION_TOKEN__ directly; kanban gated-ticket WS test. Verified live on a gated staging Fly agent: kanban /events upgrades 101 with a minted ticket (ticket_len=43, ws_auth_ok=True) where the old code got 403.
This commit is contained in:
parent
1c88360fed
commit
a6e47314f9
8 changed files with 501 additions and 95 deletions
|
|
@ -735,18 +735,29 @@ def test_board_auto_initializes_missing_db(tmp_path, monkeypatch):
|
|||
|
||||
|
||||
def test_ws_events_rejects_when_token_required(tmp_path, monkeypatch):
|
||||
"""When _SESSION_TOKEN is set (normal dashboard context), a missing or
|
||||
wrong ?token= query param must be rejected with policy-violation."""
|
||||
"""Loopback mode: a missing or wrong ?token= must be rejected with
|
||||
policy-violation; the correct token is accepted. The kanban WS now
|
||||
delegates to web_server._ws_auth_ok, so we stub that with the real
|
||||
loopback-token semantics (auth_required False → constant-time token
|
||||
compare)."""
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
kb.init_db()
|
||||
|
||||
# Stub web_server so _check_ws_token has a token to compare against.
|
||||
# Stub web_server with a loopback-mode _ws_auth_ok (auth_required False →
|
||||
# accept only the correct ?token=). Mirrors the real gate's loopback path.
|
||||
import hermes_cli
|
||||
import types
|
||||
stub = types.SimpleNamespace(_SESSION_TOKEN="secret-xyz")
|
||||
|
||||
def _fake_ws_auth_ok(ws):
|
||||
return ws.query_params.get("token", "") == "secret-xyz"
|
||||
|
||||
stub = types.SimpleNamespace(
|
||||
_SESSION_TOKEN="secret-xyz",
|
||||
_ws_auth_ok=_fake_ws_auth_ok,
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.web_server", stub)
|
||||
monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False)
|
||||
|
||||
|
|
@ -774,6 +785,51 @@ def test_ws_events_rejects_when_token_required(tmp_path, monkeypatch):
|
|||
assert ws is not None # handshake succeeded
|
||||
|
||||
|
||||
def test_ws_events_accepts_gated_ticket(tmp_path, monkeypatch):
|
||||
"""Gated OAuth mode: the WS must accept a single-use ?ticket= (and reject
|
||||
a bare ?token=, even one matching _SESSION_TOKEN). This is the regression
|
||||
for the hosted-dashboard bug where the kanban live-events WS 1008'd on
|
||||
every gated deployment because its bespoke check only knew _SESSION_TOKEN.
|
||||
We stub _ws_auth_ok with the real gated semantics (ticket-only)."""
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
kb.init_db()
|
||||
|
||||
import hermes_cli
|
||||
import types
|
||||
|
||||
def _fake_ws_auth_ok(ws):
|
||||
# Gated mode: only a known ticket is accepted; token path rejected.
|
||||
return ws.query_params.get("ticket", "") == "good-ticket"
|
||||
|
||||
stub = types.SimpleNamespace(
|
||||
_SESSION_TOKEN="secret-xyz",
|
||||
_ws_auth_ok=_fake_ws_auth_ok,
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.web_server", stub)
|
||||
monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False)
|
||||
|
||||
app = FastAPI()
|
||||
app.include_router(_load_plugin_router(), prefix="/api/plugins/kanban")
|
||||
c = TestClient(app)
|
||||
|
||||
from starlette.websockets import WebSocketDisconnect
|
||||
|
||||
# Legacy token is rejected in gated mode, even if it's the real one.
|
||||
with pytest.raises(WebSocketDisconnect) as exc:
|
||||
with c.websocket_connect("/api/plugins/kanban/events?token=secret-xyz"):
|
||||
pass
|
||||
assert exc.value.code == 1008
|
||||
|
||||
# A valid ticket is accepted.
|
||||
with c.websocket_connect(
|
||||
"/api/plugins/kanban/events?ticket=good-ticket"
|
||||
) as ws:
|
||||
assert ws is not None
|
||||
|
||||
|
||||
def test_ws_events_board_query_param_default_overrides_current_board_pointer(tmp_path, monkeypatch):
|
||||
"""The event stream must honor ``board=default`` even when the global
|
||||
current-board pointer targets a different board.
|
||||
|
|
@ -806,7 +862,10 @@ def test_ws_events_board_query_param_default_overrides_current_board_pointer(tmp
|
|||
import hermes_cli
|
||||
import types
|
||||
|
||||
stub = types.SimpleNamespace(_SESSION_TOKEN="secret-xyz")
|
||||
stub = types.SimpleNamespace(
|
||||
_SESSION_TOKEN="secret-xyz",
|
||||
_ws_auth_ok=lambda ws: ws.query_params.get("token", "") == "secret-xyz",
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.web_server", stub)
|
||||
monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False)
|
||||
|
||||
|
|
@ -842,10 +901,10 @@ def test_ws_events_swallows_cancellation_on_shutdown(tmp_path, monkeypatch):
|
|||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
kb.init_db()
|
||||
|
||||
# Short-circuit the token check — this test is about the cancellation
|
||||
# Short-circuit the auth check — this test is about the cancellation
|
||||
# path, not auth.
|
||||
import plugins.kanban.dashboard.plugin_api as pa
|
||||
monkeypatch.setattr(pa, "_check_ws_token", lambda t: True)
|
||||
monkeypatch.setattr(pa, "_ws_upgrade_authorized", lambda ws: True)
|
||||
|
||||
class _FakeWS:
|
||||
def __init__(self):
|
||||
|
|
|
|||
95
tests/plugins/test_plugin_dashboard_auth_contract.py
Normal file
95
tests/plugins/test_plugin_dashboard_auth_contract.py
Normal file
|
|
@ -0,0 +1,95 @@
|
|||
"""Guardrail: dashboard plugins must NOT read the session token directly.
|
||||
|
||||
The dashboard host exposes a sanctioned, gated-mode-aware auth surface on the
|
||||
plugin SDK (``window.__HERMES_PLUGIN_SDK__``): ``fetchJSON`` (JSON REST),
|
||||
``authedFetch`` (uploads / blob downloads), and ``buildWsUrl`` /
|
||||
``buildWsAuthParam`` (WebSockets). These handle BOTH dashboard auth modes —
|
||||
loopback (``X-Hermes-Session-Token`` header) and gated OAuth
|
||||
(``hermes_session_at`` cookie / single-use ``?ticket=``).
|
||||
|
||||
Plugins that hand-roll ``fetch`` / ``WebSocket`` and read
|
||||
``window.__HERMES_SESSION_TOKEN__`` directly send an empty token in gated mode
|
||||
and 401/1008. That bug shipped in the kanban and achievements plugins and was
|
||||
invisible until the dashboard ran gated on hosted Fly agents.
|
||||
|
||||
This test fails if any bundled plugin's frontend reads the token global
|
||||
directly, forcing new/edited plugins through the SDK surface instead. It is
|
||||
the enforcement half of the "single sanctioned auth surface" design — the SDK
|
||||
helpers are the carrot, this test is the stick.
|
||||
|
||||
If you have a legitimate reason to reference the token name (e.g. a comment
|
||||
explaining why NOT to use it), add the file to ``_ALLOWED_FILES`` with a note.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# Repo root: tests/plugins/<this file> → ../../
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
_PLUGINS_DIR = _REPO_ROOT / "plugins"
|
||||
|
||||
# The forbidden global. Reading it directly bypasses the gated-mode auth path.
|
||||
_FORBIDDEN = "__HERMES_SESSION_TOKEN__"
|
||||
|
||||
# Files explicitly allowed to mention the token (none today). Map path →
|
||||
# reason so the allowance is self-documenting if one is ever needed.
|
||||
_ALLOWED_FILES: dict[str, str] = {}
|
||||
|
||||
|
||||
def _plugin_frontend_bundles() -> list[Path]:
|
||||
"""Every plugin-shipped JS bundle the dashboard loads into the browser."""
|
||||
if not _PLUGINS_DIR.is_dir():
|
||||
return []
|
||||
# Plugin dashboards live at plugins/<name>/dashboard/dist/*.js
|
||||
return sorted(_PLUGINS_DIR.glob("*/dashboard/dist/*.js"))
|
||||
|
||||
|
||||
def test_there_are_plugin_bundles_to_check() -> None:
|
||||
"""Sanity: the glob actually finds the bundles, so a future layout change
|
||||
doesn't silently turn this guard into a no-op."""
|
||||
bundles = _plugin_frontend_bundles()
|
||||
names = {b.parent.parent.parent.name for b in bundles}
|
||||
# kanban + hermes-achievements are bundled today; assert at least one is
|
||||
# found so the guard can't pass vacuously.
|
||||
assert bundles, "no plugin dashboard bundles found — glob/layout drift?"
|
||||
assert names, "could not resolve plugin names from bundle paths"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"bundle",
|
||||
_plugin_frontend_bundles(),
|
||||
ids=lambda p: str(p.relative_to(_REPO_ROOT)),
|
||||
)
|
||||
def test_plugin_bundle_does_not_read_session_token(bundle: Path) -> None:
|
||||
rel = str(bundle.relative_to(_REPO_ROOT))
|
||||
text = bundle.read_text(encoding="utf-8", errors="replace")
|
||||
|
||||
if rel in _ALLOWED_FILES:
|
||||
return # explicitly allowed (with a documented reason)
|
||||
|
||||
# Only flag CODE reads of the token global, not mentions in ``//`` comments
|
||||
# (e.g. a comment explaining why the SDK helper is used instead). A line is
|
||||
# a code read if it contains the global and the global appears before any
|
||||
# ``//`` comment marker on that line.
|
||||
offending: list[str] = []
|
||||
for i, line in enumerate(text.splitlines(), start=1):
|
||||
idx = line.find(_FORBIDDEN)
|
||||
if idx == -1:
|
||||
continue
|
||||
comment_idx = line.find("//")
|
||||
in_comment = comment_idx != -1 and comment_idx < idx
|
||||
if not in_comment:
|
||||
offending.append(f" {i}: {line.strip()}")
|
||||
|
||||
if not offending:
|
||||
return
|
||||
|
||||
pytest.fail(
|
||||
f"{rel} reads {_FORBIDDEN} directly — this bypasses gated-mode auth "
|
||||
f"and 401/1008s on OAuth-gated dashboards. Use the plugin SDK instead: "
|
||||
f"SDK.fetchJSON (JSON), SDK.authedFetch (uploads/downloads), or "
|
||||
f"SDK.buildWsUrl (WebSockets). Offending lines:\n" + "\n".join(offending)
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue