fix(plugins): harden managed plugin install lifecycle

This commit is contained in:
Vincent Koc
2026-05-01 14:07:13 -07:00
parent 5c7362fe9d
commit e302353d61
7 changed files with 316 additions and 23 deletions

View File

@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
- Gateway/config: report failed backup restores as failed in logs and config observe audit records instead of marking them valid. (#70515) Thanks @davidangularme.
- Compaction: use the active session model fallback chain for implicit summarization failures without persisting fallback model selection, so Azure content-filter 400s can recover. Fixes #64960. (#74470) Thanks @jalehman and @OpenCodeEngineer.
- Gateway/config: allow `gateway config.patch` to update documented subagent thinking defaults. Fixes #75764. (#75802) Thanks @kAIborg24.
- Plugins/CLI: keep git plugin install paths credential-free, preserve existing git checkouts until replacement succeeds, honor duplicate npm install mode, and remove managed git repos on uninstall. Thanks @vincentkoc.
## 2026.4.30

View File

@@ -1,4 +1,9 @@
import { createHash } from "node:crypto";
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { describe, expect, it, vi, beforeEach } from "vitest";
import { redactSensitiveUrlLikeString } from "../shared/net/redact-sensitive-url.js";
const runCommandWithTimeoutMock = vi.fn();
const installPluginFromInstalledPackageDirMock = vi.fn();
@@ -20,6 +25,14 @@ vi.resetModules();
const { installPluginFromGitSpec, parseGitPluginSpec } = await import("./git-install.js");
function expectedGitRepoDir(params: { gitDir: string; normalizedSpec: string }): string {
const hash = createHash("sha256")
.update(redactSensitiveUrlLikeString(params.normalizedSpec))
.digest("hex")
.slice(0, 16);
return path.join(params.gitDir, `git-${hash}`, "repo");
}
describe("parseGitPluginSpec", () => {
it("normalizes GitHub shorthand and ref selectors", () => {
expect(parseGitPluginSpec("git:github.com/acme/demo@v1.2.3")).toMatchObject({
@@ -55,13 +68,18 @@ describe("installPluginFromGitSpec", () => {
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "abc123\n", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" });
installPluginFromInstalledPackageDirMock.mockResolvedValue({
ok: true,
pluginId: "demo",
targetDir: "/tmp/git-root/repo",
version: "1.2.3",
extensions: ["index.js"],
});
installPluginFromInstalledPackageDirMock.mockImplementation(
async (params: { packageDir: string }) => {
await fs.mkdir(params.packageDir, { recursive: true });
return {
ok: true,
pluginId: "demo",
targetDir: params.packageDir,
version: "1.2.3",
extensions: ["index.js"],
};
},
);
const result = await installPluginFromGitSpec({
spec: "git:github.com/acme/demo@v1.2.3",
@@ -115,13 +133,18 @@ describe("installPluginFromGitSpec", () => {
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "abc123\n", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" });
installPluginFromInstalledPackageDirMock.mockResolvedValue({
ok: true,
pluginId: "demo",
targetDir: "/tmp/git-root/repo",
version: "1.2.3",
extensions: ["index.js"],
});
installPluginFromInstalledPackageDirMock.mockImplementation(
async (params: { packageDir: string }) => {
await fs.mkdir(params.packageDir, { recursive: true });
return {
ok: true,
pluginId: "demo",
targetDir: params.packageDir,
version: "1.2.3",
extensions: ["index.js"],
};
},
);
await installPluginFromGitSpec({ spec: "git:github.com/acme/demo" });
@@ -134,4 +157,76 @@ describe("installPluginFromGitSpec", () => {
expect.stringContaining("/repo"),
]);
});
it("uses a credential-free managed repo path for authenticated git URLs", async () => {
const gitDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-git-install-path-"));
try {
runCommandWithTimeoutMock
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "abc123\n", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" });
installPluginFromInstalledPackageDirMock.mockImplementation(
async (params: { packageDir: string }) => {
await fs.mkdir(params.packageDir, { recursive: true });
return {
ok: true,
pluginId: "demo",
targetDir: params.packageDir,
version: "1.2.3",
extensions: ["index.js"],
};
},
);
const result = await installPluginFromGitSpec({
spec: "git:https://token@github.com/acme/demo.git",
gitDir,
});
expect(result.ok).toBe(true);
if (!result.ok) {
throw new Error(result.error);
}
expect(result.targetDir).toBe(
expectedGitRepoDir({
gitDir,
normalizedSpec: "git:https://token@github.com/acme/demo.git",
}),
);
expect(result.targetDir).not.toContain("token");
expect(result.targetDir).not.toContain("github.com");
} finally {
await fs.rm(gitDir, { recursive: true, force: true });
}
});
it("keeps the existing managed repo when replacement install fails", async () => {
const gitDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-git-install-preserve-"));
const normalizedSpec = "git:https://github.com/acme/demo.git";
const existingRepoDir = expectedGitRepoDir({ gitDir, normalizedSpec });
const markerPath = path.join(existingRepoDir, "existing.txt");
try {
await fs.mkdir(existingRepoDir, { recursive: true });
await fs.writeFile(markerPath, "keep");
runCommandWithTimeoutMock
.mockResolvedValueOnce({ code: 0, stdout: "", stderr: "" })
.mockResolvedValueOnce({ code: 0, stdout: "abc123\n", stderr: "" })
.mockResolvedValueOnce({ code: 1, stdout: "", stderr: "npm failed" });
const result = await installPluginFromGitSpec({
spec: "git:https://github.com/acme/demo.git",
gitDir,
mode: "update",
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toContain("npm install failed");
}
await expect(fs.readFile(markerPath, "utf8")).resolves.toBe("keep");
expect(installPluginFromInstalledPackageDirMock).not.toHaveBeenCalled();
} finally {
await fs.rm(gitDir, { recursive: true, force: true });
}
});
});

View File

@@ -1,6 +1,6 @@
import { createHash } from "node:crypto";
import fs from "node:fs/promises";
import path from "node:path";
import { safePathSegmentHashed } from "../infra/install-safe-path.js";
import { withTempDir } from "../infra/install-source-utils.js";
import {
createSafeNpmInstallArgs,
@@ -183,7 +183,50 @@ function resolveGitInstallRepoDir(params: {
source: ParsedGitPluginSpec;
}): string {
const gitRoot = params.gitDir ? resolveUserPath(params.gitDir) : resolveDefaultPluginGitDir();
return path.join(gitRoot, safePathSegmentHashed(params.source.normalizedSpec), "repo");
const redactedSpec = redactSensitiveUrlLikeString(params.source.normalizedSpec);
const hash = createHash("sha256").update(redactedSpec).digest("hex").slice(0, 16);
return path.join(gitRoot, `git-${hash}`, "repo");
}
async function replaceManagedGitRepo(params: {
stagedRepoDir: string;
persistentRepoDir: string;
}): Promise<{ ok: true } | { ok: false; error: string }> {
const parentDir = path.dirname(params.persistentRepoDir);
const backupDir = path.join(parentDir, `.repo-backup-${process.pid}-${Date.now()}`);
let backupCreated = false;
try {
await fs.mkdir(parentDir, { recursive: true });
try {
await fs.rename(params.persistentRepoDir, backupDir);
backupCreated = true;
} catch (err) {
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
throw err;
}
}
try {
await fs.rename(params.stagedRepoDir, params.persistentRepoDir);
} catch (err) {
if (backupCreated) {
await fs.rename(backupDir, params.persistentRepoDir);
backupCreated = false;
}
throw err;
}
if (backupCreated) {
await fs.rm(backupDir, { recursive: true, force: true });
}
return { ok: true };
} catch (err) {
return {
ok: false,
error: `failed to replace managed git plugin repository: ${String(err)}`,
};
}
}
function formatGitCommandFailure(params: {
@@ -244,11 +287,7 @@ export async function installPluginFromGitSpec(
const persistentRepoDir = resolveGitInstallRepoDir({ gitDir: params.gitDir, source: parsed });
return await withTempDir("openclaw-git-plugin-", async (tmpDir) => {
const repoDir = params.dryRun ? path.join(tmpDir, "repo") : persistentRepoDir;
if (!params.dryRun) {
await fs.rm(repoDir, { recursive: true, force: true });
await fs.mkdir(path.dirname(repoDir), { recursive: true });
}
const repoDir = path.join(tmpDir, "repo");
params.logger?.info?.(
`Cloning ${sanitizeForLog(redactSensitiveUrlLikeString(parsed.label))}...`,
);
@@ -330,9 +369,19 @@ export async function installPluginFromGitSpec(
if (!result.ok) {
return result;
}
if (!params.dryRun) {
const replaceResult = await replaceManagedGitRepo({
stagedRepoDir: repoDir,
persistentRepoDir,
});
if (!replaceResult.ok) {
return replaceResult;
}
}
return {
...result,
targetDir: params.dryRun ? result.targetDir : persistentRepoDir,
git: {
url: parsed.url,
ref: parsed.ref,

View File

@@ -269,6 +269,36 @@ describe("installPluginFromNpmSpec", () => {
}
});
it("rejects duplicate npm installs unless update mode is requested", async () => {
const stateDir = suiteTempRootTracker.makeTempDir();
const npmRoot = path.join(stateDir, "npm");
const installRoot = path.join(npmRoot, "node_modules", "@openclaw", "voice-call");
fs.mkdirSync(installRoot, { recursive: true });
mockNpmViewMetadataResult(runCommandWithTimeoutMock, {
name: "@openclaw/voice-call",
version: "0.0.1",
integrity: "sha512-plugin-test",
shasum: "pluginshasum",
});
const result = await installPluginFromNpmSpec({
spec: "@openclaw/voice-call@0.0.1",
npmDir: npmRoot,
mode: "install",
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toContain("plugin already exists");
expect(result.error).toContain(installRoot);
}
expect(
runCommandWithTimeoutMock.mock.calls.some(
(call) => Array.isArray(call[0]) && call[0][0] === "npm" && call[0][1] === "install",
),
).toBe(false);
});
it("aborts when integrity drift callback rejects the fetched artifact", async () => {
mockNpmViewMetadataResult(runCommandWithTimeoutMock, {
name: "@openclaw/voice-call",

View File

@@ -1082,7 +1082,7 @@ export async function installPluginFromNpmSpec(
},
): Promise<InstallPluginResult> {
const runtime = await loadPluginInstallRuntime();
const { logger, timeoutMs, dryRun } = runtime.resolveTimedInstallModeOptions(
const { logger, timeoutMs, mode, dryRun } = runtime.resolveTimedInstallModeOptions(
params,
defaultLogger,
);
@@ -1148,6 +1148,19 @@ export async function installPluginFromNpmSpec(
const npmRoot = params.npmDir ? resolveUserPath(params.npmDir) : resolveDefaultPluginNpmDir();
const installRoot = path.join(npmRoot, "node_modules", parsedSpec.name);
const effectiveMode = await resolveEffectiveInstallMode({
runtime,
requestedMode: mode,
targetPath: installRoot,
});
const availability = await ensureInstallTargetAvailableForMode({
runtime,
targetPath: installRoot,
mode: effectiveMode,
});
if (!availability.ok) {
return availability;
}
if (dryRun) {
return {
ok: true,
@@ -1192,6 +1205,7 @@ export async function installPluginFromNpmSpec(
dependencyTreeRootDir: npmRoot,
logger,
expectedPluginId,
mode: effectiveMode,
installPolicyRequest: {
kind: "plugin-npm",
requestedSpecifier: spec,

View File

@@ -115,6 +115,16 @@ function createNpmInstallRecord(pluginId = "my-plugin", installPath?: string): P
};
}
function createGitInstallRecord(pluginId = "my-plugin", installPath?: string): PluginInstallRecord {
return {
source: "git",
spec: `git:https://github.com/acme/${pluginId}.git`,
gitUrl: `https://github.com/acme/${pluginId}.git`,
gitCommit: "abc123",
...(installPath ? { installPath } : {}),
};
}
function createPathInstallRecord(
installPath = "/path/to/plugin",
sourcePath = installPath,
@@ -905,6 +915,31 @@ describe("uninstallPlugin", () => {
});
await expect(fs.access(installPath)).rejects.toThrow();
});
it("deletes managed git install repos outside the extensions directory", async () => {
const stateDir = path.join(tempDir, "state");
const extensionsDir = path.join(stateDir, "extensions");
const installPath = path.join(stateDir, "git", "git-abc123", "repo");
await fs.mkdir(installPath, { recursive: true });
await fs.writeFile(path.join(installPath, "index.js"), "// git plugin");
const result = await uninstallPlugin({
config: createPluginConfig({
entries: createSinglePluginEntries(),
installs: {
"my-plugin": createGitInstallRecord("my-plugin", installPath),
},
}),
pluginId: "my-plugin",
deleteFiles: true,
extensionsDir,
});
expectSuccessfulUninstallActions(result, {
directory: true,
});
await expect(fs.access(installPath)).rejects.toThrow();
});
});
describe("resolveUninstallDirectoryTarget", () => {
@@ -993,6 +1028,38 @@ describe("resolveUninstallDirectoryTarget", () => {
).toBe(installPath);
});
it("uses configured installPath when git installed it under the managed git root", () => {
const stateDir = path.join(os.tmpdir(), "openclaw-uninstall-safe");
const extensionsDir = path.join(stateDir, "extensions");
const installPath = path.join(stateDir, "git", "git-abc123", "repo");
expect(
resolveUninstallDirectoryTarget({
pluginId: "my-plugin",
hasInstall: true,
installRecord: createGitInstallRecord("my-plugin", installPath),
extensionsDir,
}),
).toBe(installPath);
});
it("does not trust git install paths outside the managed git root", () => {
const stateDir = path.join(os.tmpdir(), "openclaw-uninstall-safe");
const extensionsDir = path.join(stateDir, "extensions");
expect(
resolveUninstallDirectoryTarget({
pluginId: "my-plugin",
hasInstall: true,
installRecord: createGitInstallRecord(
"my-plugin",
path.join(os.tmpdir(), "git", "git-abc123", "repo"),
),
extensionsDir,
}),
).toBe(resolvePluginInstallDir("my-plugin", extensionsDir));
});
it("does not trust npm install paths outside the managed npm root", () => {
const stateDir = path.join(os.tmpdir(), "openclaw-uninstall-safe");
const extensionsDir = path.join(stateDir, "extensions");

View File

@@ -4,7 +4,11 @@ import path from "node:path";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import type { PluginInstallRecord } from "../config/types.plugins.js";
import { formatErrorMessage } from "../infra/errors.js";
import { resolveDefaultPluginNpmDir, resolvePluginInstallDir } from "./install-paths.js";
import {
resolveDefaultPluginGitDir,
resolveDefaultPluginNpmDir,
resolvePluginInstallDir,
} from "./install-paths.js";
import { defaultSlotIdForKey } from "./slots.js";
export type UninstallActions = {
@@ -121,6 +125,13 @@ export function resolveUninstallDirectoryTarget(params: {
if (npmManagedPath) {
return npmManagedPath;
}
const gitManagedPath = resolveGitManagedInstallPath({
installRecord: params.installRecord,
extensionsDir: params.extensionsDir,
});
if (gitManagedPath) {
return gitManagedPath;
}
let defaultPath: string;
try {
@@ -182,6 +193,32 @@ function resolveNpmManagedInstallPath(params: {
return null;
}
function resolveGitManagedInstallPath(params: {
installRecord?: PluginInstallRecord;
extensionsDir?: string;
}): string | null {
const installPath = params.installRecord?.installPath?.trim();
if (params.installRecord?.source !== "git" || !installPath) {
return null;
}
const gitRoots = new Set<string>();
if (params.extensionsDir) {
gitRoots.add(path.join(path.dirname(path.resolve(params.extensionsDir)), "git"));
}
gitRoots.add(resolveDefaultPluginGitDir());
for (const gitRoot of gitRoots) {
if (
isPathInsideOrEqual(gitRoot, installPath) &&
resolveComparablePath(gitRoot) !== resolveComparablePath(installPath)
) {
return installPath;
}
}
return null;
}
function resolveRecordedManagedInstallPath(params: {
pluginId: string;
installPath: string;