diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c945628d77..048e26ebd46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - Security/dependency audit: force `basic-ftp` to `5.2.1` to pick up the CRLF command-injection fix from GHSA-chqc-8p9q-pq6q. - Security/dependency audit: bump Hono to `4.12.12` and `@hono/node-server` to `1.19.13` in production resolution paths. - Slack/partial streaming: keep the final fallback reply path active when preview finalization fails so stale preview text cannot suppress the actual final answer. (#62859) Thanks @gumadeiras. +- Plugins/contracts: keep test-only helpers out of production contract barrels, load shared contract harnesses through bundled test surfaces, and harden guardrails so indirect re-exports and canonical `*.test.ts` files stay blocked. (#63311) Thanks @altaywtf. ## 2026.4.8 diff --git a/extensions/imessage/contract-api.ts b/extensions/imessage/contract-api.ts index c962d22f63c..f6a80c0dd53 100644 --- a/extensions/imessage/contract-api.ts +++ b/extensions/imessage/contract-api.ts @@ -1,4 +1,3 @@ -export { createIMessageTestPlugin } from "./src/test-plugin.js"; export { resolveIMessageAttachmentRoots as resolveInboundAttachmentRoots, resolveIMessageRemoteAttachmentRoots as resolveRemoteInboundAttachmentRoots, diff --git a/extensions/imessage/src/test-plugin.ts b/extensions/imessage/src/imessage.test-plugin.ts similarity index 86% rename from extensions/imessage/src/test-plugin.ts rename to extensions/imessage/src/imessage.test-plugin.ts index 43ed004aee8..5187106de35 100644 --- a/extensions/imessage/src/test-plugin.ts +++ b/extensions/imessage/src/imessage.test-plugin.ts @@ -5,20 +5,32 @@ import { collectStatusIssuesFromLastError } from "openclaw/plugin-sdk/status-hel import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime"; function normalizeIMessageTestHandle(raw: string): string { - const trimmed = raw.trim(); + let trimmed = raw.trim(); if (!trimmed) { return ""; } - const lowered = normalizeLowercaseStringOrEmpty(trimmed); - if (lowered.startsWith("imessage:")) { - return normalizeIMessageTestHandle(trimmed.slice("imessage:".length)); + + while (trimmed) { + const lowered = normalizeLowercaseStringOrEmpty(trimmed); + if (lowered.startsWith("imessage:")) { + trimmed = trimmed.slice("imessage:".length).trim(); + continue; + } + if (lowered.startsWith("sms:")) { + trimmed = trimmed.slice("sms:".length).trim(); + continue; + } + if (lowered.startsWith("auto:")) { + trimmed = trimmed.slice("auto:".length).trim(); + continue; + } + break; } - if (lowered.startsWith("sms:")) { - return normalizeIMessageTestHandle(trimmed.slice("sms:".length)); - } - if (lowered.startsWith("auto:")) { - return normalizeIMessageTestHandle(trimmed.slice("auto:".length)); + + if (!trimmed) { + return ""; } + if (/^(chat_id:|chat_guid:|chat_identifier:)/i.test(trimmed)) { return trimmed.replace(/^(chat_id:|chat_guid:|chat_identifier:)/i, (match) => normalizeLowercaseStringOrEmpty(match), diff --git a/extensions/imessage/src/test-plugin.test.ts b/extensions/imessage/src/test-plugin.test.ts index c6f13ea8716..5c6ec677cda 100644 --- a/extensions/imessage/src/test-plugin.test.ts +++ b/extensions/imessage/src/test-plugin.test.ts @@ -3,7 +3,7 @@ import { listImportedBundledPluginFacadeIds, resetFacadeRuntimeStateForTest, } from "../../../src/plugin-sdk/facade-runtime.js"; -import { createIMessageTestPlugin } from "./test-plugin.js"; +import { createIMessageTestPlugin } from "./imessage.test-plugin.js"; beforeEach(() => { resetFacadeRuntimeStateForTest(); @@ -21,4 +21,11 @@ describe("createIMessageTestPlugin", () => { expect(listImportedBundledPluginFacadeIds()).toEqual([]); }); + + it("normalizes repeated transport prefixes without recursive stack growth", () => { + const plugin = createIMessageTestPlugin(); + const prefixedHandle = `${"imessage:".repeat(5000)}+44 20 7946 0958`; + + expect(plugin.messaging?.normalizeTarget?.(prefixedHandle)).toBe("+442079460958"); + }); }); diff --git a/extensions/imessage/test-api.ts b/extensions/imessage/test-api.ts new file mode 100644 index 00000000000..c0939502702 --- /dev/null +++ b/extensions/imessage/test-api.ts @@ -0,0 +1 @@ +export { createIMessageTestPlugin } from "./src/imessage.test-plugin.js"; diff --git a/extensions/slack/contract-api.ts b/extensions/slack/contract-api.ts index ef0d709d573..dbd0586267e 100644 --- a/extensions/slack/contract-api.ts +++ b/extensions/slack/contract-api.ts @@ -3,7 +3,6 @@ export { collectRuntimeConfigAssignments, secretTargetRegistryEntries, } from "./src/secret-contract.js"; -export { createSlackOutboundPayloadHarness } from "./src/outbound-payload-harness.js"; export type { SlackInteractiveHandlerContext, SlackInteractiveHandlerRegistration, diff --git a/extensions/slack/src/outbound-payload-harness.ts b/extensions/slack/src/outbound-payload.test-harness.ts similarity index 100% rename from extensions/slack/src/outbound-payload-harness.ts rename to extensions/slack/src/outbound-payload.test-harness.ts diff --git a/extensions/slack/src/outbound-payload.test.ts b/extensions/slack/src/outbound-payload.test.ts index 466a1928a38..639a97929f4 100644 --- a/extensions/slack/src/outbound-payload.test.ts +++ b/extensions/slack/src/outbound-payload.test.ts @@ -1,6 +1,6 @@ import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime"; import { describe, expect, it } from "vitest"; -import { createSlackOutboundPayloadHarness } from "../contract-api.js"; +import { createSlackOutboundPayloadHarness } from "../test-api.js"; function createHarness(params: { payload: ReplyPayload; diff --git a/extensions/slack/test-api.ts b/extensions/slack/test-api.ts index 5e398104009..16046773255 100644 --- a/extensions/slack/test-api.ts +++ b/extensions/slack/test-api.ts @@ -3,6 +3,7 @@ export type { SlackMessageEvent } from "./src/types.js"; export { slackPlugin } from "./src/channel.js"; export { setSlackRuntime } from "./src/runtime.js"; export { createSlackActions } from "./src/channel-actions.js"; +export { createSlackOutboundPayloadHarness } from "./src/outbound-payload.test-harness.js"; export { prepareSlackMessage } from "./src/monitor/message-handler/prepare.js"; export { createInboundSlackTestContext } from "./src/monitor/message-handler/prepare.test-helpers.js"; export { slackOutbound } from "./src/outbound-adapter.js"; diff --git a/extensions/whatsapp/contract-api.ts b/extensions/whatsapp/contract-api.ts index 2e0138d49cf..1e0dc98c8c1 100644 --- a/extensions/whatsapp/contract-api.ts +++ b/extensions/whatsapp/contract-api.ts @@ -5,10 +5,6 @@ import { isWhatsAppGroupJid as isWhatsAppGroupJidImpl, normalizeWhatsAppTarget as normalizeWhatsAppTargetImpl, } from "./src/normalize-target.js"; -import { - createWhatsAppPollFixture as createWhatsAppPollFixtureImpl, - expectWhatsAppPollSent as expectWhatsAppPollSentImpl, -} from "./src/outbound-test-support.js"; import { resolveWhatsAppRuntimeGroupPolicy as resolveWhatsAppRuntimeGroupPolicyImpl } from "./src/runtime-group-policy.js"; import { canonicalizeLegacySessionKey as canonicalizeLegacySessionKeyImpl, @@ -20,8 +16,6 @@ export { } from "./src/security-contract.js"; export const canonicalizeLegacySessionKey = canonicalizeLegacySessionKeyImpl; -export const createWhatsAppPollFixture = createWhatsAppPollFixtureImpl; -export const expectWhatsAppPollSent = expectWhatsAppPollSentImpl; export const isLegacyGroupSessionKey = isLegacyGroupSessionKeyImpl; export const isWhatsAppGroupJid = isWhatsAppGroupJidImpl; export const normalizeWhatsAppTarget = normalizeWhatsAppTargetImpl; diff --git a/extensions/whatsapp/src/outbound-base.test.ts b/extensions/whatsapp/src/outbound-base.test.ts index 20ca187ff0e..d158968ca5a 100644 --- a/extensions/whatsapp/src/outbound-base.test.ts +++ b/extensions/whatsapp/src/outbound-base.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { createWhatsAppPollFixture, expectWhatsAppPollSent } from "../contract-api.js"; +import { createWhatsAppPollFixture, expectWhatsAppPollSent } from "../test-api.js"; import { createWhatsAppOutboundBase } from "./outbound-base.js"; describe("createWhatsAppOutboundBase", () => { diff --git a/extensions/whatsapp/test-api.ts b/extensions/whatsapp/test-api.ts index bfea8434f95..7b78f55f2d4 100644 --- a/extensions/whatsapp/test-api.ts +++ b/extensions/whatsapp/test-api.ts @@ -1,2 +1,3 @@ export { whatsappOutbound } from "./src/outbound-adapter.js"; export { resolveWhatsAppRuntimeGroupPolicy } from "./src/runtime-group-policy.js"; +export { createWhatsAppPollFixture, expectWhatsAppPollSent } from "./src/outbound-test-support.js"; diff --git a/src/plugin-sdk/facade-activation-check.runtime.ts b/src/plugin-sdk/facade-activation-check.runtime.ts index 2213f38e3e3..ceeb27ed2c3 100644 --- a/src/plugin-sdk/facade-activation-check.runtime.ts +++ b/src/plugin-sdk/facade-activation-check.runtime.ts @@ -16,6 +16,7 @@ import { loadPluginManifestRegistry, type PluginManifestRecord, } from "../plugins/manifest-registry.js"; +import { normalizeBundledPluginArtifactSubpath } from "../plugins/public-surface-runtime.js"; const ALWAYS_ALLOWED_RUNTIME_DIR_NAMES = new Set([ "image-generation-core", @@ -166,7 +167,7 @@ export function resolveRegistryPluginModuleLocation(params: { (plugin) => path.basename(plugin.rootDir) === params.dirName, (plugin) => plugin.channels.includes(params.dirName), ]; - const artifactBasename = params.artifactBasename.replace(/^\.\//u, ""); + const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); const sourceBaseName = artifactBasename.replace(/\.js$/u, ""); for (const matchFn of tiers) { for (const record of registry.filter(matchFn)) { diff --git a/src/plugin-sdk/facade-loader.ts b/src/plugin-sdk/facade-loader.ts index f918e51d964..1796e3c04da 100644 --- a/src/plugin-sdk/facade-loader.ts +++ b/src/plugin-sdk/facade-loader.ts @@ -4,7 +4,10 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { resolveBundledPluginsDir } from "../plugins/bundled-dir.js"; -import { resolveBundledPluginPublicSurfacePath } from "../plugins/public-surface-runtime.js"; +import { + normalizeBundledPluginArtifactSubpath, + resolveBundledPluginPublicSurfacePath, +} from "../plugins/public-surface-runtime.js"; import { buildPluginLoaderJitiOptions, resolvePluginLoaderJitiConfig, @@ -62,7 +65,8 @@ function resolveSourceFirstPublicSurfacePath(params: { dirName: string; artifactBasename: string; }): string | null { - const sourceBaseName = params.artifactBasename.replace(/\.js$/u, ""); + const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); + const sourceBaseName = artifactBasename.replace(/\.js$/u, ""); const sourceRoot = params.bundledPluginsDir ?? path.resolve(getOpenClawPackageRoot(), "extensions"); for (const ext of PUBLIC_SURFACE_SOURCE_EXTENSIONS) { diff --git a/src/plugin-sdk/facade-runtime.ts b/src/plugin-sdk/facade-runtime.ts index 1a4a190d533..2ecef96fe24 100644 --- a/src/plugin-sdk/facade-runtime.ts +++ b/src/plugin-sdk/facade-runtime.ts @@ -4,7 +4,10 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { resolveBundledPluginsDir } from "../plugins/bundled-dir.js"; import type { PluginManifestRecord } from "../plugins/manifest-registry.js"; -import { resolveBundledPluginPublicSurfacePath } from "../plugins/public-surface-runtime.js"; +import { + normalizeBundledPluginArtifactSubpath, + resolveBundledPluginPublicSurfacePath, +} from "../plugins/public-surface-runtime.js"; import { resolveLoaderPackageRoot } from "../plugins/sdk-alias.js"; import { loadBundledPluginPublicSurfaceModuleSync as loadBundledPluginPublicSurfaceModuleSyncLight, @@ -44,7 +47,8 @@ function resolveSourceFirstPublicSurfacePath(params: { dirName: string; artifactBasename: string; }): string | null { - const sourceBaseName = params.artifactBasename.replace(/\.js$/u, ""); + const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); + const sourceBaseName = artifactBasename.replace(/\.js$/u, ""); const sourceRoot = params.bundledPluginsDir ?? path.resolve(OPENCLAW_PACKAGE_ROOT, "extensions"); for (const ext of PUBLIC_SURFACE_SOURCE_EXTENSIONS) { const candidate = path.resolve(sourceRoot, params.dirName, `${sourceBaseName}${ext}`); @@ -66,7 +70,7 @@ function resolveRegistryPluginModuleLocationFromRegistry(params: { (plugin) => path.basename(plugin.rootDir) === params.dirName, (plugin) => plugin.channels.includes(params.dirName), ]; - const artifactBasename = params.artifactBasename.replace(/^\.\//u, ""); + const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); const sourceBaseName = artifactBasename.replace(/\.js$/u, ""); for (const matchFn of tiers) { for (const record of params.registry.filter(matchFn)) { diff --git a/src/plugins/contracts/plugin-entry-guardrails.test.ts b/src/plugins/contracts/plugin-entry-guardrails.test.ts index b3e2cb7bf80..9fe86947bba 100644 --- a/src/plugins/contracts/plugin-entry-guardrails.test.ts +++ b/src/plugins/contracts/plugin-entry-guardrails.test.ts @@ -1,11 +1,30 @@ -import { readFileSync } from "node:fs"; -import { resolve } from "node:path"; +import { existsSync, readFileSync } from "node:fs"; +import path, { dirname, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; import { describe, expect, it } from "vitest"; +import { listBundledPluginMetadata } from "../bundled-plugin-metadata.js"; import { loadPluginManifestRegistry } from "../manifest-registry.js"; -const CORE_PLUGIN_ENTRY_IMPORT_RE = - /import\s*\{[^}]*\bdefinePluginEntry\b[^}]*\}\s*from\s*"openclaw\/plugin-sdk\/core"/; -const RUNTIME_ENTRY_HELPER_RE = /(^|\/)plugin-entry\.runtime\.[cm]?[jt]s$/; +const REPO_ROOT = resolve(fileURLToPath(new URL("../../..", import.meta.url))); +const RUNTIME_ENTRY_HELPER_RE = /(^|\/)plugin-entry\.runtime\.[cm]?[jt]s$/; +const SOURCE_MODULE_EXTENSIONS = [".ts", ".mts", ".cts", ".js", ".mjs", ".cjs"] as const; +const FORBIDDEN_CONTRACT_MODULE_SPECIFIER_PATTERNS = [ + /^vitest$/u, + /^openclaw\/plugin-sdk\/testing$/u, + /(^|\/)test-api(?:\.[cm]?[jt]s)?$/u, + /(^|\/)__tests__(\/|$)/u, + /(^|\/)test-support(\/|$)/u, + /(^|\/)[^/]*\.test(?:[-.][^/]*)?(?:\.[cm]?[jt]s)?$/u, + /(^|\/)[^/]*(?:test-harness|test-plugin|test-helper|test-support|harness)[^/]*(?:\.[cm]?[jt]s)?$/u, +] as const; +const FORBIDDEN_CONTRACT_MODULE_PATH_PATTERNS = [ + /(^|\/)__tests__(\/|$)/u, + /(^|\/)test-support(\/|$)/u, + /(^|\/)test-api\.[cm]?[jt]s$/u, + /(^|\/)[^/]*\.test(?:[-.][^/]*)?\.[cm]?[jt]s$/u, + /(^|\/)[^/]*(?:test-harness|test-plugin|test-helper|test-support|harness)[^/]*\.[cm]?[jt]s$/u, +] as const; function listBundledPluginRoots() { return loadPluginManifestRegistry({}) .plugins.filter((plugin) => plugin.origin === "bundled") @@ -16,6 +35,189 @@ function listBundledPluginRoots() { .toSorted((left, right) => left.pluginId.localeCompare(right.pluginId)); } +function resolvePublicSurfaceSourcePath( + pluginDir: string, + artifactBasename: string, +): string | null { + const stem = artifactBasename.replace(/\.[^.]+$/u, ""); + for (const extension of SOURCE_MODULE_EXTENSIONS) { + const candidate = resolve(pluginDir, `${stem}${extension}`); + if (existsSync(candidate)) { + return candidate; + } + } + return null; +} + +function isGuardedContractArtifactBasename(artifactBasename: string): boolean { + return ( + artifactBasename === "channel-config-api.js" || artifactBasename.endsWith("contract-api.js") + ); +} + +function collectProductionContractEntryPaths(): Array<{ + pluginId: string; + entryPath: string; + pluginRoot: string; +}> { + return listBundledPluginMetadata({ rootDir: REPO_ROOT }).flatMap((plugin) => { + const pluginRoot = resolve(REPO_ROOT, "extensions", plugin.dirName); + const entryPaths = new Set(); + for (const artifact of plugin.publicSurfaceArtifacts ?? []) { + if (!isGuardedContractArtifactBasename(artifact)) { + continue; + } + const sourcePath = resolvePublicSurfaceSourcePath(pluginRoot, artifact); + if (sourcePath) { + entryPaths.add(sourcePath); + } + } + return [...entryPaths].map((entryPath) => ({ + pluginId: plugin.manifest.id, + entryPath, + pluginRoot, + })); + }); +} + +function formatRepoRelativePath(filePath: string): string { + return relative(REPO_ROOT, filePath).replaceAll(path.sep, "/"); +} + +function analyzeSourceModule(params: { filePath: string; source: string }): { + specifiers: string[]; + relativeSpecifiers: string[]; + importsDefinePluginEntryFromCore: boolean; +} { + const sourceFile = ts.createSourceFile( + params.filePath, + params.source, + ts.ScriptTarget.Latest, + true, + ); + const specifiers = new Set(); + let importsDefinePluginEntryFromCore = false; + + for (const statement of sourceFile.statements) { + if (ts.isImportDeclaration(statement)) { + const specifier = ts.isStringLiteral(statement.moduleSpecifier) + ? statement.moduleSpecifier.text + : undefined; + if (specifier) { + specifiers.add(specifier); + } + + if ( + specifier === "openclaw/plugin-sdk/core" && + statement.importClause?.namedBindings && + ts.isNamedImports(statement.importClause.namedBindings) && + statement.importClause.namedBindings.elements.some( + (element) => (element.propertyName?.text ?? element.name.text) === "definePluginEntry", + ) + ) { + importsDefinePluginEntryFromCore = true; + } + + continue; + } + + if (!ts.isExportDeclaration(statement)) { + continue; + } + + if (statement.moduleSpecifier && ts.isStringLiteral(statement.moduleSpecifier)) { + specifiers.add(statement.moduleSpecifier.text); + } + } + + const nextSpecifiers = [...specifiers]; + return { + specifiers: nextSpecifiers, + relativeSpecifiers: nextSpecifiers.filter((specifier) => specifier.startsWith(".")), + importsDefinePluginEntryFromCore, + }; +} + +function matchesForbiddenContractSpecifier(specifier: string): boolean { + return FORBIDDEN_CONTRACT_MODULE_SPECIFIER_PATTERNS.some((pattern) => pattern.test(specifier)); +} + +function collectForbiddenContractSpecifiers(specifiers: readonly string[]): string[] { + return specifiers.filter((specifier) => matchesForbiddenContractSpecifier(specifier)); +} + +function resolveRelativeSourceModulePath(fromPath: string, specifier: string): string | null { + const rawTargetPath = resolve(dirname(fromPath), specifier); + const candidates = new Set(); + const rawExtension = path.extname(rawTargetPath); + if (rawExtension) { + candidates.add(rawTargetPath); + const stem = rawTargetPath.slice(0, -rawExtension.length); + for (const extension of SOURCE_MODULE_EXTENSIONS) { + candidates.add(`${stem}${extension}`); + } + } else { + for (const extension of SOURCE_MODULE_EXTENSIONS) { + candidates.add(`${rawTargetPath}${extension}`); + candidates.add(resolve(rawTargetPath, `index${extension}`)); + } + } + + for (const candidate of candidates) { + if (existsSync(candidate)) { + return candidate; + } + } + + return null; +} + +function findForbiddenContractModuleGraphPaths(params: { + entryPath: string; + pluginRoot: string; +}): string[] { + const failures: string[] = []; + const visited = new Set(); + const pending = [params.entryPath]; + + while (pending.length > 0) { + const currentPath = pending.pop(); + if (!currentPath || visited.has(currentPath)) { + continue; + } + visited.add(currentPath); + + const repoRelativePath = formatRepoRelativePath(currentPath); + for (const pattern of FORBIDDEN_CONTRACT_MODULE_PATH_PATTERNS) { + if (pattern.test(repoRelativePath)) { + failures.push(`${repoRelativePath} matched ${pattern}`); + } + } + + const source = readFileSync(currentPath, "utf8"); + const analysis = analyzeSourceModule({ filePath: currentPath, source }); + for (const specifier of collectForbiddenContractSpecifiers(analysis.specifiers)) { + failures.push(`${repoRelativePath} imported ${specifier}`); + } + + for (const specifier of analysis.relativeSpecifiers) { + const resolvedModulePath = resolveRelativeSourceModulePath(currentPath, specifier); + if (!resolvedModulePath) { + continue; + } + if (resolvedModulePath === currentPath) { + continue; + } + if (!resolvedModulePath.startsWith(params.pluginRoot + path.sep)) { + continue; + } + pending.push(resolvedModulePath); + } + } + + return failures; +} + describe("plugin entry guardrails", () => { it("keeps bundled extension entry modules off direct definePluginEntry imports from core", () => { const failures: string[] = []; @@ -24,7 +226,7 @@ describe("plugin entry guardrails", () => { const indexPath = resolve(plugin.rootDir, "index.ts"); try { const source = readFileSync(indexPath, "utf8"); - if (CORE_PLUGIN_ENTRY_IMPORT_RE.test(source)) { + if (analyzeSourceModule({ filePath: indexPath, source }).importsDefinePluginEntryFromCore) { failures.push(`extensions/${plugin.pluginId}/index.ts`); } } catch { @@ -59,4 +261,62 @@ describe("plugin entry guardrails", () => { expect(failures).toEqual([]); }); + + it("keeps bundled production contract barrels off test-only imports and re-exports", () => { + const failures = collectProductionContractEntryPaths().flatMap( + ({ pluginId, entryPath, pluginRoot }) => + findForbiddenContractModuleGraphPaths({ + entryPath, + pluginRoot, + }).map((failure) => `${pluginId}: ${failure}`), + ); + + expect(failures).toEqual([]); + }); + + it("follows relative import edges while scanning guarded contract graphs", () => { + expect( + analyzeSourceModule({ + filePath: "guardrail-fixture.ts", + source: ` + import { x } from "./safe.js"; + import "./setup.js"; + export { x }; + export * from "./barrel.js"; + import { y } from "openclaw/plugin-sdk/testing"; + `, + }).relativeSpecifiers.toSorted(), + ).toEqual(["./barrel.js", "./safe.js", "./setup.js"]); + }); + + it("guards contract-style production artifacts beyond the legacy allowlist", () => { + expect(isGuardedContractArtifactBasename("channel-config-api.js")).toBe(true); + expect(isGuardedContractArtifactBasename("contract-api.js")).toBe(true); + expect(isGuardedContractArtifactBasename("doctor-contract-api.js")).toBe(true); + expect(isGuardedContractArtifactBasename("web-search-contract-api.js")).toBe(true); + expect(isGuardedContractArtifactBasename("test-api.js")).toBe(false); + }); + + it("flags test-support directory hops in guarded contract graphs", () => { + expect(collectForbiddenContractSpecifiers(["./test-support/index.js"])).toEqual([ + "./test-support/index.js", + ]); + expect( + FORBIDDEN_CONTRACT_MODULE_PATH_PATTERNS.some((pattern) => + pattern.test("extensions/demo/src/test-support/index.ts"), + ), + ).toBe(true); + }); + + it("detects aliased definePluginEntry imports from core", () => { + expect( + analyzeSourceModule({ + filePath: "aliased-plugin-entry.ts", + source: ` + import { definePluginEntry as dpe } from "openclaw/plugin-sdk/core"; + import { somethingElse } from "openclaw/plugin-sdk/core"; + `, + }).importsDefinePluginEntryFromCore, + ).toBe(true); + }); }); diff --git a/src/plugins/public-surface-runtime.test.ts b/src/plugins/public-surface-runtime.test.ts new file mode 100644 index 00000000000..96b57224740 --- /dev/null +++ b/src/plugins/public-surface-runtime.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { normalizeBundledPluginArtifactSubpath } from "./public-surface-runtime.js"; + +describe("bundled plugin public surface runtime", () => { + it("allows plugin-local nested artifact paths", () => { + expect(normalizeBundledPluginArtifactSubpath("src/outbound-adapter.js")).toBe( + "src/outbound-adapter.js", + ); + expect(normalizeBundledPluginArtifactSubpath("./test-api.js")).toBe("test-api.js"); + }); + + it("rejects artifact paths that escape the plugin root", () => { + expect(() => normalizeBundledPluginArtifactSubpath("../outside.js")).toThrow( + /must stay plugin-local/, + ); + expect(() => normalizeBundledPluginArtifactSubpath("src/../outside.js")).toThrow( + /must stay plugin-local/, + ); + expect(() => normalizeBundledPluginArtifactSubpath("/tmp/outside.js")).toThrow( + /must stay plugin-local/, + ); + expect(() => normalizeBundledPluginArtifactSubpath("..\\outside.js")).toThrow( + /must stay plugin-local/, + ); + expect(() => normalizeBundledPluginArtifactSubpath("C:outside.js")).toThrow( + /must stay plugin-local/, + ); + expect(() => normalizeBundledPluginArtifactSubpath("src/C:outside.js")).toThrow( + /must stay plugin-local/, + ); + }); +}); diff --git a/src/plugins/public-surface-runtime.ts b/src/plugins/public-surface-runtime.ts index 72043570fcd..d7f1e7d4571 100644 --- a/src/plugins/public-surface-runtime.ts +++ b/src/plugins/public-surface-runtime.ts @@ -4,6 +4,33 @@ import { resolveBundledPluginsDir } from "./bundled-dir.js"; const PUBLIC_SURFACE_SOURCE_EXTENSIONS = [".ts", ".mts", ".js", ".mjs", ".cts", ".cjs"] as const; +export function normalizeBundledPluginArtifactSubpath(artifactBasename: string): string { + if ( + path.posix.isAbsolute(artifactBasename) || + path.win32.isAbsolute(artifactBasename) || + artifactBasename.includes("\\") + ) { + throw new Error(`Bundled plugin artifact path must stay plugin-local: ${artifactBasename}`); + } + + const normalized = artifactBasename.replace(/^\.\//u, ""); + if (!normalized) { + throw new Error("Bundled plugin artifact path must not be empty"); + } + + const segments = normalized.split("/"); + if ( + segments.some( + (segment) => + segment.length === 0 || segment === "." || segment === ".." || segment.includes(":"), + ) + ) { + throw new Error(`Bundled plugin artifact path must stay plugin-local: ${artifactBasename}`); + } + + return normalized; +} + export function resolveBundledPluginPublicSurfacePath(params: { rootDir: string; dirName: string; @@ -11,10 +38,7 @@ export function resolveBundledPluginPublicSurfacePath(params: { env?: NodeJS.ProcessEnv; bundledPluginsDir?: string; }): string | null { - const artifactBasename = params.artifactBasename.replace(/^\.\//u, ""); - if (!artifactBasename) { - return null; - } + const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); const explicitBundledPluginsDir = params.bundledPluginsDir ?? resolveBundledPluginsDir(params.env ?? process.env); diff --git a/src/test-utils/bundled-plugin-public-surface.ts b/src/test-utils/bundled-plugin-public-surface.ts index 6c8caed644b..3acdbe99526 100644 --- a/src/test-utils/bundled-plugin-public-surface.ts +++ b/src/test-utils/bundled-plugin-public-surface.ts @@ -5,6 +5,7 @@ import { findBundledPluginMetadataById, type BundledPluginMetadata, } from "../plugins/bundled-plugin-metadata.js"; +import { normalizeBundledPluginArtifactSubpath } from "../plugins/public-surface-runtime.js"; import { resolveLoaderPackageRoot } from "../plugins/sdk-alias.js"; const OPENCLAW_PACKAGE_ROOT = @@ -28,7 +29,7 @@ export function loadBundledPluginPublicSurfaceSync(params: { const metadata = findBundledPluginMetadata(params.pluginId); return loadBundledPluginPublicSurfaceModuleSync({ dirName: metadata.dirName, - artifactBasename: params.artifactBasename, + artifactBasename: normalizeBundledPluginArtifactSubpath(params.artifactBasename), }); } @@ -46,11 +47,12 @@ export function resolveRelativeBundledPluginPublicModuleId(params: { }): string { const metadata = findBundledPluginMetadata(params.pluginId); const fromFilePath = fileURLToPath(params.fromModuleUrl); + const artifactBasename = normalizeBundledPluginArtifactSubpath(params.artifactBasename); const targetPath = path.resolve( OPENCLAW_PACKAGE_ROOT, "extensions", metadata.dirName, - params.artifactBasename, + artifactBasename, ); const relativePath = path .relative(path.dirname(fromFilePath), targetPath) diff --git a/test/helpers/channels/imessage-test-plugin.ts b/test/helpers/channels/imessage-test-plugin.ts index a0294405683..1115717d55c 100644 --- a/test/helpers/channels/imessage-test-plugin.ts +++ b/test/helpers/channels/imessage-test-plugin.ts @@ -1 +1,19 @@ -export { createIMessageTestPlugin } from "../../../extensions/imessage/contract-api.js"; +import type { ChannelOutboundAdapter } from "openclaw/plugin-sdk/channel-contract"; +import type { ChannelPlugin } from "openclaw/plugin-sdk/core"; +import { loadBundledPluginTestApiSync } from "../../../src/test-utils/bundled-plugin-public-surface.js"; + +type CreateIMessageTestPlugin = (params?: { outbound?: ChannelOutboundAdapter }) => ChannelPlugin; + +let createIMessageTestPluginCache: CreateIMessageTestPlugin | undefined; + +function getCreateIMessageTestPlugin(): CreateIMessageTestPlugin { + if (!createIMessageTestPluginCache) { + ({ createIMessageTestPlugin: createIMessageTestPluginCache } = loadBundledPluginTestApiSync<{ + createIMessageTestPlugin: CreateIMessageTestPlugin; + }>("imessage")); + } + return createIMessageTestPluginCache; +} + +export const createIMessageTestPlugin: CreateIMessageTestPlugin = (...args) => + getCreateIMessageTestPlugin()(...args); diff --git a/test/helpers/channels/outbound-payload-contract.ts b/test/helpers/channels/outbound-payload-contract.ts index 3b4227b12dd..e84620c0a60 100644 --- a/test/helpers/channels/outbound-payload-contract.ts +++ b/test/helpers/channels/outbound-payload-contract.ts @@ -1,30 +1,77 @@ import { beforeEach, expect, it, type Mock, vi } from "vitest"; -import { createSlackOutboundPayloadHarness } from "../../../extensions/slack/contract-api.js"; -import { whatsappOutbound } from "../../../extensions/whatsapp/test-api.js"; -import { - chunkTextForOutbound as chunkZaloTextForOutbound, - sendPayloadWithChunkedTextAndMedia as sendZaloPayloadWithChunkedTextAndMedia, -} from "../../../extensions/zalo/runtime-api.js"; -import { sendPayloadWithChunkedTextAndMedia as sendZalouserPayloadWithChunkedTextAndMedia } from "../../../extensions/zalouser/runtime-api.js"; import type { ReplyPayload } from "../../../src/auto-reply/types.js"; import { primeChannelOutboundSendMock } from "../../../src/channels/plugins/contracts/test-helpers.js"; import { createDirectTextMediaOutbound } from "../../../src/channels/plugins/outbound/direct-text-media.js"; import type { ChannelOutboundAdapter } from "../../../src/channels/plugins/types.js"; import { resetGlobalHookRunner } from "../../../src/plugins/hook-runner-global.js"; import { + loadBundledPluginPublicSurfaceSync, loadBundledPluginTestApiSync, resolveRelativeBundledPluginPublicModuleId, } from "../../../src/test-utils/bundled-plugin-public-surface.js"; type ParseZalouserOutboundTarget = (raw: string) => { threadId: string; isGroup: boolean }; +type CreateSlackOutboundPayloadHarness = (params: PayloadHarnessParams) => { + run: () => Promise>; + sendMock: Mock; + to: string; +}; +type ChunkZaloTextForOutbound = (text: string, maxLength?: number) => string[]; +type SendPayloadWithChunkedTextAndMedia = (params: { + ctx: { + cfg: unknown; + to: string; + text: string; + payload: ReplyPayload; + }; + sendText: (ctx: { + cfg: unknown; + to: string; + text: string; + payload: ReplyPayload; + }) => Promise<{ channel: string; messageId: string }>; + sendMedia: (ctx: { + cfg: unknown; + to: string; + text: string; + payload: ReplyPayload; + mediaUrl?: string; + }) => Promise<{ channel: string; messageId: string }>; + emptyResult: { channel: string; messageId: string }; + textChunkLimit?: number; + chunker?: ChunkZaloTextForOutbound | null; +}) => Promise<{ channel: string; messageId: string }>; const discordOutboundAdapterModuleId = resolveRelativeBundledPluginPublicModuleId({ fromModuleUrl: import.meta.url, pluginId: "discord", artifactBasename: "src/outbound-adapter.js", }); +const slackTestApiModuleId = resolveRelativeBundledPluginPublicModuleId({ + fromModuleUrl: import.meta.url, + pluginId: "slack", + artifactBasename: "test-api.js", +}); +const whatsappTestApiModuleId = resolveRelativeBundledPluginPublicModuleId({ + fromModuleUrl: import.meta.url, + pluginId: "whatsapp", + artifactBasename: "test-api.js", +}); let discordOutboundCache: Promise | undefined; let parseZalouserOutboundTargetCache: ParseZalouserOutboundTarget | undefined; +let slackTestApiPromise: + | Promise<{ + createSlackOutboundPayloadHarness: CreateSlackOutboundPayloadHarness; + }> + | undefined; +let whatsappTestApiPromise: + | Promise<{ + whatsappOutbound: ChannelOutboundAdapter; + }> + | undefined; +let chunkZaloTextForOutboundCache: ChunkZaloTextForOutbound | undefined; +let sendZaloPayloadWithChunkedTextAndMediaCache: SendPayloadWithChunkedTextAndMedia | undefined; +let sendZalouserPayloadWithChunkedTextAndMediaCache: SendPayloadWithChunkedTextAndMedia | undefined; async function getDiscordOutbound(): Promise { discordOutboundCache ??= (async () => { @@ -36,6 +83,47 @@ async function getDiscordOutbound(): Promise { return await discordOutboundCache; } +async function getCreateSlackOutboundPayloadHarness(): Promise { + slackTestApiPromise ??= import(slackTestApiModuleId) as Promise<{ + createSlackOutboundPayloadHarness: CreateSlackOutboundPayloadHarness; + }>; + const { createSlackOutboundPayloadHarness } = await slackTestApiPromise; + return createSlackOutboundPayloadHarness; +} + +async function getWhatsAppOutboundAsync(): Promise { + whatsappTestApiPromise ??= import(whatsappTestApiModuleId) as Promise<{ + whatsappOutbound: ChannelOutboundAdapter; + }>; + const { whatsappOutbound } = await whatsappTestApiPromise; + return whatsappOutbound; +} + +function getChunkZaloTextForOutbound(): ChunkZaloTextForOutbound { + if (!chunkZaloTextForOutboundCache) { + ({ chunkTextForOutbound: chunkZaloTextForOutboundCache } = loadBundledPluginPublicSurfaceSync<{ + chunkTextForOutbound: ChunkZaloTextForOutbound; + }>({ + pluginId: "zalo", + artifactBasename: "runtime-api.js", + })); + } + return chunkZaloTextForOutboundCache; +} + +function getSendZaloPayloadWithChunkedTextAndMedia(): SendPayloadWithChunkedTextAndMedia { + if (!sendZaloPayloadWithChunkedTextAndMediaCache) { + ({ sendPayloadWithChunkedTextAndMedia: sendZaloPayloadWithChunkedTextAndMediaCache } = + loadBundledPluginPublicSurfaceSync<{ + sendPayloadWithChunkedTextAndMedia: SendPayloadWithChunkedTextAndMedia; + }>({ + pluginId: "zalo", + artifactBasename: "runtime-api.js", + })); + } + return sendZaloPayloadWithChunkedTextAndMediaCache; +} + function getParseZalouserOutboundTarget(): ParseZalouserOutboundTarget { if (!parseZalouserOutboundTargetCache) { ({ parseZalouserOutboundTarget: parseZalouserOutboundTargetCache } = @@ -46,6 +134,19 @@ function getParseZalouserOutboundTarget(): ParseZalouserOutboundTarget { return parseZalouserOutboundTargetCache; } +function getSendZalouserPayloadWithChunkedTextAndMedia(): SendPayloadWithChunkedTextAndMedia { + if (!sendZalouserPayloadWithChunkedTextAndMediaCache) { + ({ sendPayloadWithChunkedTextAndMedia: sendZalouserPayloadWithChunkedTextAndMediaCache } = + loadBundledPluginPublicSurfaceSync<{ + sendPayloadWithChunkedTextAndMedia: SendPayloadWithChunkedTextAndMedia; + }>({ + pluginId: "zalouser", + artifactBasename: "runtime-api.js", + })); + } + return sendZalouserPayloadWithChunkedTextAndMediaCache; +} + type PayloadHarnessParams = { payload: ReplyPayload; sendResults?: Array<{ messageId: string }>; @@ -76,18 +177,24 @@ type ChunkingMode = function installChannelOutboundPayloadContractSuite(params: { channel: string; chunking: ChunkingMode; - createHarness: (params: { payload: PayloadLike; sendResults?: SendResultLike[] }) => { - run: () => Promise>; - sendMock: Mock; - to: string; - }; + createHarness: (params: { payload: PayloadLike; sendResults?: SendResultLike[] }) => + | { + run: () => Promise>; + sendMock: Mock; + to: string; + } + | Promise<{ + run: () => Promise>; + sendMock: Mock; + to: string; + }>; }) { beforeEach(() => { resetGlobalHookRunner(); }); it("text-only delegates to sendText", async () => { - const { run, sendMock, to } = params.createHarness({ + const { run, sendMock, to } = await params.createHarness({ payload: { text: "hello" }, }); const result = await run(); @@ -98,7 +205,7 @@ function installChannelOutboundPayloadContractSuite(params: { }); it("single media delegates to sendMedia", async () => { - const { run, sendMock, to } = params.createHarness({ + const { run, sendMock, to } = await params.createHarness({ payload: { text: "cap", mediaUrl: "https://example.com/a.jpg" }, }); const result = await run(); @@ -113,7 +220,7 @@ function installChannelOutboundPayloadContractSuite(params: { }); it("multi-media iterates URLs with caption on first", async () => { - const { run, sendMock, to } = params.createHarness({ + const { run, sendMock, to } = await params.createHarness({ payload: { text: "caption", mediaUrls: ["https://example.com/1.jpg", "https://example.com/2.jpg"], @@ -139,7 +246,7 @@ function installChannelOutboundPayloadContractSuite(params: { }); it("empty payload returns no-op", async () => { - const { run, sendMock } = params.createHarness({ payload: {} }); + const { run, sendMock } = await params.createHarness({ payload: {} }); const result = await run(); expect(sendMock).not.toHaveBeenCalled(); @@ -149,7 +256,7 @@ function installChannelOutboundPayloadContractSuite(params: { if (params.chunking.mode === "passthrough") { it("text exceeding chunk limit is sent as-is when chunker is null", async () => { const text = "a".repeat(params.chunking.longTextLength); - const { run, sendMock, to } = params.createHarness({ payload: { text } }); + const { run, sendMock, to } = await params.createHarness({ payload: { text } }); const result = await run(); expect(sendMock).toHaveBeenCalledTimes(1); @@ -163,7 +270,7 @@ function installChannelOutboundPayloadContractSuite(params: { it("chunking splits long text", async () => { const text = "a".repeat(chunking.longTextLength); - const { run, sendMock } = params.createHarness({ + const { run, sendMock } = await params.createHarness({ payload: { text }, sendResults: [{ messageId: "c-1" }, { messageId: "c-2" }], }); @@ -220,7 +327,7 @@ function createWhatsAppHarness(params: PayloadHarnessParams) { }, }; return { - run: async () => await whatsappOutbound.sendPayload!(ctx), + run: async () => await (await getWhatsAppOutboundAsync()).sendPayload!(ctx), sendMock: sendWhatsApp, to: ctx.to, }; @@ -260,10 +367,10 @@ function createZaloHarness(params: PayloadHarnessParams) { }; return { run: async () => - await sendZaloPayloadWithChunkedTextAndMedia({ + await getSendZaloPayloadWithChunkedTextAndMedia()({ ctx, textChunkLimit: 2000, - chunker: chunkZaloTextForOutbound, + chunker: getChunkZaloTextForOutbound(), sendText: async (nextCtx) => buildChannelSendResult( "zalo", @@ -299,7 +406,7 @@ function createZalouserHarness(params: PayloadHarnessParams) { }; return { run: async () => - await sendZalouserPayloadWithChunkedTextAndMedia({ + await getSendZalouserPayloadWithChunkedTextAndMedia()({ ctx, sendText: async (nextCtx) => { const target = getParseZalouserOutboundTarget()(nextCtx.to); @@ -339,7 +446,7 @@ export function installSlackOutboundPayloadContractSuite() { installChannelOutboundPayloadContractSuite({ channel: "slack", chunking: { mode: "passthrough", longTextLength: 5000 }, - createHarness: createSlackOutboundPayloadHarness, + createHarness: async (params) => (await getCreateSlackOutboundPayloadHarness())(params), }); }