fix(sandbox): use one-time noVNC observer tokens

src/agents/sandbox/browser.create.test.ts

@@ -0,0 +1,185 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { BROWSER_BRIDGES } from "./browser-bridges.js";
+import { ensureSandboxBrowser } from "./browser.js";
+import { resetNoVncObserverTokensForTests } from "./novnc-auth.js";
+import type { SandboxConfig } from "./types.js";
+
+const dockerMocks = vi.hoisted(() => ({
+  dockerContainerState: vi.fn(),
+  execDocker: vi.fn(),
+  readDockerContainerEnvVar: vi.fn(),
+  readDockerContainerLabel: vi.fn(),
+  readDockerPort: vi.fn(),
+}));
+
+const registryMocks = vi.hoisted(() => ({
+  readBrowserRegistry: vi.fn(),
+  updateBrowserRegistry: vi.fn(),
+}));
+
+const bridgeMocks = vi.hoisted(() => ({
+  startBrowserBridgeServer: vi.fn(),
+  stopBrowserBridgeServer: vi.fn(),
+}));
+
+vi.mock("./docker.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("./docker.js")>();
+  return {
+    ...actual,
+    dockerContainerState: dockerMocks.dockerContainerState,
+    execDocker: dockerMocks.execDocker,
+    readDockerContainerEnvVar: dockerMocks.readDockerContainerEnvVar,
+    readDockerContainerLabel: dockerMocks.readDockerContainerLabel,
+    readDockerPort: dockerMocks.readDockerPort,
+  };
+});
+
+vi.mock("./registry.js", () => ({
+  readBrowserRegistry: registryMocks.readBrowserRegistry,
+  updateBrowserRegistry: registryMocks.updateBrowserRegistry,
+}));
+
+vi.mock("../../browser/bridge-server.js", () => ({
+  startBrowserBridgeServer: bridgeMocks.startBrowserBridgeServer,
+  stopBrowserBridgeServer: bridgeMocks.stopBrowserBridgeServer,
+}));
+
+function buildConfig(enableNoVnc: boolean): SandboxConfig {
+  return {
+    mode: "all",
+    scope: "session",
+    workspaceAccess: "none",
+    workspaceRoot: "/tmp/openclaw-sandboxes",
+    docker: {
+      image: "openclaw-sandbox:bookworm-slim",
+      containerPrefix: "openclaw-sbx-",
+      workdir: "/workspace",
+      readOnlyRoot: true,
+      tmpfs: ["/tmp", "/var/tmp", "/run"],
+      network: "none",
+      capDrop: ["ALL"],
+      env: { LANG: "C.UTF-8" },
+    },
+    browser: {
+      enabled: true,
+      image: "openclaw-sandbox-browser:bookworm-slim",
+      containerPrefix: "openclaw-sbx-browser-",
+      cdpPort: 9222,
+      vncPort: 5900,
+      noVncPort: 6080,
+      headless: false,
+      enableNoVnc,
+      allowHostControl: false,
+      autoStart: true,
+      autoStartTimeoutMs: 12_000,
+    },
+    tools: {
+      allow: ["browser"],
+      deny: [],
+    },
+    prune: {
+      idleHours: 24,
+      maxAgeDays: 7,
+    },
+  };
+}
+
+function envEntriesFromDockerArgs(args: string[]): string[] {
+  const values: string[] = [];
+  for (let i = 0; i < args.length; i += 1) {
+    if (args[i] === "-e" && typeof args[i + 1] === "string") {
+      values.push(args[i + 1]);
+    }
+  }
+  return values;
+}
+
+describe("ensureSandboxBrowser create args", () => {
+  beforeEach(() => {
+    BROWSER_BRIDGES.clear();
+    resetNoVncObserverTokensForTests();
+    dockerMocks.dockerContainerState.mockReset();
+    dockerMocks.execDocker.mockReset();
+    dockerMocks.readDockerContainerEnvVar.mockReset();
+    dockerMocks.readDockerContainerLabel.mockReset();
+    dockerMocks.readDockerPort.mockReset();
+    registryMocks.readBrowserRegistry.mockReset();
+    registryMocks.updateBrowserRegistry.mockReset();
+    bridgeMocks.startBrowserBridgeServer.mockReset();
+    bridgeMocks.stopBrowserBridgeServer.mockReset();
+
+    dockerMocks.dockerContainerState.mockResolvedValue({ exists: false, running: false });
+    dockerMocks.execDocker.mockImplementation(async (args: string[]) => {
+      if (args[0] === "image" && args[1] === "inspect") {
+        return { stdout: "[]", stderr: "", code: 0 };
+      }
+      return { stdout: "", stderr: "", code: 0 };
+    });
+    dockerMocks.readDockerContainerLabel.mockResolvedValue(null);
+    dockerMocks.readDockerContainerEnvVar.mockResolvedValue(null);
+    dockerMocks.readDockerPort.mockImplementation(async (_containerName: string, port: number) => {
+      if (port === 9222) {
+        return 49100;
+      }
+      if (port === 6080) {
+        return 49101;
+      }
+      return null;
+    });
+    registryMocks.readBrowserRegistry.mockResolvedValue({ entries: [] });
+    registryMocks.updateBrowserRegistry.mockResolvedValue(undefined);
+    bridgeMocks.startBrowserBridgeServer.mockResolvedValue({
+      server: {} as never,
+      port: 19000,
+      baseUrl: "http://127.0.0.1:19000",
+      state: {
+        server: null,
+        port: 19000,
+        resolved: { profiles: {} },
+        profiles: new Map(),
+      },
+    });
+    bridgeMocks.stopBrowserBridgeServer.mockResolvedValue(undefined);
+  });
+
+  it("publishes noVNC on loopback and injects noVNC password env", async () => {
+    const result = await ensureSandboxBrowser({
+      scopeKey: "session:test",
+      workspaceDir: "/tmp/workspace",
+      agentWorkspaceDir: "/tmp/workspace",
+      cfg: buildConfig(true),
+    });
+
+    const createArgs = dockerMocks.execDocker.mock.calls.find(
+      (call: unknown[]) => Array.isArray(call[0]) && call[0][0] === "create",
+    )?.[0] as string[] | undefined;
+
+    expect(createArgs).toBeDefined();
+    expect(createArgs).toContain("127.0.0.1::6080");
+    const envEntries = envEntriesFromDockerArgs(createArgs ?? []);
+    const passwordEntry = envEntries.find((entry) =>
+      entry.startsWith("OPENCLAW_BROWSER_NOVNC_PASSWORD="),
+    );
+    expect(passwordEntry).toMatch(/^OPENCLAW_BROWSER_NOVNC_PASSWORD=[a-f0-9]{8}$/);
+    expect(result?.noVncUrl).toMatch(/^http:\/\/127\.0\.0\.1:19000\/sandbox\/novnc\?token=/);
+    expect(result?.noVncUrl).not.toContain("password=");
+  });
+
+  it("does not inject noVNC password env when noVNC is disabled", async () => {
+    const result = await ensureSandboxBrowser({
+      scopeKey: "session:test",
+      workspaceDir: "/tmp/workspace",
+      agentWorkspaceDir: "/tmp/workspace",
+      cfg: buildConfig(false),
+    });
+
+    const createArgs = dockerMocks.execDocker.mock.calls.find(
+      (call: unknown[]) => Array.isArray(call[0]) && call[0][0] === "create",
+    )?.[0] as string[] | undefined;
+    const envEntries = envEntriesFromDockerArgs(createArgs ?? []);
+    expect(envEntries.some((entry) => entry.startsWith("OPENCLAW_BROWSER_NOVNC_PASSWORD="))).toBe(
+      false,
+    );
+    expect(result?.noVncUrl).toBeUndefined();
+  });
+});

src/agents/sandbox/browser.novnc-url.test.ts

@@ -1,16 +1,46 @@
 import { describe, expect, it } from "vitest";
-import { buildNoVncObserverUrl } from "./browser.js";
+import {
+  buildNoVncDirectUrl,
+  buildNoVncObserverTokenUrl,
+  consumeNoVncObserverToken,
+  issueNoVncObserverToken,
+  resetNoVncObserverTokensForTests,
+} from "./novnc-auth.js";
 
-describe("buildNoVncObserverUrl", () => {
+describe("noVNC auth helpers", () => {
   it("builds the default observer URL without password", () => {
-    expect(buildNoVncObserverUrl(45678)).toBe(
+    expect(buildNoVncDirectUrl(45678)).toBe(
       "http://127.0.0.1:45678/vnc.html?autoconnect=1&resize=remote",
     );
   });
 
   it("adds an encoded password query parameter when provided", () => {
-    expect(buildNoVncObserverUrl(45678, "a+b c&d")).toBe(
+    expect(buildNoVncDirectUrl(45678, "a+b c&d")).toBe(
       "http://127.0.0.1:45678/vnc.html?autoconnect=1&resize=remote&password=a%2Bb+c%26d",
     );
   });
+
+  it("issues one-time short-lived observer tokens", () => {
+    resetNoVncObserverTokensForTests();
+    const token = issueNoVncObserverToken({
+      url: "http://127.0.0.1:50123/vnc.html?autoconnect=1&resize=remote&password=abcd1234",
+      nowMs: 1000,
+      ttlMs: 100,
+    });
+    expect(buildNoVncObserverTokenUrl("http://127.0.0.1:19999", token)).toBe(
+      `http://127.0.0.1:19999/sandbox/novnc?token=${token}`,
+    );
+    expect(consumeNoVncObserverToken(token, 1050)).toContain("/vnc.html?");
+    expect(consumeNoVncObserverToken(token, 1050)).toBeNull();
+  });
+
+  it("expires observer tokens", () => {
+    resetNoVncObserverTokensForTests();
+    const token = issueNoVncObserverToken({
+      url: "http://127.0.0.1:50123/vnc.html?autoconnect=1&resize=remote&password=abcd1234",
+      nowMs: 1000,
+      ttlMs: 100,
+    });
+    expect(consumeNoVncObserverToken(token, 1200)).toBeNull();
+  });
 });

src/agents/sandbox/browser.ts

@@ -23,29 +23,21 @@ import {
   readDockerContainerLabel,
   readDockerPort,
 } from "./docker.js";
+import {
+  buildNoVncDirectUrl,
+  buildNoVncObserverTokenUrl,
+  consumeNoVncObserverToken,
+  generateNoVncPassword,
+  isNoVncEnabled,
+  NOVNC_PASSWORD_ENV_KEY,
+  issueNoVncObserverToken,
+} from "./novnc-auth.js";
 import { readBrowserRegistry, updateBrowserRegistry } from "./registry.js";
 import { resolveSandboxAgentId, slugifySessionKey } from "./shared.js";
 import { isToolAllowed } from "./tool-policy.js";
 import type { SandboxBrowserContext, SandboxConfig } from "./types.js";
 
 const HOT_BROWSER_WINDOW_MS = 5 * 60 * 1000;
-const NOVNC_PASSWORD_ENV_KEY = "OPENCLAW_BROWSER_NOVNC_PASSWORD";
-
-function generateNoVncPassword() {
-  // VNC auth uses an 8-char password max.
-  return crypto.randomBytes(4).toString("hex");
-}
-
-export function buildNoVncObserverUrl(port: number, password?: string) {
-  const query = new URLSearchParams({
-    autoconnect: "1",
-    resize: "remote",
-  });
-  if (password?.trim()) {
-    query.set("password", password);
-  }
-  return `http://127.0.0.1:${port}/vnc.html?${query.toString()}`;
-}
 
 async function waitForSandboxCdp(params: { cdpPort: number; timeoutMs: number }): Promise<boolean> {
   const deadline = Date.now() + Math.max(0, params.timeoutMs);
@@ -158,7 +150,7 @@ export async function ensureSandboxBrowser(params: {
   let running = state.running;
   let currentHash: string | null = null;
   let hashMismatch = false;
-  const noVncEnabled = params.cfg.browser.enableNoVnc && !params.cfg.browser.headless;
+  const noVncEnabled = isNoVncEnabled(params.cfg.browser);
   let noVncPassword: string | undefined;
 
   if (hasContainer) {
@@ -331,6 +323,7 @@ export async function ensureSandboxBrowser(params: {
       authToken: desiredAuthToken,
       authPassword: desiredAuthPassword,
       onEnsureAttachTarget,
+      resolveSandboxNoVncToken: consumeNoVncObserverToken,
     });
   };
 
@@ -356,7 +349,13 @@ export async function ensureSandboxBrowser(params: {
   });
 
   const noVncUrl =
-    mappedNoVnc && noVncEnabled ? buildNoVncObserverUrl(mappedNoVnc, noVncPassword) : undefined;
+    mappedNoVnc && noVncEnabled
+      ? (() => {
+          const directUrl = buildNoVncDirectUrl(mappedNoVnc, noVncPassword);
+          const token = issueNoVncObserverToken({ url: directUrl });
+          return buildNoVncObserverTokenUrl(resolvedBridge.baseUrl, token);
+        })()
+      : undefined;
 
   return {
     bridgeUrl: resolvedBridge.baseUrl,

src/agents/sandbox/novnc-auth.ts

@@ -0,0 +1,81 @@
+import crypto from "node:crypto";
+
+export const NOVNC_PASSWORD_ENV_KEY = "OPENCLAW_BROWSER_NOVNC_PASSWORD";
+const NOVNC_TOKEN_TTL_MS = 5 * 60 * 1000;
+
+type NoVncObserverTokenEntry = {
+  url: string;
+  expiresAt: number;
+};
+
+const NO_VNC_OBSERVER_TOKENS = new Map<string, NoVncObserverTokenEntry>();
+
+function pruneExpiredNoVncObserverTokens(now: number) {
+  for (const [token, entry] of NO_VNC_OBSERVER_TOKENS) {
+    if (entry.expiresAt <= now) {
+      NO_VNC_OBSERVER_TOKENS.delete(token);
+    }
+  }
+}
+
+export function isNoVncEnabled(params: { enableNoVnc: boolean; headless: boolean }) {
+  return params.enableNoVnc && !params.headless;
+}
+
+export function generateNoVncPassword() {
+  // VNC auth uses an 8-char password max.
+  return crypto.randomBytes(4).toString("hex");
+}
+
+export function buildNoVncDirectUrl(port: number, password?: string) {
+  const query = new URLSearchParams({
+    autoconnect: "1",
+    resize: "remote",
+  });
+  if (password?.trim()) {
+    query.set("password", password);
+  }
+  return `http://127.0.0.1:${port}/vnc.html?${query.toString()}`;
+}
+
+export function issueNoVncObserverToken(params: {
+  url: string;
+  ttlMs?: number;
+  nowMs?: number;
+}): string {
+  const now = params.nowMs ?? Date.now();
+  pruneExpiredNoVncObserverTokens(now);
+  const token = crypto.randomBytes(24).toString("hex");
+  NO_VNC_OBSERVER_TOKENS.set(token, {
+    url: params.url,
+    expiresAt: now + Math.max(1, params.ttlMs ?? NOVNC_TOKEN_TTL_MS),
+  });
+  return token;
+}
+
+export function consumeNoVncObserverToken(token: string, nowMs?: number): string | null {
+  const now = nowMs ?? Date.now();
+  pruneExpiredNoVncObserverTokens(now);
+  const normalized = token.trim();
+  if (!normalized) {
+    return null;
+  }
+  const entry = NO_VNC_OBSERVER_TOKENS.get(normalized);
+  if (!entry) {
+    return null;
+  }
+  NO_VNC_OBSERVER_TOKENS.delete(normalized);
+  if (entry.expiresAt <= now) {
+    return null;
+  }
+  return entry.url;
+}
+
+export function buildNoVncObserverTokenUrl(baseUrl: string, token: string) {
+  const query = new URLSearchParams({ token });
+  return `${baseUrl}/sandbox/novnc?${query.toString()}`;
+}
+
+export function resetNoVncObserverTokensForTests() {
+  NO_VNC_OBSERVER_TOKENS.clear();
+}