Skip to content

Commit ec0f3aa

Browse files
authored
Merge fe0ffb8 into f59310a
2 parents f59310a + fe0ffb8 commit ec0f3aa

6 files changed

Lines changed: 134 additions & 42 deletions

File tree

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// Gateway auth bypass tests cover channel plugin paths allowed to skip gateway auth.
22
import { describe, expect, it, vi } from "vitest";
33

4-
const { loadBundledPluginPublicArtifactModuleSyncMock } = vi.hoisted(() => ({
5-
loadBundledPluginPublicArtifactModuleSyncMock: vi.fn(
6-
({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => {
4+
const { tryLoadActivatedBundledPluginPublicSurfaceModuleMock } = vi.hoisted(() => ({
5+
tryLoadActivatedBundledPluginPublicSurfaceModuleMock: vi.fn(
6+
async ({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => {
77
if (dirName === "mattermost" && artifactBasename === "gateway-auth-api.js") {
88
return {
99
resolveGatewayAuthBypassPaths: () => [
@@ -14,6 +14,10 @@ const { loadBundledPluginPublicArtifactModuleSyncMock } = vi.hoisted(() => ({
1414
],
1515
};
1616
}
17+
if (dirName === "disabledchannel") {
18+
// Activation-gated loads return null for disabled/denied plugins.
19+
return null;
20+
}
1721
if (dirName === "broken" && artifactBasename === "gateway-auth-api.js") {
1822
throw new Error("broken gateway auth artifact");
1923
}
@@ -24,41 +28,51 @@ const { loadBundledPluginPublicArtifactModuleSyncMock } = vi.hoisted(() => ({
2428
),
2529
}));
2630

27-
vi.mock("../../plugins/public-surface-loader.js", () => ({
28-
loadBundledPluginPublicArtifactModuleSync: loadBundledPluginPublicArtifactModuleSyncMock,
31+
vi.mock("../../plugin-sdk/facade-runtime.js", () => ({
32+
tryLoadActivatedBundledPluginPublicSurfaceModule:
33+
tryLoadActivatedBundledPluginPublicSurfaceModuleMock,
2934
}));
3035

3136
import { resolveBundledChannelGatewayAuthBypassPaths } from "./gateway-auth-bypass.js";
3237

33-
describe("bundled channel gateway auth bypass fast path", () => {
34-
it("loads the narrow gateway auth artifact for configured channels", () => {
35-
const paths = resolveBundledChannelGatewayAuthBypassPaths({
38+
describe("channel gateway auth bypass fast path", () => {
39+
it("loads the narrow gateway auth artifact for configured channels", async () => {
40+
const paths = await resolveBundledChannelGatewayAuthBypassPaths({
3641
channelId: "mattermost",
3742
cfg: { channels: { mattermost: {} } },
3843
});
3944

4045
expect(paths).toEqual(["/api/channels/mattermost/command", "/api/channels/mattermost/work"]);
41-
expect(loadBundledPluginPublicArtifactModuleSyncMock).toHaveBeenCalledWith({
46+
expect(tryLoadActivatedBundledPluginPublicSurfaceModuleMock).toHaveBeenCalledWith({
4247
dirName: "mattermost",
4348
artifactBasename: "gateway-auth-api.js",
4449
});
4550
});
4651

47-
it("treats missing gateway auth artifacts as no bypass paths", () => {
48-
expect(
52+
it("treats missing gateway auth artifacts as no bypass paths", async () => {
53+
await expect(
4954
resolveBundledChannelGatewayAuthBypassPaths({
5055
channelId: "discord",
5156
cfg: { channels: { discord: {} } },
5257
}),
53-
).toStrictEqual([]);
58+
).resolves.toStrictEqual([]);
59+
});
60+
61+
it("returns no bypass paths when plugin activation blocks the artifact", async () => {
62+
await expect(
63+
resolveBundledChannelGatewayAuthBypassPaths({
64+
channelId: "disabledchannel",
65+
cfg: { channels: { disabledchannel: {} } },
66+
}),
67+
).resolves.toStrictEqual([]);
5468
});
5569

56-
it("surfaces errors from present gateway auth artifacts", () => {
57-
expect(() =>
70+
it("surfaces errors from present gateway auth artifacts", async () => {
71+
await expect(
5872
resolveBundledChannelGatewayAuthBypassPaths({
5973
channelId: "broken",
6074
cfg: { channels: { broken: {} } },
6175
}),
62-
).toThrow("broken gateway auth artifact");
76+
).rejects.toThrow("broken gateway auth artifact");
6377
});
6478
});
Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/**
2-
* Bundled channel gateway auth bypass loader.
2+
* Channel gateway auth bypass loader.
33
*
44
* Reads optional public artifacts that declare unauthenticated Gateway callback paths.
55
*/
66
import type { OpenClawConfig } from "../../config/types.openclaw.js";
7-
import { loadBundledPluginPublicArtifactModuleSync } from "../../plugins/public-surface-loader.js";
7+
import { tryLoadActivatedBundledPluginPublicSurfaceModule } from "../../plugin-sdk/facade-runtime.js";
88

99
/**
1010
* Lightweight public artifact contract for channel gateway auth bypass paths.
@@ -16,30 +16,34 @@ type GatewayAuthBypassApi = {
1616
const GATEWAY_AUTH_API_ARTIFACT_BASENAME = "gateway-auth-api.js";
1717
const MISSING_PUBLIC_SURFACE_PREFIX = "Unable to resolve bundled plugin public surface ";
1818

19-
function loadBundledChannelGatewayAuthApi(channelId: string): GatewayAuthBypassApi | undefined {
19+
/** Resolves to null when the plugin is not activated or ships no gateway auth artifact. */
20+
async function loadChannelGatewayAuthApi(channelId: string): Promise<GatewayAuthBypassApi | null> {
2021
try {
21-
return loadBundledPluginPublicArtifactModuleSync<GatewayAuthBypassApi>({
22+
// Bypass paths grant unauthenticated ingress, so resolution goes through the
23+
// activation-gated facade seam: it also covers installed (externalized) plugin
24+
// roots and returns null instead of executing a disabled plugin's artifact.
25+
return await tryLoadActivatedBundledPluginPublicSurfaceModule<GatewayAuthBypassApi>({
2226
dirName: channelId,
2327
artifactBasename: GATEWAY_AUTH_API_ARTIFACT_BASENAME,
2428
});
2529
} catch (error) {
2630
// Missing gateway auth artifacts are optional. Any other load failure means
2731
// the artifact exists but cannot be trusted, so propagate it to callers.
2832
if (error instanceof Error && error.message.startsWith(MISSING_PUBLIC_SURFACE_PREFIX)) {
29-
return undefined;
33+
return null;
3034
}
3135
throw error;
3236
}
3337
}
3438

3539
/**
36-
* Resolves configured gateway auth bypass paths from a bundled channel artifact.
40+
* Resolves configured gateway auth bypass paths from a channel plugin artifact.
3741
*/
38-
export function resolveBundledChannelGatewayAuthBypassPaths(params: {
42+
export async function resolveBundledChannelGatewayAuthBypassPaths(params: {
3943
channelId: string;
4044
cfg: OpenClawConfig;
41-
}): string[] {
42-
const api = loadBundledChannelGatewayAuthApi(params.channelId);
45+
}): Promise<string[]> {
46+
const api = await loadChannelGatewayAuthApi(params.channelId);
4347
const paths = api?.resolveGatewayAuthBypassPaths?.({ cfg: params.cfg }) ?? [];
4448
return paths.flatMap((path) => (typeof path === "string" && path.trim() ? [path.trim()] : []));
4549
}

src/gateway/server-http.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ async function resolvePluginGatewayAuthBypassPaths(
126126
return paths;
127127
}
128128
for (const channelId of Object.keys(configuredChannels)) {
129-
for (const path of resolveBundledChannelGatewayAuthBypassPaths({
129+
for (const path of await resolveBundledChannelGatewayAuthBypassPaths({
130130
channelId,
131131
cfg: configSnapshot,
132132
})) {
@@ -386,8 +386,9 @@ function buildPluginRequestStages(params: {
386386
if ((await params.getGatewayAuthBypassPaths()).has(params.requestPath)) {
387387
return false;
388388
}
389-
// Bypass paths are limited to bundled channel callbacks; all other protected plugin
390-
// routes must produce an AuthorizedGatewayHttpRequest before runtime scopes are derived.
389+
// Bypass paths come only from activated channel plugins' gateway-auth
390+
// artifacts (bundled or installed); all other protected plugin routes must
391+
// produce an AuthorizedGatewayHttpRequest before runtime scopes are derived.
391392
const { authorizeGatewayHttpRequestOrReply } = await getHttpAuthUtilsModule();
392393
const requestAuth = await authorizeGatewayHttpRequestOrReply({
393394
req: params.req,

src/plugin-sdk/facade-loader.ts

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import path from "node:path";
44
import { fileURLToPath, pathToFileURL } from "node:url";
55
import { openRootFileSync } from "../infra/boundary-file-read.js";
66
import { resolveBundledPluginsDir } from "../plugins/bundled-dir.js";
7+
import { shouldRejectHardlinkedPluginFiles } from "../plugins/hardlink-policy.js";
78
import {
89
getCachedPluginModuleLoader,
910
type PluginModuleLoaderCache,
@@ -129,6 +130,32 @@ export type FacadeModuleLocation = {
129130
boundaryRoot: string;
130131
};
131132

133+
function isPathAtOrInside(target: string, root: string): boolean {
134+
const resolvedRoot = path.resolve(root);
135+
const resolvedTarget = path.resolve(target);
136+
return resolvedTarget === resolvedRoot || resolvedTarget.startsWith(resolvedRoot + path.sep);
137+
}
138+
139+
// Registry-resolved locations can point at installed plugin roots, which must
140+
// reject hardlinked artifacts (see hardlink-policy.ts); core-shipped roots keep
141+
// hardlinks allowed for bundled dist/Nix layouts.
142+
function resolveFacadeBoundaryOpenParams(boundaryRoot: string): {
143+
boundaryLabel: string;
144+
rejectHardlinks: boolean;
145+
} {
146+
if (isPathAtOrInside(boundaryRoot, getOpenClawPackageRoot())) {
147+
return { boundaryLabel: "OpenClaw package root", rejectHardlinks: false };
148+
}
149+
const bundledDir = resolveBundledPluginsDir();
150+
if (bundledDir && isPathAtOrInside(boundaryRoot, bundledDir)) {
151+
return { boundaryLabel: "bundled plugin directory", rejectHardlinks: false };
152+
}
153+
return {
154+
boundaryLabel: "plugin root",
155+
rejectHardlinks: shouldRejectHardlinkedPluginFiles({ origin: "global", rootDir: boundaryRoot }),
156+
};
157+
}
158+
132159
/** Load and cache a facade module after verifying it is inside its declared boundary root. */
133160
export function loadFacadeModuleAtLocationSync<T extends object>(params: {
134161
location: FacadeModuleLocation;
@@ -144,16 +171,7 @@ export function loadFacadeModuleAtLocationSync<T extends object>(params: {
144171
const opened = openRootFileSync({
145172
absolutePath: location.modulePath,
146173
rootPath: location.boundaryRoot,
147-
boundaryLabel:
148-
location.boundaryRoot === getOpenClawPackageRoot()
149-
? "OpenClaw package root"
150-
: (() => {
151-
const bundledDir = resolveBundledPluginsDir();
152-
return bundledDir && path.resolve(location.boundaryRoot) === path.resolve(bundledDir)
153-
? "bundled plugin directory"
154-
: "plugin root";
155-
})(),
156-
rejectHardlinks: false,
174+
...resolveFacadeBoundaryOpenParams(location.boundaryRoot),
157175
});
158176
if (!opened.ok) {
159177
throw new Error(`Unable to open bundled plugin public surface ${location.modulePath}`, {
@@ -225,11 +243,7 @@ export async function loadBundledPluginPublicSurfaceModule<T extends object>(par
225243
const opened = openRootFileSync({
226244
absolutePath: preparedLocation.modulePath,
227245
rootPath: preparedLocation.boundaryRoot,
228-
boundaryLabel:
229-
preparedLocation.boundaryRoot === getOpenClawPackageRoot()
230-
? "OpenClaw package root"
231-
: "plugin root",
232-
rejectHardlinks: false,
246+
...resolveFacadeBoundaryOpenParams(preparedLocation.boundaryRoot),
233247
});
234248
if (!opened.ok) {
235249
throw new Error(`Unable to open bundled plugin public surface ${preparedLocation.modulePath}`, {

src/plugin-sdk/facade-runtime.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,41 @@ describe("plugin-sdk facade runtime", () => {
366366
expect(loader).toHaveBeenCalledTimes(1);
367367
});
368368

369+
it("rejects hardlinked artifacts under installed plugin roots", () => {
370+
const installedDir = createTempDirSync("openclaw-facade-hardlink-");
371+
const originalPath = path.join(installedDir, "original.js");
372+
fs.writeFileSync(originalPath, 'export const marker = "hardlinked";\n', "utf8");
373+
const artifactPath = path.join(installedDir, "runtime-api.js");
374+
fs.linkSync(originalPath, artifactPath);
375+
376+
// Installed roots are outside the package/bundled roots, so the facade
377+
// boundary open applies shouldRejectHardlinkedPluginFiles (nlink > 1 fails).
378+
expect(() =>
379+
testing.loadFacadeModuleAtLocationSync({
380+
location: { modulePath: artifactPath, boundaryRoot: installedDir },
381+
trackedPluginId: "line",
382+
}),
383+
).toThrow(`Unable to open bundled plugin public surface ${artifactPath}`);
384+
});
385+
386+
it("keeps hardlinked artifacts loadable under core-shipped roots", () => {
387+
const rootDir = createTrustedBundledFixtureRoot("openclaw-facade-hardlink-bundled-");
388+
const pluginDir = path.join(rootDir, "demo");
389+
fs.mkdirSync(pluginDir, { recursive: true });
390+
const originalPath = path.join(pluginDir, "original.js");
391+
fs.writeFileSync(originalPath, 'export const marker = "bundled-hardlink";\n', "utf8");
392+
const artifactPath = path.join(pluginDir, "api.js");
393+
fs.linkSync(originalPath, artifactPath);
394+
395+
const loader = vi.fn(() => ({ marker: "bundled-hardlink" }));
396+
const loaded = testing.loadFacadeModuleAtLocationSync<{ marker: string }>({
397+
location: { modulePath: artifactPath, boundaryRoot: rootDir },
398+
trackedPluginId: "demo",
399+
loadModule: loader,
400+
});
401+
expect(loaded.marker).toBe("bundled-hardlink");
402+
});
403+
369404
it("resolves a globally-installed plugin whose rootDir basename matches the dirName", () => {
370405
const lineDir = createTempDirSync("openclaw-facade-global-line-");
371406
fs.mkdirSync(lineDir, { recursive: true });

src/plugin-sdk/facade-runtime.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,14 @@ function loadFacadeActivationCheckRuntime(): FacadeActivationCheckRuntimeModule
161161
throw new Error("Unable to load facade activation check runtime");
162162
}
163163

164+
// Async twin of loadFacadeActivationCheckRuntime for async call sites: dynamic
165+
// import resolves the source graph under vitest where the sync createRequire/jiti
166+
// candidates cannot, and warms the shared memo so subsequent sync loads reuse it.
167+
async function loadFacadeActivationCheckRuntimeAsync(): Promise<FacadeActivationCheckRuntimeModule> {
168+
facadeActivationCheckRuntimeModule ??= await import("./facade-activation-check.runtime.js");
169+
return facadeActivationCheckRuntimeModule;
170+
}
171+
164172
function setFacadeActivationCheckRuntimeForTest(module: FacadeActivationCheckRuntimeModule): void {
165173
facadeActivationCheckRuntimeModule = module;
166174
}
@@ -255,6 +263,22 @@ export function tryLoadActivatedBundledPluginPublicSurfaceModuleSync<T extends o
255263
return loadBundledPluginPublicSurfaceModuleSync<T>(params);
256264
}
257265

266+
/** Async variant of tryLoadActivatedBundledPluginPublicSurfaceModuleSync for async call sites. */
267+
export async function tryLoadActivatedBundledPluginPublicSurfaceModule<T extends object>(params: {
268+
dirName: string;
269+
artifactBasename: string;
270+
env?: NodeJS.ProcessEnv;
271+
}): Promise<T | null> {
272+
const runtime = await loadFacadeActivationCheckRuntimeAsync();
273+
const access = runtime.resolveBundledPluginPublicSurfaceAccess(
274+
buildFacadeActivationCheckParams(params),
275+
);
276+
if (!access.allowed) {
277+
return null;
278+
}
279+
return loadBundledPluginPublicSurfaceModuleSync<T>(params);
280+
}
281+
258282
/** Reset facade runtime caches and activation-check test overrides. */
259283
export function resetFacadeRuntimeStateForTest(): void {
260284
resetFacadeLoaderStateForTest();

0 commit comments

Comments
 (0)