From 0242d211325f496ac1fb13392433626007d20d03 Mon Sep 17 00:00:00 2001 From: cjangrist Date: Fri, 31 Jul 2026 15:19:12 +0200 Subject: [PATCH 01/25] feat: migrate viewer to native KasmVNC 1.5 client behind an nginx data plane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the KasmVNC 1.3.3 + generic noVNC compatibility bridge (a FastAPI WebSocket relay that parsed and rewrote RFB messages) with KasmVNC 1.5.0's native, matched web client served through an authenticated nginx reverse proxy. FastAPI becomes control-plane only; no framebuffer bytes traverse Python. Architecture ------------ nginx (:8080) is the public entry: /api/* and the SPA go to uvicorn on 127.0.0.1:8081, /viewer//* goes to the profile's loopback-only KasmVNC port, gated by auth_request against FastAPI. Tokens are redacted from access logs and Kasm's management /api is blocked from clients. Short-lived opaque per-profile viewer tokens are issued by POST /api/profiles//viewer-token; GET /api/viewer-auth authorises nginx subrequests and returns the upstream plus injectable Basic credentials. Tokens are revoked on stop/delete and swept on issue. Profile status gains a running | starting | stopped lifecycle plus real xvnc_alive/browser_alive liveness for reconnect classification. GET /api/profiles//kasm-stats proxies Kasm's bottleneck/session/frame stats. Frontend -------- @novnc/novnc is gone; the native Kasm 1.5 client is embedded via iframe using its ?path= override for prefix-safe WebSocket routing. A manager-owned reconnect state machine (useViewerSession) tracks postMessage connection_state, backs off 250ms..15s with full jitter, handles offline/visibility, classifies liveness by status probe, keeps the last frame under a dimmed overlay, and never lets a viewer disconnect stop the browser. It carries a connect watchdog, a 45s connected heartbeat, a bounded failure budget for "alive but unreachable", and generation/sequence guards so no stale async result can act after the machine has moved on. KasmVNC 1.5.0 (trixie package, SHA256-pinned to the Debian 13 base) ------------------------------------------------------------------ Server-authoritative encoding: 30 FPS cap, dynamic quality 6-8, video-mode 1600x900 under motion, bounded encoder threads, WebP, -IgnoreClientSettingsKasm. Quality presets via KASM_QUALITY_PRESET; KASM_RECT_THREADS override. In-band H.264/H.265/AV1 (-videoCodec auto; the binary default is empty despite the man page) with FFmpeg runtime libs and dlopen symlinks, plus VAAPI for AMD/Intel. DRI3 acceleration auto-enabled when an open-driver render node is present (skipped on proprietary NVIDIA), overridable via KASM_HW3D/KASM_DRINODE. STUN public-IP discovery disabled — WSS only. Operations ---------- The entrypoint supervises nginx and uvicorn: whichever dies takes the container down so the restart policy can restore a working data plane, with an ordered shutdown that lets the lifespan close browsers and Xvnc first. Compose gains restart: unless-stopped and stop_grace_period: 60s, passes the documented KASM_* tuning variables through, and keeps GPU passthrough as an opt-in overlay (docker-compose.gpu.yml) so the default path works on GPU-less hosts. Hardening --------- Browser lifecycle: launch is bounded and fully covered by its cleanup, aborted launches close the context they opened, teardown is guarded for its whole duration so a concurrent launch or delete cannot race a live Chromium, displays are released only after the process is reaped, and Xvnc readiness is polled rather than slept on. KasmVNC credentials are a launch prerequisite. nginx preserves the outer X-Forwarded-Proto, normalises the slash-less viewer URL with a relative 308, and sizes body limits to the API's own contract. Blocking work (profile rmtree, liveness probes) runs off the event loop. Verified end-to-end against a live container throughout: login/launch/connect, keyboard and clipboard, CDP through nginx, unclean WS drop -> automatic reconnect with a fresh token, nginx worker death -> recovery, nginx master death -> container restart -> healthy, Xvnc death -> accurate session-ended, token revocation and expiry, relaunch from a terminal overlay, delete of a running profile, and graceful shutdown in <0.5s with browsers closed cleanly. 257 backend + 70 frontend tests pass; tsc clean. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 5 + Dockerfile | 37 +- README.md | 51 +- backend/browser_manager.py | 452 ++++++- backend/main.py | 605 ++++----- backend/models.py | 12 +- backend/tests/test_api.py | 753 ++++++++++- backend/tests/test_models.py | 23 + backend/tests/test_rfb.py | 341 ----- backend/tests/test_viewer_tokens.py | 108 ++ backend/tests/test_vnc_manager.py | 1178 ++++++++++++++++- backend/viewer_tokens.py | 79 ++ backend/vnc_manager.py | 358 ++++- docker-compose.gpu.yml | 14 + docker-compose.yml | 19 + docker/nginx.conf | 155 +++ entrypoint.sh | 44 +- frontend/package-lock.json | 7 - frontend/package.json | 1 - frontend/src/App.test.tsx | 190 +++ frontend/src/App.tsx | 29 +- frontend/src/components/LaunchButton.tsx | 7 +- .../src/components/ProfileViewer.test.tsx | 150 +++ frontend/src/components/ProfileViewer.tsx | 328 ++--- frontend/src/components/StatusIndicator.tsx | 23 +- frontend/src/hooks/useViewerSession.test.ts | 1055 +++++++++++++++ frontend/src/hooks/useViewerSession.ts | 834 ++++++++++++ frontend/src/lib/api.test.ts | 43 +- frontend/src/lib/api.ts | 66 +- frontend/src/novnc.d.ts | 23 - 30 files changed, 5902 insertions(+), 1088 deletions(-) delete mode 100644 backend/tests/test_rfb.py create mode 100644 backend/tests/test_viewer_tokens.py create mode 100644 backend/viewer_tokens.py create mode 100644 docker-compose.gpu.yml create mode 100644 docker/nginx.conf create mode 100644 frontend/src/App.test.tsx create mode 100644 frontend/src/components/ProfileViewer.test.tsx create mode 100644 frontend/src/hooks/useViewerSession.test.ts create mode 100644 frontend/src/hooks/useViewerSession.ts delete mode 100644 frontend/src/novnc.d.ts diff --git a/.gitignore b/.gitignore index d5db79e7..afcbdf2b 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ frontend/dist/ # Deploy deploy.sh +.env # Claude Code .claude/ @@ -24,3 +25,7 @@ CLAUDE.md publish-docker.sh .beads debug + +# Agent scratch +tmp/ +trash/ diff --git a/Dockerfile b/Dockerfile index 962144df..a29592ff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,13 +32,42 @@ RUN echo "deb http://deb.debian.org/debian trixie contrib" >> /etc/apt/sources.l && fc-cache -f \ && rm -rf /var/lib/apt/lists/* -# Install KasmVNC (auto-selects amd64 or arm64 based on build platform) +# nginx data plane + VA-API (AMD/Intel) + FFmpeg runtime libs for KasmVNC 1.5 +# in-band H.264/H.265/AV1 video streaming (dlopen'd at runtime; JPEG/WebP +# fallback if absent) +RUN apt-get update && apt-get install -y --no-install-recommends \ + nginx \ + libva2 libva-drm2 mesa-va-drivers vainfo \ + libavcodec61 libavformat61 libavutil59 libswscale8 \ + && rm -rf /var/lib/apt/lists/* + +# KasmVNC dlopens UNVERSIONED FFmpeg names (libavcodec.so etc.) which Debian +# ships only in -dev packages — symlink them to the installed runtime libs. +# Multi-arch safe: paths come from ldconfig, not hardcoded. +RUN set -e; \ + for lib in libavcodec libavformat libavutil libswscale; do \ + target="$(ldconfig -p | grep -m1 "${lib}\.so\.[0-9]" | awk '{print $NF}')"; \ + test -n "$target"; \ + ln -sf "$target" "$(dirname "$target")/${lib}.so"; \ + done; \ + ldconfig + +# Install KasmVNC 1.5.0 (auto-selects amd64 or arm64 based on build platform), +# SHA256-verified — build fails on mismatch ARG TARGETARCH -RUN wget -q https://github.com/kasmtech/KasmVNC/releases/download/v1.3.3/kasmvncserver_bookworm_1.3.3_${TARGETARCH}.deb \ - && apt-get update && apt-get install -y -f ./kasmvncserver_bookworm_1.3.3_${TARGETARCH}.deb \ - && rm kasmvncserver_bookworm_1.3.3_${TARGETARCH}.deb \ +RUN case "${TARGETARCH}" in \ + amd64) KASM_SHA256=80b241de7dfe53bba2b7e1cc5ac8c5246d72271efa16be2d4f76607f30fab1c4 ;; \ + arm64) KASM_SHA256=fbb11589958a2acccd2d67f67944be79ac1e8e3a1d6172c0e6db6dc59e55a919 ;; \ + *) echo "Unsupported TARGETARCH: ${TARGETARCH}" >&2; exit 1 ;; \ + esac \ + && wget -q https://github.com/kasmtech/KasmVNC/releases/download/v1.5.0/kasmvncserver_trixie_1.5.0_${TARGETARCH}.deb \ + && echo "${KASM_SHA256} kasmvncserver_trixie_1.5.0_${TARGETARCH}.deb" | sha256sum -c - \ + && apt-get update && apt-get install -y -f ./kasmvncserver_trixie_1.5.0_${TARGETARCH}.deb \ + && rm kasmvncserver_trixie_1.5.0_${TARGETARCH}.deb \ && rm -rf /var/lib/apt/lists/* +COPY docker/nginx.conf /etc/nginx/nginx.conf + WORKDIR /app # Python deps diff --git a/README.md b/README.md index e413c555..5f4f7f3d 100644 --- a/README.md +++ b/README.md @@ -60,19 +60,62 @@ Each CloakBrowser profile generates a completely different device identity. To t - **Per-profile settings** — fingerprint seed, proxy, timezone, locale, user agent, screen size, platform - **One-click launch/stop** — each profile runs as an isolated CloakBrowser instance - **Session persistence** — cookies, localStorage, and cache survive browser restarts -- **In-browser viewing** — interact with launched browsers via noVNC, directly in the web GUI +- **In-browser viewing** — interact with launched browsers via KasmVNC's native web client, directly in the web GUI (JPEG/WebP + optional H.264/H.265/AV1 streaming) - **Playwright/Puppeteer API** — connect to any running profile programmatically via CDP, while still watching it live in the browser - **Optional authentication** — protect the web UI and API with a single token, or run wide open locally - **Powered by CloakBrowser** — 32 source-level C++ patches, passes Cloudflare Turnstile, 0.9 reCAPTCHA v3 score ## Stack -- **Backend**: FastAPI (Python) +- **Backend**: FastAPI (Python) — control plane only (profiles, lifecycle, auth, CDP proxy) - **Frontend**: React + Tailwind CSS -- **Browser viewer**: noVNC (WebSocket-based VNC client) +- **Browser viewer**: KasmVNC 1.5 native web client (matched server/client release) +- **Data plane**: nginx — proxies the KasmVNC HTTP/WebSocket path directly; no framebuffer bytes pass through Python - **Database**: SQLite - **Browser engine**: [CloakBrowser](https://github.com/CloakHQ/CloakBrowser) (stealth Chromium binary) +## Architecture + +``` +CloakBrowser/Chromium + → KasmVNC 1.5 Xvnc (virtual X11 display, loopback-only WebSocket/HTTP) + → nginx :8080 + ├── /api/*, SPA → FastAPI (control plane, 127.0.0.1:8081) + └── /viewer//* → that profile's KasmVNC port (auth_request-gated) + → your browser (React SPA + native KasmVNC client in an iframe) +``` + +- The native KasmVNC client and server come from the **same pinned 1.5.0 package** — no protocol translation anywhere (the old noVNC compatibility bridge is gone). +- Viewer access uses short-lived, per-profile opaque tokens issued by `POST /api/profiles//viewer-token`; nginx validates every viewer request (page, assets, WebSocket upgrade) through FastAPI's `/api/viewer-auth`. Kasm ports never leave loopback. +- The Manager owns a reconnect state machine (backoff + jitter, offline/visibility handling, session-status classification). A viewer disconnect never stops the browser — reconnecting returns you to the same running session. +- Server-authoritative encoding policy: 30 FPS cap, dynamic JPEG/WebP quality, video-mode downscale under motion, bounded encoder threads. Clients cannot override server encoding settings. + +### Environment variables + +| Variable | Default | Purpose | +|----------|---------|---------| +| `AUTH_TOKEN` | *(unset = open)* | Protect the web UI + API with a token | +| `KASM_QUALITY_PRESET` | `balanced` | Encoding preset: `text`, `balanced`, `low`, `motion` | +| `KASM_RECT_THREADS` | `2` | Encoder compression threads per profile (`0` = auto/all cores — risky with many profiles) | +| `KASM_HW3D` | `auto` | DRI3 GPU acceleration: `auto` (enable unless NVIDIA proprietary), `1` (force), `0` (disable) | +| `KASM_DRINODE` | `/dev/dri/renderD128` | GPU render node for DRI3/VAAPI | + +### GPU acceleration (optional) + +Pass the host GPU into the container to enable KasmVNC DRI3 screen capture and VAAPI H.264/H.265/AV1 streaming encode (AMD/Intel open-source drivers; closed-source NVIDIA does not support DRI3): + +```bash +docker run --device /dev/dri:/dev/dri -p 8080:8080 -v cloakprofiles:/data cloakhq/cloakbrowser-manager +``` + +With Compose, GPU passthrough is an opt-in overlay (Docker refuses to create a container when a mapped device node is missing, so the default file stays GPU-free): + +```bash +docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d --build +``` + +Without a GPU everything still works (software encoding). + ## Development ### Backend @@ -172,7 +215,7 @@ When `AUTH_TOKEN` is set: - The web UI shows a login page. Enter the token to unlock. - API consumers pass the token via `Authorization: Bearer ` header. -- VNC WebSocket connections are authenticated via the login cookie. +- Viewer page + WebSocket connections are authorized through short-lived per-profile viewer tokens issued after login. - The `/api/status` endpoint remains unauthenticated (for Docker healthcheck). > **Note**: The auth token is transmitted in cleartext over HTTP. If you expose the Manager to the internet, put it behind a reverse proxy with HTTPS (Caddy, nginx, Traefik). diff --git a/backend/browser_manager.py b/backend/browser_manager.py index 327f91b5..ad903238 100644 --- a/backend/browser_manager.py +++ b/backend/browser_manager.py @@ -15,10 +15,15 @@ from cloakbrowser import launch_persistent_context_async from .vnc_manager import VNCManager +from .viewer_tokens import viewer_tokens logger = logging.getLogger("cloakbrowser.manager.browser") +class ProfileAlreadyRunning(RuntimeError): + """A launch lost the race to another launch (or to a delete in progress).""" + + def _normalize_proxy(raw: str) -> str: """Convert common proxy formats to http://user:pass@host:port. @@ -142,8 +147,75 @@ def folder(name: str, children: list) -> dict: logger.info("Set DuckDuckGo as default search for %s", user_data_dir.name) +# A wedged Playwright connection makes context.close() hang. Cleanup paths run +# under cancellation (auto_launch_all's wait_for) and under Docker's stop +# deadline, so a best-effort close must never be able to outlive either. +CONTEXT_CLOSE_TIMEOUT_S = 10.0 +# Ceiling for a single launch. Auto-launch has always enforced this; the API +# path needs it too, or a wedged Playwright call leaves the request hanging and +# the id stuck in `_launching` — which is_starting() turns into a permanent 409 +# on launch, stop and delete alike. +LAUNCH_TIMEOUT_S = 60 +# How long a profile stays blocked waiting for a browser that will not close +# before we re-examine the claim. Without a ceiling the guard has no release +# valve: launch 409s, delete 409s and stop 404s forever, so the profile is +# unusable for the life of the container. Expiry does NOT assume the browser is +# gone — a headless profile has no X server to lose, so killing Xvnc says +# nothing about it — the claim is only released once its CDP port stops +# answering. +CLOSING_CLAIM_TTL_S = 60.0 +# Absolute ceiling on a claim, however alive its CDP port looks. Ports cycle +# through 5100-5199, so a LATER profile's Chromium can end up bound to the one +# a stale claim remembers — and "something answers" would then extend that +# claim forever, bricking a profile because of an unrelated browser. +CLOSING_CLAIM_MAX_S = 600.0 + BASE_CDP_PORT = 5100 CDP_PORT_RANGE = 100 # cycle through 5100-5199 to avoid TIME_WAIT collisions +# Loopback liveness probe: a live Chromium answers instantly, a dead one +# refuses instantly. The timeout only bounds pathological cases. +_BROWSER_PROBE_TIMEOUT_S = 0.25 + + +def _port_is_listening(port: int) -> bool: + """Whether something still accepts on 127.0.0.1:port.""" + try: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.settimeout(_BROWSER_PROBE_TIMEOUT_S) + return probe.connect_ex(("127.0.0.1", port)) == 0 + except OSError: + return False + + +async def _close_context_bounded(context: Any, profile_id: str) -> bool: + """Best-effort context.close() that cannot outlive its caller. + + Shielded so a cancellation of *this* task does not abandon the close + half-done, and bounded so a wedged Playwright connection cannot hold a + cancellation (or a container shutdown) open indefinitely. + """ + if context is None or context.is_closed(): + return True + closing = asyncio.ensure_future(context.close()) + try: + # shield: a cancellation of *this* task leaves the close running rather + # than abandoning the browser half-closed. + await asyncio.wait_for(asyncio.shield(closing), timeout=CONTEXT_CLOSE_TIMEOUT_S) + except asyncio.TimeoutError: + logger.warning( + "Timed out closing browser context for %s after %.0fs; continuing", + profile_id, CONTEXT_CLOSE_TIMEOUT_S, + ) + # Let it finish in the background, but consume the eventual result so a + # later failure is not reported as a never-retrieved exception. + closing.add_done_callback( + lambda task: task.cancelled() or task.exception() is not None + ) + return False + except Exception as exc: + logger.warning("Error closing context for %s: %s", profile_id, exc) + return False + return True @dataclass @@ -159,6 +231,20 @@ class BrowserManager: def __init__(self): self.running: dict[str, RunningProfile] = {} self._launching: set[str] = set() # profile IDs currently being launched + # Auto-launch profiles queued at startup but not yet reached. Without + # this, every profile behind the one currently launching would report + # "stopped" — which an open viewer reads as a terminal session end. + self._pending_auto_launch: set[str] = set() + # Profiles being deleted. A launch must not claim one mid-delete: the + # rmtree would then run under a starting Chromium. + self._deleting: set[str] = set() + # Profiles whose browser teardown has not completed, mapped to the + # context being closed. Held for the WHOLE teardown, not just after it + # fails: the profile is out of `running` but Chromium is still alive + # and writing to user_data_dir, so a launch or delete must not treat + # the directory as free. Keyed by context so the entry is cleared by + # THAT browser's close and not by some other instance's. + self._closing: dict[str, Any] = {} self.vnc = VNCManager() self._lock = asyncio.Lock() self._next_cdp_port = BASE_CDP_PORT @@ -169,30 +255,37 @@ async def launch(self, profile: dict[str, Any]) -> RunningProfile: profile_id = profile["id"] async with self._lock: - if profile_id in self.running or profile_id in self._launching: - raise RuntimeError(f"Profile {profile_id} is already running") + if ( + profile_id in self.running + or profile_id in self._launching + or profile_id in self._deleting + or self.is_wedged(profile_id) + ): + raise ProfileAlreadyRunning(f"Profile {profile_id} is already running") self._launching.add(profile_id) - display, ws_port = await self.vnc.allocate() - + # One handler covers everything after the claim above. The filesystem + # setup below is the most failure-prone part of the function (a full or + # read-only /data, EACCES on a lock file) and used to sit outside it — + # an exception there left the id in `_launching` forever, and + # is_starting() makes that permanent and total: /launch, /stop and + # DELETE all answer 409, /viewer-token 503, and the display is never + # reclaimed. Only a container restart cleared it. + display: int | None = None + context = None try: + display, ws_port = await self.vnc.allocate() cdp_port = self._allocate_cdp_port() - except ValueError: - async with self._lock: - self._launching.discard(profile_id) - await self.vnc.stop_vnc(display) - raise - # Clean stale Chromium lock files (left by previous container crashes) - user_data_dir = Path(profile["user_data_dir"]) - for lock_file in ("SingletonLock", "SingletonCookie", "SingletonSocket"): - lock_path = user_data_dir / lock_file - lock_path.unlink(missing_ok=True) + # Clean stale Chromium lock files (left by previous container crashes) + user_data_dir = Path(profile["user_data_dir"]) + for lock_file in ("SingletonLock", "SingletonCookie", "SingletonSocket"): + lock_path = user_data_dir / lock_file + lock_path.unlink(missing_ok=True) - # Set up bookmarks and search engine on first launch - _init_profile_defaults(user_data_dir) + # Set up bookmarks and search engine on first launch + _init_profile_defaults(user_data_dir) - try: # Start KasmVNC on the allocated display await self.vnc.start_vnc( display, @@ -233,6 +326,15 @@ async def launch(self, profile: dict[str, Any]) -> RunningProfile: env={**os.environ, "DISPLAY": f":{display}"}, ) + # Register the close handler NOW, not after the setup below. The + # setup awaits, so a context that dies (or is cancelled) in that + # window would otherwise have no handler at all — and nothing would + # ever clear a wedged entry recorded for it. + closed_context = context + context.on("close", lambda: asyncio.ensure_future( + self._on_browser_closed(profile_id, closed_context) + )) + # Inject clipboard listener: captures copied text on every page # so the GET /clipboard endpoint can read it via page.evaluate() _clipboard_init_js = """ @@ -264,15 +366,29 @@ async def launch(self, profile: dict[str, Any]) -> RunningProfile: cdp_port=cdp_port, ) - # Auto-cleanup if browser crashes or user closes Chrome via VNC - context.on("close", lambda: asyncio.ensure_future( - self._on_browser_closed(profile_id) - )) - async with self._lock: self.running[profile_id] = running self._launching.discard(profile_id) + # The close event can fire between registering the handler above + # and this point (there are awaits in between). _on_browser_closed + # would then have found nothing in `running`, skipped stop_vnc, and + # returned — leaving an orphaned Xvnc holding the display/ws_port + # while we register a dead context as "running" forever. Re-check + # now that we are visible to the handler. + if running.context.is_closed(): + logger.warning( + "Browser for profile %s exited during launch registration", profile_id, + ) + # _on_browser_closed() has already reclaimed the display. Clear + # our handle so the except-path finally does NOT tear it down a + # second time: by then a concurrent relaunch may have been + # allocated that same display, and the duplicate stop_vnc would + # pop its allocation and unlink its password file. + await self._on_browser_closed(profile_id) + display = None + raise RuntimeError("Browser exited during launch") + logger.info( "Launched profile %s on display :%d (ws_port=%d, cdp_port=%d)", profile_id, display, ws_port, cdp_port, @@ -283,38 +399,210 @@ async def launch(self, profile: dict[str, Any]) -> RunningProfile: except BaseException: async with self._lock: self._launching.discard(profile_id) - await self.vnc.stop_vnc(display) + try: + # Close the browser we opened. The context.on("close") handler + # only fires if Chromium closes itself, and nothing here asks + # it to — so aborting after launch_persistent_context_async() + # returned would leave a live Chromium with its X server killed + # out from under it, holding cdp_port (permanently skipped by + # _allocate_cdp_port) and still writing to user_data_dir. A + # later relaunch deletes the Singleton* locks and opens a + # SECOND Chromium on the same profile. + # Claim BEFORE closing, exactly as stop() does. The profile is + # already out of `_launching` and the close below can take + # CONTEXT_CLOSE_TIMEOUT_S, so claiming only on failure leaves + # the whole cleanup window unowned: a concurrent launch would + # start a second Chromium on the live user_data_dir and a + # concurrent DELETE would rmtree under it. The claim persists + # if the close never lands, and the handler registered above + # clears it when it does. + if context is not None: + self._closing[profile_id] = ( + context, time.monotonic() + CLOSING_CLAIM_TTL_S, cdp_port, + time.monotonic(), + ) + if await _close_context_bounded(context, profile_id): + self._closing.pop(profile_id, None) + else: + logger.warning( + "Aborted launch for %s left a browser that did not close", + profile_id, + ) + finally: + # In a finally: if the close hangs past its bound (or we are + # being cancelled) the display must still be reclaimed, or + # wait_for() never returns and every queued auto-launch profile + # stays stuck reporting "starting". + if display is not None: + await self.vnc.stop_vnc(display) raise - async def _on_browser_closed(self, profile_id: str): - """Called when browser exits (crash, user closed via VNC, or stop()).""" + async def _on_browser_closed(self, profile_id: str, context: Any = None): + """Called when a browser exits (crash, closed via VNC, or stop()). + + `context` identifies which instance closed. A late close from a + superseded context must be ignored: acting on profile_id alone would + pop and tear down the replacement that now owns this id, killing a + freshly launched session and orphaning its Chromium. + """ + # This browser is genuinely gone now. Resolve its wedge FIRST — before + # the superseded-context check below, which returns early — otherwise a + # wedged context whose close lands while another instance holds the id + # would leave the profile blocked forever. + entry = self._closing.get(profile_id) + if context is not None and entry is not None and entry[0] is context: + logger.info("Browser for profile %s finished closing", profile_id) + del self._closing[profile_id] + async with self._lock: + current = self.running.get(profile_id) + if context is not None and current is not None and current.context is not context: + logger.info( + "Ignoring close from a superseded context for profile %s", profile_id, + ) + return running = self.running.pop(profile_id, None) + if context is None: + # Called without an identity (the launch-time is_closed re-check): + # nothing else can be holding a wedge for this profile. + self._closing.pop(profile_id, None) + viewer_tokens.revoke_profile(profile_id) + if running: logger.info("Browser closed for profile %s, cleaning up", profile_id) await self.vnc.stop_vnc(running.display) - async def stop(self, profile_id: str): - """Stop a running browser instance.""" + async def stop(self, profile_id: str) -> bool: + """Stop a running browser instance. + + Returns whether the browser actually closed. False means Chromium is + still alive (a wedged teardown) — callers must not then treat the + profile directory as free to delete. + """ # Pop before close so _on_browser_closed() finds nothing to clean up async with self._lock: running = self.running.pop(profile_id, None) + viewer_tokens.revoke_profile(profile_id) + if not running: - return + return True logger.info("Stopping profile %s", profile_id) + # Claim BEFORE closing, not after it fails. The profile is already out + # of `running` and the close below takes up to CONTEXT_CLOSE_TIMEOUT_S, + # so recording it only on failure leaves the entire teardown window + # unguarded — a concurrent DELETE would rmtree user_data_dir, and a + # concurrent launch would start a second Chromium, while the first is + # still closing. The claim simply persists if the close never lands. + self._closing[profile_id] = ( + running.context, time.monotonic() + CLOSING_CLAIM_TTL_S, running.cdp_port, + time.monotonic(), + ) try: - await running.context.close() - except Exception as exc: - logger.warning("Error closing context for %s: %s", profile_id, exc) + # Bounded: cleanup_all() runs this for every profile inside Docker's + # stop grace period, so one wedged context must not hold it open. + closed = await _close_context_bounded(running.context, profile_id) + if closed: + self._closing.pop(profile_id, None) + else: + logger.warning( + "Browser for profile %s did not close; blocking launch/delete until it does", + profile_id, + ) + except BaseException: + # The guard is deliberately NOT released here. The close is + # shielded, so on cancellation it keeps running and Chromium may + # still be alive — dropping it now would let a relaunch or delete + # race a live browser. The claim is self-clearing: the close + # handler resolves it, and failing that the TTL does. + raise + finally: + # Xvnc is ours regardless of how the close went, and reclaiming it + # is independent of whether the browser exited. Shielded so a + # cancelled stop() still frees the display, its ws_port and its + # password file instead of leaking all three for the process + # lifetime. + await asyncio.shield( + asyncio.ensure_future(self.vnc.stop_vnc(running.display)) + ) + # The close may have landed while Xvnc was being torn down; report what + # is true now rather than what was true a moment ago. + return not self.is_wedged(profile_id) + + def claim_for_delete(self, profile_id: str) -> bool: + """Block launches for the duration of a delete. + + Exclusive: a second concurrent delete must not share the claim, or the + first to finish would release it while the second is still running and + reopen the launch window this exists to close. + """ + if profile_id in self._deleting: + return False + self._deleting.add(profile_id) + return True + + def release_delete_claim(self, profile_id: str) -> None: + self._deleting.discard(profile_id) + + def is_wedged(self, profile_id: str) -> bool: + """True while a stopped profile's Chromium has not finished exiting. + + Bounded: an expired claim is released so the profile cannot be blocked + forever by a browser that never reports its close. + """ + entry = self._closing.get(profile_id) + if entry is None: + return False + _context, deadline, cdp_port, claimed_at = entry + if time.monotonic() < deadline: + return True + # Deadline reached: decide on evidence, not on a timer. A headless + # profile keeps running after stop_vnc() kills the display, so + # releasing on time alone would hand a live browser's profile directory + # to a relaunch or a delete. + held_for = time.monotonic() - claimed_at + if ( + cdp_port is not None + and held_for < CLOSING_CLAIM_MAX_S + and _port_is_listening(cdp_port) + ): + logger.error( + "Browser for profile %s still alive on cdp port %d after %.0fs; " + "keeping the guard", + profile_id, cdp_port, held_for, + ) + self._closing[profile_id] = ( + _context, time.monotonic() + CLOSING_CLAIM_TTL_S, cdp_port, claimed_at, + ) + return True + if held_for >= CLOSING_CLAIM_MAX_S: + logger.error( + "Releasing the guard on profile %s after %.0fs: the cdp port may " + "now belong to an unrelated browser, and a profile must not stay " + "blocked indefinitely", + profile_id, held_for, + ) + logger.warning( + "Browser for profile %s never reported closing but is gone; releasing the guard", + profile_id, + ) + del self._closing[profile_id] + return False - await self.vnc.stop_vnc(running.display) + def is_starting(self, profile_id: str) -> bool: + """True while a launch is in flight or queued behind auto-launch.""" + return profile_id in self._launching or profile_id in self._pending_auto_launch def get_status(self, profile_id: str) -> dict[str, Any]: - """Get running status for a profile.""" + """Cheap lifecycle status: running | starting | stopped. + + Deliberately does no process probing — the profile list polls this for + every profile every 3 seconds and discards liveness. Use + get_liveness() where the answer actually matters. + """ running = self.running.get(profile_id) if running: return { @@ -323,15 +611,76 @@ def get_status(self, profile_id: str) -> dict[str, Any]: "display": f":{running.display}", "cdp_url": f"/api/profiles/{profile_id}/cdp", } - return {"status": "stopped", "vnc_ws_port": None, "display": None, "cdp_url": None} + # "starting" is distinct from "stopped" on purpose: an open viewer + # treats "stopped" as terminal, so reporting it during a container + # restart would kill a session that is about to come back. + return { + "status": "starting" if self.is_starting(profile_id) else "stopped", + "vnc_ws_port": None, + "display": None, + "cdp_url": None, + } + + async def get_liveness_async(self, profile_id: str) -> dict[str, Any]: + """get_liveness() with the blocking probes off the event loop. + + _browser_alive() opens a TCP connection; on loopback that is normally + instantaneous, but "normally" is not "always" and this runs in a request + handler on the single event loop that also serves nginx's viewer + auth_request subrequests. + """ + return await asyncio.get_running_loop().run_in_executor( + None, self.get_liveness, profile_id, + ) + + def get_liveness(self, profile_id: str) -> dict[str, Any]: + """Lifecycle status plus real Xvnc/browser liveness. + + Drives the viewer's reconnect classification, so both flags have to + observe the actual processes rather than local bookkeeping. + """ + status = self.get_status(profile_id) + running = self.running.get(profile_id) + return { + **status, + "xvnc_alive": self.vnc.is_alive(running.display) if running else None, + "browser_alive": self._browser_alive(running) if running else None, + } + + @staticmethod + def _browser_alive(running: RunningProfile) -> bool: + """Whether Chromium is still up, via its DevTools listener. + + `context.pages` cannot answer this: Playwright implements it as + `self._pages.copy()`, a purely local list copy that never touches the + connection and never raises — so it returns True for a browser that is + already dead or wedged, making the viewer's "browser-dead" + classification unreachable. The CDP port is bound by the Chromium + process and dies with it, so a loopback connect is the real signal. + Cheap either way: sub-millisecond when up, immediate ECONNREFUSED + when not. + """ + return _port_is_listening(running.cdp_port) async def cleanup_all(self): - """Stop all running profiles. Called on shutdown.""" + """Stop all running profiles. Called on shutdown. + + Concurrently: profiles are independent, and the entrypoint waits for + this before tearing down nginx, inside Docker's stop grace period. + Sequentially, shutdown cost is the SUM of every context close plus Xvnc + teardown, so a handful of profiles is enough to get the container + SIGKILLed mid-cleanup — killing uncleanly the very browsers the ordered + shutdown exists to protect. + """ async with self._lock: profile_ids = list(self.running.keys()) - for pid in profile_ids: - await self.stop(pid) + results = await asyncio.gather( + *(self.stop(pid) for pid in profile_ids), return_exceptions=True, + ) + for pid, result in zip(profile_ids, results): + if isinstance(result, BaseException): + logger.warning("Error stopping profile %s during shutdown: %s", pid, result) await self.vnc.cleanup_all() @@ -349,16 +698,27 @@ async def auto_launch_all(self): logger.info("No profiles configured for auto-launch") return + # Claim the whole queue up front so profiles waiting their turn report + # "starting" rather than "stopped" — launches are sequential and the + # last one can be minutes away. + self._pending_auto_launch = {p["id"] for p in auto_profiles} + logger.info("Auto-launching %d profile(s)...", len(auto_profiles)) - for profile in auto_profiles: - try: - await asyncio.wait_for(self.launch(profile), timeout=60) - logger.info("Auto-launched profile %s (%s)", profile["name"], profile["id"]) - except Exception as exc: - logger.error( - "Auto-launch failed for profile %s (%s): %s", - profile["name"], profile["id"], exc, - ) + try: + for profile in auto_profiles: + try: + await asyncio.wait_for(self.launch(profile), timeout=LAUNCH_TIMEOUT_S) + logger.info("Auto-launched profile %s (%s)", profile["name"], profile["id"]) + except Exception as exc: + logger.error( + "Auto-launch failed for profile %s (%s): %s", + profile["name"], profile["id"], exc, + ) + finally: + self._pending_auto_launch.discard(profile["id"]) + finally: + # Cancellation (shutdown) must not leave profiles stuck "starting". + self._pending_auto_launch.clear() logger.info("Auto-launch complete: %d running", len(self.running)) def _allocate_cdp_port(self) -> int: diff --git a/backend/main.py b/backend/main.py index 92727a53..5fb46ef3 100644 --- a/backend/main.py +++ b/backend/main.py @@ -8,9 +8,10 @@ import asyncio import hmac +import json import logging import os -import struct +import re import shutil from contextlib import asynccontextmanager from http.cookies import SimpleCookie @@ -25,7 +26,7 @@ from starlette.types import ASGIApp, Receive, Scope, Send from . import database as db -from .browser_manager import BrowserManager +from .browser_manager import LAUNCH_TIMEOUT_S, BrowserManager, ProfileAlreadyRunning from .models import ( ClipboardRequest, LaunchResponse, @@ -36,7 +37,9 @@ ProfileUpdate, StatusResponse, TagResponse, + ViewerTokenResponse, ) +from .viewer_tokens import VIEWER_TOKEN_TTL, viewer_tokens logger = logging.getLogger("cloakbrowser.manager") logging.basicConfig(level=logging.INFO, format="%(asctime)s %(name)s %(levelname)s %(message)s") @@ -51,7 +54,8 @@ AUTH_TOKEN: str | None = os.environ.get("AUTH_TOKEN") or None # Paths that bypass authentication even when AUTH_TOKEN is set -_AUTH_EXEMPT = frozenset({"/api/auth/status", "/api/auth/login", "/api/status"}) +# (/api/viewer-auth is exempt: the viewer token in X-Original-URI is the credential) +_AUTH_EXEMPT = frozenset({"/api/auth/status", "/api/auth/login", "/api/status", "/api/viewer-auth"}) def _check_auth(scope: Scope) -> bool: @@ -180,198 +184,6 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send): FRONTEND_DIR = Path(__file__).parent.parent / "frontend" / "dist" -# --------------------------------------------------------------------------- -# RFB server message translator — KasmVNC BinaryClipboard → standard RFB -# --------------------------------------------------------------------------- - - -def _parse_kasmvnc_clipboard(data: bytes) -> str | None: - """Extract text/plain from KasmVNC BinaryClipboard (type 180). - - Format: type(1) + action(1) + flags(4) + entries... - Each entry: mime_len(u8) + mime(N) + data_len(u32 BE) + data(M) - """ - if len(data) < 7: - return None - offset = 6 # skip type(1) + action(1) + flags(4) - while offset < len(data): - if offset + 1 > len(data): - break - mime_len = data[offset] - offset += 1 - if offset + mime_len > len(data): - break - mime_type = data[offset:offset + mime_len] - offset += mime_len - if offset + 4 > len(data): - break - data_len = struct.unpack_from(">I", data, offset)[0] - offset += 4 - if mime_type == b"text/plain": - end = min(offset + data_len, len(data)) - return data[offset:end].decode("utf-8", errors="replace") - offset += data_len - return None - - -def _build_server_cut_text(text: str) -> bytes: - """Build standard RFB ServerCutText (type 3) message. - - RFB spec mandates Latin-1 encoding for ServerCutText. - Characters outside Latin-1 (CJK, emoji, etc.) are replaced with '?'. - """ - text_bytes = text.encode("latin-1", errors="replace") - return struct.pack(">BxxxI", 3, len(text_bytes)) + text_bytes - - -# --------------------------------------------------------------------------- -# RFB client message filter — strip extension types KasmVNC doesn't support -# --------------------------------------------------------------------------- -# noVNC v1.4 batches multiple RFB messages into one WebSocket frame. -# KasmVNC 1.3.3 crashes on unsupported types (150, 248, etc.). -# We parse message boundaries using known sizes and keep only standard types. - -# Client→server message sizes (fixed, except 2 and 6 which encode length) -_RFB_MSG_SIZE: dict[int, int | None] = { - 0: 20, # SetPixelFormat - 2: None, # SetEncodings — 4 + numEncodings*4 (rewritten to strip bad pseudo-encodings) - 3: 10, # FramebufferUpdateRequest - 4: 8, # KeyEvent - 5: 6, # PointerEvent - 6: None, # ClientCutText — 8 + length -} - -# Extension types that noVNC sends — known sizes so we can skip past them -# instead of breaking and dropping all trailing data in the frame. -_RFB_EXTENSION_SIZE: dict[int, int] = { - 150: 10, # EnableContinuousUpdates (1+1+2+2+2+2) - 248: 10, # QEMU-like key event (observed from noVNC 1.4.0) - 252: 4, # xvp (1+1+1+1) - 255: 4, # QEMU audio control (1+1+2) — noVNC QEMUExtendedKeyEvent is actually 12 -} - -# Whitelist of encodings safe to send to KasmVNC. -# Instead of trying to blocklist problematic pseudo-encodings (error-prone — -# we had wrong numbers), we ONLY keep known-good encodings. -# Anything not on this list is stripped from SetEncodings. -_ALLOWED_ENCODINGS: set[int] = { - # Framebuffer encodings (standard RFB) - 0, # Raw - 1, # CopyRect - 2, # RRE - 5, # Hextile - 7, # Tight - 16, # ZRLE - # Safe pseudo-encodings - -239, # Cursor (0xFFFFFF11) — cursor shape - -224, # LastRect (0xFFFFFF20) — performance optimization - # Tight quality/compress levels (these are just hints) - *range(-32, -22), # quality levels 0-9 - *range(-256, -246), # compress levels 0-9 -} - - -def _rfb_msg_length(data: bytes, offset: int) -> int | None: - """Return total length of the RFB message at offset, or None if unrecognized.""" - if offset >= len(data): - return None - msg_type = data[offset] - fixed = _RFB_MSG_SIZE.get(msg_type) - if fixed is not None: - return fixed - remaining = len(data) - offset - if msg_type == 2 and remaining >= 4: # SetEncodings - num_enc = struct.unpack_from(">H", data, offset + 2)[0] - return 4 + num_enc * 4 - if msg_type == 6 and remaining >= 8: # ClientCutText - length = struct.unpack_from(">I", data, offset + 4)[0] - return 8 + length - # Known extension types — skip past them instead of giving up - ext_size = _RFB_EXTENSION_SIZE.get(msg_type) - if ext_size is not None: - return ext_size - return None # truly unknown type - - -def _rewrite_set_encodings(data: bytes, offset: int, msg_len: int) -> bytes: - """Keep only whitelisted encodings in a SetEncodings message.""" - _log = logging.getLogger("cloakbrowser.manager") - num_enc = struct.unpack_from(">H", data, offset + 2)[0] - kept = [] - stripped = [] - for i in range(num_enc): - enc = struct.unpack_from(">i", data, offset + 4 + i * 4)[0] # signed - if enc in _ALLOWED_ENCODINGS: - kept.append(enc) - else: - stripped.append(enc) - if not stripped: - return data[offset:offset + msg_len] - _log.info("RFB filter: SetEncodings keeping %d: %s, stripped %d: %s", len(kept), kept, len(stripped), stripped) - result = struct.pack(">BxH", 2, len(kept)) - for enc in kept: - result += struct.pack(">i", enc) - return result - - -def _rewrite_pointer_event(data: bytes, offset: int) -> bytes: - """Convert standard 6-byte PointerEvent to KasmVNC's 11-byte format. - - Standard RFB: [5:u8][mask:u8][x:u16][y:u16] = 6 bytes - KasmVNC: [5:u8][mask:u16][x:u16][y:u16][sx:s16][sy:s16] = 11 bytes - """ - mask = data[offset + 1] - x = struct.unpack_from(">H", data, offset + 2)[0] - y = struct.unpack_from(">H", data, offset + 4)[0] - # Expand mask from u8 to u16. Scroll deltas (sx, sy) are zero because - # noVNC encodes scroll as button-mask bits (3=up, 4=down, 5=left, 6=right) - # which pass through in the mask. KasmVNC accepts mask-bit scroll on its - # extended 11-byte format, so explicit deltas are unnecessary. - return struct.pack(">BHHHhh", 5, mask, x, y, 0, 0) - - -def _filter_rfb_client_messages(data: bytes) -> bytes: - """Parse concatenated RFB messages, keep only standard types (0-6). - - Rewrites PointerEvents from 6-byte standard to 11-byte KasmVNC format - and strips unsupported pseudo-encodings from SetEncodings. - """ - _log = logging.getLogger("cloakbrowser.manager") - result = bytearray() - offset = 0 - msg_idx = 0 - while offset < len(data): - msg_type = data[offset] - msg_len = _rfb_msg_length(data, offset) - if msg_len is None: - _log.info("RFB filter: DROPPING unknown type=%d at offset=%d/%d, skipping %d trailing bytes, hex=%s", - msg_type, offset, len(data), len(data) - offset, data[offset:offset+20].hex()) - break - if offset + msg_len > len(data): - # Incomplete message — DO NOT forward partial data, it desynchronizes - # the RFB stream (KasmVNC buffers partial reads across frames). - _log.warning("RFB filter: DROPPING incomplete type=%d need=%d have=%d — would desync stream", - msg_type, msg_len, len(data) - offset) - break - msg_idx += 1 - if msg_type in _RFB_MSG_SIZE: - # Standard RFB type — keep (with rewrites for KasmVNC compatibility) - _log.debug("RFB filter: KEEP type=%d len=%d at offset=%d (msg #%d in frame)", msg_type, msg_len, offset, msg_idx) - if msg_type == 2: # SetEncodings — whitelist safe encodings - result.extend(_rewrite_set_encodings(data, offset, msg_len)) - elif msg_type == 5: # PointerEvent — expand to KasmVNC's 11-byte format - result.extend(_rewrite_pointer_event(data, offset)) - else: - result.extend(data[offset:offset + msg_len]) - else: - # Extension type (150, 248, etc.) — skip but continue parsing - _log.debug("RFB filter: SKIP extension type=%d len=%d at offset=%d (msg #%d in frame)", msg_type, msg_len, offset, msg_idx) - offset += msg_len - if len(result) != len(data): - _log.info("RFB filter: input=%d output=%d (delta %+d bytes)", len(data), len(result), len(result) - len(data)) - return bytes(result) - - @asynccontextmanager async def lifespan(app: FastAPI): db.init_db() @@ -499,9 +311,50 @@ async def update_profile(profile_id: str, req: ProfileUpdate): @app.delete("/api/profiles/{profile_id}") async def delete_profile(profile_id: str): + # A launch in flight owns the user_data_dir: deleting now would rmtree it + # under a Chromium that is still starting, and the launch would then + # register a RunningProfile for a profile that no longer exists. + if browser_mgr.is_starting(profile_id): + raise HTTPException(status_code=409, detail="Profile is starting; try again") + + # Hold the id for the whole delete: the re-check after stop() narrows the + # launch race but cannot close it, because a launch can still claim the + # profile between that check and the rmtree. + if not browser_mgr.claim_for_delete(profile_id): + raise HTTPException(status_code=409, detail="Profile is already being deleted") + try: + return await _delete_profile_locked(profile_id) + finally: + browser_mgr.release_delete_claim(profile_id) + + +async def _delete_profile_locked(profile_id: str): + # A previous /stop may have left Chromium alive (bounded close timed out). + # It is out of `running`, so the stop below would be skipped and the rmtree + # would run under a live browser. + if browser_mgr.is_wedged(profile_id): + raise HTTPException( + status_code=409, detail="Browser is still shutting down; try again", + ) + # Stop browser if running if profile_id in browser_mgr.running: - await browser_mgr.stop(profile_id) + closed = await browser_mgr.stop(profile_id) + # stop() awaits a context close, and a launch can claim the profile + # during that window. Deleting now would rmtree user_data_dir under a + # Chromium that is starting on it. Re-check rather than race. + if browser_mgr.is_starting(profile_id) or profile_id in browser_mgr.running: + raise HTTPException(status_code=409, detail="Profile restarted; try again") + if not closed: + # The teardown outlived its bound, so Chromium may still be alive + # and writing to user_data_dir. Deleting it now corrupts a live + # profile; refuse rather than race a wedged browser. + raise HTTPException( + status_code=409, detail="Browser is still shutting down; try again", + ) + + # Revoke viewer tokens even if the profile wasn't running + viewer_tokens.revoke_profile(profile_id) profile = db.get_profile(profile_id) if not profile: @@ -512,9 +365,14 @@ async def delete_profile(profile_id: str): # DB first — if this fails, filesystem is untouched db.delete_profile(profile_id) - # Then clean up disk + # Then clean up disk. Off the event loop: a Chromium profile directory can + # be hundreds of MB, and rmtree here would block every other request for + # its duration — including the nginx auth_request subrequests that gate the + # viewer, stalling live sessions on an unrelated delete. if user_data_dir.exists(): - shutil.rmtree(user_data_dir, ignore_errors=True) + await asyncio.get_running_loop().run_in_executor( + None, lambda: shutil.rmtree(user_data_dir, ignore_errors=True), + ) return {"ok": True} @@ -529,9 +387,21 @@ async def launch_profile(profile_id: str): raise HTTPException(status_code=404, detail="Profile not found") if profile_id in browser_mgr.running: raise HTTPException(status_code=409, detail="Profile is already running") + if browser_mgr.is_starting(profile_id): + raise HTTPException(status_code=409, detail="Profile is already starting") try: - running = await browser_mgr.launch(profile) + # Same ceiling auto-launch uses: without it a wedged Playwright call + # hangs the request and strands the id in `_launching` forever. + running = await asyncio.wait_for( + browser_mgr.launch(profile), timeout=LAUNCH_TIMEOUT_S, + ) + except asyncio.TimeoutError: + logger.error("Launch timed out for profile %s", profile_id) + raise HTTPException(status_code=504, detail="Launch timed out") + except ProfileAlreadyRunning: + # Lost a race with another launch (or a delete) after the checks above. + raise HTTPException(status_code=409, detail="Profile is already running") except ValueError as exc: raise HTTPException(status_code=400, detail=str(exc)) except Exception as exc: @@ -550,9 +420,15 @@ async def launch_profile(profile_id: str): @app.post("/api/profiles/{profile_id}/stop") async def stop_profile(profile_id: str): if profile_id not in browser_mgr.running: + # Mid-launch is not "not running" — stopping now would race the launch + # into registering the instance we just tore down. + if browser_mgr.is_starting(profile_id): + raise HTTPException(status_code=409, detail="Profile is starting; try again") raise HTTPException(status_code=404, detail="Profile is not running") - await browser_mgr.stop(profile_id) - return {"ok": True} + closed = await browser_mgr.stop(profile_id) + # Report honestly: the display is reclaimed either way, but a wedged + # Chromium means the profile is not cleanly down. + return {"ok": True, "browser_closed": closed} @app.get("/api/profiles/{profile_id}/status", response_model=ProfileStatusResponse) @@ -560,10 +436,166 @@ async def get_profile_status(profile_id: str): profile = db.get_profile(profile_id) if not profile: raise HTTPException(status_code=404, detail="Profile not found") - status = browser_mgr.get_status(profile_id) + status = await browser_mgr.get_liveness_async(profile_id) return ProfileStatusResponse(**status) +# ── Viewer Sessions (KasmVNC native client) ────────────────────────────────── +# FastAPI is the control plane only: it issues short-lived viewer tokens and +# authorizes viewer requests for nginx (auth_request). nginx proxies the +# actual page assets and WebSocket straight to KasmVNC's own HTTP server. + + +def _extract_viewer_token(uri: str) -> str | None: + """Extract the viewer token from an X-Original-URI like /viewer//...""" + path = uri.split("?", 1)[0] + parts = path.split("/") + # expect ["", "viewer", "", ...] + if len(parts) >= 3 and parts[1] == "viewer" and parts[2]: + return parts[2] + return None + + +@app.post("/api/profiles/{profile_id}/viewer-token", response_model=ViewerTokenResponse) +async def create_viewer_token(profile_id: str): + """Issue a fresh short-lived viewer token for a running profile. + + Each page load calls this once; the token then authorizes both the viewer + page assets and the WebSocket (via /api/viewer-auth) until expiry. + """ + running = browser_mgr.running.get(profile_id) + if not running: + # A profile mid-launch (container restart, auto-launch queue) is not + # gone — 404 is terminal to the viewer, 503 tells it to keep retrying. + if browser_mgr.is_starting(profile_id): + raise HTTPException(status_code=503, detail="Profile is starting") + raise HTTPException(status_code=404, detail="Profile not running") + token = viewer_tokens.issue(profile_id, running.ws_port, ttl=VIEWER_TOKEN_TTL) + logger.info("Issued viewer token for profile %s (ttl=%ds)", profile_id, VIEWER_TOKEN_TTL) + return ViewerTokenResponse( + token=token, + viewer_url=f"/viewer/{token}/", + expires_in=VIEWER_TOKEN_TTL, + ) + + +@app.get("/api/viewer-auth") +async def viewer_auth(request: Request): + """Authorize a viewer request for nginx auth_request. + + Exempt from app auth — the viewer token itself is the credential. + On success returns 200 with X-Viewer-Upstream so nginx can proxy to the + right KasmVNC instance. Never logs the token value. + """ + token = _extract_viewer_token(request.headers.get("x-original-uri", "")) + if not token: + raise HTTPException(status_code=403, detail="Missing viewer token") + + session = viewer_tokens.validate(token) + if not session: + raise HTTPException(status_code=403, detail="Invalid or expired viewer token") + + running = browser_mgr.running.get(session.profile_id) + if not running: + # 403, not 404: nginx auth_request only understands 2xx (allow) and + # 401/403 (deny). Anything else is "auth request unexpected status" + # and the viewer gets a bare 500 instead of a clean denial. + raise HTTPException(status_code=403, detail="Profile not running") + + # Upstream and credentials must come from the same source of truth. The + # token carries the ws_port it was minted for; the credentials come from + # the live display. If a token ever outlives a relaunch, mixing the two + # would point nginx at a stale port while handing it valid-looking auth + # for a different display. + if session.ws_port != running.ws_port: + logger.info("Viewer token for profile %s predates a relaunch", session.profile_id) + raise HTTPException(status_code=403, detail="Viewer token is stale") + + headers = {"X-Viewer-Upstream": f"127.0.0.1:{running.ws_port}"} + # Kasm's HTTP layer requires Basic auth; nginx injects this on the + # origin request so the browser never handles the credentials. + creds = browser_mgr.vnc.get_api_credentials(running.display) + if creds: + import base64 + + raw = f"{creds[0]}:{creds[1]}".encode() + headers["X-Viewer-Authorization"] = "Basic " + base64.b64encode(raw).decode() + + return Response(status_code=200, headers=headers) + + +@app.get("/api/profiles/{profile_id}/kasm-stats") +async def kasm_stats(profile_id: str): + """Fetch KasmVNC's bottleneck/frame stats for a running profile.""" + running = browser_mgr.running.get(profile_id) + if not running: + raise HTTPException(status_code=404, detail="Profile not running") + + try: + # Kasm's management API always requires owner Basic auth (random + # per-display credentials generated at Xvnc launch). + auth = browser_mgr.vnc.get_api_credentials(running.display) + async with httpx.AsyncClient(timeout=5, auth=auth) as client: + bottleneck_resp, sessions_resp = await asyncio.gather( + client.get(f"http://127.0.0.1:{running.ws_port}/api/get_bottleneck_stats"), + client.get(f"http://127.0.0.1:{running.ws_port}/api/get_sessions"), + ) + # Kasm answers 401 (bad/missing owner creds) or 5xx with an HTML + # body. _json_or_raw would happily hand that back as the payload, + # so the caller would see 200 with an error page where the stats + # should be — indistinguishable from real data. + for label, resp in (("bottleneck", bottleneck_resp), ("sessions", sessions_resp)): + if resp.status_code >= 400: + logger.error( + "Kasm stats: %s returned HTTP %d for %s", + label, resp.status_code, profile_id, + ) + raise HTTPException( + status_code=502, + detail=f"KasmVNC stats endpoint returned {resp.status_code}", + ) + # With no viewer connected these can be empty/non-JSON. + bottleneck = _json_or_raw(bottleneck_resp) + sessions = _json_or_raw(sessions_resp) + + # /api/get_frame_stats only works while the client collects perf + # stats (client-side enable_perf_stats); otherwise it 503s after + # a 10s wait. Isolate it so bottleneck/sessions still return. + frame = None + if isinstance(sessions, dict) and sessions.get("users"): + try: + frame_resp = await client.get( + f"http://127.0.0.1:{running.ws_port}/api/get_frame_stats", + params={"client": "all"}, + ) + frame = _json_or_raw(frame_resp) + except Exception as exc: + logger.debug("Kasm frame stats unavailable for %s: %s", profile_id, exc) + except HTTPException: + raise + except Exception as exc: + logger.error("Kasm stats: failed to reach KasmVNC for %s: %s", profile_id, exc) + raise HTTPException(status_code=502, detail="KasmVNC stats endpoint unreachable") + + return {"bottleneck": bottleneck, "sessions": sessions, "frame": frame} + + +def _json_or_raw(resp: httpx.Response): + """Parse a Kasm API response as JSON, falling back to raw text/None. + + Kasm emits `-nan` for not-yet-measured stats, which is not valid JSON — + normalize it to null before parsing. + """ + try: + return resp.json() + except ValueError: + pass + try: + return json.loads(re.sub(r"-?nan\b", "null", resp.text)) + except ValueError: + return resp.text or None + + # ── System Status ───────────────────────────────────────────────────────────── @@ -671,175 +703,8 @@ async def get_clipboard(profile_id: str): return {"text": text} -# ── VNC WebSocket Proxy ────────────────────────────────────────────────────── - - -@app.websocket("/api/profiles/{profile_id}/vnc") -async def vnc_proxy(websocket: WebSocket, profile_id: str): - """Proxy WebSocket frames between the frontend and a profile's KasmVNC.""" - if not await _check_websocket_origin(websocket): - return - - running = browser_mgr.running.get(profile_id) - if not running: - await websocket.close(code=4004, reason="Profile not running") - return - - # Accept with client's requested subprotocol (if any) — RFC 6455 requires - # the server must not respond with a subprotocol the client didn't request. - requested = websocket.scope.get("subprotocols", []) - subprotocol = "binary" if "binary" in requested else None - await websocket.accept(subprotocol=subprotocol) - - import websockets - - vnc_url = f"ws://127.0.0.1:{running.ws_port}/websockify" - - try: - async with websockets.connect( - vnc_url, - subprotocols=["binary"], - origin=f"http://127.0.0.1:{running.ws_port}", - max_size=None, # VNC frames can be large (1920x1080 framebuffer) - ping_interval=None, # KasmVNC doesn't respond to WS pings - ping_timeout=None, - compression=None, # KasmVNC can't handle permessage-deflate - ) as vnc_ws: - logger.info( - "VNC proxy: connected to KasmVNC for %s (subprotocol=%s)", - profile_id, vnc_ws.subprotocol, - ) - - # noVNC v1.4 sends extension message types (150=ContinuousUpdates, - # 248=QEMUKey, etc.) that KasmVNC 1.3.3 doesn't support, causing - # "unknown message type" → disconnect. - # - # noVNC batches multiple RFB messages into a single WebSocket frame, - # so we must parse the RFB stream to find message boundaries and strip - # unsupported types before forwarding. Standard client→server types - # have known fixed sizes (except SetEncodings and ClientCutText which - # encode their length). - - async def client_to_vnc(): - count = 0 - handshake = 0 # first 3 messages are RFB handshake - dropped = 0 - try: - while True: - msg = await websocket.receive() - msg_type = msg.get("type", "") - if msg_type == "websocket.disconnect": - logger.info("VNC proxy [c->v]: client disconnect (code=%s) after %d msgs (%d dropped)", msg.get("code"), count, dropped) - break - if "bytes" in msg and msg["bytes"]: - count += 1 - data = msg["bytes"] - handshake += 1 - - # First 3 messages are RFB handshake — forward as-is - if handshake <= 3: - logger.debug("VNC handshake #%d: %d bytes hex=%s", handshake, len(data), data[:20].hex()) - await vnc_ws.send(data) - continue - - # Parse RFB messages and strip unsupported types - filtered = _filter_rfb_client_messages(data) - if filtered: - # Safety: verify first byte is a valid RFB client type - if filtered[0] not in _RFB_MSG_SIZE: - logger.error("RFB SAFETY: refusing to send data with invalid first byte=%d hex=%s", - filtered[0], filtered[:20].hex()) - dropped += 1 - continue - logger.debug("VNC send: %d bytes first_type=%d hex=%s", len(filtered), filtered[0], filtered[:100].hex()) - await vnc_ws.send(filtered) - else: - dropped += 1 - - elif "text" in msg and msg["text"]: - # noVNC only sends binary frames — text frames are unexpected - # and would bypass the RFB filter, so drop them. - count += 1 - logger.warning("VNC proxy [c->v]: DROPPING text frame len=%d (noVNC should only send binary)", len(msg["text"])) - dropped += 1 - else: - logger.warning("VNC proxy [c->v]: unhandled msg keys=%s type=%s", list(msg.keys()), msg_type) - except WebSocketDisconnect as exc: - logger.info("VNC proxy [c->v]: WebSocketDisconnect code=%s after %d msgs (%d dropped)", exc.code, count, dropped) - except Exception as exc: - logger.warning("VNC proxy [c->v]: %s: %s (after %d msgs)", type(exc).__name__, exc, count) - - async def vnc_to_client(): - count = 0 - try: - async for msg in vnc_ws: - count += 1 - if isinstance(msg, bytes) and len(msg) > 0: - msg_type = msg[0] - if msg_type == 180: - # KasmVNC BinaryClipboard → convert to standard - # ServerCutText (type 3) so noVNC can handle it - text = _parse_kasmvnc_clipboard(msg) - if text: - logger.info("VNC proxy [v->c]: clipboard %d chars", len(text)) - await websocket.send_bytes(_build_server_cut_text(text)) - else: - logger.info("VNC proxy [v->c]: dropped type 180 (no text/plain)") - continue - await websocket.send_bytes(msg) - elif isinstance(msg, bytes): - await websocket.send_bytes(msg) - else: - await websocket.send_text(msg) - logger.info("VNC proxy [v->c]: KasmVNC stream ended after %d msgs (close_code=%s)", count, vnc_ws.close_code) - except WebSocketDisconnect as exc: - logger.info("VNC proxy [v->c]: client disconnect code=%s after %d msgs", exc.code, count) - except Exception as exc: - logger.warning("VNC proxy [v->c]: %s: %s (after %d msgs)", type(exc).__name__, exc, count) - - c2v = asyncio.create_task(client_to_vnc(), name="c2v") - v2c = asyncio.create_task(vnc_to_client(), name="v2c") - - done, pending = await asyncio.wait( - [c2v, v2c], - return_when=asyncio.FIRST_COMPLETED, - ) - finished = [t.get_name() for t in done] - still_running = [t.get_name() for t in pending] - - # Check if Xvnc is still alive - vnc_instance = browser_mgr.vnc._allocated.get(running.display) - xvnc_alive = vnc_instance and vnc_instance.process and vnc_instance.process.poll() is None - logger.info( - "VNC proxy: finished=%s pending=%s xvnc_alive=%s display=:%d for %s", - finished, still_running, xvnc_alive, running.display, profile_id, - ) - - # Dump Xvnc log on disconnect - import os - xvnc_log = f"/tmp/xvnc-{running.display}.log" - if os.path.exists(xvnc_log): - with open(xvnc_log) as f: - log_content = f.read() - if log_content.strip(): - for line in log_content.strip().split("\n")[-20:]: - logger.info("Xvnc[:%d] %s", running.display, line) - - for task in pending: - task.cancel() - - except Exception as exc: - logger.error("VNC proxy connect error for %s: %s: %s", profile_id, type(exc).__name__, exc) - finally: - try: - await websocket.close() - except Exception as exc: - logger.debug("VNC proxy: websocket.close() failed: %s", exc) - - # ── CDP WebSocket Proxy ────────────────────────────────────────────────────── -# Simple bidirectional passthrough — CDP is standard JSON over WebSocket, -# no protocol translation needed (unlike VNC which requires RFB filtering). +# Simple bidirectional passthrough — CDP is standard JSON over WebSocket. @app.get("/api/profiles/{profile_id}/cdp") diff --git a/backend/models.py b/backend/models.py index e3ba3521..115c83f8 100644 --- a/backend/models.py +++ b/backend/models.py @@ -100,7 +100,7 @@ def coerce_clipboard_sync(cls, v: object) -> bool: created_at: str updated_at: str tags: list[TagResponse] = [] - status: str = "stopped" # "running" | "stopped" + status: str = "stopped" # "running" | "starting" | "stopped" vnc_ws_port: int | None = None cdp_url: str | None = None @@ -120,10 +120,18 @@ class StatusResponse(BaseModel): class ProfileStatusResponse(BaseModel): - status: str # "running" | "stopped" + status: str # "running" | "starting" | "stopped" vnc_ws_port: int | None = None display: str | None = None cdp_url: str | None = None + xvnc_alive: bool | None = None # null when stopped + browser_alive: bool | None = None # null when stopped + + +class ViewerTokenResponse(BaseModel): + token: str + viewer_url: str + expires_in: int class ClipboardRequest(BaseModel): diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 9b2bca33..5609379f 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -3,13 +3,16 @@ from __future__ import annotations from dataclasses import dataclass -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch import pytest +import pathlib +import socket + from starlette.testclient import TestClient from backend import main -from backend.browser_manager import RunningProfile +from backend.browser_manager import BrowserManager, RunningProfile # ── Profile CRUD ───────────────────────────────────────────────────────────── @@ -96,7 +99,9 @@ def test_delete_profile_not_found(app_client: TestClient): assert resp.status_code == 404 -def test_delete_profile_stops_running(app_client: TestClient): +def test_delete_profile_stops_running( + app_client: TestClient, monkeypatch: pytest.MonkeyPatch, +): """Deleting a running profile should stop it first.""" create = app_client.post("/api/profiles", json={"name": "Running"}) pid = create.json()["id"] @@ -107,11 +112,23 @@ def test_delete_profile_stops_running(app_client: TestClient): mock_running.ws_port = 6100 mock_running.cdp_port = 5100 main.browser_mgr.running[pid] = mock_running - main.browser_mgr.stop = AsyncMock() + # monkeypatch, not assignment: browser_mgr is a module singleton shared by + # the whole session, so a plain assignment would leave every later test + # calling this mock instead of the real method. + # emulate the real stop(): it removes the profile from `running`, which + # delete then re-checks before touching the filesystem + calls: list[str] = [] + + async def stop(target: str) -> bool: + calls.append(target) + main.browser_mgr.running.pop(target, None) + return True # browser really closed + + monkeypatch.setattr(main.browser_mgr, "stop", stop) resp = app_client.delete(f"/api/profiles/{pid}") assert resp.status_code == 200 - main.browser_mgr.stop.assert_called_once_with(pid) + assert calls == [pid] # ── Profile Status ─────────────────────────────────────────────────────────── @@ -130,6 +147,135 @@ def test_get_profile_status_not_found(app_client: TestClient): assert resp.status_code == 404 +def test_status_stopped_alive_fields_null(app_client: TestClient): + """Stopped profiles should report xvnc_alive/browser_alive as null.""" + create = app_client.post("/api/profiles", json={"name": "AliveStopped"}) + pid = create.json()["id"] + resp = app_client.get(f"/api/profiles/{pid}/status") + assert resp.status_code == 200 + data = resp.json() + assert data["xvnc_alive"] is None + assert data["browser_alive"] is None + + +def test_status_starting_while_launch_in_flight(app_client: TestClient): + """A profile mid-launch reports "starting", never "stopped".""" + create = app_client.post("/api/profiles", json={"name": "Booting"}) + pid = create.json()["id"] + + main.browser_mgr._launching.add(pid) + try: + data = app_client.get(f"/api/profiles/{pid}/status").json() + assert data["status"] == "starting" + # list/detail views agree with the status endpoint + assert app_client.get(f"/api/profiles/{pid}").json()["status"] == "starting" + finally: + main.browser_mgr._launching.discard(pid) + + assert app_client.get(f"/api/profiles/{pid}/status").json()["status"] == "stopped" + + +def test_status_starting_while_queued_for_auto_launch(app_client: TestClient): + """Auto-launch is sequential; queued profiles must not read as stopped.""" + create = app_client.post("/api/profiles", json={"name": "Queued"}) + pid = create.json()["id"] + + main.browser_mgr._pending_auto_launch.add(pid) + try: + assert app_client.get(f"/api/profiles/{pid}/status").json()["status"] == "starting" + finally: + main.browser_mgr._pending_auto_launch.discard(pid) + + +def test_viewer_token_starting_is_retryable_503(app_client: TestClient): + """503 (retry) not 404 (terminal) while the profile is coming up.""" + create = app_client.post("/api/profiles", json={"name": "BootingToken"}) + pid = create.json()["id"] + + main.browser_mgr._pending_auto_launch.add(pid) + try: + resp = app_client.post(f"/api/profiles/{pid}/viewer-token") + assert resp.status_code == 503 + finally: + main.browser_mgr._pending_auto_launch.discard(pid) + + assert app_client.post(f"/api/profiles/{pid}/viewer-token").status_code == 404 + + +def test_launch_while_starting_conflicts(app_client: TestClient): + """Launching a profile that is already coming up is a 409, not a 500.""" + create = app_client.post("/api/profiles", json={"name": "DoubleLaunch"}) + pid = create.json()["id"] + + main.browser_mgr._launching.add(pid) + try: + resp = app_client.post(f"/api/profiles/{pid}/launch") + assert resp.status_code == 409 + finally: + main.browser_mgr._launching.discard(pid) + + +def test_status_running_alive_fields(app_client: TestClient): + """Running profiles report real liveness for Xvnc and the browser. + + browser_alive comes from a real connect to the CDP port — a listening + socket stands in for Chromium's DevTools endpoint. + """ + create = app_client.post("/api/profiles", json={"name": "AliveRunning"}) + pid = create.json()["id"] + mock = _mock_running_profile(pid) + + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + mock.cdp_port = listener.getsockname()[1] + + # Fake a live Xvnc process for the profile's display + fake_vnc = MagicMock() + fake_vnc.process.poll.return_value = None + main.browser_mgr.vnc._allocated[100] = fake_vnc + + try: + resp = app_client.get(f"/api/profiles/{pid}/status") + assert resp.status_code == 200 + data = resp.json() + assert data["xvnc_alive"] is True + assert data["browser_alive"] is True + finally: + listener.close() + main.browser_mgr.vnc._allocated.pop(100, None) + main.browser_mgr.running.pop(pid, None) + + +def test_status_running_dead_processes(app_client: TestClient): + """Exited Xvnc / a CDP port nobody is listening on report False.""" + create = app_client.post("/api/profiles", json={"name": "AliveDead"}) + pid = create.json()["id"] + mock = _mock_running_profile(pid) + mock.cdp_port = _closed_port() # Chromium is gone + + fake_vnc = MagicMock() + fake_vnc.process.poll.return_value = 1 # exited + main.browser_mgr.vnc._allocated[100] = fake_vnc + + try: + resp = app_client.get(f"/api/profiles/{pid}/status") + assert resp.status_code == 200 + data = resp.json() + assert data["xvnc_alive"] is False + assert data["browser_alive"] is False + finally: + main.browser_mgr.vnc._allocated.pop(100, None) + main.browser_mgr.running.pop(pid, None) + + +def _closed_port() -> int: + """A port that was bound and released — nothing is listening on it.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.bind(("127.0.0.1", 0)) + return probe.getsockname()[1] + + # ── Launch / Stop ──────────────────────────────────────────────────────────── @@ -149,21 +295,26 @@ def test_launch_already_running(app_client: TestClient): main.browser_mgr.running.pop(pid, None) -def test_launch_invalid_proxy_400(app_client: TestClient): +def test_launch_invalid_proxy_400(app_client: TestClient, monkeypatch: pytest.MonkeyPatch): """ValueError from browser_mgr.launch should map to 400.""" create = app_client.post("/api/profiles", json={"name": "BadProxy"}) pid = create.json()["id"] - main.browser_mgr.launch = AsyncMock(side_effect=ValueError("Invalid proxy scheme 'ftp'")) + monkeypatch.setattr( + main.browser_mgr, "launch", + AsyncMock(side_effect=ValueError("Invalid proxy scheme 'ftp'")), + ) resp = app_client.post(f"/api/profiles/{pid}/launch") assert resp.status_code == 400 assert "ftp" in resp.json()["detail"] -def test_launch_failure_500(app_client: TestClient): +def test_launch_failure_500(app_client: TestClient, monkeypatch: pytest.MonkeyPatch): """Generic exception from browser_mgr.launch should map to 500.""" create = app_client.post("/api/profiles", json={"name": "Crash"}) pid = create.json()["id"] - main.browser_mgr.launch = AsyncMock(side_effect=RuntimeError("Xvnc failed")) + monkeypatch.setattr( + main.browser_mgr, "launch", AsyncMock(side_effect=RuntimeError("Xvnc failed")), + ) resp = app_client.post(f"/api/profiles/{pid}/launch") assert resp.status_code == 500 assert resp.json()["detail"] == "Failed to launch browser" @@ -174,6 +325,286 @@ def test_stop_not_running(app_client: TestClient): assert resp.status_code == 404 +# ── Viewer Sessions ────────────────────────────────────────────────────────── + + +def test_viewer_token_not_running(app_client: TestClient): + resp = app_client.post("/api/profiles/nonexistent/viewer-token") + assert resp.status_code == 404 + assert resp.json()["detail"] == "Profile not running" + + +def test_viewer_token_success(app_client: TestClient): + create = app_client.post("/api/profiles", json={"name": "ViewerTok"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + resp = app_client.post(f"/api/profiles/{pid}/viewer-token") + assert resp.status_code == 200 + data = resp.json() + assert data["token"] + assert data["viewer_url"] == f"/viewer/{data['token']}/" + assert data["expires_in"] == 3600 + + # Token actually validates against the store + session = main.viewer_tokens.validate(data["token"]) + assert session is not None + assert session.profile_id == pid + assert session.ws_port == 6100 + + # Cleanup + main.viewer_tokens.revoke_profile(pid) + main.browser_mgr.running.pop(pid, None) + + +def test_viewer_token_fresh_per_call(app_client: TestClient): + """Each call issues a fresh token; older tokens stay valid until expiry.""" + create = app_client.post("/api/profiles", json={"name": "ViewerFresh"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + t1 = app_client.post(f"/api/profiles/{pid}/viewer-token").json()["token"] + t2 = app_client.post(f"/api/profiles/{pid}/viewer-token").json()["token"] + assert t1 != t2 + assert main.viewer_tokens.validate(t1) is not None + assert main.viewer_tokens.validate(t2) is not None + + # Cleanup + main.viewer_tokens.revoke_profile(pid) + main.browser_mgr.running.pop(pid, None) + + +def test_stop_revokes_viewer_tokens(app_client: TestClient, monkeypatch: pytest.MonkeyPatch): + """Stopping a profile revokes its viewer tokens.""" + create = app_client.post("/api/profiles", json={"name": "ViewerStop"}) + pid = create.json()["id"] + mock = _mock_running_profile(pid) + # Model Playwright: is_closed() is sync, close() is a coroutine. + mock.context = MagicMock() + mock.context.is_closed.return_value = False + mock.context.close = AsyncMock() + monkeypatch.setattr(main.browser_mgr.vnc, "stop_vnc", AsyncMock()) + token = main.viewer_tokens.issue(pid, 6100) + + resp = app_client.post(f"/api/profiles/{pid}/stop") + assert resp.status_code == 200 + assert main.viewer_tokens.validate(token) is None + + # Cleanup + main.browser_mgr.running.pop(pid, None) + + +def test_delete_profile_revokes_viewer_tokens(app_client: TestClient): + """Deleting a (stopped) profile revokes its viewer tokens unconditionally.""" + create = app_client.post("/api/profiles", json={"name": "ViewerDel"}) + pid = create.json()["id"] + token = main.viewer_tokens.issue(pid, 6100) + + resp = app_client.delete(f"/api/profiles/{pid}") + assert resp.status_code == 200 + assert main.viewer_tokens.validate(token) is None + + +# ── Viewer Auth (nginx auth_request) ───────────────────────────────────────── + + +def test_viewer_auth_missing_original_uri(app_client: TestClient): + resp = app_client.get("/api/viewer-auth") + assert resp.status_code == 403 + + +def test_viewer_auth_non_viewer_uri(app_client: TestClient): + resp = app_client.get("/api/viewer-auth", headers={"X-Original-URI": "/api/profiles"}) + assert resp.status_code == 403 + + +def test_viewer_auth_bad_token(app_client: TestClient): + resp = app_client.get("/api/viewer-auth", headers={"X-Original-URI": "/viewer/bogus/"}) + assert resp.status_code == 403 + + +def test_viewer_auth_profile_stopped(app_client: TestClient): + """Valid token but the profile is no longer running → 403. + + Must be 403 (or 401): nginx auth_request turns any other non-2xx into a + 500 for the client. Verified live against the shipped config — a 404 + subrequest logs "auth request unexpected status: 404" and serves 500. + """ + create = app_client.post("/api/profiles", json={"name": "ViewerGone"}) + pid = create.json()["id"] + token = main.viewer_tokens.issue(pid, 6100) # no running profile registered + + resp = app_client.get("/api/viewer-auth", headers={"X-Original-URI": f"/viewer/{token}/"}) + assert resp.status_code == 403 + + # Cleanup + main.viewer_tokens.revoke_profile(pid) + + +def test_viewer_auth_success(app_client: TestClient): + create = app_client.post("/api/profiles", json={"name": "ViewerOk"}) + pid = create.json()["id"] + _mock_running_profile(pid) + token = main.viewer_tokens.issue(pid, 6100) + + resp = app_client.get("/api/viewer-auth", headers={"X-Original-URI": f"/viewer/{token}/"}) + assert resp.status_code == 200 + assert resp.headers["X-Viewer-Upstream"] == "127.0.0.1:6100" + + # Nested asset paths (with query strings) share the same token + resp = app_client.get( + "/api/viewer-auth", + headers={"X-Original-URI": f"/viewer/{token}/app/dist/main.js?v=1"}, + ) + assert resp.status_code == 200 + + # Cleanup + main.viewer_tokens.revoke_profile(pid) + main.browser_mgr.running.pop(pid, None) + + +def test_viewer_auth_injects_kasm_basic_auth(app_client: TestClient, monkeypatch): + """When per-display Kasm credentials exist, viewer-auth hands them to nginx.""" + create = app_client.post("/api/profiles", json={"name": "ViewerAuth"}) + pid = create.json()["id"] + _mock_running_profile(pid) + token = main.viewer_tokens.issue(pid, 6100) + monkeypatch.setattr( + main.browser_mgr.vnc, "get_api_credentials", lambda _d: ("manager", "pw123") + ) + + resp = app_client.get("/api/viewer-auth", headers={"X-Original-URI": f"/viewer/{token}/"}) + assert resp.status_code == 200 + import base64 + + expected = "Basic " + base64.b64encode(b"manager:pw123").decode() + assert resp.headers["X-Viewer-Authorization"] == expected + + # Cleanup + main.viewer_tokens.revoke_profile(pid) + main.browser_mgr.running.pop(pid, None) + + +def test_kasm_stats_not_running(app_client: TestClient): + resp = app_client.get("/api/profiles/nonexistent/kasm-stats") + assert resp.status_code == 404 + + +def test_kasm_stats_success(app_client: TestClient): + create = app_client.post("/api/profiles", json={"name": "StatsOk"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + bottleneck_resp = MagicMock() + bottleneck_resp.status_code = 200 + bottleneck_resp.json.return_value = {"code": 0, "bottleneck": "cpu"} + sessions_resp = MagicMock() + sessions_resp.status_code = 200 + sessions_resp.json.return_value = {"users": [{"username": "manager"}]} + frame_resp = MagicMock() + frame_resp.status_code = 200 + frame_resp.json.return_value = {"clients": {"all": {"fps": 60}}} + + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + mock_client.get = AsyncMock(side_effect=[bottleneck_resp, sessions_resp, frame_resp]) + + with patch("httpx.AsyncClient", return_value=mock_client): + resp = app_client.get(f"/api/profiles/{pid}/kasm-stats") + + assert resp.status_code == 200 + data = resp.json() + assert data["bottleneck"] == {"code": 0, "bottleneck": "cpu"} + assert data["sessions"] == {"users": [{"username": "manager"}]} + assert data["frame"] == {"clients": {"all": {"fps": 60}}} + + # Cleanup + main.browser_mgr.running.pop(pid, None) + + +def test_kasm_stats_no_viewers_skips_frame_stats(app_client: TestClient): + """get_frame_stats hangs with no clients — it must not be called.""" + create = app_client.post("/api/profiles", json={"name": "StatsEmpty"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + bottleneck_resp = MagicMock() + bottleneck_resp.status_code = 200 + bottleneck_resp.json.return_value = {} + sessions_resp = MagicMock() + sessions_resp.status_code = 200 + sessions_resp.json.return_value = {"users": []} + + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + mock_client.get = AsyncMock(side_effect=[bottleneck_resp, sessions_resp]) + + with patch("httpx.AsyncClient", return_value=mock_client): + resp = app_client.get(f"/api/profiles/{pid}/kasm-stats") + + assert resp.status_code == 200 + data = resp.json() + assert data["frame"] is None + assert mock_client.get.await_count == 2 # no get_frame_stats call + + # Cleanup + main.browser_mgr.running.pop(pid, None) + + +def test_kasm_stats_frame_stats_failure_isolated(app_client: TestClient): + """Frame-stats errors (503/timeout) must not fail the whole endpoint.""" + create = app_client.post("/api/profiles", json={"name": "StatsIso"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + bottleneck_resp = MagicMock() + bottleneck_resp.status_code = 200 + bottleneck_resp.json.return_value = {"manager": {}} + sessions_resp = MagicMock() + sessions_resp.status_code = 200 + sessions_resp.json.return_value = {"users": [{"username": "manager"}]} + + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + mock_client.get = AsyncMock( + side_effect=[bottleneck_resp, sessions_resp, ConnectionError("timeout")] + ) + + with patch("httpx.AsyncClient", return_value=mock_client): + resp = app_client.get(f"/api/profiles/{pid}/kasm-stats") + + assert resp.status_code == 200 + data = resp.json() + assert data["bottleneck"] == {"manager": {}} + assert data["frame"] is None + + # Cleanup + main.browser_mgr.running.pop(pid, None) + + +def test_kasm_stats_kasm_unreachable(app_client: TestClient): + """502 when the KasmVNC stats endpoint is down.""" + create = app_client.post("/api/profiles", json={"name": "StatsDown"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + mock_client.get = AsyncMock(side_effect=ConnectionError("refused")) + + with patch("httpx.AsyncClient", return_value=mock_client): + resp = app_client.get(f"/api/profiles/{pid}/kasm-stats") + + assert resp.status_code == 502 + + # Cleanup + main.browser_mgr.running.pop(pid, None) + + # ── System Status ──────────────────────────────────────────────────────────── @@ -497,21 +928,6 @@ def test_cdp_json_version_chrome_unreachable(app_client: TestClient): # ── WebSocket Origin Validation ────────────────────────────────────────────── -def test_vnc_ws_rejects_cross_origin(app_client: TestClient): - """VNC WebSocket should reject cross-origin browser connections.""" - create = app_client.post("/api/profiles", json={"name": "OriginVnc"}) - pid = create.json()["id"] - _mock_running_profile(pid) - - with pytest.raises(Exception): - with app_client.websocket_connect( - f"/api/profiles/{pid}/vnc", - headers={"origin": "http://evil.com"}, - ): - pass - main.browser_mgr.running.pop(pid, None) - - def test_cdp_ws_rejects_cross_origin(app_client: TestClient): """CDP WebSocket should reject cross-origin browser connections.""" create = app_client.post("/api/profiles", json={"name": "OriginCdp"}) @@ -533,12 +949,12 @@ def test_ws_allows_same_origin(app_client: TestClient): pid = create.json()["id"] _mock_running_profile(pid) - # Same-origin passes Origin check. VNC proxy then fails to connect to - # real KasmVNC (not running), but that's fine — we're testing Origin only. - # The connection is accepted (no 4403), then closes due to VNC connect error. + # Same-origin passes Origin check. The CDP proxy then fails to reach real + # Chrome (not running) and closes — fine, we're testing Origin only. + # The connection is accepted (no 4403), then closes on CDP connect error. try: with app_client.websocket_connect( - f"/api/profiles/{pid}/vnc", + f"/api/profiles/{pid}/cdp", headers={"origin": "http://testserver"}, ) as ws: pass # connection accepted = Origin check passed @@ -555,8 +971,287 @@ def test_ws_allows_no_origin(app_client: TestClient): _mock_running_profile(pid) try: - with app_client.websocket_connect(f"/api/profiles/{pid}/vnc") as ws: + with app_client.websocket_connect(f"/api/profiles/{pid}/cdp") as ws: pass except Exception as exc: assert "4403" not in str(exc) main.browser_mgr.running.pop(pid, None) + + +def test_kasm_stats_upstream_error_is_502(app_client: TestClient): + """A Kasm 401/5xx must not be handed back as 200 with an HTML body.""" + create = app_client.post("/api/profiles", json={"name": "StatsAuthFail"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + unauthorized = MagicMock() + unauthorized.status_code = 401 + unauthorized.text = "401 Unauthorized" + unauthorized.json.side_effect = ValueError("not json") + sessions_resp = MagicMock() + sessions_resp.status_code = 200 + sessions_resp.json.return_value = {"users": []} + + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + mock_client.get = AsyncMock(side_effect=[unauthorized, sessions_resp]) + + try: + with patch("httpx.AsyncClient", return_value=mock_client): + resp = app_client.get(f"/api/profiles/{pid}/kasm-stats") + assert resp.status_code == 502 + assert "401" in resp.json()["detail"] + finally: + main.browser_mgr.running.pop(pid, None) + + +def test_delete_while_starting_conflicts(app_client: TestClient): + """Deleting mid-launch would rmtree the user_data_dir under Chromium.""" + create = app_client.post("/api/profiles", json={"name": "DeleteRace"}) + pid = create.json()["id"] + + main.browser_mgr._launching.add(pid) + try: + assert app_client.delete(f"/api/profiles/{pid}").status_code == 409 + finally: + main.browser_mgr._launching.discard(pid) + + # still there, and deletable once the launch settles + assert app_client.get(f"/api/profiles/{pid}").status_code == 200 + assert app_client.delete(f"/api/profiles/{pid}").status_code == 200 + + +def test_stop_while_starting_conflicts(app_client: TestClient): + """Mid-launch is not "not running" — 409, not 404.""" + create = app_client.post("/api/profiles", json={"name": "StopRace"}) + pid = create.json()["id"] + + main.browser_mgr._pending_auto_launch.add(pid) + try: + assert app_client.post(f"/api/profiles/{pid}/stop").status_code == 409 + finally: + main.browser_mgr._pending_auto_launch.discard(pid) + + assert app_client.post(f"/api/profiles/{pid}/stop").status_code == 404 + + +def test_browser_mgr_singleton_is_not_permanently_shadowed(): + """No test may leave a mock bolted onto the shared BrowserManager. + + browser_mgr is a module-level singleton. A plain `main.browser_mgr.stop = + AsyncMock()` is never undone, so every later test that exercises stop() + or launch() silently asserts against the mock instead of production code. + monkeypatch.setattr restores on teardown; assignment does not. + """ + for name in ("stop", "launch"): + bound = getattr(main.browser_mgr, name) + assert getattr(bound, "__func__", None) is getattr(BrowserManager, name), ( + f"browser_mgr.{name} is still a test double — " + "use monkeypatch.setattr instead of assignment" + ) + + +def test_viewer_auth_rejects_a_token_from_a_previous_run(app_client: TestClient): + """Upstream and credentials must describe the same instance.""" + create = app_client.post("/api/profiles", json={"name": "ViewerStale"}) + pid = create.json()["id"] + mock = _mock_running_profile(pid) + mock.ws_port = 6105 # relaunched on a different display + token = main.viewer_tokens.issue(pid, 6100) # token from the previous run + + try: + resp = app_client.get( + "/api/viewer-auth", headers={"X-Original-URI": f"/viewer/{token}/"}, + ) + assert resp.status_code == 403 + finally: + main.viewer_tokens.revoke_profile(pid) + main.browser_mgr.running.pop(pid, None) + + +def test_delete_rechecks_after_stopping( + app_client: TestClient, monkeypatch: pytest.MonkeyPatch, +): + """A launch claiming the profile during stop() must not get its dir deleted.""" + create = app_client.post("/api/profiles", json={"name": "DeleteAfterStop"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + async def stop_then_relaunch(_pid: str) -> bool: + # what a concurrent POST /launch would do while stop() awaits the close + main.browser_mgr.running.pop(_pid, None) + main.browser_mgr._launching.add(_pid) + return True + + monkeypatch.setattr(main.browser_mgr, "stop", stop_then_relaunch) + try: + resp = app_client.delete(f"/api/profiles/{pid}") + assert resp.status_code == 409 + # and the profile still exists + assert app_client.get(f"/api/profiles/{pid}").status_code == 200 + finally: + main.browser_mgr._launching.discard(pid) + main.browser_mgr.running.pop(pid, None) + + +def test_delete_refuses_when_the_browser_did_not_close( + app_client: TestClient, monkeypatch: pytest.MonkeyPatch, +): + """A wedged teardown must not let rmtree run under a live Chromium.""" + create = app_client.post("/api/profiles", json={"name": "DeleteWedged"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + async def stop_but_not_closed(target: str) -> bool: + main.browser_mgr.running.pop(target, None) + return False # bounded close timed out; Chromium may still be alive + + monkeypatch.setattr(main.browser_mgr, "stop", stop_but_not_closed) + try: + assert app_client.delete(f"/api/profiles/{pid}").status_code == 409 + assert app_client.get(f"/api/profiles/{pid}").status_code == 200 + finally: + main.browser_mgr.running.pop(pid, None) + + +def test_raced_launch_is_409_not_500(app_client: TestClient, monkeypatch: pytest.MonkeyPatch): + """Losing the launch race is a conflict, not a server error.""" + from backend.browser_manager import ProfileAlreadyRunning + + create = app_client.post("/api/profiles", json={"name": "RacedLaunch"}) + pid = create.json()["id"] + + async def already(_profile): + raise ProfileAlreadyRunning("Profile is already running") + + monkeypatch.setattr(main.browser_mgr, "launch", already) + assert app_client.post(f"/api/profiles/{pid}/launch").status_code == 409 + + +def test_launch_is_blocked_while_a_delete_is_in_flight(app_client: TestClient): + """The delete claim closes the window the post-stop re-check only narrowed.""" + create = app_client.post("/api/profiles", json={"name": "DeleteClaim"}) + pid = create.json()["id"] + + main.browser_mgr.claim_for_delete(pid) + try: + assert app_client.post(f"/api/profiles/{pid}/launch").status_code == 409 + finally: + main.browser_mgr.release_delete_claim(pid) + + # released again once the delete finishes + assert main.browser_mgr.is_starting(pid) is False + + +def test_concurrent_delete_is_refused_rather_than_sharing_the_claim( + app_client: TestClient, +): + """The second delete must not be able to release the first one's claim.""" + create = app_client.post("/api/profiles", json={"name": "DoubleDelete"}) + pid = create.json()["id"] + + assert main.browser_mgr.claim_for_delete(pid) is True + try: + # a second delete arriving while the first is in flight + assert app_client.delete(f"/api/profiles/{pid}").status_code == 409 + # and the first claim is still held, so launches stay blocked + assert app_client.post(f"/api/profiles/{pid}/launch").status_code == 409 + finally: + main.browser_mgr.release_delete_claim(pid) + + assert main.browser_mgr.claim_for_delete(pid) is True + main.browser_mgr.release_delete_claim(pid) + + +def test_delete_refuses_while_a_previous_stop_is_still_wedged(app_client: TestClient): + """The round-4 guard only fired when the delete itself did the stop.""" + create = app_client.post("/api/profiles", json={"name": "WedgedThenDelete"}) + pid = create.json()["id"] + + from unittest.mock import MagicMock as _MM + import time as _t + main.browser_mgr._closing[pid] = (_MM(), _t.monotonic() + 60, None, _t.monotonic()) # a previous /stop left Chromium alive + try: + assert app_client.delete(f"/api/profiles/{pid}").status_code == 409 + assert app_client.post(f"/api/profiles/{pid}/launch").status_code == 409 + assert app_client.get(f"/api/profiles/{pid}").status_code == 200 + finally: + main.browser_mgr._closing.pop(pid, None) + + assert app_client.delete(f"/api/profiles/{pid}").status_code == 200 + + +def test_manual_launch_has_a_server_side_deadline( + app_client: TestClient, monkeypatch: pytest.MonkeyPatch, +): + """A wedged launch must not hang the request and strand `_launching`.""" + import asyncio as aio + + create = app_client.post("/api/profiles", json={"name": "LaunchHang"}) + pid = create.json()["id"] + + async def never_returns(_profile): + await aio.sleep(3600) + + monkeypatch.setattr(main.browser_mgr, "launch", never_returns) + monkeypatch.setattr(main, "LAUNCH_TIMEOUT_S", 0.05) + resp = app_client.post(f"/api/profiles/{pid}/launch") + assert resp.status_code == 504 + + +def test_delete_does_not_block_the_event_loop(app_client: TestClient, monkeypatch): + """rmtree of a large profile dir must not stall every other request.""" + create = app_client.post("/api/profiles", json={"name": "BigDelete"}) + body = create.json() + pid = body["id"] + # the dir only exists once the profile has been launched at least once + pathlib.Path(body["user_data_dir"]).mkdir(parents=True, exist_ok=True) + + called: dict[str, object] = {} + real_rmtree = main.shutil.rmtree + + def tracking_rmtree(path, **kw): + import asyncio as aio + # An executor thread has no running loop; the loop's own thread does. + # Thread NAMES prove nothing here — TestClient already runs the app off + # the main thread. + try: + aio.get_running_loop() + called["on_event_loop"] = True + except RuntimeError: + called["on_event_loop"] = False + return real_rmtree(path, **kw) + + monkeypatch.setattr(main.shutil, "rmtree", tracking_rmtree) + assert app_client.delete(f"/api/profiles/{pid}").status_code == 200 + assert called.get("on_event_loop") is False + + +def test_status_endpoint_probes_off_the_event_loop( + app_client: TestClient, monkeypatch: pytest.MonkeyPatch, +): + """_browser_alive opens a TCP connection; GET /status must not do that on + the loop that also serves nginx's viewer auth_request subrequests.""" + import asyncio as aio + + create = app_client.post("/api/profiles", json={"name": "ProbeLoop"}) + pid = create.json()["id"] + _mock_running_profile(pid) + + seen: dict[str, bool] = {} + + def probing(_running): + try: + aio.get_running_loop() + seen["on_loop"] = True + except RuntimeError: + seen["on_loop"] = False + return True + + monkeypatch.setattr(main.browser_mgr, "_browser_alive", probing) + try: + assert app_client.get(f"/api/profiles/{pid}/status").status_code == 200 + assert seen.get("on_loop") is False + finally: + main.browser_mgr.running.pop(pid, None) diff --git a/backend/tests/test_models.py b/backend/tests/test_models.py index 1bccda7d..e4437ae4 100644 --- a/backend/tests/test_models.py +++ b/backend/tests/test_models.py @@ -13,6 +13,7 @@ StatusResponse, TagCreate, TagResponse, + ViewerTokenResponse, ) @@ -175,6 +176,28 @@ def test_profile_status_response_cdp_url_stopped(): assert r.cdp_url is None +def test_profile_status_response_alive_defaults_null(): + r = ProfileStatusResponse(status="stopped") + assert r.xvnc_alive is None + assert r.browser_alive is None + + +def test_profile_status_response_alive_fields(): + r = ProfileStatusResponse(status="running", xvnc_alive=True, browser_alive=False) + assert r.xvnc_alive is True + assert r.browser_alive is False + + +# ── ViewerTokenResponse ────────────────────────────────────────────────────── + + +def test_viewer_token_response(): + r = ViewerTokenResponse(token="abc123", viewer_url="/viewer/abc123/", expires_in=300) + assert r.token == "abc123" + assert r.viewer_url == "/viewer/abc123/" + assert r.expires_in == 300 + + # ── ProfileResponse ──────────────────────────────────────────────────────── diff --git a/backend/tests/test_rfb.py b/backend/tests/test_rfb.py deleted file mode 100644 index 010b04ba..00000000 --- a/backend/tests/test_rfb.py +++ /dev/null @@ -1,341 +0,0 @@ -"""Tests for RFB binary protocol parsing functions in main.py.""" - -from __future__ import annotations - -import struct - -from backend.main import ( - _build_server_cut_text, - _filter_rfb_client_messages, - _parse_kasmvnc_clipboard, - _rewrite_pointer_event, - _rewrite_set_encodings, - _rfb_msg_length, - _ALLOWED_ENCODINGS, -) - - -# ── Helpers ────────────────────────────────────────────────────────────────── - - -def _make_key_event(down: int = 1, key: int = 0x61) -> bytes: - """Build an 8-byte RFB KeyEvent (type 4).""" - return struct.pack(">BBxxI", 4, down, key) - - -def _make_pointer_event(mask: int = 0, x: int = 100, y: int = 200) -> bytes: - """Build a 6-byte RFB PointerEvent (type 5).""" - return struct.pack(">BBHH", 5, mask, x, y) - - -def _make_fb_update_request(x: int = 0, y: int = 0, w: int = 1920, h: int = 1080, incr: int = 1) -> bytes: - """Build a 10-byte FramebufferUpdateRequest (type 3).""" - return struct.pack(">BBHHHH", 3, incr, x, y, w, h) - - -def _make_set_encodings(encodings: list[int]) -> bytes: - """Build a SetEncodings message (type 2).""" - header = struct.pack(">BxH", 2, len(encodings)) - for enc in encodings: - header += struct.pack(">i", enc) # signed - return header - - -def _make_client_cut_text(text: str) -> bytes: - """Build a ClientCutText message (type 6).""" - text_bytes = text.encode("latin-1", errors="replace") - return struct.pack(">BxxxI", 6, len(text_bytes)) + text_bytes - - -def _make_extension_150() -> bytes: - """Build a 10-byte EnableContinuousUpdates extension (type 150).""" - return struct.pack(">BBHHHH", 150, 1, 0, 0, 1920, 1080) - - -def _make_kasmvnc_clipboard(mime: str, data: str) -> bytes: - """Build a KasmVNC BinaryClipboard message (type 180).""" - mime_bytes = mime.encode() - data_bytes = data.encode("utf-8") - buf = bytearray() - buf.append(180) # type - buf.append(0) # action - buf.extend(b'\x00' * 4) # flags - buf.append(len(mime_bytes)) # mime_len (u8) - buf.extend(mime_bytes) - buf.extend(struct.pack(">I", len(data_bytes))) # data_len (u32 BE) - buf.extend(data_bytes) - return bytes(buf) - - -# ── _parse_kasmvnc_clipboard ──────────────────────────────────────────────── - - -def test_parse_clipboard_text_plain(): - data = _make_kasmvnc_clipboard("text/plain", "hello") - assert _parse_kasmvnc_clipboard(data) == "hello" - - -def test_parse_clipboard_no_text_plain(): - data = _make_kasmvnc_clipboard("image/png", "binary") - assert _parse_kasmvnc_clipboard(data) is None - - -def test_parse_clipboard_too_short(): - assert _parse_kasmvnc_clipboard(b"\xb4\x00\x00") is None - - -def test_parse_clipboard_empty_text(): - data = _make_kasmvnc_clipboard("text/plain", "") - assert _parse_kasmvnc_clipboard(data) == "" - - -def test_parse_clipboard_multiple_mimes(): - """First mime is image/png, second is text/plain — should find text/plain.""" - buf = bytearray() - buf.append(180) - buf.append(0) - buf.extend(b'\x00' * 4) - # Entry 1: image/png - mime1 = b"image/png" - buf.append(len(mime1)) - buf.extend(mime1) - buf.extend(struct.pack(">I", 3)) - buf.extend(b"PNG") - # Entry 2: text/plain - mime2 = b"text/plain" - buf.append(len(mime2)) - buf.extend(mime2) - text = b"world" - buf.extend(struct.pack(">I", len(text))) - buf.extend(text) - assert _parse_kasmvnc_clipboard(bytes(buf)) == "world" - - -# ── _build_server_cut_text ─────────────────────────────────────────────────── - - -def test_build_cut_text_basic(): - result = _build_server_cut_text("hi") - assert result[0] == 3 # ServerCutText type - length = struct.unpack_from(">I", result, 4)[0] - assert length == 2 - assert result[8:] == b"hi" - - -def test_build_cut_text_empty(): - result = _build_server_cut_text("") - assert result[0] == 3 - length = struct.unpack_from(">I", result, 4)[0] - assert length == 0 - assert len(result) == 8 - - -def test_build_cut_text_latin1_fallback(): - # CJK chars are outside Latin-1 — replaced with '?' - result = _build_server_cut_text("hello \u65e5\u672c") - text_bytes = result[8:] - assert b"?" in text_bytes # unicode replaced - assert text_bytes.startswith(b"hello ") - - -# ── _rfb_msg_length ───────────────────────────────────────────────────────── - - -def test_rfb_len_set_pixel_format(): - data = bytes(20) # type 0, 20 bytes - assert _rfb_msg_length(data, 0) == 20 - - -def test_rfb_len_fb_update_request(): - data = _make_fb_update_request() - assert _rfb_msg_length(data, 0) == 10 - - -def test_rfb_len_key_event(): - data = _make_key_event() - assert _rfb_msg_length(data, 0) == 8 - - -def test_rfb_len_pointer_event(): - data = _make_pointer_event() - assert _rfb_msg_length(data, 0) == 6 - - -def test_rfb_len_set_encodings(): - data = _make_set_encodings([0, 1, 2]) - assert _rfb_msg_length(data, 0) == 4 + 3 * 4 # 16 - - -def test_rfb_len_client_cut_text(): - data = _make_client_cut_text("hello world") - assert _rfb_msg_length(data, 0) == 8 + 11 # 19 - - -def test_rfb_len_extension_150(): - data = _make_extension_150() - assert _rfb_msg_length(data, 0) == 10 - - -def test_rfb_len_unknown(): - data = bytes([99]) # unknown type - assert _rfb_msg_length(data, 0) is None - - -def test_rfb_len_with_offset(): - """Length calculation works when message is not at start of buffer.""" - prefix = b"\x00" * 10 - key = _make_key_event() - data = prefix + key - assert _rfb_msg_length(data, 10) == 8 - - -# ── _rewrite_set_encodings ────────────────────────────────────────────────── - - -def test_rewrite_keeps_allowed(): - allowed = [0, 1, 2, 5, 7] - data = _make_set_encodings(allowed) - result = _rewrite_set_encodings(data, 0, len(data)) - # All kept — output should be identical - assert result == data - - -def test_rewrite_strips_disallowed(): - # -260 and -307 are not in _ALLOWED_ENCODINGS - data = _make_set_encodings([0, 1, -260, -307]) - result = _rewrite_set_encodings(data, 0, len(data)) - # Only 0 and 1 should remain - num_enc = struct.unpack_from(">H", result, 2)[0] - assert num_enc == 2 - enc1 = struct.unpack_from(">i", result, 4)[0] - enc2 = struct.unpack_from(">i", result, 8)[0] - assert enc1 == 0 - assert enc2 == 1 - - -def test_rewrite_with_offset(): - """Rewrite works when message is at a non-zero offset.""" - prefix = b"\xff" * 8 - data = _make_set_encodings([0, -260]) - full = prefix + data - result = _rewrite_set_encodings(full, 8, len(data)) - num_enc = struct.unpack_from(">H", result, 2)[0] - assert num_enc == 1 # only 0 kept - - -# ── _rewrite_pointer_event ─────────────────────────────────────────────────── - - -def test_rewrite_pointer_basic(): - data = _make_pointer_event(mask=1, x=100, y=200) - result = _rewrite_pointer_event(data, 0) - assert len(result) == 11 # KasmVNC format - assert result[0] == 5 # PointerEvent type preserved - mask_u16 = struct.unpack_from(">H", result, 1)[0] - x = struct.unpack_from(">H", result, 3)[0] - y = struct.unpack_from(">H", result, 5)[0] - sx = struct.unpack_from(">h", result, 7)[0] - sy = struct.unpack_from(">h", result, 9)[0] - assert mask_u16 == 1 - assert x == 100 - assert y == 200 - assert sx == 0 - assert sy == 0 - - -def test_rewrite_pointer_mask_expansion(): - """u8 mask 0xFF should be expanded to u16 0x00FF.""" - data = _make_pointer_event(mask=0xFF, x=0, y=0) - result = _rewrite_pointer_event(data, 0) - mask_u16 = struct.unpack_from(">H", result, 1)[0] - assert mask_u16 == 0x00FF - - -def test_rewrite_pointer_with_offset(): - prefix = b"\x00" * 4 - data = _make_pointer_event(mask=2, x=50, y=75) - full = prefix + data - result = _rewrite_pointer_event(full, 4) - assert len(result) == 11 - mask_u16 = struct.unpack_from(">H", result, 1)[0] - assert mask_u16 == 2 - - -# ── _filter_rfb_client_messages ────────────────────────────────────────────── - - -def test_filter_keeps_standard_types(): - key = _make_key_event() - fb = _make_fb_update_request() - data = key + fb - result = _filter_rfb_client_messages(data) - # KeyEvent stays 8 bytes, FBUpdateRequest stays 10 bytes - # Total = 18 - assert len(result) == 18 - assert result[:8] == key - assert result[8:] == fb - - -def test_filter_strips_extension_150(): - key1 = _make_key_event(down=1, key=0x61) - ext = _make_extension_150() - key2 = _make_key_event(down=0, key=0x61) - data = key1 + ext + key2 - result = _filter_rfb_client_messages(data) - # Extension stripped, both key events kept - assert len(result) == 16 # 8 + 8 - assert result[:8] == key1 - assert result[8:] == key2 - - -def test_filter_drops_unknown(): - key = _make_key_event() - unknown = bytes([99, 0, 0, 0, 0]) # unknown type with some trailing bytes - data = key + unknown - result = _filter_rfb_client_messages(data) - # Key event kept, unknown causes break (rest dropped) - assert result == key - - -def test_filter_drops_incomplete(): - # KeyEvent is 8 bytes, give only 4 - data = _make_key_event()[:4] - result = _filter_rfb_client_messages(data) - assert result == b"" - - -def test_filter_rewrites_pointer(): - """PointerEvent should be rewritten from 6→11 bytes.""" - ptr = _make_pointer_event(mask=1, x=100, y=200) - result = _filter_rfb_client_messages(ptr) - assert len(result) == 11 # expanded - assert result[0] == 5 - - -def test_filter_rewrites_set_encodings(): - """SetEncodings should have disallowed encodings stripped.""" - enc = _make_set_encodings([0, 1, -260]) - result = _filter_rfb_client_messages(enc) - num_enc = struct.unpack_from(">H", result, 2)[0] - assert num_enc == 2 # -260 stripped - - -def test_filter_mixed_frame(): - """Realistic frame: KeyEvent + Extension + PointerEvent + ClientCutText.""" - key = _make_key_event() # 8 bytes, kept - ext = _make_extension_150() # 10 bytes, stripped - ptr = _make_pointer_event() # 6 bytes → 11 bytes (rewritten) - cut = _make_client_cut_text("hi") # 8+2=10 bytes, kept - - data = key + ext + ptr + cut - result = _filter_rfb_client_messages(data) - - # key(8) + ptr_rewritten(11) + cut(10) = 29 - assert len(result) == 29 - assert result[0] == 4 # KeyEvent type - assert result[8] == 5 # PointerEvent type (rewritten) - assert result[19] == 6 # ClientCutText type - - -def test_filter_empty_input(): - assert _filter_rfb_client_messages(b"") == b"" diff --git a/backend/tests/test_viewer_tokens.py b/backend/tests/test_viewer_tokens.py new file mode 100644 index 00000000..9be7607b --- /dev/null +++ b/backend/tests/test_viewer_tokens.py @@ -0,0 +1,108 @@ +"""Tests for the viewer token store (KasmVNC native client authorization).""" + +from __future__ import annotations + +import time + +from backend.viewer_tokens import ViewerTokenStore + + +# ── issue / validate ───────────────────────────────────────────────────────── + + +def test_issue_returns_unique_fresh_tokens(): + """Each issue() call returns a fresh token; old ones stay valid.""" + store = ViewerTokenStore() + t1 = store.issue("p1", 6100) + t2 = store.issue("p1", 6100) + assert t1 != t2 + assert len(t1) > 20 + assert store.active_count == 2 + + +def test_validate_returns_session(): + store = ViewerTokenStore() + token = store.issue("p1", 6100) + session = store.validate(token) + assert session is not None + assert session.token == token + assert session.profile_id == "p1" + assert session.ws_port == 6100 + assert session.expires_at > session.issued_at + + +def test_validate_unknown_token(): + store = ViewerTokenStore() + assert store.validate("no-such-token") is None + + +# ── expiry ──────────────────────────────────────────────────────────────────── + + +def test_validate_expired_token_immediate_ttl(): + """A zero TTL token is already expired and gets lazy-purged.""" + store = ViewerTokenStore() + token = store.issue("p1", 6100, ttl=0) + assert store.validate(token) is None + assert store.active_count == 0 + + +def test_validate_expired_token_monkeypatched_time(monkeypatch): + """Token valid at issue time expires once the TTL has passed.""" + store = ViewerTokenStore() + token = store.issue("p1", 6100, ttl=300) + assert store.validate(token) is not None + + real_time = time.time + monkeypatch.setattr(time, "time", lambda: real_time() + 301) + assert store.validate(token) is None + assert store.active_count == 0 # lazy-purged + + +# ── revoke_profile ──────────────────────────────────────────────────────────── + + +def test_revoke_profile_removes_only_that_profile(): + store = ViewerTokenStore() + t1 = store.issue("p1", 6100) + t2 = store.issue("p1", 6100) + t3 = store.issue("p2", 6101) + + store.revoke_profile("p1") + + assert store.validate(t1) is None + assert store.validate(t2) is None + assert store.validate(t3) is not None + assert store.active_count == 1 + + +def test_revoke_profile_unknown_is_noop(): + store = ViewerTokenStore() + store.revoke_profile("nope") # should not raise + assert store.active_count == 0 + + +def test_issue_sweeps_abandoned_expired_tokens(monkeypatch): + """Tokens never presented again must not accumulate for their whole TTL.""" + store = ViewerTokenStore() + for _ in range(5): + store.issue("p1", 6100, ttl=300) + assert store.active_count == 5 + + real_time = time.time + monkeypatch.setattr(time, "time", lambda: real_time() + 301) + fresh = store.issue("p1", 6100, ttl=300) + + # only the new one survives, and it is usable + assert store.active_count == 1 + assert store.validate(fresh) is not None + + +def test_issue_keeps_unexpired_tokens_of_other_profiles(): + """The sweep is by expiry, not by profile.""" + store = ViewerTokenStore() + keep = store.issue("p1", 6100, ttl=300) + store.issue("p2", 6101, ttl=0) # already expired + store.issue("p3", 6102, ttl=300) + assert store.active_count == 2 + assert store.validate(keep) is not None diff --git a/backend/tests/test_vnc_manager.py b/backend/tests/test_vnc_manager.py index 99eb3a1a..c3b31db1 100644 --- a/backend/tests/test_vnc_manager.py +++ b/backend/tests/test_vnc_manager.py @@ -1,9 +1,17 @@ -"""Tests for VNCManager — allocation logic and get_ws_port.""" +"""Tests for VNCManager — allocation logic, quality presets, hw3d, get_ws_port.""" from __future__ import annotations +import os +import socket +import subprocess +import time +from pathlib import Path +from unittest.mock import MagicMock + import pytest +from backend import vnc_manager from backend.vnc_manager import VNCInstance, VNCManager @@ -95,8 +103,12 @@ async def test_active_displays_after_allocate(vnc: VNCManager): def test_get_status_stopped(): from backend.browser_manager import BrowserManager mgr = BrowserManager() - status = mgr.get_status("nonexistent") - assert status == {"status": "stopped", "vnc_ws_port": None, "display": None, "cdp_url": None} + assert mgr.get_status("nonexistent") == { + "status": "stopped", + "vnc_ws_port": None, + "display": None, + "cdp_url": None, + } def test_get_status_running(): @@ -110,10 +122,1166 @@ def test_get_status_running(): ws_port=6100, cdp_port=5100, ) - status = mgr.get_status("abc") - assert status == { + assert mgr.get_status("abc") == { "status": "running", "vnc_ws_port": 6100, "display": ":100", "cdp_url": "/api/profiles/abc/cdp", } + + +def test_get_status_does_not_probe_processes(): + """The 3s profile-list poll must not pay for liveness it discards.""" + from backend.browser_manager import BrowserManager, RunningProfile + from unittest.mock import MagicMock, patch + mgr = BrowserManager() + mgr.running["abc"] = RunningProfile( + profile_id="abc", context=MagicMock(), display=100, ws_port=6100, cdp_port=5100, + ) + with patch("backend.browser_manager.socket.socket") as sock: + mgr.get_status("abc") + sock.assert_not_called() + + +def test_get_liveness_stopped(): + from backend.browser_manager import BrowserManager + mgr = BrowserManager() + assert mgr.get_liveness("nonexistent") == { + "status": "stopped", + "vnc_ws_port": None, + "display": None, + "cdp_url": None, + "xvnc_alive": None, + "browser_alive": None, + } + + +def test_browser_alive_probes_the_cdp_port(): + """A live CDP listener means the Chromium process is up.""" + import socket as socket_mod + from backend.browser_manager import BrowserManager, RunningProfile + from unittest.mock import MagicMock + + listener = socket_mod.socket(socket_mod.AF_INET, socket_mod.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + try: + mgr = BrowserManager() + mgr.running["abc"] = RunningProfile( + profile_id="abc", context=MagicMock(), display=100, ws_port=6100, cdp_port=port, + ) + assert mgr.get_liveness("abc")["browser_alive"] is True + finally: + listener.close() + + # port gone -> the browser is gone. context.pages could never report this: + # Playwright implements it as a local list copy that cannot raise. + assert mgr.get_liveness("abc")["browser_alive"] is False + + +# ── quality presets ────────────────────────────────────────────────────────── + + +def _flag_value(flags: list[str], name: str) -> str: + return flags[flags.index(name) + 1] + + +def test_preset_default_balanced(monkeypatch): + monkeypatch.delenv("KASM_QUALITY_PRESET", raising=False) + assert vnc_manager._quality_preset_name() == "balanced" + + +@pytest.mark.parametrize("name", ["text", "balanced", "low", "motion"]) +def test_preset_from_env(monkeypatch, name: str): + monkeypatch.setenv("KASM_QUALITY_PRESET", name) + assert vnc_manager._quality_preset_name() == name + assert vnc_manager._quality_flags(name) == vnc_manager.QUALITY_PRESETS[name] + + +def test_preset_unknown_falls_back(monkeypatch, caplog): + monkeypatch.setenv("KASM_QUALITY_PRESET", "ultra") + with caplog.at_level("WARNING", logger="cloakbrowser.manager.vnc"): + assert vnc_manager._quality_preset_name() == "balanced" + assert "Unknown KASM_QUALITY_PRESET" in caplog.text + + +def test_preset_values(): + """Spot-check the brief's balanced values (man-page verified spellings).""" + flags = vnc_manager.QUALITY_PRESETS["balanced"] + assert _flag_value(flags, "-FrameRate") == "30" + assert _flag_value(flags, "-DynamicQualityMin") == "6" + assert _flag_value(flags, "-DynamicQualityMax") == "8" + assert _flag_value(flags, "-TreatLossless") == "8" + assert _flag_value(flags, "-JpegVideoQuality") == "5" + assert _flag_value(flags, "-WebpVideoQuality") == "5" + assert _flag_value(flags, "-MaxVideoResolution") == "1600x900" + assert _flag_value(flags, "-VideoTime") == "1" + assert _flag_value(flags, "-VideoArea") == "30" + assert _flag_value(flags, "-VideoOutTime") == "1" + assert _flag_value(flags, "-VideoScaling") == "2" + assert _flag_value(flags, "-webpEncodingTime") == "30" + assert _flag_value(flags, "-CompareFB") == "2" + assert _flag_value(flags, "-RectThreads") == "2" + + +def test_rect_threads_override(monkeypatch): + monkeypatch.setenv("KASM_RECT_THREADS", "4") + flags = vnc_manager._quality_flags("balanced") + assert _flag_value(flags, "-RectThreads") == "4" + + +def test_rect_threads_invalid_keeps_default(monkeypatch, caplog): + monkeypatch.setenv("KASM_RECT_THREADS", "lots") + with caplog.at_level("WARNING", logger="cloakbrowser.manager.vnc"): + flags = vnc_manager._quality_flags("balanced") + assert _flag_value(flags, "-RectThreads") == "2" + assert "Invalid KASM_RECT_THREADS" in caplog.text + + +def test_rect_threads_zero_warns(monkeypatch, caplog): + monkeypatch.setenv("KASM_RECT_THREADS", "0") + with caplog.at_level("WARNING", logger="cloakbrowser.manager.vnc"): + flags = vnc_manager._quality_flags("balanced") + assert _flag_value(flags, "-RectThreads") == "0" + assert "unbounded" in caplog.text + + +# ── hw3d (DRI3) detection ──────────────────────────────────────────────────── + + +@pytest.fixture() +def dri_present(monkeypatch): + """Pretend /dev/dri/ exists; driver resolution per test.""" + real_exists = os.path.exists + monkeypatch.setattr( + vnc_manager.os.path, "exists", + lambda p: p.startswith("/dev/dri/") or real_exists(p), + ) + + +def _fake_driver(monkeypatch, driver: str | None): + def fake_readlink(path): + if path.startswith("/sys/class/drm/"): + if driver is None: + raise OSError("no such device") + return f"../../../../bus/pci/drivers/{driver}" + return os.readlink(path) + + monkeypatch.setattr(vnc_manager.os, "readlink", fake_readlink) + + +def test_hw3d_disabled_by_env(monkeypatch, dri_present): + monkeypatch.setenv("KASM_HW3D", "0") + assert vnc_manager._hw3d_flags() == [] + + +def test_hw3d_auto_no_device(monkeypatch): + monkeypatch.delenv("KASM_HW3D", raising=False) + monkeypatch.setattr(vnc_manager.os.path, "exists", lambda p: False) + assert vnc_manager._hw3d_flags() == [] + + +def test_hw3d_auto_non_nvidia(monkeypatch, dri_present): + monkeypatch.delenv("KASM_HW3D", raising=False) + monkeypatch.delenv("KASM_DRINODE", raising=False) + _fake_driver(monkeypatch, "amdgpu") + assert vnc_manager._hw3d_flags() == ["-hw3d", "-drinode", "/dev/dri/renderD128"] + + +def test_hw3d_auto_nvidia_skipped(monkeypatch, dri_present, caplog): + monkeypatch.delenv("KASM_HW3D", raising=False) + _fake_driver(monkeypatch, "nvidia") + with caplog.at_level("INFO", logger="cloakbrowser.manager.vnc"): + assert vnc_manager._hw3d_flags() == [] + assert "nvidia" in caplog.text + + +def test_hw3d_auto_unresolvable_driver(monkeypatch, dri_present): + """Driver symlink may not resolve in-container — treated as not nvidia.""" + monkeypatch.delenv("KASM_HW3D", raising=False) + _fake_driver(monkeypatch, None) + assert vnc_manager._hw3d_flags() == ["-hw3d", "-drinode", "/dev/dri/renderD128"] + + +def test_hw3d_forced_on_nvidia(monkeypatch, dri_present): + """KASM_HW3D=1 always enables when the device exists, even on nvidia.""" + monkeypatch.setenv("KASM_HW3D", "1") + _fake_driver(monkeypatch, "nvidia") + assert vnc_manager._hw3d_flags() == ["-hw3d", "-drinode", "/dev/dri/renderD128"] + + +def test_hw3d_drinode_override(monkeypatch, dri_present): + monkeypatch.setenv("KASM_HW3D", "auto") + monkeypatch.setenv("KASM_DRINODE", "/dev/dri/renderD129") + _fake_driver(monkeypatch, "i915") + assert vnc_manager._hw3d_flags() == ["-hw3d", "-drinode", "/dev/dri/renderD129"] + + +# ── start_vnc command assembly ─────────────────────────────────────────────── + + +@pytest.fixture() +def xvnc_cmd(monkeypatch): + """Capture the Xvnc command line without spawning a process. + + start_vnc() waits for the websocket port to accept before returning, so + the stub reports the port as ready rather than pretending a sleep was + long enough. + """ + captured: dict[str, list[str]] = {} + + def fake_popen(cmd, **kwargs): + captured["cmd"] = cmd + proc = MagicMock() + proc.poll.return_value = None # still running + return proc + + async def ready_immediately(_port, _process, _timeout): + return True + + async def fake_passwd(display, _password): + return f"/tmp/kasmpasswd-{display}" + + monkeypatch.setattr(vnc_manager.subprocess, "Popen", fake_popen) + monkeypatch.setattr(vnc_manager, "_wait_until_listening", ready_immediately) + monkeypatch.setattr(vnc_manager, "_write_kasm_passwd", fake_passwd) + return captured + + +@pytest.mark.asyncio +async def test_start_vnc_base_command_unchanged(vnc: VNCManager, xvnc_cmd, monkeypatch): + """Display/port scheme and connectivity flags stay as before.""" + monkeypatch.setenv("KASM_HW3D", "0") + await vnc.start_vnc(100, 6100, width=1920, height=1080) + cmd = xvnc_cmd["cmd"] + assert ":100" in cmd + assert _flag_value(cmd, "-websocketPort") == "6100" + assert _flag_value(cmd, "-rfbport") == "-1" + assert _flag_value(cmd, "-geometry") == "1920x1080" + assert _flag_value(cmd, "-depth") == "24" + assert _flag_value(cmd, "-interface") == "127.0.0.1" + assert _flag_value(cmd, "-httpd") == "/usr/share/kasmvnc/www" + assert "-SecurityTypes" in cmd and "-AlwaysShared" in cmd + # Basic auth stays ENABLED (default) — Kasm's HTTP layer needs it for the + # management API; nginx injects per-display creds for viewer traffic. + assert "-DisableBasicAuth" not in cmd + + +@pytest.mark.asyncio +async def test_start_vnc_performance_flags(vnc: VNCManager, xvnc_cmd, monkeypatch): + monkeypatch.setenv("KASM_HW3D", "0") + monkeypatch.delenv("KASM_QUALITY_PRESET", raising=False) + monkeypatch.delenv("KASM_RECT_THREADS", raising=False) + await vnc.start_vnc(100, 6100) + cmd = xvnc_cmd["cmd"] + assert "-IgnoreClientSettingsKasm" in cmd # server owns encoding policy + assert _flag_value(cmd, "-FrameRate") == "30" + assert _flag_value(cmd, "-MaxVideoResolution") == "1600x900" + assert _flag_value(cmd, "-RectThreads") == "2" + assert "-hw3d" not in cmd + # Streaming codec must be set explicitly — the binary default is empty + # (disabled), the man page's "auto" claim is wrong. + assert _flag_value(cmd, "-videoCodec") == "auto" + + +@pytest.mark.asyncio +async def test_start_vnc_preset_from_env(vnc: VNCManager, xvnc_cmd, monkeypatch): + monkeypatch.setenv("KASM_HW3D", "0") + monkeypatch.setenv("KASM_QUALITY_PRESET", "low") + await vnc.start_vnc(100, 6100) + cmd = xvnc_cmd["cmd"] + assert _flag_value(cmd, "-FrameRate") == "24" + assert _flag_value(cmd, "-MaxVideoResolution") == "1280x720" + + +@pytest.mark.asyncio +async def test_start_vnc_hw3d_flags(vnc: VNCManager, xvnc_cmd, monkeypatch, dri_present): + monkeypatch.setenv("KASM_HW3D", "1") + await vnc.start_vnc(100, 6100) + cmd = xvnc_cmd["cmd"] + assert "-hw3d" in cmd + assert _flag_value(cmd, "-drinode") == "/dev/dri/renderD128" + + +@pytest.mark.asyncio +async def test_start_vnc_public_ip_flag(vnc: VNCManager, xvnc_cmd, monkeypatch): + """STUN public-IP discovery must be disabled (no outbound lookups).""" + monkeypatch.setenv("KASM_HW3D", "0") + await vnc.start_vnc(100, 6100) + assert _flag_value(xvnc_cmd["cmd"], "-PublicIP") == "127.0.0.1" + + +@pytest.mark.asyncio +async def test_start_vnc_kasm_passwd(vnc: VNCManager, xvnc_cmd, monkeypatch): + """API password file flag + credentials retrievable for the stats proxy.""" + monkeypatch.setenv("KASM_HW3D", "0") + await vnc.allocate() # start_vnc only stores creds on allocated displays + await vnc.start_vnc(100, 6100) + assert _flag_value(xvnc_cmd["cmd"], "-KasmPasswordFile") == "/tmp/kasmpasswd-100" + creds = vnc.get_api_credentials(100) + assert creds is not None + assert creds[0] == "manager" + assert len(creds[1]) == 32 # secrets.token_hex(16) + + +@pytest.mark.asyncio +async def test_start_vnc_fails_when_credentials_cannot_be_written( + vnc: VNCManager, xvnc_cmd, monkeypatch, +): + """No password file means a permanently 401 viewer — fail the launch.""" + monkeypatch.setenv("KASM_HW3D", "0") + + async def fail_passwd(_display, _password): + raise RuntimeError("kasmvncpasswd not found; cannot create KasmVNC credentials") + + monkeypatch.setattr(vnc_manager, "_write_kasm_passwd", fail_passwd) + await vnc.allocate() + with pytest.raises(RuntimeError, match="kasmvncpasswd"): + await vnc.start_vnc(100, 6100) + # and no Xvnc was spawned for it + assert xvnc_cmd.get("cmd") is None + + +@pytest.mark.asyncio +async def test_write_kasm_passwd_rejects_an_empty_file(monkeypatch): + """kasmvncpasswd can exit 0 after a failed write — verify the artefact.""" + class WritesEmpty: + returncode = 0 + + async def communicate(self, _data): + Path("/tmp/kasmpasswd-901").write_text("") + return (b"", b"") + + monkeypatch.setattr(vnc_manager.shutil, "which", lambda _n: "/usr/bin/kasmvncpasswd") + monkeypatch.setattr( + vnc_manager.asyncio, "create_subprocess_exec", + lambda *a, **k: _coro(WritesEmpty()), + ) + try: + with pytest.raises(RuntimeError, match="empty"): + await vnc_manager._write_kasm_passwd(901, "pw") + finally: + Path("/tmp/kasmpasswd-901").unlink(missing_ok=True) + + +async def _coro(value): + return value + + +@pytest.mark.asyncio +async def test_stop_vnc_removes_passwd_file(vnc: VNCManager, xvnc_cmd, monkeypatch, tmp_path): + monkeypatch.setenv("KASM_HW3D", "0") + await vnc.allocate() + await vnc.start_vnc(100, 6100) + passwd = Path("/tmp/kasmpasswd-100") + passwd.write_text("dummy") + assert passwd.exists() + await vnc.stop_vnc(100) + assert not passwd.exists() + + +@pytest.mark.asyncio +async def test_write_kasm_passwd_success(monkeypatch): + """The helper pipes the password twice and returns the path.""" + calls: dict[str, object] = {} + + class FakeProc: + returncode = 0 + + async def communicate(self, data): + calls["stdin"] = data + Path("/tmp/kasmpasswd-100").write_text("manager:hash\n") # like the real tool + return (b"", b"") + + async def fake_exec(*args, **kwargs): + calls["args"] = args + return FakeProc() + + monkeypatch.setattr(vnc_manager.shutil, "which", lambda _n: "/usr/bin/kasmvncpasswd") + monkeypatch.setattr(vnc_manager.asyncio, "create_subprocess_exec", fake_exec) + try: + path = await vnc_manager._write_kasm_passwd(100, "s3cret") + finally: + Path("/tmp/kasmpasswd-100").unlink(missing_ok=True) + assert path == "/tmp/kasmpasswd-100" + assert calls["args"][:3] == ("/usr/bin/kasmvncpasswd", "-u", "manager") + assert "-wro" in calls["args"] + assert calls["stdin"] == b"s3cret\ns3cret\n" + + +@pytest.mark.asyncio +async def test_write_kasm_passwd_missing_binary(monkeypatch): + """No binary means no credentials means an unusable viewer — raise.""" + monkeypatch.setattr(vnc_manager.shutil, "which", lambda _n: None) + with pytest.raises(RuntimeError, match="kasmvncpasswd not found"): + await vnc_manager._write_kasm_passwd(100, "x") + + +@pytest.mark.asyncio +async def test_stop_vnc_releases_allocation_only_after_the_process_exits(vnc: VNCManager): + """A gap-filling allocate() must not hand out a port Xvnc still holds.""" + from unittest.mock import MagicMock + + display, _ = await vnc.allocate() + observed: dict[str, bool] = {} + + proc = MagicMock() + + def _wait(_timeout=None): + observed["allocated_during_wait"] = display in vnc._allocated + return 0 + + proc.wait.side_effect = _wait + vnc._allocated[display].process = proc + + await vnc.stop_vnc(display) + + assert observed["allocated_during_wait"] is True + assert display not in vnc._allocated + proc.terminate.assert_called_once() + + +@pytest.mark.asyncio +async def test_stop_vnc_waits_after_kill(vnc: VNCManager): + """SIGKILL is followed by a wait so the port is actually released.""" + from unittest.mock import MagicMock + + display, _ = await vnc.allocate() + proc = MagicMock() + proc.wait.side_effect = [subprocess.TimeoutExpired("Xvnc", 5), 0] + vnc._allocated[display].process = proc + + await vnc.stop_vnc(display) + + proc.kill.assert_called_once() + assert proc.wait.call_count == 2 + assert display not in vnc._allocated + + +@pytest.mark.asyncio +async def test_stop_vnc_survives_a_broken_process_handle(vnc: VNCManager): + """One bad handle must not strand the remaining displays in cleanup_all.""" + from unittest.mock import MagicMock + + display, _ = await vnc.allocate() + proc = MagicMock() + proc.terminate.side_effect = ProcessLookupError("no such process") + vnc._allocated[display].process = proc + + await vnc.stop_vnc(display) # must not raise + + assert display not in vnc._allocated + + +# ── VNCManager.is_alive ────────────────────────────────────────────────────── + + +def test_is_alive_unallocated_display(vnc: VNCManager): + assert vnc.is_alive(999) is False + + +@pytest.mark.asyncio +async def test_is_alive_tracks_the_process(vnc: VNCManager): + display, _ = await vnc.allocate() + assert vnc.is_alive(display) is False # allocated, not started + + proc = MagicMock() + proc.poll.return_value = None # running + vnc._allocated[display].process = proc + assert vnc.is_alive(display) is True + + proc.poll.return_value = 1 # exited + assert vnc.is_alive(display) is False + + +# ── launch registration window ─────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_launch_reclaims_display_if_the_browser_dies_before_registration( + monkeypatch, tmp_path, +): + """A close during the registration window must not orphan Xvnc.""" + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = True # died while we were registering + context.pages = [] + context.add_init_script = AsyncMock() + context.close = AsyncMock() + context.on = MagicMock() + + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + stop_vnc = AsyncMock() + monkeypatch.setattr(mgr.vnc, "stop_vnc", stop_vnc) + + profile = {"id": "p1", "user_data_dir": str(tmp_path / "p1")} + with pytest.raises(RuntimeError, match="exited during launch"): + await mgr.launch(profile) + + assert "p1" not in mgr.running + assert "p1" not in mgr._launching + stop_vnc.assert_awaited() # the display was reclaimed + + +# ── Xvnc readiness ─────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_wait_until_listening_detects_a_bound_port(): + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + proc = MagicMock() + proc.poll.return_value = None + try: + assert await vnc_manager._wait_until_listening(port, proc, 2.0) is True + finally: + listener.close() + + +@pytest.mark.asyncio +async def test_wait_until_listening_gives_up_when_the_process_dies(): + """A dead Xvnc is reported immediately, not after the full timeout.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + proc = MagicMock() + proc.poll.return_value = 1 # exited + started = time.monotonic() + assert await vnc_manager._wait_until_listening(port, proc, 5.0) is False + assert time.monotonic() - started < 1.0 + + +@pytest.mark.asyncio +async def test_start_vnc_fails_when_the_port_never_opens(vnc: VNCManager, monkeypatch): + """Launch must not proceed against an Xvnc that is not listening.""" + monkeypatch.setenv("KASM_HW3D", "0") + + waited: dict[str, bool] = {} + + def fake_popen(cmd, **kwargs): + proc = MagicMock() + proc.poll.return_value = None # alive but never binds + proc.wait.side_effect = lambda _t=None: waited.__setitem__("waited", True) + return proc + + async def never_ready(_port, _process, _timeout): + return False + + async def fake_passwd(display, _password): + return f"/tmp/kasmpasswd-{display}" + + monkeypatch.setattr(vnc_manager.subprocess, "Popen", fake_popen) + monkeypatch.setattr(vnc_manager, "_wait_until_listening", never_ready) + monkeypatch.setattr(vnc_manager, "_write_kasm_passwd", fake_passwd) + + await vnc.allocate() + with pytest.raises(RuntimeError, match="failed to start"): + await vnc.start_vnc(100, 6100) + # the process is registered before the readiness check, so teardown goes + # through _terminate: SIGTERM, then SIGKILL, and reaped either way + assert vnc._allocated[100].process is not None + assert waited.get("waited") is True + + +@pytest.mark.asyncio +async def test_launch_releases_everything_when_profile_setup_fails(monkeypatch, tmp_path): + """A full/read-only volume must not brick the profile permanently. + + Without cleanup coverage over the whole setup, the id stays in `_launching` + forever: is_starting() then makes /launch, /stop and DELETE all answer 409 + and the display is never reclaimed, until the container restarts. + """ + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + monkeypatch.setattr( + bm, "_init_profile_defaults", + MagicMock(side_effect=OSError(28, "No space left on device")), + ) + stop_vnc = AsyncMock() + monkeypatch.setattr(mgr.vnc, "stop_vnc", stop_vnc) + + with pytest.raises(OSError): + await mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}) + + assert mgr.is_starting("p1") is False # not bricked + assert "p1" not in mgr.running + stop_vnc.assert_awaited() # display reclaimed + + +@pytest.mark.asyncio +async def test_launch_closes_the_context_when_a_later_step_fails(monkeypatch, tmp_path): + """An aborted launch must not orphan a live Chromium. + + Xvnc gets torn down, so the browser would be left with no X server, still + holding its CDP port and writing to user_data_dir — and the next relaunch + clears the Singleton locks and opens a second Chromium on the same dir. + """ + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = False + context.pages = [] + context.close = AsyncMock() + context.on = MagicMock() + context.add_init_script = AsyncMock(side_effect=RuntimeError("browser wedged")) + + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + with pytest.raises(RuntimeError, match="browser wedged"): + await mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}) + + context.close.assert_awaited() + + +@pytest.mark.asyncio +async def test_launch_closes_the_context_when_cancelled(monkeypatch, tmp_path): + """auto_launch_all wraps launch() in wait_for; a timeout must not orphan.""" + import asyncio as aio + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = False + context.pages = [] + context.close = AsyncMock() + context.on = MagicMock() + + async def never_returns(*_a, **_k): + await aio.sleep(3600) + + context.add_init_script = never_returns + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + with pytest.raises(aio.TimeoutError): + await aio.wait_for( + mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}), timeout=0.05, + ) + + context.close.assert_awaited() + + +@pytest.mark.asyncio +async def test_write_kasm_passwd_ignores_a_stale_file_from_a_previous_run(monkeypatch): + """/tmp survives `docker restart` — a leftover must not pass as success. + + Without the pre-unlink, a silently-failing kasmvncpasswd leaves the old + file in place, the non-empty check passes, and Xvnc starts with credentials + that do not match the password we just generated. + """ + class WritesNothing: + returncode = 0 + + async def communicate(self, _data): + return (b"", b"") + + stale = Path("/tmp/kasmpasswd-903") + stale.write_text("manager:credentials-from-the-previous-container\n") + monkeypatch.setattr(vnc_manager.shutil, "which", lambda _n: "/usr/bin/kasmvncpasswd") + monkeypatch.setattr( + vnc_manager.asyncio, "create_subprocess_exec", + lambda *a, **k: _coro(WritesNothing()), + ) + try: + with pytest.raises(RuntimeError, match="did not create"): + await vnc_manager._write_kasm_passwd(903, "pw") + assert not stale.exists() # and the stale credentials are gone + finally: + stale.unlink(missing_ok=True) + + +@pytest.mark.asyncio +async def test_cleanup_all_stops_displays_concurrently(vnc: VNCManager): + """Shutdown cost must be the slowest display, not the sum of all of them.""" + import asyncio as aio + + for _ in range(4): + await vnc.allocate() + + order: list[str] = [] + + async def slow_stop(display: int): + order.append(f"start-{display}") + await aio.sleep(0.05) + order.append(f"done-{display}") + vnc._allocated.pop(display, None) + + vnc.stop_vnc = slow_stop # type: ignore[assignment] + start = time.monotonic() + await vnc.cleanup_all() + elapsed = time.monotonic() - start + + assert elapsed < 0.15 # not 4 x 0.05 in series + assert order[:4] == [f"start-{d}" for d in (100, 101, 102, 103)] + + +@pytest.mark.asyncio +async def test_launch_cancellation_is_not_held_open_by_a_wedged_context( + monkeypatch, tmp_path, +): + """wait_for(launch, 60) must actually return when the context won't close. + + A wedged Playwright connection makes context.close() hang; awaiting it + unbounded inside the cancellation path means auto_launch_all's timeout + never fires, the display is never reclaimed, and every queued profile + behind it stays stuck reporting "starting". + """ + import asyncio as aio + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + monkeypatch.setattr(bm, "CONTEXT_CLOSE_TIMEOUT_S", 0.05) + + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = False + context.pages = [] + context.on = MagicMock() + + async def never_closes(): + await aio.sleep(3600) + + async def never_returns(*_a, **_k): + await aio.sleep(3600) + + context.close = never_closes + context.add_init_script = never_returns + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + stop_vnc = AsyncMock() + monkeypatch.setattr(mgr.vnc, "stop_vnc", stop_vnc) + + async def cancel_a_launch(): + with pytest.raises(aio.TimeoutError): + await aio.wait_for( + mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}), + timeout=0.05, + ) + + # Outer deadline so an unbounded close fails the test instead of hanging it. + try: + await aio.wait_for(cancel_a_launch(), timeout=3.0) + except aio.TimeoutError: + pytest.fail("cancellation was held open by the wedged context.close()") + + assert mgr.is_starting("p1") is False + stop_vnc.assert_awaited() # display reclaimed despite the wedge + + +@pytest.mark.asyncio +async def test_stop_vnc_keeps_the_display_when_the_process_survives_sigkill( + vnc: VNCManager, +): + """An unreaped Xvnc may still hold the port; don't hand it to the next launch.""" + from unittest.mock import MagicMock + + display, _ = await vnc.allocate() + proc = MagicMock() + proc.wait.side_effect = subprocess.TimeoutExpired("Xvnc", 5) # never dies + vnc._allocated[display].process = proc + + await vnc.stop_vnc(display) + + proc.kill.assert_called_once() + assert display in vnc._allocated # leaked on purpose, not reused + assert (await vnc.allocate())[0] != display + + +@pytest.mark.asyncio +async def test_late_close_does_not_evict_the_replacement(monkeypatch, tmp_path): + """A close arriving from a superseded context must be ignored. + + Otherwise a slow teardown that outlived its bounded wait pops whatever + instance owns the profile id by then, killing a freshly launched session + and orphaning its Chromium. + """ + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + old_context = MagicMock(name="A") + new_running = bm.RunningProfile( + profile_id="p1", context=MagicMock(name="B"), display=101, ws_port=6101, cdp_port=5101, + ) + mgr.running["p1"] = new_running + stop_vnc = AsyncMock() + monkeypatch.setattr(mgr.vnc, "stop_vnc", stop_vnc) + + # context A finally finishes closing, long after it was superseded + await mgr._on_browser_closed("p1", old_context) + + assert mgr.running.get("p1") is new_running # replacement untouched + stop_vnc.assert_not_awaited() + + # the live context's own close still works + await mgr._on_browser_closed("p1", new_running.context) + assert "p1" not in mgr.running + stop_vnc.assert_awaited() + + +@pytest.mark.asyncio +async def test_a_wedged_stop_blocks_relaunch_until_the_close_lands(monkeypatch, tmp_path): + """stop() pops the profile, so the wedge must be recorded somewhere. + + Otherwise a relaunch starts a SECOND Chromium on the same live + user_data_dir, and a delete rmtree's under the first one. + """ + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + monkeypatch.setattr(bm, "CONTEXT_CLOSE_TIMEOUT_S", 0.05) + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = False + + async def never_closes(): + await __import__("asyncio").sleep(3600) + + context.close = never_closes + mgr.running["p1"] = bm.RunningProfile( + profile_id="p1", context=context, display=100, ws_port=6100, cdp_port=5100, + ) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + assert await mgr.stop("p1") is False + assert mgr.is_wedged("p1") is True + + # a relaunch must be refused while the old Chromium is still alive + with pytest.raises(bm.ProfileAlreadyRunning): + await mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}) + + # ...and allowed again once the close finally lands + await mgr._on_browser_closed("p1", context) + assert mgr.is_wedged("p1") is False + + +@pytest.mark.asyncio +async def test_aborted_launch_records_a_wedged_browser(monkeypatch, tmp_path): + """A cancelled launch whose close also wedges must gate later mutations. + + The profile is in none of running/_launching, so without recording it a + relaunch starts a second Chromium on the same live user_data_dir. + """ + import asyncio as aio + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + monkeypatch.setattr(bm, "CONTEXT_CLOSE_TIMEOUT_S", 0.05) + mgr = bm.BrowserManager() + handlers: list = [] + context = MagicMock() + context.is_closed.return_value = False + context.pages = [] + context.on = MagicMock(side_effect=lambda _evt, cb: handlers.append(cb)) + + async def never(*_a, **_k): + await aio.sleep(3600) + + context.close = never + context.add_init_script = never + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + with pytest.raises(aio.TimeoutError): + await aio.wait_for( + mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}), timeout=0.05, + ) + + assert mgr.is_starting("p1") is False + assert "p1" not in mgr.running + assert mgr.is_wedged("p1") is True # gated, not forgotten + assert handlers, "close handler must be registered before the setup awaits" + + # the handler registered up-front is what eventually clears it + await mgr._on_browser_closed("p1", context) + assert mgr.is_wedged("p1") is False + + +@pytest.mark.asyncio +async def test_wedge_clears_even_when_another_instance_holds_the_id(monkeypatch): + """The wedged context's own close must resolve it. + + _on_browser_closed returns early for a superseded context, so clearing the + wedge after that check would leave the profile blocked forever once any + other instance held the id. + """ + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + wedged_context = MagicMock(name="A") + mgr._closing["p1"] = (wedged_context, time.monotonic() + 60, None, time.monotonic()) + # some other instance now owns the id + mgr.running["p1"] = bm.RunningProfile( + profile_id="p1", context=MagicMock(name="B"), display=101, ws_port=6101, cdp_port=5101, + ) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + await mgr._on_browser_closed("p1", wedged_context) + + assert mgr.is_wedged("p1") is False # resolved + assert "p1" in mgr.running # and the live instance untouched + + +@pytest.mark.asyncio +async def test_profile_is_guarded_for_the_whole_teardown_not_just_on_failure( + monkeypatch, +): + """The close window itself must be guarded. + + stop() pops the profile from `running` and then spends up to + CONTEXT_CLOSE_TIMEOUT_S closing. Recording the browser only after that + close FAILS leaves the entire window unguarded, so a DELETE arriving + mid-close rmtree's user_data_dir under a live Chromium. + """ + import asyncio as aio + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + release = aio.Event() + context = MagicMock() + context.is_closed.return_value = False + + async def slow_close(): + await release.wait() + + context.close = slow_close + mgr.running["p1"] = bm.RunningProfile( + profile_id="p1", context=context, display=100, ws_port=6100, cdp_port=5100, + ) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + task = aio.ensure_future(mgr.stop("p1")) + await aio.sleep(0) # let stop() reach the close + await aio.sleep(0) + + # mid-close: gone from running, but must NOT look free + assert "p1" not in mgr.running + assert mgr.is_wedged("p1") is True + + release.set() + assert await task is True # closed cleanly in the end + assert mgr.is_wedged("p1") is False # and the claim is released + + +@pytest.mark.asyncio +async def test_a_browser_that_never_closes_does_not_brick_the_profile(monkeypatch): + """The guard needs a release valve. + + Without a ceiling, a browser that never reports its close leaves launch + 409ing, delete 409ing and stop 404ing for the life of the container — the + profile is simply unusable. + """ + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + mgr._closing["p1"] = (MagicMock(), time.monotonic() + bm.CLOSING_CLAIM_TTL_S, None, time.monotonic()) + assert mgr.is_wedged("p1") is True + + # ...once the claim expires the profile is usable again + mgr._closing["p1"] = (MagicMock(), time.monotonic() - 0.01, None, time.monotonic()) + assert mgr.is_wedged("p1") is False + assert "p1" not in mgr._closing # and the entry is dropped + + +def test_unknown_hw3d_value_falls_back_to_auto(monkeypatch, caplog): + """A typo must not bypass the NVIDIA auto-detect and force -hw3d.""" + monkeypatch.setenv("KASM_HW3D", "enabled") # not a recognised value + monkeypatch.setenv("KASM_DRINODE", "/dev/dri/renderD128") + monkeypatch.setattr(vnc_manager.os.path, "exists", lambda _p: True) + monkeypatch.setattr(vnc_manager, "_dri_driver", lambda _n: "nvidia") + + with caplog.at_level("WARNING", logger="cloakbrowser.manager.vnc"): + flags = vnc_manager._hw3d_flags() + + assert flags == [] # auto-detect applied + assert "Unknown KASM_HW3D" in caplog.text + + +def test_explicit_hw3d_still_forces_past_the_nvidia_check(monkeypatch): + monkeypatch.setenv("KASM_HW3D", "1") + monkeypatch.setenv("KASM_DRINODE", "/dev/dri/renderD128") + monkeypatch.setattr(vnc_manager.os.path, "exists", lambda _p: True) + monkeypatch.setattr(vnc_manager, "_dri_driver", lambda _n: "nvidia") + + assert vnc_manager._hw3d_flags() == ["-hw3d", "-drinode", "/dev/dri/renderD128"] + + +@pytest.mark.asyncio +async def test_launch_abort_guards_the_profile_for_its_whole_cleanup(monkeypatch, tmp_path): + """Same window stop() closes: the abort's close must be guarded too.""" + import asyncio as aio + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + release = aio.Event() + context = MagicMock() + context.is_closed.return_value = False + context.pages = [] + context.on = MagicMock() + + async def slow_close(): + await release.wait() + + async def boom(*_a, **_k): + raise RuntimeError("setup failed") + + context.close = slow_close + context.add_init_script = boom + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + task = aio.ensure_future(mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")})) + for _ in range(8): + await aio.sleep(0) # let it reach the abort's close + + assert mgr.is_starting("p1") is False # already out of _launching + assert mgr.is_wedged("p1") is True # but NOT free + + release.set() + with pytest.raises(RuntimeError, match="setup failed"): + await task + assert mgr.is_wedged("p1") is False # claim released once closed + + +@pytest.mark.asyncio +async def test_claim_is_not_released_while_the_browser_is_still_listening(): + """The TTL must decide on evidence, not on the X-server assumption. + + A headless profile keeps running after stop_vnc() kills the display, so + releasing purely on elapsed time hands a live browser's user_data_dir to + the next launch or delete. + """ + import socket as socket_mod + from backend import browser_manager as bm + + listener = socket_mod.socket(socket_mod.AF_INET, socket_mod.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + mgr = bm.BrowserManager() + try: + # claim already expired, but the browser is demonstrably alive + mgr._closing["p1"] = (MagicMock(), time.monotonic() - 1, port, time.monotonic()) + assert mgr.is_wedged("p1") is True + # and the deadline is pushed out rather than the claim dropped + assert mgr._closing["p1"][1] > time.monotonic() + finally: + listener.close() + + # once it really is gone, the expired claim releases + mgr._closing["p1"] = (MagicMock(), time.monotonic() - 1, port, time.monotonic()) + assert mgr.is_wedged("p1") is False + assert "p1" not in mgr._closing + + +@pytest.mark.asyncio +async def test_browser_exit_during_registration_reclaims_the_display_once( + monkeypatch, tmp_path, +): + """A duplicate stop_vnc could pop a concurrent relaunch's fresh allocation.""" + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = True # died during registration + context.pages = [] + context.add_init_script = AsyncMock() + context.close = AsyncMock() + context.on = MagicMock() + + monkeypatch.setattr(bm, "launch_persistent_context_async", AsyncMock(return_value=context)) + monkeypatch.setattr(mgr.vnc, "start_vnc", AsyncMock()) + stop_vnc = AsyncMock() + monkeypatch.setattr(mgr.vnc, "stop_vnc", stop_vnc) + + with pytest.raises(RuntimeError, match="exited during launch"): + await mgr.launch({"id": "p1", "user_data_dir": str(tmp_path / "p1")}) + + assert stop_vnc.await_count == 1 # exactly once, not twice + + +@pytest.mark.asyncio +async def test_cancelling_stop_keeps_the_guard(monkeypatch): + """The close is shielded, so a cancelled stop() must not free the profile.""" + import asyncio as aio + from unittest.mock import AsyncMock + from backend import browser_manager as bm + + mgr = bm.BrowserManager() + context = MagicMock() + context.is_closed.return_value = False + + async def slow_close(): + await aio.sleep(3600) + + context.close = slow_close + mgr.running["p1"] = bm.RunningProfile( + profile_id="p1", context=context, display=100, ws_port=6100, cdp_port=5100, + ) + monkeypatch.setattr(mgr.vnc, "stop_vnc", AsyncMock()) + + stop_vnc = AsyncMock() + monkeypatch.setattr(mgr.vnc, "stop_vnc", stop_vnc) + + with pytest.raises(aio.TimeoutError): + await aio.wait_for(mgr.stop("p1"), timeout=0.05) + await aio.sleep(0) + + assert mgr.is_wedged("p1") is True # still guarded + stop_vnc.assert_awaited() # but the display is NOT leaked + + +@pytest.mark.asyncio +async def test_a_recycled_cdp_port_cannot_brick_a_profile_forever(): + """"Something answers" is not proof it is OUR browser. + + CDP ports cycle 5100-5199, so a later profile's Chromium can bind the port + a stale claim remembers. Without an absolute ceiling that unrelated browser + would extend the claim indefinitely. + """ + import socket as socket_mod + from backend import browser_manager as bm + + listener = socket_mod.socket(socket_mod.AF_INET, socket_mod.SOCK_STREAM) + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + mgr = bm.BrowserManager() + try: + # held just under the ceiling: a listening port still extends it + mgr._closing["p1"] = ( + MagicMock(), time.monotonic() - 1, port, + time.monotonic() - (bm.CLOSING_CLAIM_MAX_S - 5), + ) + assert mgr.is_wedged("p1") is True + + # past the ceiling: released despite the port still answering + mgr._closing["p1"] = ( + MagicMock(), time.monotonic() - 1, port, + time.monotonic() - (bm.CLOSING_CLAIM_MAX_S + 1), + ) + assert mgr.is_wedged("p1") is False + assert "p1" not in mgr._closing + finally: + listener.close() diff --git a/backend/viewer_tokens.py b/backend/viewer_tokens.py new file mode 100644 index 00000000..de0de791 --- /dev/null +++ b/backend/viewer_tokens.py @@ -0,0 +1,79 @@ +"""Opaque, short-lived viewer session tokens for the KasmVNC native client path.""" + +from __future__ import annotations + +import secrets +import time +from dataclasses import dataclass + +# Tokens are scoped to one running profile's viewer session: page assets and +# the WebSocket all share one token. The TTL must comfortably outlive typical +# sessions because the KasmVNC client's own (seamless, frame-preserving) +# reconnect re-uses the same WebSocket URL — an expired token forces a full +# iframe reload instead. Revocation on profile stop is the real guard. +VIEWER_TOKEN_TTL = 3600 # seconds + + +@dataclass +class ViewerSession: + token: str + profile_id: str + ws_port: int + issued_at: float + expires_at: float + + +class ViewerTokenStore: + """In-memory store for viewer tokens. + + Single event loop, plain dict, no awaits inside methods — safe enough + for async use without locking. + """ + + def __init__(self): + self._sessions: dict[str, ViewerSession] = {} + + def issue(self, profile_id: str, ws_port: int, ttl: int = VIEWER_TOKEN_TTL) -> str: + """Issue a fresh token for a profile. Old tokens stay valid until expiry.""" + token = secrets.token_urlsafe(32) + now = time.time() + # Sweep here, not only in validate(): once the iframe is re-pointed its + # old token is never presented again, so the lazy per-token purge never + # runs for it. Every reconnect mints one, and revoke_profile() only + # bounds the map when the profile eventually stops — which for an + # auto-launch profile may be never. + for stale in [t for t, sess in self._sessions.items() if sess.expires_at <= now]: + del self._sessions[stale] + self._sessions[token] = ViewerSession( + token=token, + profile_id=profile_id, + ws_port=ws_port, + issued_at=now, + expires_at=now + ttl, + ) + return token + + def validate(self, token: str) -> ViewerSession | None: + """Return the session for a valid token, else None (unknown or expired).""" + session = self._sessions.get(token) + if session is None: + return None + if time.time() >= session.expires_at: + # Lazy purge on expiry + del self._sessions[token] + return None + return session + + def revoke_profile(self, profile_id: str) -> None: + """Revoke all tokens for a profile (called on stop/delete).""" + doomed = [t for t, s in self._sessions.items() if s.profile_id == profile_id] + for t in doomed: + del self._sessions[t] + + @property + def active_count(self) -> int: + """Number of sessions currently held (for tests/diagnostics).""" + return len(self._sessions) + + +viewer_tokens = ViewerTokenStore() diff --git a/backend/vnc_manager.py b/backend/vnc_manager.py index d43e5483..a44b4360 100644 --- a/backend/vnc_manager.py +++ b/backend/vnc_manager.py @@ -4,18 +4,227 @@ import asyncio import logging +import os +import secrets import shutil +import socket import subprocess +import time from dataclasses import dataclass, field logger = logging.getLogger("cloakbrowser.manager.vnc") +# Server-authoritative quality presets (Xvnc CLI flags; spellings per +# Xkasmvnc(1) 1.5.0). Client-side quality settings are ignored via +# -IgnoreClientSettingsKasm, so the active preset is the whole policy. +# -videoCodec is set explicitly in start_vnc: the binary default is empty +# (streaming disabled) despite the man page claiming "auto". +QUALITY_PRESETS: dict[str, list[str]] = { + # Crisp text/UI priority: high quality, reluctant to enter video mode. + "text": [ + "-FrameRate", "30", + "-DynamicQualityMin", "7", "-DynamicQualityMax", "9", "-TreatLossless", "9", + "-JpegVideoQuality", "5", "-WebpVideoQuality", "5", + "-MaxVideoResolution", "1920x1080", + "-VideoTime", "2", "-VideoArea", "45", "-VideoOutTime", "1", + "-VideoScaling", "2", # progressive bilinear + "-webpEncodingTime", "30", + "-CompareFB", "2", # auto + "-RectThreads", "2", + ], + # Default: good text quality, quick switch to video mode on motion. + "balanced": [ + "-FrameRate", "30", + "-DynamicQualityMin", "6", "-DynamicQualityMax", "8", "-TreatLossless", "8", + "-JpegVideoQuality", "5", "-WebpVideoQuality", "5", + "-MaxVideoResolution", "1600x900", + "-VideoTime", "1", "-VideoArea", "30", "-VideoOutTime", "1", + "-VideoScaling", "2", + "-webpEncodingTime", "30", + "-CompareFB", "2", + "-RectThreads", "2", + ], + # Bandwidth-saver: lower framerate/quality, capped video resolution. + "low": [ + "-FrameRate", "24", + "-DynamicQualityMin", "4", "-DynamicQualityMax", "7", "-TreatLossless", "8", + "-JpegVideoQuality", "4", "-WebpVideoQuality", "4", + "-MaxVideoResolution", "1280x720", + "-VideoTime", "1", "-VideoArea", "20", "-VideoOutTime", "1", + "-VideoScaling", "2", + "-webpEncodingTime", "30", + "-CompareFB", "2", + "-RectThreads", "2", + ], + # Motion content (video/gaming): quick video-mode entry, fluid over sharp. + "motion": [ + "-FrameRate", "30", + "-DynamicQualityMin", "5", "-DynamicQualityMax", "8", "-TreatLossless", "8", + "-JpegVideoQuality", "4", "-WebpVideoQuality", "5", + "-MaxVideoResolution", "1280x720", + "-VideoTime", "1", "-VideoArea", "20", "-VideoOutTime", "1", + "-VideoScaling", "2", + "-webpEncodingTime", "30", + "-CompareFB", "2", + "-RectThreads", "2", + ], +} + + +def _quality_preset_name() -> str: + """Active preset from KASM_QUALITY_PRESET (default 'balanced').""" + name = os.environ.get("KASM_QUALITY_PRESET", "balanced").strip().lower() + if name not in QUALITY_PRESETS: + logger.warning("Unknown KASM_QUALITY_PRESET=%r, falling back to 'balanced'", name) + return "balanced" + return name + + +def _quality_flags(preset: str) -> list[str]: + flags = list(QUALITY_PRESETS[preset]) + raw = os.environ.get("KASM_RECT_THREADS") + if raw is not None: + idx = flags.index("-RectThreads") + 1 + flags[idx] = _rect_threads_value(raw, flags[idx]) + return flags + + +def _rect_threads_value(raw: str, default: str) -> str: + """Validate a KASM_RECT_THREADS override (0 = auto, see warning).""" + try: + threads = int(raw) + except ValueError: + logger.warning("Invalid KASM_RECT_THREADS=%r, using preset default %s", raw, default) + return default + if threads < 0: + logger.warning("Invalid KASM_RECT_THREADS=%r, using preset default %s", raw, default) + return default + if threads == 0: + logger.warning( + "KASM_RECT_THREADS=0: unbounded automatic encoder threads — risky multi-tenant" + ) + return str(threads) + + +def _dri_driver(node: str) -> str | None: + """Driver name for a DRI render node, or None if unresolvable in-container.""" + try: + return os.path.basename(os.readlink(f"/sys/class/drm/{os.path.basename(node)}/device/driver")) + except OSError: + return None + + +def _hw3d_flags() -> list[str]: + """DRI3 hardware 3D flags per KASM_HW3D (auto/1/true/yes/0) + KASM_DRINODE.""" + raw = os.environ.get("KASM_HW3D", "auto").strip().lower() + if raw in ("0", "false", "no"): + logger.info("KasmVNC hw3d disabled (KASM_HW3D=%s)", raw) + return [] + if raw in ("1", "true", "yes"): + mode = "force" + elif raw == "auto": + mode = "auto" + else: + # Anything unrecognised used to fall through to the force branch, so a + # typo silently bypassed the NVIDIA check and passed -hw3d to Xvnc on a + # driver without DRI3. + logger.warning("Unknown KASM_HW3D=%r, falling back to 'auto'", raw) + mode = "auto" + node = os.environ.get("KASM_DRINODE", "/dev/dri/renderD128") + if not os.path.exists(node): + logger.info("KasmVNC hw3d disabled: %s not present", node) + return [] + if mode == "auto": + # Closed-source NVIDIA lacks DRI3; unresolvable driver counts as OK. + driver = _dri_driver(node) + if driver == "nvidia": + logger.info("KasmVNC hw3d disabled: %s uses the nvidia driver (no DRI3)", node) + return [] + logger.info("KasmVNC hw3d enabled on %s (auto, driver=%s)", node, driver or "unknown") + else: + logger.info("KasmVNC hw3d enabled on %s (KASM_HW3D=%s)", node, raw) + return ["-hw3d", "-drinode", node] + + +# Xvnc binds its websocket port within milliseconds; the ceiling only covers +# a heavily loaded host. +XVNC_READY_TIMEOUT_S = 15.0 +_XVNC_POLL_INTERVAL_S = 0.05 + + +async def _wait_until_listening( + port: int, process: subprocess.Popen, timeout: float, +) -> bool: + """Poll 127.0.0.1:port until Xvnc accepts, it dies, or we run out of time.""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if process.poll() is not None: + return False # exited during startup + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.settimeout(0.25) + if probe.connect_ex(("127.0.0.1", port)) == 0: + return True + await asyncio.sleep(_XVNC_POLL_INTERVAL_S) + return False + @dataclass class VNCInstance: display: int ws_port: int process: subprocess.Popen | None = None + api_password: str | None = None + + +# KasmVNC's HTTP layer (static client, WebSocket upgrade, management /api) +# requires Basic-auth credentials from the -KasmPasswordFile file. We generate +# a random per-display password at launch; nginx injects it for viewer traffic +# and the Manager uses it for the stats proxy. It never leaves the server. +KASM_API_USER = "manager" + + +async def _write_kasm_passwd(display: int, password: str) -> str: + """Create /tmp/kasmpasswd- with an owner user, or raise. + + These credentials are not optional. Kasm's HTTP layer requires Basic auth + for the static client, the WebSocket upgrade and the management API alike, + and there is no -DisableBasicAuth in the launch flags — so starting Xvnc + without a password file yields a profile that reports itself perfectly + healthy while every viewer request 401s, which the reconnect machine can + only loop against. Failing the launch is the honest outcome. + """ + path = f"/tmp/kasmpasswd-{display}" + passwd_bin = shutil.which("kasmvncpasswd") + if not passwd_bin: + raise RuntimeError("kasmvncpasswd not found; cannot create KasmVNC credentials") + # /tmp survives `docker restart`, so a file from a previous run can still be + # here for this display. Remove it first: otherwise a silently-failing + # kasmvncpasswd leaves the stale file behind, the non-empty check below + # passes, and Xvnc starts with credentials that no longer match the password + # we generated — the permanently-401 viewer this function exists to prevent. + try: + os.unlink(path) + except FileNotFoundError: + pass + proc = await asyncio.create_subprocess_exec( + passwd_bin, "-u", KASM_API_USER, "-wro", path, + stdin=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.PIPE, + ) + _, stderr = await proc.communicate(f"{password}\n{password}\n".encode()) + if proc.returncode != 0: + raise RuntimeError( + f"kasmvncpasswd failed: {stderr.decode(errors='replace').strip()}" + ) + # It can exit 0 after a failed write/rename (e.g. /tmp full or read-only), + # so verify the artefact rather than trusting the status. + try: + if os.path.getsize(path) == 0: + raise RuntimeError(f"kasmvncpasswd wrote an empty {path}") + except OSError as exc: + raise RuntimeError(f"kasmvncpasswd did not create {path}: {exc}") from exc + return path class VNCManager: @@ -57,15 +266,44 @@ async def start_vnc( "-rfbport", "-1", # disable raw VNC TCP port — WebSocket only "-geometry", f"{width}x{height}", "-depth", "24", - "-SecurityTypes", "None", - "-DisableBasicAuth", - "-interface", "127.0.0.1", # internal only, proxied by FastAPI + "-SecurityTypes", "None", # no RFB-layer auth; HTTP Basic guards the port + "-interface", "127.0.0.1", # internal only, proxied by nginx "-AlwaysShared", "-httpd", httpd_dir, ] + # Server-authoritative performance policy (KASM_QUALITY_PRESET); + # clients cannot override quality/encoding settings. + preset = _quality_preset_name() + cmd += ["-IgnoreClientSettingsKasm"] + _quality_flags(preset) + + # Never query STUN for a public IP: UDP/WebRTC transit is out of + # scope (Cloudflare Tunnel carries WSS only), and the lookup leaks. + cmd += ["-PublicIP", "127.0.0.1"] + + # In-band H.264/H.265/AV1 streaming (WebCodecs over the same + # WebSocket). The raw binary's default is EMPTY (= disabled) despite + # the man page claiming "auto" — set it explicitly. "auto" prefers + # VAAPI (when -hw3d's GPU is usable) and falls back to software + # encoders via dlopen'd FFmpeg; without FFmpeg it silently stays on + # JPEG/WebP rects. + cmd += ["-videoCodec", "auto"] + + # Owner credentials guard Kasm's HTTP layer (static client, WS + # upgrade, management /api). nginx injects them on behalf of + # token-authorized viewer requests, so the browser never sees them + # (and the Manager's stats proxy uses them directly). + api_password = secrets.token_hex(16) + cmd += ["-KasmPasswordFile", await _write_kasm_passwd(display, api_password)] + + # DRI3 GPU acceleration (KASM_HW3D / KASM_DRINODE) + cmd += _hw3d_flags() + log_path = f"/tmp/xvnc-{display}.log" - logger.info("Starting Xvnc on :%d (ws_port=%d) log=%s", display, ws_port, log_path) + logger.info( + "Starting Xvnc on :%d (ws_port=%d, preset=%s) log=%s", + display, ws_port, preset, log_path, + ) log_file = open(log_path, "w") proc = subprocess.Popen( @@ -75,46 +313,124 @@ async def start_vnc( ) log_file.close() # Popen inherited the fd, parent doesn't need it - # Wait a moment for Xvnc to initialize - await asyncio.sleep(0.5) + # Register immediately, not after the readiness check: otherwise a + # startup failure leaves stop_vnc() with instance.process is None, so it + # skips _terminate() entirely and pops the allocation with no wait() — + # bypassing the very guarantee stop_vnc exists to provide. Every + # teardown path should go through the same reaping code. + async with self._lock: + if display in self._allocated: + self._allocated[display].process = proc + self._allocated[display].api_password = api_password - if proc.poll() is not None: + # Wait for the websocket port to actually accept, not a fixed sleep. + # A sleep that is merely usually long enough lets launch() proceed + # against an Xvnc that is not listening (or already dead), and the + # viewer then loops reconnecting against a port nobody answers. + if not await _wait_until_listening(ws_port, proc, XVNC_READY_TIMEOUT_S): try: with open(log_path) as f: err = f.read() except Exception as exc: logger.debug("Failed to read Xvnc log %s: %s", log_path, exc) err = "" + # Reap here too: a SIGKILLed X server does not remove + # /tmp/.X-lock, so leaving it unreaped can make the next + # allocation of this display fail with "Server is already active". + await self._terminate(proc, display) raise RuntimeError(f"Xvnc failed to start on :{display}: {err}") - async with self._lock: - if display in self._allocated: - self._allocated[display].process = proc - return proc async def stop_vnc(self, display: int): - """Kill Xvnc for given display and release allocation.""" + """Kill Xvnc for the given display, then release the allocation. + + Ordering matters. Releasing first lets a concurrent allocate() gap-fill + the same display/ws_port while the old Xvnc is still holding the port — + the new Xvnc then fails to bind and POST /launch answers 500 — and lets + this call's password-file unlink delete the *new* instance's file. A + routine stop->relaunch is enough to hit it. + + Never raises: cleanup_all() iterates over displays, and one bad process + handle must not strand the rest. + """ async with self._lock: - instance = self._allocated.pop(display, None) + instance = self._allocated.get(display) + reaped = True if instance and instance.process: logger.info("Stopping Xvnc on :%d", display) - instance.process.terminate() try: - await asyncio.get_event_loop().run_in_executor( - None, instance.process.wait, 5, - ) - except subprocess.TimeoutExpired: - instance.process.kill() + reaped = await self._terminate(instance.process, display) + except Exception as exc: + logger.warning("Error stopping Xvnc on :%d: %s", display, exc) + # A handle that raises (ProcessLookupError and friends) usually + # means the process is already gone; trust poll() over the + # exception rather than leaking the display on a stale handle. + reaped = instance.process.poll() is not None + + if not reaped: + # The process outlived SIGKILL, so it may still hold the X display + # and the websocket port. Releasing the allocation would hand both + # to the next launch, which would then fail to bind. Leaking one + # display is the lesser failure — and allocate() skips it. + logger.error( + "Xvnc on :%d could not be reaped; leaving the display allocated", display, + ) + return + + async with self._lock: + self._allocated.pop(display, None) + + # Remove the per-display API password file + try: + os.unlink(f"/tmp/kasmpasswd-{display}") + except OSError: + pass + + @staticmethod + async def _terminate(process: subprocess.Popen, display: int) -> bool: + """SIGTERM, then SIGKILL if it outlives the grace period. + + Returns whether the process was actually reaped. Both paths wait(): + reaping here is what makes "the port is free" true by the time the + allocation is released, so a False result must block that release. + """ + loop = asyncio.get_running_loop() + process.terminate() + try: + await loop.run_in_executor(None, process.wait, 5) + return True + except subprocess.TimeoutExpired: + logger.warning("Xvnc on :%d ignored SIGTERM; killing", display) + process.kill() + try: + await loop.run_in_executor(None, process.wait, 5) + return True + except subprocess.TimeoutExpired: + logger.error("Xvnc on :%d survived SIGKILL", display) + return False + + def is_alive(self, display: int) -> bool: + """Whether this display's Xvnc process is still running.""" + instance = self._allocated.get(display) + return bool(instance and instance.process and instance.process.poll() is None) + + def get_api_credentials(self, display: int) -> tuple[str, str] | None: + """Basic-auth credentials for Kasm's management API on this display.""" + instance = self._allocated.get(display) + if instance and instance.api_password: + return (KASM_API_USER, instance.api_password) + return None async def cleanup_all(self): """Kill all managed Xvnc processes. Called on shutdown.""" async with self._lock: displays = list(self._allocated.keys()) - for display in displays: - await self.stop_vnc(display) + # Concurrently, for the same reason as BrowserManager.cleanup_all(): + # each display's SIGTERM grace would otherwise be additive. + await asyncio.gather(*(self.stop_vnc(d) for d in displays)) async def cleanup_stale(self): """Kill orphan Xvnc processes from previous runs.""" diff --git a/docker-compose.gpu.yml b/docker-compose.gpu.yml new file mode 100644 index 00000000..e85b2549 --- /dev/null +++ b/docker-compose.gpu.yml @@ -0,0 +1,14 @@ +# GPU passthrough overlay — opt in only if the host actually has /dev/dri. +# +# docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d --build +# +# Enables KasmVNC DRI3 screen capture (-hw3d) and VAAPI H.264/H.265/AV1 +# streaming encode on AMD/Intel open-source drivers. Closed-source NVIDIA does +# NOT support DRI3; the manager auto-detects that and stays on software encode. +# +# Docker fails container creation outright when a mapped device node does not +# exist, which is why this lives outside the default compose file. +services: + manager: + devices: + - "/dev/dri:/dev/dri" diff --git a/docker-compose.yml b/docker-compose.yml index 24deb6c1..fc6fcf55 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,9 +1,28 @@ services: manager: build: . + # The entrypoint exits if nginx or uvicorn dies; this is what turns that + # into recovery instead of a permanently broken container. + restart: unless-stopped + # The entrypoint waits for uvicorn's lifespan to close every browser and + # Xvnc before stopping nginx. Docker's 10s default would SIGKILL PID 1 + # mid-cleanup with more than a couple of profiles running, leaving exactly + # the unclean Chromium shutdown (and restore prompts) that ordering avoids. + stop_grace_period: 60s ports: - "127.0.0.1:8080:8080" volumes: - ~/.cloakbrowser-manager:/data environment: + # Bare names pass the host value through when set and are ignored when + # not. Without these the tuning knobs the README documents have no effect + # under Compose — they only reached the container via `docker run -e`. - AUTH_TOKEN=${AUTH_TOKEN:-} + - KASM_QUALITY_PRESET + - KASM_RECT_THREADS + - KASM_HW3D + - KASM_DRINODE + # GPU passthrough is opt-in — Docker hard-fails container creation when a + # `devices:` source path is missing, so mapping /dev/dri here would break + # every GPU-less host. Enable it with the override file instead: + # docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d diff --git a/docker/nginx.conf b/docker/nginx.conf new file mode 100644 index 00000000..2f6b5ad3 --- /dev/null +++ b/docker/nginx.conf @@ -0,0 +1,155 @@ +# CloakBrowser Manager — nginx data plane. +# Public entry on :8080 inside the container: +# / → FastAPI control plane (SPA + /api, incl. CDP WebSockets) +# /viewer/... → per-profile KasmVNC websocket port, gated by FastAPI auth + +user www-data; +worker_processes auto; +pid /run/nginx.pid; +error_log /dev/stderr warn; + +events { + worker_connections 1024; +} + +http { + include /etc/nginx/mime.types; + default_type application/octet-stream; + + sendfile on; + tcp_nopush on; + keepalive_timeout 65; + + # WebSocket upgrade header mapping + map $http_upgrade $connection_upgrade { + default upgrade; + '' close; + } + + # Preserve the ORIGINAL client scheme. This nginx always listens plain + # HTTP, so `$scheme` is always "http" — forwarding it verbatim would erase + # the "https" an outer TLS terminator (Cloudflare Tunnel, ingress LB) sent, + # and the control plane would then advertise ws:// CDP URLs to an https + # page. Fall back to $scheme only when there is no inbound value. + map $http_x_forwarded_proto $forwarded_proto { + default $http_x_forwarded_proto; + '' $scheme; + } + + # Original request path without query string ($uri is post-rewrite, + # $request_uri carries args — the viewer token travels in both). + map $request_uri $request_path { + ~^(?[^?]*) $noargs_path; + default $uri; + } + + # Never log the viewer token: /viewer//... → /viewer/_/... + map $request_path $redacted_uri { + ~^/viewer/[^/]+/(?.*)$ /viewer/_/$redacted_rest; + default $request_path; + } + + # No query strings in the log line — the viewer URL carries the token. + log_format redacted '$remote_addr - [$time_local] ' + '"$request_method $redacted_uri $server_protocol" ' + '$status $body_bytes_sent "$http_user_agent"'; + + access_log /dev/stdout redacted; + + server { + listen 8080; + server_name _; + + # ── Authenticated KasmVNC viewer proxy ─────────────────────────── + location /viewer/ { + # Block Kasm's management /api from clients; the Manager proxies + # stats itself. `return` fires before auth_request, nothing leaks. + location ~ ^/viewer/[^/]+/api(/|$) { + return 403; + } + + # /viewer/ with no trailing slash does not match the + # prefix-strip rewrite below, so the unstripped URI would reach + # KasmVNC and 404 after a successful auth. Normalise it. + # + # absolute_redirect off: nginx would otherwise build the Location + # from its OWN listener — http://host:8080/... — which behind a TLS + # terminator is both the wrong scheme (blocked as mixed content in + # an https iframe) and a port that is not published. A relative + # Location resolves against whatever origin the client actually + # used. + location ~ ^/viewer/[^/]+$ { + absolute_redirect off; + return 308 $uri/$is_args$args; + } + + auth_request /_viewer_auth; + auth_request_set $viewer_upstream $upstream_http_x_viewer_upstream; + auth_request_set $viewer_authorization $upstream_http_x_viewer_authorization; + + # Strip the /viewer/ prefix; Kasm client assets are ./-relative. + rewrite ^/viewer/[^/]+(/.*)$ $1 break; + + # Variable upstream (IP:port from auth response) — no resolver needed. + proxy_pass http://$viewer_upstream; + # Kasm's HTTP layer requires Basic auth; the token auth response + # carries the per-display credentials. Injected server-side so the + # browser never sees them. + proxy_set_header Authorization $viewer_authorization; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + proxy_set_header Host $http_host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $forwarded_proto; + # Kasm's COEP/COOP response headers must pass through untouched. + + proxy_read_timeout 1800s; + proxy_send_timeout 1800s; + proxy_connect_timeout 1800s; + proxy_buffering off; + client_max_body_size 10M; + } + + # Viewer-token auth subrequest → FastAPI. Expects X-Viewer-Upstream + # (IP:port) in the 2xx response. + location = /_viewer_auth { + internal; + proxy_pass http://127.0.0.1:8081/api/viewer-auth; + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + proxy_set_header X-Original-URI $request_uri; + proxy_set_header Host $http_host; + } + + location = /healthz { + access_log off; + return 200; + } + + # ── Control plane: SPA + /api (CDP WebSockets under /api/profiles/*/cdp) ── + location / { + # POST /clipboard declares its limit in CHARACTERS (Pydantic + # max_length=1_048_576) while nginx counts BYTES. A JSON encoding of + # that many characters can reach ~12 bytes each — an escaped + # supplementary character is \uXXXX\uXXXX — so honouring the API's + # own stated boundary needs ~12.6 MB plus framing. Anything smaller + # rejects bodies FastAPI would accept, and nginx's default (1m) + # rejected even plain ASCII at the limit. + client_max_body_size 16M; + + proxy_pass http://127.0.0.1:8081; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + proxy_set_header Host $http_host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $forwarded_proto; + + proxy_read_timeout 1800s; + proxy_send_timeout 1800s; + } + } +} diff --git a/entrypoint.sh b/entrypoint.sh index 41f46d35..2ebdc6f8 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -18,9 +18,47 @@ find /data/profiles -maxdepth 2 -name 'SingletonSocket' -delete 2>/dev/null || t # Remove X11 lock files from previous displays rm -f /tmp/.X1*-lock 2>/dev/null || true -# Start FastAPI (serves built React + API) +# nginx is the sole ingress and uvicorn the sole control plane — the container +# is useless without either. Both run in the foreground under this shell so +# that the death of one takes the container down: Docker does not act on an +# unhealthy container by itself, so terminating is what lets `restart:` bring +# back a working data plane. Running nginx detached instead would leave the +# container "up" with port 8080 dead forever. +# set -e aborts the container if the config test fails. +nginx -t +nginx -g 'daemon off;' & +nginx_pid=$! + cd /app +uvicorn backend.main:app --host 127.0.0.1 --port 8081 --log-level warning & +uvicorn_pid=$! + +# Graceful stop: uvicorn's lifespan shutdown stops the browsers and Xvnc, so +# let it finish before pulling the data plane out from under it. +shutdown() { + trap - TERM INT + kill -TERM "$uvicorn_pid" 2>/dev/null || true + wait "$uvicorn_pid" 2>/dev/null || true + kill -TERM "$nginx_pid" 2>/dev/null || true + wait "$nginx_pid" 2>/dev/null || true + exit 0 +} +trap shutdown TERM INT + echo "" -echo " CloakBrowser Manager running at http://localhost:8080" +echo " CloakBrowser Manager running at http://localhost:8080 (nginx → uvicorn 127.0.0.1:8081)" echo "" -exec uvicorn backend.main:app --host 0.0.0.0 --port 8080 --log-level warning + +# First exit wins. +set +e +wait -n "$nginx_pid" "$uvicorn_pid" +status=$? +if kill -0 "$nginx_pid" 2>/dev/null; then + echo "FATAL: uvicorn exited (status $status) — shutting down nginx" >&2 + kill -TERM "$nginx_pid" 2>/dev/null +else + echo "FATAL: nginx exited (status $status) — shutting down uvicorn" >&2 + kill -TERM "$uvicorn_pid" 2>/dev/null +fi +wait +exit $((status == 0 ? 1 : status)) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2e5078e8..dc7faab7 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -8,7 +8,6 @@ "name": "cloakbrowser-manager", "version": "0.1.0", "dependencies": { - "@novnc/novnc": "^1.4.0", "lucide-react": "^0.468.0", "react": "^19.0.0", "react-dom": "^19.0.0" @@ -1098,12 +1097,6 @@ "node": ">= 8" } }, - "node_modules/@novnc/novnc": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/@novnc/novnc/-/novnc-1.4.0.tgz", - "integrity": "sha512-kW6ALMc5BuH08e/ond/I1naYcfjc19JYMN1EdtmgjjjzPGCjW8fMtVM3MwM6q7YLRjPlQ3orEvoKMgSS7RkEVQ==", - "license": "MPL-2.0" - }, "node_modules/@rolldown/pluginutils": { "version": "1.0.0-beta.27", "resolved": "https://registry.npmjs.org/@rolldown/pluginutils/-/pluginutils-1.0.0-beta.27.tgz", diff --git a/frontend/package.json b/frontend/package.json index 9da07e9b..4a6db0c5 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -11,7 +11,6 @@ "test:watch": "vitest" }, "dependencies": { - "@novnc/novnc": "^1.4.0", "lucide-react": "^0.468.0", "react": "^19.0.0", "react-dom": "^19.0.0" diff --git a/frontend/src/App.test.tsx b/frontend/src/App.test.tsx new file mode 100644 index 00000000..dfd067ea --- /dev/null +++ b/frontend/src/App.test.tsx @@ -0,0 +1,190 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, act, screen } from "@testing-library/react"; +import "@testing-library/jest-dom/vitest"; +import App from "./App"; + +vi.mock("./lib/api", () => { + class ApiError extends Error { + constructor( + public status: number, + message: string, + ) { + super(message); + this.name = "ApiError"; + } + } + return { + api: { + authStatus: vi.fn(), + listProfiles: vi.fn(), + profileStatus: vi.fn(), + createViewerToken: vi.fn(), + launchProfile: vi.fn(), + logout: vi.fn(), + }, + setOnUnauthorized: vi.fn(), + ApiError, + }; +}); + +import { api } from "./lib/api"; + +const mockApi = api as unknown as Record>; + +function profile(status: "running" | "stopped") { + return { + id: "p1", + name: "Test Profile", + fingerprint_seed: 1, + proxy: null, + timezone: null, + locale: null, + platform: "windows", + user_agent: null, + screen_width: 1920, + screen_height: 1080, + gpu_vendor: null, + gpu_renderer: null, + hardware_concurrency: null, + humanize: false, + human_preset: "default", + headless: false, + geoip: false, + clipboard_sync: true, + auto_launch: false, + color_scheme: null, + launch_args: [], + notes: null, + user_data_dir: "/data/profiles/p1", + created_at: "", + updated_at: "", + tags: [], + status, + vnc_ws_port: status === "running" ? 6100 : null, + cdp_url: status === "running" ? "/api/profiles/p1/cdp" : null, + }; +} + +async function flush() { + await act(async () => {}); + await act(async () => {}); + await act(async () => {}); +} + +beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + mockApi.authStatus.mockResolvedValue({ auth_required: false, authenticated: true }); + mockApi.listProfiles.mockResolvedValue([profile("running")]); + mockApi.profileStatus.mockResolvedValue({ + status: "stopped", + xvnc_alive: null, + browser_alive: null, + }); + mockApi.createViewerToken.mockResolvedValue({ + token: "tok-1", + viewer_url: "/viewer/tok-1/", + expires_in: 3600, + }); +}); + +afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); +}); + +describe("viewer lifecycle vs the 3s profile poll", () => { + it("keeps the terminal overlay mounted when the poll reports the profile stopped", async () => { + render(); + await flush(); + + // open the viewer for the running profile + await act(async () => { + screen.getByText("Test Profile").click(); + }); + await flush(); + const iframe = screen.getByTitle("Browser session") as HTMLIFrameElement; + expect(iframe).toBeInTheDocument(); + + const fromClient = (value: string) => { + const ev = new Event("message"); + Object.defineProperty(ev, "data", { value: { action: "connection_state", value } }); + Object.defineProperty(ev, "source", { value: iframe.contentWindow }); + window.dispatchEvent(ev); + }; + + await act(async () => fromClient("connected")); + await flush(); + + // the browser dies: the viewer's own machine reaches session-ended... + await act(async () => fromClient("disconnected")); + await act(async () => { + vi.advanceTimersByTime(300); + }); + await flush(); + + // ...and the 3s poll then reports "stopped". The viewer must survive that: + // gating its render on the polled status blanks the whole content pane. + mockApi.listProfiles.mockResolvedValue([profile("stopped")]); + await act(async () => { + vi.advanceTimersByTime(3_000); + }); + await flush(); + + expect(screen.getByText("Browser session ended")).toBeInTheDocument(); + expect(screen.getByText("Back to profile")).toBeInTheDocument(); + }); + + it("relaunching from a terminal overlay starts a fresh viewer session", async () => { + render(); + await flush(); + await act(async () => { + screen.getByText("Test Profile").click(); + }); + await flush(); + const iframe = screen.getByTitle("Browser session") as HTMLIFrameElement; + + const fromClient = (value: string) => { + const ev = new Event("message"); + Object.defineProperty(ev, "data", { value: { action: "connection_state", value } }); + Object.defineProperty(ev, "source", { value: iframe.contentWindow }); + window.dispatchEvent(ev); + }; + await act(async () => fromClient("connected")); + await flush(); + await act(async () => fromClient("disconnected")); + await act(async () => { + vi.advanceTimersByTime(300); + }); + await flush(); + expect(screen.getByText("Browser session ended")).toBeInTheDocument(); + + // the profile is now stopped, so the top bar offers Launch + mockApi.listProfiles.mockResolvedValue([profile("stopped")]); + await act(async () => { + vi.advanceTimersByTime(3_000); + }); + await flush(); + + mockApi.launchProfile.mockResolvedValue({ profile_id: "p1", status: "running" }); + mockApi.listProfiles.mockResolvedValue([profile("running")]); + mockApi.profileStatus.mockResolvedValue({ + status: "running", + xvnc_alive: true, + browser_alive: true, + }); + mockApi.createViewerToken.mockResolvedValue({ + token: "tok-2", + viewer_url: "/viewer/tok-2/", + expires_in: 3600, + }); + await act(async () => { + screen.getByText("Launch").click(); + }); + await flush(); + + // a relaunch must not leave the dead overlay up + expect(screen.queryByText("Browser session ended")).not.toBeInTheDocument(); + expect(mockApi.createViewerToken).toHaveBeenCalledTimes(2); + expect((screen.getByTitle("Browser session") as HTMLIFrameElement).src).toContain("tok-2"); + }); +}); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index a1e3195d..81d82c78 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -93,13 +93,17 @@ function AppContent({ authRequired, onLogout }: AppContentProps) { const [selectedId, setSelectedId] = useState(null); const [view, setView] = useState("empty"); const [sidebarOpen, setSidebarOpen] = useState(true); + /** bumped on relaunch to remount the viewer with a fresh session. */ + const [viewerEpoch, setViewerEpoch] = useState(0); const selected = profiles.find((p) => p.id === selectedId) ?? null; const handleSelect = useCallback((id: string) => { setSelectedId(id); const profile = profiles.find((p) => p.id === id); - setView(profile?.status === "running" ? "view" : "edit"); + // "starting" opens the viewer too — its state machine waits the + // profile out instead of showing a form for a session about to appear. + setView(profile && profile.status !== "stopped" ? "view" : "edit"); }, [profiles]); const handleNew = useCallback(() => { @@ -130,7 +134,14 @@ function AppContent({ authRequired, onLogout }: AppContentProps) { const handleLaunch = useCallback(async () => { if (!selectedId) return; const result = await launch(selectedId); - if (result) setView("view"); + if (result) { + // Force a fresh viewer. Dropping the status gate on the render below + // means the component is NOT remounted by a relaunch, so a viewer + // sitting on "Browser session ended" would keep showing that overlay + // over a dead token and the launch would read as a failure. + setViewerEpoch((n) => n + 1); + setView("view"); + } }, [selectedId, launch]); const handleStop = useCallback(async () => { @@ -139,7 +150,9 @@ function AppContent({ authRequired, onLogout }: AppContentProps) { setView("edit"); }, [selectedId, stop]); - const handleVncDisconnect = useCallback(() => { + // Only terminal session end navigates away — transient network drops are + // handled by the viewer's own reconnect state machine while the view stays. + const handleSessionEnded = useCallback(() => { setView("edit"); }, []); @@ -242,13 +255,17 @@ function AppContent({ authRequired, onLogout }: AppContentProps) { /> )} - {view === "view" && selected && selected.status === "running" && ( + {/* No status guard: the viewer owns its own terminal state. Gating on + the 3s profile poll would unmount it the instant the backend + reports "stopped", destroying the "Session ended" overlay (and the + only in-place way back) before the user ever sees it. */} + {view === "view" && selected && ( )} diff --git a/frontend/src/components/LaunchButton.tsx b/frontend/src/components/LaunchButton.tsx index 862d840a..5bff26fd 100644 --- a/frontend/src/components/LaunchButton.tsx +++ b/frontend/src/components/LaunchButton.tsx @@ -1,8 +1,9 @@ import { Play, Square, Loader2 } from "lucide-react"; import { useState } from "react"; +import type { ProfileLifecycle } from "../lib/api"; interface LaunchButtonProps { - status: "running" | "stopped"; + status: ProfileLifecycle; onLaunch: () => Promise; onStop: () => Promise; } @@ -29,7 +30,9 @@ export function LaunchButton({ status, onLaunch, onStop }: LaunchButtonProps) { } }; - if (loading) { + // "starting" = the backend is already bringing this profile up (container + // restart / auto-launch queue). Launching again would just 409. + if (loading || status === "starting") { return ( )} @@ -283,12 +133,72 @@ export function ProfileViewer({ profileId, cdpUrl, clipboardSync: initialClipboa - {/* VNC canvas container */} + {/* Display surface: native KasmVNC client in an iframe */}
+ > + {session.iframeSrc && ( +