Stage 388: PR #2533

2026-05-25 19:20:16 +00:00 · 2026-05-20 00:17:47 +00:00
parent 7c3dcce1d0 f14138659f
commit bd819f5e68
6 changed files with 90 additions and 23 deletions
@@ -21,6 +21,9 @@
 - **PR #2605** by @LumenYoung (refs #2581) — Make the metadata-only `/api/session?messages=0&resolve_model=0` path return the persisted sidecar `message_count` from `Session._metadata_message_count` when no session-index entry exists, so the active-session external-refresh signal still trips on legacy sessions whose sidecar contains externally-appended content. Composed cleanly with #2604 (the legacy-fallback applies only when the reconciled merged count is zero).
 - **PR #2573** by @espokaos-ops (closes #2510) — Persist session-level approvals when a "Allow for this session" click lands while a stream is active and `_pending` is empty. The approval flow now peeks `_gateway_queues[sid]` to recover the queued `_ApprovalEntry`'s `pattern_keys` so `approve_session()` records the approval; the next dangerous command in the same session no longer asks again. Reduced scope to peek-only per prior review note; the `agent_session_key` round-trip plumbing was dropped (it was dead on the WebUI streaming path).

+
+
+- Allow Settings → System to save public browser-only Official Hermes Dashboard links (for reverse-proxy URLs) without treating them as server-side probe targets.
 ### Added

 - **PR #2599** by @Michaelyklam (refs #1925) — Add the Slice 4b `RunnerRuntimeAdapter` facade — a protocol-translator client over a future runner/sidecar backend. The facade delegates `start_run`, `observe_run`, `get_run`, and control calls to an injected runner client, normalizes results into the existing `RunStartResult`/`RunEventStream`/`RunStatus`/`ControlResult` dataclasses, carries explicit `profile`/`workspace`/`model` payload fields, and returns bounded `unsupported` control results without owning `AIAgent`, stream lifecycle, cancel/approval/clarify queues, goal state, or cached-agent table. No route wiring, no default-on runner mode, no public response-shape change.
@@ -12,7 +12,7 @@ import json
 import logging
 import os
 import urllib.request
-from urllib.parse import urlparse
+from urllib.parse import urlparse, urlunparse

 logger = logging.getLogger(__name__)

@@ -61,6 +61,41 @@ def normalize_dashboard_url(raw_url: str | None) -> tuple[str, int, str, str] |
    return normalized_host, port, parsed.scheme, base


+def normalize_dashboard_browser_url(raw_url: str | None) -> str:
+    """Return a safe browser-only dashboard link URL.
+
+    Unlike the server-side probe target, this value is only returned to the
+    browser for navigation.  It may point at a public reverse-proxy hostname, but
+    it still rejects credentials, paths, query strings, fragments, and non-HTTP
+    schemes so it cannot hide secrets or script URLs in config.
+    """
+    raw = str(raw_url or "").strip()
+    if not raw:
+        return ""
+    parsed = urlparse(raw)
+    if parsed.scheme not in {"http", "https"}:
+        raise ValueError("invalid dashboard URL scheme")
+    if parsed.username or parsed.password:
+        raise ValueError("invalid dashboard URL credentials")
+    if not parsed.hostname:
+        raise ValueError("invalid dashboard URL host")
+    if parsed.params or parsed.query or parsed.fragment:
+        raise ValueError("invalid dashboard URL path")
+    path = parsed.path or ""
+    if path not in ("", "/"):
+        raise ValueError("invalid dashboard URL path")
+    try:
+        port = parsed.port
+    except ValueError as exc:
+        raise ValueError("invalid dashboard URL port") from exc
+    netloc = parsed.hostname.lower()
+    if port is not None:
+        if not (1 <= port <= 65535):
+            raise ValueError("invalid dashboard URL port")
+        netloc = f"{netloc}:{port}"
+    return urlunparse((parsed.scheme, netloc, "", "", "", ""))
+
+
 def _looks_like_official_dashboard(payload: object) -> bool:
    if not isinstance(payload, dict):
        return False
@@ -132,8 +167,7 @@ def get_dashboard_config(config_data: dict | None = None) -> dict:
        enabled = "auto"
    raw_url = str(dashboard_cfg.get("url") or "").strip()
    if raw_url:
