diff --git a/extensions/bluebubbles/src/attachments.test.ts b/extensions/bluebubbles/src/attachments.test.ts index 48330ee9286..acf8e9be437 100644 --- a/extensions/bluebubbles/src/attachments.test.ts +++ b/extensions/bluebubbles/src/attachments.test.ts @@ -341,8 +341,12 @@ describe("downloadBlueBubblesAttachment", () => { }, }); + // Default-deny policy via the guard, NOT unguarded fetch. Aisle #68234 + // flagged the previous `undefined` fallback as a real SSRF bypass because + // `blueBubblesFetchWithTimeout` treats `undefined` as "skip the SSRF + // guard entirely", exactly when the user asked us to block private nets. const fetchMediaArgs = fetchRemoteMediaMock.mock.calls[0][0] as Record; - expect(fetchMediaArgs.ssrfPolicy).toBeUndefined(); + expect(fetchMediaArgs.ssrfPolicy).toEqual({}); }); it("allowlists public serverUrl hostname when allowPrivateNetwork is not set", async () => { diff --git a/extensions/bluebubbles/src/client.test.ts b/extensions/bluebubbles/src/client.test.ts index 2610f56eb2d..441fa648baa 100644 --- a/extensions/bluebubbles/src/client.test.ts +++ b/extensions/bluebubbles/src/client.test.ts @@ -115,24 +115,47 @@ describe("resolveBlueBubblesClientSsrfPolicy (3-mode policy)", () => { expect(result.trustedHostnameIsPrivate).toBe(false); }); - it("mode 3: private hostname + explicit opt-out → undefined (falls back to non-SSRF path)", () => { + it("mode 3: private hostname + explicit opt-out → {} (guarded default-deny, honors the opt-out) (aisle #68234)", () => { + // Previously returned `undefined`, which routed through the unguarded + // fetch fallback and effectively bypassed SSRF protection exactly when + // the user had explicitly asked to disable private-network access. const result = resolveBlueBubblesClientSsrfPolicy({ baseUrl: "http://192.168.1.50:1234", allowPrivateNetwork: false, allowPrivateNetworkConfig: false, }); - expect(result.ssrfPolicy).toBeUndefined(); + expect(result.ssrfPolicy).toEqual({}); expect(result.trustedHostnameIsPrivate).toBe(true); }); - it("mode 3: unparseable baseUrl → undefined policy", () => { + it("mode 3: unparseable baseUrl → {} (fail-safe guarded, never bypass)", () => { const result = resolveBlueBubblesClientSsrfPolicy({ baseUrl: "not a url", allowPrivateNetwork: false, }); - expect(result.ssrfPolicy).toBeUndefined(); + expect(result.ssrfPolicy).toEqual({}); expect(result.trustedHostname).toBeUndefined(); }); + + it("never returns undefined ssrfPolicy — every mode is guarded (aisle #68234 invariant)", () => { + // This invariant is what closes the SSRF bypass aisle flagged. Any + // refactor that reintroduces `ssrfPolicy: undefined` should break here. + const cases = [ + { baseUrl: "http://localhost:1234", allowPrivateNetwork: true }, + { baseUrl: "http://localhost:1234", allowPrivateNetwork: false }, + { + baseUrl: "http://192.168.1.50:1234", + allowPrivateNetwork: false, + allowPrivateNetworkConfig: false, + }, + { baseUrl: "https://bb.example.com", allowPrivateNetwork: false }, + { baseUrl: "not a url", allowPrivateNetwork: false }, + ]; + for (const c of cases) { + const result = resolveBlueBubblesClientSsrfPolicy(c); + expect(result.ssrfPolicy).toBeDefined(); + } + }); }); // --- Auth strategies ------------------------------------------------------- diff --git a/extensions/bluebubbles/src/client.ts b/extensions/bluebubbles/src/client.ts index 6f998f88fbb..2454f03f05d 100644 --- a/extensions/bluebubbles/src/client.ts +++ b/extensions/bluebubbles/src/client.ts @@ -87,7 +87,9 @@ function safeExtractHostname(baseUrl: string): string | undefined { } /** - * Resolve the BB client's SSRF policy at construction time. Three modes: + * Resolve the BB client's SSRF policy at construction time. Three modes — + * all of which go through `fetchWithSsrFGuard`; we never hand back a policy + * that skips the guard: * * 1. `{ allowPrivateNetwork: true }` — user explicitly opted in * (`network.dangerouslyAllowPrivateNetwork: true`). Private/loopback @@ -99,8 +101,11 @@ function safeExtractHostname(baseUrl: string): string | undefined { * that closes #34749, #57181, #59722, #60715 for self-hosted BB on * private/localhost addresses without requiring a full opt-in. * - * 3. `undefined` — no policy; use the non-SSRF fallback path. Applied only - * when we can't identify a trusted hostname. (#64105) + * 3. `{}` — guarded with the default-deny policy. Applied when we can't + * produce a valid allowlist (opt-out on a private hostname, or an + * unparseable baseUrl). Previously returned `undefined` and skipped + * the guard entirely, which was an SSRF bypass when a user explicitly + * opted out of private-network access. Aisle #68234 found this. * * Prior to this helper, the logic lived inline in `attachments.ts` and was * inconsistently replicated across 15+ callsites. Resolving once ensures @@ -111,7 +116,7 @@ export function resolveBlueBubblesClientSsrfPolicy(params: { allowPrivateNetwork: boolean; allowPrivateNetworkConfig?: boolean; }): { - ssrfPolicy: SsrFPolicy | undefined; + ssrfPolicy: SsrFPolicy; trustedHostname?: string; trustedHostnameIsPrivate: boolean; } { @@ -137,7 +142,9 @@ export function resolveBlueBubblesClientSsrfPolicy(params: { }; } - return { ssrfPolicy: undefined, trustedHostname, trustedHostnameIsPrivate }; + // Mode 3: default-deny guard. Honors an explicit opt-out on a private + // hostname and fails-safe on unparseable URLs. Never undefined. (aisle #68234) + return { ssrfPolicy: {}, trustedHostname, trustedHostnameIsPrivate }; } // --- Client ---------------------------------------------------------------- @@ -155,7 +162,7 @@ type ClientConstructorParams = { accountId: string; baseUrl: string; password: string; - ssrfPolicy: SsrFPolicy | undefined; + ssrfPolicy: SsrFPolicy; trustedHostname: string | undefined; trustedHostnameIsPrivate: boolean; defaultTimeoutMs: number; @@ -181,7 +188,7 @@ export class BlueBubblesClient { readonly trustedHostnameIsPrivate: boolean; private readonly password: string; - private readonly ssrfPolicy: SsrFPolicy | undefined; + private readonly ssrfPolicy: SsrFPolicy; private readonly defaultTimeoutMs: number; private readonly authStrategy: BlueBubblesAuthStrategy; @@ -200,7 +207,7 @@ export class BlueBubblesClient { * Read the resolved SSRF policy for this client. Exposed primarily for tests * and diagnostics; production code should never need to inspect it. */ - getSsrfPolicy(): SsrFPolicy | undefined { + getSsrfPolicy(): SsrFPolicy { return this.ssrfPolicy; }