diff --git a/.changeset/validate-installed-plugin-records.md b/.changeset/validate-installed-plugin-records.md new file mode 100644 index 0000000000..e8737e9a11 --- /dev/null +++ b/.changeset/validate-installed-plugin-records.md @@ -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. diff --git a/packages/agent-core-v2/src/app/plugin/store.ts b/packages/agent-core-v2/src/app/plugin/store.ts index 6439ae235c..37d6af323b 100644 --- a/packages/agent-core-v2/src/app/plugin/store.ts +++ b/packages/agent-core-v2/src/app/plugin/store.ts @@ -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 = z.enum(['local-path', 'zip-url', 'github']); + +const PluginGithubMetadataSchema: z.ZodType = 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 = 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; @@ -38,7 +77,17 @@ export async function readInstalled(kimiHomeDir: string): Promise 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 }); } diff --git a/packages/agent-core-v2/test/app/plugin/manager.test.ts b/packages/agent-core-v2/test/app/plugin/manager.test.ts index 8bd5150cc9..157aa60679 100644 --- a/packages/agent-core-v2/test/app/plugin/manager.test.ts +++ b/packages/agent-core-v2/test/app/plugin/manager.test.ts @@ -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 { diff --git a/packages/agent-core/src/plugin/store.ts b/packages/agent-core/src/plugin/store.ts index 1a2f74672d..b962416296 100644 --- a/packages/agent-core/src/plugin/store.ts +++ b/packages/agent-core/src/plugin/store.ts @@ -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, @@ -9,6 +11,51 @@ import type { const INSTALLED_REL = path.join('plugins', 'installed.json'); +const PluginSourceSchema: z.ZodType = z.enum([ + 'local-path', + 'zip-url', + 'github', +]); + +const PluginGithubMetadataSchema: z.ZodType = 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 = 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; @@ -42,7 +89,17 @@ export async function readInstalled(kimiHomeDir: string): Promise 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}`, diff --git a/packages/agent-core/test/plugin/store.test.ts b/packages/agent-core/test/plugin/store.test.ts index a4900e52e9..9476d5785e 100644 --- a/packages/agent-core/test/plugin/store.test.ts +++ b/packages/agent-core/test/plugin/store.test.ts @@ -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 { + 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); + }); });