Skip to content

Commit da405c2

Browse files
fix(doctor): tighten staged-marketplace import proof and duplicate-hook matching
Address Codex review round 4 on #414: - AB7323 only accepts a receipted cache entry whose segment matches the staged HEAD commit (or plugin version when the commit is unknown), and skips malformed cache entries instead of aborting Doctor. - The staged plugins/<entry>/.cursor-plugin/plugin.json must be named <entry>, otherwise the repository is corrupt. - AB7322 matches plugin paths on a component boundary so a sibling plugin with a shared path prefix is not flagged as duplicate delivery.
1 parent 8ce842a commit da405c2

2 files changed

Lines changed: 75 additions & 17 deletions

File tree

‎packages/agent-bundle/src/install/cursor-hooks-registration.ts‎

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,17 @@ const userHookDuplicates = async (
120120
};
121121
}
122122
const resolvedPlugin = resolve(pluginDirectory);
123-
const duplicates = parsed.commands.filter((command) => command.includes(resolvedPlugin));
123+
// Match on a path-component boundary so `plugins/local/foo` does not claim hooks aimed at `plugins/local/foo-tools`.
124+
const pointsIntoPlugin = (command: string): boolean => {
125+
let offset = command.indexOf(resolvedPlugin);
126+
while (offset !== -1) {
127+
const next = command.charAt(offset + resolvedPlugin.length);
128+
if (next === '' || next === '/' || /["'\s]/u.test(next)) return true;
129+
offset = command.indexOf(resolvedPlugin, offset + 1);
130+
}
131+
return false;
132+
};
133+
const duplicates = parsed.commands.filter(pointsIntoPlugin);
124134
if (duplicates.length === 0) return { diagnostics: Object.freeze([]), duplicates: Object.freeze([]) };
125135
return {
126136
diagnostics: Object.freeze([finding(
@@ -233,29 +243,45 @@ export interface CursorMarketplaceStagingFinding {
233243
/**
234244
* Cursor's cache is partitioned per marketplace: `plugins/cache/<sanitize(marketplaceSlug)>/<pluginId>/<version>`
235245
* (observed 2026-09-03; e.g. `cache/cursor-public/continual-learning/<sha>/`), and Cursor writes an empty
236-
* `.cache-complete` receipt beside `.cursor-plugin/` once the copy finished. Only a receipted entry in the
237-
* staged marketplace's partition proves that *this* staged repository was imported; the same plugin cached
238-
* from another marketplace, or a half-written cache directory, does not.
246+
* `.cache-complete` receipt beside `.cursor-plugin/` once the copy finished. The `<version>` segment observed
247+
* for Git-backed marketplaces is the marketplace commit SHA. Only a receipted entry in the staged
248+
* marketplace's partition whose segment matches the staged HEAD commit (or, when the commit is unknown,
249+
* the plugin version) proves that *this* staged repository was imported; the same plugin cached from another
250+
* marketplace, a receipt from an earlier staging commit, or a half-written cache directory, does not.
251+
* Malformed cache entries are skipped rather than aborting Doctor.
239252
*/
240253
const sanitizeCacheSegment = (segment: string): string => segment.replaceAll(/[^A-Za-z0-9._-]/gu, '-');
241254

255+
const readableFile = async (path: string): Promise<boolean> => {
256+
try {
257+
await readFile(path);
258+
return true;
259+
} catch {
260+
return false;
261+
}
262+
};
263+
242264
const cacheHasPlugin = async (
243265
home: string,
244266
marketplace: string,
245267
name: string,
246268
version: string | undefined,
269+
commit: string | undefined,
247270
): Promise<boolean> => {
248271
const pluginRoot = join(home, '.cursor', 'plugins', 'cache', sanitizeCacheSegment(marketplace), sanitizeCacheSegment(name));
249-
let versions: readonly string[];
272+
let segments: readonly string[];
250273
try {
251-
versions = await readdir(pluginRoot);
252-
} catch (error) {
253-
if (isErrno(error, 'ENOENT') || isErrno(error, 'ENOTDIR')) return false;
254-
throw error;
274+
segments = await readdir(pluginRoot);
275+
} catch {
276+
return false;
255277
}
256-
for (const entry of versions) {
257-
if (!(await exists(join(pluginRoot, entry, '.cache-complete')))) continue;
258-
const installed = await readJson(join(pluginRoot, entry, '.cursor-plugin', 'plugin.json'));
278+
const expectedSegments = new Set(
279+
[commit, version].filter((candidate): candidate is string => candidate !== undefined).map(sanitizeCacheSegment),
280+
);
281+
for (const segment of segments) {
282+
if (expectedSegments.size > 0 && !expectedSegments.has(segment)) continue;
283+
if (!(await readableFile(join(pluginRoot, segment, '.cache-complete')))) continue;
284+
const installed = await readJson(join(pluginRoot, segment, '.cursor-plugin', 'plugin.json'));
259285
if (installed.error !== undefined || !Predicate.isObject(installed.value)) continue;
260286
if (installed.value.name !== name) continue;
261287
if (version === undefined || installed.value.version === version) return true;
@@ -316,19 +342,20 @@ export const inspectCursorMarketplaceStaging = async (
316342
const listsPlugin = manifest.error === undefined && Predicate.isObject(manifest.value) && Array.isArray(manifest.value.plugins) &&
317343
manifest.value.plugins.some((candidate: unknown) =>
318344
Predicate.isObject(candidate) && candidate.name === entry && candidate.source === `plugins/${entry}`);
319-
if (marketplace === undefined || !listsPlugin || plugin.error !== undefined || !(await exists(join(path, '.git')))) {
345+
const pluginNamed = plugin.error === undefined && Predicate.isObject(plugin.value) && plugin.value.name === entry;
346+
if (marketplace === undefined || !listsPlugin || !pluginNamed || !(await exists(join(path, '.git')))) {
320347
findings.push({ entry, name: entry, path, state: 'corrupt', ...(marketplace === undefined ? {} : { marketplace }) });
321348
diagnostics.push(finding(
322349
'AB7323',
323350
`Staged Cursor marketplace ${JSON.stringify(path)} is incomplete (marketplace.json missing or not listing ` +
324-
`${entry} at plugins/${entry}, plugin manifest missing, or no .git).`,
351+
`${entry} at plugins/${entry}, plugins/${entry}/.cursor-plugin/plugin.json missing or not named ${entry}, or no .git).`,
325352
'Remove the staged directory and rerun `agent-bundle install cursor --mode marketplace`.',
326353
'error',
327354
));
328355
continue;
329356
}
330357
const commit = await readHeadCommit(path);
331-
const registered = await cacheHasPlugin(home, marketplace, entry, version);
358+
const registered = await cacheHasPlugin(home, marketplace, entry, version, commit);
332359
findings.push({
333360
...(commit === undefined ? {} : { commit }),
334361
entry,

‎packages/agent-bundle/tests/doctor.test.ts‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { cp, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises';
1+
import { cp, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises';
22
import { spawn, type ChildProcess } from 'node:child_process';
33
import { createServer, type Server, type Socket } from 'node:net';
44
import { tmpdir } from 'node:os';
@@ -1496,6 +1496,15 @@ it('proves plugin-scoped Cursor hook registration and flags stale, missing, and
14961496
expect(hostReport(await doctor(), 'cursor').inventory.findings[0]?.hooks).toMatchObject({ state: 'stale' });
14971497
await writeHookedCursorPlugin(pluginRoot);
14981498

1499+
// A sibling plugin whose path shares this plugin's path as a prefix must not count as duplicate delivery.
1500+
await writeJson(join(fixture.home, '.cursor', 'hooks.json'), {
1501+
hooks: { preToolUse: [{ command: `node ${pluginRoot}-tools/hooks/before-tool.mjs` }] },
1502+
version: 1,
1503+
});
1504+
const sibling = await doctor();
1505+
expect(hostReport(sibling, 'cursor').inventory.findings[0]?.hooks).toMatchObject({ duplicates: [], state: 'registered' });
1506+
expect(hookDiagnostics(sibling).map((entry) => entry.code)).toEqual(['AB7321']);
1507+
14991508
await writeJson(join(fixture.home, '.cursor', 'hooks.json'), {
15001509
hooks: { preToolUse: [{ command: `node ${join(pluginRoot, 'hooks/before-tool.mjs')}` }] },
15011510
version: 1,
@@ -1596,11 +1605,20 @@ it('tracks staged Cursor marketplaces from staged to imported', async () => {
15961605
expect(hostReport(foreign, 'cursor').inventory.findings).toEqual([{ ...stagedFinding, state: 'unregistered' }]);
15971606

15981607
// A cache directory without Cursor's `.cache-complete` receipt is a half-written copy, not an import.
1599-
const cached = join(fixture.home, '.cursor', 'plugins', 'cache', 'doctor-fixture-marketplace', 'doctor-fixture', commit);
1608+
const cacheRoot = join(fixture.home, '.cursor', 'plugins', 'cache', 'doctor-fixture-marketplace', 'doctor-fixture');
1609+
const cached = join(cacheRoot, commit);
16001610
await writeJson(join(cached, '.cursor-plugin', 'plugin.json'), { name: 'doctor-fixture', version: '1.2.3' });
16011611
const incomplete = await doctor();
16021612
expect(hostReport(incomplete, 'cursor').inventory.findings).toEqual([{ ...stagedFinding, state: 'unregistered' }]);
16031613

1614+
// A receipted copy from an earlier staging commit, or a malformed cache entry, does not prove this commit was imported.
1615+
const previous = join(cacheRoot, 'b'.repeat(40));
1616+
await writeJson(join(previous, '.cursor-plugin', 'plugin.json'), { name: 'doctor-fixture', version: '1.2.3' });
1617+
await writeFile(join(previous, '.cache-complete'), '');
1618+
await writeFile(join(cacheRoot, 'not-a-directory'), 'stray');
1619+
const staleReceipt = await doctor();
1620+
expect(hostReport(staleReceipt, 'cursor').inventory.findings).toEqual([{ ...stagedFinding, state: 'unregistered' }]);
1621+
16041622
await writeFile(join(cached, '.cache-complete'), '');
16051623
const imported = await doctor();
16061624
expect(hostReport(imported, 'cursor').inventory.findings).toEqual([{ ...stagedFinding, state: 'registered' }]);
@@ -1627,6 +1645,19 @@ it('tracks staged Cursor marketplaces from staged to imported', async () => {
16271645
const corruptBundle = await doctor();
16281646
expect(hostReport(corruptBundle, 'cursor').bundle?.state).toBe('corrupt');
16291647

1648+
// A plugin manifest naming a different plugin than the entry is corrupt even with a valid marketplace entry.
1649+
await writeJson(join(repo, '.cursor-plugin/marketplace.json'), {
1650+
name: 'doctor-fixture-marketplace',
1651+
owner: { name: 'doctor-fixture' },
1652+
plugins: [{ name: 'doctor-fixture', source: 'plugins/doctor-fixture' }],
1653+
});
1654+
const stagedManifest = await readFile(join(repo, 'plugins', 'doctor-fixture', '.cursor-plugin', 'plugin.json'), 'utf8');
1655+
await writeJson(join(repo, 'plugins', 'doctor-fixture', '.cursor-plugin', 'plugin.json'), { name: 'someone-else', version: '1.2.3' });
1656+
const misnamed = await doctor();
1657+
expect(hostReport(misnamed, 'cursor').inventory.findings).toEqual([expect.objectContaining({ entry: 'doctor-fixture', state: 'corrupt' })]);
1658+
expect(misnamed.diagnostics.filter((entry) => entry.code === 'AB7323')[0]?.message).toContain('not named doctor-fixture');
1659+
await writeFile(join(repo, 'plugins', 'doctor-fixture', '.cursor-plugin', 'plugin.json'), stagedManifest);
1660+
16301661
await rm(join(repo, '.git'), { recursive: true });
16311662
const corrupt = await doctor();
16321663
expect(hostReport(corrupt, 'cursor').inventory.findings).toEqual([expect.objectContaining({ entry: 'doctor-fixture', state: 'corrupt' })]);

0 commit comments

Comments
 (0)