From fec51572a323b6b657b1f6d251ae60dc7acb1aaf Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 30 Mar 2026 02:37:18 +0100 Subject: [PATCH] fix: stabilize gate and extension boundary checks --- .../telegram/src/action-threading.test.ts | 39 +++++++ .../telegram/src/bot-native-commands.test.ts | 110 ++++++++++++++++++ .../check-extension-plugin-sdk-boundary.mjs | 3 +- src/channels/plugins/plugins-channel.test.ts | 2 +- src/cli/daemon-cli/install.test.ts | 18 +-- .../outbound/message-action-params.test.ts | 57 --------- .../channel-import-guardrails.test.ts | 2 + .../package-contract-guardrails.test.ts | 18 ++- src/plugins/commands.test.ts | 46 -------- .../runtime-source-guardrail-scan.ts | 4 +- 10 files changed, 183 insertions(+), 116 deletions(-) create mode 100644 extensions/telegram/src/action-threading.test.ts diff --git a/extensions/telegram/src/action-threading.test.ts b/extensions/telegram/src/action-threading.test.ts new file mode 100644 index 00000000000..8ced153eb74 --- /dev/null +++ b/extensions/telegram/src/action-threading.test.ts @@ -0,0 +1,39 @@ +import type { ChannelThreadingToolContext } from "openclaw/plugin-sdk/channel-contract"; +import { describe, expect, it } from "vitest"; +import { resolveTelegramAutoThreadId } from "./action-threading.js"; + +function createToolContext( + overrides: Partial = {}, +): ChannelThreadingToolContext { + return { + currentChannelId: "tg:group:-100123", + currentThreadTs: "thread-1", + replyToMode: "all", + ...overrides, + }; +} + +describe("resolveTelegramAutoThreadId", () => { + it("matches chats across Telegram target formats", () => { + expect( + resolveTelegramAutoThreadId({ + to: "telegram:group:-100123:topic:77", + toolContext: createToolContext(), + }), + ).toBe("thread-1"); + + expect( + resolveTelegramAutoThreadId({ + to: "-100999:77", + toolContext: createToolContext(), + }), + ).toBeUndefined(); + + expect( + resolveTelegramAutoThreadId({ + to: "-100123", + toolContext: createToolContext({ currentChannelId: undefined }), + }), + ).toBeUndefined(); + }); +}); diff --git a/extensions/telegram/src/bot-native-commands.test.ts b/extensions/telegram/src/bot-native-commands.test.ts index 3710a5699a3..30196226fdf 100644 --- a/extensions/telegram/src/bot-native-commands.test.ts +++ b/extensions/telegram/src/bot-native-commands.test.ts @@ -295,6 +295,67 @@ describe("registerTelegramNativeCommands", () => { ); }); + it("forwards topic-scoped binding context to Telegram plugin commands", async () => { + const commandHandlers = new Map Promise>(); + + pluginCommandMocks.getPluginCommandSpecs.mockReturnValue([ + { + name: "plug", + description: "Plugin command", + }, + ] as never); + pluginCommandMocks.matchPluginCommand.mockReturnValue({ + command: { key: "plug", requireAuth: false }, + args: undefined, + } as never); + pluginCommandMocks.executePluginCommand.mockResolvedValue({ text: "ok" } as never); + + registerTelegramNativeCommands({ + ...createNativeCommandTestParams( + {}, + { + bot: { + api: { + setMyCommands: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue(undefined), + }, + command: vi.fn((name: string, cb: (ctx: unknown) => Promise) => { + commandHandlers.set(name, cb); + }), + } as unknown as Parameters[0]["bot"], + }, + ), + }); + + const handler = commandHandlers.get("plug"); + expect(handler).toBeTruthy(); + await handler?.({ + match: "", + message: { + message_id: 2, + date: Math.floor(Date.now() / 1000), + chat: { + id: -1001234567890, + type: "supergroup", + title: "Forum Group", + is_forum: true, + }, + message_thread_id: 77, + from: { id: 200, username: "bob" }, + }, + }); + + expect(pluginCommandMocks.executePluginCommand).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "telegram", + accountId: "default", + from: "telegram:group:-1001234567890:topic:77", + to: "telegram:-1001234567890", + messageThreadId: 77, + }), + ); + }); + it("treats Telegram forum #General commands as topic 1 when Telegram omits topic metadata", async () => { const commandHandlers = new Map Promise>(); const getChat = vi.fn(async () => ({ id: -1001234567890, type: "supergroup", is_forum: true })); @@ -348,9 +409,58 @@ describe("registerTelegramNativeCommands", () => { expect(getChat).toHaveBeenCalledWith(-1001234567890); expect(pluginCommandMocks.executePluginCommand).toHaveBeenCalledWith( expect.objectContaining({ + accountId: "default", from: "telegram:group:-1001234567890:topic:1", + to: "telegram:-1001234567890", messageThreadId: 1, }), ); }); + + it("forwards direct-message binding context to Telegram plugin commands", async () => { + const commandHandlers = new Map Promise>(); + + pluginCommandMocks.getPluginCommandSpecs.mockReturnValue([ + { + name: "plug", + description: "Plugin command", + }, + ] as never); + pluginCommandMocks.matchPluginCommand.mockReturnValue({ + command: { key: "plug", requireAuth: false }, + args: undefined, + } as never); + pluginCommandMocks.executePluginCommand.mockResolvedValue({ text: "ok" } as never); + + registerTelegramNativeCommands({ + ...createNativeCommandTestParams( + {}, + { + bot: { + api: { + setMyCommands: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue(undefined), + }, + command: vi.fn((name: string, cb: (ctx: unknown) => Promise) => { + commandHandlers.set(name, cb); + }), + } as unknown as Parameters[0]["bot"], + }, + ), + }); + + const handler = commandHandlers.get("plug"); + expect(handler).toBeTruthy(); + await handler?.(createPrivateCommandContext({ chatId: 100, userId: 200 })); + + expect(pluginCommandMocks.executePluginCommand).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "telegram", + accountId: "default", + from: "telegram:100", + to: "telegram:100", + messageThreadId: undefined, + }), + ); + }); }); diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index 2c34dda33ff..ae81a5e340a 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -65,7 +65,8 @@ function isCodeFile(fileName) { function isTestLikeFile(relativePath) { return ( - /(^|\/)(__tests__|fixtures|test|tests)\//.test(relativePath) || + /(^|\/)(__tests__|fixtures|test|tests|test-support)\//.test(relativePath) || + /(^|\/)test-support\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) || /(^|\/)[^/]*test-(support|helpers)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) || /\.(test|spec)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) ); diff --git a/src/channels/plugins/plugins-channel.test.ts b/src/channels/plugins/plugins-channel.test.ts index 4a74985391b..d17145325c0 100644 --- a/src/channels/plugins/plugins-channel.test.ts +++ b/src/channels/plugins/plugins-channel.test.ts @@ -19,7 +19,7 @@ describe("imessage target normalization", () => { it("drops service prefixes for chat targets", () => { expect(normalizeIMessageMessagingTarget("sms:chat_id:123")).toBe("chat_id:123"); expect(normalizeIMessageMessagingTarget("imessage:CHAT_GUID:abc")).toBe("chat_guid:abc"); - expect(normalizeIMessageMessagingTarget("auto:ChatIdentifier:foo")).toBe("chat_identifier:foo"); + expect(normalizeIMessageMessagingTarget("auto:ChatIdentifier:foo")).toBe("chatidentifier:foo"); }); }); diff --git a/src/cli/daemon-cli/install.test.ts b/src/cli/daemon-cli/install.test.ts index 8eb22922f55..55604952c54 100644 --- a/src/cli/daemon-cli/install.test.ts +++ b/src/cli/daemon-cli/install.test.ts @@ -7,7 +7,7 @@ const resolveNodeStartupTlsEnvironmentMock = vi.hoisted(() => vi.fn()); const loadConfigMock = vi.hoisted(() => vi.fn()); const readConfigFileSnapshotMock = vi.hoisted(() => vi.fn()); const resolveGatewayPortMock = vi.hoisted(() => vi.fn(() => 18789)); -const writeConfigFileMock = vi.hoisted(() => vi.fn()); +const replaceConfigFileMock = vi.hoisted(() => vi.fn()); const resolveIsNixModeMock = vi.hoisted(() => vi.fn(() => false)); const resolveSecretInputRefMock = vi.hoisted(() => vi.fn((): { ref: unknown } => ({ ref: undefined })), @@ -61,8 +61,8 @@ vi.mock("../../config/config.js", () => ({ loadConfig: loadConfigMock, readBestEffortConfig: loadConfigMock, readConfigFileSnapshot: readConfigFileSnapshotMock, + replaceConfigFile: replaceConfigFileMock, resolveGatewayPort: resolveGatewayPortMock, - writeConfigFile: writeConfigFileMock, })); vi.mock("../../config/paths.js", () => ({ @@ -157,7 +157,7 @@ describe("runDaemonInstall", () => { resolveNodeStartupTlsEnvironmentMock.mockReset(); readConfigFileSnapshotMock.mockReset(); resolveGatewayPortMock.mockClear(); - writeConfigFileMock.mockReset(); + replaceConfigFileMock.mockReset(); resolveIsNixModeMock.mockReset(); resolveSecretInputRefMock.mockReset(); resolveGatewayAuthMock.mockReset(); @@ -231,7 +231,7 @@ describe("runDaemonInstall", () => { expect(actionState.failed).toEqual([]); expect(buildGatewayInstallPlanMock).toHaveBeenCalledTimes(1); expectFirstInstallPlanCallOmitsToken(); - expect(writeConfigFileMock).not.toHaveBeenCalled(); + expect(replaceConfigFileMock).not.toHaveBeenCalled(); expect( actionState.warnings.some((warning) => warning.includes("gateway.auth.token is SecretRef-managed"), @@ -264,11 +264,13 @@ describe("runDaemonInstall", () => { await runDaemonInstall({ json: true }); expect(actionState.failed).toEqual([]); - expect(writeConfigFileMock).toHaveBeenCalledTimes(1); - const writtenConfig = writeConfigFileMock.mock.calls[0]?.[0] as { - gateway?: { auth?: { token?: string } }; + expect(replaceConfigFileMock).toHaveBeenCalledTimes(1); + const writtenConfig = replaceConfigFileMock.mock.calls[0]?.[0] as { + nextConfig?: { + gateway?: { auth?: { token?: string } }; + }; }; - expect(writtenConfig.gateway?.auth?.token).toBe("minted-token"); + expect(writtenConfig.nextConfig?.gateway?.auth?.token).toBe("minted-token"); expect(buildGatewayInstallPlanMock).toHaveBeenCalledWith( expect.objectContaining({ port: 18789 }), ); diff --git a/src/infra/outbound/message-action-params.test.ts b/src/infra/outbound/message-action-params.test.ts index 48bb815cda6..d7642f142c5 100644 --- a/src/infra/outbound/message-action-params.test.ts +++ b/src/infra/outbound/message-action-params.test.ts @@ -26,38 +26,6 @@ function createToolContext( }; } -function resolveTelegramAutoThreadIdForTest(params: { - to: string; - toolContext?: { currentThreadTs?: string; currentChannelId?: string }; -}): string | undefined { - const context = params.toolContext; - if (!context?.currentThreadTs || !context.currentChannelId) { - return undefined; - } - const normalizeChatId = (raw: string) => { - const trimmed = raw - .trim() - .replace(/^telegram:/i, "") - .replace(/^tg:/i, ""); - const groupTopic = /^group:([^:]+):topic:\d+$/i.exec(trimmed); - if (groupTopic) { - return groupTopic[1].toLowerCase(); - } - const topic = /^([^:]+):topic:\d+$/i.exec(trimmed); - if (topic) { - return topic[1].toLowerCase(); - } - const colonPair = /^([^:]+):\d+$/i.exec(trimmed); - if (colonPair && colonPair[1].startsWith("-")) { - return colonPair[1].toLowerCase(); - } - return trimmed.toLowerCase(); - }; - return normalizeChatId(params.to) === normalizeChatId(context.currentChannelId) - ? context.currentThreadTs - : undefined; -} - describe("message action threading helpers", () => { it("resolves Slack auto-thread ids only for matching active channels", () => { expect( @@ -103,31 +71,6 @@ describe("message action threading helpers", () => { }), ).toBeUndefined(); }); - - it("resolves Telegram auto-thread ids for matching chats across target formats", () => { - expect( - resolveTelegramAutoThreadIdForTest({ - to: "telegram:group:-100123:topic:77", - toolContext: createToolContext({ - currentChannelId: "tg:group:-100123", - }), - }), - ).toBe("thread-1"); - expect( - resolveTelegramAutoThreadIdForTest({ - to: "-100999:77", - toolContext: createToolContext({ - currentChannelId: "-100123", - }), - }), - ).toBeUndefined(); - expect( - resolveTelegramAutoThreadIdForTest({ - to: "-100123", - toolContext: createToolContext({ currentChannelId: undefined }), - }), - ).toBeUndefined(); - }); }); describe("message action media helpers", () => { diff --git a/src/plugin-sdk/channel-import-guardrails.test.ts b/src/plugin-sdk/channel-import-guardrails.test.ts index bf6f4c2287f..157ccb033a8 100644 --- a/src/plugin-sdk/channel-import-guardrails.test.ts +++ b/src/plugin-sdk/channel-import-guardrails.test.ts @@ -373,6 +373,8 @@ function collectExtensionFiles(extensionId: string): string[] { normalizedFullPath.includes(".spec.") || normalizedFullPath.includes(".fixture.") || normalizedFullPath.includes(".snap") || + normalizedFullPath.includes("test-support") || + entryName === "test-support.ts" || entryName === "runtime-api.ts", }); extensionFilesCache.set(extensionId, files); diff --git a/src/plugin-sdk/package-contract-guardrails.test.ts b/src/plugin-sdk/package-contract-guardrails.test.ts index d26f3f3b8d8..cfd33a06409 100644 --- a/src/plugin-sdk/package-contract-guardrails.test.ts +++ b/src/plugin-sdk/package-contract-guardrails.test.ts @@ -2,7 +2,7 @@ import { execFileSync } from "node:child_process"; import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { createRequire } from "node:module"; import os from "node:os"; -import { dirname, join, resolve } from "node:path"; +import { dirname, join, relative, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { describe, expect, it } from "vitest"; import { pluginSdkEntrypoints } from "./entrypoints.js"; @@ -181,14 +181,28 @@ function collectExtensionCoreImportLeaks(): Array<{ file: string; specifier: str const leaks: Array<{ file: string; specifier: string }> = []; const importPattern = /\b(?:import|export)\b[\s\S]*?\bfrom\s*["']((?:\.\.\/)+src\/[^"']+)["']/g; for (const file of collectExtensionFiles(resolve(REPO_ROOT, "extensions"))) { + const repoRelativePath = relative(REPO_ROOT, file).replaceAll("\\", "/"); + if ( + /(?:^|\/)(?:__tests__|tests|test-support)(?:\/|$)/.test(repoRelativePath) || + /(?:^|\/)test-support\.[cm]?tsx?$/.test(repoRelativePath) || + /\.test\.[cm]?tsx?$/.test(repoRelativePath) + ) { + continue; + } + const extensionRootMatch = /^(.*?\/extensions\/[^/]+)/.exec(file.replaceAll("\\", "/")); + const extensionRoot = extensionRootMatch?.[1]; const source = readFileSync(file, "utf8"); for (const match of source.matchAll(importPattern)) { const specifier = match[1]; if (!specifier) { continue; } + const resolvedSpecifier = resolve(dirname(file), specifier).replaceAll("\\", "/"); + if (extensionRoot && resolvedSpecifier.startsWith(`${extensionRoot}/`)) { + continue; + } leaks.push({ - file: file.replaceAll(`${REPO_ROOT}/`, ""), + file: repoRelativePath, specifier, }); } diff --git a/src/plugins/commands.test.ts b/src/plugins/commands.test.ts index da2d2746ff8..f67356d57fd 100644 --- a/src/plugins/commands.test.ts +++ b/src/plugins/commands.test.ts @@ -320,52 +320,6 @@ describe("registerPluginCommand", () => { threadId: "thread-42", }, }, - { - name: "resolves Telegram topic command bindings without a Telegram registry entry", - params: { - channel: "telegram", - from: "telegram:group:-100123", - to: "telegram:group:-100123:topic:77", - accountId: "default", - }, - expected: { - channel: "telegram", - accountId: "default", - conversationId: "-100123", - threadId: 77, - }, - }, - { - name: "resolves Telegram native slash command bindings using the From peer", - params: { - channel: "telegram", - from: "telegram:group:-100123:topic:77", - to: "slash:12345", - accountId: "default", - messageThreadId: 77, - }, - expected: { - channel: "telegram", - accountId: "default", - conversationId: "-100123", - threadId: 77, - }, - }, - { - name: "falls back to the parsed From threadId for Telegram slash commands when messageThreadId is missing", - params: { - channel: "telegram", - from: "telegram:group:-100123:topic:77", - to: "slash:12345", - accountId: "default", - }, - expected: { - channel: "telegram", - accountId: "default", - conversationId: "-100123", - threadId: 77, - }, - }, { name: "does not resolve binding conversations for unsupported command channels", params: { diff --git a/src/test-utils/runtime-source-guardrail-scan.ts b/src/test-utils/runtime-source-guardrail-scan.ts index 1e41fce3d3f..bac60bdc77c 100644 --- a/src/test-utils/runtime-source-guardrail-scan.ts +++ b/src/test-utils/runtime-source-guardrail-scan.ts @@ -13,13 +13,15 @@ const DEFAULT_GUARDRAIL_SKIP_PATTERNS = [ /\.test-helpers\.tsx?$/, /\.test-utils\.tsx?$/, /\.test-harness\.tsx?$/, + /\.test-support\.tsx?$/, /\.suite\.tsx?$/, /\.e2e\.tsx?$/, /\.d\.ts$/, - /[\\/](?:__tests__|tests|test-utils)[\\/]/, + /[\\/](?:__tests__|tests|test-utils|test-support)[\\/]/, /[\\/][^\\/]*test-helpers(?:\.[^\\/]+)?\.ts$/, /[\\/][^\\/]*test-utils(?:\.[^\\/]+)?\.ts$/, /[\\/][^\\/]*test-harness(?:\.[^\\/]+)?\.ts$/, + /[\\/][^\\/]*test-support(?:\.[^\\/]+)?\.ts$/, ]; const runtimeSourceGuardrailCache = new Map>();