diff --git a/CHANGELOG.md b/CHANGELOG.md index e12722932..038539822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Docs: https://docs.openclaw.ai - Synology Chat/webhook compatibility: accept JSON and alias payload fields, allow token resolution from body/query/header sources, and ACK webhook requests with `204` to avoid persistent `Processing...` states in Synology Chat clients. (#26635) Thanks @memphislee09-source. - Synology Chat/webhook ingress hardening: enforce bounded body reads (size + timeout) via shared request-body guards to prevent unauthenticated slow-body hangs before token validation. (#25831) Thanks @bmendonca3. +- Synology Chat/reply delivery: resolve webhook usernames to Chat API `user_id` values for outbound chatbot replies, avoiding mismatches between webhook user IDs and `method=chatbot` recipient IDs in multi-account setups. (#23709) Thanks @druide67. - Auto-reply/followup queue: avoid stale callback reuse across idle-window restarts by caching the followup runner only when a drain actually starts, preserving enqueue ordering after empty-finalize paths. (#31902) Thanks @Lanfei. - Gateway/Heartbeat model reload: treat `models.*` and `agents.defaults.model` config updates as heartbeat hot-reload triggers so heartbeat picks up model changes without a full gateway restart. (#32046) Thanks @stakeswky. - Slack/inbound debounce routing: isolate top-level non-DM message debounce keys by message timestamp to avoid cross-thread collisions, preserve DM batching, and flush pending top-level buffers before immediate non-debounce follow-ups to keep ordering stable. (#31951) Thanks @scoootscooob. diff --git a/extensions/synology-chat/src/channel.integration.test.ts b/extensions/synology-chat/src/channel.integration.test.ts index e901da43d..dd2b6273b 100644 --- a/extensions/synology-chat/src/channel.integration.test.ts +++ b/extensions/synology-chat/src/channel.integration.test.ts @@ -102,6 +102,8 @@ describe("Synology channel wiring integration", () => { expect(res._body).toContain("not authorized"); expect(dispatchReplyWithBufferedBlockDispatcher).not.toHaveBeenCalled(); - started.stop(); + if (started && typeof started === "object" && "stop" in started) { + (started as { stop: () => void }).stop(); + } }); }); diff --git a/extensions/synology-chat/src/channel.ts b/extensions/synology-chat/src/channel.ts index 2ce7e1c98..61fbc7450 100644 --- a/extensions/synology-chat/src/channel.ts +++ b/extensions/synology-chat/src/channel.ts @@ -243,6 +243,10 @@ export function createSynologyChatPlugin() { const rt = getSynologyRuntime(); const currentCfg = await rt.config.loadConfig(); + // The Chat API user_id (for sending) may differ from the webhook + // user_id (used for sessions/pairing). Use chatUserId for API calls. + const sendUserId = msg.chatUserId ?? msg.from; + // Build MsgContext using SDK's finalizeInboundContext for proper normalization const msgCtx = rt.channel.reply.finalizeInboundContext({ Body: msg.body, @@ -275,7 +279,7 @@ export function createSynologyChatPlugin() { await sendMessage( account.incomingUrl, text, - msg.from, + sendUserId, account.allowInsecureSsl, ); } diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index edb483069..ef5ff06be 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -4,16 +4,18 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; // Mock http and https modules before importing the client vi.mock("node:https", () => { const mockRequest = vi.fn(); - return { default: { request: mockRequest }, request: mockRequest }; + const mockGet = vi.fn(); + return { default: { request: mockRequest, get: mockGet }, request: mockRequest, get: mockGet }; }); vi.mock("node:http", () => { const mockRequest = vi.fn(); - return { default: { request: mockRequest }, request: mockRequest }; + const mockGet = vi.fn(); + return { default: { request: mockRequest, get: mockGet }, request: mockRequest, get: mockGet }; }); // Import after mocks are set up -const { sendMessage, sendFileUrl } = await import("./client.js"); +const { sendMessage, sendFileUrl, fetchChatUsers, resolveChatUserId } = await import("./client.js"); const https = await import("node:https"); let fakeNowMs = 1_700_000_000_000; @@ -111,3 +113,122 @@ describe("sendFileUrl", () => { expect(result).toBe(false); }); }); + +// Helper to mock the user_list API response for fetchChatUsers / resolveChatUserId +function mockUserListResponse( + users: Array<{ user_id: number; username: string; nickname: string }>, +) { + const httpsGet = vi.mocked((https as any).get); + httpsGet.mockImplementation((_url: any, _opts: any, callback: any) => { + const res = new EventEmitter() as any; + res.statusCode = 200; + process.nextTick(() => { + callback(res); + res.emit("data", Buffer.from(JSON.stringify({ success: true, data: { users } }))); + res.emit("end"); + }); + const req = new EventEmitter() as any; + req.destroy = vi.fn(); + return req; + }); +} + +function mockUserListResponseOnce( + users: Array<{ user_id: number; username: string; nickname: string }>, +) { + const httpsGet = vi.mocked((https as any).get); + httpsGet.mockImplementationOnce((_url: any, _opts: any, callback: any) => { + const res = new EventEmitter() as any; + res.statusCode = 200; + process.nextTick(() => { + callback(res); + res.emit("data", Buffer.from(JSON.stringify({ success: true, data: { users } }))); + res.emit("end"); + }); + const req = new EventEmitter() as any; + req.destroy = vi.fn(); + return req; + }); +} + +describe("resolveChatUserId", () => { + const baseUrl = + "https://nas.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22test%22"; + const baseUrl2 = + "https://nas2.example.com/webapi/entry.cgi?api=SYNO.Chat.External&method=chatbot&version=2&token=%22test-2%22"; + + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + // Advance time to invalidate any cached user list from previous tests + fakeNowMs += 10 * 60 * 1000; + vi.setSystemTime(fakeNowMs); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("resolves user by nickname (webhook username = Chat nickname)", async () => { + mockUserListResponse([ + { user_id: 4, username: "jmn67", nickname: "jmn" }, + { user_id: 7, username: "she67", nickname: "sarah" }, + ]); + const result = await resolveChatUserId(baseUrl, "jmn"); + expect(result).toBe(4); + }); + + it("resolves user by username when nickname does not match", async () => { + mockUserListResponse([ + { user_id: 4, username: "jmn67", nickname: "" }, + { user_id: 7, username: "she67", nickname: "sarah" }, + ]); + // Advance time to invalidate cache + fakeNowMs += 10 * 60 * 1000; + vi.setSystemTime(fakeNowMs); + const result = await resolveChatUserId(baseUrl, "jmn67"); + expect(result).toBe(4); + }); + + it("is case-insensitive", async () => { + mockUserListResponse([{ user_id: 4, username: "JMN67", nickname: "JMN" }]); + fakeNowMs += 10 * 60 * 1000; + vi.setSystemTime(fakeNowMs); + const result = await resolveChatUserId(baseUrl, "jmn"); + expect(result).toBe(4); + }); + + it("returns undefined when user is not found", async () => { + mockUserListResponse([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); + fakeNowMs += 10 * 60 * 1000; + vi.setSystemTime(fakeNowMs); + const result = await resolveChatUserId(baseUrl, "unknown_user"); + expect(result).toBeUndefined(); + }); + + it("uses method=user_list instead of method=chatbot in the API URL", async () => { + mockUserListResponse([]); + fakeNowMs += 10 * 60 * 1000; + vi.setSystemTime(fakeNowMs); + await resolveChatUserId(baseUrl, "anyone"); + const httpsGet = vi.mocked((https as any).get); + expect(httpsGet).toHaveBeenCalledWith( + expect.stringContaining("method=user_list"), + expect.any(Object), + expect.any(Function), + ); + }); + + it("keeps user cache scoped per incoming URL", async () => { + mockUserListResponseOnce([{ user_id: 4, username: "jmn67", nickname: "jmn" }]); + mockUserListResponseOnce([{ user_id: 9, username: "jmn67", nickname: "jmn" }]); + + const result1 = await resolveChatUserId(baseUrl, "jmn"); + const result2 = await resolveChatUserId(baseUrl2, "jmn"); + + expect(result1).toBe(4); + expect(result2).toBe(9); + const httpsGet = vi.mocked((https as any).get); + expect(httpsGet).toHaveBeenCalledTimes(2); + }); +}); diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index 316a38799..95240e556 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -9,6 +9,28 @@ import * as https from "node:https"; const MIN_SEND_INTERVAL_MS = 500; let lastSendTime = 0; +// --- Chat user_id resolution --- +// Synology Chat uses two different user_id spaces: +// - Outgoing webhook user_id: per-integration sequential ID (e.g. 1) +// - Chat API user_id: global internal ID (e.g. 4) +// The chatbot API (method=chatbot) requires the Chat API user_id in the +// user_ids array. We resolve via the user_list API and cache the result. + +interface ChatUser { + user_id: number; + username: string; + nickname: string; +} + +type ChatUserCacheEntry = { + users: ChatUser[]; + cachedAt: number; +}; + +// Cache user lists per bot endpoint to avoid cross-account bleed. +const chatUserCache = new Map(); +const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes + /** * Send a text message to Synology Chat via the incoming webhook. * @@ -92,6 +114,107 @@ export async function sendFileUrl( } } +/** + * Fetch the list of Chat users visible to this bot via the user_list API. + * Results are cached for CACHE_TTL_MS to avoid excessive API calls. + * + * The user_list endpoint uses the same base URL as the chatbot API but + * with method=user_list instead of method=chatbot. + */ +export async function fetchChatUsers( + incomingUrl: string, + allowInsecureSsl = true, + log?: { warn: (...args: unknown[]) => void }, +): Promise { + const now = Date.now(); + const listUrl = incomingUrl.replace(/method=\w+/, "method=user_list"); + const cached = chatUserCache.get(listUrl); + if (cached && now - cached.cachedAt < CACHE_TTL_MS) { + return cached.users; + } + + return new Promise((resolve) => { + let parsedUrl: URL; + try { + parsedUrl = new URL(listUrl); + } catch { + log?.warn("fetchChatUsers: invalid user_list URL, using cached data"); + resolve(cached?.users ?? []); + return; + } + const transport = parsedUrl.protocol === "https:" ? https : http; + + transport + .get(listUrl, { rejectUnauthorized: !allowInsecureSsl } as any, (res) => { + let data = ""; + res.on("data", (c: Buffer) => { + data += c.toString(); + }); + res.on("end", () => { + try { + const result = JSON.parse(data); + if (result.success && result.data?.users) { + const users = result.data.users.map((u: any) => ({ + user_id: u.user_id, + username: u.username || "", + nickname: u.nickname || "", + })); + chatUserCache.set(listUrl, { + users, + cachedAt: now, + }); + resolve(users); + } else { + log?.warn( + `fetchChatUsers: API returned success=${result.success}, using cached data`, + ); + resolve(cached?.users ?? []); + } + } catch { + log?.warn("fetchChatUsers: failed to parse user_list response"); + resolve(cached?.users ?? []); + } + }); + }) + .on("error", (err) => { + log?.warn(`fetchChatUsers: HTTP error — ${err instanceof Error ? err.message : err}`); + resolve(cached?.users ?? []); + }); + }); +} + +/** + * Resolve a webhook username to the correct Chat API user_id. + * + * Synology Chat outgoing webhooks send a user_id that may NOT match the + * Chat-internal user_id needed by the chatbot API (method=chatbot). + * The webhook's "username" field corresponds to the Chat user's "nickname". + * + * @param incomingUrl - Bot incoming webhook URL (used to derive user_list URL) + * @param webhookUsername - The username from the outgoing webhook payload + * @param allowInsecureSsl - Skip TLS verification + * @returns The correct Chat user_id, or undefined if not found + */ +export async function resolveChatUserId( + incomingUrl: string, + webhookUsername: string, + allowInsecureSsl = true, + log?: { warn: (...args: unknown[]) => void }, +): Promise { + const users = await fetchChatUsers(incomingUrl, allowInsecureSsl, log); + const lower = webhookUsername.toLowerCase(); + + // Match by nickname first (webhook "username" field = Chat "nickname") + const byNickname = users.find((u) => u.nickname.toLowerCase() === lower); + if (byNickname) return byNickname.user_id; + + // Then by username + const byUsername = users.find((u) => u.username.toLowerCase() === lower); + if (byUsername) return byUsername.user_id; + + return undefined; +} + function doPost(url: string, body: string, allowInsecureSsl = true): Promise { return new Promise((resolve, reject) => { let parsedUrl: URL; diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index ee87e3698..2f6bd8778 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -7,9 +7,10 @@ import { createWebhookHandler, } from "./webhook-handler.js"; -// Mock sendMessage to prevent real HTTP calls +// Mock sendMessage and resolveChatUserId to prevent real HTTP calls vi.mock("./client.js", () => ({ sendMessage: vi.fn().mockResolvedValue(true), + resolveChatUserId: vi.fn().mockResolvedValue(undefined), })); function makeAccount( diff --git a/extensions/synology-chat/src/webhook-handler.ts b/extensions/synology-chat/src/webhook-handler.ts index a0bcb6412..197ec2cee 100644 --- a/extensions/synology-chat/src/webhook-handler.ts +++ b/extensions/synology-chat/src/webhook-handler.ts @@ -10,7 +10,7 @@ import { readRequestBodyWithLimit, requestBodyErrorToText, } from "openclaw/plugin-sdk"; -import { sendMessage } from "./client.js"; +import { sendMessage, resolveChatUserId } from "./client.js"; import { validateToken, authorizeUserForDm, sanitizeInput, RateLimiter } from "./security.js"; import type { SynologyWebhookPayload, ResolvedSynologyChatAccount } from "./types.js"; @@ -225,6 +225,8 @@ export interface WebhookHandlerDeps { chatType: string; sessionKey: string; accountId: string; + /** Chat API user_id for sending replies (may differ from webhook user_id) */ + chatUserId?: string; }) => Promise; log?: { info: (...args: unknown[]) => void; @@ -330,8 +332,29 @@ export function createWebhookHandler(deps: WebhookHandlerDeps) { // ACK immediately so Synology Chat won't remain in "Processing..." respondNoContent(res); + // Default to webhook user_id; may be replaced with Chat API user_id below. + let replyUserId = payload.user_id; + // Deliver to agent asynchronously (with 120s timeout to match nginx proxy_read_timeout) try { + // Resolve the Chat-internal user_id for sending replies. + // Synology Chat outgoing webhooks use a per-integration user_id that may + // differ from the global Chat API user_id required by method=chatbot. + // We resolve via the user_list API, matching by nickname/username. + const chatUserId = await resolveChatUserId( + account.incomingUrl, + payload.username, + account.allowInsecureSsl, + log, + ); + if (chatUserId !== undefined) { + replyUserId = String(chatUserId); + } else { + log?.warn( + `Could not resolve Chat API user_id for "${payload.username}" — falling back to webhook user_id ${payload.user_id}. Reply delivery may fail.`, + ); + } + const sessionKey = `synology-chat-${payload.user_id}`; const deliverPromise = deliver({ body: cleanText, @@ -341,6 +364,7 @@ export function createWebhookHandler(deps: WebhookHandlerDeps) { chatType: "direct", sessionKey, accountId: account.accountId, + chatUserId: replyUserId, }); const timeoutPromise = new Promise((_, reject) => @@ -349,11 +373,11 @@ export function createWebhookHandler(deps: WebhookHandlerDeps) { const reply = await Promise.race([deliverPromise, timeoutPromise]); - // Send reply back to Synology Chat + // Send reply back to Synology Chat using the resolved Chat user_id if (reply) { - await sendMessage(account.incomingUrl, reply, payload.user_id, account.allowInsecureSsl); + await sendMessage(account.incomingUrl, reply, replyUserId, account.allowInsecureSsl); const replyPreview = reply.length > 100 ? `${reply.slice(0, 100)}...` : reply; - log?.info(`Reply sent to ${payload.username} (${payload.user_id}): ${replyPreview}`); + log?.info(`Reply sent to ${payload.username} (${replyUserId}): ${replyPreview}`); } } catch (err) { const errMsg = err instanceof Error ? `${err.message}\n${err.stack}` : String(err); @@ -361,7 +385,7 @@ export function createWebhookHandler(deps: WebhookHandlerDeps) { await sendMessage( account.incomingUrl, "Sorry, an error occurred while processing your message.", - payload.user_id, + replyUserId, account.allowInsecureSsl, ); }