From 2fc386f0dff236a6d0e96278957bbe12a1ba3282 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 27 Mar 2026 22:43:34 +0000 Subject: [PATCH] test: dedupe plugin sdk helper suites --- src/plugin-sdk/allow-from.test.ts | 158 ++++--- src/plugin-sdk/allowlist-config-edit.test.ts | 58 +-- src/plugin-sdk/channel-send-result.test.ts | 95 ++-- src/plugin-sdk/group-access.test.ts | 442 ++++++++++-------- src/plugin-sdk/ssrf-policy.test.ts | 230 +++++---- src/plugin-sdk/webhook-memory-guards.test.ts | 54 ++- src/plugin-sdk/webhook-request-guards.test.ts | 169 ++++--- src/plugin-sdk/webhook-targets.test.ts | 154 +++--- 8 files changed, 752 insertions(+), 608 deletions(-) diff --git a/src/plugin-sdk/allow-from.test.ts b/src/plugin-sdk/allow-from.test.ts index f2c5d681559..9859732ab32 100644 --- a/src/plugin-sdk/allow-from.test.ts +++ b/src/plugin-sdk/allow-from.test.ts @@ -31,80 +31,83 @@ function parseAllowTarget( } describe("isAllowedParsedChatSender", () => { - it("denies when allowFrom is empty", () => { - const allowed = isAllowedParsedChatSender({ - allowFrom: [], - sender: "+15551234567", - normalizeSender: (sender) => sender, - parseAllowTarget, - }); - - expect(allowed).toBe(false); - }); - - it("allows wildcard entries", () => { - const allowed = isAllowedParsedChatSender({ - allowFrom: ["*"], - sender: "user@example.com", - normalizeSender: (sender) => sender.toLowerCase(), - parseAllowTarget, - }); - - expect(allowed).toBe(true); - }); - - it("matches normalized handles", () => { - const allowed = isAllowedParsedChatSender({ - allowFrom: ["User@Example.com"], - sender: "user@example.com", - normalizeSender: (sender) => sender.toLowerCase(), - parseAllowTarget, - }); - - expect(allowed).toBe(true); - }); - - it("matches chat IDs when provided", () => { - const allowed = isAllowedParsedChatSender({ - allowFrom: ["chat_id:42"], - sender: "+15551234567", - chatId: 42, - normalizeSender: (sender) => sender, - parseAllowTarget, - }); - - expect(allowed).toBe(true); + it.each([ + { + name: "denies when allowFrom is empty", + input: { + allowFrom: [], + sender: "+15551234567", + normalizeSender: (sender: string) => sender, + parseAllowTarget, + }, + expected: false, + }, + { + name: "allows wildcard entries", + input: { + allowFrom: ["*"], + sender: "user@example.com", + normalizeSender: (sender: string) => sender.toLowerCase(), + parseAllowTarget, + }, + expected: true, + }, + { + name: "matches normalized handles", + input: { + allowFrom: ["User@Example.com"], + sender: "user@example.com", + normalizeSender: (sender: string) => sender.toLowerCase(), + parseAllowTarget, + }, + expected: true, + }, + { + name: "matches chat IDs when provided", + input: { + allowFrom: ["chat_id:42"], + sender: "+15551234567", + chatId: 42, + normalizeSender: (sender: string) => sender, + parseAllowTarget, + }, + expected: true, + }, + ])("$name", ({ input, expected }) => { + expect(isAllowedParsedChatSender(input)).toBe(expected); }); }); describe("isNormalizedSenderAllowed", () => { - it("allows wildcard", () => { - expect( - isNormalizedSenderAllowed({ + it.each([ + { + name: "allows wildcard", + input: { senderId: "attacker", allowFrom: ["*"], - }), - ).toBe(true); - }); - - it("normalizes case and strips prefixes", () => { - expect( - isNormalizedSenderAllowed({ + }, + expected: true, + }, + { + name: "normalizes case and strips prefixes", + input: { senderId: "12345", allowFrom: ["ZALO:12345", "zl:777"], stripPrefixRe: /^(zalo|zl):/i, - }), - ).toBe(true); - }); - - it("rejects when sender is missing", () => { - expect( - isNormalizedSenderAllowed({ + }, + expected: true, + }, + { + name: "rejects when sender is missing", + input: { senderId: "999", allowFrom: ["zl:12345"], stripPrefixRe: /^(zalo|zl):/i, - }), - ).toBe(false); + }, + expected: false, + }, + ])("$name", ({ input, expected }) => { + expect(isNormalizedSenderAllowed(input)).toBe(expected); }); }); @@ -120,21 +123,24 @@ describe("formatAllowFromLowercase", () => { }); describe("formatNormalizedAllowFromEntries", () => { - it("applies custom normalization after trimming", () => { - expect( - formatNormalizedAllowFromEntries({ + it.each([ + { + name: "applies custom normalization after trimming", + input: { allowFrom: [" @Alice ", "", " @Bob "], - normalizeEntry: (entry) => entry.replace(/^@/, "").toLowerCase(), - }), - ).toEqual(["alice", "bob"]); - }); - - it("filters empty normalized entries", () => { - expect( - formatNormalizedAllowFromEntries({ + normalizeEntry: (entry: string) => entry.replace(/^@/, "").toLowerCase(), + }, + expected: ["alice", "bob"], + }, + { + name: "filters empty normalized entries", + input: { allowFrom: ["@", "valid"], - normalizeEntry: (entry) => entry.replace(/^@$/, ""), - }), - ).toEqual(["valid"]); + normalizeEntry: (entry: string) => entry.replace(/^@$/, ""), + }, + expected: ["valid"], + }, + ])("$name", ({ input, expected }) => { + expect(formatNormalizedAllowFromEntries(input)).toEqual(expected); }); }); diff --git a/src/plugin-sdk/allowlist-config-edit.test.ts b/src/plugin-sdk/allowlist-config-edit.test.ts index 45305fcc0ed..704c55c20c4 100644 --- a/src/plugin-sdk/allowlist-config-edit.test.ts +++ b/src/plugin-sdk/allowlist-config-edit.test.ts @@ -101,30 +101,28 @@ describe("createNestedAllowlistOverrideResolver", () => { }); describe("createAccountScopedAllowlistNameResolver", () => { - it("returns empty results when the resolved account has no token", async () => { + it.each([ + { + name: "returns empty results when the resolved account has no token", + token: "", + expected: [], + }, + { + name: "delegates to the resolver when a token is present", + token: " secret ", + expected: [{ input: "a", resolved: true, name: "secret:a" }], + }, + ])("$name", async ({ token, expected }) => { const resolveNames = createAccountScopedAllowlistNameResolver({ - resolveAccount: () => ({ token: "" }), - resolveToken: (account) => account.token, - resolveNames: async ({ token, entries }) => - entries.map((entry) => ({ input: `${token}:${entry}`, resolved: true })), - }); - - expect(await resolveNames({ cfg: {}, accountId: "alt", scope: "dm", entries: ["a"] })).toEqual( - [], - ); - }); - - it("delegates to the resolver when a token is present", async () => { - const resolveNames = createAccountScopedAllowlistNameResolver({ - resolveAccount: () => ({ token: " secret " }), + resolveAccount: () => ({ token }), resolveToken: (account) => account.token, resolveNames: async ({ token, entries }) => entries.map((entry) => ({ input: entry, resolved: true, name: `${token}:${entry}` })), }); - expect(await resolveNames({ cfg: {}, accountId: "alt", scope: "dm", entries: ["a"] })).toEqual([ - { input: "a", resolved: true, name: "secret:a" }, - ]); + expect(await resolveNames({ cfg: {}, accountId: "alt", scope: "dm", entries: ["a"] })).toEqual( + expected, + ); }); }); @@ -147,10 +145,14 @@ describe("buildDmGroupAccountAllowlistAdapter", () => { resolveGroupOverrides: (account) => account.groupOverrides, }); - it("supports dm, group, and all scopes", () => { - expect(adapter.supportsScope?.({ scope: "dm" })).toBe(true); - expect(adapter.supportsScope?.({ scope: "group" })).toBe(true); - expect(adapter.supportsScope?.({ scope: "all" })).toBe(true); + const scopeCases: Array<{ scope: "dm" | "group" | "all"; expected: boolean }> = [ + { scope: "dm", expected: true }, + { scope: "group", expected: true }, + { scope: "all", expected: true }, + ]; + + it.each(scopeCases)("supports $scope scope", ({ scope, expected }) => { + expect(adapter.supportsScope?.({ scope })).toBe(expected); }); it("reads dm/group config from the resolved account", () => { @@ -200,10 +202,14 @@ describe("buildLegacyDmAccountAllowlistAdapter", () => { resolveGroupOverrides: (account) => account.groupOverrides, }); - it("supports only dm scope", () => { - expect(adapter.supportsScope?.({ scope: "dm" })).toBe(true); - expect(adapter.supportsScope?.({ scope: "group" })).toBe(false); - expect(adapter.supportsScope?.({ scope: "all" })).toBe(false); + const scopeCases: Array<{ scope: "dm" | "group" | "all"; expected: boolean }> = [ + { scope: "dm", expected: true }, + { scope: "group", expected: false }, + { scope: "all", expected: false }, + ]; + + it.each(scopeCases)("supports $scope scope", ({ scope, expected }) => { + expect(adapter.supportsScope?.({ scope })).toBe(expected); }); it("reads legacy dm config from the resolved account", () => { diff --git a/src/plugin-sdk/channel-send-result.test.ts b/src/plugin-sdk/channel-send-result.test.ts index 37d29a5a190..83ddf6bfb58 100644 --- a/src/plugin-sdk/channel-send-result.test.ts +++ b/src/plugin-sdk/channel-send-result.test.ts @@ -73,26 +73,43 @@ describe("createAttachedChannelResultAdapter", () => { sendPoll: async () => ({ messageId: "m3", pollId: "p1" }), }); - await expect(adapter.sendText!({ cfg: {} as never, to: "x", text: "hi" })).resolves.toEqual({ - channel: "discord", - messageId: "m1", - channelId: "c1", - }); - await expect(adapter.sendMedia!({ cfg: {} as never, to: "x", text: "hi" })).resolves.toEqual({ - channel: "discord", - messageId: "m2", - }); - await expect( - adapter.sendPoll!({ - cfg: {} as never, - to: "x", - poll: { question: "t", options: ["a", "b"] }, - }), - ).resolves.toEqual({ - channel: "discord", - messageId: "m3", - pollId: "p1", - }); + const sendCases = [ + { + name: "sendText", + run: () => adapter.sendText!({ cfg: {} as never, to: "x", text: "hi" }), + expected: { + channel: "discord", + messageId: "m1", + channelId: "c1", + }, + }, + { + name: "sendMedia", + run: () => adapter.sendMedia!({ cfg: {} as never, to: "x", text: "hi" }), + expected: { + channel: "discord", + messageId: "m2", + }, + }, + { + name: "sendPoll", + run: () => + adapter.sendPoll!({ + cfg: {} as never, + to: "x", + poll: { question: "t", options: ["a", "b"] }, + }), + expected: { + channel: "discord", + messageId: "m3", + pollId: "p1", + }, + }, + ]; + + for (const testCase of sendCases) { + await expect(testCase.run()).resolves.toEqual(testCase.expected); + } }); }); @@ -104,17 +121,31 @@ describe("createRawChannelSendResultAdapter", () => { sendMedia: async () => ({ ok: false, error: "boom" }), }); - await expect(adapter.sendText!({ cfg: {} as never, to: "x", text: "hi" })).resolves.toEqual({ - channel: "zalo", - ok: true, - messageId: "m1", - error: undefined, - }); - await expect(adapter.sendMedia!({ cfg: {} as never, to: "x", text: "hi" })).resolves.toEqual({ - channel: "zalo", - ok: false, - messageId: "", - error: new Error("boom"), - }); + const sendCases = [ + { + name: "sendText", + run: () => adapter.sendText!({ cfg: {} as never, to: "x", text: "hi" }), + expected: { + channel: "zalo", + ok: true, + messageId: "m1", + error: undefined, + }, + }, + { + name: "sendMedia", + run: () => adapter.sendMedia!({ cfg: {} as never, to: "x", text: "hi" }), + expected: { + channel: "zalo", + ok: false, + messageId: "", + error: new Error("boom"), + }, + }, + ]; + + for (const testCase of sendCases) { + await expect(testCase.run()).resolves.toEqual(testCase.expected); + } }); }); diff --git a/src/plugin-sdk/group-access.test.ts b/src/plugin-sdk/group-access.test.ts index fec5738e85d..13d3ba5c74b 100644 --- a/src/plugin-sdk/group-access.test.ts +++ b/src/plugin-sdk/group-access.test.ts @@ -8,256 +8,306 @@ import { } from "./group-access.js"; describe("resolveSenderScopedGroupPolicy", () => { - it("preserves disabled policy", () => { - expect( - resolveSenderScopedGroupPolicy({ + const cases: Array<{ + name: string; + input: Parameters[0]; + expected: ReturnType; + }> = [ + { + name: "preserves disabled policy", + input: { groupPolicy: "disabled", groupAllowFrom: ["a"], - }), - ).toBe("disabled"); - }); - - it("maps open/allowlist based on effective sender allowlist", () => { - expect( - resolveSenderScopedGroupPolicy({ + }, + expected: "disabled", + }, + { + name: "keeps allowlist policy when sender allowlist is present", + input: { groupPolicy: "allowlist", groupAllowFrom: ["a"], - }), - ).toBe("allowlist"); - expect( - resolveSenderScopedGroupPolicy({ + }, + expected: "allowlist", + }, + { + name: "maps allowlist to open when sender allowlist is empty", + input: { groupPolicy: "allowlist", groupAllowFrom: [], - }), - ).toBe("open"); + }, + expected: "open", + }, + ]; + + it.each(cases)("$name", ({ input, expected }) => { + expect(resolveSenderScopedGroupPolicy(input)).toBe(expected); }); }); describe("evaluateSenderGroupAccessForPolicy", () => { - it("blocks disabled policy", () => { - const decision = evaluateSenderGroupAccessForPolicy({ - groupPolicy: "disabled", - groupAllowFrom: ["123"], - senderId: "123", - isSenderAllowed: () => true, - }); + const cases: Array<{ + name: string; + input: Parameters[0]; + expected: Partial>; + }> = [ + { + name: "blocks disabled policy", + input: { + groupPolicy: "disabled", + groupAllowFrom: ["123"], + senderId: "123", + isSenderAllowed: () => true, + }, + expected: { allowed: false, reason: "disabled", groupPolicy: "disabled" }, + }, + { + name: "blocks allowlist with empty list", + input: { + groupPolicy: "allowlist", + groupAllowFrom: [], + senderId: "123", + isSenderAllowed: () => true, + }, + expected: { + allowed: false, + reason: "empty_allowlist", + groupPolicy: "allowlist", + }, + }, + ]; - expect(decision).toMatchObject({ allowed: false, reason: "disabled", groupPolicy: "disabled" }); - }); - - it("blocks allowlist with empty list", () => { - const decision = evaluateSenderGroupAccessForPolicy({ - groupPolicy: "allowlist", - groupAllowFrom: [], - senderId: "123", - isSenderAllowed: () => true, - }); - - expect(decision).toMatchObject({ - allowed: false, - reason: "empty_allowlist", - groupPolicy: "allowlist", - }); + it.each(cases)("$name", ({ input, expected }) => { + expect(evaluateSenderGroupAccessForPolicy(input)).toMatchObject(expected); }); }); describe("evaluateGroupRouteAccessForPolicy", () => { - it("blocks disabled policy", () => { - expect( - evaluateGroupRouteAccessForPolicy({ + const cases: Array<{ + name: string; + input: Parameters[0]; + expected: ReturnType; + }> = [ + { + name: "blocks disabled policy", + input: { groupPolicy: "disabled", routeAllowlistConfigured: true, routeMatched: true, routeEnabled: true, - }), - ).toEqual({ - allowed: false, - groupPolicy: "disabled", - reason: "disabled", - }); - }); - - it("blocks allowlist without configured routes", () => { - expect( - evaluateGroupRouteAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "disabled", + reason: "disabled", + }, + }, + { + name: "blocks allowlist without configured routes", + input: { groupPolicy: "allowlist", routeAllowlistConfigured: false, routeMatched: false, - }), - ).toEqual({ - allowed: false, - groupPolicy: "allowlist", - reason: "empty_allowlist", - }); - }); - - it("blocks unmatched allowlist route", () => { - expect( - evaluateGroupRouteAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "allowlist", + reason: "empty_allowlist", + }, + }, + { + name: "blocks unmatched allowlist route", + input: { groupPolicy: "allowlist", routeAllowlistConfigured: true, routeMatched: false, - }), - ).toEqual({ - allowed: false, - groupPolicy: "allowlist", - reason: "route_not_allowlisted", - }); - }); - - it("blocks disabled matched route even when group policy is open", () => { - expect( - evaluateGroupRouteAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "allowlist", + reason: "route_not_allowlisted", + }, + }, + { + name: "blocks disabled matched route even when group policy is open", + input: { groupPolicy: "open", routeAllowlistConfigured: true, routeMatched: true, routeEnabled: false, - }), - ).toEqual({ - allowed: false, - groupPolicy: "open", - reason: "route_disabled", - }); + }, + expected: { + allowed: false, + groupPolicy: "open", + reason: "route_disabled", + }, + }, + ]; + + it.each(cases)("$name", ({ input, expected }) => { + expect(evaluateGroupRouteAccessForPolicy(input)).toEqual(expected); }); }); describe("evaluateMatchedGroupAccessForPolicy", () => { - it("blocks disabled policy", () => { - expect( - evaluateMatchedGroupAccessForPolicy({ + const cases: Array<{ + name: string; + input: Parameters[0]; + expected: ReturnType; + }> = [ + { + name: "blocks disabled policy", + input: { groupPolicy: "disabled", allowlistConfigured: true, allowlistMatched: true, - }), - ).toEqual({ - allowed: false, - groupPolicy: "disabled", - reason: "disabled", - }); - }); - - it("blocks allowlist without configured entries", () => { - expect( - evaluateMatchedGroupAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "disabled", + reason: "disabled", + }, + }, + { + name: "blocks allowlist without configured entries", + input: { groupPolicy: "allowlist", allowlistConfigured: false, allowlistMatched: false, - }), - ).toEqual({ - allowed: false, - groupPolicy: "allowlist", - reason: "empty_allowlist", - }); - }); - - it("blocks allowlist when required match input is missing", () => { - expect( - evaluateMatchedGroupAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "allowlist", + reason: "empty_allowlist", + }, + }, + { + name: "blocks allowlist when required match input is missing", + input: { groupPolicy: "allowlist", requireMatchInput: true, hasMatchInput: false, allowlistConfigured: true, allowlistMatched: false, - }), - ).toEqual({ - allowed: false, - groupPolicy: "allowlist", - reason: "missing_match_input", - }); - }); - - it("blocks unmatched allowlist sender", () => { - expect( - evaluateMatchedGroupAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "allowlist", + reason: "missing_match_input", + }, + }, + { + name: "blocks unmatched allowlist sender", + input: { groupPolicy: "allowlist", allowlistConfigured: true, allowlistMatched: false, - }), - ).toEqual({ - allowed: false, - groupPolicy: "allowlist", - reason: "not_allowlisted", - }); - }); - - it("allows open policy", () => { - expect( - evaluateMatchedGroupAccessForPolicy({ + }, + expected: { + allowed: false, + groupPolicy: "allowlist", + reason: "not_allowlisted", + }, + }, + { + name: "allows open policy", + input: { groupPolicy: "open", allowlistConfigured: false, allowlistMatched: false, - }), - ).toEqual({ - allowed: true, - groupPolicy: "open", - reason: "allowed", - }); + }, + expected: { + allowed: true, + groupPolicy: "open", + reason: "allowed", + }, + }, + ]; + + it.each(cases)("$name", ({ input, expected }) => { + expect(evaluateMatchedGroupAccessForPolicy(input)).toEqual(expected); }); }); describe("evaluateSenderGroupAccess", () => { - it("defaults missing provider config to allowlist", () => { - const decision = evaluateSenderGroupAccess({ - providerConfigPresent: false, - configuredGroupPolicy: undefined, - defaultGroupPolicy: "open", - groupAllowFrom: ["123"], - senderId: "123", - isSenderAllowed: () => true, - }); + const cases: Array<{ + name: string; + input: Parameters[0]; + expected: Partial>; + matcher: "equal" | "match"; + }> = [ + { + name: "defaults missing provider config to allowlist", + input: { + providerConfigPresent: false, + configuredGroupPolicy: undefined, + defaultGroupPolicy: "open", + groupAllowFrom: ["123"], + senderId: "123", + isSenderAllowed: () => true, + }, + expected: { + allowed: true, + groupPolicy: "allowlist", + providerMissingFallbackApplied: true, + reason: "allowed", + }, + matcher: "equal", + }, + { + name: "blocks disabled policy", + input: { + providerConfigPresent: true, + configuredGroupPolicy: "disabled", + defaultGroupPolicy: "open", + groupAllowFrom: ["123"], + senderId: "123", + isSenderAllowed: () => true, + }, + expected: { allowed: false, reason: "disabled", groupPolicy: "disabled" }, + matcher: "match", + }, + { + name: "blocks allowlist with empty list", + input: { + providerConfigPresent: true, + configuredGroupPolicy: "allowlist", + defaultGroupPolicy: "open", + groupAllowFrom: [], + senderId: "123", + isSenderAllowed: () => true, + }, + expected: { + allowed: false, + reason: "empty_allowlist", + groupPolicy: "allowlist", + }, + matcher: "match", + }, + { + name: "blocks sender not allowlisted", + input: { + providerConfigPresent: true, + configuredGroupPolicy: "allowlist", + defaultGroupPolicy: "open", + groupAllowFrom: ["123"], + senderId: "999", + isSenderAllowed: () => false, + }, + expected: { + allowed: false, + reason: "sender_not_allowlisted", + groupPolicy: "allowlist", + }, + matcher: "match", + }, + ]; - expect(decision).toEqual({ - allowed: true, - groupPolicy: "allowlist", - providerMissingFallbackApplied: true, - reason: "allowed", - }); - }); - - it("blocks disabled policy", () => { - const decision = evaluateSenderGroupAccess({ - providerConfigPresent: true, - configuredGroupPolicy: "disabled", - defaultGroupPolicy: "open", - groupAllowFrom: ["123"], - senderId: "123", - isSenderAllowed: () => true, - }); - - expect(decision).toMatchObject({ allowed: false, reason: "disabled", groupPolicy: "disabled" }); - }); - - it("blocks allowlist with empty list", () => { - const decision = evaluateSenderGroupAccess({ - providerConfigPresent: true, - configuredGroupPolicy: "allowlist", - defaultGroupPolicy: "open", - groupAllowFrom: [], - senderId: "123", - isSenderAllowed: () => true, - }); - - expect(decision).toMatchObject({ - allowed: false, - reason: "empty_allowlist", - groupPolicy: "allowlist", - }); - }); - - it("blocks sender not allowlisted", () => { - const decision = evaluateSenderGroupAccess({ - providerConfigPresent: true, - configuredGroupPolicy: "allowlist", - defaultGroupPolicy: "open", - groupAllowFrom: ["123"], - senderId: "999", - isSenderAllowed: () => false, - }); - - expect(decision).toMatchObject({ - allowed: false, - reason: "sender_not_allowlisted", - groupPolicy: "allowlist", - }); + it.each(cases)("$name", ({ input, expected, matcher }) => { + const decision = evaluateSenderGroupAccess(input); + if (matcher === "equal") { + expect(decision).toEqual(expected); + return; + } + expect(decision).toMatchObject(expected); }); }); diff --git a/src/plugin-sdk/ssrf-policy.test.ts b/src/plugin-sdk/ssrf-policy.test.ts index fc4eac6679f..64ccb0eb904 100644 --- a/src/plugin-sdk/ssrf-policy.test.ts +++ b/src/plugin-sdk/ssrf-policy.test.ts @@ -18,119 +18,167 @@ function createLookupFn(addresses: Array<{ address: string; family: number }>): } describe("ssrfPolicyFromAllowPrivateNetwork", () => { - it("returns undefined unless private-network access is explicitly enabled", () => { - expect(ssrfPolicyFromAllowPrivateNetwork(undefined)).toBeUndefined(); - expect(ssrfPolicyFromAllowPrivateNetwork(false)).toBeUndefined(); - expect(ssrfPolicyFromAllowPrivateNetwork(true)).toEqual({ allowPrivateNetwork: true }); + it.each([ + { + name: "returns undefined for missing input", + input: undefined, + expected: undefined, + }, + { + name: "returns undefined when private-network access is disabled", + input: false, + expected: undefined, + }, + { + name: "returns an explicit allow-private-network policy when enabled", + input: true, + expected: { allowPrivateNetwork: true }, + }, + ])("$name", ({ input, expected }) => { + expect(ssrfPolicyFromAllowPrivateNetwork(input)).toEqual(expected); }); }); describe("assertHttpUrlTargetsPrivateNetwork", () => { - it("allows https targets without private-network checks", async () => { - await expect( - assertHttpUrlTargetsPrivateNetwork("https://matrix.example.org", { + it.each([ + { + name: "allows https targets without private-network checks", + url: "https://matrix.example.org", + policy: { allowPrivateNetwork: false, - }), - ).resolves.toBeUndefined(); - }); - - it("allows internal DNS names only when they resolve exclusively to private IPs", async () => { - await expect( - assertHttpUrlTargetsPrivateNetwork("http://matrix-synapse:8008", { + }, + outcome: "resolve", + }, + { + name: "allows internal DNS names only when they resolve exclusively to private IPs", + url: "http://matrix-synapse:8008", + policy: { allowPrivateNetwork: true, lookupFn: createLookupFn([{ address: "10.0.0.5", family: 4 }]), - }), - ).resolves.toBeUndefined(); - }); - - it("rejects cleartext public hosts even when private-network access is enabled", async () => { - await expect( - assertHttpUrlTargetsPrivateNetwork("http://matrix.example.org:8008", { + }, + outcome: "resolve", + }, + { + name: "rejects cleartext public hosts even when private-network access is enabled", + url: "http://matrix.example.org:8008", + policy: { allowPrivateNetwork: true, lookupFn: createLookupFn([{ address: "93.184.216.34", family: 4 }]), errorMessage: "Matrix homeserver must use https:// unless it targets a private or loopback host", - }), - ).rejects.toThrow( - "Matrix homeserver must use https:// unless it targets a private or loopback host", - ); + }, + outcome: "reject", + expectedError: + "Matrix homeserver must use https:// unless it targets a private or loopback host", + }, + ])("$name", async ({ url, policy, outcome, expectedError }) => { + const result = assertHttpUrlTargetsPrivateNetwork(url, policy); + if (outcome === "reject") { + await expect(result).rejects.toThrow(expectedError); + return; + } + await expect(result).resolves.toBeUndefined(); }); }); describe("normalizeHostnameSuffixAllowlist", () => { - it("uses defaults when input is missing", () => { - expect(normalizeHostnameSuffixAllowlist(undefined, ["GRAPH.MICROSOFT.COM"])).toEqual([ - "graph.microsoft.com", - ]); - }); - - it("normalizes wildcard prefixes and deduplicates", () => { - expect( - normalizeHostnameSuffixAllowlist([ - "*.TrafficManager.NET", - ".trafficmanager.net.", - " * ", - "x", - ]), - ).toEqual(["*"]); + it.each([ + { + name: "uses defaults when input is missing", + input: undefined, + defaults: ["GRAPH.MICROSOFT.COM"], + expected: ["graph.microsoft.com"], + }, + { + name: "normalizes wildcard prefixes and deduplicates", + input: ["*.TrafficManager.NET", ".trafficmanager.net.", " * ", "x"], + defaults: undefined, + expected: ["*"], + }, + ])("$name", ({ input, defaults, expected }) => { + expect(normalizeHostnameSuffixAllowlist(input, defaults)).toEqual(expected); }); }); describe("isHttpsUrlAllowedByHostnameSuffixAllowlist", () => { - it("requires https", () => { - expect( - isHttpsUrlAllowedByHostnameSuffixAllowlist("http://a.example.com/x", ["example.com"]), - ).toBe(false); - }); - - it("supports exact and suffix match", () => { - expect( - isHttpsUrlAllowedByHostnameSuffixAllowlist("https://example.com/x", ["example.com"]), - ).toBe(true); - expect( - isHttpsUrlAllowedByHostnameSuffixAllowlist("https://a.example.com/x", ["example.com"]), - ).toBe(true); - expect(isHttpsUrlAllowedByHostnameSuffixAllowlist("https://evil.com/x", ["example.com"])).toBe( - false, - ); - }); - - it("supports wildcard allowlist", () => { - expect(isHttpsUrlAllowedByHostnameSuffixAllowlist("https://evil.com/x", ["*"])).toBe(true); + it.each([ + { + name: "requires https", + url: "http://a.example.com/x", + allowlist: ["example.com"], + expected: false, + }, + { + name: "supports exact match", + url: "https://example.com/x", + allowlist: ["example.com"], + expected: true, + }, + { + name: "supports suffix match", + url: "https://a.example.com/x", + allowlist: ["example.com"], + expected: true, + }, + { + name: "rejects non-matching hosts", + url: "https://evil.com/x", + allowlist: ["example.com"], + expected: false, + }, + { + name: "supports wildcard allowlist", + url: "https://evil.com/x", + allowlist: ["*"], + expected: true, + }, + ])("$name", ({ url, allowlist, expected }) => { + expect(isHttpsUrlAllowedByHostnameSuffixAllowlist(url, allowlist)).toBe(expected); }); }); describe("buildHostnameAllowlistPolicyFromSuffixAllowlist", () => { - it("returns undefined when allowHosts is empty", () => { - expect(buildHostnameAllowlistPolicyFromSuffixAllowlist()).toBeUndefined(); - expect(buildHostnameAllowlistPolicyFromSuffixAllowlist([])).toBeUndefined(); - }); - - it("returns undefined when wildcard host is present", () => { - expect(buildHostnameAllowlistPolicyFromSuffixAllowlist(["*"])).toBeUndefined(); - expect(buildHostnameAllowlistPolicyFromSuffixAllowlist(["example.com", "*"])).toBeUndefined(); - }); - - it("expands a suffix entry to exact + wildcard hostname allowlist patterns", () => { - expect(buildHostnameAllowlistPolicyFromSuffixAllowlist(["sharepoint.com"])).toEqual({ - hostnameAllowlist: ["sharepoint.com", "*.sharepoint.com"], - }); - }); - - it("normalizes wildcard prefixes, leading/trailing dots, and deduplicates patterns", () => { - expect( - buildHostnameAllowlistPolicyFromSuffixAllowlist([ - "*.TrafficManager.NET", - ".trafficmanager.net.", - " blob.core.windows.net ", - ]), - ).toEqual({ - hostnameAllowlist: [ - "trafficmanager.net", - "*.trafficmanager.net", - "blob.core.windows.net", - "*.blob.core.windows.net", - ], - }); + it.each([ + { + name: "returns undefined when allowHosts is empty", + input: undefined, + expected: undefined, + }, + { + name: "returns undefined for an explicit empty list", + input: [], + expected: undefined, + }, + { + name: "returns undefined when wildcard host is present", + input: ["*"], + expected: undefined, + }, + { + name: "returns undefined when wildcard is mixed with concrete hosts", + input: ["example.com", "*"], + expected: undefined, + }, + { + name: "expands a suffix entry to exact + wildcard hostname allowlist patterns", + input: ["sharepoint.com"], + expected: { + hostnameAllowlist: ["sharepoint.com", "*.sharepoint.com"], + }, + }, + { + name: "normalizes wildcard prefixes, leading/trailing dots, and deduplicates patterns", + input: ["*.TrafficManager.NET", ".trafficmanager.net.", " blob.core.windows.net "], + expected: { + hostnameAllowlist: [ + "trafficmanager.net", + "*.trafficmanager.net", + "blob.core.windows.net", + "*.blob.core.windows.net", + ], + }, + }, + ])("$name", ({ input, expected }) => { + expect(buildHostnameAllowlistPolicyFromSuffixAllowlist(input)).toEqual(expected); }); }); diff --git a/src/plugin-sdk/webhook-memory-guards.test.ts b/src/plugin-sdk/webhook-memory-guards.test.ts index a4e639179de..4a51452cf71 100644 --- a/src/plugin-sdk/webhook-memory-guards.test.ts +++ b/src/plugin-sdk/webhook-memory-guards.test.ts @@ -21,16 +21,20 @@ describe("createFixedWindowRateLimiter", () => { expect(limiter.isRateLimited("k", 1_003)).toBe(true); }); - it("resets counters after the window elapses", () => { + it.each([ + { + name: "resets counters after the window elapses", + calls: [100, 101, 111], + expected: [false, true, false], + }, + ])("$name", ({ calls, expected }) => { const limiter = createFixedWindowRateLimiter({ windowMs: 10, maxRequests: 1, maxTrackedKeys: 100, }); - expect(limiter.isRateLimited("k", 100)).toBe(false); - expect(limiter.isRateLimited("k", 101)).toBe(true); - expect(limiter.isRateLimited("k", 111)).toBe(false); + expect(calls.map((nowMs) => limiter.isRateLimited("k", nowMs))).toEqual(expected); }); it("caps tracked keys", () => { @@ -69,9 +73,7 @@ describe("createBoundedCounter", () => { it("increments and returns per-key counts", () => { const counter = createBoundedCounter({ maxTrackedKeys: 100 }); - expect(counter.increment("k", 1_000)).toBe(1); - expect(counter.increment("k", 1_001)).toBe(2); - expect(counter.increment("k", 1_002)).toBe(3); + expect([1_000, 1_001, 1_002].map((nowMs) => counter.increment("k", nowMs))).toEqual([1, 2, 3]); }); it("caps tracked keys", () => { @@ -121,33 +123,29 @@ describe("createWebhookAnomalyTracker", () => { logEvery: 2, }); - expect( - tracker.record({ - key: "k", + const counts = [ + { statusCode: 415, - message: (count) => `ignored:${count}`, - log: (msg) => logs.push(msg), - }), - ).toBe(0); - - expect( + message: (count: number) => `ignored:${count}`, + }, + { + statusCode: 401, + message: (count: number) => `hit:${count}`, + }, + { + statusCode: 401, + message: (count: number) => `hit:${count}`, + }, + ].map(({ statusCode, message }) => tracker.record({ key: "k", - statusCode: 401, - message: (count) => `hit:${count}`, + statusCode, + message, log: (msg) => logs.push(msg), }), - ).toBe(1); - - expect( - tracker.record({ - key: "k", - statusCode: 401, - message: (count) => `hit:${count}`, - log: (msg) => logs.push(msg), - }), - ).toBe(2); + ); + expect(counts).toEqual([0, 1, 2]); expect(logs).toEqual(["hit:1", "hit:2"]); }); }); diff --git a/src/plugin-sdk/webhook-request-guards.test.ts b/src/plugin-sdk/webhook-request-guards.test.ts index 91b7f4823db..339bb3cfbb0 100644 --- a/src/plugin-sdk/webhook-request-guards.test.ts +++ b/src/plugin-sdk/webhook-request-guards.test.ts @@ -46,15 +46,45 @@ function createMockRequest(params: { return req; } -describe("isJsonContentType", () => { - it("accepts application/json and +json suffixes", () => { - expect(isJsonContentType("application/json")).toBe(true); - expect(isJsonContentType("application/cloudevents+json; charset=utf-8")).toBe(true); - }); +async function readJsonBody(chunks: string[], emptyObjectOnEmpty = false) { + const req = createMockRequest({ chunks }); + const res = createMockServerResponse(); + return { + result: await readJsonWebhookBodyOrReject({ + req, + res, + maxBytes: 1024, + emptyObjectOnEmpty, + }), + res, + }; +} - it("rejects non-json media types", () => { - expect(isJsonContentType("text/plain")).toBe(false); - expect(isJsonContentType(undefined)).toBe(false); +async function readRawBody(params: Parameters[0], profile?: "pre-auth") { + const req = createMockRequest(params); + const res = createMockServerResponse(); + return { + result: await readWebhookBodyOrReject({ + req, + res, + profile, + }), + res, + }; +} + +describe("isJsonContentType", () => { + it.each([ + { name: "accepts application/json", input: "application/json", expected: true }, + { + name: "accepts +json suffixes", + input: "application/cloudevents+json; charset=utf-8", + expected: true, + }, + { name: "rejects non-json media types", input: "text/plain", expected: false }, + { name: "rejects missing media types", input: undefined, expected: false }, + ])("$name", ({ input, expected }) => { + expect(isJsonContentType(input)).toBe(expected); }); }); @@ -103,89 +133,82 @@ describe("applyBasicWebhookRequestGuards", () => { expect(res2.statusCode).toBe(429); }); - it("rejects non-json requests when required", () => { - const req = createMockRequest({ - method: "POST", - headers: { "content-type": "text/plain" }, - }); + it.each([ + { + name: "allows matching JSON requests", + req: createMockRequest({ + method: "POST", + headers: { "content-type": "application/json" }, + }), + expectedOk: true, + expectedStatusCode: 200, + }, + { + name: "rejects non-json requests when required", + req: createMockRequest({ + method: "POST", + headers: { "content-type": "text/plain" }, + }), + expectedOk: false, + expectedStatusCode: 415, + }, + ])("$name", ({ req, expectedOk, expectedStatusCode }) => { const res = createMockServerResponse(); const ok = applyBasicWebhookRequestGuards({ req, res, requireJsonContentType: true, }); - expect(ok).toBe(false); - expect(res.statusCode).toBe(415); + expect(ok).toBe(expectedOk); + expect(res.statusCode).toBe(expectedStatusCode); }); }); describe("readJsonWebhookBodyOrReject", () => { - it("returns parsed JSON body", async () => { - const req = createMockRequest({ chunks: ['{"ok":true}'] }); - const res = createMockServerResponse(); - await expect( - readJsonWebhookBodyOrReject({ - req, - res, - maxBytes: 1024, - emptyObjectOnEmpty: false, - }), - ).resolves.toEqual({ ok: true, value: { ok: true } }); - }); - - it("preserves valid JSON null payload", async () => { - const req = createMockRequest({ chunks: ["null"] }); - const res = createMockServerResponse(); - await expect( - readJsonWebhookBodyOrReject({ - req, - res, - maxBytes: 1024, - emptyObjectOnEmpty: false, - }), - ).resolves.toEqual({ ok: true, value: null }); - }); - - it("writes 400 on invalid JSON payload", async () => { - const req = createMockRequest({ chunks: ["{bad json"] }); - const res = createMockServerResponse(); - await expect( - readJsonWebhookBodyOrReject({ - req, - res, - maxBytes: 1024, - emptyObjectOnEmpty: false, - }), - ).resolves.toEqual({ ok: false }); - expect(res.statusCode).toBe(400); - expect(res.body).toBe("Bad Request"); + it.each([ + { + name: "returns parsed JSON body", + chunks: ['{"ok":true}'], + expected: { ok: true, value: { ok: true } }, + expectedStatusCode: 200, + expectedBody: undefined, + }, + { + name: "preserves valid JSON null payload", + chunks: ["null"], + expected: { ok: true, value: null }, + expectedStatusCode: 200, + expectedBody: undefined, + }, + { + name: "writes 400 on invalid JSON payload", + chunks: ["{bad json"], + expected: { ok: false }, + expectedStatusCode: 400, + expectedBody: "Bad Request", + }, + ])("$name", async ({ chunks, expected, expectedStatusCode, expectedBody }) => { + const { result, res } = await readJsonBody(chunks); + expect(result).toEqual(expected); + expect(res.statusCode).toBe(expectedStatusCode); + expect(res.body).toBe(expectedBody); }); }); describe("readWebhookBodyOrReject", () => { it("returns raw body contents", async () => { - const req = createMockRequest({ chunks: ["plain text"] }); - const res = createMockServerResponse(); - await expect( - readWebhookBodyOrReject({ - req, - res, - }), - ).resolves.toEqual({ ok: true, value: "plain text" }); + const { result } = await readRawBody({ chunks: ["plain text"] }); + expect(result).toEqual({ ok: true, value: "plain text" }); }); it("enforces strict pre-auth default body limits", async () => { - const req = createMockRequest({ - headers: { "content-length": String(70 * 1024) }, - }); - const res = createMockServerResponse(); - await expect( - readWebhookBodyOrReject({ - req, - res, - profile: "pre-auth", - }), - ).resolves.toEqual({ ok: false }); + const { result, res } = await readRawBody( + { + headers: { "content-length": String(70 * 1024) }, + }, + "pre-auth", + ); + expect(result).toEqual({ ok: false }); expect(res.statusCode).toBe(413); }); }); diff --git a/src/plugin-sdk/webhook-targets.test.ts b/src/plugin-sdk/webhook-targets.test.ts index 02ad40b1f1c..7dfca230e76 100644 --- a/src/plugin-sdk/webhook-targets.test.ts +++ b/src/plugin-sdk/webhook-targets.test.ts @@ -24,6 +24,28 @@ function createRequest(method: string, url: string): IncomingMessage { return req; } +function createResponse() { + const setHeader = vi.fn(); + const end = vi.fn(); + return { + res: { + statusCode: 200, + setHeader, + end, + } as unknown as ServerResponse, + setHeader, + end, + }; +} + +function createPipelineRequest(url: string): IncomingMessage { + const req = createRequest("POST", url); + (req as unknown as { socket: { remoteAddress: string } }).socket = { + remoteAddress: "127.0.0.1", + }; + return req; +} + afterEach(() => { setActivePluginRegistry(createEmptyPluginRegistry()); }); @@ -141,31 +163,31 @@ describe("registerWebhookTargetWithPluginRoute", () => { }); describe("resolveWebhookTargets", () => { - it("resolves normalized path targets", () => { - const targets = new Map>(); - targets.set("/hook", [{ id: "A" }]); - - expect(resolveWebhookTargets(createRequest("POST", "/hook/"), targets)).toEqual({ - path: "/hook", - targets: [{ id: "A" }], - }); - }); - - it("returns null when path has no targets", () => { - const targets = new Map>(); - expect(resolveWebhookTargets(createRequest("POST", "/missing"), targets)).toBeNull(); + it.each([ + { + name: "resolves normalized path targets", + requestPath: "/hook/", + targets: new Map([["/hook", [{ id: "A" }]]]), + expected: { + path: "/hook", + targets: [{ id: "A" }], + }, + }, + { + name: "returns null when path has no targets", + requestPath: "/missing", + targets: new Map>(), + expected: null, + }, + ])("$name", ({ requestPath, targets, expected }) => { + expect(resolveWebhookTargets(createRequest("POST", requestPath), targets)).toEqual(expected); }); }); describe("withResolvedWebhookRequestPipeline", () => { it("returns false when request path has no registered targets", async () => { const req = createRequest("POST", "/missing"); - req.headers = {}; - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: vi.fn(), - } as unknown as ServerResponse; + const { res } = createResponse(); const handled = await withResolvedWebhookRequestPipeline({ req, res, @@ -177,16 +199,8 @@ describe("withResolvedWebhookRequestPipeline", () => { }); it("runs handler when targets resolve and method passes", async () => { - const req = createRequest("POST", "/hook"); - req.headers = {}; - (req as unknown as { socket: { remoteAddress: string } }).socket = { - remoteAddress: "127.0.0.1", - }; - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: vi.fn(), - } as unknown as ServerResponse; + const req = createPipelineRequest("/hook"); + const { res } = createResponse(); const handle = vi.fn(async () => {}); const handled = await withResolvedWebhookRequestPipeline({ req, @@ -200,16 +214,8 @@ describe("withResolvedWebhookRequestPipeline", () => { }); it("releases in-flight slot when handler throws", async () => { - const req = createRequest("POST", "/hook"); - req.headers = {}; - (req as unknown as { socket: { remoteAddress: string } }).socket = { - remoteAddress: "127.0.0.1", - }; - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: vi.fn(), - } as unknown as ServerResponse; + const req = createPipelineRequest("/hook"); + const { res } = createResponse(); const limiter = createWebhookInFlightLimiter(); await expect( @@ -231,20 +237,14 @@ describe("withResolvedWebhookRequestPipeline", () => { describe("rejectNonPostWebhookRequest", () => { it("sets 405 for non-POST requests", () => { - const setHeaderMock = vi.fn(); - const endMock = vi.fn(); - const res = { - statusCode: 200, - setHeader: setHeaderMock, - end: endMock, - } as unknown as ServerResponse; + const { res, setHeader, end } = createResponse(); const rejected = rejectNonPostWebhookRequest(createRequest("GET", "/hook"), res); expect(rejected).toBe(true); expect(res.statusCode).toBe(405); - expect(setHeaderMock).toHaveBeenCalledWith("Allow", "POST"); - expect(endMock).toHaveBeenCalledWith("Method Not Allowed"); + expect(setHeader).toHaveBeenCalledWith("Allow", "POST"); + expect(end).toHaveBeenCalledWith("Method Not Allowed"); }); }); @@ -291,11 +291,7 @@ describe("resolveSingleWebhookTarget", () => { describe("resolveWebhookTargetWithAuthOrReject", () => { it("returns matched target", async () => { - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: vi.fn(), - } as unknown as ServerResponse; + const { res } = createResponse(); await expect( resolveWebhookTargetWithAuthOrReject({ targets: [{ id: "a" }, { id: "b" }], @@ -305,50 +301,36 @@ describe("resolveWebhookTargetWithAuthOrReject", () => { ).resolves.toEqual({ id: "b" }); }); - it("writes unauthorized response on no match", async () => { - const endMock = vi.fn(); - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: endMock, - } as unknown as ServerResponse; + it.each([ + { + name: "writes unauthorized response on no match", + targets: [{ id: "a" }], + isMatch: () => false, + expectedEnd: "unauthorized", + }, + { + name: "writes ambiguous response on multi-match", + targets: [{ id: "a" }, { id: "b" }], + isMatch: () => true, + expectedEnd: "ambiguous webhook target", + }, + ])("$name", async ({ targets, isMatch, expectedEnd }) => { + const { res, end } = createResponse(); await expect( resolveWebhookTargetWithAuthOrReject({ - targets: [{ id: "a" }], + targets, res, - isMatch: () => false, + isMatch, }), ).resolves.toBeNull(); expect(res.statusCode).toBe(401); - expect(endMock).toHaveBeenCalledWith("unauthorized"); - }); - - it("writes ambiguous response on multi-match", async () => { - const endMock = vi.fn(); - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: endMock, - } as unknown as ServerResponse; - await expect( - resolveWebhookTargetWithAuthOrReject({ - targets: [{ id: "a" }, { id: "b" }], - res, - isMatch: () => true, - }), - ).resolves.toBeNull(); - expect(res.statusCode).toBe(401); - expect(endMock).toHaveBeenCalledWith("ambiguous webhook target"); + expect(end).toHaveBeenCalledWith(expectedEnd); }); }); describe("resolveWebhookTargetWithAuthOrRejectSync", () => { it("returns matched target synchronously", () => { - const res = { - statusCode: 200, - setHeader: vi.fn(), - end: vi.fn(), - } as unknown as ServerResponse; + const { res } = createResponse(); const target = resolveWebhookTargetWithAuthOrRejectSync({ targets: [{ id: "a" }, { id: "b" }], res,