From 342d13a914bbcbe0d206e85b2f0cc49dab48814f Mon Sep 17 00:00:00 2001 From: Alex Knight Date: Sat, 4 Jul 2026 10:57:42 +1000 Subject: [PATCH] fix(plugins): resolve public artifacts from installed plugin roots (#98819) * fix(plugins): resolve public artifacts from installed plugin roots Externalized official plugins ship gateway-auth/doctor/secret/message-tool public artifacts in their installed npm package, but the public-surface loader only searched bundled locations. On packaged installs this emptied the gateway auth bypass set for /api/channels/mattermost/command, so every Mattermost slash callback was rejected 401 before the plugin ran. Fall back to the plugin's installed package root (install-record index) when bundled resolution misses; bundled/source locations keep precedence. Fixes #98740 * fix(gateway): resolve channel auth-bypass artifacts via activation-gated facade Route gateway-auth bypass artifact loading through the activation-gated facade seam so externalized (installed) channel plugins keep their unauthenticated callback paths, while denied/disabled plugins contribute nothing and never execute artifact code. Reject hardlinked artifacts for installed plugin roots in the facade loader per hardlink-policy. Replaces the earlier install-record fallback in the shared bundled artifact loader, which broadened every artifact surface past its trust boundary (see PR review); other externalized-plugin surfaces are #98842. Fixes #98740 * fix(gateway): load facade activation runtime via async lazy import The sync createRequire/jiti candidate loader for facade-activation-check.runtime.js only resolves from built dist; under vitest source runs it cannot follow .js->.ts specifiers or workspace package subpaths, so the gateway auth-bypass path threw 'Unable to load facade activation check runtime' (CI shard failure). Add an async activated-load API backed by dynamic import (vitest- and dist-native), warm the shared runtime memo, and make the bypass consumer async - its single caller already awaits behind the config-snapshot promise cache. * revert(plugins): drop install-record fallback from public-surface loader Superseded by the activation-gated facade seam in gateway-auth-bypass; review found the loader-level fallback broadened every artifact surface past its trust boundary (see PR #98819 discussion). * fix(plugin-sdk): drop unused oxlint disable on async facade loader * test(plugin-sdk): cover facade hardlink policy for installed vs core roots * test(plugin-sdk): cover facade hardlink policy for installed vs core roots --------- Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com> --- .../plugins/gateway-auth-bypass.test.ts | 44 ++++++++++++------- src/channels/plugins/gateway-auth-bypass.ts | 22 ++++++---- src/gateway/server-http.ts | 7 +-- src/plugin-sdk/facade-loader.ts | 44 ++++++++++++------- src/plugin-sdk/facade-runtime.test.ts | 35 +++++++++++++++ src/plugin-sdk/facade-runtime.ts | 24 ++++++++++ 6 files changed, 134 insertions(+), 42 deletions(-) diff --git a/src/channels/plugins/gateway-auth-bypass.test.ts b/src/channels/plugins/gateway-auth-bypass.test.ts index 1558c33cebef..0bcdc5912e98 100644 --- a/src/channels/plugins/gateway-auth-bypass.test.ts +++ b/src/channels/plugins/gateway-auth-bypass.test.ts @@ -1,9 +1,9 @@ // Gateway auth bypass tests cover channel plugin paths allowed to skip gateway auth. import { describe, expect, it, vi } from "vitest"; -const { loadBundledPluginPublicArtifactModuleSyncMock } = vi.hoisted(() => ({ - loadBundledPluginPublicArtifactModuleSyncMock: vi.fn( - ({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => { +const { tryLoadActivatedBundledPluginPublicSurfaceModuleMock } = vi.hoisted(() => ({ + tryLoadActivatedBundledPluginPublicSurfaceModuleMock: vi.fn( + async ({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => { if (dirName === "mattermost" && artifactBasename === "gateway-auth-api.js") { return { resolveGatewayAuthBypassPaths: () => [ @@ -14,6 +14,10 @@ const { loadBundledPluginPublicArtifactModuleSyncMock } = vi.hoisted(() => ({ ], }; } + if (dirName === "disabledchannel") { + // Activation-gated loads return null for disabled/denied plugins. + return null; + } if (dirName === "broken" && artifactBasename === "gateway-auth-api.js") { throw new Error("broken gateway auth artifact"); } @@ -24,41 +28,51 @@ const { loadBundledPluginPublicArtifactModuleSyncMock } = vi.hoisted(() => ({ ), })); -vi.mock("../../plugins/public-surface-loader.js", () => ({ - loadBundledPluginPublicArtifactModuleSync: loadBundledPluginPublicArtifactModuleSyncMock, +vi.mock("../../plugin-sdk/facade-runtime.js", () => ({ + tryLoadActivatedBundledPluginPublicSurfaceModule: + tryLoadActivatedBundledPluginPublicSurfaceModuleMock, })); import { resolveBundledChannelGatewayAuthBypassPaths } from "./gateway-auth-bypass.js"; -describe("bundled channel gateway auth bypass fast path", () => { - it("loads the narrow gateway auth artifact for configured channels", () => { - const paths = resolveBundledChannelGatewayAuthBypassPaths({ +describe("channel gateway auth bypass fast path", () => { + it("loads the narrow gateway auth artifact for configured channels", async () => { + const paths = await resolveBundledChannelGatewayAuthBypassPaths({ channelId: "mattermost", cfg: { channels: { mattermost: {} } }, }); expect(paths).toEqual(["/api/channels/mattermost/command", "/api/channels/mattermost/work"]); - expect(loadBundledPluginPublicArtifactModuleSyncMock).toHaveBeenCalledWith({ + expect(tryLoadActivatedBundledPluginPublicSurfaceModuleMock).toHaveBeenCalledWith({ dirName: "mattermost", artifactBasename: "gateway-auth-api.js", }); }); - it("treats missing gateway auth artifacts as no bypass paths", () => { - expect( + it("treats missing gateway auth artifacts as no bypass paths", async () => { + await expect( resolveBundledChannelGatewayAuthBypassPaths({ channelId: "discord", cfg: { channels: { discord: {} } }, }), - ).toStrictEqual([]); + ).resolves.toStrictEqual([]); }); - it("surfaces errors from present gateway auth artifacts", () => { - expect(() => + it("returns no bypass paths when plugin activation blocks the artifact", async () => { + await expect( + resolveBundledChannelGatewayAuthBypassPaths({ + channelId: "disabledchannel", + cfg: { channels: { disabledchannel: {} } }, + }), + ).resolves.toStrictEqual([]); + }); + + it("surfaces errors from present gateway auth artifacts", async () => { + await expect( resolveBundledChannelGatewayAuthBypassPaths({ channelId: "broken", cfg: { channels: { broken: {} } }, }), - ).toThrow("broken gateway auth artifact"); + ).rejects.toThrow("broken gateway auth artifact"); }); }); diff --git a/src/channels/plugins/gateway-auth-bypass.ts b/src/channels/plugins/gateway-auth-bypass.ts index b02fba423453..12a21b560b45 100644 --- a/src/channels/plugins/gateway-auth-bypass.ts +++ b/src/channels/plugins/gateway-auth-bypass.ts @@ -1,10 +1,10 @@ /** - * Bundled channel gateway auth bypass loader. + * Channel gateway auth bypass loader. * * Reads optional public artifacts that declare unauthenticated Gateway callback paths. */ import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { loadBundledPluginPublicArtifactModuleSync } from "../../plugins/public-surface-loader.js"; +import { tryLoadActivatedBundledPluginPublicSurfaceModule } from "../../plugin-sdk/facade-runtime.js"; /** * Lightweight public artifact contract for channel gateway auth bypass paths. @@ -16,9 +16,13 @@ type GatewayAuthBypassApi = { const GATEWAY_AUTH_API_ARTIFACT_BASENAME = "gateway-auth-api.js"; const MISSING_PUBLIC_SURFACE_PREFIX = "Unable to resolve bundled plugin public surface "; -function loadBundledChannelGatewayAuthApi(channelId: string): GatewayAuthBypassApi | undefined { +/** Resolves to null when the plugin is not activated or ships no gateway auth artifact. */ +async function loadChannelGatewayAuthApi(channelId: string): Promise { try { - return loadBundledPluginPublicArtifactModuleSync({ + // Bypass paths grant unauthenticated ingress, so resolution goes through the + // activation-gated facade seam: it also covers installed (externalized) plugin + // roots and returns null instead of executing a disabled plugin's artifact. + return await tryLoadActivatedBundledPluginPublicSurfaceModule({ dirName: channelId, artifactBasename: GATEWAY_AUTH_API_ARTIFACT_BASENAME, }); @@ -26,20 +30,20 @@ function loadBundledChannelGatewayAuthApi(channelId: string): GatewayAuthBypassA // Missing gateway auth artifacts are optional. Any other load failure means // the artifact exists but cannot be trusted, so propagate it to callers. if (error instanceof Error && error.message.startsWith(MISSING_PUBLIC_SURFACE_PREFIX)) { - return undefined; + return null; } throw error; } } /** - * Resolves configured gateway auth bypass paths from a bundled channel artifact. + * Resolves configured gateway auth bypass paths from a channel plugin artifact. */ -export function resolveBundledChannelGatewayAuthBypassPaths(params: { +export async function resolveBundledChannelGatewayAuthBypassPaths(params: { channelId: string; cfg: OpenClawConfig; -}): string[] { - const api = loadBundledChannelGatewayAuthApi(params.channelId); +}): Promise { + const api = await loadChannelGatewayAuthApi(params.channelId); const paths = api?.resolveGatewayAuthBypassPaths?.({ cfg: params.cfg }) ?? []; return paths.flatMap((path) => (typeof path === "string" && path.trim() ? [path.trim()] : [])); } diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index 6d6ee9d10840..7e4dcc23c372 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -126,7 +126,7 @@ async function resolvePluginGatewayAuthBypassPaths( return paths; } for (const channelId of Object.keys(configuredChannels)) { - for (const path of resolveBundledChannelGatewayAuthBypassPaths({ + for (const path of await resolveBundledChannelGatewayAuthBypassPaths({ channelId, cfg: configSnapshot, })) { @@ -386,8 +386,9 @@ function buildPluginRequestStages(params: { if ((await params.getGatewayAuthBypassPaths()).has(params.requestPath)) { return false; } - // Bypass paths are limited to bundled channel callbacks; all other protected plugin - // routes must produce an AuthorizedGatewayHttpRequest before runtime scopes are derived. + // Bypass paths come only from activated channel plugins' gateway-auth + // artifacts (bundled or installed); all other protected plugin routes must + // produce an AuthorizedGatewayHttpRequest before runtime scopes are derived. const { authorizeGatewayHttpRequestOrReply } = await getHttpAuthUtilsModule(); const requestAuth = await authorizeGatewayHttpRequestOrReply({ req: params.req, diff --git a/src/plugin-sdk/facade-loader.ts b/src/plugin-sdk/facade-loader.ts index 9e6808e2e19f..833e50284d88 100644 --- a/src/plugin-sdk/facade-loader.ts +++ b/src/plugin-sdk/facade-loader.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { openRootFileSync } from "../infra/boundary-file-read.js"; import { resolveBundledPluginsDir } from "../plugins/bundled-dir.js"; +import { shouldRejectHardlinkedPluginFiles } from "../plugins/hardlink-policy.js"; import { getCachedPluginModuleLoader, type PluginModuleLoaderCache, @@ -129,6 +130,32 @@ export type FacadeModuleLocation = { boundaryRoot: string; }; +function isPathAtOrInside(target: string, root: string): boolean { + const resolvedRoot = path.resolve(root); + const resolvedTarget = path.resolve(target); + return resolvedTarget === resolvedRoot || resolvedTarget.startsWith(resolvedRoot + path.sep); +} + +// Registry-resolved locations can point at installed plugin roots, which must +// reject hardlinked artifacts (see hardlink-policy.ts); core-shipped roots keep +// hardlinks allowed for bundled dist/Nix layouts. +function resolveFacadeBoundaryOpenParams(boundaryRoot: string): { + boundaryLabel: string; + rejectHardlinks: boolean; +} { + if (isPathAtOrInside(boundaryRoot, getOpenClawPackageRoot())) { + return { boundaryLabel: "OpenClaw package root", rejectHardlinks: false }; + } + const bundledDir = resolveBundledPluginsDir(); + if (bundledDir && isPathAtOrInside(boundaryRoot, bundledDir)) { + return { boundaryLabel: "bundled plugin directory", rejectHardlinks: false }; + } + return { + boundaryLabel: "plugin root", + rejectHardlinks: shouldRejectHardlinkedPluginFiles({ origin: "global", rootDir: boundaryRoot }), + }; +} + /** Load and cache a facade module after verifying it is inside its declared boundary root. */ export function loadFacadeModuleAtLocationSync(params: { location: FacadeModuleLocation; @@ -144,16 +171,7 @@ export function loadFacadeModuleAtLocationSync(params: { const opened = openRootFileSync({ absolutePath: location.modulePath, rootPath: location.boundaryRoot, - boundaryLabel: - location.boundaryRoot === getOpenClawPackageRoot() - ? "OpenClaw package root" - : (() => { - const bundledDir = resolveBundledPluginsDir(); - return bundledDir && path.resolve(location.boundaryRoot) === path.resolve(bundledDir) - ? "bundled plugin directory" - : "plugin root"; - })(), - rejectHardlinks: false, + ...resolveFacadeBoundaryOpenParams(location.boundaryRoot), }); if (!opened.ok) { throw new Error(`Unable to open bundled plugin public surface ${location.modulePath}`, { @@ -225,11 +243,7 @@ export async function loadBundledPluginPublicSurfaceModule(par const opened = openRootFileSync({ absolutePath: preparedLocation.modulePath, rootPath: preparedLocation.boundaryRoot, - boundaryLabel: - preparedLocation.boundaryRoot === getOpenClawPackageRoot() - ? "OpenClaw package root" - : "plugin root", - rejectHardlinks: false, + ...resolveFacadeBoundaryOpenParams(preparedLocation.boundaryRoot), }); if (!opened.ok) { throw new Error(`Unable to open bundled plugin public surface ${preparedLocation.modulePath}`, { diff --git a/src/plugin-sdk/facade-runtime.test.ts b/src/plugin-sdk/facade-runtime.test.ts index e115d86f4aba..0cc354604540 100644 --- a/src/plugin-sdk/facade-runtime.test.ts +++ b/src/plugin-sdk/facade-runtime.test.ts @@ -366,6 +366,41 @@ describe("plugin-sdk facade runtime", () => { expect(loader).toHaveBeenCalledTimes(1); }); + it("rejects hardlinked artifacts under installed plugin roots", () => { + const installedDir = createTempDirSync("openclaw-facade-hardlink-"); + const originalPath = path.join(installedDir, "original.js"); + fs.writeFileSync(originalPath, 'export const marker = "hardlinked";\n', "utf8"); + const artifactPath = path.join(installedDir, "runtime-api.js"); + fs.linkSync(originalPath, artifactPath); + + // Installed roots are outside the package/bundled roots, so the facade + // boundary open applies shouldRejectHardlinkedPluginFiles (nlink > 1 fails). + expect(() => + testing.loadFacadeModuleAtLocationSync({ + location: { modulePath: artifactPath, boundaryRoot: installedDir }, + trackedPluginId: "line", + }), + ).toThrow(`Unable to open bundled plugin public surface ${artifactPath}`); + }); + + it("keeps hardlinked artifacts loadable under core-shipped roots", () => { + const rootDir = createTrustedBundledFixtureRoot("openclaw-facade-hardlink-bundled-"); + const pluginDir = path.join(rootDir, "demo"); + fs.mkdirSync(pluginDir, { recursive: true }); + const originalPath = path.join(pluginDir, "original.js"); + fs.writeFileSync(originalPath, 'export const marker = "bundled-hardlink";\n', "utf8"); + const artifactPath = path.join(pluginDir, "api.js"); + fs.linkSync(originalPath, artifactPath); + + const loader = vi.fn(() => ({ marker: "bundled-hardlink" })); + const loaded = testing.loadFacadeModuleAtLocationSync<{ marker: string }>({ + location: { modulePath: artifactPath, boundaryRoot: rootDir }, + trackedPluginId: "demo", + loadModule: loader, + }); + expect(loaded.marker).toBe("bundled-hardlink"); + }); + it("resolves a globally-installed plugin whose rootDir basename matches the dirName", () => { const lineDir = createTempDirSync("openclaw-facade-global-line-"); fs.mkdirSync(lineDir, { recursive: true }); diff --git a/src/plugin-sdk/facade-runtime.ts b/src/plugin-sdk/facade-runtime.ts index fa0d9297a112..ff87542817b3 100644 --- a/src/plugin-sdk/facade-runtime.ts +++ b/src/plugin-sdk/facade-runtime.ts @@ -161,6 +161,14 @@ function loadFacadeActivationCheckRuntime(): FacadeActivationCheckRuntimeModule throw new Error("Unable to load facade activation check runtime"); } +// Async twin of loadFacadeActivationCheckRuntime for async call sites: dynamic +// import resolves the source graph under vitest where the sync createRequire/jiti +// candidates cannot, and warms the shared memo so subsequent sync loads reuse it. +async function loadFacadeActivationCheckRuntimeAsync(): Promise { + facadeActivationCheckRuntimeModule ??= await import("./facade-activation-check.runtime.js"); + return facadeActivationCheckRuntimeModule; +} + function setFacadeActivationCheckRuntimeForTest(module: FacadeActivationCheckRuntimeModule): void { facadeActivationCheckRuntimeModule = module; } @@ -255,6 +263,22 @@ export function tryLoadActivatedBundledPluginPublicSurfaceModuleSync(params); } +/** Async variant of tryLoadActivatedBundledPluginPublicSurfaceModuleSync for async call sites. */ +export async function tryLoadActivatedBundledPluginPublicSurfaceModule(params: { + dirName: string; + artifactBasename: string; + env?: NodeJS.ProcessEnv; +}): Promise { + const runtime = await loadFacadeActivationCheckRuntimeAsync(); + const access = runtime.resolveBundledPluginPublicSurfaceAccess( + buildFacadeActivationCheckParams(params), + ); + if (!access.allowed) { + return null; + } + return loadBundledPluginPublicSurfaceModuleSync(params); +} + /** Reset facade runtime caches and activation-check test overrides. */ export function resetFacadeRuntimeStateForTest(): void { resetFacadeLoaderStateForTest();