diff --git a/pyproject.toml b/pyproject.toml index a0793f6c..39806bde 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -217,6 +217,8 @@ ignore = [ # Stdlib HTTP handler overrides (do_GET/do_POST) and lazy imports that avoid a # circular dependency with strix.telemetry / strix.viewer.report_pdf. "strix/viewer/server.py" = ["N802", "PLC0415"] +# Lazy telemetry import to avoid importing PostHog before the viewer starts. +"strix/viewer/cli.py" = ["PLC0415"] # Lazy imports inside functions to avoid circular dependency with # strix.telemetry / strix.report.dedupe / cvss. "strix/tools/notes/tools.py" = ["PLC0415", "TC002"] diff --git a/strix/interface/main.py b/strix/interface/main.py index a23ca427..e307d61c 100644 --- a/strix/interface/main.py +++ b/strix/interface/main.py @@ -981,11 +981,14 @@ def main() -> None: viewer_httpd = None web_url = None if not args.non_interactive and sys.stdout.isatty(): - from strix.viewer.server import bundle_is_built, serve + from strix.viewer.server import authorized_url, bundle_is_built, serve if bundle_is_built(): try: - viewer_httpd, web_url = serve(results_path, open_browser=False) + viewer_httpd, base_url, token = serve(results_path, open_browser=False) + # The completion panel's "View in web" link must authorize the + # browser, so hand it the tokened URL rather than the bare host. + web_url = authorized_url(base_url, token) posthog.viewer_opened(source="post_scan", live=False) except Exception: logger.debug("could not start local viewer", exc_info=True) diff --git a/strix/interface/tui/app.py b/strix/interface/tui/app.py index efcbf6d6..5f350898 100644 --- a/strix/interface/tui/app.py +++ b/strix/interface/tui/app.py @@ -1838,7 +1838,7 @@ class StrixTUIApp(App): # type: ignore[misc] webbrowser.open(self._viewer_url) return try: - from strix.viewer.server import bundle_is_built, serve + from strix.viewer.server import authorized_url, bundle_is_built, serve if not bundle_is_built(): self._set_viewer_cta("[#eab308]Viewer UI not built[/]") @@ -1856,14 +1856,16 @@ class StrixTUIApp(App): # type: ignore[misc] message=message, ) - httpd, url = serve(run_dir, open_browser=True, steer_handler=_viewer_steer) + httpd, url, token = serve(run_dir, open_browser=True, steer_handler=_viewer_steer) except Exception: logger.debug("failed to start local viewer", exc_info=True) self._set_viewer_cta("[red]Viewer failed to start[/]") return self._viewer_httpd = httpd - self._viewer_url = url - self._set_viewer_cta(self._viewer_cta_markup(url)) + # Store the tokened URL so reopening the CTA re-authorizes the browser + # (this viewer carries a steer handler, so the session is required). + self._viewer_url = authorized_url(url, token) + self._set_viewer_cta(self._viewer_cta_markup(self._viewer_url)) with contextlib.suppress(Exception): from strix.telemetry import posthog diff --git a/strix/viewer/cli.py b/strix/viewer/cli.py index eb37951a..852fbfcc 100644 --- a/strix/viewer/cli.py +++ b/strix/viewer/cli.py @@ -16,7 +16,7 @@ from strix.core.paths import ( run_record_path, runs_base_dir, ) -from strix.viewer.server import bundle_is_built, serve +from strix.viewer.server import authorized_url, bundle_is_built, serve from strix.viewer.transcript import read_run_summary @@ -64,25 +64,30 @@ def run_view(argv: list[str]) -> None: run_dir = _resolve_run_dir(args.run, console) - httpd, url = serve( + httpd, url, token = serve( run_dir, host=args.host, port=args.port, open_browser=not args.no_open, ) + # The tokened URL is what authorizes the browser (steering, report sending, + # history). Print it rather than the bare URL so the operator -- and only + # the operator -- can open or share an authorized link. + open_url = authorized_url(url, token) run_name = run_dir.name summary = read_run_summary(run_dir) live = not summary.get("finished", False) - from strix.telemetry import posthog # noqa: PLC0415 + from strix.telemetry import posthog posthog.viewer_opened(source="cli", live=live) state_label = "[#eab308]live[/]" if live else "[#22c55e]finished[/]" console.print() - console.print(f"Serving [bold white]{run_name}[/] ({state_label}) at [#60a5fa]{url}[/]") - console.print("[dim]Press Ctrl-C to stop the viewer.[/]") + console.print(f"Serving [bold white]{run_name}[/] ({state_label}) at [#60a5fa]{open_url}[/]") + console.print("[dim]This link authorizes the browser; anyone you share it with can steer[/]") + console.print("[dim]a live scan and browse history. Press Ctrl-C to stop the viewer.[/]") console.print() try: @@ -110,10 +115,14 @@ def _resolve_run_dir(run: str | None, console: Console) -> Path: def _fail_no_run(console: Console, *, requested: str | None) -> NoReturn: base = runs_base_dir() - available = sorted( - (child.name for child in base.iterdir() if run_record_path(child).is_file()), - reverse=True, - ) if base.is_dir() else [] + available = ( + sorted( + (child.name for child in base.iterdir() if run_record_path(child).is_file()), + reverse=True, + ) + if base.is_dir() + else [] + ) if requested: console.print(f"[bold red]No run named '{requested}' under ./{RUNS_DIR_NAME}.[/]") diff --git a/strix/viewer/server.py b/strix/viewer/server.py index e107a073..4d177fab 100644 --- a/strix/viewer/server.py +++ b/strix/viewer/server.py @@ -24,7 +24,7 @@ from http import HTTPStatus from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from pathlib import Path from typing import TYPE_CHECKING, Any -from urllib.parse import parse_qs, unquote, urlsplit +from urllib.parse import parse_qs, unquote, urlencode, urlsplit from strix.core.paths import run_record_path from strix.viewer import auth @@ -127,12 +127,13 @@ class _ViewerState: # launcher), which can deliver a message to a running agent. Absent for # standalone ``strix view`` / finished runs, so steering is unavailable. self.steer_handler = steer_handler - # Unguessable per-process capability handed to the local browser (via a - # cookie on index.html) and required on the sensitive routes. It is the - # request-level authorization Greptile asked for: reachability of the - # port (e.g. when bound with ``--host``) is no longer sufficient to - # steer a live scan or trigger a report; only the browser that loaded - # the page this process served holds the token. + # Unguessable per-process capability. It is minted here, printed/opened + # for the operator who started the server (see ``authorized_url``), and + # exchanged for a session cookie only when presented on the initial page + # load. It is the request-level authorization the review asked for: + # reachability of the port (e.g. when bound with ``--host``) is not + # enough to steer a live scan, trigger a report, or browse history -- + # the token is never handed to a caller who merely reaches ``/``. self.session_token = secrets.token_urlsafe(32) @@ -150,7 +151,7 @@ def _make_handler(state: _ViewerState) -> type[BaseHTTPRequestHandler]: if path.startswith("/api/"): self._handle_api(path, parse_qs(parts.query)) else: - self._handle_static(path) + self._handle_static(path, parse_qs(parts.query)) except BrokenPipeError: # The browser closed the connection mid-response (e.g. it # navigated away between polls). Not an error. @@ -233,11 +234,14 @@ def _make_handler(state: _ViewerState) -> type[BaseHTTPRequestHandler]: self._send_json(HTTPStatus.OK, {"can_steer": state.steer_handler is not None}) return if path == "/api/auth/status": + # Report verification through is_verified() so an expired record + # is advertised as unverified -- otherwise the SPA would suppress + # re-verification while history stays locked, stranding the user. record = auth.read_auth() self._send_json( HTTPStatus.OK, { - "verified": record is not None, + "verified": auth.is_verified(), "email": record.get("email") if record else None, }, ) @@ -400,7 +404,17 @@ def _make_handler(state: _ViewerState) -> type[BaseHTTPRequestHandler]: supplied = self._cookies().get(SESSION_COOKIE, "") return bool(supplied) and secrets.compare_digest(supplied, state.session_token) - def _handle_static(self, path: str) -> None: + def _token_presented(self, query: dict[str, list[str]]) -> bool: + """True when the request carries the correct bootstrap token. + + The token reaches the operator's browser through the URL printed / + opened by the process that started the server, a channel an + arbitrary network caller on an exposed port cannot observe. + """ + supplied = (query.get("token") or [""])[0] + return bool(supplied) and secrets.compare_digest(supplied, state.session_token) + + def _handle_static(self, path: str, query: dict[str, list[str]]) -> None: target = self._resolve_asset(path) if target is None: # SPA fallback: unknown non-asset routes render index.html so @@ -415,8 +429,10 @@ def _make_handler(state: _ViewerState) -> type[BaseHTTPRequestHandler]: self.send_response(HTTPStatus.OK) self.send_header("Content-Type", content_type or "application/octet-stream") self.send_header("Content-Length", str(len(content))) - if is_index: - # Hand the loading browser the per-process session capability. + if is_index and self._token_presented(query): + # Exchange the bootstrap token for the per-process session + # capability. Issued only when the correct token is presented, + # so a caller who merely reaches ``/`` never obtains it. # HttpOnly (JS never needs it; fetch sends it automatically) and # SameSite=Strict (never sent from a cross-site context). self.send_header( @@ -449,6 +465,17 @@ def _make_handler(state: _ViewerState) -> type[BaseHTTPRequestHandler]: return ViewerHandler +def authorized_url(base_url: str, token: str) -> str: + """URL that bootstraps the viewer session for the operator. + + Presenting ``token`` on the initial page load is what mints the session + cookie, so this URL is printed / opened only for the operator who started + the server. Sharing it (rather than the bare ``base_url``) is what lets a + trusted remote user authorize when the viewer is exposed with ``--host``. + """ + return f"{base_url}/?{urlencode({'token': token})}" + + def serve( run_dir: Path, *, @@ -456,8 +483,11 @@ def serve( port: int = 0, open_browser: bool = True, steer_handler: Callable[[str, str], bool] | None = None, -) -> tuple[ThreadingHTTPServer, str]: - """Start the viewer server on a background thread and return (server, url). +) -> tuple[ThreadingHTTPServer, str, str]: + """Start the viewer server on a background thread; return (server, url, token). + + ``url`` is the bare base; pass it through ``authorized_url(url, token)`` to + build the operator link that authorizes the browser. Binds an ephemeral port by default. If a fixed ``port`` is requested but in use, falls back to an ephemeral port. Reused by both the ``strix view`` @@ -487,9 +517,9 @@ def serve( thread.start() if open_browser: - _open_browser(url) + _open_browser(authorized_url(url, state.session_token)) - return httpd, url + return httpd, url, state.session_token def _open_browser(url: str) -> None: @@ -499,4 +529,4 @@ def _open_browser(url: str) -> None: logger.debug("could not open browser for %s", url, exc_info=True) -__all__ = ["bundle_dir", "bundle_is_built", "serve"] +__all__ = ["authorized_url", "bundle_dir", "bundle_is_built", "serve"] diff --git a/tests/test_viewer.py b/tests/test_viewer.py index bbc5c0c0..fb629e1d 100644 --- a/tests/test_viewer.py +++ b/tests/test_viewer.py @@ -100,7 +100,7 @@ def test_server_serves_api_and_static(tmp_path: Path, monkeypatch: pytest.Monkey (assets / "assets" / "app.js").write_text("console.log(1)", encoding="utf-8") monkeypatch.setattr("strix.viewer.server.bundle_dir", lambda: assets) - httpd, url = serve(run_dir, open_browser=False) + httpd, url, _ = serve(run_dir, open_browser=False) try: status, ctype, body = _get(f"{url}/api/run") assert status == 200 @@ -138,7 +138,7 @@ def test_server_event_endpoint_forwards_cta( lambda cta, surface=None: seen.append((cta, surface)), ) - httpd, url = serve(run_dir, open_browser=False) + httpd, url, _ = serve(run_dir, open_browser=False) try: body = json.dumps( {"event": "cta_clicked", "cta": "PR reviews", "surface": "sidebar_nav"} @@ -169,7 +169,7 @@ def test_server_event_endpoint_forwards_email_funnel( lambda step, purpose=None: seen.append((step, purpose)), ) - httpd, url = serve(run_dir, open_browser=False) + httpd, url, _ = serve(run_dir, open_browser=False) try: # A whitelisted funnel event is forwarded; an unknown event is ignored. for payload, expected in ( @@ -205,9 +205,10 @@ def _post( return exc.code, exc.read() -def _session_cookie(url: str) -> str: - """Fetch index.html and return its ``name=value`` session cookie.""" - with urllib.request.urlopen(url + "/") as resp: # noqa: S310 - localhost test server +def _session_cookie(url: str, token: str) -> str: + """Bootstrap a session via the tokened URL and return its ``name=value`` cookie.""" + bootstrap = f"{url}/?token={token}" + with urllib.request.urlopen(bootstrap) as resp: # noqa: S310 - localhost test server raw = str(resp.headers.get("Set-Cookie", "")) return raw.split(";", 1)[0] @@ -227,7 +228,7 @@ def _bundle(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("strix.viewer.server.bundle_dir", lambda: assets) -def test_index_sets_session_cookie_but_assets_do_not( +def test_capability_issued_only_for_tokened_bootstrap( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: run_dir = _make_run(tmp_path, "cookie", status="running", end_time=None) @@ -237,13 +238,23 @@ def test_index_sets_session_cookie_but_assets_do_not( (assets / "assets" / "app.js").write_text("1", encoding="utf-8") monkeypatch.setattr("strix.viewer.server.bundle_dir", lambda: assets) - httpd, url = serve(run_dir, open_browser=False) + httpd, url, token = serve(run_dir, open_browser=False) try: + # A bare index load -- all a reachable client can do -- hands out nothing. with urllib.request.urlopen(url + "/") as resp: # noqa: S310 - cookie = resp.headers.get("Set-Cookie", "") + assert resp.headers.get("Set-Cookie") is None + + # A wrong token is likewise refused the capability. + with urllib.request.urlopen(f"{url}/?token=wrong") as resp: # noqa: S310 + assert resp.headers.get("Set-Cookie") is None + + # Only the correct bootstrap token mints the session cookie. + with urllib.request.urlopen(f"{url}/?token={token}") as resp: # noqa: S310 + cookie = str(resp.headers.get("Set-Cookie", "")) assert "strix_viewer_session=" in cookie assert "HttpOnly" in cookie and "SameSite=Strict" in cookie + # Static assets never carry it. with urllib.request.urlopen(url + "/assets/app.js") as resp: # noqa: S310 assert resp.headers.get("Set-Cookie") is None finally: @@ -251,6 +262,58 @@ def test_index_sets_session_cookie_but_assets_do_not( httpd.server_close() +def test_unauthorized_client_cannot_acquire_capability( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + run_dir = _make_run(tmp_path, "exposed", status="running", end_time=None) + _bundle(tmp_path, monkeypatch) + + delivered: list[tuple[str, str]] = [] + + def handler(agent_id: str, message: str) -> bool: + delivered.append((agent_id, message)) + return True + + httpd, url, _ = serve(run_dir, open_browser=False, steer_handler=handler) + try: + # A direct network client can reach the page but is handed no capability, + # so replaying an empty/guessed cookie cannot steer a live scan. + with urllib.request.urlopen(url + "/") as resp: # noqa: S310 + assert resp.headers.get("Set-Cookie") is None + status, _ = _post( + url, + "/api/agents/steer", + {"agent_id": "root", "message": "pwn"}, + cookie="strix_viewer_session=", + ) + assert status == 403 + assert delivered == [] + finally: + httpd.shutdown() + httpd.server_close() + + +def test_auth_status_reflects_expiry(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + run_dir = _make_run(tmp_path, "status", status="running", end_time=None) + _bundle(tmp_path, monkeypatch) + monkeypatch.setattr("strix.viewer.auth.read_auth", lambda: {"email": "a@b.com", "token": "t"}) + verified = {"value": True} + monkeypatch.setattr("strix.viewer.auth.is_verified", lambda: verified["value"]) + + httpd, url, _ = serve(run_dir, open_browser=False) + try: + _, _, body = _get(f"{url}/api/auth/status") + assert json.loads(body) == {"verified": True, "email": "a@b.com"} + + # Once expired, status must advertise unverified so the SPA re-prompts. + verified["value"] = False + _, _, body = _get(f"{url}/api/auth/status") + assert json.loads(body)["verified"] is False + finally: + httpd.shutdown() + httpd.server_close() + + def test_steer_requires_session_cookie(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: run_dir = _make_run(tmp_path, "steer", status="running", end_time=None) _bundle(tmp_path, monkeypatch) @@ -261,7 +324,7 @@ def test_steer_requires_session_cookie(tmp_path: Path, monkeypatch: pytest.Monke delivered.append((agent_id, message)) return True - httpd, url = serve(run_dir, open_browser=False, steer_handler=handler) + httpd, url, token = serve(run_dir, open_browser=False, steer_handler=handler) try: body = {"agent_id": "root", "message": "focus on auth"} # No cookie: rejected before reaching the live coordinator. @@ -270,7 +333,7 @@ def test_steer_requires_session_cookie(tmp_path: Path, monkeypatch: pytest.Monke assert delivered == [] # With the session cookie the message is delivered. - status, raw = _post(url, "/api/agents/steer", body, cookie=_session_cookie(url)) + status, raw = _post(url, "/api/agents/steer", body, cookie=_session_cookie(url, token)) assert status == 200 assert json.loads(raw)["ok"] is True assert delivered == [("root", "focus on auth")] @@ -288,7 +351,7 @@ def test_report_send_requires_session_cookie( # A verified machine token exists, but that alone must not authorize a caller. monkeypatch.setattr("strix.viewer.auth.read_auth", lambda: {"email": "a@b.com", "token": "t"}) - httpd, url = serve(run_dir, open_browser=False) + httpd, url, token = serve(run_dir, open_browser=False) try: # No cookie: forbidden before the machine token is ever consulted. status, _ = _post(url, "/api/report/send", {}) @@ -297,7 +360,7 @@ def test_report_send_requires_session_cookie( # With the cookie the request clears the session gate; it then reaches # the run resolver, so an unknown run is a 404 rather than a 403. status, _ = _post( - url, "/api/report/send", {"run": "does-not-exist"}, cookie=_session_cookie(url) + url, "/api/report/send", {"run": "does-not-exist"}, cookie=_session_cookie(url, token) ) assert status == 404 finally: @@ -315,7 +378,7 @@ def test_historical_run_data_requires_verification( verified = {"value": False} monkeypatch.setattr("strix.viewer.auth.is_verified", lambda: verified["value"]) - httpd, url = serve(launched, open_browser=False) + httpd, url, _ = serve(launched, open_browser=False) try: # The launched run is always viewable, no verification required. status, _, _ = _get(f"{url}/api/run") @@ -343,7 +406,7 @@ def test_server_rejects_path_traversal(tmp_path: Path, monkeypatch: pytest.Monke (assets / "index.html").write_text("index", encoding="utf-8") monkeypatch.setattr("strix.viewer.server.bundle_dir", lambda: assets) - httpd, url = serve(run_dir, open_browser=False) + httpd, url, _ = serve(run_dir, open_browser=False) try: # A traversal target must never leak the file; it falls back to index.html. _, _, body = _get(f"{url}/..%2f..%2fsecret.txt")