From f90972d9421dcc8dcb70ab87065e26a62752506c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 20:42:15 +0100 Subject: [PATCH] fix: install plugins through symlinked extension roots --- CHANGELOG.md | 1 + src/cli/plugins-cli.install.test.ts | 25 +++++++++++++ src/cli/plugins-command-helpers.ts | 6 ++++ src/infra/install-package-dir.ts | 4 +-- src/infra/install-safe-path.test.ts | 56 ++++++++++++++++++++--------- src/infra/install-safe-path.ts | 22 ++++++++++-- src/plugins/install.path.test.ts | 51 ++++++++++++++++++++++++++ 7 files changed, 144 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f78ddee8312..7d42b0704c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Sessions: ignore future-dated session activity timestamps during reset freshness checks and cap future `updatedAt` values at the merge boundary so clock-skewed messages cannot keep stale sessions alive forever. Fixes #72989. Thanks @martingarramon. +- Plugins/CLI: allow managed plugin installs when the active extensions root is a symlink to a real state directory, while keeping nested target symlinks blocked and suppressing misleading hook-pack fallback errors for install-boundary failures. Fixes #72946. Thanks @mayank6136. - Gateway/startup: keep hot Gateway boot paths on leaf config imports and add max-RSS reporting to the gateway startup bench so low-memory startup regressions are visible before release. Thanks @vincentkoc. - WebChat/TTS: persist automatic final-mode TTS audio as a supplemental audio-only transcript update instead of adding a second assistant message with the same visible text. Fixes #72830. Thanks @lhtpluto. - Agents/LSP: terminate bundled stdio LSP process trees during runtime disposal and Gateway shutdown, so nested children such as `tsserver` do not survive stop or restart. Fixes #72357. Thanks @ai-hpc and @bittoby. diff --git a/src/cli/plugins-cli.install.test.ts b/src/cli/plugins-cli.install.test.ts index 92ed192cbf1..b8c85613db9 100644 --- a/src/cli/plugins-cli.install.test.ts +++ b/src/cli/plugins-cli.install.test.ts @@ -1060,6 +1060,31 @@ describe("plugins cli install", () => { expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack"); }); + it("does not append hook-pack fallback details for managed extensions boundary failures", async () => { + const localPluginDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-local-plugin-")); + + loadConfig.mockReturnValue({} as OpenClawConfig); + installPluginFromPath.mockResolvedValue({ + ok: false, + error: "Invalid path: must stay within extensions directory", + }); + installHooksFromPath.mockResolvedValue({ + ok: false, + error: "package.json missing openclaw.hooks", + }); + + try { + await expect(runPluginsCommand(["plugins", "install", localPluginDir])).rejects.toThrow( + "__exit__:1", + ); + } finally { + fs.rmSync(localPluginDir, { recursive: true, force: true }); + } + + expect(runtimeErrors.at(-1)).toBe("Invalid path: must stay within extensions directory"); + expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack"); + }); + it("passes the install logger to the --link dry-run probe", async () => { const localPluginDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-link-plugin-")); const cfg = { diff --git a/src/cli/plugins-command-helpers.ts b/src/cli/plugins-command-helpers.ts index e0ce101951a..8bc2a39b0da 100644 --- a/src/cli/plugins-command-helpers.ts +++ b/src/cli/plugins-command-helpers.ts @@ -106,6 +106,12 @@ export function formatPluginInstallWithHookFallbackError( if (/plugin already exists: .+ \(delete it first\)/.test(pluginError)) { return `${pluginError}\nUse \`openclaw plugins update \` to upgrade the tracked plugin, or rerun install with \`--force\` to replace it.`; } + if ( + pluginError.startsWith("Invalid extensions directory:") || + pluginError === "Invalid path: must stay within extensions directory" + ) { + return pluginError; + } return `${pluginError}\nAlso not a valid hook pack: ${hookError}`; } diff --git a/src/infra/install-package-dir.ts b/src/infra/install-package-dir.ts index 3ad43a58c0a..357f800f165 100644 --- a/src/infra/install-package-dir.ts +++ b/src/infra/install-package-dir.ts @@ -120,8 +120,8 @@ async function assertInstallBaseStable(params: { installBaseDir: string; expectedRealPath: string; }): Promise { - const baseLstat = await fs.lstat(params.installBaseDir); - if (!baseLstat.isDirectory() || baseLstat.isSymbolicLink()) { + const baseStat = await fs.stat(params.installBaseDir); + if (!baseStat.isDirectory()) { throw new Error(INSTALL_BASE_CHANGED_ERROR_MESSAGE); } const currentRealPath = await fs.realpath(params.installBaseDir); diff --git a/src/infra/install-safe-path.test.ts b/src/infra/install-safe-path.test.ts index 30e2a5f753a..82538979223 100644 --- a/src/infra/install-safe-path.test.ts +++ b/src/infra/install-safe-path.test.ts @@ -136,7 +136,7 @@ describe("assertCanonicalPathWithinBase", () => { candidatePath: path.join(baseFile, "child"), boundaryLabel: "install directory", }), - ).rejects.toThrow(/base directory must be a real directory/i); + ).rejects.toThrow(/base directory must be a directory/i); }); }); @@ -173,19 +173,43 @@ describe("assertCanonicalPathWithinBase", () => { }, ); - it.runIf(process.platform !== "win32")("rejects symlinked base directories", async () => { - await withTempDir({ prefix: "openclaw-install-safe-" }, async (parentDir) => { - const realBaseDir = path.join(parentDir, "real-base"); - const symlinkBaseDir = path.join(parentDir, "base-link"); - await fs.mkdir(realBaseDir, { recursive: true }); - await fs.symlink(realBaseDir, symlinkBaseDir); - await expect( - assertCanonicalPathWithinBase({ - baseDir: symlinkBaseDir, - candidatePath: path.join(symlinkBaseDir, "tool"), - boundaryLabel: "install directory", - }), - ).rejects.toThrow(/base directory must be a real directory/i); - }); - }); + it.runIf(process.platform !== "win32")( + "accepts symlinked base directories when the target stays in the real base", + async () => { + await withTempDir({ prefix: "openclaw-install-safe-" }, async (parentDir) => { + const realBaseDir = path.join(parentDir, "real-base"); + const symlinkBaseDir = path.join(parentDir, "base-link"); + await fs.mkdir(realBaseDir, { recursive: true }); + await fs.symlink(realBaseDir, symlinkBaseDir); + await expect( + assertCanonicalPathWithinBase({ + baseDir: symlinkBaseDir, + candidatePath: path.join(symlinkBaseDir, "tool"), + boundaryLabel: "install directory", + }), + ).resolves.toBeUndefined(); + }); + }, + ); + + it.runIf(process.platform !== "win32")( + "rejects nested symlinked candidate directories", + async () => { + await withTempDir({ prefix: "openclaw-install-safe-" }, async (parentDir) => { + const realBaseDir = path.join(parentDir, "base"); + const nestedRealDir = path.join(parentDir, "nested-real"); + const nestedSymlinkDir = path.join(realBaseDir, "nested-link"); + await fs.mkdir(realBaseDir, { recursive: true }); + await fs.mkdir(nestedRealDir, { recursive: true }); + await fs.symlink(nestedRealDir, nestedSymlinkDir); + await expect( + assertCanonicalPathWithinBase({ + baseDir: realBaseDir, + candidatePath: path.join(nestedSymlinkDir, "tool"), + boundaryLabel: "install directory", + }), + ).rejects.toThrow(/must stay within install directory/i); + }); + }, + ); }); diff --git a/src/infra/install-safe-path.ts b/src/infra/install-safe-path.ts index e2a9a19f187..e9847ef5701 100644 --- a/src/infra/install-safe-path.ts +++ b/src/infra/install-safe-path.ts @@ -91,14 +91,30 @@ export async function assertCanonicalPathWithinBase(params: { } const baseLstat = await fs.lstat(baseDir); - if (!baseLstat.isDirectory() || baseLstat.isSymbolicLink()) { - throw new Error(`Invalid ${params.boundaryLabel}: base directory must be a real directory`); + if (baseLstat.isSymbolicLink()) { + const baseStat = await fs.stat(baseDir); + if (!baseStat.isDirectory()) { + throw new Error( + `Invalid ${params.boundaryLabel}: base directory must resolve to a directory`, + ); + } + } else if (!baseLstat.isDirectory()) { + throw new Error(`Invalid ${params.boundaryLabel}: base directory must be a directory`); } const baseRealPath = await fs.realpath(baseDir); const validateDirectory = async (dirPath: string): Promise => { + const resolvedDirPath = path.resolve(dirPath); const dirLstat = await fs.lstat(dirPath); - if (!dirLstat.isDirectory() || dirLstat.isSymbolicLink()) { + if (dirLstat.isSymbolicLink()) { + if (resolvedDirPath !== baseDir) { + throw new Error(`Invalid path: must stay within ${params.boundaryLabel}`); + } + const dirStat = await fs.stat(dirPath); + if (!dirStat.isDirectory()) { + throw new Error(`Invalid path: must stay within ${params.boundaryLabel}`); + } + } else if (!dirLstat.isDirectory()) { throw new Error(`Invalid path: must stay within ${params.boundaryLabel}`); } const dirRealPath = await fs.realpath(dirPath); diff --git a/src/plugins/install.path.test.ts b/src/plugins/install.path.test.ts index a4fd7f72399..7e005544696 100644 --- a/src/plugins/install.path.test.ts +++ b/src/plugins/install.path.test.ts @@ -105,6 +105,32 @@ function setupDualFormatInstallFixture(params: { bundleFormat: "codex" | "claude return { pluginDir, extensionsDir: path.join(stateDir, "extensions") }; } +function setupNativePluginInstallFixture() { + const caseDir = suiteTempRootTracker.makeTempDir(); + const stateDir = path.join(caseDir, "state"); + const pluginDir = path.join(caseDir, "plugin-src"); + fs.mkdirSync(path.join(pluginDir, "dist"), { recursive: true }); + fs.writeFileSync( + path.join(pluginDir, "package.json"), + JSON.stringify({ + name: "symlink-plugin", + version: "1.0.0", + openclaw: { extensions: ["./dist/index.js"] }, + }), + "utf-8", + ); + fs.writeFileSync( + path.join(pluginDir, "openclaw.plugin.json"), + JSON.stringify({ + id: "symlink-plugin", + configSchema: { type: "object", properties: {} }, + }), + "utf-8", + ); + fs.writeFileSync(path.join(pluginDir, "dist", "index.js"), "export {};\n", "utf-8"); + return { caseDir, pluginDir, extensionsDir: path.join(stateDir, "extensions") }; +} + async function installFromFileWithWarnings(params: { extensionsDir: string; filePath: string; @@ -263,6 +289,31 @@ describe("installPluginFromPath", () => { expect(fs.readFileSync(victimPath, "utf-8")).toBe("ORIGINAL"); }); + it.runIf(process.platform !== "win32")( + "installs local plugin directories when the managed extensions root is a symlink", + async () => { + const { caseDir, pluginDir, extensionsDir } = setupNativePluginInstallFixture(); + const realExtensionsDir = path.join(caseDir, "data", "extensions"); + fs.mkdirSync(realExtensionsDir, { recursive: true }); + fs.mkdirSync(path.dirname(extensionsDir), { recursive: true }); + fs.symlinkSync(realExtensionsDir, extensionsDir, "dir"); + + const result = await installPluginFromPath({ + path: pluginDir, + extensionsDir, + }); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + expect(result.targetDir).toBe(path.join(extensionsDir, "symlink-plugin")); + expect(fs.existsSync(path.join(realExtensionsDir, "symlink-plugin", "package.json"))).toBe( + true, + ); + }, + ); + it("installs Claude bundles from an archive path", async () => { const { pluginDir, extensionsDir } = setupBundleInstallFixture({ bundleFormat: "claude",