From 1d4e5b874c6ee79f8eca87ae47769f70cb35b130 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Fri, 13 Feb 2026 11:37:10 -0800 Subject: [PATCH] fix: add defense-in-depth for SPAWN_HOME path validation and manifest JSON sanitization (#984) - Validate SPAWN_HOME is an absolute path, reject relative paths to prevent unintended file writes (addresses #980) - Resolve SPAWN_HOME to canonical form to collapse .. segments - Strip __proto__, constructor, and prototype keys from parsed manifest JSON to prevent prototype pollution (addresses #979) - Apply sanitization to all manifest ingestion paths (GitHub fetch, disk cache, local dev manifest) - Add 12 tests covering path validation and JSON sanitization Agent: security-auditor Co-authored-by: A <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- cli/src/__tests__/history.test.ts | 20 +++++++ cli/src/__tests__/security-encoding.test.ts | 64 +++++++++++++++++++++ cli/src/history.ts | 18 +++++- cli/src/manifest.ts | 27 +++++++-- 4 files changed, 121 insertions(+), 8 deletions(-) diff --git a/cli/src/__tests__/history.test.ts b/cli/src/__tests__/history.test.ts index af067d0c..708ff5cf 100644 --- a/cli/src/__tests__/history.test.ts +++ b/cli/src/__tests__/history.test.ts @@ -42,6 +42,26 @@ describe("history", () => { const { homedir } = require("os"); expect(getSpawnDir()).toBe(join(homedir(), ".spawn")); }); + + it("throws for relative SPAWN_HOME path", () => { + process.env.SPAWN_HOME = "relative/path"; + expect(() => getSpawnDir()).toThrow("must be an absolute path"); + }); + + it("throws for dot-relative SPAWN_HOME path", () => { + process.env.SPAWN_HOME = "./local/dir"; + expect(() => getSpawnDir()).toThrow("must be an absolute path"); + }); + + it("resolves .. segments in absolute SPAWN_HOME", () => { + process.env.SPAWN_HOME = "/tmp/foo/../bar"; + expect(getSpawnDir()).toBe("/tmp/bar"); + }); + + it("accepts normal absolute SPAWN_HOME", () => { + process.env.SPAWN_HOME = "/home/user/.spawn"; + expect(getSpawnDir()).toBe("/home/user/.spawn"); + }); }); // ── getHistoryPath ────────────────────────────────────────────────────── diff --git a/cli/src/__tests__/security-encoding.test.ts b/cli/src/__tests__/security-encoding.test.ts index 2da5a536..116d45a9 100644 --- a/cli/src/__tests__/security-encoding.test.ts +++ b/cli/src/__tests__/security-encoding.test.ts @@ -160,3 +160,67 @@ describe("Security Encoding Edge Cases", () => { }); }); }); + +// ── stripDangerousKeys (prototype pollution defense) ───────────────────────── + +import { stripDangerousKeys } from "../manifest"; + +describe("stripDangerousKeys", () => { + it("strips __proto__ from parsed JSON", () => { + // JSON.parse produces an own-property __proto__ key (not inherited) + const input = JSON.parse('{"agents":{},"clouds":{},"matrix":{},"__proto__":{"polluted":true}}'); + expect(Object.prototype.hasOwnProperty.call(input, "__proto__")).toBe(true); + const result = stripDangerousKeys(input); + expect(Object.prototype.hasOwnProperty.call(result, "__proto__")).toBe(false); + expect(result.agents).toEqual({}); + }); + + it("strips constructor key", () => { + const input = Object.assign(Object.create(null), { name: "test", constructor: { evil: true } }); + const result = stripDangerousKeys(input); + expect(Object.keys(result)).toEqual(["name"]); + expect(result.name).toBe("test"); + }); + + it("strips prototype key", () => { + const input = Object.assign(Object.create(null), { data: 1, prototype: { inject: true } }); + const result = stripDangerousKeys(input); + expect(Object.keys(result)).toEqual(["data"]); + expect(result.data).toBe(1); + }); + + it("strips dangerous keys from nested objects", () => { + const input = { agents: { claude: { __proto__: { evil: true }, name: "Claude" } } }; + const result = stripDangerousKeys(input); + expect(result.agents.claude.name).toBe("Claude"); + expect(Object.keys(result.agents.claude)).toEqual(["name"]); + }); + + it("handles arrays correctly", () => { + const input = { items: [{ name: "a" }, { name: "b", __proto__: {} }] }; + const result = stripDangerousKeys(input); + expect(result.items).toHaveLength(2); + expect(result.items[0].name).toBe("a"); + expect(result.items[1].name).toBe("b"); + }); + + it("passes through primitives unchanged", () => { + expect(stripDangerousKeys("hello")).toBe("hello"); + expect(stripDangerousKeys(42)).toBe(42); + expect(stripDangerousKeys(true)).toBe(true); + expect(stripDangerousKeys(null)).toBe(null); + }); + + it("preserves normal keys", () => { + const input = { agents: { a: 1 }, clouds: { b: 2 }, matrix: { c: 3 } }; + const result = stripDangerousKeys(input); + expect(result).toEqual(input); + }); + + it("handles deeply nested dangerous keys", () => { + const input = { a: { b: { c: { constructor: "bad", value: "good" } } } }; + const result = stripDangerousKeys(input); + expect(result.a.b.c.value).toBe("good"); + expect(Object.keys(result.a.b.c)).toEqual(["value"]); + }); +}); diff --git a/cli/src/history.ts b/cli/src/history.ts index 8d8c7e37..825af62e 100644 --- a/cli/src/history.ts +++ b/cli/src/history.ts @@ -1,5 +1,5 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync } from "fs"; -import { join } from "path"; +import { join, resolve, isAbsolute } from "path"; import { homedir } from "os"; export interface SpawnRecord { @@ -9,9 +9,21 @@ export interface SpawnRecord { prompt?: string; } -/** Returns the directory for spawn data, respecting SPAWN_HOME env var */ +/** Returns the directory for spawn data, respecting SPAWN_HOME env var. + * SPAWN_HOME must be an absolute path if set; relative paths are rejected + * to prevent unintended file writes. */ export function getSpawnDir(): string { - return process.env.SPAWN_HOME || join(homedir(), ".spawn"); + const spawnHome = process.env.SPAWN_HOME; + if (!spawnHome) return join(homedir(), ".spawn"); + // Require absolute path to prevent path traversal via relative paths + if (!isAbsolute(spawnHome)) { + throw new Error( + `SPAWN_HOME must be an absolute path (got "${spawnHome}").\n` + + `Example: export SPAWN_HOME=/home/user/.spawn` + ); + } + // Resolve to canonical form (collapses .. segments) + return resolve(spawnHome); } export function getHistoryPath(): string { diff --git a/cli/src/manifest.ts b/cli/src/manifest.ts index dbd56cfe..d5a75b74 100644 --- a/cli/src/manifest.ts +++ b/cli/src/manifest.ts @@ -66,7 +66,8 @@ function logError(message: string, err?: unknown): void { function readCache(): Manifest | null { try { - return JSON.parse(readFileSync(CACHE_FILE, "utf-8")) as Manifest; + const raw = JSON.parse(readFileSync(CACHE_FILE, "utf-8")); + return stripDangerousKeys(raw) as Manifest; } catch (err) { // Cache file missing, corrupted, or unreadable logError(`Failed to read cache from ${CACHE_FILE}`, err); @@ -81,8 +82,22 @@ function writeCache(data: Manifest): void { // ── Fetching ─────────────────────────────────────────────────────────────────── +/** Recursively strip __proto__, constructor, and prototype keys from parsed JSON + * to prevent prototype pollution attacks (defense in depth). */ +function stripDangerousKeys(obj: any): any { + if (obj === null || typeof obj !== "object") return obj; + if (Array.isArray(obj)) return obj.map(stripDangerousKeys); + const clean: Record = {}; + for (const key of Object.keys(obj)) { + if (key === "__proto__" || key === "constructor" || key === "prototype") continue; + clean[key] = stripDangerousKeys(obj[key]); + } + return clean; +} + function isValidManifest(data: any): data is Manifest { - return data && data.agents && data.clouds && data.matrix; + return data && typeof data === "object" && !Array.isArray(data) && + data.agents && data.clouds && data.matrix; } async function fetchManifestFromGitHub(): Promise { @@ -94,7 +109,8 @@ async function fetchManifestFromGitHub(): Promise { logError(`Failed to fetch manifest from GitHub: HTTP ${res.status} ${res.statusText}`); return null; } - const data = (await res.json()) as Manifest; + const raw = await res.json(); + const data = stripDangerousKeys(raw) as Manifest; if (!isValidManifest(data)) { logError("Manifest structure validation failed: missing required fields (agents, clouds, or matrix)"); return null; @@ -132,7 +148,8 @@ function tryLoadLocalManifest(): Manifest | null { // Try loading manifest.json from current directory (development mode) const localPath = join(process.cwd(), "manifest.json"); if (existsSync(localPath)) { - const data = JSON.parse(readFileSync(localPath, "utf-8")); + const raw = JSON.parse(readFileSync(localPath, "utf-8")); + const data = stripDangerousKeys(raw); if (isValidManifest(data)) { return data as Manifest; } @@ -226,4 +243,4 @@ export function _resetCacheForTesting(): void { _staleCache = false; } -export { RAW_BASE, REPO, CACHE_DIR }; +export { RAW_BASE, REPO, CACHE_DIR, stripDangerousKeys };