feat: migrate viewer to native KasmVNC 1.5 client behind nginx data plane - #50
Draft
cjangrist wants to merge 25 commits into
Draft
feat: migrate viewer to native KasmVNC 1.5 client behind nginx data plane#50cjangrist wants to merge 25 commits into
cjangrist wants to merge 25 commits into
Conversation
cjangrist
marked this pull request as draft
July 31, 2026 03:30
…a plane 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/<token>/* 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/<id>/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/<id>/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) <noreply@anthropic.com>
cjangrist
force-pushed
the
feat/kasmvnc-1.5-native-client
branch
from
July 31, 2026 13:19
71d220b to
0242d21
Compare
The 68 commits of this branch were squashed, so the per-finding history is no longer in git. This records what it contained and, more usefully, what is still open. Deliberately explicit about verification level: what was exercised live against a running container, what was driven through a real browser, what is unit-tested only, and what was never tested at all (headless profiles end-to-end, more than one profile at a time, a real TLS deployment, arm64, GPU acceleration, sessions longer than ~10 minutes). The main open item is identifying a closing browser by its DevTools GUID rather than by its CDP port — ports cycle 5100-5199, so the port cannot distinguish our browser from a later profile's, and every setting of the resulting dial is wrong in one direction. Two reviewers converged on the same fix independently. It is written up with the reasoning, why it needs its own review cycle, and what else must change with it. Also records three deliberately-unfixed items together with the disproof of the obvious wrong fix for each, so they are not re-litigated blindly. Pre-squash history remains locally as branch backup/pre-squash-kasmvnc and tag pre-squash-kasmvnc (2c53933, 68 commits); neither is pushed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The teardown guard asked "is anything listening on the CDP port?" to decide when a profile whose Chromium had not finished exiting could be released. Ports cycle 5100-5199, so that question has no correct answer: trusting it lets an unrelated browser brick a profile forever, capping it releases the guard under a live browser. Every setting of the dial was wrong in one direction, which is why this subsystem produced a defect in seven of nine prior review rounds. Identify the process instead. BrowserProcess(pid, starttime, user_data_dir, cdp_port, driver_pid, driver_starttime) is discovered by a /proc scan restricted to descendants of os.getpid() and excluding --type= children. (pid, starttime) cannot alias, so CLOSING_CLAIM_TTL_S and CLOSING_CLAIM_MAX_S are deleted rather than retuned: a claim releases only on positive evidence the process is gone, and a browser that will not exit is escalated SIGTERM -> SIGKILL together with its Playwright driver, with the identity re-verified immediately before every os.kill. A DevTools-GUID probe was evaluated first and rejected: against a SIGSTOP'd Chromium the port still accepts in 0.000s while /json/version times out, so it is ambiguous in exactly the state a wedged browser is in. Three details are load-bearing and were only findable by running it: Chromium space-joins its argv into one NUL-terminated blob, so the textbook split(b"\0") matches nothing (and a subprocess.Popen-based unit test cannot catch that); a zombie must count as dead, because PID 1 in this container does not reap adopted children; and SIGTERM fires at 20s, not 30s, because Playwright's own gracefullyClose force-kills at exactly 30.0s and was winning the race every time. Also closes two holes in the same area: a launch cancelled inside launch_persistent_context_async left a live Chromium with no guard at all, and Playwright driver death fires no context close event, stranding a profile as "running" with its display leaked. is_wedged() is split so the 3s status poll neither probes nor mutates, and _closing is written only from the event loop. Adds a fourth lifecycle value "stopping" so a teardown in progress stops reporting "stopped" beside a Launch button that answers 409, bounds the clipboard endpoints (one wedged page used to kill the endpoint for a profile permanently), reaps orphaned xclip processes, and stops a malformed Authorization header returning 500 instead of 401. Encoding: -IgnoreClientSettingsKasm and -videoCodec are mutually exclusive in 1.5.0, so the advertised H.264/H.265/AV1 path was dead code. Make it an explicit KASM_ENCODING_POLICY knob defaulting to today's working behaviour, and name the encoder rather than passing "auto", which let a client select software AV1 (~364ms per 1080p keyframe, then a silent fallback to Tight). Drop -RectThreads, which upstream made inert. Headless profiles no longer start an Xvnc they cannot use: no display, no ws_port, null status fields, and /viewer-token answers 409. Backend tests 257 -> 378. Every regression test was mutation-checked by reverting its fix and confirming the test fails; several pre-existing tests that could not fail were rewritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st tests Putting nginx in front of the control plane silently regressed long-lived CDP sessions: Playwright/Puppeteer park on an attached WebSocket, and location /'s read timeout FIN'd the tunnel with no close frame and nothing in the error log. CDP now has its own regex location that is never idle- reaped, verified by a 35-minute idle soak that stayed open and answered Browser.getVersion immediately afterwards. The comment justifying it has been corrected to match measurement rather than assumption: nginx re-arms the read timer on traffic in EITHER direction, and uvicorn's own 20s WebSocket keepalive already resets it, so the long timeout is defence-in-depth. The real ceiling on an idle CDP session is uvicorn, not nginx — a client that does not answer pings is closed at 40s. Also: a second SIGTERM during shutdown killed PID 1 mid-cleanup, defeating the ordered teardown; KasmVNC's 301 for a directory path dropped the /viewer/<token> prefix and sent the browser out of the authenticated namespace into the SPA's static mount (restored via proxy_redirect, with absolute_redirect off so the Location is not rebuilt from nginx's own listener behind a TLS terminator); the two env knobs added alongside the encoding policy were unreachable under Compose; HEALTHCHECK gained a start-period; and the Dockerfile no longer redoes a 119MB pip install and a 337MB download on every nginx.conf edit. The data plane had zero automated coverage. scripts/dataplane_probe.py and backend/tests/test_dataplane.py boot the shipped config against stub upstreams and assert the contracts that were previously only ever checked by hand: auth_request status mapping, the 308 with a relative Location, /viewer/<t>/api blocked, websocket upgrade pass-through, and the CDP idle-timeout fix above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding a fourth lifecycle value was compile-checked in exactly one of four consumers; LaunchButton, App.handleSelect and classify() all fell through silently to the enabled "Launch", which for a busy profile is a guaranteed 409. The union is now consumed through Record<ProfileLifecycle, ...> maps so a fifth value cannot be added silently again. LaunchButton also labelled a teardown "Launching...": it derived the busy label from the polled status, which flips running -> stopping while the stop POST is still on the wire. It now remembers which action WE invoked instead of re-deriving it — the same dot/button contradiction the "stopping" state exists to remove. Headless profiles have no viewer, so selecting or launching one opens the form rather than mounting a viewer that can only ever render "Connection failed" for a browser that is running perfectly well. The profile form has always SUBMITTED headless but never rendered a control for it, so the option was unreachable from the UI — it now has a checkbox that states the consequence. LaunchResult's display/vnc_ws_port are nullable to match. api.test.ts claimed to bound "every request" while checking a single endpoint: the two calls the reconnect machine actually serialises on could be made unbounded with the whole suite green. It is now table-driven over every exported method. Adds vitest coverage config (the suite's real reach was invisible) and the first tests for StatusIndicator, LaunchButton, ProfileForm, the degraded "Reconnect now" button and the issue-#186 wheel guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… budget `generation` was doing two jobs: promise-cancellation epoch and iframe document identity. Every bump for the first invalidated the second, so a "connected" posted by the OUTGOING document — the old client's own 2s retry succeeding one beat after our grace expired, which is the ordinary case, not a corner case — was accepted. That reset aliveReconnects, so MAX_ALIVE_RECONNECTS never fired: a measured run minted 62 viewer tokens across 60 rounds and never reached a terminal state, with the status dot flapping the whole time. Document identity now has its own sequence, gated on the client's `init` message, which the 1.5.0 bundle emits exactly once per document and strictly before any "connected" — unlike the iframe `load` event, which can genuinely lose the race and is why gating on it was tried and reverted earlier. Other unbounded loops closed: alternating xvnc-dead <-> browser-dead verdicts never escalated and consumed no budget; reconnectNow() with a probe outstanding parked the machine in "reconnecting" with no timer, no countdown and no button; consecutive heartbeat failures never escalated; the 401 halt was not honoured by scheduleReconnect / armConnectWatchdog / handleOnline, so a tab switch restarted the loop and re-minted tokens; and connect() ran undebounced on every "online" event. classify() gains "stopping" as terminal with its own reason. Routing it to the transient branch would spin forever by construction: that branch has no failure budget, and the profile is already out of `running` with its tokens revoked, so the session cannot come back. Adds coverage for the two paths that could be deleted with the suite green — classify()'s browser-dead branch and the whole client self-reconnect grace window — and an explicit test of the invariant that no non-terminal state is reachable with no timer armed and no user affordance rendered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WORKLOG.md is removed. It was written at the end of the migration and asserted a great deal that does not hold: the image it said had been "rebuilt and re-verified after every round" did not exist on the host at all, its central root-cause conclusion (that a DevTools-GUID probe would resolve the teardown dilemma) is refuted by measurement, and its claim that "every regression test was verified to fail without its fix" was false — five named tests were proved vacuous by mutation. A narrative file that confidently misreports what was verified is worse than none, and the reasoning that survives it now lives in the code comments and commit messages, next to what it explains. README: the feature bullet advertised H.264/H.265/AV1 streaming that could never engage, because -IgnoreClientSettingsKasm and -videoCodec are mutually exclusive in KasmVNC 1.5.0. It now documents KASM_ENCODING_POLICY and its trade-off, the new KASM_VIDEO_CODEC and KASM_XVNC_LOG_LEVEL knobs, that KASM_RECT_THREADS is inert on 1.5.0, how headless profiles behave, and the caveat that naming an encoder narrows the server's probe but cannot restrict what a client selects — 1.5.0 exposes no enforcement point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop the change-history framing from the architecture notes and the environment table. The encoding-policy and video-codec entries described decisions and their rationale; they now describe behaviour. The KASM_RECT_THREADS row no longer explains what upstream changed, only that the variable has no effect and what to use instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`FRONTEND_DIR / full_path` served any file on the filesystem. Path's `/`
operator DISCARDS the left side when the right side is absolute, so
`Path("/app/frontend/dist") / "/etc/passwd"` is `/etc/passwd`.
The route is reachable unauthenticated even with AUTH_TOKEN set — the auth
middleware gates only paths starting with /api/ — and nginx forwards a
percent-encoded %2f to the upstream without decoding it, so the path arrived
here with a leading slash intact. Verified against the built image with
AUTH_TOKEN set, raw request line so the client cannot normalise it:
GET /api/profiles -> 401 {"detail":"Unauthorized"}
GET /%2fdata/profiles.db -> 200 b'SQLite format 3\x00...'
GET /%2fetc/passwd -> 200 b'root:x:0:0:root:/root:/bin/bash...'
/data/profiles.db holds every profile's proxy string, credentials included.
/tmp/kasmpasswd-<display> holds the KasmVNC owner password.
Resolution moves into a pure `_resolve_spa_file(base, full_path)` that
rejects absolute paths and `..` segments up front and then requires the
resolved path to sit under the resolved base. Using resolve() rather than a
string prefix also refuses a symlink inside the build directory that points
out of it. It is a free function because the route is only registered when
the built frontend exists, which it does not in a source checkout — the
containment rule has to be testable without it.
Same requests against the fixed image now return the SPA shell, and
/assets/* still serves the real build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
probeAfterResume ordered its replies (resumeSeq) but had no admission control, so every trigger issued a control-plane request: an "online" burst during a VPN flap, and ordinary tab switching, each cost a round-trip, indefinitely, for the life of the session. Coalescing on an in-flight probe would not have fixed it — when the control plane answers quickly, each switch finds no probe running — so the bound is on the rate. Skipping is safe because `connected` always has the heartbeat armed, so a suppressed probe always has a successor. The exception is the probe that matters: droppedWhileConnected means the network went away UNDER a live socket, where "the heartbeat will ask again" is exactly wrong — nothing else re-establishes it. That path bypasses the limit entirely. Measured from a probe's start rather than its reply, so a probe that never resolves stops blocking after one interval and the next trigger supersedes it — which is what the two reordered tests rely on. Their invariants are unchanged; they only needed a real time gap to produce two concurrent probes now that same-instant triggers collapse. Verified: both halves mutation-checked (removing the limit fails the burst test; removing the bypass fails the drop-while-connected test). 136 frontend tests, tsc clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…atus
/api/profiles/<id>/viewer-attached existed, was tested, and nothing
called it. So the connected heartbeat could only ever prove the
PROCESSES were up: a viewer socket that black-holed left a green dot
over a frozen frame until the client keepalive and TCP RTO expired —
minutes — with no watchdog, no retry and no overlay.
Verified the signal against KasmVNC 1.5.0 source at the exact commit
the image ships (17265fa) rather than inferring it:
- GetAPIMessager.cxx: bottleneckStats is map<peerEndpoint, stats>,
written by mainUpdateBottleneckStats and erased ONLY by
mainClearBottleneckStats, whose sole caller is ~VNCSConnectionST.
No idle eviction, no wholesale clear, reads are non-destructive —
so an idle session keeps its entry and empty really does mean every
connection object is gone.
- VNCServerST::writeUpdate calls sendStats(false) per frame-timer
tick, so the entry does not exist until the first tick. Hence a
streak of two rather than acting on one reading.
The load-bearing guard is sawViewerAttached. sendStats only writes when
the server has an apimessager; where it does not, the map is never
populated and the probe answers false forever — acted on blindly that
would tear down every healthy session on a fixed beat. Requiring a
confirmed attach for THAT connection first means the signal can only
fire where it has already shown it works, and is inert everywhere else.
null stays strictly distinct from false throughout.
api.test.ts's endpoint table caught the new method, so it also picks up
the request-bounding check.
Verified: 6 of 7 mutations caught (baseline gate, streak length, null
handling, per-connection reset, the call itself, streak reset). The
seventh — double-counting the /status failure budget — is unobservable
by construction and the test now says why instead of implying coverage
it does not have. 144 frontend tests, tsc clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_browser_alive answered "is something listening on this CDP port", which is a different question from "is our Chromium running". A CDP port is an ordinary ephemeral port: once the browser dies the kernel is free to hand it to anything, and the manager itself rotates these ports across relaunches (2603d6f). The result is a false ALIVE — the harmful direction, because the viewer then never reaches its browser-dead classification and instead burns the entire MAX_ALIVE_RECONNECTS budget minting viewer tokens against a browser that no longer exists. RunningProfile has carried (pid, starttime) since 7bc41bf and this was the last liveness site still not using it. process_is_alive cannot be fooled by a recycled port, and counts a zombie as dead — which matters because PID 1 here is the entrypoint shell and does not reap adopted children, so a killed Chromium can sit in state 'Z' indefinitely. The port probe survives only for proc is None, which launch failing closed on discovery makes unreachable for a registered profile; the existing test is renamed to say that is what it covers. Also corrects get_liveness_async's docstring, which justified its executor hop by a TCP connect that is now only the fallback path. Verified: both new tests fail when reverted to the port probe (recycled port reads alive; a live process with no listener yet reads dead). 384 backend tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t rests on session_epoch looked like dead code: the comparison in /api/viewer-auth cannot fire over HTTP. Traced why, and it is not an accident of the check — it is a property of the teardown paths. stop(), DELETE and _on_browser_closed() all revoke a profile's tokens before (or as) `running` is cleared, so a token reaching the comparison always belongs to the session currently registered, and validate() has already rejected everything else. _on_browser_closed's superseded-context branch returns without revoking, but only ever after an earlier teardown already did — and revoking there would destroy the LIVE session's tokens, so it is correct as written. That makes the guard redundant, not useless: it is what stands between a future reorder of that ordering and cross-session authorization, where a surviving token would be handed the new session's upstream port and the new display's Kasm credentials. Neither the guard nor its premise had a single test, so "redundant" was an assumption. Now both are pinned. The guard is exercised directly (epoch mismatch and ws_port mismatch each 403 and emit no X-Viewer-Upstream), and the premise is asserted at the moment of revocation on both teardown paths — "revoked eventually" is a weaker guarantee than "revoked no later than leaving `running`", and the gap between them is exactly what the guard backstops. The comment claimed the epoch "is what makes this check real". It now says what is true: nothing reaches it today, here is why, and here is why it stays. Verified: 4/4 mutations caught (dropping either comparison term; making either teardown path skip revocation). 388 backend tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reconnect overlay covered nothing. The embedded client destroys its
framebuffer canvas on disconnect, so by the time the machine learns the
connection dropped there is no image left underneath — the user got a
black rectangle with a spinner on it, which reads as "the session is
gone" during exactly the event the overlay exists to say it is not.
Fixed by copying the frame out on a 2s beat while connected, and
revealing that copy only while reconnecting. It cannot be grabbed at
disconnect time; the source is already gone by then.
Kept as a canvas rather than a data URL deliberately: drawing a tainted
canvas taints the destination but does not throw, whereas
toDataURL/getImageData on one would. Nothing ever reads pixels back, so
however the client sourced its frames this cannot fail. Downscaled to
640px — the overlay dims it anyway — and skipped entirely while the tab
is hidden, since nothing new is being painted.
Identifying the right canvas was the substance. The client builds six
(ui-BOjwDkC7.js) and the framebuffer is the one the Display constructor
appends to _screen; the bundle is minified, so selecting by class or id
is not a contract that will hold. It is instead identified structurally:
the backbuffer and WebGL surface are never appended, the cursor canvas
is visibility:hidden, the watermark canvas and the pre-first-frame
framebuffer are 0x0.
Two test-quality problems found and fixed while verifying, both of which
made a check look covered when it was not:
- document.implementation.createHTMLDocument has no browsing context,
so defaultView is null and getComputedStyle was never reached; the
visibility filter had no coverage at all and the realistic fixture
passed on the size comparison alone. Now uses a real iframe document
and asserts defaultView, plus a case where a hidden canvas is larger
than the screen.
- an isConnected guard was dead: querySelectorAll only walks the
document tree, so a detached canvas is never returned. Removed, and
the comment no longer credits it for excluding the backbuffer.
Verified: 6 mutations run, 5 caught; the sixth found the dead guard
above rather than a missing test. 156 frontend tests, tsc clean.
Not verified live: the selector has only been exercised against
synthetic documents. jsdom cannot run the real client, and the failure
mode if it is wrong is a silent fall back to today's black pane.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
useProfiles sat at 14% branch coverage with all five catch blocks
unexercised. Every one of those is a write the user asked for that
silently did not happen: the hook swallows the rejection, so `error` is
the only thing that tells them, and nothing checked that it was set —
or that a rejected write leaves the cached list alone instead of
optimistically showing a change the server refused.
Now covered, and each test pins the consequence rather than the line:
- a failed delete keeps the row, because the 409 for a wedged
teardown lands here and dropping it would report a profile gone
while its Chromium is still writing to user_data_dir
- a failed launch returns undefined, because App.handleLaunch routes
to the viewer on a truthy result
- a failed update leaves the cached profile untouched
- an update touches only the profile it names — without the id guard
the map overwrites every other row in the sidebar
- a non-Error rejection falls back to a readable message instead of
rendering "undefined" in the banner
- a stale error clears once a poll succeeds
Two paths turned out never to have been executed at all, not just
their error branches: the launch success path (result returned and
refresh awaited) and the stop success path.
100% statements/branches/functions/lines, and a per-file threshold so
it stays there. Verified the threshold gates rather than merely being
declared: skipping one failure test drops branches to 92.85% and the
run fails. 166 frontend tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every toolbar control was unexercised: fullscreen, the CDP copy button
and the clipboard-sync toggle, plus the fullscreenchange listener. All
of them are the user's only handle on something, and all of them were
free to break silently.
What the new tests pin, beyond the lines:
- leaving fullscreen via Esc only fires the event, so without the
listener the button stays stuck reading "Exit fullscreen"
- the CDP button copies an ABSOLUTE url; cdp_url is a path and a CDP
client cannot use it as-is
- a refused clipboard write (denied permission, insecure context)
rejects rather than throws, and must not leave the toolbar
claiming "Copied!"
- the "Copied!" confirmation reverts, so it cannot report a stale
success
- the clipboard-sync title says the change applies on the next
connect, because the flag is baked into the iframe URL and the
toggle otherwise reads as broken
- frames are not copied while the tab is hidden
ProfileViewer.tsx goes 81% -> 97.91% statements, 100% lines and
functions, and the per-file threshold moves up to match. The two
remaining uncovered statements are the !wrapperRef.current and
!surface early returns — defensive guards on refs React has already
populated, where faking a null ref would test the fake.
173 frontend tests, tsc clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… first
Both entrypoint checks asserted only that the shell exits 0. Verified
against the shipped image that a shutdown() reduced to a bare `exit 0`
— killing nothing, waiting for nothing — passes both of them. That is
the container going away while Chromium is still live and still writing
to user_data_dir, which is the exact outcome the ordered teardown and
stop_grace_period: 60s exist to prevent.
The teardown ORDER was equally unasserted, and it is not incidental:
uvicorn's lifespan shutdown is what closes every profile, and it reaches
those browsers over the CDP tunnels nginx proxies. Signalling nginx
first pulls the data plane out from under the cleanup still using it.
Both shims now journal their own lifecycle (start / term / exit, with a
timestamp) when DP_JOURNAL is set, and the new check requires all four
terminal events plus nginx being signalled only after uvicorn has
finished. The uvicorn shim's existing TERM delay is what makes "nginx
after uvicorn FINISHED" distinguishable from "both signalled at once".
Verified against the real image, three mutations, all caught with a
diagnostic that names the fault rather than just an exit code:
- shutdown() kills nothing -> missing all four events
- nginx torn down first -> "signalled 1.01s before uvicorn
finished"
- uvicorn signalled but not waited -> missing 'uvicorn exit'
Also corrects an earlier claim of mine: entrypoint_sigterm_before_children
was reported as unable to fail. It can — arming the trap after the
children start makes it exit -15. It was the shutdown BODY that had no
coverage, not the trap timing.
389 backend tests; the probe runs green against cloakbrowser-manager:kasm15.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The note said KASM_VIDEO_CODEC "narrows the server's probe but does NOT
restrict what a client may select" and that 1.5.0 "exposes no
enforcement point", which made the video policy read as a feature that
does not work. Half right, and the wrong half was load-bearing.
Traced the whole chain instead of the one link:
1. -videoCodec narrows the probe. Measured against the shipped image:
h264 -> "Using CLI-specified video codecs (supported subset):
libx264"; auto -> "libx264 libx265".
2. That probed set is what the client is told: SMsgWriter.cxx's
writeVideoEncoders() iterates cp->available_encoders.
3. The bundled client can only choose from what it was told.
ui-BOjwDkC7.js filters every path — the menu
(getAvailableStreamingModes), the selection
(getBestStreamingMode), a persisted stream_mode preference and the
kasmvnc_mode_preference URL override — against that list.
So the codec IS determined for the viewer we ship. What is true is that
the server does not ENFORCE it: ConnParams.cxx accepts any
streaming-mode pseudo-encoding a client offers and builds a config for
it even when the encoder is not in available_encoders — which is how a
client asking for -1037 gets software AV1 on a box whose probe rejected
it, then dies at encode time. That is a different, narrower claim.
Link 1 is the empirical fact the policy rests on and nothing checked it,
so it is now a probe check (codec_probe_narrowing) that runs Xvnc in the
image and asserts h264 yields exactly libx264 and that auto is strictly
wider. Verified by mutation: pointing it at "auto" fails with the actual
sets, and dropping its -publicIP fails with no probe line at all —
Xvnc EXITS under --network none when it cannot reach a STUN server,
before it ever probes encoders.
The remaining reason `video` is opt-in is the one that survives: it
hands quality to the client, so KASM_QUALITY_PRESET stops binding.
Both the startup warning and the README now say that.
389 backend tests; 15 dataplane checks green against the image.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The manager announced a launch with one line — display, ws_port, preset, policy — and then either worked or raised a readiness timeout with an unstructured blob attached. Every actual decision (which encoding flags won, which preset flags were dropped as inert, whether ICE was pinned, the argv itself) existed only as a code comment. start_vnc now records each decision as (label, value, why) as it builds the command and logs them as one contiguous block, plus the exact argv and the measured time to first accept. One block rather than a line per decision because launches are concurrent — interleaved lines from four displays cannot be read as a sequence — and every line still carries its display so `grep ':100'` recovers a single launch. The failure path now says why it is calling it a failure: exit status, argv, and the tail of Xvnc's own log. Xvnc dies for reasons invisible from "the port never opened" — a fatal DRI3 init, a display lock left by a SIGKILLed predecessor, or seven STUN timeouts then exit(1). On the network-restricted question: -PublicIP 127.0.0.1 was ALREADY passed (vnc_manager.py, capital P — my earlier report that it was missing came from a case-sensitive grep and was wrong). Re-verified against the shipped image under `--network none`: Xvnc logs "ICE: Using public IP 127.0.0.1 from args", is accepting in ~0.05s, and its HTTP layer answers. Removing the flag in that same environment reproduces the death — exit 1 after seven STUN failures, before the websocket port opens — and the new error log now names that cause outright. Its test was strengthened rather than duplicated: parametrized over the encoding policy so the flag cannot come to depend on one branch, and carrying the evidence for why it is mandatory rather than cosmetic. Verified: 5 mutations, all caught — removing -PublicIP, making it conditional on the policy, dropping the plan log, dropping the argv log, and untagging the plan lines. 394 backend tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… documents `Xvnc -help` lists only "auto, h264, h264_vaapi, h265, h265_vaapi, av1, av1_vaapi", so the allowlist rejected every *_nvenc name as unknown and silently substituted software h264 — turning a request FOR hardware encoding into a guarantee against it. The names are real. The shipped binary carries a contiguous h264/h264_vaapi/ h264_nvenc/... run in its string table and instantiates rfb::FFMPEGHWEncoder<(AVHWDeviceType)2, (AVPixelFormat)117> beside the <3, 44> one — FFmpeg's type 2 is CUDA and format 117 is AV_PIX_FMT_CUDA. The 1.5.0 release notes say so outright; only -help was never updated. Each name here was then confirmed against the live parser on an RTX 3080 Ti rather than inferred: h265_nvenc is accepted and normalised to hevc_nvenc, and h264_nvenc reports back "Hardware video encoding acceleration capability: h264_nvenc". Also accept a comma-separated list, because -videoCodec does. "h264_nvenc,h264" is the natural way to ask for "NVENC, else software H.264", and validating the whole string as one name would have hit exactly the substitution above. viewer_stream_mode_preference() is added here but unused until the viewer starts sending it; it is the codec->pseudo-encoding mapping the client needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--use-angle=swiftshader was hardcoded with the comment "software GL for VNC (no
GPU in container)". That is now conditional on CHROME_GPU_ACCEL, which defaults
to auto: the GPU path engages only when /dev/nvidiactl is present, so a GPU-less
container is unchanged.
The backend is Vulkan, NOT the gl-egl pairing CloakBrowser PR #476 landed, and
the difference is the whole point. Measured headed on KasmVNC's Xvnc, same host:
gl-egl, as-is -> ANGLE (Mesa, llvmpipe ...) SOFTWARE
gl-egl + NVIDIA ICD -> ANGLE (NVIDIA ... RTX 3080 Ti) but
gpu_compositing = disabled_software (readback)
vulkan -> ANGLE (NVIDIA, Vulkan 1.4.312 ...)
gpu_compositing = enabled HARDWARE
NVIDIA's EGL declines the X11 platform on an X server it does not drive, so
GLVND falls through to Mesa. PR #476 concluded headed sessions stay
software-rendered under a virtual X server; that holds for the EGL path it
changed, and is what Vulkan gets past.
__EGL_VENDOR_LIBRARY_FILENAMES is set as insurance only: if Vulkan is
unavailable ANGLE falls back to gl-egl, and pinning the NVIDIA manifest makes
that path "GPU with readback" instead of silently llvmpipe.
Fingerprinting was checked rather than assumed — JS still reads the spoofed
renderer with these flags.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… pick
With everything else correct, Xvnc logged "Hardware video encoding acceleration
capability: h264_nvenc" while every frame still went out as JPEG/WebP and the
GPU encoder sat at 0%. The client is why: getBestStreamingMode() intersects
what the server advertises with a hardcoded candidate list,
const JA = [AVCVAAPI, AVCSW, HEVCVAAPI, HEVCSW]
which contains no *_nvenc entry. Against an NVENC-only server that intersection
is empty and it settles on pseudoEncodingStreamingModeJpegWebp — no video codec
at all, and nothing in the manager's logs to say so.
So the server names it. viewer-token now carries a stream_mode derived from
KASM_VIDEO_CODEC, and buildViewerUrl passes it as kasmvnc_mode_preference, which
sets forcedCodecs — the one path that bypasses that list. Forcing -1029 on the
previously-stuck setup moved all 1445 frames onto FFMPEGHWEncoder
(AV_PIX_FMT_CUDA) with "Total: 0 rects, 0 pixels" left on the Tight path.
Emitted for NVENC only, deliberately. forcedCodecs is consulted BEFORE the
client's "fall back to image mode after an encoding error" branch, so forcing a
codec the client already picks correctly would trade a working safety net for
nothing. A '|'-separated list keeps the software fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docker compose -f docker-compose.yml -f docker-compose.nvidia.yml up -d --build builds both images from one command: the overlay binds Dockerfile.nvidia's `FROM base` to the CPU image via additional_contexts, so the 107-line recipe stays in one file. The base build service needs deploy.replicas: 0 rather than profiles: — a profile removes it from the resolved model and the context reference then fails config. Dockerfile.nvidia is a thin layer, and specifically NOT an nvidia/cuda base. Nothing CUDA has to be baked in: NVENC reaches the GPU through libnvidia-encode.so.1 + libcuda.so.1 and Chromium through libEGL_nvidia.so.0, all three injected by the container runtime and required to match the host kernel module. A CUDA base would add gigabytes, invite version skew, and drag the image off Debian trixie — where the KasmVNC .deb and the libavcodec61 soname both come from. Measured: h264_nvenc opens on the stock python:3.12-slim base with nothing but `--gpus all`. The overlay defaults to KASM_ENCODING_POLICY=video because under the repo-wide default Xvnc gets -IgnoreClientSettingsKasm, which makes -videoCodec structurally unreachable — no codec, hardware or software, is ever negotiated. No /dev/dri mapping: the NVIDIA runtime already injects the render node, and a hardcoded mapping is what breaks container creation on hosts without it. docker-compose.gpu.yml now says it is the wrong file for an NVIDIA GPU — "no DRI3" rules out -hw3d capture, not the GPU. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both halves of GPU acceleration fail silently, so neither "it started" nor "it did not crash" is evidence. A missing libnvidia-encode makes KasmVNC log "capability: none" once, at INFO, from another process; a missing libEGL makes ANGLE fall back to SwiftShader rather than fail. Either way the session looks perfectly healthy while everything runs on the CPU. This asserts the observable consequences instead: the driver libraries, the EGL loader, the encoder KasmVNC actually selected, and the renderer Chromium actually bound — headed on a real Xvnc, because that is the configuration that breaks the EGL path. It reads GL_RENDERER via CDP SystemInfo.getInfo and NOT chrome://gpu's feature table, which reported webgl/rasterization/gpu_compositing "enabled" for both llvmpipe and SwiftShader. WebGL's UNMASKED_RENDERER_WEBGL is useless here too — CloakBrowser's fingerprint layer synthesizes a plausible NVIDIA string on a software run. Expected to FAIL without a GPU; it imports the manager's own flag resolution so it cannot keep passing once browser_manager stops emitting them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… modes Covers the one-command build, what is and is not accelerated (NVENC encode and Chromium ANGLE/Vulkan yes; NVDEC video decode and -hw3d capture no, both for want of DRI3), and how to verify with the probe or two live checks. Calls out the two traps by name, because both fail silently and both look like the obvious right answer: --use-angle=gl-egl is the SOFTWARE path under a virtual X server, and the bundled client cannot select NVENC by itself. Plus the hardware caveats — av1_nvenc needs Ada, and consumer GeForce caps concurrent NVENC sessions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the KasmVNC 1.3.3 + generic noVNC compatibility bridge (FastAPI WebSocket relay parsing/rewriting every RFB message) with KasmVNC 1.5.0's native, matched web client served through an authenticated nginx reverse proxy. FastAPI stops being the display data plane entirely — no framebuffer bytes traverse Python anymore.
What changed
Data plane (new
docker/nginx.conf)/viewer/<token>/*to the per-profile KasmVNC port with prefix strip, WS upgrade headers, 1800s timeouts,proxy_buffering offauth_requestsubrequest to FastAPI authorizes every viewer request (page, assets, WS upgrade)/api/*blocked from clients; viewer token redacted from access logs (verified)Control plane (FastAPI)
/api/profiles/{id}/vncrelayPOST /api/profiles/<id>/viewer-token(1h opaque tokens, revoked on stop/delete),GET /api/viewer-auth(returns upstream + injectable per-display Basic creds),GET /api/profiles/<id>/kasm-stats(bottleneck/session/frame stats),xvnc_alive/browser_aliveon profile statusFrontend
@novnc/novncdependency and all noVNC glue removed; native Kasm 1.5 client embedded via iframe using its?path=query override for prefix-safe WS routinguseViewerSessionreconnect state machine: postMessageconnection_statetracking, backoff 250ms→15s w/ full jitter, offline/visibility handling, status-probe classification, dimmed keep-last-frame overlay, accurate terminal states. A network drop never stops the browser or navigates away (fixes the oldonDisconnectbehavior)KasmVNC 1.5.0 (trixie package, SHA256-pinned — the old image ran a bookworm build on a Debian 13 base)
-RectThreads 2, WebP,-IgnoreClientSettingsKasmtext/balanced/low/motionviaKASM_QUALITY_PRESET;KASM_RECT_THREADSoverride-videoCodec auto— the binary default is empty despite the man page claimingauto); FFmpeg libs + dlopen symlinks in the image; VAAPI packages for AMD/Intel-hw3d -drinode) auto-enabled when a render node with an open driver exists — skipped on proprietary NVIDIA (no DRI3);KASM_HW3D/KASM_DRINODEoverrides;/dev/drimapped in compose-PublicIP 127.0.0.1) — WSS only, no outbound lookupsTesting
Live end-to-end runs against the built image (agent-browser driven UI + raw curl):
SW H.264/AVC, live mode switch renders correctlytsc && vite buildclean, image builds reproduciblyNotes for reviewers
-DisableBasicAuthwas removed and per-display random owner credentials are generated at Xvnc launch; nginx injects them for token-authorized viewer traffic (browser never sees them) and the stats proxy uses them directly.KASM_QUALITY_PRESET,KASM_RECT_THREADS,KASM_HW3D,KASM_DRINODE.enable_perf_statswiring for live frame stats in a diagnostics panel, Cloudflare Tunnel deployment docs.