diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f31dee6058..2c22023db69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai - Auth/commands: require owner identity (an owner-candidate match or internal `operator.admin`) for owner-enforced commands instead of treating wildcard channel `allowFrom` or empty owner-candidate lists as sufficient, so non-owner senders can no longer reach owner-only commands through a permissive fallback when `enforceOwnerForCommands=true` and `commands.ownerAllowFrom` is unset. (#69774) Thanks @drobison00. - Control UI/CSP: tighten `img-src` to `'self' data:` only, and make Control UI avatar helpers drop remote `http(s)` and protocol-relative URLs so the UI falls back to the built-in logo/badge instead of issuing arbitrary remote image fetches. Same-origin avatar routes (relative paths) and `data:image/...` avatars still render. (#69773) - CLI/channels: keep `status`, `health`, `channels list`, and `channels status` on read-only channel metadata when Telegram, Slack, Discord, or third-party channel plugins are configured, avoiding full bundled plugin runtime imports on those cold paths. Fixes #69042. (#69479) Thanks @gumadeiras. +- Synology Chat: validate outbound webhook `file_url` values against the shared SSRF policy before forwarding to the NAS, rejecting malformed URLs, non-`http(s)` schemes, and private/blocked network targets so the NAS cannot be used as a confused deputy to fetch internal addresses. (#69784) Thanks @eleqtrizit. ## 2026.4.20 diff --git a/docs/channels/synology-chat.md b/docs/channels/synology-chat.md index a7a8774f7d3..4194539dee8 100644 --- a/docs/channels/synology-chat.md +++ b/docs/channels/synology-chat.md @@ -113,6 +113,7 @@ openclaw message send --channel synology-chat --target synology-chat:123456 --te ``` Media sends are supported by URL-based file delivery. +Outbound file URLs must use `http` or `https`, and private or otherwise blocked network targets are rejected before OpenClaw forwards the URL to the NAS webhook. ## Multi-account diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index ee4bbfc3d93..8392c13aaed 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -2,6 +2,10 @@ import { EventEmitter } from "node:events"; import type { ClientRequest, IncomingMessage, RequestOptions } from "node:http"; import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vitest"; +const ssrfMocks = { + resolvePinnedHostnameWithPolicy: vi.fn(), +}; + // Mock http and https modules before importing the client vi.mock("node:https", () => { const httpsRequest = vi.fn(); @@ -16,6 +20,11 @@ vi.mock("node:http", () => { return { default: { request: httpRequest, get: httpGet }, request: httpRequest, get: httpGet }; }); +vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({ + formatErrorMessage: (err: unknown) => (err instanceof Error ? err.message : String(err)), + resolvePinnedHostnameWithPolicy: ssrfMocks.resolvePinnedHostnameWithPolicy, +})); + const https = await import("node:https"); let fakeNowMs = 1_700_000_000_000; let sendMessage: typeof import("./client.js").sendMessage; @@ -33,7 +42,7 @@ type MockRequestHandler = ( function createMockResponseEmitter(statusCode: number): IncomingMessage { const res = new EventEmitter() as Partial; res.statusCode = statusCode; - return res as IncomingMessage; + return res as unknown as IncomingMessage; } function createMockRequestEmitter(): ClientRequest { @@ -41,7 +50,7 @@ function createMockRequestEmitter(): ClientRequest { req.write = vi.fn() as ClientRequest["write"]; req.end = vi.fn() as ClientRequest["end"]; req.destroy = vi.fn() as ClientRequest["destroy"]; - return req as ClientRequest; + return req as unknown as ClientRequest; } async function settleTimers(promise: Promise): Promise { @@ -83,6 +92,10 @@ function installFakeTimerHarness() { vi.useFakeTimers(); fakeNowMs += 10_000; vi.setSystemTime(fakeNowMs); + ssrfMocks.resolvePinnedHostnameWithPolicy.mockResolvedValue({ + hostname: "example.com", + addresses: ["93.184.216.34"], + }); }); afterEach(() => { @@ -156,6 +169,51 @@ describe("sendFileUrl", () => { const httpsRequest = vi.mocked(https.request); expect(httpsRequest.mock.calls[0]?.[1]).toMatchObject({ rejectUnauthorized: true }); }); + + it("respects the shared send interval before posting a file URL", async () => { + mockSuccessResponse(); + await settleTimers(sendMessage("https://nas.example.com/incoming", "hello")); + vi.mocked(https.request).mockClear(); + + const promise = sendFileUrl("https://nas.example.com/incoming", "https://example.com/file.png"); + await Promise.resolve(); + expect(vi.mocked(https.request)).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(499); + expect(vi.mocked(https.request)).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + await promise; + expect(vi.mocked(https.request)).toHaveBeenCalledTimes(1); + }); + + it("rejects malformed file URLs before making a request", async () => { + const result = await settleTimers(sendFileUrl("https://nas.example.com/incoming", "not-a-url")); + expect(result).toBe(false); + expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled(); + expect(vi.mocked(https.request)).not.toHaveBeenCalled(); + }); + + it("rejects non-http file URLs before making a request", async () => { + const result = await settleTimers( + sendFileUrl("https://nas.example.com/incoming", "file:///tmp/secret.txt"), + ); + expect(result).toBe(false); + expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled(); + expect(vi.mocked(https.request)).not.toHaveBeenCalled(); + }); + + it("rejects SSRF-blocked hosts before making a request", async () => { + ssrfMocks.resolvePinnedHostnameWithPolicy.mockRejectedValueOnce( + new Error("Blocked private network target"), + ); + const result = await settleTimers( + sendFileUrl("https://nas.example.com/incoming", "http://169.254.169.254/latest/meta-data"), + ); + expect(result).toBe(false); + expect(ssrfMocks.resolvePinnedHostnameWithPolicy).toHaveBeenCalledWith("169.254.169.254"); + expect(vi.mocked(https.request)).not.toHaveBeenCalled(); + }); }); // Helper to mock the user_list API response for fetchChatUsers / resolveLegacyWebhookNameToChatUserId diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index 0dc8af611b4..86b137e6037 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -6,6 +6,10 @@ import * as http from "node:http"; import * as https from "node:https"; import { safeParseJsonWithSchema, safeParseWithSchema } from "openclaw/plugin-sdk/extension-shared"; +import { + formatErrorMessage, + resolvePinnedHostnameWithPolicy, +} from "openclaw/plugin-sdk/ssrf-runtime"; import { z } from "zod"; const MIN_SEND_INTERVAL_MS = 500; @@ -131,9 +135,16 @@ export async function sendFileUrl( userId?: string | number, allowInsecureSsl = false, ): Promise { - const body = buildWebhookBody({ file_url: fileUrl }, userId); - try { + const safeFileUrl = await assertSafeWebhookFileUrl(fileUrl); + const body = buildWebhookBody({ file_url: safeFileUrl }, userId); + + const now = Date.now(); + const elapsed = now - lastSendTime; + if (elapsed < MIN_SEND_INTERVAL_MS) { + await sleep(MIN_SEND_INTERVAL_MS - elapsed); + } + const ok = await doPost(incomingUrl, body, allowInsecureSsl); lastSendTime = Date.now(); return ok; @@ -209,6 +220,22 @@ export async function fetchChatUsers( }); } +async function assertSafeWebhookFileUrl(fileUrl: string): Promise { + let parsed: URL; + try { + parsed = new URL(fileUrl); + } catch (err) { + throw new Error(`Invalid Synology Chat file URL: ${formatErrorMessage(err)}`, { cause: err }); + } + + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + throw new Error("Synology Chat file URL must use HTTP or HTTPS"); + } + + await resolvePinnedHostnameWithPolicy(parsed.hostname); + return parsed.toString(); +} + /** * Resolve a mutable webhook username/nickname to the correct Chat API user_id. *