mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-10 00:11:19 +00:00
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>
This commit is contained in:
parent
50479c2a66
commit
342d13a914
6 changed files with 134 additions and 42 deletions
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<GatewayAuthBypassApi | null> {
|
||||
try {
|
||||
return loadBundledPluginPublicArtifactModuleSync<GatewayAuthBypassApi>({
|
||||
// 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<GatewayAuthBypassApi>({
|
||||
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<string[]> {
|
||||
const api = await loadChannelGatewayAuthApi(params.channelId);
|
||||
const paths = api?.resolveGatewayAuthBypassPaths?.({ cfg: params.cfg }) ?? [];
|
||||
return paths.flatMap((path) => (typeof path === "string" && path.trim() ? [path.trim()] : []));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<T extends object>(params: {
|
||||
location: FacadeModuleLocation;
|
||||
|
|
@ -144,16 +171,7 @@ export function loadFacadeModuleAtLocationSync<T extends object>(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<T extends object>(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}`, {
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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> {
|
||||
facadeActivationCheckRuntimeModule ??= await import("./facade-activation-check.runtime.js");
|
||||
return facadeActivationCheckRuntimeModule;
|
||||
}
|
||||
|
||||
function setFacadeActivationCheckRuntimeForTest(module: FacadeActivationCheckRuntimeModule): void {
|
||||
facadeActivationCheckRuntimeModule = module;
|
||||
}
|
||||
|
|
@ -255,6 +263,22 @@ export function tryLoadActivatedBundledPluginPublicSurfaceModuleSync<T extends o
|
|||
return loadBundledPluginPublicSurfaceModuleSync<T>(params);
|
||||
}
|
||||
|
||||
/** Async variant of tryLoadActivatedBundledPluginPublicSurfaceModuleSync for async call sites. */
|
||||
export async function tryLoadActivatedBundledPluginPublicSurfaceModule<T extends object>(params: {
|
||||
dirName: string;
|
||||
artifactBasename: string;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
}): Promise<T | null> {
|
||||
const runtime = await loadFacadeActivationCheckRuntimeAsync();
|
||||
const access = runtime.resolveBundledPluginPublicSurfaceAccess(
|
||||
buildFacadeActivationCheckParams(params),
|
||||
);
|
||||
if (!access.allowed) {
|
||||
return null;
|
||||
}
|
||||
return loadBundledPluginPublicSurfaceModuleSync<T>(params);
|
||||
}
|
||||
|
||||
/** Reset facade runtime caches and activation-check test overrides. */
|
||||
export function resetFacadeRuntimeStateForTest(): void {
|
||||
resetFacadeLoaderStateForTest();
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue