fix(marketplace): close client review findings

This commit is contained in:
2026-08-29 10:31:38 +08:00
parent 57962591de
commit 3df794c2e7
14 changed files with 840 additions and 68 deletions

View File

@@ -86,7 +86,7 @@ function makeResolveResult(input: ResolveRequest, itemOverrides: Record<string,
sha256: SHA256,
sizeBytes: 1,
releaseAdmissionId: ADMISSION_ID,
expiresAt: '2026-08-29T00:00:00Z',
expiresAt: '2100-01-01T00:00:00Z',
channel: input.channel,
reason: null,
...itemOverrides,
@@ -97,17 +97,20 @@ function makeResolveResult(input: ResolveRequest, itemOverrides: Record<string,
};
}
function buildSkillOnlyArchive(): Buffer {
function buildSkillOnlyArchive(
overrides: Readonly<Record<string, Buffer | string>> = {},
): Buffer {
const zip = new AdmZip();
zip.addFile('plugin.json', Buffer.from(JSON.stringify({
const files = new Map<string, Buffer | string>([
['plugin.json', JSON.stringify({
$schema: 'https://agent-plugins.org/schemas/1.0.0/plugin.schema.json',
name: PLUGIN_ID,
version: '1.0.0',
description: 'Example Skill',
author: { name: 'MakeLore' },
extensions: { 'com.makelore': { capabilityManifest: './com.makelore/capability.json' } },
})));
zip.addFile('com.makelore/capability.json', Buffer.from(JSON.stringify({
})],
['com.makelore/capability.json', JSON.stringify({
schemaVersion: 2,
pluginId: PLUGIN_ID,
contractVersion: 1,
@@ -115,8 +118,13 @@ function buildSkillOnlyArchive(): Buffer {
runtime: { kind: 'skill_only' },
skills: [{ id: 'example-skill', entry: '../skills/example-skill/SKILL.md', grants: [] }],
tools: [],
})));
zip.addFile('skills/example-skill/SKILL.md', Buffer.from('# Example\n'));
})],
['skills/example-skill/SKILL.md', '# Example\n'],
]);
for (const [filePath, content] of Object.entries(overrides)) files.set(filePath, content);
for (const [filePath, content] of files) {
zip.addFile(filePath, Buffer.isBuffer(content) ? content : Buffer.from(content));
}
return zip.toBuffer();
}
@@ -156,7 +164,7 @@ function signedGrant(
sha256,
signingKeyId: options.signingKeyId ?? 'test-key',
descriptorSignature: signature,
expiresAt: '2026-08-29T00:00:00Z',
expiresAt: '2100-01-01T00:00:00Z',
contentUrl: `/api/plugin-marketplace/v1/releases/${releaseId}/content?release_admission_id=${ADMISSION_ID}`,
},
publicKey: publicKey.export({ type: 'spki', format: 'der' }) as Buffer,
@@ -749,12 +757,87 @@ describe('PluginPackageStore', () => {
current = second;
await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
await expect(store.uninstall(PLUGIN_ID)).resolves.toMatchObject({ status: 'kept', releaseId: 'release-current' });
await expect(store.removeUnused(PLUGIN_ID)).resolves.toMatchObject({ status: 'kept', releaseId: 'release-current' });
await expect(store.readInstalledIndex()).resolves.toHaveLength(1);
await expect(store.getInstalled(PLUGIN_ID)).resolves.toMatchObject({ releaseId: 'release-current' });
await expect(readFile(path.join(temporaryRoot, 'current.json'), 'utf8')).resolves.toContain('release-current');
});
it('explicitly uninstalls every unprotected release without selecting by install time', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const archive = buildSkillOnlyArchive();
const first = signedGrant(archive, { releaseId: 'release-old' });
const second = signedGrant(archive, { releaseId: 'release-current', signingKeyId: 'test-key-2' });
let current = first;
const marketplace: MarketplaceClient = {
resolve: vi.fn(async (input: ResolveRequest) => makeResolveResult(input, {
releaseId: current.grant.releaseId,
sha256: current.grant.sha256,
sizeBytes: current.grant.sizeBytes,
})),
issueDownload: vi.fn(async () => current.grant),
downloadContent: async () => archive,
getCurrentAccountBinding: () => ACCOUNT_A,
} as MarketplaceClient;
const store = new PluginPackageStore({
rootDir: temporaryRoot,
marketplace,
getAccountBinding: () => ACCOUNT_A,
keyStore: new Map([['test-key', first.publicKey], ['test-key-2', second.publicKey]]),
clientVersion: '1.0.0',
});
await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
current = second;
await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
await expect(store.uninstall(PLUGIN_ID)).resolves.toMatchObject({
status: 'removed', pluginId: PLUGIN_ID, releaseId: 'release-current',
});
await expect(store.readInstalledIndex()).resolves.toEqual([]);
await expect(store.getInstalled(PLUGIN_ID)).resolves.toBeNull();
await expect(readFile(path.join(temporaryRoot, 'current.json'), 'utf8')).resolves.toContain('"current":{}');
});
it('removes an unprotected current Release while retaining only an active-worker Release', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const archive = buildSkillOnlyArchive();
const protectedRelease = signedGrant(archive, { releaseId: 'release-worker' });
const selectedRelease = signedGrant(archive, {
releaseId: 'release-current', signingKeyId: 'test-key-current',
});
let current = protectedRelease;
const marketplace: MarketplaceClient = {
resolve: vi.fn(async (input: ResolveRequest) => makeResolveResult(input, {
releaseId: current.grant.releaseId,
sha256: current.grant.sha256,
sizeBytes: current.grant.sizeBytes,
})),
issueDownload: vi.fn(async () => current.grant),
downloadContent: async () => archive,
getCurrentAccountBinding: () => ACCOUNT_A,
} as MarketplaceClient;
const store = new PluginPackageStore({
rootDir: temporaryRoot, marketplace, getAccountBinding: () => ACCOUNT_A,
keyStore: new Map([
['test-key', protectedRelease.publicKey],
['test-key-current', selectedRelease.publicKey],
]),
clientVersion: '1.0.0',
});
await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
current = selectedRelease;
await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
store.registerActiveWorker('release-worker');
await expect(store.uninstall(PLUGIN_ID)).resolves.toEqual({
status: 'kept', pluginId: PLUGIN_ID, reason: 'active_worker_reference',
});
await expect(store.readInstalledIndex()).resolves.toEqual([
expect.objectContaining({ releaseId: 'release-worker' }),
]);
await expect(store.getInstalled(PLUGIN_ID)).resolves.toBeNull();
});
it('preserves the old release across download, signature, and extraction failures', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const oldArchive = buildSkillOnlyArchive();
@@ -886,6 +969,82 @@ describe('PluginPackageStore', () => {
await expect(store.readInstalledIndex()).resolves.toEqual([]);
});
it('captures the account before queued package mutations execute', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const archive = buildSkillOnlyArchive();
const { grant, publicKey } = signedGrant(archive);
let binding: AccountBinding | null = ACCOUNT_A;
const firstResolve = deferred<ResolveSnapshot>();
const resolve = vi.fn(async (input: ResolveRequest) => (
resolve.mock.calls.length === 1
? firstResolve.promise
: makeResolveResult(input, { sha256: grant.sha256, sizeBytes: grant.sizeBytes })
));
const marketplace: MarketplaceClient = {
resolve,
issueDownload: vi.fn(async () => grant),
downloadContent: async () => archive,
getCurrentAccountBinding: () => binding,
} as MarketplaceClient;
const store = new PluginPackageStore({
rootDir: temporaryRoot,
marketplace,
clientVersion: '1.0.0',
keyStore: new Map([['test-key', publicKey]]),
getAccountBinding: () => binding,
});
const inFlight = store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
await vi.waitFor(() => expect(resolve).toHaveBeenCalledTimes(1));
const queued = store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
binding = ACCOUNT_B;
firstResolve.resolve(makeResolveResult({ makeloreVersion: '1.0.0', channel: 'stable' }));
await expect(inFlight).rejects.toMatchObject({ code: 'plugin_account_changed' });
await expect(queued).rejects.toMatchObject({ code: 'plugin_account_changed' });
expect(resolve).toHaveBeenCalledTimes(1);
await expect(store.readInstalledIndex()).resolves.toEqual([]);
});
it('does not let a queued uninstall switch accounts while waiting', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const archive = buildSkillOnlyArchive();
const { grant, publicKey } = signedGrant(archive);
let binding: AccountBinding | null = ACCOUNT_A;
const resolve = vi.fn(async (input: ResolveRequest) => makeResolveResult(input, {
sha256: grant.sha256,
sizeBytes: grant.sizeBytes,
}));
const marketplace: MarketplaceClient = {
resolve,
issueDownload: vi.fn(async () => grant),
downloadContent: async () => archive,
getCurrentAccountBinding: () => binding,
} as MarketplaceClient;
const store = new PluginPackageStore({
rootDir: temporaryRoot,
marketplace,
clientVersion: '1.0.0',
keyStore: new Map([['test-key', publicKey]]),
getAccountBinding: () => binding,
});
await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
const firstResolve = deferred<ResolveSnapshot>();
resolve.mockImplementationOnce(async () => firstResolve.promise);
const inFlight = store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' });
await vi.waitFor(() => expect(resolve).toHaveBeenCalledTimes(2));
const queued = store.uninstall(PLUGIN_ID);
binding = ACCOUNT_B;
firstResolve.resolve(makeResolveResult({ makeloreVersion: '1.0.0', channel: 'stable' }, {
action: 'keep',
}));
await expect(inFlight).rejects.toMatchObject({ code: 'plugin_account_changed' });
await expect(queued).rejects.toMatchObject({ code: 'plugin_account_changed' });
await expect(store.getInstalled(PLUGIN_ID)).resolves.toMatchObject({ releaseId: RELEASE_ID });
});
it('rejects archive traversal before materializing a package', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const zip = new AdmZip();
@@ -912,6 +1071,68 @@ describe('PluginPackageStore', () => {
await expect(store.readInstalledIndex()).resolves.toEqual([]);
});
it('enforces the canonical archive contract before installing a package', async () => {
temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-'));
const oversizedManifest = JSON.stringify({
$schema: 'https://agent-plugins.org/schemas/1.0.0/plugin.schema.json',
name: PLUGIN_ID,
version: '1.0.0',
description: 'x'.repeat(256 * 1024),
author: { name: 'MakeLore' },
extensions: { 'com.makelore': { capabilityManifest: './com.makelore/capability.json' } },
});
const cases = [
{
name: 'unsupported asset extension',
archive: buildSkillOnlyArchive({ 'skills/example-skill/assets/run.exe': Buffer.from('not executable') }),
},
{
name: 'non-UTF-8 Skill text',
archive: buildSkillOnlyArchive({ 'skills/example-skill/SKILL.md': Buffer.from([0xff, 0xfe, 0xfd]) }),
},
{
name: 'non-UTF-8 capability manifest',
archive: buildSkillOnlyArchive({ 'com.makelore/capability.json': Buffer.from([0xff, 0xfe, 0xfd]) }),
},
{
name: 'asset outside a declared Skill',
archive: buildSkillOnlyArchive({ 'skills/other-skill/notes.md': '# Hidden\n' }),
},
{
name: 'oversized manifest',
archive: buildSkillOnlyArchive({ 'plugin.json': oversizedManifest }),
},
{
name: 'oversized Skill',
archive: buildSkillOnlyArchive({ 'skills/example-skill/SKILL.md': Buffer.alloc(256 * 1024 + 1, 0x61) }),
},
];
for (const [index, scenario] of cases.entries()) {
const { grant, publicKey } = signedGrant(scenario.archive, { releaseId: `release-invalid-${index}` });
const marketplace: MarketplaceClient = {
resolve: vi.fn(async (input: ResolveRequest) => makeResolveResult(input, {
releaseId: grant.releaseId,
sha256: grant.sha256,
sizeBytes: grant.sizeBytes,
})),
issueDownload: vi.fn(async () => grant),
downloadContent: async () => scenario.archive,
getCurrentAccountBinding: () => ACCOUNT_A,
} as MarketplaceClient;
const store = new PluginPackageStore({
rootDir: temporaryRoot,
marketplace,
clientVersion: '1.0.0',
keyStore: new Map([['test-key', publicKey]]),
getAccountBinding: () => ACCOUNT_A,
});
await expect(store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' }), scenario.name)
.rejects.toMatchObject({ code: 'plugin_artifact_invalid' });
await expect(store.readInstalledIndex()).resolves.toEqual([]);
}
});
it('requires explicit beta selection and rejects content paths outside the server route', async () => {
const fetcher = vi.fn<typeof fetch>().mockResolvedValue(response({
release_admission_id: ADMISSION_ID,