-        # Normalize before echoing so the UI never displays unsafe/stale values.
-        _host, _port, _scheme, raw_url = normalize_dashboard_url(raw_url)
+        raw_url = normalize_dashboard_browser_url(raw_url)
    return {"enabled": enabled, "url": raw_url}


@@ -143,9 +177,7 @@ def save_dashboard_config(payload: dict) -> dict:
    if enabled not in _DASHBOARD_ENABLED_VALUES:
        raise ValueError("invalid dashboard enabled mode")
    raw_url = str((payload or {}).get("url", "") or "").strip()
-    normalized_url = ""
-    if raw_url:
-        _host, _port, _scheme, normalized_url = normalize_dashboard_url(raw_url)
+    normalized_url = normalize_dashboard_browser_url(raw_url) if raw_url else ""

    from api import config as webui_config

@@ -186,9 +218,13 @@ def get_dashboard_status(config_data: dict | None = None) -> dict:

    raw_url = dashboard_cfg.get("url") or dashboard_cfg.get("target") or ""
    try:
-        override = normalize_dashboard_url(raw_url)
+        browser_url = normalize_dashboard_browser_url(raw_url) if raw_url else ""
    except ValueError:
        return {"running": False, "enabled": enabled, "error": "invalid dashboard url"}
+    try:
+        override = normalize_dashboard_url(raw_url)
+    except ValueError:
+        override = None

    targets: list[tuple[str, int, str, str]]
    if override:
@@ -197,8 +233,10 @@ def get_dashboard_status(config_data: dict | None = None) -> dict:
        targets = [(host, port, "http", _base_url(host, port)) for host, port in DEFAULT_DASHBOARD_TARGETS]

    if enabled == "always":
+        if browser_url and not override:
+            return {"running": True, "enabled": enabled, "url": browser_url, "browser_url": browser_url}
        host, port, scheme, base = targets[0]
-        return {"running": True, "enabled": enabled, "host": host, "port": port, "url": base}
+        return {"running": True, "enabled": enabled, "host": host, "port": port, "url": browser_url or base, "browser_url": browser_url or base}

    if not _webui_bind_host_allows_auto_probe():
        return {"running": False, "enabled": enabled}
@@ -207,5 +245,8 @@ def get_dashboard_status(config_data: dict | None = None) -> dict:
        result = probe_official_dashboard(host, port, timeout=DEFAULT_DASHBOARD_TIMEOUT, scheme=scheme)
        if result.get("running"):
            result["enabled"] = enabled
+            if browser_url:
+                result["browser_url"] = browser_url
+                result["url"] = browser_url
            return result
    return {"running": False, "enabled": enabled}
@@ -1222,7 +1222,7 @@
            <button class="sm-btn" id="btnSignOut" onclick="signOut()" style="margin-top:6px;width:100%;padding:8px;font-weight:600;color:var(--accent);border-color:rgba(233,69,96,.3);display:none" data-i18n="sign_out">Sign Out</button>
            <div class="settings-field" style="margin-top:18px;padding-top:16px;border-top:1px solid var(--border)">
              <label for="settingsDashboardMode">Official Hermes Dashboard</label>
-              <div style="font-size:11px;color:var(--muted);margin-bottom:8px">Show a nav-rail link when the official <code>hermes dashboard</code> is reachable. Overrides are restricted to loopback URLs.</div>
+              <div style="font-size:11px;color:var(--muted);margin-bottom:8px">Show a nav-rail link when the official <code>hermes dashboard</code> is reachable. Public reverse-proxy URLs are stored as browser-only links and are never server-probed.</div>
              <select id="settingsDashboardMode" style="width:100%;padding:8px;background:var(--code-bg);color:var(--text);border:1px solid var(--border2);border-radius:6px">
                <option value="auto">Auto-detect</option>
                <option value="always">Always show</option>
