fix: enforce secure control ui auth
This commit is contained in:
@@ -187,6 +187,7 @@ const FIELD_LABELS: Record<string, string> = {
|
||||
"tools.web.fetch.maxRedirects": "Web Fetch Max Redirects",
|
||||
"tools.web.fetch.userAgent": "Web Fetch User-Agent",
|
||||
"gateway.controlUi.basePath": "Control UI Base Path",
|
||||
"gateway.controlUi.allowInsecureAuth": "Allow Insecure Control UI Auth",
|
||||
"gateway.http.endpoints.chatCompletions.enabled": "OpenAI Chat Completions Endpoint",
|
||||
"gateway.reload.mode": "Config Reload Mode",
|
||||
"gateway.reload.debounceMs": "Config Reload Debounce (ms)",
|
||||
@@ -345,6 +346,8 @@ const FIELD_HELP: Record<string, string> = {
|
||||
"gateway.auth.password": "Required for Tailscale funnel.",
|
||||
"gateway.controlUi.basePath":
|
||||
"Optional URL prefix where the Control UI is served (e.g. /clawdbot).",
|
||||
"gateway.controlUi.allowInsecureAuth":
|
||||
"Allow Control UI auth over insecure HTTP (token-only; not recommended).",
|
||||
"gateway.http.endpoints.chatCompletions.enabled":
|
||||
"Enable the OpenAI-compatible `POST /v1/chat/completions` endpoint (default: false).",
|
||||
"gateway.reload.mode": 'Hot reload strategy for config changes ("hybrid" recommended).',
|
||||
|
||||
@@ -51,6 +51,8 @@ export type GatewayControlUiConfig = {
|
||||
enabled?: boolean;
|
||||
/** Optional base path prefix for the Control UI (e.g. "/clawdbot"). */
|
||||
basePath?: string;
|
||||
/** Allow token-only auth over insecure HTTP (default: false). */
|
||||
allowInsecureAuth?: boolean;
|
||||
};
|
||||
|
||||
export type GatewayAuthMode = "token" | "password";
|
||||
|
||||
@@ -282,6 +282,7 @@ export const ClawdbotSchema = z
|
||||
.object({
|
||||
enabled: z.boolean().optional(),
|
||||
basePath: z.string().optional(),
|
||||
allowInsecureAuth: z.boolean().optional(),
|
||||
})
|
||||
.strict()
|
||||
.optional(),
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
startServerWithClient,
|
||||
testState,
|
||||
} from "./test-helpers.js";
|
||||
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
|
||||
|
||||
installGatewayTestHooks();
|
||||
|
||||
@@ -127,6 +128,52 @@ describe("gateway server auth/connect", () => {
|
||||
await server.close();
|
||||
});
|
||||
|
||||
test("rejects control ui without device identity by default", async () => {
|
||||
const { server, ws, prevToken } = await startServerWithClient("secret");
|
||||
const res = await connectReq(ws, {
|
||||
token: "secret",
|
||||
device: null,
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_NAMES.CONTROL_UI,
|
||||
version: "1.0.0",
|
||||
platform: "web",
|
||||
mode: GATEWAY_CLIENT_MODES.WEBCHAT,
|
||||
},
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.message ?? "").toContain("secure context");
|
||||
ws.close();
|
||||
await server.close();
|
||||
if (prevToken === undefined) {
|
||||
delete process.env.CLAWDBOT_GATEWAY_TOKEN;
|
||||
} else {
|
||||
process.env.CLAWDBOT_GATEWAY_TOKEN = prevToken;
|
||||
}
|
||||
});
|
||||
|
||||
test("allows control ui without device identity when insecure auth is enabled", async () => {
|
||||
testState.gatewayControlUi = { allowInsecureAuth: true };
|
||||
const { server, ws, prevToken } = await startServerWithClient("secret");
|
||||
const res = await connectReq(ws, {
|
||||
token: "secret",
|
||||
device: null,
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_NAMES.CONTROL_UI,
|
||||
version: "1.0.0",
|
||||
platform: "web",
|
||||
mode: GATEWAY_CLIENT_MODES.WEBCHAT,
|
||||
},
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
ws.close();
|
||||
await server.close();
|
||||
if (prevToken === undefined) {
|
||||
delete process.env.CLAWDBOT_GATEWAY_TOKEN;
|
||||
} else {
|
||||
process.env.CLAWDBOT_GATEWAY_TOKEN = prevToken;
|
||||
}
|
||||
});
|
||||
|
||||
test("accepts device token auth for paired device", async () => {
|
||||
const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js");
|
||||
const { approveDevicePairing, getPairedDevice, listDevicePairing } =
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
validateConnectParams,
|
||||
validateRequestFrame,
|
||||
} from "../../protocol/index.js";
|
||||
import { GATEWAY_CLIENT_IDS } from "../../protocol/client-info.js";
|
||||
import { MAX_BUFFERED_BYTES, MAX_PAYLOAD_BYTES, TICK_INTERVAL_MS } from "../../server-constants.js";
|
||||
import type { GatewayRequestContext, GatewayRequestHandlers } from "../../server-methods/types.js";
|
||||
import { handleGatewayRequest } from "../../server-methods.js";
|
||||
@@ -293,24 +294,53 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
|
||||
const device = connectParams.device;
|
||||
let devicePublicKey: string | null = null;
|
||||
// Allow token-authenticated connections (e.g., control-ui) to skip device identity
|
||||
const hasTokenAuth = !!connectParams.auth?.token;
|
||||
if (!device && !hasTokenAuth) {
|
||||
setHandshakeState("failed");
|
||||
setCloseCause("device-required", {
|
||||
client: connectParams.client.id,
|
||||
clientDisplayName: connectParams.client.displayName,
|
||||
mode: connectParams.client.mode,
|
||||
version: connectParams.client.version,
|
||||
});
|
||||
send({
|
||||
type: "res",
|
||||
id: frame.id,
|
||||
ok: false,
|
||||
error: errorShape(ErrorCodes.NOT_PAIRED, "device identity required"),
|
||||
});
|
||||
close(1008, "device identity required");
|
||||
return;
|
||||
const hasTokenAuth = Boolean(connectParams.auth?.token);
|
||||
const hasPasswordAuth = Boolean(connectParams.auth?.password);
|
||||
const isControlUi = connectParams.client.id === GATEWAY_CLIENT_IDS.CONTROL_UI;
|
||||
|
||||
if (!device) {
|
||||
const allowInsecureControlUi =
|
||||
isControlUi && loadConfig().gateway?.controlUi?.allowInsecureAuth === true;
|
||||
const canSkipDevice =
|
||||
isControlUi && allowInsecureControlUi ? hasTokenAuth || hasPasswordAuth : hasTokenAuth;
|
||||
|
||||
if (isControlUi && !allowInsecureControlUi) {
|
||||
const errorMessage = "control ui requires HTTPS or localhost (secure context)";
|
||||
setHandshakeState("failed");
|
||||
setCloseCause("control-ui-insecure-auth", {
|
||||
client: connectParams.client.id,
|
||||
clientDisplayName: connectParams.client.displayName,
|
||||
mode: connectParams.client.mode,
|
||||
version: connectParams.client.version,
|
||||
});
|
||||
send({
|
||||
type: "res",
|
||||
id: frame.id,
|
||||
ok: false,
|
||||
error: errorShape(ErrorCodes.INVALID_REQUEST, errorMessage),
|
||||
});
|
||||
close(1008, errorMessage);
|
||||
return;
|
||||
}
|
||||
|
||||
// Allow token-authenticated connections (e.g., control-ui) to skip device identity
|
||||
if (!canSkipDevice) {
|
||||
setHandshakeState("failed");
|
||||
setCloseCause("device-required", {
|
||||
client: connectParams.client.id,
|
||||
clientDisplayName: connectParams.client.displayName,
|
||||
mode: connectParams.client.mode,
|
||||
version: connectParams.client.version,
|
||||
});
|
||||
send({
|
||||
type: "res",
|
||||
id: frame.id,
|
||||
ok: false,
|
||||
error: errorShape(ErrorCodes.NOT_PAIRED, "device identity required"),
|
||||
});
|
||||
close(1008, "device identity required");
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (device) {
|
||||
const derivedId = deriveDeviceIdFromPublicKey(device.publicKey);
|
||||
|
||||
@@ -210,6 +210,7 @@ export const testState = {
|
||||
cronEnabled: false as boolean | undefined,
|
||||
gatewayBind: undefined as "auto" | "lan" | "tailnet" | "loopback" | undefined,
|
||||
gatewayAuth: undefined as Record<string, unknown> | undefined,
|
||||
gatewayControlUi: undefined as Record<string, unknown> | undefined,
|
||||
hooksConfig: undefined as HooksConfig | undefined,
|
||||
canvasHostPort: undefined as number | undefined,
|
||||
legacyIssues: [] as Array<{ path: string; message: string }>,
|
||||
@@ -443,6 +444,7 @@ vi.mock("../config/config.js", async () => {
|
||||
: {};
|
||||
if (testState.gatewayBind) fileGateway.bind = testState.gatewayBind;
|
||||
if (testState.gatewayAuth) fileGateway.auth = testState.gatewayAuth;
|
||||
if (testState.gatewayControlUi) fileGateway.controlUi = testState.gatewayControlUi;
|
||||
const gateway = Object.keys(fileGateway).length > 0 ? fileGateway : undefined;
|
||||
|
||||
const fileCanvasHost =
|
||||
|
||||
@@ -101,6 +101,7 @@ export function installGatewayTestHooks() {
|
||||
testTailnetIPv4.value = undefined;
|
||||
testState.gatewayBind = undefined;
|
||||
testState.gatewayAuth = undefined;
|
||||
testState.gatewayControlUi = undefined;
|
||||
testState.hooksConfig = undefined;
|
||||
testState.canvasHostPort = undefined;
|
||||
testState.legacyIssues = [];
|
||||
@@ -280,7 +281,7 @@ export async function connectReq(
|
||||
signature: string;
|
||||
signedAt: number;
|
||||
nonce?: string;
|
||||
};
|
||||
} | null;
|
||||
},
|
||||
): Promise<ConnectResponse> {
|
||||
const { randomUUID } = await import("node:crypto");
|
||||
@@ -294,6 +295,7 @@ export async function connectReq(
|
||||
const role = opts?.role ?? "operator";
|
||||
const requestedScopes = Array.isArray(opts?.scopes) ? opts?.scopes : [];
|
||||
const device = (() => {
|
||||
if (opts?.device === null) return undefined;
|
||||
if (opts?.device) return opts.device;
|
||||
const identity = loadOrCreateDeviceIdentity();
|
||||
const signedAtMs = Date.now();
|
||||
|
||||
@@ -227,6 +227,29 @@ describe("security audit", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("warns when control UI allows insecure auth", async () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
gateway: {
|
||||
controlUi: { allowInsecureAuth: true },
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: false,
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "gateway.control_ui.insecure_auth",
|
||||
severity: "warn",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("warns when multiple DM senders share the main session", async () => {
|
||||
const cfg: ClawdbotConfig = { session: { dmScope: "main" } };
|
||||
const plugins: ChannelPlugin[] = [
|
||||
|
||||
@@ -235,6 +235,17 @@ function collectGatewayConfigFindings(cfg: ClawdbotConfig): SecurityAuditFinding
|
||||
});
|
||||
}
|
||||
|
||||
if (cfg.gateway?.controlUi?.allowInsecureAuth === true) {
|
||||
findings.push({
|
||||
checkId: "gateway.control_ui.insecure_auth",
|
||||
severity: "warn",
|
||||
title: "Control UI allows insecure HTTP auth",
|
||||
detail:
|
||||
"gateway.controlUi.allowInsecureAuth=true allows token-only auth over HTTP and skips device identity.",
|
||||
remediation: "Disable it or switch to HTTPS (Tailscale Serve) or localhost.",
|
||||
});
|
||||
}
|
||||
|
||||
const token =
|
||||
typeof auth.token === "string" && auth.token.trim().length > 0 ? auth.token.trim() : null;
|
||||
if (auth.mode === "token" && token && token.length < 24) {
|
||||
|
||||
Reference in New Issue
Block a user