Merge 3c9e3190476a6f090b7b94d0a68b9367ccb9ffac into 43513cd1df63af0704dfb351ee7864607f955dcc

src/gateway/input-allowlist.test.ts

@@ -2,13 +2,16 @@ import { describe, expect, it } from "vitest";
 import { normalizeInputHostnameAllowlist } from "./input-allowlist.js";
 
 describe("normalizeInputHostnameAllowlist", () => {
-  it("treats missing and empty allowlists as unset", () => {
+  it("treats a missing allowlist as unset", () => {
     expect(normalizeInputHostnameAllowlist(undefined)).toBeUndefined();
-    expect(normalizeInputHostnameAllowlist([])).toBeUndefined();
   });
 
-  it("drops whitespace-only entries and treats the result as unset", () => {
-    expect(normalizeInputHostnameAllowlist(["", "   "])).toBeUndefined();
+  it("preserves an explicit empty allowlist as deny-all", () => {
+    expect(normalizeInputHostnameAllowlist([])).toEqual([]);
+  });
+
+  it("fails closed when configured entries trim down to nothing", () => {
+    expect(normalizeInputHostnameAllowlist(["", "   "])).toEqual([]);
   });
 
   it("preserves trimmed hostname patterns", () => {

src/gateway/input-allowlist.ts

@@ -2,15 +2,17 @@
  * Normalize optional gateway URL-input hostname allowlists.
  *
  * Semantics are intentionally:
- * - missing / empty / whitespace-only list => no hostname allowlist restriction
- * - deny-all URL fetching => use the corresponding `allowUrl: false` switch
+ * - missing list => no hostname allowlist restriction
+ * - explicit empty / fully trimmed-empty list => deny all URL fetches
  */
 export function normalizeInputHostnameAllowlist(
   values: string[] | undefined,
 ): string[] | undefined {
-  if (!values || values.length === 0) {
+  if (values === undefined) {
     return undefined;
   }
-  const normalized = values.map((value) => value.trim()).filter((value) => value.length > 0);
-  return normalized.length > 0 ? normalized : undefined;
+  if (values.length === 0) {
+    return [];
+  }
+  return values.map((value) => value.trim()).filter((value) => value.length > 0);
 }

src/gateway/openai-http.test.ts

@@ -1,3 +1,5 @@
+import fs from "node:fs/promises";
+import path from "node:path";
 import { afterAll, beforeAll, describe, expect, it } from "vitest";
 import { HISTORY_CONTEXT_MARKER } from "../auto-reply/reply/history.js";
 import { CURRENT_MESSAGE_MARKER } from "../auto-reply/reply/mentions.js";
@@ -45,6 +47,15 @@ async function startServer(port: number, opts?: { openAiChatCompletionsEnabled?:
   });
 }
 
+async function writeGatewayConfig(config: Record<string, unknown>) {
+  const configPath = process.env.OPENCLAW_CONFIG_PATH;
+  if (!configPath) {
+    throw new Error("OPENCLAW_CONFIG_PATH is required for gateway config tests");
+  }
+  await fs.mkdir(path.dirname(configPath), { recursive: true });
+  await fs.writeFile(configPath, JSON.stringify(config, null, 2), "utf-8");
+}
+
 async function postChatCompletions(port: number, body: unknown, headers?: Record<string, string>) {
   const res = await fetch(`http://127.0.0.1:${port}/v1/chat/completions`, {
     method: "POST",
@@ -636,6 +647,61 @@ describe("OpenAI-compatible HTTP API (e2e)", () => {
     );
   });
 
+  it("treats an explicit empty image URL allowlist as deny-all", async () => {
+    const configPath = process.env.OPENCLAW_CONFIG_PATH;
+    if (!configPath) {
+      throw new Error("OPENCLAW_CONFIG_PATH is required for gateway config tests");
+    }
+    const previousConfig = await fs.readFile(configPath, "utf-8").catch(() => undefined);
+    const denyAllConfig = {
+      gateway: {
+        http: {
+          endpoints: {
+            chatCompletions: {
+              enabled: true,
+              images: {
+                allowUrl: true,
+                urlAllowlist: [],
+              },
+            },
+          },
+        },
+      },
+    };
+
+    await writeGatewayConfig(denyAllConfig);
+
+    const port = await getFreePort();
+    const server = await startServer(port);
+    try {
+      agentCommand.mockClear();
+      const res = await postChatCompletions(port, {
+        model: "openclaw",
+        messages: [
+          {
+            role: "user",
+            content: [
+              { type: "text", text: "look at this" },
+              { type: "image_url", image_url: { url: "https://example.com/image.png" } },
+            ],
+          },
+        ],
+      });
+      expect(res.status).toBe(400);
+      const json = (await res.json()) as { error?: { type?: string; message?: string } };
+      expect(json.error?.type).toBe("invalid_request_error");
+      expect(json.error?.message ?? "").toMatch(/invalid image_url content|allowlist|blocked/i);
+      expect(agentCommand).not.toHaveBeenCalled();
+    } finally {
+      await server.close({ reason: "openai empty allowlist hardening test done" });
+      if (previousConfig === undefined) {
+        await fs.rm(configPath, { force: true });
+      } else {
+        await fs.writeFile(configPath, previousConfig, "utf-8");
+      }
+    }
+  });
+
   it("streams SSE chunks when stream=true", async () => {
     const port = enabledPort;
     try {

src/gateway/openresponses-http.test.ts

@@ -673,6 +673,69 @@ describe("OpenResponses HTTP API (e2e)", () => {
       await allowlistServer.close({ reason: "responses allowlist hardening test done" });
     }
 
+    const configPath = process.env.OPENCLAW_CONFIG_PATH;
+    const previousConfig = configPath
+      ? await fs.readFile(configPath, "utf-8").catch(() => undefined)
+      : undefined;
+
+    const denyAllConfig = {
+      gateway: {
+        http: {
+          endpoints: {
+            responses: {
+              enabled: true,
+              maxUrlParts: 1,
+              files: {
+                allowUrl: true,
+                urlAllowlist: [],
+              },
+              images: {
+                allowUrl: true,
+                urlAllowlist: [],
+              },
+            },
+          },
+        },
+      },
+    };
+    await writeGatewayConfig(denyAllConfig);
+
+    const denyAllPort = await getFreePort();
+    const denyAllServer = await startServer(denyAllPort, { openResponsesEnabled: true });
+    try {
+      agentCommand.mockClear();
+
+      const fileBlocked = await postResponses(denyAllPort, {
+        model: "openclaw",
+        input: buildUrlInputMessage({
+          kind: "input_file",
+          text: "fetch this",
+          url: "https://cdn.example.com/file.txt",
+        }),
+      });
+      await expectInvalidRequest(fileBlocked, /invalid request|allowlist|blocked/i);
+
+      const imageBlocked = await postResponses(denyAllPort, {
+        model: "openclaw",
+        input: buildUrlInputMessage({
+          kind: "input_image",
+          text: "fetch this",
+          url: "https://images.example.com/image.png",
+        }),
+      });
+      await expectInvalidRequest(imageBlocked, /invalid request|allowlist|blocked/i);
+      expect(agentCommand).not.toHaveBeenCalled();
+    } finally {
+      await denyAllServer.close({ reason: "responses empty allowlist hardening test done" });
+      if (configPath) {
+        if (previousConfig === undefined) {
+          await fs.rm(configPath, { force: true });
+        } else {
+          await fs.writeFile(configPath, previousConfig, "utf-8");
+        }
+      }
+    }
+
     const capConfig = buildResponsesUrlPolicyConfig(0);
     await writeGatewayConfig(capConfig);
 

src/infra/net/fetch-guard.ssrf.test.ts

@@ -144,6 +144,18 @@ describe("fetchWithSsrFGuard hardening", () => {
     expect(fetchImpl).toHaveBeenCalledTimes(1);
   });
 
+  it("blocks all fetches when hostnameAllowlist is an explicit empty array", async () => {
+    const fetchImpl = vi.fn();
+    await expect(
+      fetchWithSsrFGuard({
+        url: "https://example.com/file.txt",
+        fetchImpl,
+        policy: { hostnameAllowlist: [] },
+      }),
+    ).rejects.toThrow(/allowlist/i);
+    expect(fetchImpl).not.toHaveBeenCalled();
+  });
+
   it("enforces hostname allowlist policies", async () => {
     const fetchImpl = vi.fn();
     await expect(

src/infra/net/ssrf.pinning.test.ts

@@ -108,6 +108,54 @@ describe("ssrf pinning", () => {
     ).toThrow("Pinned lookup requires at least one address for example.com");
   });
 
+  it("blocks all hostnames when hostnameAllowlist is an explicit empty array", async () => {
+    const lookup = vi.fn(async () => [
+      { address: "93.184.216.34", family: 4 },
+    ]) as unknown as LookupFn;
+
+    await expect(
+      resolvePinnedHostnameWithPolicy("example.com", {
+        lookupFn: lookup,
+        policy: { hostnameAllowlist: [] },
+      }),
+    ).rejects.toThrow(/allowlist/i);
+    expect(lookup).not.toHaveBeenCalled();
+  });
+
+  it("blocks all hostnames when hostnameAllowlist contains only bare wildcards", async () => {
+    const lookup = vi.fn(async () => [
+      { address: "93.184.216.34", family: 4 },
+    ]) as unknown as LookupFn;
+
+    await expect(
+      resolvePinnedHostnameWithPolicy("example.com", {
+        lookupFn: lookup,
+        policy: { hostnameAllowlist: ["*"] },
+      }),
+    ).rejects.toThrow(/allowlist/i);
+    expect(lookup).not.toHaveBeenCalled();
+
+    await expect(
+      resolvePinnedHostnameWithPolicy("example.com", {
+        lookupFn: lookup,
+        policy: { hostnameAllowlist: ["*."] },
+      }),
+    ).rejects.toThrow(/allowlist/i);
+  });
+
+  it("allows all hostnames when hostnameAllowlist is undefined (not configured)", async () => {
+    const lookup = vi.fn(async () => [
+      { address: "93.184.216.34", family: 4 },
+    ]) as unknown as LookupFn;
+
+    await expect(
+      resolvePinnedHostnameWithPolicy("example.com", {
+        lookupFn: lookup,
+        policy: {},
+      }),
+    ).resolves.toMatchObject({ hostname: "example.com" });
+  });
+
   it("enforces hostname allowlist when configured", async () => {
     const lookup = vi.fn(async () => [
       { address: "93.184.216.34", family: 4 },

src/infra/net/ssrf.ts

@@ -50,10 +50,18 @@ function normalizeHostnameSet(values?: string[]): Set<string> {
   return new Set(values.map((value) => normalizeHostname(value)).filter(Boolean));
 }
 
-function normalizeHostnameAllowlist(values?: string[]): string[] {
-  if (!values || values.length === 0) {
+// Returns `null` when not configured (allow all), `[]` when explicit empty (deny all).
+// Non-empty input where every entry is invalid (e.g. ["*"]) also returns [] (fail closed).
+function normalizeHostnameAllowlist(values?: string[]): string[] | null {
+  if (values === undefined || values === null) {
+    return null;
+  }
+  if (values.length === 0) {
     return [];
   }
+  // Bare "*" and "*." are not valid hostname patterns (they would only match a literal
+  // "*" hostname in isHostnameAllowedByPattern). Strip them so operators who mistakenly
+  // configure ["*"] get a clear deny-all rather than a silent pass-through.
   return Array.from(
     new Set(
       values
@@ -91,10 +99,13 @@ function isHostnameAllowedByPattern(hostname: string, pattern: string): boolean
   return hostname === pattern;
 }
 
-function matchesHostnameAllowlist(hostname: string, allowlist: string[]): boolean {
-  if (allowlist.length === 0) {
+function matchesHostnameAllowlist(hostname: string, allowlist: string[] | null): boolean {
+  if (allowlist === null) {
     return true;
   }
+  if (allowlist.length === 0) {
+    return false;
+  }
   return allowlist.some((pattern) => isHostnameAllowedByPattern(hostname, pattern));
 }