mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-23 16:01:17 +00:00
fix: gate synology chat reply name matching
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 <CODE>`
|
||||
@@ -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.
|
||||
|
||||
@@ -313,6 +313,8 @@ schema:
|
||||
- `channels.googlechat.dangerouslyAllowNameMatching`
|
||||
- `channels.googlechat.accounts.<accountId>.dangerouslyAllowNameMatching`
|
||||
- `channels.msteams.dangerouslyAllowNameMatching`
|
||||
- `channels.synology-chat.dangerouslyAllowNameMatching` (extension channel)
|
||||
- `channels.synology-chat.accounts.<accountId>.dangerouslyAllowNameMatching` (extension channel)
|
||||
- `channels.zalouser.dangerouslyAllowNameMatching` (extension channel)
|
||||
- `channels.irc.dangerouslyAllowNameMatching` (extension channel)
|
||||
- `channels.irc.accounts.<accountId>.dangerouslyAllowNameMatching` (extension channel)
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -101,6 +101,7 @@ export function makeSecurityAccount(overrides: Record<string, unknown> = {}) {
|
||||
incomingUrl: "https://nas/incoming",
|
||||
nasHost: "h",
|
||||
webhookPath: "/w",
|
||||
dangerouslyAllowNameMatching: false,
|
||||
dmPolicy: "allowlist" as const,
|
||||
allowedUserIds: [],
|
||||
rateLimitPerMinute: 30,
|
||||
|
||||
@@ -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" });
|
||||
|
||||
@@ -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<ResolvedSynologyChatAccount>({
|
||||
channelKey: CHANNEL_ID,
|
||||
@@ -51,6 +57,7 @@ const synologyChatConfigAdapter = createHybridChannelConfigAdapter<ResolvedSynol
|
||||
"incomingUrl",
|
||||
"nasHost",
|
||||
"webhookPath",
|
||||
"dangerouslyAllowNameMatching",
|
||||
"dmPolicy",
|
||||
"allowedUserIds",
|
||||
"rateLimitPerMinute",
|
||||
@@ -73,6 +80,9 @@ const collectSynologyChatSecurityWarnings =
|
||||
(account) =>
|
||||
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.',
|
||||
|
||||
17
extensions/synology-chat/src/config-schema.test.ts
Normal file
17
extensions/synology-chat/src/config-schema.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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(),
|
||||
);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -374,6 +374,10 @@ async function resolveSynologyReplyUserId(params: {
|
||||
payload: SynologyWebhookPayload;
|
||||
log?: WebhookHandlerDeps["log"];
|
||||
}): Promise<string> {
|
||||
if (!params.account.dangerouslyAllowNameMatching) {
|
||||
return params.payload.user_id;
|
||||
}
|
||||
|
||||
const chatUserId = await resolveChatUserId(
|
||||
params.account.incomingUrl,
|
||||
params.payload.username,
|
||||
|
||||
Reference in New Issue
Block a user