From 3c9e3190476a6f090b7b94d0a68b9367ccb9ffac Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Thu, 19 Mar 2026 08:41:30 -0400 Subject: [PATCH] fix(ssrf): document bare-wildcard deny-all behavior and restore config in test --- src/gateway/openresponses-http.test.ts | 12 ++++++++++++ src/infra/net/ssrf.pinning.test.ts | 21 +++++++++++++++++++++ src/infra/net/ssrf.ts | 4 ++++ 3 files changed, 37 insertions(+) diff --git a/src/gateway/openresponses-http.test.ts b/src/gateway/openresponses-http.test.ts index 1dab33a3f57..50a0149b3fa 100644 --- a/src/gateway/openresponses-http.test.ts +++ b/src/gateway/openresponses-http.test.ts @@ -636,6 +636,11 @@ 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: { @@ -685,6 +690,13 @@ describe("OpenResponses HTTP API (e2e)", () => { 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); diff --git a/src/infra/net/ssrf.pinning.test.ts b/src/infra/net/ssrf.pinning.test.ts index df92d38b1b6..0162384be09 100644 --- a/src/infra/net/ssrf.pinning.test.ts +++ b/src/infra/net/ssrf.pinning.test.ts @@ -122,6 +122,27 @@ describe("ssrf pinning", () => { 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 }, diff --git a/src/infra/net/ssrf.ts b/src/infra/net/ssrf.ts index 83001d2a2da..dd044356a4f 100644 --- a/src/infra/net/ssrf.ts +++ b/src/infra/net/ssrf.ts @@ -51,6 +51,7 @@ function normalizeHostnameSet(values?: string[]): Set { } // 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; @@ -58,6 +59,9 @@ function normalizeHostnameAllowlist(values?: string[]): string[] | 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