From 754fac42bba8a0fe1fc616fc3437b56413b48e5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 30 Jul 2026 05:09:13 +0000 Subject: [PATCH 1/2] stop reading the owner's vnc password in the main process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The password lives in the owner's ~/.vnc, and the sidecar is the process that writes it — together with the rfbauth file x11vnc actually authenticates against. Officer read the plaintext half directly and answered with it before ever asking the sidecar, which is both a secret the proxy has no business opening and a way to hand out a password that no longer matches: if `passwd` went missing while `password` survived, officer kept serving the old plaintext and the desktop refused every login. `vnc:ensure-password` reconciles the pair, so ask it every time and delete the reader. Co-Authored-By: Claude Opus 4.8 --- src/servers/api/desktop/rest.ts | 12 ++++++------ src/servers/api/desktop/vnc-config.ts | 10 ---------- 2 files changed, 6 insertions(+), 16 deletions(-) delete mode 100644 src/servers/api/desktop/vnc-config.ts diff --git a/src/servers/api/desktop/rest.ts b/src/servers/api/desktop/rest.ts index 3cac087a..671b06a3 100644 --- a/src/servers/api/desktop/rest.ts +++ b/src/servers/api/desktop/rest.ts @@ -1,19 +1,19 @@ import { createRouter } from '../../create-router'; -import { getVncPassword } from './vnc-config'; import * as sidecar from '@@/sidecar-registry'; export const desktopRouter = createRouter(); // The desktop UI asks for the password before it can open the WebSocket — and that WebSocket is what // starts the VNC session. So this cannot wait for a session to exist: on a fresh install nothing has -// ever written the password, and answering "not configured" deadlocked the page permanently. Ask the -// sidecar to provision it instead; it owns the .vnc directory and the call is idempotent. +// ever written the password, and answering "not configured" deadlocked the page permanently. +// +// Officer does not read the password file itself. It lives in the owner's ~/.vnc, next to the rfbauth +// file x11vnc authenticates against, and the sidecar is the process that writes both — so it is the +// process that answers for them too. `vnc:ensure-password` is idempotent: it returns the existing +// pair when both files are already there, and provisions them when they are not. desktopRouter.get('/vnc-password', async (ctx) => { const user = ctx.get('user'); - const existing = await getVncPassword(user.email); - if (existing) return ctx.json({ password: existing }); - if (!sidecar.isVncConnected()) { return ctx.json({ error: 'VNC sidecar is not connected' }, 503); } diff --git a/src/servers/api/desktop/vnc-config.ts b/src/servers/api/desktop/vnc-config.ts deleted file mode 100644 index f808255f..00000000 --- a/src/servers/api/desktop/vnc-config.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { join } from 'node:path'; -import { getOwnerHomeDir } from '@@/data-path'; - -const getVncDir = (email: string): string => join(getOwnerHomeDir(email), '.vnc'); - -export async function getVncPassword(email: string): Promise { - const file = Bun.file(join(getVncDir(email), 'password')); - if (!(await file.exists())) return null; - return (await file.text()).trim(); -} From 1014739b1b90409e8dff5be7b7d7b1c198532d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 30 Jul 2026 05:11:19 +0000 Subject: [PATCH 2/2] reattach the desktop when officer restarts under it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x11vnc mirrors :0 with `-forever`, so the desktop survives officer going away — only the bridge socket dies. The panel treated that as the end of the session: "Disconnected from desktop" until someone closed and reopened it. Retry with backoff instead, and cover the password fetch too, since that is what fails first while officer is still coming up. Retries are generation-guarded so tearing an instance down cannot be mistaken for a lost connection, and a security failure or a missing noVNC is terminal — those do not come back on their own. Co-Authored-By: Claude Opus 4.8 --- .../src/apps/Desktop/DesktopView.tsx | 109 +++++++++++++----- 1 file changed, 82 insertions(+), 27 deletions(-) diff --git a/src/workspaces/officerdev/src/apps/Desktop/DesktopView.tsx b/src/workspaces/officerdev/src/apps/Desktop/DesktopView.tsx index 41793379..625030f7 100644 --- a/src/workspaces/officerdev/src/apps/Desktop/DesktopView.tsx +++ b/src/workspaces/officerdev/src/apps/Desktop/DesktopView.tsx @@ -41,7 +41,9 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => { const rfbRef = useRef(null); const isMounted = useMounted(); const client = useClient(); - const [status, setStatus] = useState<'connecting' | 'connected' | 'disconnected' | 'error'>('connecting'); + const [status, setStatus] = useState<'connecting' | 'connected' | 'reconnecting' | 'disconnected' | 'error'>( + 'connecting', + ); const [errorMsg, setErrorMsg] = useState(''); useEffect(() => { @@ -50,20 +52,64 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => { if (!container) return; let disposed = false; + let attempts = 0; + let retryTimer: ReturnType | null = null; + // Tearing an RFB down makes it fire its own `disconnect`, and a bad password fires `securityfailure` + // and then `disconnect` too. Both would otherwise be read as "officer went away, reattach". + let generation = 0; + let fatal = false; + const MAX_ATTEMPTS = 5; + const RETRY_DELAYS = [1000, 2000, 3000, 5000, 5000]; + + const detach = () => { + const rfb = rfbRef.current; + rfbRef.current = null; + generation++; + if (!rfb) return; + try { + rfb.disconnect(); + } catch { + /* ignore */ + } + }; + + // x11vnc mirrors :0 with `-forever`, so the desktop itself outlives this socket — losing it means + // officer restarted under us, not that the session ended. Reattach instead of parking on + // "Disconnected" until someone reopens the panel. + const retry = (reason: string) => { + if (disposed || fatal || retryTimer) return; + detach(); + if (attempts >= MAX_ATTEMPTS) { + setStatus('disconnected'); + setErrorMsg(reason); + return; + } + const delay = RETRY_DELAYS[attempts] ?? 5000; + attempts++; + setStatus('reconnecting'); + setErrorMsg(`${reason} — reconnecting (${attempts}/${MAX_ATTEMPTS})...`); + retryTimer = setTimeout(() => { + retryTimer = null; + void connect(); + }, delay); + }; const connect = async () => { + const gen = ++generation; + const isCurrent = () => !disposed && gen === generation; let password = ''; try { const res = await client.get<{ password: string }>('/desktop/vnc-password'); password = res.password; } catch { - if (disposed) return; - setStatus('error'); - setErrorMsg('Failed to fetch VNC password'); + if (!isCurrent()) return; + // Officer being down is the common case here, and it comes back — so this is a retry, not a + // dead end. A sidecar that is genuinely missing still ends up at "Disconnected" after five. + retry('Failed to fetch VNC password'); return; } - if (disposed) return; + if (!isCurrent()) return; let RFB: Awaited>['default']; try { @@ -71,13 +117,14 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => { RFB = mod.default; } catch (err) { console.error('[desktop] Failed to load noVNC:', err); - if (disposed) return; + if (!isCurrent()) return; + fatal = true; setStatus('error'); setErrorMsg('Failed to load noVNC library'); return; } - if (disposed) return; + if (!isCurrent()) return; const wsUrl = buildWsUrl(); const rfb = new RFB(container, wsUrl, { @@ -90,15 +137,18 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => { rfbRef.current = rfb; rfb.addEventListener('connect', () => { - if (!disposed) setStatus('connected'); + if (!isCurrent()) return; + attempts = 0; + setErrorMsg(''); + setStatus('connected'); }); - rfb.addEventListener('disconnect', (ev: CustomEvent) => { - if (disposed) return; - setStatus('disconnected'); - if (!ev.detail.clean) { - setErrorMsg('Connection lost'); - } + // Every disconnect we did not ask for is worth retrying, clean or not: officer closing its side + // tidily during a restart still reports `clean`, and the desktop behind it is still there. + rfb.addEventListener('disconnect', () => { + if (!isCurrent()) return; + rfbRef.current = null; + retry('Connection lost'); }); rfb.addEventListener('credentialsrequired', () => { @@ -106,25 +156,30 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => { }); rfb.addEventListener('securityfailure', (ev: CustomEvent) => { - if (!disposed) { - setStatus('error'); - setErrorMsg(ev.detail.reason || 'Security failure'); - } + if (!isCurrent()) return; + fatal = true; + setStatus('error'); + setErrorMsg(ev.detail.reason || 'Security failure'); }); }; + // Coming back to the tab after the retries ran out should try once more rather than stay dead. + const handleVisibility = () => { + if (disposed || fatal || document.visibilityState !== 'visible') return; + if (rfbRef.current || retryTimer) return; + attempts = 0; + setStatus('connecting'); + void connect(); + }; + document.addEventListener('visibilitychange', handleVisibility); + void connect(); return () => { disposed = true; - if (rfbRef.current) { - try { - rfbRef.current.disconnect(); - } catch { - /* ignore */ - } - rfbRef.current = null; - } + document.removeEventListener('visibilitychange', handleVisibility); + if (retryTimer) clearTimeout(retryTimer); + detach(); }; }, [isMounted, client]); @@ -138,7 +193,7 @@ export const DesktopView = ({ className, style }: DesktopViewProps) => { Connecting to desktop... )} - {(status === 'disconnected' || status === 'error') && ( + {(status === 'reconnecting' || status === 'disconnected' || status === 'error') && (
{errorMsg || 'Disconnected from desktop'}