mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-17 04:11:23 +00:00
fix(update-check): validate install script content before execution (#3302)
Add pre-execution validation of downloaded install scripts to catch corrupted or truncated downloads. Checks minimum size threshold and expected shebang/header for the platform. Documents current HTTPS-only security posture and absence of checksum infrastructure. Agent: code-health Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
655a909955
commit
352c55c068
3 changed files with 70 additions and 15 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@openrouter/spawn",
|
||||
"version": "1.0.9",
|
||||
"version": "1.0.10",
|
||||
"type": "module",
|
||||
"bin": {
|
||||
"spawn": "cli.js"
|
||||
|
|
|
|||
|
|
@ -6,6 +6,9 @@ import path from "node:path";
|
|||
import { tryCatch } from "@openrouter/spawn-shared";
|
||||
import pkg from "../../package.json";
|
||||
|
||||
// Fake install script returned by the mocked curl call — must pass validateInstallScript()
|
||||
const FAKE_INSTALL_SCRIPT = "#!/bin/bash\n# fake install script for tests\necho 'installing spawn'\n" + "x".repeat(200);
|
||||
|
||||
// ── Test Helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
/** Remove the .update-failed backoff file so it doesn't interfere with tests */
|
||||
|
|
@ -98,7 +101,9 @@ describe("update-check", () => {
|
|||
|
||||
// Mock execFileSync to prevent actual update + re-exec
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -113,7 +118,9 @@ describe("update-check", () => {
|
|||
|
||||
// Mock execFileSync to prevent actual update + re-exec
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -141,7 +148,9 @@ describe("update-check", () => {
|
|||
|
||||
// Mock executor to prevent actual commands
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -223,7 +232,7 @@ describe("update-check", () => {
|
|||
args,
|
||||
options,
|
||||
});
|
||||
return Buffer.from("");
|
||||
return Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : "");
|
||||
},
|
||||
);
|
||||
|
||||
|
|
@ -260,7 +269,7 @@ describe("update-check", () => {
|
|||
args,
|
||||
options,
|
||||
});
|
||||
return Buffer.from("");
|
||||
return Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : "");
|
||||
},
|
||||
);
|
||||
|
||||
|
|
@ -300,7 +309,7 @@ describe("update-check", () => {
|
|||
args,
|
||||
options,
|
||||
});
|
||||
return Buffer.from("");
|
||||
return Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : "");
|
||||
},
|
||||
);
|
||||
|
||||
|
|
@ -356,7 +365,7 @@ describe("update-check", () => {
|
|||
|
||||
const { executor } = await import("../update-check.js");
|
||||
let callCount = 0;
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((): Buffer => {
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string): Buffer => {
|
||||
callCount++;
|
||||
// First 3 calls succeed (curl, bash, which), 4th call (re-exec) fails
|
||||
if (callCount >= 4) {
|
||||
|
|
@ -366,7 +375,7 @@ describe("update-check", () => {
|
|||
});
|
||||
throw err;
|
||||
}
|
||||
return Buffer.from("");
|
||||
return Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : "");
|
||||
});
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
|
|
@ -443,7 +452,7 @@ describe("update-check", () => {
|
|||
file,
|
||||
args,
|
||||
});
|
||||
return Buffer.from("");
|
||||
return Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : "");
|
||||
});
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
|
|
@ -481,7 +490,9 @@ describe("update-check", () => {
|
|||
process.env.SPAWN_AUTO_UPDATE = undefined;
|
||||
const fetchSpy = spyOn(global, "fetch").mockImplementation(() => Promise.resolve(new Response("1.0.99\n")));
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -501,7 +512,9 @@ describe("update-check", () => {
|
|||
process.env.SPAWN_AUTO_UPDATE = undefined;
|
||||
const fetchSpy = spyOn(global, "fetch").mockImplementation(() => Promise.resolve(new Response("1.1.0\n")));
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -523,7 +536,9 @@ describe("update-check", () => {
|
|||
process.env.SPAWN_AUTO_UPDATE = undefined;
|
||||
const fetchSpy = spyOn(global, "fetch").mockImplementation(() => Promise.resolve(new Response("2.0.0\n")));
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -540,7 +555,9 @@ describe("update-check", () => {
|
|||
process.env.SPAWN_AUTO_UPDATE = "1";
|
||||
const fetchSpy = spyOn(global, "fetch").mockImplementation(() => Promise.resolve(new Response("1.1.0\n")));
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
@ -558,7 +575,9 @@ describe("update-check", () => {
|
|||
process.env.SPAWN_NO_AUTO_UPDATE = "1";
|
||||
const fetchSpy = spyOn(global, "fetch").mockImplementation(() => Promise.resolve(new Response("1.0.99\n")));
|
||||
const { executor } = await import("../update-check.js");
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation(() => Buffer.from(""));
|
||||
const execFileSyncSpy = spyOn(executor, "execFileSync").mockImplementation((file: string) =>
|
||||
Buffer.from(file === "curl" ? FAKE_INSTALL_SCRIPT : ""),
|
||||
);
|
||||
|
||||
const { checkForUpdates } = await import("../update-check.js");
|
||||
await checkForUpdates();
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ export const executor = {
|
|||
// ── Constants ──────────────────────────────────────────────────────────────────
|
||||
|
||||
const FETCH_TIMEOUT = 10000; // 10 seconds
|
||||
const MIN_INSTALL_SCRIPT_BYTES = 100; // reject suspiciously small scripts
|
||||
const UPDATE_BACKOFF_MS = 60 * 60 * 1000; // 1 hour
|
||||
const UPDATE_CHECK_INTERVAL_MS = 60 * 60 * 1000; // 1 hour — skip network check if last success was recent
|
||||
|
||||
|
|
@ -259,6 +260,39 @@ function reExecWithArgs(): void {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a downloaded install script before execution.
|
||||
*
|
||||
* Checks:
|
||||
* 1. Non-empty and above a minimum size threshold (rejects truncated downloads)
|
||||
* 2. Starts with the expected shebang / header for its platform
|
||||
*
|
||||
* Security note: This is NOT a substitute for cryptographic integrity
|
||||
* verification (SHA256 checksum or code signing). The release pipeline does
|
||||
* not currently publish checksums for the install script, so we rely on
|
||||
* HTTPS (TLS) for transport integrity. These checks catch corruption or
|
||||
* truncation, not a compromised CDN. See GitHub issue #3297.
|
||||
*/
|
||||
function validateInstallScript(content: string, platform: "unix" | "windows"): void {
|
||||
if (content.length < MIN_INSTALL_SCRIPT_BYTES) {
|
||||
throw new Error(
|
||||
`Install script too small (${content.length} bytes, minimum ${MIN_INSTALL_SCRIPT_BYTES}). ` +
|
||||
"Download may be corrupted or truncated.",
|
||||
);
|
||||
}
|
||||
|
||||
if (platform === "unix") {
|
||||
if (!content.startsWith("#!/")) {
|
||||
throw new Error("Install script missing expected shebang (#!/...). Download may be corrupted.");
|
||||
}
|
||||
} else {
|
||||
// PowerShell scripts should contain recognizable PS content
|
||||
if (!content.includes("$") && !content.includes("function")) {
|
||||
throw new Error("Install script does not appear to be valid PowerShell. Download may be corrupted.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function performAutoUpdate(latestVersion: string, jsonOutput = false): void {
|
||||
printUpdateBanner(latestVersion);
|
||||
|
||||
|
|
@ -295,6 +329,8 @@ function performAutoUpdate(latestVersion: string, jsonOutput = false): void {
|
|||
},
|
||||
);
|
||||
const scriptContent = scriptBytes ? scriptBytes.toString() : "";
|
||||
const platform = isWindows() ? "windows" : "unix";
|
||||
validateInstallScript(scriptContent, platform);
|
||||
|
||||
if (isWindows()) {
|
||||
// Windows: write to temp file and execute via PowerShell
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue