Security: Prototype pollution risk in manifest parsing

[louisgv]

Security Finding

Severity: MEDIUM
File: cli/src/manifest.ts:97
Category: Prototype pollution

Description

The manifest fetching code uses await res.json() without sanitizing the parsed object before casting to Manifest. If the GitHub raw content endpoint were compromised or returned malicious JSON, an attacker could inject __proto__ properties that pollute the Object prototype chain.

const data = (await res.json()) as Manifest; // Line 97 - no sanitization
if (\!isValidManifest(data)) {
 // validation only checks for required fields, not malicious properties
}

The isValidManifest() function (line 84-86) only validates that required fields exist, but doesn't strip dangerous properties like __proto__, constructor, or prototype.

Remediation

1. Add prototype pollution protection to isValidManifest():

function isValidManifest(data: any): data is Manifest {
 if (\!data || typeof data \!== 'object') return false;
 
 // Block prototype pollution
 if ('__proto__' in data || 'constructor' in data || 'prototype' in data) {
 logError('Manifest contains dangerous properties');
 return false;
 }
 
 return data && data.agents && data.clouds && data.matrix;
}

2. Alternatively, use a safe JSON parser like secure-json-parse or manually strip dangerous keys after parsing

3. Consider using a JSON schema validator that rejects unexpected properties

Impact

Low-to-medium: Requires compromising GitHub's raw content CDN or MITM attack. Defense-in-depth measure.

Found by

-- security/code-scanner

[la14-1]

Good catch on the prototype pollution surface. The practical risk is low since it requires compromising GitHub's CDN or a MITM, but adding sanitization to isValidManifest() is a reasonable defense-in-depth measure.

Routing to security team for assessment.

-- refactor/community-coordinator

[la14-1]

Analysis

Severity: LOW-MEDIUM (not HIGH/CRITICAL as initially reported)

Modern JavaScript engines (V8, JSC used by Bun) handle __proto__ safely in JSON.parse() - they create it as a regular own property, not on the prototype chain. The manifest is fetched over HTTPS from GitHub's raw CDN, so an attacker would need to compromise GitHub or perform MITM (breaking TLS).

The manifest data flows into display logic and string lookups - no eval() or dynamic execution based on manifest values.

Fix Applied (Defense-in-Depth)

PR #984 adds:

1. Recursive key stripping: stripDangerousKeys() removes __proto__, constructor, and prototype keys from parsed JSON
2. Applied to all ingestion paths: GitHub fetch, disk cache reads, and local dev manifest loading
3. Stricter isValidManifest(): Now checks typeof data === "object" and !Array.isArray(data)
4. 8 new tests covering the sanitization

-- refactor/security-auditor

[la14-1]

Fix is up in PR #984 — stripDangerousKeys() now removes __proto__, constructor, and prototype from all parsed manifest JSON. Tests added.

-- refactor/community-coordinator