mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(synology-chat): stop fabricating send message ids (#110770)
* fix(synology-chat): stop fabricating send message ids
The incoming-webhook send contract acks with a bare boolean and carries
no platform message id, but the adapter returned a synthetic
`sc-${Date.now()}` id and stamped it into the delivery receipt. Return
the established empty-id sentinel and an empty receipt instead, matching
the honest no-platform-id shape used by sibling channels (qqbot,
googlechat) and the plugin-sdk empty-id contract.
Co-Authored-By: Claude <noreply@anthropic.com>
* test(synology-chat): verify truthful webhook receipts
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
committed by
GitHub
parent
b11ed1877c
commit
77e9ee092f
@@ -475,8 +475,9 @@ describe("createSynologyChatPlugin", () => {
|
||||
text: "hello",
|
||||
to: "user1",
|
||||
});
|
||||
expect(result?.receipt.parts[0]?.kind).toBe("text");
|
||||
expect(result?.receipt.platformMessageIds).toHaveLength(1);
|
||||
expect(result?.messageId).toBe("");
|
||||
expect(result?.receipt.platformMessageIds).toHaveLength(0);
|
||||
expect(result?.receipt.parts).toHaveLength(0);
|
||||
},
|
||||
media: async () => {
|
||||
const result = await plugin.message.send?.media?.({
|
||||
@@ -485,8 +486,9 @@ describe("createSynologyChatPlugin", () => {
|
||||
mediaUrl: "https://example.com/img.png",
|
||||
to: "user1",
|
||||
});
|
||||
expect(result?.receipt.parts[0]?.kind).toBe("media");
|
||||
expect(result?.receipt.platformMessageIds).toHaveLength(1);
|
||||
expect(result?.messageId).toBe("");
|
||||
expect(result?.receipt.platformMessageIds).toHaveLength(0);
|
||||
expect(result?.receipt.parts).toHaveLength(0);
|
||||
},
|
||||
messageSendingHooks: () => {
|
||||
expect(plugin.message.durableFinal?.capabilities?.messageSendingHooks).toBe(true);
|
||||
@@ -517,7 +519,7 @@ describe("createSynologyChatPlugin", () => {
|
||||
).rejects.toThrow("not configured");
|
||||
});
|
||||
|
||||
it("sendText returns OutboundDeliveryResult on success", async () => {
|
||||
it("sendText returns an honest empty-id result on success", async () => {
|
||||
const plugin = synologyChatPlugin;
|
||||
const malformedLink = `[${"\\".repeat(32)}`;
|
||||
const result = await plugin.outbound.sendText({
|
||||
@@ -536,9 +538,11 @@ describe("createSynologyChatPlugin", () => {
|
||||
});
|
||||
expect(result.channel).toBe("synology-chat");
|
||||
expect(result.chatId).toBe("user1");
|
||||
expect(result.messageId).toMatch(/^sc-\d+$/);
|
||||
expect(result.receipt.primaryPlatformMessageId).toBe(result.messageId);
|
||||
expect(result.receipt.parts[0]?.kind).toBe("text");
|
||||
expect(result.messageId).toBe("");
|
||||
expect(result.receipt.primaryPlatformMessageId).toBeUndefined();
|
||||
expect(result.receipt.platformMessageIds).toHaveLength(0);
|
||||
expect(result.receipt.parts).toHaveLength(0);
|
||||
expect(result.receipt.threadId).toBe("user1");
|
||||
expect(mockSendMessage).toHaveBeenLastCalledWith(
|
||||
"https://nas/incoming",
|
||||
`**Read** <https://example.com/a_(b)|the docs> <https://example.com|titled> \`[literal](https://example.com)\` \\[escaped](https://example.com) [x > y](https://example.com) [bad](<https://example.com) [bad title](https://example.com "oops')  ${malformedLink}`,
|
||||
@@ -547,6 +551,38 @@ describe("createSynologyChatPlugin", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("sendMedia returns an honest empty-id result on success", async () => {
|
||||
const plugin = synologyChatPlugin;
|
||||
const result = await plugin.outbound.sendMedia({
|
||||
cfg: {
|
||||
channels: {
|
||||
"synology-chat": {
|
||||
enabled: true,
|
||||
token: "t",
|
||||
incomingUrl: "https://nas/incoming",
|
||||
allowInsecureSsl: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
mediaUrl: "https://example.com/img.png",
|
||||
to: "user1",
|
||||
});
|
||||
|
||||
expect(result.channel).toBe("synology-chat");
|
||||
expect(result.chatId).toBe("user1");
|
||||
expect(result.messageId).toBe("");
|
||||
expect(result.receipt.primaryPlatformMessageId).toBeUndefined();
|
||||
expect(result.receipt.platformMessageIds).toHaveLength(0);
|
||||
expect(result.receipt.parts).toHaveLength(0);
|
||||
expect(result.receipt.threadId).toBe("user1");
|
||||
expect(mockSendFileUrl).toHaveBeenLastCalledWith(
|
||||
"https://nas/incoming",
|
||||
"https://example.com/img.png",
|
||||
"user1",
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it("sendMedia throws when missing incomingUrl", async () => {
|
||||
const plugin = synologyChatPlugin;
|
||||
await expect(
|
||||
|
||||
@@ -240,23 +240,17 @@ function normalizeSynologyChatTarget(target: string): string | undefined {
|
||||
}
|
||||
|
||||
function createSynologyChatSendResult(params: {
|
||||
messageId: string;
|
||||
chatId: string;
|
||||
kind: MessageReceiptPartKind;
|
||||
}): SynologyChatOutboundResult {
|
||||
return {
|
||||
channel: CHANNEL_ID,
|
||||
messageId: params.messageId,
|
||||
// The webhook acknowledges delivery without returning a platform message id.
|
||||
// Keep the empty receipt so a chat id cannot become a fabricated message id.
|
||||
messageId: "",
|
||||
chatId: params.chatId,
|
||||
receipt: createMessageReceiptFromOutboundResults({
|
||||
results: [
|
||||
{
|
||||
channel: CHANNEL_ID,
|
||||
messageId: params.messageId,
|
||||
chatId: params.chatId,
|
||||
conversationId: params.chatId,
|
||||
},
|
||||
],
|
||||
results: [],
|
||||
threadId: params.chatId,
|
||||
kind: params.kind,
|
||||
}),
|
||||
@@ -281,7 +275,6 @@ async function sendSynologyChatText(
|
||||
throw new Error("Failed to send message to Synology Chat");
|
||||
}
|
||||
return createSynologyChatSendResult({
|
||||
messageId: `sc-${Date.now()}`,
|
||||
chatId: ctx.to,
|
||||
kind: "text",
|
||||
});
|
||||
@@ -297,7 +290,6 @@ async function sendSynologyChatMedia(
|
||||
throw new Error("Failed to send media to Synology Chat");
|
||||
}
|
||||
return createSynologyChatSendResult({
|
||||
messageId: `sc-${Date.now()}`,
|
||||
chatId: ctx.to,
|
||||
kind: "media",
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { once } from "node:events";
|
||||
import * as http from "node:http";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { synologyChatPlugin } from "./channel.js";
|
||||
import { resolveLegacyWebhookNameToChatUserId, sendMessage } from "./client.js";
|
||||
|
||||
const USER_LIST_RESPONSE_MAX_BYTES = 1 * 1024 * 1024;
|
||||
@@ -31,6 +32,162 @@ describe("Synology Chat user_list loopback", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("delivers authenticated text and media without inventing platform message ids", async () => {
|
||||
const receivedPayloads: Array<Record<string, unknown>> = [];
|
||||
const port = await listenLoopback((req, res) => {
|
||||
const requestUrl = new URL(req.url ?? "/", "http://127.0.0.1");
|
||||
if (
|
||||
req.method !== "POST" ||
|
||||
requestUrl.pathname !== "/webapi/entry.cgi" ||
|
||||
requestUrl.searchParams.get("api") !== "SYNO.Chat.External" ||
|
||||
requestUrl.searchParams.get("method") !== "chatbot" ||
|
||||
requestUrl.searchParams.get("version") !== "2" ||
|
||||
requestUrl.searchParams.get("token") !== "synology-loopback-proof"
|
||||
) {
|
||||
res.writeHead(403, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ success: false }));
|
||||
return;
|
||||
}
|
||||
|
||||
let formBody = "";
|
||||
req.setEncoding("utf8");
|
||||
req.on("data", (chunk: string) => {
|
||||
formBody += chunk;
|
||||
});
|
||||
req.on("end", () => {
|
||||
const payload = new URLSearchParams(formBody).get("payload");
|
||||
if (!payload) {
|
||||
res.writeHead(400, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ success: false }));
|
||||
return;
|
||||
}
|
||||
receivedPayloads.push(JSON.parse(payload) as Record<string, unknown>);
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ success: true }));
|
||||
});
|
||||
});
|
||||
const incomingUrl =
|
||||
`http://127.0.0.1:${port}/webapi/entry.cgi?` +
|
||||
"api=SYNO.Chat.External&method=chatbot&version=2&token=synology-loopback-proof";
|
||||
const cfg = {
|
||||
channels: {
|
||||
"synology-chat": {
|
||||
enabled: true,
|
||||
token: "synology-loopback-proof",
|
||||
incomingUrl,
|
||||
},
|
||||
},
|
||||
};
|
||||
const mediaUrl = "https://example.com/synology-receipt-proof.png";
|
||||
|
||||
const outboundText = await synologyChatPlugin.outbound.sendText({
|
||||
cfg,
|
||||
text: "native outbound text",
|
||||
to: "42",
|
||||
});
|
||||
const outboundMedia = await synologyChatPlugin.outbound.sendMedia({
|
||||
cfg,
|
||||
mediaUrl,
|
||||
to: "42",
|
||||
});
|
||||
const durableText = await synologyChatPlugin.message.send?.text?.({
|
||||
cfg,
|
||||
text: "durable adapter text",
|
||||
to: "42",
|
||||
});
|
||||
const durableMedia = await synologyChatPlugin.message.send?.media?.({
|
||||
cfg,
|
||||
text: "durable adapter media",
|
||||
mediaUrl,
|
||||
to: "42",
|
||||
});
|
||||
|
||||
expect(receivedPayloads).toEqual([
|
||||
{ text: "native outbound text", user_ids: [42] },
|
||||
{ file_url: mediaUrl, user_ids: [42] },
|
||||
{ text: "durable adapter text", user_ids: [42] },
|
||||
{ file_url: mediaUrl, user_ids: [42] },
|
||||
]);
|
||||
expect(durableText).toBeDefined();
|
||||
expect(durableMedia).toBeDefined();
|
||||
for (const result of [outboundText, outboundMedia, durableText, durableMedia]) {
|
||||
expect(result).toMatchObject({
|
||||
channel: "synology-chat",
|
||||
messageId: "",
|
||||
chatId: "42",
|
||||
receipt: {
|
||||
platformMessageIds: [],
|
||||
parts: [],
|
||||
threadId: "42",
|
||||
},
|
||||
});
|
||||
expect(result?.receipt.primaryPlatformMessageId).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects unauthenticated webhook sends without fabricating delivery receipts", async () => {
|
||||
let rejectedRequests = 0;
|
||||
const port = await listenLoopback((_req, res) => {
|
||||
rejectedRequests += 1;
|
||||
res.writeHead(403, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ success: false }));
|
||||
});
|
||||
const cfg = {
|
||||
channels: {
|
||||
"synology-chat": {
|
||||
enabled: true,
|
||||
token: "synology-loopback-rejected",
|
||||
incomingUrl:
|
||||
`http://127.0.0.1:${port}/webapi/entry.cgi?` +
|
||||
"api=SYNO.Chat.External&method=chatbot&version=2&token=synology-loopback-rejected",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
await expect(
|
||||
synologyChatPlugin.outbound.sendText({ cfg, text: "rejected", to: "42" }),
|
||||
).rejects.toThrow("Failed to send message to Synology Chat");
|
||||
expect(rejectedRequests).toBe(3);
|
||||
|
||||
await expect(
|
||||
synologyChatPlugin.outbound.sendMedia({
|
||||
cfg,
|
||||
mediaUrl: "https://example.com/synology-receipt-proof.png",
|
||||
to: "42",
|
||||
}),
|
||||
).rejects.toThrow("Failed to send media to Synology Chat");
|
||||
expect(rejectedRequests).toBe(4);
|
||||
});
|
||||
|
||||
it("rejects private file URLs before contacting the authenticated webhook", async () => {
|
||||
let webhookRequests = 0;
|
||||
const port = await listenLoopback((_req, res) => {
|
||||
webhookRequests += 1;
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ success: true }));
|
||||
});
|
||||
const cfg = {
|
||||
channels: {
|
||||
"synology-chat": {
|
||||
enabled: true,
|
||||
token: "synology-loopback-proof",
|
||||
incomingUrl:
|
||||
`http://127.0.0.1:${port}/webapi/entry.cgi?` +
|
||||
"api=SYNO.Chat.External&method=chatbot&version=2&token=synology-loopback-proof",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
await expect(
|
||||
synologyChatPlugin.outbound.sendMedia({
|
||||
cfg,
|
||||
mediaUrl: `http://127.0.0.1:${port}/private-proof.png`,
|
||||
to: "42",
|
||||
}),
|
||||
).rejects.toThrow("Failed to send media to Synology Chat");
|
||||
expect(webhookRequests).toBe(0);
|
||||
});
|
||||
|
||||
it("aborts a streamed overflow and returns the stale cached identity", async () => {
|
||||
let requestCount = 0;
|
||||
const port = await listenLoopback((_req, res) => {
|
||||
|
||||
Reference in New Issue
Block a user