diff --git a/CHANGELOG.md b/CHANGELOG.md index e04014c346e..ee31be5203a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ Docs: https://docs.openclaw.ai - Web tools/Exa: align the bundled Exa plugin with the current Exa API by supporting newer search types and richer `contents` options, while fixing the result-count cap to honor Exa's higher limit. Thanks @vincentkoc. - Plugins/Matrix: move bundled plugin `KeyedAsyncQueue` imports onto the stable `plugin-sdk/core` surface so Matrix Docker/runtime builds do not depend on the brittle keyed-async-queue subpath. Thanks @ecohash-co and @vincentkoc. - Nostr/security: enforce inbound DM policy before decrypt, route Nostr DMs through the standard reply pipeline, and add pre-crypto rate and size guards so unknown senders cannot bypass pairing or force unbounded crypto work. Thanks @kuranikaran. +- Synology Chat/security: keep reply delivery bound to stable numeric `user_id` by default, and gate mutable username/nickname recipient lookup behind `dangerouslyAllowNameMatching` with new regression coverage. - Agents/default timeout: raise the shared default agent timeout from `600s` to `48h` so long-running ACP and agent sessions do not fail unless you configure a shorter limit. - Gateway/Linux: auto-detect nvm-managed Node TLS CA bundle needs before CLI startup and refresh installed services that are missing `NODE_EXTRA_CA_CERTS`. (#51146) Thanks @GodsBoy. - Android/pairing: resolve portless secure setup URLs to `443` while preserving direct cleartext gateway defaults and explicit `:80` manual endpoints in onboarding. (#43540) Thanks @fmercurio. diff --git a/docs/channels/synology-chat.md b/docs/channels/synology-chat.md index 4da1aaa132b..21b3245f866 100644 --- a/docs/channels/synology-chat.md +++ b/docs/channels/synology-chat.md @@ -79,6 +79,7 @@ Config values override env vars. - In `allowlist` mode, an empty `allowedUserIds` list is treated as misconfiguration and the webhook route will not start (use `dmPolicy: "open"` for allow-all). - `dmPolicy: "open"` allows any sender. - `dmPolicy: "disabled"` blocks DMs. +- Reply recipient binding stays on stable numeric `user_id` by default. `channels.synology-chat.dangerouslyAllowNameMatching: true` is break-glass compatibility mode that re-enables mutable username/nickname lookup for reply delivery. - Pairing approvals work with: - `openclaw pairing list synology-chat` - `openclaw pairing approve synology-chat ` @@ -132,3 +133,4 @@ on two different Synology accounts does not share transcript state. - Keep `allowInsecureSsl: false` unless you explicitly trust a self-signed local NAS cert. - Inbound webhook requests are token-verified and rate-limited per sender. - Prefer `dmPolicy: "allowlist"` for production. +- Keep `dangerouslyAllowNameMatching` off unless you explicitly need legacy username-based reply delivery. diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index d9e06ea7321..ec1902b4d5d 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -313,6 +313,8 @@ schema: - `channels.googlechat.dangerouslyAllowNameMatching` - `channels.googlechat.accounts..dangerouslyAllowNameMatching` - `channels.msteams.dangerouslyAllowNameMatching` +- `channels.synology-chat.dangerouslyAllowNameMatching` (extension channel) +- `channels.synology-chat.accounts..dangerouslyAllowNameMatching` (extension channel) - `channels.zalouser.dangerouslyAllowNameMatching` (extension channel) - `channels.irc.dangerouslyAllowNameMatching` (extension channel) - `channels.irc.accounts..dangerouslyAllowNameMatching` (extension channel) diff --git a/extensions/synology-chat/src/accounts.test.ts b/extensions/synology-chat/src/accounts.test.ts index 627afb37378..9428f69bffe 100644 --- a/extensions/synology-chat/src/accounts.test.ts +++ b/extensions/synology-chat/src/accounts.test.ts @@ -66,6 +66,7 @@ describe("resolveAccount", () => { expect(account.accountId).toBe("default"); expect(account.enabled).toBe(true); expect(account.webhookPath).toBe("/webhook/synology"); + expect(account.dangerouslyAllowNameMatching).toBe(false); expect(account.dmPolicy).toBe("allowlist"); expect(account.rateLimitPerMinute).toBe(30); expect(account.botName).toBe("OpenClaw"); @@ -100,8 +101,13 @@ describe("resolveAccount", () => { "synology-chat": { token: "base-tok", botName: "BaseName", + dangerouslyAllowNameMatching: false, accounts: { - work: { token: "work-tok", botName: "WorkBot" }, + work: { + token: "work-tok", + botName: "WorkBot", + dangerouslyAllowNameMatching: true, + }, }, }, }, @@ -109,6 +115,42 @@ describe("resolveAccount", () => { const account = resolveAccount(cfg, "work"); expect(account.token).toBe("work-tok"); expect(account.botName).toBe("WorkBot"); + expect(account.dangerouslyAllowNameMatching).toBe(true); + }); + + it("inherits dangerous name matching from base config when not overridden", () => { + const cfg = { + channels: { + "synology-chat": { + dangerouslyAllowNameMatching: true, + accounts: { + work: { token: "work-tok" }, + }, + }, + }, + }; + + const account = resolveAccount(cfg, "work"); + expect(account.dangerouslyAllowNameMatching).toBe(true); + }); + + it("allows a named account to disable inherited dangerous name matching", () => { + const cfg = { + channels: { + "synology-chat": { + dangerouslyAllowNameMatching: true, + accounts: { + work: { + token: "work-tok", + dangerouslyAllowNameMatching: false, + }, + }, + }, + }, + }; + + const account = resolveAccount(cfg, "work"); + expect(account.dangerouslyAllowNameMatching).toBe(false); }); it("parses comma-separated allowedUserIds string", () => { diff --git a/extensions/synology-chat/src/accounts.ts b/extensions/synology-chat/src/accounts.ts index 23378bf4226..d6ac7c6aae2 100644 --- a/extensions/synology-chat/src/accounts.ts +++ b/extensions/synology-chat/src/accounts.ts @@ -89,6 +89,7 @@ export function resolveAccount( incomingUrl: merged.incomingUrl ?? envIncomingUrl, nasHost: merged.nasHost ?? envNasHost, webhookPath: merged.webhookPath ?? "/webhook/synology", + dangerouslyAllowNameMatching: merged.dangerouslyAllowNameMatching ?? false, dmPolicy: merged.dmPolicy ?? "allowlist", allowedUserIds: parseAllowedUserIds(merged.allowedUserIds ?? envAllowedUserIds), rateLimitPerMinute: merged.rateLimitPerMinute ?? envRateLimitValue, diff --git a/extensions/synology-chat/src/channel.test-mocks.ts b/extensions/synology-chat/src/channel.test-mocks.ts index ee578e416ef..e743859ea4f 100644 --- a/extensions/synology-chat/src/channel.test-mocks.ts +++ b/extensions/synology-chat/src/channel.test-mocks.ts @@ -101,6 +101,7 @@ export function makeSecurityAccount(overrides: Record = {}) { incomingUrl: "https://nas/incoming", nasHost: "h", webhookPath: "/w", + dangerouslyAllowNameMatching: false, dmPolicy: "allowlist" as const, allowedUserIds: [], rateLimitPerMinute: 30, diff --git a/extensions/synology-chat/src/channel.test.ts b/extensions/synology-chat/src/channel.test.ts index ae6e34761cb..9eda9a29484 100644 --- a/extensions/synology-chat/src/channel.test.ts +++ b/extensions/synology-chat/src/channel.test.ts @@ -107,6 +107,7 @@ describe("createSynologyChatPlugin", () => { incomingUrl: "u", nasHost: "h", webhookPath: "/w", + dangerouslyAllowNameMatching: false, dmPolicy: "allowlist" as const, allowedUserIds: ["user1"], rateLimitPerMinute: 30, @@ -171,6 +172,13 @@ describe("createSynologyChatPlugin", () => { expect(warnings.some((w: string) => w.includes("SSL"))).toBe(true); }); + it("warns when dangerous name matching is enabled", () => { + const plugin = createSynologyChatPlugin(); + const account = makeSecurityAccount({ dangerouslyAllowNameMatching: true }); + const warnings = plugin.security.collectWarnings({ account }); + expect(warnings.some((w: string) => w.includes("dangerouslyAllowNameMatching"))).toBe(true); + }); + it("warns when dmPolicy is open", () => { const plugin = createSynologyChatPlugin(); const account = makeSecurityAccount({ dmPolicy: "open" }); diff --git a/extensions/synology-chat/src/channel.ts b/extensions/synology-chat/src/channel.ts index 9df2dfec995..623e6260ff0 100644 --- a/extensions/synology-chat/src/channel.ts +++ b/extensions/synology-chat/src/channel.ts @@ -29,7 +29,13 @@ import { synologyChatSetupAdapter, synologyChatSetupWizard } from "./setup-surfa import type { ResolvedSynologyChatAccount } from "./types.js"; const CHANNEL_ID = "synology-chat"; -const SynologyChatConfigSchema = buildChannelConfigSchema(z.object({}).passthrough()); +const SynologyChatConfigSchema = buildChannelConfigSchema( + z + .object({ + dangerouslyAllowNameMatching: z.boolean().optional(), + }) + .passthrough(), +); const resolveSynologyChatDmPolicy = createScopedDmSecurityResolver({ channelKey: CHANNEL_ID, @@ -51,6 +57,7 @@ const synologyChatConfigAdapter = createHybridChannelConfigAdapter account.allowInsecureSsl && "- Synology Chat: SSL verification is disabled (allowInsecureSsl=true). Only use this for local NAS with self-signed certificates.", + (account) => + account.dangerouslyAllowNameMatching && + "- Synology Chat: dangerouslyAllowNameMatching=true re-enables mutable username/nickname recipient matching for replies. Prefer stable numeric user IDs.", (account) => account.dmPolicy === "open" && '- Synology Chat: dmPolicy="open" allows any user to message the bot. Consider "allowlist" for production use.', diff --git a/extensions/synology-chat/src/config-schema.test.ts b/extensions/synology-chat/src/config-schema.test.ts new file mode 100644 index 00000000000..18ad92ccad0 --- /dev/null +++ b/extensions/synology-chat/src/config-schema.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; +import { SynologyChatChannelConfigSchema } from "./config-schema.js"; + +describe("SynologyChatChannelConfigSchema", () => { + it("exports dangerouslyAllowNameMatching in the JSON schema", () => { + const properties = (SynologyChatChannelConfigSchema.schema.properties ?? {}) as Record< + string, + { type?: string } + >; + + expect(properties.dangerouslyAllowNameMatching?.type).toBe("boolean"); + }); + + it("keeps the schema open for plugin-specific passthrough fields", () => { + expect(SynologyChatChannelConfigSchema.schema.additionalProperties).toBe(true); + }); +}); diff --git a/extensions/synology-chat/src/config-schema.ts b/extensions/synology-chat/src/config-schema.ts index 4a9f868a87f..ab780a0b6d7 100644 --- a/extensions/synology-chat/src/config-schema.ts +++ b/extensions/synology-chat/src/config-schema.ts @@ -1,4 +1,10 @@ import { buildChannelConfigSchema } from "openclaw/plugin-sdk/channel-config-schema"; import { z } from "zod"; -export const SynologyChatChannelConfigSchema = buildChannelConfigSchema(z.object({}).passthrough()); +export const SynologyChatChannelConfigSchema = buildChannelConfigSchema( + z + .object({ + dangerouslyAllowNameMatching: z.boolean().optional(), + }) + .passthrough(), +); diff --git a/extensions/synology-chat/src/types.ts b/extensions/synology-chat/src/types.ts index 842c2ee97bb..154acf99e9e 100644 --- a/extensions/synology-chat/src/types.ts +++ b/extensions/synology-chat/src/types.ts @@ -8,6 +8,7 @@ type SynologyChatConfigFields = { incomingUrl?: string; nasHost?: string; webhookPath?: string; + dangerouslyAllowNameMatching?: boolean; dmPolicy?: "open" | "allowlist" | "disabled"; allowedUserIds?: string | string[]; rateLimitPerMinute?: number; @@ -31,6 +32,7 @@ export interface ResolvedSynologyChatAccount { incomingUrl: string; nasHost: string; webhookPath: string; + dangerouslyAllowNameMatching: boolean; dmPolicy: "open" | "allowlist" | "disabled"; allowedUserIds: string[]; rateLimitPerMinute: number; diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index 19e1f8f6d2e..d7d8123403b 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; +import { resolveChatUserId, sendMessage } from "./client.js"; import { makeFormBody, makeReq, makeRes, makeStalledReq } from "./test-http-utils.js"; import type { ResolvedSynologyChatAccount } from "./types.js"; import type { WebhookHandlerDeps } from "./webhook-handler.js"; @@ -23,6 +24,7 @@ function makeAccount( incomingUrl: "https://nas.example.com/incoming", nasHost: "nas.example.com", webhookPath: "/webhook/synology", + dangerouslyAllowNameMatching: false, dmPolicy: "open", allowedUserIds: [], rateLimitPerMinute: 30, @@ -327,6 +329,111 @@ describe("createWebhookHandler", () => { ); }); + it("keeps replies bound to payload.user_id by default", async () => { + const deliver = vi.fn().mockResolvedValue("Bot reply"); + const handler = createWebhookHandler({ + account: makeAccount({ accountId: "stable-id-test-" + Date.now() }), + deliver, + log, + }); + + const req = makeReq("POST", validBody); + const res = makeRes(); + await handler(req, res); + + expect(res._status).toBe(204); + expect(resolveChatUserId).not.toHaveBeenCalled(); + expect(deliver).toHaveBeenCalledWith( + expect.objectContaining({ + from: "123", + chatUserId: "123", + }), + ); + expect(sendMessage).toHaveBeenCalledWith( + "https://nas.example.com/incoming", + "Bot reply", + "123", + true, + ); + }); + + it("only resolves reply recipient by username when break-glass mode is enabled", async () => { + vi.mocked(resolveChatUserId).mockResolvedValueOnce(456); + const deliver = vi.fn().mockResolvedValue("Bot reply"); + const handler = createWebhookHandler({ + account: makeAccount({ + accountId: "dangerous-name-match-test-" + Date.now(), + dangerouslyAllowNameMatching: true, + }), + deliver, + log, + }); + + const req = makeReq("POST", validBody); + const res = makeRes(); + await handler(req, res); + + expect(res._status).toBe(204); + expect(resolveChatUserId).toHaveBeenCalledWith( + "https://nas.example.com/incoming", + "testuser", + true, + log, + ); + expect(deliver).toHaveBeenCalledWith( + expect.objectContaining({ + from: "123", + chatUserId: "456", + }), + ); + expect(sendMessage).toHaveBeenCalledWith( + "https://nas.example.com/incoming", + "Bot reply", + "456", + true, + ); + }); + + it("falls back to payload.user_id when break-glass resolution does not find a match", async () => { + vi.mocked(resolveChatUserId).mockResolvedValueOnce(undefined); + const deliver = vi.fn().mockResolvedValue("Bot reply"); + const handler = createWebhookHandler({ + account: makeAccount({ + accountId: "dangerous-name-fallback-test-" + Date.now(), + dangerouslyAllowNameMatching: true, + }), + deliver, + log, + }); + + const req = makeReq("POST", validBody); + const res = makeRes(); + await handler(req, res); + + expect(res._status).toBe(204); + expect(resolveChatUserId).toHaveBeenCalledWith( + "https://nas.example.com/incoming", + "testuser", + true, + log, + ); + expect(log.warn).toHaveBeenCalledWith( + 'Could not resolve Chat API user_id for "testuser" — falling back to webhook user_id 123. Reply delivery may fail.', + ); + expect(deliver).toHaveBeenCalledWith( + expect.objectContaining({ + from: "123", + chatUserId: "123", + }), + ); + expect(sendMessage).toHaveBeenCalledWith( + "https://nas.example.com/incoming", + "Bot reply", + "123", + true, + ); + }); + it("sanitizes input before delivery", async () => { const deliver = vi.fn().mockResolvedValue(null); const handler = createWebhookHandler({ diff --git a/extensions/synology-chat/src/webhook-handler.ts b/extensions/synology-chat/src/webhook-handler.ts index db4d756b551..aab77c34bd2 100644 --- a/extensions/synology-chat/src/webhook-handler.ts +++ b/extensions/synology-chat/src/webhook-handler.ts @@ -374,6 +374,10 @@ async function resolveSynologyReplyUserId(params: { payload: SynologyWebhookPayload; log?: WebhookHandlerDeps["log"]; }): Promise { + if (!params.account.dangerouslyAllowNameMatching) { + return params.payload.user_id; + } + const chatUserId = await resolveChatUserId( params.account.incomingUrl, params.payload.username,