Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/validate-installed-plugin-records.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@moonshot-ai/agent-core": patch
"@moonshot-ai/agent-core-v2": patch
---

Report a malformed record in `plugins/installed.json` as a load error naming the offending entry, instead of handing the record to the plugin manager and crashing later on a missing field.
51 changes: 50 additions & 1 deletion packages/agent-core-v2/src/app/plugin/store.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,49 @@
/**
* `plugin` domain (L3) — owns the on-disk `installed.json` record store.
*
* Reads, validates, and atomically rewrites the installed-plugin file for
* `PluginManager`. Records are validated rather than cast because the file is
* hand-editable and survives downgrades; every schema is loose at every level
* so a record written by a newer client round-trips through an older one.
*/

import { mkdir, readFile, rename, writeFile } from 'node:fs/promises';
import path from 'node:path';

import { z } from 'zod';

import type { PluginCapabilityState, PluginGithubMetadata, PluginSource } from './types';

const INSTALLED_REL = path.join('plugins', 'installed.json');

const PluginSourceSchema: z.ZodType<PluginSource> = z.enum(['local-path', 'zip-url', 'github']);

const PluginGithubMetadataSchema: z.ZodType<PluginGithubMetadata> = z.looseObject({
owner: z.string(),
repo: z.string(),
ref: z.looseObject({
kind: z.enum(['branch', 'tag', 'sha']),
value: z.string(),
}),
installedSha: z.string().optional(),
});

const PluginCapabilityStateSchema: z.ZodType<PluginCapabilityState> = z.looseObject({
mcpServers: z.record(z.string(), z.looseObject({ enabled: z.boolean() })).optional(),
});

const InstalledRecordSchema = z.looseObject({
id: z.string(),
root: z.string(),
source: PluginSourceSchema,
enabled: z.boolean(),
installedAt: z.string(),
updatedAt: z.string().optional(),
originalSource: z.string().optional(),
capabilities: PluginCapabilityStateSchema.optional(),
github: PluginGithubMetadataSchema.optional(),
});

export interface InstalledRecord {
readonly id: string;
readonly root: string;
Expand Down Expand Up @@ -38,7 +77,17 @@ export async function readInstalled(kimiHomeDir: string): Promise<InstalledFile>
if (typeof parsed !== 'object' || parsed === null || !Array.isArray(parsed.plugins)) {
throw new Error('installed.json is not a valid InstalledFile object');
}
return parsed;
const plugins = parsed.plugins.map((entry, index) => {
const record = InstalledRecordSchema.safeParse(entry);
if (!record.success) {
throw new Error(
`plugins[${index}] is not a valid installed record: ${record.error.message}`,
{ cause: record.error },
);
}
return record.data as InstalledRecord;
});
return { ...parsed, plugins };
} catch (error) {
throw new Error(`Failed to parse ${filePath}: ${(error as Error).message}`, { cause: error });
}
Expand Down
10 changes: 10 additions & 0 deletions packages/agent-core-v2/test/app/plugin/manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ describe('PluginManager', () => {
]);
});

it('rejects a bad installed.json record at the store, naming its index, rather than dereferencing it in load()', async () => {
await writeFile(
join(home, 'plugins', 'installed.json'),
JSON.stringify({ version: 1, plugins: [null] }),
'utf8',
);
const manager = new PluginManager({ kimiHomeDir: home });
await expect(manager.load()).rejects.toThrow(/plugins\[0\]/);
});

