Files
hyperframes/packages/cli/src/utils/updateCheck.test.ts
T
Miguel Ángel 4b3c73d941 fix(cli): upgrade and update notice use the detected install method
* fix(cli): upgrade + update-notice use the detected install method

hyperframes upgrade hardcoded 'npm install -g', so bun/pnpm/brew users either
saw it fail or silently got a shadowed npm copy while their real (older) binary
kept running. Route the install through detectInstaller() via a new
installInvocation() argv helper; for skip kinds (ephemeral npx/bunx,
project-local, unknown) print 'npx hyperframes@latest' instead of guessing.

The passive update notice now shows the detected manager's command too. Semver
safety guard consolidated into a shared isSafeVersion(). Suppression gates and
the background auto-update flow are unchanged.

* test(cli): pin the shell:false contract of the --yes install path

Export runDetectedInstall and add a mocked-execFileSync test asserting the
detected manager binary is spawned with the exact installInvocation argv,
{stdio:inherit, shell:false}, and that an install failure sets a non-zero exit
code without throwing. Addresses review nit on the untested --yes path.

* fix(cli): guard the registry version at the boundary; execFile the auto-installer

Security (addresses review): a poisoned registry data.version (e.g.
'1.2.3; rm -rf /') was cached unvalidated and flowed into the background
auto-updater, which ran it via exec() -- a shell -- so a registry compromise
meant RCE on the next CLI run. isSafeVersion only covered the two touched
consumers (upgrade, notice), not this third sibling (scheduleBackgroundInstall).

- Guard at the registry boundary in checkForUpdate: only a strict-semver STRING
  is trusted; a non-string or metachar-bearing data.version is never cached and
  falls back to the last known-good version. The cache-read and fallback paths
  re-validate too, so a pre-existing poisoned cache can't leak through. One gate
  closes all three consumers and any future one; per-consumer checks stay as
  defense in depth.
- The detached auto-installer now runs via execFile(bin, args, shell:false),
  reusing installInvocation, matching the interactive runDetectedInstall path --
  the shell is gone from that path entirely.

Tests: reject poisoned / non-string registry version (never cached); accept a
valid semver.
2026-07-07 18:40:43 -04:00

178 lines
6.0 KiB
TypeScript

