From 561bacd06a2dcd00550f6b6559c2934970211628 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Apr 2026 20:17:43 +0900 Subject: [PATCH] fix: harden synology chat TLS helper defaults --- CHANGELOG.md | 1 + extensions/synology-chat/src/client.test.ts | 34 +++++++++++++++++++++ extensions/synology-chat/src/client.ts | 8 ++--- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96121d30ec8..e4778df85bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ Docs: https://docs.openclaw.ai - Device pairing: reject rotating device tokens into roles that were never approved during pairing, and keep reconnect role checks bounded to the paired device's approved role set. (#60462) Thanks @eleqtrizit. - Mobile pairing/security: fail closed for internal `/pair` setup-code issuance, cleanup, and approval paths when gateway pairing scopes are missing, and keep approval-time requested-scope enforcement on the internal command path. (#55996) Thanks @coygeek. - Exec approvals/node host: forward prepared `system.run` approval plans on the async node invoke path so mutable script operands keep their approval-time binding and drift revalidation instead of dropping back to unbound execution. +- Synology Chat/security: default low-level HTTPS helper TLS verification to on so helper/API defaults match the shipped safe account default, and only explicit `allowInsecureSsl: true` opts out. ## 2026.4.2 diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index 524a5585ea0..67ecf8ae8e6 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -94,6 +94,20 @@ describe("sendMessage", () => { const callArgs = httpsRequest.mock.calls[0]; expect(callArgs[0]).toBe("https://nas.example.com/incoming"); }); + + it("verifies TLS by default", async () => { + mockSuccessResponse(); + await settleTimers(sendMessage("https://nas.example.com/incoming", "Hello")); + const httpsRequest = vi.mocked(https.request); + expect(httpsRequest.mock.calls[0]?.[1]).toMatchObject({ rejectUnauthorized: true }); + }); + + it("only disables TLS verification when explicitly requested", async () => { + mockSuccessResponse(); + await settleTimers(sendMessage("https://nas.example.com/incoming", "Hello", undefined, true)); + const httpsRequest = vi.mocked(https.request); + expect(httpsRequest.mock.calls[0]?.[1]).toMatchObject({ rejectUnauthorized: false }); + }); }); describe("sendFileUrl", () => { @@ -114,6 +128,15 @@ describe("sendFileUrl", () => { ); expect(result).toBe(false); }); + + it("verifies TLS by default", async () => { + mockSuccessResponse(); + await settleTimers( + sendFileUrl("https://nas.example.com/incoming", "https://example.com/file.png"), + ); + const httpsRequest = vi.mocked(https.request); + expect(httpsRequest.mock.calls[0]?.[1]).toMatchObject({ rejectUnauthorized: true }); + }); }); // Helper to mock the user_list API response for fetchChatUsers / resolveLegacyWebhookNameToChatUserId @@ -293,4 +316,15 @@ describe("fetchChatUsers", () => { expect(users).toEqual([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); }); + + it("verifies TLS by default for user_list lookups", async () => { + mockUserListResponse([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); + const freshUrl = + "https://fresh-nas.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22fresh%22"; + + await fetchChatUsers(freshUrl); + + const httpsGet = vi.mocked((https as any).get); + expect(httpsGet.mock.calls[0]?.[1]).toMatchObject({ rejectUnauthorized: true }); + }); }); diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index 3b32f439eeb..6d99cd7359b 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -82,7 +82,7 @@ export async function sendMessage( incomingUrl: string, text: string, userId?: string | number, - allowInsecureSsl = true, + allowInsecureSsl = false, ): Promise { // Synology Chat API requires user_ids (numeric) to specify the recipient // The @mention is optional but user_ids is mandatory @@ -123,7 +123,7 @@ export async function sendFileUrl( incomingUrl: string, fileUrl: string, userId?: string | number, - allowInsecureSsl = true, + allowInsecureSsl = false, ): Promise { const body = buildWebhookBody({ file_url: fileUrl }, userId); @@ -145,7 +145,7 @@ export async function sendFileUrl( */ export async function fetchChatUsers( incomingUrl: string, - allowInsecureSsl = true, + allowInsecureSsl = false, log?: { warn: (...args: unknown[]) => void }, ): Promise { const now = Date.now(); @@ -246,7 +246,7 @@ function parseNumericUserId(userId?: string | number): number | undefined { return Number.isNaN(numericId) ? undefined : numericId; } -function doPost(url: string, body: string, allowInsecureSsl = true): Promise { +function doPost(url: string, body: string, allowInsecureSsl = false): Promise { return new Promise((resolve, reject) => { let parsedUrl: URL; try {