From 79f9b1b4f6b2464c4c63c35c4ca8d9e8d05e94d0 Mon Sep 17 00:00:00 2001 From: LHMQ878 <72402929@cityu-dg.edu.cn> Date: Tue, 28 Jul 2026 14:48:03 +0800 Subject: [PATCH 1/3] fix(plugin): validate installed.json records instead of casting them `readInstalled` checked only that the file parsed and that `plugins` was an array, then cast the result to `InstalledFile`. Every non-optional field in `InstalledRecord` was therefore unverified against the on-disk data, even though `installed.json` is hand-editable and is written by older versions. Consumers dereference those fields unconditionally, so a bad record surfaced as a crash far from its cause: `PluginManager.load` reads `entry.id` on every entry (a `null` entry threw "Cannot read properties of null (reading 'id')"), and the plugins status panel renders `github.ref.value`, which threw for a github record persisted without a `ref`. Validate each record with zod and fail the read with the offending index, so the error names the file and the entry. This keeps the existing "fail loudly" contract: both PluginManagers already capture a load failure and rethrow it from `/plugins` and every mutator with a message pointing at `installed.json`, rather than persisting a partially-loaded state. Unknown keys are preserved so a record written by a newer version still round-trips. Applied to both the agent-core and agent-core-v2 copies of the store. --- .../agent-core-v2/src/app/plugin/store.ts | 53 ++++++++++++++- .../test/app/plugin/manager.test.ts | 12 ++++ packages/agent-core/src/plugin/store.ts | 57 +++++++++++++++- packages/agent-core/test/plugin/store.test.ts | 65 +++++++++++++++++++ 4 files changed, 185 insertions(+), 2 deletions(-) diff --git a/packages/agent-core-v2/src/app/plugin/store.ts b/packages/agent-core-v2/src/app/plugin/store.ts index 6439ae235c..bafadc5268 100644 --- a/packages/agent-core-v2/src/app/plugin/store.ts +++ b/packages/agent-core-v2/src/app/plugin/store.ts @@ -1,10 +1,51 @@ 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.object({ + owner: z.string(), + repo: z.string(), + ref: z.object({ + kind: z.enum(['branch', 'tag', 'sha']), + value: z.string(), + }), + installedSha: z.string().optional(), +}); + +const PluginCapabilityStateSchema: z.ZodType = z.object({ + mcpServers: z.record(z.string(), z.object({ 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. + + Unknown keys are kept (`loose`) so a record written by a newer version round-trips + through an older one instead of losing fields on the next write. +*/ +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 +79,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..6f081710c9 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,18 @@ describe('PluginManager', () => { ]); }); + it('fails the load with the offending index when installed.json holds a bad record', async () => { + // `load()` dereferences `entry.id` and `entry.root` for every record, so a + // malformed one has to be rejected at the store instead of crashing here. + 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..f29b5773ff 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,49 @@ 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.object({ + owner: z.string(), + repo: z.string(), + ref: z.object({ + kind: z.enum(['branch', 'tag', 'sha']), + value: z.string(), + }), + installedSha: z.string().optional(), +}); + +const PluginCapabilityStateSchema: z.ZodType = z.object({ + mcpServers: z.record(z.string(), z.object({ 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. + + Unknown keys are kept (`loose`) so a record written by a newer version round-trips + through an older one instead of losing fields on the next write. +*/ +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 +87,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..1ea01ffd5e 100644 --- a/packages/agent-core/test/plugin/store.test.ts +++ b/packages/agent-core/test/plugin/store.test.ts @@ -117,4 +117,69 @@ 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, + }); + }); }); From d191f66270a4547ee28e2b78796337f3d9bfb3e6 Mon Sep 17 00:00:00 2001 From: LHMQ878 <72402929@cityu-dg.edu.cn> Date: Tue, 28 Jul 2026 14:52:10 +0800 Subject: [PATCH 2/3] chore: add changeset --- .changeset/validate-installed-plugin-records.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/validate-installed-plugin-records.md 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. From f6127aeed816775e3b33eb62856647791873e1d1 Mon Sep 17 00:00:00 2001 From: LHMQ878 Date: Tue, 28 Jul 2026 18:41:40 +0800 Subject: [PATCH 3/3] fix(plugin): keep unknown keys at every level of the installed-record schema Only the top-level record schema was loose. `github`, `github.ref` and the per-server `capabilities.mcpServers` entries used `z.object`, which strips unknown keys, so `readInstalled` silently dropped fields a newer client had written -- and since the store is read-modify-write, the next `writeInstalled` persisted the loss. Also addresses the agent-core-v2 comment convention: the schema rationale moves into the module header and the test rationale into the test name. --- .../agent-core-v2/src/app/plugin/store.ts | 28 +++++++++--------- .../test/app/plugin/manager.test.ts | 4 +-- packages/agent-core/src/plugin/store.ts | 14 +++++---- packages/agent-core/test/plugin/store.test.ts | 29 +++++++++++++++++++ 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/packages/agent-core-v2/src/app/plugin/store.ts b/packages/agent-core-v2/src/app/plugin/store.ts index bafadc5268..37d6af323b 100644 --- a/packages/agent-core-v2/src/app/plugin/store.ts +++ b/packages/agent-core-v2/src/app/plugin/store.ts @@ -1,3 +1,12 @@ +/** + * `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'; @@ -9,31 +18,20 @@ const INSTALLED_REL = path.join('plugins', 'installed.json'); const PluginSourceSchema: z.ZodType = z.enum(['local-path', 'zip-url', 'github']); -const PluginGithubMetadataSchema: z.ZodType = z.object({ +const PluginGithubMetadataSchema: z.ZodType = z.looseObject({ owner: z.string(), repo: z.string(), - ref: z.object({ + ref: z.looseObject({ kind: z.enum(['branch', 'tag', 'sha']), value: z.string(), }), installedSha: z.string().optional(), }); -const PluginCapabilityStateSchema: z.ZodType = z.object({ - mcpServers: z.record(z.string(), z.object({ enabled: z.boolean() })).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. - - Unknown keys are kept (`loose`) so a record written by a newer version round-trips - through an older one instead of losing fields on the next write. -*/ const InstalledRecordSchema = z.looseObject({ id: z.string(), root: z.string(), 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 6f081710c9..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,9 +84,7 @@ describe('PluginManager', () => { ]); }); - it('fails the load with the offending index when installed.json holds a bad record', async () => { - // `load()` dereferences `entry.id` and `entry.root` for every record, so a - // malformed one has to be rejected at the store instead of crashing here. + 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] }), diff --git a/packages/agent-core/src/plugin/store.ts b/packages/agent-core/src/plugin/store.ts index f29b5773ff..b962416296 100644 --- a/packages/agent-core/src/plugin/store.ts +++ b/packages/agent-core/src/plugin/store.ts @@ -17,18 +17,18 @@ const PluginSourceSchema: z.ZodType = z.enum([ 'github', ]); -const PluginGithubMetadataSchema: z.ZodType = z.object({ +const PluginGithubMetadataSchema: z.ZodType = z.looseObject({ owner: z.string(), repo: z.string(), - ref: z.object({ + ref: z.looseObject({ kind: z.enum(['branch', 'tag', 'sha']), value: z.string(), }), installedSha: z.string().optional(), }); -const PluginCapabilityStateSchema: z.ZodType = z.object({ - mcpServers: z.record(z.string(), z.object({ enabled: z.boolean() })).optional(), +const PluginCapabilityStateSchema: z.ZodType = z.looseObject({ + mcpServers: z.record(z.string(), z.looseObject({ enabled: z.boolean() })).optional(), }); /* @@ -39,8 +39,10 @@ const PluginCapabilityStateSchema: z.ZodType = z.object({ 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. - Unknown keys are kept (`loose`) so a record written by a newer version round-trips - through an older one instead of losing fields on the next write. + 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(), diff --git a/packages/agent-core/test/plugin/store.test.ts b/packages/agent-core/test/plugin/store.test.ts index 1ea01ffd5e..9476d5785e 100644 --- a/packages/agent-core/test/plugin/store.test.ts +++ b/packages/agent-core/test/plugin/store.test.ts @@ -182,4 +182,33 @@ describe('plugin store', () => { 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); + }); });