fix(security): harden sandbox novnc observer flow

This commit is contained in:
Agent
2026-03-01 22:44:15 +00:00
parent 4ab13eca4d
commit 002539c01e
10 changed files with 128 additions and 32 deletions

View File

@@ -162,7 +162,7 @@ describe("ensureSandboxBrowser create args", () => {
const passwordEntry = envEntries.find((entry) =>
entry.startsWith("OPENCLAW_BROWSER_NOVNC_PASSWORD="),
);
expect(passwordEntry).toMatch(/^OPENCLAW_BROWSER_NOVNC_PASSWORD=[a-f0-9]{8}$/);
expect(passwordEntry).toMatch(/^OPENCLAW_BROWSER_NOVNC_PASSWORD=[A-Za-z0-9]{8}$/);
expect(result?.noVncUrl).toMatch(/^http:\/\/127\.0\.0\.1:19000\/sandbox\/novnc\?token=/);
expect(result?.noVncUrl).not.toContain("password=");
});

View File

@@ -2,45 +2,55 @@ import { describe, expect, it } from "vitest";
import {
buildNoVncDirectUrl,
buildNoVncObserverTokenUrl,
buildNoVncObserverTargetUrl,
consumeNoVncObserverToken,
generateNoVncPassword,
issueNoVncObserverToken,
resetNoVncObserverTokensForTests,
} from "./novnc-auth.js";
describe("noVNC auth helpers", () => {
it("builds the default observer URL without password", () => {
expect(buildNoVncDirectUrl(45678)).toBe(
"http://127.0.0.1:45678/vnc.html?autoconnect=1&resize=remote",
);
expect(buildNoVncDirectUrl(45678)).toBe("http://127.0.0.1:45678/vnc.html");
});
it("adds an encoded password query parameter when provided", () => {
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("builds a fragment-based observer target URL with password", () => {
expect(buildNoVncObserverTargetUrl({ port: 45678, password: "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",
noVncPort: 50123,
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)).toEqual({
noVncPort: 50123,
password: "abcd1234",
});
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",
noVncPort: 50123,
password: "abcd1234",
nowMs: 1000,
ttlMs: 100,
});
expect(consumeNoVncObserverToken(token, 1200)).toBeNull();
});
it("generates 8-char alphanumeric passwords", () => {
const password = generateNoVncPassword();
expect(password).toMatch(/^[a-zA-Z0-9]{8}$/);
});
});

View File

@@ -24,7 +24,6 @@ import {
readDockerPort,
} from "./docker.js";
import {
buildNoVncDirectUrl,
buildNoVncObserverTokenUrl,
consumeNoVncObserverToken,
generateNoVncPassword,
@@ -390,8 +389,10 @@ export async function ensureSandboxBrowser(params: {
const noVncUrl =
mappedNoVnc && noVncEnabled
? (() => {
const directUrl = buildNoVncDirectUrl(mappedNoVnc, noVncPassword);
const token = issueNoVncObserverToken({ url: directUrl });
const token = issueNoVncObserverToken({
noVncPort: mappedNoVnc,
password: noVncPassword,
});
return buildNoVncObserverTokenUrl(resolvedBridge.baseUrl, token);
})()
: undefined;

View File

@@ -1,13 +1,21 @@
import crypto from "node:crypto";
export const NOVNC_PASSWORD_ENV_KEY = "OPENCLAW_BROWSER_NOVNC_PASSWORD";
const NOVNC_TOKEN_TTL_MS = 5 * 60 * 1000;
const NOVNC_TOKEN_TTL_MS = 60 * 1000;
const NOVNC_PASSWORD_LENGTH = 8;
const NOVNC_PASSWORD_ALPHABET = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
type NoVncObserverTokenEntry = {
url: string;
noVncPort: number;
password?: string;
expiresAt: number;
};
export type NoVncObserverTokenPayload = {
noVncPort: number;
password?: string;
};
const NO_VNC_OBSERVER_TOKENS = new Map<string, NoVncObserverTokenEntry>();
function pruneExpiredNoVncObserverTokens(now: number) {
@@ -24,22 +32,31 @@ export function isNoVncEnabled(params: { enableNoVnc: boolean; headless: boolean
export function generateNoVncPassword() {
// VNC auth uses an 8-char password max.
return crypto.randomBytes(4).toString("hex");
let out = "";
for (let i = 0; i < NOVNC_PASSWORD_LENGTH; i += 1) {
out += NOVNC_PASSWORD_ALPHABET[crypto.randomInt(0, NOVNC_PASSWORD_ALPHABET.length)];
}
return out;
}
export function buildNoVncDirectUrl(port: number, password?: string) {
export function buildNoVncDirectUrl(port: number) {
return `http://127.0.0.1:${port}/vnc.html`;
}
export function buildNoVncObserverTargetUrl(params: { port: number; password?: string }) {
const query = new URLSearchParams({
autoconnect: "1",
resize: "remote",
});
if (password?.trim()) {
query.set("password", password);
if (params.password?.trim()) {
query.set("password", params.password);
}
return `http://127.0.0.1:${port}/vnc.html?${query.toString()}`;
return `${buildNoVncDirectUrl(params.port)}#${query.toString()}`;
}
export function issueNoVncObserverToken(params: {
url: string;
noVncPort: number;
password?: string;
ttlMs?: number;
nowMs?: number;
}): string {
@@ -47,13 +64,17 @@ export function issueNoVncObserverToken(params: {
pruneExpiredNoVncObserverTokens(now);
const token = crypto.randomBytes(24).toString("hex");
NO_VNC_OBSERVER_TOKENS.set(token, {
url: params.url,
noVncPort: params.noVncPort,
password: params.password?.trim() || undefined,
expiresAt: now + Math.max(1, params.ttlMs ?? NOVNC_TOKEN_TTL_MS),
});
return token;
}
export function consumeNoVncObserverToken(token: string, nowMs?: number): string | null {
export function consumeNoVncObserverToken(
token: string,
nowMs?: number,
): NoVncObserverTokenPayload | null {
const now = nowMs ?? Date.now();
pruneExpiredNoVncObserverTokens(now);
const normalized = token.trim();
@@ -68,7 +89,7 @@ export function consumeNoVncObserverToken(token: string, nowMs?: number): string
if (entry.expiresAt <= now) {
return null;
}
return entry.url;
return { noVncPort: entry.noVncPort, password: entry.password };
}
export function buildNoVncObserverTokenUrl(baseUrl: string, token: string) {