import { afterEach, describe, expect, it, vi } from "vitest";
import { isSafeVersion } from "./updateCheck.js";
describe("isSafeVersion", () => {
it("accepts strict semver, incl. prerelease/build metadata", () => {
expect(isSafeVersion("1.2.3")).toBe(true);
expect(isSafeVersion("0.7.28")).toBe(true);
expect(isSafeVersion("1.2.3-beta.1")).toBe(true);
expect(isSafeVersion("1.2.3+build.5")).toBe(true);
});
it("rejects anything that could carry shell metacharacters or isn't semver", () => {
expect(isSafeVersion("")).toBe(false);
expect(isSafeVersion("latest")).toBe(false);
expect(isSafeVersion("1.2")).toBe(false);
expect(isSafeVersion("1.2.3; rm -rf /")).toBe(false);
expect(isSafeVersion("1.2.3 && curl evil")).toBe(false);
expect(isSafeVersion("$(whoami)")).toBe(false);
});
});
/**
* Drive printUpdateNotice under controlled mocks. isDevMode() is true under
* vitest (the module path ends in .ts), which would suppress the notice, so we
* mock ./env.js. detectInstaller and readConfig are mocked to pick the branch.
*/
async function noticeWith(opts: {
installerCommand: string | null;
latestVersion?: string;
isTTY?: boolean;
env?: Record<string, string | undefined>;
}): Promise<string> {
vi.resetModules();
vi.doMock("./env.js", () => ({ isDevMode: () => false }));
vi.doMock("./installerDetection.js", () => ({
detectInstaller: () => ({
kind: opts.installerCommand ? "npm" : "skip",
installCommand: () => opts.installerCommand,
reason: "test",
}),
}));
vi.doMock("../telemetry/config.js", () => ({
readConfig: () => ({ latestVersion: opts.latestVersion ?? "9.9.9" }),
writeConfig: () => {},
}));
const origEnv = { ...process.env };
for (const [k, v] of Object.entries(opts.env ?? {})) {
if (v === undefined) delete process.env[k];
else process.env[k] = v;
}
// Default to a non-CI interactive terminal unless the test overrides env.
if (!("CI" in (opts.env ?? {}))) delete process.env["CI"];
const origTTY = process.stderr.isTTY;
Object.defineProperty(process.stderr, "isTTY", {
value: opts.isTTY ?? true,
configurable: true,
});
const writes: string[] = [];
const origWrite = process.stderr.write.bind(process.stderr);
process.stderr.write = ((chunk: unknown) => {
writes.push(String(chunk));
return true;
}) as typeof process.stderr.write;
try {
const mod = await import("./updateCheck.js");
mod.printUpdateNotice();
} finally {
process.stderr.write = origWrite;
Object.defineProperty(process.stderr, "isTTY", { value: origTTY, configurable: true });
process.env = origEnv;
}
return writes.join("");
}
describe("printUpdateNotice — install-method-aware command", () => {
afterEach(() => {
vi.doUnmock("./env.js");
vi.doUnmock("./installerDetection.js");
vi.doUnmock("../telemetry/config.js");
vi.resetModules();
});
it("shows the detected manager's command for an owned global install", async () => {
const out = await noticeWith({ installerCommand: "brew upgrade hyperframes" });
expect(out).toContain("Update available");
expect(out).toContain("brew upgrade hyperframes");
expect(out).not.toContain("npx hyperframes@latest");
});
it("falls back to npx hyperframes@latest when the install method is skip/unknown", async () => {
const out = await noticeWith({ installerCommand: null });
expect(out).toContain("npx hyperframes@latest");
});
it("is suppressed on a non-TTY stderr", async () => {
const out = await noticeWith({ installerCommand: "brew upgrade hyperframes", isTTY: false });
expect(out).toBe("");
});
it("is suppressed in CI", async () => {
const out = await noticeWith({
installerCommand: "brew upgrade hyperframes",
env: { CI: "true" },
});
expect(out).toBe("");
});
it("is suppressed by the HYPERFRAMES_NO_UPDATE_CHECK opt-out", async () => {
const out = await noticeWith({
installerCommand: "brew upgrade hyperframes",
env: { HYPERFRAMES_NO_UPDATE_CHECK: "1" },
});
expect(out).toBe("");
});
});
/**
* The registry-boundary guard: a poisoned or non-string data.version must
* never be cached, because it flows into the auto-updater's install command.
* This closes the injection class for every downstream consumer at one point.
*/
async function checkWith(registryVersion: unknown): Promise<{
latest: string;
wroteVersion: string | undefined;
}> {
vi.resetModules();
const writes: Array<Record<string, unknown>> = [];
vi.doMock("../telemetry/config.js", () => ({
readConfig: () => ({}),
writeConfig: (c: Record<string, unknown>) => writes.push({ ...c }),
}));
const origFetch = globalThis.fetch;
globalThis.fetch = (async () => ({
ok: true,
json: async () => ({ version: registryVersion }),
})) as unknown as typeof fetch;
try {
const mod = await import("./updateCheck.js");
const result = await mod.checkForUpdate(true);
const lastWrite = writes.at(-1);
return {
latest: result.latest,
wroteVersion: lastWrite ? (lastWrite["latestVersion"] as string | undefined) : undefined,
};
} finally {
globalThis.fetch = origFetch;
}
}
describe("checkForUpdate — registry boundary guard", () => {
afterEach(() => {
vi.doUnmock("../telemetry/config.js");
vi.resetModules();
});
it("caches and returns a valid semver from the registry", async () => {
const { latest, wroteVersion } = await checkWith("9.9.9");
expect(latest).toBe("9.9.9");
expect(wroteVersion).toBe("9.9.9");
});
it("rejects a version carrying shell metacharacters (no cache, no surface)", async () => {
const { latest, wroteVersion } = await checkWith("1.2.3; rm -rf /");
expect(latest).not.toContain(";");
expect(latest).not.toBe("1.2.3; rm -rf /");
expect(wroteVersion).toBeUndefined(); // never written to config
});
it("rejects a non-string data.version", async () => {
const { latest, wroteVersion } = await checkWith({ evil: true });
expect(typeof latest).toBe("string");
expect(wroteVersion).toBeUndefined();
});
});