it('installs a local-path plugin into the managed root', async () => {
const sourceRoot = await mkdtemp(join(tmpdir(), 'plugin-install-source-'));
try {
Expand Down
59 changes: 58 additions & 1 deletion packages/agent-core/src/plugin/store.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { mkdir, readFile, rename, writeFile } from 'node:fs/promises';
import path from 'node:path';

import { z } from 'zod';

import type {
PluginCapabilityState,
PluginGithubMetadata,
Expand All @@ -9,6 +11,51 @@ import type {

const INSTALLED_REL = path.join('plugins', 'installed.json');

const PluginSourceSchema: z.ZodType<PluginSource> = z.enum([
'local-path',
'zip-url',
'github',
]);

const PluginGithubMetadataSchema: z.ZodType<PluginGithubMetadata> = z.looseObject({
owner: z.string(),
repo: z.string(),
ref: z.looseObject({
kind: z.enum(['branch', 'tag', 'sha']),
value: z.string(),
}),
installedSha: z.string().optional(),
});

const PluginCapabilityStateSchema: z.ZodType<PluginCapabilityState> = z.looseObject({
mcpServers: z.record(z.string(), z.looseObject({ enabled: z.boolean() })).optional(),
});

/*
Records are validated field by field rather than trusted from disk: `installed.json`
is hand-editable, survives downgrades, and is written by older versions, so a
structurally valid file can still carry records that violate the type. Consumers
dereference these fields unconditionally — `PluginManager.load` reads `entry.id`
and `entry.root`, and the TUI renders `github.ref.value` — so an unchecked cast
turns a bad record into a crash far from the file that caused it.

Every level is loose, not just the top one: `readInstalled` returns what
`writeInstalled` later persists, so a nested `z.object` would strip fields a
newer client wrote — `github.ref` and the per-server capability entries most
of all — and the next write would drop them for good.
*/
const InstalledRecordSchema = z.looseObject({
id: z.string(),
root: z.string(),
source: PluginSourceSchema,
enabled: z.boolean(),
installedAt: z.string(),
updatedAt: z.string().optional(),
originalSource: z.string().optional(),
capabilities: PluginCapabilityStateSchema.optional(),
github: PluginGithubMetadataSchema.optional(),
});

export interface InstalledRecord {
readonly id: string;
readonly root: string;
Expand Down Expand Up @@ -42,7 +89,17 @@ export async function readInstalled(kimiHomeDir: string): Promise<InstalledFile>
if (typeof parsed !== 'object' || parsed === null || !Array.isArray(parsed.plugins)) {
throw new Error('installed.json is not a valid InstalledFile object');
}
return parsed;
const plugins = parsed.plugins.map((entry, index) => {
const record = InstalledRecordSchema.safeParse(entry);
if (!record.success) {
throw new Error(
`plugins[${index}] is not a valid installed record: ${record.error.message}`,
{ cause: record.error },
);
}
return record.data as InstalledRecord;
});
return { ...parsed, plugins };
} catch (error) {
throw new Error(
`Failed to parse ${filePath}: ${(error as Error).message}`,
Expand Down
94 changes: 94 additions & 0 deletions packages/agent-core/test/plugin/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,98 @@ describe('plugin store', () => {
expect(record?.source).toBe('zip-url');
expect((record as { github?: unknown } | undefined)?.github).toBeUndefined();
});

async function writeRaw(home: string, file: unknown): Promise<void> {
await writeInstalled(home, { version: 1, plugins: [] });
await writeFile(
path.join(home, 'plugins', 'installed.json'),
JSON.stringify(file),
'utf8',
);
}

const VALID_RECORD = {
id: 'demo',
root: '/tmp/demo',
source: 'local-path',
enabled: true,
installedAt: '2026-05-25T09:00:00Z',
};

it.each([
['a null entry', null],
['a non-object entry', 'demo'],
['a record with no id', { ...VALID_RECORD, id: undefined }],
['a record with a non-string root', { ...VALID_RECORD, root: 42 }],
['a record with an unknown source', { ...VALID_RECORD, source: 'ftp' }],
['a record with a non-boolean enabled', { ...VALID_RECORD, enabled: 'yes' }],
['a record with a non-string installedAt', { ...VALID_RECORD, installedAt: 5 }],
[
'a github record with no ref',
{ ...VALID_RECORD, source: 'github', github: { owner: 'a', repo: 'b' } },
],
[
'a github record with an unknown ref kind',
{
...VALID_RECORD,
source: 'github',
github: { owner: 'a', repo: 'b', ref: { kind: 'commit', value: 'abc' } },
},
],
])('rejects %s instead of returning it as an InstalledRecord', async (_label, entry) => {
const home = await makeKimiHome();
await writeRaw(home, { version: 1, plugins: [entry] });
await expect(readInstalled(home)).rejects.toThrow(/plugins\[0\]/);
});

it('names the offending index so the user can find the bad record', async () => {
const home = await makeKimiHome();
await writeRaw(home, {
version: 1,
plugins: [VALID_RECORD, { ...VALID_RECORD, id: 'other', enabled: 'yes' }],
});
await expect(readInstalled(home)).rejects.toThrow(/plugins\[1\]/);
});

it('keeps unknown keys so a record from a newer version round-trips', async () => {
const home = await makeKimiHome();
await writeRaw(home, {
version: 1,
plugins: [{ ...VALID_RECORD, futureField: { nested: true } }],
});
const result = await readInstalled(home);
expect(result.plugins).toHaveLength(1);
expect((result.plugins[0] as { futureField?: unknown }).futureField).toEqual({
nested: true,
});
});

// A top-level unknown key is not enough: `readInstalled` returns what the next
// `writeInstalled` persists, so any level that strips loses the field for good.
it('keeps unknown keys nested inside github and capabilities, and survives a rewrite', async () => {
const home = await makeKimiHome();
const fromNewerVersion = {
...VALID_RECORD,
futureTopLevel: 'kept',
github: {
owner: 'wbxl2000',
repo: 'superpowers',
ref: { kind: 'tag', value: 'v1.2.0', resolvedAt: '2026-07-28T00:00:00Z' },
installedSha: '45b441d62b81b5f27d3bfd8700e04436cd4de5b3',
signature: 'ed25519:abc',
},
capabilities: {
mcpServers: { finance: { enabled: true, transport: 'stdio' } },
},
};
await writeRaw(home, { version: 1, plugins: [fromNewerVersion] });

const read = await readInstalled(home);
expect(read.plugins[0]).toEqual(fromNewerVersion);

// The load-edit-save path an older client actually takes.
await writeInstalled(home, read);
const reread = await readInstalled(home);
expect(reread.plugins[0]).toEqual(fromNewerVersion);
});
});