fix(synology-chat): fail closed empty allowlist

This commit is contained in:
Peter Steinberger
2026-02-24 22:55:03 +00:00
parent 270ab03e37
commit 0ee30361b8
7 changed files with 63 additions and 13 deletions

View File

@@ -72,6 +72,7 @@ Config values override env vars.
- `dmPolicy: "allowlist"` is the recommended default.
- `allowedUserIds` accepts a list (or comma-separated string) of Synology user IDs.
- In `allowlist` mode, an empty `allowedUserIds` list blocks all senders (use `dmPolicy: "open"` for allow-all).
- `dmPolicy: "open"` allows any sender.
- `dmPolicy: "disabled"` blocks DMs.
- Pairing approvals work with:

View File

@@ -183,6 +183,25 @@ describe("createSynologyChatPlugin", () => {
expect(warnings.some((w: string) => w.includes("open"))).toBe(true);
});
it("warns when dmPolicy is allowlist and allowedUserIds is empty", () => {
const plugin = createSynologyChatPlugin();
const account = {
accountId: "default",
enabled: true,
token: "t",
incomingUrl: "https://nas/incoming",
nasHost: "h",
webhookPath: "/w",
dmPolicy: "allowlist" as const,
allowedUserIds: [],
rateLimitPerMinute: 30,
botName: "Bot",
allowInsecureSsl: false,
};
const warnings = plugin.security.collectWarnings({ account });
expect(warnings.some((w: string) => w.includes("empty allowedUserIds"))).toBe(true);
});
it("returns no warnings for fully configured account", () => {
const plugin = createSynologyChatPlugin();
const account = {

View File

@@ -141,6 +141,11 @@ export function createSynologyChatPlugin() {
'- Synology Chat: dmPolicy="open" allows any user to message the bot. Consider "allowlist" for production use.',
);
}
if (account.dmPolicy === "allowlist" && account.allowedUserIds.length === 0) {
warnings.push(
'- Synology Chat: dmPolicy="allowlist" with empty allowedUserIds blocks all senders. Add users or set dmPolicy="open".',
);
}
return warnings;
},
},

View File

@@ -24,8 +24,8 @@ describe("validateToken", () => {
});
describe("checkUserAllowed", () => {
it("allows any user when allowlist is empty", () => {
expect(checkUserAllowed("user1", [])).toBe(true);
it("rejects user when allowlist is empty", () => {
expect(checkUserAllowed("user1", [])).toBe(false);
});
it("allows user in the allowlist", () => {

View File

@@ -22,10 +22,9 @@ export function validateToken(received: string, expected: string): boolean {
/**
* Check if a user ID is in the allowed list.
* Empty allowlist = allow all users.
* Allowlist mode must be explicit; empty lists should not match any user.
*/
export function checkUserAllowed(userId: string, allowedUserIds: string[]): boolean {
if (allowedUserIds.length === 0) return true;
return allowedUserIds.includes(userId);
}

View File

@@ -156,6 +156,26 @@ describe("createWebhookHandler", () => {
});
});
it("returns 403 when allowlist policy is set with empty allowedUserIds", async () => {
const deliver = vi.fn();
const handler = createWebhookHandler({
account: makeAccount({
dmPolicy: "allowlist",
allowedUserIds: [],
}),
deliver,
log,
});
const req = makeReq("POST", validBody);
const res = makeRes();
await handler(req, res);
expect(res._status).toBe(403);
expect(res._body).toContain("Allowlist is empty");
expect(deliver).not.toHaveBeenCalled();
});
it("returns 403 when DMs are disabled", async () => {
await expectForbiddenByPolicy({
account: { dmPolicy: "disabled" },

View File

@@ -138,20 +138,26 @@ export function createWebhookHandler(deps: WebhookHandlerDeps) {
}
// User allowlist check
if (
account.dmPolicy === "allowlist" &&
!checkUserAllowed(payload.user_id, account.allowedUserIds)
) {
log?.warn(`Unauthorized user: ${payload.user_id}`);
respond(res, 403, { error: "User not authorized" });
return;
}
if (account.dmPolicy === "disabled") {
respond(res, 403, { error: "DMs are disabled" });
return;
}
if (account.dmPolicy === "allowlist") {
if (account.allowedUserIds.length === 0) {
log?.warn("Synology Chat allowlist is empty while dmPolicy=allowlist; rejecting message");
respond(res, 403, {
error: "Allowlist is empty. Configure allowedUserIds or use dmPolicy=open.",
});
return;
}
if (!checkUserAllowed(payload.user_id, account.allowedUserIds)) {
log?.warn(`Unauthorized user: ${payload.user_id}`);
respond(res, 403, { error: "User not authorized" });
return;
}
}
// Rate limit
if (!rateLimiter.check(payload.user_id)) {
log?.warn(`Rate limit exceeded for user: ${payload.user_id}`);