Skip to content

Commit e72d92e

Browse files
committed
Harden plugin remove against cwd and missing-path holes
Runner glue now goes through executePluginRemove so ownership and settings policy are tested with session cwd, not process.cwd(). Missing paths outside the origin plugins root are refused instead of treated as already gone.
1 parent 951101f commit e72d92e

5 files changed

Lines changed: 466 additions & 119 deletions

File tree

‎src/plugins/uninstall.test.ts‎

Lines changed: 183 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import { afterEach, describe, expect, test } from "bun:test";
2-
import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises";
2+
import { mkdir, mkdtemp, rm, stat, symlink, lstat, writeFile } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55

6+
import { SETTINGS_DIR_NAME } from "../branding.js";
7+
68
import type { PluginConfig } from "../config/settings.js";
79
import {
810
classifyPluginRemove,
911
deleteOwnedPluginDir,
1012
disablePluginSettings,
13+
executePluginRemove,
1114
isOwnedDiskInstall,
1215
nextPluginPathsAfterRemove,
1316
ownedDiskOriginRoot,
@@ -47,6 +50,7 @@ describe("deleteOwnedPluginDir", () => {
4750
pluginPath: plugin,
4851
originRoot: root,
4952
claudeRoot: join(home, ".claude"),
53+
cwd: home,
5054
});
5155
expect(result).toEqual({ ok: true });
5256
expect(await exists(plugin)).toBe(false);
@@ -63,6 +67,7 @@ describe("deleteOwnedPluginDir", () => {
6367
pluginPath: outside,
6468
originRoot: root,
6569
claudeRoot: join(home, ".claude"),
70+
cwd: home,
6671
});
6772
expect(result.ok).toBe(false);
6873
expect(await exists(outside)).toBe(true);
@@ -76,6 +81,7 @@ describe("deleteOwnedPluginDir", () => {
7681
pluginPath: root,
7782
originRoot: root,
7883
claudeRoot: join(home, ".claude"),
84+
cwd: home,
7985
});
8086
expect(result.ok).toBe(false);
8187
expect(await exists(root)).toBe(true);
@@ -92,6 +98,7 @@ describe("deleteOwnedPluginDir", () => {
9298
pluginPath: plugin,
9399
originRoot: root,
94100
claudeRoot,
101+
cwd: home,
95102
});
96103
expect(result.ok).toBe(false);
97104
if (!result.ok) expect(result.message).toContain("~/.claude");
@@ -107,13 +114,14 @@ describe("deleteOwnedPluginDir", () => {
107114
await mkdir(outside, { recursive: true });
108115
const claudeRoot = join(cwd, "home", ".claude");
109116
expect(
110-
await deleteOwnedPluginDir({ pluginPath: inside, originRoot: root, claudeRoot }),
117+
await deleteOwnedPluginDir({ pluginPath: inside, originRoot: root, claudeRoot, cwd }),
111118
).toEqual({ ok: true });
112119
expect(await exists(inside)).toBe(false);
113120
const refused = await deleteOwnedPluginDir({
114121
pluginPath: outside,
115122
originRoot: root,
116123
claudeRoot,
124+
cwd,
117125
});
118126
expect(refused.ok).toBe(false);
119127
expect(await exists(outside)).toBe(true);
@@ -128,10 +136,42 @@ describe("deleteOwnedPluginDir", () => {
128136
pluginPath: missing,
129137
originRoot: root,
130138
claudeRoot: join(home, ".claude"),
139+
cwd: home,
131140
});
132141
expect(result).toEqual({ ok: true });
133142
expect(await exists(missing)).toBe(false);
134143
});
144+
145+
test("missing path outside origin root is refused", async () => {
146+
const home = await tempDir("uninstall-missing-out-");
147+
const root = userPluginsRoot(home);
148+
await mkdir(root, { recursive: true });
149+
const missing = join(home, "elsewhere", "gone");
150+
const result = await deleteOwnedPluginDir({
151+
pluginPath: missing,
152+
originRoot: root,
153+
claudeRoot: join(home, ".claude"),
154+
cwd: home,
155+
});
156+
expect(result.ok).toBe(false);
157+
if (!result.ok) expect(result.message).toContain("outside the origin plugins root");
158+
});
159+
160+
test("dangling symlink inside origin root is removed", async () => {
161+
const home = await tempDir("uninstall-dangle-");
162+
const root = userPluginsRoot(home);
163+
await mkdir(root, { recursive: true });
164+
const plugin = join(root, "exa");
165+
await symlink(join(root, "missing-target"), plugin);
166+
const result = await deleteOwnedPluginDir({
167+
pluginPath: plugin,
168+
originRoot: root,
169+
claudeRoot: join(home, ".claude"),
170+
cwd: home,
171+
});
172+
expect(result).toEqual({ ok: true });
173+
await expect(lstat(plugin)).rejects.toMatchObject({ code: "ENOENT" });
174+
});
135175
});
136176

137177
describe("isOwnedDiskInstall", () => {
@@ -174,6 +214,20 @@ describe("isOwnedDiskInstall", () => {
174214
expect(isOwnedDiskInstall({ origin: "path", pluginPath, home, cwd })).toBe(false);
175215
expect(ownedDiskOriginRoot({ pluginPath, home, cwd })).toBeUndefined();
176216
});
217+
218+
test("path origin under projectPluginsRoot is owned for that cwd", () => {
219+
const cwd = "/tmp/project-a";
220+
const pluginPath = join(projectPluginsRoot(cwd), "from-path");
221+
expect(isOwnedDiskInstall({ origin: "path", pluginPath, home: "/tmp/home", cwd })).toBe(true);
222+
expect(
223+
isOwnedDiskInstall({
224+
origin: "path",
225+
pluginPath,
226+
home: "/tmp/home",
227+
cwd: "/tmp/other",
228+
}),
229+
).toBe(false);
230+
});
177231
});
178232

