fix: harden doctor gateway exposure warnings (#2016) (thanks @Alex-Alaniz) (#2016)

Co-authored-by: Peter Steinberger <steipete@gmail.com>

src/commands/doctor-security.test.ts

@@ -0,0 +1,71 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import type { ClawdbotConfig } from "../config/config.js";
+
+const note = vi.hoisted(() => vi.fn());
+
+vi.mock("../terminal/note.js", () => ({
+  note,
+}));
+
+vi.mock("../channels/plugins/index.js", () => ({
+  listChannelPlugins: () => [],
+}));
+
+import { noteSecurityWarnings } from "./doctor-security.js";
+
+describe("noteSecurityWarnings gateway exposure", () => {
+  let prevToken: string | undefined;
+  let prevPassword: string | undefined;
+
+  beforeEach(() => {
+    note.mockClear();
+    prevToken = process.env.CLAWDBOT_GATEWAY_TOKEN;
+    prevPassword = process.env.CLAWDBOT_GATEWAY_PASSWORD;
+    delete process.env.CLAWDBOT_GATEWAY_TOKEN;
+    delete process.env.CLAWDBOT_GATEWAY_PASSWORD;
+  });
+
+  afterEach(() => {
+    if (prevToken === undefined) delete process.env.CLAWDBOT_GATEWAY_TOKEN;
+    else process.env.CLAWDBOT_GATEWAY_TOKEN = prevToken;
+    if (prevPassword === undefined) delete process.env.CLAWDBOT_GATEWAY_PASSWORD;
+    else process.env.CLAWDBOT_GATEWAY_PASSWORD = prevPassword;
+  });
+
+  const lastMessage = () => String(note.mock.calls.at(-1)?.[0] ?? "");
+
+  it("warns when exposed without auth", async () => {
+    const cfg = { gateway: { bind: "lan" } } as ClawdbotConfig;
+    await noteSecurityWarnings(cfg);
+    const message = lastMessage();
+    expect(message).toContain("CRITICAL");
+    expect(message).toContain("without authentication");
+  });
+
+  it("uses env token to avoid critical warning", async () => {
+    process.env.CLAWDBOT_GATEWAY_TOKEN = "token-123";
+    const cfg = { gateway: { bind: "lan" } } as ClawdbotConfig;
+    await noteSecurityWarnings(cfg);
+    const message = lastMessage();
+    expect(message).toContain("WARNING");
+    expect(message).not.toContain("CRITICAL");
+  });
+
+  it("treats whitespace token as missing", async () => {
+    const cfg = {
+      gateway: { bind: "lan", auth: { mode: "token", token: "   " } },
+    } as ClawdbotConfig;
+    await noteSecurityWarnings(cfg);
+    const message = lastMessage();
+    expect(message).toContain("CRITICAL");
+  });
+
+  it("skips warning for loopback bind", async () => {
+    const cfg = { gateway: { bind: "loopback" } } as ClawdbotConfig;
+    await noteSecurityWarnings(cfg);
+    const message = lastMessage();
+    expect(message).toContain("No channel security warnings detected");
+    expect(message).not.toContain("Gateway bound");
+  });
+});

src/commands/doctor-security.ts

@@ -1,10 +1,12 @@
 import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js";
 import { listChannelPlugins } from "../channels/plugins/index.js";
 import type { ChannelId } from "../channels/plugins/types.js";
-import type { ClawdbotConfig } from "../config/config.js";
+import type { ClawdbotConfig, GatewayBindMode } from "../config/config.js";
 import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
 import { note } from "../terminal/note.js";
 import { formatCliCommand } from "../cli/command-format.js";
+import { resolveGatewayAuth } from "../gateway/auth.js";
+import { isLoopbackHost, resolveGatewayBindHost } from "../gateway/net.js";
 
 export async function noteSecurityWarnings(cfg: ClawdbotConfig) {
   const warnings: string[] = [];
@@ -16,50 +18,55 @@ export async function noteSecurityWarnings(cfg: ClawdbotConfig) {
   // Check for dangerous gateway binding configurations
   // that expose the gateway to network without proper auth
 
-  const gatewayBind = cfg.gateway?.bind ?? "loopback";
+  const gatewayBind = (cfg.gateway?.bind ?? "loopback") as string;
   const customBindHost = cfg.gateway?.customBindHost?.trim();
-  const authMode = cfg.gateway?.auth?.mode ?? "off";
-  const authToken = cfg.gateway?.auth?.token;
-  const authPassword = cfg.gateway?.auth?.password;
+  const bindModes: GatewayBindMode[] = ["auto", "lan", "loopback", "custom", "tailnet"];
+  const bindMode = bindModes.includes(gatewayBind as GatewayBindMode)
+    ? (gatewayBind as GatewayBindMode)
+    : undefined;
+  const resolvedBindHost = bindMode
+    ? await resolveGatewayBindHost(bindMode, customBindHost)
+    : "0.0.0.0";
+  const isExposed = !isLoopbackHost(resolvedBindHost);
 
-  const isLoopbackBindHost = (host: string) => {
-    const normalized = host.trim().toLowerCase();
-    return (
-      normalized === "localhost" ||
-      normalized === "::1" ||
-      normalized === "[::1]" ||
-      normalized.startsWith("127.")
-    );
-  };
-
-  // Bindings that expose gateway beyond localhost
-  const exposedBindings = ["all", "lan", "0.0.0.0"];
-  const isExposed =
-    exposedBindings.includes(gatewayBind) ||
-    (gatewayBind === "custom" && (!customBindHost || !isLoopbackBindHost(customBindHost)));
+  const resolvedAuth = resolveGatewayAuth({
+    authConfig: cfg.gateway?.auth,
+    env: process.env,
+    tailscaleMode: cfg.gateway?.tailscale?.mode ?? "off",
+  });
+  const authToken = resolvedAuth.token?.trim() ?? "";
+  const authPassword = resolvedAuth.password?.trim() ?? "";
+  const hasToken = authToken.length > 0;
+  const hasPassword = authPassword.length > 0;
+  const hasSharedSecret =
+    (resolvedAuth.mode === "token" && hasToken) ||
+    (resolvedAuth.mode === "password" && hasPassword);
+  const bindDescriptor = `"${gatewayBind}" (${resolvedBindHost})`;
 
   if (isExposed) {
-    if (authMode === "off") {
+    if (!hasSharedSecret) {
+      const authFixLines =
+        resolvedAuth.mode === "password"
+          ? [
+              `  Fix: ${formatCliCommand("clawdbot configure")} to set a password`,
+              `  Or switch to token: ${formatCliCommand("clawdbot config set gateway.auth.mode token")}`,
+            ]
+          : [
+              `  Fix: ${formatCliCommand("clawdbot doctor --fix")} to generate a token`,
+              `  Or set token directly: ${formatCliCommand(
+                "clawdbot config set gateway.auth.mode token",
+              )}`,
+            ];
       warnings.push(
-        `- CRITICAL: Gateway bound to "${gatewayBind}" with NO authentication.`,
+        `- CRITICAL: Gateway bound to ${bindDescriptor} without authentication.`,
         `  Anyone on your network (or internet if port-forwarded) can fully control your agent.`,
         `  Fix: ${formatCliCommand("clawdbot config set gateway.bind loopback")}`,
-        `  Or enable auth: ${formatCliCommand("clawdbot config set gateway.auth.mode token")}`,
-      );
-    } else if (authMode === "token" && !authToken) {
-      warnings.push(
-        `- CRITICAL: Gateway bound to "${gatewayBind}" with empty auth token.`,
-        `  Fix: ${formatCliCommand("clawdbot doctor --fix")} to generate a token`,
-      );
-    } else if (authMode === "password" && !authPassword) {
-      warnings.push(
-        `- CRITICAL: Gateway bound to "${gatewayBind}" with empty password.`,
-        `  Fix: ${formatCliCommand("clawdbot configure")} to set a password`,
+        ...authFixLines,
       );
     } else {
       // Auth is configured, but still warn about network exposure
       warnings.push(
-        `- WARNING: Gateway bound to "${gatewayBind}" (network-accessible).`,
+        `- WARNING: Gateway bound to ${bindDescriptor} (network-accessible).`,
         `  Ensure your auth credentials are strong and not exposed.`,
       );
     }