@@ -401,10 +401,15 @@ function _dashboardIsBrowserLoopback(){
  return host==='127.0.0.1'||host==='localhost'||host==='::1';
 }
 function _dashboardBrowserUrl(status){
-  if(!status||!status.running||!status.port) return '';
+  if(!status||!status.running) return '';
+  if(status.browser_url||status.url){
+    try{return new URL(status.browser_url||status.url).toString().replace(/\/$/,'');}
+    catch(_){}
+  }
+  if(!status.port) return '';
  let source;
-  try{source=new URL(status.url||('http://127.0.0.1:'+status.port));}
-  catch(_){source=new URL('http://127.0.0.1:'+status.port);}
+  try{source=new URL('http://127.0.0.1:'+status.port);}
+  catch(_){return '';}
  const browserHost=window.location.hostname||source.hostname;
  const displayHost=browserHost.includes(':')&&!browserHost.startsWith('[')?'['+browserHost+']':browserHost;
  return source.protocol+'//'+displayHost+':'+status.port;
@@ -59,3 +59,12 @@ def test_dashboard_settings_controls_live_in_system_panel():
    assert 'id="settingsDashboardUrl"' in INDEX_HTML
    assert "function saveDashboardSettings" in UI_JS
    assert "api('/api/dashboard/config'" in UI_JS
+
+
+def test_dashboard_frontend_uses_browser_url_without_requiring_probe_port():
+    match = re.search(r"function _dashboardBrowserUrl\(status\).*?\n}\nfunction _applyDashboardStatus", UI_JS, re.DOTALL)
+    assert match is not None
+    helper = match.group(0)
+    assert "status.browser_url||status.url" in helper
+    assert "!status.port" in helper
+    assert helper.index("status.browser_url||status.url") < helper.index("!status.port")
@@ -132,11 +132,11 @@ def test_status_tries_default_loopback_targets_until_dashboard_found(monkeypatch
    assert attempts == [("127.0.0.1", 9119, 0.5, "http"), ("localhost", 9119, 0.5, "http")]


-def test_status_honors_never_and_strict_override(monkeypatch):
+def test_status_honors_never_and_external_browser_link_without_probe(monkeypatch):
    from api import dashboard_probe

    def fail_probe(*args, **kwargs):
-        raise AssertionError("disabled dashboard must not probe")
+        raise AssertionError("this status path must not probe a dashboard")

    monkeypatch.setattr(dashboard_probe, "probe_official_dashboard", fail_probe)
    assert dashboard_probe.get_dashboard_status(config_data={"webui": {"dashboard": {"enabled": "never"}}}) == {
@@ -144,9 +144,13 @@ def test_status_honors_never_and_strict_override(monkeypatch):
        "enabled": "never",
    }

-    result = dashboard_probe.get_dashboard_status(config_data={"webui": {"dashboard": {"url": "http://example.com:9119"}}})
-    assert result["running"] is False
-    assert "invalid" in result["error"]
+    result = dashboard_probe.get_dashboard_status(config_data={"webui": {"dashboard": {"enabled": "always", "url": "https://dashboard.example.test"}}})
+    assert result == {
+        "running": True,
+        "enabled": "always",
+        "url": "https://dashboard.example.test",
+        "browser_url": "https://dashboard.example.test",
+    }



@@ -203,9 +207,14 @@ def test_dashboard_config_roundtrip_writes_profile_config_yaml(tmp_path, monkeyp
    assert saved == {"enabled": "auto", "url": "http://127.0.0.1:19119"}
    assert "dashboard:" in (tmp_path / "config.yaml").read_text(encoding="utf-8")

-    try:
-        save_dashboard_config({"enabled": "auto", "url": "http://example.com:9119"})
-    except ValueError:
-        pass
-    else:
-        raise AssertionError("external dashboard URL override must be rejected")
+    saved = save_dashboard_config({"enabled": "always", "url": "https://dashboard.example.test"})
+    assert saved == {"enabled": "always", "url": "https://dashboard.example.test"}
+    assert get_dashboard_config() == {"enabled": "always", "url": "https://dashboard.example.test"}
+
+    for unsafe_url in ("https://example.com/path", "https://user:pass@example.com", "javascript:alert(1)"):
+        try:
+            save_dashboard_config({"enabled": "auto", "url": unsafe_url})
+        except ValueError:
+            pass
+        else:
+            raise AssertionError(f"unsafe dashboard URL must be rejected: {unsafe_url}")