diff --git a/src/infra/heartbeat-runner.returns-default-unset.test.ts b/src/infra/heartbeat-runner.returns-default-unset.test.ts index a6e6ea19398..36b502b72ce 100644 --- a/src/infra/heartbeat-runner.returns-default-unset.test.ts +++ b/src/infra/heartbeat-runner.returns-default-unset.test.ts @@ -166,19 +166,17 @@ describe("resolveHeartbeatIntervalMs", () => { }); describe("resolveHeartbeatPrompt", () => { - it("uses default or trimmed override prompts", () => { - const cases = [ - { cfg: {} as OpenClawConfig, expected: HEARTBEAT_PROMPT }, - { - cfg: { - agents: { defaults: { heartbeat: { prompt: " ping " } } }, - } as OpenClawConfig, - expected: "ping", - }, - ] as const; - for (const { cfg, expected } of cases) { - expect(resolveHeartbeatPrompt(cfg)).toBe(expected); - } + it.each([ + { name: "default prompt", cfg: {} as OpenClawConfig, expected: HEARTBEAT_PROMPT }, + { + name: "trimmed override prompt", + cfg: { + agents: { defaults: { heartbeat: { prompt: " ping " } } }, + } as OpenClawConfig, + expected: "ping", + }, + ])("uses $name", ({ cfg, expected }) => { + expect(resolveHeartbeatPrompt(cfg)).toBe(expected); }); }); @@ -351,12 +349,12 @@ describe("resolveHeartbeatDeliveryTarget", () => { } }); - it("parses optional telegram :topic: threadId suffix", () => { - const cases = [ - { to: "-100111:topic:42", expectedTo: "-100111", expectedThreadId: 42 }, - { to: "-100111", expectedTo: "-100111", expectedThreadId: undefined }, - ] as const; - for (const { to, expectedTo, expectedThreadId } of cases) { + it.each([ + { name: "topic suffix", to: "-100111:topic:42", expectedTo: "-100111", expectedThreadId: 42 }, + { name: "plain chat id", to: "-100111", expectedTo: "-100111", expectedThreadId: undefined }, + ])( + "parses optional telegram :topic: threadId suffix: $name", + ({ to, expectedTo, expectedThreadId }) => { const cfg: OpenClawConfig = { agents: { defaults: { @@ -368,34 +366,35 @@ describe("resolveHeartbeatDeliveryTarget", () => { expect(result.channel).toBe("telegram"); expect(result.to).toBe(expectedTo); expect(result.threadId).toBe(expectedThreadId); - } - }); + }, + ); - it("handles explicit heartbeat accountId allow/deny", () => { - const cases = [ - { + it.each([ + { + name: "known account", + accountId: "work", + expected: { + channel: "telegram", + to: "-100123", accountId: "work", - expected: { - channel: "telegram", - to: "-100123", - accountId: "work", - lastChannel: undefined, - lastAccountId: undefined, - }, + lastChannel: undefined, + lastAccountId: undefined, }, - { + }, + { + name: "missing account", + accountId: "missing", + expected: { + channel: "none", + reason: "unknown-account", accountId: "missing", - expected: { - channel: "none", - reason: "unknown-account", - accountId: "missing", - lastChannel: undefined, - lastAccountId: undefined, - }, + lastChannel: undefined, + lastAccountId: undefined, }, - ] as const; - - for (const { accountId, expected } of cases) { + }, + ] as const)( + "handles explicit heartbeat accountId allow/deny: $name", + ({ accountId, expected }) => { const cfg: OpenClawConfig = { agents: { defaults: { @@ -405,8 +404,8 @@ describe("resolveHeartbeatDeliveryTarget", () => { channels: { telegram: { accounts: { work: { botToken: "token" } } } }, }; expect(resolveHeartbeatDeliveryTarget({ cfg, entry: baseEntry })).toEqual(expected); - } - }); + }, + ); it("prefers per-agent heartbeat overrides when provided", () => { const cfg: OpenClawConfig = { @@ -738,45 +737,36 @@ describe("runHeartbeatOnce", () => { } }); - it("resolves configured and forced session key overrides", async () => { - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - const cases = typedCases<{ - name: string; - caseDir: string; - peerKind: "group" | "direct"; - peerId: string; - message: string; - applyOverride: (params: { cfg: OpenClawConfig; sessionKey: string }) => void; - runOptions: (params: { sessionKey: string }) => { sessionKey?: string }; - }>([ - { - name: "heartbeat.session", - caseDir: "hb-explicit-session", - peerKind: "group" as const, - peerId: "120363401234567890@g.us", - message: "Group alert", - applyOverride: ({ cfg, sessionKey }: { cfg: OpenClawConfig; sessionKey: string }) => { - if (cfg.agents?.defaults?.heartbeat) { - cfg.agents.defaults.heartbeat.session = sessionKey; - } - }, - runOptions: ({ sessionKey: _sessionKey }: { sessionKey: string }) => ({ - sessionKey: undefined as string | undefined, - }), - }, - { - name: "runHeartbeatOnce sessionKey arg", - caseDir: "hb-forced-session-override", - peerKind: "group" as const, - peerId: "120363401234567891@g.us", - message: "Forced alert", - applyOverride: () => {}, - runOptions: ({ sessionKey }: { sessionKey: string }) => ({ sessionKey }), - }, - ]); - - for (const { name, caseDir, peerKind, peerId, message, applyOverride, runOptions } of cases) { + it.each([ + { + name: "heartbeat.session", + caseDir: "hb-explicit-session", + peerKind: "group" as const, + peerId: "120363401234567890@g.us", + message: "Group alert", + applyOverride: ({ cfg, sessionKey }: { cfg: OpenClawConfig; sessionKey: string }) => { + if (cfg.agents?.defaults?.heartbeat) { + cfg.agents.defaults.heartbeat.session = sessionKey; + } + }, + runOptions: ({ sessionKey: _sessionKey }: { sessionKey: string }) => ({ + sessionKey: undefined as string | undefined, + }), + }, + { + name: "runHeartbeatOnce sessionKey arg", + caseDir: "hb-forced-session-override", + peerKind: "group" as const, + peerId: "120363401234567891@g.us", + message: "Forced alert", + applyOverride: () => {}, + runOptions: ({ sessionKey }: { sessionKey: string }) => ({ sessionKey }), + }, + ])( + "resolves configured and forced session key overrides: $name", + async ({ name, caseDir, peerKind, peerId, message, applyOverride, runOptions }) => { + const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); + try { const tmpDir = await createCaseDir(caseDir); const storePath = path.join(tmpDir, "sessions.json"); const cfg: OpenClawConfig = { @@ -850,11 +840,11 @@ describe("runHeartbeatOnce", () => { expect.objectContaining({ isHeartbeat: true, suppressToolErrorWarnings: false }), cfg, ); + } finally { + replySpy.mockRestore(); } - } finally { - replySpy.mockRestore(); - } - }); + }, + ); it("suppresses duplicate heartbeat payloads within 24h", async () => { const tmpDir = await createCaseDir("hb-dup-suppress"); @@ -909,30 +899,31 @@ describe("runHeartbeatOnce", () => { } }); - it("handles reasoning payload delivery variants", async () => { - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - const cases = typedCases<{ - name: string; - caseDir: string; - replies: Array<{ text: string }>; - expectedTexts: string[]; - }>([ - { - name: "reasoning + final payload", - caseDir: "hb-reasoning", - replies: [{ text: "Reasoning:\n_Because it helps_" }, { text: "Final alert" }], - expectedTexts: ["Reasoning:\n_Because it helps_", "Final alert"], - }, - { - name: "reasoning + HEARTBEAT_OK", - caseDir: "hb-reasoning-heartbeat-ok", - replies: [{ text: "Reasoning:\n_Because it helps_" }, { text: "HEARTBEAT_OK" }], - expectedTexts: ["Reasoning:\n_Because it helps_"], - }, - ]); - - for (const { name, caseDir, replies, expectedTexts } of cases) { + it.each( + typedCases<{ + name: string; + caseDir: string; + replies: Array<{ text: string }>; + expectedTexts: string[]; + }>([ + { + name: "reasoning + final payload", + caseDir: "hb-reasoning", + replies: [{ text: "Reasoning:\n_Because it helps_" }, { text: "Final alert" }], + expectedTexts: ["Reasoning:\n_Because it helps_", "Final alert"], + }, + { + name: "reasoning + HEARTBEAT_OK", + caseDir: "hb-reasoning-heartbeat-ok", + replies: [{ text: "Reasoning:\n_Because it helps_" }, { text: "HEARTBEAT_OK" }], + expectedTexts: ["Reasoning:\n_Because it helps_"], + }, + ]), + )( + "handles reasoning payload delivery variants: $name", + async ({ name, caseDir, replies, expectedTexts }) => { + const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); + try { const tmpDir = await createCaseDir(caseDir); const storePath = path.join(tmpDir, "sessions.json"); const cfg: OpenClawConfig = { @@ -990,11 +981,11 @@ describe("runHeartbeatOnce", () => { expect.any(Object), ); } + } finally { + replySpy.mockRestore(); } - } finally { - replySpy.mockRestore(); - } - }); + }, + ); it("loads the default agent session from templated stores", async () => { const tmpDir = await createCaseDir("openclaw-hb"); diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index f3be6e0cf69..21cd821d3b9 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -32,6 +32,7 @@ import { type TempPlugin = { dir: string; file: string; id: string }; type PluginLoadConfig = NonNullable[0]>["config"]; +type PluginRegistry = ReturnType; function chmodSafeDir(dir: string) { if (process.platform === "win32") { @@ -242,6 +243,64 @@ function loadRegistryFromAllowedPlugins( }); } +function runRegistryScenarios< + T extends { assert: (registry: PluginRegistry, scenario: T) => void }, +>(scenarios: readonly T[], loadRegistry: (scenario: T) => PluginRegistry) { + for (const scenario of scenarios) { + scenario.assert(loadRegistry(scenario), scenario); + } +} + +function loadRegistryFromScenarioPlugins(plugins: readonly TempPlugin[]) { + return plugins.length === 1 + ? loadRegistryFromSinglePlugin({ + plugin: plugins[0], + pluginConfig: { + allow: [plugins[0].id], + }, + }) + : loadRegistryFromAllowedPlugins([...plugins]); +} + +function expectOpenAllowWarnings(params: { + warnings: string[]; + pluginId: string; + expectedWarnings: number; + label: string; +}) { + const openAllowWarnings = params.warnings.filter((msg) => msg.includes("plugins.allow is empty")); + expect(openAllowWarnings, params.label).toHaveLength(params.expectedWarnings); + if (params.expectedWarnings > 0) { + expect( + openAllowWarnings.some((msg) => msg.includes(params.pluginId)), + params.label, + ).toBe(true); + } +} + +function expectLoadedPluginProvenance(params: { + scenario: { label: string }; + registry: PluginRegistry; + warnings: string[]; + pluginId: string; + expectWarning: boolean; + expectedSource?: string; +}) { + const plugin = params.registry.plugins.find((entry) => entry.id === params.pluginId); + expect(plugin?.status, params.scenario.label).toBe("loaded"); + if (params.expectedSource) { + expect(plugin?.source, params.scenario.label).toBe(params.expectedSource); + } + expect( + params.warnings.some( + (msg) => + msg.includes(params.pluginId) && + msg.includes("loaded without install/load-path provenance"), + ), + params.scenario.label, + ).toBe(params.expectWarning); +} + function createWarningLogger(warnings: string[]) { return { info: () => {}, @@ -1840,22 +1899,19 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip }, ] as const; - for (const scenario of scenarios) { + runRegistryScenarios(scenarios, (scenario) => { const plugin = writePlugin({ id: scenario.pluginId, filename: `${scenario.pluginId}.cjs`, body: scenario.body, }); - - const registry = loadRegistryFromSinglePlugin({ + return loadRegistryFromSinglePlugin({ plugin, pluginConfig: { allow: [scenario.pluginId], }, }); - - scenario.assert(registry); - } + }); }); it("registers plugin http routes", () => { @@ -1869,6 +1925,24 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip expectedPath: "/demo", expectedAuth: "gateway", expectedMatch: "exact", + assert: ( + registry: PluginRegistry, + scenario: { + pluginId: string; + expectedPath: string; + expectedAuth: string; + expectedMatch: string; + label: string; + }, + ) => { + const route = registry.httpRoutes.find((entry) => entry.pluginId === scenario.pluginId); + expect(route, scenario.label).toBeDefined(); + expect(route?.path, scenario.label).toBe(scenario.expectedPath); + expect(route?.auth, scenario.label).toBe(scenario.expectedAuth); + expect(route?.match, scenario.label).toBe(scenario.expectedMatch); + const httpPlugin = registry.plugins.find((entry) => entry.id === scenario.pluginId); + expect(httpPlugin?.httpRoutes, scenario.label).toBe(1); + }, }, { label: "keeps explicit auth and match options", @@ -1878,10 +1952,28 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip expectedPath: "/webhook", expectedAuth: "plugin", expectedMatch: "prefix", + assert: ( + registry: PluginRegistry, + scenario: { + pluginId: string; + expectedPath: string; + expectedAuth: string; + expectedMatch: string; + label: string; + }, + ) => { + const route = registry.httpRoutes.find((entry) => entry.pluginId === scenario.pluginId); + expect(route, scenario.label).toBeDefined(); + expect(route?.path, scenario.label).toBe(scenario.expectedPath); + expect(route?.auth, scenario.label).toBe(scenario.expectedAuth); + expect(route?.match, scenario.label).toBe(scenario.expectedMatch); + const httpPlugin = registry.plugins.find((entry) => entry.id === scenario.pluginId); + expect(httpPlugin?.httpRoutes, scenario.label).toBe(1); + }, }, ] as const; - for (const scenario of scenarios) { + runRegistryScenarios(scenarios, (scenario) => { const plugin = writePlugin({ id: scenario.pluginId, filename: `${scenario.pluginId}.cjs`, @@ -1889,22 +1981,13 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip api.registerHttpRoute(${scenario.routeOptions}); } };`, }); - - const registry = loadRegistryFromSinglePlugin({ + return loadRegistryFromSinglePlugin({ plugin, pluginConfig: { allow: [scenario.pluginId], }, }); - - const route = registry.httpRoutes.find((entry) => entry.pluginId === scenario.pluginId); - expect(route, scenario.label).toBeDefined(); - expect(route?.path, scenario.label).toBe(scenario.expectedPath); - expect(route?.auth, scenario.label).toBe(scenario.expectedAuth); - expect(route?.match, scenario.label).toBe(scenario.expectedMatch); - const httpPlugin = registry.plugins.find((entry) => entry.id === scenario.pluginId); - expect(httpPlugin?.httpRoutes, scenario.label).toBe(1); - } + }); }); it("rejects duplicate plugin registrations", () => { @@ -1920,6 +2003,26 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip selectCount: (registry: ReturnType) => registry.hooks.filter((entry) => entry.entry.hook.name === "shared-hook").length, duplicateMessage: "hook already registered: shared-hook (hook-owner-a)", + assert: ( + registry: PluginRegistry, + scenario: { + selectCount: (registry: PluginRegistry) => number; + ownerB: string; + duplicateMessage: string; + label: string; + }, + ) => { + expect(scenario.selectCount(registry), scenario.label).toBe(1); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === scenario.ownerB && + diag.message === scenario.duplicateMessage, + ), + scenario.label, + ).toBe(true); + }, }, { label: "plugin service ids", @@ -1931,6 +2034,26 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip selectCount: (registry: ReturnType) => registry.services.filter((entry) => entry.service.id === "shared-service").length, duplicateMessage: "service already registered: shared-service (service-owner-a)", + assert: ( + registry: PluginRegistry, + scenario: { + selectCount: (registry: PluginRegistry) => number; + ownerB: string; + duplicateMessage: string; + label: string; + }, + ) => { + expect(scenario.selectCount(registry), scenario.label).toBe(1); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === scenario.ownerB && + diag.message === scenario.duplicateMessage, + ), + scenario.label, + ).toBe(true); + }, }, { label: "plugin context engine ids", @@ -1942,6 +2065,26 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip selectCount: () => 1, duplicateMessage: "context engine already registered: shared-context-engine-loader-test (plugin:context-engine-owner-a)", + assert: ( + registry: PluginRegistry, + scenario: { + selectCount: (registry: PluginRegistry) => number; + ownerB: string; + duplicateMessage: string; + label: string; + }, + ) => { + expect(scenario.selectCount(registry), scenario.label).toBe(1); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === scenario.ownerB && + diag.message === scenario.duplicateMessage, + ), + scenario.label, + ).toBe(true); + }, }, { label: "plugin CLI command roots", @@ -1956,6 +2099,28 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip assertPrimaryOwner: (registry: ReturnType) => { expect(registry.cliRegistrars[0]?.pluginId).toBe("cli-owner-a"); }, + assert: ( + registry: PluginRegistry, + scenario: { + selectCount: (registry: PluginRegistry) => number; + ownerB: string; + duplicateMessage: string; + label: string; + assertPrimaryOwner?: (registry: PluginRegistry) => void; + }, + ) => { + expect(scenario.selectCount(registry), scenario.label).toBe(1); + scenario.assertPrimaryOwner?.(registry); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === scenario.ownerB && + diag.message === scenario.duplicateMessage, + ), + scenario.label, + ).toBe(true); + }, }, { label: "plugin cli backend ids", @@ -1971,10 +2136,32 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip assertPrimaryOwner: (registry: ReturnType) => { expect(registry.cliBackends?.[0]?.pluginId).toBe("cli-backend-owner-a"); }, + assert: ( + registry: PluginRegistry, + scenario: { + selectCount: (registry: PluginRegistry) => number; + ownerB: string; + duplicateMessage: string; + label: string; + assertPrimaryOwner?: (registry: PluginRegistry) => void; + }, + ) => { + expect(scenario.selectCount(registry), scenario.label).toBe(1); + scenario.assertPrimaryOwner?.(registry); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === scenario.ownerB && + diag.message === scenario.duplicateMessage, + ), + scenario.label, + ).toBe(true); + }, }, ] as const; - for (const scenario of scenarios) { + runRegistryScenarios(scenarios, (scenario) => { const first = writePlugin({ id: scenario.ownerA, filename: `${scenario.ownerA}.cjs`, @@ -1985,23 +2172,8 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip filename: `${scenario.ownerB}.cjs`, body: scenario.buildBody(scenario.ownerB), }); - - const registry = loadRegistryFromAllowedPlugins([first, second]); - - expect(scenario.selectCount(registry), scenario.label).toBe(1); - if ("assertPrimaryOwner" in scenario) { - scenario.assertPrimaryOwner?.(registry); - } - expect( - registry.diagnostics.some( - (diag) => - diag.level === "error" && - diag.pluginId === scenario.ownerB && - diag.message === scenario.duplicateMessage, - ), - scenario.label, - ).toBe(true); - } + return loadRegistryFromAllowedPlugins([first, second]); + }); }); it("rewrites removed registerHttpHandler failures into migration diagnostics", () => { @@ -2184,19 +2356,9 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip }, ] as const; - for (const scenario of scenarios) { - const plugins = scenario.buildPlugins(); - const registry = - plugins.length === 1 - ? loadRegistryFromSinglePlugin({ - plugin: plugins[0], - pluginConfig: { - allow: [plugins[0].id], - }, - }) - : loadRegistryFromAllowedPlugins(plugins); - scenario.assert(registry); - } + runRegistryScenarios(scenarios, (scenario) => + loadRegistryFromScenarioPlugins(scenario.buildPlugins()), + ); }); it("respects explicit disable in config", () => { @@ -2669,10 +2831,7 @@ module.exports = { }, ] as const; - for (const { loadRegistry, assert } of scenarios) { - const registry = loadRegistry(); - assert(registry); - } + runRegistryScenarios(scenarios, ({ loadRegistry }) => loadRegistry()); }); it("resolves duplicate plugin ids by source precedence", () => { @@ -2710,6 +2869,25 @@ module.exports = { }, expectedLoadedOrigin: "config", expectedDisabledOrigin: "bundled", + assert: ( + registry: PluginRegistry, + scenario: { + pluginId: string; + expectedLoadedOrigin: string; + expectedDisabledOrigin: string; + label: string; + expectedDisabledError?: string; + }, + ) => { + const entries = registry.plugins.filter((entry) => entry.id === scenario.pluginId); + const loaded = entries.find((entry) => entry.status === "loaded"); + const overridden = entries.find((entry) => entry.status === "disabled"); + expect(loaded?.origin, scenario.label).toBe(scenario.expectedLoadedOrigin); + expect(overridden?.origin, scenario.label).toBe(scenario.expectedDisabledOrigin); + if (scenario.expectedDisabledError) { + expect(overridden?.error, scenario.label).toContain(scenario.expectedDisabledError); + } + }, }, { label: "bundled beats auto-discovered global duplicate", @@ -2752,6 +2930,25 @@ module.exports = { expectedLoadedOrigin: "bundled", expectedDisabledOrigin: "global", expectedDisabledError: "overridden by bundled plugin", + assert: ( + registry: PluginRegistry, + scenario: { + pluginId: string; + expectedLoadedOrigin: string; + expectedDisabledOrigin: string; + label: string; + expectedDisabledError?: string; + }, + ) => { + const entries = registry.plugins.filter((entry) => entry.id === scenario.pluginId); + const loaded = entries.find((entry) => entry.status === "loaded"); + const overridden = entries.find((entry) => entry.status === "disabled"); + expect(loaded?.origin, scenario.label).toBe(scenario.expectedLoadedOrigin); + expect(overridden?.origin, scenario.label).toBe(scenario.expectedDisabledOrigin); + if (scenario.expectedDisabledError) { + expect(overridden?.error, scenario.label).toContain(scenario.expectedDisabledError); + } + }, }, { label: "installed global beats bundled duplicate", @@ -2800,20 +2997,29 @@ module.exports = { expectedLoadedOrigin: "global", expectedDisabledOrigin: "bundled", expectedDisabledError: "overridden by global plugin", + assert: ( + registry: PluginRegistry, + scenario: { + pluginId: string; + expectedLoadedOrigin: string; + expectedDisabledOrigin: string; + label: string; + expectedDisabledError?: string; + }, + ) => { + const entries = registry.plugins.filter((entry) => entry.id === scenario.pluginId); + const loaded = entries.find((entry) => entry.status === "loaded"); + const overridden = entries.find((entry) => entry.status === "disabled"); + expect(loaded?.origin, scenario.label).toBe(scenario.expectedLoadedOrigin); + expect(overridden?.origin, scenario.label).toBe(scenario.expectedDisabledOrigin); + if (scenario.expectedDisabledError) { + expect(overridden?.error, scenario.label).toContain(scenario.expectedDisabledError); + } + }, }, ] as const; - for (const scenario of scenarios) { - const registry = scenario.loadRegistry(); - const entries = registry.plugins.filter((entry) => entry.id === scenario.pluginId); - const loaded = entries.find((entry) => entry.status === "loaded"); - const overridden = entries.find((entry) => entry.status === "disabled"); - expect(loaded?.origin, scenario.label).toBe(scenario.expectedLoadedOrigin); - expect(overridden?.origin, scenario.label).toBe(scenario.expectedDisabledOrigin); - if ("expectedDisabledError" in scenario) { - expect(overridden?.error, scenario.label).toContain(scenario.expectedDisabledError); - } - } + runRegistryScenarios(scenarios, (scenario) => scenario.loadRegistry()); }); it("warns about open allowlists only for auto-discovered plugins", () => { @@ -2883,14 +3089,12 @@ module.exports = { scenario.loadRegistry(warnings); } - const openAllowWarnings = warnings.filter((msg) => msg.includes("plugins.allow is empty")); - expect(openAllowWarnings, scenario.label).toHaveLength(scenario.expectedWarnings); - if (scenario.expectedWarnings > 0) { - expect( - openAllowWarnings.some((msg) => msg.includes(scenario.pluginId)), - scenario.label, - ).toBe(true); - } + expectOpenAllowWarnings({ + warnings, + pluginId: scenario.pluginId, + expectedWarnings: scenario.expectedWarnings, + label: scenario.label, + }); } }); @@ -3017,10 +3221,7 @@ module.exports = { }, ] as const; - for (const scenario of scenarios) { - const registry = scenario.loadRegistry(); - scenario.assert(registry); - } + runRegistryScenarios(scenarios, (scenario) => scenario.loadRegistry()); }); it("loads bundled plugins when manifest metadata opts into default enablement", () => { @@ -3186,21 +3387,15 @@ module.exports = { for (const scenario of scenarios) { const loadedScenario = scenario.loadRegistry(); - const { registry, warnings, pluginId, expectWarning } = loadedScenario; const expectedSource = - "expectedSource" in loadedScenario ? loadedScenario.expectedSource : undefined; - const plugin = registry.plugins.find((entry) => entry.id === pluginId); - expect(plugin?.status, scenario.label).toBe("loaded"); - if (expectedSource) { - expect(plugin?.source, scenario.label).toBe(expectedSource); - } - expect( - warnings.some( - (msg) => - msg.includes(pluginId) && msg.includes("loaded without install/load-path provenance"), - ), - scenario.label, - ).toBe(expectWarning); + "expectedSource" in loadedScenario && typeof loadedScenario.expectedSource === "string" + ? loadedScenario.expectedSource + : undefined; + expectLoadedPluginProvenance({ + scenario, + ...loadedScenario, + expectedSource, + }); } }); diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 9c5db29d317..48f6bf7b625 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -215,6 +215,19 @@ async function audit( }); } +async function runAuditCases< + T extends { + run: () => Promise; + assert: (res: SecurityAuditReport) => void; + }, +>(cases: readonly T[]) { + await Promise.all( + cases.map(async ({ run, assert }) => { + assert(await run()); + }), + ); +} + function hasFinding(res: SecurityAuditReport, checkId: string, severity?: string): boolean { return res.findings.some( (f) => f.checkId === checkId && (severity == null || f.severity === severity), @@ -553,12 +566,7 @@ description: test skill }, ] as const; - await Promise.all( - cases.map(async (testCase) => { - const res = await testCase.run(); - testCase.assert(res); - }), - ); + await runAuditCases(cases); }); it("scores dangerous gateway.tools.allow over HTTP by exposure", async () => { @@ -1231,12 +1239,7 @@ description: test skill }, ] as const; - await Promise.all( - cases.map(async (testCase) => { - const res = await testCase.run(); - testCase.assert(res); - }), - ); + await runAuditCases(cases); }); it("uses symlink target permissions for config checks", async () => { @@ -1471,21 +1474,23 @@ description: test skill }, ] as const; - await Promise.all( - cases.map(async (testCase) => { - const res = await audit(testCase.cfg); - if (testCase.expectedFindings.length > 0) { - expect(res.findings, testCase.name).toEqual( - expect.arrayContaining( - testCase.expectedFindings.map((finding) => expect.objectContaining(finding)), - ), - ); - } - const expectedAbsent = "expectedAbsent" in testCase ? testCase.expectedAbsent : []; - for (const checkId of expectedAbsent) { - expect(hasFinding(res, checkId), `${testCase.name}:${checkId}`).toBe(false); - } - }), + await runAuditCases( + cases.map((testCase) => ({ + run: () => audit(testCase.cfg), + assert: (res: SecurityAuditReport) => { + if (testCase.expectedFindings.length > 0) { + expect(res.findings, testCase.name).toEqual( + expect.arrayContaining( + testCase.expectedFindings.map((finding) => expect.objectContaining(finding)), + ), + ); + } + const expectedAbsent = "expectedAbsent" in testCase ? testCase.expectedAbsent : []; + for (const checkId of expectedAbsent) { + expect(hasFinding(res, checkId), `${testCase.name}:${checkId}`).toBe(false); + } + }, + })), ); }); @@ -1527,21 +1532,23 @@ description: test skill }, ] as const; - await Promise.all( - cases.map(async (testCase) => { - const res = await audit(testCase.cfg); - const finding = res.findings.find( - (f) => f.checkId === "gateway.nodes.deny_commands_ineffective", - ); - expect(finding?.severity, testCase.name).toBe("warn"); - for (const text of testCase.detailIncludes) { - expect(finding?.detail, `${testCase.name}:${text}`).toContain(text); - } - const detailExcludes = "detailExcludes" in testCase ? testCase.detailExcludes : []; - for (const text of detailExcludes) { - expect(finding?.detail, `${testCase.name}:${text}`).not.toContain(text); - } - }), + await runAuditCases( + cases.map((testCase) => ({ + run: () => audit(testCase.cfg), + assert: (res: SecurityAuditReport) => { + const finding = res.findings.find( + (f) => f.checkId === "gateway.nodes.deny_commands_ineffective", + ); + expect(finding?.severity, testCase.name).toBe("warn"); + for (const text of testCase.detailIncludes) { + expect(finding?.detail, `${testCase.name}:${text}`).toContain(text); + } + const detailExcludes = "detailExcludes" in testCase ? testCase.detailExcludes : []; + for (const text of detailExcludes) { + expect(finding?.detail, `${testCase.name}:${text}`).not.toContain(text); + } + }, + })), ); }); @@ -1581,26 +1588,28 @@ description: test skill }, ] as const; - await Promise.all( - cases.map(async (testCase) => { - const res = await audit(testCase.cfg); - if ("expectedAbsent" in testCase && testCase.expectedAbsent) { - expectNoFinding(res, "gateway.nodes.allow_commands_dangerous"); - return; - } - const expectedSeverity = - "expectedSeverity" in testCase ? testCase.expectedSeverity : undefined; - if (!expectedSeverity) { - return; - } + await runAuditCases( + cases.map((testCase) => ({ + run: () => audit(testCase.cfg), + assert: (res: SecurityAuditReport) => { + if ("expectedAbsent" in testCase && testCase.expectedAbsent) { + expectNoFinding(res, "gateway.nodes.allow_commands_dangerous"); + return; + } + const expectedSeverity = + "expectedSeverity" in testCase ? testCase.expectedSeverity : undefined; + if (!expectedSeverity) { + return; + } - const finding = res.findings.find( - (f) => f.checkId === "gateway.nodes.allow_commands_dangerous", - ); - expect(finding?.severity, testCase.name).toBe(expectedSeverity); - expect(finding?.detail, testCase.name).toContain("camera.snap"); - expect(finding?.detail, testCase.name).toContain("screen.record"); - }), + const finding = res.findings.find( + (f) => f.checkId === "gateway.nodes.allow_commands_dangerous", + ); + expect(finding?.severity, testCase.name).toBe(expectedSeverity); + expect(finding?.detail, testCase.name).toContain("camera.snap"); + expect(finding?.detail, testCase.name).toContain("screen.record"); + }, + })), ); });