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) <noreply@anthropic.com>
This commit is contained in:
A 2026-02-13 11:37:10 -08:00 committed by GitHub
parent 16b9132c7c
commit 1d4e5b874c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 121 additions and 8 deletions

View file

@ -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 ──────────────────────────────────────────────────────

View file

@ -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"]);
});
});

View file

@ -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 {

View file

@ -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<string, any> = {};
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<Manifest | null> {
@ -94,7 +109,8 @@ async function fetchManifestFromGitHub(): Promise<Manifest | null> {
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 };