mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 12:00:44 +00:00
bluebubbles: close SSRF bypass when user opts out of private network (aisle #68234 HIGH)
Aisle security analysis flagged `resolveBlueBubblesClientSsrfPolicy` mode 3
as an SSRF bypass: it returned `ssrfPolicy: undefined`, which
`blueBubblesFetchWithTimeout` treats as a signal to skip
`fetchWithSsrFGuard` and call `fetch()` directly.
Mode 3 fires in two cases:
A. Private hostname + user explicitly opted out
(`network.dangerouslyAllowPrivateNetwork: false`). The user asked
us to block private networks and we instead sent the request via
the unguarded fallback path — a real bypass.
B. Unparseable baseUrl. Would route through the unguarded path
(cosmetic in practice since the fetch would fail elsewhere, but
still wrong).
Fix: return `ssrfPolicy: {}` (default-deny guarded policy) instead of
`undefined`. All three modes now go through `fetchWithSsrFGuard`:
1. `{ allowPrivateNetwork: true }` — user opted in
2. `{ allowedHostnames: [host] }` — narrow allowlist for trusted host
3. `{}` — default-deny, honors opt-out
Tightened `ssrfPolicy` field/param types from `SsrFPolicy | undefined`
to `SsrFPolicy` so the bypass cannot be reintroduced by a future caller
handing back `undefined`.
Added a mode-3 invariant regression test that walks every resolution
case and asserts `ssrfPolicy` is always defined (caught any future
`undefined` reintroduction). Updated `attachments.test.ts` case that
was asserting the old (buggy) `undefined` behavior.
Existing-user impact: only users with
`dangerouslyAllowPrivateNetwork: false` set on a private-address BB
server see a behavior change (they now get a guarded refusal instead of
an accidental success). That's the correct posture per their config.
This commit is contained in:
@@ -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<string, unknown>;
|
||||
expect(fetchMediaArgs.ssrfPolicy).toBeUndefined();
|
||||
expect(fetchMediaArgs.ssrfPolicy).toEqual({});
|
||||
});
|
||||
|
||||
it("allowlists public serverUrl hostname when allowPrivateNetwork is not set", async () => {
|
||||
|
||||
@@ -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 -------------------------------------------------------
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user