Skip to content

Commit 37a14c9

Browse files
committed
fix(plugins): normalize file-path grants and surface marketplace members
A file-path add granted the raw file path while revoke removed the containing dir the loader stamps, leaving a live grant; a hybrid root granted marketplace siblings the result never named.
1 parent c586e1d commit 37a14c9

2 files changed

Lines changed: 120 additions & 12 deletions

File tree

‎src/trust/path-plugin-trust.test.ts‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,12 @@ await withMockedModule(
339339
);
340340
return real.trustPathPlugins(paths, home ?? addPathProbe.home);
341341
},
342+
// revokeTrust in the backend calls without a home (production default).
343+
// Redirect here too so an add→revoke round trip in this file reads back
344+
// the same store the grant went to.
345+
revokePathPlugin: async (path: string, home?: string) => {
346+
return real.revokePathPlugin(path, home ?? addPathProbe.home);
347+
},
342348
}),
343349
);
344350

@@ -356,6 +362,7 @@ async function makeAddPathAdmin(base: string): Promise<{
356362
addPath: (
357363
path: string,
358364
) => Promise<{ ok: boolean; message: string; id?: string }>;
365+
revokeTrust: (id: string) => Promise<{ ok: boolean; message: string }>;
359366
}> {
360367
const backend = await import("../../src/tui/plugins-admin-backend.js");
361368
const emptyProjectTrust: ProjectTrustStore = {
@@ -439,7 +446,7 @@ describe("addPath grants path trust before importing plugin code", () => {
439446
const result = await admin.addPath("marketplace");
440447
expect(result).toEqual({
441448
ok: true,
442-
message: "Added mkt-root",
449+
message: `Added mkt-root (trusted 2 marketplace members: ${alpha}, ${beta})`,
443450
id: "mkt-root",
444451
});
445452

@@ -517,4 +524,89 @@ describe("addPath grants path trust before importing plugin code", () => {
517524
await rm(base, { recursive: true, force: true });
518525
}
519526
});
527+
528+
test("file-path add grants the containing dir so revokeTrust clears it and reload stays metadata-only", async () => {
529+
const base = await mkdtemp(join(tmpdir(), "corbits-addpath-file-"));
530+
try {
531+
const home = join(base, "home");
532+
await mkdir(home, { recursive: true });
533+
const pluginDir = join(base, "p");
534+
const marker = join(base, "FILE_MARKER");
535+
await writeCommandPlugin(pluginDir, "file-plugin", marker);
536+
resetAddPathProbe(home, marker);
537+
538+
const admin = await makeAddPathAdmin(base);
539+
// Operator points at the file, not the directory.
540+
const result = await admin.addPath(join(pluginDir, "index.ts"));
541+
expect(result).toEqual({
542+
ok: true,
543+
message: "Added file-plugin",
544+
id: "file-plugin",
545+
});
546+
// The grant is the normalized dir — the identity loadPluginEntry stamps
547+
// and revokeTrust removes — never the raw file path.
548+
expect(addPathProbe.trustCalls).toEqual([[pluginDir]]);
549+
expect((await loadPathTrust(home)).trustedPluginPaths).toEqual([
550+
pluginDir,
551+
]);
552+
expect(await Bun.file(marker).exists()).toBe(true);
553+
554+
const revoked = await admin.revokeTrust("file-plugin");
555+
expect(revoked.ok).toBe(true);
556+
expect((await loadPathTrust(home)).trustedPluginPaths).toEqual([]);
557+
558+
// Next boot resolves the persisted entry against the emptied store: the
559+
// module stays metadata-only and its code never re-executes.
560+
await rm(marker, { force: true });
561+
const store = await loadPathTrust(home);
562+
const mods = await loadPluginsFromPaths([pluginDir], base, {
563+
isPluginTrusted: (p) => isPathPluginTrusted(store, p),
564+
});
565+
expect(
566+
mods.find((m) => m.manifest?.id === "file-plugin")?.metadataOnly,
567+
).toBe(true);
568+
expect(await Bun.file(marker).exists()).toBe(false);
569+
} finally {
570+
await rm(base, { recursive: true, force: true });
571+
}
572+
});
573+
574+
test("hybrid root add surfaces exactly the expanded member set", async () => {
575+
const base = await mkdtemp(join(tmpdir(), "corbits-addpath-hybrid-"));
576+
try {
577+
const home = join(base, "home");
578+
await mkdir(home, { recursive: true });
579+
const root = join(base, "hybrid");
580+
const rootMarker = join(base, "ROOT_MARKER");
581+
await writeCommandPlugin(root, "hybrid-root", rootMarker);
582+
const sibling = join(base, "agents", "evil-sibling");
583+
const siblingMarker = join(base, "SIBLING_MARKER");
584+
await writeCommandPlugin(sibling, "evil-sibling", siblingMarker);
585+
await mkdir(join(root, ".claude-plugin"), { recursive: true });
586+
await writeFile(
587+
join(root, ".claude-plugin", "marketplace.json"),
588+
JSON.stringify({
589+
plugins: [{ name: "evil-sibling", source: "../agents/evil-sibling" }],
590+
}),
591+
"utf8",
592+
);
593+
resetAddPathProbe(home, rootMarker);
594+
595+
const admin = await makeAddPathAdmin(base);
596+
const result = await admin.addPath("hybrid");
597+
// Only the expanded member set is granted, and the result names it so
598+
// the operator sees the sibling consent covers.
599+
expect(addPathProbe.trustCalls).toEqual([[sibling]]);
600+
expect((await loadPathTrust(home)).trustedPluginPaths).toEqual([sibling]);
601+
expect(result).toEqual({
602+
ok: true,
603+
message: `Added hybrid-root (trusted 1 marketplace member: ${sibling})`,
604+
id: "hybrid-root",
605+
});
606+
// The sibling is granted but never imported by the add itself.
607+
expect(await Bun.file(siblingMarker).exists()).toBe(false);
608+
} finally {
609+
await rm(base, { recursive: true, force: true });
610+
}
611+
});
520612
});

‎src/tui/plugins-admin-backend.ts‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -302,17 +302,22 @@ export function createPluginsAdmin(args: {
302302
if (probe === null)
303303
return { ok: false, message: `Could not load a plugin at ${path}` };
304304
// Persist global path trust only once the path resolves to a real
305-
// plugin, so a bogus path never leaves a dangling entry. Expand
306-
// marketplaces so each member is trusted (exact-path match on reload).
305+
// plugin, so a bogus path never leaves a dangling entry. The granted
306+
// identity is the probe's normalized pluginPath (the containing dir for
307+
// a file entry), not the raw typed path: loadPluginEntry stamps
308+
// pluginPath=dirname for file entries and revokeTrust removes exactly
309+
// that stamped path, so granting the raw file path would leave a grant
310+
// no revoke can clear. Expand marketplaces so each member is trusted
311+
// (exact-path match on reload).
307312
// `onSkip` collects into `addDiag` instead of a raw stderr write — a raw
308313
// write lands mid-frame and corrupts the rendered transcript.
314+
const grantBase = probe.pluginPath ?? abs;
309315
const addDiag = createPluginLoadDiagnostics();
310-
const members = await expandPluginPath(abs, {
316+
const members = await expandPluginPath(grantBase, {
311317
onSkip: expandSkipDiagnosticsHandler(addDiag),
312318
});
313-
state.pathTrust = await trustPathPlugins(
314-
members.length > 0 ? members : [abs],
315-
);
319+
const granted = members.length > 0 ? members : [grantBase];
320+
state.pathTrust = await trustPathPlugins(granted);
316321
// Only now import plugin code: the grant above is the user's explicit
317322
// consent. A failed import keeps the grant and reports the same load
318323
// error as before — only bogus paths return grantless above.
@@ -365,18 +370,29 @@ export function createPluginsAdmin(args: {
365370
descriptor.id,
366371
);
367372
registerCommandPluginModule(mod, () => state.pluginConfig);
368-
// Persist the resolved absolute path so it reloads regardless of the cwd
369-
// the next session starts from.
370-
if (!state.pluginPaths.includes(abs)) state.pluginPaths.push(abs);
373+
// Persist the normalized identity (not the raw typed path) so reload-time
374+
// trust checks match the grant: a file entry reloads as its containing
375+
// dir, which is exactly what was granted above.
376+
if (!state.pluginPaths.includes(grantBase))
377+
state.pluginPaths.push(grantBase);
371378
await persistPluginSettings();
372379
const warnings = formatPluginWarningsSummary(addDiag.warnings);
373380
args.noteWarnings(addDiag.warnings);
381+
// Member-aware consent: a hybrid root (own manifest plus marketplace
382+
// members) grants its expanded members, not just the typed path — the
383+
// result names exactly what was granted so the operator consents to the
384+
// siblings too. A single-path grant keeps the plain message.
385+
const grantedSuffix =
386+
granted.length === 1 && granted[0] === grantBase
387+
? ""
388+
: ` (trusted ${granted.length} marketplace member${granted.length === 1 ? "" : "s"}: ${granted.join(", ")})`;
389+
const addedMessage = `Added ${descriptor.name}${grantedSuffix}`;
374390
return {
375391
ok: true,
376392
message:
377393
warnings === undefined
378-
? `Added ${descriptor.name}`
379-
: `Added ${descriptor.name} (${warnings})`,
394+
? addedMessage
395+
: `${addedMessage} (${warnings})`,
380396
id: descriptor.id,
381397
};
382398
},

0 commit comments

Comments
 (0)