diff --git a/src/state/config.test.ts b/src/state/config.test.ts index de331ad..d78355c 100644 --- a/src/state/config.test.ts +++ b/src/state/config.test.ts @@ -34,6 +34,32 @@ describe("loadConfig", () => { expect(config.historySize).toBe(100); expect(config.temperature).toBe(0.7); // default preserved }); + + it("strips prototype-hijacking keys from a crafted config file (#301)", async () => { + // Write a RAW JSON string (not JSON.stringify of an object literal — `__proto__:` + // in a literal is the prototype setter and never serialises) so the file genuinely + // contains __proto__/constructor as JSON keys. + const { writeFile, mkdir } = await import("node:fs/promises"); + const cfgDir = join(tmpHome, ".opencli"); + await mkdir(cfgDir, { recursive: true }); + await writeFile( + join(cfgDir, "config.json"), + '{"__proto__":{"polluted":true},"constructor":{"prototype":{"polluted2":true}},"model":"x"}', + ); + + const config = await loadConfig(); + // Legitimate field survives. + expect(config.model).toBe("x"); + // The poison keys are NOT carried onto the returned object as own properties. + // (Without stripPoisonKeys, `{ ...saved }` copies them through as data properties — + // this assertion fails on unpatched main. Object.prototype itself is not polluted + // either way via this code path; the strip is hardening against a future deep merge.) + const keys = Object.keys(config); + expect(keys).not.toContain("__proto__"); + expect(keys).not.toContain("constructor"); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(({} as any).polluted).toBeUndefined(); + }); }); describe("saveConfig", () => { diff --git a/src/state/config.ts b/src/state/config.ts index cb47d53..22fd500 100644 --- a/src/state/config.ts +++ b/src/state/config.ts @@ -47,10 +47,27 @@ const DEFAULTS: Config = { historySize: 50, }; +// Object keys that can hijack the prototype chain if the merge ever becomes deep. +// Today JSON.parse + object spread does NOT pollute Object.prototype via this path +// (spread uses [[DefineOwnProperty]], not [[Set]]), so this is hardening against a +// future deep/recursive merge — where a live prototype-pollution vector would appear +// — not a fix for a current one. See #301. +const POISON_KEYS = new Set(["__proto__", "constructor", "prototype"]); + +/** Return a copy of `obj` with prototype-hijacking keys removed. */ +function stripPoisonKeys(obj: Record): Record { + const clean: Record = {}; + for (const [k, v] of Object.entries(obj)) { + if (POISON_KEYS.has(k)) continue; + clean[k] = v; + } + return clean; +} + export async function loadConfig(): Promise { try { const raw = await readFile(CONFIG_FILE, "utf8"); - const saved = JSON.parse(raw) as Record; + const saved = stripPoisonKeys(JSON.parse(raw) as Record); // Migrate legacy apiKey → geminiApiKey if (typeof saved["apiKey"] === "string" && !saved["geminiApiKey"]) { saved["geminiApiKey"] = saved["apiKey"];