fix: validate manifest fields are plain objects, not just truthy (#2921)

* fix: validate manifest fields are plain objects, not just truthy

isValidManifest used !!data.agents/clouds/matrix which accepts strings,
numbers, and arrays. Downstream Object.keys() then silently returns
character indices or array indices instead of real agent/cloud names.
Replace with isPlainObject() checks to reject non-object values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add validation tests for non-object manifest fields

Tests that loadManifest rejects manifests where agents/clouds/matrix
are strings, arrays, or numbers instead of plain objects.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: L <6723574+louisgv@users.noreply.github.com>
This commit is contained in:
Ahmed Abushagur 2026-03-23 16:48:54 -07:00 committed by GitHub
parent 472b315762
commit fd2d661e27
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 146 additions and 5 deletions

View file

@ -1,8 +1,8 @@
import type { Manifest } from "../manifest";
import type { TestEnvironment } from "./test-helpers";
import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test";
import { mkdirSync, writeFileSync } from "node:fs";
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test";
import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import {
_resetCacheForTesting,
@ -179,6 +179,147 @@ describe("manifest", () => {
await loadManifest();
expect(fetchMock.mock.calls.length).toBe(fetchCount);
});
it("falls back to stale cache when fetch fails", async () => {
const cacheDir = join(env.testDir, "spawn");
mkdirSync(cacheDir, {
recursive: true,
});
writeFileSync(join(cacheDir, "manifest.json"), JSON.stringify(mockManifest));
_resetCacheForTesting();
global.fetch = mock(
async () =>
new Response("error", {
status: 500,
}),
);
const m = await loadManifest(true);
expect(m.agents.claude).toBeDefined();
expect(isStaleCache()).toBe(true);
});
it("throws when no cache and fetch fails", async () => {
_resetCacheForTesting();
global.fetch = mock(
async () =>
new Response("error", {
status: 500,
}),
);
const cacheFile = join(env.testDir, "spawn", "manifest.json");
if (existsSync(cacheFile)) {
rmSync(cacheFile);
}
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
});
it("throws when manifest from GitHub is invalid", async () => {
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
global.fetch = mock(
async () =>
new Response(
JSON.stringify({
not: "a manifest",
}),
),
);
const cacheFile = join(env.testDir, "spawn", "manifest.json");
if (existsSync(cacheFile)) {
rmSync(cacheFile);
}
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
consoleSpy.mockRestore();
});
it("rejects manifest with string agents field", async () => {
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
global.fetch = mock(
async () =>
new Response(
JSON.stringify({
agents: "claude",
clouds: {},
matrix: {},
}),
),
);
const cacheFile = join(env.testDir, "spawn", "manifest.json");
if (existsSync(cacheFile)) {
rmSync(cacheFile);
}
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
consoleSpy.mockRestore();
});
it("rejects manifest with array clouds field", async () => {
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
global.fetch = mock(
async () =>
new Response(
JSON.stringify({
agents: {},
clouds: [
"sprite",
"hetzner",
],
matrix: {},
}),
),
);
const cacheFile = join(env.testDir, "spawn", "manifest.json");
if (existsSync(cacheFile)) {
rmSync(cacheFile);
}
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
consoleSpy.mockRestore();
});
it("rejects manifest with numeric matrix field", async () => {
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
global.fetch = mock(
async () =>
new Response(
JSON.stringify({
agents: {},
clouds: {},
matrix: 42,
}),
),
);
const cacheFile = join(env.testDir, "spawn", "manifest.json");
if (existsSync(cacheFile)) {
rmSync(cacheFile);
}
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
consoleSpy.mockRestore();
});
it("throws when network errors occur and no cache exists", async () => {
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
global.fetch = mock(async () => {
throw new Error("Network timeout");
});
const cacheFile = join(env.testDir, "spawn", "manifest.json");
if (existsSync(cacheFile)) {
rmSync(cacheFile);
}
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
consoleSpy.mockRestore();
});
});
});

View file

@ -156,9 +156,9 @@ function isValidManifest(data: unknown): data is Manifest {
"agents" in data &&
"clouds" in data &&
"matrix" in data &&
!!data.agents &&
!!data.clouds &&
!!data.matrix
isPlainObject(data.agents) &&
isPlainObject(data.clouds) &&
isPlainObject(data.matrix)
);
}