179233
describe("classifyPluginRemove", () => {
@@ -239,3 +293,130 @@ describe("plugin remove settings policy", () => {
239293
expect(shared.keptSharedRoot).toBe(true);
240294
});
241295
});
296+
297+
describe("executePluginRemove", () => {
298+
const expand = async (abs: string): Promise<readonly string[]> => [abs];
299+
300+
test("project plugin under session cwd is deleted; process.cwd() does not own it", async () => {
301+
const sessionCwd = await tempDir("remove-session-");
302+
const home = await tempDir("remove-home-");
303+
expect(sessionCwd).not.toBe(process.cwd());
304+
const plugin = join(projectPluginsRoot(sessionCwd), "local");
305+
await mkdir(plugin, { recursive: true });
306+
await writeFile(join(plugin, "manifest.json"), "{}");
307+
const ok = await executePluginRemove({
308+
id: "local",
309+
name: "local",
310+
origin: "project",
311+
pluginPath: plugin,
312+
hadTools: false,
313+
home,
314+
cwd: sessionCwd,
315+
plugins: { local: { enabled: true } },
316+
pluginPaths: [],
317+
otherLivePluginPaths: [],
318+
expandMembers: expand,
319+
});
320+
expect(ok.ok).toBe(true);
321+
if (ok.ok) {
322+
expect(ok.spliceLive).toBe(true);
323+
expect(ok.plugins.local?.enabled).toBe(false);
324+
expect("local" in ok.plugins).toBe(true);
325+
}
326+
expect(await exists(plugin)).toBe(false);
327+
328+
const other = join(projectPluginsRoot(sessionCwd), "other");
329+
await mkdir(other, { recursive: true });
330+
const refused = await executePluginRemove({
331+
id: "other",
332+
name: "other",
333+
origin: "project",
334+
pluginPath: other,
335+
hadTools: false,
336+
home,
337+
cwd: process.cwd(),
338+
plugins: { other: { enabled: true } },
339+
pluginPaths: [],
340+
otherLivePluginPaths: [],
341+
expandMembers: expand,
342+
});
343+
expect(refused.ok).toBe(false);
344+
expect(await exists(other)).toBe(true);
345+
});
346+
347+
test("relative pluginPath resolves against session cwd not process.cwd()", async () => {
348+
const sessionCwd = await tempDir("remove-rel-");
349+
const home = await tempDir("remove-rel-home-");
350+
expect(sessionCwd).not.toBe(process.cwd());
351+
const plugin = join(projectPluginsRoot(sessionCwd), "local");
352+
await mkdir(plugin, { recursive: true });
353+
await writeFile(join(plugin, "manifest.json"), "{}");
354+
const ok = await executePluginRemove({
355+
id: "local",
356+
name: "local",
357+
origin: "project",
358+
pluginPath: join(SETTINGS_DIR_NAME, "plugins", "local"),
359+
hadTools: false,
360+
home,
361+
cwd: sessionCwd,
362+
plugins: { local: { enabled: true } },
363+
pluginPaths: [],
364+
otherLivePluginPaths: [],
365+
expandMembers: expand,
366+
});
367+
expect(ok.ok).toBe(true);
368+
expect(await exists(plugin)).toBe(false);
369+
});
370+
371+
test("Claude marketplace disable keeps disk and writes enabled:false", async () => {
372+
const home = await tempDir("remove-claude-");
373+
const cwd = await tempDir("remove-claude-cwd-");
374+
const plugin = join(home, ".claude", "plugins", "exa");
375+
await mkdir(plugin, { recursive: true });
376+
await writeFile(join(plugin, "manifest.json"), "{}");
377+
const result = await executePluginRemove({
378+
id: "exa",
379+
name: "exa-search",
380+
origin: "user",
381+
pluginPath: plugin,
382+
hadTools: true,
383+
home,
384+
cwd,
385+
plugins: { exa: { enabled: true, credentials: { apiKey: "k" } } },
386+
pluginPaths: [],
387+
otherLivePluginPaths: [],
388+
expandMembers: expand,
389+
});
390+
expect(result.ok).toBe(true);
391+
if (result.ok) {
392+
expect(result.spliceLive).toBe(false);
393+
expect(result.plugins.exa?.enabled).toBe(false);
394+
expect(result.plugins.exa?.credentials).toEqual({ apiKey: "k" });
395+
expect("exa" in result.plugins).toBe(true);
396+
expect(result.message).toContain("Claude marketplace files were not removed");
397+
expect(result.message).toContain("Tools from this plugin stay until you restart");
398+
}
399+
expect(await exists(plugin)).toBe(true);
400+
});
401+
402+
test("bundled disable stays listed with enabled:false", async () => {
403+
const result = await executePluginRemove({
404+
id: "corbits-skills",
405+
name: "corbits-skills",
406+
origin: "repo",
407+
hadTools: false,
408+
home: "/tmp/home",
409+
cwd: "/tmp/cwd",
410+
plugins: { "corbits-skills": { enabled: true } },
411+
pluginPaths: [],
412+
otherLivePluginPaths: [],
413+
expandMembers: expand,
414+
});
415+
expect(result.ok).toBe(true);
416+
if (result.ok) {
417+
expect(result.spliceLive).toBe(false);
418+
expect(result.plugins["corbits-skills"]?.enabled).toBe(false);
419+
expect(result.message).toContain("cannot be uninstalled");
420+
}
421+
});
422+
});

0 commit comments

Comments
 (0)